From 0cb759b446c8428a66c042062a072dfe24df3149 Mon Sep 17 00:00:00 2001 From: LinYushen Date: Thu, 14 May 2026 14:07:26 +0800 Subject: [PATCH] fix(squad): suppress no-action leader comments (#2583) --- .../views/issues/components/issue-detail.tsx | 22 ++ packages/views/locales/en/issues.json | 7 + packages/views/locales/zh-Hans/issues.json | 7 + server/internal/handler/comment.go | 22 +- server/internal/handler/squad.go | 12 ++ .../internal/handler/squad_no_action_test.go | 196 ++++++++++++++++++ server/internal/service/squad_no_action.go | 21 ++ server/internal/service/task.go | 11 +- ...89_squad_no_action_activity_index.down.sql | 1 + .../089_squad_no_action_activity_index.up.sql | 5 + server/pkg/db/generated/activity.sql.go | 26 +++ server/pkg/db/queries/activity.sql | 12 ++ 12 files changed, 337 insertions(+), 5 deletions(-) create mode 100644 server/internal/handler/squad_no_action_test.go create mode 100644 server/internal/service/squad_no_action.go create mode 100644 server/migrations/089_squad_no_action_activity_index.down.sql create mode 100644 server/migrations/089_squad_no_action_activity_index.up.sql diff --git a/packages/views/issues/components/issue-detail.tsx b/packages/views/issues/components/issue-detail.tsx index 3ca384dc6..63a402b25 100644 --- a/packages/views/issues/components/issue-detail.tsx +++ b/packages/views/issues/components/issue-detail.tsx @@ -226,6 +226,25 @@ function formatActivity( return t(($) => $.activity.task_completed, { count: entry.coalesced_count ?? 1 }); case "task_failed": return t(($) => $.activity.task_failed, { count: entry.coalesced_count ?? 1 }); + case "squad_leader_evaluated": { + const reason = details.reason?.trim(); + switch (details.outcome) { + case "action": + return reason + ? t(($) => $.activity.squad_leader_action_reason, { reason }) + : t(($) => $.activity.squad_leader_action); + case "no_action": + return reason + ? t(($) => $.activity.squad_leader_no_action_reason, { reason }) + : t(($) => $.activity.squad_leader_no_action); + case "failed": + return reason + ? t(($) => $.activity.squad_leader_failed_reason, { reason }) + : t(($) => $.activity.squad_leader_failed); + default: + return t(($) => $.activity.squad_leader_evaluated); + } + } default: return entry.action ?? ""; } @@ -616,13 +635,16 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr // Coalesce consecutive activities from the same actor + action. // - task_completed / task_failed: no time limit (these repeat across runs) // - all other actions: within a 2-minute window + // - squad_leader_evaluated: never coalesce; outcome/reason are audit data const COALESCE_MS = 2 * 60 * 1000; const NO_TIME_LIMIT_ACTIONS = new Set(["task_completed", "task_failed"]); + const NEVER_COALESCE_ACTIONS = new Set(["squad_leader_evaluated"]); const coalesced: TimelineEntry[] = []; for (const entry of topLevel) { if (entry.type === "activity") { const prev = coalesced[coalesced.length - 1]; if ( + !NEVER_COALESCE_ACTIONS.has(entry.action!) && prev?.type === "activity" && prev.action === entry.action && prev.actor_type === entry.actor_type && diff --git a/packages/views/locales/en/issues.json b/packages/views/locales/en/issues.json index 7b6e60e9f..de223bd2f 100644 --- a/packages/views/locales/en/issues.json +++ b/packages/views/locales/en/issues.json @@ -162,6 +162,13 @@ "task_completed_other": "completed the task ({{count}} times)", "task_failed_one": "task failed", "task_failed_other": "task failed ({{count}} times)", + "squad_leader_evaluated": "evaluated the squad trigger", + "squad_leader_action": "evaluated and took action", + "squad_leader_action_reason": "evaluated and took action: {{reason}}", + "squad_leader_no_action": "evaluated: no action needed", + "squad_leader_no_action_reason": "evaluated: no action needed ({{reason}})", + "squad_leader_failed": "evaluation failed", + "squad_leader_failed_reason": "evaluation failed: {{reason}}", "coalesced_badge": "×{{count}}" }, "comment": { diff --git a/packages/views/locales/zh-Hans/issues.json b/packages/views/locales/zh-Hans/issues.json index d781be816..630271444 100644 --- a/packages/views/locales/zh-Hans/issues.json +++ b/packages/views/locales/zh-Hans/issues.json @@ -161,6 +161,13 @@ "task_completed_other": "完成了 task（{{count}} 次）", "task_failed_one": "task 失败", "task_failed_other": "task 失败（{{count}} 次）", + "squad_leader_evaluated": "评估了小队触发", + "squad_leader_action": "已评估并采取了操作", + "squad_leader_action_reason": "已评估并采取了操作：{{reason}}", + "squad_leader_no_action": "已评估：无需操作", + "squad_leader_no_action_reason": "已评估：无需操作（{{reason}}）", + "squad_leader_failed": "评估失败", + "squad_leader_failed_reason": "评估失败：{{reason}}", "coalesced_badge": "×{{count}}" }, "comment": { diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index 08cc824dc..736c1b8fa 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -11,6 +11,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "github.com/multica-ai/multica/server/internal/logger" "github.com/multica-ai/multica/server/internal/mention" + "github.com/multica-ai/multica/server/internal/service" "github.com/multica-ai/multica/server/internal/util" db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/protocol" @@ -196,10 +197,23 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { taskUUID, parseErr := util.ParseUUID(taskIDHeader) if parseErr == nil { task, err := h.Queries.GetAgentTask(r.Context(), taskUUID) - if err == nil && task.TriggerCommentID.Valid && uuidToString(task.IssueID) == uuidToString(issue.ID) { - if uuidToString(parentID) != uuidToString(task.TriggerCommentID) { - writeError(w, http.StatusConflict, - "parent_id must equal this task's trigger comment id ("+uuidToString(task.TriggerCommentID)+")") + if err == nil && task.IssueID.Valid && uuidToString(task.IssueID) == uuidToString(issue.ID) { + if task.TriggerCommentID.Valid { + if uuidToString(parentID) != uuidToString(task.TriggerCommentID) { + writeError(w, http.StatusConflict, + "parent_id must equal this task's trigger comment id ("+uuidToString(task.TriggerCommentID)+")") + return + } + } + noAction, checkErr := service.HasSquadLeaderNoActionEvaluationForTask(r.Context(), h.Queries, task) + if checkErr != nil { + slog.Warn("checking squad leader no_action evaluation failed", append(logger.RequestAttrs(r), + "error", checkErr, + "task_id", taskIDHeader, + "issue_id", issueID, + )...) + } else if noAction { + writeError(w, http.StatusConflict, "squad leader recorded no_action; comments are not allowed for this task") return } } diff --git a/server/internal/handler/squad.go b/server/internal/handler/squad.go index a65bee6eb..f3a481f6e 100644 --- a/server/internal/handler/squad.go +++ b/server/internal/handler/squad.go @@ -546,8 +546,20 @@ func (h *Handler) RecordSquadLeaderEvaluation(w http.ResponseWriter, r *http.Req return } + taskID := r.Header.Get("X-Task-ID") + taskUUID, ok := parseUUIDOrBadRequest(w, taskID, "task id") + if !ok { + return + } + task, err := h.Queries.GetAgentTask(r.Context(), taskUUID) + if err != nil || !task.IssueID.Valid || uuidToString(task.IssueID) != uuidToString(issue.ID) { + writeError(w, http.StatusBadRequest, "task does not belong to issue") + return + } + details, _ := json.Marshal(map[string]string{ "squad_id": uuidToString(squad.ID), + "task_id": util.UUIDToString(taskUUID), "outcome": req.Outcome, "reason": req.Reason, }) diff --git a/server/internal/handler/squad_no_action_test.go b/server/internal/handler/squad_no_action_test.go new file mode 100644 index 000000000..56bd92031 --- /dev/null +++ b/server/internal/handler/squad_no_action_test.go @@ -0,0 +1,196 @@ +package handler + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" +) + +type runningSquadLeaderTaskFixture struct { + IssueID string + LeaderID string + TaskID string + TriggerCommentID string +} + +func newRunningSquadLeaderTaskFixture(t *testing.T) runningSquadLeaderTaskFixture { + t.Helper() + ctx := context.Background() + + fx := newSquadCommentTriggerFixture(t) + issueID := uuidToString(fx.Issue.ID) + + var runtimeID string + if err := testPool.QueryRow(ctx, ` + SELECT runtime_id FROM agent WHERE id = $1 + `, fx.LeaderID).Scan(&runtimeID); err != nil { + t.Fatalf("load leader runtime: %v", err) + } + + var triggerCommentID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type) + VALUES ($1, $2, 'member', $3, 'LGTM', 'comment') + RETURNING id + `, issueID, testWorkspaceID, testUserID).Scan(&triggerCommentID); err != nil { + t.Fatalf("create trigger comment: %v", err) + } + + var taskID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO agent_task_queue ( + agent_id, runtime_id, issue_id, trigger_comment_id, + status, priority, started_at + ) + VALUES ($1, $2, $3, $4, 'running', 0, now()) + RETURNING id + `, fx.LeaderID, runtimeID, issueID, triggerCommentID).Scan(&taskID); err != nil { + t.Fatalf("create running squad leader task: %v", err) + } + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, taskID) + }) + + return runningSquadLeaderTaskFixture{ + IssueID: issueID, + LeaderID: fx.LeaderID, + TaskID: taskID, + TriggerCommentID: triggerCommentID, + } +} + +func recordSquadLeaderEvaluationForTask(t *testing.T, fx runningSquadLeaderTaskFixture, outcome string) { + t.Helper() + recordSquadLeaderEvaluationForTaskWithHeader(t, fx, outcome, fx.TaskID) +} + +func recordSquadLeaderEvaluationForTaskWithHeader(t *testing.T, fx runningSquadLeaderTaskFixture, outcome, taskIDHeader string) { + t.Helper() + + w := httptest.NewRecorder() + r := newRequest("POST", "/api/issues/"+fx.IssueID+"/squad-evaluated", map[string]any{ + "outcome": outcome, + "reason": "test reason", + }) + r = withURLParam(r, "id", fx.IssueID) + r.Header.Set("X-Agent-ID", fx.LeaderID) + r.Header.Set("X-Task-ID", taskIDHeader) + + testHandler.RecordSquadLeaderEvaluation(w, r) + if w.Code != http.StatusCreated { + t.Fatalf("RecordSquadLeaderEvaluation: expected 201, got %d: %s", w.Code, w.Body.String()) + } +} + +func completeRunningTask(t *testing.T, fx runningSquadLeaderTaskFixture, output string) { + t.Helper() + + w := httptest.NewRecorder() + r := newDaemonTokenRequest("POST", "/api/daemon/tasks/"+fx.TaskID+"/complete", + map[string]any{"output": output}, + testWorkspaceID, "legit-daemon") + rctx := chi.NewRouteContext() + rctx.URLParams.Add("taskId", fx.TaskID) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + + testHandler.CompleteTask(w, r) + if w.Code != http.StatusOK { + t.Fatalf("CompleteTask: expected 200, got %d: %s", w.Code, w.Body.String()) + } +} + +func countAgentCommentsForIssue(t *testing.T, issueID, agentID string) int { + t.Helper() + var count int + if err := testPool.QueryRow(context.Background(), ` + SELECT count(*) FROM comment + WHERE issue_id = $1 AND author_type = 'agent' AND author_id = $2 + `, issueID, agentID).Scan(&count); err != nil { + t.Fatalf("count agent comments: %v", err) + } + return count +} + +func TestCompleteTask_SquadLeaderNoActionDoesNotSynthesizeComment(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + + fx := newRunningSquadLeaderTaskFixture(t) + recordSquadLeaderEvaluationForTask(t, fx, "no_action") + + completeRunningTask(t, fx, "No action needed. Exiting silently.") + + if got := countAgentCommentsForIssue(t, fx.IssueID, fx.LeaderID); got != 0 { + t.Fatalf("expected no squad leader comment after no_action completion, got %d", got) + } +} + +func TestCompleteTask_SquadLeaderNoActionCanonicalizesTaskID(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + + fx := newRunningSquadLeaderTaskFixture(t) + recordSquadLeaderEvaluationForTaskWithHeader(t, fx, "no_action", strings.ToUpper(fx.TaskID)) + + completeRunningTask(t, fx, "No action needed. Exiting silently.") + + if got := countAgentCommentsForIssue(t, fx.IssueID, fx.LeaderID); got != 0 { + t.Fatalf("expected no comment when no_action was recorded with uppercase task id header, got %d", got) + } +} + +func TestCompleteTask_SquadLeaderActionStillSynthesizesComment(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + + fx := newRunningSquadLeaderTaskFixture(t) + recordSquadLeaderEvaluationForTask(t, fx, "action") + + completeRunningTask(t, fx, "Delegated the review.") + + if got := countAgentCommentsForIssue(t, fx.IssueID, fx.LeaderID); got != 1 { + t.Fatalf("expected action completion to synthesize one comment, got %d", got) + } +} + +func TestCreateComment_SquadLeaderNoActionRejectsComment(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + + fx := newRunningSquadLeaderTaskFixture(t) + recordSquadLeaderEvaluationForTask(t, fx, "no_action") + + w := httptest.NewRecorder() + r := newRequest("POST", "/api/issues/"+fx.IssueID+"/comments", map[string]any{ + "content": "No action needed.", + "parent_id": fx.TriggerCommentID, + }) + r = withURLParam(r, "id", fx.IssueID) + r.Header.Set("X-Agent-ID", fx.LeaderID) + r.Header.Set("X-Task-ID", fx.TaskID) + + testHandler.CreateComment(w, r) + if w.Code != http.StatusConflict { + t.Fatalf("CreateComment: expected 409, got %d: %s", w.Code, w.Body.String()) + } + if got := countAgentCommentsForIssue(t, fx.IssueID, fx.LeaderID); got != 0 { + t.Fatalf("expected rejected no_action comment not to be stored, got %d", got) + } + + var body map[string]any + if err := json.NewDecoder(w.Body).Decode(&body); err != nil { + t.Fatalf("decode error response: %v", err) + } + if body["error"] == "" { + t.Fatalf("expected error message in response, got %v", body) + } +} diff --git a/server/internal/service/squad_no_action.go b/server/internal/service/squad_no_action.go new file mode 100644 index 000000000..f8a06552b --- /dev/null +++ b/server/internal/service/squad_no_action.go @@ -0,0 +1,21 @@ +package service + +import ( + "context" + + "github.com/multica-ai/multica/server/internal/util" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// HasSquadLeaderNoActionEvaluationForTask reports whether this exact task +// already recorded a squad leader no_action evaluation. +func HasSquadLeaderNoActionEvaluationForTask(ctx context.Context, q *db.Queries, task db.AgentTaskQueue) (bool, error) { + if q == nil || !task.ID.Valid || !task.IssueID.Valid || !task.AgentID.Valid { + return false, nil + } + return q.HasSquadLeaderNoActionEvaluationForTask(ctx, db.HasSquadLeaderNoActionEvaluationForTaskParams{ + IssueID: task.IssueID, + AgentID: task.AgentID, + TaskID: util.UUIDToString(task.ID), + }) +} diff --git a/server/internal/service/task.go b/server/internal/service/task.go index 58059c844..839cb4556 100644 --- a/server/internal/service/task.go +++ b/server/internal/service/task.go @@ -995,12 +995,21 @@ func (s *TaskService) CompleteTask(ctx context.Context, taskID pgtype.UUID, resu // for assignment-triggered tasks it is NULL and the fallback is top-level. // Chat tasks have no IssueID and are handled separately below. if task.IssueID.Valid { + suppressNoActionComment, err := HasSquadLeaderNoActionEvaluationForTask(ctx, s.Queries, task) + if err != nil { + slog.Warn("checking squad leader no_action evaluation failed", + "task_id", util.UUIDToString(task.ID), + "issue_id", util.UUIDToString(task.IssueID), + "agent_id", util.UUIDToString(task.AgentID), + "error", err, + ) + } agentCommented, _ := s.Queries.HasAgentCommentedSince(ctx, db.HasAgentCommentedSinceParams{ IssueID: task.IssueID, AuthorID: task.AgentID, Since: task.StartedAt, }) - if !agentCommented { + if !suppressNoActionComment && !agentCommented { var payload protocol.TaskCompletedPayload if err := json.Unmarshal(result, &payload); err == nil { if payload.Output != "" { diff --git a/server/migrations/089_squad_no_action_activity_index.down.sql b/server/migrations/089_squad_no_action_activity_index.down.sql new file mode 100644 index 000000000..b54fa9eca --- /dev/null +++ b/server/migrations/089_squad_no_action_activity_index.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS idx_activity_log_squad_no_action_task; diff --git a/server/migrations/089_squad_no_action_activity_index.up.sql b/server/migrations/089_squad_no_action_activity_index.up.sql new file mode 100644 index 000000000..caa8b293c --- /dev/null +++ b/server/migrations/089_squad_no_action_activity_index.up.sql @@ -0,0 +1,5 @@ +CREATE INDEX IF NOT EXISTS idx_activity_log_squad_no_action_task + ON activity_log (issue_id, actor_id, ((details->>'task_id'))) + WHERE actor_type = 'agent' + AND action = 'squad_leader_evaluated' + AND details->>'outcome' = 'no_action'; diff --git a/server/pkg/db/generated/activity.sql.go b/server/pkg/db/generated/activity.sql.go index f328a08cb..9e6ed99e4 100644 --- a/server/pkg/db/generated/activity.sql.go +++ b/server/pkg/db/generated/activity.sql.go @@ -118,6 +118,32 @@ func (q *Queries) GetActivity(ctx context.Context, id pgtype.UUID) (ActivityLog, return i, err } +const hasSquadLeaderNoActionEvaluationForTask = `-- name: HasSquadLeaderNoActionEvaluationForTask :one +SELECT EXISTS ( + SELECT 1 + FROM activity_log + WHERE issue_id = $1 + AND actor_type = 'agent' + AND actor_id = $2 + AND action = 'squad_leader_evaluated' + AND details->>'outcome' = 'no_action' + AND details->>'task_id' = $3::text +) AS exists +` + +type HasSquadLeaderNoActionEvaluationForTaskParams struct { + IssueID pgtype.UUID `json:"issue_id"` + AgentID pgtype.UUID `json:"agent_id"` + TaskID string `json:"task_id"` +} + +func (q *Queries) HasSquadLeaderNoActionEvaluationForTask(ctx context.Context, arg HasSquadLeaderNoActionEvaluationForTaskParams) (bool, error) { + row := q.db.QueryRow(ctx, hasSquadLeaderNoActionEvaluationForTask, arg.IssueID, arg.AgentID, arg.TaskID) + var exists bool + err := row.Scan(&exists) + return exists, err +} + const listActivitiesForIssue = `-- name: ListActivitiesForIssue :many SELECT id, workspace_id, issue_id, actor_type, actor_id, action, details, created_at FROM activity_log WHERE issue_id = $1 diff --git a/server/pkg/db/queries/activity.sql b/server/pkg/db/queries/activity.sql index 72448890c..fa796e6b8 100644 --- a/server/pkg/db/queries/activity.sql +++ b/server/pkg/db/queries/activity.sql @@ -16,6 +16,18 @@ INSERT INTO activity_log ( ) VALUES ($1, $2, $3, $4, $5, $6) RETURNING *; +-- name: HasSquadLeaderNoActionEvaluationForTask :one +SELECT EXISTS ( + SELECT 1 + FROM activity_log + WHERE issue_id = @issue_id + AND actor_type = 'agent' + AND actor_id = @agent_id + AND action = 'squad_leader_evaluated' + AND details->>'outcome' = 'no_action' + AND details->>'task_id' = @task_id::text +) AS exists; + -- name: CountAssigneeChangesByActor :many -- Count how many times a user assigned each target via assignee_changed activities. SELECT