diff --git a/db/migrate/20260803000000_enqueue_copy_captain_auto_resolve_mode_to_assistants_job.rb b/db/migrate/20260803000000_enqueue_copy_captain_auto_resolve_mode_to_assistants_job.rb new file mode 100644 index 000000000..c6c9fbe84 --- /dev/null +++ b/db/migrate/20260803000000_enqueue_copy_captain_auto_resolve_mode_to_assistants_job.rb @@ -0,0 +1,7 @@ +class EnqueueCopyCaptainAutoResolveModeToAssistantsJob < ActiveRecord::Migration[7.1] + def up + Migration::CopyCaptainAutoResolveModeToAssistantsJob.perform_later if ChatwootApp.enterprise? + end + + def down; end +end diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb index c196c3463..f8291fdb8 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb @@ -14,7 +14,12 @@ class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::Base end def update - @assistant.update!(assistant_params) + @assistant.with_lock do + permitted_params = assistant_params + permitted_params[:config] = @assistant.config.merge(permitted_params[:config].to_h) if permitted_params[:config] + + @assistant.update!(permitted_params) + end end def destroy @@ -119,13 +124,14 @@ class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::Base end def assistant_params + assistant_config_attributes = [ + :product_name, :feature_faq, :feature_memory, :feature_citation, + :feature_contact_attributes, :welcome_message, :handoff_message, + :resolution_message, :instructions, :temperature, :auto_resolve_mode + ] + permitted = params.require(:assistant).permit(:name, :description, - config: [ - :product_name, :feature_faq, :feature_memory, :feature_citation, - :feature_contact_attributes, - :welcome_message, :handoff_message, :resolution_message, - :instructions, :temperature - ]) + config: assistant_config_attributes) # Handle array parameters separately to allow partial updates permitted[:response_guidelines] = params[:assistant][:response_guidelines] if params[:assistant].key?(:response_guidelines) 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 958c809bf..91244c516 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,10 @@ class Captain::InboxPendingConversationsResolutionJob < ApplicationJob queue_as :low def perform(inbox) - return if inbox.account.captain_auto_resolve_disabled? + captain_assistant = inbox.captain_assistant + return if captain_assistant.blank? || captain_assistant.inactive_conversation_resolution_disabled? - if evaluate_conversation_completion?(inbox.account) + if evaluate_conversation_completion?(captain_assistant, inbox.account) perform_with_evaluation(inbox) else perform_time_based(inbox) @@ -18,8 +19,8 @@ class Captain::InboxPendingConversationsResolutionJob < ApplicationJob private - def evaluate_conversation_completion?(account) - account.feature_enabled?('captain_tasks') && account.captain_auto_resolve_evaluated? + def evaluate_conversation_completion?(assistant, account) + account.feature_enabled?('captain_tasks') && assistant.evaluate_inactive_conversations_before_resolving? end def perform_time_based(inbox) 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 7ccebf72f..9e578cf4c 100644 --- a/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb +++ b/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb @@ -10,9 +10,10 @@ module Enterprise::Account::ConversationsResolutionSchedulerJob def resolve_captain_conversations CaptainInbox.all.find_each(batch_size: 100) do |captain_inbox| inbox = captain_inbox.inbox + assistant = captain_inbox.captain_assistant next if inbox.email? - next if inbox.account.captain_auto_resolve_disabled? + next if assistant.blank? || assistant.inactive_conversation_resolution_disabled? Captain::InboxPendingConversationsResolutionJob.perform_later( inbox diff --git a/enterprise/app/jobs/migration/copy_captain_auto_resolve_mode_to_assistants_job.rb b/enterprise/app/jobs/migration/copy_captain_auto_resolve_mode_to_assistants_job.rb new file mode 100644 index 000000000..1bb081332 --- /dev/null +++ b/enterprise/app/jobs/migration/copy_captain_auto_resolve_mode_to_assistants_job.rb @@ -0,0 +1,14 @@ +class Migration::CopyCaptainAutoResolveModeToAssistantsJob < ApplicationJob + queue_as :async_database_migration + + def perform + Captain::Assistant.includes(:account).find_each do |assistant| + assistant.with_lock do + next if assistant.config.key?('auto_resolve_mode') + + config = assistant.config.merge('auto_resolve_mode' => assistant.account.captain_auto_resolve_mode) + assistant.update!(config: config) + end + end + end +end diff --git a/enterprise/app/models/captain/assistant.rb b/enterprise/app/models/captain/assistant.rb index a738455ab..44b64d956 100644 --- a/enterprise/app/models/captain/assistant.rb +++ b/enterprise/app/models/captain/assistant.rb @@ -18,6 +18,7 @@ # class Captain::Assistant < ApplicationRecord DESCRIPTION_LENGTH_LIMIT = 500 + AUTO_RESOLVE_MODES = %w[disabled legacy evaluated].freeze include Avatarable include Concerns::CaptainToolsHelpers @@ -41,11 +42,15 @@ class Captain::Assistant < ApplicationRecord has_many :agent_sessions, class_name: 'Captain::AgentSession', dependent: :destroy_async has_many :conversation_outcomes, dependent: :destroy_async - store_accessor :config, :temperature, :feature_faq, :feature_memory, :feature_contact_attributes, :product_name + store_accessor :config, :temperature, :feature_faq, :feature_memory, :feature_contact_attributes, :product_name, + :auto_resolve_mode + + before_validation :set_default_auto_resolve_mode, on: :create validates :name, presence: true validates :description, presence: true, length: { maximum: DESCRIPTION_LENGTH_LIMIT } validates :account_id, presence: true + validates :auto_resolve_mode, inclusion: { in: AUTO_RESOLVE_MODES } scope :ordered, -> { order(created_at: :desc) } @@ -55,6 +60,18 @@ class Captain::Assistant < ApplicationRecord name end + def auto_resolve_mode + config.fetch('auto_resolve_mode') { account&.captain_auto_resolve_mode || 'evaluated' } + end + + def inactive_conversation_resolution_disabled? + auto_resolve_mode == 'disabled' + end + + def evaluate_inactive_conversations_before_resolving? + auto_resolve_mode == 'evaluated' + end + def available_agent_tools tools = self.class.built_in_agent_tools.dup @@ -92,6 +109,12 @@ class Captain::Assistant < ApplicationRecord private + def set_default_auto_resolve_mode + return if config.key?('auto_resolve_mode') + + self.auto_resolve_mode = account&.captain_auto_resolve_mode || 'evaluated' + end + def agent_name name.parameterize(separator: '_') end diff --git a/enterprise/app/views/api/v1/models/captain/_assistant.json.jbuilder b/enterprise/app/views/api/v1/models/captain/_assistant.json.jbuilder index d597ed220..65b2f88a5 100644 --- a/enterprise/app/views/api/v1/models/captain/_assistant.json.jbuilder +++ b/enterprise/app/views/api/v1/models/captain/_assistant.json.jbuilder @@ -1,5 +1,5 @@ json.account_id resource.account_id -json.config resource.config +json.config resource.config.merge('auto_resolve_mode' => resource.auto_resolve_mode) json.created_at resource.created_at.to_i json.description resource.description json.guardrails resource.guardrails diff --git a/enterprise/lib/captain/tools/resolve_conversation_tool.rb b/enterprise/lib/captain/tools/resolve_conversation_tool.rb index ba4d8323a..d480516d8 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_auto_resolve_disabled? + return 'Auto-resolve is disabled for this assistant' if @assistant.inactive_conversation_resolution_disabled? log_tool_usage('resolve_conversation', { conversation_id: conversation.id, reason: reason }) diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb index 8ba8b693a..d3d7ac419 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb @@ -125,6 +125,32 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do expect(json_response[:config][:feature_citation]).to be(false) expect(response).to have_http_status(:success) end + + it 'stores evaluated mode on the assistant when captain_tasks is enabled' do + account.enable_features!('captain_tasks') + + post "/api/v1/accounts/#{account.id}/captain/assistants", + params: valid_attributes, + headers: admin.create_new_auth_token, + as: :json + + assistant = Captain::Assistant.find(json_response[:id]) + expect(assistant.config['auto_resolve_mode']).to eq('evaluated') + expect(account.reload.settings).not_to have_key('captain_auto_resolve_mode') + end + + it 'stores legacy mode on the assistant when captain_tasks is disabled' do + account.disable_features!('captain_tasks') + + post "/api/v1/accounts/#{account.id}/captain/assistants", + params: valid_attributes, + headers: admin.create_new_auth_token, + as: :json + + assistant = Captain::Assistant.find(json_response[:id]) + expect(assistant.config['auto_resolve_mode']).to eq('legacy') + expect(account.reload.settings).not_to have_key('captain_auto_resolve_mode') + end end end @@ -206,7 +232,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do end it 'updates feature_citation config' do - assistant.update!(config: { 'feature_citation' => true }) + assistant.update!(config: { 'feature_citation' => true, 'auto_resolve_mode' => 'disabled' }) patch "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}", params: { assistant: { config: { feature_citation: false } } }, @@ -214,7 +240,19 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do as: :json expect(response).to have_http_status(:success) - expect(json_response[:config][:feature_citation]).to be(false) + expect(assistant.reload.config).to include('feature_citation' => false, 'auto_resolve_mode' => 'disabled') + end + + it 'updates auto_resolve_mode without replacing other config' do + assistant.update!(config: { 'product_name' => 'Chatwoot', 'auto_resolve_mode' => 'legacy' }) + + patch "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}", + params: { assistant: { config: { auto_resolve_mode: 'disabled' } } }, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(assistant.reload.config).to include('product_name' => 'Chatwoot', 'auto_resolve_mode' => 'disabled') end end end 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 44f0d7afd..45bd1631e 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 @@ -5,7 +5,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do let!(:resolvable_pending_conversation) { create(:conversation, inbox: inbox, last_activity_at: 2.hours.ago, status: :pending) } let!(:recent_pending_conversation) { create(:conversation, inbox: inbox, last_activity_at: 1.minute.ago, status: :pending) } let!(:open_conversation) { create(:conversation, inbox: inbox, last_activity_at: 1.hour.ago, status: :open) } - let!(:captain_assistant) { create(:captain_assistant, account: inbox.account) } + let!(:captain_assistant) { create(:captain_assistant, account: inbox.account, config: { 'auto_resolve_mode' => 'evaluated' }) } before do create(:captain_inbox, inbox: inbox, captain_assistant: captain_assistant) @@ -18,7 +18,25 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do .to have_enqueued_job.on_queue('low') end + context 'when the assistant is deleted before the queued job runs' do + before do + captain_assistant.destroy! + inbox.reload + end + + it 'leaves pending conversations unchanged' do + described_class.perform_now(inbox) + + expect(resolvable_pending_conversation.reload.status).to eq('pending') + end + end + context 'when captain_tasks is disabled' do + before do + allow(inbox.account).to receive(:feature_enabled?).and_call_original + allow(inbox.account).to receive(:feature_enabled?).with('captain_tasks').and_return(false) + end + it 'resolves pending conversations inactive for over 1 hour' do described_class.perform_now(inbox) @@ -101,8 +119,8 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do 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') + it 'uses legacy time-based resolve when configured on the assistant' do + captain_assistant.update!(auto_resolve_mode: 'legacy') allow(Captain::ConversationCompletionService).to receive(:new) described_class.perform_now(inbox) @@ -222,7 +240,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do it 'creates handoff message with configured content' do handoff_message = 'Connecting you to a human agent...' - captain_assistant.update!(config: { 'handoff_message' => handoff_message }) + captain_assistant.update!(config: captain_assistant.config.merge('handoff_message' => handoff_message)) inbox.reload allow(inbox.account).to receive(:feature_enabled?).and_call_original allow(inbox.account).to receive(:feature_enabled?).with('captain_tasks').and_return(true) @@ -238,7 +256,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do handoff_message = 'Connecting you to a human agent...' original_waiting_since = 3.hours.ago - captain_assistant.update!(config: { 'handoff_message' => handoff_message }) + captain_assistant.update!(config: captain_assistant.config.merge('handoff_message' => handoff_message)) resolvable_pending_conversation.update!(waiting_since: original_waiting_since) allow(MessageTemplates::Template::OutOfOffice).to receive(:perform_if_applicable) inbox.reload @@ -251,7 +269,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do end it 'does not create handoff message if not configured' do - captain_assistant.update!(config: {}) + captain_assistant.update!(config: captain_assistant.config.except('handoff_message')) inbox.reload allow(inbox.account).to receive(:feature_enabled?).and_call_original allow(inbox.account).to receive(:feature_enabled?).with('captain_tasks').and_return(true) @@ -365,7 +383,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_auto_resolve_mode: 'disabled') + captain_assistant.update!(auto_resolve_mode: 'disabled') expect do described_class.perform_now(inbox) @@ -376,6 +394,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do end it 'falls back to disabled mode from legacy settings key' do + captain_assistant.update!(config: captain_assistant.config.except('auto_resolve_mode')) inbox.account.update!(settings: inbox.account.settings.merge('captain_disable_auto_resolve' => true)) expect do 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 1988d346c..3b529bb44 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,12 @@ RSpec.describe Account::ConversationsResolutionSchedulerJob, type: :job do end end - context 'when account has captain auto resolve disabled' do + context 'when assistant 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_auto_resolve_mode: 'disabled') + assistant.update!(auto_resolve_mode: 'disabled') end it 'does not enqueue resolution jobs' do @@ -46,19 +46,23 @@ RSpec.describe Account::ConversationsResolutionSchedulerJob, type: :job do end end - context 'when account uses legacy disabled settings key' do + context 'when an assistant has been deleted before its inbox link is cleaned up' do + let!(:orphaned_inbox) { create(:inbox, account: account) } let!(:regular_inbox) { create(:inbox, account: account) } + let!(:active_assistant) { create(:captain_assistant, account: account) } before do - create(:captain_inbox, captain_assistant: assistant, inbox: regular_inbox) - account.update!(settings: account.settings.merge('captain_disable_auto_resolve' => true)) + create(:captain_inbox, captain_assistant: assistant, inbox: orphaned_inbox) + create(:captain_inbox, captain_assistant: active_assistant, inbox: regular_inbox) + assistant.destroy! end - it 'does not enqueue resolution jobs' do + it 'skips the missing assistant and schedules later valid inboxes' do expect do described_class.perform_now - end.not_to have_enqueued_job(Captain::InboxPendingConversationsResolutionJob) + end.to have_enqueued_job(Captain::InboxPendingConversationsResolutionJob) .with(regular_inbox) + .exactly(:once) end end diff --git a/spec/enterprise/jobs/migration/copy_captain_auto_resolve_mode_to_assistants_job_spec.rb b/spec/enterprise/jobs/migration/copy_captain_auto_resolve_mode_to_assistants_job_spec.rb new file mode 100644 index 000000000..ebd57ee32 --- /dev/null +++ b/spec/enterprise/jobs/migration/copy_captain_auto_resolve_mode_to_assistants_job_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +RSpec.describe Migration::CopyCaptainAutoResolveModeToAssistantsJob, type: :job do + it 'copies the account mode to assistants without an assistant setting' do + account = create(:account, captain_auto_resolve_mode: 'legacy') + assistant = create(:captain_assistant, account: account) + assistant.update_columns(config: assistant.config.except('auto_resolve_mode')) # rubocop:disable Rails/SkipsModelValidations + + described_class.perform_now + + expect(assistant.reload.config['auto_resolve_mode']).to eq('legacy') + end + + it 'preserves an existing assistant setting' do + account = create(:account, captain_auto_resolve_mode: 'legacy') + assistant = create(:captain_assistant, account: account, config: { 'auto_resolve_mode' => 'disabled' }) + + described_class.perform_now + + expect(assistant.reload.config['auto_resolve_mode']).to eq('disabled') + end +end 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 e3054230c..fb1775fa3 100644 --- a/spec/enterprise/lib/captain/tools/resolve_conversation_tool_spec.rb +++ b/spec/enterprise/lib/captain/tools/resolve_conversation_tool_spec.rb @@ -47,24 +47,13 @@ RSpec.describe Captain::Tools::ResolveConversationTool do end end - describe 'when auto-resolve is disabled for the account' do - before { account.update!(captain_auto_resolve_mode: 'disabled') } + describe 'when auto-resolve is disabled for the assistant' do + before { assistant.update!(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') - - expect(result).to eq('Auto-resolve is disabled for this account') + expect(result).to eq('Auto-resolve is disabled for this assistant') expect(conversation.reload).not_to be_resolved end end diff --git a/spec/enterprise/services/captain/assistant_migration/draft_applier_spec.rb b/spec/enterprise/services/captain/assistant_migration/draft_applier_spec.rb index 79342cfe5..8f87c25db 100644 --- a/spec/enterprise/services/captain/assistant_migration/draft_applier_spec.rb +++ b/spec/enterprise/services/captain/assistant_migration/draft_applier_spec.rb @@ -115,6 +115,7 @@ RSpec.describe Captain::AssistantMigration::DraftApplier do response_guidelines: ['Use plain language.'], guardrails: ['Do not disclose internal notes.'] ) + original_config = assistant.config.deep_dup described_class.new(assistant: assistant, draft: draft, dry_run: false).perform @@ -129,7 +130,7 @@ RSpec.describe Captain::AssistantMigration::DraftApplier do expect(assistant.config.dig('assistant_migration', 'original_values')).to include( 'name' => assistant.name, 'description' => 'Existing assistant description.', - 'config' => { 'product_name' => 'Test Product', 'instructions' => 'Legacy V1 custom instructions.' }, + 'config' => original_config, 'response_guidelines' => ['Use plain language.'], 'guardrails' => ['Do not disclose internal notes.'] )