Merge branch 'chore/improve-conversation-snooze' into chore/improve-custom-snooze
This commit is contained in:
@@ -1,7 +1,14 @@
|
||||
class AgentBots::WebhookJob < WebhookJob
|
||||
queue_as :high
|
||||
retry_on RestClient::TooManyRequests, RestClient::InternalServerError, wait: 3.seconds, attempts: 3 do |job, error|
|
||||
url, payload, webhook_type = job.arguments
|
||||
Webhooks::Trigger.new(url, payload, webhook_type || :agent_bot_webhook).handle_failure(error)
|
||||
end
|
||||
|
||||
def perform(url, payload, webhook_type = :agent_bot_webhook)
|
||||
super(url, payload, webhook_type)
|
||||
rescue RestClient::TooManyRequests, RestClient::InternalServerError => e
|
||||
Rails.logger.warn("[AgentBots::WebhookJob] attempt #{executions} failed #{e.class.name}")
|
||||
raise
|
||||
end
|
||||
end
|
||||
|
||||
@@ -41,6 +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] },
|
||||
'conversation_required_attributes': {
|
||||
'type': %w[array null],
|
||||
'items': { 'type': 'string' }
|
||||
@@ -90,6 +91,7 @@ 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
|
||||
|
||||
has_many :account_users, dependent: :destroy_async
|
||||
has_many :agent_bot_inboxes, dependent: :destroy_async
|
||||
|
||||
@@ -159,6 +159,7 @@ class Conversation < ApplicationRecord
|
||||
end
|
||||
|
||||
def bot_handoff!
|
||||
update(waiting_since: Time.current) if waiting_since.blank?
|
||||
open!
|
||||
dispatcher_dispatch(CONVERSATION_BOT_HANDOFF)
|
||||
end
|
||||
|
||||
+2
-3
@@ -74,10 +74,9 @@
|
||||
- name: voice_recorder
|
||||
display_name: Voice Recorder
|
||||
enabled: true
|
||||
- name: mobile_v2
|
||||
display_name: Mobile App V2
|
||||
- name: report_rollup
|
||||
display_name: Report Rollup
|
||||
enabled: false
|
||||
deprecated: true
|
||||
- name: channel_website
|
||||
display_name: Website Channel
|
||||
enabled: true
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
class DisableReportRollupForAllAccounts < ActiveRecord::Migration[7.1]
|
||||
def up
|
||||
Account.feature_report_rollup.find_each(batch_size: 100) do |account|
|
||||
account.disable_features(:report_rollup)
|
||||
account.save!(validate: false)
|
||||
end
|
||||
end
|
||||
end
|
||||
+1
-1
@@ -10,7 +10,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema[7.1].define(version: 2026_02_26_084618) do
|
||||
ActiveRecord::Schema[7.1].define(version: 2026_02_26_153427) do
|
||||
# These extensions should be enabled to support this database
|
||||
enable_extension "pg_stat_statements"
|
||||
enable_extension "pg_trgm"
|
||||
|
||||
@@ -42,10 +42,10 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob
|
||||
end
|
||||
|
||||
def process_response
|
||||
ActiveRecord::Base.transaction do
|
||||
if handoff_requested?
|
||||
process_action('handoff')
|
||||
else
|
||||
if handoff_requested?
|
||||
process_action('handoff')
|
||||
else
|
||||
ActiveRecord::Base.transaction do
|
||||
create_messages
|
||||
Rails.logger.info("[CAPTAIN][ResponseBuilderJob] Incrementing response usage for #{account.id}")
|
||||
account.increment_response_usage
|
||||
|
||||
@@ -2,6 +2,8 @@ class Captain::InboxPendingConversationsResolutionJob < ApplicationJob
|
||||
queue_as :low
|
||||
|
||||
def perform(inbox)
|
||||
return if inbox.account.captain_disable_auto_resolve
|
||||
|
||||
Current.executed_by = inbox.captain_assistant
|
||||
|
||||
resolvable_conversations = inbox.conversations.pending.where('last_activity_at < ? ', Time.now.utc - 1.hour).limit(Limits::BULK_ACTIONS_LIMIT)
|
||||
|
||||
@@ -12,6 +12,7 @@ module Enterprise::Account::ConversationsResolutionSchedulerJob
|
||||
inbox = captain_inbox.inbox
|
||||
|
||||
next if inbox.email?
|
||||
next if inbox.account.captain_disable_auto_resolve
|
||||
|
||||
Captain::InboxPendingConversationsResolutionJob.perform_later(
|
||||
inbox
|
||||
|
||||
@@ -19,6 +19,9 @@ class Messages::AudioTranscriptionService< Llm::LegacyBaseOpenAiService
|
||||
transcriptions = transcribe_audio
|
||||
Rails.logger.info "Audio transcription successful: #{transcriptions}"
|
||||
{ success: true, transcriptions: transcriptions }
|
||||
rescue Faraday::UnauthorizedError
|
||||
Rails.logger.warn('Skipping audio transcription: OpenAI configuration is invalid or disabled (401 Unauthorized).')
|
||||
{ error: 'OpenAI configuration is invalid or disabled (401)' }
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -6,6 +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
|
||||
|
||||
log_tool_usage('resolve_conversation', { conversation_id: conversation.id, reason: reason })
|
||||
|
||||
|
||||
+10
-2
@@ -15,9 +15,17 @@ class Webhooks::Trigger
|
||||
|
||||
def execute
|
||||
perform_request
|
||||
rescue RestClient::TooManyRequests, RestClient::InternalServerError => e
|
||||
raise if @webhook_type == :agent_bot_webhook
|
||||
|
||||
handle_failure(e)
|
||||
rescue StandardError => e
|
||||
handle_error(e)
|
||||
Rails.logger.warn "Exception: Invalid webhook URL #{@url} : #{e.message}"
|
||||
handle_failure(e)
|
||||
end
|
||||
|
||||
def handle_failure(error)
|
||||
handle_error(error)
|
||||
Rails.logger.warn "Exception: Invalid webhook URL #{@url} : #{error.message}"
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -92,6 +92,44 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do
|
||||
end
|
||||
end
|
||||
|
||||
# Regression (PR #13417): wrapping create_handoff_message and bot_handoff! in the
|
||||
# same transaction defers the message's after_create_commit until commit, at which
|
||||
# point it clears waiting_since (bot_response). The handoff path must stay outside
|
||||
# the transaction so the callback fires before bot_handoff! sets waiting_since.
|
||||
context 'when handoff is requested' do
|
||||
let(:conversation) { create(:conversation, inbox: inbox, account: account, status: :pending) }
|
||||
let(:agent) { create(:user, account: account, role: :agent) }
|
||||
|
||||
before do
|
||||
allow(account).to receive(:feature_enabled?).and_return(false)
|
||||
allow(account).to receive(:feature_enabled?).with('captain_integration_v2').and_return(false)
|
||||
allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'conversation_handoff' })
|
||||
end
|
||||
|
||||
it 'sets waiting_since to approximately the handoff time' do
|
||||
freeze_time do
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
conversation.reload
|
||||
expect(conversation.status).to eq('open')
|
||||
expect(conversation.waiting_since).to be_within(1.second).of(Time.current)
|
||||
end
|
||||
end
|
||||
|
||||
it 'preserves waiting_since so a human reply consumes it for reply_time tracking' do
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
conversation.reload
|
||||
expect(conversation.waiting_since).to be_present
|
||||
|
||||
# A human reply clears waiting_since (consumed by dispatch_create_events
|
||||
# to emit FIRST_REPLY_CREATED or REPLY_CREATED for reply_time tracking).
|
||||
create(:message, conversation: conversation, message_type: :outgoing,
|
||||
sender: agent, account: account, inbox: inbox)
|
||||
expect(conversation.reload.waiting_since).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when message contains an image' do
|
||||
let(:message_with_image) { create(:message, conversation: conversation, message_type: :incoming, content: 'Can you help with this error?') }
|
||||
let(:image_attachment) { message_with_image.attachments.create!(account: account, file_type: :image, external_url: 'https://example.com/error.jpg') }
|
||||
|
||||
@@ -64,4 +64,15 @@ 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)
|
||||
|
||||
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')
|
||||
expect(resolvable_pending_conversation.messages.outgoing).to be_empty
|
||||
end
|
||||
end
|
||||
|
||||
+16
@@ -30,6 +30,22 @@ RSpec.describe Account::ConversationsResolutionSchedulerJob, type: :job do
|
||||
end
|
||||
end
|
||||
|
||||
context 'when account has captain_disable_auto_resolve enabled' 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)
|
||||
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 inbox has no captain enabled' do
|
||||
let!(:inbox_without_captain) { create(:inbox, account: create(:account)) }
|
||||
|
||||
|
||||
@@ -36,6 +36,17 @@ RSpec.describe Captain::Tools::ResolveConversationTool do
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when auto-resolve is disabled for the account' do
|
||||
before { account.update!(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(conversation.reload).not_to be_resolved
|
||||
end
|
||||
end
|
||||
|
||||
describe 'resolving an already resolved conversation' do
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox, status: :resolved) }
|
||||
|
||||
|
||||
@@ -8,6 +8,16 @@ RSpec.describe AgentBots::WebhookJob do
|
||||
let(:url) { 'https://test.com' }
|
||||
let(:payload) { { name: 'test' } }
|
||||
let(:webhook_type) { :agent_bot_webhook }
|
||||
let(:retryable_error) { RestClient::InternalServerError.new(nil, 500) }
|
||||
|
||||
before do
|
||||
ActiveJob::Base.queue_adapter = :test
|
||||
end
|
||||
|
||||
after do
|
||||
clear_enqueued_jobs
|
||||
clear_performed_jobs
|
||||
end
|
||||
|
||||
it 'queues the job' do
|
||||
expect { job }.to have_enqueued_job(described_class)
|
||||
@@ -19,4 +29,23 @@ RSpec.describe AgentBots::WebhookJob do
|
||||
expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type, secret: nil, delivery_id: nil)
|
||||
perform_enqueued_jobs { job }
|
||||
end
|
||||
|
||||
it 'configures retry handlers for 429 and 500 errors' do
|
||||
handlers = described_class.rescue_handlers.map(&:first)
|
||||
|
||||
expect(handlers).to include('RestClient::TooManyRequests', 'RestClient::InternalServerError')
|
||||
end
|
||||
|
||||
it 'retries 3 times and handles failure after retries are exhausted' do
|
||||
allow(Webhooks::Trigger).to receive(:execute).and_raise(retryable_error)
|
||||
trigger_instance = instance_double(Webhooks::Trigger, handle_failure: true)
|
||||
allow(Webhooks::Trigger).to receive(:new).and_return(trigger_instance)
|
||||
allow(Rails.logger).to receive(:warn)
|
||||
|
||||
expect(Webhooks::Trigger).to receive(:execute).exactly(3).times
|
||||
expect(trigger_instance).to receive(:handle_failure).with(instance_of(RestClient::InternalServerError)).once
|
||||
expect(Rails.logger).to receive(:warn).with(/AgentBots::WebhookJob/).exactly(3).times
|
||||
|
||||
perform_enqueued_jobs { job }
|
||||
end
|
||||
end
|
||||
|
||||
@@ -77,6 +77,40 @@ describe Webhooks::Trigger do
|
||||
let!(:pending_conversation) { create(:conversation, inbox: inbox, status: :pending, account: account) }
|
||||
let!(:pending_message) { create(:message, account: account, inbox: inbox, conversation: pending_conversation) }
|
||||
|
||||
it 'raises 500 errors for retry and does not reopen conversation immediately' do
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::InternalServerError.new(nil, 500)).once
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.to raise_error(RestClient::InternalServerError)
|
||||
expect(pending_conversation.reload.status).to eq('pending')
|
||||
expect(Conversations::ActivityMessageJob).not_to have_been_enqueued
|
||||
end
|
||||
|
||||
it 'raises 429 errors for retry and does not reopen conversation immediately' do
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::TooManyRequests.new(nil, 429)).once
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.to raise_error(RestClient::TooManyRequests)
|
||||
expect(pending_conversation.reload.status).to eq('pending')
|
||||
expect(Conversations::ActivityMessageJob).not_to have_been_enqueued
|
||||
end
|
||||
|
||||
it 'reopens conversation and enqueues activity message if pending' do
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
@@ -166,6 +200,22 @@ describe Webhooks::Trigger do
|
||||
expect(activity_message.content).to eq(agent_bot_error_content)
|
||||
end
|
||||
end
|
||||
|
||||
it 'handles 500 without raising for non-agent webhooks' do
|
||||
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::InternalServerError.new(nil, 500)).once
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.not_to raise_error
|
||||
expect(message.reload.status).to eq('failed')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'request headers' do
|
||||
|
||||
@@ -313,6 +313,47 @@ RSpec.describe Conversation do
|
||||
end
|
||||
end
|
||||
|
||||
describe '#bot_handoff!' do
|
||||
let(:conversation) { create(:conversation, status: :pending) }
|
||||
|
||||
before do
|
||||
allow(Rails.configuration.dispatcher).to receive(:dispatch)
|
||||
end
|
||||
|
||||
context 'when waiting_since is blank' do
|
||||
before { conversation.update(waiting_since: nil) }
|
||||
|
||||
it 'sets waiting_since to current time' do
|
||||
freeze_time do
|
||||
conversation.bot_handoff!
|
||||
expect(conversation.reload.waiting_since).to eq(Time.current)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when waiting_since is already set' do
|
||||
let(:original_time) { 1.hour.ago }
|
||||
|
||||
before { conversation.update(waiting_since: original_time) }
|
||||
|
||||
it 'preserves existing waiting_since' do
|
||||
conversation.bot_handoff!
|
||||
expect(conversation.reload.waiting_since).to be_within(1.second).of(original_time)
|
||||
end
|
||||
end
|
||||
|
||||
it 'changes status to open' do
|
||||
conversation.bot_handoff!
|
||||
expect(conversation.reload.status).to eq('open')
|
||||
end
|
||||
|
||||
it 'dispatches CONVERSATION_BOT_HANDOFF event' do
|
||||
expect(Rails.configuration.dispatcher).to receive(:dispatch)
|
||||
.with(described_class::CONVERSATION_BOT_HANDOFF, anything, hash_including(conversation: conversation))
|
||||
conversation.bot_handoff!
|
||||
end
|
||||
end
|
||||
|
||||
describe '#toggle_priority' do
|
||||
it 'defaults priority to nil when created' do
|
||||
conversation = create(:conversation, status: 'open')
|
||||
|
||||
Reference in New Issue
Block a user