Files
multica/server/internal/handler/issue_child_done_test.go
Bohan Jiang 30318b79bc MUL-5426: fix(daemon): retire sessions whose history the provider refuses to replay (#6083)
* fix(daemon): retire sessions whose history the provider refuses to replay

A run killed mid-reply (machine shutdown, force-quit, SIGKILL) can leave an
empty assistant message in the agent CLI's transcript. Every later resume
replays it, the provider rejects the request, and the (agent, issue) pair is
bricked with no self-healing and no user-facing recovery.

Multica already has the mechanism for this — poisoned-session classification —
but its detector paired "400" with "invalid_request_error", which is the
Anthropic wire shape. The same defect reported by any other provider carried
neither token, so it classified as agent_error.unknown: resume-safe by
omission. GetLastTaskSession kept handing back the dead session on every
follow-up, manual Rerun resolved it through the same predicate, and the
in-turn fresh-session retry never fired because ResumeRejected is false here
(nothing rejected the resume — the transcript loaded and the provider refused
to replay it).

Add taskfailure.UnresumableHistory, which recognises the defect by what the
provider says is wrong — some content is empty, and here is which message in
the history — rather than by status code or provider name. Both signals are
required, so a tool reporting "field must not be empty" does not match.

Wire it into the four places that decide whether a session survives:

- classifyPoisonedError, so the task is written as api_invalid_request
- shouldRetryWithFreshSession, so the turn recovers on all 17 backends
  instead of the subset whose adapter learned to detect it; the tools == 0
  gate is unchanged, so a run that already used a tool is never re-run
- ResumeUnsafeFailure, covering the manual-Rerun path
- both resume queries, as defense-in-depth for hosts whose daemon predates
  this (self-host daemons upgrade on their own cadence)

Fixes #6066. Also covers the daemon half of #5760.

Co-authored-by: multica-agent <github@multica.ai>

* fix(session): close the Chat and fresh-retry paths that resurrect a poisoned session

Review found the previous commit stopped short in two places, both of which
put the dead transcript back in play.

Chat never consulted the guarded query. The claim handler reads
chat_session.session_id first and only falls back to GetLastChatTaskSession
when it is empty, so a poisoned pointer there bypasses every filter that query
applies. The fail path merely declined to OVERWRITE the pointer, leaving it in
place. It now clears it in the same transaction, matched on session and
runtime so a concurrent turn's newer pointer survives. The promote guard moves
to ResumeUnsafeFailure as well — the reason-only check passed an un-upgraded
daemon's agent_error.unknown row and re-pinned what the clear had just removed.

GetLastChatTaskSession also kept the row-level filter the issue query dropped
in GH #5975: it discarded the newest poisoned row and fell back to an older
completed row carrying the same dead session. It now judges each session by
its latest terminal state, matching GetLastTaskSession.

A recovered turn could not retire anything. A terminal report carried one
session_id, and an empty one meant both "nothing to report" and "forget the
old session", so a fresh-session retry that SUCCEEDED left the id it retried
away from selectable — through an older completed row on the issue, or through
the chat pointer. agent_task_queue.retired_session_id records the abandonment
itself, reported on every terminal path including completed, and both resume
lookups exclude it. This is the contract gap the previous PR deferred; the
fresh-retry path now runs on all backends, so deferring it is not safe.

Also narrows what the cross-backend test claims: it pins the shared decision,
not that all 17 adapters surface the error into Result.Error (#5760 is the
counter-example), and says so.

Co-authored-by: multica-agent <github@multica.ai>

* test(session): require pgx.ErrNoRows in the resume-exclusion assertions

The `if err == nil && prior.SessionID.Valid` form these tests shared is
false-green: any real fault — undefined column, syntax error, dead connection
— makes err non-nil, so the condition is false and the test passes. Run
against a database missing this branch's new column, the exclusion tests
reported PASS on a SQLSTATE 42703, meaning they could not have caught a broken
query.

requireSessionExcluded demands pgx.ErrNoRows specifically and fails loudly on
anything else, so a green run now means the filter worked rather than the
query never ran.

Applied to all nine sites, not just the four this branch added: the other five
guard the same GetLastTaskSession exclusion behaviour that this branch
changes, so leaving them false-green would leave the change under-tested. All
nine pass on a correctly migrated database.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-29 15:54:51 +08:00

678 lines
27 KiB
Go

package handler
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
)
// childDoneFixture creates a parent + child pair so the parent-notification
// tests can drive the child's status changes independently. Cleanup is
// registered on the test so the rows are removed even on test failure.
type childDoneFixture struct {
parent IssueResponse
child IssueResponse
}
func newChildDoneFixture(t *testing.T, parentStatus string) childDoneFixture {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "child-done parent " + time.Now().Format(time.RFC3339Nano),
"status": parentStatus,
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("create parent: expected 201, got %d: %s", w.Code, w.Body.String())
}
var parent IssueResponse
if err := json.NewDecoder(w.Body).Decode(&parent); err != nil {
t.Fatalf("decode parent: %v", err)
}
w = httptest.NewRecorder()
req = newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "child-done child " + time.Now().Format(time.RFC3339Nano),
"status": "in_progress",
"parent_issue_id": parent.ID,
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("create child: expected 201, got %d: %s", w.Code, w.Body.String())
}
var child IssueResponse
if err := json.NewDecoder(w.Body).Decode(&child); err != nil {
t.Fatalf("decode child: %v", err)
}
t.Cleanup(func() {
ctx := context.Background()
// Cascades through comment.
testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, child.ID)
testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, parent.ID)
})
return childDoneFixture{parent: parent, child: child}
}
// updateChildStatus drives an UpdateIssue HTTP call against the child issue.
func updateChildStatus(t *testing.T, childID, status string) {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("PUT", "/api/issues/"+childID, map[string]any{"status": status})
req = withURLParam(req, "id", childID)
testHandler.UpdateIssue(w, req)
if w.Code != http.StatusOK {
t.Fatalf("UpdateIssue child status=%q: expected 200, got %d: %s", status, w.Code, w.Body.String())
}
}
// countSystemCommentsOn returns the number of platform-generated comments on
// the given issue. The schema CHECK was widened in migration 107 to allow
// author_type='system'; this query is the canary that the migration applied
// and the helper inserts with the right author identity.
func countSystemCommentsOn(t *testing.T, issueID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(context.Background(),
`SELECT count(*) FROM comment WHERE issue_id = $1 AND author_type = 'system'`,
issueID,
).Scan(&n); err != nil {
t.Fatalf("count system comments: %v", err)
}
return n
}
func systemCommentOn(t *testing.T, issueID string) (content, authorIDStr string, parentNull bool, typeStr string) {
t.Helper()
row := testPool.QueryRow(context.Background(),
`SELECT content, author_id::text, parent_id IS NULL, type
FROM comment
WHERE issue_id = $1 AND author_type = 'system'
ORDER BY created_at DESC
LIMIT 1`,
issueID)
if err := row.Scan(&content, &authorIDStr, &parentNull, &typeStr); err != nil {
t.Fatalf("read system comment: %v", err)
}
return
}
// TestChildDoneNotifiesParent — the happy path for an unassigned parent. A
// child transitioning from a non-done status into `done` while its parent is
// open must produce exactly one top-level platform-generated comment on the
// parent. The comment must reference the child by its workspace-specific
// identifier (NOT a hardcoded `MUL-` prefix — that was the bug PR #2918
// review called out). When the parent has no assignee, the body must NOT
// carry any agent/member/squad mention either; the assignee-mention is the
// only mention we ever inject (see MUL-2538 Option C — covered separately
// in TestChildDoneMentionsParentAssignee_* below).
func TestChildDoneNotifiesParent(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 {
t.Fatalf("expected exactly 1 system comment on parent, got %d", got)
}
content, authorID, parentNull, typeStr := systemCommentOn(t, fx.parent.ID)
if !parentNull {
t.Errorf("system comment must be top-level (parent_id IS NULL)")
}
if typeStr != "system" {
t.Errorf("system comment type should be 'system', got %q", typeStr)
}
if authorID != "00000000-0000-0000-0000-000000000000" {
t.Errorf("system comment author_id should be the zero UUID sentinel, got %q", authorID)
}
// Identifier substring must use the real workspace prefix (HAN-, seeded
// in TestMain), never MUL-.
if !strings.Contains(content, fx.child.Identifier) {
t.Errorf("expected comment to contain child identifier %q, got: %s", fx.child.Identifier, content)
}
if strings.Contains(content, "MUL-") {
t.Errorf("comment must not hardcode MUL- prefix, got: %s", content)
}
// The comment must contain the safe issue mention. With no parent
// assignee, none of the routing mentions should appear either.
if !strings.Contains(content, "mention://issue/"+fx.child.ID) {
t.Errorf("expected mention://issue/<child-id> link in comment, got: %s", content)
}
for _, banned := range []string{"mention://agent/", "mention://member/", "mention://squad/"} {
if strings.Contains(content, banned) {
t.Errorf("parent has no assignee but comment included %q mention, got: %s", banned, content)
}
}
}
// TestChildDoneNotificationIsIdempotent — re-saving an already-done child
// must NOT fire a second notification. UpdateIssue is called with the same
// status='done' twice; only the first call is a transition and should
// produce a comment.
func TestChildDoneNotificationIsIdempotent(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 {
t.Fatalf("after first done: expected 1 comment, got %d", got)
}
// Second save of done — should be a no-op transition.
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 {
t.Fatalf("after second done: expected still 1 comment (idempotent), got %d", got)
}
}
// TestChildReopenAndDoneFiresAgain — done → in_progress → done IS a real
// new completion event and should produce a second notification. This
// captures the "reopen + done counts as a new event" line from MUL-2538.
func TestChildReopenAndDoneFiresAgain(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
updateChildStatus(t, fx.child.ID, "done")
updateChildStatus(t, fx.child.ID, "in_progress")
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 2 {
t.Fatalf("expected 2 system comments after reopen+done cycle, got %d", got)
}
}
// TestChildDoneSkippedWhenParentDone — when the parent is already at a
// terminal status, there is nothing for the parent assignee to advance to,
// so the notification must NOT fire.
func TestChildDoneSkippedWhenParentDone(t *testing.T) {
fx := newChildDoneFixture(t, "done")
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 0 {
t.Errorf("parent at 'done' should not receive notification, got %d comments", got)
}
}
// TestChildDoneSkippedWhenParentCancelled — same as above for cancelled.
func TestChildDoneSkippedWhenParentCancelled(t *testing.T) {
fx := newChildDoneFixture(t, "cancelled")
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 0 {
t.Errorf("parent at 'cancelled' should not receive notification, got %d comments", got)
}
}
// TestChildDoneSkippedWhenParentBacklog — a parent deliberately parked in
// `backlog` must not be woken when a child completes. Waking it would
// re-activate the parent assignee, which can then promote sibling backlog
// sub-issues into todo — the surprise auto-activation reported in #4320 /
// MUL-3497. No system comment, no trigger, until the user explicitly moves
// the parent out of backlog.
func TestChildDoneSkippedWhenParentBacklog(t *testing.T) {
fx := newChildDoneFixture(t, "backlog")
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 0 {
t.Errorf("parent at 'backlog' should not receive notification, got %d comments", got)
}
}
// TestChildDoneSkippedWhenNoParent — an issue with no parent_issue_id must
// not produce any system comment on anything.
func TestChildDoneSkippedWhenNoParent(t *testing.T) {
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "orphan child-done " + time.Now().Format(time.RFC3339Nano),
"status": "in_progress",
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("create orphan: expected 201, got %d: %s", w.Code, w.Body.String())
}
var orphan IssueResponse
json.NewDecoder(w.Body).Decode(&orphan)
t.Cleanup(func() {
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, orphan.ID)
})
// Sanity baseline — there should be zero system comments anywhere in
// the workspace attributable to this orphan transition. We can only
// check that the orphan didn't somehow get one itself, but combined
// with the no-parent code path returning early, that is sufficient.
updateChildStatus(t, orphan.ID, "done")
if got := countSystemCommentsOn(t, orphan.ID); got != 0 {
t.Errorf("orphan must not receive a self-notification, got %d system comments", got)
}
}
// setIssueAssigneeDirect bypasses UpdateIssue (and its assignment trigger
// side effects) by writing to the assignee columns directly. The child-done
// notification helper reads the parent row through GetIssue at fire time,
// so a direct UPDATE is enough to drive the dispatch under each assignee
// type without queuing a parallel agent task at setup.
func setIssueAssigneeDirect(t *testing.T, issueID, assigneeType, assigneeID string) {
t.Helper()
if _, err := testPool.Exec(context.Background(),
`UPDATE issue SET assignee_type = $2, assignee_id = $3 WHERE id = $1`,
issueID, assigneeType, assigneeID,
); err != nil {
t.Fatalf("set parent assignee: %v", err)
}
}
func parentSystemCommentContent(t *testing.T, issueID string) string {
t.Helper()
if got := countSystemCommentsOn(t, issueID); got != 1 {
t.Fatalf("expected exactly 1 system comment on parent, got %d", got)
}
content, _, _, _ := systemCommentOn(t, issueID)
return content
}
func countPendingTasksForAgent(t *testing.T, issueID, agentID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(context.Background(),
`SELECT count(*) FROM agent_task_queue
WHERE issue_id = $1 AND agent_id = $2
AND status IN ('queued', 'dispatched', 'running')`,
issueID, agentID,
).Scan(&n); err != nil {
t.Fatalf("count pending tasks: %v", err)
}
return n
}
func countInboxItems(t *testing.T, recipientUserID, issueID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(context.Background(),
`SELECT count(*) FROM inbox_item
WHERE recipient_id = $1 AND issue_id = $2`,
recipientUserID, issueID,
).Scan(&n); err != nil {
t.Fatalf("count inbox items: %v", err)
}
return n
}
// TestChildDoneMentionsParentAssignee_Agent verifies the MUL-2538 Option C
// happy path for an agent parent assignee: the system comment carries a
// `mention://agent/<id>` link AND a real mention-style task is enqueued on
// the parent. The trigger fires through TaskService.EnqueueTaskForMention,
// so the dedupe + readiness checks match the @-mention path users already
// rely on.
func TestChildDoneMentionsParentAssignee_Agent(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
var agentID string
if err := testPool.QueryRow(context.Background(),
`SELECT id FROM agent WHERE workspace_id = $1 AND name = $2`,
testWorkspaceID, "Handler Test Agent",
).Scan(&agentID); err != nil {
t.Fatalf("locate test agent: %v", err)
}
setIssueAssigneeDirect(t, fx.parent.ID, "agent", agentID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM agent_task_queue WHERE issue_id = $1`, fx.parent.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
wantMention := "mention://agent/" + agentID
if !strings.Contains(content, wantMention) {
t.Errorf("expected %q in system comment, got: %s", wantMention, content)
}
if got := countPendingTasksForAgent(t, fx.parent.ID, agentID); got != 1 {
t.Errorf("expected 1 pending task for parent agent, got %d", got)
}
}
// TestChildDoneSkippedWhenParentMember verifies the MUL-2538 follow-up: a
// human parent assignee should NOT receive the platform-generated system
// comment at all. Humans read their own timeline manually; the automated
// notification is pure noise and skipping it also removes the question of
// whether to mention/inbox-row the member.
//
// The assignee row uses `user_id` (NOT `member.id`) — that is the
// production invariant validated by validateAssigneePair for member
// assignees (see server/internal/handler/issue.go), so the fixture must
// match or it would be exercising a state that cannot occur for real.
func TestChildDoneSkippedWhenParentMember(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
var userID string
if err := testPool.QueryRow(context.Background(),
`SELECT user_id FROM member WHERE workspace_id = $1 LIMIT 1`,
testWorkspaceID,
).Scan(&userID); err != nil {
t.Fatalf("locate workspace member: %v", err)
}
setIssueAssigneeDirect(t, fx.parent.ID, "member", userID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM inbox_item WHERE issue_id = $1`, fx.parent.ID)
})
updateChildStatus(t, fx.child.ID, "done")
if got := countSystemCommentsOn(t, fx.parent.ID); got != 0 {
t.Errorf("parent with member assignee should not receive a system comment, got %d", got)
}
if got := countInboxItems(t, userID, fx.parent.ID); got != 0 {
t.Errorf("parent with member assignee should not receive an inbox row, got %d", got)
}
}
// TestChildDoneMentionsParentAssignee_Squad verifies the squad branch: the
// system comment carries a `mention://squad/<id>` link and the squad
// leader receives a leader-role task. Reuses the squad fixture helper from
// squad_comment_trigger_test.go.
func TestChildDoneMentionsParentAssignee_Squad(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
sq := newSquadCommentTriggerFixture(t)
setIssueAssigneeDirect(t, fx.parent.ID, "squad", sq.SquadID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM agent_task_queue WHERE issue_id = $1`, fx.parent.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
wantMention := "mention://squad/" + sq.SquadID
if !strings.Contains(content, wantMention) {
t.Errorf("expected %q in system comment, got: %s", wantMention, content)
}
if got := countPendingTasksForAgent(t, fx.parent.ID, sq.LeaderID); got != 1 {
t.Errorf("expected 1 pending leader task for parent squad, got %d", got)
}
}
// TestChildDoneTriggersParentAgentWhenSameAgentOwnsChild — when the parent
// agent assignee is the SAME agent that owns the just-finished child, the
// parent agent must still be triggered (MUL-2808). A child finishing and
// waking its parent is a serial sub-task handoff between two different
// issues, not a self-loop — and the lone-agent decomposition pattern (one
// agent owns both the parent and the sub-issues it created) has no other
// wake path. The comment is created AND exactly one task is enqueued on the
// parent; runaway re-triggering is bounded by the HasPendingTaskForIssueAndAgent
// dedup, not by suppressing the trigger.
func TestChildDoneTriggersParentAgentWhenSameAgentOwnsChild(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
var agentID string
if err := testPool.QueryRow(context.Background(),
`SELECT id FROM agent WHERE workspace_id = $1 AND name = $2`,
testWorkspaceID, "Handler Test Agent",
).Scan(&agentID); err != nil {
t.Fatalf("locate test agent: %v", err)
}
// Both child and parent assigned to the same agent. Setting the child
// assignee via direct SQL avoids the assignment-trigger side effect
// that would otherwise queue an unrelated task on the child.
setIssueAssigneeDirect(t, fx.parent.ID, "agent", agentID)
setIssueAssigneeDirect(t, fx.child.ID, "agent", agentID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM agent_task_queue WHERE issue_id IN ($1, $2)`,
fx.parent.ID, fx.child.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
if !strings.Contains(content, "mention://agent/"+agentID) {
t.Errorf("expected parent-assignee mention in system comment, got: %s", content)
}
if got := countPendingTasksForAgent(t, fx.parent.ID, agentID); got != 1 {
t.Errorf("expected 1 pending task on parent (serial sub-task handoff), got %d", got)
}
}
// TestChildDoneTriggersParentAgentWhenChildSquadSharesLeader — parent is
// assigned to agent A directly; the finished child is assigned to a squad
// whose leader is also agent A. Because the parent is an AGENT, dispatch
// routes through the agent path, which (post-MUL-2808) has no self-trigger
// guard: A coordinates the parent and must be woken to advance it when the
// child completes, regardless of who executed the child. The squad path now
// behaves identically: MUL-3969 removed its old same-squad / shared-leader
// guards, so BOTH sides being squads that share a leader also wakes the leader
// (see TestChildDoneWakesLeaderWhenParentAndChildSquadsShareLeader).
func TestChildDoneTriggersParentAgentWhenChildSquadSharesLeader(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
sq := newSquadCommentTriggerFixture(t)
// Parent agent == squad leader, child assigned to the squad.
setIssueAssigneeDirect(t, fx.parent.ID, "agent", sq.LeaderID)
setIssueAssigneeDirect(t, fx.child.ID, "squad", sq.SquadID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM agent_task_queue WHERE issue_id IN ($1, $2)`,
fx.parent.ID, fx.child.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
if !strings.Contains(content, "mention://agent/"+sq.LeaderID) {
t.Errorf("expected parent-agent mention in system comment, got: %s", content)
}
if got := countPendingTasksForAgent(t, fx.parent.ID, sq.LeaderID); got != 1 {
t.Errorf("expected 1 pending task on parent (serial sub-task handoff), got %d", got)
}
}
// TestChildDoneWakesLeaderWhenParentAndChildSquadsShareLeader — cross-squad
// shared-leader case. Parent is squad A, child is squad B, both squads have
// the same leader agent. The squad path used to suppress the leader wake here
// (effectiveChildAgentOwner reduced both sides to the shared leader), but that
// guard was removed in MUL-3969: waking the leader on the PARENT is a serial
// sub-task handoff across two DIFFERENT issues, not a self-loop, and it is the
// only signal that carries the parent-level stage-barrier instruction. The
// leader must now be woken exactly once; runaway re-triggering is bounded by
// the HasPendingTaskForIssueAndAgent idempotency check.
func TestChildDoneWakesLeaderWhenParentAndChildSquadsShareLeader(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
parentSquad := newSquadCommentTriggerFixture(t)
// Spin up a SECOND squad that reuses the same leader as parentSquad.
ctx := context.Background()
var childSquadID string
if err := testPool.QueryRow(ctx, `
INSERT INTO squad (workspace_id, name, description, leader_id, creator_id)
VALUES ($1, $2, '', $3, $4)
RETURNING id
`, testWorkspaceID, "Child Done Shared Leader Squad", parentSquad.LeaderID, testUserID).
Scan(&childSquadID); err != nil {
t.Fatalf("create second squad: %v", err)
}
t.Cleanup(func() {
testPool.Exec(context.Background(), `DELETE FROM squad WHERE id = $1`, childSquadID)
})
setIssueAssigneeDirect(t, fx.parent.ID, "squad", parentSquad.SquadID)
setIssueAssigneeDirect(t, fx.child.ID, "squad", childSquadID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM agent_task_queue WHERE issue_id IN ($1, $2)`,
fx.parent.ID, fx.child.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
if !strings.Contains(content, "mention://squad/"+parentSquad.SquadID) {
t.Errorf("expected parent-squad mention in system comment, got: %s", content)
}
if got := countPendingTasksForAgent(t, fx.parent.ID, parentSquad.LeaderID); got != 1 {
t.Errorf("expected 1 pending leader task on parent (shared-leader guard removed, MUL-3969), got %d", got)
}
}
// TestChildDoneWakesLeaderWhenChildIsSameSquad — the MUL-3969 repro. Parent
// and the just-finished child are BOTH assigned to the same squad (the common
// "a squad decomposes its parent into sub-issues it works itself" pattern).
// The old same-squad guard suppressed the leader wake, so the stage-barrier
// system comment landed on the parent but the "wrap up / advance" instruction
// was never delivered to the leader and the parent silently stalled in
// in_progress. The leader must now be woken exactly once.
func TestChildDoneWakesLeaderWhenChildIsSameSquad(t *testing.T) {
fx := newChildDoneFixture(t, "in_progress")
sq := newSquadCommentTriggerFixture(t)
setIssueAssigneeDirect(t, fx.parent.ID, "squad", sq.SquadID)
setIssueAssigneeDirect(t, fx.child.ID, "squad", sq.SquadID)
t.Cleanup(func() {
testPool.Exec(context.Background(),
`DELETE FROM agent_task_queue WHERE issue_id IN ($1, $2)`,
fx.parent.ID, fx.child.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
if !strings.Contains(content, "mention://squad/"+sq.SquadID) {
t.Errorf("expected parent-squad mention in system comment, got: %s", content)
}
if got := countPendingTasksForAgent(t, fx.parent.ID, sq.LeaderID); got != 1 {
t.Errorf("expected 1 pending leader task for same-squad child (MUL-3969), got %d", got)
}
}
// TestStageLeaderPrepareTimeoutRetryCanAdvanceNextStage covers the full server
// half of MUL-4923's recovery chain: a stage barrier wakes the squad leader,
// the pre-start attempt fails with the daemon's timeout reason, the atomic
// retry preserves leader/squad/trigger provenance, and that retry can promote
// the parked next stage as the leader actor.
func TestStageLeaderPrepareTimeoutRetryCanAdvanceNextStage(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
ctx := context.Background()
fx := newChildDoneFixture(t, "in_progress")
sq := newSquadCommentTriggerFixture(t)
setIssueAssigneeDirect(t, fx.parent.ID, "squad", sq.SquadID)
setIssueAssigneeDirect(t, fx.child.ID, "squad", sq.SquadID)
if _, err := testPool.Exec(ctx, `UPDATE issue SET stage = 1 WHERE id = $1`, fx.child.ID); err != nil {
t.Fatalf("set stage 1: %v", err)
}
// Stage 2 exists but is deliberately parked. The server wakes the leader at
// the Stage 1 barrier; only the leader decides to promote this child.
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "stage 2 after prepare timeout",
"status": "backlog",
"parent_issue_id": fx.parent.ID,
"stage": 2,
"assignee_type": "squad",
"assignee_id": sq.SquadID,
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("create stage 2: expected 201, got %d: %s", w.Code, w.Body.String())
}
var stage2 IssueResponse
if err := json.NewDecoder(w.Body).Decode(&stage2); err != nil {
t.Fatalf("decode stage 2: %v", err)
}
t.Cleanup(func() {
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE issue_id IN ($1, $2)`, fx.parent.ID, stage2.ID)
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, stage2.ID)
})
updateChildStatus(t, fx.child.ID, "done")
content := parentSystemCommentContent(t, fx.parent.ID)
if !strings.Contains(content, "Stage 2 is next") {
t.Fatalf("stage barrier comment does not identify Stage 2: %s", content)
}
var originalID, originalSquadID, originalTriggerID string
var originalLeader bool
if err := testPool.QueryRow(ctx, `
SELECT id::text, is_leader_task, squad_id::text, trigger_comment_id::text
FROM agent_task_queue
WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued'
ORDER BY created_at DESC
LIMIT 1
`, fx.parent.ID, sq.LeaderID).Scan(&originalID, &originalLeader, &originalSquadID, &originalTriggerID); err != nil {
t.Fatalf("load Stage 1 leader wake: %v", err)
}
if !originalLeader || originalSquadID != sq.SquadID || originalTriggerID == "" {
t.Fatalf("leader wake provenance = leader:%v squad:%q trigger:%q", originalLeader, originalSquadID, originalTriggerID)
}
if _, err := testPool.Exec(ctx, `
UPDATE agent_task_queue
SET status = 'dispatched', dispatched_at = now()
WHERE id = $1
`, originalID); err != nil {
t.Fatalf("dispatch original leader task: %v", err)
}
if _, err := testHandler.TaskService.FailTask(ctx, parseUUID(originalID), "task preparation timed out after 5m0s", "", "", "timeout", false, ""); err != nil {
t.Fatalf("fail original leader task: %v", err)
}
var retryID, retryStatus, retrySquadID, retryTriggerID string
var retryLeader bool
var retryAttempt int32
if err := testPool.QueryRow(ctx, `
SELECT id::text, status, is_leader_task, squad_id::text,
trigger_comment_id::text, attempt
FROM agent_task_queue
WHERE parent_task_id = $1
`, originalID).Scan(&retryID, &retryStatus, &retryLeader, &retrySquadID, &retryTriggerID, &retryAttempt); err != nil {
t.Fatalf("load automatic retry: %v", err)
}
if retryStatus != "queued" || retryAttempt != 2 || !retryLeader || retrySquadID != originalSquadID || retryTriggerID != originalTriggerID {
t.Fatalf("retry provenance = status:%q attempt:%d leader:%v squad:%q trigger:%q; want queued attempt 2 with original leader context",
retryStatus, retryAttempt, retryLeader, retrySquadID, retryTriggerID)
}
if _, err := testPool.Exec(ctx, `
UPDATE agent_task_queue
SET status = 'running', dispatched_at = now(), started_at = now()
WHERE id = $1
`, retryID); err != nil {
t.Fatalf("start retry leader task: %v", err)
}
// Act through the normal issue handler with the retry task as the agent
// identity. This is the operation the Stage handoff prompt asks the leader
// to perform, and proves the retry was not demoted to a generic worker.
w = httptest.NewRecorder()
req = newRequest("PUT", "/api/issues/"+stage2.ID, map[string]any{"status": "todo"})
req.Header.Set("X-Agent-ID", sq.LeaderID)
req.Header.Set("X-Task-ID", retryID)
req = withURLParam(req, "id", stage2.ID)
testHandler.UpdateIssue(w, req)
if w.Code != http.StatusOK {
t.Fatalf("retry leader promote Stage 2: expected 200, got %d: %s", w.Code, w.Body.String())
}
var stage2Status string
if err := testPool.QueryRow(ctx, `SELECT status FROM issue WHERE id = $1`, stage2.ID).Scan(&stage2Status); err != nil {
t.Fatalf("load promoted Stage 2: %v", err)
}
if stage2Status != "todo" {
t.Fatalf("Stage 2 status = %q, want todo", stage2Status)
}
if got := countPendingTasksForAgent(t, stage2.ID, sq.LeaderID); got != 1 {
t.Fatalf("promoted Stage 2 queued %d leader tasks, want 1", got)
}
}