diff --git a/server/internal/handler/agent_access.go b/server/internal/handler/agent_access.go index de18f45443d..bb82a971e56 100644 --- a/server/internal/handler/agent_access.go +++ b/server/internal/handler/agent_access.go @@ -332,6 +332,17 @@ func (h *Handler) taskFromRequestHeader(r *http.Request) (db.AgentTaskQueue, boo return task, true } +func (h *Handler) authoringTaskIDFromRequest(r *http.Request, actorType, actorID string) pgtype.UUID { + if actorType != "agent" { + return pgtype.UUID{} + } + task, ok := h.taskFromRequestHeader(r) + if !ok || !task.AgentID.Valid || uuidToString(task.AgentID) != actorID { + return pgtype.UUID{} + } + return task.ID +} + // accessibleAgentIDs returns the set of agent IDs in the workspace the actor // is allowed to see, for use by workspace-wide aggregation endpoints // (run counts, activity histograms, task snapshots) that need to filter out diff --git a/server/internal/handler/agent_access_test.go b/server/internal/handler/agent_access_test.go index 8aa631c7692..0be429410ef 100644 --- a/server/internal/handler/agent_access_test.go +++ b/server/internal/handler/agent_access_test.go @@ -490,7 +490,7 @@ func TestMentionAgent_RejectsCrossWorkspaceAgentUUID(t *testing.T) { t.Fatalf("count tasks before: %v", err) } - enqueueMentionedAgentTasksForTest(t, ctx, issue, comment, nil, "member", testUserID) + enqueueMentionedAgentTasksForTest(t, ctx, issue, comment, nil, "member", testUserID, commentTriggerComputeOptions{}) var afterCount int if err := testPool.QueryRow(ctx, @@ -587,7 +587,7 @@ func TestShouldEnqueueOnComment_PrivateAgentGate(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - got := testHandler.shouldEnqueueAssigneeFallback(ctx, issue, tc.actorType, tc.actorID, commentTriggerComputeOptions{}) + got := testHandler.shouldEnqueueAssigneeFallback(ctx, issue, tc.actorType, tc.actorID, commentTriggerComputeOptions{}) if got != tc.want { t.Fatalf("%s\n actor=%s/%s got=%v want=%v", tc.reason, tc.actorType, tc.actorID, got, tc.want) diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index 0ffe9caa987..9f64d083ccc 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -1527,6 +1527,10 @@ type commentAgentTrigger struct { type commentTriggerComputeOptions struct { ExcludeTriggerCommentID pgtype.UUID + // AuthoringTaskID identifies the server-trusted task that produced this + // comment. Unlike comment.source_task_id it may belong to another issue: that + // distinction is required to preserve explicit child→parent self-handoffs. + AuthoringTaskID pgtype.UUID // OriginatorUserID is the top-of-chain human user id for this trigger // (MUL-3963). Only consulted for AGENT actors — canInvokeAgent judges A2A // by the originator, not the immediate agent principal. Members are their @@ -1661,6 +1665,7 @@ func (h *Handler) PreviewCommentTriggers(w http.ResponseWriter, r *http.Request) } actorType, actorID := h.resolveActor(r, userID, uuidToString(issue.WorkspaceID)) + opts.AuthoringTaskID = h.authoringTaskIDFromRequest(r, actorType, actorID) opts.OriginatorUserID = h.invokeOriginatorFromRequest(r, actorType, actorID) opts.AutopilotDelegationAuthorityUserID = h.autopilotDelegationAuthorityFromRequest(r, issue, actorType, actorID) triggers, targets := h.computeCommentAgentTriggers(r.Context(), issue, content, parentComment, actorType, actorID, opts) @@ -1795,12 +1800,13 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { // worker's task originator is unattributed, effectiveUser resolves to "", // and the private-agent gate denies the wake (MUL-4015). var sourceTaskID pgtype.UUID + authoringTaskID := h.authoringTaskIDFromRequest(r, authorType, authorID) if authorType == "agent" { if taskIDHeader := r.Header.Get("X-Task-ID"); taskIDHeader != "" { taskUUID, parseErr := util.ParseUUID(taskIDHeader) if parseErr == nil { task, err := h.Queries.GetAgentTask(r.Context(), taskUUID) - if err == nil && task.IssueID.Valid && uuidToString(task.IssueID) == uuidToString(issue.ID) { + if err == nil && task.AgentID.Valid && uuidToString(task.AgentID) == authorID && task.IssueID.Valid && uuidToString(task.IssueID) == uuidToString(issue.ID) { if task.TriggerCommentID.Valid { if !taskCoversReplyParent(task, parentID) { // Keep this error actionable for agents (MUL-4417 / GH #5266). @@ -1902,7 +1908,7 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { // The comment is already saved; a blocked mention must not fail the whole // request. Surface the per-target outcomes so the client can show partial // success instead of a silent no-op (MUL-4525 §2). - resp.TriggerOutcomes = h.triggerTasksForComment(r.Context(), issue, comment, parentComment, authorType, authorID, originatorUserID, delegationAuthority, suppressAgentIDs) + resp.TriggerOutcomes = h.triggerTasksForComment(r.Context(), issue, comment, parentComment, authorType, authorID, originatorUserID, delegationAuthority, authoringTaskID, suppressAgentIDs) writeJSON(w, http.StatusCreated, resp) } @@ -1943,12 +1949,13 @@ func isNoteComment(content string) bool { // (MUL-4525 §2): blocked mentions from resolution plus queued / coalesced / // deferred / blocked from enqueue. UI-suppressed triggers (the user unchecked // them) are removed before enqueue and produce no outcome. -func (h *Handler) triggerTasksForComment(ctx context.Context, issue db.Issue, comment db.Comment, parentComment *db.Comment, actorType, actorID, originatorUserID, delegationAuthorityUserID string, suppressAgentIDs []pgtype.UUID) []CommentTriggerOutcome { +func (h *Handler) triggerTasksForComment(ctx context.Context, issue db.Issue, comment db.Comment, parentComment *db.Comment, actorType, actorID, originatorUserID, delegationAuthorityUserID string, authoringTaskID pgtype.UUID, suppressAgentIDs []pgtype.UUID) []CommentTriggerOutcome { if isNoteComment(comment.Content) { return nil } triggers, targets := h.computeCommentAgentTriggers(ctx, issue, comment.Content, parentComment, actorType, actorID, commentTriggerComputeOptions{ ExcludeTriggerCommentID: comment.ID, + AuthoringTaskID: authoringTaskID, OriginatorUserID: originatorUserID, AutopilotDelegationAuthorityUserID: delegationAuthorityUserID, }) @@ -2870,7 +2877,7 @@ func (h *Handler) routeAssignedSquadLeaderFallback(ctx context.Context, issue db return commentAgentTrigger{}, false } if authorType == "agent" && authorID == uuidToString(squad.LeaderID) && - h.shouldSuppressSquadLeaderSelfTrigger(ctx, issue.ID, squad.LeaderID, squad.ID) { + !h.allowsSquadWorkerToLeaderHandoff(ctx, issue.ID, squad.ID, opts.AuthoringTaskID) { return commentAgentTrigger{}, false } agent, err := h.Queries.GetAgentInWorkspace(ctx, db.GetAgentInWorkspaceParams{ @@ -2890,6 +2897,35 @@ func (h *Handler) routeAssignedSquadLeaderFallback(ctx context.Context, issue db return commentAgentTrigger{Agent: agent, Source: commentTriggerSourceIssueAssignee, Squad: &squad, AlreadyPending: hasPending}, true } +// allowsSquadWorkerToLeaderHandoff accepts only the exact task that authored +// the comment. This preserves a dual-role leader's worker→leader coordination +// resume without letting unrelated historical worker tasks disable loop safety. +func (h *Handler) allowsSquadWorkerToLeaderHandoff(ctx context.Context, issueID, squadID, taskID pgtype.UUID) bool { + if !taskID.Valid { + return false + } + task, err := h.Queries.GetAgentTask(ctx, taskID) + return err == nil && task.IssueID.Valid && uuidToString(task.IssueID) == uuidToString(issueID) && + !task.IsLeaderTask && task.SquadID.Valid && uuidToString(task.SquadID) == uuidToString(squadID) +} + +// allowsExplicitSelfHandoff distinguishes intentional role/context transitions +// from a result comment re-enqueueing its own run. Cross-issue transitions are +// explicit-only; same-issue transitions require exact same-squad worker lineage. +func (h *Handler) allowsExplicitSelfHandoff(ctx context.Context, issueID pgtype.UUID, squadID pgtype.UUID, taskID pgtype.UUID) bool { + if !taskID.Valid { + return false + } + task, err := h.Queries.GetAgentTask(ctx, taskID) + if err != nil || !task.IssueID.Valid { + return false + } + if uuidToString(task.IssueID) != uuidToString(issueID) { + return true + } + return squadID.Valid && !task.IsLeaderTask && task.SquadID.Valid && uuidToString(task.SquadID) == uuidToString(squadID) +} + func (h *Handler) hasPendingTaskForIssueAndAgent(ctx context.Context, issueID, agentID pgtype.UUID, opts commentTriggerComputeOptions) (bool, error) { // Key dedup on the reviewed head so re-pushing to the PR mid-review // invalidates dedup and a fresh run enqueues against the new HEAD (TEN-356). @@ -2913,11 +2949,9 @@ func (h *Handler) hasPendingTaskForIssueAndAgent(ctx context.Context, issueID, a // mentions from the current comment and returns the runnable agent recipients. // Skips agents with on_mention trigger disabled, and private agents mentioned // by non-owner members (only the agent owner or workspace admin/owner can -// mention a private agent). Self-mentions are intentionally allowed so an -// agent running in one issue can explicitly enqueue itself on another (e.g. -// a child-issue run notifying the parent issue whose assignee is the same -// agent); runaway loops are prevented by HasPendingTaskForIssueAndAgent -// dedupe and the natural queued/dispatched coalescing of the task queue. +// mention a private agent). Same-context self-mentions are suppressed; an +// explicit self-mention from a trusted task on another issue remains a valid +// child→parent handoff. // Note: no issue status gate here — @mention is an explicit action and should // work even on done/cancelled issues (the agent can reopen the issue if needed). // commentMentionTarget is one EXPLICIT @agent / @squad mention and how it @@ -3005,16 +3039,14 @@ func (h *Handler) resolveMentionedAgentCommentTriggers(ctx context.Context, issu continue } leaderID := squad.LeaderID - // A2A self-suppression: the author IS this squad's leader and its - // 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 + // A2A self-suppression: the author IS this squad's leader, so we do + // not re-fire the leader from its own @mention. The outcome must reflect reality, not // assume success (MUL-4525): `deferred` only when a real non-terminal // task is still active (its reconcile covers this comment); a query // failure is a non-success internal_error, never a fabricated // deferred; otherwise nothing runs → self_trigger_suppressed. if authorType == "agent" && authorID == uuidToString(leaderID) && - h.shouldSuppressSquadLeaderSelfTrigger(ctx, issue.ID, leaderID, squad.ID) { + !h.allowsExplicitSelfHandoff(ctx, issue.ID, squad.ID, opts.AuthoringTaskID) { active, activeErr := h.hasActiveTaskForIssueAndAgent(ctx, issue.ID, leaderID) status, reason := decideSuppressedLeaderOutcome(active, activeErr) addTarget(commentMentionTarget{TargetType: "squad", TargetID: m.ID, Status: status, ReasonCode: reason}) @@ -3066,6 +3098,16 @@ func (h *Handler) resolveMentionedAgentCommentTriggers(ctx context.Context, issu blockTarget("agent", m.ID, ReasonTargetUnavailable) continue } + // Same-context explicit self-mentions are not new delegations: enqueueing + // the author again lets a result comment create an unbounded loop. A + // trusted task on another issue remains a supported explicit handoff. + if authorType == "agent" && authorID == uuidToString(agentUUID) && + !h.allowsExplicitSelfHandoff(ctx, issue.ID, pgtype.UUID{}, opts.AuthoringTaskID) { + active, activeErr := h.hasActiveTaskForIssueAndAgent(ctx, issue.ID, agentUUID) + status, reason := decideSuppressedLeaderOutcome(active, activeErr) + addTarget(commentMentionTarget{TargetType: "agent", TargetID: m.ID, Status: status, ReasonCode: reason}) + continue + } // Load the agent scoped to the current issue's workspace. Using the // bare GetAgent here would let a mention resolve to an agent in a // different workspace, and the visibility check below would then be @@ -3256,7 +3298,7 @@ func (h *Handler) UpdateComment(w http.ResponseWriter, r *http.Request) { // or non-author edit left it NULL, so this fails closed rather than borrowing // the old authoring run's authority. delegationAuthority := h.autopilotDelegationAuthorityFromComment(r.Context(), issue, comment) - return h.triggerTasksForComment(r.Context(), issue, comment, parentComment, actorType, actorID, h.invokeOriginatorFromRequest(r, actorType, actorID), delegationAuthority, suppressAgentIDs) + return h.triggerTasksForComment(r.Context(), issue, comment, parentComment, actorType, actorID, h.invokeOriginatorFromRequest(r, actorType, actorID), delegationAuthority, h.authoringTaskIDFromRequest(r, actorType, actorID), suppressAgentIDs) } // Replace the comment attachment set when a modern client sends @@ -3449,6 +3491,7 @@ func (h *Handler) retriggerCancelledTaskSurvivors(ctx context.Context, issue db. } triggers, _ := h.computeCommentAgentTriggers(ctx, issue, comment.Content, parentComment, actorType, actorID, commentTriggerComputeOptions{ ExcludeTriggerCommentID: comment.ID, + AuthoringTaskID: comment.SourceTaskID, OriginatorUserID: originatorUserID, AutopilotDelegationAuthorityUserID: delegationAuthority, }) diff --git a/server/internal/handler/comment_reconcile_test.go b/server/internal/handler/comment_reconcile_test.go index efc89b668b0..8457f785998 100644 --- a/server/internal/handler/comment_reconcile_test.go +++ b/server/internal/handler/comment_reconcile_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5/pgtype" "github.com/multica-ai/multica/server/internal/util" db "github.com/multica-ai/multica/server/pkg/db/generated" ) @@ -348,7 +349,7 @@ func TestCompleteTask_ReconcilesAgentAuthoredMentionToCompletedAgent(t *testing. if err != nil { t.Fatalf("setup: load mention comment: %v", err) } - testHandler.triggerTasksForComment(ctx, issue, mentionComment, nil, "agent", agentA, "", "", nil) + testHandler.triggerTasksForComment(ctx, issue, mentionComment, nil, "agent", agentA, "", "", pgtype.UUID{}, nil) // Drop happened: the mention found no queued task to merge into and an // active (dispatched) task exists, so NO fresh queued follow-up was created. @@ -583,14 +584,14 @@ func TestConsecutiveCommentsDifferentOriginatorsFullEnqueuePath(t *testing.T) { // A's comment → creates the queued task (originator A). cA := insertMemberComment(testUserID, "first, from A") - testHandler.triggerTasksForComment(ctx, issue, cA, nil, "member", testUserID, testUserID, "", nil) + testHandler.triggerTasksForComment(ctx, issue, cA, nil, "member", testUserID, testUserID, "", pgtype.UUID{}, nil) if n := pendingTaskCountForAgentIssue(t, issueID, agentID); n != 1 { t.Fatalf("after A's comment expected exactly 1 queued task, got %d", n) } // B's comment (different originator) before start → must fold in, NOT drop. cB := insertMemberComment(userB, "second, from B — different user") - testHandler.triggerTasksForComment(ctx, issue, cB, nil, "member", userB, userB, "", nil) + testHandler.triggerTasksForComment(ctx, issue, cB, nil, "member", userB, userB, "", pgtype.UUID{}, nil) // Still exactly one task (bounded concurrency, no unique-index collision). if n := pendingTaskCountForAgentIssue(t, issueID, agentID); n != 1 { diff --git a/server/internal/handler/daemon.go b/server/internal/handler/daemon.go index 44d0460a548..5b15b000e6d 100644 --- a/server/internal/handler/daemon.go +++ b/server/internal/handler/daemon.go @@ -3161,6 +3161,7 @@ func (h *Handler) reconcileCommentsOnCompletion(ctx context.Context, task *db.Ag } triggers, _ := h.computeCommentAgentTriggers(ctx, issue, c.Content, parentComment, actorType, actorID, commentTriggerComputeOptions{ ExcludeTriggerCommentID: c.ID, + AuthoringTaskID: c.SourceTaskID, OriginatorUserID: originatorUserID, AutopilotDelegationAuthorityUserID: delegationAuthority, }) diff --git a/server/internal/handler/mention_self_trigger_test.go b/server/internal/handler/mention_self_trigger_test.go index 93c475f91c8..467913913f2 100644 --- a/server/internal/handler/mention_self_trigger_test.go +++ b/server/internal/handler/mention_self_trigger_test.go @@ -4,29 +4,48 @@ import ( "context" "testing" + "github.com/jackc/pgx/v5/pgtype" "github.com/multica-ai/multica/server/internal/util" db "github.com/multica-ai/multica/server/pkg/db/generated" ) +// A member mention is informational and must not fall through to either +// assignee route when the author is that assignee (or its squad leader). +// This is database-free because member mentions stop at the trigger boundary. +func TestComputeCommentAgentTriggers_AssigneeAuthorMemberMentionIsNoop(t *testing.T) { + h := &Handler{} + memberMention := "decision needed from [@Dmitry](mention://member/22222222-2222-2222-2222-222222222222)" + + for _, assigneeType := range []string{"agent", "squad"} { + t.Run(assigneeType, func(t *testing.T) { + issue := db.Issue{ + AssigneeType: pgtype.Text{String: assigneeType, Valid: true}, + AssigneeID: util.MustParseUUID("11111111-1111-1111-1111-111111111111"), + } + triggers, targets := h.computeCommentAgentTriggers(context.Background(), issue, memberMention, nil, "agent", "11111111-1111-1111-1111-111111111111", commentTriggerComputeOptions{}) + if len(triggers) != 0 || len(targets) != 0 { + t.Fatalf("member-only comment by %s owner produced triggers=%d targets=%d", assigneeType, len(triggers), len(targets)) + } + }) + } +} + // enqueueMentionedAgentTasksForTest mirrors the production comment path for // @mention triggers: compute the cascade trigger set, then enqueue it. Kept as a // test helper so these integration tests keep asserting enqueue side effects. -func enqueueMentionedAgentTasksForTest(t *testing.T, ctx context.Context, issue db.Issue, comment db.Comment, parentComment *db.Comment, authorType, authorID string) { +func enqueueMentionedAgentTasksForTest(t *testing.T, ctx context.Context, issue db.Issue, comment db.Comment, parentComment *db.Comment, authorType, authorID string, opts commentTriggerComputeOptions) { t.Helper() - triggers, _ := testHandler.computeCommentAgentTriggers(ctx, issue, comment.Content, parentComment, authorType, authorID, commentTriggerComputeOptions{}) + triggers, _ := testHandler.computeCommentAgentTriggers(ctx, issue, comment.Content, parentComment, authorType, authorID, opts) testHandler.enqueueCommentAgentTriggers(ctx, issue, comment.ID, triggers) } // selfMentionFixture wires the seeded "Handler Test Agent" as J plus two // fresh issues so we can exercise the agent-self-mention path on the @mention // branch of computeCommentAgentTriggers. The three tests below cover -// the behavior we want post-MUL-2338: +// the narrow loop-safe self-mention behavior: // -// - cross-issue self-mention enqueues (child→parent handoff between issues -// assigned to the same agent must not be swallowed) -// - same-issue self-mention with an in-flight running task enqueues a -// follow-up (queue coalescing already allows this — the comment handler -// must not pre-empt it with an extra in-thread guard) +// - cross-issue self-mention from a trusted task enqueues +// - same-issue self-mention with an in-flight task does not enqueue a follow-up // - same-issue self-mention with a queued/dispatched task is deduped // (HasPendingTaskForIssueAndAgent still does its job) type selfMentionFixture struct { @@ -150,12 +169,8 @@ func countQueuedOrDispatched(t *testing.T, agentID, issueID string) int { return n } -// TestEnqueueMentionedAgentTasks_SelfMentionCrossIssueEnqueues is the -// regression test for the MUL-2338 child→parent handoff. The same agent runs -// in a child issue, then posts a top-level comment on the parent issue (whose -// assignee is the same agent) that @mentions itself. The comment handler MUST -// enqueue a task on the parent issue — silently dropping the trigger was the -// bug Bohan reported. +// TestEnqueueMentionedAgentTasks_SelfMentionCrossIssueEnqueues preserves the +// explicit child→parent handoff when the trusted authoring task is on another issue. func TestEnqueueMentionedAgentTasks_SelfMentionCrossIssueEnqueues(t *testing.T) { if testHandler == nil || testPool == nil { t.Skip("database not available") @@ -166,20 +181,30 @@ func TestEnqueueMentionedAgentTasks_SelfMentionCrossIssueEnqueues(t *testing.T) if got := countQueuedOrDispatched(t, fx.JID, fx.IssueBID); got != 0 { t.Fatalf("before: expected 0 pending tasks on parent issue, got %d", got) } + var taskID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status) + VALUES ($1, $2, $3, 'running') RETURNING id + `, fx.JID, fx.RuntimeID, fx.IssueAID).Scan(&taskID); err != nil { + t.Fatalf("seed child task: %v", err) + } + opts := commentTriggerComputeOptions{AuthoringTaskID: util.MustParseUUID(taskID)} + triggers, targets := testHandler.computeCommentAgentTriggers(ctx, fx.IssueB, fx.CommentB.Content, nil, "agent", fx.JID, opts) + if len(triggers) != 1 || len(targets) != 1 || targets[0].ExecAgentID != fx.JID { + t.Fatalf("cross-issue self-handoff = triggers:%d targets:%+v, want one executable target", len(triggers), targets) + } - enqueueMentionedAgentTasksForTest(t, ctx, fx.IssueB, fx.CommentB, nil, "agent", fx.JID) + enqueueMentionedAgentTasksForTest(t, ctx, fx.IssueB, fx.CommentB, nil, "agent", fx.JID, opts) if got := countQueuedOrDispatched(t, fx.JID, fx.IssueBID); got != 1 { - t.Fatalf("after self-mention from another issue: expected 1 queued task on parent issue, got %d", got) + t.Fatalf("after self-mention from another issue: expected one queued task, got %d", got) } } // TestEnqueueMentionedAgentTasks_SelfMentionWhileRunningQueuesFollowup proves // that a self-mention posted in the same issue an agent is currently running -// in does NOT pre-empt the natural queue-coalescing behavior: a `running` -// task is not "pending" for dedup purposes, so a new queued follow-up is -// added and the agent picks it up on its next cycle. -func TestEnqueueMentionedAgentTasks_SelfMentionWhileRunningQueuesFollowup(t *testing.T) { +// in is covered by that run and does not enqueue a follow-up. +func TestEnqueueMentionedAgentTasks_SelfMentionWhileRunningIsNoop(t *testing.T) { if testHandler == nil || testPool == nil { t.Skip("database not available") } @@ -187,21 +212,27 @@ func TestEnqueueMentionedAgentTasks_SelfMentionWhileRunningQueuesFollowup(t *tes fx := newSelfMentionFixture(t) // Seed a running task for J on issue A — this is the agent's current run. - if _, err := testPool.Exec(ctx, ` + var taskID string + if err := testPool.QueryRow(ctx, ` INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status) - VALUES ($1, $2, $3, 'running') - `, fx.JID, fx.RuntimeID, fx.IssueAID); err != nil { + VALUES ($1, $2, $3, 'running') RETURNING id + `, fx.JID, fx.RuntimeID, fx.IssueAID).Scan(&taskID); err != nil { t.Fatalf("seed running task: %v", err) } if got := countQueuedOrDispatched(t, fx.JID, fx.IssueAID); got != 0 { t.Fatalf("before: expected 0 queued/dispatched tasks (only the running task), got %d", got) } + opts := commentTriggerComputeOptions{AuthoringTaskID: util.MustParseUUID(taskID)} + triggers, targets := testHandler.computeCommentAgentTriggers(ctx, fx.IssueA, fx.CommentA.Content, nil, "agent", fx.JID, opts) + if len(triggers) != 0 || len(targets) != 1 || targets[0].Status != DispatchDeferred || targets[0].ReasonCode != ReasonAlreadyActive { + t.Fatalf("active self-mention outcome = triggers:%d targets:%+v, want deferred/already_active", len(triggers), targets) + } - enqueueMentionedAgentTasksForTest(t, ctx, fx.IssueA, fx.CommentA, nil, "agent", fx.JID) + enqueueMentionedAgentTasksForTest(t, ctx, fx.IssueA, fx.CommentA, nil, "agent", fx.JID, opts) - if got := countQueuedOrDispatched(t, fx.JID, fx.IssueAID); got != 1 { - t.Fatalf("after self-mention while running: expected 1 new queued follow-up, got %d", got) + if got := countQueuedOrDispatched(t, fx.JID, fx.IssueAID); got != 0 { + t.Fatalf("after self-mention while running: expected no queued follow-up, got %d", got) } } @@ -241,7 +272,7 @@ func TestEnqueueMentionedAgentTasks_SelfMentionDedupesAgainstPendingTask(t *test t.Fatalf("before: expected 1 pre-existing %s task, got %d", tc.status, before) } - enqueueMentionedAgentTasksForTest(t, ctx, fx.IssueA, fx.CommentA, nil, "agent", fx.JID) + enqueueMentionedAgentTasksForTest(t, ctx, fx.IssueA, fx.CommentA, nil, "agent", fx.JID, commentTriggerComputeOptions{}) after := countQueuedOrDispatched(t, fx.JID, fx.IssueAID) if after != 1 { diff --git a/server/internal/handler/quick_action.go b/server/internal/handler/quick_action.go index 45e7f6bab93..98a48016a5b 100644 --- a/server/internal/handler/quick_action.go +++ b/server/internal/handler/quick_action.go @@ -914,7 +914,7 @@ func (h *Handler) RunQuickAction(w http.ResponseWriter, r *http.Request) { }) delegationAuthority := h.autopilotDelegationAuthorityFromRequest(r, issue, actorType, actorID) - resp.TriggerOutcomes = h.triggerTasksForComment(r.Context(), issue, comment, nil, actorType, actorID, originatorUserID, delegationAuthority, nil) + resp.TriggerOutcomes = h.triggerTasksForComment(r.Context(), issue, comment, nil, actorType, actorID, originatorUserID, delegationAuthority, h.authoringTaskIDFromRequest(r, actorType, actorID), nil) // Usage telemetry is best-effort and deliberately outside the run's // success path: a failed counter must never cost the user the run. diff --git a/server/internal/handler/squad.go b/server/internal/handler/squad.go index ed4368e8a7b..069cef1d73f 100644 --- a/server/internal/handler/squad.go +++ b/server/internal/handler/squad.go @@ -990,25 +990,6 @@ func (h *Handler) RecordSquadLeaderEvaluation(w http.ResponseWriter, r *http.Req // ── Squad Trigger Logic ───────────────────────────────────────────────────── -// shouldSuppressSquadLeaderSelfTrigger reports whether a squad leader's own -// comment should be blocked from re-enqueuing that same leader. The only -// leader-authored non-leader task allowed to wake the assigned leader is a -// same-squad worker task; generic agent tasks such as direct mentions and -// thread-parent replies are not worker-role proof and must not self-trigger. -func (h *Handler) shouldSuppressSquadLeaderSelfTrigger(ctx context.Context, issueID, leaderID, squadID pgtype.UUID) bool { - latest, err := h.Queries.GetLatestTaskRoleForIssueAndAgent(ctx, db.GetLatestTaskRoleForIssueAndAgentParams{ - IssueID: issueID, - AgentID: leaderID, - }) - if err != nil { - return false - } - if latest.IsLeaderTask { - return true - } - return !latest.SquadID.Valid || uuidToString(latest.SquadID) != uuidToString(squadID) -} - // commentMentionsAnyone returns true when the comment body contains at least // one routing-style mention — [@Name](mention://agent|member|squad|all/). // Issue cross-references (mention://issue/...) are ignored because they are diff --git a/server/internal/handler/squad_comment_trigger_test.go b/server/internal/handler/squad_comment_trigger_test.go index f2c15859270..018e44e7546 100644 --- a/server/internal/handler/squad_comment_trigger_test.go +++ b/server/internal/handler/squad_comment_trigger_test.go @@ -244,22 +244,22 @@ func TestShouldEnqueueSquadLeaderOnComment_AgentAuthoredWorkerCommentsWakeLeader t.Fatalf("clear tasks: %v", err) } } - // insertLeaderTask seeds a same-squad task for the leader agent so the - // self-trigger guard can read the agent's most recent role on the issue. - // Separate Exec calls get distinct created_at values, so the last inserted - // row is the "latest" task. - insertLeaderTask := func(isLeader bool, status string) { + // insertLeaderTask seeds exact same-squad task lineage for the self-trigger + // guard; unrelated older/newer tasks must not affect the decision. + insertLeaderTask := func(isLeader bool, status string) string { t.Helper() 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 runtime: %v", err) } - if _, err := testPool.Exec(ctx, ` + var taskID string + if err := testPool.QueryRow(ctx, ` INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, is_leader_task, squad_id) - VALUES ($1, $2, $3, $4, $5, $6) - `, fx.LeaderID, runtimeID, issueID, status, isLeader, fx.SquadID); err != nil { + VALUES ($1, $2, $3, $4, $5, $6) RETURNING id + `, fx.LeaderID, runtimeID, issueID, status, isLeader, fx.SquadID).Scan(&taskID); err != nil { t.Fatalf("insert task: %v", err) } + return taskID } // Case 1: a worker agent (not the leader) posts a result comment on the @@ -271,16 +271,15 @@ func TestShouldEnqueueSquadLeaderOnComment_AgentAuthoredWorkerCommentsWakeLeader } }) - // Case 2: a dual-role agent (leader of the squad, also runs worker tasks) - // posts while its latest task on the issue was a worker task — the leader - // role must still wake because the comment is a worker result, not a - // leader self-trigger. - t.Run("dual-role worker comment wakes leader when latest task is worker", func(t *testing.T) { + // Case 2: the exact authoring worker task permits the supported + // worker-slice→leader coordination resume. + t.Run("dual-role worker comment wakes leader from exact worker task", func(t *testing.T) { clearTasks() - insertLeaderTask(true, "completed") // older leader task - insertLeaderTask(false, "completed") // newer worker task → latest role is worker - if got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, "done with my worker slice", "agent", fx.LeaderID); !got { - t.Fatalf("dual-role worker comment: expected leader to wake, got skip") + insertLeaderTask(true, "completed") + workerTaskID := insertLeaderTask(false, "completed") + triggers, _ := testHandler.computeCommentAgentTriggers(ctx, fx.Issue, "done with my worker slice", nil, "agent", fx.LeaderID, commentTriggerComputeOptions{AuthoringTaskID: util.MustParseUUID(workerTaskID)}) + if !triggersContainIssueAssigneeSquadLeader(triggers) { + t.Fatalf("dual-role worker comment: expected leader wake from exact worker lineage, got skip") } }) @@ -288,9 +287,10 @@ func TestShouldEnqueueSquadLeaderOnComment_AgentAuthoredWorkerCommentsWakeLeader // is a self-trigger loop and must stay suppressed. t.Run("leader comment from latest leader task does not self-trigger", func(t *testing.T) { clearTasks() - insertLeaderTask(false, "completed") // older worker task - insertLeaderTask(true, "completed") // newer leader task → latest role is leader - if got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, "coordinating next steps", "agent", fx.LeaderID); got { + insertLeaderTask(false, "completed") + leaderTaskID := insertLeaderTask(true, "completed") + triggers, _ := testHandler.computeCommentAgentTriggers(ctx, fx.Issue, "coordinating next steps", nil, "agent", fx.LeaderID, commentTriggerComputeOptions{AuthoringTaskID: util.MustParseUUID(leaderTaskID)}) + if triggersContainIssueAssigneeSquadLeader(triggers) { t.Fatalf("leader self-trigger: expected skip, got wake") } }) @@ -390,15 +390,14 @@ func TestCreateComment_SquadPlainReplyToMemberParentKeepsRootMentionOwner(t *tes } } -// TestCreateComment_DualRoleAgentWorkerCommentWakesLeader pins the MUL-3879 -// restored coordination loop at the full-handler level. Scenario: +// TestCreateComment_DualRoleAgentWorkerCommentWakesLeader pins exact-task +// lineage at the full-handler level. Scenario: // // - Agent L is the leader of squad S and also runs worker tasks on issues // belonging to S. // - L is woken in its worker role (is_leader_task=false) and posts a result // comment. -// - A leader-role task IS enqueued so the squad leader can coordinate the -// next step — the worker result must not silently strand the issue. +// - A leader-role task is enqueued so coordination resumes. func TestCreateComment_DualRoleAgentWorkerCommentWakesLeader(t *testing.T) { if testHandler == nil || testPool == nil { t.Skip("database not available") @@ -412,9 +411,7 @@ func TestCreateComment_DualRoleAgentWorkerCommentWakesLeader(t *testing.T) { testPool.Exec(context.Background(), `DELETE FROM comment WHERE issue_id = $1`, issueID) }) - // Seed a same-squad worker task for the leader agent on this issue so the - // guard infers "agent's last activity was a worker task" — i.e. L is - // running in its worker role when it posts the comment. We make it running + // Seed a same-squad worker task for the leader agent on this issue. Make it running // (not completed) so we can hand its ID back through X-Task-ID for the // resolveActor agent-identity check. var runtimeID string @@ -444,7 +441,7 @@ func TestCreateComment_DualRoleAgentWorkerCommentWakesLeader(t *testing.T) { t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String()) } - // A new leader-role task is enqueued so the leader coordinates next steps. + // Exact worker-task lineage permits one leader-role coordination task. var leaderTasks int if err := testPool.QueryRow(ctx, ` SELECT count(*) FROM agent_task_queue @@ -453,7 +450,7 @@ func TestCreateComment_DualRoleAgentWorkerCommentWakesLeader(t *testing.T) { t.Fatalf("count leader tasks: %v", err) } if leaderTasks != 1 { - t.Fatalf("after worker comment from dual-role agent: expected 1 queued leader task, got %d", leaderTasks) + t.Fatalf("after worker comment from dual-role leader: expected 1 queued leader task, got %d", leaderTasks) } } diff --git a/server/internal/handler/trigger_test.go b/server/internal/handler/trigger_test.go index c066606cad5..3594950a30c 100644 --- a/server/internal/handler/trigger_test.go +++ b/server/internal/handler/trigger_test.go @@ -113,5 +113,5 @@ func TestTriggerTasksForComment_NoteShortCircuits(t *testing.T) { } // Must not panic — the guard short-circuits before any DB access. - h.triggerTasksForComment(context.Background(), issue, comment, nil, "member", memberID, memberID, "", nil) + h.triggerTasksForComment(context.Background(), issue, comment, nil, "member", memberID, memberID, "", pgtype.UUID{}, nil) }