984b3ed0c19fae8e477491ca272c98e098ad98d1
12857 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
984b3ed0c1 |
docs(learnings): a seventh inert shape — reusing an already-resolved local (my #3114 turned main red) (#3130)
Records the failure shape behind #3126, from the person who caused it. ## What is new about it The three inert conversions this program had catalogued — #3051, #3062, #3068 — all called `resolveTaskWorkflowIrSync` **at the call site**, where the sync resolver is visible in the diff. That is what the existing entries describe, and it is why they read as avoidable. Mine was not that. #3114 converted a `triage.ts` arm to `disposeLanes.wip`, reusing a value `resolvePlannerLanes` had produced a few lines above. It merged, and `main` went red: `triage.ts: 7 -> 8`. My reasoning at the time, verbatim from the PR body: > `resolvePlannerLanes` already called immediately above — no new resolution/await. That sentence checks the **cost** question and skips the **correctness** one. I confirmed I was not adding an `await` to a synchronous listener — the usual blocker, and a real one — and never asked what kind of resolver had produced the local I was reusing. **Reusing an already-resolved value reads as strictly safer than resolving.** No new work, no new await, no new failure mode. That intuition is correct about cost and silent about correctness, and the sync-ness sits one hop away inside the helper, where a call-site reviewer does not see it. So the check is not *"am I calling a sync resolver here?"* but **"what produced every lane value I am about to compare against, transitively?"** A local is not evidence; the resolver behind it is. #3122 widened the gate to follow wrappers for precisely this reason — and I walked through the door it was widened to cover, during the same phase I was adding it. ## Two corollaries recorded with it 1. **A gate that catches the defect but does not block is a report.** `check-inert-sync-lanes` fired correctly and the PR merged anyway, because it is not in the blocking set. #3127 fixes that, and I would prioritise it over any individual conversion — this is the second time this phase a correct non-blocking signal was ignored. 2. **`triage.ts`'s remaining 7 are not backlog.** The revert takes it to 7, and those seven are the same shape: they need the emitter-side / async-threading work tracked in #3082, not another conversion pass. `--claims` (#3124) now marks sync-resolver files as inert-risk and keeps them out of the start-here list for exactly this reason. ## Scope Docs only — one section appended to the existing learnings file, placed with the other numbered shapes and before "The rule that produced every fix above". No code, no gate, no changeset (internal docs). The revert itself is #3126, which I confirmed on a clean detached `origin/main` checkout rather than on a branch; I did not open a competing fix. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ad5172afd5 |
fix(engine): main is red on check:inert-sync-lanes — #3114's triage conversion is inert, revert the arm (#3126)
## `main` is red on `check:inert-sync-lanes` right now ``` inert-sync-lane: NEW inert conversions — a lane guard now reads a sync resolver that always answers with the DEFAULT board. packages/engine/src/triage.ts: 7 -> 8 ``` Verified on a clean `origin/main` checkout, not on my branch. #3114 converted this guard's third arm to `disposeLanes.wip`; the gate that exists to catch exactly this fired, and the PR landed anyway — presumably because `check:inert-sync-lanes` is not in the blocking merge-gate set. ## The change did not change behaviour `disposeLanes` comes from `resolvePlannerLanes`, which resolves through `resolveTaskWorkflowIrSync` — inert under PostgreSQL for two independent reasons (#3103). So `disposeLanes.wip` evaluates to `in-progress`: **the same value as the literal it replaced.** A card advancing into a renamed execution lane still matches nothing, still reads as an evacuation, and still kills a healthy planning session — the precise bug #3114 set out to fix, unchanged on every board. So the arm goes back to the literal. The gate's own failure text rules out the alternative: > Do NOT re-record the baseline to clear this — that is the same false green one layer up. ## #3114's analysis is kept — only the code reverts Its behavioural description is **correct** and is the clearest statement of this bug anywhere in the file. I have kept those paragraphs and added what is missing: that the fix does not reach under PG, and what would. Whoever supplies a lane answer that is not sync-resolved should make this line read `disposeLanes.wip` and delete the note. The specification is sitting right there for them. ## It also reconciles two contradictory notes, one of them mine My #3108 flag said converting the third arm this way adds an inert comparison and removes a census entry that is telling the truth. #3114 then converted it and added a note saying it fixes the bug. **Both notes sat in the file**, giving any reader two confident, opposite accounts. They are now one account with the evidence attached. ## Read this file's census count carefully #3114 took it to **0** while the inert count went to **8**. The census's own `--triage` output warns about exactly this shape: > for a sync-resolved file, a count of 0 is the WORST case, not the best — the file reads as fully converted Reverting restores it to 1, which is the honest signal. ## Census | | before | after | |---|---|---| | `triage.ts` | 0 | **1** | | repo backlog | 26 | **27** | **The number going up is the point.** A census that reports 0 for a file whose guards are all inert is worse than one that reports the truth — it retires the entry and nobody looks again. ## Measured - `check-inert-sync-lane-conversions`: **exits 1 on `main`, 0 here** (8 → 7). - `src/__tests__/triage*` — **25 files / 374 tests pass**. - `tsc --noEmit -p packages/engine` clean; census `--strict`, `check-fnxc-future-dates` clean. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c38d784b88 |
fix(events): carry lanes on the archive and completion task:moved emits (#3120)
Follow-up to **#3109** (merged). Independent of my other branches. ## The gap #3109 put resolved lanes on `task:moved` and wired the main move path in `moves.ts`. **Two other emit paths still fired lane-less** — and to a listener, a lane-less emit is not a *missing* answer, it is the **legacy** answer, which on a renamed board is wrong. Concretely: `archiveTaskBackendImpl` emits its own `task:moved`. The executor's archive branch releases the task's active-session registry entry, and **that entry is what blocks a successor task from acquiring the same path**. Fixing the listener alone (#3112) leaves the leak reachable *through this emitter*, because the listener still falls back to the literal when the payload carries nothing. That's the part worth noting for the pattern generally: **a payload-carrying design is only as good as its emitters.** Converting consumers without sweeping producers leaves a hole that looks fixed at the call site. ## Scope | Emit path | Action | |---|---| | `archive-lifecycle-2.ts:373` (`archiveTaskBackendImpl`) | carries lanes | | `task-artifacts-ops.ts:549` (`moveToDoneImpl`) | carries lanes | | `lifecycle-ops.ts:715` | **untouched** — fires from a watcher callback | | `task-update.ts:962` | **untouched** — sync path | Both converted sites are already `async` and already import `resolveWorkflowIrForTask`, so this costs one IR read on a transition that has just done database work. Fail-soft to `undefined`, matching `moves.ts` — "unknown", never a wrong answer. The two untouched ones each need their own look rather than a blanket sweep; flagged, not guessed. ## Verification - `@fusion/core` archive/lifecycle suites — **156 green** - **`pnpm test:gate` green**; `tsc` clean - Changeset added; `check:changesets` passes ## Census **No change**, and that's expected — this converts emit *payloads*, not comparison literals. The effect is that guards already converted in #3109/#3112 receive a correct answer on these paths instead of a legacy fallback. 🤖 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** * Task move events now include accurate board lane information when tasks are archived or marked complete. * Improved handling for renamed workflows, ensuring task transitions use the correct lane details. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c5d5978a01 |
test(core): the emitter route does not generalise from task:moved to task:updated — measured (#3123)
#3109 solved the inert-guard class for `task:moved` by having the **emitter** resolve lanes once and carry them on the payload. It is the right fix there, and it retired several flags within a day — two of them mine (#3118, #3121). **The obvious next step is to do the same for `task:updated`**, which is where every remaining sync-listener guard lives: `scheduler.ts`'s mission-failure and PR-monitoring guards, and `triage.ts`'s planning-evacuation guard. I went to do exactly that, and the cost profile is opposite. ## Measured | event | emit sites | files | |---|---|---| | `task:moved` | **7** | — | | `task:updated` | **26** | 10 | #3109 could argue its resolution away because a move *"is already async and already post-commit, so the resolution costs one IR read on a transition that has just done database work."* `task:updated` fires on every **log append, comment, artifact write and steering message**. `audit-ops.ts`'s logEntry **fast path** is one of those emit sites — and that path exists specifically to avoid re-reading the task. An IR read per emit there is a regression on the hottest write path in the system. ## Partial coverage does not rescue it The tempting narrower version is "add lanes only to the emit paths those guards react to". `triage.ts` rules that out: its evacuation handler reacts to **any** `task:updated` carrying a column, and explicitly tolerates partial payloads (that is why it checks `typeof task.column !== "string"`). It needs lanes on essentially every emit, or it keeps its literal fallback regardless. So those guards are **not one commit behind #3109**. Unblocking them wants either a cached lane answer the emitter can attach for free, or the sync reader this file already specifies. ## Pinned as a ratio, not prose The new case asserts `task:updated` emits **more than twice** `task:moved` — the shape of the argument rather than today's exact numbers, which move with ordinary work. If the ratio ever converges, the trade-off has genuinely changed and the note above it should be re-read. That is deliberate: a comment stating "26 vs 7" would be wrong within a week and would then argue for the opposite conclusion with full confidence. This is the third time this session I have found a note that was accurate when written and had quietly stopped being true. ## Measured - 5 cases pass (4 pre-existing + 1 new). - `tsc --noEmit -p packages/core` clean; census `--strict`, `check-fnxc-future-dates` clean. ## Census **No movement — nothing converted.** This records why the cheap-looking follow-on is not cheap, in the file that already owns this decision (#3103). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4b61170a51 |
fix(executor): read task:moved lanes from the payload (executor.ts 4 → 0) (#3112)
**Stacked on #3109** — merge that first; this is its first consumer. ## Census | Metric | Before | After | |---|---:|---:| | COLUMN guards (backlog) | 47 | **43** | | `executor.ts` | 4 | **0** | `executor.ts` is off the census top-files list. ## Why these four could not be converted in place This listener is synchronous and its branches **start execution**, dispose worktrees and release sessions. An await ahead of them defers the `execute()` dispatch itself. The sync IR resolver isn't an option either — it answers with the default workflow under PostgreSQL, so a guard written through it is inert. Reading the lanes the emitter already resolved costs nothing and leaves the prologue synchronous. This listener is the reason #3109 has the shape it does. ## The archive branch is the one with teeth `to === "archived"` matched nothing on a board with a renamed terminal lane, so **archiving never released the task's active-session registry entry** — and that entry is what blocks a **successor** task from acquiring the same path. Not cosmetic: the next task wanting that path fails to register. ## Verification - **Revert-proof:** the new case drives a `shipped` terminal lane (matching no legacy id) and asserts the release. Reverting the branch to the literal leaves the entry held — `expected [Array(1)] to have a length of 0`. - 43 executor suites — **483 green** - **`pnpm test:gate` green**; eslint clean ## Note on shape Lanes are read as **single ids, not sets**, because each branch here is a lane-identity test on one column — exactly what the literals were. Widening to membership would change behaviour, not just vocabulary. Fail-soft to the legacy ids when the emit path could not resolve, matching every other consumer of this payload. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
218086bea2 |
fleet(engine): self-healing 6 → 1 — the board-stall counter, the last guard that needed a sync answer (#3121)
The last fan-out guard, and the one I explicitly said needed a synchronous answer. #3109 made that answer available without an await, so the flag comes off. ## Why this one was last The other two guards in this listener gated work the listener **already `void`s**, so they moved onto the async resolver in #3094. This one increments in-memory state **in the handler's own tick**, so it genuinely needed a synchronous answer. The sync IR path was never that answer: `resolveTaskWorkflowIrSync` cannot resolve a **custom** workflow at all — two independent blockers, #3103 — which is why I wrote that conversion, measured it, and withdrew it. #3109's emitter-carried `lanes` removes the dilemma rather than trading one horn for the other: reading them needs **no await**, so the increment stays in the same tick *and* the guard becomes correct. ## What it fixes On a renamed board this counter read **zero**. The board-stall watchdog was blind to a board whose cards were moving out of implementation the whole time — the signal it exists to raise was never raised. ## Census | | before | after | |---|---|---| | `self-healing.ts` | 6 | **1** | | repo backlog | 29 | **24** | The remaining 1 is the log-dedup closure — a pre-existing flag whose degraded answer costs a duplicate log line, not a lifecycle decision. ## Measured - 3 new cases; `self-healing-completion-fanout.test.ts` **13/13 pass**. - **MUTATION**: restoring the literal pair fails the renamed case. - **The paired negative is the load-bearing one.** The guard means *"left implementation for somewhere that is not implementation"*, so a move **between two non-wip lanes** must not count. Without that case, a conversion that counted every move would pass the positive and inflate the watchdog's denominator — breaking it in the opposite direction, which is harder to notice than a zero. - A **fail-soft** case pins that an emit carrying no `lanes` still counts on the legacy ids. - **Asserted through the counter itself**, not a downstream alert. The increment *is* what this guard decides; routing the assertion through the watchdog would let an unrelated threshold change mask a regression here. - `src/__tests__/self-healing*` + `task-agent*` — **42 files / 848 tests pass**. - `tsc --noEmit -p packages/engine` clean; census `--strict`, `check-lane-wiring`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean. ## On the withdrawal this reverses #3094 withdrew a sync-IR conversion of this listener and recorded why, precisely. That record is what made this cheap: I could tell in one read that #3109 addressed the *specific* obstacle rather than a general "async is hard". A flag that names its blocker exactly is a flag that can be retired the day the blocker goes. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
56b5cdfeed |
test(notifications): cover the wedge-episode renamed-lane clear that main's conversion lacked (#3096)
**Rebased onto current `main`, and it shrank to a test.** Was "close the wedge-episode race, then resolve its lanes." ## What happened Another worker landed **both halves of this PR independently** while it was open. Rebasing showed their versions are better, so I took theirs and dropped mine: - **The serialisation** — theirs is `enqueueWedgeHandling(taskId, run)`, a general callback; mine was wedge-specific. - **The conversion** — theirs is **project-union membership** over the four roles; mine was first-match-per-role via `resolveLifecycleColumns`. Membership is correct: more than one lane can fill a role on a renamed board, and first-match silently ignores the rest. My rebased branch initially compiled to a **duplicate `wedgeHandlingChains` field and duplicate method** — caught by `tsc`, removed. Nothing of my implementation survives, and it shouldn't. ## What's left is worth landing Their conversion has **no renamed-board test**. This adds one. A card recovering into a renamed hold lane must **clear** its episode. Asserted through the *second* notification, because a stale active episode also **refuses the next genuine wedge its claim** — so the visible symptom is a real wedge going unannounced, not merely a stale alert. The fixture needed `listWorkflowDefinitions`: `resolveProjectColumnsForRoles` unions across the project's workflows, so the per-task selection readers alone leave it resolving nothing and the test would pass for the wrong reason. That's how I found the mismatch — my original test failed against their implementation. ## Verification - Green as written against **their** implementation - **Revert-proof against theirs:** restoring the four literals fails it — 1 delivered, 2 expected - 7 notification suites — **80 green**; `tsc` clean 🤖 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** * Fixed wedge notifications so they can trigger again after a task recovers into a renamed workflow’s hold lane. * **Tests** * Added regression coverage confirming that recovered tasks correctly clear their wedge state and support subsequent notifications. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6050d6eb83 |
chore(engine): mark the auto-merge-finalization reviewed literals DELIBERATE (census 47→45) (#3107)
Fleet phase. `packages/engine/src/auto-merge-finalization.ts` was the last census file with no branch, worktree, or open PR against it. Claim published by pushing the branch before starting. ## Census before / after | | total | this file | |---|---|---| | before | **47** | 2 | | after | **45** | 0 | `--strict` exits 0, baseline re-recorded. **Reclassification, not conversion** — both lines are unchanged. ## Both sites were already reasoned, in a note that calls them non-defects - **Line 30** is the resolver's **degraded fallback arm**, inside `catch`. The live arm two lines up calls `columnHasFlag(ir, columnId, "complete")`. The literal is reached only when IR resolution throws, where the legacy id is the only answer left — removing it would make a failed resolve return nothing. - **Line 99** picks an **error string**. The note above it works through threading `isCompleteColumn` in and concludes the signature widening costs more than the sharper diagnostic buys. I did not revisit either judgement. The gap was mechanical: prose the census cannot read, so both stayed in `byFile` as apparent debt for the next pass to re-derive. ## This is the fourth, and it closes the set With #3056, #3060, and #3063, **every census file that was unclaimed during this phase has now been examined, and not one needed a conversion.** Each site was a three-state fallback arm, or a site a prior pass had already reviewed and kept. The corollary is the finding I would most want carried forward: the remaining count is not a work queue. A worker told to "claim the largest cluster" reads the number, finds most of it already reasoned, and reaches for whatever moves it — which is how three PRs converted guards to a synchronous resolver that is inert under PostgreSQL. One exception worth preserving: **`taskRevert.ts` should stay counted.** I claimed, inspected, and released it without marking. Converting it would classify a *neighbour* row using the modal task's flags — wrong on data, not merely stale on vocabulary — and its note correctly calls the entry **accurate debt** blocked on a per-neighbour flag map. Fallback arms and dead paths → mark. Placeholders awaiting a capability → leave counted. ## Verification - `census --strict` exit 0; `tsc --noEmit` (engine) **0 errors** - No dedicated test file for this module (`vitest` reports none), so no suite to run — comment-only diff, no behaviour change Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
82329819f7 |
fleet: resolve the pre-archive unarchive target (census 69 → 68) (#3091)
## Census
| | column guards |
|---|---|
| before | **69** |
| after | **68** |
## How this was found
By finishing a triage I'd left incomplete. Of the 19 single-guard files,
I had actually examined six and flagged the rest partly on assumption —
so I went back and read them.
Five of the remaining ones turned out to be `archived` comparisons
**pinned by `archived-column-gate-parity.test.ts`** (`audit-ops`,
`task-id-integrity`, `mission-store`, `async-comments-attachments`, plus
`merge-queue-ops-2` for review). Converting any of those moves one of
three encodings that must move together.
**This one isn't pinned**, and that difference is the whole PR.
## What changed
```ts
if (!declaresPreArchiveColumn || preArchiveColumn === archivedColumn
|| preArchiveColumn === "archived")
```
Belt-and-braces: the condition already accepted the resolved lane **or**
the legacy id, stated twice. A set says it once, so the two halves can't
drift apart — the real risk with a duplicated condition, rather than the
census count.
## Why this isn't the split brain
The parity guard pins comparisons of a **task's column** — one of three
encodings of *"an archived task is not live."* This compares a **stored
`preArchiveColumn` value** against the board's archive lane: a different
question, about where to send a card on unarchive.
Verified rather than argued — that suite runs **green** here, and it
went **red** the last time I touched a pinned site (#3076, where I named
an arm and immediately reverted). It's a live check, not an assumption.
## Measured
| check | result |
|---|---|
| archive / artifact / unarchive suites | 7 files, **27 tests green** |
| `archived-column-gate-parity` | **2 passed** |
| five gates + strict census | green |
| core `tsc` | clean |
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
89b21e2906 |
fleet: triage's planning-evacuation check uses the resolved wip lane (census 45 → 44) (#3114)
## Census
| | column guards |
|---|---|
| before | **45** |
| after | **44** |
## What changed
```ts
if (task.column === disposeLanes.hold || task.column === disposeLanes.intake
|| task.column === "in-progress") return;
```
Two role questions and one id question on the same line.
`resolvePlannerLanes` is **already called immediately above**, and its
result carries `wip` — so this needs no new resolution and no new await.
The literal just stops being the odd one out among its neighbours.
## What it cost on a renamed board
This handler aborts a planning session when a card leaves the planner
lanes. `in-progress` is excluded because *a card advancing into
execution is not an evacuation* — that's stated in the note directly
above it.
Against the literal, that exclusion **never matched** on a board whose
execution lane is renamed. So a legitimate advance into execution read
as an evacuation and **killed a healthy planning session** — precisely
the case the comment says must not abort.
`wip` is optional by design (PR #2628: a missing role stays `undefined`
so callers refuse rather than invent a column). Undefined here means the
board declares no execution lane, so there's no advance-into-execution
to exclude and the comparison is correctly false.
## Not addressed, and pre-existing
This line resolves through `resolvePlannerLanes` — the **sync** twin,
which returns the default workflow's lanes under PostgreSQL. That
affects all three lanes on the line equally and predates this change:
the handler is `(task: Task) => {}` with no await available, so fixing
it needs the same emitter-side change as #3082.
Making the third lane consistent with the other two doesn't deepen that,
and it leaves **one** shape to fix there rather than two.
## Measured
| check | result |
|---|---|
| triage / evacuation / planner-lane suites | **453 tests green** |
| four gates + strict census | green |
| engine `tsc` | clean |
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
7d75633daa |
gate: follow sync-lane sources across module boundaries (11 inert guards were invisible) (#3079)
## Eleven inert guards were invisible to the check that exists to find them #3062 shipped this check collecting sync-lane sources **per file**, and recorded the module-boundary gap as a known limit. That limit has stopped being theoretical. `resolvePlannerLanes` is defined in `replan-target.ts` and consumed in `triage.ts` and `executor.ts`. `triage.ts:723` already reads: ```ts if (task.column === disposeLanes.hold || task.column === disposeLanes.intake || task.column === "in-progress") return; ``` A sync-resolved pair sitting directly beside a literal. That is precisely the site a fleet worker reaches for next — the "conversion" is a one-word edit, it drops the census by one, and it changes nothing, because `resolvePlannerLanes` resolves through `store.resolveTaskWorkflowIrSync` and always describes the default board. **The shipped check could not have seen it.** ## What changed Sources are collected in a **first pass over the whole tree**; consumption is counted in a **second pass** against that repo-wide set. | file | guards now visible | |---|---| | `packages/engine/src/scheduler.ts` | 18 | | `packages/engine/src/triage.ts` | **7 — previously invisible** | | `packages/engine/src/executor.ts` | **4 — previously invisible** | **Baseline 20 → 29. The rise is detection, not regression.** Those eleven guards were already inert; nothing in the tree got worse and no production file is touched by this PR. Flagging that explicitly because the check's own failure text says *"do NOT re-record the baseline to clear this"* — that rule is about a code change, and this is a detector widening. The distinction matters and I would rather state it than have it inferred. ## Mutation evidence, on the motivating site Converting `triage.ts`'s literal to `disposeLanes.wip` — the exact inert edit this widening exists to prevent: | | result | |---|---| | check as shipped on main | **exit 0** — missed entirely | | this PR | **exit 1**, `triage.ts: 7 → 8` | `scheduler.ts` restored clean after every run. ## Third widening — and the honest read on that This is the third hole found in this check, each discovered the same way: 1. **#3062**: matched only the local-variable spelling; the inline `resolveX(...).review` walked past. 2. **#3068**: matched only comparisons; `parked.terminal.has(to)` walked past — and worse, made the count *fall*, inviting a re-record that would have retired live sites. 3. **here**: matched only same-file sources; a cross-module helper walked past. Every time, the check was correct about the shape in front of it and blind to a trivially different spelling of the same defect. I said in #3068 that the durable fix is to key on **the source** rather than enumerate consuming syntax; this closes the module-boundary half of that. The remaining half is value-level dataflow (a sync-resolved id assigned through an intermediate, passed as an argument, or returned), which needs a type-aware pass and is a larger change than a gate script should carry casually. I am not claiming this version is complete. It is measurably better than the shipped one on a site that exists in the tree today. ## Census before / after ``` before: COLUMN guards (the backlog): 84 after: COLUMN guards (the backlog): 84 ``` Unchanged — this converts nothing. It makes eleven already-inert guards countable so the next conversion of them fails loudly instead of reading as progress. ## Verification `test:gate` exit 0 · `pnpm check:inert-sync-lanes` exit 0 · lifecycle-column census exit 0 · `pnpm lint` clean. Gate script + baseline only. Still open and unrelated: **#3073** re-greens the archived-gate parity ratchet, which is red on `main` right now and sits outside the merge gate. |
||
|
|
b633faacab |
census: --triage splits the backlog into documented deferrals vs unexamined (#3097)
## The headline number stopped tracking work
I have hand-computed this breakdown every round of the phase to decide
what to claim. Making it a first-class mode so nobody else has to, and
so "census before/after" in a PR body means something.
On current `main`:
```
COLUMN guards (the backlog): 51
TRIAGE (heuristic, opt-in; changes no count and no exit code)
documented deferral (flag note within 40 lines): 13
unexamined: 38
unexamined, by file — this is the list to pick work from:
21 packages/engine/src/self-healing.ts
4 packages/engine/src/executor.ts
2 packages/engine/src/auto-merge-finalization.ts
2 packages/engine/src/scheduler.ts
1 packages/core/src/eval-signal-collector.ts
...
```
**51 reads as a lot of available work. It is not.** 13 carry an explicit
reason for staying a literal, and 21 of the remaining 38 are
`self-healing.ts` — which has **eight** open PRs on it. What is actually
loose is roughly a dozen scattered singletons, most of them in
synchronous listeners where the only available resolver is inert.
That gap is not cosmetic; it is causal. A worker told to "claim the
largest cluster" reads 51, finds little that is both unclaimed and
convertible, and reaches for whatever moves the number. That is exactly
how #3051 converted ten `scheduler.ts` guards to
`resolveTaskWorkflowIrSync` — inert under PostgreSQL, refuted end-to-end
in #3058 — and how five open PRs came to share the same helper.
## Design constraints I held to
- **Opt-in.** No flag, no change. Verified: default output is
**byte-identical** to `main` (`diff` clean), and `--json`, `--strict`
and `--compare` all still exit 0.
- **Beside the totals, never inside them.** It changes no count and no
exit code — the same discipline `traitFallbackCount` already documents
two lines above, and for the same reason: a deferred guard is still a
guard.
- **Nothing downstream consumes it.** It is a triage aid for choosing
work, not a gate.
## Stated limits
Classification is **comment proximity**: an FNXC note within 40 lines
above the guard whose text marks a deliberate deferral. It cannot tell a
good reason from a bad one, and a note that sits far above its guard
reads as unexamined. That is why it is opt-in and why no gate reads it.
## The bug I shipped into my own draft, and what it cost
The first version reported **`documented deferral: 0`** — for a tree I
knew had them, because I had counted them by hand that morning. Cause:
it referenced an undefined path constant, and my `try/catch` turned the
`ReferenceError` into an empty file list, so every proximity window was
the empty string and nothing ever matched.
That is the same silent-catch shape I have flagged in review twice this
phase. The catch is now gone: **a triage aid that fails to zero is worse
than one that throws**, because zero reads as a clean answer rather than
a broken instrument. Post-fix it reports 13/38, which matches the hand
counts I have been posting all phase.
## Census before / after
```
before: COLUMN guards (the backlog): 51
after: COLUMN guards (the backlog): 51
```
Unchanged by construction — this converts nothing. It makes the number
interpretable.
## Verification
`test:gate` exit 0 · default census output byte-identical to main ·
`--json` / `--strict` / `--compare` exit 0 · `pnpm lint` clean. One
script; no production file touched.
Related and still open: **#3079** (makes the five inert
`resolveMoveFanoutColumnsSync` guards fail the build instead of
registering as a census win) and **#3095**.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Added an optional triage mode to classify column findings as
documented deferrals or items requiring review.
- Added aggregate and top-file triage results to make findings easier to
assess.
- Added guidance for the new command-line option.
- **Bug Fixes**
- Improved source-reading error handling so failures are reported
clearly instead of being silently ignored.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
6eeeb43d4b |
test(engine): pin #3047's archive-sweep conversion — measured uncovered (825 tests passed against the reverted fix) (#3115)
Not a conversion — the fleet's conversions are landing faster than their coverage, and this is the audit that shows which ones actually have any. ## Method For each of today's fleet commits to `self-healing.ts`: revert that single commit, re-run the file's suites, see whether anything fails. If nothing fails, the conversion has no regression protection and the "N tests passed" cited on its PR was measuring something else. | commit | reverted | verdict | |---|---|---| | #3075 pause-abort recovery | suites **fail** | covered | | **#3047 archiveStaleDoneTasks** | **825 tests all pass** | **uncovered** | | #3078 (mine) | 204 tests all passed | was uncovered — closed by #3090, #3102 | Every fixture in the `archiveStaleDoneTasks` describe block uses the id `done`, where the literal is correct, so none of them could see the conversion at all. ## What the literal cost The sweep's dependent scan skips tasks in terminal lanes. On a renamed board **nothing matched `done`/`archived`, so every task read as active** — which means every archive candidate looked like it had active dependents, and the sweep archived **nothing**. The board quietly stops auto-archiving: no error, no log line, no failing test. ## The case covers both halves of #3047 - a stale card in a **renamed complete lane** is archived — the `complete` role - a card whose dependent is still live in the **renamed wip lane** is **not** archived, and a dependent already in the **renamed archive lane** does not count as live — the `terminal` role That second assertion is the one that matters: it stops the fix from degenerating into "archive everything", which is the failure mode a one-sided test would miss. ## Measured **413 pass** on current main; reverting #3047 fails **exactly this test**. ## Remaining audit I have now audited 3 of ~13 fleet conversions to this file this way. The method is cheap (one revert, one 17s suite run) and I will keep working through the rest unless someone else picks it up. #3049 could not be auto-reverted — later commits overlap its hunks — so it needs a manual read rather than a mechanical revert. ## Verification `self-healing.test.ts` **413 passed** · `pnpm test:gate` 13 + 161 + 487 + 71 · lint — green. |
||
|
|
ec2921b958 |
fleet(engine): self-healing 23 → 6 — async-reachable guards, plus 3 of 4 fan-out guards the sync path could not serve (#3094)
**Replaces #3093, which I am closing.** Third rebuild of this work. ## A coordination note first, because it is costing more than the code `self-healing.ts` has had **three** overlapping conversions land from other lanes while my branch was open — #3049, #3075, #3078. Every time, replaying my commits produced conflicts that were all the same shape: *same guard, two spellings, different variable names*. Each rebuild is a full cycle spent on merge mechanics rather than on lanes. I have rebuilt against `main`'s own census each time rather than argue about whose spelling wins, and this PR contains only what `main` (23) does not have. But if this file is going to keep receiving concurrent fleet passes, one lane should own it — otherwise the next PR pays the same tax again. ## Converted | site | note | |---|---| | `isPhantomExecutorBinding` | caller resolved `lanesOfReclaim(task.id).wip` **three lines above the call**, then passed a task whose column the predicate compared against `in-progress` | | `isWorkspaceOwnerLive` | required `completeColumns` | | `recoverPausedAbortFailures` **body** | #3075 converted this sweep's *router* and left three body guards comparing ids | | `reconcilePreExecutionWorktrees` | a four-id literal in a sweep that **removes worktrees** | | `recoverStarvedRefinementTriageTasks` | 2 peer counts that read zero, so escalation never fired | | `evaluateParkedAgentTaskLink` | the omitted `parkedColumns` argument | All through the **async** `resolveProjectColumnsForRoles`, whose only store read is `listWorkflowDefinitions()` — answerable under PostgreSQL. That is what separates these from the inert kind. **The half-converted sweep is the important one.** A router that resolves correctly feeding a body that compares ids is worse than converting neither: the route now fires on a renamed board and the body then acts on the wrong lane. The `moveTask` **target** is the sharp end — an undeclared target is rejected *except* under `recoveryRehome` with a legacy id (`moves.ts:570`, the #1411 escape hatch), so a converted route feeding the literal `"todo"` rehomes the card into a column its workflow does not declare, which is the state other reconcilers exist to repair. **A real dropped-behaviour bug**: `evaluateParkedAgentTaskLink` was called without `parkedColumns`, falling back to `LEGACY_PARKED_COLUMNS`. A live durable agent linked to a card resting in a renamed hold lane read as not-parked, so the safeguard preserving its task link never applied. ## Withdrawn: the `task:moved` fan-out I wrote the sync-IR conversion, measured it, removed it. `getTaskWorkflowSelectionImpl` returns `undefined` **unconditionally** under PostgreSQL, so `resolveTaskWorkflowIrSync` always answers with the default builtin IR and `columnsWithFlag` on it yields exactly the legacy ids — inert on every board. Worse than the literal, because **the literal is counted**. My own test passed only because its store mock supplied a renamed IR: it pinned the helper's shape, not production behaviour. The refutation is recorded in place, and `check-inert-sync-lane-conversions` exits 0 on this branch. ## One question, one answer An earlier pass of this work mapped the notification-attach guard onto a wider `activeWork` set, and `self-healing-paused-abort-recovery > "rehomes an in-progress pause-abort park back to todo"` caught it — an in-progress park attached a transition notification it should not have. The fix is not a narrower set. Both guards ask **one** question — *"is the card already at the requeue target?"* — which the literal happened to spell twice as `=== "todo"`. The target now resolves once, before the write, and both read it. Deriving one question two ways is exactly how a converted guard and an unconverted target drift apart. ## Census | | before | after | |---|---|---| | `self-healing.ts` | 23 | **11** | | repo backlog | 53 | **41** | ## Measured - `src/__tests__/self-healing*` + `task-agent*` — **42 files / 836 tests pass** - `tsc --noEmit -p packages/engine` clean; **`check-inert-sync-lane-conversions` exits 0**; census `--strict`, `check-lane-wiring`, `check-fnxc-future-dates` clean ## The remaining 11, flagged not guessed - **4** — the fan-out, withdrawn above; blocked on a sync-capable selection reader. - **1** — the log-dedup closure: pre-existing flag; it sits before the lane prefetch it needs, and the degraded answer costs a duplicate log line, not a lifecycle decision. - **1** — the synthetic `{ column: "todo" }` for a *missing* task: deliberate, and now correct rather than unconverted, because `parkedColumns` is legacy-seeded. - The rest are status/deliberate classifications the census counts but that are not lane guards. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved workflow automation for boards using renamed or customized workflow lanes. * Fixed task completion fan-out, branch rebinding, recovery, and stalled-task detection across custom lifecycle columns. * Prevented completion actions from triggering when tasks move back to the work-in-progress lane. * Improved cleanup and pause recovery behavior for customized workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
50f089a14b |
test(core): archived is renameable today — remove a wrong reason for the cheap parity option (#3113)
Follow-on from #3110, where the archived-gate parity test blocked my conversion and laid out two ways forward. This removes a **wrong reason** for picking the cheap one. ## The cheap option looks like it has already been taken `archived-column-gate-parity.test.ts` offers: convert all three encodings, **or** *"declare `archived` a non-renameable system column and mark the sites deliberate"*. The second is far cheaper — and the codebase reads as if it is already true: > `"Globally archived; hidden from the board. RESTRICTED (built-in only)."` — `trait-types.ts` At a glance that says a custom board cannot have an archive lane of its own, which would make all 52 sites correct by construction and the whole family **deliberate rather than debt**. That is a very attractive conclusion for anyone facing a 52-site conversion. ## It does not mean that The restriction is over trait **registration**. `trait-registry.ts` rejects a **non-builtin** (plugin-defined) trait that declares `archived` or `complete` (R22). It says nothing about which **column** may carry the built-in trait, and a custom workflow may put it on a column with any id. **Verified, not argued:** | input | result | |---|---| | `columnsWithFlag(ir, "archived")` where the lane is named `filed` | `["filed"]` | | `resolveColumnFlags` on that column | `{ archived: true, hiddenFromBoard: true }` | | `columnsWithFlag(ir, "complete")` where the lane is named `shipped` | `["shipped"]` | So option two is a **capability removal** — it would silently break any board that has already renamed its archive lane — not the documentation of a constraint that already exists. ## What this does and does not do It **does not** decide between the two options; that is a product call with real blast radius either way. It removes the reason someone would most plausibly reach for after reading that flag comment, and it does so with an executable check rather than a claim. The `complete` case is pinned alongside it, because both restricted flags behave identically — anyone reaching the same conclusion about `complete` would be wrong for the same reason. The first case also doubles as a **tripwire**: if `columnsWithFlag(ir, "archived")` ever returns `[]` or `["archived"]` for that fixture, the cheap option has effectively been taken and the parity test's framing needs revisiting. That is the signal the case exists to give. ## Measured - 4 new cases pass. - The parity test still passes with its corrected framing — it excludes `__tests__` from its scan, so the added prose cannot move its own inventory. I checked that before editing it rather than after. - `src/__tests__/{archived,trait,log-entry}*` — **3 files / 26 tests pass**. - `tsc --noEmit -p packages/core` clean; census `--strict`, `check-fnxc-future-dates` clean. ## Census **No movement — nothing converted.** Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
77d11a0a9a |
docs(core): resolveLifecycleColumns returns one id per role, not a set (#3111)
Comment-only. No behaviour change. `tsc` 0 errors, eslint clean, `census --strict` and `check:fnxc-future-dates` exit 0. ## Why this is worth a PR Three fleet PRs have independently read this function's fields as **membership**: | where | finding | |---|---| | `self-healing.ts` `hold` guard | **#3084**, `Major` — *"resolve `hold` by membership, not first-per-role"* | | `notification-service.ts` progressed-lane check | **#3096** — reads `?.hold / ?.wip / ?.complete / ?.archived` | | sync vs async review set | **#3088** — same shape, different symptom | Each field is a **single id**: the first column carrying that trait. A board may declare several columns with one role — two pre-review working lanes, or a review lane plus a merge-blocked lane — and every one after the first is invisible. Used for membership, that reads as a working check while silently ignoring lanes: a card resting in the second hold column classifies as **not-held**, and the guard depending on it never fires. It is the quiet direction of wrong, which is why it survives review three times. **Three occurrences of one misreading is a property of the signature, not three unlucky authors.** Fixing it at the call sites one at a time leaves the next author to rediscover it, so this documents the contract where the mistake is made — at the definition — and names the alternatives: - `columnsWithFlag(ir, role)` — every column with the role on one board - `resolveProjectColumnsForRoles(store, roles)` — the union across a project's workflows Rule of thumb recorded in the doc: **a routing/move target wants this function; a `.has(task.column)` test does not.** ## What this does not do It does not fix the three call sites. #3084's is a live `Major` and needs a real fix with mutation verification; #3096's needs its author to resolve a two-implementation conflict first; #3088's is flagged and open. This only stops the fourth occurrence. A stronger version would rename the fields (`firstHoldColumn`) or return branded single-id types so misuse fails to compile. That is a wider change across every caller and belongs to whoever owns the helper's API — worth considering once the current fleet PRs land, since doing it now would conflict with all three. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
41cdcc741e |
fix(events): carry resolved lanes on task:moved so listener guards stop being inert (#3109)
Removes the **inert-guard class at its source** instead of one call site at a time. Independent of my other branches. ## The problem `task:moved` listeners run synchronously, so a listener needing a lane answer had to resolve one synchronously — and `resolveTaskWorkflowIrSync` returns the **default** workflow under PostgreSQL, the shipped backend. Every such guard behaved exactly as the literal it replaced, while the census scored it as converted. **Resolving asynchronously inside the listener is not available**, and that is measured rather than assumed. The scheduler's `snapshotManager.invalidate` is asserted to run in the listener's **synchronous prologue**; putting an await ahead of it produced **3 failures across 21 scheduler suites**. ## The fix The emitter carries the answer, which removes the dilemma rather than trading one horn for the other. `moves.ts` is already async and already post-commit, so it resolves the moving task's lanes **once** and hands them to every listener. The guard becomes correct **and** the prologue stays synchronous. This is the file's own recorded preferred fix — *"having the emitter carry the resolved lanes on the event payload so no listener resolves at all"* — now that the audit it was waiting on is done and came back as **one** prologue-dependent consumer, not a class. ## Design choices - **`lanes` is optional and fail-soft to `undefined`** — "unknown", never "legacy". Some emit paths fire from sync contexts or a cached row mid-teardown. Listeners keep their existing fallback, so those paths are no better than before but **no worse**, and they become the exception rather than the rule. - **`mergeParkedColumns` overlays only fields the emitter actually resolved**, so a partial payload cannot blank a lane back to a wrong answer. - **The sync resolver stays** as that fallback. Deleting it would strand the emit paths that cannot resolve. ## Verification - **Revert-proof and it pins the prologue:** the new case asserts invalidation on a **renamed** hold lane with **no `waitFor`**. Ignoring the payload gives **0 calls**. - 21 scheduler suites — **361 green** - self-healing + notification suites — **491 green** - core moves + the `sync-workflow-ir-callsite-allowlist` ratchet — green - **`pnpm test:gate` green** (71) - Changeset added; `check:changesets` passes ## What it unblocks `scheduler.ts`'s 10 allow-listed guards now resolve correctly for every move that goes through `moves.ts` — the path real moves take. Those were already absent from the backlog, so **the census number does not move**; what changes is that they now do what the number claimed. `executor.ts`'s 4 remaining sites can follow the same pattern in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4afb32ef98 |
test(core): cover the untested log-entry archive gate; correct a deferral that named the wrong blocker (#3110)
I converted `audit-ops.ts`'s archived gate, measured, and **backed it
out**. Both halves of that are the deliverable.
## The old deferral was stale on its own terms
It declined the conversion because *"the fix is the same one
`getLiveTaskColumn` needs"* and doing one of the pair would leave them
disagreeing.
But `getLiveTaskColumn` now **takes** a resolved `archivedColumns` set,
and both of its callers already pass `await resolveArchivedLanes(store)`
— including the sentinel path **twenty lines up in this same function**.
The pair it worried about was already half-converted, and this arm was
the half out of step. Converting it would have made them *agree*.
That is the third deferral I have found this session whose stated
blocker had dissolved. A deferral note records the blocker at the moment
it was written, and nothing re-checks it.
## The real blocker is one neither note named
`archived-column-gate-parity.test.ts` failed my conversion, and its
reasoning is correct and not obvious. This gate has **three encodings**:
1. TypeScript comparisons
2. Drizzle `eq`/`ne` predicates
3. raw SQL templates
Converting only the TypeScript arm makes them **diverge**: the gate
would call the row archived while the SQL side still returns it as live
— a log write rejected by its gate while its parent is listed as live.
Every builtin workflow names the column `archived`, so all three agree
*by accident* on every board we ship, and nothing except that parity
test can see the split.
Unblocking means converting all three together — the SQL sides need the
resolved id as a query-build value, including inside `for update`
transactions that receive no store today — or declaring `archived` a
non-renameable system column. That test lays out both options and owns
the inventory that has to move in the same commit. I am not doing it
here; it is a different change from a lane conversion.
## What ships
**The corrected note**, and **a test for a gate that had no coverage in
any form**.
The test asserts the legacy refusal and — the case that matters more —
that a **live lane is not refused**. A gate that refused everything
would satisfy a one-sided test and silently break every log write on the
board.
The renamed case is recorded as a **deliberate, explained omission**
rather than left as a silent hole, so the next reader knows it is a
decision.
## Measured
- 3 new cases pass; the parity gate passes.
- **MUTATION**, on the conversion before I reverted it: restoring the
literal failed the renamed case. The conversion *worked* — which is
exactly why the parity gate mattered. A working change can still be the
wrong change.
- The live-lane negative asserts **the gate did not fire**, not that the
call succeeded: past the gate the fast path performs a real Drizzle
write this fake layer cannot serve, so asserting success would drag a
database fixture into a test about a lane comparison, and asserting a
bare rejection would pass even if the gate *had* fired.
- `src/__tests__/{log-entry,archive,cold-storage,unarchive}*` — **7
files / 23 tests pass**.
- `tsc --noEmit -p packages/core` clean; census `--strict`,
`check-fnxc-future-dates` clean.
## Census
**No movement — nothing converted, deliberately.** The count stays where
it is because the gate is blocked, not because it is fine.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
15a664a8f5 |
docs(engine): flag executor's four task:moved literals — the obvious conversion is provably inert (#3104)
The largest unclaimed census cluster. **Nothing in this file said why the sync-lane pass skipped it**, and that silence is the hazard: the obvious next move is to convert these the way `scheduler.ts`'s ten were converted, which would make them **inert rather than fixed**. ## The literals are genuinely wrong — this is not a "non-issue" flag All four sit in one synchronous `task:moved` listener, and on a renamed board: - execution **never starts** on a move into the board's own wip lane; - terminal session release **never runs** on a move into its archive lane; - both `from` guards never fire, so **in-flight work is not aborted** when a card leaves implementation. Nothing errors. The engine simply stops reacting. ## Why the obvious fix is inert — proved, not argued `task:moved` is emitted synchronously, so an `await` here reorders this handler against every other subscriber. That points at the sync IR path, which cannot answer for a renamed board for **two independent reasons** (`sync-workflow-ir-second-blocker.test.ts`, #3103): 1. `getTaskWorkflowSelectionImpl` returns `undefined` unconditionally under PostgreSQL, so `resolveTaskWorkflowIrSync` always takes its `!workflowId` branch. 2. Even **with** a selection, the custom-workflow branch loads its IR through `store.db`, whose implementation is an **unconditional throw** — so it falls into the catch and returns the default IR anyway. **A renamed lane is a custom workflow, so (2) alone is decisive.** The sync path can never serve this listener's case, whatever the selection reader is fixed to do. That is the part the existing notes across this repo miss, and it is why flagging beats attempting here. `check-inert-sync-lane-conversions` already baselines **twenty** guards in exactly that state in `scheduler.ts`. These four must not join them. ## Census **Unchanged at 4, deliberately.** Marking them DELIBERATE-LITERAL would buy a smaller number by asserting the code is *fine*. It is not fine — it is *blocked*. Those are different claims with different expiries, and the census should keep pointing here until the block is lifted. An unconverted literal is visible; an inert conversion leaves the backlog and takes the evidence with it. ## Measured - Comment-only change. - `src/__tests__/executor*` — **84 files / 853 tests pass**. - `tsc --noEmit -p packages/engine` clean; census `--strict`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean. ## Unblocking, for whoever takes it Either an async listener contract — a behaviour change to handler ordering, not a column conversion — or a sync reader that answers for **custom** workflows *and* survives a writer on another node. All three constraints are written up in `sync-workflow-ir-second-blocker.test.ts` (#3103). --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f5926d3b54 |
docs(engine): flag triage's evacuation guard — it looks two-thirds converted and is fully literal (#3108)
Completes the sync-listener audit across the three files holding the remaining blocked guards — `executor.ts` (#3104), `scheduler.ts` (#3100), and this one. Triage is the most misleading of the three. ## The shape lies The guard reads as **two resolved arms and one literal**: > `task.column === disposeLanes.hold || task.column === disposeLanes.intake || task.column === "in-progress"` So the obvious next move is to convert the third arm with the same helper. That is wrong twice: **1. The two "resolved" arms are not resolved.** `resolvePlannerLanes` goes through `resolveTaskWorkflowIrSync`, which cannot answer for a **custom** workflow — the sync selection reader returns `undefined` unconditionally, *and* the custom-workflow IR read goes through `store.db`, whose implementation is an unconditional throw (#3103). So `disposeLanes.hold` / `.intake` are `todo` / `triage` on every board. **All three arms are literal in effect.** Converting the third the same way adds a third inert comparison and retires a census entry that is currently telling the truth. **2. The guard's answer is consumed synchronously** — the criterion I had to correct in #3104. Below it, `pauseAborted.add`, `session.dispose()` and `activeSessions.delete` mutate in-memory state in this tick, and other paths read those maps. Contrast `self-healing.ts`'s fan-out, where three of four guards only gated work the listener already `void`s and so *were* convertible via the async resolver (#3094). ## What it costs, and the obvious reading is backwards An evacuation **into** a renamed destination still falls through and disposes correctly — no bug there. The failure is the other direction: on a board whose **hold or intake** lane is renamed, arms 1 and 2 stop matching, so a card **sitting still in its own planning lane** is treated as evacuated and its live triage session is aborted mid-run. I state it that way because "renamed board → guard misses → nothing happens" is the pattern everywhere else in this program, and here it inverts. ## Census **Unchanged at 1**, deliberately. Blocked, and now documented as *fully literal* rather than part-converted — which is the fact a future pass needs in order not to make it worse. ## Measured - Comment-only. - `src/__tests__/triage*` — **25 files / 374 tests pass**. - `tsc --noEmit -p packages/engine` clean; `check-fnxc-future-dates` clean. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e43650416d |
fix(census): surface files where a conversion may be INERT (#3105)
Tooling fix for a measurement gap **I created and then found.** Independent of my other branches. ## The gap `resolveTaskWorkflowIrSync` returns the **default** workflow IR for every task under PostgreSQL — the shipped backend — because the sync selection reader answers `undefined` unconditionally. A guard resolved through it behaves **exactly as the literal it replaced**, yet the census scored it as converted. The backlog number fell; production did not change. I did this twice. One was caught by the new call-site ratchet (`self-healing.ts`, since reverted to an honest literal). The other — `scheduler.ts` in my merged #3051 — was not, because it has an allow-list entry. **The allow-list stops the class growing. It does not stop it counting.** `scheduler.ts` alone holds **10 guards** fed by its allow-listed sync resolver, all already subtracted from the backlog by earlier PRs. ## What this adds ``` SYNC-RESOLVED files (conversions here may be INERT): 5 `resolveTaskWorkflowIrSync` answers with the DEFAULT workflow in production, so a guard resolved through it behaves exactly as the literal did. Counts are REMAINING literals; a count of 0 is the WORST case, not the best — the file reads as fully converted. 2 packages/engine/src/scheduler.ts 0 packages/core/src/store.ts 0 packages/core/src/task-store/task-store-helpers.ts 0 packages/core/src/task-store/workflow-task-create-ops.ts 0 packages/engine/src/replan-target.ts ``` Two deliberate choices: - **Scans every censused file, not just those with remaining literals.** A file converted *entirely* through the sync resolver has zero remaining and would be invisible — which is exactly the case worth surfacing, because it reads as 100% done. Four of the five are at zero, including `replan-target.ts`, which the ratchet's own notes record as found only by the ratchet. - **A warning, not a subtraction.** Attributing individual guards to the resolver needs dataflow this parser doesn't do, so the honest output is "this file contains a sync call site, conversions in it may be inert" rather than a precise number wrong in the other direction. ## Verification - Totals and `--json` **byte-identical** before/after (stash-compare: 47 guards, 132 deliberate both ways) — the section is purely additive - Census suites green: 53 tests + the node-test fallback suite - Regex requires a *call*, not a mention, so the many files discussing this in prose don't trip it ## Why it matters to the program You steer by the backlog number. Right now a real conversion and an inert one are indistinguishable in it, and my own reports of "20 / 45 / 74 sites resolved" were computed that way. This makes the ambiguity visible at the point of measurement instead of relying on a reviewer remembering the PG caveat. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0e4a559a0a |
test(census): make the tighten fixture self-maintaining instead of pinned to committed state (#3106)
## What broke, and why it will break again The two cases in this block assert the CLI tightens an inflated allowance **by exactly the inflation**. That arithmetic only held while the *committed* baseline matched the tree — so it broke the moment a fleet PR took `self-healing.ts` from 26 to 22 without re-recording. The CLI correctly tightened to 22 while the fixture expected 26, and both cases went red for a reason that had nothing to do with the code under test. #3101 fixed that instance by committing the number. **This fixes the class.** ## Why it recurs The census **exits 0 on a drop** — deliberately, so one worker's merge can't redden the gate for everyone else. The cost is that the committed baseline goes stale *silently*: every run rewrites the file, prints `COMMIT IT`, and exits 0. This fixture is what eventually trips over it. With a fleet actively converting the largest file (eight open PRs against `self-healing.ts` as I write this), that's a recurring red, not a one-off. ## The change The fixture syncs its temp copy to the tree with `--strict --update-baseline` **before** inflating. The assertion is then about the CLI's behaviour rather than about what happens to be recorded on disk. ## Differential proof, both directions Against an artificially staled baseline (22 → 26): | | result | |---|---| | with this change | **40 passed** | | without it | **2 failed / 38 passed** | So the fixture now tolerates drift it previously broke on — and still fails if the CLI stops tightening, which is the property it was written to guard. That second half matters: a fixture made tolerant of everything would be worse than the flake. The CLI invocation is extracted to a `runCli` helper so the sync run and the assertion run share one path. No behaviour rides on that extraction. ## Measured | check | result | |---|---| | census suite, clean tree | 40/40 | | census suite, staled baseline | 40/40 | | `--strict` | exits 0, no residual drift | Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
39e6891c93 |
chore(core): mark the dead sync-path lane literal DELIBERATE-LITERAL (census 104→102) (#3060)
Fleet phase. Claimed `packages/core/src/task-store/project-store-ops.ts` — the largest census file with no branch, worktree, or open PR against it. Claim published by pushing the branch **before** starting work. ## Census before / after | | total | this file | deliberate | |---|---|---|---| | before | **104** | 2 | 130 | | after | **102** | 0 | 130 | `--strict` exits 0, baseline re-recorded in the same commit. **Reclassification, not conversion** — the line is unchanged. ## The site was already audited today, in prose the tool cannot read ``` FNXC:WorkflowLifecycleColumns 2026-07-31-02:45 (audited — DEAD SYNC PATH, do not convert): … It is the SQLite-mode twin. The live path is `dequeueMergeQueueOnColumnExitInTransaction` … and it is ALREADY converted … This body reaches for `store.db.prepare`, which throws in PostgreSQL backend mode … ``` The reasoning is sound and I did not second-guess it: the live path is converted, this twin cannot execute in production, and converting it would mean threading a lane set into a function whose first statement throws. The problem is purely mechanical — **the note is prose, and the census reads markers.** So the site stayed in `byFile` looking like unconverted debt, and each fleet pass pays to re-derive the same conclusion. Adding `DELIBERATE-LITERAL` moves it to `deliberateByFile`, where a reviewed-and-kept literal belongs. ## This is the second one, which makes it a pattern Same shape as #3056 (`async-mission-store-queries.ts`, fallback arms). Across the files I have checked this phase — `agent-store`, `github-tracking-state`, `planner-overseer`, `auto-merge-finalization`, `async-mission-store-queries`, and this one — **every site was either a fallback arm or an already-documented deliberate leave**, and `agent-store.ts:236` carries its own "FLAGGED AND LEFT COUNTED" note from today. So the count is not a work queue, and the gap is not judgement — previous passes reached the right answer. They recorded it where only a human reader would find it. Two lines of marker per site closes that, and the number then means "conversions owed", which is how every worker reads it when picking a cluster. ## Verification - `census --strict` exit 0; `tsc --noEmit` **0 errors** - `check:fnxc-future-dates`, `check:lane-wiring`, `check:sql-column-literals`, `check:inert-flag-seams` — all exit 0 - No behaviour change: only a comment added Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0da19f7963 |
fix(core): a renamed archive lane was recorded as done in the eval corpus; flag the scheduler's two honest literals (#3100)
Two pieces, both about the same distinction: which literals are worth **converting** and which are worth **naming**. ## Converted — the eval corpus was mislabelling renamed archive lanes `collectDeterministicSignals` writes `column` as a two-value eval-record field. Against the `archived` literal, a card resting in a renamed archive lane was recorded as `"done"`. No crash, no lifecycle decision — a **mislabelled row in the eval corpus**, which is a dataset every later comparison reads. That is the expensive kind of quiet: nothing fails, the numbers just drift. The collector is sync and pure (no store, no workflow), so the lane answer arrives as an optional parameter. `HybridEvaluatorService.evaluateTask` is async and already holds an optional store, which is where the resolution is paid; a store-less evaluator degrades to the legacy literal rather than failing. **Only the archived arm was ever wrong.** A renamed *complete* lane was, and remains, recorded as `"done"` — which is correct. So only that answer is resolved, and a third case pins that the widening did not turn every renamed lane into `"archived"`. ## Flagged, not converted — the scheduler's two honest literals These are the two `scheduler.ts` literals the sync-lane pass did not take, and **nothing in the file said why**. That silence is the problem: the obvious next move is to "finish the job" the way the other ten were converted, and that would make them **inert, not fixed**. `getTaskWorkflowSelectionImpl` returns `undefined` unconditionally under PostgreSQL, so `resolveTaskWorkflowIrSync` always answers with the default builtin IR — proved in `postgres/sync-workflow-ir-is-always-default.pg.test.ts`, and `check-inert-sync-lane-conversions` already baselines **twenty** guards in that state in this same file. They stay literal and **counted**, which is the honest state. An unconverted literal is visible to the census; an inert conversion leaves the backlog and takes the evidence with it. The note names the real blocker — a sync-capable workflow-selection reader — so the next pass does not spend a cycle discovering this the way I did. ## Measured - 3 new cases in `eval-signal-collector.test.ts` — file **5/5 pass**. - **MUTATION**: restoring the `archived` literal fails the renamed case and leaves **both** the legacy control and the renamed-complete negative green. The negative matters here: the fix must not turn every renamed lane into `"archived"`. - core eval suites — **4 files / 20 tests**; engine scheduler + evaluator — **14 files / 143 tests**. - `tsc --noEmit` clean in both packages; census `--strict`, `check-lane-wiring`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean. ## Census Both files keep their counts, deliberately: - `eval-signal-collector.ts` — the remaining entry is the new parameter's documented default, which is the fallback doing its job. - `scheduler.ts` — the two literals this PR deliberately leaves visible. A census that fell here would mean the flags had been marked exempt, which would assert the code is fine. It is not fine; it is blocked, and those are different claims with different expiries. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f1e96f7a17 |
test(engine): pin the agent-link-drift terminal check on a renamed board (an agent stayed linked to finished work) (#3102)
Second of the two uncovered sweeps I flagged when #3078 merged. Not a conversion — the conversion is already on main (`driftedTerminalColumns`, landed by another fleet PR). This is the coverage it shipped without. ## What was unprotected Every existing case in this file uses `done` or `archived`, where the literal is correct. So the terminal check had **no renamed-board case at all**, and the same measurement that caught #3078 applies: a green file proves nothing about a conversion whose fixtures can't express the failure. What the literal cost on a renamed board: **a durable agent stayed linked to a finished task forever.** A linked agent is not free to pick up new work, so the drift this sweep exists to clear is exactly the drift it stopped clearing. ## Measured, both directions | case | result | |---|---| | agent linked to a task in a RENAMED complete lane is cleared | **fails on revert** — `taskId` still `"FN-9"`, agent pinned to finished work | | agent linked to a task still in a RENAMED wip lane keeps its link | passes either way — the sweep must narrow, not widen | 14 pass on current main; reverting the terminal check to the id pair fails exactly one. ## Note on the fleet This sweep was converted by someone else's PR while I was writing the test for it — I found out because my revert probe hit `driftedTerminalColumns`, a name I did not write. That is the collision pattern working in a *useful* direction for once: their conversion, my coverage, no duplicated code. It also means the two of us independently chose the same sweep from a 7-PR pileup on this file. Assigning files from the census list would still be cheaper than discovering the overlap in a test harness. ## Verification `self-healing-agent-link-drift` **14 passed** · `pnpm test:gate` 13 + 161 + 487 + 71 · lint — green. |
||
|
|
827386dde6 |
test(core): the sync IR path is blocked TWICE, not once — every note in the repo undercounts it (#3103)
Every remaining census cluster I could not convert — `executor.ts` (4),
`scheduler.ts` (2), `triage.ts` (1), and the four-guard fan-out I
withdrew from my own PR — is waiting on the same thing. So I went to
unblock it, and found the record is wrong.
## The repo says one blocker. There are two, plus a constraint
The call-site allow-list header, the live-PG proof, and a dozen FNXC
notes across engine and core — **several of which I wrote** — all say:
`resolveTaskWorkflowIrSync` is inert because the sync selection reader
returns `undefined`, and the fix is "a sync-capable workflow-selection
reader".
That understates the work by half, and the undercount is load-bearing:
it makes the unblock read like a caching job, so the next person ships a
selection cache and finds the rest at integration time.
### Blocker 2 — the IR read is dead too
`resolveTaskWorkflowIrSyncImpl` loads a **custom** workflow's IR through
`store.db.prepare("SELECT ir FROM workflows WHERE id = ?")`.
`TaskStore.db` is not "SQLite-only". Its implementation (`dbImpl`,
`task-id-integrity.ts`) is an **unconditional throw with no mode branch
at all**. That read always throws into the surrounding `catch`, which
always returns the default IR.
The consequence is precisely the one this program cares about:
| workflow kind | after a perfect selection reader |
|---|---|
| built-in | resolves — that branch never touches `store.db` |
| **custom** | **still the default IR, always** |
**A renamed lane is by definition a custom workflow.** So the sync path
cannot serve the renamed-board case *at all* until this second read is
replaced. Fixing the selection reader alone would produce a change that
looks like it works — on default boards.
### Blocker 3 — a node-local cache is unsafe here
Not a bug; a constraint that bounds the fix's shape. Multiple Fusion
nodes run their own engines against **one shared PostgreSQL**
(`docs/multi-project.md` → "Shared Postgres multi-node runbook").
A node-local synchronous cache of `task_workflow_selection` therefore
goes stale whenever *another node* rewrites a selection — and answers
with full confidence. That is **worse than today's default**, which is
at least uniformly wrong rather than intermittently wrong. Any sync
reader needs an invalidation story that survives a writer on a different
host.
## Why a test rather than a comment
A comment saying "db always throws" decays the moment someone adds a
mode branch, and the whole argument silently inverts — which is the same
decay mode this conversion program keeps hitting with allow-list entries
and stale notes.
The assertions are deliberately about `dbImpl`'s **source** rather than
a call. Calling it proves one construction path throws; the fix depends
on the stronger claim that **no mode returns a database**. Reintroducing
an `if`/`return` there fails the test, which is the correct outcome: the
premise really has changed and the file must be re-read.
## Measured
- 4 new cases pass; the allow-list file's own 7 still pass with its
corrected header.
- **MUTATION**: adding a mode branch to `dbImpl` fails the first case.
- An **anti-vacuity** case pins that the resolver is still live and
still allow-listed, so these source assertions cannot keep passing after
the concern is deleted.
- `tsc --noEmit -p packages/core` clean; census `--strict`,
`check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean.
## Census
**No movement — this converts nothing.** It corrects the record about
what the remaining conversions are waiting on, and it corrects notes I
authored. I would rather spend a PR making the next attempt cheap than
leave a half-true blocker in place that costs someone a full cycle to
rediscover.
## What I did not do
I did not build the sync reader. With blockers 2 and 3 in view it is a
store-substrate change — a second read to replace, and an invalidation
story that survives a writer on another host — not a fleet conversion,
and starting it mid-sweep on a shared file would repeat the collision
pattern that has already cost this branch three rebuilds. It remains
unclaimed, and now it is fully specified.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
d448ab6951 |
fix(engine): the merge-refusal reason was classified by a column id, and it lands in run-audit (#3098)
Claimed `auto-merge-finalization.ts` — and this one is a **reversal of
an earlier audit in the same file**, which is the interesting part.
## The earlier note said "diagnostic only". It was wrong about the
consequence
`validateWorkflowDoneMergeProof` picks between two refusal reasons with
`task.column === "done"`. Both arms return `{ ok: false }`, so this
never changed which branch ran — and on that basis a prior pass recorded
it as *"REAL but DIAGNOSTIC-ONLY"* and declined it, reasoning that
widening a signature to improve an error string is a poor trade.
**The reason is not an error string.** It is written to run-audit
metadata alongside `previousColumn` — `merger-merge-lifecycle.test.ts`
asserts exactly that — and that row is what an operator reads to find
out why a merge was refused.
So on a board whose complete lane is not called `done`, a card resting
in that lane was refused with the generic `missing-merge-confirmation`:
the classification for a card that is **not in the complete lane at
all**. The audit trail recorded the opposite of what happened. A wrong
record is worse than a vague one, because it gets acted on.
## The trade was also cheaper than the note claimed
The function is **already async** and **already takes an options bag**.
`resolveFinalizationColumns`, two functions up in the same file,
**already builds this exact predicate** for its own guard.
Nothing new is resolved. The answer that existed is handed down instead
of being re-asked with an id — the half-conversion shape this program
keeps finding, here inside a single file, one line apart: the caller
guards on the resolved `isCompleteColumn(latest.column)`, then calls a
validator that re-asked the same question with the literal.
`isCompleteColumn` is **optional with the legacy literal as its
default** — the same default-to-legacy contract the lane-parameter
vocabulary uses elsewhere — and `check-lane-wiring` watches the
parameter, so the two call sites cannot silently stop passing it.
## Measured
- New `merge-proof-reason-renamed-complete-lane.test.ts` — **2 pass**.
- **MUTATION**: dropping the parameter fails the renamed case and leaves
the legacy **control** green. The control earns its place: a failure now
means *"renamed board"*, not *"the refusal stopped working"*.
- **Driven through `finalizeProvenAutoMergeTask`**, not by calling the
validator with the new argument. The contract under test is the
**wiring** — a test that passed the argument directly would assert my
own parameter works and prove nothing about the seam that was broken.
- **The audit row is asserted, not just the return value.** The return
value alone is not the contract that failed here.
- merger / auto-merge suites — **5 files / 159 tests pass**.
- `tsc --noEmit -p packages/engine` clean; census `--strict`,
`check-lane-wiring`, `check-inert-sync-lane-conversions`,
`check-fnxc-future-dates` clean.
## Census
`auto-merge-finalization.ts` stays at **2**, deliberately. Both
remaining entries are now documented **degraded-fallback arms** — the
resolver's `catch` and this parameter's default — which is the right
kind of literal rather than a missed conversion. Converting a fallback
to a resolution would defeat its purpose.
## A note on the FNXC gate
My first stamps were dated `2026-08-01` while local today is
`2026-07-31`. `check-fnxc-future-dates` caught it and I re-stamped.
Worth mentioning because it is the second time this session that a
date-only local-calendar comparison has caught a stamp written near
midnight — the gate is doing real work, not ceremony.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
dca67a79a3 |
fix(census): a DELIBERATE-LITERAL marker on a ternary arm was invisible (#3099)
## The gap The marker works above a statement or a function. It does **not** work in the position people actually use it — on the fallback arm itself, right beside the literal it excuses: ```ts flags ? flags.hold === true /* DELIBERATE-LITERAL — the no-metadata fallback. */ : column === "in-progress"; ``` That comment sits **before the `:` token**, so it's the colon's leading trivia rather than the arm expression's — `getLeadingCommentRanges` at the arm's full start never sees it. The ancestor walk doesn't rescue it either: the next ancestor is the `ConditionalExpression`, whose own leading comments are somewhere else entirely. ## Measured, not hypothesised `in-review-stall.ts` carried a marker in exactly this position **and stayed on the backlog**. The only way I could clear it was to restructure the code into a named set (#3064). That's the tool dictating shape rather than reading intent — and a marker that silently does nothing trains people to stop marking. Given this fleet phase has had several workers reach for `DELIBERATE-LITERAL` (#3056 used it successfully at statement level), the failure mode is one worker's marker working and another's not, for reasons neither can see. ## Scope and controls Scoped to the span between the previous arm (or the condition) and this one, so it can't pick up a comment belonging to anything else. Verified both directions with a probe: | case | before | after | |---|---|---| | marker above the statement | `deliberate` | `deliberate` | | marker on the ternary arm | **`column`** | `deliberate` | | unrelated marker on a neighbouring statement | `column` | `column` (unchanged) | **The real tree's count is unchanged at 51** — nothing is silently reclassified, because the one site that had an arm marker was already converted away. This is forward-looking. ## Not fixed here — pre-existing red on `main` `lifecycle-column-census.test.ts` has two failures (`expected 22 to be 26`, `expected +0 to be 1`) whose fixture arithmetic drifts with real tree counts as fleet conversions land. **Verified identical on `origin/main`** before and after this change, so it isn't mine — but someone should decide whether that fixture ought to be derived rather than pinned, since every fleet merge moves it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a7b2a757fa |
fix(engine): serialise wedge handling per task, then convert the lane guards it was blocking (5 → 1) (#3087)
The largest unclaimed census cluster, and the one two earlier fleet passes explicitly declined. ## The standing blocker, taken on Both passes converted these four ids and reverted, each time after the same test went red: ``` task-wedge-notification.test.ts > sends one actionable push and mailbox message per active terminal episode expected 2 calls, got 1 ``` Their diagnosis was right and I have kept it: this branch **resolves** a wedge episode, `handleTaskUpdated` starts it fire-and-forget from a synchronous `(task) => void` listener, and **any** await introduced before the resolve lets a re-wedge arriving close behind reach `claim` while the previous episode is still active — `claimed: false`, second operator notification silently dropped. Column resolution needs an await, so the conversion could not be made safe from inside the branch. Both notes named the fix and left it for "whoever owns the wedge episode contract": *serialise wedge handling per task*. This PR does that, then takes the conversion. ## 1. Serialisation `enqueueWedgeHandling` chains handling per task id, so resolve-then-claim keeps its order however many awaits either branch acquires. Details that matter: - **Keyed by task, not global** — different tasks stay concurrent, so this is not a throughput regression on a busy board. - **The map entry is dropped when its chain drains**, and only if no later link was appended while it ran, so it does not grow with the task table. - **Links never reject.** `maybeNotifyTaskWedge` already owns its error handling; a rejected link would poison every later notification for that task. ## 2. The conversion it was blocking The four ids are an enumeration of *"every lane except review"* — the lanes whose occupancy proves a wedged card's lifecycle has visibly resumed. On a renamed board none of them matched, so a recovered card's episode never resolved. Two consequences, and the second is worse than the first: 1. the operator keeps an open "needs operator action" alert for work that has moved on; 2. an active episode **suppresses re-claim**, so the *next* genuine wedge on that task is never delivered. Membership over the four roles, legacy-seeded, so an unconverted board resolves exactly the four ids it used to compare. ## Measured **The acceptance test the earlier notes named is the gate on both halves.** With the conversion and *without* the serialisation, "sends one actionable push and mailbox message per active terminal episode" fails exactly as they reported. With the serialisation, green. I reproduced their finding rather than taking it on trust — it is the evidence that the serialisation is load-bearing and not incidental refactoring. | | result | |---|---| | `task-wedge-notification.test.ts` | **15/15** (2 new) | | notification suites | **11 files / 234 tests pass** | | `tsc --noEmit -p packages/engine` | clean | | census `--strict`, `check-lane-wiring`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` | clean | **MUTATION**: restoring the four literals fails the renamed-recovery case and leaves its paired negative green. **A vacuity I caught and fixed, worth stating plainly.** My first version of the renamed case recovered the card with `status: "queued"`. `hasProgressed` is an OR whose other arm is *"status is a non-failed string"* — so that arm answered true and the column comparison never ran. The mutation did not fail it. The case now clears `status` and `error` together, which makes column membership the only thing that can resolve the episode, and the paired negative uses the identical shape so only the lane differs. ## Census | | before | after | |---|---|---| | `notification-service.ts` | 5 | **1** | | repo backlog | 71 | **67** | ## The remaining 1, flagged not guessed `isManualMergeHold` (`task.column !== "in-review"`) is sync, and so is its only caller `classifyWorkflowTransitionNotification`, reached from the same `handleTaskUpdated` listener. Converting it means making that whole chain async — a change to notification *classification ordering* against every other `task:updated` handler, which is a different contract from the episode one this PR owns. The serialisation added here does not cover it: it wraps wedge handling, not transition classification. Threading a pre-resolved `LifecycleColumns` in as a parameter is the likely fix, and it wants the same gate-placement judgement applied deliberately rather than swept in behind this. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
701677a2e5 |
test(engine): pin #3078's executor-owned skip — it merged without coverage (204 tests passed against the reverted fix) (#3090)
#3078 merged its conversion of the orphaned-pending-step-results sweep **before this test landed**, so that sweep is on main with no coverage. This closes the gap. ## The gap was measured, not assumed With all three of #3078's conversions reverted, **all 204 self-healing tests still passed**. I had cited that number as verification when I opened it. It was meaningless for that change: every existing test in the file uses `in-review` / `in-progress`, where the literal is correct, so none of them could see the defect. This is the same "a green suite is not coverage" failure I flagged in other PRs today — in my own work, twice. The only reason I caught it is that I finally ran the revert check on myself. ## Two cases - **An executor-owned card in a renamed wip lane is SKIPPED.** Against the pre-#3078 sweep this fails: the sweep reaches a card an executor is actively running and rewrites its `pending` step results to `failed` — the one thing that file's header says it must never do. The liveness triple does not cover it; those legs prove an *in-process* session, and an executor on another node or between session handles is exactly what the column skip is for. - **A genuine orphan on that same renamed board is still recovered** — the skip must narrow, not disable. Passes either way, deliberately. ## What it pins, precisely The **invariant**, not a line. Reverting either single guard still passes, because the page-snapshot check and the fresh-row re-read protect independently. What fails is reverting the sweep's column handling as a whole — which is the condition worth pinning, and matches the project's "fix the invariant, not the repro" rule. ## Still uncovered, said plainly #3078's other two sweeps — worktree-metadata liveness and agent-link drift — have no dedicated case. The orphaned-step-results sweep got the test first because it is the one that can corrupt a live executor's state. The other two remain honest debt rather than implied coverage. ## Verification `self-healing-orphaned-pending-step-results` **10 passed** on current main · full self-healing suites 204 · `pnpm test:gate` 161 + 13 + 487 + 71 · lint — green. |
||
|
|
f7a7347e1b |
test(core): cover #3057's cold-storage conversion; audit two archived literals as dead sync (#3089)
**Replaces #3085, which I am closing.** #3057 landed the same cold-storage conversion while that PR was open. Rather than argue about which spelling wins, this keeps only what `main` does not have: the coverage, and two audits. ## #3057 converted this and shipped no test `listTasksImpl`'s `columnFilterIsArchive` replaced `columnFilter === "archived"`. Correct change — and the kind that needs a test more than most, because of how it fails. Archived rows do not live in `tasks`; `archiveTask` copies them into the archive store and removes them. This decision is whether that second store is read **at all**. Against the literal, a caller naming a renamed archive lane — `listTasks({ column: "filed", includeArchived: true })`, which is what an archive view does — got an empty page from the only API that can reach those rows. Note the shape: the **unfiltered** read (`!columnFilter`) was always correct. It fails only for the caller that names the lane, so it survives any board-level smoke test and presents as *"the archive is empty"* rather than as a bug. That is precisely the class that regresses quietly once the conversion that fixed it has nothing holding it. Three cases, and each earns its place: | case | what it stops | |---|---| | renamed archive lane | the regression itself | | legacy `archived` id (**control**) | a future conversion that resolves the renamed lane and *drops* the legacy seed — the seeding hazard in its other direction | | non-archive lane (**negative**) | the widening turning every filtered board read into a second-store round-trip | **MUTATION**: restoring `columnFilter === "archived"` fails **only** the renamed case. **A trap worth recording.** My first version left the LIVE read real against a fake `layer.db`. It threw, the `.catch` swallowed it, and all three cases passed the negative — *including the legacy control*, which is what exposed it. A test whose subject is never reached looks identical to one whose subject answered no. `readLiveTaskRows` is mocked now, and the control is what made it detectable. ## Audited, not converted — two dead sync paths Both would be real defects if they ran. Neither runs. | site | why it is dead | |---|---| | `mission-store.ts` (feature-delete link check) | `getMissionStoreImpl` returns the AsyncDataLayer-backed `AsyncMissionStore` under PostgreSQL; the sync `MissionStore` reached via `this.db.prepare` is legacy SQLite only | | `lifecycle-ops.ts` (polling-replica archive emit) | `checkForChangesImpl` opens with `store.db.getLastModified()` / `store.db.prepare`, which throw in backend mode | The second is the sharper one: **both** its guard and the `to: "archived"` it emits are literals, so a polling replica on a renamed board would emit a move to a column the board does not declare. Recorded in place — the treatment `project-store-ops.ts`'s dequeue twin already has — so the census entries are not mistaken for unconverted debt, and whoever deletes the sync SQLite residue takes these with it. **They stay COUNTED.** Marking them DELIBERATE-LITERAL would buy a smaller number by asserting the code is *correct*. It is not correct; it is unreachable. Those are different claims with different expiries, and the census should keep pointing here until the code is gone. ## Census No movement — by design. This PR adds coverage and audits; it converts nothing that was not already converted on `main`. ## Measured - 3 new cases pass; `src/__tests__/{cold-storage,archive,unarchive}*` — **6 files / 21 tests pass** - `tsc --noEmit -p packages/core` clean; census `--strict` clean Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6949f22ef8 |
fleet: resolve the same-column handoff review target (census 84 → 83) (#3076)
## Census | | column guards | |---|---| | before | **84** | | after | **83** | `moves.ts`: 2 → 1. One of its two sites converts; the other **must not**, and that difference is the useful part of this PR. ## Converted — the move target at the same-column handoff ```ts if (internal.fromHandoff && toColumn === "in-review") ``` Against the literal this **never fired on a renamed board**, so a same-column handoff into a renamed review lane silently took the *other* branch — the sync-SQLite path, which throws under PostgreSQL. It now asks `moveReviewColumns`: the broad membership set (`mergeOrchestration ∪ mergeBlocker ∪ humanReview`) already resolved **three lines above** for the merge-queue pair. Same value, so this branch cannot disagree with the enqueue/dequeue calls that receive it. ## Not converted — the archived fallback arm I named it, and `archived-column-gate-parity.test.ts` went red on **`TypeScript encoding changed`**. That guard's argument holds: the archived gate is enforced in three encodings, the SQL halves still compare the raw string, and moving the TypeScript half alone is the split brain it exists to prevent. Restored inline **with a note recording the measurement**, so the next person doesn't retry it and rediscover the same red. This is the second time that guard has stopped me this session. It's doing exactly what it was built for. ## On the pre-existing red That suite is red on `origin/main` for an unrelated raw-SQL drift (#3072 fixes it — the drift is from my own merged #3042/#3046). I verified this branch produces the **identical** failure and no other, so it doesn't compound it. ## Measured | check | result | |---|---| | moves / handoff / merge-queue suites | green | | four gates + strict census | green | | core `tsc` | clean | | parity suite | same single raw-SQL failure as `origin/main`, nothing added | Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9e242ea294 |
fix(engine): backlog pressure called every dependency unfinished on a renamed board (#3081)
## The third lane question This reporter had **three** lane questions. Two were resolved when the file's query-blindness was fixed — hold and wip, both through `resolveProjectColumnsForRoles`. The third sat one method down and was never touched: ```ts if (dependency.column !== "done") return false; ``` One board, two lane answers. ## What it cost On a renamed board every dependency reads unfinished, so `isRunnableCandidate` rejects every card that has one. The backlog-pressure alert then names **only dependency-free cards** as the runnable ones. The failure mode is the quiet kind: the report still renders, the counts are right, and the candidate list looks plausible. The operator is told the queue is blocked on nothing in particular. No default-board test can see it — which is exactly why the earlier conversion of this same file, which fixed its reads, left this behind. ## Fix `finishedColumns` (complete ∪ archived) resolved once by the async caller alongside hold and wip, then passed into the sync predicate. - **Required parameter, not optional-with-a-literal-default.** An optional parameter leaves `done` in the file as a silent fallback and the next caller gets pre-conversion behaviour by writing nothing. - **Archived is included** because a dependency that has been archived is finished too — and this reporter already reads with `includeArchived: true` precisely so archived blockers resolve. - **Async resolution.** `resolveProjectColumnsForRoles`' only store read is `listWorkflowDefinitions()`, a project-wide async read that works under PostgreSQL. That is the line between a real conversion and the inert sync-IR kind (#3058), and the new test supplies its board through that same reader so it exercises the production path. ## Census | | before | after | |---|---|---| | `backlog-pressure-reporter.ts` | 1 | **0** | ## Measured - One new case; file **11/11 pass**. - **MUTATION**: restoring `dependency.column !== "done"` fails it. - The case asserts **both directions in one test** — a dependency resting in the board's own complete lane makes its card runnable, *and* a dependency still in the hold lane still blocks it. Asserting only the first would pass against a predicate that had simply stopped checking dependencies. - The file already had a `RENAMED_IR` scoped to its second describe; mine is a distinct `RENAMED_DEPENDENCY_IR` with different lane names. I hit the shadowing first and the test failed as `under-threshold` — worth noting because a same-named fixture that silently resolves to the *other* board is precisely how a renamed-lane test goes vacuous. - `tsc --noEmit -p packages/engine` clean; census `--strict`, `check-lane-wiring`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean. ## Flagged, not guessed Adjacent census entries I looked at and deliberately left: - **`executor.ts` (4)** — all inside a sync `task:moved` listener. Converting via `resolveTaskWorkflowIrSync` would be inert for #3058's reason, and making the listener async reorders it against every other subscriber. Correctly out of scope, as #3048 judged. - **`triage.ts:724`** — half-converted in the same shape: `disposeLanes.hold`/`.intake` come from a sync resolver, so the resolved arms are themselves inert and "finishing" the guard would add a third inert comparison. - **`auto-merge-finalization.ts` (2)** — one is the resolver's documented degraded fallback (the live arm calls `columnHasFlag`), the other is already recorded as a deferred signature-widening whose cost exceeds the error string it sharpens. - **`in-review-stall.ts:196`** — an explicitly marked DELIBERATE-LITERAL no-metadata fallback. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3c531d984c |
fix(engine): self-healing lane cluster round 2 — 38 → 26 (two sweeps could disturb live work) (#3078)
The largest census cluster became **unclaimed again** when #3055 closed conflicting. I had closed my own #3050 an hour earlier expecting #3055 to land, so this re-applies the conversions #3047 and #3049 did not cover. **Re-applied from current main rather than rebasing the closed branch.** The conversions are small; the conflict archaeology is what went wrong last time — nine conflicts against #3049, several on variable names identical to mine, and my mechanical fixup corrupted the file badly enough that I aborted. Starting from main cost less than resolving that and carries no risk of resurrecting a stale line. ## Census | | before | after | |---|---|---| | `packages/engine/src/self-healing.ts` | **38** | **26** | | repo-wide column guards | 84 | **72** | ## Three sweeps, existing role helpers only | sweep | roles | what it did on a renamed board | |---|---|---| | worktree metadata | terminal + wip + review | rebound finished cards every pass, **and the FN-5256 liveness guard went silent** | | orphaned pending step results | wip | **could rewrite `pending` results under a live executor run** | | agent-link drift | wip + review + terminal | evaluated agents whose task was plainly still executing | Two of these disturb **live** work, which is why they were worth redoing now rather than leaving for the next fleet round: - The worktree-metadata sweep clears `worktree`/`branch` metadata. Its liveness guard is the thing standing between that and a running shell (FN-5256). Keyed on ids, it matched nothing on a renamed board. The scope-override safety condition beside it now reads the **same resolved sets**, so the two cannot disagree about which lanes are live — previously they were two independent literal lists. - The orphaned-step-results sweep's own header says it must never touch an executor-owned row. The id-keyed skip made it do exactly that. Resolved once per sweep, outside the paging loop, so a large board still pays one resolve. ## Flagged, not guessed — the 26 that remain Unchanged from my earlier audit and re-verified on this base: - **Sync predicates** (`isWorkspaceOwnerLive`, the pause-abort classifier, the phantom-binding check, the `task:moved` listener guards). No store handle; converting means a signature change or making a synchronous event listener async, which reorders handlers against a synchronous emitter. - **Already-converted fallbacks** — `own.length > 0 ? own.includes(...) : task.column === "in-review"`. The resolved answer wins; the literal is the documented no-metadata path. - **The notification-route `fresh.column === "todo"` sites** — measured previously: any `await` before the wedge resolve drops an operator notification. Needs the wedge-episode contract, not a column pass. ## Verification self-healing suites **204 passed** · agent-link-drift + query-filter-blindness **83 passed** · `pnpm test:gate` 161 + 13 + 487 + 71 · lint · census `--strict` · lane-wiring — green. |
||
|
|
58791fac88 |
fix(self-healing): route pause-abort recovery on resolved columns (self-healing 38 → 34) (#3075)
First real cut into `self-healing.ts`, the last large cluster. Converts the **pause-abort recovery router** — a coherent unit with one owner, rather than a scattered pass. ## Census before/after | Metric | Before | After | |---|---:|---:| | COLUMN guards (backlog) | 86 | **82** | | `self-healing.ts` | 38 | **34** | ## The bug this was hiding The router keyed on three literals — `in-review` twice (review progress, manual merge hold) and `todo || in-progress` (active work). On a renamed board **all three stop matching**, so a parked card in a renamed lane falls through to `no-action` and is never recovered — silently, no log line, and with every existing test still green because they all use legacy ids. ## Conversion Used the file's own resolvers. `resolveReviewColumnsFor` already existed; added `resolveActiveWorkColumnsFor` as its sibling from the same `columnsWithFlag` / `resolveLifecycleColumns` helpers — no new vocabulary. **ACTIVE WORK is hold + `countsTowardWip`, deliberately NOT the intake + hold that the neighbouring `resolvePreWipColumns` returns.** The router asks *"is this card mid-flight, so a requeue is right?"* — an intake lane is not mid-flight; a WIP lane is. Reusing the intake-shaped helper would have widened the requeue to triage rows and dropped in-progress ones. Because the two sets **overlap on `hold`**, that mistake looks correct in every legacy-id test. This is the trap worth knowing about for the rest of the file: it has several resolvers, and picking the nearest one is not the same as picking the right one. **`columns` is required, not optional-with-a-fallback.** The router has exactly two callers — the candidate filter and the post-re-read re-verify — and they must agree. An optional parameter lets one resolve and the other default, and that divergence surfaces as a sweep that selects a card and then declines to act on it, writing nothing. Required makes it a compile error. (Same reasoning as #3059; safe here because both resolvers union the legacy ids internally, so "required" never means callers invent a column set.) **On the filter/re-verify hazard I flagged earlier:** both call sites are the *same function*, so converting it once keeps them consistent by construction — no split-CAS risk. Per-task resolution can't hoist out of the loop but must not read an IR per row, so the marker test (column-independent) stays a cheap sync prefilter and the IR is read only for rows that pass it, over a shared cache. `parked` keeps its exact former membership, so the log count still means what it said. ## Verification - **Revert-proof:** reverting the three guards to literals fails 2 of 3 routing cases. The third exercises the new resolver directly, so it cannot fail on revert — stated rather than counted as evidence. - 4 self-healing suites, **437 tests green** - `tsc --noEmit` clean; eslint clean - Degraded-resolution case included, since the legacy-id union is what keeps recovery alive on an unreadable workflow ## Still flagged in this file (not guessed) 34 remain. They are not one batch: the sweeps around L2855/3297/8877/9018 depend on the unowned `listTasks({ column })` decision, and L5330/5359 sit under the FN-5256 liveness guard whose own comment says those columns can be live when the heuristic calls them stale. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
98aac40ca8 |
fleet: reads.ts 2 → 0 lifecycle-column guards (#3057)
> **Rebased.** Main landed another worker's conversion of the review gate while this was open — the overlap was a whole rewritten function, so I reset to main and rebuilt only my remaining delta on top of their work rather than resolving hunks. Their conversion is kept as-is. ## Census | | column guards | |---|---| | before | **104** | | after | **102** | `reads.ts`: **2 → 0**. ## Two changes **1. `includeColdStorage`** asks whether the *caller* is filtering to the archive lane. Against the literal, a caller filtering to a renamed archive lane took the false branch — cold storage was skipped and the filtered view returned only whatever archived rows still sat in `project.tasks`, **a short list presented as the whole archive**. Still literal on main; converted here. **2. Both fallbacks become named sets** instead of inline arms — including the one on the just-landed review gate. ## The second point is the one worth the fleet's attention This is bookkeeping correctness, not style. The census counts an inline comparison **whether or not it sits in a fallback branch**, because its `traitFallback` hint is advisory and never changes `kind`. So a correctly-converted guard with an inline legacy arm **stays on the backlog permanently**, and the number stops distinguishing real debt from documented degraded answers. Concretely: converting with an inline fallback is correct work that scores **zero**. My own first pass at this file did exactly that. There are roughly **12 such sites** across the tree — `github-tracking-state`, `planner-overseer`, `async-mission-store-queries`, `register-task-workflow-routes`, `restart-recovery-coordinator` — and I have that cluster converted and ready to open next. ## Measured | check | result | |---|---| | reads / get-task / stall suites | 5 files, **87 tests green** | | renamed-archive PG suite | green | | strict census | green; `tsc` clean | | unconverted boards | byte-identical — the named sets hold the previous ids | <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Review and archive checks now work correctly with resolved workflow columns while retaining legacy compatibility. * Fresh agent activity is detected in resolved review lanes. * Lists filtered by a resolved archive lane now include archived items stored in cold storage. * **Chores** * Updated internal lifecycle tracking baselines. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3c12a51627 |
fix(core): the merge result reported a column the finaliser did not write (merge-queue-ops 3 → 0) (#3071)
Largest unclaimed census cluster in `packages/core` — three `done`
literals in `mergeTaskImpl`. Two of them produced **wrong state**, not
merely a guard that stopped firing.
## 1. The result overrode the writer
`moveToDoneImpl` resolves the board's completion lane and writes it onto
the task object:
```ts
task.column = completeColumn; // task-artifacts-ops.ts
```
Both merge call sites then did:
```ts
result.task = { ...task, column: "done" };
```
putting the literal back over what the writer had just set. Every
`task:merged` listener — GitHub tracking, the auto-merge handoff — was
told the card landed in `done` while the persisted row said `shipped`.
The row was right and the event was wrong, which is the worse direction:
the listeners act on the event, not the row.
Fixed by reading back what the writer set (`{ ...task }`). Deliberately
**not** a second resolution — that would only be a second chance to
disagree with the finaliser.
## 2. The guard disagreed with the writer
The already-complete short-circuit asked `task.column === "done"`, while
the finaliser it guards short-circuits on the resolved `task.column ===
completeColumn`. On a renamed board those two answers differ, so a card
already resting in the board's completion lane fell through and the
merge ran again against a branch that was already landed and deleted.
Converted with the **same resolution and the same shape** — a single
first-match column, not membership — because the whole point is that
these two answers cannot differ. A workflow declaring no complete lane
resolves to `undefined`, which matches no column; the finaliser refuses
such a board explicitly one function later.
## Census
| | before | after |
|---|---|---|
| `merge-queue-ops.ts` | 3 | **0** |
## Measured
- Two new cases added to `merge-blocker-renamed-review-lane.test.ts`
(same renamed-board fixture, same PG harness) — file **5/5 pass**.
- **MUTATION**: restoring either literal fails **both** new cases and
leaves the three pre-existing ones green.
- Reached with **no git fixture**: with no branch present, `git
rev-parse --verify` fails and the function takes its own documented
*"branch not found — moving to done without merge"* path — which is
exactly the path that calls `moveToDone` and then builds the result. No
repo setup, no flake surface.
- `packages/core` targeted run: **38 tests pass**.
- `tsc --noEmit -p packages/core` clean; census `--strict`,
`check-lane-wiring` ("none added"), `check-fnxc-future-dates` clean.
## Not done here (flagged, not guessed)
The other `done`/`archived` literals still in the core census are each
blocked for a *different* documented reason, so sweeping them into this
PR would have meant guessing:
- `agent-store.ts:236` — a pure formatter over `Pick<Task,"column">`
that prints the column for a human; degrades gracefully and has no store
to resolve from.
- `async-mission-store-queries.ts` — already converted with
caller-threaded lane sets.
- `taskRevert.ts:119` — classifies a **neighbour** task; the only flags
in scope describe the modal's own task, so wiring them would answer the
question for the wrong row. Needs per-neighbour flags.
- `moves.ts:310` — a **refusal**, where a legacy-seeded superset is the
documented hazard rather than the safe direction. Wants its own change
with its own test.
- `mission-store.ts:2332` — a sync SQLite path with no async seam.
Each is real debt; none is a mechanical conversion.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
8eef8852a0 |
fleet: 4 long-tail fallback arms become named sets (census 101 → 97) (#3064)
## Census | | column guards | |---|---| | before | **101** | | after | **97** | The single-guard long tail is **19 files**. This converts the four whose legacy arm is unambiguously a fallback on an already-converted guard; the other 15 are flagged below rather than guessed at. ## Two shapes **`in-review-stall.ts`, `stalled-review-detector.ts`** — the resolved answer with an inline legacy arm: ```ts reviewColumns ? reviewColumns.has(col) : col === "in-review" → (reviewColumns ?? LEGACY_REVIEW_LANES).has(col) ``` **`merger.ts`, `in-process-runtime.ts`** — belt-and-braces: ```ts col !== (lifecycle?.complete ?? "done") && col !== "done" ``` That accepted the resolved lane **or** the legacy id, stated twice. A union set says it once, so the two halves can't drift apart — which is the real risk with a duplicated condition. ## A finding for anyone else marking fallbacks `in-review-stall.ts` **already carried a `DELIBERATE-LITERAL` marker** on that arm and was counted anyway. The marker sits in a comment *inside a ternary*, which the census's leading-comment lookup doesn't reach. So: **naming the set works, marking it does not.** Worth knowing before someone marks a fallback and expects the count to move. ## No behaviour change `new Set(["in-review"]).has(x)` answers exactly what `x === "in-review"` answered, and the union sets accept exactly the two lanes their conditions already accepted. ## Flagged, not converted The remaining 15 single-guard sites need individual judgement, not a mechanical pass: - **plain unconverted guards with no resolution in scope** — `audit-ops`, `lifecycle-ops`, `merge-queue-ops`, `task-id-integrity`, `backlog-pressure-reporter`, `ephemeral-worker-manager`, `ResearchTaskActionModal` - **sites where the literal IS the answer** — `eval-signal-collector` maps a column to an archive-vs-done *label*; `TaskCard` reads a completion timestamp - **already resolved on their line** — `triage.ts`, `restart-recovery-coordinator.ts`, both covered by open PRs ## Measured | check | result | |---|---| | core stall suites | 4 files, **85 tests green** | | engine merger/runtime suites | **1044 tests green** | | five gates + strict census | green | | `tsc` (core, engine) | clean | Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c220455e3a |
fleet: 10 inline fallback arms become named sets (census 102 → 92) (#3061)
## Census | | column guards | |---|---| | before | **102** | | after | **92** | Five files drop to **0** guards each. Baseline re-recorded in the same commit. ## A cluster the census could not distinguish from real debt **Every site here is already converted.** Each reads resolved lanes when it has them and falls back to a legacy id when it doesn't: ```ts reviewColumns ? reviewColumns.has(task.column) : task.column === "in-review" ``` The census counts an inline comparison **whether or not it sits in a fallback branch** — its `traitFallback` hint is advisory and never changes `kind`. So ten correctly-converted guards sat on the backlog permanently, and the number stopped distinguishing *work still to do* from *documented degraded answers*. Naming the fallback set fixes the bookkeeping without touching behaviour: `new Set(["in-review"]).has(x)` answers exactly what `x === "in-review"` answered. ## Files | file | sites | what they gate | |---|---|---| | `restart-recovery-coordinator.ts` | 4 | three shared review gates + one `??` default | | `github-tracking-state.ts` | 2 | complete / archived lane predicates | | `planner-overseer.ts` | 2 | wip / review classification | | `async-mission-store-queries.ts` | 2 | terminal complete / archived | | `register-task-workflow-routes.ts` | 2 | wip promotion target, archived respecify guard | **No behaviour change is claimed and none is intended** — that's the point. These were already right; only the accounting was wrong. ## Worth the fleet's attention Converting a guard while leaving an inline fallback is **correct work that scores zero** on the census. My own first pass at `reads.ts` did exactly that — behaviourally correct, census unmoved. Anyone converting this way is doing real work the number won't credit, and the backlog will look stuck. ## Measured | check | result | |---|---| | engine suites | **173 tests green** | | core mission suites | **70 tests green** | | dashboard route suites | **211 tests green** | | five gates + strict census | green | | `tsc` (core, engine, dashboard) | clean | <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Standardized fallback handling for workflow stages, including in-progress, review, completed, and archived states. * Preserved existing behavior when explicit workflow column settings are available or unavailable. * Improved consistency across task tracking, planning, and recovery workflows. * **Chores** * Updated lifecycle tracking baselines to reflect current source-file coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
befbd299a9 |
fleet: mark 4 reviewed literals DELIBERATE (backlog 88 → 84, comment-only) (#3066)
Comment-only. **No code changed** — 23 lines added, all comments. ## Census before/after | Metric | Before | After | |---|---:|---:| | COLUMN guards (backlog) | 88 | **84** | | DELIBERATE-LITERAL (reviewed) | 128 | **132** | | File | Sites marked | |---|---:| | `packages/core/src/agent-store.ts` | 2 | | `packages/core/src/async-mission-store-queries.ts` | 2 | ## Why marking, not converting Both files already carried prose explaining why their literals are correct. Without the marker the census still counts them as backlog, so the fleet keeps dispatching workers at them — **three separate workers have now independently re-derived the same two conclusions.** An unmarked correct site costs a cycle every time it is re-examined, and the cost repeats for every worker. **`agent-store.ts`** picks a *word* for a human reader, not a lifecycle decision: `(not active — done)` versus `(done)`. It degrades gracefully on a renamed board — falls through to `(<column>)`, still accurate, just less specific. Threading a resolution into a synchronous string builder to choose an adjective is the wrong trade. **`async-mission-store-queries.ts`** are the fallback arms of an *already-converted* predicate, and the undefined branch is a **live intended path**: `AsyncMissionStore.taskStore` is optional, every store constructed without one relies on the legacy ids answering, and the caller's two `resolveProjectColumnsForRoles(...).catch(() => undefined)` calls mean each field can be undefined even *with* a store. That last point is the distinction worth keeping: this is **not** the `restart-recovery-coordinator` shape (#3059), where making a parameter required deleted a production-dead fallback. Requiring it here would force callers to fabricate a column set — inventing a vocabulary rather than resolving one, which is the "guess" the fleet rules forbid. ## One mechanical note for future markers **A marker only excuses the construct it precedes.** My first pass put one comment above `isComplete` and moved 3 of 4 sites — `isArchived`, two lines below, needed its own. Worth knowing before someone marks a block and assumes it covered the siblings. ## Verification - census: backlog 88 → 84, deliberate 128 → 132 - `agent-store-pause-marker-clear`, `agent-store-routing-policy`, `mission-store.sync-auto-merge` — 18 tests green - `tsc --noEmit` on `@fusion/core` clean; `pnpm lint` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified how task-column wording handles renamed board columns. * Documented the fallback to “done” when terminal-column information is unavailable. * No user-facing behavior changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
18fab9b8e5 |
fleet: name the WIP half of an existing fallback (census 88 → 87) (#3070)
## Census | | column guards | |---|---| | before | **88** | | after | **87** | ## What `ephemeral-worker-manager.ts` answers its unresolvable-workflow default two ways, two lines apart: ```ts if (TERMINAL_TASK_COLUMNS.has(task.column)) return true; // named set — not counted return task.column !== "in-progress"; // inline — counted ``` Both are the **same documented fallback** — the block carries one `DELIBERATE-LITERAL` marker covering both — but only the inline one was on the backlog, because the census reads comparisons regardless of which branch they sit in while a set is a definition. Naming it makes the pair consistent and stops the site reading as unconverted debt. ## Correction to my own flag in #3064 I listed `ephemeral-worker-manager`, `backlog-pressure-reporter`, `merge-queue-ops` and `lifecycle-ops` as *"plain unconverted guards with no resolution in scope."* **That was wrong for all four.** Each already imports the resolvers — 7, 4, 3 and 2 references respectively. I wrote the flag without checking, which is the same mistake as an untested deferral rationale, just inside a PR body instead of an issue. Re-examined, the other three are genuinely harder rather than unresolved — and these are the real reasons: - **`backlog-pressure-reporter:197`** classifies a **dependency**, a different row from the one the caller resolved. Per-dependency resolution is needed or it repeats the wrong-row shape that `taskRevert` is blocked on. - **`lifecycle-ops:655`** guards an emit whose **target** is also a literal (`to: "archived"`). Converting the guard alone leaves the pair inconsistent — the move-target half is invisible to this census. - **`merge-queue-ops:352`** is an early return on an already-complete task inside a merge path that resolves lanes elsewhere; the placement needs its own judgement about which resolution it should share. They stay flagged, now with the real reason rather than an unchecked one. ## Measured | check | result | |---|---| | ephemeral-worker suites | green | | four gates + strict census | green | | engine `tsc` | clean | Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
95e4d1f246 |
fix(test): main is red — the archived-gate parity inventory is stale by two of my conversions (#3072)
## `main` is red `archived-column-gate-parity.test.ts` fails on clean `origin/main`, on the **raw-SQL** half. Not a branch artifact — reproduced by checking out `origin/main` and running it alone. ## Both dropped sites are mine | file | was → is | cause | |---|---|---| | `async-mission-store.ts` | 2 → 0 | **#3046** resolved `archiveDefinedFeatureBootstrapDuplicate`'s two `<> 'archived'` guards *together with* the `column: "archived"` write they gate | | `task-store/async-archive-lineage.ts` | 3 → 2 | **#3042** deleted `liveParentFilter`, an export with no callers anywhere | Neither PR knew this inventory existed. The archived gate is enforced in **three encodings** and only the census-visible one announces itself when it moves. Worth noting the mission-store conversion was *complete within its function*: the `column: "archived"` write is a move **target**, invisible to the column census, so converting the guards alone would have been this file's split brain one level in. ## The total is re-recorded, not loosened 8 → 5, rather than relaxing to `toBeLessThanOrEqual`. A fixed total is what makes a raw template **arriving** as visible as one leaving — and this guard exists precisely because arrivals are what nothing else counts. ## What this cost me, since it's the reusable part Earlier this session I converted a TypeScript `archived` comparison in `lifecycle-ops.ts` — the guard *and* its emit target together. **This test caught it**, and its argument is right: converting one encoding of the archived gate splits the brain, because the SQL halves still compare the raw string. I reverted. Then the test stayed red — for an unrelated reason. So the ratchet simultaneously **stopped a bad conversion** and **was carrying a stale number from two good ones**. Both halves of that are the ratchet working; the second half is why a guard needs its inventory updated by whoever moves it, not by whoever trips over it next. ## Measured | check | result | |---|---| | parity suite | **red on `origin/main`**, green here | | archived / lifecycle / parity suites | green | | four gates + strict census | green | Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
141f54e51d |
chore(core): mark the agent-store status formatter DELIBERATE-LITERAL (census 101→99) (#3063)
Fleet phase. `packages/core/src/agent-store.ts` was the **last** census file with no branch, worktree, or open PR against it. Claim published by pushing the branch before starting. ## Census before / after | | total | this file | |---|---|---| | before | **101** | 2 | | after | **99** | 0 | `--strict` exits 0, baseline re-recorded. **Reclassification, not conversion** — the line is unchanged. ## Already decided, in prose the census cannot read The site was flagged earlier today by another pass, as `FLAGGED AND LEFT COUNTED`: a pure formatter over `Pick<Task, "column">` with no store and no task id, whose output is a human-readable status line. On a renamed board it falls through to `(<column>)` — still accurate, just less specific. Converting it would mean threading a lane resolution into a string builder. That reasoning is right and I did not revisit it. The only gap was mechanical: a prose note is invisible to the tool, so the site kept reading as backlog. ## This completes the sweep of unclaimed files Third and last of these. Together with #3056 (fallback arms) and #3060 (dead sync path), **every census file that was unclaimed this phase has now been examined, and not one of them needed a conversion.** Each was either a three-state fallback arm — where the legacy id is the answer when resolution fails, and removing it would break the caller — or a site a previous pass had already reviewed and deliberately kept. That is the finding worth carrying forward. The remaining **99** is not a work queue: a meaningful share is correct code the tool cannot distinguish from owed work, and every fleet pass pays to re-derive it. Since all workers rank by the same `byFile` output, we also converge on the same top file — which is how `self-healing.ts` drew three parallel conversions, two of which are now unmergeable. Two cheap changes would fix both symptoms: 1. **Mark reviewed-and-kept sites** so the count means *conversions owed*. Two lines each. 2. **Push the branch at claim time** so `git ls-remote` is authoritative before work starts. Costs nothing; I did it for all three of these. ## Verification - `census --strict` exit 0; `tsc --noEmit` **0 errors** - `check:fnxc-future-dates`, `check:lane-wiring` — exit 0 - Comment-only diff; no behaviour change Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
581e6fba43 |
chore(core): mark the async-mission fallback arms DELIBERATE-LITERAL (census 108→106) (#3056)
Fleet phase. Claimed `packages/core/src/async-mission-store-queries.ts` — **the only census file with no branch, no worktree, and no open PR against it.** Claim published by pushing the branch before doing any work. ## Census before / after | | total | this file | deliberate | |---|---|---|---| | before | **108** | 2 | 128 | | after | **106** | 0 | **130** | `--strict` exits 0, baseline re-recorded in the same commit. **This is a reclassification, not a conversion.** The same two lines are still there. A reader comparing 108 → 106 against my #3047's 126 → 121 should know only the latter changed behaviour. ## Why marking is the right answer here Both sites are the **fallback arm** of the three-state rule: ```ts terminalColumns?.complete ? terminalColumns.complete.has(column) : column === "done"; ``` `terminalColumns` undefined means the caller could not resolve lanes. The legacy id is then the only answer that keeps the query working at all — converting it would delete the fallback and make an unresolvable caller return nothing. The census counts the literal, but **the literal is the design**. The file's own comment shows a previous worker already reached this conclusion. Nothing recorded it in a form the tool reads, so it stayed in `byFile` as apparent backlog for the next pass to re-derive. ## The finding this makes concrete I checked five unclaimed files this phase (`agent-store`, `github-tracking-state`, `planner-overseer`, `auto-merge-finalization`, this one). **Every site in them was either a fallback arm or an already-documented deliberate leave** — `agent-store.ts:236` carries a comment from today's fleet phase explaining why it stays. So the remaining count is not a work queue. A meaningful share is correct code the tool cannot distinguish from owed work, and each fleet pass pays to re-derive that. Marking them is cheap, mechanical, and makes the number mean "conversions owed" — which is what every worker reads it as when picking a cluster. I marked only the file I claimed. The others belong to whoever holds them. ## Verification - `census --strict` exit 0; `tsc --noEmit` **0 errors** - `check:fnxc-future-dates`, `check:lane-wiring`, `check:sql-column-literals`, `check:inert-flag-seams` — all exit 0 - No behaviour change: the two expressions are byte-identical, only comments added ## Note on the marker's granularity The first marker covered only `isComplete` — the census attaches markers by *preceding comment*, so the sibling `isArchived` needed its own. Caught by re-running the census (2 → 1, not 2 → 0) rather than by reading. Worth knowing before marking a group of related literals. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
21ef60047e |
fix(engine): a second complete lane is terminal too — restore the scheduler's dependency reconciliation (main is red) (#3065)
## main is red, and this is the fix ``` FAIL src/__tests__/scheduler-renamed-hold-events.test.ts > dependency unblocking (failure mode is a card that waits forever) > finds dependents resting in the renamed hold column when a blocker completes AssertionError: expected [] to include 'drafting' ``` Not in the thin merge gate's `engine-core` allow-list, so CI stayed green and only the non-blocking full suite sees it. The test file is unchanged since #2518; #3051 converted the guard underneath it. ## What broke #3051 turned the guard into `to === parked.complete || to === parked.archived`. `resolveLifecycleColumns` answers **first match per role** — the right shape for a move *target*, the wrong shape for *"did this card just reach a finished lane"*, which is a membership question. Two consequences, both silent: 1. **A board with more than one complete-trait column reconciles nothing** when a blocker finishes in the second one. The test file's own header flags this path specifically: *"This one is NOT latency: a dependent never gets unblocked, so it waits on a blocker that is already done."* 2. The legacy `done`/`archived` ids stopped matching at all — the failing assertion. ## Fix `resolveTaskParkedColumnsSync` gains `terminal`, a membership set: legacy `done`/`archived` seeded, then **every** complete- and archived-trait column from the task's own IR. Seeding legacy ids is safe in the direction that matters here. This is an **inclusion**: a superset makes the reconciliation run on a move it would otherwise ignore — one extra query, and it cannot wrongly withhold work. Seeding a **refusal** is the bug (`node-override-guard.ts` documents that one); this is not that. Same sync IR path and same fail-soft legacy default as the single-column answers, so event ordering and unresolvable-workflow behaviour are unchanged — the constraint the sync resolver's own header sets. The two sibling guards in the same listener (dispatch-oscillation reset at what is now line 1097, and the scheduling wake at 1114) had the identical arity defect and convert with it. ## Measured | | result | |---|---| | before | `scheduler-renamed-hold-events`: **1 failed / 9 passed** | | after | **11 passed** (one new case) | | `src/__tests__/scheduler*` | **14 files / 143 tests pass** | | `tsc --noEmit -p packages/engine` | clean | | census `--strict` / `check-lane-wiring` / `check-fnxc-future-dates` | clean, no baseline movement | **Proved it is main's red, not my branch's:** I checked out `origin/main:packages/engine/src/self-healing.ts` over my unrelated fleet branch and re-ran — identical failure. Then branched this fix straight off `origin/main`. **Mutation-tested.** Restoring `to === parked.complete || to === parked.archived` fails **both** the pre-existing case and the new second-complete-lane case. The new test is not vacuous: the second complete lane is invisible to first-match resolution, so it cannot pass against the old guard. ## Not done here I did not convert the remaining `to === parked.review` / `from === parked.wip` single-column comparisons in this listener. Review is genuinely two roles (`mergeBlocker` + `humanReview`) and wip has its own limit-setting semantics — both want the same membership-vs-target judgement applied deliberately rather than swept in behind a red-fix. Flagged, not guessed. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
998d75da3b |
refactor(core): resolve the hand-off archive guard by role (fleet) (#3054)
## Census | | column guards | |---|---| | before (this branch) | **122** | | after | **118** | Baseline re-recorded in the same commit, as the ratchet requires. (Four of the delta land with #3052; this PR carries `moves.ts`.) ## What changed `handoffToReviewImpl` refuses a hand-off from an archived card. Against the literal `archived`, a board whose archive lane is renamed **never matched** — so an archived card could be handed to review, and the invariant `HandoffInvariantViolationError` exists to protect was silently unenforced. The IR is resolved at the guard rather than 28 lines below where `handoffTarget` already reads it; the later read now **reuses** it instead of resolving twice. The hoist is safe because this function has already awaited `readTaskRowAsync` above — no new tick boundary. That's the specific hazard blocking the scheduler cluster, so I checked it here rather than assuming. Absent or trait-free IR keeps the legacy id: unconverted boards are byte-identical. ## Fleet intelligence: the backlog is now essentially fully triaged I worked down the census top-files list and verified each before writing. **Every remaining cluster is claimed, fallback-by-design, or documented-blocked:** | cluster | guards | status | |---|---|---| | `self-healing.ts` | 51 | **claimed** — checked out in another worktree (`convert/self-healing-lane-cluster-u7`) | | `scheduler.ts` | 12 | **blocked**, documented at line 907 — `task:moved` prologue is synchronous; hoisting reorders this listener against every other subscriber | | `notification-service.ts` | 5 | **blocked**, documented — needs the wedge-episode contract serialised first; the second site needs gate-placement judgement in `handleTaskUpdated` | | `executor.ts` | 4 | **claimed** (`fleet/executor-lifecycle-roles`) | | `restart-recovery-coordinator.ts` | 4 | **trait-fallback arms** — the census counts these as already converted | | `taskRevert.ts` | 2 | **blocked**, documented — would classify a *neighbour* task with the modal's own flags (the wrong-row shape, worse than the literal) | | `project-store-ops.ts` | 2 | **blocked**, documented — the dead SQLite twin; its first statement throws under PostgreSQL | | `github-tracking-state.ts`, `planner-overseer.ts`, `async-mission-store-queries.ts`, `register-task-workflow-routes.ts` | 2 each | **trait-fallback arms** | | `auto-merge-finalization.ts` | 2 | one is the `catch`-block degraded fallback; the other is a reason string | So the mechanical conversions are done. What's left needs either a design change (scheduler's event payload, notification's episode contract) or per-row lane data that doesn't exist at the call site yet (`taskRevert`). **That's the useful signal for the fleet**: further census reduction isn't a matter of more conversion passes. Forcing these would produce exactly the "conversions that break the code and improve the number" the learnings doc is named for. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b16318d3e3 |
gate: the inert-sync-lane check missed the Set-membership spelling (#3068)
## The check I shipped in #3062 has a hole, and there is an open PR standing in it #3062 counts `to === parked.complete`. **#3065 rewrites exactly that into `parked.terminal.has(to)`.** #3065's change is **correct and should merge** — a board may declare more than one complete-trait column, so a single `parked.complete` was genuinely wrong, and the PR fixes a red main. But the answer still comes from `resolveTaskParkedColumnsSync`, which resolves through `store.resolveTaskWorkflowIrSync` and therefore always describes the default board. The guard is exactly as inert; it simply stopped being a comparison node, and my check only counted comparisons. ## Measured, both ways Applying #3065's rewrite shape to `scheduler.ts`: | | result | |---|---| | check as shipped in #3062 | **20 → 15**, exit 0, prints *"total fell — re-record with `--update-baseline`"* | | this PR | **20 → 18**, the three `.has()` guards stay counted | The shipped behaviour is the bad half: it does not merely miss them, it invites re-recording a smaller baseline, permanently retiring sites that are still inert. And on the case the ratchet actually exists for — a **fresh literal** converted via the Set spelling: | | result | |---|---| | check as shipped | **exit 0** — missed | | this PR | **exit 1** — caught, `20 → 21` | ## What changed - `terminal` (plus `lanes`, `columns`) added to the role-field vocabulary — Set-valued roles, not single ids. - Membership calls counted alongside comparisons: `X.<role>.has(...)` / `.includes(...)` where `X` is sync-resolved, inline or via a local. Neither exit code changes on current main: still 20, still exit 0. ## The pattern, stated plainly This is the **second** evasion of this check, found the same way as the first. #3062's own body records the first: the initial draft matched only the local-variable spelling and a mutation run proved the inline `resolveX(...).review` walked straight past it. Both times the guard was correct about the shape in front of it and blind to a trivially different spelling of the same defect. Worth generalising rather than patching a third time — the durable fix is to key on **the source** (any value derived from `resolveTaskWorkflowIrSync`) rather than enumerate the syntax that consumes it. That is a dataflow question and a larger change than this; recording it here as the known limit rather than claiming this version is complete. Related and unresolved: the same "a rewrite makes the counter drop without the guard changing" shape is live in the census itself — see my comment on #3057 about `LEGACY_*` named sets, and #3058, where ten guards left the census for zero behaviour change. ## Census before / after ``` before: COLUMN guards (the backlog): 88 after: COLUMN guards (the backlog): 88 ``` Unchanged — this converts nothing. It stops a counter from falling for the wrong reason. ## Verification `pnpm check:inert-sync-lanes` exit 0 · `test:gate` exit 0 · lifecycle-column census exit 0 · `pnpm lint` clean. Gate-script only; no production file touched (`scheduler.ts` restored clean after every mutation run). |
||
|
|
bdedb6cf1a |
gate: fail the build on a NEW inert sync-lane conversion (#3062)
## Claim, and why it turned into a gate
I claimed the largest unclaimed unflagged cluster, `executor.ts` (4
guards at 3557/3581/3632/3642). All four are conditions of a
**synchronous** `store.on("task:moved", …)` listener — the same class as
`scheduler.ts`. Converting them needs either an `await` in a sync
prologue or the sync resolver, and the sync resolver is inert.
`executor.ts` already says so, at line 10459, dated 2026-07-30:
> **THE SYNCHRONOUS RESOLVER IS A NO-OP IN PRODUCTION.** … every
sync-resolved conversion resolves the DEFAULT workflow and answers with
the legacy ids no matter what board the task is on. That makes a sync
conversion cosmetic: the census counts it as converted, `--strict` goes
down by one, and the guard behaves exactly as the literal did. **Worse
than leaving the literal, because the number says the site is done.**
The next day, #3051 did exactly that to ten `scheduler.ts` arms. Census
fell by ten; nothing changed on any board (refuted live in #3058).
So the finding was already written down, in the file a converter would
be reading, in capitals — and the fleet phase produced the defect
anyway. **A comment cannot fail a build.** Converting `executor.ts`'s
four the only available way would have made me the third instance. I
flagged them and built the guard instead.
## What the check does
Per file: finds functions reaching `resolveTaskWorkflowIrSync`, the
locals assigned from them, and the `===`/`!==` guards consuming those
roles. Baselined per file; **fails on a rise.**
Not zero, deliberately. The existing sync guards are real and documented
— the scheduler's listeners genuinely cannot `await` today and their
authors said so. Demanding zero forces a revert or a day-one exemption
marker. What must not happen is *more* literals quietly becoming
inert-resolved.
Complements `check-inert-flag-seams.mjs`, which catches the opposite
shape (a lane parameter **no** caller supplies). This catches a
parameter that **is** supplied, from a source that always answers the
same thing — which passes that check cleanly.
## Why the shape is invisible
The obvious reading is wrong, and it is what makes this survive review.
The helper does **not** receive `undefined` and fall through to `??
"in-review"`. It receives a **real IR that resolves real traits** — the
default board's — so it answers with full confidence and the `??` arms
beside it are dead code.
```
tsc passes the value is a string, correctly typed
tests pass on the default board the constant answer IS the right answer
the census DROPS it counts comparisons against literals, and the literal really is gone
```
## Mutation evidence — including one against this check itself
| Mutant | Result |
|---|---|
| baseline | exit 0, 20 guards in `scheduler.ts` |
| convert one more literal to a sync-resolved lane (the #3051 move) |
**exit 1, 20 → 21** |
| convert the same literal to an **async**-resolved lane | exit 0 —
correctly silent |
The first draft **failed its own mutation test**: it matched only the
local-variable spelling (`const parked = resolveX(...)` then
`parked.review`), which is what #3051 used, and the inline spelling
`resolveX(store, id).review` walked straight past it while being exactly
as inert. A ratchet one rewrite evades is worse than none, because the
green result reads as proof. Both spellings now count.
## Limits, stated so nobody over-trusts it
Sources are matched **within a file by function name**, so a helper
imported from another module is not followed — this finds the dominant
local-helper shape and will miss a cross-module one
(`resolvePlannerLanes`, consumed in `executor.ts`/`triage.ts`, is
currently outside its reach). It proves a guard consumes a sync-resolved
answer, not that the answer is wrong for every caller. Tests are
excluded. Treat a report as a pointer to investigate.
## Census before / after
```
before: COLUMN guards (the backlog): 104
after: COLUMN guards (the backlog): 104
```
Unchanged by design — this converts nothing. It stops the count from
moving for the wrong reason.
Worth recording alongside it, measured across the current backlog: **21
of 104 already carry an explicit flag note**, **51 are
`self-healing.ts`** (concurrently claimed by **#3055, #3050 and #3049**
— three PRs, one file, still worth de-conflicting), and **28 are
genuinely unclaimed and unflagged**, the largest being these
`executor.ts` four. The cluster-sized work is close to exhausted; what
is left is scattered and mostly blocked, which is the pressure that
produced #3051.
## Verification
`test:gate` exit 0 · `pnpm lint` clean · lifecycle-column census exit 0
· `pnpm check:inert-sync-lanes` exit 0. No production file touched.
|
||
|
|
af470f7c05 |
convert(engine): self-healing lane cluster 56 -> 38 guards (repo 126 -> 108) (#3049)
## Census before / after
```
before after
self-healing.ts column guards 56 38
repo-wide COLUMN guards (backlog) 126 108
```
`self-healing.ts` was the largest single cluster by a wide margin — 56
guards against 12 in the next file. Baseline re-recorded in the same
commit; `--strict` green.
## Converted: 15 guards across 11 sweeps
Existing helpers only — `resolveProjectColumnsForRoles` with
`TERMINAL_ROLES` / `REVIEW_ROLES` / `countsTowardWip` / `hold` /
`archived`, the same shape this file already uses. No new helper, no new
resolution pattern.
What each was silently doing on a renamed board:
| sweep | behaviour before |
| --- | --- |
| `archiveStaleDoneTasks` | **both** guards inert, so every card counted
as an active dependent and the sweep archived **nothing at all** |
| `reconcileDependencyBlockingLeases` | no holder matched, so a stale
file-scope lease blocking an unmet dependency was never cleared |
| `reconcileCompletedBlockedTasks` | work whose blocker had cleared
stayed parked instead of advancing |
| `reconcileInReviewUnmetDependencies` | a card sat in review with unmet
dependencies and no rebound |
| `reclaimStaleActiveBranches` | archived cards were eligible for branch
reclaim |
| `reconcileInReviewBranchRebind` | the rebind list was empty |
| `autoReboundPausedScopeDecayDetailed` | no card was ever seen as
executing |
| `detectStalledCards` | finished cards counted as stall candidates |
| `recoverApprovedStrandedAiMergeCommit`,
`recoverDriftedAgentTaskLinks`, `cleanupStaleTempMergeWorktrees` | same
shape |
**Reused rather than duplicated:** `recoverWedgedActiveMerge` already
resolves `wedgedReviewColumns` via `resolveReviewColumnsFor` three lines
above the site I was converting, so the site now uses it instead of a
second resolution of the same question.
## One site I converted and then reverted
`clearStaleBlockedBy`'s memo closure carries an FNXC note stating the
literal is **deliberate**: the closure only decides whether to re-log an
already-logged blocker, so a renamed board costs a duplicate log line —
not a wrong lifecycle decision — and restructuring a sweep's control
flow to convert a logging decision is the wrong trade.
I read that note *after* editing the line. Restored.
Worth flagging separately: **it has the reasoning but no
`DELIBERATE-LITERAL` marker**, so the census keeps counting it and it
re-appears in the backlog as if unexamined. That is a marker gap, not a
conversion gap — the next person will make the same mistake I did.
## Not converted — flagged, not guessed
Ten of the twenty-four remaining sites are in **sync predicates with no
resolution seam**:
- `classifyPausedAbortWorkflowRecovery` (3)
- the `start()` task-moved listener (5) — compares event `from`/`to`
columns inside a sync callback
- `isWorkspaceOwnerLive` (1)
- `isPhantomExecutorBinding` (1)
Converting these means threading a flags parameter down from every
caller — precisely the unwired-optional-parameter shape this program
keeps finding inert (five were live on `main` at once per
`unwired-lane-parameter-guard`). They need a decision about *where the
resolution lives*, not a guess from me.
The other **14** are in async sweeps with a seam available and are
ordinary follow-on work in this same file.
## Verification (measured)
- self-healing suites — **816 passed / 41 files**
- `tsc --noEmit`, `eslint` — clean
- `pnpm test:gate` — green
- `lifecycle-column-census --strict`, `check-lane-wiring`,
`check-sql-column-literals`, `check-inert-flag-seams`,
`check-fnxc-future-dates` — green
No changeset: `@fusion/engine` is private.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Self-healing workflows now continue functioning when workflow columns
are renamed.
* Improved recovery for stalled, blocked, paused, or disconnected
workflow states while preserving existing filters and actions.
* Temporary merge worktrees and drifted agent links are cleaned up more
reliably.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
0fd3e38628 |
test(engine): PR #3051's scheduler conversion is inert — live-PG refutation (#3058)
## Escalation — a conversion on main changed nothing
**#3051 ("scheduler.ts 12 → 2 lifecycle-column guards") is inert.** It
widened `resolveTaskParkedColumnsSync` from `{hold,intake}` to the full
role set and replaced ten handler literals with `parked.review` /
`parked.wip` / `parked.complete` / `parked.archived`. The census fell by
ten. The behaviour did not change, on any board.
Everything rests on one line in that helper:
```ts
const l = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId));
```
`resolveTaskWorkflowIrSync` resolves through the sync workflow
**selection** reader, which answers `undefined` for every task under
PostgreSQL — the shipped backend. The resolver takes its `!workflowId`
branch and returns the **default builtin IR**.
Note the shape precisely, because the obvious reading is wrong and this
PR corrected itself on it mid-run: the helper does **not** get
`undefined` and fall through to `?? legacy.review`. It gets a **real IR
that resolves real traits** — the default board's. So `parked.review` is
`"in-review"` for every card on every board, the `?? legacy` arms are
dead code, and the helper answers with full confidence. It looks
resolved at every level except the one that decides the answer.
## Evidence (live PostgreSQL, 3/3 passing)
For a card bound to a **stored** renamed workflow and sitting in that
board's review column (`checking`, carrying `human-review` +
`merge-blocker` + `merge`):
| | sync path (what every converted arm uses) | async resolver | board
actually declares |
|---|---|---|---|
| review | `in-review` | `checking` | `checking` |
| wip | `in-progress` | — | `building` |
| complete | `done` | — | `shipped` |
The async arm is in the test on purpose: it attributes the failure to
the **sync path** and nothing else.
The **control** is the point of the whole thing — on the default board
the sync answer is *correct*, by coincidence rather than resolution.
That is why every default-board scheduler test passes either way, and
how ten inert conversions read as a fix.
## Why this is worse than leaving the literals
A conversion that changes nothing is worse than an unconverted literal,
because **the literal was counted and this is not.** Ten guards left the
backlog, the file now reads as converted, and the next reader has no
reason to look again.
Two further signals that this was not a deliberate trade-off:
1. The FNXC block still standing directly above the handler (unmodified
by #3051) **contradicts the code beneath it** — it says these ten arms
cannot be converted this way and names `resolveTaskParkedColumnsSync` as
the hazard-avoidance device, not the fix.
2. #3051's own added note asserts the fix as fact: *"so on a renamed
board PR monitoring never started or stopped, failure bookkeeping never
recorded, and terminal cleanup never ran."* Those failures are real.
This conversion does not fix them.
## Scope — driven vs argued, stated in the file
- **Driven:** the roles the sync path yields for a real card on a real
stored renamed board, against a real PostgreSQL store, versus the async
resolver on the same card.
- **Not driven, and the file says so rather than substituting a spy:**
the `parked.review` arm's own side effect. Its only outputs are four
dispatch-oscillation fields that do not round-trip through `updateTask`
on this store (measured: writing `dispatchStormCount: 3` reads back
`undefined`), so there is no persisted observable. The behavioural half
is carried by the sibling
`workflow-scheduler-parked-columns-live-e2e.pg.test.ts`, which drives
the **same helper** on the hold role through to persisted state.
## The real unblock
Unchanged from the note already in the file: carry the resolved lanes
**on the `task:moved` payload**, so no listener resolves at all. That
removes the class rather than one instance, and it is the only option
that survives the synchronous-prologue constraint — these listeners run
in the same tick as a synchronous emitter, which is why an `await`
cannot simply be added.
## Verification
`test:gate` exit 0 · live-PG E2E surface **174/174** · census exit 0 ·
`pnpm lint` clean. Test-only; no production file touched.
## Recommendation
Do not revert #3051 — the widened helper is harmless and the note it
added is useful once the resolution is real. **Restore the ten guards to
the census**, or land the payload change. Either way the count must not
read as paid.
Related: **#3055, #3050 and #3049 are all converting `self-healing.ts`
concurrently** — three PRs, one file, 51 guards. Worth de-conflicting
before any of them merges.
|