docs(recovery): the reason this parameter is optional stopped being true (#2903)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user