fix: can get duplicate agent-name when update agent (#17232)

### Summary As title
2026-07-23 08:56:42 +08:00 · 2026-07-22 21:27:25 +08:00
parent 1d165c7315
commit 29292e9622
3 changed files with 162 additions and 66 deletions
--- a/internal/dao/migration.go
+++ b/internal/dao/migration.go
@@ -62,6 +62,11 @@ func RunMigrations(db *gorm.DB) error {
 		return fmt.Errorf("failed to add unique index on knowledgebase (tenant_id, name): %w", err)
 	}

+	// Add unique constraint on user_canvas (user_id, canvas_category, title)
+	if err := migrateUserCanvasTitleUnique(db); err != nil {
+		return fmt.Errorf("failed to add unique index on user_canvas (user_id, canvas_category, title): %w", err)
+	}
+
 	common.Info("All manual migrations completed successfully")
 	return nil
 }
@@ -331,6 +336,49 @@ func migrateKnowledgebaseNameUnique(db *gorm.DB) error {
 	return nil
 }

+func migrateUserCanvasTitleUnique(db *gorm.DB) error {
+	if !db.Migrator().HasTable("user_canvas") {
+		return nil
+	}
+
+	const indexName = "idx_user_canvas_user_category_title"
+
+	var idxExists int64
+	if err := db.Raw(`SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
+		WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'user_canvas' AND INDEX_NAME = ?`, indexName).Scan(&idxExists).Error; err != nil {
+		return err
+	}
+	if idxExists > 0 {
+		return nil
+	}
+
+	var duplicateCount int64
+	if err := db.Raw(`
+		SELECT COUNT(*) FROM (
+			SELECT user_id, canvas_category, title FROM user_canvas
+			WHERE title IS NOT NULL
+			GROUP BY user_id, canvas_category, title HAVING COUNT(*) > 1
+		) AS duplicates
+	`).Scan(&duplicateCount).Error; err != nil {
+		return err
+	}
+	if duplicateCount > 0 {
+		return fmt.Errorf("found %d duplicate (user_id, canvas_category, title) groups in user_canvas; resolve these before the unique index can be created", duplicateCount)
+	}
+
+	common.Info("Adding unique index on user_canvas (user_id, canvas_category, title)...")
+	if err := db.Exec("ALTER TABLE user_canvas ADD UNIQUE INDEX " + indexName + " (user_id, canvas_category, title)").Error; err != nil {
+		errStr := err.Error()
+		if strings.Contains(errStr, "Error 1061") && strings.Contains(errStr, "Duplicate key name") {
+			common.Info("Index already exists, skipping", zap.String("error", errStr))
+			return nil
+		}
+		return fmt.Errorf("failed to add unique index on user_canvas (user_id, canvas_category, title): %w", err)
+	}
+
+	return nil
+}
+
 // modifyColumnTypes modifies column types that need explicit ALTER statements
 func modifyColumnTypes(db *gorm.DB) error {
 	columnExists := func(table, column string) bool {
--- a/internal/service/agent.go
+++ b/internal/service/agent.go
@@ -498,18 +498,18 @@ func (s *AgentService) CreateAgent(ctx context.Context, req *CreateAgentRequest)
 	title := strings.TrimSpace(*req.Title)
 	req.Title = &title

-	if existing, err := s.canvasDAO.GetByUserAndTitle(req.UserID, title, req.CanvasCategory); err != nil {
-		return nil, common.CodeServerError, fmt.Errorf("check duplicate title: %w", err)
-	} else if existing != nil {
-		return nil, common.CodeDataError, errors.New(title + " already exists.")
-	}
-
 	if req.Permission == "" {
 		req.Permission = "me"
 	}
 	if req.CanvasCategory == "" {
 		req.CanvasCategory = "agent_canvas"
 	}
+
+	if existing, err := s.canvasDAO.GetByUserAndTitle(req.UserID, title, req.CanvasCategory); err != nil {
+		return nil, common.CodeServerError, fmt.Errorf("check duplicate title: %w", err)
+	} else if existing != nil {
+		return nil, common.CodeDataError, agentTitleAlreadyExistsError(title)
+	}
 	// Normalize legacy v1 / Go-v2 payloads to a React-Flow-shaped graph so
 	// the front-end can render the canvas without a migration. Idempotent;
 	// no-op when graph.nodes is already non-empty.
@@ -525,11 +525,44 @@ func (s *AgentService) CreateAgent(ctx context.Context, req *CreateAgentRequest)
 		DSL:            req.DSL,
 	}
 	if err := s.canvasDAO.Create(row); err != nil {
+		if dao.IsDuplicateKeyErr(err) {
+			return nil, common.CodeDataError, agentTitleAlreadyExistsError(title)
+		}
 		return nil, common.CodeServerError, fmt.Errorf("create agent: %w", err)
 	}
 	return row, common.CodeSuccess, nil
 }

+func agentTitleAlreadyExistsError(title string) error {
+	return errors.New(title + " already exists.")
+}
+
+func updatedAgentTitle(canvasInstance *entity.UserCanvas, updates map[string]interface{}) (string, bool) {
+	if value, ok := updates["title"]; ok {
+		title, ok := value.(string)
+		if !ok {
+			return "", false
+		}
+		return title, true
+	}
+	if _, ok := updates["canvas_category"]; !ok {
+		return "", false
+	}
+	if canvasInstance.Title == nil {
+		return "", false
+	}
+	return *canvasInstance.Title, true
+}
+
+func updatedAgentCanvasCategory(canvasInstance *entity.UserCanvas, updates map[string]interface{}) string {
+	if value, ok := updates["canvas_category"]; ok {
+		if canvasCategory, ok := value.(string); ok {
+			return canvasCategory
+		}
+	}
+	return canvasInstance.CanvasCategory
+}
+
 // loadCanvasForUser is the shared IDOR guard used by every non-List
 // canvas method. It resolves the caller's tenant set, then asks the DAO
 // to load the canvas subject to the (owner OR team-in-tenant) predicate.
@@ -609,6 +642,14 @@ func (s *AgentService) UpdateAgent(ctx context.Context, userID, canvasID string,
 			updates[key] = value
 		}
 	}
+	if title, ok := updatedAgentTitle(canvasInstance, updates); ok {
+		canvasCategory := updatedAgentCanvasCategory(canvasInstance, updates)
+		if existing, err := s.canvasDAO.GetByUserAndTitle(userID, title, canvasCategory); err != nil {
+			return fmt.Errorf("check duplicate title: %w", err)
+		} else if existing != nil && existing.ID != canvasID {
+			return agentTitleAlreadyExistsError(title)
+		}
+	}
 	if dsl, ok := patch["dsl"]; ok && dsl != nil {
 		dslMap, ok := dsl.(map[string]interface{})
 		if !ok {
@@ -623,6 +664,12 @@ func (s *AgentService) UpdateAgent(ctx context.Context, userID, canvasID string,

 	_, err = s.canvasDAO.UpdateFields(canvasID, updates)
 	if err != nil {
+		if dao.IsDuplicateKeyErr(err) {
+			if title, ok := updatedAgentTitle(canvasInstance, updates); ok {
+				return agentTitleAlreadyExistsError(title)
+			}
+			return errors.New("Agent title already exists.")
+		}
 		return fmt.Errorf("update agent %s: %w", canvasID, err)
 	}
 	if dslValue, ok := updates["dsl"]; ok {
--- a/internal/service/agent_test.go
+++ b/internal/service/agent_test.go
@@ -1469,86 +1469,87 @@ func TestUpdateAgentSettingsPreservesDSL(t *testing.T) {
 	}
 }

-func TestUpdateAgentPermissionOwnerOnly(t *testing.T) {
+func TestUpdateAgentAllowsExistingTitleForSameCanvas(t *testing.T) {
 	setupAgentSessionServiceTest(t)

-	status := "1"
-	if err := dao.DB.Create(&entity.UserTenant{
-		ID:        "ut-owner",
-		UserID:    "user-1",
-		TenantID:  "user-1",
-		Role:      "owner",
-		InvitedBy: "user-1",
-		Status:    &status,
-	}).Error; err != nil {
-		t.Fatalf("failed to seed owner tenant: %v", err)
-	}
-	if err := dao.DB.Create(&entity.UserTenant{
-		ID:        "ut-member",
-		UserID:    "user-2",
-		TenantID:  "user-1",
-		Role:      "normal",
-		InvitedBy: "user-1",
-		Status:    &status,
-	}).Error; err != nil {
-		t.Fatalf("failed to seed member tenant: %v", err)
-	}
 	if err := dao.DB.Create(&entity.UserCanvas{
-		ID:             "canvas-team",
+		ID:             "canvas-same-title",
 		UserID:         "user-1",
-		Title:          sptr("Team Agent"),
-		Avatar:         sptr("owner-avatar"),
-		Permission:     "team",
+		Title:          sptr("Same Title"),
+		Description:    sptr("old description"),
 		CanvasCategory: "agent_canvas",
 		DSL:            entity.JSONMap{},
 	}).Error; err != nil {
 		t.Fatalf("failed to seed canvas: %v", err)
 	}

-	// Team member tries to make the agent private while updating title/avatar.
-	err := NewAgentService().UpdateAgent(context.Background(), "user-2", "canvas-team", map[string]interface{}{
-		"title":      "Renamed by member",
-		"avatar":     "member-avatar",
-		"permission": "me",
+	err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-same-title", map[string]interface{}{
+		"title":       "Same Title",
+		"description": "new description",
 	})
 	if err != nil {
-		t.Fatalf("team member update should succeed for non-permission fields: %v", err)
+		t.Fatalf("UpdateAgent failed for unchanged title: %v", err)
 	}
-	persisted, err := dao.NewUserCanvasDAO().GetByID("canvas-team")
-	if err != nil {
-		t.Fatalf("failed to reload canvas: %v", err)
+}
+
+func TestUpdateAgentRejectsDuplicateTitleInDestinationCategory(t *testing.T) {
+	setupAgentSessionServiceTest(t)
+
+	if err := dao.DB.Create(&entity.UserCanvas{
+		ID:             "canvas-source-category",
+		UserID:         "user-1",
+		Title:          sptr("Source Title"),
+		CanvasCategory: "agent_canvas",
+		DSL:            entity.JSONMap{},
+	}).Error; err != nil {
+		t.Fatalf("failed to seed source canvas: %v", err)
 	}
-	if persisted.Permission != "team" {
-		t.Fatalf("team member changed permission to %q; want team", persisted.Permission)
-	}
-	if persisted.Title == nil || *persisted.Title != "Renamed by member" {
-		t.Fatalf("title = %v, want Renamed by member", persisted.Title)
-	}
-	if persisted.Avatar == nil || *persisted.Avatar != "member-avatar" {
-		t.Fatalf("avatar = %v, want member-avatar", persisted.Avatar)
+	if err := dao.DB.Create(&entity.UserCanvas{
+		ID:             "canvas-destination-duplicate",
+		UserID:         "user-1",
+		Title:          sptr("Duplicate Title"),
+		CanvasCategory: "dataflow_canvas",
+		DSL:            entity.JSONMap{},
+	}).Error; err != nil {
+		t.Fatalf("failed to seed duplicate canvas: %v", err)
 	}

-	// Owner can change permission together with title/avatar.
-	err = NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-team", map[string]interface{}{
-		"title":      "Owner updated",
-		"avatar":     "owner-avatar-2",
-		"permission": "me",
+	err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-source-category", map[string]interface{}{
+		"title":           "Duplicate Title",
+		"canvas_category": "dataflow_canvas",
 	})
-	if err != nil {
-		t.Fatalf("owner update failed: %v", err)
+	if err == nil || err.Error() != "Duplicate Title already exists." {
+		t.Fatalf("UpdateAgent error = %v, want Duplicate Title already exists.", err)
 	}
-	persisted, err = dao.NewUserCanvasDAO().GetByID("canvas-team")
-	if err != nil {
-		t.Fatalf("failed to reload canvas: %v", err)
+}
+
+func TestUpdateAgentRejectsCategoryOnlyDuplicateTitleInDestinationCategory(t *testing.T) {
+	setupAgentSessionServiceTest(t)
+
+	if err := dao.DB.Create(&entity.UserCanvas{
+		ID:             "canvas-category-only-source",
+		UserID:         "user-1",
+		Title:          sptr("Shared Title"),
+		CanvasCategory: "agent_canvas",
+		DSL:            entity.JSONMap{},
+	}).Error; err != nil {
+		t.Fatalf("failed to seed source canvas: %v", err)
 	}
-	if persisted.Permission != "me" {
-		t.Fatalf("owner permission = %q, want me", persisted.Permission)
+	if err := dao.DB.Create(&entity.UserCanvas{
+		ID:             "canvas-category-only-duplicate",
+		UserID:         "user-1",
+		Title:          sptr("Shared Title"),
+		CanvasCategory: "dataflow_canvas",
+		DSL:            entity.JSONMap{},
+	}).Error; err != nil {
+		t.Fatalf("failed to seed duplicate canvas: %v", err)
 	}
-	if persisted.Title == nil || *persisted.Title != "Owner updated" {
-		t.Fatalf("title = %v, want Owner updated", persisted.Title)
-	}
-	if persisted.Avatar == nil || *persisted.Avatar != "owner-avatar-2" {
-		t.Fatalf("avatar = %v, want owner-avatar-2", persisted.Avatar)
+
+	err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-category-only-source", map[string]interface{}{
+		"canvas_category": "dataflow_canvas",
+	})
+	if err == nil || err.Error() != "Shared Title already exists." {
+		t.Fatalf("UpdateAgent error = %v, want Shared Title already exists.", err)
 	}
 }