fix(comments): honest role + status in trigger_outcomes fan-out (MUL-4525)

Addresses Elon's round-3 review of the §2 fan-out.

1. Execution merge preserves the squad-leader role instead of first-mention-
   wins. The dedup now UPGRADES an already-added plain @agent trigger to a
   @squad-leader trigger for the same agent, so @Agent A + @Squad S(leader=A)
   always runs as a leader task (is_leader_task + squad_id=S) regardless of
   mention order — the daemon still injects S's briefing. When two DIFFERENT
   squads share one leader, the single run carries one squad's context and the
   other squad is reported `coalesced` (folded), never a second `queued`. The
   enqueue result now records the executed squad so the fan-out can tell them
   apart. Tests assert the task role in both orders and the two-squad split.

2. Squad-leader self-suppression no longer fakes success. The self-trigger
   guard keys on the latest task ROLE with no status filter, so a long-completed
   task also suppresses; reporting `deferred/already_active` when nothing is
   active was a false success. The branch now reports `deferred` only when a
   real non-terminal task is active (its reconcile covers the comment), else a
   non-success `blocked` + new `already_handled` reason. Fixed the reversed
   helper-semantics comment. New handler test covers the completed-task branch.

Backend handler+service suites and go vet green.

Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
J
2026-07-14 21:21:18 +08:00
parent 7b03baa377
commit 507cc23c4f
4 changed files with 218 additions and 39 deletions

View File

@@ -36,6 +36,12 @@ const (
// ReasonAlreadyActive: a run is already active/pending for this target and
// this trigger did not coalesce.
ReasonAlreadyActive ReasonCode = "already_active"
// ReasonAlreadyHandled: the target was intentionally not (re-)triggered
// because the acting agent's own prior activity already covered it and no
// active run remains — e.g. a squad leader's self-@mention that the
// self-trigger guard suppresses with no active task left. Not a permission
// block, but NOT success: nothing new runs.
ReasonAlreadyHandled ReasonCode = "already_handled"
// ReasonInternalError: an unexpected server error prevented a clean decision.
ReasonInternalError ReasonCode = "internal_error"
)

View File

@@ -58,6 +58,7 @@ const (
ReasonRuntimeOffline = dispatch.ReasonRuntimeOffline
ReasonAttributionBlocked = dispatch.ReasonAttributionBlocked
ReasonAlreadyActive = dispatch.ReasonAlreadyActive
ReasonAlreadyHandled = dispatch.ReasonAlreadyHandled
ReasonInternalError = dispatch.ReasonInternalError
)

View File

@@ -1452,9 +1452,14 @@ func filterSuppressedCommentAgentTriggers(triggers []commentAgentTrigger, suppre
}
// commentEnqueueResult is the domain outcome of enqueuing ONE executing agent.
// execSquadID is the squad whose leader context the run actually carries (set
// only for a squad-leader execution), so a DIFFERENT squad that shares this
// leader can be reported honestly as coalesced rather than as if its own leader
// context ran.
type commentEnqueueResult struct {
status DispatchStatus
reason DispatchReasonCode
status DispatchStatus
reason DispatchReasonCode
execSquadID string
}
// enqueueCommentAgentTriggers enqueues each resolved trigger (already deduped by
@@ -1475,7 +1480,11 @@ func (h *Handler) enqueueCommentAgentTriggers(ctx context.Context, issue db.Issu
}
results := make(map[string]commentEnqueueResult, len(triggers))
record := func(trigger commentAgentTrigger, status DispatchStatus, reason DispatchReasonCode) {
results[uuidToString(trigger.Agent.ID)] = commentEnqueueResult{status: status, reason: reason}
execSquadID := ""
if trigger.Squad != nil {
execSquadID = uuidToString(trigger.Squad.ID)
}
results[uuidToString(trigger.Agent.ID)] = commentEnqueueResult{status: status, reason: reason, execSquadID: execSquadID}
}
for _, trigger := range triggers {
if trigger.AlreadyPending {
@@ -1531,7 +1540,16 @@ func commentTriggerOutcomes(targets []commentMentionTarget, enqueued map[string]
if !ok {
continue
}
outcomes = append(outcomes, CommentTriggerOutcome{TargetType: t.TargetType, TargetID: t.TargetID, Status: res.status, ReasonCode: res.reason})
status, reason := res.status, res.reason
// A @squad whose shared leader ran, but under a DIFFERENT squad's
// context, did not get its own leader briefing injected — the single
// leader run (one task per issue+agent) merely folds it in. Report
// coalesced, not queued, so we never claim this squad's leader
// context executed (MUL-4525, Elon round 3).
if t.TargetType == "squad" && res.execSquadID != "" && res.execSquadID != t.TargetID && status == DispatchQueued {
status, reason = DispatchCoalesced, ReasonCoalesced
}
outcomes = append(outcomes, CommentTriggerOutcome{TargetType: t.TargetType, TargetID: t.TargetID, Status: status, ReasonCode: reason})
continue
}
outcomes = append(outcomes, CommentTriggerOutcome{TargetType: t.TargetType, TargetID: t.TargetID, Status: t.Status, ReasonCode: t.ReasonCode})
@@ -2098,14 +2116,22 @@ func (h *Handler) resolveMentionedAgentCommentTriggers(ctx context.Context, issu
wsID := uuidToString(issue.WorkspaceID)
triggers := make([]commentAgentTrigger, 0, len(mentions))
// seen dedups EXECUTION by resolved agent id: two mentions resolving to the
// same agent still enqueue only one task.
seen := make(map[string]struct{}, len(mentions))
// same agent enqueue only one task. Mapping to the trigger's index lets a
// squad-leader mention UPGRADE an already-added plain @agent trigger for the
// same agent — the leader task is a strict superset (it sets is_leader_task
// and squad_id so the daemon injects the squad briefing), so the merged run
// must carry that role regardless of mention order.
seen := make(map[string]int, len(mentions))
add := func(trigger commentAgentTrigger) {
id := uuidToString(trigger.Agent.ID)
if _, ok := seen[id]; ok {
if idx, ok := seen[id]; ok {
if triggers[idx].Source != commentTriggerSourceMentionSquadLeader &&
trigger.Source == commentTriggerSourceMentionSquadLeader {
triggers[idx] = trigger
}
return
}
seen[id] = struct{}{}
seen[id] = len(triggers)
triggers = append(triggers, trigger)
}
// targets record one outcome per EXPLICIT mention — deduped by the target
@@ -2143,12 +2169,21 @@ func (h *Handler) resolveMentionedAgentCommentTriggers(ctx context.Context, issu
}
leaderID := squad.LeaderID
// A2A self-suppression: the author IS this squad's leader and its
// last activity here was a worker task for this squad, so we do not
// re-fire the leader. This is recognized-and-handled, not a block —
// it reports a definite `deferred` outcome (leader already active).
// most recent task on this issue was a leader/generic role (NOT a
// fresh same-squad worker→leader handoff), so we do not re-fire the
// leader from its own @mention. The outcome must reflect reality, not
// assume success (MUL-4525, Elon round 3): report `deferred` only when
// a real non-terminal task is still active (its completion reconcile
// covers this comment); otherwise the latest task is already terminal
// and nothing runs, so report a non-success `already_handled` — never
// a success-shaped `deferred`.
if authorType == "agent" && authorID == uuidToString(leaderID) &&
h.shouldSuppressSquadLeaderSelfTrigger(ctx, issue.ID, leaderID, squad.ID) {
addTarget(commentMentionTarget{TargetType: "squad", TargetID: m.ID, Status: DispatchDeferred, ReasonCode: ReasonAlreadyActive})
if h.hasActiveTaskForIssueAndAgent(ctx, issue.ID, leaderID) {
addTarget(commentMentionTarget{TargetType: "squad", TargetID: m.ID, Status: DispatchDeferred, ReasonCode: ReasonAlreadyActive})
} else {
addTarget(commentMentionTarget{TargetType: "squad", TargetID: m.ID, Status: DispatchBlocked, ReasonCode: ReasonAlreadyHandled})
}
continue
}
agent, err := h.Queries.GetAgentInWorkspace(ctx, db.GetAgentInWorkspaceParams{

View File

@@ -128,25 +128,96 @@ func TestCreateComment_BlockedMentionReasonDoesNotEnumeratePrivateAgent(t *testi
}
}
// TestCreateComment_AgentAndSameLeaderSquadYieldsOneTaskTwoOutcomes is Elon's
// round-2 must-fix 1 acceptance test: when a comment names BOTH @Agent A and
// @Squad S whose leader is A, the run is correctly coalesced to ONE task, but
// each explicitly-named target still gets its own outcome — execution dedup must
// not drop a named target's result (MUL-4525 §2).
func TestCreateComment_AgentAndSameLeaderSquadYieldsOneTaskTwoOutcomes(t *testing.T) {
// TestCreateComment_AgentAndSameLeaderSquad is Elon's round-3 must-fix 1
// acceptance test: when a comment names BOTH @Agent A and @Squad S whose leader
// is A, the run coalesces to ONE task that carries the LEADER role
// (is_leader_task + squad_id=S, so the daemon injects S's briefing) regardless
// of mention order, and each explicitly-named target still gets its own outcome
// (MUL-4525). The old first-mention-wins dedup could drop the leader role when
// @Agent A came first — this asserts the role independent of order.
func TestCreateComment_AgentAndSameLeaderSquad(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
ctx := context.Background()
cases := []struct {
name string
content func(agentID, squadID string) string
}{
{"agent mention first", func(a, s string) string {
return fmt.Sprintf("[@A](mention://agent/%s) [@S](mention://squad/%s) please", a, s)
}},
{"squad mention first", func(a, s string) string {
return fmt.Sprintf("[@S](mention://squad/%s) [@A](mention://agent/%s) please", s, a)
}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
agentID := createHandlerTestAgent(t, "Shared Leader Agent "+tc.name, nil)
squadID := createCommentTriggerPreviewSquad(t, "Shared Leader Squad "+tc.name, agentID)
issueID := createCommentTriggerPreviewIssue(t, "same-leader "+tc.name, "", "")
w := httptest.NewRecorder()
r := withURLParam(newRequest(http.MethodPost, "/api/issues/"+issueID+"/comments", map[string]any{"content": tc.content(agentID, squadID)}), "id", issueID)
testHandler.CreateComment(w, r)
if w.Code != http.StatusCreated {
t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String())
}
var resp CommentResponse
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode comment: %v", err)
}
// One coalesced execution carrying the leader role for squad S.
var taskCount int
var isLeader bool
var taskSquadID string
if err := testPool.QueryRow(ctx, `
SELECT count(*), COALESCE(bool_or(is_leader_task), false), COALESCE(max(squad_id::text), '')
FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued'
`, issueID, agentID).Scan(&taskCount, &isLeader, &taskSquadID); err != nil {
t.Fatalf("read task role: %v", err)
}
if taskCount != 1 {
t.Fatalf("queued tasks = %d, want 1 (coalesced execution)", taskCount)
}
if !isLeader {
t.Errorf("execution is_leader_task = false, want true (leader role must win regardless of order)")
}
if taskSquadID != squadID {
t.Errorf("execution squad_id = %q, want %q", taskSquadID, squadID)
}
// Two outcomes — one per explicitly-named target — both queued.
if len(resp.TriggerOutcomes) != 2 {
t.Fatalf("trigger_outcomes = %+v, want 2 (agent + squad)", resp.TriggerOutcomes)
}
if o := findCommentOutcome(t, resp.TriggerOutcomes, agentID); o.TargetType != "agent" || o.Status != DispatchQueued {
t.Errorf("agent outcome = %+v, want agent/queued", o)
}
if o := findCommentOutcome(t, resp.TriggerOutcomes, squadID); o.TargetType != "squad" || o.Status != DispatchQueued {
t.Errorf("squad outcome = %+v, want squad/queued", o)
}
})
}
}
// TestCreateComment_TwoSquadsSharingLeaderCoalescesNonWinner is Elon's round-3
// must-fix 1 (multi-squad case): two DIFFERENT squads share the same leader and
// both are @mentioned. The single leader agent runs ONCE carrying one squad's
// context; the other squad's mention folds into that run and is reported
// coalesced — never a second task, and never both reported queued (MUL-4525).
func TestCreateComment_TwoSquadsSharingLeaderCoalescesNonWinner(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
agentID := createHandlerTestAgent(t, "Shared Leader Agent", nil)
// Squad whose leader is the very same agent.
squadID := createCommentTriggerPreviewSquad(t, "Shared Leader Squad", agentID)
issueID := createCommentTriggerPreviewIssue(t, "agent and same-leader squad outcomes", "", "")
content := fmt.Sprintf(
"[@A](mention://agent/%s) [@S](mention://squad/%s) please take a look",
agentID, squadID,
)
leaderID := createHandlerTestAgent(t, "Two-Squad Shared Leader", nil)
squad1 := createCommentTriggerPreviewSquad(t, "Two-Squad S1", leaderID)
squad2 := createCommentTriggerPreviewSquad(t, "Two-Squad S2", leaderID)
issueID := createCommentTriggerPreviewIssue(t, "two squads sharing leader", "", "")
content := fmt.Sprintf("[@S1](mention://squad/%s) [@S2](mention://squad/%s) please", squad1, squad2)
w := httptest.NewRecorder()
r := withURLParam(newRequest(http.MethodPost, "/api/issues/"+issueID+"/comments", map[string]any{"content": content}), "id", issueID)
@@ -159,21 +230,87 @@ func TestCreateComment_AgentAndSameLeaderSquadYieldsOneTaskTwoOutcomes(t *testin
t.Fatalf("decode comment: %v", err)
}
// One coalesced execution: exactly one queued task for the shared leader.
if got := countQueuedCommentTriggerTasks(t, issueID, agentID); got != 1 {
t.Fatalf("shared-leader queued tasks = %d, want 1 (coalesced execution)", got)
// Exactly one queued task (a single leader agent can only run once).
if got := countQueuedCommentTriggerTasks(t, issueID, leaderID); got != 1 {
t.Fatalf("queued tasks = %d, want 1", got)
}
// Two outcomes — one per explicitly-named target — both success-shaped.
// Two squad outcomes; exactly one queued (the executed squad) and one
// coalesced (the folded squad) — never both queued.
if len(resp.TriggerOutcomes) != 2 {
t.Fatalf("trigger_outcomes = %+v, want 2 (agent + squad)", resp.TriggerOutcomes)
t.Fatalf("trigger_outcomes = %+v, want 2", resp.TriggerOutcomes)
}
agentOutcome := findCommentOutcome(t, resp.TriggerOutcomes, agentID)
if agentOutcome.TargetType != "agent" || agentOutcome.Status != DispatchQueued {
t.Errorf("agent outcome = %+v, want agent/queued", agentOutcome)
statuses := map[DispatchStatus]int{}
for _, o := range resp.TriggerOutcomes {
if o.TargetType != "squad" {
t.Errorf("outcome %+v: want squad target", o)
}
statuses[o.Status]++
}
squadOutcome := findCommentOutcome(t, resp.TriggerOutcomes, squadID)
if squadOutcome.TargetType != "squad" || squadOutcome.Status != DispatchQueued {
t.Errorf("squad outcome = %+v, want squad/queued", squadOutcome)
if statuses[DispatchQueued] != 1 || statuses[DispatchCoalesced] != 1 {
t.Errorf("outcome statuses = %v, want exactly 1 queued + 1 coalesced (not both queued)", statuses)
}
}
// TestCreateComment_SquadLeaderSelfMentionCompletedTaskDoesNotFakeSuccess is
// Elon's round-3 must-fix 2: a squad leader's own @mention of its squad is
// suppressed by the self-trigger guard, but when its latest task is already
// TERMINAL (no active run), the outcome must NOT be a success-shaped `deferred`
// — it is a non-success `blocked/already_handled`, and no new task is enqueued.
func TestCreateComment_SquadLeaderSelfMentionCompletedTaskDoesNotFakeSuccess(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
ctx := context.Background()
leaderID := createHandlerTestAgent(t, "Self-Mention Leader", nil)
squadID := createCommentTriggerPreviewSquad(t, "Self-Mention Squad", leaderID)
issueID := createCommentTriggerPreviewIssue(t, "self-mention completed task", "", "")
// The leader's latest task on this issue is a COMPLETED leader task: the
// self-trigger guard suppresses (latest role is leader) AND there is no
// active run to cover the comment.
var completedTaskID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent_task_queue (agent_id, runtime_id, status, priority, issue_id, is_leader_task, squad_id, started_at, completed_at)
VALUES ($1, $2, 'completed', 0, $3, true, $4, now(), now())
RETURNING id
`, leaderID, handlerTestRuntimeID(t), issueID, squadID).Scan(&completedTaskID); err != nil {
t.Fatalf("seed completed leader task: %v", err)
}
content := fmt.Sprintf("[@S](mention://squad/%s) revisit please", squadID)
w := httptest.NewRecorder()
r := withURLParam(newRequest(http.MethodPost, "/api/issues/"+issueID+"/comments", map[string]any{"content": content}), "id", issueID)
// Author the comment AS the leader agent (A2A self-mention).
r.Header.Set("X-Agent-ID", leaderID)
r.Header.Set("X-Task-ID", completedTaskID)
testHandler.CreateComment(w, r)
if w.Code != http.StatusCreated {
t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String())
}
var resp CommentResponse
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode comment: %v", err)
}
// The self-mention neither re-fired the leader nor is covered by an active
// run: the outcome is non-success, never a fake `deferred`.
if len(resp.TriggerOutcomes) != 1 {
t.Fatalf("trigger_outcomes = %+v, want 1", resp.TriggerOutcomes)
}
o := resp.TriggerOutcomes[0]
if o.TargetType != "squad" || o.TargetID != squadID {
t.Fatalf("outcome target = %+v, want squad %s", o, squadID)
}
if o.Status != DispatchBlocked || o.ReasonCode != ReasonAlreadyHandled {
t.Errorf("outcome = %+v, want blocked/already_handled (must not fake success)", o)
}
// No new task was enqueued (only the pre-seeded completed one exists).
var total int
if err := testPool.QueryRow(ctx, `SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2`, issueID, leaderID).Scan(&total); err != nil {
t.Fatalf("count leader tasks: %v", err)
}
if total != 1 {
t.Errorf("leader tasks = %d, want 1 (self-mention suppressed, no new task)", total)
}
}