H-338: close H-335 release blockers (#59)

* H-16: align takeover with channel AI workflow (#2) * feat(conversations): complete manual AI takeover * fix(conversations): align AI takeover flow with channel AI * fix(conversations): close takeover review gaps --------- Co-authored-by: Rogee <rogee@ipao.vip> * feat(shangwutong): sync customer names back to channel (#3) Co-authored-by: Rogee <rogee@ipao.vip> * fix(shangwutong): close contact sync review gaps (#4) Co-authored-by: Rogee <rogee@ipao.vip> * H-28: harden Shangwutong CID sync (#5) * fix(shangwutong): close contact sync review gaps * fix(shangwutong): harden CID sync boundaries --------- Co-authored-by: Rogee <rogee@ipao.vip> * fix(conversations): sync AI takeover exit in realtime (#6) Co-authored-by: Rogee <rogee@ipao.vip> * test(shangwutong): cover CID rename reliability (#7) Co-authored-by: Rogee <rogee@ipao.vip> * 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 <rogee@ipao.vip> * H-55: make Captain bindings atomic (#9) Co-authored-by: Rogee <rogee@ipao.vip> * H-60: harden Captain migration rollback and concurrency * chore(agent): baseline — uncommitted work from the local directory * H-335: add safe Captain skills and user deactivation * H-338: close auth and Captain review blockers * H-338: close assignment and session races * H-338: close assignment and websocket invalidation gaps * H-338: enforce assignment write invariants --------- Co-authored-by: Rogee <rogee@ipao.vip>
2026-08-20 10:21:19 +08:00
parent 1d8145f3b5
commit 6c78820a1f
53 changed files with 1534 additions and 216 deletions
@@ -5,10 +5,17 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"testing"

 	"github.com/gin-gonic/gin"
+	"github.com/gochat/gochat/internal/channel"
+	"github.com/gochat/gochat/internal/model"
+	"github.com/gochat/gochat/internal/repository"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/driver/sqlite"
+	"gorm.io/gorm"

 	"github.com/gochat/gochat/internal/service"
 )
@@ -84,6 +91,38 @@ func TestAgentBulkHandler_BulkAssign_InvalidAccountID(t *testing.T) {
 	assert.Equal(t, http.StatusBadRequest, w.Code)
 }

+func TestAgentBulkHandlerBulkAssignRejectsInactiveAgent(t *testing.T) {
+	db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=private"), &gorm.Config{})
+	require.NoError(t, err)
+	require.NoError(t, db.AutoMigrate(&model.Account{}, &model.User{}, &model.AccountUser{}, &model.Inbox{}, &model.Contact{}, &model.Conversation{}))
+	account := &model.Account{Name: "Account"}
+	agent := &model.User{Name: "Inactive", Email: "inactive@example.com", Password: "hash", Active: true}
+	require.NoError(t, db.Create(account).Error)
+	require.NoError(t, db.Create(agent).Error)
+	require.NoError(t, db.Model(agent).Update("active", false).Error)
+	require.NoError(t, db.Create(&model.AccountUser{AccountID: account.ID, UserID: agent.ID, Role: "agent"}).Error)
+	inbox := &model.Inbox{AccountID: account.ID, Name: "Inbox", ChannelType: string(model.InboxChannelTypeWebWidget)}
+	contact := &model.Contact{AccountID: account.ID, Name: "Contact"}
+	require.NoError(t, db.Create(inbox).Error)
+	require.NoError(t, db.Create(contact).Error)
+	conversation := &model.Conversation{AccountID: account.ID, InboxID: inbox.ID, ContactID: contact.ID, Status: "open", ChannelType: "web_widget", Channel: "web_widget"}
+	require.NoError(t, db.Create(conversation).Error)
+	conversationService := service.NewConversationService(
+		repository.NewConversationRepo(db), repository.NewMessageRepo(db), channel.NewDispatcher(), nil,
+		repository.NewAccountUserRepo(db), nil, nil,
+	)
+	router := setupAgentBulkRouter(NewAgentBulkHandler(conversationService))
+	body, err := json.Marshal(map[string]any{"conversation_ids": []uint{conversation.ID}, "agent_id": agent.ID})
+	require.NoError(t, err)
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/accounts/"+strconv.FormatUint(uint64(account.ID), 10)+"/agents/bulk_assign", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	router.ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code)
+	require.Contains(t, w.Body.String(), "not an agent or administrator")
+}
+
 func TestAgentBulkHandler_BulkUnassign_BadJSON(t *testing.T) {
 	handler := NewAgentBulkHandler(&service.ConversationService{})
 	router := setupAgentBulkRouter(handler)
@@ -126,4 +165,4 @@ func TestAgentBulkHandler_BulkUnassign_InvalidAccountID(t *testing.T) {
 	router.ServeHTTP(w, req)

 	assert.Equal(t, http.StatusBadRequest, w.Code)
-}
+}
@@ -18,7 +18,13 @@ import (
 // Reference: Chatwoot app/controllers/api/v1/accounts/agents_controller.rb
 // An "agent" in Chatwoot is a User with an AccountUser membership in a specific account.
 type AgentHandler struct {
-	svc *service.AgentService
+	svc   *service.AgentService
+	audit *service.AuditService
+}
+
+func (h *AgentHandler) WithAuditService(audit *service.AuditService) *AgentHandler {
+	h.audit = audit
+	return h
 }

 // NewAgentHandler creates a new AgentHandler.
@@ -93,7 +99,6 @@ func (h *AgentHandler) Create(c *gin.Context) {
 		response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, err.Error())
 		return
 	}
-
 	// Chatwoot: validate_limit → can_add_agent? — returns 402 if limit exceeded
 	canAdd, err := h.svc.CanAddAgent(c.Request.Context(), accountID)
 	if err != nil {
@@ -121,6 +126,7 @@ func (h *AgentHandler) Create(c *gin.Context) {
 	}

 	c.Header("Cache-Control", "no-store")
+	recordAuditMutation(c, h.audit, auditMutation{AccountID: accountID, AuditableType: "User", AuditableID: agent.ID, Action: "create", AuditedChanges: gin.H{"role": agent.Role, "active": agent.Active}})
 	c.JSON(http.StatusOK, serializeAgentDetail(agent, accountID))
 }

@@ -145,6 +151,10 @@ func (h *AgentHandler) Update(c *gin.Context) {
 		response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, err.Error())
 		return
 	}
+	if req.Active != nil && !*req.Active && getUserID(c) == uint(id) {
+		response.AbortWithStatusError(c, http.StatusUnprocessableEntity, response.ErrValidation, "administrators cannot deactivate themselves")
+		return
+	}

 	agent, svcErr := h.svc.Update(c.Request.Context(), uint(id), accountID, req)
 	if svcErr != nil {
@@ -160,6 +170,7 @@ func (h *AgentHandler) Update(c *gin.Context) {
 		return
 	}

+	recordAuditMutation(c, h.audit, auditMutation{AccountID: accountID, AuditableType: "User", AuditableID: uint(id), Action: "update", AuditedChanges: agentUpdateAuditChanges(req)})
 	c.JSON(http.StatusOK, serializeAgentDetail(agent, accountID))
 }

@@ -186,6 +197,7 @@ func (h *AgentHandler) Delete(c *gin.Context) {
 		return
 	}

+	recordAuditMutation(c, h.audit, auditMutation{AccountID: accountID, AuditableType: "User", AuditableID: uint(id), Action: "destroy", AuditedChanges: gin.H{"account_id": accountID}})
 	c.Status(http.StatusOK)
 }

@@ -266,6 +278,29 @@ func serializeAgentDetails(agents []repository.AgentDetail, accountID uint) []ma
 	return payload
 }

+func agentUpdateAuditChanges(req service.UpdateAgentRequest) gin.H {
+	changes := gin.H{}
+	if req.NameSet() {
+		changes["name_changed"] = true
+	}
+	if req.Role != "" {
+		changes["role"] = req.Role
+	}
+	if req.Availability != "" {
+		changes["availability"] = req.Availability
+	}
+	if req.AutoOfflineSet() {
+		changes["auto_offline"] = req.AutoOffline
+	}
+	if req.CustomRoleIDSet() {
+		changes["custom_role_id"] = req.CustomRoleID
+	}
+	if req.Active != nil {
+		changes["active"] = *req.Active
+	}
+	return changes
+}
+
 func serializeAgentDetail(agent *repository.AgentDetail, accountID uint) map[string]any {
 	if agent == nil {
 		return map[string]any{}
@@ -287,6 +322,7 @@ func serializeAgentUser(user *model.User, accountID uint, role string, availabil
 		"account_id":          accountID,
 		"availability_status": availabilityStatus,
 		"auto_offline":        autoOffline,
+		"active":              user.Active,
 		"confirmed":           user.ConfirmedAt != nil,
 		"email":               user.Email,
 		"provider":            nonEmpty(user.Provider, "email"),
@@ -35,12 +35,12 @@ func (s *AgentHandlerTestSuite) SetupSuite() {
 		Logger: logger.Default.LogMode(logger.Silent),
 	})
 	s.Require().NoError(err)
-	s.Require().NoError(db.AutoMigrate(&model.Account{}, &model.User{}, &model.AccountUser{}, &model.InstallationConfig{}))
+	s.Require().NoError(db.AutoMigrate(&model.Account{}, &model.User{}, &model.AccountUser{}, &model.UserSession{}, &model.Audit{}, &model.InstallationConfig{}))
 	s.db = db

 	agentRepo := repository.NewAgentRepo(db)
 	svc := service.NewAgentService(agentRepo, db)
-	s.handler = NewAgentHandler(svc)
+	s.handler = NewAgentHandler(svc).WithAuditService(service.NewAuditService(repository.NewAuditRepo(db)))

 	s.account = &model.Account{Name: "test-agent-account"}
 	s.Require().NoError(db.Create(s.account).Error)
@@ -56,6 +56,8 @@ func (s *AgentHandlerTestSuite) SetupSuite() {

 func (s *AgentHandlerTestSuite) SetupTest() {
 	s.db.Exec("DELETE FROM account_users")
+	s.db.Exec("DELETE FROM user_sessions")
+	s.db.Exec("DELETE FROM audits")
 	// Don't delete users — we need the inviter user to persist
 	// Only delete agent users (not the inviter)
 	s.db.Exec("DELETE FROM users WHERE id != ?", s.user.ID)
@@ -439,6 +441,50 @@ func (s *AgentHandlerTestSuite) TestUpdateAgent() {
 	assert.Equal(s.T(), false, disableData["auto_offline"])
 }

+func (s *AgentHandlerTestSuite) TestUpdateAgentDeactivatesAndRevokesSessions() {
+	create, createCtx := s.makeRequest("POST", "/api/v1/accounts/1/agents", service.CreateAgentRequest{Email: "inactive@test.com", Name: "Inactive Agent", Role: "agent", Availability: "online"}, s.account.ID, s.user.ID)
+	s.handler.Create(createCtx)
+	s.Require().Equal(http.StatusOK, create.Code)
+	var created map[string]interface{}
+	s.Require().NoError(json.Unmarshal(create.Body.Bytes(), &created))
+	agentID := uint(created["id"].(float64))
+	s.Require().NoError(s.db.Create(&model.UserSession{UserID: agentID, ClientID: "active-client"}).Error)
+	otherAccount := model.Account{Name: "other membership"}
+	s.Require().NoError(s.db.Create(&otherAccount).Error)
+	s.Require().NoError(s.db.Create(&model.AccountUser{UserID: agentID, AccountID: otherAccount.ID, Role: "agent", Availability: "online"}).Error)
+
+	update := map[string]any{"agent": map[string]any{"active": false}}
+	w, c := s.makeRequest("PATCH", fmt.Sprintf("/api/v1/accounts/1/agents/%d", agentID), update, s.account.ID, s.user.ID)
+	s.handler.Update(c)
+	s.Equal(http.StatusOK, w.Code, w.Body.String())
+	var payload map[string]interface{}
+	s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &payload))
+	s.Equal(false, payload["active"])
+	s.Equal("offline", payload["availability_status"])
+
+	var sessions int64
+	s.Require().NoError(s.db.Model(&model.UserSession{}).Where("user_id = ?", agentID).Count(&sessions).Error)
+	s.Zero(sessions)
+	var user model.User
+	s.Require().NoError(s.db.First(&user, agentID).Error)
+	s.False(user.Active)
+	var memberships []model.AccountUser
+	s.Require().NoError(s.db.Where("user_id = ?", agentID).Find(&memberships).Error)
+	for _, membership := range memberships {
+		s.Equal("offline", membership.Availability)
+	}
+	var audit model.Audit
+	s.Require().NoError(s.db.Where("auditable_type = ? AND auditable_id = ? AND action = ?", "User", agentID, "update").First(&audit).Error)
+	s.Contains(string(audit.AuditedChanges), `"active":false`)
+}
+
+func (s *AgentHandlerTestSuite) TestUpdateAgentCannotDeactivateSelf() {
+	active := false
+	w, c := s.makeRequest("PATCH", fmt.Sprintf("/api/v1/accounts/1/agents/%d", s.user.ID), map[string]any{"agent": map[string]any{"active": active}}, s.account.ID, s.user.ID)
+	s.handler.Update(c)
+	s.Equal(http.StatusUnprocessableEntity, w.Code)
+}
+
 func (s *AgentHandlerTestSuite) TestUpdateAgentBlankNameReturnsRecordInvalidShape() {
 	req := service.CreateAgentRequest{
 		Email: "blank-update@test.com",