From d0d03394288435b0df18592c390d8321c8d841db Mon Sep 17 00:00:00 2001 From: Hz_ Date: Thu, 2 Jul 2026 13:41:24 +0800 Subject: [PATCH] fix(go): agent settings update clearing DSL (#16495) ### Summary This PR fixes a Go backend bug where updating agent settings, such as description, could clear the agent DSL. Root cause: PUT /api/v1/agents/:canvas_id only bound the dsl field in Go. When the frontend submitted settings without dsl, the service still updated the canvas with an empty DSL value. Changes: - Treat agent updates as partial patches. - Preserve existing DSL when dsl is not present in the request. - Update only specified user_canvas fields instead of saving the full row. - Add a regression test for settings updates preserving DSL. Test: `go test ./internal/service ./internal/handler` Co-authored-by: Zhichang Yu --- internal/dao/user_canvas.go | 6 ++ internal/handler/agent.go | 15 ++-- internal/handler/agent_test.go | 2 +- internal/handler/agent_wait_for_user_test.go | 2 +- internal/service/agent.go | 41 ++++++++-- internal/service/agent_test.go | 86 ++++++++++++++++++++ 6 files changed, 134 insertions(+), 18 deletions(-) diff --git a/internal/dao/user_canvas.go b/internal/dao/user_canvas.go index 67adf7538..0b1ee5063 100644 --- a/internal/dao/user_canvas.go +++ b/internal/dao/user_canvas.go @@ -494,6 +494,12 @@ func (dao *UserCanvasDAO) UpdateDSL(canvasID string, dsl entity.JSONMap) (int64, return result.RowsAffected, result.Error } +// UpdateFields updates only the supplied user_canvas columns. +func (dao *UserCanvasDAO) UpdateFields(canvasID string, fields map[string]interface{}) (int64, error) { + result := DB.Model(&entity.UserCanvas{}).Where("id = ?", canvasID).Updates(fields) + return result.RowsAffected, result.Error +} + // UpdateTags updates a canvas's comma-separated tags by canvas ID. func (dao *UserCanvasDAO) UpdateTags(canvasID, tags string) (int64, error) { result := DB.Model(&entity.UserCanvas{}).Where("id = ?", canvasID).Update("tags", tags) diff --git a/internal/handler/agent.go b/internal/handler/agent.go index 1e42e3798..121b4b83d 100644 --- a/internal/handler/agent.go +++ b/internal/handler/agent.go @@ -306,17 +306,15 @@ func (h *AgentHandler) GetAgent(c *gin.Context) { } // updateAgentRequest is the wire shape for PUT /api/v1/agents/:canvas_id. -type updateAgentRequest struct { - DSL entity.JSONMap `json:"dsl"` -} +type updateAgentRequest map[string]interface{} -// UpdateAgent writes a new draft DSL to the canvas (no version created). -// @Summary Update Agent (Draft) +// UpdateAgent applies a partial update to the canvas draft. +// @Summary Update Agent // @Tags agents // @Accept json // @Produce json // @Param canvas_id path string true "canvas id" -// @Param request body updateAgentRequest true "draft DSL payload" +// @Param request body updateAgentRequest true "agent update payload" // @Success 200 {object} map[string]interface{} // @Router /api/v1/agents/{canvas_id} [put] func (h *AgentHandler) UpdateAgent(c *gin.Context) { @@ -331,7 +329,10 @@ func (h *AgentHandler) UpdateAgent(c *gin.Context) { jsonError(c, common.CodeArgumentError, "Invalid request: "+err.Error()) return } - if err := h.agentService.UpdateAgent(c.Request.Context(), user.ID, canvasID, req.DSL); err != nil { + if req == nil { + req = updateAgentRequest{} + } + if err := h.agentService.UpdateAgent(c.Request.Context(), user.ID, canvasID, map[string]interface{}(req)); err != nil { ec, em := mapAgentError(err) jsonError(c, ec, em) return diff --git a/internal/handler/agent_test.go b/internal/handler/agent_test.go index 3f4028aab..2dc6731eb 100644 --- a/internal/handler/agent_test.go +++ b/internal/handler/agent_test.go @@ -441,7 +441,7 @@ func (f *fullFakeAgentService) GetAgent(context.Context, string, string) (*entit } return f.getRow, nil } -func (f *fullFakeAgentService) UpdateAgent(context.Context, string, string, entity.JSONMap) error { +func (f *fullFakeAgentService) UpdateAgent(context.Context, string, string, map[string]interface{}) error { return nil } func (f *fullFakeAgentService) DeleteAgent(context.Context, string, string) error { diff --git a/internal/handler/agent_wait_for_user_test.go b/internal/handler/agent_wait_for_user_test.go index 1baaea7eb..28d15f181 100644 --- a/internal/handler/agent_wait_for_user_test.go +++ b/internal/handler/agent_wait_for_user_test.go @@ -84,7 +84,7 @@ func (f *waitFakeAgentService) CreateAgent(context.Context, *service.CreateAgent func (f *waitFakeAgentService) GetAgent(context.Context, string, string) (*entity.UserCanvas, error) { return &entity.UserCanvas{ID: "canvas-wait"}, nil } -func (f *waitFakeAgentService) UpdateAgent(context.Context, string, string, entity.JSONMap) error { +func (f *waitFakeAgentService) UpdateAgent(context.Context, string, string, map[string]interface{}) error { return nil } func (f *waitFakeAgentService) DeleteAgent(context.Context, string, string) error { diff --git a/internal/service/agent.go b/internal/service/agent.go index 4cf98ed06..bd72e1342 100644 --- a/internal/service/agent.go +++ b/internal/service/agent.go @@ -418,17 +418,40 @@ func (s *AgentService) GetAgent(ctx context.Context, userID, canvasID string) (* return s.loadCanvasForUser(ctx, userID, canvasID) } -// UpdateAgent writes a new DSL to the draft (user_canvas.dsl) and toggles -// release=false. The call does NOT create a new user_canvas_version row — -// versions are produced only by PublishAgent. -func (s *AgentService) UpdateAgent(ctx context.Context, userID, canvasID string, dsl entity.JSONMap) error { - row, err := s.loadCanvasForUser(ctx, userID, canvasID) - if err != nil { +// UpdateAgent applies a draft patch to user_canvas. Settings updates may omit +// dsl; in that case the existing draft DSL must be preserved. +func (s *AgentService) UpdateAgent(ctx context.Context, userID, canvasID string, patch map[string]interface{}) error { + if _, err := s.loadCanvasForUser(ctx, userID, canvasID); err != nil { return err } - row.DSL = dslpkg.NormalizeForCanvas(dsl) - row.Release = false - if err := s.canvasDAO.Update(row); err != nil { + + updates := map[string]interface{}{ + "release": false, + } + for _, key := range []string{"title", "avatar", "description", "permission", "canvas_type", "canvas_category"} { + if value, ok := patch[key]; ok && value != nil { + if key == "title" { + if title, ok := value.(string); ok { + value = strings.TrimSpace(title) + } + } + updates[key] = value + } + } + if dsl, ok := patch["dsl"]; ok && dsl != nil { + dslMap, ok := dsl.(map[string]interface{}) + if !ok { + if typed, ok := dsl.(entity.JSONMap); ok { + dslMap = map[string]interface{}(typed) + } else { + return fmt.Errorf("update agent %s: dsl must be an object", canvasID) + } + } + updates["dsl"] = entity.JSONMap(dslpkg.NormalizeForCanvas(entity.JSONMap(dslMap))) + } + + _, err := s.canvasDAO.UpdateFields(canvasID, updates) + if err != nil { return fmt.Errorf("update agent %s: %w", canvasID, err) } return nil diff --git a/internal/service/agent_test.go b/internal/service/agent_test.go index a6d9f6b8f..37c1a8142 100644 --- a/internal/service/agent_test.go +++ b/internal/service/agent_test.go @@ -1186,6 +1186,92 @@ func TestResetAgentServiceNotFound(t *testing.T) { } } +func TestUpdateAgentSettingsPreservesDSL(t *testing.T) { + setupAgentSessionServiceTest(t) + + originalDSL := entity.JSONMap{ + "graph": map[string]any{ + "nodes": []any{map[string]any{"id": "begin"}}, + "edges": []any{}, + }, + "components": map[string]any{ + "begin": map[string]any{ + "obj": map[string]any{"component_name": "Begin"}, + }, + }, + } + if err := dao.DB.Create(&entity.UserCanvas{ + ID: "canvas-settings", + UserID: "user-1", + Title: sptr("Settings Agent"), + Description: sptr("old description"), + CanvasCategory: "agent_canvas", + DSL: originalDSL, + }).Error; err != nil { + t.Fatalf("failed to seed canvas: %v", err) + } + + err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-settings", map[string]interface{}{ + "description": "new description", + }) + if err != nil { + t.Fatalf("UpdateAgent failed: %v", err) + } + + persisted, err := dao.NewUserCanvasDAO().GetByID("canvas-settings") + if err != nil { + t.Fatalf("failed to reload canvas: %v", err) + } + if persisted.Description == nil || *persisted.Description != "new description" { + t.Fatalf("Description = %v, want new description", persisted.Description) + } + if _, ok := persisted.DSL["graph"]; !ok { + t.Fatalf("DSL graph was removed: %#v", persisted.DSL) + } + if _, ok := persisted.DSL["components"]; !ok { + t.Fatalf("DSL components were removed: %#v", persisted.DSL) + } +} + +func TestUpdateAgentPersistsDSLAsJSONMap(t *testing.T) { + setupAgentSessionServiceTest(t) + + if err := dao.DB.Create(&entity.UserCanvas{ + ID: "canvas-dsl-update", + UserID: "user-1", + Title: sptr("DSL Agent"), + CanvasCategory: "agent_canvas", + DSL: entity.JSONMap{}, + }).Error; err != nil { + t.Fatalf("failed to seed canvas: %v", err) + } + + err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-dsl-update", map[string]interface{}{ + "dsl": map[string]interface{}{ + "graph": map[string]interface{}{ + "nodes": []interface{}{map[string]interface{}{"id": "begin"}}, + "edges": []interface{}{}, + }, + "components": map[string]interface{}{ + "begin": map[string]interface{}{ + "obj": map[string]interface{}{"component_name": "Begin"}, + }, + }, + }, + }) + if err != nil { + t.Fatalf("UpdateAgent failed: %v", err) + } + + persisted, err := dao.NewUserCanvasDAO().GetByID("canvas-dsl-update") + if err != nil { + t.Fatalf("failed to reload canvas: %v", err) + } + if _, ok := persisted.DSL["graph"]; !ok { + t.Fatalf("DSL graph was not persisted: %#v", persisted.DSL) + } +} + // TestResetAgentServiceOtherTenant asserts the access-denied path: // a canvas owned by user-2 is not visible to user-1, so the same // not-found error type is returned. The service layer does not