From 74eb00698545845cd36810ced12cc9a270f7e650 Mon Sep 17 00:00:00 2001 From: Rogee Date: Thu, 13 Aug 2026 14:49:04 +0800 Subject: [PATCH] H-43: fix WEB Captain takeover E2E flow (#8) * test(shangwutong): cover CID rename reliability * H-43: fix WEB Captain takeover flow * H-48: preserve compatible provider model * H-49: make Captain takeover atomic * H-50: prevent duplicate widget initialization --------- Co-authored-by: Rogee --- .../api/v1/captain_assistant_handler_test.go | 2 + backend/internal/llm/provider_manager.go | 7 +- backend/internal/llm/provider_manager_test.go | 41 +++++++++++- .../internal/repository/conversation_repo.go | 4 +- backend/internal/service/ai_takeover_test.go | 55 ++++++++++++++++ .../service/captain_assistant_service.go | 64 +++++++++++++++++-- .../internal/service/conversation_service.go | 50 ++++++++------- .../copilot_feature_model_integration_test.go | 2 +- backend/internal/service/coverage7_test.go | 11 ++++ backend/internal/service/widget_service.go | 60 ++++++++++++++--- .../internal/service/widget_service_test.go | 60 +++++++++++++++++ ...d_contact_inbox_channel_metadata.down.sql} | 0 ...add_contact_inbox_channel_metadata.up.sql} | 0 .../conversation/ConversationHeader.vue | 2 +- .../store/modules/conversations/index.js | 11 +++- .../specs/conversations/mutations.spec.js | 22 +++++++ .../entrypoints/specs/widget.spec.js | 58 +++++++++++++++++ frontend/app/javascript/entrypoints/widget.js | 10 ++- .../javascript/widget/mixins/messageMixin.js | 2 +- .../widget/mixins/specs/messageMixin.spec.js | 7 ++ frontend/widget.html | 26 ++++---- 21 files changed, 431 insertions(+), 63 deletions(-) rename backend/migrations/{000077_add_contact_inbox_channel_metadata.down.sql => 000078_add_contact_inbox_channel_metadata.down.sql} (100%) rename backend/migrations/{000077_add_contact_inbox_channel_metadata.up.sql => 000078_add_contact_inbox_channel_metadata.up.sql} (100%) create mode 100644 frontend/app/javascript/entrypoints/specs/widget.spec.js diff --git a/backend/internal/handler/api/v1/captain_assistant_handler_test.go b/backend/internal/handler/api/v1/captain_assistant_handler_test.go index 6c184cab..7075671a 100644 --- a/backend/internal/handler/api/v1/captain_assistant_handler_test.go +++ b/backend/internal/handler/api/v1/captain_assistant_handler_test.go @@ -48,6 +48,8 @@ func setupCaptainAssistantHandlerTestWithSummaryProvider(t *testing.T, provider &model.CaptainDocument{}, &model.CaptainAssistantResponse{}, &model.CaptainMessageReport{}, + &model.AgentBot{}, + &model.AgentBotInbox{}, )) t.Cleanup(func() { sqlDB, _ := db.DB() diff --git a/backend/internal/llm/provider_manager.go b/backend/internal/llm/provider_manager.go index 5b64fd84..697168fb 100644 --- a/backend/internal/llm/provider_manager.go +++ b/backend/internal/llm/provider_manager.go @@ -268,7 +268,12 @@ func resolveFeatureModel(ctx context.Context, resolver AccountModelResolver, fal } func applyRuntimeChatConfig(ctx context.Context, req ChatRequest, cfg RuntimeProviderConfig, resolver AccountModelResolver) ChatRequest { - req.Model = resolveFeatureModel(ctx, resolver, cfg.ChatModel) + req.Model = cfg.ChatModel + // Account overrides store only a model name. Keep provider/model pairs intact + // for compatible endpoints, where an OpenAI model may not exist. + if cfg.ChatProvider == "openai" { + req.Model = resolveFeatureModel(ctx, resolver, cfg.ChatModel) + } req.Temperature = cfg.Temperature if override, ok := ctx.Value(generationOverrideContextKey{}).(generationOverrideContext); ok && override.Temperature != nil { req.Temperature = *override.Temperature diff --git a/backend/internal/llm/provider_manager_test.go b/backend/internal/llm/provider_manager_test.go index 3544de84..154699aa 100644 --- a/backend/internal/llm/provider_manager_test.go +++ b/backend/internal/llm/provider_manager_test.go @@ -34,7 +34,7 @@ func TestProviderManagerUsesAccountFeatureModelAndGenerationSettings(t *testing. return "account-editor-model", nil }) require.NoError(t, manager.Configure(RuntimeProviderConfig{ - ChatProvider: "openai_compatible", + ChatProvider: "openai", ChatBaseURL: server.URL, ChatAPIKey: "test-key", ChatModel: "platform-model", @@ -51,6 +51,45 @@ func TestProviderManagerUsesAccountFeatureModelAndGenerationSettings(t *testing. assert.Equal(t, 777, request.MaxTokens) } +func TestProviderManagerCompatibleKeepsConfiguredModelAndAPIError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request ChatRequest + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + assert.Equal(t, "provider-model", request.Model) + w.Header().Set("Content-Type", "application/json") + if request.Messages[0].Content == "fail" { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":{"message":"rejected","type":"invalid_request_error","code":"model_not_found"}}`)) + return + } + _, _ = w.Write([]byte(`{"id":"chat-1","choices":[{"message":{"role":"assistant","content":"ok"}}]}`)) + })) + defer server.Close() + + manager := NewProviderManager() + manager.SetAccountModelResolver(func(context.Context, uint, string) (string, error) { + return "stale-openai-model", nil + }) + require.NoError(t, manager.Configure(RuntimeProviderConfig{ + ChatProvider: "openai_compatible", + ChatBaseURL: server.URL, + ChatAPIKey: "test-key", + ChatModel: "provider-model", + EmbeddingMode: EmbeddingModeReuseChat, + })) + + ctx := WithAccountFeature(context.Background(), 42, "assistant") + _, err := manager.ChatCompletion(ctx, ChatRequest{Messages: []ChatMessage{{Role: "user", Content: "ok"}}}) + require.NoError(t, err) + + _, err = manager.ChatCompletion(ctx, ChatRequest{Messages: []ChatMessage{{Role: "user", Content: "fail"}}}) + var apiErr *APIError + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, http.StatusBadRequest, apiErr.StatusCode) + assert.Equal(t, "invalid_request_error", apiErr.Type) + assert.Equal(t, "model_not_found", apiErr.Code) +} + func TestProviderManagerPreservesExplicitAssistantTemperature(t *testing.T) { var request ChatRequest server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/backend/internal/repository/conversation_repo.go b/backend/internal/repository/conversation_repo.go index 5d570402..8147840c 100644 --- a/backend/internal/repository/conversation_repo.go +++ b/backend/internal/repository/conversation_repo.go @@ -333,8 +333,8 @@ func (r *ConversationRepo) AssignAgentBot(ctx context.Context, id, agentBotID ui Updates(map[string]any{"assignee_id": nil, "assignee_agent_bot_id": value, "status": status}).Error } -func (r *ConversationRepo) StartAITakeover(ctx context.Context, id, agentBotID uint) error { - return r.db.WithContext(ctx).Model(&model.Conversation{}).Where("id = ?", id). +func (r *ConversationRepo) StartAITakeover(ctx context.Context, tx *gorm.DB, id, agentBotID uint) error { + return tx.WithContext(ctx).Model(&model.Conversation{}).Where("id = ?", id). Updates(map[string]any{ "assignee_id": nil, "assignee_agent_bot_id": agentBotID, "status": model.ConversationStatusPending, diff --git a/backend/internal/service/ai_takeover_test.go b/backend/internal/service/ai_takeover_test.go index 8883ecc9..8e7403ce 100644 --- a/backend/internal/service/ai_takeover_test.go +++ b/backend/internal/service/ai_takeover_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "errors" "fmt" "testing" @@ -10,6 +11,7 @@ import ( "github.com/gochat/gochat/internal/worker" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gorm.io/gorm" ) func configureInboxAI(t *testing.T, svc *ConversationService, accountID, inboxID uint) (*model.AgentBot, *model.CaptainAssistant) { @@ -83,6 +85,59 @@ func TestConversationServiceAITakeoverRejectsMissingOrAmbiguousAI(t *testing.T) assert.ErrorContains(t, err, "exactly one active AI") } +func TestConversationServiceAITakeoverRejectsConflictWithoutCreatingBinding(t *testing.T) { + svc, db := setupConversationService(t) + account := createConversationServiceTestAccount(t, db) + inbox := createConversationServiceTestInbox(t, db, account.ID) + contact := createConversationServiceTestContact(t, db, account.ID) + conversation := createConversationServiceTestConversation(t, db, account.ID, inbox.ID, contact.ID, string(model.ConversationStatusOpen)) + require.NoError(t, db.AutoMigrate(&model.AgentBot{}, &model.AgentBotInbox{}, &model.CaptainAssistant{}, &model.CaptainInbox{})) + assistant := &model.CaptainAssistant{AccountID: account.ID, Name: "Captain", Status: model.AssistantStatusActive} + require.NoError(t, db.Create(assistant).Error) + require.NoError(t, db.Create(&model.CaptainInbox{AccountID: account.ID, InboxID: inbox.ID, AssistantID: assistant.ID}).Error) + conflictingBot := &model.AgentBot{AccountID: &account.ID, Name: "Webhook", BotType: "webhook"} + require.NoError(t, db.Create(conflictingBot).Error) + require.NoError(t, db.Create(&model.AgentBotInbox{AgentBotID: conflictingBot.ID, InboxID: inbox.ID, Status: model.AgentBotInboxActive}).Error) + + _, err := svc.StartAITakeover(context.Background(), account.ID, conversation.ID) + require.ErrorContains(t, err, "exactly one active AI") + var captainBotCount int64 + require.NoError(t, db.Model(&model.AgentBot{}).Where("bot_type = ?", "captain").Count(&captainBotCount).Error) + assert.Zero(t, captainBotCount) + var bindingCount int64 + require.NoError(t, db.Model(&model.AgentBotInbox{}).Count(&bindingCount).Error) + assert.Equal(t, int64(1), bindingCount) + require.NoError(t, db.First(conversation, conversation.ID).Error) + assert.Equal(t, string(model.ConversationStatusOpen), conversation.Status) + assert.Nil(t, conversation.AssigneeAgentBotID) + assert.Zero(t, conversation.AITakeoverVersion) +} + +func TestConversationServiceAITakeoverRollsBackBindingWhenTakeoverFails(t *testing.T) { + svc, db := setupConversationService(t) + account := createConversationServiceTestAccount(t, db) + inbox := createConversationServiceTestInbox(t, db, account.ID) + contact := createConversationServiceTestContact(t, db, account.ID) + conversation := createConversationServiceTestConversation(t, db, account.ID, inbox.ID, contact.ID, string(model.ConversationStatusOpen)) + require.NoError(t, db.AutoMigrate(&model.AgentBot{}, &model.AgentBotInbox{}, &model.CaptainAssistant{}, &model.CaptainInbox{})) + assistant := &model.CaptainAssistant{AccountID: account.ID, Name: "Captain", Status: model.AssistantStatusActive} + require.NoError(t, db.Create(assistant).Error) + require.NoError(t, db.Create(&model.CaptainInbox{AccountID: account.ID, InboxID: inbox.ID, AssistantID: assistant.ID}).Error) + require.NoError(t, db.Callback().Update().Before("gorm:update").Register("test:fail_takeover", func(tx *gorm.DB) { + if tx.Statement.Table == "conversations" { + tx.AddError(errors.New("takeover failed")) + } + })) + + _, err := svc.StartAITakeover(context.Background(), account.ID, conversation.ID) + require.ErrorContains(t, err, "takeover failed") + var botCount, bindingCount int64 + require.NoError(t, db.Model(&model.AgentBot{}).Count(&botCount).Error) + require.NoError(t, db.Model(&model.AgentBotInbox{}).Count(&bindingCount).Error) + assert.Zero(t, botCount) + assert.Zero(t, bindingCount) +} + func TestMessageServiceHumanOutgoingExitsAITakeoverAtomically(t *testing.T) { db, _, dispatcher, svc := setupMessageServiceWithDefaultLLM(t) capture := &captureConversationEventsListener{} diff --git a/backend/internal/service/captain_assistant_service.go b/backend/internal/service/captain_assistant_service.go index 320cc180..ea169082 100644 --- a/backend/internal/service/captain_assistant_service.go +++ b/backend/internal/service/captain_assistant_service.go @@ -559,7 +559,8 @@ func (s *CaptainAssistantService) SetConfig(ctx context.Context, id uint, cfg *m // AssociateInbox binds an assistant to an inbox. func (s *CaptainAssistantService) AssociateInbox(ctx context.Context, assistantID, inboxID, accountID uint) (*model.Inbox, error) { - if _, err := s.assistantRepo.GetByAccountAndID(ctx, accountID, assistantID); err != nil { + assistant, err := s.assistantRepo.GetByAccountAndID(ctx, accountID, assistantID) + if err != nil { return nil, fmt.Errorf("assistant not found: %w", err) } inbox, err := s.inboxRepo.FindAccountInbox(ctx, accountID, inboxID) @@ -567,12 +568,17 @@ func (s *CaptainAssistantService) AssociateInbox(ctx context.Context, assistantI return nil, fmt.Errorf("inbox not found: %w", err) } - ci := &model.CaptainInbox{ - AssistantID: assistantID, - InboxID: inboxID, - AccountID: accountID, - } - if err := s.inboxRepo.Create(ctx, ci); err != nil { + db := s.assistantRepo.DB().WithContext(ctx) + err = db.Transaction(func(tx *gorm.DB) error { + ci := &model.CaptainInbox{AssistantID: assistantID, InboxID: inboxID, AccountID: accountID} + if err := tx.Where("captain_assistant_id = ? AND inbox_id = ?", assistantID, inboxID).FirstOrCreate(ci).Error; err != nil { + return err + } + + _, err := ensureCaptainAgentBotBinding(ctx, tx, assistant, inboxID) + return err + }) + if err != nil { applogger.L().Errorf("AssociateInbox: %v", err) return nil, fmt.Errorf("associate inbox: %w", err) } @@ -593,9 +599,53 @@ func (s *CaptainAssistantService) DissociateInbox(ctx context.Context, accountID applogger.L().Errorf("DissociateInbox: %v", err) return fmt.Errorf("dissociate inbox: %w", err) } + var bots []model.AgentBot + if err := s.assistantRepo.DB().WithContext(ctx).Where("account_id = ? AND bot_type = ?", accountID, "captain").Find(&bots).Error; err != nil { + return err + } + for _, bot := range bots { + if extractAssistantIDFromBotConfig(bot.Config) == assistantID { + if err := s.assistantRepo.DB().WithContext(ctx).Where("inbox_id = ? AND agent_bot_id = ?", inboxID, bot.ID).Delete(&model.AgentBotInbox{}).Error; err != nil { + return fmt.Errorf("dissociate inbox bot: %w", err) + } + } + } return nil } +func ensureCaptainAgentBotBinding(ctx context.Context, db *gorm.DB, assistant *model.CaptainAssistant, inboxID uint) (*model.AgentBot, error) { + var bots []model.AgentBot + if err := db.WithContext(ctx).Where("account_id = ? AND bot_type = ?", assistant.AccountID, "captain").Find(&bots).Error; err != nil { + return nil, err + } + var bot *model.AgentBot + for i := range bots { + if extractAssistantIDFromBotConfig(bots[i].Config) == assistant.ID { + bot = &bots[i] + break + } + } + if bot == nil { + token, err := generateBotAccessToken() + if err != nil { + return nil, err + } + secret, err := generateBotSecret() + if err != nil { + return nil, err + } + bot = &model.AgentBot{AccountID: &assistant.AccountID, Name: fmt.Sprintf("Captain Assistant #%d", assistant.ID), Description: assistant.Name, BotType: "captain", AccessToken: token, Secret: secret, Config: json.RawMessage(fmt.Sprintf(`{"assistant_id":%d}`, assistant.ID))} + if err := db.WithContext(ctx).Create(bot).Error; err != nil { + return nil, err + } + } + binding := &model.AgentBotInbox{AgentBotID: bot.ID, InboxID: inboxID, AccountID: &assistant.AccountID, Status: model.AgentBotInboxActive} + if err := db.WithContext(ctx).Where("agent_bot_id = ? AND inbox_id = ?", bot.ID, inboxID).Assign("status", model.AgentBotInboxActive).FirstOrCreate(binding).Error; err != nil { + return nil, err + } + return bot, nil +} + func (s *CaptainAssistantService) ListInboxes(ctx context.Context, accountID, assistantID uint) ([]model.Inbox, error) { if _, err := s.assistantRepo.GetByAccountAndID(ctx, accountID, assistantID); err != nil { return nil, fmt.Errorf("assistant not found: %w", err) diff --git a/backend/internal/service/conversation_service.go b/backend/internal/service/conversation_service.go index a06db3f3..c1b5d5fb 100644 --- a/backend/internal/service/conversation_service.go +++ b/backend/internal/service/conversation_service.go @@ -545,36 +545,42 @@ func (s *ConversationService) StartAITakeover(ctx context.Context, accountID, id if err != nil { return nil, err } + var captainInbox model.CaptainInbox + if err := s.repo.DB().WithContext(ctx).Where("account_id = ? AND inbox_id = ?", accountID, conversation.InboxID).First(&captainInbox).Error; err != nil { + return nil, errors.New("invalid inbox AI configuration: expected exactly one active AI") + } + var configuredAssistant model.CaptainAssistant + if err := s.repo.DB().WithContext(ctx).Where("account_id = ? AND id = ? AND status = ?", accountID, captainInbox.AssistantID, model.AssistantStatusActive).First(&configuredAssistant).Error; err != nil { + return nil, errors.New("invalid inbox AI configuration: assistant is disabled or not bound to this inbox") + } var bindings []model.AgentBotInbox - if err := s.repo.DB().WithContext(ctx). + db := s.repo.DB().WithContext(ctx) + if err := db. Where("inbox_id = ? AND status = ?", conversation.InboxID, model.AgentBotInboxActive). Find(&bindings).Error; err != nil { return nil, err } - if len(bindings) != 1 { + if len(bindings) > 1 { return nil, errors.New("invalid inbox AI configuration: expected exactly one active AI") } - var bot model.AgentBot - if err := s.repo.DB().WithContext(ctx). - Where("id = ? AND bot_type = ? AND (account_id IS NULL OR account_id = ?)", bindings[0].AgentBotID, "captain", accountID). - First(&bot).Error; err != nil { - return nil, errors.New("invalid inbox AI configuration: active AI is not a Captain bot") + if len(bindings) == 1 { + var activeBot model.AgentBot + if err := db.Where("id = ? AND bot_type = ? AND (account_id IS NULL OR account_id = ?)", bindings[0].AgentBotID, "captain", accountID).First(&activeBot).Error; err != nil || extractAssistantIDFromBotConfig(activeBot.Config) != configuredAssistant.ID { + return nil, errors.New("invalid inbox AI configuration: expected exactly one active AI") + } + if conversation.AssigneeAgentBotID != nil && *conversation.AssigneeAgentBotID == activeBot.ID && conversation.Status == string(model.ConversationStatusPending) { + return conversation, nil + } } - assistantID := extractAssistantIDFromBotConfig(bot.Config) - if assistantID == 0 { - return nil, errors.New("active inbox AI has invalid assistant configuration") - } - var assistant model.CaptainAssistant - if err := s.repo.DB().WithContext(ctx). - Joins("JOIN captain_inboxes ON captain_inboxes.captain_assistant_id = captain_assistants.id"). - Where("captain_assistants.id = ? AND captain_assistants.account_id = ? AND captain_assistants.status = ? AND captain_inboxes.inbox_id = ?", assistantID, accountID, model.AssistantStatusActive, conversation.InboxID). - First(&assistant).Error; err != nil { - return nil, errors.New("invalid inbox AI configuration: assistant is disabled or not bound to this inbox") - } - if conversation.AssigneeAgentBotID != nil && *conversation.AssigneeAgentBotID == bot.ID && conversation.Status == string(model.ConversationStatusPending) { - return conversation, nil - } - if err := s.repo.StartAITakeover(ctx, conversation.ID, bot.ID); err != nil { + var bot *model.AgentBot + if err := db.Transaction(func(tx *gorm.DB) error { + var err error + bot, err = ensureCaptainAgentBotBinding(ctx, tx, &configuredAssistant, conversation.InboxID) + if err != nil { + return err + } + return s.repo.StartAITakeover(ctx, tx, conversation.ID, bot.ID) + }); err != nil { return nil, err } conversation.AssigneeID = nil diff --git a/backend/internal/service/copilot_feature_model_integration_test.go b/backend/internal/service/copilot_feature_model_integration_test.go index c494c8ee..4de3f100 100644 --- a/backend/internal/service/copilot_feature_model_integration_test.go +++ b/backend/internal/service/copilot_feature_model_integration_test.go @@ -98,7 +98,7 @@ func TestCopilotFeatureModelsReachProviderRequests(t *testing.T) { return models[feature], nil }) require.NoError(t, manager.Configure(llm.RuntimeProviderConfig{ - ChatProvider: "openai_compatible", + ChatProvider: "openai", ChatBaseURL: server.URL, ChatAPIKey: "test-key", ChatModel: "platform-model", diff --git a/backend/internal/service/coverage7_test.go b/backend/internal/service/coverage7_test.go index bfeb7679..cfee6db9 100644 --- a/backend/internal/service/coverage7_test.go +++ b/backend/internal/service/coverage7_test.go @@ -46,6 +46,8 @@ func newCov7TestDB(t *testing.T, extra ...interface{}) *gorm.DB { &model.InboxAssignmentPolicy{}, &model.UserSession{}, &model.BackgroundJob{}, + &model.AgentBot{}, + &model.AgentBotInbox{}, }, extra...)...) return db } @@ -496,6 +498,12 @@ func TestCaptainAssistant_AssociateInbox_Cov7(t *testing.T) { assocInbox, err := svc.AssociateInbox(context.Background(), created.ID, inbox.ID, account.ID) require.NoError(t, err) assert.Equal(t, inbox.ID, assocInbox.ID) + var bot model.AgentBot + require.NoError(t, db.Where("account_id = ? AND bot_type = ?", account.ID, "captain").First(&bot).Error) + assert.Equal(t, created.ID, extractAssistantIDFromBotConfig(bot.Config)) + var binding model.AgentBotInbox + require.NoError(t, db.Where("agent_bot_id = ? AND inbox_id = ?", bot.ID, inbox.ID).First(&binding).Error) + assert.True(t, binding.IsActive()) } func TestCaptainAssistant_AssociateInbox_AssistantNotFound_Cov7(t *testing.T) { @@ -539,6 +547,9 @@ func TestCaptainAssistant_DissociateInbox_Cov7(t *testing.T) { err = svc.DissociateInbox(context.Background(), account.ID, created.ID, inbox.ID) require.NoError(t, err) + var bindingCount int64 + require.NoError(t, db.Model(&model.AgentBotInbox{}).Where("inbox_id = ?", inbox.ID).Count(&bindingCount).Error) + assert.Zero(t, bindingCount) } func TestCaptainAssistant_DissociateInbox_NotFound_Cov7(t *testing.T) { diff --git a/backend/internal/service/widget_service.go b/backend/internal/service/widget_service.go index 0426aa6d..879ec581 100644 --- a/backend/internal/service/widget_service.go +++ b/backend/internal/service/widget_service.go @@ -22,6 +22,7 @@ import ( ws "github.com/gochat/gochat/internal/ws" applogger "github.com/gochat/gochat/pkg/logger" "gorm.io/datatypes" + "gorm.io/gorm" ) var ( @@ -418,6 +419,9 @@ func (s *WidgetService) SendMessage(ctx context.Context, req WidgetSendMessageRe if err != nil { return nil, err } + if _, err := EnqueueCaptainConversationResponseForMessage(ctx, s.worker, s.messageRepo.DB(), msg.ID); err != nil { + return nil, err + } // Dispatch message.created event to trigger auto-reply listener if s.dispatcher != nil { @@ -1012,6 +1016,9 @@ func (s *WidgetService) PublicCreateMessage(ctx context.Context, inboxIdentifier if err != nil { return nil, nil, nil, err } + if _, err := EnqueueCaptainConversationResponseForMessage(ctx, s.worker, s.messageRepo.DB(), message.ID); err != nil { + return nil, nil, nil, err + } // Dispatch message.created event to trigger auto-reply listener if s.dispatcher != nil { @@ -1713,16 +1720,51 @@ func (s *WidgetService) createWidgetConversation(ctx context.Context, contactInb return nil, err } + status := string(model.ConversationStatusOpen) + var agentBotID *uint + var takeoverVersion uint + db := s.conversationRepo.DB().WithContext(ctx) + if db.Migrator().HasTable(&model.CaptainPreference{}) && db.Migrator().HasTable(&model.CaptainInbox{}) { + var autoReplyCount int64 + if err := db.Model(&model.CaptainPreference{}). + Where("account_id = ? AND auto_reply_enabled = ?", inbox.AccountID, true).Count(&autoReplyCount).Error; err != nil { + return nil, err + } + if autoReplyCount > 0 { + var captainInbox model.CaptainInbox + if err := db.Where("account_id = ? AND inbox_id = ?", inbox.AccountID, inbox.ID).First(&captainInbox).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + goto create + } + return nil, err + } + var assistant model.CaptainAssistant + if err := db.Where("account_id = ? AND id = ? AND status = ?", inbox.AccountID, captainInbox.AssistantID, model.AssistantStatusActive).First(&assistant).Error; err != nil { + return nil, err + } + bot, err := ensureCaptainAgentBotBinding(ctx, db, &assistant, inbox.ID) + if err != nil { + return nil, err + } + status = string(model.ConversationStatusPending) + agentBotID = &bot.ID + takeoverVersion = 1 + } + } + +create: conversation := model.Conversation{ - AccountID: inbox.AccountID, - InboxID: inbox.ID, - ContactID: contactInbox.ContactID, - ContactInboxID: &contactInbox.ID, - Status: "open", - ChannelType: inbox.ChannelType, - Channel: inbox.ChannelType, - CustomAttributes: mustJSON(customAttributes), - Labels: strings.Join(s.validWidgetLabels(ctx, inbox.AccountID, labels), ","), + AccountID: inbox.AccountID, + InboxID: inbox.ID, + ContactID: contactInbox.ContactID, + ContactInboxID: &contactInbox.ID, + Status: status, + AssigneeAgentBotID: agentBotID, + AITakeoverVersion: takeoverVersion, + ChannelType: inbox.ChannelType, + Channel: inbox.ChannelType, + CustomAttributes: mustJSON(customAttributes), + Labels: strings.Join(s.validWidgetLabels(ctx, inbox.AccountID, labels), ","), } if err := s.conversationRepo.Create(ctx, &conversation); err != nil { diff --git a/backend/internal/service/widget_service_test.go b/backend/internal/service/widget_service_test.go index dbbb8148..f2283a6c 100644 --- a/backend/internal/service/widget_service_test.go +++ b/backend/internal/service/widget_service_test.go @@ -53,6 +53,11 @@ func setupWidgetServiceTest(t *testing.T) (*gorm.DB, *WidgetService) { &model.WidgetFileUpload{}, &model.WidgetOfflineMessage{}, &model.BackgroundJob{}, + &model.CaptainAssistant{}, + &model.CaptainInbox{}, + &model.CaptainPreference{}, + &model.AgentBot{}, + &model.AgentBotInbox{}, ), "failed to auto-migrate models") t.Cleanup(func() { @@ -296,6 +301,61 @@ func TestWidgetService_SendMessage_NewConversation(t *testing.T) { assert.Equal(t, "incoming", sendResp.Message.MessageType) } +func TestWidgetService_SendMessage_AutomaticCaptainTakeover(t *testing.T) { + db, svc := setupWidgetServiceTest(t) + account, inbox := seedWidgetInbox(t, db) + assistant := &model.CaptainAssistant{AccountID: account.ID, Name: "Auto", Status: model.AssistantStatusActive, Config: json.RawMessage(`{}`)} + require.NoError(t, db.Create(assistant).Error) + require.NoError(t, db.Create(&model.CaptainInbox{AccountID: account.ID, InboxID: inbox.ID, AssistantID: assistant.ID}).Error) + bot := &model.AgentBot{AccountID: &account.ID, Name: "Auto", BotType: "captain", Config: json.RawMessage(fmt.Sprintf(`{"assistant_id":%d}`, assistant.ID))} + require.NoError(t, db.Create(bot).Error) + require.NoError(t, db.Create(&model.AgentBotInbox{AgentBotID: bot.ID, InboxID: inbox.ID, Status: model.AgentBotInboxActive}).Error) + require.NoError(t, db.Create(&model.CaptainPreference{AccountID: account.ID, AutoReplyEnabled: true}).Error) + svc.SetWorkerPool(worker.NewWorkerPool(db)) + + initResp, err := svc.Init(context.Background(), WidgetInitRequest{WebsiteToken: "test_ws_token_123"}) + require.NoError(t, err) + response, err := svc.SendMessage(context.Background(), WidgetSendMessageRequest{WidgetToken: initResp.WidgetToken, Content: "automatic takeover"}) + require.NoError(t, err) + + var conversation model.Conversation + require.NoError(t, db.First(&conversation, response.ConversationID).Error) + assert.Equal(t, string(model.ConversationStatusPending), conversation.Status) + require.NotNil(t, conversation.AssigneeAgentBotID) + assert.Equal(t, bot.ID, *conversation.AssigneeAgentBotID) + assert.Equal(t, uint(1), conversation.AITakeoverVersion) + var job model.BackgroundJob + require.NoError(t, db.Where("job_type = ?", TaskTypeCaptainConversationResponseBuilder).First(&job).Error) +} + +func TestWidgetService_SendMessage_AutomaticCaptainTakeoverDoesNotClaimExistingOpenConversation(t *testing.T) { + db, svc := setupWidgetServiceTest(t) + account, inbox := seedWidgetInbox(t, db) + initResp, err := svc.Init(context.Background(), WidgetInitRequest{WebsiteToken: "test_ws_token_123"}) + require.NoError(t, err) + first, err := svc.SendMessage(context.Background(), WidgetSendMessageRequest{WidgetToken: initResp.WidgetToken, Content: "before enabling"}) + require.NoError(t, err) + + assistant := &model.CaptainAssistant{AccountID: account.ID, Name: "Auto", Status: model.AssistantStatusActive, Config: json.RawMessage(`{}`)} + require.NoError(t, db.Create(assistant).Error) + require.NoError(t, db.Create(&model.CaptainInbox{AccountID: account.ID, InboxID: inbox.ID, AssistantID: assistant.ID}).Error) + bot := &model.AgentBot{AccountID: &account.ID, Name: "Auto", BotType: "captain", Config: json.RawMessage(fmt.Sprintf(`{"assistant_id":%d}`, assistant.ID))} + require.NoError(t, db.Create(bot).Error) + require.NoError(t, db.Create(&model.AgentBotInbox{AgentBotID: bot.ID, InboxID: inbox.ID, Status: model.AgentBotInboxActive}).Error) + require.NoError(t, db.Create(&model.CaptainPreference{AccountID: account.ID, AutoReplyEnabled: true}).Error) + svc.SetWorkerPool(worker.NewWorkerPool(db)) + + _, err = svc.SendMessage(context.Background(), WidgetSendMessageRequest{WidgetToken: initResp.WidgetToken, ConversationID: &first.ConversationID, Content: "after enabling"}) + require.NoError(t, err) + var conversation model.Conversation + require.NoError(t, db.First(&conversation, first.ConversationID).Error) + assert.Equal(t, string(model.ConversationStatusOpen), conversation.Status) + assert.Nil(t, conversation.AssigneeAgentBotID) + var jobCount int64 + require.NoError(t, db.Model(&model.BackgroundJob{}).Where("job_type = ?", TaskTypeCaptainConversationResponseBuilder).Count(&jobCount).Error) + assert.Zero(t, jobCount) +} + func TestWidgetService_SendMessage_ExistingConversation(t *testing.T) { db, svc := setupWidgetServiceTest(t) ctx := context.Background() diff --git a/backend/migrations/000077_add_contact_inbox_channel_metadata.down.sql b/backend/migrations/000078_add_contact_inbox_channel_metadata.down.sql similarity index 100% rename from backend/migrations/000077_add_contact_inbox_channel_metadata.down.sql rename to backend/migrations/000078_add_contact_inbox_channel_metadata.down.sql diff --git a/backend/migrations/000077_add_contact_inbox_channel_metadata.up.sql b/backend/migrations/000078_add_contact_inbox_channel_metadata.up.sql similarity index 100% rename from backend/migrations/000077_add_contact_inbox_channel_metadata.up.sql rename to backend/migrations/000078_add_contact_inbox_channel_metadata.up.sql diff --git a/frontend/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue b/frontend/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue index 4a46afe7..36edf131 100644 --- a/frontend/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue +++ b/frontend/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue @@ -68,7 +68,7 @@ const isHMACVerified = computed(() => { }); const currentContact = computed(() => - store.getters['contacts/getContact'](props.chat.meta.sender.id) + store.getters['contacts/getContact'](props.chat?.meta?.sender?.id) ); const isSnoozed = computed( diff --git a/frontend/app/javascript/dashboard/store/modules/conversations/index.js b/frontend/app/javascript/dashboard/store/modules/conversations/index.js index df052533..1ed121f1 100644 --- a/frontend/app/javascript/dashboard/store/modules/conversations/index.js +++ b/frontend/app/javascript/dashboard/store/modules/conversations/index.js @@ -245,7 +245,8 @@ export const mutations = { if (!conversation?.id) return; const { allConversations } = _state; - const index = allConversations.findIndex(c => c.id === conversation.id); + const normalizedId = Number(conversation.id); + const index = allConversations.findIndex(c => Number(c.id) === normalizedId); if (index > -1) { const selectedConversation = allConversations[index]; @@ -256,8 +257,12 @@ export const mutations = { } const { messages, ...updates } = conversation; - allConversations[index] = { ...selectedConversation, ...updates }; - if (_state.selectedChatId === conversation.id) { + allConversations[index] = { + ...selectedConversation, + ...updates, + id: selectedConversation.id, + }; + if (Number(_state.selectedChatId) === normalizedId) { emitter.emit(BUS_EVENTS.SCROLL_TO_MESSAGE); } } else { diff --git a/frontend/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js b/frontend/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js index d1fc6480..7f7d2085 100644 --- a/frontend/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js +++ b/frontend/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js @@ -802,6 +802,28 @@ describe('#mutations', () => { }); }); + it('should update when route and API ids use different primitive types', () => { + const state = { + allConversations: [{ id: '3', status: 'pending', updated_at: 100 }], + selectedChatId: '3', + }; + + mutations[types.UPDATE_CONVERSATION](state, { + id: 3, + status: 'open', + ai_takeover_active: false, + updated_at: 200, + }); + + expect(state.allConversations).toHaveLength(1); + expect(state.allConversations[0]).toMatchObject({ + id: '3', + status: 'open', + ai_takeover_active: false, + }); + expect(emitter.emit).toHaveBeenCalled(); + }); + it('should add conversation if not found on normal view', () => { const state = { allConversations: [], diff --git a/frontend/app/javascript/entrypoints/specs/widget.spec.js b/frontend/app/javascript/entrypoints/specs/widget.spec.js new file mode 100644 index 00000000..fa4e0a5c --- /dev/null +++ b/frontend/app/javascript/entrypoints/specs/widget.spec.js @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { app, ActionCableConnector } = vi.hoisted(() => ({ + app: { + use: vi.fn(), + directive: vi.fn(), + mount: vi.fn(() => ({ $store: {} })), + }, + ActionCableConnector: vi.fn(), +})); + +vi.mock('vue', () => ({ createApp: () => app })); +vi.mock('vue-i18n', () => ({ createI18n: vi.fn() })); +vi.mock('vue-dompurify-html', () => ({ default: {} })); +vi.mock('../../widget/store', () => ({ default: {} })); +vi.mock('../../widget/App.vue', () => ({ default: {} })); +vi.mock('../../widget/helpers/actionCable', () => ({ + default: ActionCableConnector, +})); +vi.mock('../../widget/i18n', () => ({ default: {} })); +vi.mock('../../widget/router', () => ({ default: {} })); +vi.mock('vue3-click-away', () => ({ directive: {} })); +vi.mock('../../shared/helpers/HTMLSanitizer', () => ({ + domPurifyConfig: {}, +})); +vi.mock('@formkit/vue', () => ({ + plugin: {}, + defaultConfig: vi.fn(), +})); +vi.mock('shared/helpers/Validators', () => ({ + startsWithPlus: vi.fn(), + isPhoneNumberValidWithDialCode: vi.fn(), +})); + +describe('widget entrypoint', () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + delete window.WOOT_WIDGET; + delete window.actionCable; + window.chatwootPubsubToken = 'pubsub-token'; + }); + + it('mounts and connects only once when load fires repeatedly', async () => { + vi.spyOn(document, 'readyState', 'get').mockReturnValue('loading'); + + await import('../widget'); + window.dispatchEvent(new Event('load')); + window.dispatchEvent(new Event('load')); + + expect(app.mount).toHaveBeenCalledOnce(); + expect(ActionCableConnector).toHaveBeenCalledOnce(); + expect(ActionCableConnector).toHaveBeenCalledWith( + window.WOOT_WIDGET, + 'pubsub-token' + ); + }); +}); diff --git a/frontend/app/javascript/entrypoints/widget.js b/frontend/app/javascript/entrypoints/widget.js index 5c1fac7a..f85417f4 100644 --- a/frontend/app/javascript/entrypoints/widget.js +++ b/frontend/app/javascript/entrypoints/widget.js @@ -45,10 +45,18 @@ app.use( // Vue.config.productionTip = false; -window.onload = () => { +export const initWidget = () => { + if (window.WOOT_WIDGET) return; + window.WOOT_WIDGET = app.mount('#app'); window.actionCable = new ActionCableConnector( window.WOOT_WIDGET, window.chatwootPubsubToken ); }; + +if (document.readyState === 'loading') { + window.addEventListener('load', initWidget, { once: true }); +} else { + initWidget(); +} diff --git a/frontend/app/javascript/widget/mixins/messageMixin.js b/frontend/app/javascript/widget/mixins/messageMixin.js index dfa1e0e6..d7529f2a 100644 --- a/frontend/app/javascript/widget/mixins/messageMixin.js +++ b/frontend/app/javascript/widget/mixins/messageMixin.js @@ -2,7 +2,7 @@ export default { computed: { messageContentAttributes() { const { content_attributes: attribute = {} } = this.message; - return attribute; + return attribute || {}; }, hasAttachments() { return !!( diff --git a/frontend/app/javascript/widget/mixins/specs/messageMixin.spec.js b/frontend/app/javascript/widget/mixins/specs/messageMixin.spec.js index a6443ccd..9360f29f 100644 --- a/frontend/app/javascript/widget/mixins/specs/messageMixin.spec.js +++ b/frontend/app/javascript/widget/mixins/specs/messageMixin.spec.js @@ -34,4 +34,11 @@ describe('messageMixin', () => { expect(wrapper.vm.messageContentAttributes).toEqual({}); expect(wrapper.vm.hasAttachments).toBe(true); }); + + it('normalizes null content attributes', () => { + const wrapper = shallowMount(Component, { + data: () => ({ message: { content_attributes: null } }), + }); + expect(wrapper.vm.messageContentAttributes).toEqual({}); + }); }); diff --git a/frontend/widget.html b/frontend/widget.html index d0095c17..361b95be 100644 --- a/frontend/widget.html +++ b/frontend/widget.html @@ -49,6 +49,7 @@ (function() { var params = new URLSearchParams(window.location.search); var websiteToken = params.get('website_token'); + var widgetToken = params.get('cw_conversation') || localStorage.getItem('cw_conversation'); if (!websiteToken) return; window.chatwootWebChannel = { websiteToken: websiteToken, @@ -75,22 +76,20 @@ if (xhr.status === 200) { try { var config = JSON.parse(xhr.responseText); - if (config.widgetColor) window.chatwootWebChannel.widgetColor = config.widgetColor; - if (config.enabledLanguages) window.chatwootWebChannel.enabledLanguages = config.enabledLanguages; - if (config.workingHours) window.chatwootWebChannel.workingHours = config.workingHours; - if (config.workingHoursEnabled !== undefined) window.chatwootWebChannel.workingHoursEnabled = config.workingHoursEnabled; - if (config.replyTime) window.chatwootWebChannel.replyTime = config.replyTime; - if (config.preChatFormEnabled !== undefined) window.chatwootWebChannel.preChatFormEnabled = config.preChatFormEnabled; - if (config.businessHoursEnabled !== undefined) window.chatwootWebChannel.businessHoursEnabled = config.businessHoursEnabled; - if (config.offlineMessageEnabled !== undefined) window.chatwootWebChannel.offlineMessageEnabled = config.offlineMessageEnabled; - if (config.portal) window.chatwootWebChannel.portal = config.portal; - if (config.enabledFeatures) window.chatwootWebChannel.enabledFeatures = config.enabledFeatures; - if (config.allowMessageAfterResolved !== undefined) window.chatwootWebChannel.allowMessageAfterResolved = config.allowMessageAfterResolved; - if (config.hmacEnabled !== undefined) window.chatwootWebChannel.hmacEnabled = config.hmacEnabled; + var channel = config.website_channel_config || {}; + window.authToken = channel.auth_token; + localStorage.setItem('cw_conversation', window.authToken); + window.chatwootPubsubToken = config.contact && config.contact.pubsub_token; + channel.widgetColor = channel.widget_color || window.chatwootWebChannel.widgetColor; + channel.websiteName = channel.website_name; + channel.welcomeTitle = channel.welcome_title; + channel.welcomeTagline = channel.welcome_tagline; + window.chatwootWebChannel = Object.assign(window.chatwootWebChannel, channel); + import('/app/javascript/entrypoints/widget.js'); } catch(e) {} } }; - xhr.send('{"website_token":"' + websiteToken + '"}'); + xhr.send(JSON.stringify({ website_token: websiteToken, widget_token: widgetToken })); })(); })(); @@ -98,6 +97,5 @@
-