diff --git a/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts b/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts index c3094ca6b8..bbc3664341 100644 --- a/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts +++ b/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts @@ -37,6 +37,19 @@ literal, because each one individually looks right. `reviewColumns` is optional and defaults to the legacy id, so the three existing call sites are unchanged until each passes its own resolved set. */ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-11:20 (fleet: restart-recovery roles): +`reviewColumns` is REQUIRED now. It was optional with a `task.column === "in-review"` fallback that +production never took — `self-healing.ts` supplies the resolved set at every call site — so the +literal survived only because these tests omitted the argument. Passing the set preserves exactly +what each case asserts while removing the last thing keeping the fallback alive. + +Worth recording: making the parameter required produced ZERO tsc errors, because the engine +tsconfig covers `src` and not `__tests__`. A clean typecheck was not evidence here; only running +the tests found these call sites. +*/ +const REVIEW_LANES: ReadonlySet = new Set(["in-review"]); + describe("isInReviewMissingWorktreeSessionStartFailure", () => { /* FNXC:MissingWorktreeRetry 2026-07-30-10:05 (PR #2728, aligned to #2736's signature): @@ -107,13 +120,13 @@ describe("RestartRecoveryCoordinator", () => { steps: [{ id: "s1", title: "step", status: "done" }] as any, }); - expect(isRecoverableMissingWorktreeReviewFailure({ ...baseTask, error: "Refusing to start coding agent in missing worktree: /tmp/wt" })).toBe(true); - expect(isRecoverableMissingWorktreeReviewFailure({ ...baseTask, error: "Refusing to start coding agent in incomplete worktree: /tmp/wt" })).toBe(true); - expect(isRecoverableMissingWorktreeReviewFailure({ ...baseTask, error: "Refusing to start coding agent in unregistered git worktree: /tmp/wt" })).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailure({ ...baseTask, error: "Refusing to start coding agent in missing worktree: /tmp/wt" }, REVIEW_LANES)).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailure({ ...baseTask, error: "Refusing to start coding agent in incomplete worktree: /tmp/wt" }, REVIEW_LANES)).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailure({ ...baseTask, error: "Refusing to start coding agent in unregistered git worktree: /tmp/wt" }, REVIEW_LANES)).toBe(true); - expect(isRecoverableMissingWorktreeReviewFailureWithProgress({ ...baseTask, paused: true, error: "Refusing to start coding agent in missing worktree: /tmp/wt" })).toBe(false); - expect(isRecoverableMissingWorktreeReviewFailureWithProgress({ ...baseTask, error: "other" })).toBe(false); - expect(isRecoverableMissingWorktreeReviewFailureWithProgress({ ...baseTask, steps: [{ id: "s2", title: "y", status: "pending" }] as any, error: "Refusing to start coding agent in missing worktree: /tmp/wt" })).toBe(false); + expect(isRecoverableMissingWorktreeReviewFailureWithProgress({ ...baseTask, paused: true, error: "Refusing to start coding agent in missing worktree: /tmp/wt" }, REVIEW_LANES)).toBe(false); + expect(isRecoverableMissingWorktreeReviewFailureWithProgress({ ...baseTask, error: "other" }, REVIEW_LANES)).toBe(false); + expect(isRecoverableMissingWorktreeReviewFailureWithProgress({ ...baseTask, steps: [{ id: "s2", title: "y", status: "pending" }] as any, error: "Refusing to start coding agent in missing worktree: /tmp/wt" }, REVIEW_LANES)).toBe(false); const errors = [ "Refusing to start coding agent in missing worktree: /tmp/wt", @@ -123,9 +136,9 @@ describe("RestartRecoveryCoordinator", () => { for (const error of errors) { const withProgressTask = { ...baseTask, error }; const noProgressTask = { ...baseTask, steps: [{ id: "s2", title: "y", status: "pending" }] as any, error }; - expect(isRecoverableMissingWorktreeReviewFailureWithProgress(withProgressTask)).toBe(true); - expect(isRecoverableMissingWorktreeReviewFailureNoProgress(noProgressTask)).toBe(true); - expect(isRecoverableMissingWorktreeReviewFailure(noProgressTask)).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailureWithProgress(withProgressTask, REVIEW_LANES)).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailureNoProgress(noProgressTask, REVIEW_LANES)).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailure(noProgressTask, REVIEW_LANES)).toBe(true); } }); @@ -139,13 +152,13 @@ describe("RestartRecoveryCoordinator", () => { for (const status of ["merging", "merging-pr", "merging-fix"] as const) { const task = { ...baseTask, status }; - expect(isMergeActiveMissingWorktreeSessionStartFailure(task)).toBe(true); - expect(isRecoverableMissingWorktreeReviewFailure(task)).toBe(true); + expect(isMergeActiveMissingWorktreeSessionStartFailure(task, REVIEW_LANES)).toBe(true); + expect(isRecoverableMissingWorktreeReviewFailure(task, REVIEW_LANES)).toBe(true); } - expect(isMergeActiveMissingWorktreeSessionStartFailure({ ...baseTask, status: "failed" })).toBe(false); - expect(isMergeActiveMissingWorktreeSessionStartFailure({ ...baseTask, status: null as any })).toBe(false); - expect(isMergeActiveMissingWorktreeSessionStartFailure({ ...baseTask, status: "merging", error: "ordinary merge failure" })).toBe(false); + expect(isMergeActiveMissingWorktreeSessionStartFailure({ ...baseTask, status: "failed" }, REVIEW_LANES)).toBe(false); + expect(isMergeActiveMissingWorktreeSessionStartFailure({ ...baseTask, status: null as any }, REVIEW_LANES)).toBe(false); + expect(isMergeActiveMissingWorktreeSessionStartFailure({ ...baseTask, status: "merging", error: "ordinary merge failure" }, REVIEW_LANES)).toBe(false); }); it("requeues interrupted failed tasks with no progress, then resumes remaining orphans", async () => { diff --git a/packages/engine/src/restart-recovery-coordinator.ts b/packages/engine/src/restart-recovery-coordinator.ts index 4150d56d88..d80d211529 100644 --- a/packages/engine/src/restart-recovery-coordinator.ts +++ b/packages/engine/src/restart-recovery-coordinator.ts @@ -81,9 +81,9 @@ Optional, defaulting to the legacy id, so no existing caller or test changes beh */ export function isRecoverableMissingWorktreeReviewFailureWithProgress( task: Task, - reviewColumns?: ReadonlySet, + reviewColumns: ReadonlySet, ): boolean { - return (reviewColumns ? reviewColumns.has(task.column) : task.column === "in-review") + return reviewColumns.has(task.column) && !task.paused && task.status === "failed" && isMissingWorktreeSessionStartFailure(task.error) @@ -92,9 +92,9 @@ export function isRecoverableMissingWorktreeReviewFailureWithProgress( export function isRecoverableMissingWorktreeReviewFailureNoProgress( task: Task, - reviewColumns?: ReadonlySet, + reviewColumns: ReadonlySet, ): boolean { - return (reviewColumns ? reviewColumns.has(task.column) : task.column === "in-review") + return reviewColumns.has(task.column) && !task.paused && task.status === "failed" && isMissingWorktreeSessionStartFailure(task.error) @@ -106,9 +106,9 @@ const MERGE_ACTIVE_MISSING_WORKTREE_STATUS_SET = new Set(MERGE_ACTIVE_MI export function isMergeActiveMissingWorktreeSessionStartFailure( task: Task, - reviewColumns?: ReadonlySet, + reviewColumns: ReadonlySet, ): boolean { - return (reviewColumns ? reviewColumns.has(task.column) : task.column === "in-review") + return reviewColumns.has(task.column) && !task.paused && typeof task.status === "string" && MERGE_ACTIVE_MISSING_WORKTREE_STATUS_SET.has(task.status) @@ -152,7 +152,7 @@ export function isInReviewMissingWorktreeSessionStartFailure( export function isRecoverableMissingWorktreeReviewFailure( task: Task, - reviewColumns?: ReadonlySet, + reviewColumns: ReadonlySet, ): boolean { /* The combiner threads the set to all three, so a caller cannot convert the outer question and leave one of the three inner ones on the legacy id — the half-conversion shape this program keeps finding. */