mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-12 19:06:06 +02:00
* fix(daemon): gate codex session pointer writes on rollout presence (MUL-5305) Codex issue follow-ups on local_directory projects intermittently lost their session: the server sent a prior session whose rollout was not in the task CODEX_HOME, so the daemon dropped the resume and started a fresh thread (gateCodexResumeToRolloutPresence), losing the conversation. Root of the bad pointer: the daemon persists a Codex session id as the resumable pointer at two points -- the mid-flight pin and the terminal report -- before the rollout is guaranteed on disk. A task that exits early (crash / runtime offline / timeout) leaves a pinned/reported session id with no rollout; GetLastTaskSession (which accepts failed rows) then hands it to the next follow-up, which drops it. Enforce the invariant at write time: only record a Codex session as the resumable pointer once its rollout is present in the per-issue store, with a short bounded wait for flush. If it never lands, don't overwrite the last good pointer -- a blanked session_id becomes NULL server-side, so GetLastTaskSession falls back to the most recent session whose rollout is real. Non-Codex providers are unaffected; crash recovery is preserved because a present rollout still pins. - codexSessionResumable: shared write-time presence check (bounded wait) - runTask: gate the terminal session_id before reporting - executeAndDrain: gate the mid-flight pin (thread codexHome through) - tests: helper cases + behavioral pin test Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): address review — don't silently downgrade completed sessions (MUL-5305) Follow-up to review feedback on #5960: - Must-fix 1 (silent downgrade): limit the write-time session withholding to NON-completed terminal states. A missing rollout means no resumable conversation was persisted, so a withheld non-completed attempt loses nothing; a completed session is authoritative and, if its rollout is anomalously absent, is still recorded so the next run's resume gate discloses the loss (PriorSessionResumeUnavailable, MUL-4424) instead of silently falling back to an older session. Extracted resumableTerminalSessionID. - Non-blocking risk: pin the mid-flight resume pointer with a per-status presence check instead of one fixed 2s window, and set sessionPinned only once the rollout is confirmed, so a rollout that lands shortly after the first status is still pinned this run. - Must-fix 2 (regression coverage): pin skipped when rollout absent (no /session call); terminal helper (completed keeps / failed withholds); and a DB-backed GetLastTaskSession test proving the next claim falls back to the older recorded session when the latest was blanked. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): disclose Codex session continuity gaps end-to-end (MUL-5305) Addresses review feedback on #5960. Must-fix 1 — a completed turn whose rollout is missing is exactly the #5934 case (the reporter waits for each turn to finish), so it can no longer be excluded from withholding. Withhold the session for ANY terminal state, and pair the withhold with a persisted continuity-gap signal so the next claim still discloses the loss even while resuming an older good session: - new agent_task_queue.session_rollout_missing column (migration 224) - daemon sends session_rollout_missing on the terminal report; the handler clears the resume pointer (MarkTaskSessionRolloutMissing, overriding FailAgentTask's COALESCE) and flags the row - claim reads GetLatestTaskRolloutMissing and sets a new prior_session_resume_unavailable response field, which the daemon ORs into the brief's PriorSessionResumeUnavailable disclosure Must-fix 2 — Codex reveals the session id on a single task_started status, so a one-shot presence check missed a rollout that flushed later and lost in-flight crash recovery. Pin via a background waiter bounded by the run's context that pins the moment the rollout lands. Tests: - completed + rollout missing -> next claim withholds the bad session AND flags the continuity gap (cross-layer DB test) - session pinned once its rollout appears after the status (mid-run) - pin skipped while the rollout is absent Co-authored-by: multica-agent <github@multica.ai> * fix(server): make continuity-gap write atomic + disclose on all claim paths (MUL-5305) Addresses review round 3 of #5960. Must-fix 1 — the previous handler-level marker ran AFTER the terminal transaction committed, and FailTask creates + wakes the auto-retry inside that same transaction, so a retry could claim the rollout-missing session before the marker cleared it (and a marker failure was swallowed). Move session_rollout_missing INTO the terminal write: CompleteAgentTask and FailAgentTask now force session_id NULL (overriding Fail's COALESCE that would keep a stale mid-flight pin) and set the flag in the SAME UPDATE, so the withhold + gap flag commit atomically with the retry creation. The flag is threaded through TaskService.CompleteTask/FailTask; the swallowed best-effort MarkTaskSessionRolloutMissing query is removed. Must-fix 2 — the daemon withholds for all Codex tasks, but only the issue non-rerun claim consumed the disclosure. Now every fallback path sets prior_session_resume_unavailable: the manual-rerun branch reads the source task's session_rollout_missing, and the chat branch reads a new GetLatestChatTaskRolloutMissing. Tests (cross-layer DB): - completed + rollout missing via the real CompleteAgentTask terminal write -> session withheld AND gap flagged - failed + rollout missing forces session_id NULL over the COALESCE- preserved mid-flight pin in ONE statement Deploy order: migration + server first, daemon second (new fields are omitempty and ignored by an old peer). Co-authored-by: multica-agent <github@multica.ai> * fix(handler): return 5xx on FailTask error + cover claim-response gap paths (MUL-5305) Addresses review round 4 of #5960. Must-fix 1 — the FailTask handler returned 400 on a service/DB error, but the daemon's terminal callback treats 400 as permanent (postJSONWithRetry / isTransientError bails without retrying). Since the fail transaction is now the sole persistence point for the withheld session + continuity-gap flag + auto-retry, a rolled-back fail must be retried, so return 5xx (an invalid request body still returns 400), mirroring CompleteTask. Regression: client.FailTask retries on a transient 5xx and eventually succeeds. Must-fix 2 — add claim-response-level regressions that drive the two new disclosure branches through buildClaimedTaskResponse: - chat: the latest terminal task on the session withheld -> the next chat claim sets prior_session_resume_unavailable - manual rerun: the source task withheld -> the rerun claim discloses These handler DB tests run under CI's fully-migrated database (the local workspace DB cannot set up the handler fixture). Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai>
678 lines
27 KiB
Go
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)
|
|
}
|
|
}
|