3445 Commits

Author	SHA1	Message	Date
gsxdsm	8e211d1870	TAKING scripts/: parse instead of grep — an AST classifier for the lifecycle-column bar, cross-checked by a second implementation (#2633) ... The program's completion bar is "`column === "triage"` reaches zero". This measures what that bar actually covers, and checks the measurement in so it cannot drift. ## The number, measured by the checked-in tool ``` lifecycle-column-census: scanned 1956 source files COLUMN guards (the backlog): 1031 ROLE comparisons (not guards): 10 DELIBERATE-LITERAL (reviewed): 4 by column id: 313 done 217 in-review 201 in-progress 177 archived 83 todo 40 triage top files: 151 packages/engine/src/executor.ts 136 packages/engine/src/self-healing.ts 50 packages/dashboard/app/components/TaskCard.tsx 44 packages/core/src/task-store/moves.ts 34 packages/dashboard/app/components/TaskDetailModal.tsx ``` **`triage` is under 4% of the class.** Every one of those 1031 sites is the same defect: a lifecycle decision made by column NAME, which stops matching the moment a board renames a column. The bar can be met in full while 991 identical guards remain — and two files hold a quarter of them. ## The tracked count is wrong in three directions at once Each of these cost real work this week, which is why this is a PR and not a comment. 1. **Vocabulary.** It measures one of six legacy ids. 2. **Receiver.** It is anchored on locals named `column`/`toColumn`/`fromColumn`, so it never saw the three real guards in `executor.ts` written against `from` and `originColumn`. One of those meant completed-but-stranded work was never recovered on a renamed board, with nothing else owning that state (converted in #2628). 3. **Collision.** `role === "triage"`, `agentType === "triage"`, `entry.agent === "triage"` compare an **AGENT ROLE**. The planner *lane* is named `triage` and keeps that name — U11 removed the *column*. Ten such sites were counted as backlog, and the "obvious" fix (renaming the role) silently empties the planner's prompt template and mis-binds its model markers. A count that is too high and too low simultaneously sends work to the wrong files while hiding the files that need it. So the census reports **three separate numbers** and never nets them. ## Proven to fail on the original defect Not asserted — exercised: ``` $ # reintroduce `task.column === "triage" || task.column === "todo"` into live-agent-count.ts $ node scripts/lifecycle-column-census.mjs --strict; echo "exit=$?" packages/core/src/live-agent-count.ts: 10 -> 12 exit=1 $ # restore the file $ node scripts/lifecycle-column-census.mjs --strict >/dev/null; echo "exit=$?" exit=0 ``` The CLI also exits 1 when its own file list comes back empty — a guard that reports success without checking anything is worse than no guard. ## 12 regression cases, split by what they defend Must catch: all six ids; a guard on a local named `from`/`originColumn` (verbatim the executor.ts shape); single quotes; negation; several comparisons on one line. Must **not** catch: role comparisons; comment prose (two tracked "guards" in `replan-target.ts` were prose about a filter that lives in another file); a trailing `// … === "triage"` on a code line; sites carrying a `DELIBERATE-LITERAL` marker. Plus: **one marker cannot launder a distant guard in the same file** — that is how allowlists rot. ## Report-only, deliberately `--strict` compares per-file counts against `scripts/lib/lifecycle-column-census-baseline.json` and fails when any file's count **rises**. It is **not** wired into the merge gate: a thousand-site backlog cannot be a blocking check the day it is first measured, and a guard nobody can pass is a guard everyone disables. Owners tightening their own area re-record the baseline in the PR that lowers it. This is the ratchet shape the `DELIBERATE-LITERAL` markers scattered through the program already anticipate. ## Stated limitation Classification is by receiver **name**, so a future field named `agent` that holds a column would be misclassified as a role comparison. Recorded at the site, and it is precisely why the two classes are reported separately instead of netted into one figure. ## Verification - 12/12 new cases (`packages/engine/src/__tests__/lifecycle-column-census.test.ts`) - `pnpm test:gate` **71/71**; `pnpm lint` clean - `pnpm census:lifecycle-columns`, `--json`, and `--strict` all exercised end to end - documented in `docs/testing.md`; no production code touched 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:52:18 -07:00
gsxdsm	26c82ebc18	ratchet the planner-liveness gate so a fourth door fails CI (FN-6756) (#2540) ... Test-only follow-up to the merged P0 (#2531). No production change, no changeset (internal). ## Why This bug reached users **three times**, each as the same mistake in a new place: | | What happened | |---|---| | FN-8600 | the reclaim sweep removed a worktree a live **planner** was using — fixed by registering planning paths and teaching *that* sweep `isPathActive` | | FN-6756 | the leaked-slot reaper never got the same signal; its last line of defense computed liveness from four TaskExecutor-owned maps, so a triage planner matched none of them | | (same PR) | fixing that was not enough — `recoverPausedAbortFailures` **discarded** the refusal and still logged `"Auto-recovered…"`, audited and counted it. The whole bug again, while reporting success | The shared cause is not any one sweep: **“liveness” was re-derived per call site**, so closing one door left the next open and nothing failed. Every one of those fixes was found by review, not by CI. This makes the next one a CI failure. ## Four properties, each written to fail on the exact defect that got through 1. **Every `clearPhantomExecutorBinding?.(` call site consumes its return** — a bare expression statement (including `void`/`await`-prefixed) is the signature of the pause-abort defect. 2. **The destructive path delegates to `hasLiveSessionSurface`** rather than inlining the session-map disjunction — a second copy can drift from the one callers gate on, which is precisely how each sweep got “fixed” without fixing the next. 3. **The probe is wired** in `in-process-runtime`. `self-healing.ts` already records `releaseExecutorWorktreeOwnership` as a declared-but-never-wired option that silently no-opped; an unwired *probe* is worse, since `?.() === true` is `false` when unwired and every gate would quietly stop deferring with nothing failing. 4. **The probe counts registered session paths**, not just executor maps — a triage planner appears in no executor-owned map, so that term is the only thing that sees it. Grep-level, comment-stripped, production source only; no engine boot and no fixtures (FN-5048). Fails closed on an empty/moved source file so a rename cannot make it silently check nothing. ## Proven, one injection at a time **The first draft of property 1 was worthless** — its filter chain was convoluted enough to discard every candidate, so the injected bare call passed. Caught by actually running the injection instead of trusting the green, and rewritten as a single “is this a bare expression statement” rule. | Injection | Result | |---|---| | discard the return value | fails, naming the call site | | re-derive liveness inline | fails on the delegation assertion | | unwire the probe | fails, naming `in-process-runtime` | | drop the registry term | fails, naming `activeSessionRegistry` | Clean tree passes 4/4; all three sources restored byte-identical (`git status` shows only the new file). **Verified:** `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (414 + 10 + 71). 🤖 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 safeguards to ensure liveness checks remain consistently enforced. * Verified phantom executor cleanup uses shared session-liveness detection. * Added coverage for registered session paths to prevent false inactive states. * Added fail-closed checks when required runtime source is unavailable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:42:26 -07:00
gsxdsm	55ce01335c	self-healing.ts: resolve pre-WIP columns by role — 11 → 0 receiver-agnostic (largest single item) (#2560) ... Taken ahead of my capacity slice, per the drift review. U11 merges the two pre-implementation columns into one that **keeps the id `todo`** and **deletes `triage`**. Every `column === "triage"` here is live breakage the moment that IR lands — and it does not throw, it simply **stops matching**, so the sweep never fires again and the suite stays green. That is the Problem Frame’s measured failure mode, landing on self-healing, where a silently-dead recovery is least likely to be noticed. ## Count for tracking convergence `self-healing.ts`, code only, `column === / !== "todo" | "triage"`: | | before | after | |---|---:|---:| | `"triage"` comparisons | **10** | **0** | | `todo` + `triage` combined | 24 | 15 | The 15 remaining are all `"todo"`, whose id **survives** U11 — not breakage, and deliberately left for the hold-column conversion rather than mixed in here. ## Ten sites, converted by role (intake / hold) advanced-triage recovery (3, one sweep) · dependency-deadlock blocked dependents · parked-agent task link · orphaned-approved planning · orphaned planning · duplicate-decision candidates · refine-source sweep · leaked-slot reaper ## Two literals the grep did not count — and they would have silently killed their sweeps `listTasks({ column: "triage" })` in both orphaned-planning sweeps. Converting only the predicate would have left the **query** returning nothing once the id is gone; the sweep would have looked converted and done nothing. Both now query the board and filter by role. All ten route through one seam (`resolvePreWipColumns` / `filterByPreWipRole`) with a caller-owned per-sweep cache, so 400 cards over three workflows read three IRs, not 400. ## Two judgement calls, stated **Unresolvable workflows fall back to the legacy literals, not to nothing.** These are *recovery* sweeps: a card whose IR cannot be read must keep its current behaviour rather than silently drop out of every sweep. That is the conservative direction *here*, and deliberately differs from conversions whose failure mode is a destructive move. Pinned by test. **The leaked-slot reaper’s predicate is left as-is and flagged in place.** It is arguably too *wide* under plan-in-place — a card being specified sits in the hold column while a planner works in its worktree, so “waiting to run must not pin a worktree” no longer holds. What stops that being live is the FN-6756 liveness gate (already merged). Narrowing it is a behaviour change and gets its own commit; this PR is vocabulary only. The dependency-deadlock site needed restructuring rather than substitution: its filter is synchronous and role resolution reads the IR, so membership is precomputed once per sweep. ## Verification **Revert-proof, measured:** making the resolver return the literals regardless of workflow turns **4 of the 6** new cases red — the renamed-workflow ones, precisely the case a literal cannot serve. `self-healing.ts` restored byte-identical. `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (414 + 10 + 71) · self-healing suites **436 passed / 2 failed** — the same 2 pre-existing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Re-verified against current `origin/main` (2026-07-29) Per the definitive-list instruction, re-measured rather than re-asserted — comment-stripped, so FNXC prose quoting a removed literal does not inflate the count. - `origin/main` `self-healing.ts`: **10** code sites (lines 2964, 2984, 3019, 9218, 10703, 11282, 12173, 12218, 12321, 12494 — the same ten, line numbers shifted only). - This branch: **0**. - Main touched this file after my branch point (#2600, the R7 dead-guard fix). Checked: its diff adds **no** `"triage"` literal, and git reports this PR `MERGEABLE`, so the merge result stays at 0 — the branch being behind does not hide a new site. So merging this moves the tracked number **45 → 35**. Both stated constraints hold: every site resolves to the **intake/hold ROLE** (not a renamed literal), and the count is zero across the whole file rather than per-branch — there is no surviving guard in another branch of the same function. The two `listTasks({ column: "triage" })` **queries** are converted too; predicate-only conversion would have left these sweeps looking converted while returning nothing. ## Re-measured RECEIVER-AGNOSTICALLY (2026-07-30, after the revised bar) The revised count matches any receiver, not just `.column`. Re-measured with that pattern, comment-stripped, this file is **11 → 0**, not 10 → 0. | pattern | origin/main | this branch | |---|---:|---:| | `<anything> === / !== "triage"` (any receiver, both quote styles, line-splits) | **11** | **0** | The eleventh is the one your list attributes separately to `engine/self-healing.ts`: ``` origin/main:3017 if (!resumeColumn || resumeColumn === "triage") continue; ``` `resumeColumn` is a bare local holding `live.workflowIrPinColumnId`, so a `.column`-anchored pattern cannot see it. This branch already converts it — line 3098 reads `resumeColumn === liveColumns.intake`, resolved from the same per-sweep role cache as the other ten. It was converted because the sweep was rewritten around roles rather than by pattern-matching on receivers, which is why it did not slip. **Merging this therefore moves the revised 56 by 11, to 45.** ### The 5 literal mentions that remain, and why each is not a guard Nothing above is a comparison. For completeness, since "a file is not done because the pattern is gone from it": - **3 legacy fallbacks** (`?? "triage"`) at 2978, 2980, 10821 — the unresolvable-workflow path in `resolvePreWipColumns`. These are *recovery* sweeps: a card whose IR cannot be read must keep its current behaviour rather than silently drop out of every sweep. Pinned by test. - **2 union reads** at 6044 (`["triage", "todo"]`) and 9291 (`listTasks({ column: "triage" })` alongside a `todo` read) — unions covering both vocabularies, with the role filter deciding membership. Neither replaces nor disables anything. Known residual gap, stated rather than hidden: those unions do not cover a **renamed** intake (Coding (Ideas)'s `ideas`). That gap **pre-dates U11** — the same unions missed `ideas` before the merge — so it is not a regression here, and closing it needs a cross-workflow lane union rather than a vocabulary edit. ## AST-VERIFIED, replacing the grep-derived figure (2026-07-30) Since no grep-derived number is authoritative, I re-measured this file by PARSING it — `ts.createSourceFile`, walking binary expressions, classifying on the left-hand side. Not a pattern match. | | origin/main | this branch | |---|---:|---:| | lifecycle-column comparisons (AST-classified) | **11** | **0** | All eleven classify as `COLUMN`; none is an agent role, session purpose or surface name, so all eleven are real guards and every one is converted: ``` 2964 task.column 9218 dep.column 12218 task.column 2984 live.column 10703 task.column 12321 task.column 3017 resumeColumn 11282 linkedTask.column 12494 t.column 3019 current.column 12173 t.column ``` `3017` is `resumeColumn` — a bare local holding `workflowIrPinColumnId`, which is exactly the receiver class the `.column`-anchored greps missed. It is converted here to `resumeColumn === liveColumns.intake`, resolved from the same per-sweep role cache as the other ten. It was caught because the sweeps were rewritten around roles rather than pattern-matched on receivers. The classifier is on #2623 as `scripts/lib/lifecycle-column-ast.mjs` and is offered to #2630 to import. My earlier "11 → 0" was correct — but it was a regex reading, and this is the same number arrived at by parsing. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:42:14 -07:00
gsxdsm	833f379fbd	U7: ratchet the planning claim to a single writer (extracted from #2517 so it can land) (#2587) ... > Test-only, based on `main`, no dependencies. **Extracted from #2517**, which has been overtaken by events — see the bottom. U7's stated verification is *"a grep-level assertion that planning status literals have one writer module"*. This is that assertion, and the plan names **FN-8504** as the acceptance case: a store-open sweep cleared a live planner's status because two owners wrote it. ## What it asserts 1. **`status: "planning"` — the claim on a card — is written by exactly one production module** (`triage.ts`). 2. That single write goes through `updatePlanningStateIfStillCurrent`, never a bare `store.updateTask`. FN-7977 and FN-8361 are both the latter bug. ## What it deliberately does not assert Stated in the file so the guard is not oversold: - **Who clears the status.** Eleven modules write `status: null` for unrelated reasons — "one clearer" would be *false*, and the only way to make it pass is to weaken it into meaninglessness. - **`needs-replan`.** Post-U3 it is the graph's own durable replan signal with multiple writers **by design**. - **Mission `status: "planning"`.** A different entity, excluded by *path* rather than by pattern — a pattern loose enough to tell them apart is loose enough to miss a real task write. ## Writes and bindings are separate contracts Adding constant-indirection detection (`const CLAIM = "planning"` defeats every shape pattern) flagged `replan-target.ts` — which binds the literal only to **exclude** it from a status set and writes no task status anywhere. Calling that a second writer would have been a false accusation; dropping the binding rule would have reopened the hole. So the **write** list stays at one module and **binding** is its own allowlist. ## Proven to fail — twice A guard that cannot be shown to fail is not a guard. **1. The real scan function is re-run over a fixture tree** by four of its own tests — a re-implementation would prove only that the copy works. Covers every evasion form (single quotes, template literal, whitespace, plain assignment, computed key), the indirection route, a comparison-only counter-case, and a brand-new package. **2. End-to-end against real source**, in a form the *original* detector missed on **both** axes — a single-quoted writer in `packages/desktop`, a package the first version never scanned. Re-verified on post-U11 `main` just now: ``` FAIL ... + "packages/desktop/src/bundled-plugin-dirs.ts" Tests 1 failed | 13 passed (14) ``` Both holes — double-quote-only matching, and a hardcoded four-package scope — were found by greptile on #2527, and are why the detector now **discovers** its roots by enumerating `packages/<name>/src` rather than listing them. ## Why this is a separate PR **#2517 has been overtaken.** Main absorbed a better discovery conversion from another worker (it handles U11's merged column, which mine did not); #2515 then changed the column vocabulary underneath it; and what remains there is entangled with 14 U11 fixture updates. This file is a source scan with no dependency on any of that, so it lands on its own. My recommendation on #2517: **close it** and re-land its remaining unique content — the sweeps, the handler snapshot, stuck-abort, `recoverApprovedTask`'s intake gate — as focused PRs against post-U11 main. Holding a 1262-line PR open through three vocabulary changes has cost more than it has delivered. That is a reversible call and it is yours if you disagree; say so and I will rebase it instead. ## Verification | Check | Result | |---|---| | suite | 14/14 | | `pnpm lint` | clean | | `pnpm test:gate` | green (482 + 10 + 71) | Cheap by construction (FN-5048): grep-level over production source, no engine boot. 🤖 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 automated safeguards to ensure planning task cards have exactly one valid writer. * Added checks covering direct and indirect status updates, supported syntax variations, package discovery, and comment or test-file exclusions. * Added validation that planning updates occur only through the approved guarded workflow. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:41:30 -07:00
gsxdsm	8beba5f543	U7 E2E evidence: the planning lane, real PostgreSQL + real hold-release sweep (3/7 red without the guard) (#2611) ... > Completion-bar **item 3** for my phase. Test-only, based on `main`, no dependencies, no changeset. ## The gap this closes The planning lane was the **one lifecycle lane with no E2E coverage**. The existing live-E2E files cover the lifecycle spine, agent count, agent link, lease rebound, the merge family and the rebound family — **not one exercises a planning decision.** Every U7 fix shipped with the caveat the other units already removed for their lanes: *"all evidence is unit-level."* That caveat matters more here than anywhere, because the planning fixes are **guards that refuse things**, and a refusal is what unit tests are worst at proving. Nine times on this program a planning test passed without exercising its subject: a fake that ignored its predicate, a store stub returning a non-promise into `.catch`, a fixture that silently resolved to the default IR, a control that passed when it should have failed. ## What is real - a per-file **throwaway PostgreSQL** TaskStore (never the operator's) - the **real `runHoldReleaseSweep`** — every guard, trait resolution, reservation ordering, and the in-transaction `moveTaskIf` predicate - **persisted rows** read back with the store's task cache defeated, so an assertion can only have come from the row Nothing about the AI is substituted, because none of these decisions involve it — there is no seam here to script. ## The proof, which is the point With **#2491's two approval guards removed** from `hold-release.ts`, this file goes **3 of 7 red against real PostgreSQL**: ``` FAIL does NOT release a card blocked on manual plan approval on a default board FAIL does NOT release a card blocked on manual plan approval on a renamed board FAIL holds a card parked for approval MID-SWEEP, after the snapshot was read Tests 3 failed | 4 passed (7) ``` Restored: **7/7**. So the file demonstrably exercises the guard rather than merely observing that a sweep works — which the two control cases (an ordinary held card **is** released, on both vocabularies) exist to keep falsifiable. ## The case no unit test could honestly make The **mid-sweep** case needs the in-transaction predicate enforced by a real store. The hand-built fake that shipped with #2491 originally **did not honour the predicate at all** — exactly what greptile caught. Here PostgreSQL enforces it, and the sweep's own log confirms the refusal: ``` [scheduler] Hold release for FN-RACE skipped — task became paused or left todo ``` It parks the card inside `reserveSlot`, which runs *after* the snapshot and *before* the move — the precise window the in-txn half exists for. ## Coverage Both approval hold shapes (`status: "awaiting-approval"` and `paused` + `pausedReason`), on **both** vocabularies, so no assertion can pass by matching a legacy id. The renamed run's log shows the real sweep releasing the control card to `building`, not `in-progress`. ## 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, no temp-root walk. ## Verification | Check | Result | |---|---| | E2E suite (real PostgreSQL) | 7/7 | | same suite with #2491's guards reverted | **3/7 fail** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (482 + 10 + 71) | ## Still owed on the completion bar for my lane E2E for the other three U7 fixes — approved-plan recovery (#2593), the spec-staleness exemption (#2583), and the discovery advancement guard (#2576) — is **not** in this PR. Those need a driver for triage's own poll/recovery path rather than the sweep, which is a different harness shape; adding it here would have made this PR a harness project rather than evidence. Taking that next. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:40:05 -07:00
gsxdsm	31e49b684a	TAKING default-workflow-hooks.ts + executor.ts + live-agent-count.ts + 6 dashboard files: reopen semantics by role, and the census's blind spot in both directions (13 sites) (#2628) ... Batched conversion of every lifecycle-column guard I hold, plus the three the census could not see. **Six files to zero, repo-wide 60 → 49 by a comment-stripped unanchored sweep.** Each conversion has an isolated revert proof and a paired negative case, and the one code move is a separate commit from the behavior changes. ## Per-file before → after Counts from a comment-stripped, unanchored `(===|!==) ["']triage["']` sweep over `packages/*/src` + `plugins/*/src`, excluding tests. | file | before | after | note | |---|---:|---:|---| | `core/default-workflow-hooks.ts` | 4 | **0** | | | `core/task-store/moves.ts` | 5 | **4** | only the flag-ON mirror converted; the flag-OFF inline block is the parity reference and stays | | `engine/executor.ts` | 3 | **0** | **absent from the 45-guard list** — see below | | `core/live-agent-count.ts` | 2 | **0** | duplication removed; answer deliberately unchanged | | `engine/replan-target.ts` | 2 | **0** | both were comment prose, not guards | | `core/agent-prompts.ts` | 3 | **0** | ROLE comparisons, never column guards | | `engine/usage-limit-detector.ts` | 2 | **0** | ROLE comparisons | | `dashboard/app/components/DocumentsView.tsx` | 1 | **0** | real column guard | | `dashboard/app/components/TaskChatTab.tsx` | 2 | **0** | ROLE | | `dashboard/app/components/AgentLogViewer.tsx` | 1 | **0** | ROLE | | `dashboard/app/components/effective-model-resolution.ts` | 1 | **0** | ROLE | | `dashboard/app/hooks/useTasks.ts` | 1 | **0** | ROLE | | `dashboard/…/command-center/MissionControlPanel.tsx` | 1 | 1 | alias table, marked `DELIBERATE-LITERAL` with its reason | ## The census errs in BOTH directions This is the finding I would most like carried into the remaining work. - It **flagged 10 sites that were never column guards.** `role === "triage"` / `agentType === "triage"` compare an **AGENT ROLE**. The planner *lane* is named `triage` and keeps that name — U11 removed the *column*. Worse than noise: the obvious "finish the migration" edit is to rename the role, and that silently empties the planner's prompt template and mis-binds its model markers. `PLANNER_AGENT_ROLE` now names it, so the two vocabularies are distinguishable by grep and a rename fails loudly (revert proof: 4 tests, two of them pre-existing). - It **missed 3 real guards in `executor.ts`**, because the pattern matches `column`/`toColumn`/`fromColumn` and those locals are named `from` and `originColumn`. A census keyed on variable names will keep missing guards wherever a local was named for its role in the function. ## Two real defects, not tidying **1. A renamed board could merge with its re-review never run.** `default-workflow-hooks.ts` is named for the default workflow, but the store runs it on the flag-ON path for *every* workflow — the trait registry resolves hooks by trait id, not by workflow. Its reopen predicates listed the default lineage's column names, so on a renamed board **no reopen effect fired at all**. One of them clears `workflowStepResults`, which `getTaskMergeBlocker` reads: a card bounced out of review carried its old `passed` result back in, and that satisfies the merge gate. Same regression the graph-owned-crossing carve-out exists to prevent, arriving through the other door. (Two smaller ones rode along: failure state never cleared on a renamed reopen, and an operator dragging a card back to the queue never parked it, so the scheduler re-dispatched what they had just pulled back.) **I forgot the carve-out on my first pass, and that was worse than not converting.** A role-resolved clear plus a *name*-matched exemption means a renamed board takes the clear and never the exemption, destroying the remediation input the graph had just written. My own paired negative test caught it. **2. The last-resort recovery for completed-but-stranded work did not exist off the default lineage.** In `recoverCompletedTask`, `promotedFromPlannerColumn` was false on a renamed board, so finished work resting in the planning lane was never promoted — the code fell through to `handoffTaskToReview` straight from the planning column, and role adjacency has no planning → review edge, so the handoff was rejected and the card stayed stuck with its work complete. I converted the promotion **target** too: resolving the lane and then moving to a literal `in-progress` is the half-conversion I have already been burned by twice this program, where the guard starts admitting cards and the move then sends them to a column the board does not declare. ## E2E evidence `renamed-board-reopen.pg.test.ts` drives a **real PostgreSQL store** and a real `moveTask` on a workflow whose columns carry the standard traits under non-default names. The unit tests cannot show this: if `moves.ts` passed `undefined`, every unit case still passes via the no-basis fallback while the real board keeps the old behavior. **Proof it is load-bearing: forcing `moveLifecycleColumns` to `undefined` fails 2 of 3.** The executor suite covers both the split-role and the MERGED post-U11 shape. ## Revert proofs, isolated per site | change reverted | result | |---|---| | reopen predicate → literal names | 4 of 10 fail | | reopen field clears → literal names | 2 of 10 fail | | `userPaused` hold lane → literal `todo` | 1 of 10 fail | | graph carve-out → literal names | 1 of 10 fail | | store passes `undefined` lifecycle columns | 2 of 3 fail (real PG) | | `promotedFromPlannerColumn` → literals | 3 of 7 fail | | two-hop condition → `=== "triage"` | 1 of 7 fails | | promotion target → `"in-progress"` | 3 of 7 fail | | `isPlannerColumnFor` → literals | 1 of 7 fails | | live-agent-count: one arm dropped | 2 of 11 fail | | DocumentsView: trait branch removed | 3 of 7 fail | | planner role renamed to `"planner"` | 4 fail (2 pre-existing) | Every conversion is paired with a negative case (a forward move, a not-a-planner-lane card, a default-lineage card, a renamed column with no traits), so neither "always fire" nor "never fire" can pass for "resolve the role". ## Deliberately NOT converted, with reasons - **`moves.ts` flag-OFF inline block (4).** That branch *is* the legacy path, kept verbatim so the two can be parity-checked. Converting it erases the reference implementation. - **`live-agent-count.ts`'s no-flags fallback.** Reachable, and there is nothing to resolve from — `enrich…FromFlags` exists for callers with board flags rather than an IR, so a column missing from that map is the renamed case. "Not intake" is as much a guess as "todo is intake", and Running/Waiting are complements, so a card matching neither arm is reported as neither and the footer's queued total under-reports it. The real fix is at the caller; four new cases pin that flags override the legacy answer **in both directions**. What did change is the duplication: two hand-written copies of one rule now call one named function. - **`MissionControlPanel`'s `FUNNEL_STAGES`.** An alias table of column *names* where `triage` sits beside `signal` and `backlog`. Command Center aggregates across projects, so there is no single workflow to resolve traits from — the honest conversion is a data change, not a predicate change. - **`DocumentsView` with no traits.** Same no-basis rule; the documents list is full of historical columns absent from the current board. A case asserts a renamed column with no traits still reads as "working", documenting the gap rather than hiding it. ## Fixture findings Each cost a red run that looked like the code under test: - a `merge-blocker` column needs a reachable merge-class node, or `parseWorkflowIr` rejects the workflow; - a back-edge must be `kind: "rework"`, and a rework edge is legal only **into** a node with `config.reworkRegion: true`; - a workflow gets role-level transitions only when it declares wip + review + complete + **archived** plus a planning lane — without the archived column, adjacency falls back to order-derived neighbours and `checking -> queued` is not a legal move at all; - `recoverCompletedTask` only *reaches* the promotion seam when nothing is left to gate; without passed `plan-review`/`code-review` rows it re-enters the workflow graph and returns first, so a naive fixture silently tests the wrong branch and every assertion reads "no moves happened" for an unrelated reason. ## Verification - `pnpm test:gate` **71/71** - new suites: 10/10 reopen-semantics, 3/3 renamed-board-reopen (real PG), 7/7 executor-planner-lanes, 7/7 documents-status-dot, 4/4 planner-role-is-not-a-column - neighbours: 132 + 10 + 482 (gate shards), 350/351 engine planning/replan suites, 64/64 agent-prompts, 51/51 usage-limit-detector, 11/11 live-agent-count, 11/11 dashboard hook/log suites - the single engine failure (`executor-fast-mode-workflows.test.ts` › "raw fast mode still invokes non-executable review seam nodes") **reproduces with my changes stashed** — pre-existing on `origin/main` - typechecks clean for core, engine, and dashboard-app (`tsconfig.app.json`; `tsconfig.json` checks nothing under `app/`); `pnpm lint` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:39:14 -07:00
gsxdsm	c9f117bcd9	fix(test): dep-abort rebound asserts the resolved column, not the retired triage literal (#2641) ... **Review-lane test, 1 failed → 21 passed.** Test-only. `pnpm test:gate` green, `pnpm lint` clean. ## Production was right; the test was stale ``` expected moveTask("FN-DEP", "triage") received moveTask("FN-DEP", "todo") ``` `handleDepAbortCleanup` no longer hardcodes a column. It moves the card to `resolveReboundColumnFor(store, taskId)` (`executor.ts:16576`), which resolves the task's **own** workflow rebound target by trait — hold, else intake, else first column — with a `todo` fallback. For this fixture's default workflow that resolves to `todo`, which post-U11 **is** the merged Planning column. `triage` is not declared on the default lineage at all, so the old expectation was asserting a column the workflow does not have. ## Why the concrete value, not the resolver I asserted `"todo"` rather than re-calling `resolveReboundColumnFor` in the test. Deriving the expectation from the code under test makes the assertion agree with whatever the resolver happens to return — the exact anti-pattern `task-delete-notice.test.ts` documents for its notify table ("deriving the expectation from the value under test makes the suite agree with whatever the production constant happens to say"). That's only legitimate because **per-workflow resolution already has its own coverage** — `replan-target-merged-planning-column.test.ts` and `replan-target-renamed-planner.test.ts`. I checked they exist rather than assuming; without them, pinning a concrete id here would be hiding the interesting behaviour. Also added the negative: the move must **not** be `triage`, so a regression that reinstates the literal fails instead of quietly passing on a column the default lineage no longer declares. **Red-green:** reinstating `moveTask(taskId, "triage")` in production fails exactly this test (`NEW-failures=1`). ## Context — main's engine-default census Measured on `origin/main` just now: **66 failed / 9382 passed across 13 files**. This clears one. `workflow-lifecycle-live-e2e.pg.test.ts` is another and is already fixed in #2634 (pending merge), which takes it from 2 failures to 0 and adds two new scenarios. Of the remaining 11, none are in the review/merge lane: `agent-tools-intake-column`, `builtin-workflows-lifecycle`, `workflow-graph-optional-step-fix` and `workflow-settings-fallback-alignment` are column-vocabulary drift belonging to the U11/U12 owners (the optional-step-fix one expects `triage` where the replan rebound now resolves `todo` — same class as this fix, different owner's file); the rest are executor/CE/goal-anchoring. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:38:47 -07:00
gsxdsm	73338502e5	fix(test) + E2E: re-green main's lifecycle release leg, and prove the MERGED board + REVISE rework (#2634) ... **Second batch.** Three commits, no production code. `pnpm test:gate` green, `pnpm lint` clean, all three E2E suites together **3 files / 41 tests, exit 0**. ## 1. main's lifecycle E2E is RED right now — this fixes it Independently of my work, on a detached `origin/main`: **2 failed / 18 passed**. Scenarios 1 and 2 fail with `sweep.released` **empty**. **Cause:** `seedTask` relied on task creation's PROMPT.md, which is a bootstrap seed (`"# <id>\n\n<description>"`). FN-7648's `isUnplannedForExecution` reads that file for any card resting in an intake- **or** hold-trait column and refuses to move an unplanned card into a processing column. The sweep reported `held: [{ reason: "move-rejected-or-no-slot" }]`. **That is the gate working.** The fixture was asking the scheduler to release a card that had never been specified. The fix is the one the graph-entry contract doc already prescribes: *"Scheduler/release test fixtures must model a card that cleared the gate ... A held unreviewed card is the gate working."* `seedTask` now writes a planned PROMPT.md. **Verified it repairs main, not just this branch:** applying only that file to a detached `origin/main` leaves scenarios 1 and 2 **passing**, with the 4 residual failures being scenarios 3 and 6 — which need the fixture-options commit main does not have. ### I was wrong in #2627 and this corrects it In #2627 I named the in-transaction capacity gate (#2488/#2499) as the likely cause. **It was not.** Two hypotheses died, both recorded in the code comment so nobody re-runs them: | Hypothesis | Result | |---|---| | E2E settings lack `maxConcurrent` → capacity gate rejects the move | added `maxConcurrent`/`maxWorktrees` → **still 2 failed**. Not the cause. | | the move itself is refused | a direct `moveTask(id, wip)` → **succeeded**. Never the blocker. | Only then did probing the two release gates give `isTaskBlockedOnApproval=false`, `isUnplannedForExecution=true`, and dumping the file show the stub. I've flagged the wrong lead on #2627 too — a plausible-sounding cause pointed at another worker's PR is worse than no lead. ## 2. E2E evidence: the MERGED intake+hold board U11's shape — one column carrying intake **and** hold — had no end-to-end coverage; every prior E2E drove intake and hold as separate columns. - shared fixture gains opt-in `mergedIntakeAndHold`, plus `MERGED_VOCAB` (legacy ids, so a failure is attributable to the **role** merge alone) and `MERGED_RENAMED_VOCAB` (ids move too). - lifecycle scenario 3 drives the full spine: planning runs **in place** on the dual-role column, the real `runHoldReleaseSweep` releases **from** it, the graph runs to complete. - 4 merge-safeguard cases on the merged board (finalize, proofless refusal with the same reason, merged+renamed landing no legacy id, at-most-once). ## 3. E2E evidence: a REVISE routes back through rework The plan's `InReview → InProgress: review requests changes` had **no** live-engine evidence on any board — the fixture's review seam always succeeded. Two things the engine taught me, both corrected here: - the **IR validator refused** my rework edge: it is only legal into a node with `config.reworkRegion: true`. A real contract, and the validator catching it is the system working. `exec` now declares it (the shape the builtin uses on `merge-attempt`). - my first assertion was wrong. A REVISE does **not** leave the card in wip — rework re-enters `exec` within the same run, review approves on its second call, and the card finishes at complete. The evidence is the **seam sequence** `["planning","execute","review","execute","review","merge"]`, not an intermediate column the run has already passed. Asserting the final column alone would have been satisfied by a graph that ignored the REVISE entirely. ## Both families are mutation-attributed | Scenario | Mutation | Result | |---|---|---| | 3 — merged intake+hold | `isHeldTask` treats intake/hold as exclusive | **exactly its 2 tests** fail | | 6 — REVISE → rework | disable rework re-entry in `workflow-graph-executor` | **exactly its 2 tests** fail | Both fixture options are opt-in; the two pre-existing suites are behaviourally unchanged (27 → 29 → 41 passed across the additions, no existing assertion touched). ## Still not shipped: safeguard 2's graph E2E Attempted twice, deleted both times. Attempt 1 passed and then survived mutating `merge-gate` to ignore `task.autoMerge` — the card parked on the review column's `merge-blocker` trait, not the gate. Attempt 2 removed that trait to isolate the gate, and the **control** case parked too. Isolating it needs a merge path mirroring the builtin (`merge-gate → merge node → end`) rather than a direct edge to `end` — a real redesign, not a speculative edit. The enforcement that holds today is `allowInReviewMergeProcessing` in `project-engine` (unit-mutation verified, NEW=9; gated via #2526). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:23:32 -07:00
gsxdsm	88df46bedb	test: re-green executor-workspace onto FN-6756's contract (+ flag a dead branch) (#2617) ... **Test-only.** One file. No production changes. `executor-workspace.test.ts`: **2 failed / 10 passed → 13 passed**. (This commit was pushed earlier and I failed to open its PR — the work was finished and sitting on a dangling branch, which is why `executor-workspace` still shows in main's failure census.) ## Why it was red Both cases asserted that `clearPhantomExecutorBinding` **succeeds** while session-registry paths are held. PR #2531 (FN-6756, P0: *"stop reaping worktrees out from under live planners"*) inverted that. `hasLiveSessionSurface` now includes `activeSessionRegistry.pathsForTask(taskId).length > 0`, and that guard runs **before** both branches — so any registered path refuses the clear. The FNXC note at `executor.ts:2729` says the kind-blind guard is deliberate: *"A leaked entry now blocks THIS sweep rather than a live planner losing its worktree — the strictly safer failure."* So the old expectations describe the pre-#2531 contract. Rewritten to the current one, which had **no direct coverage**: a refusal leaves `activeWorktrees` and the registry entries untouched. The FN-6736 KTD2 invariant ("every held path, not one") is kept, exercised with no registry paths so the guard permits it. **Verified the new tests guard:** removing the registry term from `hasLiveSessionSurface` fails 2 of them (`NEW-failures=2`), and only them. ## Flagged, not fixed — a possible dead branch The guard appears to make **both branches it precedes** unreachable for their stated purpose: - the default branch exists to unregister every held registry path (FN-6736); - `preserveWorktrees: true` exists to **keep** those paths so a `moveTask(preserveWorktree: true)` re-dispatch reattaches to the same worktree (FN-7249) — and its **only** production caller is the self-healing reclaim at `self-healing.ts:3565`. Both need registered paths to do anything, and the guard rejects exactly that case. With none registered, one sweeps nothing and the other preserves nothing. The third new test pins this so the conflict is **executable rather than prose**: `preserveWorktrees: true` returns `false` while the path it exists to preserve is registered. I did not "fix" it by rewriting the assertion to match production — that would bury a possible regression in FN-7249's reattach path. Resolving it (exempting the non-destructive `preserveWorktrees` path, or narrowing the guard by kind) is a product decision for the FN-6756 owner. ## Main's engine-default census, measured just now | Point | Failed | Files | |---|---|---| | when I started this sweep | 283 | 28 | | after the logger-mock fix (#2573) | 106 | 23 | | **now** | **65** | **12** | This PR clears one of the remaining 12. `executor-review-verdicts.test.ts` is newly red and in my lane — taking that next. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 21:37:14 -07:00
gsxdsm	89d6d76d60	Unowned: the R7 sweep guessed with another workflow's columns — its "do not guess" guard was unreachable dead code (#2600) ... ## Unowned: the R7 sweep's "do not guess a column" guard could not fire Picked up from my own #2543 finding. Independent of my other PRs. ### The guard existed in comment form only `reconcileUndeclaredTaskColumns` wraps IR resolution in a try/catch whose comment reads: > An unresolvable workflow is its own fault path; do not guess a column. But `resolveWorkflowIrById` catches **every** failure and returns `defaultCodingWorkflowIr()`, and `resolveWorkflowIrForTask` does the same for a failed selection read. The resolver never rejects, so that catch is **dead code**. What actually happened to a card whose workflow could not be loaded: it was judged against the **default** workflow, and if its column was not one the default declares, the sweep re-homed it to the **default's** rebound target. It guessed, using a workflow that is not the card's own — the precise outcome the guard was written to prevent, in a **startup recovery path that runs against every task**. ### How it was found, which is the part worth keeping By being **unable to make a test of the guard fail**. Three separate mutations all passed — deleting the `continue`, deleting the try/catch, and simulating a whole-sweep abort at that very catch. I had written that off once as "this case pins the outcome, not the mechanism". The inability was the signal, not a limitation of the assertion: the branch is unreachable. This is the seventh instance of the program's core shape, and the first I found in a guard I had just finished writing coverage for. ### The fix The sweep now **proves the resolved IR belongs to the task** before moving its card: it reads the task's workflow selection and confirms that id resolves to a real definition (built-in or stored). - A task with **no** selection legitimately resolves to the default workflow — not treated as unresolvable. - An unreadable selection **read** is itself grounds not to guess. Placed at the **move site**, not at resolution, deliberately: it costs one definition read only for a card already about to be moved — a healthy board reaches that line for nobody — and it keeps the fix inside the sweep instead of changing a resolver whose soft-failure many other callers depend on. Changing `resolveWorkflowIrById` to reject would have been the tidier-looking fix and a much wider blast radius. ### Revert-proof, both directions - Remove the proof → the case fails `expected 2 to be 1`: the unloadable card is re-homed on a guess. - The same case asserts the neighbour **is** still repaired, so the fix cannot be mistaken for letting one bad card disable the sweep for everyone else. That is the per-task isolation property, and a single-task fixture cannot distinguish it from a whole-sweep abort — verified by injecting a throw at the loop head (`expected 0 to be 2`). ### Verification `pnpm test:gate` (482 + 10 + 71), `pnpm lint`, engine typecheck green. Sweep suite + `legacy-tombstones`: 13 passed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented startup recovery from moving cards into incorrect columns when their workflow cannot be loaded or resolved. * Cards with unreadable workflow information now remain in place, while other recoverable cards continue to be repaired correctly. * Added safeguards to avoid guessing a fallback workflow during column reconciliation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 21:21:04 -07:00
gsxdsm	534798dea0	test(U9): E2E evidence for the merge safeguards on a real PG store (completion-bar item 3) (#2615) ... **U9 E2E evidence.** One new `.pg.test.ts`, 6 tests, green. No production changes. `pnpm test:gate` green — `pgDescribe`-skipped without PostgreSQL, so the gate is unaffected. ## What this closes The U9 safeguard baseline verified all six merge safeguards by **mutation at unit level**. The sibling `workflow-merge-family-live-e2e` covers exactly **one** end-to-end. This drives `finalizeProvenAutoMergeTask` — the last move a card makes — against a real PostgreSQL `TaskStore`, asserting on the **persisted column** read back after clearing the task cache. Never on "a function was called". Only the merge **proof** is seeded (`mergeDetails.mergeConfirmed`), which is what a real merger writes; there's no git and none is needed. Column resolution, blocker evaluation, the move and its guards, and persistence are all real. Includes the rename differential, where a guard keyed on a literal goes silent. ## Three things I expected and measured wrong Corrected in the file rather than worked around — each is a claim I would otherwise have shipped: **1. Dependency gating does not reach this seam.** My first draft asserted a refusal. A proven-merged card with a live `blockedBy` finalizes to the complete column anyway. That's coherent: dependency gating lives in `getTaskCompletionBlocker` and gates whether work may be *called* complete, while this seam runs after `mergeConfirmed` — refusing would strand a merged card in review and misreport the repository without un-merging anything. Now pinned as designed behavior *with* that reasoning, not filed as a hole. **2. The at-most-once outcome is `already-done`**, not the `already-complete` I guessed. **3. `expect(outcome).toBe("blocked")` cannot attribute a refusal.** The finalizer has **three layered refusal gates**, and the two proof gates emit the *same* reason (`missing-merge-confirmation`, also returned by `validateWorkflowDoneMergeProof`). So removing either one left my original assertion **green**: | Mutation | Result | |---|---| | remove the durable-proof gate | 6 passed — invisible | | remove the main-path proof gate | 6 passed — invisible | | remove **both** | **2 failed** / 4 passed | Fixed by pinning the **reason**, not just the refusal. The lesson generalises: single-gate mutation cannot detect redundant defense-in-depth from outside, so the unit-level attribution in the baseline doc and this E2E are **complementary**, not duplicative. I nearly labelled these tests as proving a specific gate they don't. ## Flagged, not changed — safeguard 1 at this seam Written as open questions and answered by running them. **Both a `paused` and a `userPaused` proven-merged card are moved to the complete column.** For `paused` that's documented design — `auto-merge-finalization.ts:243` evaluates hard blockers with `paused: false` because the branch already landed. For `userPaused` it sits against the invariant re-ratified in #2486: *never MUTATE lifecycle state of a user-paused card.* The mitigating argument is the same one — the merge is durable, so the move is bookkeeping that reflects reality, and refusing would leave an operator's card permanently misfiled in review. **Either reading may be right. What was not acceptable is that it was untested.** Both are now explicit named assertions with the tension in the comment, so tightening the pause contract becomes a decision rather than a discovery. Resolution belongs to whoever owns the pause contract — I'm not quietly changing merge behavior on a paused card. ## Safeguard coverage after this PR | # | Safeguard | Unit (mutation) | E2E | |---|---|---|---| | 1 | user pause | ✅ | ✅ pinned as an exception at this seam — flagged above | | 2 | autoMerge:false | ✅ | ✗ gate lives upstream in `project-engine`, not this seam | | 3 | dependency gating | ✅ | ✅ pinned as *not* applying here, with rationale | | 4 | capacity single-flight | ✅ | ✗ in-memory pump, no store seam to observe | | 5 | merge-proof | ✅ | ✅ both vocabularies, reason-attributed | | 6 | at-most-once | ✅ | ✅ second finalize classifies `already-done`, no second move | The two gaps are stated rather than implied: safeguard 2's gate is `allowInReviewMergeProcessing` in `project-engine`, which needs an engine harness rather than a store one, and safeguard 4 is an in-memory single-flight latch with nothing persisted to assert on. Both are covered by mutation at unit level and both are in the gate as of #2526/#2569. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 21:20:17 -07:00
gsxdsm	592fd5c0c6	U11 [mission-feature-sync + spec-staleness]: convert the last two planner-lane guards (48 -> 46) (#2610) ... **Taking: `engine/mission-feature-sync.ts`, `engine/spec-staleness.ts`** — the last two planner-lane guards in my area. ## Census (comment-stripped, `=== "triage"` / `!== "triage"` in `packages/*/src`, tests excluded) | file | before | after | |---|---:|---:| | `packages/engine/src/mission-feature-sync.ts` | 1 | **0** | | `packages/engine/src/spec-staleness.ts` | 1 | **0** | | **repo total** | **48** | **46** | ## Both are real conversions, not seams Each guard takes its vocabulary from the **caller**, which holds the store — so unlike a defaulted parameter nothing passes, these can actually be driven. **`reconcileMissionFeatureState`** — a card back in a planner lane returns the mission feature to `triaged`. Keyed on literals, a renamed workflow left the feature reading `in-progress` forever: the roadmap claims work is underway while the card waits to be re-planned. Nothing errors; the rollup is just wrong. The vocabulary arrives via `MissionFeatureSyncContext` rather than by widening this module's deliberately narrowed `Pick<TaskStore, "getTask">`. **`shouldSkipSpecStalenessForPreservedProgress`** — returning `false` for a planner-lane card is what *keeps* staleness evaluation on. Miss the lane and it falls through to the preserved-progress branch, so a card with progress skips staleness and keeps a spec that should have been re-validated. ## The two take different defaults — and I got it wrong first I defaulted **both** to the `triage`/`todo` pair and broke the pre-existing U11 proof in `spec-staleness.test.ts`, which states the reason exactly: > same column, different status, opposite correct answer - **mission-feature-sync → the PAIR.** It asks "is this card waiting to be planned?", true in either lane. - **spec-staleness → the DEDICATED planner column only.** On a merged lineage `todo` is *also* the hold lane, so the planner distinction there is carried by **status** (`planning` / `needs-replan`), not by the column. Treating the merged column as a planner lane stops a parked card with preserved progress from skipping staleness. Its default is now the single legacy id — byte-identical to the literal it replaced. That asymmetry is now pinned by its own test rather than left for the next reader to rediscover. ## Findings on the remaining census, from measuring it Two of the 46 are **not lifecycle-column guards** and converting them would be wrong: - `tool-availability.ts:32` — `surface === "triage"` where `surface: "triage" | "executor"` is an **agent lane**, not a column. - `skill-resolver.ts:432` — `sessionPurpose === "triage"`, a **session purpose**. Also worth noting for the count: `replan-target.ts` reads as 2 in a raw grep but is **0** — both hits are inside comments. `board-workflows.ts` (2) and `archive-planning.ts` (1) are likewise comment-only. A raw grep says 52; comment-stripped says 46. ## Not wired at the call sites yet `scheduler.ts` / `mission-autopilot.ts` (mission sync) and `executor.ts` / `scheduler.ts` (staleness) still omit the new option, so behaviour is byte-identical today. Deliberate: `executor.ts` belongs to u8's active slice and I would rather not create a textual collision for a pass-through. The seam is proven by tests and the count is real; wiring is a follow-up. ## Verification - **Mutation-verified:** restoring either literal fails a test - 35 tests green across the three suites, merge gate green (482 + 132 + 10), tsc clean, lint clean No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-29 21:20:09 -07:00
gsxdsm	45e8b5f7ac	U8: pin the completion-finalize ordering invariant before moving the last out-of-band exit (#2599) ... Groundwork for moving `paused-after-completion`, the **last** out-of-band exit. Stacked on #2590. ## What lands 1. **An indentation defect I introduced.** My bulk edit when the exit vocabulary landed left the second `paused-after-completion` site mis-indented inside a `finally` block. Cosmetic, but misleading indentation in a `finally` is how a future reader misjudges scope. 2. **The adjacency ratchet now requires `markCompletionFinalized` before the handoff, at every reporting site.** It previously checked only the first occurrence, and only for the handoff itself. That ordering is the invariant `handleGraphFailure` depends on and **cannot check for itself**: `alreadyFinalizedToReview` / `completionFinalized` exist to recognise this out-of-band move when a later teardown re-marks the abort as `hard-cancel`. Without the durable marker set first, a completed no-commit task is re-parked `failed` — FN-6644/FN-6641. It is asserted **structurally, and labelled as such in the test**. Both call sites sit in pause and `finally` paths that cannot be driven without mocking an entire agent session; presenting a source assertion as behavioural coverage would repeat the overclaim I have been correctly pulled up on twice in this unit. Red-green: removing `markCompletionFinalized` from either site fails the ratchet. ## Why the move itself is not in this PR `paused-after-completion` is structurally harder than the pending-review ending that #2590 moved, and the difference is worth recording before someone assumes it is a copy-paste: - it does **four** things, not one — `markCompletionFinalized`, `handoffTaskToReview`, `clearCompletedTaskWatchdog`/`signalTaskComplete`. Only the handoff is lifecycle; the rest is substrate that must stay put. - one of the two sites is inside a **`finally`**. Moving a transition out of a `finally` is not the same operation as moving one out of a branch: the graph may already be unwinding, so "report and let the graph route" needs a defined answer for a run that is already ending. - there is **no behavioural coverage of either site today** — the closest tests only exercise the exit vocabulary. The pending-review move succeeded on the fourth attempt precisely because FN-5436 existed to catch each wrong version; this exit has no equivalent, so the move needs that floor built first, and building it means real session mocking rather than a shortcut. ## Verification - exit-events + primitive-exit-events + step-session + ownership ledger — green - `pnpm lint` clean; `tsc --noEmit` clean - No user-facing behaviour change, so no changeset 🤖 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 handling of workflow steps that pause for review. - Tasks now remain in review when a review request has no subsequent decision. - Added clearer completion events for primitive prompt steps. - Preserved correct failure handling when later workflow steps fail. - **Workflow Improvements** - Built-in workflows now route pending reviews through a dedicated review handoff. - User-authored workflows retain compatible review parking behavior when routing is unavailable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 21:04:39 -07:00
gsxdsm	3f763cba87	U8: the graph owns the pending-review park — ownership ledger 28 → 27 (#2590) ... The routing move this unit has been building toward, landing on the path the engine actually runs. **Includes #2578's commit** (the live-path fix it depends on) — merge that first, or this supersedes it. ## What changes Three things together, because a half-routed move is a card that silently does not advance: 1. The **live** implementation primitive (`runCodingSession`) returns `{outcome: "failure", value: "review-pending"}` for that ending. 2. The primitive step handler stops flattening every ending to `step-done`/`step-failed`, so the value survives the foreach — `runForeach` propagates a failing instance's value as the node's own — and reaches an edge. 3. The inline `handoffTaskToReview` in `runImplementation` is **deleted**. The phase reports and stops, which is all an implementation phase should do. Built-in workflows route to the `review-pending-handoff` node added in #2519/#2546, which performs the handoff and ends the run: the same two effects in the same order, with the graph as the owner. ## Proof, end to end FN-5436 — the test that blocked this move twice and was right both times — now passes, with a **stronger** assertion than it had: ```ts expect(store.moveTask).toHaveBeenCalledWith("FN-5436-B", "in-review", expect.objectContaining({ workflowMoveSource: "workflow-graph", workflowMoveMetadata: expect.objectContaining({ nodeId: "review-pending-handoff" }), })); ``` The old two-argument `moveTask(id, "in-review")` could not distinguish a graph-owned park from an out-of-band one — which is the entire distinction this unit exists to make. The invariant (park in review, never `failed`) is unchanged; the owner is now proven. ## Every ratchet fired, and each records a real change | Ratchet | Before | After | Why | |---|---|---|---| | Ownership ledger — `runImplementation` review handoffs | 3 | **2** | the handoff left the phase | | Ownership ledger — `handleGraphFailure` | 0 | **1** | the named compat classifier | | Ledger headline — executor-owned dispositions | 28 | **27** | first decrement of the unit | | Out-of-band exit list | 2 | **1** | pending-review is graph-owned now | | Primitive routing pin | "must not reroute" | routes *only* the moved ending | declared, not discovered | None was relaxed. The `handleGraphFailure` 0 → 1 is the honest one: for a user-authored graph without the edge this is a **relocation, not an elimination** — the transition is still executor-performed, but from one named classifier in the failure ladder rather than a call buried two thousand lines into a session loop. The ledger says so rather than letting the headline number imply more progress than there is. ## Why it took four attempts Recorded because the reason is reusable: the value was being produced on `createAuthoritativeWorkflowSeams`, a handler that never runs (#2578). Every earlier attempt was correct code on a dead path, and the only thing that showed it was instrumenting until a negative result was proven observable rather than assumed. ## Verification - step-session + exit-events + primitive-exit-events + ownership ledger + graph-requeue-gate + task-done-blocked — **83 tests green** - `pnpm test:gate` green (10 / 482 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `internal`) 🤖 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 handling of tasks awaiting review so they are correctly routed to the review workflow. * Tasks now remain in review instead of being marked as failed when no follow-up review route is configured. * Review handoffs now include workflow ownership and provenance details. * Preserved standard failure handling for tasks that are not awaiting review. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:59:06 -07:00
gsxdsm	d5f1ce7abd	U11 [writes]: stop CREATING cards into a column the workflow no longer declares (9 -> 0, engine+cli) (#2603) ... **Taking: `engine/triage.ts`, `engine/pr-comment-handler.ts`, `engine/eval-followups.ts`, `cli/commands/task.ts`, `cli/extension.ts`** (write class — no collision with the comparison backlog). ## A class the census does not count The 48-guard work list tracks `=== "triage"` **comparisons**. These are `column: "triage"` **writes** — and post-#2515 every one creates a card directly into the state STALL 3 was about, except **manufactured continuously** rather than left behind by the upgrade. ## Why they bite `createTaskImpl` resolves the column as: ```ts column: input.column || options?.resolvedEntryColumn || fallbackIntakeColumn || "triage" ``` `input.column` **wins**, so an explicit `column: "triage"` overrides the workflow's resolved intake column entirely. `store-create-intake-column.test.ts` already pins that a create with **no** column lands in the default workflow's intake (now `todo`) — these callers opted out of it. The sharpest is `triage.ts`'s `fn_task_create` agent tool: it passed `workflowId: params.workflow_id` **and** `column: "triage"` in the same call. The caller chose a workflow and the column ignored it — a Coding (Ideas) create landed in `triage` instead of `ideas`. ## Counts **Comparison guards: unchanged by this PR.** This is the write class; conflating the two would misreport convergence toward the zero bar. | file | `column: "triage"` writes before | after | |---|---:|---:| | `packages/engine/src/triage.ts` | 1 | **0** | | `packages/engine/src/pr-comment-handler.ts` | 1 | **0** | | `packages/engine/src/eval-followups.ts` | 1 | **0** | | `packages/cli/src/commands/task.ts` | 3 | **0** | | `packages/cli/src/extension.ts` | 3 | **0** | | **total** | **9** | **0** | ## A test that pinned the defect `pr-comment-handler.test.ts` asserted `column: "triage"` in the createTask call — so it would have **failed the fix and passed the bug**. Rewritten to assert the invariant (the caller passes no column, so the workflow's intake wins) plus an explicit `Object.hasOwn(arg, "column") === false`, which is what actually catches a reintroduction. ## Interaction with #2591 My merged #2591 rescues these cards once created — they sit on a legacy planner id their workflow doesn't declare and are still in planning stage. So this isn't a *visible* stall today; the rescue absorbs it. **That's the reason to fix it rather than leave it:** a self-healing path silently absorbing a steady stream of malformed creates is exactly how the underlying defect stays invisible. ## Deliberately not touched - `{ id: "start", kind: "start", column: "triage" }` in the builtin coding / PR / lead-generation IRs — workflow-internal **node declarations** for workflows that still legitimately declare a `triage` column, not lifecycle writes. - Left for their owners: `core/task-store/project-store-ops.ts:210`, `core/task-store/update-task-deps.ts:111` (main worker), `dashboard/src/routes/register-gitlab.ts:108` (u12). Same defect, same one-line shape. ## Verification - 304 engine/CLI tests green across the affected suites - merge gate green (482 + 132 + 10), engine + CLI tsc clean, lint clean No changeset: `@fusion/engine` and `@fusion/core` are private; the CLI change is a bug fix with no user-facing API change — happy to add one if you'd rather it appear in release notes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-29 20:58:52 -07:00
gsxdsm	9c1c6f7479	docs(engine): close the unproven-sites ledger — one entry was wrong, the rest need two named lanes (#2544) ... Comment-only change to the ledger. No test or production code moves. ## Why this is a PR and not a note The ledger is the artifact that keeps *"the E2E covers the conversion"* honest. It gets the same treatment as the code: claims verified by mutation, not by reading. ## Correction: one entry was wrong `core/task-store/reads.ts` was listed as **unproven**. It isn't. Core's `store-stale-paused-renamed-hold.pg.test.ts` is a real-store test that drives `listTasks` against a renamed hold column — and forcing the hydration back to the `todo` literal **fails exactly that file's renamed case**. I had listed it as unproven because I assumed a separate E2E was needed. Verified *before* removing it, since "already covered somewhere else" is precisely the assumption that lets a gap hide. ## What remains, and why it is not another table row **Lane 1 — real git.** `merger.ts`'s `resolveMergerLifecycleColumn` and `executor.ts`'s `resolveReboundColumnFor` are module-private helpers whose only callers sit inside merge/session machinery needing a real worktree, branch and squash; `merger-ai.ts` is the same. Re-checked with the lens that freed `auto-merge-finalization` and both self-healing rebounds — **these genuinely need the lane.** The earlier over-broad claim doesn't retroactively excuse them. **Lane 2 — dashboard HTTP.** The four `register-task-workflow-routes` sites sit behind `registerTaskWorkflowRoutes(ctx, deps)`, needing a full `ApiRoutesContext` plus twelve injected deps. Standing that up is the mock-the-world shell FN-5048 says not to add. The narrower alternative — exporting the two private resolvers — yields **unit** evidence while looking like E2E. Deliberately not done rather than done badly and overclaimed. `live-agent-count`'s `columnIsIntakeOrHold` is the same lane: only the *waiting* predicate reads it, and its consumers are dashboard-side. ## Running total **10 of 15 census sites proven end to end** across six suites; 5 remain, each named with the lane it needs. ## Verification - lifecycle suite 20/20; engine `tsc --noEmit` clean; `pnpm test:gate` green (414 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated end-to-end test coverage documentation to accurately reflect verified workflow and task-store behavior. * Clarified coverage gaps for live agent-count logic and dashboard workflow routes. * Added a two-lane breakdown describing remaining coverage work. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:56:04 -07:00
gsxdsm	c9df4b9dee	U11 migration proof: the path an operator actually hits, with all three caveats answered (#2597) ... Tests only. Proves the upgrade path the existing E2E does not cover, and answers the three caveats. ## Why the existing coverage was not enough The existing cases strand a card in a synthetic `a-column-no-workflow-declares` on a **fixture** vocabulary. The real upgrade leaves cards in **`triage`**, on the **real `builtin:coding`** workflow. That difference is the whole point: `triage` is still a legal `ColumnId` and is still declared by legacy-coding, Ideas and every linear built-in (R11), so nothing rejects it and **nothing throws**. The card simply sits in a column its *own* workflow no longer declares — where it carries no trait flags and is invisible to every trait-driven sweep. ## What is proven, on a temp PostgreSQL project - A card left in the deleted `triage` column on a default-workflow board is re-homed to `todo`, the merged Planning column. - **Revert check in-suite:** without the sweep running, the card stays in `triage`. Without this, the case above could pass because some *other* sweep or a store-open reconcile moved the card — and would keep passing if the sweep were deleted outright. - **Progress and the plan artifact survive.** `preserveProgress: true` is asserted end-to-end rather than trusted from the option name. - A `userPaused` card is skipped and stays in the deleted column. **Mutation-verified:** stubbing `reconcileUndeclaredTaskColumns` to `return 0` turns **5 of 11** tests red, including all three positive migration cases. The sweep is demonstrably the mover. ## The three caveats — answered **1. `userPaused` cards are skipped → caveat, not a stall.** An operator park is authoritative and the sweep must not override it, so the card does stay in a column its workflow no longer declares. But it is reachable two ways: unpausing makes the next sweep re-home it, and **U11's undeclared-source escape hatch in `resolveAllowedColumns` (merged with #2515) lets an operator move it by hand meanwhile** — that path returns the workflow's rebound target instead of `Valid targets: none`. Recorded as a test so the behaviour is a decision rather than an accident. My recommendation: **leave it skipped.** Re-homing a paused card silently moves work an operator deliberately froze, and the escape hatch already gives them a way out. Overriding a park to fix a column is the wrong trade. **2. Sweep only runs when self-healing is enabled → caveat, not a stall, for the same reason.** The escape hatch lives in the **move-validation** path, not in self-healing, so it works with self-healing off entirely. A card stranded that way is draggable out of the deleted column by hand. Worth knowing: before #2515's escape hatch this *would* have been a hard stall — `resolveAllowedColumns` returned `[]` for an undeclared source, so the card could not be moved anywhere at all, by anyone. **3. Re-home targets the HOLD column → correct, and progress survives.** Under U11 the hold column **is** the Planning column, so "everything lands in Planning" is the intended destination rather than a compromise. Asserted with real step progress on the row. **None of the three is worse than a caveat.** The reason all three are survivable is the same single mechanism — the undeclared-source escape hatch — which is worth knowing because removing it would silently promote all three to hard stalls. ## Incidental Fixed two fixture-level PostgreSQL column-name errors found while writing this: `currentStep` → `current_step`, and `user_paused` is an **integer** flag rather than a boolean. Both would have made a future test here fail confusingly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:31:15 -07:00
gsxdsm	131feb243c	U8: the exit announcement was on a dead code path — move it to the handler the engine actually runs (#2578) ... A merged behavior of mine has never executed. This fixes it and adds the ratchet that would have caught it. ## The finding `createDefaultNodeHandlers` chooses the prompt-node handler like this: ```ts const promptLike = deps?.primitives ? createPrimitivePromptLikeHandler(deps.primitives, runCustomNode) : createPromptLikeHandler(seams, runCustomNode); ``` `executeWorkflowGraph` always passes `primitives: this.createAuthoritativeWorkflowPrimitives(settings)` (`executor.ts:6051`). **So `createPromptLikeHandler` — and with it every `execute` / `step-execute` function in `createAuthoritativeWorkflowSeams` — is unreachable for prompt nodes.** Both objects are passed to the graph executor and only one is consulted. The `NodeCompleted.exit` announcement added in #2507 was wired into that seam. It type-checks, its tests pass (they call the seam object directly), and it has never run in production. `runCodingSession` in the primitives is the live twin, and that is where it emits now. ## How it was found — and why the negative is trustworthy Instrumenting `createAuthoritativeWorkflowSeams.stepExecute` produced no output for a run that demonstrably visits `steps#0:step-execute`. So did instrumenting `createPromptLikeHandler`'s dispatch. A negative result from instrumentation is worthless until the instrumentation is shown to be observable, so: a `process.stderr.write` at module load of the same file **did** appear, exactly once, in the same run. The two negatives were real, not swallowed output. This is also the answer to the open question I left in #2546 — the pending-review routing move kept failing because the seam value it depends on is never produced. **That move is still not landed here.** This commit only relocates the announcement, so it stays small and separately revertable; the routing move follows once its value originates on the live path. ## The ratchet A source assertion pins the dispatch rule: `deps?.primitives ? createPrimitivePromptLikeHandler` and the executor's wiring of `primitives`. Inverting or conditionalising that preference would silently disable every behavior attached to the primitives path — the same failure in the other direction — and **a seam-level unit test cannot tell the two apart**, which is precisely how this survived review twice. ## Red-green Removing the emit fails 2 of the 4 new tests (`Tests 2 failed | 2 passed (4)`). The other two are the regression floor: an ordinary completion emits `success` with no `exit`, and the returned routing outcome is unchanged — announcing must not reroute. ## Scope note I did **not** delete the now-known-dead seam wiring in this PR. `createAuthoritativeWorkflowSeams` is still passed to the graph executor and its non-prompt entries (`stepReview`, `merge`) are reached through other handlers, so deciding what is genuinely dead there is a deletion audit of its own — and this program's rule is that deletions never ride along with behavior changes. Filed as the next slice. ## Verification - 4 new tests + exit-events + step-session + triage audit + ownership ledger — **54 tests green** - `pnpm test:gate` green (10 / 414 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `fix`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:30:54 -07:00
gsxdsm	2aa68867e5	U11 follow-up: usage-limit parking silently stopped covering the planning lane (#2567) ... Second of the 39 audited `triage` sites from #2515's safety audit. Unlike the first, this one is **real breakage**, not a proof of safety. ## The defect The usage-limit pauser decides which tasks are on a rate-limited provider by asking, per lane, whether the card sits in that lane's column. The **planning** lane asked for the literal `triage`. Now that Todo is merged into Planning, a card being planned on the default workflow rests in `todo`. The branch resolves to an empty provider list, so the card is not recognised as using the planning provider — and is **neither parked when that provider hits its limit nor resumed when it recovers**. It runs into the limit and fails. Silent by construction: the detector reports nothing, it simply matches no tasks. ## The test caught my own first attempt at testing it The initial version asserted on the task that **triggered** the usage-limit hit — and **passed against unfixed code**, because the trigger is always parked directly without consulting `taskUsesProvider`. Only a **bystander** card reaches the lane/column branch. All three assertions now use a separate trigger, and the comment says why, because the obvious test shape is the one that proves nothing. ## Why a paired literal rather than trait resolution `taskUsesProvider` is a synchronous predicate over a task and settings, with no IR in scope and no call site that could supply one without a signature change reaching several callers. Both ids name a pre-implementation column in every built-in — `triage` for the split shape, `todo` for the merged one and for Coding (Ideas) — so the pair covers the planning lane in all of them. Flagged for U12's ratchet allowlist with that reason attached. **Over-inclusion is the safe direction and is deliberate.** On a split workflow a `todo` card is capacity-parked rather than actively planning, so it may now be parked during an outage it was not using. Parking one extra idle card is recoverable; failing to park a card whose provider is rate-limited is not. Regression direction asserted: widening the **column** match must not widen the **provider** match — a planning card on a different provider is still not parked. ## Audit progress 39 exclusive `triage` sites (from `docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md`, merged in #2515): | status | sites | |---|---| | proven safe as-is | `spec-staleness.ts` — the guard is carried by **status**, not column; the mechanical conversion was tried and is *wrong* | | confirmed safe by inspection | `mission-feature-sync.ts` (already OR-pairs), `TaskContextMenu.tsx` (already trait-paired) | | **fixed here** | `usage-limit-detector.ts` | | remaining | 35, with the owners named in the audit | Gate 309/309, lint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:30:47 -07:00
gsxdsm	b0b9614fd5	U12 part 10: pin the R7 undeclared-column sweep — the repair three earlier PRs cited had no test of its own (#2543) ... ## U12 part 10 — the R7 sweep everything else leans on was itself unpinned `reconcileUndeclaredTaskColumns` re-homes a card resting in a column its workflow no longer declares. It is the shipped answer to **R7**, and it is the reason several earlier U12 deletions were safe — I cited it when deleting the superseded `runWorkflowColumnsIntegrityPass` (#2500), and again when arguing that a torn workflow switch leaves *recoverable* state (#2512). Its only coverage was **incidental**: two live PostgreSQL e2e suites that exercise it in passing. A repair the rest of the unit leans on had no test of its own — a guarantee everyone cites and nobody checks, which is the exact shape this unit keeps finding. ### Six cases The plan names three scenarios for U12; those are the three ways this sweep can be wrong, plus I added the over-fire direction: - repairs the stranded card to its workflow's **own** rebound target (not a hardcoded legacy id) - leaves a **user-paused** card alone - leaves an **unresolvable-workflow** card alone - is **idempotent** — a second run does not move the card again - ignores a card already resting in a declared column - repairs one stranded card **without disturbing** healthy or paused neighbours The leave-alone cases matter more than the repair. A sweep that over-fires rewrites an operator's board, and this one runs at startup against every task. It also asserts `recoveryRehome: true` explicitly, because that flag is load-bearing rather than incidental: the stranded card's *source* column is undeclared too, so adjacency resolves to `[]` and every target is rejected without it. Its absence once made this sweep a repair that never repaired anything (#2462). ### Mechanism coverage — measured, and one case that isn't Verified by mutation rather than asserted: | mutation | result | |---|---| | delete the user-pause guard | **2 cases fail** | | delete the already-declared short-circuit | **2 cases fail** | | delete the unresolvable-workflow `continue` | still green | That last row is stated at the assertion rather than hidden. The unresolvable-workflow case pins the **outcome**, not the mechanism: every mutation I could construct — dropping the `continue`, dropping the try/catch so the throw reaches the outer handler — also ends in "no move". So it is a regression guard on observable behaviour, not proof the specific guard is reached, and I am not claiming otherwise. ### A decision I made Store double rather than PostgreSQL. The sweep's decisions are pure functions of the task list and the resolved IR, and a double makes the "did **not** move" assertions exact rather than inferred from an absence of change. It also keeps the suite off the slow lane, per the standing rule against adding slow tests. ### Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, engine typecheck green. New suite: 6 passed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added coverage for automatically restoring tasks stranded in undeclared workflow columns. * Verified paused tasks, unresolved workflows, and tasks already in valid columns remain unchanged. * Confirmed repairs are idempotent and affect only the intended task. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:30:27 -07:00
gsxdsm	88c7502eae	test(engine): prove the recovered-lease rebound AND its audit on a renamed board (#2539) ... Test-only. Sixth E2E family. Closes the `mesh-lease-manager` ledger entry. ## Two things to prove, and only one is where the card lands The conversion note records the defect precisely: > They were previously two independent `=== "todo"` comparisons that could disagree, which is how **the audit came to claim a card landed in `todo` when the workflow has no such column**. 1. the card rebounds to the renamed workflow's own rebound column 2. the unreachable-owner **audit** reports the column the card actually reached **(2) is the half that rotted silently, and it is the worse one.** The audit is what an operator reads to find out where a recovered card went. Confidently wrong is worse than absent — and on a renamed board it named a column the workflow does not even declare. ## One thing deliberately NOT renamed `decisionPath` keeps its legacy `lease-recovered-to-todo` wording. The code explains why: it is a stable discriminator that existing queries and dashboards match on, and renaming it would break them in order to describe the same decision. The column actually used travels in `newColumn`. I've pinned that split with an explicit assertion so a future vocabulary "cleanup" cannot quietly rename a field that **is not a column at all**. Stating it here so it reads as a decision rather than an oversight. ## Mutation-verified Forcing the legacy literal fails **exactly the three renamed cases**, leaving the default-vocabulary floor and the fresh-lease negative green. ## Negative half A lease renewed just now is not recoverable — "rebound anything with a checkout" would tear live work off its owner. ## Fixture guard The stale-lease seed writes lease bookkeeping through the admin client and then **asserts the seed took effect**. A silently-dropped write would make the recovery look correctly declined — the same trap that produced a vacuous paused-park test earlier in this program, so it is now guarded by default. ## Verification - six live-E2E suites green together: **58/58** - engine `tsc --noEmit` clean - `pnpm test:gate` green (414 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 20:30:19 -07:00
gsxdsm	a68785a41d	P0: two silent triage guards in the executor's ownership — one strands a card with nothing to rescue it (#2572) ... P0 audit of the executor's assigned `triage` sites after the Planning-column merge. **One of them can strand a card**, so leading with that. ## The stall — `handleDepAbortCleanup` `executor.ts` moved a dependency-aborted task to the **literal** `triage`. The default coding lineage no longer declares that column. A card that gains a dependency mid-execution has its work discarded and is then parked in a column its own workflow does not define. Nothing in the graph routes a card out of an undeclared column. The only rescue is `reconcileUndeclaredTaskColumns`, which runs on the **next engine start** — so between the abort and a restart the card is stalled with no automatic recovery. It does not throw, so it would have surfaced as a user report, not a red test. Fixed to `resolveReboundColumnFor`, the helper the other ~16 executor rebounds already use. ## The silent skip — `UsageLimitPauser.taskUsesProvider` The planning lane was identified by the same literal. For a default card the lane resolved to **no providers**, so when a provider hit a usage limit during a *planning* session, the fan-out that pauses peers on that provider skipped every default-workflow card and they kept hammering the rate-limited provider. Not a stall: the triggering task is still paused by the explicit fallback below the filter. What was lost is blast-radius containment. A planning session runs while the card is pre-implementation, and the caller has already excluded `done`/`archived`, so that is exactly "not the implementation column and not the review column" — which matches `todo`, `triage`, `ideas`, and a renamed planner alike. ## Full audit table for my assigned sites | Site | (a) Still fires for a default card? | (b) What silently stops | (c) Action | |---|---|---|---| | `executor.ts:16395` `moveTask(id, "triage")` | **No** — writes an undeclared column | Card parked where nothing routes it; rescue only at next engine start | **Fixed** — `resolveReboundColumnFor` | | `usage-limit-detector.ts:126` `column === "triage"` | **No** | Usage-limit fan-out skips every default card; peers keep hitting the limited provider | **Fixed** — pre-implementation predicate | | `executor.ts:3409` `from === "todo" \|\| from === "triage"` | **Yes**, via the `todo` arm | — | Unchanged; `triage` arm still live for legacy-coding | | `executor.ts:4951` `originColumn === "todo" \|\| === "triage"` | **Yes**, via the `todo` arm | — | Unchanged | | `executor.ts:4963` `originColumn === "triage"` double-hop | No, and correctly so | Nothing — the extra hop exists only for shapes that declare `triage` | Unchanged; still required by legacy-coding | | `executor.ts:1110` `Type.Literal("triage")` | n/a | — | **Not a column** — an agent ROLE in `spawnAgentParams` | Counts for my ownership: **6 sites audited, 2 defects, 2 fixed, 3 correct as-is, 1 false positive.** ## Red-green Reverting each fix fails its own test: ``` Tests 2 failed | 2 passed (4) × dependency-abort cleanup requeues to a DECLARED column × usage-limit fan-out … pauses a peer card sitting in the merged Planning column (id `todo`) ``` The other two are the regression floor and pass both ways by design: a legacy workflow that **does** declare `triage` still fans out, and an in-progress card is still **not** swept into the planning lane (the guard must stay narrow — "any non-wip column" would have been the easy wrong fix). ## Verification - New audit suite + graph-boundary + step-session + ownership ledger — **45 tests green** - `pnpm test:gate` green (10 / 414 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `fix`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 19:03:10 -07:00
gsxdsm	beb33b5dd1	P0 STALL 3: rescue cards stranded in a column their workflow no longer declares (fixes 8 red tests on main) (#2591) ... Based on `main`. **Fixes STALL 3 — and it needs no data migration.** ## The stall #2515 removed `triage` from the default lineage while leaving the id legal for stored rows, and shipped **no migration**. Planning discovery resolves a card's lanes from its own workflow, and for a default card `intake` and `hold` **both** resolve to `todo` — so a card *sitting* in `triage` matched neither branch and was admitted by nothing. `triage` was the default intake column before #2515, so **every existing project has cards there.** Nothing else rescued them. #2515's escape hatch makes an undeclared source column resolve to the workflow's rebound target, but every path that *uses* it (executor, agent-heartbeat, merger) is triggered by **active work**, and a parked card has none. The card sat until an operator dragged it by hand. ## Proof this is a real regression, not a stale test **8 tests in `triage.test.ts` were RED on clean `origin/main`** — verified by swapping main's `triage.ts` into this tree and re-running. **All 8 pass with this change.** The sharpest: ``` expected "specifyTask" to be called 4 times, but got 0 times ``` Discovery was admitting zero triage cards. ## The fix A card resting on a legacy pre-implementation id that its own workflow no longer declares is **unowned by construction** — no lane's rules apply to it. Admitting it to **planning** heals it through the normal path: it gets planned, and finalize releases it to the workflow's hold column, **re-homing the row as a side effect of ordinary work**. No migration, no backfill, no operator action. ## The narrowing is the load-bearing part My first version rescued **any** undeclared column, and it was wrong. A card can also sit in a column its workflow genuinely owns while the **selection** fails to resolve — the resolved default IR then doesn't declare that column either. That version re-specified a parked Coding (Ideas) `ideas` card, breaking **FN-7596's manual-intake rule** (an ideas card is promoted by an *operator*, never auto-planned). `triage.test.ts` caught it. The rescue is now scoped to the legacy planner ids, so a workflow-specific column name is never second-guessed. That distinction — healing #2515's orphans vs. overruling a workflow about its own board — is the whole design. ## A user-pause hole this would have opened `couldBeCandidate` screens `paused` but not `userPaused`, so a row carrying `userPaused` alone slipped through. Harmless before (an undeclared-column card was admitted by nothing) and **reachable the moment admission widens**. Planning a card mutates its lifecycle state, which the ratified safeguard forbids for a user-paused card — so the guard is now explicit rather than inherited. Covered by a test and mutation-verified. ## Cost Resolution now derives roles **and** declared column ids from one `resolveWorkflowIrForTask` call, replacing `resolveTaskLifecycleColumns`. Same call, same `irCache`, same bounded concurrency window — **cost unchanged**, no added read. ## Verification - **Mutation-verified three ways**, each failing a different test: remove the rescue; widen it back to any undeclared column; drop the user-pause guard - 8 previously-red-on-main tests now green - 263 triage/scheduler tests green, merge gate green (482 + 10 + 71), tsc clean, lint clean ## What this does NOT do It does not re-home rows that are past the planning stage. Admission still requires `isTaskStillInPlanningStage`, so a card that advanced past planning in an undeclared column stays with self-healing's advanced-recovery sweep rather than being re-specified here. If such rows exist and are also stranded, that is a separate sweep and a separate PR. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-29 19:02:55 -07:00
gsxdsm	cf7b1a3d46	Drift review (unowned): gridlock detection + autopilot retries resolve the hold column — main 103→101 (#2561) ... > **Based on `main`, not on my U7 stack** — merges in any order, no dependency on #2517. My assigned files (`triage.ts`, `replan-target.ts`) are at zero, so this picks up two lifecycle-column literals **no unit's file list claims**. Both ask *"is this card in the hold column?"* by the id `todo`, and both are broken **today** for any workflow that renamed it. ## gridlock-detector — the worse of the two `column !== "todo"` decides which cards count as **schedulable**, and an empty schedulable set is an **early return**. On a renamed board the detector concluded *"no gridlock"* at exactly the moment a real one would be visible. > A detector that goes quiet on the boards it cannot parse is worse than one that is absent, because its silence reads as health. **Converting only the `todo` half would have shipped a still-broken detector**, and the test caught it. The `active` filter is equally literal (`in-progress` / `in-review`) — and an empty active set is *also* an early return. Two literals, one silence. The `in-progress` half sits **outside the drift review's `todo|triage` pattern**, which is precisely why a count-driven sweep would have left it behind and declared the file done. Converted here rather than deferred as out of scope. Worth flagging to the other workers: the convergence metric is a good *tracker* but a bad *definition of done* — an adjacent literal in the same predicate can preserve the whole bug at a lower score. ## mission-autopilot The retry compared against `todo` **and moved to the literal `todo`** — so on a renamed workflow it relocated the card into a column the workflow may not declare (R7) on **every retry**. Now resolves the hold role; when the workflow declares none it leaves the card in place and says so, because the error/status clear still runs, so the retry is not lost — the card just stays in its own lane. ## Two fixture defects of my own, both caught by the tests failing wrongly **My first autopilot tests re-implemented the decision** and asserted on the copy — proving only that the copy works. That is the anti-pattern named in `docs/solutions/store-fake-defects-that-masquerade-as-production-bugs.md` (#2534) and in the #2527 ratchet review, and I had no excuse: the constructor takes two stores and `handleTaskFailure` is public. Rewritten to drive the real method. **My first gridlock fixture failed on both vocabularies** — the detector needs three preconditions and I supplied one. A test that fails on its *no-regression* half is a broken fixture, not a discovered bug. The "both halves failed" heuristic from that same doc is what flagged it. That is eight fixture defects across this unit, every one caught by reading *why* a test failed rather than making it pass. ## Revert proofs, each isolated to one literal | Restored | Result | |---|---| | gridlock hold filter | **1 of 5 fails** (renamed case) | | autopilot move target | **1 of 5 fails** (renamed case) | Default-vocabulary halves pass either way — the correct signature for conversions that change no existing behavior. ## Convergence Measured against `origin/main` with a comment-stripped scan of `column === / !== "todo" | "triage"` in `packages/*/src`, excluding tests: **103 → 101.** (The gridlock `active` filter is a third site fixed here that this pattern does not count.) ## Verification | Check | Result | |---|---| | new suite | 5/5 | | pre-existing gridlock + autopilot suites | 85/85, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (414 + 10 + 71) | | `pnpm check:changesets` | clean | ## Still unowned after this `mission-feature-sync.ts` (1: a planning-lane check) and `auto-claim-snapshot.ts` (1: `isRunnableAutoClaimCandidate`, a **pure sync** predicate that needs the injected-lane pattern from #2551, not a resolve). `notification-service.ts` has one more with a different semantic — *"has progressed past"* — which needs its own thinking rather than a mechanical swap. I will take these next unless someone claims them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 10:54:35 -07:00
gsxdsm	bbaa254dc3	test: add the missing debug to 27 logger mocks (206 → 4 failures) (#2573) ... **Test-infrastructure fix.** 29 test files. No production code, no altered assertions, no widened timeouts. Now **3 commits** (#2584 merged into this branch): the logger-mock sweep, a cron-runner follow-up from review, and the 4 residual failures the sweep deliberately deferred. **Whole branch: 764 tests, 0 failures** across the touched set. --- ## Commit 1 — the missing `debug` on 27 logger mocks `createLogger`'s real shape is `{ log, debug, warn, error }`. 27 engine test files mock `../logger.js` with logger-shaped literals that **omit `debug`**, so any production path reaching `log.debug` threw: ``` TypeError: schedulerLog.debug is not a function TypeError: runtimeLog.debug is not a function TypeError: log.debug is not a function (SelfHealingManager.start) ``` Measured, same commit, same 27 files: | | Failed | Passed | |---|---|---| | before | **206** | 558 | | after | **4** | 760 | **202 failures fixed by one missing mock export.** Per-file: `notifier` 36→0, `plugin-runner` 56→0, `grok-runtime-routing` 14→0, `self-healing-completion-fanout` 1→0. That last one also leaked an unhandled rejection out of `startMaintenance`, which vitest warns "might cause false positive tests" elsewhere in the file. *A note on the number:* a full `engine-default` run went 283 → 106 across my two sessions, but `main` moved in between (U11 landed), so that spread is **not** attributable here. 206 → 4 is the honest figure: same commit, same file set, only this diff varying. ## Commit 2 — cron-runner's factory (greptile P1) My regex required `log: vi.fn()`; `cron-runner.test.ts` uses `log: cronLoggerSpies.log`, so the `createLogger` factory's returned literal never matched and the logger production received still lacked `debug`. **Measured before claiming a live fix, and the numbers don't support that part:** `cronLoggerSpies.debug.mock.calls.length` is **0** across all 155 tests, and the suite is 155 passed both before and after. The described failure mode — `tick()` hitting `log.debug`, throwing, and being swallowed by its own error handler — is **not reachable today**, because no test exercises those three branches (`cron-runner.ts:377`, `:385`, `:410`). The fix is defensive, not curative. The real gap it surfaced is **missing coverage** for schedule dedupe / scope mismatch / lost atomic claim, which I did not write blind to close a thread. ## Commit 3 — the 4 residuals **`notification-service` (3):** messages moved to DEBUG in production (`:580`, `:846`) while tests asserted `schedulerLog.log`. The token case needed more than a relocation. It asserted `expect(schedulerLog.log).not.toHaveBeenCalledWith(containing("new-token"))`. Moving only the *positive* assertion to `debug` would leave the secrecy check watching a channel the message no longer uses — a token could leak through `debug` and the test would still pass. The negative now runs across all four channels. **Verified it bites:** interpolating the token into the debug line fails the test. **`openclaw-runtime-integration` (1):** `../pi.js` mock missing `wrapToolsWithOutputBudget` (same class as #2547); this suite exercises a non-pi runtime, exactly where that wrapper applies. **Not swept repo-wide, and the measurement is why.** 37 `pi.js` mocks omit that export. Patching 30 moved the set from **11 failed to 10** — thirty files of churn for one test. Reverted. Commit 1 earned its 27-file diff with 202 fixes; this one earned nothing, and a no-op sweep is just future merge conflicts for other workers on this program. --- ## Why none of this is appeasement AGENTS.md forbids making a red test pass by loosening it. This does the opposite: the mocks were **wrong** — they claimed to stand in for `createLogger` while missing part of its interface. Nothing was relaxed; stubs were completed, and the one assertion I did move got **stronger** (four channels instead of one). ## Also deliberately not done Extending `scripts/check-mock-completeness.mjs` to catch this class. Measured first: a naive rule over relative intra-package mocks flags **147** factories of which **146 are green** — almost pure false positives. The barrel heuristic works because `cliSrc` gives a tight import surface; that doesn't transfer. A gate that noisy gets ignored, which is worse than no gate. ## How this was found While characterizing U9's review lane. These files were pre-existing baseline noise under mutation runs — and that noise is exactly what made my own safeguard baseline (#2511, corrected in #2520) report two false verdicts. **A red suite does not merely lack coverage; it makes every nearby measurement untrustworthy.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 10:50:46 -07:00
gsxdsm	d2ce1ba8b5	U11: resolve the scheduler's event-handler columns by trait (10 live sites, sync resolution) (#2518) ... Based on `main`. Ten live `"todo"` sites in `scheduler.ts` now resolve the column by trait. ## Four groups, converted together They fail **independently**, and a half-conversion is indistinguishable from a working system: | group | sites | failure mode | |---|---:|---| | **Wake triggers** | 4 | **Latency** — snapshot invalidation, mission-failure tracking, engine requeue tracking, move-to-backlog wake. The wake doesn't fire and the card waits up to a poll interval. Exactly why it would go unnoticed indefinitely. | | **Parked wakes** | 2 | Latency — unpause and planning-finished, keyed on hold OR intake. | | **Dependency** | 3 | **Not latency.** After a blocker completes or is soft-deleted, the query returns nothing, so the dependent is *never* unblocked and waits on a blocker that already finished. | | **Agent link** | 1 | `rollbackRunningAgentsForQueuedTodoTask` passes a synthetic `{ column: "todo" }`. Wrong here **drops a running agent's task link** — the worse direction of that safeguard. Resolved `parkedColumns` is now passed through too, rather than letting the helper fall back to its legacy default. | ## Resolution is synchronous, deliberately — the part worth reading My first cut used the async resolver and made the `task:updated` listener `async` to suit it. **That broke 5 pre-existing tests, and the tests were right:** introducing a new `await` *before* a listener's existing synchronous work defers everything after it to a microtask and reorders handlers relative to a synchronous emitter. A conversion must not change event ordering. It now uses the store's sync IR path (`resolveTaskWorkflowIrSync`), so **no new suspension point is introduced anywhere**. That's the fifth time in this program a change that looked like a move quietly altered behavior — and the first time the existing suite caught it before review. ## Verification - **Mutation-verified:** forcing the resolver back to the literals fails **4 of the 6** new tests - 110 tests green across all 8 scheduler suites (6 new) - Fail-soft to the legacy pair: an unresolvable workflow behaves exactly as before rather than losing the wake - merge gate green (309 + 10 + 71), tsc clean, lint clean ## Measured 10 of my unit's 68 remaining code sites converted. `scheduler.ts` now has **one** `"todo"` literal left in live code: `isRunnableQueuedOverlapCandidate`, which is **exported but has no production caller** — its only consumer was the legacy dispatcher deleted in #2505. That's a **deletion, not a conversion**, so it is deliberately not in this PR. No changeset: `@fusion/engine` is private. 🤖 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** * Scheduling now correctly recognizes workflow-specific hold and intake columns, including renamed columns. * Tasks entering a hold column reliably trigger scheduling and wake-up behavior. * Dependency recovery now finds blocked tasks in renamed hold columns. * Planning, unpausing, task completion, deletion, and requeue flows now respect each workflow’s configured parked columns. * Prevented unnecessary scheduling for moves between unrelated workflow columns. * **Tests** * Added coverage for renamed hold-column scheduling, wake-up, and dependency-unblocking scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-07-29 10:44:12 -07:00
gsxdsm	fb7ab6df26	test: re-green self-healing, worktree-pool and DB-corruption assertions (#2592) ... **Test-only.** Three files, two commits. No production changes. | File | Before | After | |---|---|---| | `self-healing-db-corruption` | 5 failed / 1 passed | **6 passed** | | `self-healing` | 1 failed / 411 passed | **412 passed** | | `worktree-pool` | 2 failed / 57 passed | **59 passed** | All three are the same underlying story in different costumes: **the assertion is watching a channel production stopped using**, or a step that aborts before it can log at all. ## Commit 1 — the fake store was missing the health refreshers `surfaceDbCorruption` *refreshes* health before reading the snapshot (`FNXC:IncompletePgPorts 2026-07-26-20:45`, so PG connectivity is re-checked instead of trusting an always-healthy sentinel). The fake carried **neither** refresher, so the async branch fell through to `this.store.refreshDatabaseHealth()` — undefined — and the step threw before reaching dispatch. **Every assertion in the file was measuring zero calls against a step that had already aborted.** Both stubs are **no-ops on purpose.** Production ignores the refresh return and reads `getDatabaseHealth()` immediately after, so the snapshot mock stays the single source of truth. My first attempt delegated them to `getDatabaseHealth`, which consumed a *second* value per pass from the test that queues three `mockReturnValueOnce` snapshots (one per `runMaintenance`) and broke its corruption → clear → corruption ordering. Faithful beats convenient. ## Commit 2 — two more debug-level assertions - **`self-healing`**: `"auto-archive: archived …"` is emitted at DEBUG (`self-healing.ts:2747`); the test asserted `.log`. The mock already had `debug` (from #2573), so only the target was stale. - **`worktree-pool`**: both checkout-failure cases assert on `console.error`, which is *correct* — `createLogger`'s `debug` writes there. But debug is **gated on `FUSION_DEBUG`** (`logger.ts:43`), unset under vitest, so the line was never emitted. One test is literally named *"logs checkout -- failure at debug level"* while asserting a channel debug could not reach. Fixed by enabling `FUSION_DEBUG="worktree-pool"` for the suite and deleting it in `afterEach` so the flag can't leak into sibling files. **Deliberately not** fixed by re-pointing the assertions at another channel — that describes whatever the code happens to do rather than the behavior the test names. ## Verified each actually guards A test that merely stops failing can still assert nothing, so every fix was mutation-checked: | Mutation | NEW failures | |---|---| | `surfaceDbCorruption` returns early | **5** | | remove the auto-archive debug line | **1** — that test, only it | | remove the checkout-failure debug line | **2** — both cases, only them | ## Known residual, stated rather than hidden `self-healing-db-corruption` **still exits non-zero** with 9 unhandled `this.store.listTasks is not a function` rejections from `openSurfacingCycle` (`self-healing.ts:7737`). These **predate this change** — identical count before and after. The maintenance pass opens one shared surfacing cycle up front, independent of which steps `stubMaintenance` stubs. I tried to clear them and backed it out, twice: - adding `listTasks: async () => []` lets the cycle open, but then *other* unstubbed sweeps run for real — an orphaned-planning-segment audit fires and breaks 3 assertions expecting `recordRunAuditEvent` never to be called; - stubbing the four `surface-*` siblings didn't help either, because the cycle is opened by the **pass**, not by the steps. Making that file honestly green needs a fake complete enough for the whole maintenance registry — a bigger change than the bug in front of me, and one that would bury the fix above. Flagging it rather than shipping a half-sweep. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 10:43:14 -07:00
gsxdsm	21497b23db	P0: approved plans never released after #2515 + triage.ts 11 -> 0 (#2549) ... Rebased onto post-#2515 `main`. **This PR is the fix for a P0 stall**, not just a conversion. ## Stall 1 — approved plans were never released `recoverApprovedTask` opened with a bare `task.column !== "triage"`. #2515 merged Todo into Planning on the default lineage, so every default card now sits in `todo` and **this guard rejected all of them**. An approved plan whose finalize was interrupted was never released, and nothing else owns that card. Callers: `triage.ts:1296` (stuck-kill recovery) and `in-process-runtime.ts:1460`. `triage` stayed a legal id, so nothing threw — the guard just stopped matching. I verified the fix **mechanism** rather than assuming it. `resolveLifecycleColumns` on the IR #2515 actually shipped returns: ``` { intake: "todo", hold: "todo", wip: "in-progress", review: "in-review", complete: "done", archived: "archived" } ``` so the converted guard admits default cards. The new regression test asserts the **return value**, because on a merged lineage the card is already where the release would send it — "no move issued" is what *both* the broken and the fixed code do, so only the outcome discriminates. **Mutation-verified:** restoring the literal `!== "triage"` fails 2 of 5 tests. ## A defect of my own, found while auditing — same shape as the P0 `clearStaleSpecifyingStatuses` is a board-wide startup sweep with no single task to resolve lanes against, and I had resolved **both** its queries from the default workflow. Post-#2515 that workflow's `intake` and `hold` are the **same** column, so both queries collapsed onto `todo` and **nothing ever swept `triage`**. A legacy or Coding (Ideas) card holding a stale `planning` status would then occupy a planning admission slot permanently — exactly the failure the 2026-07-04 note above that function warns about. Now queries the **union** of the legacy planner ids and the resolved lanes, deduped by task id. Querying extra columns is free here: the sweep only reads, and every row is filtered on `status === "planning"` before anything is written. Caught by `triage.test.ts`, **not by my own tests** — worth recording, since it is the same collapse the P0 is about. ## Rebase note The discovery conflict was resolved **in favour of `main`**. Main's version is strictly better than mine: it resolves lanes with the **async** `resolveTaskLifecycleColumns` (so it is not subject to the sync-resolver limitation below), keeps the two admission branches disjoint for a merged column, and bounds concurrency. My sync version was dropped. ## Measured | file | comparisons before | after | |---|---:|---:| | `packages/engine/src/triage.ts` | **11** | **0** | ## Known red, NOT from this PR 8 tests in `triage.test.ts` fail on **clean `origin/main`** — confirmed by swapping main's `triage.ts` into this tree and re-running (same 8). They are reporting the upgrade stall, not stale expectations: a card *sitting* in `triage` is admitted by nothing after #2515 (`expected "specifyTask" to be called 4 times, but got 0 times`), and #2515 shipped no data migration re-homing those rows. Left untouched here — the fix is a data migration, not a conversion. Reported to the coordinator separately. ## Verification - merge gate green (414 + 10 + 71), tsc clean, lint clean - mutation-verified as above ## Separate finding — affects every worker `resolveTaskWorkflowIrSync` **cannot resolve a task's selection in production.** `getTaskWorkflowSelectionImpl` is `return undefined` unconditionally and `getTaskWorkflowSelectionAsyncImpl` is *"always PostgreSQL path"*, so the sync resolver **always** returns the DEFAULT workflow IR. `moves.ts` already hit this and fixed it by going async. Consequence for `resolvePlannerLanes` here: correct for default-lineage cards (the default IR is exactly what comes back — which is why Stall 1 is genuinely fixed) and **inert for custom workflows**. Not papered over; the async path is main's discovery code, and converting the remaining event-listener sites needs the handler-reordering problem solved first. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## P0 audit table — every `triage` site in my assigned files (a) does it still fire for a default-workflow card after #2515? (b) if not, what silently stops happening? (c) fix. | site | (a) still fires? | (b) what silently stops | (c) disposition | |---|---|---|---| | `triage.ts:613` wake handler | **yes** | — OR-shaped (`todo \|\| triage`), still matches | converted anyway | | `triage.ts:651` evacuation guard | **yes** | — OR-shaped, still matches | converted anyway | | `triage.ts:741` stale-planning sweep | **yes** | — OR-shaped, still matches | converted anyway | | `triage.ts:1088` `recoverApprovedTask` | **NO** | **STALL 1** — approved plan never released; nothing else owns the card | **fixed + regression test + mutation-verified** | | `triage.ts:1396` advanced-recovery discovery | **NO** | that recovery never matches a default card | fixed by the same conversion | | `clearStaleSpecifyingStatuses` (mine) | **NO** | **my own defect** — both queries collapsed onto `todo`, `triage` never swept; stale `planning` holds an admission slot forever | **fixed** (union of legacy + resolved lanes) | | `replan-target.ts:177` / `:185` | **NO** | **STALL 2** — see #2552 | fixed in #2552 | | `spec-staleness.ts:95` | **NO** | narrow: a Planning card with null status and `currentStep > 0` now skips staleness where it previously did not | **recorded, not fixed** — see below | | discovery (`isAtIntakeColumn`) | **NO** | **STALL 3** — a card *sitting* in `triage` is admitted by nothing | **reported, not fixed** — needs a data migration | **Why `spec-staleness.ts:95` is not fixed here.** The guard already returns `false` for `status === "planning"` and `needs-replan`, so an *actively* planning card is still covered by status. The `column === "triage"` arm only added coverage for a planner-lane card with **no** status — and post-merge that case is genuinely ambiguous, because `todo` is now both the planning lane and the hold lane, so a card with progress there may legitimately be a released card that *should* skip. Guessing either way is a behaviour change without evidence, so I recorded it rather than picking one.	2026-07-29 10:30:18 -07:00
gsxdsm	c92bce2f8c	test: delete 2 project-engine-manager tests for the deleted cross-project cap (#2575) ... **Test-only.** One file, 2 obsolete tests + 1 dead import removed. No production change. `project-engine-manager.test.ts` has been **red on main: 2 failed / 44 passed** → now **44 passed**. ## The failures Both threw `TypeError: Cannot read properties of undefined (reading 'acquire')`, because both reach `(manager as any).globalSemaphore` — a private field that no longer exists. `project-engine-manager.ts:88` records why (`FNXC:CapacityModel 2026-07-28-20:10`, *"drop the cross-project cap"*): > The shared cross-project semaphore, its mutable limit and the `concurrency:changed` subscription are **DELETED**. Capacity is two numbers per project; a machine-wide cap was a third limiter with its own separate authority (a central-DB singleton row), and reconciling it against the per-project gates is exactly the multi-limiter arbitration this simplification removes. So both tests assert residual-slot accounting on a shared pool that was **deliberately** removed — not a regression. ## Why deleted rather than repaired There is no shared semaphore left for them to describe. Reconstructing one inside the test would assert a capacity model the engine no longer has — a test that passes while describing fiction, which is worse than the red it replaces. Also drops the now-dead `ScopedAgentSemaphore` import (these were its only uses). Lint does not flag unused imports here, so it would otherwise have sat as quiet dead code. ## What I did NOT take, and why `workflow-graph-optional-step-fix.test.ts` — the other red file adjacent to this lane, 5 failures. Its failures are **U11 column-vocabulary drift**: the replan rebound now resolves to `todo` where the test expects `triage`, and one case gets a hard-cancel pause-abort log instead of the Plan Review replan message. That is the U11/U12 owner's semantics to settle. Picking whichever column makes the assertion pass could silently encode the wrong lifecycle target — and per the graph-entry contract doc, a rebound landing in a column the workflow does not declare is precisely the failure mode that "does not fail a test; it disables a recovery path in production." Flagging it rather than guessing. ## Running tally of this cleanup thread | File | Before | After | |---|---|---| | 27 logger mocks (#2573) | 206 failed | 4 failed | | `merge-error-recovery` (#2559) | 10 failed | 0 | | `reviewer` (#2547) | 2 failed | 0 | | `project-engine-manager` (this) | 2 failed | 0 | Every one was a test describing behavior that had moved or been deleted, or a mock that had drifted from its real shape — none was a product defect. That pattern is worth naming: on this repo a red non-blocking suite has mostly meant *stale tests*, which is exactly what makes it easy to ignore, and exactly why it silently corrupted my own safeguard measurements in #2511. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 10:22:46 -07:00
gsxdsm	1c9f6c546b	P0: default Planning cards read as ADVANCED after #2515 (FN-8596 stranding re-opened) + 6 red tests repaired (#2552) ... Rebased onto `main` (post-#2515) and **upgraded from a conversion to a P0 stall fix**. ## What changed since review Greptile's P1 on this PR said the `plannerColumn` seam was unused — *"every current production caller omits `plannerColumn`, so this default still compares against `triage`."* That was correct, and **#2515 turned it from an unused seam into a live stall.** ## The stall #2515 merged Todo into Planning on the default lineage (one pre-implementation column, id `todo`, display "Planning"). `triage` stayed a legal id, so nothing throws — the bare `column === "triage"` guards in `hasAdvancedPastPlanning` just **stopped matching for default-workflow cards**. A default card in `todo`, status cleared to null by the stale-status sweep, carrying execution stamps from a previous pass, now returns **ADVANCED**. So `isTaskStillInPlanningStage` is false and nine guarded call sites refuse planning updates, finalize, delete and handoff: `triage.ts` 3108 / 3117 / 3160 / 3438 / 3937 — `self-healing.ts` 12126 / 12448 / 12454 The file's own FNXC note at `:150` already records what that costs: > Nobody owned the card and it sat indefinitely. This is that same FN-8596 stranding, re-opened by the column merge. Confirmed empirically — the rescue test fails on pre-fix code. ## The fix is an asymmetry, and that's the point The two guards are **not the same rule**: 1. the FN-8596 **arrival-order rescue** — a stamp predating arrival in the planner lane means replanning, not advancement 2. **"the planner column itself is never advanced"** Rule 1 must recognise the merged Planning column. **Rule 2 must not** — on the merged lineage `todo` is *also* the released/hold lane, so making it blanket "not advanced" would strand the release path instead: a released card with steps would read as still-planning and `hasAdvancedPastPlanning(t) || releasedToTodo` would stop distinguishing anything. Rule 1 is already gated on the stamp predating arrival, so a released card later claimed by execution keeps its newer stamp and still reads as advanced. **Closed via the default** (`mergedPlanningColumn = "todo"`) rather than by wiring call sites — the stall closes everywhere at once, with no call-site change and nothing to collide with another worker's slice. Dedicated-planner workflows (Coding (Ideas), and every workflow still declaring `triage`) are byte-identical. ## Second commit: 6 tests left RED on main by #2515 Verified pre-existing by stashing every local change and re-running — same 6 failures on a clean branch. `resolveReplanTargetColumn` reads the IR rather than a literal, so it **self-healed** to the correct post-merge answer (`todo`); the expectations were the stale half. Updated to the post-merge truth, not loosened — each still pins one exact column. ## Audit table for this file (P0 sweep) | site | still fires for a default card? | what silently stopped | disposition | |---|---|---|---| | `replan-target.ts:177` `inPlannerLane` | **NO** | FN-8596 rescue — planning writes no-op, card strands | **fixed** (rule 1) | | `replan-target.ts:185` never-advanced | NO | nothing — must stay dedicated-planner-only | **deliberately unchanged** (rule 2) | | `resolveReplanTargetColumn` | yes (IR-driven) | — self-healed to `todo` | tests repaired | | its two `return "triage"` fallbacks | n/a | reachable only for workflows declaring neither column | **recorded, not fixed** — column-policy decision, has its own covering test | ## Verification - **Mutation-verified both directions:** dropping the merged lane from rule 1 fails **2** tests; wrongly extending rule 2 to the merged lane fails **1** - 50 replan-target tests green (7 new) - merge gate green (414 + 10 + 71), tsc clean, lint clean ## Separate finding — affects every worker `resolveTaskWorkflowIrSync` **cannot resolve a task's selection in production.** `getTaskWorkflowSelectionImpl` is `return undefined` unconditionally and `getTaskWorkflowSelectionAsyncImpl` is *"always PostgreSQL path"*, so the sync resolver **always** returns the DEFAULT workflow IR. `moves.ts` already hit this and fixed it by going async. Any conversion built on the sync resolver is inert for **custom** workflows — harmless for default cards, since the default IR is exactly what comes back. Reported to the coordinator for the other workers; not actionable in this PR, which uses no sync resolution. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-29 10:15:54 -07:00
gsxdsm	41031dbe2c	Drift review (unowned): auto-claim candidacy resolves hold + completion roles — three literals, two opposite failures (#2565) ... > **Based on `main`** — independent of my U7 stack and of #2561; merges in any order. Third unowned drift-review site. `isRunnableAutoClaimCandidate` is the single source of truth for *"may an agent claim this task?"* (FN-6873), and it carried **three** lifecycle literals that fail in **opposite directions**. ## The two failures **`column === "todo"` gated candidacy** on the hold role. Keyed on the literal, a renamed workflow's candidate set was **permanently empty** — agents were never offered its work, and nothing anywhere reported it. Silence, not an error. **`dependency?.column === "done" || "archived"` gated dependency satisfaction**, and this is the more dangerous half: a dependency that finished in a renamed **complete** column was never recognised as done, so the dependent stayed **blocked forever**. One makes work invisible; the other makes it permanently ineligible. Both are silent. ## Roles resolve per task, not per pass The non-obvious part: **a dependency may sit on a different workflow from the claimant.** A single per-pass answer is wrong for one of them on any mixed board — so the map is keyed by task id, and the dependency check reads the *dependency's* roles, not the claimant's. Asserted directly: a dependency completed in `done` (default vocabulary) satisfying a claimant waiting in `drafting` (renamed). ## Shape Both callers already have the store and are async, so they resolve for real rather than taking the injected-lane fallback the *synchronous* predicates needed (#2551). The predicate itself stays synchronous — a resolved-roles map is passed in — because it runs inside two `filter`/`flatMap` bodies. Tasks absent from the map keep the legacy ids, so a partially-resolvable board degrades to today's behavior instead of silently emptying the candidate set. **Type narrowing preserved.** The two callers take `Pick<TaskStore, "listTasks">`, which is what makes them testable without a real store. Rather than widening to the whole `TaskStore`, they now take `Pick<TaskStore, "listTasks"> & WorkflowIrResolverStore` — the minimal additional shape resolution needs. ## Revert proofs, isolated per literal | Restored | Result | |---|---| | hold literal only | **3 of 6 fail** | | dependency-completion literals only | **1 of 6 fails** | The three default-vocabulary cases pass under both. Splitting the proof matters here: it confirms the two halves are **independently** load-bearing rather than one masking the other — a single combined revert would have shown 3 failures and told me nothing about the dependency half. ## Convergence Measured on `main`, comment-stripped scan of `column === / !== "todo" | "triage"` in `packages/*/src` excluding tests: - this file alone: **103 → 102** - with #2561: **103 → 100** The `done` / `archived` literals fixed here sit outside that pattern and are not counted — same caveat as #2561's gridlock `active` filter. Two PRs now where the real fix is larger than the metric shows. ## Verification | Check | Result | |---|---| | new suite | 6/6 | | pre-existing auto-claim suite | 17/17, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (414 + 10 + 71) | | `pnpm check:changesets` | clean | ## Remaining unowned in my area `mission-feature-sync.ts` (1, a planning-lane check) and `notification-service.ts` (1, *"has progressed past"* — a different semantic needing its own thinking, not a mechanical swap). Taking those next unless claimed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 10:01:14 -07:00
gsxdsm	8578a1d27d	U8 PR5: thread the implementation exit to the step seam, and declare the stepwise pending-review park (inert) (#2546) ... Follows **#2519** (U8 PR4). Both halves are inert — **no behavior change** — and this removes the blocker PR4 documented. ## What was blocking PR4 could only land its IR half because the pending-review ending could not reach a graph edge on the **default** workflow. Three links in the chain: | Link | Problem | |---|---| | `runGraphTaskStep` | awaited the memoized implementation pass and **discarded** its result | | `RunTaskStepResult` / `RunSingleStep` | had nowhere to carry an exit | | `stepExecute` seam | flattened every ending to `step-done` / `step-failed` | All three are fixed. The outcome stays `failure` (the step genuinely did not complete) while the **value** now names the ending — which is what `runForeach` propagates upward, since it returns a failing instance's value as the foreach node's own. Every other ending keeps `step-failed` byte-identically. One design note: the exit is a property of the **pass**, not of a step. A single memoized pass serves every foreach instance, so all instances report the same ending — correct, because the ending is what stopped the whole session. With the value surviving, the stepwise IR declares the same `review-handoff` park node and `steps --outcome:review-pending--> review-pending-handoff --success--> end` edge the plain-`execute` shape got in PR4, inherited by the final-review and Ideas variants that clone it. ## A bug my own threading introduced, and what caught it The first threading commit covered **one of the two** paths out of `runProjectedGraphTaskStep`. The early-return branch carried the exit; the main path goes through `runTaskStep` in `step-runner.ts`, which builds its own result and dropped it — i.e. it worked on the path I happened to read, and not on the path the default workflow actually takes. **FN-5436's regression test caught it, not code review.** That is the second time this test has stood between this unit and a silent regression, which is worth recording somewhere durable: `executor-step-session.test.ts > FN-5436: pending-review skip on no-fn_task_done exit` is the load-bearing test for this area. ## Why the seam flip is still not here With the threading complete I applied the behavior half again — flip the execute seam to return `review-pending`, delete the inline `handoffTaskToReview`, add a named compat classifier for user-authored graphs. **FN-5436 still failed**: the card did not reach `in-review`, so something between the seam value and the park node is not routing under that harness. I have not isolated whether that is the mock store's IR resolution (it exposes no `getWorkflowDefinition`, so the run resolves the built-in through a different path), a foreach aggregation detail, or the park node's own seam. I stopped rather than keep guessing, and reverted the behavior edits so this lands green and inert. Shipping a half-routed move is exactly the failure this unit exists to remove — a lifecycle transition that silently does not happen. The alternative on offer was to relax FN-5436's assertion, which would have been appeasing a test that is telling the truth. ### What the instrumentation showed (done after opening this PR) I ran the bounded next step rather than leaving it as a note. Two facts, both measured: 1. **The IR is correct.** Resolving `BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR` at runtime shows the node and the edge survive the final-review variant's edge rewiring: ``` EDGES [{"from":"steps","to":"browser-verification","condition":"success"}, {"from":"steps","to":"review-pending-handoff","condition":"outcome:review-pending"}, {"from":"steps","to":"end","condition":"failure"}] HAS NODE true ``` That matters because the variant does `template.edges = [ ... ]` (a wholesale replacement) and filters outer edges touching `review` — `review-pending-handoff` is not `review`, so it survives. Worth knowing before anyone adds another node near it. 2. **The `stepExecute` seam is never invoked in that harness**, even though the run terminates at `steps#0:step-execute` and the implementation session demonstrably runs (`"Agent finished without calling fn_task_done but Step 0 is blocked on pending review"` is in the task log). A `console.log` at the seam's value computation produced no output. So the exit is threaded correctly and the IR can route it, but under this harness the value never originates. 3. **Nor is `createPromptLikeHandler`'s returned handler.** Instrumenting its dispatch (`node.id` + resolved seam) produced nothing either — so the node is not reaching the prompt-like path at all. **Control experiment, because a negative result from instrumentation is worthless until you prove the instrumentation is observable.** A `process.stderr.write` at module load of the same file appears exactly once in the same run, so writes from that module *are* captured under this harness and the two negatives above are real, not artifacts of swallowed output. That narrows the remaining work to one question — what actually drives `steps#0:step-execute` in this run, if neither the prompt-like handler nor the `stepExecute` seam does — and rules out the IR, the foreach propagation, the threading, and the instrumentation as suspects. **Next step, now much narrower:** find the handler registration this run resolves for a foreach instance node (the graph executor's handler map, not the seam table), then flip the seam, delete the inline handoff, and update the three ratchets that will correctly fire — PR3's routing pin, the out-of-band adjacency check, and PR1's ownership ledger (`runImplementation` 3 → 2; `handleGraphFailure` 0 → 1 for custom graphs only). ## Verification - `executor-step-session` + exit-events + ownership ledger + graph-boundary — **56 tests green** - `builtin-workflows` + `builtin-coding-workflow-ir` — green. The layout-completeness contract required a layout entry for the new node in all four stepwise-derived workflows; placed off the main line, because a park is an exit and not a stage. - `pnpm test:gate` green (10 / 309 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `internal`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:54:39 -07:00
gsxdsm	3681a9f9a5	test(U9): re-green merge-error-recovery.test.ts (10 stale tests deleted, replacement contract covered) (#2559) ... **U9, PR8.** Test-only, two commits (deletion and new coverage deliberately separate). `merge-error-recovery.test.ts` has been **red on main: 10 failed / 23 passed**. Now **24 passed**. ## Commit 1 — the 10 failures test a feature that no longer exists All 10 assert that `ProjectEngine` creates recovery follow-up **tasks** and dedupes them by parent/branch. Evidence this was deliberate, not a regression: - `project-engine.ts` contains **zero** `createTask` calls. - The string the dedupe tests assert on — `"follow-up already exists"` — exists **only in the test file**; no production code emits it. - `project-engine.ts:4801` documents it outright (`FNXC:AutostashRecovery 2026-07-26`): *"This used to file an automated recovery follow-up card via the shared follow-up engine; that engine was deleted ... So the card is replaced by a durable log entry AND an operator comment on the parent."* Deleted rather than repaired — there is nothing left for them to assert. ## Commit 2 — cover the contract that replaced them The production comment is explicit that `record.label` *"must never be dropped from the message or truncated"* — it is the handle `git stash` recovery needs, and the parent may already be `done`, so the notice is the only trace of real uncommitted work. **That invariant had no working assertion.** The file was red, so every claim it made was inert. The new test asserts one log entry + one comment for a `live` orphan (a `subsumed` record stays silent), and that label, short sha, detecting task and source phase all survive into the comment, with the label in both the log message and its detail field. | Mutation | Result | |---|---| | replace the label with `(omitted)` | 1 failed / 23 passed — this test | | notify on non-live orphans too | `NEW-failures=1` — this test | ## A tooling bug this uncovered, which matters beyond this PR The new test originally reported **zero** new failures under mutation while passing normally — i.e. it looked vacuous. It was not. A thrown assertion left the engine running, which **crashed the vitest worker**, and a crashed run emits no parseable `FAIL` lines — so my mutation harness parsed zero failures and printed **NOT COVERED for a guard that had just correctly failed**. Two fixes: - The test stops the engine in a `finally`, so a failure reports as an assertion instead of killing the worker. - The harness now treats *non-zero exit with zero parsed failures* as **INCONCLUSIVE**, never as a coverage verdict, and prints the crash signature. This is the **second** time a blind spot in my own tooling manufactured a false "uncovered" result — after the `|project|` regex that matched nothing for `@fusion/core`. Both had the same shape: the measuring instrument reported success without checking anything, which is precisely the defect class this program is chasing. Worth stating plainly rather than quietly fixing. ## Why this file matters to U9 Its 10 pre-existing failures are what corrupted my own safeguard measurements in #2511 — an absolute-count mutation run credited them to the mutation. **A red file in the merge lane does not merely lack coverage; it poisons the measurement of everything near it.** ## Wider context, measured `engine-default` on clean `main` is **283 failed / 9062 passed across 28 files**. This PR clears one of those files. I did not attempt the rest: most are outside the merge/review lane and plausibly owned by other workers on this program. Also measured and abandoned: extending `check:mock-completeness` to relative intra-package mocks — the naive rule flags **147** factories of which **146 are green**, so it would be almost pure false positives; the barrel heuristic does not transfer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:54:04 -07:00
gsxdsm	67904f8a2c	U11: merge Todo into Planning on the default lineage (+ the migration mechanism, and a measured safety audit that cuts the work list 32%) (#2515) ... **Merges Todo into Planning on the operator's real default workflow.** Held from merge pending the `triage` literal audit below — see *Gating*. ## The board change `builtin:coding` → `BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR` → clones `BUILTIN_STEPWISE_CODING_WORKFLOW_IR`. That IR now declares **five** columns, and `plan`, `plan-review`, `plan-replan` and `start` all live in the merged Planning column: ``` columns: todo="Planning", in-progress, in-review, done, archived start -> todo plan -> todo plan-review -> todo plan-replan -> todo parse -> in-progress (first implementation node) ``` The id stays `todo`, the display name becomes "Planning". That is the cheaper half: `todo` was already the hold column, so every trait lookup, task row, stored selection and the 121 `column === "todo"` guards keep their meaning, and **no stored row needs re-homing**. Promoting `triage` instead would have produced the same board while making those guards workflow-*dependent* — live for Coding (Ideas), silently dead for Coding. `builtin:legacy-coding` keeps its six-column shape, per the operator's decision. It exists to be the old thing. ## Entry contract, before and after each IR edit | | result | |---|---| | before the default-lineage edit | **15 passed** | | after the edit | **13 passed, 2 failed** | | after reading both | **15 passed** | Neither failure was routed around. One was a genuine expectation change (two planning entry points became one); the other was my own `mergeTodoIntoPlanning` helper throwing *"source IR is not the split-column shape this merge transforms"* — because production **is** the merged shape now. I **deleted** the helper rather than making it tolerant: a transform that has silently become a no-op asserts nothing. ## The safety argument, proven not asserted Entering at `start` is exactly what dragged cards backward in the three earlier reverted attempts. `merged-planning-start-node-no-move.test.ts` proves against the **real** boundary controller and **real** default IR that entering `start` performs no move (`moveTask` is never *called*), reaches no hold→wip capacity seam, and **still moves on a genuine crossing** so the no-op is same-column rather than a disabled boundary. Removing the controller's same-column short-circuit turns exactly the two no-move tests red. ## The migration mechanism A card can outlive its column. `resolveAllowedColumns` derives targets from graph adjacency, and an undeclared source has none — so it returned `[]` and **every** move was rejected with "Valid targets: none", including the one that would rescue the card. An undeclared source now resolves to the workflow's rebound target. Escape hatch, not relaxation: declared columns are untouched, and it offers the rebound target *only*, so a stranded card gets back **into** the lifecycle rather than a free jump past review. ## A real regression this surfaced `isDefaultWorkflowColumns` matched the legacy **six** ids as a set. The merged default declares five, so the match stopped firing and the default board fell through to neighbor-only adjacency, which **drops legal moves and invents an illegal one**: | edge | effect | |---|---| | `in-progress → done` | **dropped** — the mission-validation cross edge | | `in-review → todo` | **dropped** — review work back to planning | | `todo/done → archived` | **dropped** — the FN-4892 direct-archival edges | | `done → in-review` | **invented** — a backward edge no rule allows | Adjacency now derives from lifecycle **roles**. The load-bearing assertion: the legacy six still reproduce `VALID_TRANSITIONS` **verbatim**. Applied only when a workflow declares the full role set, so custom boards keep neighbor adjacency. ## Failure accounting (core package, vs a 49-failure baseline) | stage | failed | new | |---|---:|---:| | after the merge | 65 | 18 | | after the escape hatch | 52 | 5 | | after role-derived adjacency | 53 | 4 | The 4 remaining are 3 `builtin-workflows` expectations encoding the pre-merge shape and 1 create-intake expectation naming `triage` on `builtin:coding`. Two `schema-applier` and two `workflow-reconciliation-production-shape` failures appeared in intermediate runs and are **not mine** — both files pass in isolation (75/75 and 7/7). I re-ran each before attributing them, which is why the earlier "priority" flag on the reconciliation pair was withdrawn. Gate: **309/309**. Lint clean. ## Gating: the `triage` audit (`docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md`) Program tracking cited **58** `triage` comparisons. Measured with the same pattern: | | count | |---|---:| | raw comparisons | 87 | | inside comments | 1 | | **not a lifecycle column at all** | **15** | | column comparisons | 71 | | OR-paired with `"todo"` in the same expression | 32 | | **exclusive `triage` — the real work list** | **39** | **15 do not compare a column.** `role === "triage"`, `surface === "triage"`, `sessionPurpose === "triage"`, `entry.agent === "triage"` name the planning **agent**. Converting them would be actively wrong, and the failure — a planning agent that can't resolve its prompt template — would look nothing like a column bug. **One site changes an operator-visible affordance**, which is why per-site review beat a sweep: `TaskCard.tsx:1927` — `taskColumnFlags?.intake === true && task.column !== "triage"`. The literal is a **narrowing**, not a match. After the merge a Planning card has `intake === true` and `column === "todo"`, so the narrowing stops applying and **Start begins rendering on default Planning cards where it previously did not.** A sweep would have "converted" the literal and shipped the new affordance silently. These guards do not go **dead**, they go **workflow-dependent** — `triage` stays live for legacy-coding, Ideas, every linear built-in and any user workflow (R11) — which is harder to detect than dead. Work list and ownership are in the audit doc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:39:20 -07:00
gsxdsm	69790dc3e7	test(U9): revive two permanently-red testMode guards in reviewer.test.ts (#2547) ... **U9, PR7.** One test file, +15 lines, no production change. ## Two safety tests that could never pass `reviewer.test.ts`'s `vi.mock("../pi.js")` is missing `wrapToolsWithOutputBudget`, which `wrapCustomToolsForPluginRuntime` (`agent-session-helpers.ts:104`) calls as the outermost tool wrapper. Both test-mode-forcing cases therefore threw: ``` No "wrapToolsWithOutputBudget" export is defined on the "../pi.js" mock ``` They have been **permanently red on main** — dead enforcement on the invariant that **testMode never issues real AI calls**. `reviewer.test.ts`: 83 passed | 2 failed → **85 passed**. Found while characterizing the reviewer lane for U9: they surfaced as pre-existing baseline failures under an unrelated mutation run. This is exactly why the delta harness records a baseline — under the old absolute-count method these two would have been silently credited to whatever mutation was running. **Not a product bug.** testMode forcing works correctly; its guard did not. ## Verified the revived tests actually guard something A dead test can also be a vacuous one, so passing again is not sufficient evidence. Mutating `isTestModeActive` in `model-resolution.ts` to ignore `settings.testMode` fails **exactly these two** (`NEW-failures=2`). Both assert `expect(mockedCreateFnAgent).not.toHaveBeenCalled()` — no live agent spawn. ## Why the existing gate didn't catch it `pnpm check:mock-completeness` runs in the merge gate and passes. It inspects only the `@fusion/engine` and `@fusion/dashboard` **barrels**, under `cli/` and `dashboard/` test dirs — never a relative intra-package mock like `"../pi.js"`. So the whole class of engine-internal mock drift is outside it. **Deliberately not fixed here.** Extending the checker is its own change and I want the violation count measured before proposing it, rather than opening a PR that turns out to touch dozens of files. That's the next PR. ## Also observed, stated rather than buried Mutating `useMockRuntime` in `agent-session-helpers.ts` produces **no** failure in this file — the reviewer path routes through model resolution instead. That downstream seam has its own coverage question which I have not answered; flagging it rather than implying this PR closes it. ## Scope note This was initially committed onto #2541's branch. I split it onto its own branch so each PR stays independently revertable — #2541 is now one commit (the FN-7720 verdict assertion) and this is one commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:32:34 -07:00
gsxdsm	c2705f292f	U11: delete the dead isRunnableQueuedOverlapCandidate export (scheduler.ts now has zero live todo literals) (#2542) ... Based on `main`. **Pure deletion — zero production callers.** `isRunnableQueuedOverlapCandidate`'s only consumer was the legacy pull-from-todo dispatcher deleted in #2505. The three remaining references were all in tests. This was `scheduler.ts`'s **last `"todo"` literal in live code**, so removing it rather than converting it is what actually finishes the file — converting a dead predicate would have added a trait lookup nothing calls, and reported U11 progress for a site that cannot execute. ## Why deleting its tests does not lose coverage Worth checking, because the function carried a real invariant — *"a busy merge lane must not block unrelated dispatch"* — and its own doc comment claims it's a shared contract with self-healing and repair paths. Two facts settle it: 1. **The overlap logic is still live**, implemented inline inside `runHoldReleaseSweepPass` (`activeScopes`, `overlapIgnorePaths`, `getFilteredFileScope`). The behavior didn't die with the predicate; only this copy of it did. 2. **`scheduler-overlap-starvation.test.ts` exercises that live path** through `scheduler.schedule()`, including *"does not defer ready work behind queued overlap blocked by an active lease"* — the same invariant the deleted test asserted, against code that actually runs. The doc comment's claim that self-healing *"must use this same predicate"* is **stale**: no self-healing path imports it. That claim outlived the coupling it described. ## The three test references were not equal Treating them identically would have been wrong: - **Two were incidental trailing assertions** in tests about other subjects (stuck-loop exhaustion parking; transient merge-error classification). Only the assertion line is removed — each test keeps its real subject. - **One test's entire subject was this function** (*"does not block unrelated executor dispatch when merge lane is busy"*), so it goes with it; its invariant is covered on the live path per (2). ## Measured `scheduler.ts` **2,840 → 2,820 = −20**, and it now holds **zero `"todo"` literals in live code**. Combined with #2505's −929, `scheduler.ts` is down **949 lines** across this unit — all genuine removal, not relocation. ## Verification 582 tests green across the reliability-interactions suite and four scheduler suites; merge gate green (414 + 10 + 71); tsc clean; lint clean. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Simplified internal task scheduling logic by removing obsolete overlap coordination checks. * Preserved existing task progress, parking behavior, logging, and review handling. * **Tests** * Updated reliability checks to align with the streamlined scheduler behavior. * Continued validating transient errors, non-progress handling, and correct task dispatch without changing the end-user experience. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-07-29 09:25:16 -07:00
gsxdsm	7fd1c7f124	P0 fix: stop reaping worktrees out from under live planners (FN-6756) (#2531) ... User-reported: worktrees deleted while a planning agent was still working in them. Small, isolated, ahead of all remaining capacity work. ## Mechanism `clearPhantomExecutorBinding` is documented as *"the last line of defense against pulling a worktree out from under a running agent"*. It computed liveness from four sets — `activeSessions`, `activeStepExecutors`, `activeWorkflowStepSessions`, `activeCliTaskSessions` — **all TaskExecutor-owned**. A triage PLANNING session is owned by `TriageProcessor`, lives in *its own* `activeSessions` map, and registers in the module-level `activeSessionRegistry`. It matched none of the four. Worse: the method **writes** to that registry (unregistering the task’s paths) but never **read** it as a liveness signal. It destroyed the very evidence that proved the planner alive. Under plan-in-place a card is specified while it sits in `todo`/`triage`, and `reapLeakedConcurrencySlots` treats both as reapable on a rationale written *before* planning moved there (“a task waiting to run must not pin a worktree”). Every gate ahead of the last one passes for a planner: | Gate | Saves a planner? | |---|---| | in `listWorktreeHolders()`? | **No** — `ensureTaskWorktreeForPlanning` → `ensureGraphCustomNodeWorktree` → `addActiveWorktree` (`executor.ts:8581`) | | reapable column? | **No** — plan-in-place keeps the card in `todo`/`triage` | | in the executor’s `executing` set? | **No** — a planner is triage-owned | | 60 s `LEAKED_WORKTREE_SLOT_GRACE_MS` | **No** — keyed on `columnMovedAt`, and planning routinely runs for minutes | So the broken guard decided alone. ## This is FN-8600 recurring through a second sweep That fix registered planning paths in the registry and taught the **self-owned-branch reclaim** sweep to consult `isPathActive`. The leaked-slot reaper never got the same signal — fixed at one surface, not enumerated across all. Exactly what the AGENTS.md Surface Enumeration rule exists to prevent. ## Fix The refusal now also fires when `activeSessionRegistry.pathsForTask(taskId)` is non-empty. Keyed on **any** registered path rather than on kind: the point is that a registered surface of any kind means someone is working in that worktree. ## Enumeration — the part that stops a third recurrence The guard is a **chokepoint**, so this covers every caller rather than just the reported one: - `reapLeakedConcurrencySlots` — the reported path - `recoverPausedAbortFailures` — **had the identical executor-only pre-gate** - the `preserveWorktrees: true` reclaim Audited the rest of self-healing’s liveness gates: the self-owned-branch reclaim, worktree-metadata reconcile and PR-branch sweeps already consult `isPathActive`/`lookupByPath`. The three that read only `getExecutingTaskIds` — `checkStuckBudget`, `recoverCompletedTasks`, `recoverStrandedCompletedTodoTasks` — move columns and never destroy a worktree, so they are noted rather than changed. ## Trade-off, stated plainly A leaked registry entry now blocks this sweep instead of a live planner losing its worktree. That is the strictly safer failure and the one the “last line of defense” wording already promises. The registry is process-local and in-memory, so a leak cannot outlive the process, and stale entries have their own reconciler. **A test pins that a genuine phantom — no executor surface AND no registration — still clears**, so this is not a blanket refusal that would trade this bug for a wedged queue. **The 60 s grace is deliberately unchanged.** Raising it would only make the bug rarer and harder to reproduce; the liveness gate was the defect. ## Verification Revert-proof, measured: removing the registry term turns **3 of the 4** new tests red, including the end-to-end sweep case (card in `triage`, past the grace, executor sets empty → asserts the slot is not reaped and the worktree survives). The 4th stays green both ways *by design* — it is the anti-overcorrection guard. `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (309 + 10 + 71) · new suite 4/4. The 2 failures in `self-healing.test.ts` / `-completion-fanout.test.ts` are **pre-existing** — identical with this change stashed. 🤖 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** * Prevented active planning worktrees from being mistakenly deleted or reclaimed while related planning sessions are still active. * Enhanced session liveness checks so phantom executor bindings are not cleared when a live session is registered. * Updated paused abort recovery to defer or abort safely when a live planning session is detected, avoiding unintended task/worktree mutations. * **Tests** * Added regression coverage for leaked-slot reaping, paused abort recovery behavior, phantom binding refusal, and end-to-end sweep outcomes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:04:51 -07:00
Phil Larson	72391c90b2	fix(engine): route workflow reviews through validator models (#2533) ... ## Summary - classify review-type workflow steps with the existing review-step classifier - resolve their primary, fallback, and thinking-level settings from the validator model lane - retain per-step model overrides and executor-purpose workflow-step tooling - keep ordinary workflow steps on the execution lane - make missing-fallback diagnostics identify the correct lane ## Why Code Review, Plan Review, verification, and inline-review gates were executed through the implementation model lane merely because they run inside `executeWorkflowStep()`. That defeats configured reviewer-model separation and can make the same model implement and validate its own work. This changes model selection—not the workflow-step session/tooling contract—so review steps remain executor-purpose sessions while using validator lane models. ## Verification - `FUSION_PG_TEST_SKIP=1 corepack pnpm@10.33.0 --filter @fusion/engine exec vitest run src/__tests__/executor-workflow-step-model.test.ts` — 14 passed - `corepack pnpm@10.33.0 --filter @fusion/engine typecheck` - `corepack pnpm@10.33.0 changeset status --since=origin/main` - `git diff --check origin/main...HEAD` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Review-type workflow steps now route through the configured validator model lane (instead of the execution lane). * Validator primary/fallback and thinking-level settings are applied correctly for review steps. * Step/task overrides still take priority over lane-based resolution. * Fallback retry sessions now use the appropriate validator/executor configuration, with lane-specific fallback guidance when fallback settings are missing. * **Tests** * Expanded executor workflow-step model resolution and routing/fallback precedence assertions for validator-lane behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-07-29 00:05:04 -07:00
gsxdsm	18d654a5ff	capacity, part 3: delete the globalMaxConcurrent setting, API and UI (#2529) ... Part 3 of the capacity simplification, and the half that removes the **knob**. Enforcement (shared semaphore, runtime wiring) went in #2509; this removes everything an operator or API client can still see, so nothing is left readable-but-ignored. ## Deleted Settings key + schema default · CentralCore’s `getGlobalConcurrencyState` / `updateGlobalConcurrency` / `acquireGlobalSlot` / `releaseGlobalSlot` and the `concurrency:changed` event · the whole Global Concurrency block in `async-central-core` · `PUT /api/global-concurrency` · the Scheduling · Global settings section · the footer and Command Center global sliders · the dead `getGlobalConcurrencyLimit` reader whose only caller went in #2509. ## Kept, deliberately **`GET /api/global-concurrency` survives as telemetry only** — live `currentlyActive` / `projectsActive` from CentralCore’s side-effect-safe source. “How busy is this machine?” is still a real question once the cap that used to answer it is gone. It no longer reports `globalMaxConcurrent`/`queuedCount`: those came from the deleted cap and from slot bookkeeping production code never incremented, so publishing them was publishing zeros dressed as state. **`useGlobalConcurrency` becomes read-only.** Everything that existed to *persist* went with the cap — the 500 ms debounce, the save-state machine, the commit-on-close/unmount flush, the slider clamp, the `interactive` gate. The module-level shared store is **kept**: its original justification (two mounted consumers drift apart with private copies) holds for a polled read exactly as it did for a cap, and one fetch now serves both. The live “N running (all projects)” readout survives in both surfaces, moved onto the per-project row. ## Two sections become one Scheduling · Global existed to host exactly one control. With it deleted the section renders an empty pane, so the Global/Project pair merges back into **“Scheduling”**. An empty nav entry is a promise of settings that are not there. ## One real fix found on the way `SchedulingSection`’s `concurrencyLoading` gated the **project** concurrency inputs on the **global**-concurrency fetch — never the right source, since `maxConcurrent` and `maxWorktrees` come from the settings form. It is repointed at the form’s own load, preserving the invariant it existed for: a concurrency input stays disabled until its live value arrives, so an operator cannot overwrite a resolved limit with a blank fallback. ## Migration A stored `globalMaxConcurrent` is **ignored** — it is a project-blob key nothing reads, so dropping it needs no schema change. The `central.global_concurrency` **table** is dropped in a follow-up; this slice stops seeding and reading it first, so that drop has no live writer to race. ## Verification, and how the wider suite was controlled `pnpm lint` clean · core/engine/dashboard `tsc` clean · `pnpm test:gate` green (309 + 10 + 71) · dashboard settings/footer/command-center/hooks **2237/2237** · core `central-core-backend` 9/9. The broader dashboard suite shows failures, and I checked rather than assumed: running the suspect files on **clean main** reproduces `api-git` (49), `TaskDetailModal.rendering` (28) and `settings-mobile` (17) identically. Two were genuinely mine — `SettingsModal.scheduling-merge` (0 on main, 17 on this branch: my nav rename) and one `settings-mobile` picker case asserting `scheduling` is a scoped pair — and both are fixed. Tests for deleted behaviour are removed with it (footer confirm/cancel/flush/dedupe, global marker geometry, the hook’s PUT case, the CentralCore slot cases), each carrying a note on what it guarded and where the surviving **project-side** equivalent lives. Fixture-only references were updated, not deleted. Nothing booted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 23:23:13 -07:00
gsxdsm	919f68f9bc	test(U9): cover the two unguarded merge safeguards and admit them to the gate (#2526) ... **U9, PR5.** Closes the gap #2520 measured. Tests + gate config only; no production behavior change. ## The gap #2520 found that safeguards **1 (user pause)** and **4 (capacity single-flight)** had **zero test coverage**. Deleting either guard produced no new failure anywhere in the merge, project-engine, self-healing, or concurrency suites. Both guards work correctly today — nothing would have noticed if they stopped. U9 moves merge behind graph nodes, so this is exactly the state not to convert on top of. ## Two tests - **`merge admission excludes a user-paused card`** — safeguard 1, the pause invariant re-ratified in #2486. Without the `paused || userPaused` filter, the admission provider offers a user-paused card to the merge pump. - **`drainMergeQueue is single-flight`** — safeguard 4. Asserted via `reconcileStaleMergeActive`, the first statement *inside* the guard, so the probe isolates the guard rather than dispatching a real merge. (Driving a real drain crashed the vitest worker; probing the guard directly is both safer and more precise.) **Both are two-sided** — they assert the guard blocks *and* permits. A one-sided test would still pass against a guard that rejects everything, which is a real failure mode for a filter. ## Proven by mutation delta Baseline fail-set vs mutated fail-set on the identical selection, NEW failures only: | Mutation | NEW failures | |---|---| | remove the pause filter | **1** — the pause test, and only it | | remove the single-flight guard | **1** — the single-flight test, and only it | | filter rejects *everything* | **1** — proves not one-sided | | drain *always* refuses | **1** — proves not one-sided | ## Gate admission `project-engine.test.ts` joins the `engine-core` allow-list. **One file proves five safeguards** — user pause, `autoMerge:false`, capacity single-flight, the pre-enqueue merge-proof consult, and at-most-once enqueue. Before this, **none of the six safeguards was defended by blocking CI**. A regression surfaced only in non-blocking full-suite, after the merge. Measured, not assumed: | | Files | Tests | Wall (3 runs) | |---|---|---|---| | before | 17 | 309 | 5.19 / 5.51 / 5.19s | | after | 18 | 412 | 6.19 / 6.24 / 6.21s | **+~1.0s against a ~60s ceiling.** **Verified the gate fires**, rather than assuming the allow-list edit took — the failure mode greptile caught in #2494: - remove safeguard 1 → `pnpm test:gate` **exits 1** (1 failed / 411 passed) - remove safeguard 4 → **exits 1** likewise - restored → **exits 0** Deterministic: store, runtime, merger and notifier all mocked; no real git, no network, no real timers in these two cases. ## Reversible calls I made rather than asking - **Added to `project-engine.test.ts` rather than a new file.** A dedicated file would need ~200 lines of duplicated `vi.mock` scaffolding; reusing the existing harness also means one gate admission covers five safeguards instead of two. - **Did not wait for U8.** These guard code that exists today and the conversion needs them in place first. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 22:20:04 -07:00
gsxdsm	e9bfd0d313	test(engine): prove the admission-control agent count on a renamed board — and two of my own cases were vacuous (#2516) ... Test-only. Fifth E2E family. Closes three of the four `live-agent-count` classifications. ## Why this one is a scheduler bug, not a display bug `live-agent-count.ts` classifies a card's column, and `persistedTopLevelAgentSlotsFromStore` turns that into **the number admission control compares against the cap**. So a mis-classified column fails in whichever direction hurts: | mis-classification | consequence | |---|---| | wip column not recognised | under-count → **over-admits past the operator's cap** | | complete column not recognised | a finished card counts forever → **board silently stalls** | Both are silent, and both land only on a renamed board. Everything in the path is real: PostgreSQL store, real persisted workflows, cards walked through the real transition policy, and the real counting function resolving each card's own IR. Nothing about counting is reimplemented here. ## Two of my own cases were vacuous — mutation-testing caught it This is the more useful half of the PR. **1. "does not count a card in the COMPLETE column" passed with the terminal classification hardcoded to `done`.** `isRunningAgentTask` rejects that card at the *wip* check anyway, so the test was really asserting "shipped isn't a wip column". `terminalKind` short-circuits **first**, so it only changes the answer for a card whose status would otherwise make it count. Now covered by a complete card carrying a live-looking `planning` status — the state a crashed run leaves behind, which on a renamed board consumes a slot forever. **2. The review/merge lane had no case at all.** A review status is deliberately *not* globally live (a stale `fixing` in wip must not consume capacity), so it is gated on `columnIsReviewOrMerge`. If the renamed review lane isn't recognised, a genuinely-active reviewer stops counting and admission control lets another agent in over the cap. Now covered by a review card with an active merge-pipeline status. Both new cases assert their fixture took effect first, so they can't degrade back into the weaker version silently. ## Mutation-verified independently | classification | mutation | result | |---|---|---| | `countsTowardWip` | → `"in-progress"` | 3 renamed cases fail | | `complete` | → `id === "done"` | exactly the new terminal case fails | | `mergeBlocker` | → `id === "in-review"` | exactly the new review case fails | ## What this does NOT cover, stated plainly The fourth classification, `columnIsIntakeOrHold`, is read only by the **waiting** predicate, which the admission count never calls. It stays in the ledger as unproven rather than being claimed by proximity — the mistake I made last slice with `resolveMergeOrchestrationColumn`. ## Verification - five live-E2E suites green together: **52/52** - engine `tsc --noEmit` clean - `pnpm test:gate` green (309 + 10 + 71) 🤖 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 for live agent-count admission across workflow lanes and lifecycle states. * Verified slot handling for active, completed, held, and mid-review tasks, including stale statuses. * Confirmed mixed-lane counts and renamed board vocabularies produce consistent results. * **Documentation** * Expanded coverage notes for live agent-count classifications and waiting-state behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 22:19:22 -07:00
gsxdsm	2e39763930	test(engine): prove agent-link hygiene on a renamed board — a leaked agent slot, not a stale link (#2514) ... Stacked on #2510. Test-only. Closes the `task-agent-sync` ledger entry. ## The defect this reproduces, in the code's own words `task-agent-sync.ts`'s conversion note: > a move into a renamed terminal column matched nothing and this handler returned early — so the agent kept a `taskId` pointing at a finished card and stayed `running`, **with no error and no failing test**. "No error and no failing test" is the whole problem — and the cost is not a stale link. **The scheduler counts `running` agents against its cap**, so on a renamed board every completed task permanently consumes an agent slot until a human notices. A board would just get slower and slower. ## Everything in the path is real Real PostgreSQL `TaskStore`, real `AgentStore`, the real `attachAgentLinkSync` subscribed to the store's real `task:moved` event (the same call `in-process-runtime` makes), and a real `moveTask` to trigger it. Assertions read the **agent row** back out of the store — never "the handler was called". ## Mutation-verified Forcing the legacy literal sets (the pre-conversion behavior) fails **exactly the two renamed cases**, leaving the default-vocabulary floor and both negatives green. So this reproduces the original defect rather than merely covering the file. ## Negative half An ordinary mid-lifecycle move (`wip → review`) must **not** release the agent — given the same time to run as the positive case. "Clear the link whenever the card moves" would drop the binding the moment work started, a louder failure than the leak it fixes. ## Two anti-flake, anti-vacuity details - **Async delivery.** `task:moved` is a plain EventEmitter and the handler is async, so the assertions **poll the persisted row** to a bounded deadline and fail with the row's actual contents. A fixed sleep would flake in both directions. - **The fixture asserts itself.** The link and `running` state are verified *before* the move, so an agent that was never linked cannot make this pass for the wrong reason — the failure mode I hit twice already in this program. ## Verification - four live-E2E suites green together: **39/39** - engine `tsc --noEmit` clean - `pnpm test:gate` green (307 + 10 + 71) 🤖 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 agent-link cleanup when tasks reach workflow completion. * Ensured completed task links are released even when workflow columns have been renamed. * Preserved active agent links when tasks move through non-terminal workflow stages. * Improved reporting of link cleanup outcomes and handling of synchronization errors. * **Tests** * Added live PostgreSQL end-to-end coverage for completion, in-progress moves, and renamed-column scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 18:52:59 -07:00
gsxdsm	3badc244a7	U12 part 2: bind the three U5 reconciliation guards — USER-VISIBLE (and one path that couldn't run under PostgreSQL at all) (#2512) ... ## U12 part 2 — the three U5 reconciliation guards now actually fire USER-VISIBLE. Taken on standing authority; here is exactly what changed for operators. All three read the RAW `experimentalFeatures.workflowColumns` key via `store.workflowColumnsFlagOn()`. Nothing in production writes it, so all three have been inert since the workflow-columns cutover. | Guard | Before (every real project) | After | |---|---|---| | Workflow edit removing an **occupied** column | Save succeeded; cards left in a column the workflow no longer declares | Save fails with `OccupiedColumnsError` unless `rehomeTo` is supplied | | Workflow **delete** | Occupant capture returned `[]`; cards sat in the deleted workflow's columns until the next engine start | Cards move to the default workflow's entry column as part of the delete | | Workflow **switch** | Never reconciled; the `reconciliation` field in the declared return type was never populated | Card in an undeclared column moves to the resolved target; a declared column is preserved | Both consumers already handle the new outcomes and needed no change: `register-workflow-routes.ts` maps `OccupiedColumnsError` to a structured 409 carrying per-column occupant counts, and `fn_workflow_update` returns a retryable structured result. The dashboard editor's `rehomeTo` retry flow becomes reachable for the first time. I only updated two stale "flag-ON" comments there — that code was correct all along and simply never fired. ### What an operator actually sees (USER-VISIBLE — read this bit) Four changes to what the board and the API do. Nothing here is silent. 1. **Editing a workflow to remove a column that has cards in it now FAILS.** Previously the save succeeded and the cards were left in a column their workflow no longer declared. The dashboard shows the existing 409 with per-column occupant counts and prompts for a re-home target; retrying with `rehomeTo` moves the cards and saves. Removing an EMPTY column is unaffected. 2. **Deleting a workflow moves its cards immediately** to the default workflow's entry column, instead of leaving them until the next engine start. 3. **Switching a task's workflow moves the card** when the new workflow does not declare its current column. A card whose column IS declared stays exactly where it is. The API response now carries the `reconciliation` summary it always promised. 4. **A switch whose re-home would be REJECTED is now refused before anything is written.** If the destination column is at its WIP limit, the switch fails with a structured 409 (`workflow-switch-rehome-failed`) naming the task, both columns and the reason — and **nothing changes**: the task keeps its current workflow AND its current column. Retry after making room. Previously this combination committed the selection and then silently reported a move that never happened, leaving selection and column disagreeing. **Can a torn card still happen? Yes, in one narrow case, and here is how you recover.** If the destination fills in the window between the pre-flight and the move, the selection is already committed and the card ends up in a column its new workflow does not declare. That case is not silent: it writes a `task:workflow-switch-torn` run-audit row, and the error carries `selectionCommitted: true` with both columns. Recovery: make room in the destination and move the card there, or switch the task back — and if neither happens, the R7 startup sweep `reconcileUndeclaredTaskColumns` re-homes it on the next engine start. The card is never lost; it is visible in a lane the board may not draw until one of those runs. The one thing to watch after merge: (1) converts a previously-silent success into a visible failure, so an operator mid-edit on a busy workflow will start seeing a 409 they never saw before. That is the point — the alternative was stranding their cards — but it is the change most likely to generate a "this used to work" report. ### The thing that made this more than a gate removal Un-gating the switch guard surfaced that `selectTaskWorkflowAndReconcileImpl` read the task through `store.readTaskFromDb` — the **synchronous SQLite** reader, which throws under PostgreSQL: ``` TaskStore.db: SQLite Database is not available in backend mode ``` The flag returned before that line, so the gate was hiding a path that **could not execute at all in the production backend**, not merely a disabled feature. Ported to the async `readTaskRow`. Found by the new tests, not by reading the code. ### Review round 2 (both findings real, both fixed) **Torn write with no alarm — fixed by ORDERING, not by a louder message.** My first attempt only made the error loud, which left the torn state intact. The real fix is that the deterministic rejection cause (destination at its WIP limit) is now checked BEFORE `selectTaskWorkflow` commits, by resolving the target IR straight from `workflowId` instead of through the task's selection. Nothing commits on that path. For the residual race the failure is loud AND recorded: `rehomeOccupant` now returns `{ moved, error? }` (additive; sweep callers ignore it), the switch writes a `task:workflow-switch-torn` run-audit row, and throws `WorkflowSwitchRehomeFailedError` with `committed: true`. Consumers translate it: the dashboard route returns a structured 409 with `selectionCommitted`, and `fn_task_set_workflow` returns the same fields — no more generic "something went wrong". **Fabricated column for a deleted task.** My first fix fell back to `fromColumn` when the final read found no row, so a task soft-deleted mid-switch was reported as having its old column *preserved*. Absent now reads as absent (the optional `reconciliation` is omitted). Extracted as the pure `buildSwitchReconciliation` seam because the window is not reachable through the public call — `selectTaskWorkflow` rejects an already-deleted task up front — so it is a genuine race, and I test the decision directly rather than asserting it from reading the code. ### Revert-proof, measured New `workflow-reconciliation-production-shape.pg.test.ts` — 6 cases, with the flag **never written**, which is the configuration every real project has. Each flip reverted individually: - re-gate the edit guard → **2 failures** (OccupiedColumnsError case; rehomeTo re-home case) - re-gate the delete capture → **1 failure** (card stays in `custom-hold`) - restore the switch early return → **2 failures** (`reconciliation` undefined; card does not move) - all three in place → **6/6 green** Round-2 fixes, also measured: - restore the `fromColumn` fallback → the "row is gone" case fails (reports `preserved: true` for a deleted task) - drop the `!outcome.moved` throw → the capacity-blocked case fails (resolves instead of raising) - **move the capacity pre-flight back AFTER the commit → the case fails on the SELECTION assertion** (expected `WF-002`, received `WF-001`), i.e. it proves the ordering, not the wording The pre-existing coverage in `workflow-authoritative-reads.pg.test.ts` reached the occupied-column guard by **writing the flag ON itself** — same pattern as the ListView/Board suites in part 1. Its flag write is removed; it now runs in the production shape. ### Where I nearly got this wrong My first revert harness was buggy and I briefly concluded the delete re-home was **redundant** — I had probed the stored column and seen `triage` with what I thought was the flip reverted. It wasn't. `workflow-ops.ts` contains two identical `const occupantTaskIds = await store.listWorkflowOccupantTaskIds(id, false)` lines (field-reconcile block, delete path), so my first-match edit reverted the wrong one. Re-run anchored on surrounding context, the delete case fails as predicted. Recorded in the test header as a caution. I also chased and **refuted** a scarier hypothesis along the way — that an unrelated `updateTask` coerces a custom column back to `triage`. It does not; the column survives. ### Deliberately NOT in this PR The v1-IR rollback-compat persistence (`downgradeIrToV1IfPure`) on the workflow UPDATE path. It shared the same `flagOn` variable, which is how it surfaced: **one flag read was feeding two unrelated decisions, so the flag has more decision sites than call sites** — my earlier 9-site inventory undercounted. It chooses the stored *shape* of the graph rather than gating a guard, so it is a persistence-format change with a different blast radius. It now reads the flag explicitly, behaviour unchanged, for a follow-up. The `moves.ts` group remains U2b's. ### Verification `pnpm test:gate` (307 + 10 + 71), `pnpm lint`, `pnpm verify:fast` (17 steps), both typechecks green. Full `packages/core` PostgreSQL suite: **1042 passed, 3 failed** — `central-archive-secrets.test.ts` (log-prefix assertion) and `workflow-settings-project-identity.pg.test.ts` (×2, project-id resolution). I confirmed the identical 3 failures on a stashed clean tree: pre-existing, unrelated. No Fusion instance booted. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Workflow edits now prevent removal of occupied columns unless cards are moved to a specified destination. * Cards are automatically re-homed when workflows are deleted or switched. * Workflow switches now check destination capacity before committing and provide clear conflict details when re-homing fails. * Reconciliation results now indicate whether cards were moved or preserved. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 18:52:46 -07:00
gsxdsm	3bbb6ffc6b	capacity, part 2: delete the cross-project concurrency cap (enforcement half) (#2509) ... Stacked on #2502 — review that first; this branch contains its three commits. Operator: two capacities **per project**. `globalMaxConcurrent` is a machine-wide *third* limiter kept in a separate authority (a central-DB singleton row) that every runtime had to subscribe to and periodically re-reconcile. It goes. This slice removes **enforcement and wiring only**. The setting key, central DB state, API route and Settings UI come out in part 3, so each half lands green and independently revertable. **Deleted:** the shared `AgentSemaphore` instance in `ProjectManager` and `ProjectEngineManager`; the per-project `ScopedAgentSemaphore` in `InProcessRuntime`; the `globalSemaphore` runtime-config field; both `concurrency:changed` subscriptions; ProjectManager’s 30s limit-refresh poll; the residual-slot return on project stop. The scheduler/triage semaphore gate is now simply **absent** — same shape as the worktrees-off gate in #2502. `semaphoreGate?` was already optional, so no gate object is constructed rather than one holding an infinite limit. Absence cannot start binding again by accident. --- ## Two findings that changed the shape of this slice **1. `AgentSemaphore` the class stays — my earlier estimate was wrong and I withdraw it.** I previously told the coordinator that ~75% of `concurrency.ts` (≈662 of 886 lines) was semaphore machinery that could go with this cap. That was line-range arithmetic, and it was wrong. `AgentSemaphore` is a general primitive with four consumers unrelated to the global cap: | Consumer | Governs | |---|---| | `verification-concurrency.ts` | `maxConcurrentVerifications` | | `research-orchestrator.ts` | research `maxConcurrentRuns` | | `experiment-executor.ts` | `maxConcurrentExperiments` — **a knob absent from my original inventory** | | `step-session-executor.ts` | parallel workflow steps | What goes is the global **instance** and its wiring, not the class. I will report the measured `concurrency.ts` delta after part 3 rather than repeat an estimate. **2. `acquireGlobalSlot` / `releaseGlobalSlot` had no production callers — only tests.** So the cross-project cap had *two* mechanisms: the in-memory semaphore (live) and a durable central-DB `currentlyActive` counter (dead — never incremented by real work). Both deleted, along with the tests that pinned the dead passthrough. ## The regression this almost introduced `runWithMergeAdmission` in `project-engine.ts` opened with: ```ts if (!semaphore) return await start(); ``` Unreachable while a global semaphore always existed. With the semaphore gone it would have fired on **every** merge and skipped `projectAdmissionCoordinator.admitOldest` entirely — silently stopping merges from counting against the **per-project** agent count. That is the opposite of the intent: a merge *is* an agent and still consumes one of the project’s slots; it just no longer consumes a machine-wide one. So the early return is **deleted rather than left to fire**. `admitOldest` already declares `semaphore` as optional and enforces `maxConcurrent` independently of it (`claimed() + reservations >= maxConcurrent`), so dropping the argument preserves per-project admission and oldest-first fairness exactly. Worth flagging as a pattern: this is the third time in this unit that a branch which was *unreachable* became *always-taken* once a limiter was removed. The type system caught the worktree one; this one was only visible by reading the branch, because the semaphore was reached through an `any` cast. ## Verification `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (309 + 10 + 71) · project-manager + hybrid-executor + merge-single-flight + scheduler 93/93. Nothing booted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 18:52:39 -07:00
gsxdsm	743df98aa4	capacity, part 1: merge pinned at 1, worktrees-off mode, and one dead knob deleted (#2502) ... First slice of the capacity simplification. Operator: *"just have two capacity — overall per project agent count and max worktrees. Remove all other capacities and counts."* Plus two later additions: **merge is always 1, fixed**, and **worktrees off ⇒ limit by total agents only**. Three independently revertable commits. No limiter is added anywhere; one is deleted, one is made structurally absent, and one is pinned. --- ## 1. Merge concurrency ratcheted at 1 (test-only) I was asked to add a limiter if merge concurrency could be raised. **It cannot** — there is no setting, workflow property, pool or trait config anywhere that raises it, so this adds no code and pins what already holds. Serialization lives in the **pump**: `drainMergeQueue`’s `mergeRunning` re-entrancy latch, `activeMergeTaskId` as a single-slot identity, the `mergeBodyInFlight` next-generation latch, and one `ProjectEngine` per projectId. **Not** in the merge-queue lease, which is a per-task ROW (`primaryKey [projectId, taskId]`) — two tasks can hold leases simultaneously by construction, and it has exactly one caller (the worktree-reuse handoff). Ordinary merges never take it. A lease-level test would have been describing an invariant that layer has never held. The second half guards the other direction: a merge-concurrency *setting* would not fail the pump ratchet — it would sit unread until someone wired it up. **Revert-proof:** deleting the latch → `expected 1 times, but got 2 times`; deleting the `finally` → latch-stuck; injecting `maxConcurrentMerges: 2` → fails naming the key; injecting a `maxParallelLanes` merge-trait field → fails naming the field. Sources restored byte-identical after each injection. ## 2. `worktreesEnabled` — off means the worktree limit cannot bind No worktrees-off mode existed (no `worktreesEnabled`/`useWorktrees`/`worktreeMode` anywhere — only worktree *configuration*). **Why not `maxWorktrees: 0`, which needs no new key:** it deadlocks. `?? 4` keeps `0` (not nullish), the gate is `used >= limit`, so `0 >= 0` holds **on an empty board** and nothing ever dispatches — while the operator-visible reason reads `gate=maxWorktrees; used=0/0`, a limiter that looks like it is working while the board is dead. It also needs the Command Center `{min:1}` clamp relaxed. So `0` costs the gate rewrite *and* the clamp change *and* encodes a mode as a magic value. **Off is absence, not a big number.** `resolveWorktreeCapacityLimit` returns `number | null`; `ConcurrencyGateDiagnostic.maxWorktreesGate` is now optional, so consulting a worktree limit in OFF mode does not type-check. A gate holding `Infinity` can start binding again the moment someone "fixes" a comparison; an absent gate cannot. That paid for itself immediately: making it nullable surfaced a **second, independent** worktree gate (`activeWorktrees >= maxWorktrees` early-return) that a skip-by-convention approach would have missed silently. **Scope, deliberately:** this is a statement about *counting*, not isolation. It does not make concurrent agents safe to share one checkout and builds nothing toward that — the non-worktree paths that exist today are fallbacks to the operator’s own tree, one of which caused FN-8600. **Revert-proof:** a resolver ignoring the flag turns both OFF scheduler tests red while every ON test stays green — they reuse the *same* fixture (5 in-progress, limit 4) that pre-existing tests prove blocks, so the pair moves in opposite directions. Removing `disabled:` reddens the UI test. ## 3. `maxTriageConcurrent` deleted — it controlled nothing **Measured: zero enforcement reads.** The only `.maxTriageConcurrent` reference in the repo was a route echoing it back in `/config`. FN-8453 removed the pool it gated and left the knob shipping in `DEFAULT_SETTINGS`, the settings type, the section registry, the API response and six i18n catalogs, doing nothing, for releases. Historical FNXC comments are **updated, not deleted** — they explain a real past incident; they now say "planning admission slot" so they stop implying a live setting. Tombstoned so it cannot return. `/config` loses a field; safe in-repo since `fetchConfig`’s own return type never declared it. --- ## Two corrections worth recording - I earlier reported `maxWorktrees` had **no** Settings UI. Wrong — `WorktreesSection.tsx:47`; my grep was truncated by `head`. It changed the placement (toggle beside it, rather than a duplicate key in Scheduling). - I planned to assert the queued-reason string is rewritten in OFF mode. Measured that it is **unreachable**: when `maxConcurrent` binds, the sweep bails before the per-task reason and logs nothing. The test asserts absence instead. Two near-misses caught before commit: a pre-existing FN-7505 guard caught my *new* key missing a description mapping; and editing i18n via `json.load/dump` silently dropped unrelated duplicate keys (`autoUpdateAndRestart` in `fr`) — Python keeps only the last of a duplicated key. Redone textually, every catalog re-validated. ## Verification `pnpm lint` clean · core/engine/dashboard/i18n typecheck clean · `pnpm test:gate` green (309 + 10 + 71) · capacity/worktree suites 11/11 · engine merge-invariant + scheduler 45/45 · dashboard settings 114/114. Rebased onto current main and re-verified. Nothing was booted at any point. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a project setting to enable or disable running tasks in worktrees. * Disabling worktrees removes worktree capacity limits from task scheduling. * The “Max Worktrees” setting is disabled when worktree execution is turned off. * **Changes** * Removed the unused triage concurrency setting from configuration and dashboard responses. * Updated scheduling diagnostics and queue messages to reflect disabled worktree capacity limits. * Added localized labels and help text for the new setting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 18:52:31 -07:00
gsxdsm	35b0df1838	U11 PR2: entry contract under the merged column + a real intake-column bug the audit surfaced (#2503) ... Second small PR for **U11**. Two commits: a tests-only entry-contract pin, then a **real present-day bug fix** the audit surfaced. ## The audit you asked for, finished — no design fork You named four surfaces as the remaining risk. All four can take a combined `intake` + `hold` column. One needed a code change; here it is. | Surface | Verdict | Evidence | |---|---|---| | `isUnplannedForExecution` | Safe | PR1 (#2495) — passed unmodified; a mutation now fails exactly the merged-column test | | Capacity hold / release | Safe | PR1 — `hold-release.ts:260` already accepts intake **or** hold | | `start`'s column / entry contract | Safe | commit 1 — all 6 assertions passed unmodified | | `createTask` intake wiring | **Broken today** | commit 2 — fixed, revert-proven | | *(also found)* triage auto-discovery | Needs conversion | `triage.ts:1382` — deferred to PR3, see below | ## Commit 1 — entry contract under the merged column (tests only) All 6 new assertions passed on the first run. **Regression floor, not evidence of a fix** — I could not make them fail and am not claiming otherwise. They pin one real behavioral **difference** rather than asserting sameness everywhere: the merged shape answers `start` where the split shape answers `plan`, because `start` becomes the first node in that column once the columns collapse. That is equivalent *only* because `start` reaches the specification node by a single unconditional success edge — asserted, so if a node is ever inserted between them this fails instead of silently admitting an unspecified card into implementation. Also pinned: past planning both shapes agree exactly; a card past the merged column still never resumes at a planning node (the backward drag that fires `abort-on-exit`); and a row persisted in the **deleted** `triage` column resolves to `undefined`, safe only while the executor's start-node fallback exists. ## Commit 2 — a real bug, found by the audit The intake column was resolved **only** as a by-product of materializing workflow steps. A create supplying `enabledWorkflowSteps` without an explicit `workflowId` takes **neither** materialization branch, so `resolvedEntryColumn` stays `undefined` and `column:` falls through to the hard-coded `|| "triage"`. Today, on Coding (Ideas), that lands the card in `triage` — **a column that workflow does not declare.** Created straight into a phantom lane. Measured: the new test fails `expected 'triage' to be 'ideas'` against unmodified sources. **Why it blocks U11.** Once `triage` leaves the coding IRs this stops being an Ideas edge case and becomes the default workflow's behavior for every create down this path: the card lands in an undeclared column **and** — because `isIntakeColumn` keys on the same `"triage"` literal — gets `generateSpecifiedPrompt` instead of the bootstrap seed. Triage admits a card for planning only when its `PROMPT.md` reads as a seed, so a placeholder spec is classified "already planned" and never planned. The card sits in Planning forever with no log line in any lane — **FN-8587's exact failure mode, promoted from one edge case to every new card.** The fix resolves the intake column **side-effect-free** (read the IR, ask which column carries `intake`). It deliberately does *not* call `materializeDefaultWorkflowSteps`, which would persist step rows the caller explicitly opted out of by supplying its own toggles. Unresolvable workflow returns `undefined` and each call site keeps its legacy fallback, so no path loses behavior when the IR cannot be read. Applied to both create paths. Branch ordering preserved in both — the explicit empty-toggle case (`length === 0` hydrating back as `[]`) still runs, now nested rather than sequential. **Revert check:** with `task-creation.ts` reverted, *"lands a Coding (Ideas) task in ideas even when enabledWorkflowSteps is supplied"* fails `expected 'triage' to be 'ideas'`. The companion bootstrap-`PROMPT.md` assertion passes either way today — it is correct **by accident of the `"triage"` literal** — and is kept precisely because that accident disappears with U11. ## Verification 37 tests green across the three intake/create suites; 119 across the entry-contract, merged-column and lifecycle suites; `pnpm test:gate` green (307 + 10 + 71); lint and core typecheck clean. Changeset added. ## Deferred to PR3, with the line numbers `discoverReadyPlanningTasks` has two hardcoded branches: ```ts (t) => t.column === "triage" && isTaskStillInPlanningStage(t) // triage.ts:1382 (t) => t.column === "todo" && !this.processing.has(t.id) … // triage.ts:1389 ``` Delete `triage` and branch 1 matches nothing for coding cards; branch 2 then does all the work and is **narrower** (it admits only `needs-replan` or bootstrap-stub cards). Commit 2 is what makes branch 2 sufficient — every new card now gets a real bootstrap seed. They cannot double-fire: a card is in `todo` xor `triage`. Two adjacent sites are already merged-shape-ready: `triage.ts:3899` skips the redundant same-column move for a plan-in-place card, and `triage.ts:753`'s stale-status sweep already scans both columns. Then the ~10-line IR change, then the migration proof. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:35:14 -07:00
gsxdsm	9d3e53d0c5	U8 PR3: the implementation phase announces HOW it ended — including when the executor moved the card itself (#2507) ... Third PR of **U8 — the graph owns execution**. Independent of everything merged so far; small, green, revertable on its own. ## The problem this makes visible `result.taskDone` is the entire language the execute seam has for talking to the graph: ```ts if (result.taskDone) return { outcome: "success", value: "implemented" }; return { outcome: "failure", value: paused ? "implementation-paused" : "implementation-incomplete" }; ``` The endings that one bit cannot express are exactly the ones the implementation phase **transitions itself**: - a session that paused *after* the work was already complete → finalizes to review inline; - a session that stopped because a step is blocked on a pending review → hands off to review inline (a pending-review block is a wait, not a failure; marking it failed deadlocks a row that is both `in-review` and `failed`). The graph then sees `taskDone === false`, reports `implementation-incomplete`, and `handleGraphFailure` compensates with `alreadyFinalizedToReview` / `completionFinalized` — classifiers whose entire job is recognising a move the graph did not make. **That was invisible.** An out-of-band transition and a genuine implementation failure were indistinguishable in logs, in events, and in tests. You cannot remove a transition you cannot see, and you cannot prove you removed it either. ## What lands A closed `ImplementationExit` enum (`engine/executor/implementation-exit.ts`) reported from six completion-adjacent exits in `runImplementation`, announced by the execute seam as `NodeCompleted.exit` on the U3 lifecycle bus. Two ids are flagged as out-of-band — the ones where the executor, not the graph, performs the transition. **Routing is unchanged, and that is the point.** The seam returns byte-identically what it returned before for every exit, so this PR cannot move a card. The routing move needs new IR edges and lands separately; splitting them is what keeps both independently revertable. Per R5 an exit id is a **reaction** — nothing branches on one, and dropping every subscriber must change no outcome (a named U8 test scenario, asserted here). `NodeCompleted.exit` is added to the event key allow-list deliberately — which is exactly what that allow-list is for — and carries closed enum ids only, never prose. ## Revert-proofs, each observed failing | Injected change | Result | |---|---| | Remove the emit entirely | **6 failures** | | Let an exit change the returned outcome | **2 failures** (the routing-unchanged pins) | | Delete one `reportImplementationExit(...)` call site | **1 failure** (the wiring ratchet) | **The third proof exists because of a hole I found in my own tests.** These tests stub `runImplementationPhase` — the only way to reach all six exits deterministically — which means deleting a real call site left the entire file **green**. A stubbed seam can only prove the seam. I'd also written "every exit is reported — the signal is real, not a placeholder" in the header, which the tests did not support. Both are fixed: there is now a ratchet asserting every enum id is wired at a real call site and that each out-of-band id sits adjacent to the handoff it describes, and the header says what the tests actually prove. ## Scope **6 of `runImplementation`'s ~28 dispositions** (per the ownership ledger merged in #2490), chosen as the ones the routing move needs. The remaining ~22 report nothing yet — the ledger, not this enum, stays the record of that gap, and the module says so. ## Verification - 15 new tests + ledger + graph-boundary + task-done-blocked + graph-requeue-gate + step-session + review-verdicts + tool-failure-retry — **9 files, 115 tests green** - `@fusion/core` `workflow-events` — 20 tests green (allow-list change covered) - `pnpm test:gate` green (17/307, 2/10, 1/71); `pnpm lint` clean; `tsc --noEmit` clean on both packages - Changeset included (`patch`, `internal`), passes `check:changesets` ## Next PR4 is the routing move itself: `review-handoff-pending-review` becomes a graph outcome with its own IR edge, and `alreadyFinalizedToReview` becomes provably unreachable for that path. The IR edge change will be its own commit, separate from the seam change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:35:07 -07:00
gsxdsm	d5030c55ea	test(engine): prove BOTH rebound paths on a renamed board — 2 more ledger entries closed (#2510) ... Stacked on #2508. Test-only. ## Why this site matters more than most `resolveReboundTarget` answers one question: **where does a recovered card go back to?** Keyed on the literal `todo`, a recovered card on a renamed board is requeued to a column that board **does not declare**. That is not cosmetic — an undeclared column carries no trait flags, so `findColumn` returns undefined and the card becomes invisible to every trait-driven sweep: nothing schedules it, nothing releases it, the board does not draw the column. **The "recovery" strands the card harder than the failure it was recovering from.** One of the two covered paths, `reconcileUndeclaredTaskColumns`, exists *specifically* to repair that state — which makes it the worst possible place for this bug to live. ## Covered, each mutation-verified independently | site | mutation | result | |---|---|---| | `reconcileUndeclaredTaskColumns` | target → `"todo"` | exactly the 2 renamed cases fail | | `autoRecoverWorktreeSessionStartFailure` | rebound → `"todo"` | exactly the renamed requeue fails | Neither needs git — the corrected ledger lens from #2508 (*what the function touches*, not *what family it sits in*) made that obvious rather than assumed. ## Both negatives included "Re-home anything whose column looks wrong" would be a louder failure than the strand it repairs, so: a card whose column **is** declared is left alone, and an operator `userPaused` park is never undone. ## Fixture finding, kept in-file `updateTask({ userPaused: true })` leaves the field `undefined` on both `getTask` and `listTasks({slim:true})`. Seeding it that way produced a card the sweep **correctly** saw as unpaused — a broken fixture that would have read as a broken guard, and would have looked like a real safety hole in the paused-park protection. Found by probing the persisted row rather than trusting the write. Now seeded through the integer column directly, and the test asserts the seed took effect *before* exercising the sweep, so this cannot silently regress into a vacuous pass. ## Verification - three live-E2E suites green together (lifecycle, merge-family, rebound-family) - engine `tsc --noEmit` clean - `pnpm test:gate` green (307 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:35:01 -07:00
gsxdsm	4eaa509024	test(engine): prove merge finalization on a renamed board — 3 ledger entries closed, and the ledger itself corrected (#2508) ... Test-only. Closes three `auto-merge-finalization` entries from the unproven-sites ledger. ## Why this one first It is the **last move a card makes**. Keyed on the literal `done`, a renamed board's proven-merged card is moved to a column its own workflow does not declare — or refused and left stranded in review **with the work already landed**. That is the most expensive failure shape in the lifecycle, and nothing had run it against a renamed workflow. ## The ledger was wrong, and is corrected in this PR My own ledger said this family *"needs a REAL git worktree, branch, and squash … an engine-slow real-git lane, not another table row"*. `finalizeProvenAutoMergeTask` **needs no git at all** — the merge proof is a field on the row. It was reachable the whole time. The inference came from *the family the code sits in* rather than from what the function actually touches, and it parked reachable coverage for a slice. The correction is written into the ledger so the remaining entries get re-checked the same way rather than inheriting the assumption. ## A second correction, from mutation-testing rather than reading I first claimed `resolveMergeOrchestrationColumn` as covered because it sits in the same resolver as the other two. **All cases passed with it hardcoded.** It changes only whether finalization records a column-mismatch *repair* — never where the card lands, which is why the other cases are blind to it. It got its own case. Keyed on `in-review`, a renamed board's card resting in `checking` compares unequal, so **every ordinary finalization would be audited as repairing a mismatch that never existed** — a healthy board reads as one constantly self-healing, and the audit trail operators use to spot real strandings fills with false positives. Sitting next to covered code is not coverage. ## Mutation-verified independently | mutation | result | |---|---| | `completeColumn` → `"done"` | 3 fail — both renamed cases + the differential | | `isCompleteColumn` → `id === "done"` | exactly the already-done case fails | | `mergeColumn` → `"in-review"` | exactly the new audit case fails | ## Shared fixture extracted (pure move) The vocabulary + IR builder moved to `_workflow-vocabulary-fixture.ts` so the two suites cannot drift into testing different workflows — two copies of a differential fixture is precisely how a renamed-workflow test starts passing for reasons unrelated to the code under test. The lifecycle suite is unchanged: **20/20 before and after**. The `mergeOrchestration` trait is an opt-in option so the existing suite's IR stays byte-identical. ## Fixture note worth keeping Seeding needed **completed steps**: task creation parses three pending steps out of the bootstrap PROMPT even with `applyDefaultWorkflowSteps: false`, and `getTaskHardMergeBlocker` refuses on them (`"task has incomplete steps"`). Found by the suite blocking on **both** vocabularies — the signature of a broken fixture rather than a broken guard. ## Verification - 51/51 across the lifecycle, merge-family, ratchet and hold-release suites - engine `tsc --noEmit` clean - `pnpm test:gate` green (307 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:15:35 -07:00
gsxdsm	8288e4a8ab	U7 PR3: the specification reaction acts on what finalize DID, not on the fact that planning stopped (#2506) ... Completes the pair started in #2498. That landed the outcome; this makes the engine's reaction consume it. ## The bug `onSpecifyComplete` fired on **every** finished specification, because the seam announcing it fired unconditionally. So a card parked at the manual plan-approval gate — finalize writes `status: "awaiting-approval"` and **returns early**, before the release move — was logged as `Specified X → todo` and had a Plan Review run armed for a plan the operator had not approved. #2491 stopped the **seeder** from acting on that, defensively, at the seeder. This removes the reason it was ever asked. Both layers are deliberate and neither is redundant: - the seeder guard covers **every caller**, including self-healing's re-seed; - this one stops the engine doing work nobody asked for, and stops it telling the operator something false about their own board. `released` is the only outcome that licenses arming a run — the only one meaning the card crossed into the hold column (or was already resting there, plan-in-place) and is the graph's now. `parked` belongs to a human; `withheld` belongs to the caller's retry budget. ## The event still fires on every outcome Deliberately. Dropping the reaction for a non-release would also drop the runtime's `recordActivity()` idle signal, and a reaction that silently does not happen is harder to reason about than one that happens with an accurate payload. R5's division of labour: **the seam announces, the subscriber decides what a given outcome licenses.** ## Why there is a new extracted function `reactToSpecificationComplete` is pulled out of the inline `InProcessRuntime` callback for the same reason the continuation drain was in #2491: the callback is built inside a class whose construction attaches to the real central project registry, so no test could distinguish *"the reaction respects the outcome"* from *"the reaction ignores it"*. **Revert proof:** with the outcome gate removed from the reaction, **5 of 8 fail**. ## Two call-site decisions worth naming **`tryFinalizeExplicitDuplicateMarker` reports through a mutable ref, not a widened return type.** Its boolean answers a *different* question — "was this a duplicate marker at all?" — and 16 existing tests assert it directly. I tried the widened return first and it turned all 16 red. Expectation edits are exactly how a behavior change travels disguised as churn, so I backed it out. **This diff touches zero existing test expectations.** **A duplicate-marker redirect reports `parked`**, which is accurate: it deletes, flags, or clears the marker; it never releases the card into the hold column. ## A fixture note — third of this shape on the program My "task vanished between release and reaction" case passed `undefined`, which triggered the harness **default parameter** and silently handed the reaction a live task — making it a duplicate of the control rather than the case it claimed to be. It now passes `null`, with a comment saying why. Running tally of near-false-greens on this unit, all the same family: a fake that ignores its predicate (#2491), a stub that ignores its callback (#2498), a default parameter that swallows the interesting input (here). Each was caught by the test failing for the *wrong reason* and being read rather than fixed. ## Verification | Check | Result | |---|---| | new suite | 8/8 | | 15 triage / planning / continuation suites | 361/361, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (307 + 10 + 71) | | `pnpm check:changesets` | clean | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:15:29 -07:00