fleet: restart-recovery-coordinator.ts 4 → 1 (dead fallbacks deleted, not converted) (#3059)

Claiming `packages/engine/src/restart-recovery-coordinator.ts`.

## Census before/after

| File | Before | After |
|---|---:|---:|
| `packages/engine/src/restart-recovery-coordinator.ts` | 4 | **1** |

## These were deletions, not conversions

All three sites were fail-soft fallbacks behind an **optional**
`reviewColumns` parameter:

```ts
return (reviewColumns ? reviewColumns.has(task.column) : task.column === "in-review")
```

Production never took that branch — `self-healing.ts:13646-13649`
supplies the resolved set at every call site. So the correct change is
to make the parameter required and delete the literal, not to swap it
for a role lookup.

## The trap this hit, which would have shipped a crash

**Making the parameter required produced ZERO tsc errors.** That looked
like proof the fallback was unreachable. It is not: the engine
`tsconfig` covers `src` and not `__tests__`, so the type-checker cannot
see the callers that actually relied on the default. Running the tests
surfaced them immediately as `TypeError: Cannot read properties of
undefined (reading 'has')`.

This is the same class as finding 2 in
`docs/solutions/best-practices/proving-a-code-path-actually-runs.md` — a
negative result from a checker that cannot see the thing it is being
asked about. Anyone converting a `src`-only-typechecked package should
assume tsc is blind to test call sites.

The blast radius was also one site larger than grep suggested: the
`isRecoverableMissingWorktreeReviewFailure` **combiner** threads the set
to all three inner predicates. Its own comment already names why — *"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."*

Tests now pass the set production always passes, preserving exactly what
each case asserted.

## Remaining 1, flagged not guessed

`L149` uses a different shape (`isReviewColumn ?? task.column ===
"in-review"`) whose callers I did not establish. Absence from grep is
not proof of no caller, so it stays counted.

## Verification

- census: 4 → 1
- `restart-recovery-coordinator` + `self-healing` — **424 tests green**
- `tsc --noEmit` clean; `pnpm lint` clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

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<string> = 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 () => {

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<string>,
+  reviewColumns: ReadonlySet<string>,
 ): 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<string>,
+  reviewColumns: ReadonlySet<string>,
 ): 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<string>(MERGE_ACTIVE_MI
 
 export function isMergeActiveMissingWorktreeSessionStartFailure(
   task: Task,
-  reviewColumns?: ReadonlySet<string>,
+  reviewColumns: ReadonlySet<string>,
 ): 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<string>,
+  reviewColumns: ReadonlySet<string>,
 ): 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. */