31b9fafe1193519d44c97427cc2db45bf64eb0f9
3509 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
698bded476 |
fix(tests): 4 engine reds on main — each was green for a reason the fleet removed (#2778)
## Context A full `@fusion/engine` run on `origin/main` (`9b61d795c9`) reports **39 failures / 10835 passed**. 32 are the notifier harness, fixed in #2776. This PR takes 4 of the remaining 7. All three files share one shape: **each case was passing off something the lifecycle conversions have since correctly taken away.** In every one, the product is fine and a good change landed as a red test. --- ### 1. `executor-graph-failure-lanes-resolved.ts` — an equality that fails on its own fix The guard forbids resolving a lifecycle *guard* through the synchronous `resolvePlannerLanes` (a no-op under the shipped PostgreSQL backend, so the census counts the site as converted while it behaves like the literal). It asserted `expect(callSites).toBe(3)`. #2764 converted the promotion-path site to `resolvePlannerLanesForTaskAsync` — exactly the direction this guard wants. Count went **3 → 2** and the assertion failed. The guard's own comment states the invariant as *"Any FOURTH is a new sync resolution"* — one-directional. Coded as equality, it fails on removal, which is the change it exists to encourage. Now `toBeLessThanOrEqual(2)`. **Mutation:** adding a third sync call site → `expected 3 to be less than or equal to 2`. Still load-bearing. ### 2. `restart.integration.test.ts` — a fixture matching a fallback constant `recoverCompletedTask` re-homes intake → hold → wip only when the origin is the board's **intake** lane; otherwise it hands straight to review. The failure showed the 1st move as `in-review` with no re-home. Nothing regressed. The fixture put the card in `triage` and resolved lanes through the sync resolver, so it fell through to `LEGACY_PLANNER_LANES` — where `intake` is literally `"triage"`. **It was matching a hardcoded fallback, not a declared lane.** #2764 made the site await the real resolver; the mock selects `builtin:coding`, and **U11 merged intake and hold onto one Planning column (`todo`)**, so `triage` is not a lane on that board and the two-hop correctly collapses. The invariant the test is named for — completed work in a distinct intake lane is re-homed along a legal path, not moved intake → review, which role adjacency rejects — is still real. So the fixture now **declares** a board with intake separate from hold, the only shape where the two-hop is reachable. **Mutation:** removing the re-home hop from the product → fails with the expected `todo` first-move. Load-bearing. ### 3. `executor-abort-provenance.test.ts` — a call one argument short Both provenance cases returned `false` for a clean completed in-review row. This reads as an FN-6796 regression stranding rows that are already handed off for review. It is not. #2703 added a 7th `reviewLane` parameter so the lane is resolved by the caller. **The call goes through `as any`, so the missing argument was not a type error** — it arrived `undefined`, `live.column !== reviewLane` held for every row, and the classifier answered false for everything. Passed explicitly rather than defaulted inside the classifier: a default would restore the literal the parameter exists to remove. Added a **differential** — a card resting in a *renamed* review lane classifies the same, a mismatched one does not — so the parameter cannot be re-literalized while still looking converted. **Mutation:** `live.column !== "in-review"` → the differential fails. The other cases pass, which is precisely why it was worth adding. --- ## Evidence | file | result | |---|---| | `executor-graph-failure-lanes-resolved` | **24 passed** | | `restart.integration` | **48 passed** | | `executor-abort-provenance` | **16 passed** | Gate **732 green** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors**. Test-only — no product file is touched by this PR (the mutations above were run and reverted; `git diff` confirms clean). ## Deliberately NOT fixed here 3 cases in `executor-prompt.test.ts` ("global pause behavior") remain red on main: **a user-paused todo task now reaches `createFnAgent`**. That is a safety invariant rather than a stale fixture, and neither `executeCore` nor the graph executor holds a pause gate — the refusal #2371 documented is not where its note implies. It gets its own change; editing the fixture to match current behaviour would hide it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c643d62e85 |
fix(executor): wipDeclared must ask ALL six lifecycle roles, not two (#2777)
## What this fixes `resolveResumeLanes` returns `wipDeclared`, which gates whether `routeGraphFailureToExecutionResume` may route a graph failure back into execution resume. Getting it wrong terminalizes tasks on boards that should resume. Two prior versions were wrong, both caught in review rather than by me at write time: **1. Two-state (`lifecycle?.wip !== undefined`)** — greptile P1 on #2760. A v1-upgraded board terminalizes: `synthesizeDefaultColumns` emits `{ id, name: id, traits: [] }`, so *every* role resolves `undefined` even though those columns literally are the legacy lanes. Verified by parsing a real v1 IR. **2. Proxying "synthesized" as "hold and review are both undefined"** — my own fix for (1), and also wrong. I caught this against #2765 rather than shipping it. A **v2** board that declares only `intake` + `complete` has hold and review undefined too, so it would be misread as synthesized and treated as declaring wip when it deliberately does not. The failure mode both versions share: reading a *sample* of the roles and treating the answer as a verdict about the whole IR. #2765 says it directly — an empty result has two meanings, and you cannot tell them apart from a subset. ## The rule ```ts wipDeclared: lifecycle?.wip !== undefined || !declaresAnyLifecycleRole(lifecycle), ``` Three states, asking all six roles: - **wip declared** → true, the board says so. - **some role declared but not wip** → false. A v2 board that omits wip means it; do not resume into a lane it did not define. - **no role declared at all** → true. That is the synthesized/v1-upgraded shape, whose columns *are* the legacy lanes; the pre-existing behaviour is correct there and must not regress. `declaresAnyLifecycleRole` iterates `Object.values(lifecycle)` rather than naming roles, so a seventh role added later is included automatically instead of silently falling into the wrong branch. ## Evidence - `executor-resume-lanes-resolved.test.ts`: **7 passed**, +23 lines covering the v1-synthesized board and the declares-some-but-not-wip board. - **Mutation:** restoring the naive two-state rule → **1 failed / 6 passed**. The added coverage is load-bearing and pins exactly the regression greptile caught. - Gate **732 green** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors**. - Rebased on current main. ## Scope `executor.ts` (+22) and its test (+23). One predicate; no other behavior touched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
|
8c9b84ae38 |
batch-core: packages/core + dashboard/src lifecycle conversion (129 → 92) (#2780)
## batch-core — `packages/core` + `packages/dashboard/src` Shared branch: two workers are converting into it. Opening the PR because the branch was green with none, and a branch without a PR merges nothing. ### Census Measured with `node scripts/lifecycle-column-census.mjs --json`. | | guards | |---|---| | batch-core scope at branch point | 129 | | batch-core scope now | **92** (51 files) | | repo total now | 358 | Files closed so far: `store.ts` 11→0, `task-merge.ts` 6→0, `live-agent-count.ts` 6→0 (marked, not converted — see #2762), `task-update.ts` 3→0, display-ordering + Wake Delta ranking 5→0, `register-git-github.ts` 4→0. ### The `register-git-github.ts` slice Three PR routes — `pr/create`, `pr/push-branch`, `pr/resolve-conflicts` — plus the `CHANGES_REQUESTED` handler each compared `task.column !== "in-review"`. On a renamed board **none** of them matched, so every PR affordance the dashboard offers was refused for a card sitting in the lane that board calls review, and the refusal named a column that does not exist there. All four now share one helper, `reviewColumnsForTask`, which gets two things right that this program has repeatedly gotten wrong: - **Membership, not a single id.** It takes the broad review set (`mergeOrchestration ∪ mergeBlocker ∪ humanReview`). `resolveLifecycleColumns` returns the *first* column per trait, so a single-id answer silently ignores a board that declares a merge lane **and** a separate human sign-off lane. These guards only refuse or permit — they never move the card — so over-admitting costs nothing while under-admitting refuses a request that should have worked. - **An empty resolved set means UNEXPRESSED, not absent.** `synthesizeDefaultColumns` upgrades a v1 graph by emitting every default column with `traits: []`, so a v1-upgraded workflow resolves to an empty review set while its `in-review` column plainly exists and holds the card. Reading empty as "this board has no review lane" would refuse these routes on **every pre-v2 project** — a worse regression than the one being fixed, and invisible to any v2 test. This is the dashboard twin of the `fn pr create` guard in `packages/cli/src/commands/pr.ts` (#2775). The two surfaces answer the same question and now agree — FN-5893 surface enumeration. ### Testing note: why the seam and not the routes I wrote route-level HTTP tests first and **deleted them**. An express fixture over `registerGitGitHubRoutes` hangs — every case, including the pure refusals, times out at 4s, because registering the router starts background work the fixture never satisfies. Making it run would mean mocking git, the GitHub client, and the pollers: a mock-the-world shell, which is what the project's do-not-add-slow-tests rule (FN-5048) says to avoid in favour of a narrow seam. `reviewColumnsForTask` *is* the narrow seam — it holds the entire decision, and the four call sites now do nothing but ask it and render its answer. Six cases pin it: the renamed lane is returned and `in-review` is not, a two-lane board returns both, a v1-upgraded board falls back, an unresolvable workflow falls back, and the refusal renders lanes an operator can act on. **Mutation-verified, both directions:** reverting the helper to the legacy literal fails 2 of 6; treating an empty set as an answer fails 1 of 6. One fixture bug worth recording, since it would have made the two-lane case vacuous: the trait id is kebab-case `human-review`, not `humanReview`, and the built-in traits must be registered via `import "@fusion/core"` before flags resolve. ### Verification - `pnpm --filter @fusion/dashboard exec tsc --noEmit -p tsconfig.json` → 0 errors - `pnpm lint` → 0 errors - `register-git-github.review-lanes.test.ts` → 6 passed --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b42b40aa48 |
fix: make flushAsyncWork actually drain — clears 32 notifier failures on main (#2776)
## Red on main, fix-forward batch-engine (#2773) left 32 failing cases on main: `notifier.runtime.test.ts` (19) and `notifier.test.ts` (13), all `expected "vi.fn()" to be called 1 times, but got 0 times`. ## The failure is in the harness, not the notifier `notifier.ts` is untouched by #2773. `notification/notification-service.ts` was converted, and `handleTaskMoved` is fire-and-forget (`void this.handleTaskMovedAsync(data)`). The async path now awaits `resolveLifecycleColumnsForTask` and then `resolveReviewColumnsForTask` (which awaits `resolveWorkflowIrForTask`) — several more await hops after `store.emit(...)` returns. The harness had no slack to absorb them: ```ts export async function flushAsyncWork(): Promise<void> { await vi.waitFor(() => { expect(true).toBe(true); }); } ``` The condition is true on the first tick, so `waitFor` resolves immediately. **It never waited for anything.** It worked only while the handler completed within a single turn — and it reported the resulting breakage as a notifier defect rather than as its own. Another entry in the recurring pattern this program keeps hitting: a cheap check that reads as authoritative. A `waitFor` looks like synchronization at the call site; this one was a no-op. ## Evidence | run | result | |---|---| | before | 32 failed / 68 passed (100) | | after | **100 passed (100)**, 3.25s | | after, mutated back to a single `await Promise.resolve()` | 32 failed / 68 passed — the same 32 | The mutation run is the point: the fix is load-bearing, not a coincidence of timing. Gate 732 green · `pnpm lint` clean · engine `tsc --noEmit` clean. ## Reversible decision, noted: microtasks only A `setTimeout(0)` drain also turns all 100 green, and it was my first version. Rejected on measurement: - it costs real wall-clock at every call site — the two files went **~2s → over 2 minutes**; - it **stalls under the fake timers** `notifier.test.ts` installs (lines 355/381/589), where a pending `setTimeout` never fires — 4 cases hung. The awaits being drained are promise-based (workflow-IR resolution), so microtask turns are the right currency, they work identically under real and fake timers, and they cost nothing. Per AGENTS.md *"Do Not Add Slow Tests"* — prefer fake timers over real time waits. 16 turns is slack, not a tuned number; the chain is ~4 deep today. ## Scope One test-harness file. No product code, no behavior change. Tests asserting a specific outcome should still prefer `vi.waitFor` on *that outcome* — this helper covers the "let the fire-and-forget handler finish" case, and now actually does it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1fb53f9924 |
docs(scheduler): the task:moved arms are blocked by a synchronous prologue — measured, and deliberately left counted (#2771)
## No behaviour change, and deliberately **no markers** The ten `from`/`to` comparisons in `scheduler.ts`'s `task:moved` handler stay **counted** in the census. They are genuinely wrong on a renamed board — real backlog. Marking them `DELIBERATE-LITERAL` would claim *"reviewed, correct"* when the truth is *"reviewed, still broken, blocked on an ordering question"*, and that is the opposite of what #2767's markers were for. This records the blockage instead. I have deferred these twice citing risk; this is the analysis that deferral was standing in for. ## The measured constraint **The handler is `async`, but its prologue is not.** There is no `await` anywhere between the handler's first line and the terminal-blocker branch ~55 lines down. The snapshot invalidation, the PR-monitor start/stop pair, the mission hand-off and the failed-task tracking all run in the **same tick as the emitter**. So hoisting a resolution to convert those arms does not cost "one await" — it converts the **whole prologue into a microtask**, reordering this listener against every other synchronous `task:moved` subscriber and against the emitter's own continuation. That makes `resolveTaskParkedColumnsSync`'s *"SYNCHRONOUS on purpose"* note **load-bearing rather than stale** — verified by measurement, not assumed. I had been treating it as possibly-stale boilerplate. ## Why the two obvious workarounds don't apply - **Resolve lazily inside the branch.** Doesn't help: the *condition* is what needs the lanes, and it is evaluated in the prologue. - **A cheap sync superset prefilter** — the shape that worked in `usage-limit-detector` — needs a literal predicate that cannot wrongly *exclude* on an unknown vocabulary. For *"is `to` the terminal lane?"* no such predicate exists: a renamed board's terminal id is unknown by construction. (That is precisely why the prefilter *was* safe there — literals can only fail to exclude, never over-exclude.) ## What would actually unblock it 1. **Audit the ordering**, then hoist one await and convert all ten together. That is an audit across every `task:moved` emitter and subscriber — not a scheduler-local change, and not something to do speculatively. 2. **Carry the resolved lanes on the event payload**, so no listener resolves at all. This is the only option that scales to the *other* synchronous listeners with the same problem, and it removes the class rather than one instance. I'd recommend (2) if this is worth funding — it is the same shape as the fix that removed the sync-resolution class in #2759, one layer up. ## Verification 11 scheduler suites — **130 passed** · `pnpm test:gate` **158 / 487 / 10 / 71** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors** · `--strict` exits 0, census unchanged at 12 for this file (which is the point). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added internal documentation explaining synchronous event-ordering requirements when resolving task lanes. * Clarified why asynchronous resolution must not be introduced in this scheduling flow. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9b61d795c9 |
fix(engine): heartbeat asked 'is this task finished?' with legacy ids — and one of the two sites writes status:failed onto completed work (#2769)
## Two heartbeat sites asked "is this task finished?" with the legacy ids `agent-heartbeat.ts` **4 → 0**. Neither site is cosmetic. **Linked-task clear.** The heartbeat clears an agent's assignment once its card is finished. Keyed on the literals, an agent on a renamed board stayed bound to a **completed** card indefinitely — every later heartbeat ran with stale task context instead of picking up new work, and nothing else clears it. **Worktree-acquisition gate.** Its failure bookkeeping runs only for a **non-terminal** task. A card in a renamed complete lane read as non-terminal, so an acquisition failure could stamp `status: "failed"` and an error message onto work that was **already done**. That second site *writes*, which drives the fallback direction: an unresolvable workflow degrades toward "terminal", because treating a finished card as unfinished is the expensive mistake here. Both sit in async paths — the first has `await taskStore.getTask(...)` three lines above — so this is an `await`, not a restructure. Extracted to one predicate rather than converted twice: they are the same question, and the two must not drift when one of them acts destructively. ## How this was found, and the part worth recording Generalising #2767. That PR marked a documented false positive the census kept advertising, so I swept for **other** files whose lifecycle literals were reasoned about in prose but still counted — to find out whether the trap was systemic. **It is not.** Of twelve candidate files, only this one carried real unconverted guards, and its "false positive" mentions turned out to be unrelated (detection heuristics, not column literals). The sweep mostly came back **negative**, and that is worth saying so nobody repeats it expecting a haul. ## Revert proof | reverted | result | |---|---| | neuter the resolution (predicate → literals) | **3 failed** / 2 passed | | shipped | **5 passed** | The two that survive the revert are the degraded-mode pair, which is correct — they assert the *legacy* answer, so they must pass either way. One case also pins that a legacy `done` id is **not** terminal on a board that does not declare it, which is what a board-wide union would get wrong. ## Verification - 13 heartbeat suites — **501 passed** (496 before, +5 new) - `pnpm test:gate` — **158 / 10 / 487 / 71** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors** · `--strict` exits 0 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7119432c79 |
fix(engine): the stranded-completed recovery never resolved on a renamed board — and its suite fed the broken reader the right answer (#2764)
## The "recovery of last resort" never resolved anything
`recoverCompletedTask` carries this note in its own source:
> *This is the recovery of last resort — a literal here means the last
resort does not exist off the default lineage.*
It was resolving through `resolvePlannerLanes`, which reads
`resolveTaskWorkflowIrSync` — whose selection reader returns `undefined`
**unconditionally** in PostgreSQL mode, the shipped backend. So it
resolved the **default** workflow for every card,
`promotedFromPlannerColumn` was `false` on every renamed board, and the
recovery never fired.
That is precisely the stranding it exists to fix — completed work
sitting in a planning lane with nothing left to rescue it — **with the
conversion in place and the census counting it as done.**
The call site is inside an async method that has already awaited store
reads, so the fix is an `await`, not a restructure.
`resolvePlannerLanesForTaskAsync` is the async twin: identical logic,
identical fallbacks, one `await`. Answers are unchanged on the default
lineage and correct everywhere else.
## The existing suite could not see any of it — the more important half
`executor-planner-lanes-resolved.test.ts` injected **only**
`resolveTaskWorkflowIrSync`:
```ts
(store as { resolveTaskWorkflowIrSync: ... }).resolveTaskWorkflowIrSync = () => ir;
```
It fed the broken reader **the right answer**. Every case proved the
promotion *logic* while being structurally blind to whether production
resolves at all — and it was green the entire time. A suite that cannot
fail for the reason the code is broken is the same defect as the code,
one level up.
The harness now feeds the sync reader the **default lineage** (what it
actually returns) and the async readers the task's real workflow.
| | reverting the call site to sync |
|---|---|
| before this PR | **0 failed** — suite blind |
| after | **5 failed** / 13 passed |
Two cases opt back in via `syncResolvesIr`, and only those two: they
cover `isPlannerColumnFor` and `isBackwardMoveOutOfPlanning`, which are
still synchronous, so there the sync reader genuinely *is* the input
path and feeding it the IR tests the classifier rather than the reader.
## Not converted, deliberately
**Those two classifiers.** They sit in an else-if chain whose next arm
is `from === "in-progress"`, so deferring the decision into an async
body changes which arm runs. That branch's own comment records a
previous half-conversion there:
> *a half-conversion turned a missed rescue into active damage. Third
time this program has produced that shape — gates converted,
destinations left literal.*
That needs the chain enumerated first, not a fast restructure at the end
of a sweep. Their production inertness is held by the
`resolveTaskWorkflowIrSync` call-site allow-list in #2759, so they
cannot be forgotten.
## Verification
- new suite **4 passed** · strengthened suite **14 passed** (18
together)
- `pnpm test:gate` — **158 / 10 / 487 / 71** · `pnpm lint` clean ·
engine `tsc --noEmit` **0 errors** · `--strict` exits 0
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
7f3eee9db7 |
docs(engine): mark the usage-limit terminal filters DELIBERATE-LITERAL — a documented false positive that has now baited two workers (#2767)
## No behaviour change. This is the work order retracting a documented false positive. I went to convert the `done`/`archived` filters in `usage-limit-detector.ts`, reasoning that on a renamed board a provider rate limit would pause already-finished work. I wrote the conversion — and only then read the note a previous worker had left directly above it: > *"The FIRST thing I suspected there — the `done`/`archived` terminal filter — turned out to be a **FALSE POSITIVE**: its revert stayed green, because the lane check already excludes finished cards."* **They are right and I was wrong.** A terminal card is already excluded downstream: `taskUsesProvider` resolves the task's active lane, a finished card matches no active lane, so it resolves no providers and cannot be affected. The suite pins exactly this — `pauses a PEER executing in the renamed WIP column` asserts `FN-SHIPPED` is not paused. My conversion is reverted. It changed nothing at runtime and would have lowered the census count while behaviour stayed identical — the precise shape this program keeps warning about, produced by me this time. ## Why a marker and not just the existing prose The note was already there and I walked into it anyway, because **the census kept listing this file as 4 unconverted guards**. The work order advertised the work; the reasoning against it lived in a comment you only reach after you have started. Prose informs a reader who is already looking; a marker informs the *instrument*, so the file drops out of the work order. Two distinct reasons are recorded rather than one blanket marker, because they are not the same argument: - **the prefilter** is a deliberate cheap **superset** (#2672 review). Converting it reintroduces the whole-board resolution that review removed. Literals are safe here in the direction that matters — a renamed board declares no `done`/`archived` id, so nothing is wrongly *excluded*. - **the final filter** is redundant with the lane check, and that redundancy is already proven by an existing test. ## Census | | before | after | |---|---|---| | `usage-limit-detector.ts` | 4 | **0** | | repo backlog | 437 | **433** | | DELIBERATE-LITERAL (reviewed) | 38 | **42** | Every one of the 4 is a marker, not a conversion. The backlog moved because reviewed literals left it honestly, not because behaviour changed. ## Verification `usage-limit-detector.test.ts` **58 passed**, unchanged before and after · `pnpm test:gate` **10 / 158 / 487 / 71** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors** · `--strict` exits 0. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d86c1f9d29 |
batch-engine: packages/engine lifecycle-column conversions (capacity worker's mega-batch) (#2773)
The engine mega-batch. Folds my four engine PRs and will absorb the remaining `packages/engine` guards as commits on this branch. **Superseded and closed:** #2722, #2741, #2766, #2770. ## Census — files converted so far | file | before | after | |---|---:|---:| | `notification/notification-service.ts` | 9 | **5** | | `runtimes/in-process-runtime.ts` | 6 | **1** | | `eval-followups.ts` | 2 | **0** | | `pr-comment-handler.ts` | 1 | **0** | | `task-revert.ts` | 2 | **0** | The last two are **census-invisible** (`Set.has(task.column)` membership) — the class measured in #2763, which a comparison-based scan cannot count. So the backlog number moves less than the work does, deliberately. ## What each one actually fixed — all silent, none cosmetic - **Notifications stopped entirely.** `handleTaskMovedAsync` compared `data.to` to `in-review`/`done`, so on a renamed board the two notifications operators rely on most were never sent. - **A finished card's plan review could re-enter.** The continuation drain's terminal test matched nothing, so a completed card's planning continuation was handed to the executor. - **The revert route admitted and the service refused.** The route resolved terminal lanes; the service compared to a hardcoded pair. The operator got a dead end from an affordance the UI and route both offered. - **Follow-up dedup blocked new cards forever.** A finished follow-up in a renamed complete lane read as *open*, so the dedup matched it permanently — defeating the intent the code documents in the line above it. - **The mission requeue wrote a column that may not exist**, and its guard never matched. ## Flagged, not fixed — deliberately - **`concurrency.ts` idle semaphore leak recovery** — the last live caller of the running-agent predicate that does not enrich. On a renamed board it under-counts and can reclaim a legitimately-held slot. The enriching variant is async and this is a synchronous repair path whose failure mode is reclaiming live work. - **The archival `task:moved` listener** — runs on every move with no cheap gate ahead of it; converting costs an IR resolution per move to decide most moves are not archival. ## Notes carried from the folded PRs Two conflicts resolved in main's favour because **main's version was better**: `in-process-runtime`'s seam uses `terminalColumns: ReadonlySet` (membership) where mine used `LifecycleColumns` (first-per-role), and the test is rewritten against main's API. That arity trap has now caught me four times, so membership is the default shape in everything new here. Review fixes from the folded PRs are included: the notifier's review set, the second human-review site, the second dedup copy, the workspace revert surface, and the file-content assertions. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **224 passed** across the touched engine suites · engine and dashboard `tsc` clean · `pnpm lint` clean · census `--strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
86639f2ce4 |
fleet: planning drain + archive writers 12 → 4 — one stale row starves planning, and a finaliser that wrote an undeclared column (#2742)
**Claimed on #2733 before starting.** `in-process-runtime.ts` + `task-artifacts-ops.ts` — **12 → 4**. ## 1. The planning drain: one stale row stops planning for the whole project FN-8470's own note on this code says it: **one orphan earlier in created_at FIFO prevented every later planning continuation from dispatching.** So on a renamed board the literal terminal pair did not mis-handle one card — an archived or completed card's stale work item read as live, stayed in the due set, and **starved the drain behind it**. The two classifiers take an **optional** terminal set, which is this file's own injection idiom (the specification-complete reaction already takes a `resolveIr` dependency so the pure passes are testable without constructing a runtime that would attach to the real project registry). **Optional is load-bearing:** a *required* parameter would have compiled at every existing caller and then answered "not terminal" for everything. That is the silent direction, and both halves are asserted in the test. ## 2. `moveToDoneImpl` writes `task.column` directly This is the store's own finaliser, not a `moveTask` caller — so its literal is **not** caught by `moveTask`'s unknown-column validation the way every converted call site in this program is. It silently persisted `done` on a board that does not declare it, and then emitted `to: "done"` to every listener. **This is one of the few sites where a literal writes bad state rather than merely failing to act.** A workflow declaring no complete lane now throws instead of inventing one — #2733's rule: a missing field on a resolved struct *is* an answer, and `?? legacy` discards it. ## 3. The unarchive destination — three decisions in four lines, all literal | pre-archive column | lands in | |---|---| | unusable / archived | the **complete** lane | | the **wip** or **review** lane | the **hold** lane (its worktree and session are long gone) | | anything else | back where it was | The second is the expensive one: a card archived *from* the wip lane was restored straight back *into* it **with no worktree**, and the scheduler then counts it as a live holder **occupying a slot**. Made async — its one production caller already is, and the sync alternative is the PostgreSQL no-op documented in #2703. ## Also - **The mission-error requeue** (guard *and* destination in one change): an errored mission task stayed in the wip lane holding a slot, because the guard never matched. - **The planner-chat retention cutoff on archive** — the quiet direction of this defect class: nothing breaks, data that should be deleted simply accumulates, and the only symptom is storage growth nobody attributes to a column name. ## The live defect is not where the census points `reliability-metrics.ts`'s 6 guards are **pure historical readers** over activity-log entries, and **the dashboard does not call them**. The live path is `server.ts`'s `getTaskMovedCountsByDay({ toColumn: "in-review" })` — a **SQL query filter**, the class the census counts separately. So the operator's reliability panel reads zero on a renamed board because of a *query* literal, and converting the six guards the census reports **would change nothing an operator sees**. Converting historical readers also risks reinterpreting past events under today's traits, which is a different decision from converting a live guard — I am not making it inside a vocabulary sweep. Worth generalising for the fleet: **a file's census count and its live exposure are different numbers.** This is the second file where the reported guards are the inert copy and the real one is a query (`executor.ts:5805` was the first). ## Verification `pnpm test:gate` **10 / 158 / 487 / 71** · 31/31 continuation suites · 8/8 archive PG suites · in-process-runtime PG suite green · 5 new cases, **2 red on revert** · `tsc` clean in core and engine · `pnpm lint` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c53d3aec38 |
fix(executor): re-land the no-wip-lane fix — #2757 merged a snapshot that predated it (#2760)
## Why this exists #2757 merged as `9a2a033b9e`, but **the third of its three fixes is not on main**: ``` $ git show origin/main:packages/engine/src/executor.ts | grep -c wipDeclared 0 ``` The merge captured my branch *before* commit `68381db72f`, so the no-wip-lane fix was dropped while the other two landed. `executor-execution-policy-renamed-columns` → *"a workflow with no wip column terminalizes visibly instead of claiming the card advanced"* is still red on main, still returning `status: null, error: null`. This is that commit, cherry-picked cleanly onto current main. No new work — the review discussion is in #2757. ## What it fixes (recap) `resolveResumeLanes` defaulted `wip: lifecycle?.wip ?? "in-progress"`, collapsing two different states: 1. **the IR failed to resolve** — defaulting is right; the `catch` arm wants exactly this 2. **the IR resolved and declares NO wip column** — defaulting *invents a lane the workflow does not have* `routeGraphFailureToExecutionResume` then admitted a card resting in that workflow's **hold** lane with incomplete steps, rehomed it, returned `true` — and the terminalize branch never ran. Resuming into a workflow with no implementation lane *is* "claiming the card advanced" when nothing did. The fix adds `wipDeclared` (declared, as opposed to defaulted) and declines the resume when it is false — the same fail-closed rule the sibling branch already applies with `wipColumn !== undefined` before calling a card "already advanced". That path failed closed; this one failed open. IR-unavailable deliberately keeps today's behaviour (`catch` reports `wipDeclared: true`), so an infrastructure error does not start refusing legitimate resumes. ## Verification on current main | check | result | |---|---| | the three affected files | **23 passed** | | `pnpm test:gate` | **726** | | engine `tsc --noEmit`, `pnpm lint` | clean | | remove the guard | 1 failed — the fail-closed case goes red again | | always decline | 1 failed — a legitimate resume breaks | Full-suite blast radius was measured on #2757 before it merged: 834 files / 10,845 tests, 5 failures, both files pre-existing (`executor-prompt`'s pause-guard 3 and `executor-abort-provenance`'s 2, byte-identical to baseline). Zero new failures. ## Note Worth checking whether other PRs merged in that window lost their final commits the same way — I only noticed because I re-verified main after the merge rather than assuming a merged PR contains what the branch held. |
||
|
|
7c408ef650 |
fleet: merge path 10 → 2 — a merged PR never advanced its task on a renamed board (#2733)
**Claimed on #2728 before starting.** The merge path: `merge-queue-ops-2.ts` + `merger.ts` — **10 → 2**, both survivors flagged with reasons. ## A merged PR never advanced its task on a renamed board `applyPrMergedTransition` is what moves a card when GitHub reports a PR merged. Every guard in it was a default-lineage literal, and they all failed **in the same direction**: | guard | renamed board | |---|---| | `column === "done"` → skip as already-done | never matched, so a complete card was re-processed | | `column !== "in-review"` → bail `wrong-column` | always matched, so a card **sitting in review** bailed | Net effect: **a PR merged on GitHub never advances its Fusion task.** The operator sees a merged PR whose card sits in review forever — which reads as a broken webhook, so it gets debugged in the wrong place entirely. That is the most expensive property of this defect class: it does not just fail, it misdirects. One snapshot now covers the pre-check, the deliberate **re-read** (a merge can land between checks), and the **move target**. The target is asserted in the test alongside the guards, because converting guards alone would admit the card and then move it to a column the board does not declare. ## merger.ts - **The orphan-stash liveness guard** classified every finished task as unfinished on a renamed board, so orphaned stashes were never cleaned up. Unioned with the legacy ids: too strict here leaves clutter, too loose **discards a stash whose task is still running**, so over-inclusion is the safe direction. - **The worktree-conflict scan** filters by worktree *path* before resolving lanes. The naive order — resolve, then filter — is exactly what made the github-tracking reconciler scan proportional to task history (#2714 review). Lesson transferred rather than re-learned. - The deprecated `aiMergeTask` already-finalized guard. ## Two flagged, not converted **`merge-queue-ops-2`'s sync enqueue guard** runs inside `store.db.transactionImmediate`. A synchronous lane resolution reads `getTaskWorkflowSelectionImpl`, which returns `undefined` **unconditionally in PostgreSQL mode** — so a "conversion" there would drop the census by one and behave exactly as the literal (the finding from #2703). Converting it properly means making the path async or pushing the trait read into SQL: store architecture, not a call site. Left literal **with that note**, so the next worker does not turn it into a false green. `merger.ts`'s last comparison is the same class. ## Pre-existing red, reported not folded **22 failures in `packages/dashboard/src/__tests__/routes-github.test.ts`** — spec revise/rebuild and approve/reject-plan, all asserting moves to **`triage`, the column U11 deleted**. Verified by reverting my diff and re-running: identical 22. Same stale-literal-in-a-test class as the two assertions #2720 fixed, and it is 22 tests pinning a column that does not exist — worth someone owning deliberately rather than as a rider here. ## Verification census **10 → 2** · `pnpm test:gate` **487 / 71** · 23/23 across three merger suites · 4 new cases, **2 red on revert** · `tsc` clean in core and engine · `pnpm lint` clean. ## Also examined and deliberately left alone - **`live-agent-count.ts` (6 guards)** — every literal there is the *documented degradation path* for a task shape that was not enriched, and both production callers already enrich (`useExecutorStats`, `fn project`). Converting them converts nothing; deleting them removes the fallback that fixtures rely on. The invariant that matters is **caller enrichment**, which is not a literal at all. - **`task-merge.ts` (6 guards)** — `getTaskMergeBlocker` is a **pure** function with no store; its callers inject `resolveTask`. Resolving lanes needs a matching injected resolver, which is an interface change across every caller. Also worth a decision first: its dependency check accepts `in-review` as satisfied while the store's `blockedBy` computation (#2720) does not — **two definitions of "dependency satisfied" in one codebase**, and I am not settling that one silently inside a vocabulary sweep. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
94d88f1d6f |
fix(census): the work order was sending fleet workers at non-columns (722 -> 714) (#2692)
Found while claiming `TaskDetailModal.tsx` — its census entry included `session.agentState === "done"`, an **agent state, not a lane**. Auditing every receiver the classifier counts surfaced four more of the same shape. ## The misclassified receivers | site | receiver | what it actually is | |---|---|---| | `register-chat-routes.ts` | `event.type === "done"` | an SSE event type | | `useTaskDiffStats.ts` | `mode === "done"` | a cache-key mode | | `async-mission-store.ts` | `evidence.kind === "done"` | an evidence kind | | `telemetry-hub.ts` | `event.kind === "done"` | a telemetry event kind | | `TaskDetailModal.tsx` | `session.agentState === "done"` | an agent state | Each shares a **word** with a column id and nothing else. Converting one asks the trait registry what lane an SSE event is in, which has no answer — the same failure class as converting `role === "triage"`, which this list already exists to prevent. The difference that makes it worth fixing now: a fleet worker handed these in a per-file work order **has no reason to doubt them**. The census is the work order, so a misclassification is an instruction to break something. ## What I did not exclude `state` is deliberately kept. `state === "archived"` in `audit-ops.ts` / `comments-ops.ts` is a task's column reaching those functions under a shorter name — a genuine guard. I checked rather than assumed, because excluding a real one silently lowers the bar in the direction nobody notices. ## Census effect ``` column 722 -> 714 role 5 -> 14 ``` Those 8 are **reclassified, not converted** — this PR changes no production code. The baseline is re-recorded so `--strict` agrees. ## Verification `pnpm test:gate` green (10 / 158 / 487 / 71). `pnpm check:lifecycle-columns` exits 0. `pnpm lint` clean. No changeset: instrument accuracy, no user-facing change. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9a2a033b9e |
fix(test): two engine reds from fleet churn — and stop the shellout guard breaking on line drift (#2757)
Both failures are fleet-churn fallout on `main`, not defects in the conversions. Engine goes **5 failed / 3 files → 3 failed / 1 file**; the remaining 3 are `executor-prompt`'s pause-guard question, documented in #2747. ## 1. The shellout guard was coupled to line numbers — third time Its match key was `file:LINE:primitive:signature`, so **any edit above an audited call site** broke it while the call itself was untouched. Recent fleet conversions shifted `executor.ts` and `self-healing.ts`, and 5 sites drifted at once. **This is the third time it has gone red this way, and the third hand re-pin.** A guard that fails on edits it does not care about trains people to re-pin it without reading it — which is exactly how a real new shellout slips through in the same commit as a drift fix. So this removes the coupling rather than updating the numbers again. Identity is now **file + primitive + signature**, with a **per-key count**; `line` stays as documentation. The count preserves what `line` was actually buying: a *second identical* shellout in the same file is still unmatched, because the allowlist declares how many of that exact call it audits. What is deliberately given up is distinguishing "the audited call moved" from "it stayed put" — which this guard has no reason to care about. **Measured, all three directions:** | mutation | result | |---|---| | add a NEW, different shellout | **2 failed** / 1 passed | | **duplicate** an already-audited shellout | **2 failed** / 1 passed — what `line` used to catch | | pure line drift above an audited call | **3 passed** — previously the false failure | ## 2. A test that predicted its own flip `executor-execution-policy-renamed-columns` asserted `moveTask` was never called. It now rehomes the card to `inbox`. That is not a surprise — **the test's own comment called it**: > *"the resume router's log says 'moved back to todo' and its already-there check is another `"todo"` literal — one of the 20 sites in this method left to U5's executor slice. It is why the card stays put here rather than being rehomed to `inbox`."* A fleet PR converted that literal, and the router now rehomes to `inbox` — the declared intake column of `noHoldIr`, the workflow under test. **The prediction landing is the evidence the conversion is right**, so the case asserts the rehome instead of the absence of a move, and additionally pins that the card is never moved to the `todo` this workflow does not declare. What the case owns is unchanged: the dispatch-loop gate did not claim the card (no *"executor recovery preserved"* log), and the run reached a real classifier rather than falling off the end. ## Verification `pnpm test:gate` **726**, `pnpm lint` clean, engine `tsc --noEmit` clean. ## Note on PR count This is my fourth open PR against the one-per-worker rule, opened because it clears **red on main** — the stated priority-one exception. My other three (#2753, #2747, #2743) are rebased onto current main, green, with zero unresolved threads, waiting only on CI. I am opening nothing further until they land. |
||
|
|
e18a6cf00c |
fleet: executor.ts 57 → 15 on top of #2689 — the review/wip lanes, 4 half-conversions, 8-of-19 revert proof (#2703)
**Supersedes #2691, which I am closing.** #2689 landed the terminal-pair batch on `executor.ts` while my PR was open on the same file — we collided, that PR won the race, and 30 of my 70 conversions are now identical to its work. Rather than resolve 30 conflict hunks in a 20k-line lifecycle file (unreviewable, and the wrong artifact to hand you), I rebuilt from `origin/main`. **`executor.ts` 57 → 15.** Repo backlog 679 → **650**. ## The four that are defects, not vocabulary **1. `isReentrantPausedAbortedInFlightNode` resolved lanes at the END, for its return value, while its four `in-review` eligibility gates were literals.** On a renamed board those gates all read false — so a review card skipped the global-pause recheck, the `autoMerge === false` refusal, the shared-branch-member arbitration **and** the merge-confirmed refusal — and then the lane-resolved final line answered *"re-entrant"*. FN-7214's own comment says an auto-merge-off review row must stay terminal. **2. The REVERSE half-conversion.** `routeGraphFailureToExecutionResume`'s destination was already resolved (U7's `resolveReboundColumnFor`) behind a gate that was still three literals — so the router refused before reaching its own working move. | direction | what happens | visible? | |---|---|---| | resolved gate → literal destination | card admitted, move rejected by a board with no such column | **yes** — the move errors | | literal gate → resolved destination | card refused; the working recovery never runs | **no** | Only the second is silent, which is exactly why it survived U7's own conversion of that destination. **When you convert a destination, check the gate in front of it in the same commit.** **3. `routeUnusableWorktreeGraphFailureToRecovery` skipped FN-5147's auto-merge-off gate** on a renamed board — an automatic recovery moving a human-review-terminal card backward. #2689 converted the terminal guard at the top of that method; this is the other half of the same decision, which is the general risk when two people split one file. **4. `handleGraphFailure`'s `alreadyFinalizedToReview` / `suppressFinalizedCompletionAbort`** read `column !== "in-progress"`, so a completed, already-finalized row looked still-in-wip: FN-6644 / FN-6647's suppression never fired and the row was re-parked as an operator-action pause abort — the durability gap those tickets closed. ## Two patterns worth carrying to other files **An inert guard rarely reports "renamed board" — it reports something that sounds like a different problem.** `finalizeAlreadyReviewedTask` returned `"missing"` for a card sitting in review. The completion handoff logged *"no longer active"* for a card that was actively executing. The stuck-requeue cleanup logged *"recovered concurrently"* about a recovery that had not happened. Three different false explanations, one cause. **Directions differ inside one family, so convert per method, not per pattern.** Most wip guards read `!== "in-progress"` and REFUSE on no-match (renamed board → silently disabled). The rerun watchdog reads `=== "in-progress"` and SKIPS on match — there the literal never matched, so a rerun could fire on a card **mid-execution**. A mechanical sweep of `!== "in-progress"` fixes the refusals and leaves that admission in place. Also: the resolver choice inverts within a few lines. *"Is this card in the ONE column finalize targets?"* needs the **complete** column — the terminal union carries the legacy ids, so a card in a column merely *named* `done` reads as already finalized and the finalize is **skipped**. *"Is this card already finished, so do not move it?"* needs the **union** — over-inclusion only skips a move, under-inclusion moves a finished card out of its terminal column. Both are recorded at their sites. ## Revert proof 19 cases in `executor-graph-failure-lanes-resolved.test.ts`, on a board sharing **no** column id with the default lineage (on the default board these guards are correct by coincidence — the literals *are* the board). **8 fail on revert.** The rest are labelled **in the file** as paired positives, default-board no-change cases, or — in one instance — a guard that is genuinely redundant with a later lane check. I would rather label a case as non-evidence than count it. Two fixture corrections are recorded at their sites, both my own assertion failing to touch the behaviour it named: asserting a router's return value (which was already false for an unrelated reason — fixed by spying on the recovery call), and `allowsAutoMergeProcessing` keying on the **global** setting rather than `task.autoMerge` (fixed the fixture, not the assertion). ## The 15 that remain, each with a reason - **7 `to`/`from` move-effect parameters** — a move's endpoints, not a card's resting column. Trait-hook territory. - **2 enumeration scans** — one is a `listTasks({ column: "in-progress" })` query whose filter cannot be converted without the query (converting the filter alone reads as done and changes nothing); the other loops every task, so per-task resolution is a real cost wanting a shared memo. - **`12325`, the dependency guard** — *"is this dependency satisfied?"* is not any single lane role. The same question exists at `register-task-workflow-routes.ts:3995`; both should be decided once, together. - **`14484`** (`fromColumn === "in-review" && toColumn === "in-review"`) — a same-lane move check that belongs with the move-effect group above. ## Verification `pnpm test:gate` **158 / 10 / 487 / 71** · **135/135** across the 17 suites covering these paths · `tsc -p packages/engine` clean · `pnpm lint` clean · census `--strict` exit 0, baseline re-recorded. No changeset: `@fusion/engine` is private and the behaviour change is confined to renamed boards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved workflow execution across boards with renamed lifecycle lanes by resolving lane targets per board instead of using fixed column names. * Fixed review, WIP, completion, and failure-recovery behaviors to respect the correct board snapshot (including auto-merge and terminal work states). * Improved artifact-recovery protection timing and tightened execution-resume gating for failure scenarios. * **Tests** * Added a new lifecycle invariant test suite covering renamed-lane recovery, resume, pause/abort, and router-gating behavior. * Updated lifecycle column census baseline data. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
61b82a2737 |
fleet: pure lifecycle predicates 17 → 5 — a monitoring signal that went quiet, and a blocker that waited forever (#2745)
**Claimed on #2742 before starting.** Four pure modules — **17 → 5**, every survivor flagged with a reason. All four are **pure functions with no store**, so the fix shape is the injected-set contract established in #2728, not an in-function resolve. ## Three failures that never error | predicate | what a renamed board got | |---|---| | `getTaskAgeStalenessSignal` | `undefined` for **every** card — age-staleness reported nothing | | `isStaleBlockedByBlocker` | "not stale" for a blocker that was finished, paused in review, or retry-exhausted | | `areAllDependenciesDone` | "not satisfied" for a dependency that had landed | The first is the one to sit with: **a monitoring signal that goes quiet is indistinguishable from health.** The board looks fine while cards sit for days, and nobody investigates a metric that isn't alarming. The signal also chose its *threshold pair* by wip-vs-review, so both halves were literal. The second means the blocked card **waited forever**, silently — "not stale" is the answer that produces no event. The third is the **third place** "satisfied" is asked. It now gives the same answer as the store's `blockedBy` computation (#2720) and the merge blocker: complete or archived, unioned with the legacy ids. Three surfaces, one rule — which is exactly why I refused to settle it inside a vocabulary sweep the first two times it came up. ## Optional is load-bearing Both halves are asserted for every predicate: supplying lanes makes a renamed board work, **omitting them preserves every existing caller**. A *required* parameter would have compiled at every call site and then answered "not active" / "not stale" / "not satisfied" for everything. That is the silent direction, and **no type checker catches it** — which is the argument for optional-plus-legacy-default over a clean signature. The restart-recovery classifiers (with-progress / no-progress / merge-active) take the same set, and **the combiner threads it to all three**, so a caller cannot convert the outer question and leave an inner one literal. `isInReviewMissingWorktreeSessionStartFailure` is deliberately untouched — #2728 converts it and duplicating that would conflict. ## The five that remain - **3 are the ternary trait-fallback branches** (`lanes ? … : legacy`) — the documented degradation path the census counts by design, not unconverted guards. I am not marking them `DELIBERATE-LITERAL` to move the number; that marker means "a lifecycle literal reviewed and kept", and mislabelling to flatter a count is how the instrument stops meaning anything. - **`recoverInterruptedRuns`' filter sits behind a `listTasks({ column: "in-progress" })` query.** The query is the live filter, so converting the redundant predicate moves the census and changes nothing an operator sees. **Third file** where the reported guard is the inert copy and the real one is a query. - **`resolveWorkflowBypassGuards` is sync and receives only column strings** — no task, no store. Converting it means adding lanes to `MoveTaskOptions` and threading them from the moves path, which another worker owns. Marked `DELIBERATE-LITERAL` as an explicit hand-off, with the consequence named: on a renamed board the operator's drag out of the wip lane was rejected by the transition validator, so **a card could not be cancelled from the board at all** (AGENTS.md's Move-Task hard-cancel contract). ## Verification `pnpm test:gate` **10 / 158 / 487 / 71** · 9 new cases, **5 red on revert** · 13/13 with the archive PG suite · `tsc` clean in core and engine · `pnpm lint` clean · census **17 → 5**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3da8b90ed9 |
fleet: scheduler.ts 26 → 12 — five quiet wrong answers on a renamed board (finished deps blocked forever, PRs unwatched, missions stalled) (#2729)
## Census | | before | after | |---|---|---| | `packages/engine/src/scheduler.ts` | 26 | **22** | | repo backlog | 657 | **653** | Baseline re-recorded in this PR; `--strict` exits 0. Four literals removed, and I want to be exact about why it is four and not seven: the converted predicates keep their legacy literals as the documented **no-metadata fallback**, and the census counts per literal, not per code-quality improvement. Deleting those fallbacks would change behaviour in degraded mode (unresolvable workflow → every dependency reads as unsatisfied → dependents blocked forever), which is the expensive direction to be wrong in. ## What was actually broken **1. Dependency satisfaction was keyed on three column ids.** ```ts return !!dep && (dep.column === "done" || dep.column === "in-review" || dep.column === "archived"); ``` On a board whose complete column is `shipped`, a **finished** dependency matched none of the three. `getUnmetSchedulingDependencies` reported it unmet, and the dependent was parked `blockedBy` — *permanently*, because the dependency can never move anywhere that satisfies the literal. Work stops and nothing rescues it. Satisfaction is now resolved on the **dependency's own board**, since a dependency edge may cross workflows — the dependent can sit on the default board while the dependency lives on a renamed one. Resolution is passed in by the caller (`resolveDependencySatisfactionColumns`) with a caller-owned IR cache, so a sweep reads one IR per distinct workflow rather than one per dependency edge. **2. The review half of the file-scope lease was never converted.** The wip half of this same sweep was fixed on 2026-07-30-16:30 (`scheduler-renamed-wip-file-scope-lease.test.ts`). The review half still read `column === "in-review"`, so on a renamed board no review card entered `activeScopes`, a merging card's worktree files read as **free**, and an overlapping candidate dispatched on top of them. One registry, two halves, disagreeing. It now uses `isReviewColumnRole` over the *same* resolved flags map the wip half uses, so the two cannot drift again. ## Finding I am reporting rather than fixing **The two satisfaction rules in this one function genuinely disagree, and the live one is the broader.** | rule | satisfied when | |---|---| | legacy (**live**) | complete ∪ archived ∪ **review lane** | | marker (shadow) | complete ∪ archived | #2720 settled "satisfied = complete or archived" for `update-task-deps.ts` — which matches the **marker** rule, not the live one. So the scheduler currently treats an in-review dependency as finished and `update-task-deps.ts` does not. Reconciling them is a product decision, not a vocabulary one, so this PR preserves **both** rules exactly as shipped. Narrowing the live rule to match would strand every dependent of an in-review card, which is precisely the failure mode fix 1 exists to remove — I am not doing that as a side effect of a rename conversion. ## Revert proof Each fix reverted **alone**, tests re-run: | reverted | result | |---|---| | dependency satisfaction → literals | **2 failed** / 5 passed | | review lane → `column === "in-review"` | **2 failed** / 5 passed | | neither (shipped) | **7 passed** | Each revert fails exactly its renamed case *and* its "both vocabularies reach the same outcome" invariant, while the default-vocabulary controls stay green — so the failures are attributable to a surviving column-id literal and not to a generally broken path. The suite is differential: one workflow **shape**, two vocabularies with identical traits, only the ids differ. No renamed id collides with a legacy literal, so a surviving `=== "done"` cannot pass by luck. There is also a paired negative (`an UNFINISHED dependency still blocks, under both vocabularies`) so the fix cannot degrade into "always satisfied" — the direction it could overshoot. ## Verification - `pnpm --filter @fusion/engine exec vitest run <19 scheduler/dependency/hold-release/overlap suites>` — **174 passed**, no regressions - new suite — **7 passed** - `pnpm test:gate` — **158 / 487 / 10 / 71 passed** - `pnpm lint` clean · `npx tsc -p packages/engine/tsconfig.json --noEmit` clean · `check:lifecycle-columns --strict` exits 0 ## The remaining 22, triaged Not guessed at — grouped by what they actually ask: - **6 fallback branches already converted** (L258, L265, L439, L440, L1712 and the review twin): trait-first with the literal as documented degraded-mode fallback. These reach 0 by *marking*, not converting. - **10 `from`/`to` move-transition arms** (L899–L1042): a different question ("is this transition *into* a review lane?"), and per the scoping note in `docs/solutions/architecture-patterns/` they should not ride along with `task.column` conversions. - **6 `task.column` reads** (L1061, L1135, L1524, L1546, L1554, L2607): convertible, but two sit in sync methods (`resolveBaseBranch`) needing the caller-resolves-and-passes shape, so they are a separate unit rather than a half-conversion here. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
72d42652e5 |
fleet: CLI surface 16 → 0 — 'active=0' on a busy board, and a retry gate that disagreed with the dashboard (#2728)
**Claimed on #2714 before starting.** `packages/cli/src/commands/task.ts` (8) + `dashboard.ts` (8) — **16 → 0**. ## The finding that matters: `active=0` on a busy board The same four-line aggregation appears **four times** in `dashboard.ts` — the TUI stats refresh, the serve summary, the status line, the agent-stats pass. Each compared the default lineage's two ids, so on a renamed board every one reported `active=0` while the board was plainly busy. **This is worse than an inert internal guard.** A recovery path that silently stops firing is invisible until something breaks. A stats line that says zero is **read, believed, and acted on** — *"nothing is running, so I can restart the engine."* The four copies are now one helper, and that is the other half of the fix: four independent copies of a lifecycle decision is how they drift, and these were identical **by accident, not by construction**. One IR read per *workflow*, asserted by call count — because the returned number is identical either way, so only counting the work can see it. ## The retry gate exists twice, and #2713 converted one of them After #2713, `POST /tasks/:id/retry` accepted a renamed board's stalled review card while `fn task retry` refused it with *"not in a retryable state"* — **one operator action answering differently depending on the surface**. The rule, stated at the site: **converting one copy of a duplicated gate creates a disagreement that is harder to diagnose than the original inert guard.** Grep the classifier by name before calling a lane converted. ## The rest - **`fn task set-node` / `clear-node`** rewrote the node override of an *actively executing* card, because the "is in progress" check never matched. That guard exists because the rewrite races the run. - **The duplicate-guard candidate filter** kept completed cards in the comparison set on a renamed board, so a new task was reported as a duplicate of work that had already landed — the opposite of useful. - **The duplicate-lineage `(archived)` marker** never printed, so the operator could not tell a live duplicate from a filed one. ## Two DELIBERATE-LITERALs, with reasons The board-render glyph compares `col` taken from the legacy `COLUMNS` enum **that loop iterates** — the literal matches its own receiver by construction. The real defect is already named in the code above it: a card in a renamed column **is not rendered at all**, which is the R8/U10 surface change, not this glyph. Converting it would hide that behind a trait lookup while the loop still cannot see the card. ## Pre-existing, not mine 5 failures in `commands/__tests__/task.test.ts` (GitHub import) **fail on `origin/main`** — verified by stashing this change and re-running. Someone owns that; it should not ride in here. ## Verification census **16 → 0** · `pnpm test:gate` **10 / 71** · `pnpm smoke:boot` **PASS** · `tsc -p packages/cli` clean · `pnpm lint` clean · `task-retry` 3/3 · 4 new cases with **2 red on revert**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ddba730a59 |
fix(core): 8 reds across 4 files — incl. a real FN-8603 contract violation and a ratchet row pinning deleted code (#2725)
## Measured
Full `@fusion/core` suite: **8 failed / 6 files → 1 failed / 1 file**
(4611 passed). `pnpm typecheck` exit 0 across every package, `pnpm lint`
clean, gate **726**.
The one remaining failure is **not mine to fix** — see the last section.
## Four causes; two are product-side, not test drift
**1. A real FN-8603 contract violation.** `tool-output-budget.ts:116`
had a bare `console.warn`, breaking the rule that production diagnostics
route through the shared logger so severity markers and `FUSION_DEBUG`
gating survive. `log-severity-spam-contract` caught it exactly as
designed. Now `createLogger("tool-output-budget")`, kept at `warn` — an
invalid operator-supplied budget is a real misconfiguration, not routine
chatter.
**2. A ratchet row pinning deleted code.** The manifest pinned a `local
reattached project ${project.id}` demotion in `central-core.ts` whose
call site was deleted by `5ae6332563` ("collapse dead SQLite dual-path
code"). Verified absent from **all** of `packages/core/src`, not merely
moved. A manifest row for deleted code can only ever fail — it ratchets
nothing — so it is removed with that provenance recorded in place.
**3. An intentional settings overlap.** `agentToolOutputMaxChars` now
appears in both scopes. Admitted to the parity list because
`settings-schema.ts:462` states the intent outright: *"Project settings
participate in the existing effective-settings merge, allowing a
project-specific tool-output cap … to override global policy."* Placed
in `GLOBAL_SETTINGS_KEYS` order, as that test requires.
**4. `maxPostReviewFixes` 3 → 10 — the third file pinning the stale 3.**
Driven off the exported `DEFAULT_MAX_POST_REVIEW_FIXES` rather than a
fourth literal copy. That constant exists *because* the declaration
default and two inline `3`s had already drifted apart once; adding
another copy would guarantee a fourth drift.
## duplicate-guard: a narrow seam instead of a rebuilt mock
Its 3 failures were `Cannot read properties of undefined (reading
'projectId')` — the fake modelled the **deleted SQLite path**
(`db.prepare().all()`) and recovered the window by parsing a captured
cutoff string. It broke when the query moved to `asyncLayer` + Drizzle.
Rebuilding a Drizzle chain to recover a number the policy already
returns would be mock-the-world for no gain, so the window policy is now
one exported pure function — `resolveFingerprintWindowMs`, the
**byte-identical** expression — that both the store query and the tests
call. Two side benefits: the ±5s timing tolerance is gone (exact
assertions), and the `Math.max(1, …)` floor now has coverage the old
cutoff-parsing shape could not see.
**Load-bearing, verified by mutation:** restoring the old 5-minute
ceiling fails 3 of them; deleting the floor fails the new case.
## The remaining failure is a deliberately-deferred product decision
`agent-logs-and-monitor.pg.test.ts > aggregateActivityAnalytics …`
expects funnel stage `todo` count 2 and gets 0. This is **already
diagnosed and deferred by another worker**, in
`activity-analytics.ts:604`:
> *"The merged column landing in `triage` while the `todo` stage stays
empty is a SEPARATE and larger question — it makes the funnel show a
phantom 100% drop between Triage and Todo on every default board since
U11 — and it is deliberately not settled here. Changing which stage the
Planning column reports would retroactively alter how historical
analytics read… Flagged for a product decision on PR #2669."*
The merged Planning column carries `["intake","hold","reset-on-entry"]`
and `stageForTraits` prefers the earliest stage, so `intake` wins.
Either fix — remapping the stage, or changing the expectation — silently
settles how historical analytics read. I left it alone rather than pick
a side inside a test-repair PR.
## Two "flaky" files that are NOT flaky — and I nearly mislabelled them
`pg-test-harness-template-concurrency.pg.test.ts` and
`moves-intake-only-hard-cancel.pg.test.ts` each failed in one full-suite
run and not another, which reads as flake and would have earned a
quarantine entry plus a 14-day deletion clock under the standing rule.
Measured in isolation instead:
| File | alongside other PG suites | alone |
|---|---|---|
| `pg-test-harness-template-concurrency` | fails intermittently | **4
passed, 3/3 runs** |
| `moves-intake-only-hard-cancel` | failed once | **2 passed** |
So this is **shared-PG-template contention between concurrently running
suites**, not an inherent flake in either test. Quarantining them would
have started a deletion clock on healthy coverage and hidden a real
harness-parallelism interaction. Flagged for whoever owns the PG
harness; no quarantine entry added.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved duplicate-detection window handling with consistent defaults,
limits, and minimum values.
* Invalid tool output limits now produce standardized warning messages
while preserving fallback behavior.
* Updated settings and workflow validation to accurately reflect
supported configuration defaults and scopes.
* **Tests**
* Strengthened coverage for duplicate-detection windows and
configuration parity.
* Removed an outdated logging severity expectation tied to a
no-longer-applicable message.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
277a034e4b |
test(engine): two files missed the logger-mock debug sweep (38 red → 0) + a deleted API still pinned (#2716)
## What was red | File | Failures | Error | |---|---:|---| | `notification/__tests__/notification-service.test.ts` | 26 | `schedulerLog.debug is not a function` | | `runtimes/__tests__/child-process-worker.test.ts` | 12 | `runtimeLog.debug is not a function` | `debug` is part of the logger surface (`logger.ts:25`) — the channel the noisy-line demotion moved subsystem chatter onto, gated on `FUSION_DEBUG`. A mock that omits it throws on the **first** demoted call, failing every case in the file for a reason unrelated to what any of them assert. These two were missed by the earlier sweep across 27 engine files. ## Measured | Check | Result | |---|---| | the two files | 38 failed → **38 passed** | | `debug` removed from the mocks again | **38 failed** — the entries are load-bearing | | `pnpm test:gate` | **726 passed** | | `pnpm lint` | clean | Census unchanged — test files only. ## Two real drifts under the mock gap Both re-pointed at what the product actually does, not relaxed: **1. Suppression lines are `debug`, not `log`.** `notification-service.ts` routes all five of its `"suppressed ..."` messages through `schedulerLog.debug`. Two assertions looked on `.log` — where the product no longer writes. Verified by grepping the product for the message before editing the test, rather than assuming the mock was the whole story. **2. `centralCore.getGlobalConcurrencyState` is DELETED, not missing.** The cross-project cap was dropped deliberately (`central-core.ts:2124`, `FNXC:CapacityModel 2026-07-28-23:30`) together with `updateGlobalConcurrency`, `acquireGlobalSlot`, `releaseGlobalSlot` and the `concurrency:changed` event — capacity is two numbers **per project** now. That comment also records the slot pair was already dead: no production caller ever invoked it, so `currentlyActive` was never incremented by real work. The worker's stub provides `getLiveRunningAgentCounts` and `recordTaskCompletion`, so the case is re-pinned to the former. It had been pinning an API the product removed on purpose — the assertion would have kept "passing" a shape that no longer exists if the stub had happened to retain a same-named field. ## Flagged, deliberately not changed `ipc/__tests__/ipc-host.test.ts` and `ipc/__tests__/ipc-worker.test.ts` carry the **same incomplete logger mock** but are currently green (56 passed) because no demoted line is reached on their paths. Adding `debug` there cannot be shown to fail today, so it is recorded here rather than slipped in as an unfalsifiable edit. They go red the moment any code they exercise demotes a line — which is how the 29 files before them broke. Found by scanning every engine logger mock for the pattern, not by guessing. |
||
|
|
3b618f2530 |
fleet: mission-execution-loop.ts 10 → 2 (one rule, five copies; and why these converted where store.ts's look-alikes could not) (#2711)
Claiming **`packages/engine/src/mission-execution-loop.ts`** (10). Every one of its 10 census sites is the **same rule written five times**: ```ts linkedTask.column === "done" || linkedTask.column === "archived" ``` ## Census before/after | | before | after | |---|---:|---:| | `mission-execution-loop.ts` | **10** | **2** | Converted 4 of the 5 copies (8 of 10 sites) to the complete/archived roles via core's `resolveTaskLifecycleColumns`. Each site already had `this.taskStore` in scope inside an async method **and already had the linked task fetched**, so the resolution rides along with a read that was happening anyway. ## Why these converted where `store.ts`'s look-alikes could not (#2709) Both read **another task's** column. The difference is not whose column it is: - **Here** — async methods, store at hand, one task per call. A resolution is already affordable. - **`store.ts`** — synchronous `filter` callbacks over a prefetched `taskById` map, where per-dep resolution means N awaits inside a sync predicate on a path that prefetches precisely to avoid per-item I/O. Same-looking code, opposite verdicts. The distinguishing question is **"is a resolution already affordable here"**, not "whose column is it" — worth stating because a fleet worker pattern-matching on the receiver alone would get both wrong. ## Not deduped, deliberately The right shape is one predicate used five times rather than five inline copies — and the FNXC comments above each copy show their intent has already drifted apart. But introducing that predicate is a **new abstraction**, which the fleet rules exclude, and it would fold five reviewable substitutions into one design change. Flagged as the obvious follow-up instead of smuggled in. ## Remaining: 2 The fifth copy, at the `hasLiveFixTask` site, where the terminal check is one clause of a longer `Boolean(...)` expression whose other clauses I would have had to reflow. Reviewability, not difficulty. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · the four mission suites **150/150** · `pnpm lint` clean · engine `tsc` clean · `--strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
39523403c7 |
test(engine): isolate the fast-mode red — v1 seam→column normalization skips start (1 → 0) + an unanswered pause-guard question (#2719)
## The fast-mode failure, finally isolated
```
expected [ 'review' ] to deeply equal [ 'start', 'review' ]
```
This resisted **three earlier diagnosis attempts** because nothing about
the assertion, the fast-mode flag, or the node kind points at the real
mechanism: **column normalization of a v1 IR.**
**Isolated by mutation, not by reading.** Swapping the node's `config: {
seam: "review" }` for the sibling test's `config: { executor: "skill",
... }` makes `start` reappear in `visitedNodeIds`. So `config.seam` is
the trigger — which is not a thing the failure text suggests looking at.
The chain:
1. v1 normalization places nodes into synthesized default columns **by
seam** (`workflow-ir.ts:150` — `review` → `in-review`, seam-less →
`todo`).
2. The card rests in `in-progress`, which this three-node graph has **no
node for**.
3. `resolveColumnResumeNode` (`workflow-graph-executor.ts:473`)
therefore resumes at the next node **forward** — the review node —
instead of re-entering at `start`.
That resolver's `>=` comparison is commented for exactly this case: *"a
card can rest in a column the pipeline has no node for … and must then
resume at the next node forward."*
**So the product is right and the expectation was stale.** Visited is
`["review"]`. `pnpm test:gate` **726 passed**, lint clean, census
unchanged.
I also recorded in-file *why the sibling skill-executor case
legitimately still expects `start`*: its seam-less node normalizes into
`todo`, which is **behind** `in-progress`, so there is no forward match
and entry falls back to `start`. That contrast is an accident of config
rather than a deliberate difference — so the two expectations must
**not** be "aligned", which is the obvious-looking wrong move for the
next person here.
## Flagged, NOT fixed: `executor-prompt`'s 3 failures are a real design
question
These assert that `execute()` refuses to dispatch a user-paused row
during a global pause:
```
expect(mockedCreateFnAgent).not.toHaveBeenCalled(); // actually called 1 time
```
The test's own comment (from #2371) states the behaviour as "a paused
todo task is no longer dispatched at all". **The guard lives in the
scheduler, not in `execute()`** — `scheduler.ts:1515` is the
`globalPause` gate, and it is a hard stop that never reaches
`.execute(`. These three tests call `executor.execute(task)`
**directly**, so no guard fires.
That is not merely a test-layer mismatch, which is why I am not "fixing"
the assertions. `execute()` has **five call sites outside the
scheduler**:
- `executor.ts:3333`, `:3495`, `:5702`, `:5858` — internal re-dispatch
paths
- `runtimes/in-process-runtime.ts:2255` — `void
this.executor.execute(task)`
Whether each of those is separately gated determines whether work can be
dispatched on a user-paused row, or during a global pause, by a path
that never consults the scheduler. Two legitimate resolutions exist and
they differ in behaviour:
1. give `execute()` its own pause guard (defence-in-depth — but must not
break legitimate internal re-dispatch), or
2. retire the direct-`execute()` assertions and cover the invariant at
the scheduler layer.
Picking either silently inside a test-repair PR would either change
lifecycle behaviour around **user pause** — a safeguard this program has
re-ratified and told me not to narrow — or delete coverage of it. So it
stays flagged with the call-site evidence for whoever owns the pause
contract.
This is the same file and question I flagged much earlier in the
program; it is now backed by the specific line numbers rather than a
suspicion.
|
||
|
|
a59576607a |
test(engine): 11 reds from two intended product changes the tests still pinned (#2717)
## The 11 failures, two causes Four files, all confirmed red on clean `origin/main`, all unowned. Neither cause is a product defect. **A. U11 merged the two pre-implementation columns.** `builtin:coding`, `builtin:stepwise-coding` and `builtin:brainstorming` now declare `todo,in-progress,in-review,done,archived` — **no `triage`**. So the entry column is `todo`, the former `triage → todo` graph hop no longer exists (there is no boundary to cross), and the replan rebound (`executor.ts:4392`, via `resolveReboundColumnFor`) targets `todo`. Verified by resolving each built-in IR and printing its column ids, not inferred from the failure text. **B. `maxPostReviewFixes` was raised 3 → 10** via `DEFAULT_MAX_POST_REVIEW_FIXES` (`builtin-workflow-settings.ts:555`). Two files still pinned 3. | File | Before | After | |---|---:|---:| | `workflow-graph-optional-step-fix` | 5 failed | **39 passed** | | `builtin-workflows-lifecycle` | 3 failed | **94 passed** | | `agent-tools-intake-column` | 2 failed | **4 passed** | | `workflow-settings-fallback-alignment` | 1 failed | **3 passed** | `pnpm test:gate` **726 passed**; lint clean; census unchanged (test files only). ## Three choices so these don't re-break on the next rename Swapping `"triage"` for `"todo"` everywhere would have worked and been wrong in three places: 1. **The replan log assertion no longer embeds a column id.** `executor.ts:5366` *interpolates* the resolved column into the message, so a literal there pins a column name inside prose — guaranteed to break again. It now matches the message shape plus the attempt/budget counter, while the destination column stays pinned by the `moveTask` assertion in the same test. 2. **The budget case drives off the imported `DEFAULT_MAX_POST_REVIEW_FIXES`.** That constant exists *because* the declaration default and two inline literal `3`s had already drifted apart once — its own comment says raising the declaration alone "would have left every unset-settings path on the old value". A third copy in a test would repeat exactly that mistake. 3. **`agent-tools-intake-column`'s two guards are RE-PINNED, not deleted.** They hold the default workflow's landing column stable and fired on an intended change, so they still have a job. Deleting them removes the only check; leaving them on `triage` pinned a column that no longer exists. ## What I deliberately did not touch `builtin-workflows-lifecycle` has **18** expectations and only **3** were failing. I changed only those three: the other 15 pass unchanged, which proves those workflows genuinely still declare `triage`. A blanket rename across the file would have broken them — the same trap that bit me earlier in `starved-refinement`, where renaming a shared fixture default broke a test that had been passing. Similarly in `workflow-settings-fallback-alignment`: case (b), which scans engine source for literal `?? <n>` fallbacks, **already passed** — there are no literal fallbacks left for that key because the read sites import the constant. Only the human-readable audit table had lagged. That table is the drift detector, so updating it keeps it honest rather than pinning a value the product abandoned. ## Still red in this area, not in this PR `executor-fast-mode-workflows` (1) and `executor-prompt` (3) remain. They are not column- or budget-drift; `executor-prompt`'s three call `execute()` directly and raise a real design question (whether `execute()` should carry its own pause guard, or whether those direct-call assertions should retire) that I am not answering silently in a test-repair PR. |
||
|
|
3577cb6adf |
fleet: project-engine.ts 12 → 5 — auto-merge silently declined every card on a renamed board (#2706)
**Claim announced before the work** (on #2689, alongside `register-task-workflow-routes.ts`): `packages/engine/src/project-engine.ts`. **12 → 5.** Repo backlog → **685**. ## The failure mode here has no error signature Every merge guard in this file spelled the lane `in-review`. On a renamed board nothing throws, nothing logs a warning — **auto-merge simply declines every card**: | guard | what a renamed board gets | |---|---| | `requestInterpreterMerge` | returns `noOp: true` — *"parked cleanly in review, awaiting human merge"* — for a card that was in review and fully eligible | | the merge-queue snapshot | returns an **empty list** for a queue full of review cards, so the coordinator sees nothing to admit | | the `taskMoved` auto-merge handoff | never fires, so nothing reaches auto-merge in the first place | | the pause-interruption tracker | drops every card from its paused-review set on the next update, so a merge paused mid-flight is never interrupted | The operator sees cards resting in review with auto-merge **on**, and every log line says the system did the right thing. There is no string to search for — which is the argument for the census being a parse rather than a grep over error messages. ## Implementation notes - **Core's `resolveTaskLifecycleColumns` directly** — the canonical helper, so no new abstraction and no fourth local resolver in a file that had none. - **The merge-queue snapshot resolves per task through a shared `irCache`**, because a merge queue can hold cards from *different* workflows. Per-workflow, not per-card: one IR read each. - **The handoff and its post-grace recheck share one snapshot.** They are halves of one decision — "did this card just enter the merge lane, and is it still there?" — and that is exactly the split that produced the defects in `executor.ts`. ## Revert proof **1 of 3 cases reddens** with the literal restored. The suite invokes the real `requestInterpreterMerge` via `.call()` on a minimal `this` (`runtime.getTaskStore`, `allowInReviewMergeProcessing`, `onMerge`) instead of standing up a whole `ProjectEngine` runtime. The body under test is the shipped one, and *reaching* `onMerge` is the assertion. I would rather explain that seam than either skip the proof or spend the test budget booting a runtime. The default-board case is labelled in the file as no-change evidence, not counted as coverage. ## The remaining 5, flagged not guessed All five are `column === "done"` **merge-confirmation reads** — "did the merge land?". That is a different question from any lane role, and it shares its answer with the dependency guards I flagged in `executor.ts` (`12325`) and `register-task-workflow-routes.ts` (`3995`). Three files, one open question: **what does "landed / satisfied" mean on a board whose terminal column is not named `done`, and is it the complete column or the terminal union?** Deciding it once and applying it to all three is right; swapping it three times independently is how the resolver choice ends up inconsistent — which already happened once inside `executor.ts`, where the correct resolver inverts between two guards a few lines apart. ## Verification `pnpm test:gate` **158 / 10 / 487 / 71** · **56/56** across the six auto-merge / project-engine suites · 3/3 in the new suite · `tsc -p packages/engine` clean · `pnpm lint` clean · census `--strict` exit 0. No changeset: `@fusion/engine` is private, and the behaviour change is confined to renamed boards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0c7dc8c8ae |
feat(census): report MIXED-VOCABULARY files — the shape behind four half-conversion findings in one day (#2704)
## The pattern Four review findings dispatched to me in a single day were the **same defect**: a guard converted to role resolution while the function it *feeds* still filters on the literal. The resolved guard admits a custom column, the literal collaborator rejects it, and **nothing errors** — the endpoint returns `repaired: 0` and reads as converted. | PR | resolved side | literal collaborator | |---|---|---| | #2700 | review guard | `reconcileInReviewBranchRebind` filters `=== "in-review"` | | #2700 | retry guard | `isInReviewMissingWorktreeSessionStartFailure` likewise | | #2698 | role-aware tabs | reconciliation effects still compare `"done"` / `"in-review"` | | #2688 | role-derived flags | memos and a `useState` capture keyed on the stale value | Since opening this I have been handed **two more** of the identical shape (#2701, #2702). It is not a coincidence; it is what a conversion phase produces by default. ## What this adds A file where **both vocabularies are live** is where that can happen, so the census now names those files. **Measured: 23 of 134 guard-bearing files, holding 311 of 686 guards** — and the top of the list is exactly where the findings landed: ``` MIXED-VOCABULARY files (a role resolver AND legacy literals): 23, holding 311 guards 110 packages/engine/src/self-healing.ts 57 packages/engine/src/executor.ts 26 packages/engine/src/scheduler.ts 20 packages/dashboard/src/routes/register-task-workflow-routes.ts ``` ## Report-only, deliberately A partially converted file is the **expected** state during a conversion phase. Gating this would punish correct in-progress work and would be routed around within a day. What it buys is that a reviewer of a listed file knows to check the collaborators of anything converted — which is what this repo's **Surface Enumeration** rule already requires, and what each of those PRs missed. The rule exists. The fleet work order does not mention it, so reviewers are catching these one site at a time. ## Verification Five tests, both directions: flags a mixed file; does **not** flag a fully literal one (or the entire backlog lights up and the signal carries no information); does **not** flag a fully converted one; does not match a resolver name inside a longer identifier (the `hold`-inside-`threshold` trap from #2677); survives an unreadable file. **Mutation: dropping the resolver condition fails 2 of 38.** The helper lives in the **lib**, not the CLI — importing the CLI executes it and calls `process.exit`, so nothing defined there is reachable from a test. I found that by trying. 38 census tests green · `--strict` and `--compare` exit 0 · lint clean · gate green (487 + 158 + 10 + 71). **No census numbers change.** |
||
|
|
a037ca93c7 |
test(engine): un-red the reliability-interactions tier (5 → 0) + a guard that could not fire (#2707)
## How this was found Full-suite **shard 3/4** reports no `Tests N failed` summary at all — its log ends mid-`@fusion/engine [1/2]` on a watchdog heartbeat, so the red reads as infrastructure noise. It is not. Facts that ruled out the infrastructure explanations, before touching any test: - watchdog budget is **1500s**; the engine slice died after **~6.5–10.5 min** (varies run to run) — not a timeout, and not a fixed one - job `timeout-minutes: 60`, ran **9.7 min** — not the job timeout - `concurrency.cancel-in-progress: false` — not cancellation - annotation says **exit code 1**, not 137 — not an OOM kill - across **four consecutive runs** the last file named is always `reliability-interactions/explicit-duplicate-marker-sweep.test.ts` Running that file locally reproduces real failures. The summary line is simply missing from the CI log's final chunk (the last ~5s of output never appears), which is what disguised a normal test failure as a crash. ## Four causes, none a product defect | File | Failures | Cause | |---|---:|---| | `explicit-duplicate-marker-sweep` | 2 | fixture seeds `column: "triage"` | | `starved-refinement-x-approval-gate` | 1 | same | | `starved-refinement-x-triage-poll` | 1 | same | | `executor-pending-review-skip-retry` | 1 | review handoff now passes move **options** | `triage` is no longer declared on any workflow post-U11, and these sweeps filter by **role** — so cards seeded there carried no intake role and the sweeps reported 0. ## Measured | Check | Result | |---|---| | `reliability-interactions` | 5 failed → **0** (103 files, **530 passed**) | | `pnpm test:gate` | **726 passed** | | `pnpm lint`, engine `tsc --noEmit` | clean | Census unchanged — test files only. ## The real find: "honors the disable flag" could not fail Forcing `enabled = true` in `resolveExplicitDuplicateMarkerTasks` — i.e. making the sweep **ignore the disable flag entirely** — left all 16 cases **green**. The fixture never set `triageDuplicateResolution`, so the sweep had no resolution action to take and the duplicate survived whether the flag was honoured or ignored. The assertion held for a reason unrelated to the test's name. It had been red only because of the column literal, which would have made "fix the literal, go green" a repair that left a guard guarding nothing. Setting the resolution mode makes that same mutation delete the duplicate and the case fail. Verified both directions: | | mutation applied | |---|---| | before | 16 passed — **guard cannot fire** | | after | **1 failed** / 15 passed | Recorded in-file with the measurement, so nobody strips the setting back out as redundant. ## Two assertions strengthened rather than relaxed - The disable-flag and failed-delete cases now assert the column is **unchanged from a value read before the sweep**, rather than equal to a literal. They cannot pass because a seed happened to land where the assertion looked, and they survive the next column rename. The invariants those cases own are "the flag stops the sweep" and "the task whose delete threw survives" — the column id was always incidental. - The handoff move asserts its **provenance** (`nodeId: "review-pending-handoff"`, `preserveProgress: true`) instead of the `expect.anything()` its siblings in that file use, so a move to the same column by another path cannot satisfy it. ## Still open for whoever owns CI Shard 3/4's log loses its final chunk, which is why a plain test failure presented as a crash and stayed unexplained across at least four runs. Anyone triaging full-suite from shard conclusions alone will keep mis-reading this one; the failure has to be reproduced locally to be visible. |
||
|
|
339f6e7830 |
fix(census): stop the baseline serialising the fleet — every fleet PR conflicted with every other one (#2699)
## The problem Every fleet PR conflicts with every other fleet PR in `lifecycle-column-census-baseline.json` — **even when they convert entirely different files**. I have rebased **six** of my own branches for nothing but this file, and the resolution was *always* "take main's, re-run `--update-baseline`". Never once a real merge. That makes a generated artifact the serialisation point for the whole fleet phase. ## The cause `totals`, `byColumnId`, `properties` and `queryByColumnId` are **derived** — recomputable from the per-file maps — and **`--strict` never reads any of them**. It compares `byFile`, `deliberateByFile` and `queryByFile`, and nothing else. But every conversion changes at least one aggregate line. So those lines were a **shared write on a file whose real content is per-file and disjoint**. Removing them, two PRs converting different files touch no common lines. ## Trade-off, stated because it undoes a deliberate choice An earlier note kept the totals in the pin *"so the new number lands in the diff where a reviewer sees it"*. That was a good reason. The signal survives elsewhere: - the CLI prints the totals on every run; - `--update-baseline` prints each tightened entry by name; - the fleet rules already require a census before/after **in the PR body**. Reversible if the diff-visible number proves to matter more than the conflicts. ## Cost, stated too Merging this makes every in-flight fleet PR re-record once. That is one more instance of an operation they are already performing on every rebase — a one-time cost against a recurring one. ## Verification The end-to-end test that asserted the write via `totals.column` now asserts the same claim via the per-file entry: the stale pin says 1, the rewritten pin must carry the tree's real higher count for that file. **Mutation: suppressing the `--update-baseline` write still fails it**, so the assertion did not weaken. 71 census tests green · `--strict` and `--strict --exact` exit 0 · lint clean · gate green (487 + 158 + 10 + 71). ## Not done A merge driver. `.gitattributes` can name one, but registering it needs `git config` per clone and this repo has no `postinstall`/`prepare` hook to do that — so it would silently not apply for most people. Removing the shared lines fixes the conflicts without needing any local setup. |
||
|
|
78b6b5ba37 |
fleet: packages/engine/src/executor.ts 85 → 57 (in progress; 4 batches, plus the structural measurement this cluster needs) (#2689)
**Claiming `packages/engine/src/executor.ts`** — the largest unclaimed cluster (self-healing.ts and scheduler.ts are taken). ## Census | | before | after | |---|---:|---:| | `executor.ts` | 85 | **75** | | repo total | 722 | **712** | | `done` | 195 | 190 | | `archived` | 147 | 142 | Baseline re-recorded in the same commit; it shrinks by exactly the converted count (10 literals across 5 sites). ## Batch 1 — terminal-lane guards Five identical *"this card is already finished, refuse"* guards, all the literal pair `live.column === "done" || live.column === "archived"`. On a renamed board neither matches, so the refusal falls through — the same inert-guard shape as #2670. Converted to `resolveTerminalColumnsFor`, **the helper this file already established** at line 4509 — no new abstraction. It unions the resolved terminal columns with the legacy pair, so each converted guard is a strict **superset** of the literal: it can refuse in more cases, never fewer. That is what makes this batch safe without per-site behavior review. ## The structural measurement this cluster needs #2683 found self-healing.ts unsafe to batch because of **sync** workflow reads — a converted guard there would resolve through a sync path that cannot resolve a selection in production, silently falling back to defaults. I measured whether executor.ts has the same problem, per guard (not per line): | context | guards | |---|---:| | **async** — safe, can `await resolveWorkflowIrForTask` | **71** | | **sync** — needs threading or is not convertible in place | **14** | | module scope | 0 | The 14 sync-context guards are at lines 3455, 3479, 3530, 3540, 4611, 5501, 5502, 5504, 5777, 10213, 12306 (×3), 15782 — `in-progress` 4, `in-review` 4, `archived` 3, `done` 2, `todo` 1. **I am not converting those in place**, and I will flag rather than guess if threading resolved data changes behavior. So: unlike self-healing, this cluster is **83% safely convertible**, which is why it is worth working as a batch. ## Note on #2685 Engine code converts through core's resolvers (`resolveLifecycleColumns`, `resolveTerminalColumns`), not the dashboard `columnRoles` helpers. So the 680-guard helper gap #2685 fixes is **dashboard-side** — this cluster is not blocked on it. ## Verification engine `tsc` clean · lint clean · gate green (487 + 158 + 10 + 71). **Pre-existing failures, not caused by this change:** five tests in `src/__tests__/reliability-interactions` fail, all in `SelfHealingManager.recoverStarvedRefinementTriageTasks`. I confirmed by stashing this change and re-running on a clean tree — they fail there too. This change touches only `executor.ts` and does not go near that path. Flagging rather than fixing: it is someone's cluster and not mine to alter mid-flight. ## Not done Batches 2+ (the remaining 75). I will keep working this file in this PR with small commits, per the fleet rules. |
||
|
|
3d28e264c1 |
test(self-healing): three suites seeded a column id the product stopped declaring (7 red → 0) (#2695)
## What was red
`self-healing-advanced-triage`, `self-healing-agent-link-drift`, and
`self-healing-starved-refinement` — **7 failed / 19 passed** on clean
`origin/main`. All three fail the same way: the sweep returns `0`
recoveries where the test expects `1`.
## Cause
Those three sweeps were converted from `listTasks({ column: "triage" })`
to **role** filters (`filterByPreWipRole(..., ["intake"])`). Each store
fake has no workflow-selection readers, so the sweep resolves the
**default IR** — in which `triage` is not a declared column at all. The
seeded cards carried no intake role, the filters returned no candidates,
and the sweeps did nothing.
The product change was correct; the fixtures were asserting against a
column id the product had stopped declaring. `self-healing.ts` even
warns about this exact shape in its own comment: *"Converting a
predicate while leaving its source query on a literal produces a sweep
that LOOKS converted and does nothing."* The fixtures are the mirror
image — a seed on the old literal makes a correctly-converted sweep look
broken.
**Verified, not assumed.** I resolved the default IR and printed the
flags rather than reasoning from column names:
```
todo {"intake":true,"hold":true,"resetOnEntry":true}
in-progress {"countsTowardWip":true,"abortOnExit":true,"timing":true}
in-review {"mergeBlocker":true,"humanReview":true,...}
done {"complete":true}
archived {"archived":true,"hiddenFromBoard":true}
```
`todo` is the intake column post-U11 (the merged Planning column).
`triage` does not appear.
## Measured
| Check | Before | After |
|---|---|---|
| the three files | 7 failed / 19 passed | **26 passed** |
| all 40 `self-healing-*` files | 3 failed files / 7 failed tests | **40
passed / 695 tests** |
| `pnpm test:gate` | — | **726 passed** |
| `pnpm lint` | — | clean |
Restoring the deleted column id reintroduces the failures, so the seeds
are load-bearing rather than cosmetic.
**Pre-existing, untouched:** the same 9 vitest *"unhandled errors"* and
the identical non-zero exit appear on clean main. Verified by reverting
only these three test files and re-running — unrelated to the fixtures,
so flagged rather than folded in.
## One deliberately surgical spot
In `starved-refinement` only the starved refinement's own seed moved. My
first pass renamed the file's default seed and **broke a test that was
passing** — the auto-approve-all case calls `recoverApprovedTask`
directly (no role filter) and asserts a move **into** `todo`, so seeding
it in `todo` makes that move degenerate. I reverted and moved one seed
instead.
That leaves a real question I did **not** answer: post-U11 intake and
hold are one column, so a `triage → todo` move may no longer encode
anything. Deciding that means changing what the test is *about*, which
is a behaviour judgement on someone else's assertion. **Flagged in-file,
not guessed.**
Also worth noting for whoever converts the remaining backlog: line 43 of
that file pairs a `triage` seed against an explicit `todo` seed as a
contrast. U11 collapsed those two columns, so the contrast the fixture
was drawing no longer exists in the product — not a defect, but any
fixture built on intake-vs-hold being distinct is now suspect.
Census unchanged (722) — test files are not scanned by the census.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Tests**
* Updated self-healing workflow test scenarios to reflect the current
intake column.
* Improved test coverage for triage, agent-link drift, and starved
refinement handling.
* Added documentation clarifying workflow column and role-based
filtering assumptions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
dfbab18fd5 |
fix(scheduler): a renamed wip column held NO file-scope lease — two agents could edit the same files (#2693)
## The defect `activeScopes` is the file-scope lease registry the dispatch path reads (`scheduler.ts:2167`) to decide whether a candidate overlaps work already in flight. Two column-id literals kept it empty on any board whose columns are renamed: 1. the lease loop gated on `task.column !== "in-progress"`; 2. `shouldHoldActiveFileScopeLease` keyed **both** its branches on `in-progress` / `in-review`, so it returned `false` for *every* card on a renamed board. Forty lines above that loop, the same sweep resolves `countsTowardWip` from the workflow IR for capacity arithmetic. **The scheduler was simultaneously right about capacity and wrong about leases.** Consequence: a second task sharing a file scope **dispatched instead of queueing** — two agents editing the same files, which is precisely what `groupOverlappingFiles` exists to prevent. ## Measured, differential Same workflow *shape* under two vocabularies with identical traits; only the column ids differ, so any difference is attributable to a surviving literal. No renamed id collides with a legacy one, so a surviving `=== "in-progress"` cannot pass by luck. | | default vocabulary (control) | renamed vocabulary | |---|---|---| | fix reverted | queued on lease ✓ | **dispatched into the wip column** ✗ | | fix applied | queued on lease ✓ | queued on lease ✓ | `2 of 3 fail` reverted → `3 of 3 pass` applied. The control passes on **both** sides, so a change that breaks overlap protection generally cannot hide behind this test. I checked the test wasn't vacuous before trusting it: instrumented the run to print the actual `moveTask` calls, and confirmed the renamed case really produced `[["FN-CAND","building"]]` — a genuine dispatch — rather than the candidate simply never being considered. Both failure modes look identical in the assertion. ## Why optional booleans, not a flags object `shouldHoldActiveFileScopeLease` is **exported** and shared with the self-healing / repair paths (`self-healing.ts:4488`, `:5406`) — its own comment says those "must use this same predicate so stale `overlapBlockedBy` cleanup does not preserve blockers the scheduler would ignore". So the role questions became optional parameters that **default to today's literals**: a caller that resolved the traits passes the answer, a caller that has not gets exactly current behaviour. No existing call site changes meaning, and no dependency on #2690. ## Verification | Check | Result | |---|---| | scheduler / capacity / hold-release / overlap / self-healing | **59 test files green** | | `pnpm test:gate` | **726 passed** | | `pnpm lint`, engine `tsc --noEmit` | clean | `self-healing-advanced-triage`, `-agent-link-drift`, `-starved-refinement` are **7 failed / 19 passed both before and after** — verified pre-existing on clean `origin/main` by reverting only `scheduler.ts` and re-running. Flagged, not fixed, and not in scope here. ## Census **722 → 721**, `scheduler.ts` 28 → 27. Baseline re-recorded in the same commit. To be precise about what that −1 is: the *loop* literal is gone, while the two literals **inside** the predicate remain by design as the documented defaults. So this is not "scheduler is now trait-aware" — it is one site, plus the seam that lets callers be. ## Merge-order note **#2690 also records `scheduler.ts` 28 → 27**, converting a *different* site (`isWipColumnTask`'s hand-rolled flags-first copy, `:1690`). The two are independent and do not double-count: if both land, `scheduler.ts` is **26**, and whichever merges second will conflict on `scripts/lib/lifecycle-column-census-baseline.json` and must re-record to 26 rather than resolve to 27. Flagging so the merger does not take one side blindly. ## Still broken, flagged for an owner The **in-review** half. `activeScopes` is also populated for review-lane cards via `t.column === "in-review"` (`scheduler.ts:1751`, `:1757`), and this PR leaves those literals in place: the sweep's flags map holds only `countsTowardWip`, so no review-role answer is available to pass in. Fixing it needs the flags-object change in #2690, after which the same optional parameter added here carries it. Until then a renamed review column still holds no lease. |
||
|
|
e9e63d8e0f |
consolidate/capacity: --strict was red on main (my #2621), 14 stale baselines, routines seeding a deleted column, worktrees-off audit (#2652)
Capacity unit consolidation. Three coherent themes, small commits inside. ## Census before/after (`node scripts/lifecycle-column-census.mjs`) | | before | after | |---|---:|---:| | triage column guards (the bar) | 10 | **10** | | `--strict` on main | ❌ **RED** | ✅ green | | baseline staleness | 14 files stale | **0** | This branch does **not** move the triage bar — its remaining 10 are moves.ts (dies with the flag), the dashboard cluster, and one deliberate site. It fixes the instrument that measures the bar, plus a live defect the comparison count cannot see. --- ## 1. `--strict` was RED on clean `origin/main`, and it was my fault ``` packages/dashboard/src/routes/register-task-workflow-routes.ts: 22 -> 23 ``` My merged #2621 added a v1-IR pre-WIP fallback answering a greptile P1 and shipped no marker or baseline update, so the program's measuring instrument has been failing on main since it landed. Fixed **at the site** with a `DELIBERATE-LITERAL` marker, not by bumping the baseline. That branch runs only when the IR declares no columns and no nodes, so there is no role to resolve — `resolveLifecycleColumns` returns nothing and the legacy pre-implementation ids are the only pre-WIP signal that exists there. It is *unconvertible*, not unfinished; the sibling `else` two lines down is the trait path for every IR that can answer. A rise that is genuinely correct belongs where a reader will see it. ## 2. The baseline was stale for 14 files — a hole, not cosmetics A stale allowance lets converted guards return while the check stays green. Measured gaps: ``` self-healing.ts allows 126, tree has 111 executor.ts allows 112, tree has 104 moves.ts allows 44, tree has 39 default-workflow-hooks allows 25, tree has 7 mission-feature-sync allows 5, tree has 0 MissionControlPanel allows 4, tree has 0 (+8 more) ``` **Only two of the fourteen are mine.** The other twelve are already-merged conversions by other workers where nobody re-recorded. Re-recorded all fourteen here rather than waiting for twelve PRs, because until it happens the ratchet is not holding the 779 it exists to hold. Flagging it plainly: those drops are other people's work being locked in, not mine being claimed. ## 3. Routines created tasks into the column U11 deleted The routine editor's "Target Column" defaulted to `triage`. That value is submitted as the create step's `taskColumn`, and an **explicit** column bypasses the workflow entry-column resolution added for column-less creates (#2589) — so every routine saved with the untouched default seeded its tasks into a column the board does not declare. Defaulting to `todo` would be the same mistake one column over: a custom workflow declaring no `todo` is seeded into an undeclared column just as surely, because an explicit column overrides entry resolution whatever its value. So the default sends **nothing** and each workflow's own intake resolution decides. The `triage` **option** is removed too, not merely un-defaulted — fixing the initializer alone left the operator able to pick the deleted column one click away, and it was the option labelled "Planning", the name the merged `todo` column now displays. Removing it retires that label inversion as well. Found by scanning **membership** forms rather than comparisons: the comparison census cannot see a `?? "triage"` default, so no count showed this and nobody was looking. Revert-proof — restoring the default fails with *"the default must not name a column at all"*. ## 4. "Worktrees off is INERT" had one unaudited reader The constraint was that `maxWorktrees` become genuinely inert, "not set very high and not skipped by convention". `resolveWorktreeCapacityLimit` returns `null` for that, and its unit tests can only prove the **resolver** is right — they cannot see a second reader, which is the only way the constraint breaks. Audited every `maxWorktrees` read that bounds anything. **Exactly two:** `scheduler.ts` (the admission gate, via the resolver, single call site, optional gate snapshot) and `self-healing.ts`'s `enforceWorktreeCap` — `(settings.maxWorktrees ?? 4) * 2`, a **raw** read. The second is **not a bug** and is left alone: it bounds worktree *directories on disk* and only removes *idle* ones. Worktrees still exist in OFF mode, so that bound must keep applying or idle directories accumulate unbounded. Recorded consequence: in OFF mode the number still governs disk retention while gating no admission — an edge you scoped out. The note says explicitly **not** to unify the two readers: routing hygiene through the resolver returns `null` in OFF mode and silently removes the disk bound, which is a leak dressed as a simplification. New ratchet requires every file bounding on `maxWorktrees` to be named with a reason, and rejects a **stale** allowlist entry. Proven by injecting `active >= (settings.maxWorktrees ?? 4)` into `hybrid-executor.ts`. --- ## Deliberately NOT included - **My own census script.** #2633 landed the canonical one, and it is better than mine — an AST classifier *plus* an independent text classifier with `--compare`, and a baseline that fails on unrecorded **drops** as well as rises. Mine only caught rises. I deleted mine rather than ship a second measuring instrument; three copies of "strip comments" is the drift shape this program keeps paying for, so the worktree ratchet now imports #2633's `stripComments`. - **My TaskContextMenu fix.** Superseded, and by a better answer: main's `isPureIntakeColumn` (intake *without* hold) keeps the merged Planning column shown and suppresses only a bare Ideas capture, which resolves the exact hold-lane objection coderabbit raised against my version. I briefly clobbered that merged work by checking my old file out wholesale, caught it in the diff, and reverted. ## Verification `pnpm lint` clean · core + dashboard `tsc` clean · census suite 23/23 · worktree ratchet 8/8 · RoutineEditor 49/49 · `routes-task-retry-planning-column` 16/16 · `lifecycle-column-census --strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## Added after review (all four greptile threads were real, and two of them mattered) **The routine fix was half a fix.** `routine-runner.ts:515` *and* `cron-runner.ts:982` both did `column: (step.taskColumn as Column) || "triage"` **after** the step is read, so every routine — including ones saved through the fixed editor — still created tasks into the deleted column. Both now omit it. **The advanced steps editor MANUFACTURED the defect.** `ScheduleStepsEditor.tsx` had three `triage` defaults: the new-step template (`:64`), the per-step initializer (`:95`), and the select still offering it (`:344`). So the path I had *not* fixed produced the bug by default, on fresh data. Template names no column; initializer coerces a persisted `triage`; `triage` removed from the options; empty submits `undefined`. **Four pre-existing tests pinned the defect** and are rewritten to the corrected invariant rather than appeased: | test | asserted | |---|---| | `cron-runner`: "defaults column to triage when taskColumn is not set" | `column: "triage"` | | `ScheduleStepsEditor`: "adds a create-task step..." | `taskColumn` toBe `"triage"` | | `ScheduleStepsEditor`: "allows saving create-task step..." | the legacy column is **resubmitted** | | plus the explicit-column case added beside each, so the fix cannot swallow a deliberate choice | **The allowlist hole was the worst finding.** `AUDITED_BOUNDS` was keyed by FILE, so every bounding expression in an allowlisted file was exempt — a second raw bound in `scheduler.ts` stayed green, the one case that ratchet exists for. Per-expression now, and making it so **immediately surfaced a real second bound the file-level version was hiding** (`maxWorktreesGate.used >= maxWorktreesGate.limit`, safe by construction since the snapshot is `undefined` in OFF mode). Proven by injection. ## Found while re-reading my own deletion, not reported A **rendered tooltip** still named a deleted cap. The "Queued to plan" badge read *"planning starts when a concurrency slot frees up (maxConcurrent / globalMaxConcurrent)"*. The cross-project cap is gone — capacity is two numbers per project — so it told operators their planning waited on a limiter they can no longer find a setting for. Names the surviving dimension only now. ## Coding (Ideas): enforcing #2651 rather than repeating it I took the unowned coding-ideas IR merge, concluded it must not be done, then found **#2651 had already implemented, reverted and documented exactly that** — with better grounding than my own argument. It added no test, so nothing stops the next person reaching the same dead end. So this ships their reasoning as a ratchet, not a second opinion: triage discovery keys on the column's `autoTriage`, so a merged column is either never scanned (cards sit on a bootstrap stub until the **capacity hold** releases them, sending **unplanned** work into in-progress — worse than stalling) or scanning wins and the manual gate is gone. Their scope caveat is kept: `autoTriage` is a general trait field, so only *this preset's* collapse is dead, not manual intake as a concept. The registry does not reject the merged shape, which is why prose was not enough. ## Verification (re-run) `pnpm lint` clean · core + engine + dashboard-app `tsc` clean · `lifecycle-column-census --strict` exits 0 ("every file matches its baseline exactly") · routine-runner 24/24 · cron-runner 156/156 · ScheduleStepsEditor 41/41 · RoutineEditor 49/49 · worktree + coding-ideas 12/12. TaskCard has 2 failures **pre-existing on main** — confirmed identical with my changes stashed. --- ## Bears directly on the closing bar: this PR already removes the 67-guard ratchet slack Measured on current `origin/main` with the census itself: ``` tree total: 787 baseline total: 854 SLACK: 67 FILES ABOVE BASELINE (1): +1 packages/dashboard/src/routes/register-task-workflow-routes.ts (22 -> 23) FILES BELOW BASELINE: 13, totalling 68 unrecorded conversions -18 core/default-workflow-hooks.ts (25->7) -15 engine/self-healing.ts (126->111) -8 engine/executor.ts (112->104) -5 core/task-store/moves.ts (44->39) -5 engine/mission-feature-sync.ts (5->0) -4 core/live-agent-count.ts (10->6) ``` **The slack is not regression — it is 13 files of merged conversions nobody re-recorded**, against exactly **one** rise. This PR re-records the baseline **854 → 782 across 140 files**, which closes it. **And the "+3 that slipped in" is +1, and it is mine.** `register-task-workflow-routes.ts 22 → 23` is the v1-IR pre-WIP fallback my #2621 added; it is justified (that branch runs only when the IR declares no columns or nodes, so there is no role to resolve) but it shipped with no marker and no baseline update — which is why `--strict` has been **red on main since it merged**. Fixed here at the site with a `DELIBERATE-LITERAL` marker rather than by bumping the baseline, because a rise that is genuinely correct belongs where a reader will see it. Sequencing note for the auto-lowering change: if this lands first, that work is purely the mechanism (auto-lower, or fail with tighten instructions) rather than a cleanup, and the two re-records will not collide in the same file. Also worth carrying into that mechanism, from building the same guard here: **`--update` must refuse to RAISE.** An earlier version of mine wrote current counts verbatim, so a developer who added a literal and ran the documented update command locked the regression in as the new ceiling — the mirror of the high-water problem. Lowering can be unattended; raising should be a hand edit with the reason recorded. ## Third piece of residue from my own deletion `updateGlobalConcurrency` in the dashboard API client PUT to `/api/global-concurrency`, a route removed when the machine-wide cap went. Zero callers; the only reference was the `legacy.ts` barrel re-export. Deleted both. `fetchGlobalConcurrency` **survives on purpose** — the GET route remains and serves live utilization telemetry to the footer and Command Center; nothing gates on it. That is the third: after the second raw `maxWorktrees` reader and the "Queued to plan" tooltip. A deletion is not finished when the enforcement goes — the client, the label and the tooltip outlive it. --- ## Re-greened the dashboard API tests: 117 failures on main, ONE root cause These would have polluted the closing verification pass, and nobody owned them. `api()` builds headers via `new Headers(...)` and returns `Object.fromEntries(headers.entries())` — and `Headers.entries()` **lowercases every key**, so the object reaching `fetch` is `content-type`, not `Content-Type`. `ab87d0d80` then added `x-fusion-client: dashboard-ui` for run-audit attribution. Both changes are correct; neither is visible at a call site, so **114 assertions across 7 files** kept asserting the old shape and went red together. Fixed by naming the shape **once** in `app/test/apiRequestHeaders.ts` rather than patching 114 literals — restating a shared fact 114 times is what made a two-line client change look like 117 failures. Deliberately not a loose `objectContaining`: these tests are the only thing pinning that the attribution header is sent *at all*. **117 → 4.** The remaining 4 are unrelated pre-existing CSS failures (`task-detail-modal-tablet-width` ×3, `space-token-defined` ×1) — confirmed identical on clean main with my changes stashed. ### A gap this surfaced, recorded not papered over Three routes failed in the *opposite* direction — they send the old shape because they call `fetch()` **directly**, bypassing `api()`, so they never get the attribution header. `client.ts` claims the opposite: > "Applied once here rather than per-call so no future mutation route has to remember it." That does not hold for a route that bypasses the helper it is applied in. **Measured in `app/api/`: 8 files make direct `fetch()` calls and 7 include mutations (POST/DELETE)** — among them `ai-sessions.ts`'s DELETE, which is the same class as the four-delete incident the header was added for. So the attribution fix has a hole in exactly its motivating case. Not fixed here: routing those onto `api()` is a behaviour change across the API layer and belongs to its owner, not to a test re-green. Those assertions use a separate `API_JSON_HEADERS_NO_ATTRIBUTION` constant so the gap stays **visible** — if a route is later moved onto `api()`, its test fails and points at the note explaining why. --- ## This branch takes the triage bar 10 → 5, and makes `--strict` green `node scripts/lifecycle-column-census.mjs` on this branch reports **triage 5**, against **10** on `origin/main`. The five removed are the ScheduleStepsEditor template/initializer/option and the RoutineEditor default/option — the automation paths that were creating tasks into the deleted column. **`--strict` was also RED on clean main, twice over, and both causes were the same mistake:** a thorough written rationale the tool cannot read, because the marker was not where the census looks. The census reads a comparison node's **leading comments**; a `DELIBERATE-LITERAL` in the JSDoc above the enclosing function or declaration does not reach the comparison inside it. | site | why it is legitimate | why the tool could not see it | |---|---|---| | `columnRoles.ts:80` `isHoldColumnRole` | degrades to `columnId === "todo"` only when a column has **no resolved traits** — identical in kind to `LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS` directly above, which escapes counting only because a Set is a membership form | rationale written, **no marker token** | | `MissionControlPanel.tsx` ×3 | the SDLC funnel **alias table** — maps `to-do`/`ready`/`review`/`shipped` onto one display stage with an explicit `other` bucket, and nothing branches on it | marker in the JSDoc; the comparisons are arrow bodies **inside the array literal**, which it does not reach | The second only surfaced because converting the `triage` stage to a Set removed its count and exposed the siblings — red gate, justification sitting three lines above, unreachable. Both are markers, no behaviour change. Neither is a conversion candidate: resolving the funnel table to traits would **drop the non-column aliases it exists to accept**. **For the auto-lowering work:** the marker-placement rule is now the recurring trap — three instances, three different authors, including me. A marker that does not register is indistinguishable from no marker, and the failure mode is a red gate with a written explanation nobody can act on. If the census accepted a marker anywhere in the enclosing declaration's comments, none of the three would have happened. Baseline re-recorded per the tool's own instruction ("Re-record the baseline in the SAME PR that lowered the count"). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Task “Actions” menus no longer appear on bare cards in the Planning column. - Routines, scheduled tasks, and create-task steps now respect each board’s configured workflow intake column instead of using a retired default. - Legacy tasks saved with the retired intake column are migrated to automatic workflow resolution. - Target-column selection now offers only “Automatic (workflow intake)” and “Planning,” removing the obsolete option. - Capacity/planning messaging and related UI tooltip text were clarified; concurrency cap updates are managed per project. - **Tests** - Added/updated coverage for workflow intake resolution, create-task target column behavior (including legacy coercion), capacity safeguards, and API request consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
aa02db5782 |
fleet: scheduler.ts 28 → 27 + make the column-role predicates reachable from the other 80% of the backlog (#2690)
## Census **722 → 721**; `packages/engine/src/scheduler.ts` **28 → 27**. Exactly the one site converted. Baseline re-recorded in the same commit — `--strict` flagged the stale allowance itself, and `in-progress` went 138 → 137. ## The unblocker (commit 1) The role helpers live in `packages/dashboard/app/utils/columnRoles.ts`, a dashboard-**app** module. Measured against the census: | Location | Guards | Share | Helpers importable? | |---|---:|---:|---| | `packages/engine/**` | 316 | 43% | no | | `packages/dashboard/app/**` | 150 | 20% | **yes** | | `packages/core/**` | 148 | 20% | no | | `packages/dashboard/src/**` | 78 | 10% | no | | `packages/cli/**` | 24 | 3% | no | **Only 20% of the backlog can call them at all.** #2685 widens the helper *set* correctly; that is coverage, not location. `packages/core/src/column-roles.ts` is the same flags-first / legacy-id-fallback predicate placed where the other 80% can reach it — core already exports `resolveColumnFlags`, so no new resolution machinery comes with it. **Semantics are mirrored from #2685, not invented**, so the two sets cannot answer the same question differently: `complete` EXCLUDES `archived`; `wip` keys on `countsTowardWip` (the same flag capacity arithmetic uses); `review` accepts `mergeBlocker` OR `humanReview`. One addition — `isTerminalColumnRole` for the `!== "done" && !== "archived"` union, the most repeated shape in the backlog. 10 tests cover both modes of all 8 predicates, including the **degraded no-flags fallback** — the half with no coverage when these lived only in the dashboard app — plus the two cases that prove the predicate does something rather than nothing: a renamed column carrying the right trait answers yes, and a legacy id carrying the WRONG trait answers no. ## A trap every fleet worker converting engine code will hit A new core export must be added to **both** `index.ts` and `index.gate.ts`. The `engine-core` gate project resolves `@fusion/core` to a bundle built from `index.gate.ts` (`scripts/build-engine-core-gate-bundle.mjs`). An export present only in `index.ts` is `undefined` at runtime under the gate: 13 `scheduler-workflow-cutover` tests failed with `isWipColumnRole is not a function`, in a file that does not mock `@fusion/core` at all. The symptom points at the consumer, the cause is the barrel. I nearly mis-attributed this. Baseline first: `scheduler-workflow-cutover` is **42 passed on clean main**, so the 13 were mine — not pre-existing. That measurement is the only reason I looked at the barrel instead of "fixing" the tests. ## The conversion (commit 2) `scheduler.ts:1690`'s `isWipColumnTask` was a hand-rolled copy of `isWipColumnRole` — it stored only `countsTowardWip` as a boolean and re-implemented flags-first-then-legacy-id inline. It now stores the resolved flags object and lets the shared predicate decide. Behaviour is identical in all four states: column present with the flag true or false (flags win), column absent from a resolved IR, and IR resolution failed (both defer to the legacy id). | Check | Result | |---|---| | `scheduler-workflow-cutover` | **42 passed** before and after | | 21 scheduler/capacity/hold-release files | **372 passed** | | `pnpm test:gate` | **726 passed** | | `pnpm lint` | clean | ## Flagged and skipped, not guessed **`scheduler.ts:1736` — a latent legacy-vocabulary defect, not a conversion.** `if (task.column !== "in-progress") continue;` gates the file-scope-lease loop on the literal, ~40 lines below capacity arithmetic that is trait-aware. On a renamed WIP column the loop silently does nothing while capacity counts the same cards correctly. Converting it *changes behaviour* on renamed boards (from wrong to right), which the fleet rules put out of scope — so it is flagged here for whoever owns that fix. It is the same class as U10's six legacy-vocabulary defects. **`hold-release.ts:343`** — already marked `DELIBERATE-LITERAL`. It is the legacy half of FN-5719's dual-accept pair; converting it would make both halves compute the same answer, deleting the compatibility signal *and* its divergence detector while looking like a cleanup. Untouched. **`task-merge.ts:254`** — the documented fallback for callers that have not proven lane identity; trait-aware callers pass `skipColumnIdentityCheck`. Untouched. **The other 14 `scheduler.ts` sites** have no flags in scope (e.g. `isLegacyDependencySatisfied(dep: Task | undefined)`, `shouldHoldActiveFileScopeLease(...)` — task-only pure functions). Threading an IR in changes signatures and call graphs: behaviour change, out of scope. This is why the cluster is 28 → 27 and not 28 → 0, and the reachability measurement behind it is #2687. No changeset: `@fusion/*` are private and no `@runfusion/fusion` behaviour changes. |
||
|
|
bb3bdab999 |
The ratchet follows the count down — a drop tightens instead of reddening the gate (coordinator item 2) (#2679)
Taken after asking twice for reassignment with no reply, and after the same failure bit a **third** time. No open PR touches the census CLI, so this is unowned in practice — **U12, say so if you have started and I will close this in favour of yours.** ## What changed A **drop** now tightens the baseline instead of failing. Failing hard was defensible in isolation — a stale allowance is a hole, since those guards can return up to the old count while the check stays green. What it missed: **The drop is almost never the failing author's to fix.** Eleven files dropped during one merge wave, none of those PRs re-recorded, and none of their authors did anything wrong. Measured three times since CI began gating this: `columnRoles.ts` 0 → 1, then `executor.ts` twice. A permanently-red gate is a bigger hole than a stale allowance, because it gets ignored and then nothing is guarded at all. **The rise check — the ratchet's actual purpose — is untouched and still fails hard.** ## The residual, named rather than glossed In CI the write is discarded with the runner, so the committed baseline stays stale until someone commits a tightened one. The exposure is bounded (regrowth only up to the old count), printed on every run, and strictly smaller than the exposure from a check people route around. `--strict --exact` restores hard failure for the pinned end state. **One writer:** the write is now a named `writeBaseline()` shared by the tighten path and `--update-baseline`, rather than a second `writeFileSync`. Two writers for one artifact is how they drift — a lesson this file already learned once. ## Exercised end to end | scenario | result | |---|---| | drop, `--strict` | exit **0**, `TIGHTENED`, allowance rewritten 9 → 6 | | drop, `--strict --exact` | exit **1**, baseline untouched | | rise, `--strict` | exit **1** | | clean | exit **0** | Pinned through the real CLI with an isolated baseline. Revert proof: restoring the hard failure fails **1 of 32**. ## Two of my own mistakes, recorded **A vacuous assertion, in the case that guards against vacuity.** I first wrote `expect(allowedAfter).toBeLessThan(4 + allowedAfter)` — true for every number. Replaced with a comparison against the inflated value the fixture started from. This file documents that trap repeatedly and I still walked into it, which is the argument for the mechanical revert check over careful reading. **The env override is `FUSION_CENSUS_BASELINE_PATH`**, not the `FUSION_CENSUS_BASELINE` I used in the first draft — so the first version of these cases silently ran against the **real** baseline and passed for the wrong reason. A test whose fixture never took effect is the same failure as a test whose fixture can't fail. ## Verification 32/32 census suites, `pnpm test:gate` **71/71**, `--strict` exits 0, `pnpm lint` clean, `docs/testing.md` updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## Update — the base-ref ratchet (review round 2, commit `4895845579`) The first version of this PR shipped a **named residual**: the tightening write dies with the CI runner, so the committed allowance stays high and a later PR can regrow guards up to it while `--strict` prints green. I called the exposure bounded and moved on. Greptile flagged it P1 and was right — naming a hole is not closing one. `--strict` now stops trusting the committed number for files the branch touched. It measures each **changed** file at the base commit (`FUSION_CENSUS_BASE_REF`, else the PR base branch, else `origin/main`) and fails if the file carries more guards than the base ref has. **The enforced ceiling is what main has today**, so a stale, missing, or long-unrecorded baseline no longer opens a window. | decision | why | |---|---| | changed files only, `<ref>...HEAD` | untouched files have main's counts by construction; censusing all ~400 at the base ref is ~400 `git show` calls to re-derive numbers that cannot have moved. Three-dot also stops charging this branch for guards that landed on main after the fork. | | a new file's base allowance is **0** | "absent at the base ref" as unbounded would make a new file the cheapest place to hide a fresh guard | | fails **open** on an unresolvable ref, printing `SKIPPED` | a shallow clone cannot produce an honest comparison; a degraded run must not read as a clean one. The baseline comparison still applies. | | merged into the existing `regressions` list | one failure per file, and `--update-baseline` keeps working as the deliberate escape hatch. No new exit path. | **Revert proof, measured both ways.** With the base-ref block removed, the regrowth fixture — base commit 2 guards, HEAD 5, baseline allowing 9 — exits **0** with `TIGHTENED`, which is precisely the reported scenario. With it: exit **1**, `column-guard count ROSE`, `above its count on the base ref`, baseline left at 9. **3 of the 4** end-to-end cases go red on revert. The fourth passes without the fix by design — it is the genuine-conversion case the auto-tighten exists to keep green, and a case that reddens either way proves nothing. The end-to-end suite builds a throwaway two-commit `git init` repo under the temp dir, because this exploit is a property of the **plumbing**, not of the comparison: resolving a ref, working out the changed set, reading base source through `git show`. The comparator itself is pure with the reader injected (`findRegrowthAgainstBase`), with its own cases in `lifecycle-column-census-ast.test.ts` — including the one that would silently pass everything, looking up the wrong key in `summarize().byFile`. **Rebased onto `origin/main` @ bc782d8d92** (the branch was forked before the recent merge wave; its baseline read 746 against a tree of 722). Verification on the rebased branch: census **722** / `--strict` exit 0 · **70/70** across both census suites · `pnpm test:gate` **71/71** · `pnpm lint` clean. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
da77e61118 |
A rate-limited provider kept getting hammered — the executor and merger lane checks were still literals (#2672)
`taskUsesProvider` resolves a task's **active lane** to decide which providers it is running on. The **planner** half was converted to traits — its own note in that function describes this exact failure and says it was fixed — and the **executor** and **merger** halves were left as `task.column === "in-progress"` / `=== "in-review"`. So on a renamed board an actively-executing card resolved **no providers**, and a provider rate limit never paused it: the engine kept sending work to the limited provider with that card. ## Measured Renamed board (`building` = wip, `checking` = review), limit triggered by a peer: | lane | before | after | |---|---|---| | executor | `["FN-TRIGGER"]` | `["FN-TRIGGER","FN-PEER"]` | | merger | `["FN-TRIGGER"]` | `["FN-TRIGGER","FN-PEER"]` | The trigger was paused either way — but only through the *always-include-the-trigger* fallback, and **that is what made the hole quiet**: one task always gets paused, so the behaviour looks like it works. Resolved from the **same per-workflow IR cache** the planner lane already uses, so this adds no reads. Both halves fail soft to the legacy literal, so an unresolvable workflow behaves exactly as before. ## How it was found — the part worth keeping A scan for *"legacy literal within a few lines of a role-resolved call"* flagged this file. That is the same heuristic that produced #2670, and it is now 2 for 3. The first thing I suspected here was the `done`/`archived` terminal filter. I wrote that fix, and **its revert stayed green.** That is not a dead end — chasing *why* it would not go red showed the lane check already excludes finished cards, so the terminal literal there is genuinely redundant, and the real defect was one line over in the lane check itself. **A revert that stays green is information: either the guard is vacuous, or you are looking at the wrong line.** I dropped the unprovable change and kept the provable one. The `done`/`archived` filter is deliberately unchanged, with that reason recorded in the test. ## Revert proofs, isolated - wip lane back to the literal → **1 of 55 fails** (the executing peer) - review lane back to the literal → **1 of 55 fails** (the merger peer) Three paired cases pass under both, so "pause everything with that provider" cannot pass for "resolve the lane": a card parked in the renamed planning lane is **not** paused on an executor limit, a wip card is **not** paused on a merger limit, and the legacy vocabulary still pauses peers when no workflow resolves. ## Verification - 55/55 `usage-limit-detector`; `pnpm test:gate` **71/71**; engine typecheck clean; `pnpm lint` clean - census unchanged at 748 — the fail-soft literals remain by design, which is why the count is not the measure of this fix 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
dc50425e98 |
docs: correct 104 future-dated FNXC timestamps across 61 files (#2680)
## What The FNXC convention exists so a reader can place a note against the change that motivated it. A stamp dated *after* the edit landed defeats exactly that. This is program-wide drift, not one author's slip — I contributed to it in my own commits this week, which is how I noticed it. ## Measured, on this tree **104 stamps across 61 files** dated later than the day they were written, from one day ahead to **2026-10-19 (81 days)**: | count | date | count | date | count | date | |---|---|---|---|---|---| | 50 | 2026-07-31 | 6 | 2026-08-05 | 3 | 2026-08-13 | | 17 | 2026-08-01 | 1 | 2026-08-07 | 1 | 2026-08-19 | | 7 | 2026-08-02 | 1 | 2026-08-12 | 2 | 2026-08-26 | | 11 | 2026-08-03 | | | 3 | 2026-10-19 | An earlier number I circulated was ~70. That came from a narrower pathspec and was wrong; **104** is the measurement. ## How Each stamp is rewritten to the date of the commit that introduced **that line**, via per-line `git blame` — deliberately *not* stamped uniformly with today's date. A uniform stamp swaps a wrong date for a different wrong date and flattens the ordering that makes these comments navigable; blame preserves it. Times of day are untouched, and a blame date in the future is clamped rather than trusted. ## Why the verification is listed A docs sweep across 61 files is precisely where a stray edit hides, so the safety claims are mechanical rather than asserted: - every changed line begins with a comment marker — **no code touched**; - **no test asserts an FNXC date later than today**, so no `toContain` assertion on embedded source text can be silently invalidated (several such assertions do exist); - CSS files, which carry several of those assertions, are outside the pathspec. ## Verified lint clean · merge gate green (487 + 158 + 10 + 71) · `census --strict` exit 0 · tsc clean for core, engine, and dashboard (`tsconfig.app.json`). **No behavior change.** Comment text only. ## Not done here A guard preventing recurrence. A check that rejects an FNXC stamp dated after the commit would stop this returning, but it needs a decision about where it runs (lint rule vs. gate) and it is a behavior change to CI — it does not belong riding inside the sweep it would police. |
||
|
|
543f4a556c |
Tell an already-converted fallback literal from an unconverted guard — 19 of 19 dashboard scan hits were the former (#2677)
The batch phase is about to hand per-file guard lists to cheap workers, and the census currently cannot distinguish **"not yet converted"** from **"converted, with a documented degradation."** ## The measurement that makes this a class, not a preference A proximity scan for *"legacy literal near a role-resolved call"* — the heuristic that produced #2670 and #2672 from the engine — returned **19 hits across the dashboard and zero defects.** Every one was: ```ts if (flags) return flags.hold === true || flags.countsTowardWip === true; return column === "todo" || column === "in-progress"; // reachable only without traits ``` That literal is **correct**: it answers for callers with no resolved column metadata, which is the case `resolveLifecycleColumns` returns `undefined`-for-the-whole-struct to preserve. A worker told to "convert" it would delete the only answer available when traits are absent. ## And the difference is structural, so the parser can see it In **both** engine defects the literal sat in a **separate statement beside resolved data**, not in a fallback branch. Proximity cannot tell those apart; an AST can. `traitFallback` flags the ternary form and the **early-return** form (which is how most are actually written), and deliberately does **not** flag a fallback whose test is itself a column-*name* check — otherwise any `if/else` over column names would launder itself. ## Reported beside the backlog, not subtracted from it ``` COLUMN guards (the backlog): 746 of the column guards, 9 are trait-fallback branches (already converted) ``` A fallback literal is still a literal and should go when the trait path becomes unconditional. This only says which **kind** of work it is. **Advisory, and structurally so:** `traitFallback` never changes `kind`, and the count lives *outside* `totals`. My first attempt put it in `totals` and broke two existing suites that correctly deep-equal that shape — an advisory number does not belong in the structure that defines the bar. ## Revert proof Forcing `traitFallback: false` fails **3 of 33** (both fallback forms, plus the kind-unchanged case). The two *negative* cases pass under the revert — which is the point: they assert what must **not** be flagged, and a classifier that flags nothing satisfies them trivially. Worth stating, because a revert proof that only counts failures would look stronger than it is. ## Baseline Re-recorded: `executor.ts` 87 → 85 was **main's own drift** from #2568 landing, so `--strict` was red on main again. #2668 made the re-record possible; the auto-tighten (coordinator item 2) is still open, and this is the third time in this program that a legitimate merge has left the gate red for everyone else. ## Verification 62/62 across both census suites, `pnpm test:gate` **71/71**, `--strict` exits 0, `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 * **New Features** * Added reporting for legacy column comparisons found in trait-fallback branches. * Census results now include a separate count for these fallback-related column guards. * Human-readable reports display the new metric alongside the existing backlog totals. * **Tests** * Added coverage for fallback detection across ternary, early-return, and conditional patterns. * Added safeguards to prevent false positives in resolved-data and column-name checks. * **Maintenance** * Updated baseline census metrics to reflect revised classifications. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
87a4dbdc60 |
test(u9): a release-leg E2E fixture that diagnoses itself (same defect found 3x independently) (#2678)
## What The planned-spec release-leg fixture defect has now been diagnosed **three times independently** — #2634 (`workflow-lifecycle`), #2643 (`workflow-merged-board`), and again in `workflow-planning-lane`. Each time it cost real time, because it presents as a *scheduler* bug rather than a fixture bug. The mechanism: `createTaskWithReservedId` leaves a bootstrap seed (`# <id>\n\n<description>`), `isUnplannedForExecution` (`hold-release.ts`) refuses to move an unplanned card out of any intake- or hold-trait column, and the sweep reports `held: [{ reason: "move-rejected-or-no-slot" }]` while releasing nothing. That is **the gate working.** This extracts the write into `seedPlannedSpec` (`_planned-spec-fixture.ts`) which **self-checks against the real predicate the gate uses** (`isUnplannedSeedPrompt`, imported — not restated) and throws naming the fixture as the cause. Three call sites converted; their ~12-line comments collapse to a pointer, so the diagnosis and both dead-end hypotheses live once, at the seam that causes them. ## Measured (real PostgreSQL, not estimated) | Check | Result | |---|---| | 3 converted families + new ratchet | **41 pass / 41** | | Guard neutered (`if (false)`) | **4 of 5** ratchet tests fail | | Original defect reproduced faithfully | **3 of 7** planning-lane tests fail | | `pnpm lint`, engine `tsc --noEmit` | clean | **The ratchet fails on the original defect.** It drives the two real seed shapes through the production builders (`buildBootstrapPrompt`, `buildRefinementSeedPrompt`) rather than local imitations, so it cannot keep passing if the seed shape drifts — which is exactly the drift the fixture absorbs. The one test that survives the neutered guard is the happy path, which should not move. **The fixture is load-bearing, not decorative.** Proven by reproducing the defect end-to-end: production `buildBootstrapPrompt` on the created row with the guard bypassed → 3 planning-lane tests fail, including its own control case. A false mutation is worth recording, because it nearly produced a wrong "not load-bearing" verdict: a *hand-written* seed passed 7/7. `isUnplannedSeedPrompt` is **byte-equality** against a prompt built from the task's own title/description, so only a byte-exact seed reproduces it. A *missing* prompt does not either — `isUnplannedForExecution` catches the read error and returns `false`. ## Deliberately not converted `workflow-rebound-family`'s `PROMPT.md` write. It is a content-preservation artifact asserted byte-identical across a re-home, not a release-gate fixture; the helper would overwrite the very bytes under assertion. ## Reversible decisions taken (per standing authority) - **`opts.content` seam.** Exists so the ratchet can drive a known seed and prove the throw fires. Without it the guard could only ever be observed passing — the "guard that reports success without checking anything" failure mode. Documented as test-only. - **`title`/`description` optional.** The check is shape-based: both recognised seed forms are `<heading>\n\n<description>` with no section headings, so the written spec cannot match either for *any* description. Omitting them cannot mask a positive, and a test asserts that directly. - **`merged-board`'s spec text lengthened** to match the other two (both were already non-seed, so behaviour is unchanged; verified by the 41-pass run). ## Method correction worth propagating My collision scan was wrong and I nearly acted on it. `git diff origin/main origin/<branch> -- <file>` reports a difference when a branch is merely **stale** (the file did not exist at its base), so it flagged dashboard and CLI PRs as touching engine E2E files. Diffing against each branch's **merge base** is correct. Re-run under the fixed method: all five files here are uncontested, and `feature/code-organization-wave17` genuinely does touch `packages/engine/src/triage.ts` (so that one stays hands-off). ## Lane `.pg.test.ts` under engine-default, `pgDescribe`-skipped without PostgreSQL — the merge gate is unaffected. Throwaway per-file database, never port 4040, no temp-root walk. |
||
|
|
07c29757a3 |
The archived half of the terminal pair, end to end (#2568's fix landed first and is better) (#2670)
**Live on `main`.** `parkCompletedBlockedTask` opens with *"is this card already finished?"* and answered it with: ```ts if (task.column === "done" || task.column === "archived") return false; ``` On a renamed board neither matches, so the guard was **inert** — and inert here is not a missed rescue, it is active damage: the very next block rebounds the card to its planning lane. **A completed card sitting in a renamed complete or archived column was moved backwards out of it.** ## Why this survived two reviews #2644 (mine) converted the **rebound** half to resolve its target by role. This **terminal** half stayed a literal. A role-resolved rebound behind a name-matched guard means the renamed board takes the rebound and never the guard — the same half-conversion shape as the evacuation branch, except the two halves were owned by different changes, so neither review saw both. Worth keeping as a review heuristic for the remaining conversions: **when a converted site sits next to an unconverted one, the conversion can make the neighbour worse — and a diff showing only one of them looks complete.** ## Relationship to #2568 The fix exists there, stranded four deep in a stack whose bottom (#2544) has not merged, so nothing in that chain has reached `main`. This re-lands **only** the guard, directly against `main`. I have noted it on #2568 so its author can drop that hunk rather than resolve it twice; I took no other part of that PR (its extraction and the terminal-pair ownership change are still theirs). ## Deliberate choices - **Fail-soft to `["done", "archived"]`** — an unresolvable workflow behaves exactly as the literal pair did. - **An unclassifiable column is NOT terminal.** Being unable to prove a card is finished must not be the same as proving it is; that is what keeps a stranded card moving. ## Revert proof Restoring the literal pair fails **2 of 5** — the renamed complete and archived cases. Three paired cases pass under the revert, so neither "never park" nor "always park" can pass for resolving the lanes: a genuinely mid-pipeline card is still parked, the legacy pair still answers when no workflow resolves, and an unclassified column still gets the park. ## Verification - 5/5 new; 22/22 with `executor-rebound-already-there` and `executor-task-done-blocked` - `pnpm test:gate` **71/71**; engine typecheck clean; `pnpm lint` clean - census: `executor.ts` **87 → 85** column guards; baseline re-recorded in the same commit 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * Improved handling of completed/blocked tasks when workflow column naming changes, ensuring tasks aren’t parked or moved incorrectly in archived or non-terminal lanes. * **Tests** * Added Vitest coverage to verify terminal-lane decisions, including cases where workflow resolution occurs mid-operation and lane placement changes during the wait. * **Maintenance** * Updated lifecycle column census baseline numbers to match current workflow usage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e711fbab15 |
The ratchet's baseline could not be re-recorded once a file rose — the one state that blocks a correct conversion (#2668)
Unowned (no open PR touches the census CLI — only its baseline JSON) and **live**, since #2654 gates CI on `--strict`. ## The problem `--update-baseline` sat **behind** the rise exit, so the only supported way to re-record was unavailable in exactly the situation that needs it. That matters because **a conversion legitimately adds a literal.** The correct shape for a caller that may have no traits is `flags ? flags.x : columnId === "legacy"`, and each one raises a file's count by one. Measured on current main: `columnRoles.ts` went **0 → 1** from precisely that shape (added by #2647, documented at the site, correct code). So a worker doing the right thing meets a red gate whose only escape is hand-editing the JSON. That is how a ratchet becomes something people route around rather than run — and then it guards nothing. This is the same failure mode as a guard that cannot fire, arrived at from the other side. ## The change `--update-baseline` is an explicit operator action, so it re-records **unconditionally** and prints what it accepted under `ACCEPTED RISES`. Swallowing a rise silently is the real danger; refusing to let anyone re-record is the same danger one step later, wearing a red check nobody trusts. **The rise check is unchanged** and still exits 1 without the flag. **One writer now.** The old second `writeFileSync` behind the rise exit is deleted rather than left unreachable — two writers for one artifact is how they drift. The `!deliberateTracked && updateBaseline` special case went with it, since the unconditional block covers the legacy-shape migration too. ## Exercised end to end On a real rise injected into `live-agent-count.ts`: ``` rise + plain --strict exit 1 (the ratchet still bites) rise + --strict --update-baseline exit 0 "ACCEPTED RISES live-agent-count.ts: 6 -> 7" ``` Four cases assert the CLI's own source, because exit codes are the contract and the pure summarizer cannot express them: the write precedes the rise check, the branches exit 0 and 1 respectively, accepted rises are **named**, and there is exactly **one** writer. ## A note on the revert proof, because it caught me twice My first attempt to move the block back was a **no-op**: the marker I sliced on (`if (regressions.length > 0) {`) also appears *inside* the update block, so the "revert" reassembled the file unchanged and the suite stayed green. **A revert proof that does not go red can mean the guard is vacuous *or* that the revert did not land** — and the second is easy to miss when you are expecting the first. The real revert fails **2 of 27**, and the assertions now verify marker *uniqueness* before slicing on it. ## Verification - 27/27 census suites; `--strict` exits 0; `pnpm test:gate` **71/71**; `pnpm lint` clean - census on this tree: 748 column guards, **4 triage** (all in `moves.ts`'s flag-OFF block, deletion-scheduled with #2655) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
632d10a9b4 |
fix(engine): a completed-blocked guard was inert on renamed boards — plus one owner for the terminal pair (#2568)
Two commits: a behaviour-preserving extraction, then the behaviour change. ## ⚠️ Stack note worth acting on **#2550 and #2554 both report MERGED, but their content is not on `main`.** They merged into their *base branches*, and the bottom of that stack (**#2544**) is still open. Nothing in this chain has reached `main` yet. Nothing is lost — everything is in `origin/feature/workflow-e2e-merge-rebound`, which is why this PR targets it. But "merged" reads as "landed" and here it doesn't. **Merging #2544 flows the whole chain down.** ## The bug `parkCompletedBlockedTask` opens with *"is this card already finished?"* and answered it with: ```ts if (task.column === "done" || task.column === "archived") return false; ``` On a renamed board neither matches, so **the guard was inert** — and the very next branch (`if (task.column !== "todo")`) would then have **moved a completed card back out of its own terminal column**. A guard that never fires does not fail a test. This one was found by tracing the last ledger site, not by anything going red. ## Why a shared owner, not a local fix `merger-ai`'s `isAlreadyFinalizedColumn` held the **only** copy of the per-role terminal-pair rule — a P1 learned the hard way (PR #2471 review): a per-**set** fallback collapses to one element for a workflow declaring `complete` but no `archived`, silently dropping the archived half of every already-finished check. Executor's guard was the raw literal pair, so **whoever converted it next would have re-made exactly that mistake** — the lesson lived in a comment in another file. Hence `resolveTerminalColumns(ir)` in core: one owner, one place for the rule. ## Evidence, and its limits **Commit 1 (extraction) is proven behaviour-preserving**: `workflow-already-finalized-live-e2e` is unchanged and green through the delegation, and the per-set mutation **still fails** through the shared helper. **Commit 2 (the fix) is unproven at the call site, and I'm labelling it rather than implying otherwise.** `parkCompletedBlockedTask` is private and reached only from inside executor dispatch — I could not drive it end to end. So the shared helper gets its **own** tests, in both partial-role directions, precisely because its other consumer can't vouch for it. The call site is a one-line delegation to a tested function. Weaker evidence than the rest of this unit's work. Saying so, because quietly counting it as proven is the exact failure this unit exists to catch. ## Census 417 → 416. That ratchet (#2557) is a **ceiling**, so it stays green without coordination; lower the pin when convenient. ## Verification - E2E suites 10/10; helper unit tests 5/5 - core + engine `tsc --noEmit` clean - `pnpm test:gate` green (414 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## Note on the conflict status (2026-07-31) GitHub reports this PR `CONFLICTING / DIRTY`. **It is not.** Three independent checks: - `git rebase origin/main` on the pushed head reports *"up to date"* and leaves the SHA unchanged — the branch is already on top of main. - `git merge-tree` against the merge base produces **zero** conflict markers. - `origin/main` is unchanged at the commit this was rebased onto. The remote SHA matches the local head, so the push landed. The `mergeable` field is a **stale computation** — it goes stale after a force-push and doesn't always recompute. This branch has now been rebased and force-pushed four times against that cached value. Worth guarding at the source: the auto-retry treats `mergeable` as ground truth, so a stale value generates conflict notices indefinitely. Confirming with a trial rebase or `git merge-tree` before dispatching distinguishes "actually conflicting" from "GitHub hasn't recomputed" — one command, and it ends the loop. Verification on the current head: merge gate green (487 + 158 + 10), engine + core tsc clean, lint clean, 20 tests in the affected suite, zero unresolved threads. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1f149d21de |
fix(engine): TAKING spec-staleness.ts + mission-feature-sync.ts — planner lanes (2 triage guards → 0) (#2616)
**Claiming `packages/engine/src/spec-staleness.ts` and `packages/engine/src/mission-feature-sync.ts`.** Deliberately *not* `self-healing.ts` (contended) or `task-creation.ts` (#2589 in flight). ## Guard counts | scope | before | after | |---|---|---| | `spec-staleness.ts` | 1 | **0** | | `mission-feature-sync.ts` | 1 | **0** | | repo-wide `column === / !== "triage"` in `packages/*/src` (excl. tests) | 26 | **24** | **21** once #2612 (comments-ops, 3 guards) also lands. ## What was silently broken **mission-feature-sync** — *"has this task returned to a planner lane?"* decided whether a mission feature drops from `in-progress` back to `triaged`. Keyed on the legacy pair, a card sent back for re-planning on a renamed board left its feature stuck at `in-progress` **forever**: the mission board showed work in flight that nobody was doing, and nothing said so. **spec-staleness** — the preserved-progress skip refuses to fire for an **intake** card, since a card being specified has no progress to protect. Keyed on `triage`, a renamed-board intake card looked like started work and its stale spec was skipped instead of re-planned. ## The union is a deliberate call, and the existing suite forced it My first cut *replaced* the legacy pair with the resolved lanes. That broke a real case: **post-U11 the default lineage has no `triage`**, so a legacy row still resting there stopped counting as a planner lane. `usage-limit-detector` already made this call for the same situation and wrote down why — **over-inclusion is the safe direction**. Marking a feature `triaged` for a card in a legacy planner column is recoverable; a mission board permanently showing phantom work is the bug. So the legacy pair stands and resolved lanes are *added* to it. Worth noting the existing test is what caught this, not review — which is the argument for converting against a real suite rather than in isolation. ## A parameter, and why that needs the ratchet `spec-staleness`'s predicate is **pure** (a task, no store), so the role arrives as a parameter and both callers resolve it. That optionality is exactly the caller-omission hazard this program has already shipped twice, so the function is also registered in core's `role-parameter-caller-audit` (#2588). **This PR's tests prove the parameter is honoured; the audit proves it is passed. Neither alone is enough** — that split is the whole lesson of #2586. ## Mutation-verified | mutation | result | |---|---| | mission-feature-sync → legacy pair only | the two renamed cases fail | | spec-staleness → restore the `triage` literal | its renamed case fails | Negatives included in both: demoting a **WIP** card's feature would report running work as un-started, and never-skipping would discard every card with real progress. ## Verification - new suites 4/4 and 3/3; `mission-feature-sync` + `spec-staleness` 27/27 - engine `tsc --noEmit` clean; `pnpm test:gate` green (482 + 132 + 10) - `executor-prompt` reports 3 failures **both with and without** this change — pre-existing, baselined by stashing rather than assumed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
dca20496f4 |
consolidate/u7: plugins to zero + 8 executor rebound guards + resume lanes (supersedes #2607, #2635, #2640) (#2644)
Consolidation branch for U7, per the new one-branch working mode. **Supersedes #2607, #2635, #2640** — the three of my PRs that were stuck on review threads. My other seven (#2602, #2605, #2606, #2611, #2621, #2628, #2633) are green with **zero unresolved threads** and are deliberately left alone for the merge sweep. ## What is in here, file by file | file | change | guards before → after | |---|---|---| | `plugins/…/glasses/src/agent-actions.ts` | gates, destinations and degraded-resolution refusal all resolve from the task's own workflow | 2 → 0 | | `plugins/…/glasses/src/quick-capture.ts` | accepted capture columns come from the board; default no longer names the deleted column | 1 → 0 | | `plugins/…/glasses/src/settings.ts` | quick-capture default was `triage`, the column #2515 removed | (assignment, uncounted) | | `plugins/…/dependency-graph/src/GraphTaskNode.tsx` | redundant column condition deleted | 1 → 0 | | `packages/engine/src/executor.ts` | 8 rebound guards compare the resolved column; 4 resume-eligibility literals share one resolver | 151 → 143 (+4 off-bar) | | `packages/engine/src/__tests__/` | 4 new suites, 26 cases | — | `plugins/` reaches **zero** column guards with this branch. ## The three threads it closes **#2607 — five findings, all mine, all the same rule.** I kept *qualifying* a legacy-id fallback instead of removing it: | attempt | rule | hole review found | |---|---|---| | 1 | fall back to `todo` when the role is missing | moved cards to phantom columns | | 2 | …only if the workflow **declares** `todo` | aliased **review** lane named `todo` | | 3 | …and only if no other role is assigned to it | **traitless** parking column named `todo` | The qualifications were the mistake. Once `resolveLanes` returns a lane set the workflow *has* a column vocabulary, so "no column carries the hold trait" is a complete answer — refuse. `destination()` is two lines now, with no aliasing surface left to qualify. Plus a sixth, which is a genuinely different state: **degraded resolution is indistinguishable from the default board.** `resolveWorkflowIrForTask` is total by design — a missing definition silently returns the *default* coding IR — so a card on a custom board whose definition could not be read resolved to `todo`/`in-progress`. `undefined` lanes cannot express that (it means "no workflow at all", where the legacy ids *are* the answer). The actions now refuse with 409. #2618 would replace this check with resolver provenance; it is not merged, so this does not depend on it. **#2635 — "seven rebound sites remain untested."** Fair; my "same shape" note was an assertion, not coverage. Seven of the eight need a live graph run to reach, so the *shape* is pinned instead: a static check that no guard in front of a rebound move compares against a column literal, with a vacuity case (the same detection run against the original shape) and a match-count floor (≥8), because a guard reporting success on zero matches is worse than no guard. **#2640 — duplicate workflow resolution.** Framed as I/O; it is also a correctness bug. Eligibility and re-entry are two halves of one decision and resolved the workflow separately, so a workflow edit landing between them has the halves reading *different boards*. Now one caller-owned memo per decision — caller-owned because a process-lifetime cache would have to guess when a mid-flight workflow edit invalidates it. ## Behavioural findings, not tidying - **The last-resort recovery for completed-but-stranded work did not exist off the default lineage.** `promotedFromPlannerColumn` was false on a renamed board, so finished work resting in planning was never promoted; the code fell through to a review handoff that role adjacency rejects, and the card stayed stuck with its work complete. - **Rebound guards could not see the column their own move targeted.** U5b converted the move target; the eight `column !== "todo"` checks in front of it were left literal, so on a renamed board the engine moved a card into the column it was already in — and `moveTaskInternal` runs reset-on-entry on every real move, so at the `preserveProgress: false` site it reset step progress a second time. - **The FN-1404 `task:move` audit row was lying**, recording `to: "todo"` while the move target was resolved. A run-audit trail that disagrees with the move it describes is worse than none. Not a comparison, so no census counts it. - **A task interrupted by an engine pause never resumed on a renamed board** (off-bar, `in-review`/`in-progress` literals): four comparisons decided one question and had to agree; two of them disagreed on a renamed board, so re-entry silently never fired. ## Revert proofs, isolated per site | reverted | result | |---|---| | `destination()` back to attempt 3 | 3 of 38 fail | | degraded-resolution refusals removed | 2 of 42 fail | | capture set back to the legacy five | 2 of 3 fail (renamed-board suite) | | forward exclusions → literals | 1 of 14 fails | | missing-wip refusal removed | 2 of 14 fail | | `promotedFromPlannerColumn` → literals | 3 of 7 fail | | promotion target → `"in-progress"` | 3 of 7 fail | | one rebound guard → `!== "todo"` | 1 of 3 fails (static shape) | | resume lanes → legacy trio | 1 of 5 fails | Every conversion is paired with a negative — a forward move, a not-a-planner-lane card, a default-lineage card, an unresolvable workflow — so neither "always fire" nor "never fire" can pass for "resolve the role". ## Commit discipline Twelve commits, each one thing: the code move (`resolvePlannerLanes` out of `triage.ts`) is separate from every behavior change, and each review fix is its own commit with its own revert proof. ## Verification - `pnpm test:gate` **71/71** - 162/162 across the glasses plugin's 19 files; 26/26 across the four new engine suites - engine + glasses typecheck 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 * **Bug Fixes** * Engine recovery and retries now work correctly with renamed or customized workflow columns. * Tasks in manual-intake columns are no longer automatically planned. * Agent actions and quick capture now respect each board’s declared columns and lifecycle stages. * Awaiting-approval tasks are recognized regardless of their current column. * Command Center SDLC funnel stages now accurately reflect customized workflows. * **Documentation** * Added guidance for safely changing workflow-column logic and interpreting lifecycle-column checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f6010ef558 |
fix(test): planning-lane E2E is RED on main (2/7, incl. its own control) — same unplanned-spec fixture defect #2634 fixed next door (#2658)
Found while establishing the pre-closing E2E baseline for the final verification pass (closing bar, item 4). **Two of this family's seven cases are failing on `origin/main` right now**, and one of them is its own control. ``` releases an ordinary held card on a default board (the control) expected [] to include 'FN-OK' holds a card parked for approval MID-SWEEP, after the snapshot read expected false to be true ``` ## Cause — the same defect #2634 repaired in the file next door `seedHeldTask` creates the task and never writes a `PROMPT.md`, so the card carries only the bootstrap seed. FN-7648's `isUnplannedForExecution` reads that file for any card resting in an intake- or hold-trait column and refuses to move an unplanned card into a processing column, so the sweep released nothing. **Being held was the gate working.** The fixture was exercising the gate rather than the sweep — which is exactly why the *control* failed, and a failing control means the rest of the family's assertions cannot be trusted either. `workflow-lifecycle-live-e2e` had the identical problem and #2634 fixed it the same way. This file landed alongside it (#2611) and did not get the same treatment. Worth stating twice because it is a general rule for this directory: **a release/scheduler fixture that does not model a card which cleared specification is testing the gate, not the sweep.** ## The check that matters more than the fix #2611's stated value is "3/7 red without the guard". Making red tests green is the easiest thing in the world to do wrongly, so I verified the family still discriminates *after* the seed — disabling `isTaskBlockedOnApproval` in `hold-release.ts` still kills exactly three, and the same three: | killed by mutation | |---| | does NOT release a card blocked on manual plan approval on a **default** board | | does NOT release a card blocked on manual plan approval on a **renamed** board | | holds a card parked for approval **MID-SWEEP**, after the snapshot was read | Two cases turned green, zero discriminating power lost. Without that mutation this change would be indistinguishable from weakening the tests until they passed, which the standing rule forbids. Note the third killed case is also one of the two that were failing: it was red for the fixture reason **and** genuinely proves the guard. ## Why it is worth a PR of its own The closing bar's final verification pass (gate, `verify:fast`, all E2E, census) has to run on a green tree. Two red E2E cases on main would otherwise show up in that report as a new failure and cost a diagnosis at exactly the wrong moment. Pre-closing baseline for the record — **13 E2E families, 109 tests, these 2 the only failures**: ``` green agent-count 13 · agent-link 5 · lease-rebound 6 · lifecycle 24 · merge-family 7 merge-rebound 4 · merge-safeguards 10 · merged-board 5 · planner-lane 5 planner-lane-resolution 3 · rebound-family 15 · stranded-column 5 RED planning-lane 7 (2 failing) ``` ## Verification 7/7 green, mutation 3/7 as designed, engine typecheck clean (0 lines), `pnpm lint` exit 0, `pnpm test:gate` exit 0. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
2fb0df9da8 |
docs(replan-target): the flagged follow-up is done — the note said otherwise (#2665)
Comment only. The block above `resolveReplanTargetColumn` still reads: > **STILL A REAL FOLLOW-UP** … the `return "triage"` fallbacks on the no-match and throw paths name a column the default lineage no longer declares … flagged rather than fixed That directly contradicts the code six lines below it. **#2598 landed the fix:** the no-match path now returns `roles?.hold ?? roles?.intake`, and the throw path returns `undefined`. I wrote that note. A stale *"not fixed yet"* sitting above a fixed implementation is worse than no note — the next reader either distrusts the code or re-does work that is already done. This is the closing-bar item 3 I was assigned, and I nearly re-did it myself: I had the change written and reverted before checking whether main had overtaken me. ## One thing worth recording about #2598's version Its catch-path answer is **stronger than the one I had drafted**. I was going to return `"todo"` — the better guess, since post-U11 the default lineage declares `todo` and not `triage`. #2598 returns `undefined` instead, which forces callers to handle "this workflow could not be resolved" explicitly rather than papering over it with a plausible column id that the move path may then reject. That is the same lesson as the sync-reader audit in #2653: **a defective lookup that returns a valid-looking answer is worse than one that admits it does not know.** Recorded in the comment so the reasoning survives. ## Verification Engine typecheck clean · **51/51** across both replan-target suites · comment-only, no executable change. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b6b2fdcdc6 |
test(U7): rescue the orphan-triage regression test — main has the fix but not its test (#2663)
Main already carries **every other artifact** from #2593 — the provenance fix, the `DELIBERATE-LITERAL` markers in `TaskCard`/`TaskDetailModal`/`register-routes`, the audit doc. The one thing missing is the test. That is the same artifact class that vanished when #2645's branch was force-pushed, so I rebased #2593 onto current main, found every commit conflicting because the work had landed by other routes, and rescued the one piece that had not. **#2593 can now be closed** — it carries nothing else main lacks. **#2654 needs rebasing onto main** rather than stacking on it. ## What makes this test worth rescuing It took three attempts to write honestly, and the reason is pinned in the test body: on a bare mock, `resolvePlannerLanes` reads `resolveTaskWorkflowIrSync`, which the mock does not define, so it returns `LEGACY_PLANNER_LANES` (`intake: "triage"`) and a `triage` card matches the **first** arm — the orphan arm is never reached. Every earlier fixture I wrote passed through that short-circuit and proved nothing. All three cases stub that reader with the merged default (`intake: "todo"`), which is what production resolves, leaving the orphan arm as the only thing deciding. They differ **only** in the workflow readers. | case | role | |---|---| | **C** — workflow declares `triage` as a review lane | **the discriminator.** Pre-fix, the sync reader ignores the selection, returns the default IR declaring no `triage`, so the arm fires and a card is finalized out of a custom workflow's code-review column | | **B** — workflow resolves, declares no `triage` | positive control; without it "returns false" is unfalsifiable | | **A** — workflow unresolvable | **behavior pin, NOT a regression test** — passes in both worlds | I had A labelled "REGRESSION" until the mutation said otherwise. It is relabelled with the null result documented, because a future edit making it flip would mean the arm's scope changed. ## Verification, stated precisely **231/231** against main's implementation. The mutation that proved C discriminates was run on the branch where the pre-fix code still compiled. **It cannot be re-run against main**: the `WorkflowIr` type import was removed along with the fix, so a naive revert no longer transforms. I am stating that rather than implying I re-verified it here — the discrimination was demonstrated, just not on this base. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3bf9bf5f74 |
collapse the plan-admission-throttle payload to one gate (+ AGENTS.md) (#2562)
The cross-project semaphore is deleted, so `task:plan-admission-throttled` was describing a gate that no longer exists. Nothing wires `options.semaphore` any more, which left three things dead-but-visible: - `semaphoreAvailable` was permanently `Infinity`, so `Math.min(projectRoom, …)` was a no-op keeping a deleted limiter in the arithmetic - `blockedBy` was a **discriminator** between `"running-agent cap"` and `"global semaphore"`; only the first can occur - four `semaphore*` metadata fields were always `undefined`, and two more terms in the dedupe signature were constant ## `blockedBy` is kept, not dropped Even though it is now a constant. The event exists (FN-8600) to answer *“why did this card sit queued to plan?”* after the fact — a named reason answers that even when there is one gate, whereas a payload with **no** reason field reads as “unknown”. It costs nothing and preserves the shape if a second gate is ever added. The dedupe signature drops the two semaphore terms and keeps the eligible task IDs — that term is what stops a **new** card’s stall being swallowed when the counts land on an unchanged tuple, which is the property the event depends on. ## AGENTS.md It documented the removed field names verbatim, so it is updated in the same commit. Leaving docs describing a payload the code cannot emit is exactly the readable-but-wrong artifact this program keeps deleting. ## Verification `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green · triage suites **234/234**. --- **Correction I owe on `concurrency.ts`, measured rather than estimated.** I earlier told the coordinator ~75% of its 886 lines could go with the cross-project cap. That was line-range arithmetic and it was wrong. With the cap now fully removed, `concurrency.ts` is **still 886 lines**, because `AgentSemaphore` has four consumers unrelated to it — `verification-concurrency` (maxConcurrentVerifications), `research-orchestrator` (research runs), `experiment-executor` (maxConcurrentExperiments), `step-session-executor` (parallel steps) — plus `ProjectAdmissionCoordinator`, which is FN-8453 oldest-first **ordering**, not a limiter. The real remaining win there is the pre-held-slot bookkeeping and the idle-semaphore leak recovery, which existed to service the global instance; I will measure that as its own slice rather than quote a fraction. 🤖 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** * Updated plan admission throttling to consistently use the project’s running-agent capacity. * Improved throttle audit events by reporting stable capacity details and removing obsolete semaphore information. * Preserved accurate deduplication for repeated throttling events, including changes in stalled tasks. * **Documentation** * Updated run-audit guidance to match the revised throttling event format. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8393bba7dc |
U7: the replan rebound targets a column the workflow declares (R7) — re-landed on main (#2598)
> Based on `main`, no dependencies. Re-landed after closing the stacked chain (#2517, and #2551 below) that never reached main. ## What main already has, and what it lacks Main independently converted the planner-lane **parameters** in this file — and **better than I had**: it splits `plannerColumn` from `roles.mergedPlanningColumn`, because a merged lane joins the FN-8596 arrival-order rescue but *not* the "planner column is never advanced" shortcut. That work is main's and untouched here. What main still lacks is the **R7 fix**: `resolveReplanTargetColumn` returns `"triage"` **by fiat** for any workflow declaring neither legacy id — `builtin:marketing` (ideation/backlog/drafting/…) and every fully renamed set. A Plan Review REVISE therefore moves the card into a column its workflow **does not declare**, for `reconcileUndeclaredTaskColumns` to clean up after. A move the engine makes on purpose, not drift. | Workflow | Target | Changed? | |---|---|---| | `builtin:coding` / stepwise | `todo` | no | | Coding (Ideas) | `todo` | no | | `builtin:marketing` | `backlog` (its own hold) | **yes** — was `triage`, undeclared | | declares no planning lane | `undefined` → park | **yes** — was `triage` by fiat | ## The ordering the existing suite taught me Legacy ids stay preferred **first**, and the trait resolution prefers **hold over intake**. That is not arbitrary: Coding (Ideas) declares `ideas` as its intake, and `ideas` is **manual capture with no AI** (plan R10) — a rejected plan sent there stops being replanned at all. The old code got Ideas right **by accident**: it never recognised `ideas` as intake and fell through to `todo`. An "intake first" trait rule would have shipped that regression dressed as a cleanup, and three existing Ideas tests were the only thing between me and doing it. ## An inverted comment, corrected The function's own U11 note read: *"the second lookup asks for `todo`, which U11 deletes… the first lookup still matches `triage` (which U11 keeps)"*. **That is backwards.** #2515 keeps `todo` and deletes `triage`, so the consequence is the opposite of what was written — the `todo` branch is what saves builtin coding. Fixed rather than left, because a comment that inverts a merge's direction sends the next reader to the wrong branch. ## Fail-closed callers `undefined` means "nowhere to replan" (plan U5: *skipped with a log rather than moved arbitrarily*). All four call sites park **visibly** rather than log a move they did not make. The scheduler's rebound still writes `needs-replan` — deliberately, since that is what blocks dispatch and the branch has already decided the card must not be released — with only the *log* made conditional. ## The superseded test is deleted, not skipped A skipped test is a guard that cannot fire. Its replacement asserts the new contract **and** the R7 invariant directly — *"a column this workflow declares"*, not just an id — plus a new case for a workflow with no planning lane at all. ## Verification | Check | Result | |---|---| | replan-target | 41/41 | | with scheduler-trait-dispatch + pre-release-plan-review | 55/55 | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (482 + 10 + 71) | | `pnpm check:changesets` | clean | `triage.test.ts` still shows main's **8 pre-existing #2515 failures** — unchanged by this, fixed by **#2576**. ## Closing #2551 Its parameter work is superseded by main's better version; this PR carries the only part main lacked. Same story as #2517: a stacked PR outlived the surface it was converting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cf6133da8b |
consolidate/e2e — E2E evidence: already-finalized terminal roles (real merge entry, no git) + ledger corrections (#2648)
Consolidation branch for the E2E-evidence worker. Two commits, both engine test/comment only — **no production code, census unchanged**. ## Census (the authoritative instrument) `node scripts/lifecycle-column-census.mjs` on this branch: **triage 10, total 784** — identical to its base. `lifecycle-column-census-ast.test.ts` and `lifecycle-column-census.test.ts` pass (15). This PR neither shrinks nor grows the backlog; it is evidence. ## What it contains, file by file | file | change | |---|---| | `packages/engine/src/__tests__/workflow-already-finalized-live-e2e.pg.test.ts` | **new** — 3 cases, live PG store + real `runAiMerge` | | `packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts` | comment only — retires two unproven-ledger entries | ## The evidence: `isAlreadyFinalizedColumn` never needed the real-git lane My unproven-sites ledger listed it as requiring a git harness because it is module-private inside `runAiMerge`. Reading the function instead of costing the lane: `runAiMerge` reaches it after only `store.getTask`, a pure workspace assert, and a pure branch resolve — **before** the merge blocker, settings, and any branch sync. `projectRootDir` is never touched on that path, and the short-circuit returns a `noOp` rather than throwing. Reachable through the real public entry point with no repository at all. ### Why two cases and not one The guard resolves terminal columns **per role**: ```ts terminal = [lifecycle.complete ?? "done", lifecycle.archived ?? "archived"] ``` #2471's P1 caught the first cut replacing the whole legacy **pair** as soon as *any* terminal role resolved — a workflow declaring `complete` but no `archived` collapsed to one element, silently lost the archived short-circuit, and an archived card then threw *"must be in 'in-review'"* for a card whose real state was "already done, nothing to do". A per-set rule passes for whichever role **is** declared and fails the other, so a single case cannot tell the two rules apart. The shared fixture declares `complete` (renamed `shipped`) and **no** `archived`, so it is exactly that partially-declared shape — resolved half and fallback half live on one board. Mutation-verified, each killing only its own case: | mutation | kills | |---|---| | per-**set** replacement (the #2471 defect) | the legacy-`archived` fallback case | | legacy pair only (conversion reverted) | the renamed-`shipped` case | Plus a differential: a renamed **review** card must not report already-finalized. Without it both cases above would pass for a guard that finalizes everything — turning every merge into a silent no-op, the worst failure this function has. Evidence strength is stated in the file header rather than overclaimed: this reads a returned **decision**, not a persisted row, so it proves the renamed board resolves and short-circuits — not that a card moves. ## Ledger corrections (comment only) Two entries retired, both wrong the same way — each stated a **lane cost** as if it were an impossibility: - `columnIsIntakeOrHold` — "consumers are dashboard-side" is true and irrelevant; its one consumer is an exported pure function. Proven on merged and renamed boards by work already merged in #2631. - `register-task-workflow-routes.ts` — "standing up the route shell is mock-the-world" was false; `createApiRoutes` + `test-request.js` is this repo's established convention with ten existing suites for that file. Covered by its owner in #2614. Counting this PR's own subject, that is **seven** wrong lane-cost inferences in that ledger. The rule it keeps violating is unchanged and now recorded in the file: read what the FUNCTION touches before costing a lane for it. ## Verification `pnpm test:gate` exit 0 (695), `pnpm lint` exit 0, engine typecheck clean, 28 tests green across the AST ratchet and the three merged-board/planner-lane/already-finalized families. ## Not in scope here The `performWorkflowRerunBounce` E2E. The harness exists (`new TaskExecutor(store, "/tmp/test", {})`), but `executor.ts:4305` gates the rebound on the legacy `in-progress`/`in-review` pair while resolving its target by role — so on a renamed board the bounce never fires and the resolved target is unreachable. An E2E asserting today's behaviour would cement that. It is executor.ts's owner's fix; evidence should follow it. Detail in #2632's thread. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
20878e9d5f |
census: count column: "<legacy>" query filters as a separate, separately-pinned instrument (backlog unchanged at 784) (#2650)
Pre-launch input for the 779-guard fleet. **The backlog number does not
move: 784 before, 784 after.** This adds a second number beside it.
## The problem it measures
A guard is not the only way a legacy column id decides behaviour:
```ts
const todo = await this.store.listTasks({ column: "todo", slim: true });
```
That is a **source query** — it selects the rows a sweep considers *at
all*. On a renamed or merged board it returns nothing, so a sweep whose
per-task predicate was correctly converted still does nothing, while
looking converted. `self-healing.ts:2849` names the pairing in prose,
and #2560 had to repair exactly that combination after a converted
predicate was left with a literal query.
The census walks comparison `BinaryExpression`s. A `PropertyAssignment`
is not one, so this class was invisible to the instrument **and to its
ratchet** — it could grow silently.
Measured: **83 query filters, 43 IR node definitions.**
I proved one live consequence earlier on #2648:
`recoverStuckMergeDeadlocks` cannot see a renamed board at all — the
renamed rows exist and none appear in its three-literal union
(`renamedInsideUnion=0`, on a live PG store).
## Why this matters *before* the fleet is briefed
The fleet rule is *"the baseline ratchet must shrink by exactly the
converted count."* In `self-healing.ts` — the largest batch at 111 —
both classes sit in the same functions, so today a worker either:
- converts only the comparisons → arithmetic is clean, and sweeps whose
source query still filters a dead literal stay blind; or
- converts the query too → the count does **not** move by the converted
amount, and a more-correct PR looks like a miscount.
The second punishes the better worker. With a second pinned number,
converting a query becomes visible work instead of an apparent error.
## Counted separately, deliberately
`totals.column` is a published shape — the baseline, the reporter, and
other workers' in-flight PRs read it, and the completion bar is defined
against it. Growing it would move a number the program is actively
driving to zero.
So the new counts live in `summary.properties` / `queryByFile`, under
their own baseline keys, with their own both-directions ratchet (same
rule as #2633's, including the stale-allowance half). `totals` keeps its
**exact** shape — two existing tests assert it with `toEqual`, and
breaking a contract others depend on mid-flight to add a number is not
worth it.
## Definitions are not queries
Workflow IR graph nodes carry `column:` to declare where a node lives —
`{ id: "review", kind: "...", column: "in-review" }`. That is the
lineage describing itself: not a lookup, not convertible, and ~43 of the
raw matches. They are told apart **structurally** (an `id`/`kind`
sibling in the same object literal), not by filename, so a definition
written anywhere classifies the same way.
## Baseline seeding, stated plainly
`--update-baseline` could not pin a **new** category: the regression
check runs before the write, and with no prior key every file reads as a
rise. I seeded the three new keys once, directly, leaving every guard
field byte-identical. The diff is purely additive — no removals.
## Finding, not caused by this change
**`--strict` is already red on clean main**:
`register-task-workflow-routes.ts` is **23** against a baseline of
**22**. Verified by stashing this branch and re-running on an unmodified
tree. Until that is reconciled the guard ratchet is passing nothing —
worth fixing before the fleet starts relying on it as the work order.
## Verification
- census suites **44 green**, 6 new cases: counted; kept out of the
backlog; definition-not-query; both instruments independent (a bug
routing comparisons into the query bucket would otherwise look clean on
both); `DELIBERATE-LITERAL` honoured; non-legacy id ignored
- `node scripts/lifecycle-column-census.mjs` → backlog still 784
- `pnpm lint` exit 0, `pnpm test:gate` exit 0 (695)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
2771408bba |
ci: enforce the lifecycle-column ratchet — it has never actually run (#2654)
**The ratchet was advisory.** `scripts/lifecycle-column-census.mjs` existed only as `pnpm census:lifecycle-columns` — without `--strict` — and **no workflow invoked it**. Nothing has ever compared the tree to the baseline. Every "the baseline ratchet holds them" assumption in this program rested on a check that does not run. That explains both classes of hole: **1. Three PRs lowered counts without re-recording,** leaving allowances the deleted guards could return through while every check stayed green. I've tightened them across #2593 and earlier PRs, but nothing stops the next one. **2. #2621 GREW the count while its own title claimed "count 0 → 0".** It added `column === "triage"` and `column === "todo"` at `register-task-workflow-routes.ts:2681`, taking that file to **23 against an allowance of 22**. It landed unchallenged. This is the failure mode the ratchet exists to prevent, and it happened *inside this program*, in a PR that asserted the opposite. ## The change Adds `check:lifecycle-columns` (the census with `--strict`) to the `pr-checks.yml` lint job, next to `check:changesets` and `check:routes-modular` — the established pattern. **~1.8s over ~1950 files**, so this is not a slow-test addition. ## Proven to fail, in both directions A guard that reports success without checking anything is worse than no guard, so: | injected defect | result | |---|---| | `const __probe = (c: string) => c === "triage"` added to `moves.ts` | `count ROSE — moves.ts: 39 -> 40`, exit 1 | | run against main's current baseline | exit 1 on `mission-feature-sync.ts: allows 5, tree has 0` | Both reverted; exit 0 restored. Note the second row: **this check is RED on main right now**, which is the point. ## Merge order **Stacked on #2593**, which carries the `DELIBERATE-LITERAL` marker for the #2621 site (a v1 IR declares no roles, so no trait can answer that question) plus the baseline re-record. Standalone on main this PR is red — correctly. **Merge #2593 first**, then this. I stacked rather than duplicating those two edits because I already caused one conflict today by appending related content from two branches, and #2651 merged a correction ahead of the section it corrected. Same-content edits in two PRs is the same mistake. ## Census Unchanged by this PR: **776 total, triage 5, reviewed 16** — it adds no guards and converts none. It only makes the numbers enforceable. ## For the fleet This should land before the 776-guard fleet launches. The brief says "the baseline ratchet must shrink by exactly the converted count" — until now nothing verified that claim, so a batch worker could report a shrink that did not happen, or grow the count while converting, and CI would agree. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |