diff --git a/app/models/account.rb b/app/models/account.rb index eabaa5c26..087226710 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -41,7 +41,7 @@ class Account < ApplicationRecord 'audio_transcriptions': { 'type': %w[boolean null] }, 'auto_resolve_label': { 'type': %w[string null] }, 'keep_pending_on_bot_failure': { 'type': %w[boolean null] }, - 'captain_disable_auto_resolve': { 'type': %w[boolean null] }, + 'captain_auto_resolve_mode': { 'type': %w[string null], 'enum': ['evaluated', 'legacy', 'disabled', nil] }, 'conversation_required_attributes': { 'type': %w[array null], 'items': { 'type': 'string' } @@ -91,7 +91,8 @@ class Account < ApplicationRecord store_accessor :settings, :audio_transcriptions, :auto_resolve_label store_accessor :settings, :captain_models, :captain_features store_accessor :settings, :keep_pending_on_bot_failure - store_accessor :settings, :captain_disable_auto_resolve + store_accessor :settings, :captain_auto_resolve_mode + include AccountCaptainAutoResolve has_many :account_users, dependent: :destroy_async has_many :agent_bot_inboxes, dependent: :destroy_async diff --git a/app/models/concerns/account_captain_auto_resolve.rb b/app/models/concerns/account_captain_auto_resolve.rb new file mode 100644 index 000000000..5d17e92f9 --- /dev/null +++ b/app/models/concerns/account_captain_auto_resolve.rb @@ -0,0 +1,21 @@ +module AccountCaptainAutoResolve + extend ActiveSupport::Concern + + VALID_CAPTAIN_AUTO_RESOLVE_MODES = %w[evaluated legacy disabled].freeze + + included do + VALID_CAPTAIN_AUTO_RESOLVE_MODES.each do |mode| + define_method("captain_auto_resolve_#{mode}?") do + captain_auto_resolve_mode == mode + end + end + end + + def captain_auto_resolve_mode + mode = settings&.[]('captain_auto_resolve_mode') + return mode if VALID_CAPTAIN_AUTO_RESOLVE_MODES.include?(mode) + return 'disabled' if settings&.[]('captain_disable_auto_resolve') == true + + feature_enabled?('captain_tasks') ? 'evaluated' : 'legacy' + end +end diff --git a/enterprise/app/jobs/captain/inbox_pending_conversations_resolution_job.rb b/enterprise/app/jobs/captain/inbox_pending_conversations_resolution_job.rb index 538c03d22..0be179f04 100644 --- a/enterprise/app/jobs/captain/inbox_pending_conversations_resolution_job.rb +++ b/enterprise/app/jobs/captain/inbox_pending_conversations_resolution_job.rb @@ -5,9 +5,9 @@ class Captain::InboxPendingConversationsResolutionJob < ApplicationJob queue_as :low def perform(inbox) - return if inbox.account.captain_disable_auto_resolve + return if inbox.account.captain_auto_resolve_disabled? - if inbox.account.feature_enabled?('captain_tasks') + if evaluate_conversation_completion?(inbox.account) perform_with_evaluation(inbox) else perform_time_based(inbox) @@ -18,6 +18,10 @@ class Captain::InboxPendingConversationsResolutionJob < ApplicationJob private + def evaluate_conversation_completion?(account) + account.feature_enabled?('captain_tasks') && account.captain_auto_resolve_evaluated? + end + def perform_time_based(inbox) Current.executed_by = inbox.captain_assistant diff --git a/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb b/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb index 8b6527c93..7ccebf72f 100644 --- a/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb +++ b/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb @@ -12,7 +12,7 @@ module Enterprise::Account::ConversationsResolutionSchedulerJob inbox = captain_inbox.inbox next if inbox.email? - next if inbox.account.captain_disable_auto_resolve + next if inbox.account.captain_auto_resolve_disabled? Captain::InboxPendingConversationsResolutionJob.perform_later( inbox diff --git a/enterprise/lib/captain/tools/resolve_conversation_tool.rb b/enterprise/lib/captain/tools/resolve_conversation_tool.rb index 2b71098c9..eeffc3f06 100644 --- a/enterprise/lib/captain/tools/resolve_conversation_tool.rb +++ b/enterprise/lib/captain/tools/resolve_conversation_tool.rb @@ -6,7 +6,7 @@ class Captain::Tools::ResolveConversationTool < Captain::Tools::BasePublicTool conversation = find_conversation(tool_context.state) return 'Conversation not found' unless conversation return "Conversation ##{conversation.display_id} is already resolved" if conversation.resolved? - return 'Auto-resolve is disabled for this account' if conversation.account.captain_disable_auto_resolve + return 'Auto-resolve is disabled for this account' if conversation.account.captain_auto_resolve_disabled? log_tool_usage('resolve_conversation', { conversation_id: conversation.id, reason: reason }) diff --git a/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb b/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb index 63b8c1bca..f432aae62 100644 --- a/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb +++ b/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb @@ -84,6 +84,16 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do expect(resolvable_pending_conversation.reload.status).to eq('pending') expect(resolvable_pending_conversation.messages.outgoing).to be_empty end + + it 'falls back to legacy time-based resolve when legacy auto-resolve is forced' do + inbox.account.update!(captain_auto_resolve_mode: 'legacy') + allow(Captain::ConversationCompletionService).to receive(:new) + + described_class.perform_now(inbox) + + expect(Captain::ConversationCompletionService).not_to have_received(:new) + expect(resolvable_pending_conversation.reload.status).to eq('resolved') + end end context 'when LLM evaluation returns complete' do @@ -322,7 +332,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do end it 'does not resolve conversations when auto-resolve is disabled at execution time' do - inbox.account.update!(captain_disable_auto_resolve: true) + inbox.account.update!(captain_auto_resolve_mode: 'disabled') expect do described_class.perform_now(inbox) @@ -331,4 +341,14 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do expect(resolvable_pending_conversation.reload.status).to eq('pending') expect(resolvable_pending_conversation.messages.outgoing).to be_empty end + + it 'falls back to disabled mode from legacy settings key' do + inbox.account.update!(settings: inbox.account.settings.merge('captain_disable_auto_resolve' => true)) + + expect do + described_class.perform_now(inbox) + end.not_to(change { resolvable_pending_conversation.reload.status }) + + expect(resolvable_pending_conversation.reload.status).to eq('pending') + end end diff --git a/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb b/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb index 343100a50..1988d346c 100644 --- a/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb +++ b/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb @@ -30,12 +30,28 @@ RSpec.describe Account::ConversationsResolutionSchedulerJob, type: :job do end end - context 'when account has captain_disable_auto_resolve enabled' do + context 'when account has captain auto resolve disabled' do let!(:regular_inbox) { create(:inbox, account: account) } before do create(:captain_inbox, captain_assistant: assistant, inbox: regular_inbox) - account.update!(captain_disable_auto_resolve: true) + account.update!(captain_auto_resolve_mode: 'disabled') + end + + it 'does not enqueue resolution jobs' do + expect do + described_class.perform_now + end.not_to have_enqueued_job(Captain::InboxPendingConversationsResolutionJob) + .with(regular_inbox) + end + end + + context 'when account uses legacy disabled settings key' do + let!(:regular_inbox) { create(:inbox, account: account) } + + before do + create(:captain_inbox, captain_assistant: assistant, inbox: regular_inbox) + account.update!(settings: account.settings.merge('captain_disable_auto_resolve' => true)) end it 'does not enqueue resolution jobs' do diff --git a/spec/enterprise/lib/captain/tools/resolve_conversation_tool_spec.rb b/spec/enterprise/lib/captain/tools/resolve_conversation_tool_spec.rb index f676b2974..2eae2905a 100644 --- a/spec/enterprise/lib/captain/tools/resolve_conversation_tool_spec.rb +++ b/spec/enterprise/lib/captain/tools/resolve_conversation_tool_spec.rb @@ -41,7 +41,18 @@ RSpec.describe Captain::Tools::ResolveConversationTool do end describe 'when auto-resolve is disabled for the account' do - before { account.update!(captain_disable_auto_resolve: true) } + before { account.update!(captain_auto_resolve_mode: 'disabled') } + + it 'does not resolve and returns a disabled message' do + result = tool.perform(tool_context, reason: 'Possible spam') + + expect(result).to eq('Auto-resolve is disabled for this account') + expect(conversation.reload).not_to be_resolved + end + end + + describe 'when auto-resolve is disabled via legacy settings key' do + before { account.update!(settings: account.settings.merge('captain_disable_auto_resolve' => true)) } it 'does not resolve and returns a disabled message' do result = tool.perform(tool_context, reason: 'Possible spam') diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 5ccff6517..166b4c34b 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -198,6 +198,44 @@ RSpec.describe Account do expect(account.settings['auto_resolve_message']).to eq(message) end + it 'defaults captain_auto_resolve_mode to legacy when captain_tasks is disabled' do + allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(false) + + expect(account.captain_auto_resolve_mode).to eq('legacy') + expect(account).to be_captain_auto_resolve_legacy + end + + it 'defaults captain_auto_resolve_mode to evaluated when captain_tasks is enabled' do + allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(true) + + expect(account.captain_auto_resolve_mode).to eq('evaluated') + expect(account).to be_captain_auto_resolve_evaluated + end + + it 'correctly gets and sets captain_auto_resolve_mode' do + account.captain_auto_resolve_mode = 'legacy' + + expect(account.captain_auto_resolve_mode).to eq('legacy') + expect(account.settings['captain_auto_resolve_mode']).to eq('legacy') + expect(account).to be_captain_auto_resolve_legacy + end + + it 'allows clearing captain_auto_resolve_mode to fall back to feature defaults' do + allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(false) + account.captain_auto_resolve_mode = nil + + expect(account).to be_valid + expect(account.captain_auto_resolve_mode).to eq('legacy') + expect(account.settings['captain_auto_resolve_mode']).to be_nil + end + + it 'falls back to disabled mode from legacy settings key' do + account.settings = { 'captain_disable_auto_resolve' => true } + + expect(account.captain_auto_resolve_mode).to eq('disabled') + expect(account).to be_captain_auto_resolve_disabled + end + it 'handles nil values correctly' do account.auto_resolve_after = nil account.auto_resolve_message = nil