diff --git a/app/models/concerns/activity_message_handler.rb b/app/models/concerns/activity_message_handler.rb index c25aba47f..e21593a78 100644 --- a/app/models/concerns/activity_message_handler.rb +++ b/app/models/concerns/activity_message_handler.rb @@ -9,12 +9,36 @@ module ActivityMessageHandler private def create_activity + debug_file = "tmp/conversation-#{id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n========== CREATE_ACTIVITY START ==========" + f.puts "Time: #{Time.current}" + f.puts "Conversation ID: #{id}" + f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Current.user: #{Current.user&.class&.name} - #{Current.user&.id}" + f.puts "saved_change_to_status?: #{saved_change_to_status?}" + f.puts "saved_change_to_priority?: #{saved_change_to_priority?}" + f.puts "saved_change_to_label_list?: #{saved_change_to_label_list?}" + f.puts "saved_change_to_sla_policy_id?: #{saved_change_to_sla_policy_id?}" + f.puts "Status: #{status}" + f.puts "Previous changes: #{previous_changes.inspect}" + end + user_name = determine_user_name + File.open(debug_file, 'a') do |f| + f.puts "\nDetermined user_name: #{user_name.inspect}" + end + handle_status_change(user_name) handle_priority_change(user_name) handle_label_change(user_name) handle_sla_policy_change(user_name) + + File.open(debug_file, 'a') do |f| + f.puts "========== CREATE_ACTIVITY END ==========\n" + end end def determine_user_name @@ -22,9 +46,22 @@ module ActivityMessageHandler end def handle_status_change(user_name) + debug_file = "tmp/conversation-#{id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n----- handle_status_change START -----" + f.puts "saved_change_to_status?: #{saved_change_to_status?}" + f.puts "user_name: #{user_name.inspect}" + f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + end + return unless saved_change_to_status? status_change_activity(user_name) + + File.open(debug_file, 'a') do |f| + f.puts "----- handle_status_change END -----\n" + end end def handle_priority_change(user_name) @@ -47,12 +84,32 @@ module ActivityMessageHandler end def status_change_activity(user_name) + debug_file = "tmp/conversation-#{id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n----- status_change_activity START -----" + f.puts "Time: #{Time.current}" + f.puts "user_name: #{user_name.inspect}" + f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Current.executed_by present?: #{Current.executed_by.present?}" + f.puts "Will call automation_status_change_activity_content?: #{Current.executed_by.present?}" + end + content = if Current.executed_by.present? + File.open(debug_file, 'a') { |f| f.puts 'Calling automation_status_change_activity_content' } automation_status_change_activity_content else + File.open(debug_file, 'a') { |f| f.puts 'Calling user_status_change_activity_content' } user_status_change_activity_content(user_name) end + File.open(debug_file, 'a') do |f| + f.puts "Content generated: #{content.inspect}" + f.puts "Will enqueue ActivityMessageJob?: #{content.present?}" + f.puts "Enqueueing ActivityMessageJob with params: #{activity_message_params(content).inspect}" if content + f.puts "----- status_change_activity END -----\n" + end + ::Conversations::ActivityMessageJob.perform_later(self, activity_message_params(content)) if content end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 4ec63acc2..06e6faeab 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -155,8 +155,37 @@ class Conversation < ApplicationRecord end def bot_handoff! + debug_file = "tmp/conversation-#{id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n========== BOT_HANDOFF! START ==========" + f.puts "Time: #{Time.current}" + f.puts "Conversation ID: #{id}" + f.puts "Status BEFORE open!: #{status}" + f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Transaction open?: #{self.class.connection.transaction_open?}" + f.puts "Thread ID: #{Thread.current.object_id}" + end + open! + + File.open(debug_file, 'a') do |f| + f.puts "\nAfter open! called:" + f.puts "Status AFTER open!: #{status}" + f.puts "Changed?: #{changed?}" + f.puts "Changes: #{changes.inspect}" + f.puts "Previous changes: #{previous_changes.inspect}" + f.puts "Saved changes to status?: #{saved_change_to_status?}" + f.puts "Will callbacks fire?: #{saved_change_to_status? ? 'YES' : 'NO'}" + f.puts 'About to dispatch CONVERSATION_BOT_HANDOFF' + end + dispatcher_dispatch(CONVERSATION_BOT_HANDOFF) + + File.open(debug_file, 'a') do |f| + f.puts 'Dispatcher dispatch completed' + f.puts "========== BOT_HANDOFF! END ==========\n" + end end def unread_messages @@ -198,10 +227,39 @@ class Conversation < ApplicationRecord private def execute_after_update_commit_callbacks + debug_file = "tmp/conversation-#{id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n========== AFTER_UPDATE_COMMIT CALLBACKS START ==========" + f.puts "Time: #{Time.current}" + f.puts "Conversation ID: #{id}" + f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Transaction open?: #{self.class.connection.transaction_open?}" + f.puts "Thread ID: #{Thread.current.object_id}" + f.puts "Previous changes: #{previous_changes.inspect}" + f.puts "Saved change to status?: #{saved_change_to_status?}" + f.puts "Status: #{status}" + end + handle_resolved_status_change notify_status_change + + File.open(debug_file, 'a') do |f| + f.puts "\nAbout to call create_activity" + f.puts "Will create_activity run?: #{saved_change_to_status? ? 'YES' : 'NO (no status change)'}" + end + create_activity + + File.open(debug_file, 'a') do |f| + f.puts 'create_activity completed' + end + notify_conversation_updation + + File.open(debug_file, 'a') do |f| + f.puts "========== AFTER_UPDATE_COMMIT CALLBACKS END ==========\n" + end end def handle_resolved_status_change diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 15f2ace56..b1936221b 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -8,17 +8,67 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob @inbox = conversation.inbox @assistant = assistant + debug_file = "tmp/conversation-#{conversation.id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n\n" + ('='*100) + f.puts '='*100 + f.puts '[RESPONSE BUILDER] ========== JOB PERFORM START ==========' + f.puts "[RESPONSE BUILDER] Time: #{Time.current}" + f.puts "[RESPONSE BUILDER] Conversation ID: #{conversation.id}" + f.puts "[RESPONSE BUILDER] Assistant: #{assistant&.class&.name} - #{assistant&.id}" + f.puts "[RESPONSE BUILDER] Current.executed_by BEFORE setting: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + end + Current.executed_by = @assistant + File.open(debug_file, 'a') do |f| + f.puts "[RESPONSE BUILDER] Current.executed_by AFTER setting: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts '[RESPONSE BUILDER] About to start transaction' + end + ActiveRecord::Base.transaction do + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Inside transaction' } generate_and_process_response + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Transaction completed successfully' } + end + + File.open(debug_file, 'a') do |f| + f.puts '[RESPONSE BUILDER] After transaction block' + f.puts "[RESPONSE BUILDER] Checking if handoff was requested: #{@handoff_requested.inspect}" + end + + # Process handoff OUTSIDE the transaction so after_commit callbacks work properly + if @handoff_requested + File.open(debug_file, 'a') do |f| + f.puts '[RESPONSE BUILDER] Processing deferred handoff OUTSIDE transaction' + f.puts "[RESPONSE BUILDER] Transaction open?: #{ActiveRecord::Base.connection.transaction_open?}" + end + process_handoff end rescue StandardError => e + File.open(debug_file, 'a') do |f| + f.puts "[RESPONSE BUILDER] ERROR occurred: #{e.class} - #{e.message}" + f.puts "[RESPONSE BUILDER] Backtrace (first 5): #{e.backtrace.first(5).join("\n")}" + end raise e if e.is_a?(ActiveStorage::FileNotFoundError) || e.is_a?(Faraday::BadRequestError) handle_error(e) ensure + File.open(debug_file, 'a') do |f| + f.puts '[RESPONSE BUILDER] In ensure block' + f.puts "[RESPONSE BUILDER] Current.executed_by BEFORE clearing: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "[RESPONSE BUILDER] Current.handoff_requested BEFORE clearing: #{Current.handoff_requested.inspect}" + end Current.executed_by = nil + Current.handoff_requested = nil + File.open(debug_file, 'a') do |f| + f.puts "[RESPONSE BUILDER] Current.executed_by AFTER clearing: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "[RESPONSE BUILDER] Current.handoff_requested AFTER clearing: #{Current.handoff_requested.inspect}" + f.puts '[RESPONSE BUILDER] ========== JOB PERFORM END ==========' + f.puts '='*100 + f.puts ('='*100) + "\n\n" + end end private @@ -26,21 +76,47 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob delegate :account, :inbox, to: :@conversation def generate_and_process_response + debug_file = "tmp/conversation-#{@conversation.id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n[RESPONSE BUILDER] generate_and_process_response START" + f.puts "[RESPONSE BUILDER] captain_v2_enabled?: #{captain_v2_enabled?}" + end + @response = if captain_v2_enabled? + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Using AgentRunnerService (V2)' } Captain::Assistant::AgentRunnerService.new(assistant: @assistant, conversation: @conversation).generate_response( message_history: collect_previous_messages ) else + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Using AssistantChatService (V1)' } Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( message_history: collect_previous_messages ) end - return process_action('handoff') if handoff_requested? + File.open(debug_file, 'a') do |f| + f.puts "[RESPONSE BUILDER] Response received: #{@response.inspect}" + f.puts "[RESPONSE BUILDER] handoff_requested? (from response): #{handoff_requested?}" + f.puts "[RESPONSE BUILDER] Current.handoff_requested (from tool): #{Current.handoff_requested.inspect}" + f.puts "[RESPONSE BUILDER] Will process handoff?: #{handoff_requested? || Current.handoff_requested}" + end + + if handoff_requested? || Current.handoff_requested + File.open(debug_file, 'a') do |f| + f.puts '[RESPONSE BUILDER] Handoff requested - deferring until AFTER transaction commits' + f.puts '[RESPONSE BUILDER] Setting @handoff_requested = true' + end + @handoff_requested = true + create_handoff_message + return + end create_messages Rails.logger.info("[CAPTAIN][ResponseBuilderJob] Incrementing response usage for #{account.id}") account.increment_response_usage + + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] generate_and_process_response END' } end def collect_previous_messages @@ -73,14 +149,31 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob @response['response'] == 'conversation_handoff' end - def process_action(action) - case action - when 'handoff' - I18n.with_locale(@assistant.account.locale) do - create_handoff_message - @conversation.bot_handoff! + def process_handoff + debug_file = "tmp/conversation-#{@conversation.id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n[RESPONSE BUILDER] process_handoff START (OUTSIDE TRANSACTION)" + f.puts "[RESPONSE BUILDER] Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "[RESPONSE BUILDER] Current.handoff_requested: #{Current.handoff_requested.inspect}" + f.puts "[RESPONSE BUILDER] Triggered by: #{Current.handoff_requested ? 'HandoffTool (Current flag)' : 'LLM response'}" + f.puts "[RESPONSE BUILDER] Transaction open?: #{ActiveRecord::Base.connection.transaction_open?}" + end + + I18n.with_locale(@assistant.account.locale) do + File.open(debug_file, 'a') do |f| + f.puts "[RESPONSE BUILDER] Locale set to: #{@assistant.account.locale}" + f.puts "[RESPONSE BUILDER] Conversation status BEFORE bot_handoff!: #{@conversation.reload.status}" + end + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Calling bot_handoff!' } + @conversation.bot_handoff! + File.open(debug_file, 'a') do |f| + f.puts '[RESPONSE BUILDER] bot_handoff! completed' + f.puts "[RESPONSE BUILDER] Conversation status AFTER bot_handoff!: #{@conversation.reload.status}" end end + + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] process_handoff END' } end def create_handoff_message @@ -113,8 +206,24 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob end def handle_error(error) + debug_file = "tmp/conversation-#{@conversation.id}.txt" + + File.open(debug_file, 'a') do |f| + f.puts "\n[RESPONSE BUILDER] handle_error called" + f.puts "[RESPONSE BUILDER] Error: #{error.class} - #{error.message}" + f.puts '[RESPONSE BUILDER] Will handoff due to error' + end + log_error(error) - process_action('handoff') + + # Create handoff message and set flag so handoff happens outside transaction + @handoff_requested = true + create_handoff_message if @conversation.messages.where(content: @assistant.config['handoff_message'].presence || I18n.t('conversations.captain.handoff')).where( + 'created_at > ?', 1.minute.ago + ).empty? + + File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] @handoff_requested set to true due to error' } + true end diff --git a/enterprise/app/models/enterprise/activity_message_handler.rb b/enterprise/app/models/enterprise/activity_message_handler.rb index e6a93718f..0b750aa93 100644 --- a/enterprise/app/models/enterprise/activity_message_handler.rb +++ b/enterprise/app/models/enterprise/activity_message_handler.rb @@ -1,22 +1,55 @@ module Enterprise::ActivityMessageHandler def automation_status_change_activity_content - if Current.executed_by.instance_of?(Captain::Assistant) - locale = Current.executed_by.account.locale - if resolved? - I18n.t( - 'conversations.activity.captain.resolved', - user_name: Current.executed_by.name, - locale: locale - ) - elsif open? - I18n.t( - 'conversations.activity.captain.open', - user_name: Current.executed_by.name, - locale: locale - ) - end - else - super + debug_file = "tmp/conversation-#{id}.txt" + + File.open(debug_file, "a") do |f| + f.puts "\n----- Enterprise automation_status_change_activity_content START -----" + f.puts "Time: #{Time.current}" + f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Is Captain::Assistant?: #{Current.executed_by.instance_of?(Captain::Assistant)}" + f.puts "Status: #{status}" + f.puts "resolved?: #{resolved?}" + f.puts "open?: #{open?}" end + + result = if Current.executed_by.instance_of?(Captain::Assistant) + locale = Current.executed_by.account.locale + File.open(debug_file, "a") do |f| + f.puts "Captain::Assistant detected!" + f.puts "Locale: #{locale}" + f.puts "Assistant name: #{Current.executed_by.name}" + end + + if resolved? + content = I18n.t( + 'conversations.activity.captain.resolved', + user_name: Current.executed_by.name, + locale: locale + ) + File.open(debug_file, "a") { |f| f.puts "Generated resolved message: #{content}" } + content + elsif open? + content = I18n.t( + 'conversations.activity.captain.open', + user_name: Current.executed_by.name, + locale: locale + ) + File.open(debug_file, "a") { |f| f.puts "Generated open message: #{content}" } + content + else + File.open(debug_file, "a") { |f| f.puts "Neither resolved nor open - returning nil" } + nil + end + else + File.open(debug_file, "a") { |f| f.puts "Not Captain::Assistant, calling super" } + super + end + + File.open(debug_file, "a") do |f| + f.puts "Final result: #{result.inspect}" + f.puts "----- Enterprise automation_status_change_activity_content END -----\n" + end + + result end end diff --git a/enterprise/app/services/captain/assistant/agent_runner_service.rb b/enterprise/app/services/captain/assistant/agent_runner_service.rb index 9c4e56841..8435b79a5 100644 --- a/enterprise/app/services/captain/assistant/agent_runner_service.rb +++ b/enterprise/app/services/captain/assistant/agent_runner_service.rb @@ -51,7 +51,7 @@ class Captain::Assistant::AgentRunnerService { conversation_history: conversation_history, - state: build_state + state: build_state, } end diff --git a/enterprise/lib/captain/tools/handoff_tool.rb b/enterprise/lib/captain/tools/handoff_tool.rb index 7f0372709..6d9191703 100644 --- a/enterprise/lib/captain/tools/handoff_tool.rb +++ b/enterprise/lib/captain/tools/handoff_tool.rb @@ -24,17 +24,40 @@ class Captain::Tools::HandoffTool < Captain::Tools::BasePublicTool private def trigger_handoff(conversation, reason) - # post the reason as a private note - conversation.messages.create!( - message_type: :outgoing, - private: true, - sender: @assistant, - account: conversation.account, - inbox: conversation.inbox, - content: reason - ) + debug_file = "tmp/conversation-#{conversation.id}.txt" - conversation.bot_handoff! + File.open(debug_file, "a") do |f| + f.puts "\n\n" + "-"*80 + f.puts "========== HANDOFF TOOL TRIGGER START ==========" + f.puts "Time: #{Time.current}" + f.puts "Conversation ID: #{conversation.id}" + f.puts "Conversation Status BEFORE: #{conversation.status}" + f.puts "Conversation persisted?: #{conversation.persisted?}" + f.puts "Current.executed_by BEFORE: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Current.executed_by object_id: #{Current.executed_by&.object_id}" + f.puts "Current.handoff_requested BEFORE: #{Current.handoff_requested.inspect}" + f.puts "Reason: #{reason}" + f.puts "Assistant: #{@assistant&.class&.name} - #{@assistant&.id}" + f.puts "ActiveRecord transaction open?: #{conversation.class.connection.transaction_open?}" + f.puts "Thread ID: #{Thread.current.object_id}" + f.puts "Caller stack (first 10):" + caller.first(10).each { |line| f.puts " #{line}" } + end + + File.open(debug_file, "a") do |f| + f.puts "\nSetting Current.handoff_requested = true" + f.puts "NOT calling bot_handoff! - letting ResponseBuilderJob handle it" + end + + # Signal to ResponseBuilderJob that handoff is requested + Current.handoff_requested = true + + File.open(debug_file, "a") do |f| + f.puts "Current.handoff_requested set to: #{Current.handoff_requested.inspect}" + f.puts "Current.executed_by AFTER: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" + f.puts "Conversation Status (unchanged): #{conversation.status}" + f.puts "========== HANDOFF TOOL TRIGGER END ==========\n\n" + end end # TODO: Future enhancement - Add team assignment capability diff --git a/lib/current.rb b/lib/current.rb index 3376099f8..36b25c34c 100644 --- a/lib/current.rb +++ b/lib/current.rb @@ -4,6 +4,7 @@ module Current thread_mattr_accessor :account_user thread_mattr_accessor :executed_by thread_mattr_accessor :contact + thread_mattr_accessor :handoff_requested def self.reset Current.user = nil @@ -11,5 +12,6 @@ module Current Current.account_user = nil Current.executed_by = nil Current.contact = nil + Current.handoff_requested = nil end end diff --git a/solution.md b/solution.md new file mode 100644 index 000000000..1a8b9856c --- /dev/null +++ b/solution.md @@ -0,0 +1,22 @@ +# Captain Handoff Activity Message – Proposed Fix + +When the Captain assistant triggers a handoff, we need two things to happen reliably: +1. Persist the conversation status change from `pending` to `open`. +2. Attribute that state change to the assistant so the existing activity callback emits the “Captain opened” message. + +The clean approach is to give `Conversation` a helper (e.g., `handoff_by(executor)`) that wraps the status update in a `with_executed_by(executor)` block: + +```ruby +def handoff_by(executor) + with_executed_by(executor) do + open! unless open? + dispatcher_dispatch(CONVERSATION_BOT_HANDOFF) + end +end +``` + +Within that helper, `with_executed_by` should set `Current.executed_by` just for the duration of the update and then restore the previous value. Both the V1 handoff path and the Captain tool call this helper, ensuring the status change is tracked and the enterprise override sees the assistant. + +## Why Not Store `Current.handoff_requested?` + +Using a thread-local flag would mean the tool sets `Current.handoff_requested = true` and hopes the job checks it later. That requires carefully clearing the flag on every execution path, and it leaves state lingering on `Current` while unrelated callbacks run. It’s easy to forget the cleanup and leak the flag into subsequent jobs or observers. By contrast, a push/pop around the actual update keeps responsibility localized and avoids coordination hazards.