diff --git a/enterprise/app/services/captain/llm/translate_query_service.rb b/enterprise/app/services/captain/llm/translate_query_service.rb index bdff88150..93f68b05b 100644 --- a/enterprise/app/services/captain/llm/translate_query_service.rb +++ b/enterprise/app/services/captain/llm/translate_query_service.rb @@ -27,9 +27,9 @@ class Captain::Llm::TranslateQueryService < Captain::BaseTaskService end # Translation is an internal operation, not customer-initiated. - # Prefer the system key; fall back to the account's hook key for self-hosted setups without one. - def api_key - @api_key ||= system_api_key.presence || openai_hook&.settings&.dig('api_key') + # It should always use the installation key. + def llm_credential + @llm_credential ||= system_llm_credential end def query_in_target_language?(query) diff --git a/enterprise/lib/captain/conversation_completion_service.rb b/enterprise/lib/captain/conversation_completion_service.rb index e9cdc8937..aa40e8000 100644 --- a/enterprise/lib/captain/conversation_completion_service.rb +++ b/enterprise/lib/captain/conversation_completion_service.rb @@ -56,12 +56,10 @@ class Captain::ConversationCompletionService < Captain::BaseTaskService { complete: false, reason: reason } end - # Prefer the system API key over the account's OpenAI hook key. # This is an internal operational evaluation, not a customer-triggered feature, - # so it should not consume the customer's OpenAI credits on hosted platforms. - # Falls back to the account hook for self-hosted deployments without a system key. - def api_key - @api_key ||= system_api_key.presence || openai_hook&.settings&.dig('api_key') + # so it should always use the installation key. + def llm_credential + @llm_credential ||= system_llm_credential end def event_name diff --git a/lib/captain/base_task_service.rb b/lib/captain/base_task_service.rb index 60e6ac579..123377ea0 100644 --- a/lib/captain/base_task_service.rb +++ b/lib/captain/base_task_service.rb @@ -1,6 +1,7 @@ class Captain::BaseTaskService include Integrations::LlmInstrumentation include Captain::ToolInstrumentation + include Llm::ExceptionTrackable # gpt-4o-mini supports 128,000 tokens # 1 token is approx 4 characters @@ -55,7 +56,9 @@ class Captain::BaseTaskService end def execute_ruby_llm_request(model:, messages:, schema: nil, tools: []) - Llm::Config.with_api_key(api_key, api_base: api_base) do |context| + credential = llm_credential + + Llm::Config.with_api_key(credential[:api_key], api_base: api_base) do |context| chat = build_chat(context, model: model, messages: messages, schema: schema, tools: tools) conversation_messages = messages.reject { |m| m[:role] == 'system' } @@ -65,7 +68,7 @@ class Captain::BaseTaskService build_ruby_llm_response(chat.ask(conversation_messages.last[:content]), messages) end rescue StandardError => e - ChatwootExceptionTracker.new(e, account: account).capture_exception + capture_llm_exception(e, credential: credential) { error: e.message, request_messages: messages } end @@ -147,11 +150,24 @@ class Captain::BaseTaskService end def api_key_configured? - api_key.present? + llm_credential.present? end def api_key - @api_key ||= openai_hook&.settings&.dig('api_key') || system_api_key + llm_credential&.dig(:api_key) + end + + def llm_credential + @llm_credential ||= hook_llm_credential || system_llm_credential + end + + def hook_llm_credential + key = openai_hook&.settings&.dig('api_key').presence + { api_key: key, source: :hook } if key + end + + def system_llm_credential + { api_key: system_api_key, source: :system } if system_api_key.present? end def openai_hook @@ -162,6 +178,10 @@ class Captain::BaseTaskService @system_api_key ||= InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_API_KEY')&.value end + def exception_tracking_account + account + end + def prompt_from_file(file_name) Rails.root.join('lib/integrations/openai/openai_prompts', "#{file_name}.liquid").read end diff --git a/lib/integrations/llm_base_service.rb b/lib/integrations/llm_base_service.rb index 397888b83..8410130ee 100644 --- a/lib/integrations/llm_base_service.rb +++ b/lib/integrations/llm_base_service.rb @@ -1,5 +1,6 @@ class Integrations::LlmBaseService include Integrations::LlmInstrumentation + include Llm::ExceptionTrackable # gpt-4o-mini supports 128,000 tokens # 1 token is approx 4 characters @@ -100,13 +101,14 @@ class Integrations::LlmBaseService def execute_ruby_llm_request(parsed_body) messages = parsed_body['messages'] model = parsed_body['model'] + credential = llm_credential - Llm::Config.with_api_key(hook.settings['api_key'], api_base: api_base) do |context| + Llm::Config.with_api_key(credential[:api_key], api_base: api_base) do |context| chat = context.chat(model: model) setup_chat_with_messages(chat, messages) end rescue StandardError => e - ChatwootExceptionTracker.new(e, account: hook.account).capture_exception + capture_llm_exception(e, credential: credential) build_error_response_from_exception(e, messages) end @@ -164,6 +166,14 @@ class Integrations::LlmBaseService } end + def llm_credential + @llm_credential ||= { api_key: hook.settings['api_key'], source: :hook } + end + + def exception_tracking_account + hook.account + end + def build_error_response_from_exception(error, messages) { error: error.message, request_messages: messages } end diff --git a/lib/llm/exception_trackable.rb b/lib/llm/exception_trackable.rb new file mode 100644 index 000000000..a2bb48618 --- /dev/null +++ b/lib/llm/exception_trackable.rb @@ -0,0 +1,11 @@ +module Llm::ExceptionTrackable + private + + def capture_llm_exception(error, credential:) + if credential && credential[:source] == :system + ChatwootExceptionTracker.new(error, account: exception_tracking_account).capture_exception + else + Rails.logger.error("[LLM] account=#{exception_tracking_account&.id} #{error.class}: #{error.message}") + end + end +end diff --git a/spec/enterprise/lib/captain/conversation_completion_service_spec.rb b/spec/enterprise/lib/captain/conversation_completion_service_spec.rb index 58b2b2ce6..5c2000a84 100644 --- a/spec/enterprise/lib/captain/conversation_completion_service_spec.rb +++ b/spec/enterprise/lib/captain/conversation_completion_service_spec.rb @@ -141,15 +141,15 @@ RSpec.describe Captain::ConversationCompletionService do service.perform end - it 'falls back to the account hook key when no system key exists' do + it 'does not fall back to the account hook key when no system key exists' do InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_API_KEY').update!(value: nil) - expect(Llm::Config).to receive(:with_api_key).with('customer-own-key', api_base: anything).and_yield(mock_context) - allow(mock_chat).to receive(:ask).and_return( - instance_double(RubyLLM::Message, content: { 'complete' => true, 'reason' => 'Done' }, input_tokens: 10, output_tokens: 5) - ) + expect(Llm::Config).not_to receive(:with_api_key) - service.perform + result = service.perform + + expect(result[:complete]).to be false + expect(result[:reason]).to eq(I18n.t('captain.api_key_missing')) end end diff --git a/spec/lib/captain/base_task_service_spec.rb b/spec/lib/captain/base_task_service_spec.rb index b3c330252..cb8a2ae2c 100644 --- a/spec/lib/captain/base_task_service_spec.rb +++ b/spec/lib/captain/base_task_service_spec.rb @@ -258,6 +258,18 @@ RSpec.describe Captain::BaseTaskService do expect(result[:error]).to eq('API Error') expect(result[:request_messages]).to eq(messages) end + + it 'does not track exceptions for account hook failures' do + create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'hook-key' }) + + expect(Llm::Config).to receive(:with_api_key).with('hook-key', api_base: anything).and_raise(error) + expect(ChatwootExceptionTracker).not_to receive(:new) + + result = service.send(:make_api_call, model: model, messages: messages) + + expect(result[:error]).to eq('API Error') + expect(result[:request_messages]).to eq(messages) + end end describe '#api_key' do @@ -276,6 +288,16 @@ RSpec.describe Captain::BaseTaskService do expect(service.send(:api_key)).to eq('test-key') end end + + context 'when no API key is configured' do + before do + InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_API_KEY')&.destroy + end + + it 'returns nil' do + expect(service.send(:api_key)).to be_nil + end + end end describe '#prompt_from_file' do diff --git a/spec/lib/integrations/llm_base_service_spec.rb b/spec/lib/integrations/llm_base_service_spec.rb new file mode 100644 index 000000000..fc23d18ba --- /dev/null +++ b/spec/lib/integrations/llm_base_service_spec.rb @@ -0,0 +1,28 @@ +require 'rails_helper' + +RSpec.describe Integrations::LlmBaseService do + let(:account) { create(:account) } + let(:inbox) { create(:inbox, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + let(:hook) { create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'hook-key' }) } + let(:event) { { 'name' => 'summarize', 'data' => { 'conversation_display_id' => conversation.display_id } } } + let(:service) { described_class.new(hook: hook, event: event) } + let(:error) { StandardError.new('API Error') } + let(:body) { { model: 'gpt-4', messages: [{ role: 'user', content: 'Hello' }] }.to_json } + + describe '#make_api_call' do + before do + allow(service).to receive(:instrument_llm_call).and_yield + allow(Llm::Config).to receive(:with_api_key).and_raise(error) + end + + it 'does not track exceptions for hook key failures' do + expect(ChatwootExceptionTracker).not_to receive(:new) + + result = service.send(:make_api_call, body) + + expect(result[:error]).to eq('API Error') + expect(result[:request_messages]).to eq([{ 'role' => 'user', 'content' => 'Hello' }]) + end + end +end