fusion

Author	SHA1	Message	Date
gsxdsm	8e505dc4e1	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>	2026-07-30 17:46:47 -07:00
gsxdsm	10f9df1600	fix(overseer): the whole oversight loop was inert on a renamed board (#2898 ) `resolveWatchedStage` keyed on the literals `in-progress`/`in-review`, so on a board that renames either it returned `null` for every card. That is three literals with an outsized blast radius. `observeTask` returns early on a null stage, so: - no `OverseerStageObservation` is recorded, - no `overseer:intervention` entry is emitted, - and `PlannerRecoveryController`, which consumes those observations, has nothing to steer, retry or targeted-fix. The entire oversight loop was inert and silent about it — the same shape as the self-healing sweeps whose queries returned empty arrays. ## I deferred this myself, on a cost argument that was wrong The audit note I wrote for this site said resolving inside `observeTask` "buys a workflow read per card per poll". Then I read the caller: the poll already awaits `resolveEffectiveSettings` per task. It is a per-task async loop regardless, so with an IR cache keyed by workflow the addition is (distinct workflows) resolutions, not (cards). Pricing the fix before checking the caller cost a deferral. Worth recording, because "this needs a cost judgement" is the most comfortable place in this program to leave something. ## The review test is the three-trait union, deliberately `isReviewColumnRole` checks only `mergeBlocker \|\| humanReview`. A board whose review lane carries `merge` (mergeOrchestration) — the built-in default's own shape — would classify as not in review and be skipped. Reaching for the obvious helper would have reintroduced the bug this change removes, through the helper meant to fix it. There is a case asserting exactly that. ## Wiring Both call sites, because either alone leaves a hole: \| site \| why it matters \| \|---\|---\| \| the poll (`project-engine.ts`) \| per-poll IR cache — a workflow edit is picked up next tick rather than served stale \| \| the manual nudge \| otherwise a renamed board answers `no-active-stage` to an operator pressing the button \| `columnFlags` is in the `unwired-lane-parameter` vocabulary, so the wiring cannot silently rot — the guard reports it if a future change drops the argument. Fail-soft throughout: an unresolvable workflow yields `undefined` and the callee falls back to the legacy ids, which is exactly today's behaviour. A v1 IR declares no columns, so it takes the same path. ## Revert proof (measured) Drop the `columnFlags` branch and exactly the three renamed-lane cases fail: ``` expected null to be "executor" expected null to be "merger" (mergeOrchestration lane) expected null to be "merger" (humanReview lane) ``` The legacy-id and neither-role cases stay green — the gate must still gate, and watching every column would be its own defect. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/engine`) — clean - `planner-overseer.test.ts` + `planner-recovery-controller-human-control.test.ts` — 64 passed - unwired-lane guard — 9/9, no new entries Carries the one-line SQL-baseline re-record (`team-analytics.ts: 6 → 3`) that #2864 left behind, same as my other open branches — main is red on it, and identical changes to that line merge without conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 17:43:43 -07:00
gsxdsm	9366bc8382	fix(workflow): the review handoff killed the walk on a renamed review lane (#2900 ) The sharpest lane defect left in the backlog, and the one I have been deferring since the first sweep. ```ts if (seam === "review-handoff") { const result = await primitives.transitionTask(primitiveCtx, context.task, { column: "in-review", // ← post-U12 this is a rejected destination on a renamed board ``` Post-U12 `moveTask` rejects a destination the workflow does not declare. So on any board with a renamed review lane, the handoff threw `TransitionRejectionError` and killed the workflow walk mid-run. Not a silent wrong answer for once — a hard failure in the middle of a task, which is why it outranked everything else once it became reachable. Why it was deferred: every fix threads a resolver out of `executor.ts`, and #2820 was editing that file. It merged at 22:08, so this was finally free of the conflict. ## The role travels, not the column Seam handlers in `workflow-node-handlers.ts` are pure functions over an IR node and a task — no store, no task id to resolve from — so a handler can only ever name a literal. The runtime primitive in `executor.ts` does hold the store, so the seam now asks for `columnRole: "review"` and the primitive resolves it against the task's own selection. One authority, deliberately. Answering one question with two reads is what took #2843 five review rounds, and I would rather not relearn it here. Compatibility is preserved in both directions: - `column` still wins when both are supplied — an explicit destination is an explicit destination; - an unresolvable role falls back to the legacy `in-review` rather than failing the transition, which is exactly the behaviour every caller had before. ## The test asserts the literal is gone, not merely accompanied `column` takes precedence over `columnRole` downstream, so a diff that added the role while leaving the literal would look converted and be completely inert. That is the exact shape this program keeps finding — a documented fallback in front of a literal that still decides everything — so the assertion is: ```ts expect(input.columnRole).toBe("review"); expect(input.column).toBeUndefined(); // ← the half that matters ``` Revert proof, measured: restore `column: "in-review"` in the seam and it fails with `expected undefined to be 'review'`. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/engine`) — clean - new `review-handoff-lane.test.ts` plus the two neighbouring seam suites — 41 passed Carries the one-line SQL-baseline re-record (`team-analytics.ts: 6 → 3`) that #2864 left behind, same as my other open branches — main is red on it, and identical changes to that line merge without conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 17:39:59 -07:00
gsxdsm	a453912ddf	self-healing: merged-but-unfinished tasks never finalized on a renamed board (fifteenth sweep) (#2897 ) `recoverMergedReviewTasks` finalizes a task whose merge is confirmed but which never reached the complete lane. Two literal reads meant that on a renamed board it was never found, so a card whose commit is already on the base branch sat in review or hold indefinitely — merged work the board still shows as unfinished. ## The two redundant guards convert, they don't get deleted Both `t.column === …` checks were redundant while the query pinned the column. Under a resolved read they become the per-card verdict. Deleting them would have silently widened the sweep — the same trap called out in #2891. ## Carries the two shapes review established earlier in this series - Narrow when the card can answer, broad when it cannot (#2891). `resolveWorkflowIrForTask` substitutes the built-in IR rather than failing, so a card with an unreadable selection would otherwise be rejected by the very verdict that the project-scoped query had just admitted it under. It falls back to the project sets instead. - Deduped across the buckets (#2879), so a column carrying both a review role and the hold role cannot finalize one card twice. Both were review findings on earlier PRs in this series, applied here up front rather than waiting to be caught again. ## Revert results Each applied alone and the file re-run: \| conversion \| reverted → \| \| --- \| --- \| \| the resolved reads \| fails — the card is never listed \| \| the per-card review verdict \| fails — the renamed review lane does not match \| Observable is `resolveSelfHealingMergeTarget`, a private method called once per candidate, so the assertion sits downstream of both halves without a git fixture. A non-vacuous companion (merge-confirmed card in the wip lane → untouched) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly.	2026-07-30 17:22:41 -07:00
gsxdsm	cfb713bda1	notification: record the measured reason four wedge-progress ids stay literal, and un-red main's gate (#2882 ) Two small things, neither of which changes behaviour. ## 1. A conversion I attempted, measured, and reverted `hasProgressed` in the wedge-episode path names four column ids outright. I converted them to a resolved lane set. It broke an existing gate test — `task-wedge-notification.test.ts` → "sends one actionable push and mailbox message per active terminal episode": 1 message delivered, 2 expected. The note already in that file was right, and stronger than it read. The hazard is not specific to the resolve/claim ordering — it is any `await` added before the resolve. Column resolution needs one. `task:updated` listeners fire synchronously, so a re-wedge arriving close behind a recovery reaches `claim` while the first episode is still open, and the operator's second alert is dropped. Product change reverted; only the comment lands, now carrying the measurement and naming the failing test as the acceptance check for whoever owns the wedge-episode contract. Left counted, not exempted — the census should keep pointing here. Worth stating: the pre-existing note was a warning written speculatively. Attempting the conversion is what turned it into evidence, and the evidence says the blocker is real but sits somewhere else (per-task serialisation) than the note implied. ## 2. `main`'s gate is red, and not from this branch `pnpm test:gate` fails on a clean `origin/main` tree at `check-sql-column-literals`: ``` packages/core/src/team-analytics.ts: 3 site(s) now, baseline still allows 6 — re-record it ``` A reduction landed without re-recording the baseline in the same commit, which that check explicitly asks for. Reproduced on `origin/main` with my changes stashed, so it is not mine — but it blocks every open PR until recorded. Ratchets 31 → 28 sites across 14 files, downward only. ## The vacuous assertion this round (sixth) The first version of the reverted test passed with the fix reverted. `hasProgressed` is a three-clause OR, and the middle clause — status is a string and is not `failed` — is true for a recovered task on any board, so `status: "in-progress"` in the fixture satisfied it regardless of column. Same shape as the other five: something the code does anyway. Found by running the revert, not by reading it. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71 (green only with the baseline commit); `tsc` engine clean; notification suite 77 passed; `pnpm lint` and census `--strict` clean.	2026-07-30 16:59:03 -07:00
gsxdsm	ab15e5f9f7	docs(lanes): audit the three files the census points at with no reason attached (#2873 ) No source change. Every literal stays counted and none gets an exemption marker. What changes is that the census now points at these three with the analysis attached, instead of making each worker who reaches them re-derive it. Peers have already done this well for `notification-service.ts` — converted it, measured a real delivery regression, reverted, and left it counted with the reason. These three had nothing at all, and one of them is the highest-impact unowned site I found. ## `planner-overseer.ts` (3 guards) — REAL, and larger than three literals suggest On a renamed board `resolveWatchedStage` returns `null` for every card. `observeTask` returns early on a null stage, so no observation is recorded, no `overseer:intervention` entry is emitted, and `PlannerRecoveryController` — which consumes those observations — has nothing to steer, retry, or targeted-fix. The entire oversight loop is inert and silent about it, exactly like the self-healing sweeps whose queries returned empty arrays. Not mechanical, which is why it is flagged rather than converted. `resolveWatchedStage` is a pure sync function over a `Partial<OverseerTaskRef>` with no store and no task id, so the lane answer has to arrive as a parameter. Its only production caller, `observeTask`, is async and the monitor does hold a store — but it runs once per task per poll, so resolving inside it buys a workflow read per card on a timer. The shape that works is the one the board-load enrichment landed on in #2845: resolve at the poll, once, with an IR cache keyed by workflow, and pass the flags down. That makes it a change to `project-engine.ts`'s poll as much as to this file — a cost judgement about a periodic engine loop, not a rename. `columnFlags` is in the unwired-lane-parameter vocabulary, so whoever adds the parameter cannot leave it unwired. ## `async-mission-store-queries.ts` (1 of 3) — REAL `getTerminalTaskEvidence` tests only `column === "done"` for its `done` verdict, so a completed card on a renamed board falls through every branch to `{ kind: "nonterminal" }`. The caller is mission terminal evidence repair, so a finished feature reads as unfinished — a wrong verdict, not an error. The `archived` test beside it has the same defect, masked for soft-deleted rows by its `deletedAt` companion, which is why only the `done` half bites in practice. Takes a bare `QueryHandle`: no store, no task object, no workflow. The fix is a resolved terminal-lane set threaded in by the caller — the same shape `getLiveTaskColumn` needs, and it should land with it so the two cannot disagree about what "finished" means. ## `audit-ops.ts` (2) — one sentinel, one real, and they look identical ```ts if (state === "archived") // ← getLiveTaskColumn's MANUFACTURED value: do NOT convert if (pgRow.column === "archived") // ← a real board lane: convertible ``` The first compares against a string `getLiveTaskColumn` fabricates for an archived-or-soft-deleted parent, so converting it to `isArchivedColumnRole` would keep passing on the built-in board and start failing on a renamed one — a soft-deleted task's log would become writable. The second reads the task row, so a renamed archived column keeps accepting log writes; its `deletedAt` companion masks that in practice. Two lines that look the same and need opposite treatment is precisely the reason these notes are worth more than the count. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`, `@fusion/engine`) — clean - `planner-overseer.test.ts` — 50 passed - census `--strict` — exit 0, counts unchanged (that is the point) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 16:43:43 -07:00
gsxdsm	5cb731420c	test(engine): re-point the stale-spec ratchet at the improved guard (#2863 ) ## Main is red; this fixes it `executor-stale-spec-active-lanes.test.ts` — 2 failures on `main`: ``` × resolves the task's lifecycle columns before deciding the skip → expected source to contain 'const activeLifecycle = resolveLifecy…' × adds the wip, review and complete lanes to the active set ``` Nothing regressed — the guard got better and the ratchet didn't follow. ## What changed in the product The stale-spec skip used to build its active set from `resolveLifecycleColumns(...)`, taking `?.wip`, `?.review` and `?.complete`. That returns the first column carrying each trait, so a board with two wip lanes — or a review lane plus a second merge-blocking one — had only one of each recognised as active. A card in the other read as inactive, and its prompt file was treated as reclaimable. It now resolves the IR once and unions `columnsWithFlag` over five flags, which returns every column carrying each: ```ts const activeIr = await resolveWorkflowIrForTask(this.store, task.id); const activeColumns = new Set<string>(["in-progress", "in-review", "done"]); if (activeIr) { for (const flag of ["countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview", "complete"] as const) { for (const lane of columnsWithFlag(activeIr, flag)) activeColumns.add(lane); ``` That is a real fix to an arity bug, and the legacy trio stays unioned in for the documented reason: under-reporting active is the destructive direction. ## Why re-point rather than loosen This is a source ratchet in the `engine-no-blocking-shellout` style, and its entire value is that it fails on a revert. A substring loose enough to match both the old and new shapes would keep the file green through exactly the regression it exists to catch. So the assertions now name the new shape precisely, including the flag list, so dropping one of the five is caught here too. Mutation-verified: replacing the IR resolution with `undefined` fails the ratchet. It still does its job. ## Scope The file's own header notes this is not a behavioural proof — the guard sits inside `execute()` behind worktree and session setup a unit test has no business standing up, and it asks whoever next touches that scaffolding to add the end-to-end case. Re-pointing is maintenance; I have not taken on that harness here, and the note still stands. ## Verification - `executor-stale-spec-active-lanes.test.ts` — 4/4, fails on revert - `pnpm lint` — clean Test-only; no changeset. Found by running the full `engine-default` project after #2855 merged — the remaining `main` red is my own audit case, fixed by the pending #2857. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 16:00:41 -07:00
gsxdsm	b9785ec10f	fix(tests): stop the census guard reddening main on every legitimate conversion (#2856 ) ## This is the cause of four main reds today, not a fifth instance of them I have now fixed the census baseline on `main` three times (#2814, plus a withdrawn branch, plus watching #2811 and #2844 do the same). Rather than do it a fourth time, here is why it keeps happening. `census-baseline-corruption-guard` asserted: ```ts expect(result).toContain("every file matches its baseline exactly"); ``` That demands the committed baseline be byte-in-step with the tree at all times. It is not, by design. A conversion PR that removes guards leaves the tree holding fewer than the baseline allows, and the CLI treats that as the good case — it tightens the pin and exits 0. Measured directly: ``` simulated drop → EXIT ON DROP: 0 "The baseline file has been rewritten downward. COMMIT IT … in CI this write is discarded with the runner, which is why the gate is green and not silent." ``` So the ratchet was already happy while this test went red. Every conversion that did not also re-record the baseline turned `main` red for a condition that was never a defect. That is the mechanism behind #2783's markers, #2837's query split and two more — plus three collisions between workers racing to re-record the same file (#2811/#2814, #2844, and a branch of mine I deleted rather than open as a duplicate). ## The fix matches the guard's own stated intent Its comment says: "a guard that always fails is no guard" — its job is to prove the corruption diagnosis does not false-positive on a healthy file. A tightened baseline is healthy. So it now asserts what that needs: - the run succeeds — `execFileSync` throws on a non-zero exit, so a rise still fails before any assertion runs; a rise is real debt and must stay loud - the corruption diagnosis is absent - the outcome is one of the two healthy shapes the CLI can report ## Measured discrimination — all four cases \| scenario \| before \| after \| \|---\|---\|---\| \| drop (legitimate conversion) \| ❌ 1 failed — the false red \| ✅ 3 passed \| \| rise (real debt) \| ❌ fails \| ❌ still fails \| \| corrupt JSON \| ❌ fails \| ❌ still fails (case unchanged) \| \| unreadable file \| ❌ fails \| ❌ still fails (case unchanged) \| Only "somebody converted guards and has not re-recorded the pin yet" stops being a red. ## Scope Engine 11022 passed / 0 failed · gate 732 green · lint clean. Test-only. Does not change the CLI, the ratchet, or what `--strict` reports. Re-recording the baseline on a drop is still the right thing to do — it just stops being an emergency that reddens main and blocks everyone else while three people race to fix it. The baseline file is restored byte-clean after every simulation above (`git diff` verified). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Tests - Expanded baseline validation coverage to detect unreadable or invalid baseline data. - Added support for both exact baseline matches and successfully tightened baselines. - Improved health checks for baseline verification outcomes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 15:52:10 -07:00
gsxdsm	9d103fb171	test(engine): record the partial planning-continuation conversion the audit caught (#2857 ) ## The audit fired, as designed `workflow-planning-continuation-terminal-gap-live-e2e.pg.test.ts` went red on `main`: ``` × AUDIT — one of the two classifier call sites is converted, and the inner predicate is not → expected 2 to be 1 ``` That is the alarm working. #2799 built this case to fail in both directions — a new unconverted caller or a conversion of an existing one — precisely so a change here cannot land silently. Someone converted the inner half: ```ts if (!isPlanningContinuationTaskDispatchable(task, terminal)) { ``` ## What actually changed, and what did not \| site \| before \| now \| \|---\|---\|---\| \| `drainDuePlanningContinuations` \| passes `{ terminalColumns }` \| unchanged ✅ \| \| `resolvePlanningContinuationCandidate` → inner predicate \| not threaded \| threaded ✅ \| \| `selectActionablePlanningContinuations` \| passes nothing \| still passes nothing ❌ \| The behavioural case is untouched and still passes. A card in a renamed COMPLETE lane still comes back `actionable` and still re-enters plan-review, because `selectActionablePlanningContinuations` is the site that decides that. That combination is the useful shape of a partial conversion: the code reads more converted than before, and the operator-visible defect is exactly where it was. Without the behavioural case sitting next to the audit, the arity change would have looked like the fix. The fixer's own comment agrees, and names the narrow case the inner half reaches — a board declaring `done` as a non-terminal column id, where the outer check passes and the inner one skipped the continuation as "paused". Real, and not the case the characterization covers. ## The change Arity assertion updated `1` → `2`, with the reasoning recorded at the assertion and in the header. Updated deliberately, not loosened — arity is still the property asserted, so the next change to either site lands here again. Nothing else moved. ## Verification - suite — 4/4 - full live-PG E2E surface — 171/171 - `pnpm lint` — clean Test-only; no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 15:49:19 -07:00
gsxdsm	7784cb1fe8	self-healing: six recovery sweeps that never ran on a renamed board — and the guards widening their queries activates (#2838 ) Six self-healing sweeps did not run at all on a renamed board. Each is a recovery path — the thing that unsticks a card when something has already gone wrong. #2800 measured this class and could not fix it: a read happens before any task is in hand, so there is nothing to resolve a per-task lane from. `resolveProjectColumnsForRoles` (landed separately) is the seam that was missing. ## What was silently dead \| sweep \| what stayed broken on a renamed board \| \| --- \| --- \| \| `reconcileDoneTaskIntegrity` \| a landed card kept no commit sha, forever \| \| `recoverAlreadyMergedReviewTasks` \| a card whose merge succeeded stayed parked with `status: "failed"` \| \| `recoverStuckMergeDeadlocks` \| doubly blind — no candidates and no dependents \| \| `recoverInterruptedMergingTasks` \| a task interrupted mid-merge sat in `merging` indefinitely \| \| `recoverMergeableReviewTasks` \| a card ready to merge was never re-enqueued \| \| `recoverReviewTasksWithFailedPreMergeSteps` \| a card parked on a failed review step was never revived \| The census scored the `task.column === "..."` re-assertion inside each loop, never the query above it. Converting those comparisons would have dropped six counts and changed nothing — the loop bodies were already unreachable. ## The conversion shape — five parts, three of which review taught me Documented in `self-healing-sweeps-are-blind-on-a-renamed-board.md`, because the second sweep drifted from the first: I wrote it from the pre-review version and reproduced a flaw already fixed one commit earlier. 1. Read — project union, query each column, dedupe by id. Legacy ids unioned so a board mid-rename is not skipped. 2. Verdict — per card against its own workflow. Widening the read and widening the verdict are different decisions: a missed row is invisible, a wrong row is a write. Using the project union as a per-card test claims a card because some other board calls its column that role. 3. Provenance — the resolver substitutes the built-in IR rather than failing, so `length > 0` reads as "this card answered" when nobody did. It does not change the verdict (measured: identical) — it makes the unrepaired card reportable. 4. The log strings — widening a query invalidates every message naming the old literal. One logged `"stale merging task(s) in in-review"` after its read covered several lanes. 5. The guards the query ACTIVATES. ## Part 5 is the one that bites A guard downstream of a literal query is unreachable on a renamed board — and unreachable is indistinguishable from correct. That is why these sit unwired indefinitely. `recoverReviewTasksWithFailedPreMergeSteps` filters on `blocker !== "task has failed pre-merge workflow steps"` — an exact string match. Unwired, the blocker returns `"task is in 'checking', must be in 'in-review'"`, so widening the query alone would have made the sweep find every card and reject every card. Measured: 6 sweeps hold both a literal query and an unwired lane guard; 30 hold a literal query with no such guard. All six are named in the doc. One of the six was my own already-converted sweep. I widened `recoverAlreadyMergedReviewTasks` two commits before noticing its `getTaskHardMergeBlocker` was unwired — so for two commits it found renamed-board cards and declined them. The scan must run before widening; I did it after, and only caught it because the next sweep forced the question. `getTaskHardMergeBlocker` was the blind spot for four of the six: a wrapper, no lane parameter at all, every caller behind a literal query. ## Corrections to my own work, kept visible - The project union used as a per-card verdict — the flat-set mistake `project-lane-vocabulary.ts` warns about in its own header, which I quoted while writing it. - A provenance fix that was a no-op: measured identical verdicts in every state, revert passed its own new test, so it was thrown away rather than shipped with a comment claiming otherwise. - The second sweep reproducing the first's pre-fix shape. - Three assertions that were vacuous until the revert exposed them — including one where the write needed a real git repo, so `commitSha` could not distinguish accepted from rejected. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71 - `self-healing.test.ts` 412, query-blindness suite 12 - `tsc` on core and engine; `pnpm lint`; `check:changesets`; census `--strict` — all clean, each run explicitly - Every conversion revert-measured, each direction independently where a sweep has two (read and guard) ## Scope 42 queries remain, 5 of the 6 activation-risk sweeps among them. Each is per-sweep work — its own filter semantics, its own downstream guards, its own log strings — so they land one at a time with the pattern proven, never swept. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Self-healing workflows now work correctly on boards with renamed lifecycle columns. - Improved recovery for completed, in-review, interrupted, stalled, and failed-merge tasks. - Prevented tasks from being incorrectly classified using another workflow’s columns. - Added warnings when a task’s workflow lanes cannot be resolved. - Documentation - Expanded guidance on renamed-board recovery behavior and related diagnostic limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 15:38:10 -07:00
gsxdsm	68fd781eb8	test(engine): stop engine-default exiting 1 with zero failing tests (#2855 ) ## The problem On `main`, the whole `engine-default` project exits 1 while reporting 736 files passed and 0 failed: ``` Test Files 736 passed \| 1 skipped (737) Tests 9944 passed \| 14 skipped \| 1 todo (9959) Errors 9 errors ``` Nine unhandled rejections — `TypeError: this.store.listTasks is not a function`, all from `self-healing-db-corruption.test.ts`, raised after its tests had passed. That is worse than a plain failure: the lane is red, nothing names a test, and a genuine regression landing later arrives in an already-red lane where it reads as more of the same noise. ## Why the existing isolation didn't hold The suite stubs every other maintenance sweep so only the corruption path runs. But `runMaintenance` invokes the surfacing family like this: ```ts { name: "surface-in-review-stalled", fn: () => this.surfaceInReviewStalled(maintenanceSurfacing()) }, ``` `maintenanceSurfacing()` lazily calls `openSurfacingCycle()`, which does `await this.store.listTasks({ slim: false })`. Spying on `surfaceInReviewStalled` replaces the method, but the call site still evaluates its argument — so the shared cycle opened regardless, against a mock store that deliberately implements only what corruption surfacing needs. ## Two fixes that look right and are not Both tried, both reverted — recorded because each is the obvious next move: \| attempt \| result \| \|---\|---\| \| add `listTasks` to the mock \| 3 tests fail — the sweeps then run far enough to record audit events the assertions don't expect \| \| stub the 27 maintenance methods missing from the suite's lists \| 5 tests fail, and all 9 errors remain — `openSurfacingCycle` isn't among `runMaintenance`'s own calls, so the drift was a real but unrelated gap \| The second result is what located the fault: stubbing everything `runMaintenance` calls does not silence the rejections, so they don't originate there. The stack confirmed it — `SelfHealingManager.openSurfacingCycle` at `self-healing.ts:8015`. I should have read that stack before guessing twice. ## The fix Stub the cycle itself. `null` is exactly what `openSurfacingCycle` returns when the engine is paused, so every consumer already handles it, and the corruption-surfacing path this suite exists to test is untouched. ## Verification - `self-healing-db-corruption.test.ts` — 6/6, 0 errors, exit 0 - full `engine-default` project — 736 files passed, 9944 tests, 0 errors, exit 0 (was exit 1) - `pnpm lint` — clean Test-only change; no production file is touched, so no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 15:27:22 -07:00
gsxdsm	92d82b7a17	fix(guard): the unwired-lane check reported 0 because its question was too weak (#2852 ) A guard nobody has proven can fail is a number, not a check. This one was returning a clean `[]` while 18 real unwired lane declarations sat on `main` — including one it was specifically built to catch. ## The escape that found it `diffSnapshots` in the glasses plugin: ```ts opts: { notifyOnColumns: ReadonlySet<ColumnId>; completeColumnsByTaskId?: ReadonlyMap<...> } ... const completeColumns = opts.completeColumnsByTaskId?.get(task.id); const isComplete = completeColumns ? completeColumns.has(task.column) : task.column === "done"; ``` No file anywhere builds that map. The conversion was decorative — the literal decided every real poll, so on a renamed board the wearer is notified of every column transition except the card finishing, the one they care about. The name was already in the guard's vocabulary list, the declaration was exported and optional; it satisfied every condition the guard checks, and the guard said nothing. ## Three independent blind spots \| # \| blind spot \| why it mattered \| \|---\|---\|---\| \| 1 \| `SCANNED_PACKAGES` omitted `plugins/` \| plugins hold lane logic like anything else — this one resolves workflow IRs and decides what "finished" means \| \| 2 \| inline options-object types were not walked \| only bare parameters and named interfaces were. Whether a lane answer arrives as a parameter, an interface property, or an inline field is a style choice — a check evadable by a style choice is decorative \| \| 3 \| the mention rule was `source.includes(parameter)` anywhere \| satisfied by coincidence for any ordinarily-named parameter \| Fixing 1 or 2 alone would still have missed it: measured on `main`, the guard found 0 unwired across 1753 files, and 0 again across 2114 once `plugins` was added, because the shape was invisible too. ### On (3), I proved it on myself Renaming the unwired parameter from `completeColumnsByTaskId` to `completeColumns` — a better name, chosen for good reasons — silenced the guard instantly, because 15 unrelated production files declare a local called `completeColumns`. The check had not been satisfied; it had been switched off by a rename. That is exactly the failure the code comment two lines up condemns, committed one edit later. The fix is the cause, not a name blocklist: a file that never references `diffSnapshots` cannot be the thing that wires `diffSnapshots`'s options. Still deliberately loose — a co-occurrence test, not call-graph analysis — which keeps the low false-positive rate that makes the guard bearable while removing a false negative that scaled with how ordinary a parameter's name was. ## The 17 this uncovered Tightening (3) surfaced 17 further unwired declarations across core, engine and dashboard. Spot-checked, not assumed: `buildUnblockWeightMap` in `task-priority.ts` declares `terminalColumns` and `reviewColumns`, and the only files that pass either are its own tests — the production caller silently uses the built-in `{done, archived}` default. That is the inert-conversion shape this module exists to name. They span three other batches, so they are recorded as a ratcheted baseline in the shape `scripts/lifecycle-column-census.mjs` already uses here — keyed on `file + parameter` so an unrelated edit above them cannot manufacture a failure. A new one fails immediately; these can only leave the list. Listing them beats pretending for another week that they do not exist. ## The glasses fix `completeColumnsByTaskId` -> a flat `completeColumns` set, matching its sibling `notifyOnColumns` in the same options object, resolved once per poll by `notifier.ts` via `resolveProjectColumnsForRoles`. Project-scoped and not per task because this runs on a polling timer over the whole board — a per-card workflow read would scale with the board on every tick. Best-effort: a failed resolve leaves the diff on its documented legacy default rather than dropping a poll. Still gated by `alsoNotifyOnDone`, which the production caller passes as `false`, so it remains unobservable at runtime. Wired anyway: the day someone enables the flag the resolution must already be right — and now the guard will say so if the wiring is removed. ## Revert proofs (measured, one per fix) \| revert \| failure \| \|---\|---\| \| unwire `notifier.ts` \| baseline gains `plugins/…/diff.ts completeColumns` \| \| drop the inline-options walk \| "covers an INLINE options-object type" fails `expected [] to deeply equal [ 'completeColumns' ]`, and the repo scan loses the glasses entry \| \| drop the owner scoping \| the repo scan loses all 17 pre-existing entries \| \| restore `task.column === "done"` \| both new `diff.test.ts` cases fail \| The new diff cases assert both directions — a card in the resolved lane fires, and a card in the legacy `done` does not once the caller resolved other lanes. The second is what proves the resolved set replaces the default rather than being unioned with it. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` — clean - guard suite — 9/9; full glasses plugin — 188 passed across 19 files 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 15:24:33 -07:00
gsxdsm	89aaf341d0	the unwired-seam audit: 9 defects the census cannot see, incl. a reviewed card that cannot merge (#2820 ) Nine operator-visible defects in a class the census cannot see, plus the audit method that found them. The census scans for lifecycle-column comparisons. This PR is about guards that have no literal to find: a helper takes an optional resolved lane set, its own test passes it, the census entry is gone — and the callers pass nothing. A resolved seam nobody wired is indistinguishable from no seam at all. ## What was broken \| defect \| operator sees \| \| --- \| --- \| \| `getTaskMergeBlocker` unwired in `mergeTaskImpl` \| `Cannot merge FN-1: task is in 'checking', must be in 'in-review'` — a reviewed card cannot merge \| \| …and in the completion move \| `Cannot move FN-1 to done: …` — and cannot complete \| \| `isParkedTaskColumn` unwired ×2 (`agent-heartbeat`) \| a durable agent keeps claiming a parked card; Health Check renders it RUNNING \| \| `resolveLinkSyncColumnRoles` first-per-role \| link hygiene skips a second hold lane entirely \| \| `executor` active-task predicate first-per-role \| a card in a second wip lane reads as INACTIVE; its prompt file becomes reclaimable \| \| `isPlanningContinuationTaskDispatchable` partially threaded \| a board declaring `done` as non-terminal stalls its cards — stalled by a lane name \| \| `default-workflow-hooks:72`, `executor:2404` \| resolved gate admits the move, unresolved blocker refuses it \| ## The recurring shape, which is sharper than "a caller forgot an argument" Four sites resolve the lane and then re-ask with the literal, a few lines apart in the same function: - `task-artifacts-ops` resolves `completeColumn`, then asks the blocker with the literal. - `default-workflow-hooks:72` gates on `lifecycleColumns?.review`, then the literal. - `executor:2404` compares `resolveResumeLanes(…).review`, then the literal. - `resolvePlanningContinuationCandidate` applies the caller's terminal set, then delegates without it. Grep for the helper, not the literal. The literal is one function away, correctly annotated as a fallback — which is exactly why the census is blind to all of it. ## The arity trap, named and measured (six occurrences, one caught by review here) `resolveLifecycleColumns` answers "which column is the* hold lane?". A `.includes()`/`.has()` test asks "is this any hold lane?". Nothing distinguishes them — same types, no literal. A default-vs-renamed differential cannot catch it, because the default board declares one column per role and therefore cannot express the failing shape. It needs a structurally* different fixture. That is a sharper rule than "test both vocabularies", and it would have caught all six. Scanned: 12 candidate sites. 4 fixed · 3 blocked (2 on the inert sync IR reader; `triage:833` also query-shaped) · 1 needs a hook-contract change · 3 not defects (a returned tuple; an ordering-sensitive precedence list) · 1 false positive of my own scan. A sweep over all twelve would have broken the ordering-sensitive pair, delivered nothing at the sync-blocked ones, and "fixed" a site that was already correct. ## Two traps in fixing this class — I hit both here 1. The legacy id is a FALLBACK, not a member. Pre-seeding `"in-review"` admits a board that declares `in-review` as its WIP column — a card mid-implementation merges prematurely. A real resolved answer must replace the default. (Caught by review; it is the same unscoped-legacy-acceptance the glasses plugin's review caught earlier, which I had read and reintroduced.) 2. Two guards, one assertion. `toContain("must be in")` passed with `mergeTaskImpl` reverted, because the completion guard caught the card instead. The assertion now names the site (`Cannot merge` vs `Cannot move … to done`) so the two fail independently. ## Corrections I made to my own work, recorded rather than quietly fixed - My first PG test was vacuous three ways: `saveWorkflowDefinition?.()`/`setTaskWorkflowSelection?.()` do not exist (the `?.` swallowed both, so the task kept the builtin workflow), `updateTask({column})` does not move a card, and a two-node IR made every setup move illegal. Premise is now asserted, not assumed. - My doc claimed the audit was complete. It enumerated helpers, not every caller — `getTaskMergeBlocker` alone has 13 call sites. Corrected in place, with the still-unwired ones listed by file and line and a note to distrust any "audit complete" claim including mine. - A severity correction to another worker's E2E: `selectActionablePlanningContinuations` has no production caller, so its stated consequence is latent, not live. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71 - `tsc` on core and engine; `pnpm lint`; `check:changesets`; census `--strict` — all clean, each run explicitly - Every fix revert-measured; each has a non-vacuous companion. The two-hold-lane and repurposed-`in-review` cases exist because the default board cannot express those shapes. ## Deliberately not done, with reasons in `resolved-seams-nobody-wired.md` `isTaskReadyForMerge` (dead in production — wiring it would be the anti-pattern itself); `getTaskHardMergeBlocker` (3 of 4 callers are query-gated sweeps); `getInReviewStallReason` (needs a batch prefetch, not a per-task resolve — its callers decorate every task on every list read; the in-review stall badge is wrong on renamed boards until then); `default-workflow-hooks` planning/live-work sets (needs `DefaultWorkflowMoveContext` to carry the IR — a shared contract change).	2026-07-30 15:08:01 -07:00
gsxdsm	cea9637dfc	feat(census): split the query class into read-shaped and write — most of the remainder must not be converted (#2837 ) Splits the census's query class into read-shaped (convertible) and write (must not be converted), reported beside the existing total. On current `main`: ``` QUERY filters (column: "<legacy>"): 63 of those: 48 read-shaped (convertible), 5 writes (do NOT convert), 10 other ``` ## Why the single number misleads `column:` sits in an options-shaped object for both a source query and a write, so the existing definition-vs-query rule cannot separate them. The result reads as "dead reads to convert" — and after #2818 landed, 48 of the 63 are `self-healing.ts` and the rest are largely not convertible at all. Converting a write in this class is harmful, not merely pointless. `async-persistence.ts` soft-deletes with `.set({ column: "archived", deletedAt, … })`, and `getLiveTaskColumn` returns `"archived"` as a sentinel for any soft-deleted row — the write and the sentinel have to agree. A sweep that "finished the query class" by converting all 63 would break live-column resolution for every deleted task. That is the same shape #2808 flagged for `recoveryRehome` moves. Two of the census-invisible classes now have members that must not be fixed, and in both cases the count alone cannot tell you which. ## Reported, not ratcheted `properties.query` and `queryByFile` are byte-identical, so the pinned baseline does not move and no open PR's Lint changes. The split is one extra line of output. Changing what a ratchet enforces is the owner's call; improving what it says is not. Same line I drew when making marker-only failures legible without loosening them. ## Honest limit Read-shaped is a better filter than the raw count and still not a verdict. `auto-merge-finalization.ts:242` is classified read-shaped and must NOT be converted — its own comment records that it is `getTaskHardMergeBlocker`'s review-eligible sentinel, deliberately not re-keyed. Nothing mechanical would catch that; only the comment beside it does. The split narrows a haystack to a readable list; it does not decide the list. ## Verification 4 new cases — a `listTasks` filter counts read, a `.set()` tombstone counts write, IR node definitions stay excluded from the class entirely (the pre-existing rule must keep working), and the pinned total is unchanged by the split. Revert proof: dropping the write branch fails the tombstone case. Census's own suites 87 passed, gate 161 / 13 / 487 / 71, lint clean, `--strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 14:29:01 -07:00
gsxdsm	a6af3188df	fix(core): startup recovery deadlocked on its own per-task lock (#2809 ) ## The bug `recoverStaleTransitionPendingImpl` runs its whole per-task body inside `store.withTaskLock(id, …)`. On the PostgreSQL arm it then read the task with `store.getTask(id)` — and `getTaskImpl` opens with `store.withTaskLock(id, …)` too. The per-task lock is non-reentrant. This codebase states that invariant in prose in two other files: > "nesting inside `withTaskLock` would deadlock since the lock is non-reentrant" — `branch-and-pr-entities.ts:561` > "because the per-task lock is non-reentrant" — `workflow-ops.ts:464` So the sweep waited forever on a lock its own frame was holding. PostgreSQL-only — which is every production install. The SQLite arm on the very next line reads through `readTaskFromDb`, a lock-free row read. The backend-mode port swapped only the PostgreSQL arm to `getTask`. The fix restores a lock-free read (`readTaskRow`) on that arm; nothing else changes. ## Why it survived until now The branch is entered only when a stale marker names a plugin hook the trait registry still knows (`hasSurvivingPluginHook`). Three nearby cases all miss it: \| marker \| path \| \|---\|---\| \| none \| the row is never scanned \| \| only `default-workflow:postCommit` \| `hasSurvivingPluginHook` false — marker just cleared \| \| names an uninstalled plugin hook \| reconciled away as degraded; nothing survives to re-run \| \| names a registered plugin hook \| reaches the in-lock read → deadlock \| Those first three are what the existing tests cover. The fourth is precisely the state a crash mid-hook leaves behind. All four are asserted in the new suite so the path cannot be re-narrowed and called covered. ## Impact This sweep runs at startup. A task left with such a marker deadlocks startup recovery — and because it deadlocks while holding the task's lock, that task is also left permanently unlockable. ## How it was found, including a correction By bisection, not by reading. An earlier attempt of mine to drive this recovery reported that "the sweep never returns". That was wrong in a way worth recording: the sweep returns fine in three of the four cases, and generalising the one hang to the whole function is what hid the actual trigger across several sessions. Narrowing case by case — empty store, plain task, default-only marker, unknown-hook marker, registered-hook marker — put the fault on one line. ## Verification - Mutation-verified against the real defect. With the fix reverted, the regression case fails by name — `recoverStaleTransitionPendingImpl did not settle within 8000ms — deadlock` — while the other three stay green. That is the actual pre-fix behaviour, not a simulation of it. - Every case is timeboxed on purpose: a deadlock otherwise surfaces as a suite-level timeout naming no case, which is useless for locating the fault. The deadline is not a flake knob — the fixed code settles in ~150 ms and the broken code never settles, so there is no value in between to tune. - A vacuity guard (no markers → scans nothing) so a change that stopped listing marked rows can't leave the other cases green. - `pnpm test:gate` — exit 0 - full live-PG E2E surface — 152/152 - `pnpm lint` — clean Changeset included (`patch`, category `fix`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:58:02 -07:00
gsxdsm	109204c590	fix: the query class — three sweeps that never ran on a renamed board (#2818 ) Three sweeps that never ran at all on a renamed board, plus the shared answer the rest of the class needs. Consolidated from three handoff branches so the helper appears once. #2811 merged, so this is my only open PR. `#2800` measured this class and shipped evidence deliberately without conversions: `listTasks({ column: "<literal>" })` filters in the store, so on a renamed board the read returns an empty array and the sweep it feeds does nothing. The census scores the comparison inside the loop, never the query above it. ## What was broken \| file \| census count \| what actually happened on a renamed board \| \|---\|---\|---\| \| `backlog-pressure-reporter.ts` \| 0 \| both reads empty, ratio computed as 0/0 — the alert never fired, on a board that may be under exactly the pressure it reports \| \| `stale-task-reporter.ts` \| 0 \| both reads empty — no stale-task signal ever raised, where work is most likely sitting unnoticed \| \| `restart-recovery-coordinator.ts` \| flagged \| sweep never ran — an engine restart left interrupted tasks stuck with no requeue \| Two of the three have a census count of zero. They contain no lifecycle comparison at all, so they have never appeared in the backlog, in a per-file list, or in any "N → 0" claim — and were completely inert. A file at zero is not evidence of anything. ## The shared answer, and what it is not Every existing resolver answers a per-task question. A query has no task in hand, so it needs the project-level one: every column any workflow declares for a role, unioned with the legacy ids so a board mid-rename still finds rows under the old ones. The set is never empty, so a caller cannot accidentally query nothing. The header states what it is not: answering a per-card question from the union would mark a card as review because some other workflow calls its column review — the flat-set mistake this program has made four times. ## The finding that generalises: the query is rarely the whole defect `stale-task-reporter` still reported zero after the query was fixed — `getTaskAgeStalenessSignal` defaults to the legacy pair, so a card the query now returned was refused inside the signal. Converting only the query would have looked like a fix and changed nothing. That is a caveat on #2800's approach, offered as refinement rather than correction: asserting the query ARGUMENT is right when pinning a known defect (the outcome is 0 either way) and insufficient when proving a fix, because the outcome is the only thing that distinguishes a real conversion from a deeper one. All three conversions here assert outcomes. `restart-recovery` had three layers — query, a redundant re-assertion (deleted; a test pins the `paused` guard it did contribute), and a move destination that was already resolved but whose warning comment was stale. A stale warning is its own hazard: it told the next reader a defect existed where none did. ## Verification - helper 8 passed · three reporter/coordinator suites 29 passed - `pnpm test:gate` 161 / 13 / 487 / 71 · lint clean · `--strict` exits 0 · four `tsc` targets clean - each conversion revert-proven independently; the failing case is named in each test header ## Two mistakes worth recording The helper's own test caught a bug in it. My first draft wrapped the definition loop in one `try`, and `parseWorkflowIr` validates rather than parses — one malformed row would have returned legacy-only lanes for every workflow, indistinguishable from the bug it exists to fix. Now isolated per definition. I clobbered the core barrel by taking `index.ts` wholesale from a handoff branch, dropping two exports `main` had added since; three packages stopped compiling. Taking a file from another branch takes its whole contents, including what is now stale — for a barrel that is nearly always wrong. Re-applied as a single edit on top of `main`. ## Not included `self-healing.ts`'s 49 — actively owned and mid-conversion; an outside refactor there produces conflicting halves of one sweep. `project-engine.ts` (7) and `executor.ts` (2) need their own read of what each sweep does with the rows, which these three are the argument for. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:55:10 -07:00
gsxdsm	1824c04584	fix(core): restore an archived card to the lane it came from (#2832 ) ## What Fixes the defect #2824 measured. That PR's characterization cases — merged and asserting the wrong-but-real behaviour — are flipped here to the correct lanes, which is what they were written to do. ## The bug ```ts // archive-lifecycle-2.ts const preArchiveColumn = task.preArchiveColumn ?? "todo"; ``` `preArchiveColumn` has no database column. It exists on the `Task` type and in the archive snapshot, and nowhere else — so the in-place restore cannot carry it, `store.getTask(id)` reads a live row that never had it, and the literal decided the destination for every unarchive that has ever run. \| board \| what happened \| \|---\|---\| \| default \| `todo` is declared, so the resolver returned it. Restores landed in the queue and looked right. \| \| custom \| `todo` is declared nowhere, so the resolver took its "no usable history" branch and returned the complete lane. A card archived mid-implementation came back marked finished. \| That coincidence is why this survived three separate fixes to `resolveUnarchiveTargetColumnImpl` — a `?? "done"` that invented a column, an `isColumn` legacy-enum gate, and the same gate one function over. Every one was correcting how the resolver interprets a value that never arrived. ## The fix is two halves, and either alone does nothing 1. Capture — `taskToArchiveEntryImpl` records `task.column` into the snapshot. That is the last place the original is still in hand, since the entry's own `column` is set to `"archived"` on the line above. 2. Read — `unarchiveTaskImpl` reads the snapshot it already loaded, not the restored row. I shipped half of this first and watched the destination stay wrong, which is how I found that the field has no row to live on. Mutation matrix: \| state \| result \| \|---\|---\| \| both halves \| 5/5 pass \| \| capture only (read reverted) \| 3 fail \| \| read only (capture reverted) \| 3 fail \| I also tried carrying it through `restoreTaskFromArchive`'s row update — that fails to typecheck, which is the proof that no such column exists and the snapshot is the only source. ## Behaviour changes, deliberately - Custom boards — a card returns to the lane it was archived from instead of appearing finished. - Default board — a card archived from `done` restored to `todo` under the literal and now restores to `done`. Returning finished work to the queue was the fallback showing through, not a rule anyone chose; the resolver's own branches say a card archived from a declared column goes back to it. ## One expectation of mine was wrong, and the resolver was right I expected a card archived from the review lane to return to hold. It returns to the review lane, and that is correct: `.review` is derived from the `mergeOrchestration` flag, not from `human-review`. The fixture's review column declares `human-review` + `merge-blocker` only, so it is not a `.review` lane to the resolver — just a declared column with usable history. The case now asserts that with the reasoning attached, so the next reader does not "fix" it back to hold. ## Verification - unarchive suite — 5/5, mutation matrix above - `pnpm test:gate` — exit 0 - full live-PG E2E surface — 164/164 - `pnpm lint` — clean Changeset included (`patch`, category `fix`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:54:53 -07:00
gsxdsm	654d28c104	test(engine): live-PG coverage for the timing role — and it is correct (#2828 ) ## What One new live-PostgreSQL E2E suite, 2 tests. No production file is touched — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-timing-trait-live-e2e.pg.test.ts` ## This one is a clean bill of health Coverage for the last lifecycle role that had none — and the seam turns out to be correct. Recorded deliberately: a working conversion with no test is one refactor away from a silent regression, and this program should be able to say which seams are right, not only which are broken. `cumulativeActiveMs` accrues while a card sits in the WIP lane, and both halves of the segment boundary resolve that lane by role: ```ts const isWip = (column) => inRole(column, ctx.lifecycleColumnSets?.wip, ctx.lifecycleColumns?.wip, "in-progress"); ``` Note the literal at the end. That is the same optional-parameter shape this series found broken at four other seams — `shouldHoldActiveFileScopeLease` (#2795), `evaluateParkedAgentTaskLink` (#2798), `resolvePlanningContinuationCandidate` (#2799), `hasAutoHealableVerificationBufferFailure` (#2802) — where a caller failed to supply the resolved answer and the literal silently took over. Here the move path does supply it. Measured on a live store: ``` default: after exit cumulativeActiveMs=84 renamed: after exit cumulativeActiveMs=75 <- accrues on `building`, not just `in-progress` ``` ## What it guards If the move path ever stops populating the resolved columns, the literal takes over and a renamed board's cards accrue exactly zero active time — silently, because zero is a legitimate value for a card that has not run. The operator sees no error, just wrong numbers on every custom board. Mutation-verified against precisely that regression: blinding the resolved answer (`inRole(column, undefined, undefined, "in-progress")`) fails the renamed case and leaves the control green. This is a guard with demonstrated power, not a test that happens to pass. ## Both boundaries are asserted `cumulativeActiveMs` is `0` while the card is still in WIP and only accrues when it leaves; `firstExecutionAt` is stamped on entry. A test that checked only the final number would pass against an implementation that accrued at the wrong boundary, so both are pinned. ## Verification - new suite — 2/2 passed, mutation-verified - full live-PG E2E surface — 161/161 - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:36:22 -07:00
gsxdsm	e5c9ea3870	fix(core): resolve the task's own terminal node in the node-override guard (#2812 ) ## What Fixes the defect #2793 measured but deliberately did not fix. That PR's two characterization tests are flipped here to assert the correct behaviour — which is what they were written to do. ## The bug `updateTask({ nodeId })` passes through `validateNodeOverrideChange` twice, and both calls were wrong in different ways: \| call \| how it answered "is this terminal?" \| \|---\|---\| \| `branch-and-pr-entities.ts:568` \| `resolveTaskWorkflowIrSync` — the default workflow for every task under PostgreSQL \| \| `task-update.ts:53` \| no options at all → `defaultIsTerminalNodeId`, the bare literal `nodeId === "end"` \| On a board whose terminal node is not named `end`, the FN-7641 guard inverted in both directions: - an override to a non-terminal node that happens to be named `end` was rejected with a merge-proof error about finalizing a card the operator was not finalizing; - an override to the board's real terminal node was written verbatim, no error, card unadvanced — the silent no-op FN-7641 exists to prevent. ## The fix A new `isTaskTerminalNodeIdAsync` resolves the task's own workflow, with the identical literal fail-soft for an unresolvable one. Both call sites use it — pre-resolved, because `validateNodeOverrideChange`'s callback is synchronous and it asks the question at most once. Nothing forced the sync call at either site: both frames are already `async` and already awaiting. That is the same finding as #2809's review, one file over. The sync helper is deleted, not kept as a fallback. Keeping both would re-create the half-conversion this program keeps finding — one caller resolved, one not, and no way to tell from a call site which it got. `branch-and-pr-entities.ts` also leaves the sync-resolver call-site allow-list (ratchet green, 3/3), the second of the six allow-listed sites to close. ## Why both halves were needed — and how that is proven #2793's mutation matrix showed the rejected-`end` case is over-determined: both guards independently called it terminal, so correcting either one alone changed nothing an operator could see. That is why fixing only the allow-listed sync site would have looked like progress and delivered none. Re-measured here, on the fixed tree: \| state \| result \| \|---\|---\| \| both guards fixed \| 3/3 pass \| \| inner guard reverted to no-options \| 1 fails \| \| outer guard reverted to the literal \| 1 fails \| ## Tests #2793's two cases now assert the fixed behaviour and keep their reasoning: - the non-terminal `end` override is written, and the card stays in review — a routing change, not a finalize; - the real terminal `finish` override is refused without merge proof, and the field is not written on the way to refusing. The fixture-integrity case (`finish` is the end node, `end` is not) is unchanged — it is what stops both assertions passing for the wrong reason. ## Verification - terminal-node suite — 3/3, both-halves matrix above - sync-resolver call-site allow-list ratchet — 3/3 - `pnpm test:gate` — exit 0 - full live-PG E2E surface — 151/151 - `pnpm lint` — clean Changeset included (`patch`, category `fix`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:15:04 -07:00
gsxdsm	7c30b54255	test(engine): live-PG evidence that restore lands every custom-board card in DONE (#2824 ) ## What One new live-PostgreSQL E2E suite, 5 tests. No production file is touched — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-unarchive-target-live-e2e.pg.test.ts` ## The finding I went looking for coverage of the one lifecycle role that had none — `archived` — in the function with the worst track record in this program. `resolveUnarchiveTargetColumnImpl`'s own comments record three separate defects fixed on those few lines: a `?? "done"` that invented an undeclared column, an `isColumn` legacy-enum gate that rejected every renamed id, and the same gate one function over that dropped a renamed board's stored history on read. All three were reasoned from source. None had a live-store test. With one, the path is still broken — and the cause is upstream of everything those fixes touched: ```ts // archive-lifecycle-2.ts:441 const preArchiveColumn = task.preArchiveColumn ?? "todo"; ``` `preArchiveColumn` is never written. Across `packages/core`, every occurrence reads it or copies it through — into the archive entry, back out of it, through serialization. Nothing ever sets it from `task.column` when a card is archived. Measured on both boards: ``` default: after archive column=archived preArchiveColumn=undefined -> resolver target=todo renamed: after archive column=archived preArchiveColumn=undefined -> resolver target=shipped ``` So the fallback fires for every restore that has ever happened, and the two boards diverge on what `"todo"` means to each: \| board \| what happens \| \|---\|---\| \| default \| `todo` is declared, so the resolver returns it. Every restore lands in the queue — right for a card archived mid-implementation, wrong for one archived from `done`, and it looks right. \| \| renamed \| `todo` is declared nowhere, so the resolver takes its "no usable history" branch and returns the complete lane. Archive a card mid-implementation, restore it, and it comes back marked finished. \| That coincidence on the default board is why this survived three rounds of fixes to the resolver: they were correcting how it interprets a value that never arrives. ## Evidence discipline - Observed state — the persisted `column` after a real `archiveTask` + `unarchiveTask` against a real store and real stored workflows. - Characterization: the cases assert today's wrong-but-real behaviour so the defect is executable rather than argued, and they are written to flip when the write is added. - The default-board control is what shows the coincidence, not just the failure. - Mutation-verified: changing the hardcoded fallback from `"todo"` to the renamed hold id fails all five cases — every assertion binds to that literal, which is the claim. ## The fix, proposed rather than included One line: persist the card's column when archiving, so `preArchiveColumn` carries real history and the resolver — already correct — can use it. I did not include it because it also changes default-board behaviour: a card archived from `done` currently restores to `todo` (the fallback), and with real history it would restore to `done`. That is the better outcome, but it is a visible placement change for every existing install, and that decision belongs to whoever owns archive semantics rather than to an evidence PR. The five cases above are the acceptance criteria for it — the three renamed expectations become the lane the card was actually in. ## Verification - new suite — 5/5 passed, mutation-verified - full live-PG E2E surface — 159/159 - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:14:47 -07:00
gsxdsm	9a5155c71a	fix(tests): main red — a census source guard that a COMMENT could invert (#2825 ) ## Red on main ``` lifecycle-column-census > the baseline can always be re-recorded > writes the baseline BEFORE the rise check can exit AssertionError: expected 19345 to be greater than 26374 ``` Read literally, that says the CLI now runs its rise check before the `--update-baseline` write — which would break the one command whose entire job is re-recording, and would be a genuine bug worth stopping for. It does not. The order in code is correct and unchanged: \| \| line \| \|---\|---\| \| `if (updateBaseline) { … writeBaseline() … process.exit(0)` \| `scripts/lifecycle-column-census.mjs:487` \| \| `"column-guard count ROSE"` + `process.exit(1)` \| `:510` \| ## What actually moved was a comment Line 359 explains this exact failure mode and quotes the marker verbatim: > …`"column-guard count ROSE"`, which is the opposite of what happened and sends the reader looking for… So `cli.indexOf("column-guard count ROSE")` found the prose, 7000 characters before the branch it was meant to locate. A guard that a comment can invert is not measuring control flow. And the honest-looking fix — reword the comment — silently re-arms the same trap for whoever explains this next. `cliSource()` now strips comments before indexing. The same defence is already used by `archived-column-gate-parity.test.ts`, for the same reason: notes documenting why a literal is dangerous have to mention the literal. ## Kept, not deleted The end-to-end block below these does cover the contract — it drives the real CLI and asserts exit code, baseline content and printed output, and its own comment names the ordering bug. It would have been defensible to delete the two source-text cases as redundant. I kept them because two guards at different levels is the point: e2e proves the behaviour, these locate the branch that provides it. They only needed to stop being defeated by prose. ## Evidence The real ordering bug — make a rise exit before the update branch writes — fires all three: \| guard \| failure \| \|---\|---\| \| source order \| `expected 9454 to be greater than 9505` \| \| slice / uniqueness \| `expected 10433 to be -1` \| \| end-to-end \| `expected 1 to be +0` (exit code) \| My first mutation attempt was invalid and I nearly reported it as evidence: it moved the block by line range, mangled the file, and both markers disappeared — the resulting `-1`s look like a firing guard but prove nothing. A mutation that corrupts its target is not evidence that a guard works. Engine 10991 passed / 0 failed · gate 732 green · lint clean. Test-only; the CLI is restored clean. ## Note on duplicated effort #2811 and my #2814 both re-recorded the census baseline for #2783's rise, concurrently. No harm done — but this file is now a fleet-wide contention point, and the per-file baseline shape exists precisely to avoid that. Worth one owner for census/ratchet fixes rather than whoever notices first. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:14:36 -07:00
gsxdsm	8c79da364c	fix(engine): FN-8356's duplicate-marker cleanup was inert on a renamed board (5 call sites) (#2827 ) Fourth unowned finding picked up after both batch PRs (#2783 core, #2785 engine) merged without addressing their reports. Completes the `isNearDuplicateCanonicalInactive` seam alongside #2823, which fixed the sixth (core) site. ## The defect All five engine call sites — `self-healing.ts` ×2, `triage.ts` ×3 — called the predicate without the canonical's resolved column flags, so it fell back to the legacy `done`/`archived` ids. A canonical resting in a renamed complete column (`shipped`) read as still active, so every "the canonical is inactive, so clear the marker" branch failed to fire. The user-visible result is precisely the stranding FN-8356 was written to remove: a card keeps its "Needs your decision" duplicate badge pointing at work that shipped days ago, and no decision can resolve it — the detail banner deliberately offers none for an inactive canonical. ## Measured With the fix reverted, exactly one case flips: ``` ✓ clears the FN-8353-shaped hidden decision for every inactive canonical state × renamed vocabulary: clears the decision for a canonical resting in a RENAMED complete column ✓ renamed vocabulary: leaves the decision alone while the canonical is still in the WIP lane ✓ leaves active canonical decisions, user pauses, unrelated reasons, non-marker sources untouched Tests 1 failed \| 3 passed (4) ``` With it: `Tests 4 passed (4)`. ## Wiring is proven separately from behaviour A behaviour test on one call site says nothing about the other four — that is the failure this whole lane keeps re-finding, so I did not rely on it. With #2822's barrel-import fix applied locally, the seam gate's staleness check fails both engine allow-list entries as "now supplied" — it can no longer find an omitting call site in either file. That is the proof for all five. Consequence worth flagging: when the second of #2822 / this PR lands, the two engine entries in #2822 must be deleted. CI fails until they are, by design — the exemption cannot outlive its fix. ## Both negatives included A canonical still in the WIP lane must not have its decision cleared, under each vocabulary. Resolving real flags must not degrade into "every column is terminal", which would dismiss a duplicate decision the operator has not made yet. ## Design note The helper is module-private in each file rather than shared. `findColumn` is already duplicated exactly this way in `hold-release.ts`, `merge-trait.ts`, and `workflow-capacity.ts` — following the established shape beat adding a cross-module abstraction for a bug fix. ## Verification `pnpm test:gate` green · 27 triage suites / 387 tests green · engine `tsc` 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:04:58 -07:00
gsxdsm	bcc8bf17c1	fix(core): brand the unregistered built-in workflow fallback (#2815 ) > Corrected after opening. The first version of this PR claimed the id cross-check reported `"default"` for every authored workflow and stranded cards in triage recovery forever. That was wrong, and the correction is below in full. The reachable defect is the branding hole; removing the id check is correctness, not a live bug fix. ## The bug (reachable) `resolveWorkflowIrById` has four ways to substitute the default coding IR. Three brand the result via `markFellBack`. The fourth did not: ```ts if (isBuiltinWorkflowId(workflowId)) { const builtin = getBuiltinWorkflow(workflowId); const ir = builtin?.ir ?? defaultCodingWorkflowIr(); // <- unmarked substitution ``` An id that looks built-in but is not registered — a workflow removed between releases, a typo'd selection — lands there and silently gets the default coding IR. `resolveWorkflowIrForTaskWithProvenance` then reports `source: "selection"` for it, handing a caller the default board's graph under the selected workflow's name. That is precisely the lying signal the API exists to prevent. Found by review on this PR (thanks — see the thread), not by me. ## The correction to my own claim I also deleted an id cross-check that ran after the marker check and reported `"default"` when the resolved IR's `id` differed from the requested workflow id. I justified that by saying it misfired for every authored workflow, because `createWorkflowDefinition` stores an IR verbatim while minting `WF-NNN` separately: ``` store workflow id = WF-001 stored ir.id = custom:prov PROVENANCE source = default resolved ir.id = custom:prov <- the CORRECT IR, called a guess ``` That measurement is real but it came from this suite's own fixture. Neither `WorkflowIrV1` nor `WorkflowIrV2` declares an `id` field. An editor-authored workflow carries none, so `resolvedId` is `undefined` and the check passed it as `"selection"` — the misfire never reached `triage.ts`'s post-U11 intake recovery, the one production consumer. My "declined forever, not deferred" claim does not hold. Removing the check is still right: it is unreliable (it interrogates a property the IR types do not declare, and when one is present it is the author's id, unrelated to the store-minted row id) and redundant (all four substitutions are now branded, and the marker is checked first). But it is a cleanup, not a fix — and saying otherwise is how a narrow change gets backported as a critical one. ## Tests \| case \| asserts \| \|---\|---\| \| unregistered built-in id \| `source: "default"` — the reachable defect \| \| missing definition \| still `"default"` \| \| no selection \| still `"default"` \| \| IR carrying its own id \| `"selection"`, with the id mismatch asserted explicitly so it can't pass for the wrong reason \| \| the resolved IR is the task's own board \| contains the renamed hold column, not a default-board id \| Mutation-verified: removing only the branding fails the unregistered-builtin case and nothing else — which shows it covers that specific hole rather than overlapping the others. Restoring the id check fails only the carries-its-own-id case, leaving both fallback cases green, which shows the deletion did not widen trust. ## Blast radius `triage.ts:1211` is the only production consumer of the provenance result across core, engine, dashboard and CLI. Every other `.source ===` hit is an unrelated field on an unrelated type; the two index hits are re-exports. ## Verification - new suite — 5/5, mutation-verified in both directions - `pnpm test:gate` — exit 0 - `pnpm lint` — clean Changeset included (`patch`, category `fix`), rewritten to describe the branding fix rather than the overstated one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 13:01:54 -07:00
gsxdsm	2ccd78abbc	fix: main is red on the lifecycle ratchet — re-record the census baseline (#2811 ) `main` is RED on the lifecycle ratchet right now. `node scripts/lifecycle-column-census.mjs --strict` exits 1 on pristine `origin/main`, which is the `Lint` job's Lifecycle-column ratchet step — so every open PR fails Lint until this lands, regardless of its own contents. Verified on a detached checkout of `origin/main`, not on a branch of mine. ## Cause Eight `DELIBERATE-LITERAL` markers were added across seven files without re-recording the baseline: ``` packages/core/src/task-move-disposer.ts (in-progress, todo) packages/core/src/task-store/archive-lifecycle-2.ts (archived) packages/dashboard/src/github-tracking-comments.ts (done) packages/dashboard/src/gitlab-tracking-comments.ts (in-progress) packages/dashboard/src/server.ts (archived) packages/dashboard/src/task-planner-chat-context.ts (done) packages/dashboard/src/test/mockCoreEngine.ts (in-review) ``` Adding a marker RECLASSIFIES a site (column-guard → deliberate), so the tracked deliberate totals move and `--strict` fails until the baseline records the new shape. It is the same mechanism that turned #2775 red earlier today — a marker landing without its baseline — which is worth noting because it has now happened twice from different PRs. ## The fix Baseline re-recorded, nothing else. Zero source changes; the diff is one derived file. - `--strict` exits 0 - `pnpm test:gate` — 161 / 13 / 487 / 71 - `pnpm lint` clean ## Worth a follow-up by whoever owns the ratchet The failure is structural rather than careless: a PR that adds a marker is doing the right thing, and the baseline requirement is only discovered when CI goes red — after merge, for everyone else. Two options, neither of which I am taking unilaterally on a red-main fix: 1. have `--strict` treat a marker-only reclassification as an accepted rise (it is not new debt — the count of unconverted guards goes down); 2. or fail the PR that adds the marker, by comparing against the base ref rather than the recorded baseline — the machinery for that already exists in this script. I would take (1): a marker is the documented way to close a site, and requiring a second mechanical step to record it is a trap that catches good behaviour. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Bug Fixes * Improved lifecycle census error messages to distinguish genuine increases in column-guard debt from reclassified deliberate literals. * Added clearer remediation guidance for reclassified results, including when to update the baseline. * Updated lifecycle census baseline mappings to reflect current classifications. * Tests * Added coverage for unchanged baselines, genuine guard-count increases, and marker-only reclassification scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 12:40:05 -07:00
gsxdsm	9fab32e1d9	fix(tests): 5 engine reds from #2783 — a stale census baseline and a turn-counting test (#2814 ) ## Red on main #2783 (batch-core) landed and put 5 failures on `main`. Both causes are the bookkeeping half of correct changes, not defects in them. ## 1. The census baseline — 4 failures `census-baseline-corruption-guard` plus 3 `lifecycle-column-census` ratchet cases, all downstream of one thing: ``` lifecycle-column-census --strict: column-guard count ROSE packages/core/src/task-move-disposer.ts (DELIBERATE-LITERAL: in-progress): 0 -> 1 packages/core/src/task-move-disposer.ts (DELIBERATE-LITERAL: todo): 0 -> 1 packages/core/src/task-store/archive-lifecycle-2.ts (DELIBERATE-LITERAL: archived): 0 -> 1 packages/dashboard/src/github-tracking-comments.ts (DELIBERATE-LITERAL: done): 0 -> 1 packages/dashboard/src/gitlab-tracking-comments.ts (DELIBERATE-LITERAL: in-progress): 0 -> 1 packages/dashboard/src/server.ts (DELIBERATE-LITERAL: archived): 0 -> 1 ``` The rise is legitimate. #2783 annotated documented fast-path literals — e.g. `task-move-disposer.ts`'s "a fast path, not the guard … the actual lane decision is the RESOLVED membership test inside this block" — and the census tracks marked literals per file. Re-recorded with `--strict --update-baseline`; the same run also tightened 20 entries whose counts dropped, so this moves the ratchet down as well as up. ## 2. The disposal-order test — 1 failure ``` executor-user-cancel > re-dispatch (task:moved → in-progress) awaits prior disposal before execute() AssertionError: expected -1 to be greater than 2 ``` `-1` reads like the re-dispatch was dropped. It was not — that would be a real cancel-race bug, so I checked before touching the test: ``` PROBE_MICROTASK callOrder=["abort-started","abort-resolved","dispose","execute"] PROBE_AFTER_TIMER callOrder=["abort-started","abort-resolved","dispose","execute"] ``` Correct order, reached once drained, unchanged after a real 50ms timer. The test drained exactly two microtask turns and #2783's disposer refactor added await hops, so `execute` had not been recorded yet. A fixed turn count encodes today's await depth into the test: any added `await` on the product path fails it for a reason that has nothing to do with the invariant. It now waits on the outcome via `vi.waitFor`. The ordering assertion is untouched and is still the point. ## Evidence \| mutation \| result \| \|---\|---\| \| `execute` never recorded (stands in for a dropped re-dispatch) \| fails — `waitFor` times out \| \| `execute` observed before `dispose` \| fails — `expected 'execute' to be 'dispose'` \| \| baseline: fresh `--strict` run \| "every file matches its baseline exactly" \| Engine 10985 passed / 0 failed (was 5 failed) · gate 732 green · lint clean. ## Method note, against myself I pre-flighted #2783 and reported it clean — but I ran only `@fusion/core` and the dashboard `api` group, because that is what the diff touches. The census and disposal tests live in `packages/engine`, which batch-core does not modify at all. The suite that breaks is not always the suite the diff points at. A cross-package ratchet like the census is exactly the case where scoping pre-flight to the changed packages produces a confident "clean" that is wrong. Pre-flight needs the engine suite regardless of which package a batch touches; I have adjusted accordingly for the remaining queue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 12:33:55 -07:00
gsxdsm	6fc98fd6c7	the third census-invisible class: 51 hardcoded moveTask destinations, measured — and duplicates never archived on a renamed board (#2808 ) A third census-invisible class, measured — plus the two worst instances fixed. ## The shape ```ts if (task.column !== "in-review") { … return; } // the census counts THIS await this.store.moveTask(taskId, "in-progress"); // and cannot see THIS ``` The census is an AST scan for comparisons. A `moveTask` destination is a call argument, so no backlog entry ever points at one. Converting the guard alone is worse than converting neither: the handler starts admitting work on a renamed board and then tries to move the card into a lane that board may not declare. This bit twice in one week — #2797 (`branch-worktree` requeued into a lane that may not exist) and #2807 (a GitHub "changes requested" review dropped, then a move to a hardcoded `in-progress`). Both times it was found only because the guard next to it happened to be under conversion. So I went looking. ## Measured Across `core`/`engine`/`dashboard`/`cli`/`plugins`, excluding `__tests__`/`.test.` and comment lines: \| \| count \| \| --- \| ---: \| \| hardcoded `moveTask` destinations in production \| 51 \| \| …passing `recoveryRehome: true` — deliberate, not defects \| 22 \| \| …plain, rejected on a board that does not declare the target \| 29 \| The 22 must not be "fixed". `moves.ts` exempts them on purpose (#1411): a card stranded in an undeclared column has to stay rescuable to a legacy safe-landing column, or it can never be recovered at all. A sweep that converts them deletes the rescue path. That distinction is the reason this is 29 and not 51, and it is why I measured before writing. ## Why this got sharper recently The `workflowHasColumn(workflowIr, toColumn)` rejection used to sit inside a block gated on `isWorkflowColumnsCompatibilityFlagEnabled` — a settings key nothing in production writes — so it never executed and the legacy `VALID_TRANSITIONS` table decided instead. U12 hoisted it out of that dead branch and it is now live, proven on a real store by `live-move-path-undeclared-target.test.ts`: ``` moveTask(card in "todo" -> "triage") now REJECTS: /Unknown column for this workflow/ ``` That changed the failure mode of all 29 from "silently lands the card in an undeclared column" to "throws". 29 is not a crash count. Whether a throw surfaces or disappears depends on whether the caller catches, which is per-site and I did not measure it — the doc says so explicitly rather than letting the number imply severity it hasn't earned. ## Fixed here: 9 of the 29 `duplicate-intake` and `duplicate-guard` both archive a duplicate. On a renamed archive lane the move is rejected, so the duplicate is never archived and keeps sitting on the operator's board as live work — and in `duplicate-guard` the row has already been stamped `deterministicDuplicateOf`, so it is marked a duplicate while occupying an active lane. Half-applied, which is the same trap as #2797's branch clear. Both now resolve the `archived`-trait column from the task's own workflow through one shared helper, unioned with the legacy id. `cli/commands/task-lifecycle` — `finalizePullRequestMerge` and `finalizeNoOpMergeTask` both move the card to a hardcoded `"done"`, and both run `updateTask({ status: null, mergeRetries: 0 })` first. On a rejection the merge has already landed and the bookkeeping is already cleared while the card never reaches its complete lane: the operator sees a merged branch, a card still sitting in review, and a reset retry counter. Same half-applied shape as #2797's branch clear. Both now route through one resolver so they cannot drift. `contamination` / `foreign-only-contamination` (×2) / `restart-recovery-coordinator` — four recovery requeues to a hardcoded `"todo"`, none of them a `recoveryRehome` escape. On a board without that column the move is rejected and the recovery never completes — the card stays contaminated or stranded, which is precisely the state these paths exist to clear. Consolidation. `resolveReboundTargetForTask` and `resolveArchiveTargetForTask` now live beside `resolveTaskLifecycleColumns` in `workflow-lifecycle-traits`, already the store-dependent resolution seam. My first pass put the archive helper inside `duplicate-intake` and had `duplicate-guard` import it from there — wrong home, and it would have grown a copy per caller as more sites converted. Seven call sites now share two definitions. Plain (non-`recoveryRehome`) destinations: 29 → 21. Coverage on the CLI pair is scoped, and I'd rather say so than imply more: the test covers the resolver, not the two call sites. Both enclosing functions are private and reachable only through `processPullRequest`, which needs a live GitHub surface — exporting them purely to test wiring is a worse trade than stating what is covered. Three cases: renamed lane resolves, no-workflow falls back to the legacy id (which also pins that a default board is byte-identical), and a throwing lookup falls back. ## Revert result (measured) \| conversion \| reverted → \| \| --- \| --- \| \| duplicate archive destination \| new case fails — `moveTask` called with `"archived"` on a board whose archive lane is `boxed` \| \| CLI complete-lane resolver \| replacing the body with a bare `return "done"` fails the renamed case \| \| both move-target resolvers \| replacing either body with a bare return of its legacy id fails 5 cases across the resolver suite and `duplicate-guard` \| Each resolver has a non-vacuous companion asserting it does not return the legacy id on a renamed board — without it, a resolver returning any string would pass. The fallback cases are load-bearing rather than padding: `resolveWorkflowIrForTask` degrades to the built-in IR rather than throwing, and the built-in rebound/archive lanes are `todo`/`archived`, so those cases also pin that a default board is byte-identical. The pre-existing case asserting the legacy `"archived"` passes both ways, which is exactly why it could not detect this and why the new one supplies a workflow. ## Ownership note `packages/core` was `batch-core`'s territory and `packages/cli` was `batch-cli-plugins`'. Both batches have landed, and this is newly-discovered work in the class documented here rather than leftover conversion backlog. Four sites, two shared helpers — happy for either half to move if those owners would rather carry it. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `duplicate-guard` + `duplicate-intake` — 40 passed - `tsc` on core and engine — clean - `pnpm lint`, `check:changesets`, census `--strict` — all clean (run explicitly; a clean `pnpm lint` alone is not evidence the CI Lint check passes) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Duplicate tasks are now archived to each workflow’s configured archive lane. - Completed tasks are moved to the workflow-specific completion lane, with a safe fallback for older workflows. - Recovery and requeue actions now use each workflow’s configured rebound lane instead of assuming a fixed destination. - Documentation - Added guidance on avoiding failures caused by hardcoded workflow destinations and incomplete lifecycle conversions. - Tests - Added coverage for renamed workflow lanes, fallback behavior, duplicate archiving, and recovery destinations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 12:25:00 -07:00
gsxdsm	240a6be0aa	fix(core): dependency update deadlocked on a self-blocked task (#2810 ) ## The bug `updateTaskDependenciesImpl` wraps its whole body in `store.withTaskLock(id, …)`, then reads the current blocker with `readDepTask(task.blockedBy)` → `store.getTask()`. `getTaskImpl` opens with `withTaskLock(id, …)` too, and the per-task lock is non-reentrant. So when `blockedBy` is the task's own id, the call waits forever on a lock its own frame holds — and holds that lock while doing so, leaving the row permanently unlockable. ## Found by generalising #2809, not by luck #2809 removed one `getTask`-inside-`withTaskLock`. An AST scan for the same shape across `packages/core` and `packages/engine` returned exactly three sites: \| site \| verdict \| \|---\|---\| \| `lifecycle-ops.ts:1049` \| the deadlock fixed in #2809 \| \| `update-task-deps.ts:233` (`assertTaskExists`) \| safe — a self-dependency is rejected 15 lines earlier \| \| `update-task-deps.ts:344` (`readDepTask`) \| this bug \| Both surviving sites carry the same `FNXC:SqliteDualPathCleanup` note — "In backend mode, readTaskFromDb uses store.db (SQLite) which is unavailable. Replace with async store.getTask() calls." That port is the common cause across the whole class: it swapped a lock-free read for a lock-acquiring one. ## Why `blockedBy === id` is reachable The dependencies list rejects self-reference explicitly ("Task X cannot depend on itself") — and that guard is precisely why the sibling `assertTaskExists` read on this same lock is safe, so it is left unchanged. `blockedBy` has no such guard: `updateTask({ blockedBy })` accepts the task's own id. The first test asserts that rather than assuming it. The whole regression rests on that state being reachable, so it is proven, not stipulated — and it also pins the asymmetry, so a future guard on `blockedBy` will show up here as a deliberate change. ## The fix Return the in-lock copy already in scope instead of re-reading. One line, no new read path, and strictly more correct than a re-read: it is the state this mutation is reasoning about, rather than whatever a concurrent writer left behind. ## Verification - Mutation-verified against the real defect. With the fix reverted the regression case fails by name — `updateTaskDependencies did not settle within 8000ms — deadlock` — while the precondition and the ordinary-path cases stay green. That is the actual pre-fix behaviour. - A differential covering the ordinary case (blocked by another task). Without it, a fix that short-circuited every blocker read would pass everything else. - Timeboxed for the same reason as #2809: a deadlock otherwise surfaces as a suite-level timeout naming no case. Not a flake knob — the fixed path settles in ~0.5 s and the broken one never settles. - `pnpm test:gate` — exit 0 - `pnpm lint` — clean Changeset included (`patch`, category `fix`). Independent of #2809 — different file, no overlap — but the same class, and the scan above is the argument that the class is now closed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 12:12:33 -07:00
gsxdsm	b7288572a1	engine: a GitHub "changes requested" review was silently dropped on a renamed board (1 → 0) (#2807 ) A human reviewer's feedback was being thrown away. `PrCommentHandler.handleChangesRequested` gated on `task.column !== "in-review"` and returned early. On any board whose review lane is renamed, a GitHub "changes requested" review produced no steering comment and the card never went back to work — the feedback vanished behind a log line nobody reads. No error, no audit row. ## Census \| file \| main \| here \| \| --- \| ---: \| ---: \| \| `packages/engine/src/pr-comment-handler.ts` \| 1 \| 0 \| ## Two literals, only one countable — again ```ts if (task.column !== "in-review") { … return; } // counted … await this.store.moveTask(taskId, "in-progress"); // INVISIBLE to the census ``` The census scores comparisons. The requeue destination is a call argument, so nothing in the backlog pointed at it — the same pairing as the branch-worktree auto-requeue in #2797, and the same trap: converting the gate alone would make the handler admit the review and then attempt a move into a lane the board may not declare, which `moveTask` rejects. A half-conversion here turns a silent drop into a thrown rejection. They convert together or not at all. That is now the second confirmed instance of this shape. The pattern to look for is a counted guard whose body performs a hardcoded `moveTask` — the guard is the visible half and the move is the dangerous one. ## Revert results (measured, each run independently) \| conversion \| reverted → \| \| --- \| --- \| \| review-lane gate \| RENAMED case fails — `updateTask`/`moveTask` never called; the review is dropped \| \| requeue destination \| RENAMED case fails — `moveTask` called with `"in-progress"` instead of the board's wip lane \| The legacy case passes both ways, which is why both vocabularies run. A non-vacuous companion (renamed board, card sitting in the hold lane) keeps a gate that admits everything from passing. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `pr-comment-handler.test.ts` — 34 passed - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean - `node scripts/lifecycle-column-census.mjs --strict` — exit 0 (Running the census explicitly, not just `pnpm lint`: CI's Lint job runs both, and a clean local `pnpm lint` is not evidence the Lint check passes — that cost a round-trip on #2797.)	2026-07-30 12:06:14 -07:00
gsxdsm	e84e9d7f60	fix: the caller audit — five unwired parameters, five defects in their callers (#2803 ) Seven fixes that were sitting on separate handoff branches with no owner while `main` moved. Consolidated, rebased onto current `main`, and verified together rather than only per-branch. The individual branches remain if a subset is preferred. This is the same consolidation that got `batch-core` and #2787 adopted. Close it if it breaks queue policy — the branch keeps the work safe either way. ## Where these came from #2787's review found an optional parameter whose production caller never passed it. That is a class, so I ran it against everything I had landed and found five more. All five turned out to have their real defect in the CALLER, not the parameter — in four of them the parameter was unreachable: \| unwired parameter \| what was actually wrong \| \|---\|---\| \| `blocker-fanout.escalationColumns` \| the hold default made the count zero — no bottleneck warning was emitted at all \| \| analytics `columnFlagsByName` \| routes never built a map — 0 in-progress / 0 in-review beside correct cost totals \| \| `isLegacyAutoMergeStampCandidate` \| the read queried a column a renamed board does not have, so the backfill iterated nothing \| \| `rankAssignedTasksForWakeDelta` \| `getTasksByAssignedAgent`'s `excludeArchived` used the literal — archived cards returned as open work \| \| `duplicate-intake.columnFlagsByColumnId` \| intake could archive or soft-delete a newly created task as a duplicate of finished work \| The heuristic worth keeping: an optional parameter no production caller fills is a marker pointing at an unexamined caller. The census cannot see any of these five — every gate is a `Set`/array literal or a query filter, i.e. a definition rather than a comparison. ## Also included - `executor.ts` — the stale-spec guard did the exact thing its own comment forbids: on a renamed board it ran on a LIVE task and pulled it out of execution into replan. `activeMergeStatuses` protected merging cards by accident, which is why the symptom looked arbitrary. - `register-project-routes.ts` — project health reported 0 active tasks; its list also still contained `triage`, dead since U11. - `dashboard/app/utils/taskTiming.ts` — a second copy of `getTotalAgentActiveMs`. Core's was converted; the card chip imports this one, so the census counted the site as done while the rendered number stayed keyed on `"in-progress"`. ## Verification Verified as a set: `pnpm test:gate` 161 / 13 / 487 / 71 · core suites 15 passed · engine 7 · dashboard 12 · four `tsc` targets clean · lint clean · census `--strict` exits 0. Each fix is revert-proven individually; the specific case that fails is named in each test header. ## Two honesty notes Three guards here are structural, not behavioural, and say so in their headers. `sanitizeAgentTaskLinks` is a closure inside `createApiRoutes`; the analytics aggregators need a live `AsyncDataLayer`; the stale-spec guard sits deep inside `execute()`. Each ratchet fails on revert — verified — but none is an end-to-end proof, and the headers state which half they cover. One of my behavioural test sets would have lied. The intake-dedup cases drive `findSameAgentDuplicates` directly; I removed the wiring to measure the revert and they stayed green, because they pin the predicate and not the caller. That is the exact illusion this audit was chasing, reproduced in my own file. The forward now has its own structural check. ## Deliberately not included `worktree-pool.ts:1205` — it fails safe (a missed match protects a branch from cleanup rather than deleting it) and sits in the merger's branch-reaping path where the opposite error destroys work. That deserves its owner's judgement, not a drive-by conversion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 12:02:53 -07:00
gsxdsm	a8cfce8fbd	executor: stale merge evidence re-entering execution, and a live checkout that read as unowned (12 → 8) (#2805 ) Two executor conversions with real operator consequences, one census false positive, and three sites deliberately left with their reasons recorded. ## Census \| file \| main \| here \| \| --- \| ---: \| ---: \| \| `packages/engine/src/executor.ts` \| 12 \| 8 \| Of the 4: three genuine conversions, one reclassification. ## What was broken `resetMergeStateIfNeeded` — cards re-entered execution carrying stale merge evidence. Merge state is cleared when a card leaves a lane where a merge could have been recorded. Keyed on `in-review`/`done`, a renamed board matched neither, so a card bouncing back into execution kept `mergeDetails` — including a commit sha from its previous pass — into its next run. `review` is not a trait, so this resolves through the same five flags (`complete`, `mergeOrchestration`, `mergeBlocker`, `humanReview`) the dependency gates in this file already use; two gates answering "is this a merge-bearing lane?" differently would be a split brain. The worktree-owner scan — a live checkout read as unowned. `findActiveWorktreeOwner` asks "is anyone else working in this checkout?". Its in-memory `activeWorktrees` leg is vocabulary-independent, but the durable leg — the one that answers after an engine restart, when the in-memory map is empty — filtered with `t.column !== "in-progress"`. On a renamed board that matched nobody, so the worktree read as free and a second task could be handed a checkout another task is live in. Post-restart is exactly when this function matters. Not the query-filter class: that `listTasks` call passes no `column`, so the predicate is the only lane gate on the path. ## A third census false positive in this package Line 16094's `to` is a review-addressing record status — the method signature is `to: "queued" \| "in-progress" \| "addressed" \| "failed"`, and the next two lines test it against `"addressed"` and `"failed"`, which are not columns at all. Marked `DELIBERATE-LITERAL`. That is the third in `packages/engine` after the two `cli-agent` `CliMachineState` ones (#2797). The backlog total includes non-columns; a sweep that "converts" them turns a status machine into a workflow role. ## Revert results (measured, each run independently) \| conversion \| reverted → \| \| --- \| --- \| \| worktree-owner wip predicate \| RENAMED case fails — checkout reads as free while another task is live in it \| \| `resetMergeStateIfNeeded` lanes \| RENAMED case fails — card keeps `commitSha: "abc123"` from its previous pass \| Both DEFAULT cases pass before and after, which is why both vocabularies run. Each has a non-vacuous companion (holder sitting in the complete lane; a return from the hold lane) so a predicate matching every column would not pass. Both reach their seam directly through a cast. The public routes are `handleBranchConflict` (needs a real `BranchConflictError` plus a git repo) and the `task:moved` listener (drags in the whole `execute()` path); going through either would make these tests about a git fixture rather than about the lane predicate. The alternative was the status quo — all 91 `executor-worktree.test.ts` cases seed `column: "in-progress"`, so they assert the legacy fallback and pass either way. I shipped the conversions in one commit stating* they were unproven, then closed that gap in the next; the history shows both. ### Two fake defects found while writing those tests Worth naming, because both are the documented green-for-the-wrong-reason shape: 1. The first fake had no `updateTask`, so the cleanup threw rather than asserting anything. 2. The second returned a new object without persisting — and `cleanupMergeStateForReverification` re-reads through `getTask`. The re-read handed back the stale row, so both vocabularies reported "nothing changed" and it would have read as a passing negative test. ## Deliberately NOT converted, with reasons - The `task:moved` listener cluster (`3521`/`3545`/`3596`/`3606`), including the AGENTS Move-Task hard-cancel contract `userCanceled: source === "user" && to === "todo"`. Its prologue is synchronous (`userCanceledTaskIds.delete`, watchdog clear) and deferring it to a microtask changes hard-cancel ordering. The sync IR reader is not an option — it returns the DEFAULT workflow for every task in production. A safe conversion needs lanes resolved on an earlier async boundary: new machinery plus an ordering change, which is out of fleet scope and not a guess worth making on a hard-cancel path. - `17081` pairs `latestColumn === "in-progress"` with a hardcoded `moveTask(taskId, "in-progress")` two lines above — census-invisible, the same shape as the branch-worktree requeue bug in #2797. They have to convert together, and the move needs the same rejection guard. - `5903` is the query-filter class: `listTasks({ column: "in-progress" })` followed by a re-assertion of the same literal. Converting it drops a count and changes nothing — see `docs/solutions/architecture-patterns/self-healing-sweeps-are-blind-on-a-renamed-board.md` (#2800). ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean - `--strict` exits 0 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 12:02:41 -07:00
gsxdsm	7dc7a41e0e	fix(tests): the load-lane guard matched a VARIABLE NAME — make it AST-based (#2804 ) ## A red that no CI run can see Found by pre-flighting #2796 against current `main`. Merged, the engine suite fails: ``` scheduler-load-lane-union.test.ts > the scheduler builds this same union expected 'import {…' to contain '...columnsWithFlag(loadLaneIr, "intake")' ``` Neither side is red on its own. `main` is green (10913 passed, 0 failed) and #2796's own CI is green — this test landed on `main` via #2787, after #2796 was cut, and #2796 does not touch the file. It fails only in the merged state, which is exactly the shape no branch's CI checks. ## #2796 is not at fault The union is still built (`scheduler.ts`, the `columnsWithFlag(ir, ...)` spread). Resolving assignment load per task renamed the local from `loadLaneIr` to `ir`, and the guard hardcoded that name: ```ts expect(source).toContain(`...columnsWithFlag(loadLaneIr, "${flag}")`); ``` It would fail identically on a reformat, a line wrap, or any rename — reporting drift that did not happen. And the reflex fix is to edit the string to match, which protects nothing and teaches nobody anything. ## The fix Parse `scheduler.ts` and collect the string literal passed as the second argument to every `columnsWithFlag(...)` call, whatever the first argument is called. Same invariant — every legacy role is unioned somewhere in the scheduler — now actually checked. It also asserts the parse found something before checking the six flags. A visitor that matched nothing would make every assertion below it vacuous, which is the specific failure mode this guard family keeps producing. Still structural rather than behavioural, for the reason the file header already gives: the call site sits inside a dispatch path a unit test has no business standing up. The three sibling cases cover the resolver's behaviour; this one covers the wiring. ## Evidence — the discrimination is the right way round \| mutation \| expected \| result \| \|---\|---\|---\| \| rename `ir` → `loadLaneIr` (behaviour identical) \| pass \| 4/4 passed \| \| drop `...columnsWithFlag(ir, "hold")` from the union \| fail \| fails: `scheduler.ts no longer passes "hold" to columnsWithFlag` \| Engine 10936 passed / 0 failed · gate 732 green · lint clean · engine `tsc --noEmit` 0 errors. Test-only; `scheduler.ts` restored clean after the mutations. Unblocks #2796 with no change needed on its side — commented there. ## Pre-flight results for the rest of the queue Same method (merge with current `main`, run the suites), since batch-engine's earlier landing put 32 failures on main that were only caught post-merge: \| PR \| result \| \|---\|---\| \| #2785 batch-engine-tail \| clean — 10903 passed \| \| #2783 batch-core-2 \| clean — core 4751 passed; its one api failure is pre-existing on main \| \| #2797 engine tail \| clean — 10941 passed \| \| #2772 batch-dashboard-app \| clean — backfill total 112 → 111, no lane regresses \| \| #2796 assignment load \| this failure only \| 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:59:25 -07:00
gsxdsm	b74d4cdd98	test(engine): a third inert-conversion mechanism — a resolver called with a sentinel task id (#2806 ) ## What One new live-PostgreSQL E2E suite, 3 tests. No production file is touched — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-sweep-sentinel-task-id-live-e2e.pg.test.ts` ## A third mechanism Two are already measured in this series: \| mechanism \| PRs \| \|---\|---\| \| the site resolves the workflow synchronously, so PostgreSQL hands it the default board \| #2789–#2794 \| \| the role answer is an optional parameter and the caller does not pass it \| #2795–#2802 \| This is a third, and it is inert by construction rather than by environment. `triage.ts`'s startup sweep resolves its lane vocabulary with a sentinel task id: ```ts const sweepLanes = resolvePlannerLanes(this.store, ""); const sweepColumns = [...new Set(["triage", "todo", sweepLanes.intake, sweepLanes.hold])]; ``` There is no task `""`, so no selection can be read for it and no board can be resolved from it. The lanes come back as the default board's and the union collapses to the legacy pair `{triage, todo}`. Note what this means for the other mechanisms' fixes: making `resolvePlannerLanes` async would not repair this site. The defect is the argument, not the resolver. ## What breaks The sweep clears stale `planning` status so a card cannot hold a planning admission slot forever. Its own comment says the union is "load-bearing, not defensive", because the merged post-U11 default collapses `intake` and `hold` onto `todo` and "nothing ever swept `triage`". That reasoning fixes the merged case and leaves the renamed one. A card parked in a renamed hold column with a stale `planning` status is in none of the four queried columns, is never swept, and occupies a planning admission slot permanently — the exact failure the comment describes, on every custom board. This one is sweep-wide, which is what makes the sentinel distinct from the other two mechanisms: they resolve per task and get one card's answer wrong; this resolves once for the whole board and cannot be right for any workflow but the default, however many boards the project runs. ## Evidence discipline - Observed state. Whether the card's persisted `status` is still `planning` after the real sweep runs against a real store. - The sweep is a private method, invoked through a cast. That is production code executing, not a stand-in, and nothing about the assertion depends on the cast. `processor.stop()` runs in a `finally` so one case's admission provider cannot outlive it and observe another's store. - The first case isolates the mechanism: two custom workflows exist by the time it runs and neither can influence the answer, because the argument names no task. ## Mutation-verified Adding the renamed hold column to the swept set: \| case \| result \| \|---\|---\| \| sentinel resolves the default lanes \| passes — correct, it asserts the resolver, not the query \| \| CONTROL (default board) \| passes — correct, unaffected \| \| CHARACTERIZATION (renamed board) \| fails \| Exactly one case moves, and it is the one that should. ## Not done, and why No fix. The sweep needs a lane vocabulary for every board in the project, not one board's — so the fix is a union over the distinct workflows present, or a per-task filter after a broader query, not a swap of the sentinel for a task id. That is a design decision in `triage.ts`, another worker's file. The differential says what the fix must make true. ## Verification - new suite — 3/3 passed, mutation matrix above - full live-PG E2E surface — 151/151 passed (148 on main + 3) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:50:05 -07:00
gsxdsm	f9f06a4fb7	engine tail: nine files to zero — incl. a census-INVISIBLE requeue into a lane that does not exist (−13) (#2797 ) Follow-up to #2785. Nine engine files to zero, all one question — "is this task finished?" — asked in nine places, wrong in every one on a renamed board. ## Census, per file (measured, `--strict` verified) \| file \| main \| here \| \| --- \| ---: \| ---: \| \| `agent-reflection.ts` \| 2 \| 0 \| \| `merger-scope-auto-widen.ts` \| 2 \| 0 \| \| `worktree-pool.ts` \| 2 \| 0 \| \| `cli-agent/state-machine.ts` \| 2 \| 0 (reclassified — see below) \| \| `auto-recovery-handlers/branch-worktree.ts` \| 1 \| 0 \| \| `cli-agent/task-session.ts` \| 1 \| 0 (reclassified) \| \| `merger-integration-worktree.ts` \| 1 \| 0 \| \| `merger-orphan-rehome.ts` \| 1 \| 0 \| \| `plugin-runner.ts` \| 1 \| 0 \| \| net \| \| −13 \| ## The one worth reading: `branch-worktree` had TWO defects, and the census could only see one ```ts if (task.column === "in-progress") { …clear branch… } // counted await this.deps.taskStore.moveTask(task.id, "todo", { … }); // INVISIBLE ``` The census scores comparisons. The requeue destination is a call argument, so nothing in the backlog ever pointed at it — and it is the worse of the two: a board with no `todo` column was requeued into a lane that does not exist. The counted literal is the smaller half (a renamed wip lane meant the stale branch was never cleared, so the card carried a dead branch back into execution). Converting the comparison alone would have dropped a census count and left the board requeuing into nowhere. Destination now resolves through `resolveReboundTarget` (KTD-10 ordering: hold → intake → first column). Reverted independently: destination restored → 2 fail (`moveTask` called with `"todo"`, not `"backlog"`); wip test restored → 1 fail (`updateTask` never called). ## The rest - `plugin-runner` — `onTaskCompleted` never fired on a renamed board. Every plugin that closes an issue, posts a notification, or records a metric on completion silently stopped, with nothing logged. Resolved asynchronously inside the existing fire-and-forget seam, not via `resolveTaskWorkflowIrSync` — per `sync-workflow-ir-callsite-allowlist` that reader returns the DEFAULT workflow for every task in production, so a sync guard here would read as converted and still be wrong. The listener is already `void`-dispatched, so awaiting inside it changes no observable ordering (the shape `NotificationService` already uses). - `merger-orphan-rehome` — a renamed complete lane made every source task read as unfinished, so orphaned commits were never rehomed and stayed stranded off the integration branch. Resolves by the trailer id, not `sourceTask.id`, which the fake store does not populate. - `agent-reflection` — `classifyOutcome` returned `null` for every finished task, so both callers treated completed work as nothing to reflect on: one recorded `reflection:skipped` with reason `"not-completed"`, the other silently `continue`d. Reflection captured nothing at all on a custom board. - `worktree-pool` — shipped tasks' worktrees stayed in the ACTIVE set, so the reclaim pass never returned them and the board walks into worktree exhaustion — a stall whose cause is invisible from the symptom. - `merger-scope-auto-widen` — finished cards counted as active claimants, so a merge was blocked by a task that no longer exists in any meaningful sense. - `merger-integration-worktree` — a shipped task still counted as a live worktree user, so the integration worktree could never be reused and the merge path took the slower rebuild every time. ## The census OVERSTATED the engine backlog by 3 `cli-agent/state-machine.ts` and `cli-agent/task-session.ts` compare against `done` — but that is a `CliMachineState` (`ready`/`busy`/`waitingOnInput`/`done`/`resuming`/`idle`) tracking one CLI agent process. It never reads a board column. The census matches the bare string. Marked `DELIBERATE-LITERAL` rather than left for a later sweep to "convert" a process state into a workflow role. Worth flagging fleet-wide: the backlog total includes at least these three non-columns. ## Revert results (measured, each run) \| conversion \| reverted → \| \| --- \| --- \| \| `plugin-runner` complete gate \| RENAMED case fails — `onTaskCompleted` never invoked \| \| `merger-orphan-rehome` source gate \| RENAMED case fails — `orphan:false, reason:"source-task-not-done"` \| \| `branch-worktree` destination \| 2 fail — `moveTask` called with `"todo"` \| \| `branch-worktree` wip test \| 1 fail — `updateTask` never called \| Each has a non-vacuous companion (renamed board, non-complete lane / mid-flight source / non-wip column) so a guard that fired unconditionally would not pass. Four are NOT revert-proven, and I am not claiming otherwise: `agent-reflection`, `merger-scope-auto-widen`, `merger-integration-worktree`, `worktree-pool`. Their suites omit a workflow and therefore assert the legacy fallback — they pass before and after. `merger-scope-auto-widen` has no test file at all; `scanIdleWorktrees` is mocked in every suite that touches it and driving it for real needs git worktrees on disk. All four strictly widen the finished set (resolved roles ∪ the legacy ids), so default boards are byte-identical. That is the argument for shipping them, not a substitute for coverage. ## Examined and deliberately NOT converted - `backlog-pressure-reporter:173` — fed by `listTasks({ column: "todo" })`, a hardcoded query filter. On a renamed board `todoFull` is empty and the predicate never runs. Converting it drops a census count and changes nothing observable; the fix belongs at the query layer. - `auto-merge-finalization:28` — the catch-arm legacy fallback, which must stay for the same reason `columnRoles.ts` keeps its id fallback. - `auto-merge-finalization:84` — only selects between two diagnostic reason strings that are both `ok: false`, on a pure validator with no store in scope. Converting it would thread a store through a pure function to change a label. ## Merge resolution note Merging main brought conflicts in `agent-assignment.ts` and `ephemeral-worker-manager.ts`. Main's versions won both and mine are dropped: main threads an optional `activeColumns` from `scheduler.ts:2340` (a cleaner seam than widening the store type to resolve internally), and its `isAgentIdle` carries a greptile P1 fix mine lacked — `columnsWithFlag` membership rather than first-per-role, so a workflow declaring two wip lanes has both recognised. That is the fourth time in this program main's version of a contested file was the better one. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean - `--strict` exits 0 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Bug Fixes * Workflow-dependent task completion now recognizes custom lifecycle columns, including renamed boards. * Recovery requeues tasks to the configured destination and clears branch details only from the appropriate work-in-progress column. * Improved handling of completed tasks, orphaned work, shared worktrees, and scope evaluation across custom workflows. * Plugin completion hooks now trigger for any column configured as complete. * Tests * Added coverage for renamed workflow columns and custom completion, recovery, and rehoming behavior. * Documentation * Clarified CLI state terminology in internal developer comments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:28:54 -07:00
gsxdsm	bb8be93c52	test(engine): audit the auto-heal review-lane call sites (a DELIBERATE-LITERAL note that is not true) (#2802 ) ## What One new source-level audit test, 3 cases. No production file is touched. Fourth instance of the optional-role-parameter class (#2795, #2798, #2799) — and the only one so far where the source annotation asserts the opposite of the fact. `packages/engine/src/__tests__/auto-heal-review-lane-callsite-audit.test.ts` ## The finding `project-engine.ts`'s `hasAutoHealableVerificationBufferFailure` takes the review-lane answer as an optional parameter defaulting to `task.column === "in-review"`. Its `DELIBERATE-LITERAL` note says: > "Both call sites pass the resolved answer; the default exists so an unconverted caller keeps exactly today's behaviour rather than silently changing meaning." There are three call sites, not two: \| site \| passes the resolved lane? \| \|---\|---\| \| `canMergeTask:2657` (threads its own param) \| ✅ \| \| ← `canMergeTask:2903` \| ✅ `t.column === reviewLane` \| \| ← `canMergeTask:3334` \| ✅ `task.column === mergeLoopReviewLane` \| \| merge loop `:3655` — direct call \| ❌ nothing \| The note counts the two gating callers and misses the healing one. Note which half is converted: the sites deciding whether a card MAY merge resolve the lane; the site that would RECOVER a stuck card does not. The consequence is in the same comment: on a renamed board "a task whose merge verification died on a buffer-overflow error was never auto-healed — it sat retry-exhausted until a human reset it. The failure is invisible because 'no auto-heal' looks identical to 'nothing to heal'." ## Why this is a source audit and not an E2E The predicate and its caller are both private methods of `ProjectEngine`. The three sibling files in this series each carry a live behavioural differential because their predicates are exported; this one cannot, and inventing a mock `ProjectEngine` to assert a private method would prove only that the mock behaves as written. Stated plainly rather than substituted for — the finding is a call-site fact, and a call-site fact is what is asserted. The third case deliberately pins the false note itself, so the audit fails when someone corrects the sentence — forcing them to also decide what to do about the third site rather than fixing the prose and leaving the gap. ## A self-correction, forced by the mutation run The first version filtered call sites on whether the argument text contained `isReviewColumn` / `ReviewLane`. Converting the unconverted site to pass a plain `true` left the count at one and the suite stayed green — the "alarm in both directions" the header claims did not exist. Now it counts arguments (depth-aware, so nested calls and object literals do not confuse it), which is the property actually being asserted and cannot be spelled around. Re-verified: \| state \| result \| \|---\|---\| \| main \| 3/3 pass \| \| site 3655 converted to pass a third argument \| fails \| Recorded in an FNXC note next to the helper, because the first version is the exact mistake this series exists to catch. ## Not done, and why No fix. Passing the resolved lane at `:3655` means resolving the task's review column inside the merge loop; whether that resolution belongs there or should be hoisted alongside `mergeLoopReviewLane` (already computed nearby, which is what makes the omission look accidental rather than considered) is a decision for the file's owner. ## Verification - new suite — 3/3 passed, mutation-verified in both directions - `pnpm lint` — clean - Unit lane, no PostgreSQL required; adds no gate surface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:19:12 -07:00
gsxdsm	ea0af4826f	test(engine): third instance of the optional-role-parameter class, intra-file (#2799 ) ## What One new live-PostgreSQL E2E suite, 4 tests. No production file is touched — evidence, per the E2E worker's remit. Third measured instance of the pattern from #2795 and #2798. `packages/engine/src/__tests__/workflow-planning-continuation-terminal-gap-live-e2e.pg.test.ts` ## The finding — and why it is the sharpest form yet The converted and unconverted call sites are in the same file, and one is nested inside the other's call tree. `in-process-runtime.ts` resolves terminal columns through an optional parameter defaulting to `LEGACY_TERMINAL_PAIR` (`done` + `archived`): \| site \| passes the resolved set? \| \|---\|---\| \| `drainDuePlanningContinuations:386` \| ✅ `{ terminalColumns }` \| \| `selectActionablePlanningContinuations:413` \| ❌ nothing \| \| `resolvePlanningContinuationCandidate:199` → inner predicate \| ❌ not threaded \| What breaks. `selectActionablePlanningContinuations` documents its own purpose as excluding "soft-deleted / archived / done tasks so archive-fallback rows returned by getTask cannot re-enter plan-review after the card left the board." On a renamed board its terminal test is against ids that board does not have, so a card sitting in its complete column is classified `actionable` and re-enters plan-review — precisely the thing the function exists to prevent, silently, on every custom board. The third site matters too, because it shows the conversion is not whole even along the converted path: `resolvePlanningContinuationCandidate` applies the caller's resolved set to its own terminal test, then delegates to `isPlanningContinuationTaskDispatchable(task)` without passing it, so that inner predicate re-tests against the legacy pair. A partially threaded conversion is indistinguishable from a complete one at every call site that looks converted. ### The class so far \| seam \| call sites passing the resolved answer \| \|---\|---\| \| `shouldHoldActiveFileScopeLease` \| 2 of 4 — #2795 \| \| `evaluateParkedAgentTaskLink` \| 2 of 6 — #2798 \| \| `resolvePlanningContinuationCandidate` \| 1 of 2, plus one unthreaded inner call — this PR \| Verified clean and reported as such: `restart-recovery-coordinator.ts`, the `isRecoverableMissingWorktreeReviewFailure` family, `spec-staleness.ts`'s `plannerColumns`, `task-revert.ts`'s `revertableColumns`, `agent-assignment.ts`'s `activeColumns`. Audited since this PR was opened and also clean: `surfacing-sweeps.ts`'s `roleColumn` (resolved internally through the async resolver — not a caller-supplied parameter at all) and `auto-claim-snapshot.ts`'s role trio (both `isRunnableAutoClaimCandidate` call sites pass `rolesByTask`). The class audit is therefore complete: four seams have unconverted callers (#2795, #2798, this PR, #2802); every other seam is correct. None of it is visible to the lifecycle-column census: there is no column literal at any unconverted call site — the literal lives one function away, where it is correct for an unconverted caller and correctly annotated. ## Scope, stated honestly The three behavioural cases are driven end to end with real persisted rows from a live store and the real exported functions. The call-site split is asserted against source text — driving the drain needs the runtime's full dependency set, which I did not build — and the audit case says so rather than dressing it up. It is an alarm in both directions. ## Mutation-verified Flipping `LEGACY_TERMINAL_PAIR` from `done` to the renamed complete id: \| case \| result \| \|---\|---\| \| CONTROL (default board) \| fails \| \| CHARACTERIZATION (renamed board) \| fails \| \| BOUND (resolved set passed) \| passes — correct, the argument overrides the default \| \| AUDIT \| passes — correct, it is a source assertion \| ## Not done, and why No fix. Threading the resolved set into `selectActionablePlanningContinuations` changes its signature and every caller; threading it into the inner predicate changes behaviour along the already-converted path. Both are decisions for the file's owner. The differential says exactly what the fix should make true. ## Verification - new suite — 4/4 passed, mutation matrix above - full live-PG E2E surface — 137/137 passed (133 on main + 4) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:15:56 -07:00
gsxdsm	ed0df8b0e2	evidence: the self-healing sweeps do not RUN on a renamed board — 49 hardcoded column QUERIES, and 17/30 fakes hide it (#2800 ) Evidence only — no conversions, no behaviour change. One doc, one test. It changes how the fleet should read the largest remaining file in the backlog. ## The finding, measured on `origin/main` `packages/engine/src/self-healing.ts` carries: - 97 lifecycle-column comparisons the census counts, and - 49 calls of the shape `this.store.listTasks({ column: "<literal>", … })`. `listTasks`' option is `column?: ColumnId` — one literal column, applied as a filter in the store. On a workflow whose lanes are renamed, every one of those 49 queries returns an empty array, so the sweep it feeds does nothing at all. The self-healing sweeps are not mostly-correct-with-some-unconverted-guards. They never execute. The `in-review` family alone is roughly half the calls: merge recovery, wedged merges, branch rebind, pending-step reconciliation. ## Why this matters to the census specifically ```ts const tasks = await this.store.listTasks({ column: "done", slim: true }); const candidates = tasks.filter((task) => task.column === "done" && // <-- the census counts THIS … ); ``` The census scores the comparison, not the query. Converting it is a legal-looking change that drops a count and changes nothing an operator can observe — the loop body still never runs, because the list was already empty. Roughly 31 of self-healing's remaining comparisons are this shape. Driving `self-healing.ts` to 0 would report the subsystem as converted while it stays inert on custom boards. In this file the census total is not merely a floor — it is actively misleading, and I'd rather the fleet know that before someone spends a week on the 97. ## Why the existing suite cannot see it Measured across `packages/engine/src/__tests__/self-healing.test.ts`: - 30* files define a `listTasks` on their store fake. - 17 ignore the `column` option entirely. ```ts // representative of the 17 listTasks: vi.fn(async (options?: { limit?: number; offset?: number }) => { const all = [...tasksById.values()]; // options.column is never read return all.slice(offset, offset + limit); }), ``` The fake is more permissive than production. The sweep receives rows the real query would have filtered out, so the test proves the sweep's logic while saying nothing about whether the sweep is ever reached. A green self-healing suite is not evidence that self-healing runs. This is the mirror image of `store-fake-defects-that-masquerade-as-production-bugs.md`: there a fake is missing something production needs and the code looks broken; here it supplies more and the gap looks fixed. ## About the test It pins a known defect and is labelled as such in the file header — it asserts what the engine does today, which is the wrong thing. It asserts the query argument, not the outcome. The outcome is `0` either way, so an outcome assertion cannot distinguish "nothing to do" from "asked the wrong question". Asserting the argument also avoids standing up the git-evidence path these sweeps enter once they have candidates. - Ratchet proven to fire: repointing `reconcileDoneTaskIntegrity`'s query at the renamed lane makes it fail — `1 failed \| 2 passed`. A guard that reports success without checking anything is worse than no guard, so I ran it. - Guard on the guard: a first case asserts the renamed fixture really does resolve a complete lane that is not `done`. Without it, every later assertion could pass vacuously if the fixture ever collapsed to the default vocabulary. - Control case: shows the ignoring fake hands back a row whose column is `shipped` from a query that asked for `done` — the mechanism by which the suite stays green. When the query layer is fixed this test will fail, forcing an update. That is the intent. ## What I did NOT do, and why I did not fix it. `column?: ColumnId` takes one id, and the resolution is circular at the query layer — you need a task to know its workflow, and you are querying to find the tasks. A real fix is either a multi-column query option (`columns?: readonly ColumnId[]`) plus a resolved union across live workflow definitions, or dropping the filter and post-filtering by role in the engine. Either is a behaviour change to a shared store API across 49 call sites. That is a coordinator-level decision, not something a conversion PR should take unilaterally — the same reasoning that kept membership predicates out of the census. I'd take it on if you want it; it needs to be a deliberate call, not a side effect of a conversion sweep. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean	2026-07-30 11:15:43 -07:00
gsxdsm	5795d70b27	fix(engine): assignment load must be resolved per task — #2787 P1 follow-up (#2796 ) Fix-forward for the P1 that arrived on #2787 after it merged — so it lands as its own PR rather than a thread reply on merged code. ## The finding `selectPermanentAgentForTask`'s `activeColumns` was resolved from the candidate task's workflow and then applied to every row `listTasks` returned. On a project running several workflows — the normal case — assignments living in another workflow's load-bearing lanes vanished from the tally, and the already-loaded-agent-wins bug returned through a different door. A column id means something only relative to its OWN workflow. `blocker-fanout.ts` documents exactly this and offers a per-task `classify`; the option is now that same shape rather than a third invention: ```ts countsAsAssignmentLoad?: (task: Task) => boolean ``` The scheduler resolves each assigned row against its own IR, sharing one cache for the selection, so a board spanning three workflows reads three IRs — not one per assigned card. ## Why this is the third round on the same parameter, stated plainly 1. I added the parameter and never wired the caller — inert in production. 2. I wired it as a union of wip+review, which dropped hold/intake and made it a regression for backlog work. 3. I resolved it from one workflow and applied it to all — this fix. Each round was a smaller version of the same error: treating a lane answer as global when it is per-task, and per-role when it is per-membership. Worth recording because the first two rounds both looked correct and both passed their tests — the tests asserted the renamed case I was thinking about, not the shape of the data. ## Verification - new cross-workflow case; reverting the predicate to a single workflow's lanes fails it - `agent-assignment` suite 14 passed - `pnpm test:gate` — 161 / 13 / 487 / 71 · lint clean · census `--strict` exits 0 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:06:20 -07:00
gsxdsm	6bb5e4f787	test(engine): measure the optional-role-parameter conversion class (#2798 ) ## What One new live-PostgreSQL E2E suite, 4 tests. No production file is touched — evidence, per the E2E worker's remit. Follows #2795, which found the first instance of this pattern. `packages/engine/src/__tests__/workflow-optional-role-param-caller-audit-live-e2e.pg.test.ts` ## The finding #2795 showed a conversion pattern the lifecycle-column census cannot see: a role question migrated into an optional parameter whose default is the legacy literal, converted at some call sites and not others. This shows it is not a one-off, and measures it. \| seam \| call sites passing the resolved answer \| \|---\|---\| \| `shouldHoldActiveFileScopeLease` \| 2 of 4 (both `scheduler.ts`; neither `self-healing.ts`) — #2795 \| \| `evaluateParkedAgentTaskLink` \| 2 of 6 (`scheduler.ts`, `task-agent-sync.ts`; neither `agent-heartbeat.ts` ×2 nor `self-healing.ts` ×2) — this PR \| The second is the more damaging, and the callee's own FNXC note already names the outcome: without the resolved columns "the card would be treated as unparked and its live agent link cleared" — a stale-link bug turned into a dropped-link bug. Driven here: a card parked in a renamed board's hold column, with live execution proof, has its agent link dropped. ### Why the census is blind to it The callee is converted and its default is correctly marked `DELIBERATE-LITERAL` — for an unconverted caller that default genuinely is the intended behaviour. The unconverted call sites contain no column literal at all; it lives one function away. So the census counts the callee's annotated literals and sees nothing at the call sites, and the conversion reads as complete from every angle except running it. This is a class, not two bugs. The same shape exists at roughly twenty seams (`revertableColumns`, `plannerColumns`, `roleColumn`, `terminalColumns`, `activeColumns`, …). Two are now measured. I checked two others I flagged as unknown in #2795 — `restart-recovery-coordinator.ts`'s `isReviewColumn?` and the `isRecoverableMissingWorktreeReviewFailure` family — and their callers are fully converted (`extension.ts:1924`, `task.ts:1390`, `self-healing.ts:12087`), though the doc comment claiming `extension.ts` "still asks with the literal" is now stale. The rest are unaudited; the audit case is written so adding a seam is a small edit. ## Scope, stated honestly Three cases are driven end to end: real persisted rows from a live store, the real exported predicate, both call shapes. The call-site split is asserted against source text — reaching all six sites needs the heartbeat and self-healing harnesses, which I did not build, and the audit case says so in its own comment rather than dressing it up. It is an alarm in both directions: a new unconverted caller pushes the count up and fails; converting an existing one pushes it down and also fails. The second is deliberate — that is the moment someone should read the three behavioural cases and update the number on purpose. ## Mutation-verified Flipping the callee's default from the legacy parked pair to `["backlog"]`: \| case \| result \| \|---\|---\| \| CONTROL (default board, no options) \| fails \| \| CHARACTERIZATION (renamed board, no options) \| fails \| \| BOUND (renamed board, options passed) \| passes — correct, the argument overrides the default \| \| AUDIT \| passes — correct, it is a source assertion \| ## Not done, and why No fix. Passing the resolved columns at the four unconverted sites means resolving each linked task's traits inside the heartbeat and self-healing paths — async work in loops that already hold locks — and both files belong to other workers. The differential says exactly what the fix should make true. ## Verification - new suite — 4/4 passed, mutation matrix above - full live-PG E2E surface — 137/137 passed (133 on main + 4) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:03:09 -07:00
gsxdsm	60054aab0a	test(engine): live-PG evidence of an inert conversion at the CALL SITE (#2795 ) ## What One new live-PostgreSQL E2E suite, 4 tests. No production file is touched — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts` ## Why this is a different finding, not a sixth of the same one #2789/#2791/#2792/#2793/#2794 all concern one mechanism: a site resolves the workflow synchronously and silently gets the default board. This is a second mechanism, and neither the lifecycle-column census nor the sync-resolver allow-list can see it. `shouldHoldActiveFileScopeLease` was converted by turning its two role questions into optional parameters with literal defaults: ```ts const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; ``` A caller that resolved the traits passes the answer; a caller that has not gets exactly the pre-conversion behaviour. That is a deliberate migration device and the source says so — correctly marked `DELIBERATE-LITERAL`. But the migration was only half made: \| call site \| passes the resolved answer? \| \|---\|---\| \| `scheduler.ts:1986` \| ✅ `{ isWipColumn: true }` \| \| `scheduler.ts:2006` \| ✅ `{ isReviewColumn: true }` \| \| `self-healing.ts:4525` \| ❌ neither \| \| `self-healing.ts:5443` \| ❌ neither \| So the same predicate is right on the scheduler's path and wrong on self-healing's. The harm is the one the function's own FNXC note describes: on a renamed board both branches fall through, the predicate returns false for every card, `activeScopes` stays empty, and the dispatch path sees no overlap — two agents editing the same files, which is what the overlap machinery exists to prevent. At the self-healing sites the consequence is narrower but identical in shape: a stale-lease reconciler concludes a live blocker holds no lease and proceeds to clear state the scheduler would have honoured. ### Why the existing instruments are blind to it There is no column literal at the self-healing call sites. The literal lives inside the callee's default, one function away — and there it is correct, because for an unconverted caller it is the intended behaviour. A census counting `=== "in-progress"` occurrences sees the callee's two (properly marked) and nothing at all at the call sites. The conversion reads as complete from every angle except running it. This generalizes: any conversion that migrates behaviour behind an optional parameter leaves a residue the census scores as done. Worth a sweep for the same shape elsewhere — `agent-assignment.ts`'s `activeColumns?` and `restart-recovery-coordinator.ts`'s `isReviewColumn?` are the same pattern; I have not checked whether their callers supply them. ## Scope, stated honestly Three cases are driven end to end: real persisted rows from a live store, the real exported predicate, both call shapes. The call-site fact is asserted against source text, not driven — reaching those sites needs the full dependency-lease reconcile harness, which I did not build. The last case reads the file and says so in its own comment rather than dressing it up as an end-to-end result. It doubles as an alarm: when those call sites are converted it fails and points at the three cases above, which describe exactly what changes. ## Mutation-verified Flipping the callee's default from `"in-progress"` to `"building"`: \| case \| result \| \|---\|---\| \| CONTROL (default board, no options) \| fails \| \| CHARACTERIZATION (renamed board, no options) \| fails \| \| BOUND (renamed board, option passed) \| passes — correct, the option overrides the default \| \| SOURCE-LEVEL \| passes — correct, it is a source assertion \| The two default-dependent cases bind to the default; the bound case proves the override; nothing passes for the wrong reason. ## Not done, and why No fix. Passing the resolved answers at the two self-healing sites requires resolving each blocker's column traits there — an async resolution inside a reconcile path that already holds locks, and `self-healing.ts` is another worker's file. Flagging with a differential that says exactly what the fix should make true. ## Verification - new suite — 4/4 passed, mutation matrix above - full live-PG E2E surface — 137/137 passed (133 on main + 4) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:59:57 -07:00
gsxdsm	1496ba9658	test(engine): bound the inert-sync-resolution class on a live store (#2794 ) ## What One new live-PostgreSQL E2E suite, 3 tests. No production file is touched — evidence, per the E2E worker's remit. Closes the series: #2789 (scheduler), #2791 (planner lanes), #2792 (custom fields), #2793 (terminal node). `packages/engine/src/__tests__/workflow-sync-selection-blast-radius-live-e2e.pg.test.ts` ## Why this one is different The four PRs above each proved a site broken because it resolved a task's workflow synchronously. Read together they invite a conclusion that is false and would be expensive: that every synchronous consumer of the workflow selection is inert. Most are not. The difference is one line of shape: ```ts // GUARDED (correct) store.getTaskWorkflowSelectionAsync ? await store.getTaskWorkflowSelectionAsync(id) : store.getTaskWorkflowSelection(id) // UNGUARDED (inert) store.resolveTaskWorkflowIrSync(id) ``` The real PostgreSQL store does implement the async reader, so every guarded site takes the async arm and resolves the card's own workflow. Only the sync IR helper — which has no async arm to fall to — is stuck with the default. Observed on one live store, one persisted workflow, one task: ``` hasAsyncReader = function SYNC selection = undefined ASYNC selection = { workflowId: "WF-001", stepIds: [] } EFFECTIVE planReviewMaxRevisions = 9 <- the custom workflow's declared default ``` ## The point "The ternary saves them" is an inference from reading, and the whole premise of this program is that reading is what let the class survive in the first place. The guarded sites are exactly the ones a fleet worker would otherwise "fix": converting a correct site costs review time, risks behaviour, and produces a diff that looks like progress. This makes the bound checkable in the same lane as the defects. Guarded call sites (correct today): `workflow-settings-resolver.ts`, `workflow-ir-resolver.ts`, `executor.ts`, `workflow-graph-task-runner.ts`, `workflow-task-runtime.ts`, and `board-workflows.ts` in the dashboard. ## The allow-listed family is now closed \| site \| status \| \|---\|---\| \| `scheduler.ts` \| proven broken — #2789 \| \| `replan-target.ts` \| proven broken — #2791 \| \| `task-store-helpers.ts` \| proven broken — #2792 \| \| `branch-and-pr-entities.ts` \| proven broken — #2793 \| \| `workflow-task-create-ops.ts` \| legitimately correct — creation runs before any selection exists, so the default IR is the right answer \| \| `lifecycle-ops.ts` \| NOT proven, stated as such \| `lifecycle-ops.ts`'s stale-transition-pending recovery re-runs plugin column-transition hooks against the sync IR. Driving it needs a registered plugin hook plus a crash-simulated marker; I did not build that harness and I am not substituting a unit test for it. Named in the file so it is not mistaken for covered. ## Evidence discipline - Observed state. Both readers called on one live store against one persisted workflow, plus a real resolved settings value — not a spy on which arm ran. - The settings default is `9`, deliberately not the builtin's, so the value can only have come from this workflow. - Mutation-verified. Rewriting the guarded consumer to call the sync reader directly fails exactly the bound arm; the two structural arms are correctly unaffected, which is what a bound should do. ## Verification - new suite — 3/3 passed, mutation-verified - full live-PG E2E surface — 136/136 passed (133 on main + 3; #2791 landed while this branch was in flight) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:56:48 -07:00
gsxdsm	90f6319b79	batch-engine tail: re-land the ASYNC half; the sync-resolved half was inert (engine −15) (#2785 ) Tail of `batch-engine` (#2773). That PR merged as a squash while later engine work was still in flight, so `self-healing.ts`, `executor.ts` and `worktree-pool.ts` landed at their pre-conversion counts. This re-lands only the half that is real, and the reason the other half is not here is the substance of this PR. ## Census, per file (measured, `--strict` verified) \| file \| main \| here \| \| --- \| ---: \| ---: \| \| `engine/src/self-healing.ts` \| 107 \| 97 \| \| `engine/src/executor.ts` \| 15 \| 12 \| \| `engine/src/worktree-pool.ts` \| 3 \| 2 \| \| `engine/src/ephemeral-worker-manager.ts` \| 1 \| 0 \| \| `engine/src/agent-tools.ts` \| 5 \| 0 \| \| `engine/src/gridlock-detector.ts` \| 3 \| 0 \| \| `engine/src/triage.ts` \| 4 \| 1 \| \| `engine/src/mission-execution-loop.ts` \| 2 \| 0 \| \| net \| \| −28 \| Baseline re-recorded; `--strict` tightened exactly these 4 entries and no others. ## Finding: a whole class of conversions in this program is INERT, and the census scores it as progress `resolveTaskWorkflowIrSync` returns the default workflow IR for every task in production. The sync selection reader behind it is a PostgreSQL-cutover stub: ```ts // packages/core/src/task-store/workflow-definitions.ts:505 export function getTaskWorkflowSelectionImpl(_store, _taskId) { return undefined; // "Backend mode cannot synchronously read PostgreSQL" } ``` So a guard written as `resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(id))?.hold` resolves an IR, asks for a trait, and answers from the default workflow for every custom board — silently. It reads as converted and the census counts it as converted. `main` gained `sync-workflow-ir-callsite-allowlist.test.ts` for exactly this after my branch point; it is what caught me. I had built three sync resolvers on that reader — `resolveMoveLanesSync` (self-healing, executor) and a widened `resolveTaskParkedColumnsSync` (scheduler) — reasoning that a synchronous `task:moved` listener needs a synchronous reader. That reasoning was sound about the shape and never checked whether the reader reads anything. Dropped from this PR, deliberately, and NOT re-landed anywhere: - `scheduler.ts` 12 → 1 (the widening; the pre-existing narrow helper on main is untouched) - the executor `task:moved` handler, incl. the Move-Task hard-cancel lane comparison - self-healing's `task:moved` fan-out, `classifyPausedAbortWorkflowRecovery`, `reconcileInReviewBranchRebind`, `recoverWedgedActiveMerge`, `recoverPausedAbortFailures`, and 12 single-row lane conversions Those sites are back to their literals. The allow-list's own guidance is the standard I applied: > An unconverted `=== "todo"` is strictly better, because it is at least honest about being a literal. I did not add my call sites to the allow-list. Six entries would have turned the gate green in two minutes and buried the defect; the list's contract requires proving the async resolver is genuinely unreachable, and for a fire-and-forget listener it is not — the listener can `void` an async lane resolution the same way `NotificationService` already does. That is the correct fix and it is a behaviour-shaped change, so it is out of scope here. Fleet-wide consequence: any conversion routed through `resolveTaskWorkflowIrSync` is fake progress, and the census cannot see the difference. `pnpm test:gate` can: the allow-list test is the detector. Its passing here (161/161) is this PR's evidence that nothing inert survived the split. ## What IS in this PR — all async-resolved 1. `self-healing.clearStaleBlockedBy` — lanes resolved per REFERENCED task, not per iterated task. A blocker's own workflow decides whether it is still blocking. 2. `executor` dependency satisfaction — resolved per DEPENDENCY via `columnsWithFlag`. Preserves the load-bearing asymmetry that a dependency in review already satisfies a dependent; a bulk sweep flattens that to complete-only and deadlocks the board. 3. `agent-tools` — the agent task tools listed FINISHED cards as active. `fn_task_list` says it lists "tasks that aren't done or archived"; `fn_task_search` offers `includeDone: false`. Both filtered on `task.column !== "done"`, so a renamed complete lane returned finished cards as outstanding work to an agent, which then reasons and acts on them. `includeArchived` was always enforced by the QUERY and survived a rename; `"done"` was only ever a TS predicate, which is why exactly that half broke. Plus the two dedup guards in the same file. The cross-parent diagnostic filter kept a shipped card as a candidate on a renamed board, so the guard adopted it as canonical and returned `wasDuplicate: true` — absorbing new diagnostic work into a task nobody is working on (the eval-followup defect shape again). The defined-feature bootstrap preflight is not the query-filter class: its query passes `includeArchived: true`, so the TS predicate is the only archived guard there; on a renamed archive lane the archived sibling became the bootstrap canonical and `claimDefinedFeatureTask` then rejects the non-live row, so a valid first task fails to be created at all. Both dedup invariants already had tests — asserted against the legacy ids only, so both passed for the very comparison being replaced. Extended in place into vocabulary differentials rather than added as parallel files. Two helpers rather than one parameterised one: "is this finished?" and "is this archived?" are different questions, and merging them would make the archived-only guard also reject completed rows. The list/search half re-landed with the test it originally shipped without. No suite exercised either tool, so the original commit's "304/304 green" said nothing about the change — the optional-flags failure mode exactly. Both call sites are covered; converting two copies and testing one is the Surface Enumeration failure this program has already hit twice. 4. `gridlock-detector` — FALSE dependency alarms. The gate compared each blocker against `done`/`in-review`/`archived`; on a renamed board all three are true for a finished blocker, so no dependency ever counted as met and the detector reported dependency gridlock for tasks that are not blocked — `notifyGridlock` then pages the operator. Resolved per dependency using the same five flags as the executor's gate (`complete`, `archived`, `mergeOrchestration`, `mergeBlocker`, `humanReview`) — `review` is not a trait, and two gates answering "is this dependency satisfied?" differently is a split brain. Every pre-existing case in that file omits a workflow, so none could detect the change; added the renamed case plus a non-vacuous companion. 5. `triage` — its OWN copies of the same two tools. `createTriageTools` carries a `fn_task_list` and `fn_task_search` byte-identical in intent to the agent-tools pair, plus a third site filtering duplicate candidates. Same defect on all three. Reused the (now exported) agent-tools helper rather than adding a third copy — deliberately stronger than the two-parallel-tests reading of Surface Enumeration, since the copies now share one implementation and cannot drift. Not claiming call-site coverage: `createTriageTools` is private and not drivable without standing up a TriageAgent; the helper is revert-proofed, those two call sites are covered only through it. 6. `mission-execution-loop` — a finished fix task read as LIVE, stalling remediation. The comment above that line states the rule it implements: only an open task makes duplicate triage safe to suppress. On a renamed board the rule inverts — a finished fix task is not `done`/`archived`, so it reads as live, remediation for a fresh validation failure is suppressed indefinitely, and the mission stalls with no error surfaced. Not revert-proven, and I am not claiming it is. No test reaches the `hasLiveFixTask` branch, and the only case that mints a fix feature is git-gated and heavyweight; building that fixture is larger than the conversion. The change strictly widens the finished set (resolved roles ∪ the two legacy ids), so default boards are byte-identical — that is the argument for shipping it unproven, not a substitute for coverage. 7. Four census-invisible membership guards, each inverted on a renamed board — `worktree-pool` (merger-managed branch reclaim could delete a branch out from under an in-flight merge), `agent-assignment` (assignment load counted nothing), `ephemeral-worker-manager` (`isAgentIdle` inverted on both sides), and the dead constants their conversion orphaned. These are `SET.has(task.column)` shapes the census does not count, so the −15 understates them. ## Revert results (measured, each run) \| conversion \| reverted → \| \| --- \| --- \| \| `clearStaleBlockedBy` per-referenced lanes \| renamed-vocabulary case fails; stale `blockedBy` never clears \| \| executor dependency satisfaction \| dependent never unblocks on a renamed review lane \| \| `worktree-pool` merger-managed set \| reclaim proceeds against an in-flight merge \| \| `ephemeral-worker-manager.isAgentIdle` \| idle agent reads busy on a renamed board \| \| `fn_task_list` terminal filter \| RENAMED case fails — shipped card listed as active \| \| `fn_task_search` terminal filter \| RENAMED case fails — same, independently \| \| cross-parent diagnostic dedup \| RENAMED case fails — `wasDuplicate: true`, new work absorbed \| \| bootstrap preflight archived guard \| RENAMED case fails — `validate` called with the archived sibling \| \| gridlock dependency gate \| RENAMED case fails — false gridlock raised for an unblocked task \| `agent-assignment`'s widened `taskStore` type is compile-time; its revert is a tsc failure, not a test failure — stated rather than claimed as coverage. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, all green (161 includes `sync-workflow-ir-callsite-allowlist`) - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean One commit is a pure import restore: `columnsWithFlag` arrived in a sibling commit that built on the inert resolver and was left behind. The engine tsconfig excludes `src/__tests__/`, so the gate was green while tsc was not — worth knowing that on this package a green gate is not a green build. ## Verified NOT a gap — measured, so the next worker does not re-open them - `restart-recovery-coordinator` (5 counted). Four already take an optional `reviewColumns` set and the counted literals are the documented fallback arm, which must stay for the same reason `columnRoles.ts` keeps its id fallback. The sole production caller (`self-healing.ts:12151-12154`) already passes the resolved set. The fifth is documented at the site as a re-assertion behind a `listTasks({ column: "in-progress" })` query filter. Nothing to convert. - `notification/notification-service` (5 counted). Already documented in-file as deliberately counted with no exemption marker: the wedge-episode site needs per-task serialisation of wedge handling (a delivery-semantics change to operator notifications), and `isManualMergeHold` needs a pre-resolved `LifecycleColumns` threaded through `handleTaskUpdated`, which would pay resolution on every task update. Both are behaviour/placement judgements, not conversions. - `planner-overseer` (3 counted). `resolveWatchedStage`'s two literals are fed by `pollPlannerOverseer`, which calls `listTasks({ column: "in-progress" })` and `{ column: "in-review" }` — hardcoded query filters. On a renamed board those queries return no rows, so the predicate never sees a renamed column. Converting it alone would drop 3 from the census and change nothing an operator can observe. The real fix is at the query layer; that is the tracked query-filter-bounded class, not this PR. - `triage:695` reads `resolvePlannerLanes` → the allow-listed sync IR reader. Left as an honest literal per the rule above. Still open in `packages/engine`, deliberately not in this PR:** `self-healing.ts` (97, of which ~31 are the query-filter-bounded class and the rest need per-site classification in a 13k-line file), `scheduler.ts` (12, blocked on the sync reader above), `executor.ts` (12), and a tail of ~13 more copies of the "is this task finished?" question across eight small files (`agent-reflection`, `auto-merge-finalization`, `merger-scope-auto-widen`, `backlog-pressure-reporter`, `merger-orphan-rehome`, `merger-integration-worktree`, `plugin-runner`, `cli-agent/*`). That tail is a clean follow-up: one question, eight call sites, and the exported `resolveTerminalColumnsForTasks` helper already exists for it. That is the same discipline as the sync-resolver finding: a census number that drops without a behaviour change is not progress, and four of these files would have handed over exactly that. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:47:44 -07:00
gsxdsm	6bdde6f246	fix: five lifecycle gates the census cannot see — incl. live ephemeral workers reaped and duplicate follow-up cards (#2787 ) Five lifecycle-column fixes the census structurally cannot see. Each gate is a `Set` or array literal — a definition, not a comparison — so no backlog entry ever pointed at any of these files. Found by grepping for lane-shaped list literals after the same shape surfaced in `duplicate-intake` and `blocker-fanout` (both merged via #2780), then confirmed by reading each USE site. On opening this: I offered twice to fold these into a PR and kept them on handoff refs to respect one-open-PR-per-worker. They have now sat unadopted across several cycles while `main` moved, and two of them destroy or duplicate work. Opening is the reversible call — close it if it breaks queue policy and I will keep them on the branch. ## What is in it \| commit \| defect on a renamed board \| severity \| \|---\|---\|---\| \| `beb107a7bc` \| assignment load-balancing defeated — `assignmentLoad` stays empty, every candidate reads as load 0, the sort falls through to its stable `createdAt` tiebreak, so one agent wins every assignment while the rest idle \| distribution \| \| `cf4b59e1cb` \| the zombie sweep deletes LIVE ephemeral workers \| destroys work \| \| `5fe004ae64` \| eval follow-up dedup sees zero open tasks, so every run re-files follow-ups it already filed \| duplicate cards \| \| `a1021de8b2` \| agents keep a "working on" indicator for finished cards \| stale UI \| \| `86680d1220` \| the Files tab never loads — the fetch never fires \| silent empty \| ### The one that destroys work `shouldDeleteOnSweep` tested a hard-coded terminal `Set`, then fell through to `return task.column !== "in-progress"`. On a renamed board both halves miss, and they compound in the worst order: the terminal test fails, control reaches the fallthrough, and `"building" !== "in-progress"` is `true`. An ephemeral worker actively executing a task is classified as a zombie and deleted. Nothing logs. Its fallback is deliberately asymmetric, and the comment says why: an unresolvable workflow keeps the legacy literals rather than guessing. Failing to reap a dead worker costs a slot; reaping a live one destroys work in flight. Those are not symmetric, so uncertainty fails toward keeping the worker. ## Verification Verified as a set, not only per-branch: - `pnpm test:gate` — 161 / 13 / 487 / 71 - engine suites (assignment, ephemeral, eval-followups) — 44 passed - dashboard suites (agent-task-link, useSessionFiles) — 16 passed - `tsc` engine + dashboard server + dashboard app — clean - `pnpm lint` clean · census `--strict` exits 0 Revert-proven individually. Restoring each literal fails its own case: the renamed-wip zombie case, the renamed-wip assignment case, the renamed-lane dedup case, the sanitizer ratchet, and both `useSessionFiles` role cases. ## Two honesty notes, flagged rather than buried `a1021de8b2`'s guard is STRUCTURAL, not behavioural. `sanitizeAgentTaskLinks` is a closure inside `createApiRoutes`, reachable only by standing up the full express app. The ratchet asserts the source — resolver threaded per task, bare literal call gone, cache shared, fallback retained — and fails on revert, verified. It is not a substitute for a behavioural test; whoever owns the dashboard server should add one if that seam grows. `useSessionFiles`'s negative case passed in isolation and failed in the suite. Hooks are not unmounted between cases there, so a prior case's in-flight fetch landed inside it. That is the classic shape of a test that gets "fixed" by reordering; it now asserts a delta against the pre-render call count, which is independent of what leaks in. ## Deliberately NOT included `worktree-pool.ts:1205` — the sixth site from the same sweep. It fails safe: a missed match means the skip does not fire, so the branch is added to `activeBranches` and protected from cleanup. The cost is stale branches accumulating, not deletion. It also sits in the merger's branch-reaping path, where the opposite error destroys work, so it deserves its owner's judgement rather than a drive-by conversion. Flagged, not guessed. Also still open and unclaimed: roughly 69 untriaged literal-list sites across engine/dashboard/cli. The grep is one line and the file list is on #2775 — with the measured caveat that about half are false positives on shape alone (`LEGACY_*` names, seeds unioned with resolved values, and `roles: ["triage"]`, which is an `AgentCapability`, not the deleted column). Only the use site settles it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:35:31 -07:00
gsxdsm	dcf9900d61	test(engine): live-PG differential between resolvePlannerLanes and its async twin (#2791 ) ## What One new live-PostgreSQL E2E suite, 5 tests. No production file is touched — evidence, per the E2E worker's remit. Follows #2789 (same defect class, different site). `packages/engine/src/__tests__/workflow-planner-lanes-sync-vs-async-live-e2e.pg.test.ts` ## The finding `replan-target.ts` exports two functions with identical logic and identical fallbacks, differing only in how they obtain the task's IR: ``` resolvePlannerLanes(store, taskId) -> store.resolveTaskWorkflowIrSync(taskId) resolvePlannerLanesForTaskAsync(store, taskId) -> await resolveWorkflowIrForTask(store, taskId) ``` Under PostgreSQL the sync selection reader answers `undefined` for every task, so the sync twin resolves the default workflow for every card regardless of the board it is on. Nine production call sites use it (2 × `executor.ts`, 7 × `triage.ts`); one uses the async twin. The module's own doc comment argues this, and the store-level fact is proven in `sync-workflow-ir-is-always-default.pg.test.ts`. What had no executable evidence is the consequence at this seam against a real store with a real persisted workflow. That is this file — a pure differential: both twins, same store, same task, same call. ### Two harms, different severity 1. Wrong lanes, labelled authoritative. `resolvedFromWorkflow` exists to tell a caller "these came from the workflow, not the fallback". The sync twin sets it `true` — an IR did come back — while handing over the default board's ids. A caller that correctly checks the flag before trusting the lanes is misled precisely by checking it, which is strictly worse than the honest `false` an unresolvable store would give. 2. Invented forward lanes. `wip`/`review`/`complete` are optional so a caller refuses rather than moving a card into a column the board does not declare (PR #2628's review). The sync twin defeats that contract without touching it: never having seen the real board, it reports the default board's forward lanes as present. The optionality is intact in the type and unreachable in practice. The sharpest arm: a board declaring no review lane gets `undefined` from the async twin and `"in-review"` from the sync twin. ### A correction worth carrying forward "It falls back to the legacy lanes" is the wrong mental model twice over. `LEGACY_PLANNER_LANES` (`hold: "todo", intake: "triage"`) is reached only when no IR resolves at all — under PostgreSQL, never. What a caller actually receives is the post-U11 merged default, whose intake and hold are one `todo` lane. So the sync twin does not return `triage` for intake; it returns `todo`, and a caller reading `intake` gets not merely a wrong id but a lane that is not a dedicated intake at all. This is also why the control arm uses `MERGED_VOCAB`: the shape the twins agree on is the merged one. `DEFAULT_VOCAB`, which splits intake out as `triage`, already separates them. ## Evidence discipline - Observed state. These are exported pure functions over a live store; the observation is their return value against a persisted workflow definition. No spies, no mock IR anywhere in the file. Contrast the unit coverage in `planner-lanes-async-resolution.test.ts`, which must supply a mock `resolveTaskWorkflowIrSync` and therefore cannot see this divergence at all. - Control arm. On the post-U11 default shape the twins agree exactly — which is why this survived: every default-board test passes and only a renamed board separates them. - Characterization, not endorsement. The four renamed arms assert the wrong-but-current values deliberately; they flip when the call sites move to the async twin, and that flip is the point. ### Mutation-verified, including a round that found weak arms Replacing the sync twin's whole body with `return LEGACY_PLANNER_LANES`: \| \| arms failing \| \|---\|---\| \| first draft \| 3 of 5 \| \| after strengthening \| 5 of 5 \| Two arms originally asserted only `wip`/`review`/`complete`, which are identical in the merged default IR and in `LEGACY_PLANNER_LANES` — so they proved the lanes were wrong without proving why, and survived the mutation. Each now also pins `intake` (`todo` merged vs `triage` legacy), the single field that separates "resolved the wrong board" from "took the fallback". Recorded in the file next to the assertions. ## Not done, and why No fix. The async twin already exists and is documented as a drop-in ("identical logic and identical fallbacks — the ONLY difference is awaiting the authoritative resolver"), so the migration is mechanical where the caller is already async. It is not universally so: several `triage.ts` sites are inside synchronous paths, and `triage.ts:831` calls `resolvePlannerLanes(this.store, "")` with an empty task id — a sweep-wide lane read that has no single task to resolve against and needs a decision, not a mechanical swap. Both are behaviour calls in files another worker owns; flagging, not smuggling. ## Verification - new suite — 5/5 passed, mutation-verified 5/5 - full live-PG E2E surface, 20 suites — 126/126 passed (was 121/121) - `pnpm lint` — clean - `pnpm check:lifecycle-columns` — exit 0 Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Tests * Added end-to-end coverage comparing synchronous and asynchronous workflow lane resolution. * Validated lane consistency for renamed, non-default, and custom boards using persisted workflow data. * Added checks for incorrect fallback lanes, workflow resolution indicators, and absent review lanes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:32:24 -07:00
gsxdsm	87442b9664	test(engine): live-PG evidence that declared custom fields cannot be written (#2792 ) ## What One new live-PostgreSQL E2E suite, 4 tests. No production file is touched — evidence, per the E2E worker's remit. Third in the series after #2789 and #2791; same root cause, materially worse consequence. `packages/engine/src/__tests__/workflow-custom-fields-sync-resolution-live-e2e.pg.test.ts` ## The finding A workflow that declares custom fields cannot have any of them written. `TaskStore.resolveTaskCustomFieldDefsSync` reads a task's field definitions through `store.resolveTaskWorkflowIrSync`, which under PostgreSQL answers `undefined` for every task and therefore resolves the default workflow IR. The default declares no `fields`, so the function returns `[]` for every task on every board. `task-update.ts` validates every write against that empty list: ```ts const defs = store.resolveTaskCustomFieldDefsSync(id); const result = validateCustomFieldPatch(defs, updates.customFields); if (!result.ok) throw new CustomFieldRejectionError(result.rejection); ``` Observed against a real store with a real persisted workflow declaring one `text` field: ``` STORED fields = [{"id":"risk","name":"Risk","type":"text"}] SYNC defs = [] WRITE threw = CustomFieldRejectionError custom field 'risk' rejected (no-fields-defined): the resolved workflow declares no custom fields; no values may be written ``` The rejection message is a true statement about the workflow that got resolved and a false one about the workflow the card is on. ### The two halves of the feature disagree in production The executor resolves the same definitions through the async resolver (`executor.ts` → `resolveTaskCustomFieldDefs` → `resolveWorkflowIrForTask`) and sees the real field. So an agent can be prompted to supply a value that the store will then refuse to store. The last case asserts both answers against one store, one task, one workflow — which is why this cannot be dismissed as a fixture artefact. This is a different severity from the previous two PRs in the series. #2789 and #2791 are wrong-lane defects, mostly latency, one of them unbounded. This one is a declared feature that does not function off the default board. ## Scope on record Three write paths share the sync resolver: `task-update.ts` (driven here), `workflow-task-create-ops.ts:394`, and `workflow-ops.ts:488`. Only the first is exercised; the other two are named in the file so the surface is recorded rather than implied. Also worth stating plainly: because the empty list is the default IR's `fields`, the same rejection is what a default-board card gets too. The feature is not merely renamed-board-broken. ## Evidence discipline - Fixture integrity first. The opening case asserts the stored workflow really does declare the field via the async resolver. Every other assertion is about a missing definition and would pass just as well against a workflow that never declared one — that case is what makes the rest mean something. - Observed state. The thrown typed rejection plus the absence of a persisted value on a re-read row. No spy on the validator. - Mutation-verified. Replacing the sync resolver's body with a hardcoded `[{id:"risk",…}]` fails 3 of 4 arms. The fourth is the fixture-integrity case, which exercises the async path by design and correctly survives. ## Not done, and why No fix. The async resolver already exists and is already used by the executor for the same data, so the shape of the fix is clear — but `task-update.ts`'s validation runs inside a synchronous update path, and making it async is a behaviour decision in `@fusion/core` that belongs to that file's owner, not to a smuggled edit in an evidence PR. The call-site allow-list entry for `task-store-helpers.ts` ("Synchronous helper shared by txn-hot paths") should cite this suite either way: the entry is accurate about the constraint and silent about the cost. ## Verification - new suite — 4/4 passed, mutation-verified 3/4 (fourth by design) - full live-PG E2E surface, 20 suites — 125/125 passed (121 on main + 4) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:32:12 -07:00
gsxdsm	755ada91ac	test(engine): live-PG evidence that the terminal-node guard fires on the wrong node (#2793 ) ## What One new live-PostgreSQL E2E suite, 3 tests. No production file is touched — evidence, per the E2E worker's remit. Fourth in the series after #2789 (scheduler), #2791 (planner lanes), #2792 (custom fields). `packages/engine/src/__tests__/workflow-terminal-node-sync-resolution-live-e2e.pg.test.ts` ## The finding FN-7641 Signature 2 exists because setting `nodeId` to the terminal node used to be written verbatim and silently do nothing — the card sat in review with every step done, unadvanced and unexplained. The contract: a terminal override with durable merge proof finalizes the card; without proof it is rejected with an actionable error; non-terminal overrides are untouched. On a board whose terminal node is not called `end`, both halves invert: \| write \| contract says \| actually observed \| \|---\|---\|---\| \| `nodeId: "end"` — an ordinary planning node here \| written, untouched \| rejected with a merge-proof error about finalizing a card the operator was not finalizing \| \| `nodeId: "finish"` — this board's real `end`-kind node \| finalize, or reject \| written verbatim, no error, card left in review \| The second row is the original FN-7641 bug, restored on every custom board. ## The correction the mutation runs forced My first draft blamed `isTaskTerminalNodeIdImpl`'s sync IR resolution alone. Mutating it changed only one of the two cases, which is how I found there are two guards: ``` branch-and-pr-entities.ts:568 validateNodeOverrideChange(task, nodeId, { isTerminalNodeId }) -> sync IR resolution (the default board, under PostgreSQL) task-update.ts:53 validateNodeOverrideChange(task, nodeId) -> NO options, so `defaultIsTerminalNodeId` — the bare literal `nodeId === "end"` ``` The inner one is an unconverted literal sitting behind a converted call site, and it silently overrides it. Converting the outer guard alone changes nothing an operator can see. A column census cannot find the inner one either — `end` is a node id, not a column. This is the "a guard survives in a branch of the same function" shape, one function apart. ### Mutation matrix \| corrected \| `end` rejected \| `finish` silent \| \|---\|---\|---\| \| (nothing — main) \| pass \| pass \| \| outer sync-IR guard only \| pass \| fail \| \| inner `defaultIsTerminalNodeId` only \| pass \| fail \| \| both \| fail \| fail \| Two different failure structures, which is why the cases are kept apart: - `end` rejected is over-determined — both guards independently call it terminal, so it survives a mutation of either one. Not a weak assertion: a faithful record of a defect with two independent causes, and the reason a partial fix here is invisible. - `finish` silent is under-determined — both guards must miss the id, so correcting either flips it. This is the arm that notices a partial fix. The fixture-integrity case exercises the async resolver by design and correctly survives every mutation. ## Fixture The shared builder's terminal node is `end`, so it cannot express this shape. This file derives from it: one `lifecycleIr`, node ids shifted so the `end`-kind node is `finish` and the non-terminal planning node takes the name `end`. Columns, traits, edges and structure are otherwise the builder's, so the only variable is which node ids carry which kind. The first case asserts that shift really happened — both characterizations are claims about which node is terminal and would read as defects if the fixture had quietly kept the builder's ids. ## Evidence discipline - Observed state. Whether `updateTask` throws, and what the re-read row's `nodeId` and `column` actually are. No spies. - Characterization, not endorsement. Both cases assert the wrong-but-current behaviour deliberately, and the matrix above says exactly which fix flips which. ## Not done, and why No fix. It needs two coordinated edits in `@fusion/core` — threading the resolved terminal check into `task-update.ts:53`, and making the outer resolution async — and the second is the same synchronous-path constraint as #2792. Both are behaviour decisions in another worker's files. Worth flagging that fixing only the allow-listed sync site would look like progress and deliver none, which the matrix above makes checkable. ## Verification - new suite — 3/3 passed, mutation matrix above - full live-PG E2E surface — 124/124 passed (121 on main + 3) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:29:04 -07:00
gsxdsm	e467d939a5	fix(tests): the last 3 engine reds — a pause guard asserted at the wrong layer (#2779 ) ## What was red The final 3 failures in `executor-prompt.test.ts` ("global pause behavior"), all the same assertion: ```ts expect(mockedCreateFnAgent).not.toHaveBeenCalled(); ``` made after calling `executor.execute(task)` directly on a paused todo row. ## It is not a regression, and not a live safety hole I initially flagged this in #2778 as a possible live hole — "a user-paused todo task now reaches `createFnAgent`". That framing was wrong, and the bisect is what corrected it: \| commit \| result \| \|---\|---\| \| `origin/main` (HEAD) \| fail \| \| `main~40` \| fail \| \| `main~80` \| fail \| \| `main~150` \| fail \| \| `main~250` \| fail \| Red 250 commits back. It never described shipped behaviour, so nothing regressed. `execute()` holds no pause gate. Neither `executeCore` nor the workflow-graph executor consults `paused`/`userPaused` before starting a session — I checked both. Refusing to dispatch a parked row is the scheduler's invariant, enforced twice: 1. Candidacy is keyed on both flags (`scheduler.ts:138`) — `userPaused` is a durable operator stop even when legacy `paused` is false. 2. The row is re-read immediately before dispatch and refused if it comes back parked (`scheduler.ts:2086`) — this closes the race the first check cannot. The test called `execute()` directly, stepping around the component that owns the guarantee, then asserted the bypassed layer enforced it. A true statement about the system was being made to look false. Every protective outcome #2371 documented does hold and stays asserted: `fn_task_done` never completes the card, it is never handed to `in-review`, no completion watchdog is armed, the pause is never cleared, and the run narrates the benign paused park. Only "no session was created" was false. The 3 sibling assertions in `resumeOrphaned` are untouched — that path genuinely does refuse. ## The invariant moves to the layer that owns it Rather than delete an assertion and lose the coverage, `scheduler-paused-dispatch-refusal.test.ts` pins it through real `schedule()` passes: - control — an unparked ready card IS dispatched - refuses a row parked with legacy `paused` - refuses a row parked with `userPaused` alone - refuses when the operator pauses after selection, before dispatch Driven through `schedule()` rather than by calling the predicate directly: a test that calls the guard cannot tell whether the dispatch path still consults it — which is precisely how the executor-prompt version came to assert a layer that had stopped being asked. ## The control earned its place on the first run It failed immediately, and twice over: the hold-release gate refuses a card still carrying a bootstrap seed (fixed with the shared `seedPlannedSpec`), and a `moveTaskIf` stub returning `moved: false` makes the release unobservable. Without the control, all three refusals would have passed vacuously — a scheduler that dispatches nothing refuses everything. ## Evidence - 106/106 across both files. - Mutation: removing the pre-dispatch pause re-read → the race case fails. The other two are caught earlier by candidacy (defence in depth); the passing control makes their refusal attributable to the flag alone, since the same store dispatches without it. - Gate 732 green · lint clean · engine `tsc --noEmit` 0 errors. Test-only (the scheduler mutation was reverted; `git diff` clean). ## Engine suite status Measured baseline on `origin/main`: 39 failures / 10835 passed. With #2776 (32, notifier harness) and #2778 (4), this last set takes the engine suite to 0 failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:04:52 -07:00
gsxdsm	145c022af4	test(engine): live-PG evidence that the scheduler's sync parked-column read is inert (#2789 ) ## What One new live-PostgreSQL E2E suite. No production file is touched — this is evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts` (2 tests) ## The finding `scheduler.ts`'s `resolveTaskParkedColumnsSync` resolves a task's hold/intake columns through `store.resolveTaskWorkflowIrSync`. Under PostgreSQL that reader answers `undefined` for every task, so the resolver returns the default IR and the function yields `{ hold: "todo", intake: "triage" }` on every board — byte-identical to the literals it was converted away from. It is an inert conversion, and it is currently allow-listed (`sync-workflow-ir-callsite-allowlist.test.ts`) on the grounds that these handlers are synchronous. Five call sites read it. Four groups of handler fail as latency — a wake that does not fire costs up to one poll interval, which is why the class hid. The `task:deleted` dependency reconciliation is different: it queries `listTasks({ column: hold })` and re-checks `dependent.column === hold` before clearing `blockedBy`. On a renamed board both tests are against `"todo"`, a column that board does not contain, so a dependent parked in the renamed hold column is never unblocked and waits forever on a blocker that is already gone. Persisted, operator-visible, unbounded. ### It contradicts a passing unit test `scheduler-renamed-hold-events.test.ts` asserts the opposite and passes, because its mock supplies `resolveTaskWorkflowIrSync: vi.fn(() => renamedIr())` — an answer the real store provably never gives. That test is not wrong about the scheduler (given a working resolver the handlers do resolve the renamed lane); it is wrong about the resolver. Flagging rather than editing it: it is still the right unit test for its own subject, and it is not my file. ### The mechanism is not the one the code reads like The obvious reading blames the fail-soft `?? "todo"`. It is not that — `lifecycle` is never nullish, a real default IR comes back and real traits resolve off it, so both `??` arms are dead in production. Established by mutation, not by reading: \| mutation to `resolveTaskParkedColumnsSync` \| control arm \| renamed arm \| \|---\|---\|---\| \| (none — main) \| unblocks ✅ \| never unblocks ✅ \| \| both `??` fallbacks → renamed vocabulary \| unblocks (unchanged) \| never unblocks (unchanged) → dead branch \| \| returned object → renamed pair \| fails \| fails → both arms decided here \| This is the sharpest form of the defect class: the site resolves an IR and reads a trait off it, so it looks converted at every level except the one that decides the answer. ## Evidence discipline - Observed state, not spies. Each arm asserts the dependent's persisted `blockedBy` after a real soft-delete on a real store with real stored workflow definitions. - The negative is self-validating. The handler's work is fire-and-forget, so observing it needs a bounded wait — and a bounded wait proving a negative is normally worthless. The default-vocabulary arm is the control: same store, same window, and it does unblock (297 ms against a 2 000 ms window). If this ever flakes the control fails first; the fix is the quarantine ledger, never a larger number. - Differential. Both boards come from the one shared vocabulary builder and differ only in their column ids. - The renamed arm is a characterization test — it asserts the wrong-but-current behaviour deliberately, and is expected to flip when the read is fixed. ## Two fixture traps found on the way (both would have made this vacuous) 1. `updateTask({ column })` is not a column move — `column` is not in the update payload, so it typechecks as an unknown key and leaves the card where it was. Use `moveTask`. 2. Order matters. Writing the blocked state after placing the card re-homes it to the intake column (observed `todo -> triage`), and the reconciliation re-checks `dependent.column === hold` — so the control fails for a fixture reason that looks exactly like the defect. Block first, then place. Both are written down in the file. ## Not done, and why No fix. The honest fix is to make the read async, and that is not free: these run inside synchronous `task:moved` / `task:updated` listeners, where an added `await` defers the rest of the handler to a microtask and reorders handlers against a synchronous emitter. That is a behaviour decision in a file another worker owns, so it belongs to whoever owns `scheduler.ts` — not to a smuggled edit in an evidence PR. The allow-list entry should cite this suite either way. ## Verification - `workflow-scheduler-parked-columns-live-e2e.pg.test.ts` — 2/2 passed, mutation-verified in both directions (table above) - full live-PG E2E surface, 19 suites — 121/121 passed (was 119/119) - `pnpm lint` — clean - `pnpm check:lifecycle-columns` — exit 0 Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:04:34 -07:00
gsxdsm	698bded476	fix(tests): 4 engine reds on main — each was green for a reason the fleet removed (#2778 ) ## Context A full `@fusion/engine` run on `origin/main` (`9b61d795c9`) reports 39 failures / 10835 passed. 32 are the notifier harness, fixed in #2776. This PR takes 4 of the remaining 7. All three files share one shape: each case was passing off something the lifecycle conversions have since correctly taken away. In every one, the product is fine and a good change landed as a red test. --- ### 1. `executor-graph-failure-lanes-resolved.ts` — an equality that fails on its own fix The guard forbids resolving a lifecycle guard through the synchronous `resolvePlannerLanes` (a no-op under the shipped PostgreSQL backend, so the census counts the site as converted while it behaves like the literal). It asserted `expect(callSites).toBe(3)`. #2764 converted the promotion-path site to `resolvePlannerLanesForTaskAsync` — exactly the direction this guard wants. Count went 3 → 2 and the assertion failed. The guard's own comment states the invariant as "Any FOURTH is a new sync resolution" — one-directional. Coded as equality, it fails on removal, which is the change it exists to encourage. Now `toBeLessThanOrEqual(2)`. Mutation: adding a third sync call site → `expected 3 to be less than or equal to 2`. Still load-bearing. ### 2. `restart.integration.test.ts` — a fixture matching a fallback constant `recoverCompletedTask` re-homes intake → hold → wip only when the origin is the board's intake lane; otherwise it hands straight to review. The failure showed the 1st move as `in-review` with no re-home. Nothing regressed. The fixture put the card in `triage` and resolved lanes through the sync resolver, so it fell through to `LEGACY_PLANNER_LANES` — where `intake` is literally `"triage"`. It was matching a hardcoded fallback, not a declared lane. #2764 made the site await the real resolver; the mock selects `builtin:coding`, and U11 merged intake and hold onto one Planning column (`todo`), so `triage` is not a lane on that board and the two-hop correctly collapses. The invariant the test is named for — completed work in a distinct intake lane is re-homed along a legal path, not moved intake → review, which role adjacency rejects — is still real. So the fixture now declares a board with intake separate from hold, the only shape where the two-hop is reachable. Mutation: removing the re-home hop from the product → fails with the expected `todo` first-move. Load-bearing. ### 3. `executor-abort-provenance.test.ts` — a call one argument short Both provenance cases returned `false` for a clean completed in-review row. This reads as an FN-6796 regression stranding rows that are already handed off for review. It is not. #2703 added a 7th `reviewLane` parameter so the lane is resolved by the caller. The call goes through `as any`, so the missing argument was not a type error — it arrived `undefined`, `live.column !== reviewLane` held for every row, and the classifier answered false for everything. Passed explicitly rather than defaulted inside the classifier: a default would restore the literal the parameter exists to remove. Added a differential — a card resting in a renamed review lane classifies the same, a mismatched one does not — so the parameter cannot be re-literalized while still looking converted. Mutation: `live.column !== "in-review"` → the differential fails. The other cases pass, which is precisely why it was worth adding. --- ## Evidence \| file \| result \| \|---\|---\| \| `executor-graph-failure-lanes-resolved` \| 24 passed \| \| `restart.integration` \| 48 passed \| \| `executor-abort-provenance` \| 16 passed \| Gate 732 green · `pnpm lint` clean · engine `tsc --noEmit` 0 errors. Test-only — no product file is touched by this PR (the mutations above were run and reverted; `git diff` confirms clean). ## Deliberately NOT fixed here 3 cases in `executor-prompt.test.ts` ("global pause behavior") remain red on main: a user-paused todo task now reaches `createFnAgent`. That is a safety invariant rather than a stale fixture, and neither `executeCore` nor the graph executor holds a pause gate — the refusal #2371 documented is not where its note implies. It gets its own change; editing the fixture to match current behaviour would hide it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 09:40:49 -07:00
gsxdsm	c643d62e85	fix(executor): wipDeclared must ask ALL six lifecycle roles, not two (#2777 ) ## What this fixes `resolveResumeLanes` returns `wipDeclared`, which gates whether `routeGraphFailureToExecutionResume` may route a graph failure back into execution resume. Getting it wrong terminalizes tasks on boards that should resume. Two prior versions were wrong, both caught in review rather than by me at write time: 1. Two-state (`lifecycle?.wip !== undefined`) — greptile P1 on #2760. A v1-upgraded board terminalizes: `synthesizeDefaultColumns` emits `{ id, name: id, traits: [] }`, so every role resolves `undefined` even though those columns literally are the legacy lanes. Verified by parsing a real v1 IR. 2. Proxying "synthesized" as "hold and review are both undefined" — my own fix for (1), and also wrong. I caught this against #2765 rather than shipping it. A v2 board that declares only `intake` + `complete` has hold and review undefined too, so it would be misread as synthesized and treated as declaring wip when it deliberately does not. The failure mode both versions share: reading a sample of the roles and treating the answer as a verdict about the whole IR. #2765 says it directly — an empty result has two meanings, and you cannot tell them apart from a subset. ## The rule ```ts wipDeclared: lifecycle?.wip !== undefined \|\| !declaresAnyLifecycleRole(lifecycle), ``` Three states, asking all six roles: - wip declared → true, the board says so. - some role declared but not wip → false. A v2 board that omits wip means it; do not resume into a lane it did not define. - no role declared at all → true. That is the synthesized/v1-upgraded shape, whose columns are the legacy lanes; the pre-existing behaviour is correct there and must not regress. `declaresAnyLifecycleRole` iterates `Object.values(lifecycle)` rather than naming roles, so a seventh role added later is included automatically instead of silently falling into the wrong branch. ## Evidence - `executor-resume-lanes-resolved.test.ts`: 7 passed, +23 lines covering the v1-synthesized board and the declares-some-but-not-wip board. - Mutation: restoring the naive two-state rule → 1 failed / 6 passed. The added coverage is load-bearing and pins exactly the regression greptile caught. - Gate 732 green · `pnpm lint` clean · engine `tsc --noEmit` 0 errors. - Rebased on current main. ## Scope `executor.ts` (+22) and its test (+23). One predicate; no other behavior touched. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-30 09:28:41 -07:00
gsxdsm	8c9b84ae38	batch-core: packages/core + dashboard/src lifecycle conversion (129 → 92) (#2780 ) ## batch-core — `packages/core` + `packages/dashboard/src` Shared branch: two workers are converting into it. Opening the PR because the branch was green with none, and a branch without a PR merges nothing. ### Census Measured with `node scripts/lifecycle-column-census.mjs --json`. \| \| guards \| \|---\|---\| \| batch-core scope at branch point \| 129 \| \| batch-core scope now \| 92 (51 files) \| \| repo total now \| 358 \| Files closed so far: `store.ts` 11→0, `task-merge.ts` 6→0, `live-agent-count.ts` 6→0 (marked, not converted — see #2762), `task-update.ts` 3→0, display-ordering + Wake Delta ranking 5→0, `register-git-github.ts` 4→0. ### The `register-git-github.ts` slice Three PR routes — `pr/create`, `pr/push-branch`, `pr/resolve-conflicts` — plus the `CHANGES_REQUESTED` handler each compared `task.column !== "in-review"`. On a renamed board none of them matched, so every PR affordance the dashboard offers was refused for a card sitting in the lane that board calls review, and the refusal named a column that does not exist there. All four now share one helper, `reviewColumnsForTask`, which gets two things right that this program has repeatedly gotten wrong: - Membership, not a single id. It takes the broad review set (`mergeOrchestration ∪ mergeBlocker ∪ humanReview`). `resolveLifecycleColumns` returns the first column per trait, so a single-id answer silently ignores a board that declares a merge lane and a separate human sign-off lane. These guards only refuse or permit — they never move the card — so over-admitting costs nothing while under-admitting refuses a request that should have worked. - An empty resolved set means UNEXPRESSED, not absent. `synthesizeDefaultColumns` upgrades a v1 graph by emitting every default column with `traits: []`, so a v1-upgraded workflow resolves to an empty review set while its `in-review` column plainly exists and holds the card. Reading empty as "this board has no review lane" would refuse these routes on every pre-v2 project — a worse regression than the one being fixed, and invisible to any v2 test. This is the dashboard twin of the `fn pr create` guard in `packages/cli/src/commands/pr.ts` (#2775). The two surfaces answer the same question and now agree — FN-5893 surface enumeration. ### Testing note: why the seam and not the routes I wrote route-level HTTP tests first and deleted them. An express fixture over `registerGitGitHubRoutes` hangs — every case, including the pure refusals, times out at 4s, because registering the router starts background work the fixture never satisfies. Making it run would mean mocking git, the GitHub client, and the pollers: a mock-the-world shell, which is what the project's do-not-add-slow-tests rule (FN-5048) says to avoid in favour of a narrow seam. `reviewColumnsForTask` is the narrow seam — it holds the entire decision, and the four call sites now do nothing but ask it and render its answer. Six cases pin it: the renamed lane is returned and `in-review` is not, a two-lane board returns both, a v1-upgraded board falls back, an unresolvable workflow falls back, and the refusal renders lanes an operator can act on. Mutation-verified, both directions: reverting the helper to the legacy literal fails 2 of 6; treating an empty set as an answer fails 1 of 6. One fixture bug worth recording, since it would have made the two-lane case vacuous: the trait id is kebab-case `human-review`, not `humanReview`, and the built-in traits must be registered via `import "@fusion/core"` before flags resolve. ### Verification - `pnpm --filter @fusion/dashboard exec tsc --noEmit -p tsconfig.json` → 0 errors - `pnpm lint` → 0 errors - `register-git-github.review-lanes.test.ts` → 6 passed --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 09:28:27 -07:00

1 2 3 4 5 ...

3727 Commits