From 8e505dc4e18d873db877b7be84bcedae3c10d286 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 17:46:47 -0700 Subject: [PATCH] docs(recovery): the reason this parameter is optional stopped being true (#2903) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment-only. No source change, no behaviour change. The note on `isInReviewMissingWorktreeSessionStartFailure` said: > Optional rather than required because the other caller (`extension.ts`) still asks BOTH questions with the literal. **It doesn't.** All three production callers pass the resolved answer: ``` packages/cli/src/extension.ts:1927 retryReviewColumns.has(task.column) packages/cli/src/commands/task.ts:1390 retryReviewColumns.has(task.column) packages/dashboard/src/routes/register-task-workflow-routes.ts:2885 retryReviewColumns.has(task.column) ``` Left standing, that sentence tells the next reader an unconverted caller exists — and "we keep the fallback because someone still needs it" is exactly the justification that keeps an inert-conversion shape alive. It is the specific failure this program has spent the day removing, in the form of a comment rather than code. ## The parameter stays optional, for a reason that does not rot I checked whether to make it **required** — the unwired-lane-parameter guard's own failure message suggests exactly that ("make the parameter required so the compiler finds the call sites") — and decided against it, for measured reasons: - **25 test call sites** use the optional form, several of them *precisely* to pin the degraded mode (`cli-active-count-lanes.test.ts` exercises the no-argument path on both a legacy and a renamed lane). Requiring the parameter deletes that coverage. - The enforcement it would buy already exists: `isReviewColumn` is in the guard's vocabulary, so if any of those three callers stops passing it, the build fails. So the note now gives the durable reason instead of the expired one. ## How this surfaced Not from the census — the count here is unchanged, and an *omitted argument* is invisible to it anyway. It came from reading an audit note that named a specific caller and checking the claim. Notes that assert facts about other files decay silently; this one had. ## Verification - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/engine`) — clean - `restart-recovery-coordinator.test.ts` — 12 passed - `cli-active-count-lanes.test.ts` — 10 passed - unwired-lane guard — 9/9, no new entries - SQL-literal gate — green 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .../src/restart-recovery-coordinator.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/engine/src/restart-recovery-coordinator.ts b/packages/engine/src/restart-recovery-coordinator.ts index 79ca6aee62..4150d56d88 100644 --- a/packages/engine/src/restart-recovery-coordinator.ts +++ b/packages/engine/src/restart-recovery-coordinator.ts @@ -125,9 +125,22 @@ export function isMergeActiveMissingWorktreeSessionStartFailure( * the generic branch leaves the stale session metadata in place — so the next execution hit the very * same missing-worktree failure. A retry that reports success and changes nothing. * - * Optional rather than required because the other caller (`extension.ts`) still asks BOTH questions - * with the literal. It is internally consistent that way, so a default preserves its meaning exactly - * while the converted caller passes the resolved answer. + * FNXC:WorkflowLifecycleColumns 2026-07-31-02:00 (note drift — the stated reason stopped being true): + * The paragraph here used to say the parameter is optional "because the other caller (`extension.ts`) + * still asks BOTH questions with the literal". That is no longer the case, and had it stayed it would + * have told the next reader an unconverted caller exists — the kind of note that keeps an + * inert-conversion shape alive by justifying it. + * + * Verified: ALL THREE production callers pass the resolved answer — + * `cli/src/extension.ts`, `cli/src/commands/task.ts` and + * `dashboard/src/routes/register-task-workflow-routes.ts`, each as + * `retryReviewColumns.has(task.column)`. + * + * It stays optional anyway, for a reason that does not rot: the legacy fallback is DELIBERATELY + * covered (`cli-active-count-lanes.test.ts` exercises the no-argument path on both a legacy and a + * renamed lane). Making the parameter required would delete that coverage to buy an enforcement the + * unwired-lane-parameter guard already provides — it watches `isReviewColumn` and fails the build if + * any of those callers stops passing it. */ export function isInReviewMissingWorktreeSessionStartFailure( task: Task,