Compare commits

...
Author SHA1 Message Date
aakashb95 233581cd05 email loop fix initial commit 2026-02-12 13:10:20 +05:30
2c2f0547f7 fix: Captain not responding to campaign conversations (#13489)
Co-authored-by: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com>
2026-02-12 10:07:56 +05:30
Sojan JoseandGitHub c7193c7917 fix(slack): handle archived channel errors in SendOnSlackJob (#13520)
When a Slack-integrated channel is archived, posting from Chatwoot
raises `Slack::Web::Api::Errors::IsArchived` in `SendOnSlackJob`, which
retries and can end up in dead jobs. This can be reproduced by archiving
the connected Slack channel for a valid hook and creating outgoing
messages. This change adds `IsArchived` to the existing handled Slack
API rescue path in
`Integrations::Slack::SendOnSlackService#send_message`, so
archived-channel failures now follow the same flow as related Slack
failures (`prompt_reauthorization!` + `disable`) instead of bubbling and
retrying repeatedly. I tested this by running `bundle exec rubocop
lib/integrations/slack/send_on_slack_service.rb` (with `rbenv`
initialized), and it passes with no offenses.

Sentry issue: https://chatwoot-p3.sentry.io/issues/7150427066/
2026-02-11 17:05:44 -08:00
10 changed files with 160 additions and 8 deletions
+11 -3
View File
@@ -1,5 +1,7 @@
class ReplyMailbox < ApplicationMailbox
attr_accessor :conversation, :processed_mail
include IncomingEmailValidityHelper
attr_accessor :conversation, :processed_mail, :account
before_processing :find_conversation
@@ -7,12 +9,17 @@ class ReplyMailbox < ApplicationMailbox
# Return early if no conversation was found (e.g., notification emails, suspended accounts)
return unless @conversation
decorate_mail
unless incoming_email_from_valid_email?
Rails.logger.info "Email #{mail.message_id} rejected - failed incoming email validity checks"
return
end
# Wrap everything in a transaction to ensure atomicity
# This prevents orphan conversations if message/attachment creation fails
# and ensures idempotency on job retry (conversation won't be duplicated)
ActiveRecord::Base.transaction do
persist_conversation_if_needed
decorate_mail
create_message
add_attachments_to_message
end
@@ -22,6 +29,7 @@ class ReplyMailbox < ApplicationMailbox
def find_conversation
@conversation = Mailbox::ConversationFinder.new(mail).find
@account = @conversation&.account
# Log when email is rejected
Rails.logger.info "Email #{mail.message_id} rejected - no conversation found" unless @conversation
end
@@ -36,6 +44,6 @@ class ReplyMailbox < ApplicationMailbox
end
def decorate_mail
@processed_mail = MailPresenter.new(mail, @conversation.account)
@processed_mail = MailPresenter.new(mail, @account)
end
end
+21 -3
View File
@@ -174,13 +174,31 @@ class MailPresenter < SimpleDelegator
end
def notification_email_from_chatwoot?
# notification emails are send via mailer sender email address. so it should match
configured_sender = Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')).address
original_sender.to_s.casecmp?(configured_sender)
sender_address = original_sender.to_s.downcase
return false if sender_address.blank?
# Notification emails are sent via mailer sender email address.
configured_sender = parse_mail_address(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>'))&.address&.downcase
return true if configured_sender.present? && sender_address.casecmp?(configured_sender)
reply_thread_email_from_chatwoot?(sender_address)
end
private
REPLY_THREAD_SENDER_PATTERN = /^reply\+[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
def reply_thread_email_from_chatwoot?(sender_address)
inbound_domain = @account&.inbound_email_domain.to_s.downcase
return false if inbound_domain.blank?
local_part, domain = sender_address.split('@', 2)
return false if local_part.blank? || domain.blank?
return false unless domain.casecmp?(inbound_domain)
local_part.match?(REPLY_THREAD_SENDER_PATTERN)
end
def parse_mail_address(email)
return if email.blank?
@@ -2,7 +2,6 @@ class MessageTemplates::HookExecutionService
pattr_initialize [:message!]
def perform
return if conversation.campaign.present?
return if conversation.last_incoming_message.blank?
return if message.auto_reply_email?
@@ -21,6 +20,7 @@ class MessageTemplates::HookExecutionService
end
def should_send_out_of_office_message?
return false if conversation.campaign.present?
# should not send if its a tweet message
return false if conversation.tweet?
# should not send for outbound messages
@@ -37,6 +37,7 @@ class MessageTemplates::HookExecutionService
end
def should_send_greeting?
return false if conversation.campaign.present?
# should not send if its a tweet message
return false if conversation.tweet?
@@ -49,6 +50,8 @@ class MessageTemplates::HookExecutionService
# TODO: we should be able to reduce this logic once we have a toggle for email collect messages
def should_send_email_collect?
return false if conversation.campaign.present?
!contact_has_email? && inbox.web_widget? && !email_collect_was_sent?
end
@@ -20,12 +20,17 @@ class Messages::SendEmailNotificationService
def should_send_email_notification?
return false unless message.email_notifiable_message?
return false if bot_sender_message?
return false if message.conversation.contact.email.blank?
return false unless message.account.within_email_rate_limit?
email_reply_enabled?
end
def bot_sender_message?
message.sender_type.in?(%w[AgentBot Captain::Assistant])
end
def email_reply_enabled?
inbox = message.inbox
case inbox.channel.class.to_s
@@ -93,6 +93,10 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob
end
def send_out_of_office_message_if_applicable
# Campaign conversations should never receive OOO templates — the campaign itself
# serves as the initial outreach, and OOO would be confusing in that context.
return if @conversation.campaign.present?
::MessageTemplates::Template::OutOfOffice.perform_if_applicable(@conversation)
end
@@ -68,6 +68,10 @@ module Enterprise::MessageTemplates::HookExecutionService
end
def send_out_of_office_message_after_handoff
# Campaign conversations should never receive OOO templates — the campaign itself
# serves as the initial outreach, and OOO would be confusing in that context.
return if conversation.campaign.present?
::MessageTemplates::Template::OutOfOffice.perform_if_applicable(conversation)
end
@@ -42,6 +42,10 @@ class Captain::Tools::HandoffTool < Captain::Tools::BasePublicTool
end
def send_out_of_office_message_if_applicable(conversation)
# Campaign conversations should never receive OOO templates — the campaign itself
# serves as the initial outreach, and OOO would be confusing in that context.
return if conversation.campaign.present?
::MessageTemplates::Template::OutOfOffice.perform_if_applicable(conversation)
end
@@ -102,7 +102,8 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService
def send_message
post_message if message_content.present?
upload_files if message.attachments.any?
rescue Slack::Web::Api::Errors::AccountInactive, Slack::Web::Api::Errors::MissingScope, Slack::Web::Api::Errors::InvalidAuth,
rescue Slack::Web::Api::Errors::IsArchived, Slack::Web::Api::Errors::AccountInactive, Slack::Web::Api::Errors::MissingScope,
Slack::Web::Api::Errors::InvalidAuth,
Slack::Web::Api::Errors::ChannelNotFound, Slack::Web::Api::Errors::NotInChannel => e
Rails.logger.error e
hook.prompt_reauthorization!
@@ -238,6 +238,77 @@ RSpec.describe MessageTemplates::HookExecutionService do
end
end
context 'when conversation has a campaign' do
let(:campaign) { create(:campaign, account: account) }
let(:campaign_conversation) { create(:conversation, inbox: inbox, account: account, contact: contact, status: :pending, campaign: campaign) }
it 'schedules captain response job for incoming messages on pending campaign conversations' do
expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(campaign_conversation, assistant)
create(:message, conversation: campaign_conversation, message_type: :incoming)
end
it 'does not send greeting template on campaign conversations' do
inbox.update!(greeting_enabled: true, greeting_message: 'Hello! How can we help you?', enable_email_collect: false)
greeting_service = instance_double(MessageTemplates::Template::Greeting)
allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service)
allow(greeting_service).to receive(:perform).and_return(true)
create(:message, conversation: campaign_conversation, message_type: :incoming)
expect(MessageTemplates::Template::Greeting).not_to have_received(:new)
end
it 'does not send out of office template on campaign conversations' do
inbox.update!(working_hours_enabled: true, out_of_office_message: 'We are currently closed')
inbox.working_hours.find_by(day_of_week: Time.current.in_time_zone(inbox.timezone).wday).update!(
closed_all_day: true,
open_all_day: false
)
out_of_office_service = instance_double(MessageTemplates::Template::OutOfOffice)
allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service)
allow(out_of_office_service).to receive(:perform).and_return(true)
create(:message, conversation: campaign_conversation, message_type: :incoming)
expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new)
end
it 'does not send email collect template on campaign conversations' do
contact.update!(email: nil)
inbox.update!(enable_email_collect: true)
email_collect_service = instance_double(MessageTemplates::Template::EmailCollect)
allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(email_collect_service)
allow(email_collect_service).to receive(:perform).and_return(true)
create(:message, conversation: campaign_conversation, message_type: :incoming)
expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new)
end
it 'does not send out of office template after handoff on campaign conversations when quota is exceeded' do
account.update!(
limits: { 'captain_responses' => 100 },
custom_attributes: account.custom_attributes.merge('captain_responses_usage' => 100)
)
inbox.update!(
working_hours_enabled: true,
out_of_office_message: 'We are currently closed'
)
inbox.working_hours.find_by(day_of_week: Time.current.in_time_zone(inbox.timezone).wday).update!(
closed_all_day: true,
open_all_day: false
)
expect do
create(:message, conversation: campaign_conversation, message_type: :incoming)
end.not_to(change { campaign_conversation.messages.template.count })
end
end
context 'when Captain quota is exceeded and handoff happens' do
before do
account.update!(
@@ -111,6 +111,40 @@ describe MessageTemplates::HookExecutionService do
end
end
context 'when conversation has a campaign' do
let(:campaign) { create(:campaign) }
it 'does not call ::MessageTemplates::Template::Greeting on campaign conversations' do
contact = create(:contact, email: nil)
conversation = create(:conversation, contact: contact, campaign: campaign)
conversation.inbox.update(greeting_enabled: true, greeting_message: 'Hi, this is a greeting message', enable_email_collect: false)
greeting_service = double
allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service)
allow(greeting_service).to receive(:perform).and_return(true)
create(:message, conversation: conversation)
expect(MessageTemplates::Template::Greeting).not_to have_received(:new)
end
it 'does not call ::MessageTemplates::Template::OutOfOffice on campaign conversations' do
contact = create(:contact)
conversation = create(:conversation, contact: contact, campaign: campaign)
conversation.inbox.update(working_hours_enabled: true, out_of_office_message: 'We are out of office')
conversation.inbox.working_hours.today.update!(closed_all_day: true)
out_of_office_service = double
allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service)
allow(out_of_office_service).to receive(:perform).and_return(true)
create(:message, conversation: conversation)
expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new)
end
end
context 'when message is an auto reply email' do
it 'does not call any template hooks' do
contact = create(:contact)