c9df4b9deee2004331cbbcbf80e2493ed3796ffb
12428 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c9df4b9dee |
U11 migration proof: the path an operator actually hits, with all three caveats answered (#2597)
Tests only. Proves the upgrade path the existing E2E does not cover, and answers the three caveats. ## Why the existing coverage was not enough The existing cases strand a card in a synthetic `a-column-no-workflow-declares` on a **fixture** vocabulary. The real upgrade leaves cards in **`triage`**, on the **real `builtin:coding`** workflow. That difference is the whole point: `triage` is still a legal `ColumnId` and is still declared by legacy-coding, Ideas and every linear built-in (R11), so nothing rejects it and **nothing throws**. The card simply sits in a column its *own* workflow no longer declares — where it carries no trait flags and is invisible to every trait-driven sweep. ## What is proven, on a temp PostgreSQL project - A card left in the deleted `triage` column on a default-workflow board is re-homed to `todo`, the merged Planning column. - **Revert check in-suite:** without the sweep running, the card stays in `triage`. Without this, the case above could pass because some *other* sweep or a store-open reconcile moved the card — and would keep passing if the sweep were deleted outright. - **Progress and the plan artifact survive.** `preserveProgress: true` is asserted end-to-end rather than trusted from the option name. - A `userPaused` card is skipped and stays in the deleted column. **Mutation-verified:** stubbing `reconcileUndeclaredTaskColumns` to `return 0` turns **5 of 11** tests red, including all three positive migration cases. The sweep is demonstrably the mover. ## The three caveats — answered **1. `userPaused` cards are skipped → caveat, not a stall.** An operator park is authoritative and the sweep must not override it, so the card does stay in a column its workflow no longer declares. But it is reachable two ways: unpausing makes the next sweep re-home it, and **U11's undeclared-source escape hatch in `resolveAllowedColumns` (merged with #2515) lets an operator move it by hand meanwhile** — that path returns the workflow's rebound target instead of `Valid targets: none`. Recorded as a test so the behaviour is a decision rather than an accident. My recommendation: **leave it skipped.** Re-homing a paused card silently moves work an operator deliberately froze, and the escape hatch already gives them a way out. Overriding a park to fix a column is the wrong trade. **2. Sweep only runs when self-healing is enabled → caveat, not a stall, for the same reason.** The escape hatch lives in the **move-validation** path, not in self-healing, so it works with self-healing off entirely. A card stranded that way is draggable out of the deleted column by hand. Worth knowing: before #2515's escape hatch this *would* have been a hard stall — `resolveAllowedColumns` returned `[]` for an undeclared source, so the card could not be moved anywhere at all, by anyone. **3. Re-home targets the HOLD column → correct, and progress survives.** Under U11 the hold column **is** the Planning column, so "everything lands in Planning" is the intended destination rather than a compromise. Asserted with real step progress on the row. **None of the three is worse than a caveat.** The reason all three are survivable is the same single mechanism — the undeclared-source escape hatch — which is worth knowing because removing it would silently promote all three to hard stalls. ## Incidental Fixed two fixture-level PostgreSQL column-name errors found while writing this: `currentStep` → `current_step`, and `user_paused` is an **integer** flag rather than a boolean. Both would have made a future test here fail confusingly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a3a7f16977 |
Drift 4/4: reset reported a successful reset as a 409 "limbo" conflict — plus the audit verdict for every other site in the file (#2582)
## Drift 4/4 — a second live bug in the routes file, plus the audit for
the rest of it
**Stacks on #2571** (the P0). Merge that first.
### The bug
`POST /tasks/:id/reset` resolves its destination through
`resolveReboundColumnForTask` — the task's own workflow rebound column —
and then verified the outcome against the literal `todo`. **Twice.**
So on any workflow whose rebound column is not `todo` — Coding (Ideas),
any custom or renamed lineage — a reset that **succeeded** was reported
as a `409` "limbo state" conflict. The mover and its own verification
disagreed about where the card was supposed to land.
Both checks now compare against `resetColumn`, which is already in scope
two lines above the first one.
### Revert-proof, after I caught my own vacuous test
My first version of this test **passed with the fix reverted**. The
reset route demands `{ confirm: true }` and was 400ing before it ever
reached the column check, so `expect(status).not.toBe(409)` was
trivially true. That is the third time in this program a route/DOM
assertion has looked like coverage while checking nothing, and the
second time I have caught it in my own test.
With the confirmation sent, the reverted form fails: `expected 409 not
to be 409` — a correctly-reset card reported as limbo.
### Audit of the remaining sites in this file
| site | fires after #2515? | verdict |
|---|---|---|
| 2597/2607 manual retry | yes | **SAFE, by design.** Falls back to a
`todo` branch gated on the workflow declaring no `triage` column —
exactly the merged shape. Written for Coding (Ideas); the merge made the
default match it. |
| 4584 respecify | yes | **SAFE.** Already `column === "triage" \|\|
column === respecifyTarget`, and `respecifyTarget` resolves the intake
column. |
| 2887/2917 reset verification | **no** | **FIXED here** — false 409 on
a successful reset. |
| 1121 awaiting-planning enrichment | partially | **BROKEN, deliberately
not converted** — see below. |
### The one I chose not to convert, and why
`1121` filters on `column === "todo"`, so a workflow whose waiting lane
is named otherwise gets no enrichment and silently falls back to the
heuristic.
I converted it and **reverted**. Resolving each task's hold column needs
a per-task workflow read, and this is the board-load path whose own
comment exists because unbounded reads here *"turn a board load into
thousands of reads"*. My version did those reads for **every task before
the enrich limit applied** — trading a silent degradation for a
load-time regression on every board.
Converting it properly needs the hold column resolved per **workflow**
from data the board payload already carries, not per task from the
store. That is a real change with a measurable cost, not a rename. Left
with the cost written at the site rather than quietly skipped, and
flagged here so it is tracked rather than forgotten.
### Verification
`pnpm test:gate` (414 + 10 + 71), `pnpm lint`, dashboard typecheck
green. Route suites: 9 passed, including
`stranded-refinements-routes.test.ts` unchanged at 5.
### My drift set, final
| file | before | after | PR |
|---|---|---|---|
| `TaskCard.tsx` | 8 | 3 | #2558 |
| `ListView.tsx` | 5 | 3 | #2566 |
| `taskActivity.ts` (found underneath) | 1 | 1 | #2566 |
| `TaskDetailModal.tsx` | 4 | 3 | #2577 |
| `register-task-workflow-routes.ts` | 10 | 11 → 9 | #2571 + this |
Routes went 10 → 11 in #2571 (guards widened to accept resolved-intake
**or** `triage`, so a P0 fix could not reject anything previously
allowed) and back to 9 here. Every other survivor is the documented
no-metadata fallback: flags are absent during the pre-load window and
for a card stranded in a vanished lane, and a bare trait read would drop
the affordance in exactly those states. They retire with the load
window, not with a rename.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
131feb243c |
U8: the exit announcement was on a dead code path — move it to the handler the engine actually runs (#2578)
A merged behavior of mine has never executed. This fixes it and adds the ratchet that would have caught it. ## The finding `createDefaultNodeHandlers` chooses the prompt-node handler like this: ```ts const promptLike = deps?.primitives ? createPrimitivePromptLikeHandler(deps.primitives, runCustomNode) : createPromptLikeHandler(seams, runCustomNode); ``` `executeWorkflowGraph` always passes `primitives: this.createAuthoritativeWorkflowPrimitives(settings)` (`executor.ts:6051`). **So `createPromptLikeHandler` — and with it every `execute` / `step-execute` function in `createAuthoritativeWorkflowSeams` — is unreachable for prompt nodes.** Both objects are passed to the graph executor and only one is consulted. The `NodeCompleted.exit` announcement added in #2507 was wired into that seam. It type-checks, its tests pass (they call the seam object directly), and it has never run in production. `runCodingSession` in the primitives is the live twin, and that is where it emits now. ## How it was found — and why the negative is trustworthy Instrumenting `createAuthoritativeWorkflowSeams.stepExecute` produced no output for a run that demonstrably visits `steps#0:step-execute`. So did instrumenting `createPromptLikeHandler`'s dispatch. A negative result from instrumentation is worthless until the instrumentation is shown to be observable, so: a `process.stderr.write` at module load of the same file **did** appear, exactly once, in the same run. The two negatives were real, not swallowed output. This is also the answer to the open question I left in #2546 — the pending-review routing move kept failing because the seam value it depends on is never produced. **That move is still not landed here.** This commit only relocates the announcement, so it stays small and separately revertable; the routing move follows once its value originates on the live path. ## The ratchet A source assertion pins the dispatch rule: `deps?.primitives ? createPrimitivePromptLikeHandler` and the executor's wiring of `primitives`. Inverting or conditionalising that preference would silently disable every behavior attached to the primitives path — the same failure in the other direction — and **a seam-level unit test cannot tell the two apart**, which is precisely how this survived review twice. ## Red-green Removing the emit fails 2 of the 4 new tests (`Tests 2 failed | 2 passed (4)`). The other two are the regression floor: an ordinary completion emits `success` with no `exit`, and the returned routing outcome is unchanged — announcing must not reroute. ## Scope note I did **not** delete the now-known-dead seam wiring in this PR. `createAuthoritativeWorkflowSeams` is still passed to the graph executor and its non-prompt entries (`stepReview`, `merge`) are reached through other handlers, so deciding what is genuinely dead there is a deletion audit of its own — and this program's rule is that deletions never ride along with behavior changes. Filed as the next slice. ## Verification - 4 new tests + exit-events + step-session + triage audit + ownership ledger — **54 tests green** - `pnpm test:gate` green (10 / 414 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `fix`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
2aa68867e5 |
U11 follow-up: usage-limit parking silently stopped covering the planning lane (#2567)
Second of the 39 audited `triage` sites from #2515's safety audit. Unlike the first, this one is **real breakage**, not a proof of safety. ## The defect The usage-limit pauser decides which tasks are on a rate-limited provider by asking, per lane, whether the card sits in that lane's column. The **planning** lane asked for the literal `triage`. Now that Todo is merged into Planning, a card being planned on the default workflow rests in `todo`. The branch resolves to an empty provider list, so the card is not recognised as using the planning provider — and is **neither parked when that provider hits its limit nor resumed when it recovers**. It runs into the limit and fails. Silent by construction: the detector reports nothing, it simply matches no tasks. ## The test caught my own first attempt at testing it The initial version asserted on the task that **triggered** the usage-limit hit — and **passed against unfixed code**, because the trigger is always parked directly without consulting `taskUsesProvider`. Only a **bystander** card reaches the lane/column branch. All three assertions now use a separate trigger, and the comment says why, because the obvious test shape is the one that proves nothing. ## Why a paired literal rather than trait resolution `taskUsesProvider` is a synchronous predicate over a task and settings, with no IR in scope and no call site that could supply one without a signature change reaching several callers. Both ids name a pre-implementation column in every built-in — `triage` for the split shape, `todo` for the merged one and for Coding (Ideas) — so the pair covers the planning lane in all of them. Flagged for U12's ratchet allowlist with that reason attached. **Over-inclusion is the safe direction and is deliberate.** On a split workflow a `todo` card is capacity-parked rather than actively planning, so it may now be parked during an outage it was not using. Parking one extra idle card is recoverable; failing to park a card whose provider is rate-limited is not. Regression direction asserted: widening the **column** match must not widen the **provider** match — a planning card on a different provider is still not parked. ## Audit progress 39 exclusive `triage` sites (from `docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md`, merged in #2515): | status | sites | |---|---| | proven safe as-is | `spec-staleness.ts` — the guard is carried by **status**, not column; the mechanical conversion was tried and is *wrong* | | confirmed safe by inspection | `mission-feature-sync.ts` (already OR-pairs), `TaskContextMenu.tsx` (already trait-paired) | | **fixed here** | `usage-limit-detector.ts` | | remaining | 35, with the owners named in the audit | Gate 309/309, lint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
94e4b7453d |
U12 part 11: CONCEPTS.md taught the legacy enum as the model — and told operators column agents need a flag that no longer exists (#2545)
## U12 part 11 — CONCEPTS.md taught the legacy enum as the model, and one entry was simply false Docs only. No changeset: AGENTS.md excludes internal docs from changesets. ### The one that is an error, not staleness `CONCEPTS.md` → **Column agent**: > Requires both the workflow-columns and graph-executor flags; with either off, bindings are inert at execution time. **That kill switch was removed.** `executor.ts` says so at the binding site: > the former workflowColumns kill switch was removed, so stale persisted false values cannot silently disable custom-node, seam, or watcher bindings So the shared vocabulary document was telling operators that a shipped feature depends on a flag that no longer gates it — and, worse, that a stale persisted `false` would disable it. Anyone debugging "why isn't my column agent running?" would have been sent to a flag that has nothing to do with it. Corrected, with the history kept in one clause so it stays legible to anyone who remembers the old behaviour. ### The one the plan named `CONCEPTS.md` → **Task** defined the entity as moving "through columns (triage, todo, in-progress, in-review, done, archived)" — teaching the legacy enum as the model, in the document whose whole job is shared vocabulary. The plan lists this entry explicitly as U12 scope. It now says a Task moves through the columns **its workflow declares**; that two Tasks on the same board may have entirely different column sets; and that the six familiar ids are the **Default workflow's choice** — which is why they saturate stored data — rather than a property of the model. ### The same class, one doc over `docs/dashboard-guide.md` restated a trait rule as an id list: "Eligible existing tasks (triage, todo, in-progress, in-review)". The implementation gates on traits — `isMutableLiveColumn` is `complete !== true && archived !== true`. Now stated as the trait rule, with the Default workflow's ids as an illustration rather than the definition, so a renamed or custom workflow reads correctly against this doc. ### Left alone deliberately The **Column (workflow-defined)** and **Default workflow** entries already describe the workflow-scoped model accurately. Their references to the legacy enum are about the Default workflow *specifically*, which is exactly where naming those ids is correct — removing them would make the docs less true, not more. This is the same judgement as declining to delete `downgradeIrToV1IfPure` earlier in the unit: legacy vocabulary describing a legacy-shaped thing is not drift. ### Verification Docs only, no code paths touched. The factual claims were checked against source rather than assumed: the kill-switch removal against `executor.ts`, and the trait rule against `TaskContextMenu.isMutableLiveColumn`. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b0b9614fd5 |
U12 part 10: pin the R7 undeclared-column sweep — the repair three earlier PRs cited had no test of its own (#2543)
## U12 part 10 — the R7 sweep everything else leans on was itself unpinned `reconcileUndeclaredTaskColumns` re-homes a card resting in a column its workflow no longer declares. It is the shipped answer to **R7**, and it is the reason several earlier U12 deletions were safe — I cited it when deleting the superseded `runWorkflowColumnsIntegrityPass` (#2500), and again when arguing that a torn workflow switch leaves *recoverable* state (#2512). Its only coverage was **incidental**: two live PostgreSQL e2e suites that exercise it in passing. A repair the rest of the unit leans on had no test of its own — a guarantee everyone cites and nobody checks, which is the exact shape this unit keeps finding. ### Six cases The plan names three scenarios for U12; those are the three ways this sweep can be wrong, plus I added the over-fire direction: - repairs the stranded card to its workflow's **own** rebound target (not a hardcoded legacy id) - leaves a **user-paused** card alone - leaves an **unresolvable-workflow** card alone - is **idempotent** — a second run does not move the card again - ignores a card already resting in a declared column - repairs one stranded card **without disturbing** healthy or paused neighbours The leave-alone cases matter more than the repair. A sweep that over-fires rewrites an operator's board, and this one runs at startup against every task. It also asserts `recoveryRehome: true` explicitly, because that flag is load-bearing rather than incidental: the stranded card's *source* column is undeclared too, so adjacency resolves to `[]` and every target is rejected without it. Its absence once made this sweep a repair that never repaired anything (#2462). ### Mechanism coverage — measured, and one case that isn't Verified by mutation rather than asserted: | mutation | result | |---|---| | delete the user-pause guard | **2 cases fail** | | delete the already-declared short-circuit | **2 cases fail** | | delete the unresolvable-workflow `continue` | still green | That last row is stated at the assertion rather than hidden. The unresolvable-workflow case pins the **outcome**, not the mechanism: every mutation I could construct — dropping the `continue`, dropping the try/catch so the throw reaches the outer handler — also ends in "no move". So it is a regression guard on observable behaviour, not proof the specific guard is reached, and I am not claiming otherwise. ### A decision I made Store double rather than PostgreSQL. The sweep's decisions are pure functions of the task list and the resolved IR, and a double makes the "did **not** move" assertions exact rather than inferred from an absence of change. It also keeps the suite off the slow lane, per the standing rule against adding slow tests. ### Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, engine typecheck green. New suite: 6 passed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added coverage for automatically restoring tasks stranded in undeclared workflow columns. * Verified paused tasks, unresolved workflows, and tasks already in valid columns remain unchanged. * Confirmed repairs are idempotent and affect only the intended task. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
88c7502eae |
test(engine): prove the recovered-lease rebound AND its audit on a renamed board (#2539)
Test-only. Sixth E2E family. Closes the `mesh-lease-manager` ledger entry. ## Two things to prove, and only one is where the card lands The conversion note records the defect precisely: > They were previously two independent `=== "todo"` comparisons that could disagree, which is how **the audit came to claim a card landed in `todo` when the workflow has no such column**. 1. the card rebounds to the renamed workflow's own rebound column 2. the unreachable-owner **audit** reports the column the card actually reached **(2) is the half that rotted silently, and it is the worse one.** The audit is what an operator reads to find out where a recovered card went. Confidently wrong is worse than absent — and on a renamed board it named a column the workflow does not even declare. ## One thing deliberately NOT renamed `decisionPath` keeps its legacy `lease-recovered-to-todo` wording. The code explains why: it is a stable discriminator that existing queries and dashboards match on, and renaming it would break them in order to describe the same decision. The column actually used travels in `newColumn`. I've pinned that split with an explicit assertion so a future vocabulary "cleanup" cannot quietly rename a field that **is not a column at all**. Stating it here so it reads as a decision rather than an oversight. ## Mutation-verified Forcing the legacy literal fails **exactly the three renamed cases**, leaving the default-vocabulary floor and the fresh-lease negative green. ## Negative half A lease renewed just now is not recoverable — "rebound anything with a checkout" would tear live work off its owner. ## Fixture guard The stale-lease seed writes lease bookkeeping through the admin client and then **asserts the seed took effect**. A silently-dropped write would make the recovery look correctly declined — the same trap that produced a vacuous paused-park test earlier in this program, so it is now guarded by default. ## Verification - six live-E2E suites green together: **58/58** - engine `tsc --noEmit` clean - `pnpm test:gate` green (414 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
30e0a8f291 |
U11 P0 audit: no hard stall in the recovery block — and one obvious fix is wrong (#2570)
Docs only. Answers the P0 question per site: **does it still fire, what silently stops happening, is there a backup?** ## Headline: no hard stall The alarming reading — *"the orphaned-planning-status sweeps stop finding default cards, so a card whose planner died sits with `status:"planning"` forever, invisible to discovery"* — **does not hold.** `triage.ts`'s `sweepStalePlanningStatuses` is the **periodic primary** for that repair and already tests `column !== "triage" && column !== "todo"`. It covers the merged column. The two self-healing sweeps perform the same repair and are **redundant nets**, not the sole rescue. That is the difference between a P0 and a cleanup, and it is only visible by reading the **backup** path rather than the broken guard. Recorded so nobody re-derives the panic. ## Self-healing block, by blast radius | site | fires? | what stops | backup | verdict | |---|---|---|---|---| | `:12106`, `:12427` | no | clearing a stale `planning` status | `triage.sweepStalePlanningStatuses` | redundant net lost — **cleanup** | | `:2961/2981/3016` `recoverAdvancedTriageTasks` | no | re-homing a card with a worktree + durable IR pin to its **pinned** resume column | hold-release still releases it on capacity (real spec ⇒ `isUnplannedForExecution` false) | **degraded, not stuck** — fix first | | `:12254` | no | a bounded priority nudge | none needed; the doc says nudge, not rescue | **low** | | `:12151`, `:9151` | **yes** | — | already OR-paired | **safe** | **Second-order trap at `:3016`.** It skips when `resumeColumn === "triage"`, guarding against resuming a card into the column it already occupies. Post-merge the pinned column is `todo`, which is **not** skipped — so pairing the literal at `:2961` *without* also pairing `:3016` produces a `todo → todo` move. **Repair the three together.** ## Two sites in the ownership split are already handled - **`usage-limit-detector.ts:126`** (assigned to u8) — already fixed in **PR #2567**. Real breakage: the planning lane stopped being recognised, so a card being planned was neither parked when its provider hit a usage limit nor resumed when it recovered. - **`spec-staleness.ts:95`** (assigned to u7) — already proven safe as-is, merged with #2515. **Its obvious fix is wrong.** I tried `|| task.column === "todo"` and it turned an existing test red: it breaks the parked-preserved-progress path. ## The generalisation, which is the most useful thing here **On the merged column, `todo` answers two different questions.** After the merge `todo` is both the planner column *and* the capacity-hold column. So any site that used `triage` to mean *"is being planned"* **cannot simply be paired with `todo`**, because `todo` also means *"is parked waiting for capacity"*. Those sites need **status or a trait**, not a wider literal. That is precisely the mistake a bulk conversion makes, and `spec-staleness.ts` is the worked example: the guard was already asking status, and widening the column would have destroyed the distinction. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a68785a41d |
P0: two silent triage guards in the executor's ownership — one strands a card with nothing to rescue it (#2572)
P0 audit of the executor's assigned `triage` sites after the
Planning-column merge. **One of them can strand a card**, so leading
with that.
## The stall — `handleDepAbortCleanup`
`executor.ts` moved a dependency-aborted task to the **literal**
`triage`. The default coding lineage no longer declares that column.
A card that gains a dependency mid-execution has its work discarded and
is then parked in a column its own workflow does not define. Nothing in
the graph routes a card out of an undeclared column. The only rescue is
`reconcileUndeclaredTaskColumns`, which runs on the **next engine
start** — so between the abort and a restart the card is stalled with no
automatic recovery. It does not throw, so it would have surfaced as a
user report, not a red test.
Fixed to `resolveReboundColumnFor`, the helper the other ~16 executor
rebounds already use.
## The silent skip — `UsageLimitPauser.taskUsesProvider`
The planning lane was identified by the same literal. For a default card
the lane resolved to **no providers**, so when a provider hit a usage
limit during a *planning* session, the fan-out that pauses peers on that
provider skipped every default-workflow card and they kept hammering the
rate-limited provider.
Not a stall: the triggering task is still paused by the explicit
fallback below the filter. What was lost is blast-radius containment. A
planning session runs while the card is pre-implementation, and the
caller has already excluded `done`/`archived`, so that is exactly "not
the implementation column and not the review column" — which matches
`todo`, `triage`, `ideas`, and a renamed planner alike.
## Full audit table for my assigned sites
| Site | (a) Still fires for a default card? | (b) What silently stops |
(c) Action |
|---|---|---|---|
| `executor.ts:16395` `moveTask(id, "triage")` | **No** — writes an
undeclared column | Card parked where nothing routes it; rescue only at
next engine start | **Fixed** — `resolveReboundColumnFor` |
| `usage-limit-detector.ts:126` `column === "triage"` | **No** |
Usage-limit fan-out skips every default card; peers keep hitting the
limited provider | **Fixed** — pre-implementation predicate |
| `executor.ts:3409` `from === "todo" \|\| from === "triage"` | **Yes**,
via the `todo` arm | — | Unchanged; `triage` arm still live for
legacy-coding |
| `executor.ts:4951` `originColumn === "todo" \|\| === "triage"` |
**Yes**, via the `todo` arm | — | Unchanged |
| `executor.ts:4963` `originColumn === "triage"` double-hop | No, and
correctly so | Nothing — the extra hop exists only for shapes that
declare `triage` | Unchanged; still required by legacy-coding |
| `executor.ts:1110` `Type.Literal("triage")` | n/a | — | **Not a
column** — an agent ROLE in `spawnAgentParams` |
Counts for my ownership: **6 sites audited, 2 defects, 2 fixed, 3
correct as-is, 1 false positive.**
## Red-green
Reverting each fix fails its own test:
```
Tests 2 failed | 2 passed (4)
× dependency-abort cleanup requeues to a DECLARED column
× usage-limit fan-out … pauses a peer card sitting in the merged Planning column (id `todo`)
```
The other two are the regression floor and pass both ways by design: a
legacy workflow that **does** declare `triage` still fans out, and an
in-progress card is still **not** swept into the planning lane (the
guard must stay narrow — "any non-wip column" would have been the easy
wrong fix).
## Verification
- New audit suite + graph-boundary + step-session + ownership ledger —
**45 tests green**
- `pnpm test:gate` green (10 / 414 / 71); `pnpm lint` clean; `tsc
--noEmit` clean
- Changeset included (`patch`, `fix`)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
ad3dc202f8 |
P0: a fresh project created every task into a column its workflow no longer declares (#2589)
Highest-severity finding of the post-merge audit, and it is the
**out-of-the-box** shape rather than an edge case.
## The defect
`createTask` resolves the intake column only as a by-product of
materializing the project's default workflow. A project that has never
**explicitly** set a default workflow has no persisted default row — so
that materialization returns nothing, `resolvedEntryColumn` stays
`undefined`, and the row falls through to the hard-coded `|| "triage"`.
Post-merge, that column does not exist in the default workflow.
Measured, three creates on one store:
| create | column |
|---|---|
| no default row persisted | **`triage`** ← broken |
| default explicitly `builtin:coding` | `todo` |
| explicit `workflowId` | `todo` |
`builtin:coding` is the **implicit** default via `DEFAULT_WORKFLOW_ID`,
and nothing writes a default-workflow row until an operator picks one.
So this was **every new task on a fresh project.**
## What it costs
Triage discovery resolves intake **by trait**, so `isAtIntakeColumn` is
false for a card sitting in `triage` while its workflow says `todo` —
**the card is never admitted for planning.** It isn't in the hold column
either, so hold-release ignores it. Only
`reconcileUndeclaredTaskColumns` eventually re-homes it.
A newly created task is invisible to planning until that sweep runs. Not
a permanent stall, but the first thing an operator does on a new project
is create a task.
## The fix — three parts, and missing any one leaves it half-fixed
1. `resolveDefaultWorkflowIntakeColumn` falls back to
`DEFAULT_WORKFLOW_ID` when no default row is persisted — the implicit
default every other resolver already assumes.
2. Both create paths consult it as a **last** resort before the literal,
so any path that already has an explicit column or a resolved entry
column is untouched.
3. **`isIntakeColumn` honours the same fallback.** Without this the card
lands in the right column but is classified *not*-intake and receives
`generateSpecifiedPrompt` instead of the bootstrap seed — and triage
admits a card only when its `PROMPT.md` reads as a seed, so it would sit
in Planning already looking "planned". FN-8587's failure mode by another
route.
`workflowId: null` ("No workflow") is excluded and asserted — there is
no workflow whose intake could be resolved, so that path keeps the
literal.
## Fixture drift, fixed with intent preserved
Seven tests asserted a created card lands in `triage`. None had their
assertion merely retargeted:
- **`move-task-if-planning`, `delete-task-if-planning`** — the mechanism
under test is the **live predicate**, not the column. Predicates and the
"advanced" column now name where the card actually rests.
- **`task-lifecycle-e2e`, `activity-log-parity`, `mission-store`** —
first-column and first-transition expectations.
- **`workflow-reconciliation-production-shape`** — the subtle one. Its
filler must occupy the **target** workflow's capped `triage` entry
column, but was created *before* the switch and so landed in the
**project default's** intake. It now names its column explicitly, which
makes the fixture independent of the project default — exactly the
coupling that let it drift.
- **`store-create-intake-column`** — the "lands in triage" guard now
names the invariant (the default workflow's *own* intake column) and
keeps a `not.toBe("triage")` so a regression back to the literal still
fails.
## Measured
Core package, against the 47-failure post-merge main baseline: **47
failed / 4413 passed — zero new failures.**
Three engine triage tests are red and are **not from this change**:
verified by stashing these edits and re-running against clean main,
where they fail identically. They arrived with #2515 and belong to the
triage-fixture owner.
Gate 414 + 10 + 71 green. 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**
* New tasks now consistently start in the default workflow’s `todo`
intake column, including fresh projects without persisted workflow
settings.
* Bootstrap `PROMPT.md` content is now created consistently for all
supported task-creation paths.
* Task movement and deletion behavior now correctly respects current
columns and avoids acting on stale task data.
* Workflow reconciliation and activity tracking now reflect the updated
default task lifecycle.
* **Tests**
* Expanded coverage for intake-column resolution, task lifecycle
transitions, stale candidates, and workflow edge cases.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
beb33b5dd1 |
P0 STALL 3: rescue cards stranded in a column their workflow no longer declares (fixes 8 red tests on main) (#2591)
Based on `main`. **Fixes STALL 3 — and it needs no data migration.** ## The stall #2515 removed `triage` from the default lineage while leaving the id legal for stored rows, and shipped **no migration**. Planning discovery resolves a card's lanes from its own workflow, and for a default card `intake` and `hold` **both** resolve to `todo` — so a card *sitting* in `triage` matched neither branch and was admitted by nothing. `triage` was the default intake column before #2515, so **every existing project has cards there.** Nothing else rescued them. #2515's escape hatch makes an undeclared source column resolve to the workflow's rebound target, but every path that *uses* it (executor, agent-heartbeat, merger) is triggered by **active work**, and a parked card has none. The card sat until an operator dragged it by hand. ## Proof this is a real regression, not a stale test **8 tests in `triage.test.ts` were RED on clean `origin/main`** — verified by swapping main's `triage.ts` into this tree and re-running. **All 8 pass with this change.** The sharpest: ``` expected "specifyTask" to be called 4 times, but got 0 times ``` Discovery was admitting zero triage cards. ## The fix A card resting on a legacy pre-implementation id that its own workflow no longer declares is **unowned by construction** — no lane's rules apply to it. Admitting it to **planning** heals it through the normal path: it gets planned, and finalize releases it to the workflow's hold column, **re-homing the row as a side effect of ordinary work**. No migration, no backfill, no operator action. ## The narrowing is the load-bearing part My first version rescued **any** undeclared column, and it was wrong. A card can also sit in a column its workflow genuinely owns while the **selection** fails to resolve — the resolved default IR then doesn't declare that column either. That version re-specified a parked Coding (Ideas) `ideas` card, breaking **FN-7596's manual-intake rule** (an ideas card is promoted by an *operator*, never auto-planned). `triage.test.ts` caught it. The rescue is now scoped to the legacy planner ids, so a workflow-specific column name is never second-guessed. That distinction — healing #2515's orphans vs. overruling a workflow about its own board — is the whole design. ## A user-pause hole this would have opened `couldBeCandidate` screens `paused` but not `userPaused`, so a row carrying `userPaused` alone slipped through. Harmless before (an undeclared-column card was admitted by nothing) and **reachable the moment admission widens**. Planning a card mutates its lifecycle state, which the ratified safeguard forbids for a user-paused card — so the guard is now explicit rather than inherited. Covered by a test and mutation-verified. ## Cost Resolution now derives roles **and** declared column ids from one `resolveWorkflowIrForTask` call, replacing `resolveTaskLifecycleColumns`. Same call, same `irCache`, same bounded concurrency window — **cost unchanged**, no added read. ## Verification - **Mutation-verified three ways**, each failing a different test: remove the rescue; widen it back to any undeclared column; drop the user-pause guard - 8 previously-red-on-main tests now green - 263 triage/scheduler tests green, merge gate green (482 + 10 + 71), tsc clean, lint clean ## What this does NOT do It does not re-home rows that are past the planning stage. Admission still requires `isTaskStillInPlanningStage`, so a card that advanced past planning in an undeclared column stays with self-healing's advanced-recovery sweep rather than being re-specified here. If such rows exist and are also stranded, that is a separate sweep and a separate PR. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
|
a56253f426 |
P0: plan approve/reject rejects EVERY card on a merged planning column — operator-visible stall, cannot approve or reject (#2571)
## P0 — plan approve/reject is dead for cards on a merged planning column **This is the "card stuck with nothing to rescue it" case you asked to hear about immediately.** Found auditing my files after #2515. ### What happens #2515 removed `triage` from the merged default lineage — one pre-implementation column, id `todo`, displayed "Planning". Four routes guard with: ```ts if (task.column !== "triage") throw badRequest("Task must be in 'triage' column ...") ``` On a workflow with no `triage` column that condition is **true for every card**, so the routes reject all of them: | route | effect on a merged-lineage card | |---|---| | `POST /tasks/:id/approve-plan` | 400 — **cannot approve** | | `POST /tasks/:id/reject-plan` | 400 — **cannot reject** | | `task_refine` route (×2) | 400 — refine blocked | A card parked `awaiting-approval` can be **neither approved nor rejected**. It is stuck, the operator is being asked for a decision they have no way to give, and nothing throws to reveal it. ### Why it is the worst variant of this drift Everything we have chased so far is a guard that silently **stops** firing. This is a guard that silently starts firing on **everything** — same root cause, opposite symptom, and worse, because the failure is visible to the operator as a task that demands an answer and refuses every one. ### The fix, and a deliberate choice The guards resolve the workflow's own intake column through the existing `resolveIntakeColumnForTask`, and they **widen rather than replace**: a card is accepted if it is in the resolved intake column **or** in `triage`. That is on purpose for a P0. The fix cannot reject anything the route previously allowed, so it carries no regression risk of its own. Narrowing to the resolved column alone is a follow-up once the legacy id is gone everywhere — not something to do under time pressure on a route that gates operator decisions. I found the value of that when a strict replacement broke 3 pre-existing tests in `stranded-refinements-routes.test.ts`. The widened form passes all of them **and** the new P0 cases. ### The convergence number goes UP, and I am not hiding it Live-code `column === / !== "todo" | "triage"` in `register-task-workflow-routes.ts`: **10 → 11**. Each converted guard keeps the legacy id as an explicit second condition, so a widened guard has two literals where it had one. The metric counts id literals; it does not know the guard is now strictly more correct. Reporting the direction that is true rather than the one that looks better — and flagging that this file's number will only fall once the widening can be removed. ### Revert-proof Restore either bare literal and the matching case fails with **400 where 200 is expected**, on a `todo` card with `awaiting-approval`. A third case pins that the guard still **narrows** — an `in-progress` card is still rejected — so this cannot be mistaken for deleting the check. ### Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, dashboard typecheck green. New suite 3 passed; `stranded-refinements-routes.test.ts` back to 5 passed (it was 3 failed under the strict form). ### Still auditing `TaskDetailModal.tsx` conversion is in flight on a separate branch. `TaskCard.tsx` (#2558) and `ListView.tsx` + `taskActivity.ts` (#2566) are already open — and note #2566 covers `isTaskAgentActive`, whose planner-lane clause has the *silent* version of this same bug: planning cards read as idle everywhere at once. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3c46ecca14 |
Drift (unowned): the planner-activity signal was never written after #2515 — the three badge conversions were reading an empty field (#2594)
## Drift, unowned: the planner-activity signal was never being written **Stacks on #2577.** Merge order: #2566 → #2577 → this. ### This is what made the other three PRs cosmetic `addRecentPlannerActivityForFreshAgentLog` in `useTasks.ts` stamped `recentAgentActivityAt` **only for cards literally in `triage`**. #2515 removed that column from the default lineage, so after that merge the stamp never happened for a default-workflow card. Every consumer downstream then had **no data to act on**, however correctly it resolved its own column traits: - the pulsing Planning badge (TaskCard, ListView) - the agent-active row border - the column header's executing count So #2558 / #2566 / #2577 convert the *readers* of a field that nothing was *writing*. They ask the right question of an empty value. This is the fix that gives them something to read — and it was on nobody's drift list. I found it chasing why a badge test would not go green. ### The decision, and why I did not thread metadata here The hook processes SSE and has no resolved column metadata. The lane is matched by id against **both** shapes — pre-merge `triage`, post-merge `todo`. Over-stamping a legacy hold-lane card is harmless: every consumer additionally requires the column to be an **intake** lane before rendering anything, so the extra timestamps are filtered downstream. Threading board context into this hook to avoid a harmless over-stamp would be a much larger change for no behavioural gain, so I widened instead and wrote the reasoning at the site. ### Also converted `Column.tsx`'s move-progress prompt. Unlike the same prompt in TaskCard/ListView/TaskDetailModal, this component's `column` **is** the drop target, so its own `columnFlags` are the target's traits — no lookup needed. Worth noting because the same-looking regex meant three different things across four files, which is exactly why these were converted one at a time. ### Revert-proof Restore `task.column !== "triage"` and the merged-column case fails: `expected undefined to be '2026-07-28T12:00:01.000Z'` — nothing stamped, badge has nothing to render. A companion case pins that the stamp still **narrows**: an `in-progress` card is not planner activity. ### Verification `pnpm test:gate` (482 + 10 + 71), `pnpm lint`, dashboard typecheck green. `useTasks.test.ts` + `Board.test.tsx`: 210 passed. ### Audited and deliberately left | site | verdict | |---|---| | `Column.tsx:550` `workflowMode \|\| column === "triage"` | Dead in practice — `workflowMode` is true whenever lanes resolve, so the disjunct only matters with no metadata at all. Not worth a change. | | `taskSorting.ts:73` `column === "todo"` | Still correct for the default (the merged column keeps that id); wrong only for a renamed workflow's hold lane. Needs the sort to take flags — a wider signature change than this PR's scope, and cosmetic (ordering) rather than a lost affordance. | | `worktreeGrouping.ts:77` | Same shape as above; grouping only, no lost control. | Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cf7b1a3d46 |
Drift review (unowned): gridlock detection + autopilot retries resolve the hold column — main 103→101 (#2561)
> **Based on `main`, not on my U7 stack** — merges in any order, no dependency on #2517. My assigned files (`triage.ts`, `replan-target.ts`) are at zero, so this picks up two lifecycle-column literals **no unit's file list claims**. Both ask *"is this card in the hold column?"* by the id `todo`, and both are broken **today** for any workflow that renamed it. ## gridlock-detector — the worse of the two `column !== "todo"` decides which cards count as **schedulable**, and an empty schedulable set is an **early return**. On a renamed board the detector concluded *"no gridlock"* at exactly the moment a real one would be visible. > A detector that goes quiet on the boards it cannot parse is worse than one that is absent, because its silence reads as health. **Converting only the `todo` half would have shipped a still-broken detector**, and the test caught it. The `active` filter is equally literal (`in-progress` / `in-review`) — and an empty active set is *also* an early return. Two literals, one silence. The `in-progress` half sits **outside the drift review's `todo|triage` pattern**, which is precisely why a count-driven sweep would have left it behind and declared the file done. Converted here rather than deferred as out of scope. Worth flagging to the other workers: the convergence metric is a good *tracker* but a bad *definition of done* — an adjacent literal in the same predicate can preserve the whole bug at a lower score. ## mission-autopilot The retry compared against `todo` **and moved to the literal `todo`** — so on a renamed workflow it relocated the card into a column the workflow may not declare (R7) on **every retry**. Now resolves the hold role; when the workflow declares none it leaves the card in place and says so, because the error/status clear still runs, so the retry is not lost — the card just stays in its own lane. ## Two fixture defects of my own, both caught by the tests failing wrongly **My first autopilot tests re-implemented the decision** and asserted on the copy — proving only that the copy works. That is the anti-pattern named in `docs/solutions/store-fake-defects-that-masquerade-as-production-bugs.md` (#2534) and in the #2527 ratchet review, and I had no excuse: the constructor takes two stores and `handleTaskFailure` is public. Rewritten to drive the real method. **My first gridlock fixture failed on both vocabularies** — the detector needs three preconditions and I supplied one. A test that fails on its *no-regression* half is a broken fixture, not a discovered bug. The "both halves failed" heuristic from that same doc is what flagged it. That is eight fixture defects across this unit, every one caught by reading *why* a test failed rather than making it pass. ## Revert proofs, each isolated to one literal | Restored | Result | |---|---| | gridlock hold filter | **1 of 5 fails** (renamed case) | | autopilot move target | **1 of 5 fails** (renamed case) | Default-vocabulary halves pass either way — the correct signature for conversions that change no existing behavior. ## Convergence Measured against `origin/main` with a comment-stripped scan of `column === / !== "todo" | "triage"` in `packages/*/src`, excluding tests: **103 → 101.** (The gridlock `active` filter is a third site fixed here that this pattern does not count.) ## Verification | Check | Result | |---|---| | new suite | 5/5 | | pre-existing gridlock + autopilot suites | 85/85, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (414 + 10 + 71) | | `pnpm check:changesets` | clean | ## Still unowned after this `mission-feature-sync.ts` (1: a planning-lane check) and `auto-claim-snapshot.ts` (1: `isRunnableAutoClaimCandidate`, a **pure sync** predicate that needs the injected-lane pattern from #2551, not a resolve). `notification-service.ts` has one more with a different semantic — *"has progressed past"* — which needs its own thinking rather than a mechanical swap. I will take these next unless someone claims them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
bbaa254dc3 |
test: add the missing debug to 27 logger mocks (206 → 4 failures) (#2573)
**Test-infrastructure fix.** 29 test files. No production code, no altered assertions, no widened timeouts. Now **3 commits** (#2584 merged into this branch): the logger-mock sweep, a cron-runner follow-up from review, and the 4 residual failures the sweep deliberately deferred. **Whole branch: 764 tests, 0 failures** across the touched set. --- ## Commit 1 — the missing `debug` on 27 logger mocks `createLogger`'s real shape is `{ log, debug, warn, error }`. 27 engine test files mock `../logger.js` with logger-shaped literals that **omit `debug`**, so any production path reaching `log.debug` threw: ``` TypeError: schedulerLog.debug is not a function TypeError: runtimeLog.debug is not a function TypeError: log.debug is not a function (SelfHealingManager.start) ``` Measured, same commit, same 27 files: | | Failed | Passed | |---|---|---| | before | **206** | 558 | | after | **4** | 760 | **202 failures fixed by one missing mock export.** Per-file: `notifier` 36→0, `plugin-runner` 56→0, `grok-runtime-routing` 14→0, `self-healing-completion-fanout` 1→0. That last one also leaked an unhandled rejection out of `startMaintenance`, which vitest warns "might cause false positive tests" elsewhere in the file. *A note on the number:* a full `engine-default` run went 283 → 106 across my two sessions, but `main` moved in between (U11 landed), so that spread is **not** attributable here. 206 → 4 is the honest figure: same commit, same file set, only this diff varying. ## Commit 2 — cron-runner's factory (greptile P1) My regex required `log: vi.fn()`; `cron-runner.test.ts` uses `log: cronLoggerSpies.log`, so the `createLogger` factory's returned literal never matched and the logger production received still lacked `debug`. **Measured before claiming a live fix, and the numbers don't support that part:** `cronLoggerSpies.debug.mock.calls.length` is **0** across all 155 tests, and the suite is 155 passed both before and after. The described failure mode — `tick()` hitting `log.debug`, throwing, and being swallowed by its own error handler — is **not reachable today**, because no test exercises those three branches (`cron-runner.ts:377`, `:385`, `:410`). The fix is defensive, not curative. The real gap it surfaced is **missing coverage** for schedule dedupe / scope mismatch / lost atomic claim, which I did not write blind to close a thread. ## Commit 3 — the 4 residuals **`notification-service` (3):** messages moved to DEBUG in production (`:580`, `:846`) while tests asserted `schedulerLog.log`. The token case needed more than a relocation. It asserted `expect(schedulerLog.log).not.toHaveBeenCalledWith(containing("new-token"))`. Moving only the *positive* assertion to `debug` would leave the secrecy check watching a channel the message no longer uses — a token could leak through `debug` and the test would still pass. The negative now runs across all four channels. **Verified it bites:** interpolating the token into the debug line fails the test. **`openclaw-runtime-integration` (1):** `../pi.js` mock missing `wrapToolsWithOutputBudget` (same class as #2547); this suite exercises a non-pi runtime, exactly where that wrapper applies. **Not swept repo-wide, and the measurement is why.** 37 `pi.js` mocks omit that export. Patching 30 moved the set from **11 failed to 10** — thirty files of churn for one test. Reverted. Commit 1 earned its 27-file diff with 202 fixes; this one earned nothing, and a no-op sweep is just future merge conflicts for other workers on this program. --- ## Why none of this is appeasement AGENTS.md forbids making a red test pass by loosening it. This does the opposite: the mocks were **wrong** — they claimed to stand in for `createLogger` while missing part of its interface. Nothing was relaxed; stubs were completed, and the one assertion I did move got **stronger** (four channels instead of one). ## Also deliberately not done Extending `scripts/check-mock-completeness.mjs` to catch this class. Measured first: a naive rule over relative intra-package mocks flags **147** factories of which **146 are green** — almost pure false positives. The barrel heuristic works because `cliSrc` gives a tight import surface; that doesn't transfer. A gate that noisy gets ignored, which is worse than no gate. ## How this was found While characterizing U9's review lane. These files were pre-existing baseline noise under mutation runs — and that noise is exactly what made my own safeguard baseline (#2511, corrected in #2520) report two false verdicts. **A red suite does not merely lack coverage; it makes every nearby measurement untrustworthy.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d2ce1ba8b5 |
U11: resolve the scheduler's event-handler columns by trait (10 live sites, sync resolution) (#2518)
Based on `main`. Ten live `"todo"` sites in `scheduler.ts` now resolve
the column by trait.
## Four groups, converted together
They fail **independently**, and a half-conversion is indistinguishable
from a working system:
| group | sites | failure mode |
|---|---:|---|
| **Wake triggers** | 4 | **Latency** — snapshot invalidation,
mission-failure tracking, engine requeue tracking, move-to-backlog wake.
The wake doesn't fire and the card waits up to a poll interval. Exactly
why it would go unnoticed indefinitely. |
| **Parked wakes** | 2 | Latency — unpause and planning-finished, keyed
on hold OR intake. |
| **Dependency** | 3 | **Not latency.** After a blocker completes or is
soft-deleted, the query returns nothing, so the dependent is *never*
unblocked and waits on a blocker that already finished. |
| **Agent link** | 1 | `rollbackRunningAgentsForQueuedTodoTask` passes a
synthetic `{ column: "todo" }`. Wrong here **drops a running agent's
task link** — the worse direction of that safeguard. Resolved
`parkedColumns` is now passed through too, rather than letting the
helper fall back to its legacy default. |
## Resolution is synchronous, deliberately — the part worth reading
My first cut used the async resolver and made the `task:updated`
listener `async` to suit it. **That broke 5 pre-existing tests, and the
tests were right:** introducing a new `await` *before* a listener's
existing synchronous work defers everything after it to a microtask and
reorders handlers relative to a synchronous emitter.
A conversion must not change event ordering. It now uses the store's
sync IR path (`resolveTaskWorkflowIrSync`), so **no new suspension point
is introduced anywhere**.
That's the fifth time in this program a change that looked like a move
quietly altered behavior — and the first time the existing suite caught
it before review.
## Verification
- **Mutation-verified:** forcing the resolver back to the literals fails
**4 of the 6** new tests
- 110 tests green across all 8 scheduler suites (6 new)
- Fail-soft to the legacy pair: an unresolvable workflow behaves exactly
as before rather than losing the wake
- merge gate green (309 + 10 + 71), tsc clean, lint clean
## Measured
10 of my unit's 68 remaining code sites converted.
`scheduler.ts` now has **one** `"todo"` literal left in live code:
`isRunnableQueuedOverlapCandidate`, which is **exported but has no
production caller** — its only consumer was the legacy dispatcher
deleted in #2505. That's a **deletion, not a conversion**, so it is
deliberately not in this PR.
No changeset: `@fusion/engine` is private.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Scheduling now correctly recognizes workflow-specific hold and intake
columns, including renamed columns.
* Tasks entering a hold column reliably trigger scheduling and wake-up
behavior.
* Dependency recovery now finds blocked tasks in renamed hold columns.
* Planning, unpausing, task completion, deletion, and requeue flows now
respect each workflow’s configured parked columns.
* Prevented unnecessary scheduling for moves between unrelated workflow
columns.
* **Tests**
* Added coverage for renamed hold-column scheduling, wake-up, and
dependency-unblocking scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
fb7ab6df26 |
test: re-green self-healing, worktree-pool and DB-corruption assertions (#2592)
**Test-only.** Three files, two commits. No production changes. | File | Before | After | |---|---|---| | `self-healing-db-corruption` | 5 failed / 1 passed | **6 passed** | | `self-healing` | 1 failed / 411 passed | **412 passed** | | `worktree-pool` | 2 failed / 57 passed | **59 passed** | All three are the same underlying story in different costumes: **the assertion is watching a channel production stopped using**, or a step that aborts before it can log at all. ## Commit 1 — the fake store was missing the health refreshers `surfaceDbCorruption` *refreshes* health before reading the snapshot (`FNXC:IncompletePgPorts 2026-07-26-20:45`, so PG connectivity is re-checked instead of trusting an always-healthy sentinel). The fake carried **neither** refresher, so the async branch fell through to `this.store.refreshDatabaseHealth()` — undefined — and the step threw before reaching dispatch. **Every assertion in the file was measuring zero calls against a step that had already aborted.** Both stubs are **no-ops on purpose.** Production ignores the refresh return and reads `getDatabaseHealth()` immediately after, so the snapshot mock stays the single source of truth. My first attempt delegated them to `getDatabaseHealth`, which consumed a *second* value per pass from the test that queues three `mockReturnValueOnce` snapshots (one per `runMaintenance`) and broke its corruption → clear → corruption ordering. Faithful beats convenient. ## Commit 2 — two more debug-level assertions - **`self-healing`**: `"auto-archive: archived …"` is emitted at DEBUG (`self-healing.ts:2747`); the test asserted `.log`. The mock already had `debug` (from #2573), so only the target was stale. - **`worktree-pool`**: both checkout-failure cases assert on `console.error`, which is *correct* — `createLogger`'s `debug` writes there. But debug is **gated on `FUSION_DEBUG`** (`logger.ts:43`), unset under vitest, so the line was never emitted. One test is literally named *"logs checkout -- failure at debug level"* while asserting a channel debug could not reach. Fixed by enabling `FUSION_DEBUG="worktree-pool"` for the suite and deleting it in `afterEach` so the flag can't leak into sibling files. **Deliberately not** fixed by re-pointing the assertions at another channel — that describes whatever the code happens to do rather than the behavior the test names. ## Verified each actually guards A test that merely stops failing can still assert nothing, so every fix was mutation-checked: | Mutation | NEW failures | |---|---| | `surfaceDbCorruption` returns early | **5** | | remove the auto-archive debug line | **1** — that test, only it | | remove the checkout-failure debug line | **2** — both cases, only them | ## Known residual, stated rather than hidden `self-healing-db-corruption` **still exits non-zero** with 9 unhandled `this.store.listTasks is not a function` rejections from `openSurfacingCycle` (`self-healing.ts:7737`). These **predate this change** — identical count before and after. The maintenance pass opens one shared surfacing cycle up front, independent of which steps `stubMaintenance` stubs. I tried to clear them and backed it out, twice: - adding `listTasks: async () => []` lets the cycle open, but then *other* unstubbed sweeps run for real — an orphaned-planning-segment audit fires and breaks 3 assertions expecting `recordRunAuditEvent` never to be called; - stubbing the four `surface-*` siblings didn't help either, because the cycle is opened by the **pass**, not by the steps. Making that file honestly green needs a fake complete enough for the whole maintenance registry — a bigger change than the bug in front of me, and one that would bury the fix above. Flagging it rather than shipping a half-sweep. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
21497b23db |
P0: approved plans never released after #2515 + triage.ts 11 -> 0 (#2549)
Rebased onto post-#2515 `main`. **This PR is the fix for a P0 stall**, not just a conversion. ## Stall 1 — approved plans were never released `recoverApprovedTask` opened with a bare `task.column !== "triage"`. #2515 merged Todo into Planning on the default lineage, so every default card now sits in `todo` and **this guard rejected all of them**. An approved plan whose finalize was interrupted was never released, and nothing else owns that card. Callers: `triage.ts:1296` (stuck-kill recovery) and `in-process-runtime.ts:1460`. `triage` stayed a legal id, so nothing threw — the guard just stopped matching. I verified the fix **mechanism** rather than assuming it. `resolveLifecycleColumns` on the IR #2515 actually shipped returns: ``` { intake: "todo", hold: "todo", wip: "in-progress", review: "in-review", complete: "done", archived: "archived" } ``` so the converted guard admits default cards. The new regression test asserts the **return value**, because on a merged lineage the card is already where the release would send it — "no move issued" is what *both* the broken and the fixed code do, so only the outcome discriminates. **Mutation-verified:** restoring the literal `!== "triage"` fails 2 of 5 tests. ## A defect of my own, found while auditing — same shape as the P0 `clearStaleSpecifyingStatuses` is a board-wide startup sweep with no single task to resolve lanes against, and I had resolved **both** its queries from the default workflow. Post-#2515 that workflow's `intake` and `hold` are the **same** column, so both queries collapsed onto `todo` and **nothing ever swept `triage`**. A legacy or Coding (Ideas) card holding a stale `planning` status would then occupy a planning admission slot permanently — exactly the failure the 2026-07-04 note above that function warns about. Now queries the **union** of the legacy planner ids and the resolved lanes, deduped by task id. Querying extra columns is free here: the sweep only reads, and every row is filtered on `status === "planning"` before anything is written. Caught by `triage.test.ts`, **not by my own tests** — worth recording, since it is the same collapse the P0 is about. ## Rebase note The discovery conflict was resolved **in favour of `main`**. Main's version is strictly better than mine: it resolves lanes with the **async** `resolveTaskLifecycleColumns` (so it is not subject to the sync-resolver limitation below), keeps the two admission branches disjoint for a merged column, and bounds concurrency. My sync version was dropped. ## Measured | file | comparisons before | after | |---|---:|---:| | `packages/engine/src/triage.ts` | **11** | **0** | ## Known red, NOT from this PR 8 tests in `triage.test.ts` fail on **clean `origin/main`** — confirmed by swapping main's `triage.ts` into this tree and re-running (same 8). They are reporting the upgrade stall, not stale expectations: a card *sitting* in `triage` is admitted by nothing after #2515 (`expected "specifyTask" to be called 4 times, but got 0 times`), and #2515 shipped no data migration re-homing those rows. Left untouched here — the fix is a data migration, not a conversion. Reported to the coordinator separately. ## Verification - merge gate green (414 + 10 + 71), tsc clean, lint clean - mutation-verified as above ## Separate finding — affects every worker `resolveTaskWorkflowIrSync` **cannot resolve a task's selection in production.** `getTaskWorkflowSelectionImpl` is `return undefined` unconditionally and `getTaskWorkflowSelectionAsyncImpl` is *"always PostgreSQL path"*, so the sync resolver **always** returns the DEFAULT workflow IR. `moves.ts` already hit this and fixed it by going async. Consequence for `resolvePlannerLanes` here: correct for default-lineage cards (the default IR is exactly what comes back — which is why Stall 1 is genuinely fixed) and **inert for custom workflows**. Not papered over; the async path is main's discovery code, and converting the remaining event-listener sites needs the handler-reordering problem solved first. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## P0 audit table — every `triage` site in my assigned files (a) does it still fire for a default-workflow card after #2515? (b) if not, what silently stops happening? (c) fix. | site | (a) still fires? | (b) what silently stops | (c) disposition | |---|---|---|---| | `triage.ts:613` wake handler | **yes** | — OR-shaped (`todo \|\| triage`), still matches | converted anyway | | `triage.ts:651` evacuation guard | **yes** | — OR-shaped, still matches | converted anyway | | `triage.ts:741` stale-planning sweep | **yes** | — OR-shaped, still matches | converted anyway | | `triage.ts:1088` `recoverApprovedTask` | **NO** | **STALL 1** — approved plan never released; nothing else owns the card | **fixed + regression test + mutation-verified** | | `triage.ts:1396` advanced-recovery discovery | **NO** | that recovery never matches a default card | fixed by the same conversion | | `clearStaleSpecifyingStatuses` (mine) | **NO** | **my own defect** — both queries collapsed onto `todo`, `triage` never swept; stale `planning` holds an admission slot forever | **fixed** (union of legacy + resolved lanes) | | `replan-target.ts:177` / `:185` | **NO** | **STALL 2** — see #2552 | fixed in #2552 | | `spec-staleness.ts:95` | **NO** | narrow: a Planning card with null status and `currentStep > 0` now skips staleness where it previously did not | **recorded, not fixed** — see below | | discovery (`isAtIntakeColumn`) | **NO** | **STALL 3** — a card *sitting* in `triage` is admitted by nothing | **reported, not fixed** — needs a data migration | **Why `spec-staleness.ts:95` is not fixed here.** The guard already returns `false` for `status === "planning"` and `needs-replan`, so an *actively* planning card is still covered by status. The `column === "triage"` arm only added coverage for a planner-lane card with **no** status — and post-merge that case is genuinely ambiguous, because `todo` is now both the planning lane and the hold lane, so a card with progress there may legitimately be a released card that *should* skip. Guessing either way is a behaviour change without evidence, so I recorded it rather than picking one. |
||
|
|
d1cbb8ce90 |
U11: rank assigned work by lifecycle role (1 -> 0), plus two documented non-conversions (#2563)
Based on `main`. Continuing with unassigned work in my area (scheduling/ranking core). ## Measured (drift-review tracking) | file | comparisons before | after | |---|---:|---:| | `packages/core/src/assigned-task-ranking.ts` | **1** | **0** | ## What was wrong `tierForTask` identified the two **actionable** tiers by literal id — `in-progress` → `in_progress`, `todo` → `ready_todo` / `partial_blocked`. The file's own comment already recorded half of this: > Only treating default `todo`/`in-progress` as titled hid assigned work as a bare count But the fix that followed was a **floor, not a fix**: unrecognised columns fall to `other` so work stays *visible*, while a renamed hold column loses `ready_todo` and `partial_blocked` entirely. Work that is genuinely ready to start then ranks **below everything already in progress**, so an agent reading its Wake Delta sees ready work buried. Nothing errors and nothing disappears — the ordering is just wrong, which is how it survived a comment that noticed the adjacent problem. `partial_blocked` is the sharper loss: it's the **only** tier distinguishing "ready" from "waiting on a dependency" for hold-column cards, and it was unreachable for any renamed workflow. ## Two sibling files deliberately NOT converted Checked before assuming work existed: **`live-agent-count.ts` — already trait-driven.** Its literals are the else-branch of `flags ? traits : literals`, and the source says why: *"The literal fallback is fixture-only; board/store callers always supply flags/IR."* Converting a fixture-only fallback would be churn. **`task-priority.ts` → `sortTasksForDisplayColumn` — dead.** No production caller. The dashboard has its own independent implementation in `app/components/taskSorting.ts` with a richer signature (`doneSortMode`, `isArchivedColumn`), and that's the one `Lane.tsx` imports. Core's copy is reached only by its own tests and the barrel export. That's the **third dead export** this unit has found by checking reachability before converting (after the legacy dispatcher and `isRunnableQueuedOverlapCandidate`). Deletion is a separate concern from conversion and is not in this PR. ## Verification - **Mutation-verified:** not threading `roles` through to `tierForTask` fails **4 of 6** new tests - 13 tests green (6 new + the pre-existing ranking suite) - merge gate green (414 + 10 + 71), tsc clean, lint clean No changeset: `@fusion/core` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
|
bad35775a1 |
Drift 3/4: Task Detail intake affordances from traits — the UI half of the #2571 approve/reject stall (4→3) (#2577)
## Drift conversion 3 of 4 — Task Detail, the UI half of the #2571 stall **Stacks on #2566.** Merge order: #2558 → #2566 → this. (#2571 is the P0 and is independent — merge it first regardless.) ### Convergence number Live-code `column === / !== "todo" | "triage"` in `TaskDetailModal.tsx`: **4 → 3** All three survivors are the documented no-metadata fallback, same shape as TaskCard and ListView: `workflowMoveMetadata` is `null` until the detail payload resolves, and a bare trait read would drop these controls during that window. ### This is the UI half of the P0 `isAwaitingApproval` and the standalone Delete button were both gated on `task.column === "triage"`. On the merged lineage (#2515) that is false for every card, so a task parked `awaiting-approval` **loses its Approve/Reject controls in the one surface that shows them**. #2571 fixes the routes that *reject* those actions. This fixes the UI that stops *offering* them. Either half alone leaves the operator stuck — one with buttons that 400, the other with no buttons at all. ### Three conversions | site | was | now | |---|---|---| | `isAwaitingApproval` + standalone Delete | `column === "triage"` | resolved column's `intake` | | `requiresExecutionModeReplan` | `todo \|\| in-progress` | `hold \|\| countsTowardWip` | | move-progress prompt | source column ids | **target** column's flags | The replan rule is "this card may already hold a plan or a live execution context" — which the traits state directly; `todo`/`in-progress` was the Default workflow's spelling of it. The move prompt is the mistake I made first in TaskCard, where its regression test caught that the site tests the move **destination**, not the card. Carried the lesson here rather than repeating it. ### Tested through a pure seam, and why `requiresExecutionModeReplanForTest` is exported so the rule can be asserted as a function of (column id, flags). Asserting it through the modal means booting async detail loading to observe one boolean — and an earlier DOM-level attempt at exactly this class of assertion (in #2566, ListView) **passed with the conversion reverted**, because the text it matched also appears in a column header. I am not repeating that. A seam discriminates; that DOM test did not. Revert-proof: restore `column === "todo" || column === "in-progress"` and the merged-column case fails, because that column is `intake + hold` and carries no `countsTowardWip`. The suite also pins that the rule still **narrows** (a complete lane needs no replan) and that the legacy fallback is unchanged when flags are absent. ### Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, dashboard typecheck green. **No new failures**: `TaskDetailModal.rendering.test.tsx` reports the same 28 pre-existing failures with and without this change, diffed by test *name* against a stashed clean tree. ### Drift set status | file | before | after | PR | |---|---|---|---| | `TaskCard.tsx` | 8 | 3 | #2558 | | `ListView.tsx` | 5 | 3 | #2566 | | `taskActivity.ts` (found underneath) | 1 | 1 | #2566 | | `TaskDetailModal.tsx` | 4 | 3 | this | | `register-task-workflow-routes.ts` | 10 | 11 | #2571 (P0, widened on purpose) | Survivors are no-metadata fallbacks except the routes, where the guards deliberately accept resolved-intake **or** `triage` so a P0 fix cannot reject anything previously allowed. Those retire together once the legacy id is gone board-wide. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c92bce2f8c |
test: delete 2 project-engine-manager tests for the deleted cross-project cap (#2575)
**Test-only.** One file, 2 obsolete tests + 1 dead import removed. No production change. `project-engine-manager.test.ts` has been **red on main: 2 failed / 44 passed** → now **44 passed**. ## The failures Both threw `TypeError: Cannot read properties of undefined (reading 'acquire')`, because both reach `(manager as any).globalSemaphore` — a private field that no longer exists. `project-engine-manager.ts:88` records why (`FNXC:CapacityModel 2026-07-28-20:10`, *"drop the cross-project cap"*): > The shared cross-project semaphore, its mutable limit and the `concurrency:changed` subscription are **DELETED**. Capacity is two numbers per project; a machine-wide cap was a third limiter with its own separate authority (a central-DB singleton row), and reconciling it against the per-project gates is exactly the multi-limiter arbitration this simplification removes. So both tests assert residual-slot accounting on a shared pool that was **deliberately** removed — not a regression. ## Why deleted rather than repaired There is no shared semaphore left for them to describe. Reconstructing one inside the test would assert a capacity model the engine no longer has — a test that passes while describing fiction, which is worse than the red it replaces. Also drops the now-dead `ScopedAgentSemaphore` import (these were its only uses). Lint does not flag unused imports here, so it would otherwise have sat as quiet dead code. ## What I did NOT take, and why `workflow-graph-optional-step-fix.test.ts` — the other red file adjacent to this lane, 5 failures. Its failures are **U11 column-vocabulary drift**: the replan rebound now resolves to `todo` where the test expects `triage`, and one case gets a hard-cancel pause-abort log instead of the Plan Review replan message. That is the U11/U12 owner's semantics to settle. Picking whichever column makes the assertion pass could silently encode the wrong lifecycle target — and per the graph-entry contract doc, a rebound landing in a column the workflow does not declare is precisely the failure mode that "does not fail a test; it disables a recovery path in production." Flagging it rather than guessing. ## Running tally of this cleanup thread | File | Before | After | |---|---|---| | 27 logger mocks (#2573) | 206 failed | 4 failed | | `merge-error-recovery` (#2559) | 10 failed | 0 | | `reviewer` (#2547) | 2 failed | 0 | | `project-engine-manager` (this) | 2 failed | 0 | Every one was a test describing behavior that had moved or been deleted, or a mock that had drifted from its real shape — none was a product defect. That pattern is worth naming: on this repo a red non-blocking suite has mostly meant *stale tests*, which is exactly what makes it easy to ignore, and exactly why it silently corrupted my own safeguard measurements in #2511. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1c9f6c546b |
P0: default Planning cards read as ADVANCED after #2515 (FN-8596 stranding re-opened) + 6 red tests repaired (#2552)
Rebased onto `main` (post-#2515) and **upgraded from a conversion to a P0 stall fix**. ## What changed since review Greptile's P1 on this PR said the `plannerColumn` seam was unused — *"every current production caller omits `plannerColumn`, so this default still compares against `triage`."* That was correct, and **#2515 turned it from an unused seam into a live stall.** ## The stall #2515 merged Todo into Planning on the default lineage (one pre-implementation column, id `todo`, display "Planning"). `triage` stayed a legal id, so nothing throws — the bare `column === "triage"` guards in `hasAdvancedPastPlanning` just **stopped matching for default-workflow cards**. A default card in `todo`, status cleared to null by the stale-status sweep, carrying execution stamps from a previous pass, now returns **ADVANCED**. So `isTaskStillInPlanningStage` is false and nine guarded call sites refuse planning updates, finalize, delete and handoff: `triage.ts` 3108 / 3117 / 3160 / 3438 / 3937 — `self-healing.ts` 12126 / 12448 / 12454 The file's own FNXC note at `:150` already records what that costs: > Nobody owned the card and it sat indefinitely. This is that same FN-8596 stranding, re-opened by the column merge. Confirmed empirically — the rescue test fails on pre-fix code. ## The fix is an asymmetry, and that's the point The two guards are **not the same rule**: 1. the FN-8596 **arrival-order rescue** — a stamp predating arrival in the planner lane means replanning, not advancement 2. **"the planner column itself is never advanced"** Rule 1 must recognise the merged Planning column. **Rule 2 must not** — on the merged lineage `todo` is *also* the released/hold lane, so making it blanket "not advanced" would strand the release path instead: a released card with steps would read as still-planning and `hasAdvancedPastPlanning(t) || releasedToTodo` would stop distinguishing anything. Rule 1 is already gated on the stamp predating arrival, so a released card later claimed by execution keeps its newer stamp and still reads as advanced. **Closed via the default** (`mergedPlanningColumn = "todo"`) rather than by wiring call sites — the stall closes everywhere at once, with no call-site change and nothing to collide with another worker's slice. Dedicated-planner workflows (Coding (Ideas), and every workflow still declaring `triage`) are byte-identical. ## Second commit: 6 tests left RED on main by #2515 Verified pre-existing by stashing every local change and re-running — same 6 failures on a clean branch. `resolveReplanTargetColumn` reads the IR rather than a literal, so it **self-healed** to the correct post-merge answer (`todo`); the expectations were the stale half. Updated to the post-merge truth, not loosened — each still pins one exact column. ## Audit table for this file (P0 sweep) | site | still fires for a default card? | what silently stopped | disposition | |---|---|---|---| | `replan-target.ts:177` `inPlannerLane` | **NO** | FN-8596 rescue — planning writes no-op, card strands | **fixed** (rule 1) | | `replan-target.ts:185` never-advanced | NO | nothing — must stay dedicated-planner-only | **deliberately unchanged** (rule 2) | | `resolveReplanTargetColumn` | yes (IR-driven) | — self-healed to `todo` | tests repaired | | its two `return "triage"` fallbacks | n/a | reachable only for workflows declaring neither column | **recorded, not fixed** — column-policy decision, has its own covering test | ## Verification - **Mutation-verified both directions:** dropping the merged lane from rule 1 fails **2** tests; wrongly extending rule 2 to the merged lane fails **1** - 50 replan-target tests green (7 new) - merge gate green (414 + 10 + 71), tsc clean, lint clean ## Separate finding — affects every worker `resolveTaskWorkflowIrSync` **cannot resolve a task's selection in production.** `getTaskWorkflowSelectionImpl` is `return undefined` unconditionally and `getTaskWorkflowSelectionAsyncImpl` is *"always PostgreSQL path"*, so the sync resolver **always** returns the DEFAULT workflow IR. `moves.ts` already hit this and fixed it by going async. Any conversion built on the sync resolver is inert for **custom** workflows — harmless for default cards, since the default IR is exactly what comes back. Reported to the coordinator for the other workers; not actionable in this PR, which uses no sync resolution. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
|
969c2cdf1d |
capacity part 4: drop the central global_concurrency table (migration 0037) (#2555)
Final piece of the cross-project cap removal. Enforcement (#2509), settings/API/UI (#2529) are merged; this removes the storage. Nothing read the table. `global_max_concurrent` held the deleted machine-wide cap; `currently_active`/`queued_count` were written only by `acquireGlobalSlot`/`releaseGlobalSlot`, measured earlier in this program to have **no production caller**, so those counters were fiction. Live “N running (all projects)” telemetry comes from `CentralCore.getLiveRunningAgentCounts` and is unaffected. Dropped rather than left unread: a lingering table with plausible-looking counters invites a future reader to trust it — the same trap as a readable-but-ignored settings key. ## The trap this hit, because the first attempt looked correct `schema-applier.ts` warns that *“migrations are registered here explicitly (not auto-discovered from the migrations dir), so a new .sql file that is not wired through a version constant + bookkeeping check silently never runs.”* My first pass added the `.sql`, updated the drizzle model and bumped the baseline — **and the table was still present in a fresh database**. It was caught only because the test asserts the table is *gone* (`to_regclass(...) IS NULL`) rather than merely unreferenced; an absence-of-reference assertion would have passed while the table survived. Now registered properly: `DROP_GLOBAL_CONCURRENCY_VERSION = "0037"`, explicit path constant, applied-check, bookkeeping insert. The historical `0000` baseline is deliberately **not** rewritten — a fresh database CREATEs the table then drops it, converging with upgraded databases without editing history, which is how every prior migration here behaves. Also removed: the drizzle model, the `centralTableNames` entry, and the `replacesCentralSeed` special case in the SQLite migrator (a legacy SQLite `globalConcurrency` table now has no destination and is simply not migrated — correct, since its cap is deleted and its counters were never written). ## Verification `pnpm lint` clean · core `tsc` clean · `pnpm test:gate` green (414 + 10 + 71) · `schema-applier` 75/75 · `sqlite-migrator` 43/43 · full core PG suite **1044 passed / 3 failed** — the same 3 pre-existing (`central-archive-secrets` log-prefix, `workflow-settings-project-identity` legacy fallback ×2). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3da5358b33 |
test(U9): add a core unit-gate so dependency gating and FN-5819 block merges (#2569)
**U9, PR10.** Two `package.json` lines. No test or production changes —
this only decides *when* existing tests run.
## The gap
Two U9 safeguards are well covered but sat in **no blocking gate**.
Their proof lives in `packages/core/src/__tests__/task-merge.test.ts`,
and core's only gate job is `test:pg-gate` (two PG tests). A regression
in either surfaced in non-blocking full-suite — after the merge.
## What's now gated, each verified by mutation delta
**`task-merge.test.ts`**
| Invariant | Mutation | NEW failures |
|---|---|---|
| Safeguard 3 — dependency gating | `getTaskCompletionBlocker` drops the
unresolved-dependency reason | **5** |
| FN-5819 — exception bounded to a live group | drop `group.status ===
"open"` | **1** |
| FN-5819 — exception bounded to shared members | widen
`isSharedBranchGroupMemberIntegration` to every task | **4** |
Both FN-5819 directions matter. This is the **only** scoped exception to
`autoMerge:false`, so its *narrowness* is the invariant — not merely its
existence. A test that only proves the exception works would pass while
the exception swallowed every task.
**`legacy-adoption.test.ts`**
| Invariant | Mutation | NEW failures |
|---|---|---|
| FN-8492 — orphaned pending results REWRITTEN to failed, never DELETED
| delete instead of rewrite | **2** |
That one matters because deletion *silently satisfies* the merge gate:
the gate blocks on pending/failed results, not on an enabled step with
no result, so deleting lets a task merge with its review skipped.
## Implementation
Adds `packages/core` → `test:unit-gate`, a curated **non-PG allow-list**
mirroring `engine-core`'s discipline (explicit membership, not a glob),
run as a third parallel job in the root `test:gate` block alongside the
engine and PG jobs.
**Gate fires — verified, not assumed:**
- drop the dependency reason → `pnpm test:gate` **exits 1**
- drop the FN-5819 open-group bound → **exits 1**
- restored → **exits 0**
## Cost: no measurable increase
| | Runs |
|---|---|
| baseline | 13.07s, 14.95s |
| with the job | 12.20s, 12.58s |
It runs in parallel with the existing jobs and finishes well inside
them, so the delta sits inside run-to-run variance. **I am not claiming
a speedup** — the honest reading is "no measurable cost", and the
variance band here is wider than the change.
## Reversible call made rather than asked
A new `test:unit-gate` script rather than widening `test:pg-gate` or
adding a glob. `test:pg-gate` carries PG setup these pure unit tests do
not need, and a glob would admit all of core by default — which
AGENTS.md explicitly forbids ("tests never graduate into the gate by
default"). Membership stays explicit so the next addition has to state
its evidence.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
4ee6800a8f |
test(U9): gate the review-lane leniency guard (prose rejection never becomes APPROVE) (#2564)
**U9, PR9.** Config only — one line added to the `engine-core` allow-list, plus its justification. The merge half of U9's safeguards now fires in blocking CI (#2526). **This is the review half, none of which did.** ## What's admitted `workflow-step-verdict-parsing.test.ts` holds `proseSignalsClearApproval`'s leniency guard: **a prose REJECTION must never be promoted to APPROVE.** Removing the REVISE/RETHINK/negated-approval disqualifiers fails **11** of its cases. This is a **fail-open** defect on the path to an irreversible merge — a review saying *"looks good, but this must be fixed before merging"* would read as an approval. That belongs in the gate, not in a non-blocking run hours after the merge. Measured across 3 runs: | | Files | Tests | Wall | |---|---|---|---| | before | 19 | 414 | 6.16 / 6.25 / 6.21s | | after | 20 | 482 | 6.28 / 6.55 / 6.33s | **+~0.2s** against a ~60s ceiling. **Gate fires — verified, not assumed:** removing the disqualifiers → `pnpm test:gate` exits 1 (11 failed / 471 passed); restored → exits 0. ## What is deliberately NOT admitted, and why `reviewer.test.ts` holds the sibling family — *"a provider outage is not a review verdict"*. I verified by mutation that it genuinely guards this: removing the escalation branch fails **5** tests covering "escalates a rate limit as `ReviewerProviderError` instead of an `UNAVAILABLE` verdict", "does not burn the reviewer fallback retry budget on a provider outage", and "escalates as transient once the network retry budget is exhausted". That budget exists to bound *bad reviews*; spending it on an outage fails tasks that have nothing wrong with them. It is green in `engine-default` but **fails 72 cases under `engine-core`**, because that project resolves `@fusion/core` through the **reduced** `index.gate.ts` barrel/bundle and the suite reaches exports it does not carry (`__vite_ssr_import_0__.has…` TypeError). Admitting it would mean widening the gate barrel — which trades away the bundle's entire reason for existing (FN-7669 measured the barrel import phase as the gate's dominant wall-time cost). **I tried it, measured the 72 failures, and backed it out** rather than either shipping a red gate or — the tempting version — loosening the test until it passed under the reduced barrel. The reason is recorded in the config next to the allow-list so the next person doesn't rediscover it. Widening the barrel for this suite is a real option, but it is a gate-performance decision with its own measurement, not a side effect of a test-coverage PR. ## Review-lane characterization status By-name coverage search performed first in every case, per the lesson from #2520: | Invariant | Verdict | |---|---| | FN-8492 orphaned pending results rewritten, never deleted | covered (NEW=2) | | FN-7720 bypass writes `skipped` | covered (NEW=1) | | FN-7720 bypass never fabricates a verdict | **was vacuous** — fixed in #2541 | | Provider outage escalates, never becomes a verdict | covered (NEW=5), outside the gate — see above | | Prose rejection never promoted to APPROVE | covered (NEW=11) — **now gated** | | testMode never issues real AI calls | **was permanently red** — fixed in #2547 | Still uncharacterized, stated rather than implied: branch-group member integration and promotion sequencing (the FN-5819 scoped exception). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
41031dbe2c |
Drift review (unowned): auto-claim candidacy resolves hold + completion roles — three literals, two opposite failures (#2565)
> **Based on `main`** — independent of my U7 stack and of #2561; merges in any order. Third unowned drift-review site. `isRunnableAutoClaimCandidate` is the single source of truth for *"may an agent claim this task?"* (FN-6873), and it carried **three** lifecycle literals that fail in **opposite directions**. ## The two failures **`column === "todo"` gated candidacy** on the hold role. Keyed on the literal, a renamed workflow's candidate set was **permanently empty** — agents were never offered its work, and nothing anywhere reported it. Silence, not an error. **`dependency?.column === "done" || "archived"` gated dependency satisfaction**, and this is the more dangerous half: a dependency that finished in a renamed **complete** column was never recognised as done, so the dependent stayed **blocked forever**. One makes work invisible; the other makes it permanently ineligible. Both are silent. ## Roles resolve per task, not per pass The non-obvious part: **a dependency may sit on a different workflow from the claimant.** A single per-pass answer is wrong for one of them on any mixed board — so the map is keyed by task id, and the dependency check reads the *dependency's* roles, not the claimant's. Asserted directly: a dependency completed in `done` (default vocabulary) satisfying a claimant waiting in `drafting` (renamed). ## Shape Both callers already have the store and are async, so they resolve for real rather than taking the injected-lane fallback the *synchronous* predicates needed (#2551). The predicate itself stays synchronous — a resolved-roles map is passed in — because it runs inside two `filter`/`flatMap` bodies. Tasks absent from the map keep the legacy ids, so a partially-resolvable board degrades to today's behavior instead of silently emptying the candidate set. **Type narrowing preserved.** The two callers take `Pick<TaskStore, "listTasks">`, which is what makes them testable without a real store. Rather than widening to the whole `TaskStore`, they now take `Pick<TaskStore, "listTasks"> & WorkflowIrResolverStore` — the minimal additional shape resolution needs. ## Revert proofs, isolated per literal | Restored | Result | |---|---| | hold literal only | **3 of 6 fail** | | dependency-completion literals only | **1 of 6 fails** | The three default-vocabulary cases pass under both. Splitting the proof matters here: it confirms the two halves are **independently** load-bearing rather than one masking the other — a single combined revert would have shown 3 failures and told me nothing about the dependency half. ## Convergence Measured on `main`, comment-stripped scan of `column === / !== "todo" | "triage"` in `packages/*/src` excluding tests: - this file alone: **103 → 102** - with #2561: **103 → 100** The `done` / `archived` literals fixed here sit outside that pattern and are not counted — same caveat as #2561's gridlock `active` filter. Two PRs now where the real fix is larger than the metric shows. ## Verification | Check | Result | |---|---| | new suite | 6/6 | | pre-existing auto-claim suite | 17/17, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (414 + 10 + 71) | | `pnpm check:changesets` | clean | ## Remaining unowned in my area `mission-feature-sync.ts` (1, a planning-lane check) and `notification-service.ts` (1, *"has progressed past"* — a different semantic needing its own thinking, not a mechanical swap). Taking those next unless claimed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8578a1d27d |
U8 PR5: thread the implementation exit to the step seam, and declare the stepwise pending-review park (inert) (#2546)
Follows **#2519** (U8 PR4). Both halves are inert — **no behavior
change** — and this removes the blocker PR4 documented.
## What was blocking
PR4 could only land its IR half because the pending-review ending could
not reach a graph edge on the **default** workflow. Three links in the
chain:
| Link | Problem |
|---|---|
| `runGraphTaskStep` | awaited the memoized implementation pass and
**discarded** its result |
| `RunTaskStepResult` / `RunSingleStep` | had nowhere to carry an exit |
| `stepExecute` seam | flattened every ending to `step-done` /
`step-failed` |
All three are fixed. The outcome stays `failure` (the step genuinely did
not complete) while the **value** now names the ending — which is what
`runForeach` propagates upward, since it returns a failing instance's
value as the foreach node's own. Every other ending keeps `step-failed`
byte-identically.
One design note: the exit is a property of the **pass**, not of a step.
A single memoized pass serves every foreach instance, so all instances
report the same ending — correct, because the ending is what stopped the
whole session.
With the value surviving, the stepwise IR declares the same
`review-handoff` park node and `steps --outcome:review-pending-->
review-pending-handoff --success--> end` edge the plain-`execute` shape
got in PR4, inherited by the final-review and Ideas variants that clone
it.
## A bug my own threading introduced, and what caught it
The first threading commit covered **one of the two** paths out of
`runProjectedGraphTaskStep`. The early-return branch carried the exit;
the main path goes through `runTaskStep` in `step-runner.ts`, which
builds its own result and dropped it — i.e. it worked on the path I
happened to read, and not on the path the default workflow actually
takes.
**FN-5436's regression test caught it, not code review.** That is the
second time this test has stood between this unit and a silent
regression, which is worth recording somewhere durable:
`executor-step-session.test.ts > FN-5436: pending-review skip on
no-fn_task_done exit` is the load-bearing test for this area.
## Why the seam flip is still not here
With the threading complete I applied the behavior half again — flip the
execute seam to return `review-pending`, delete the inline
`handoffTaskToReview`, add a named compat classifier for user-authored
graphs. **FN-5436 still failed**: the card did not reach `in-review`, so
something between the seam value and the park node is not routing under
that harness. I have not isolated whether that is the mock store's IR
resolution (it exposes no `getWorkflowDefinition`, so the run resolves
the built-in through a different path), a foreach aggregation detail, or
the park node's own seam.
I stopped rather than keep guessing, and reverted the behavior edits so
this lands green and inert. Shipping a half-routed move is exactly the
failure this unit exists to remove — a lifecycle transition that
silently does not happen. The alternative on offer was to relax
FN-5436's assertion, which would have been appeasing a test that is
telling the truth.
### What the instrumentation showed (done after opening this PR)
I ran the bounded next step rather than leaving it as a note. Two facts,
both measured:
1. **The IR is correct.** Resolving
`BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR` at runtime shows the
node and the edge survive the final-review variant's edge rewiring:
```
EDGES [{"from":"steps","to":"browser-verification","condition":"success"},
{"from":"steps","to":"review-pending-handoff","condition":"outcome:review-pending"},
{"from":"steps","to":"end","condition":"failure"}]
HAS NODE true
```
That matters because the variant does `template.edges = [ ... ]` (a
wholesale replacement) and filters outer edges touching `review` —
`review-pending-handoff` is not `review`, so it survives. Worth knowing
before anyone adds another node near it.
2. **The `stepExecute` seam is never invoked in that harness**, even
though the run terminates at `steps#0:step-execute` and the
implementation session demonstrably runs (`"Agent finished without
calling fn_task_done but Step 0 is blocked on pending review"` is in the
task log). A `console.log` at the seam's value computation produced no
output. So the exit is threaded correctly and the IR can route it, but
under this harness the value never originates.
3. **Nor is `createPromptLikeHandler`'s returned handler.**
Instrumenting its dispatch (`node.id` + resolved seam) produced nothing
either — so the node is not reaching the prompt-like path at all.
**Control experiment, because a negative result from instrumentation is
worthless until you prove the instrumentation is observable.** A
`process.stderr.write` at module load of the same file appears exactly
once in the same run, so writes from that module *are* captured under
this harness and the two negatives above are real, not artifacts of
swallowed output.
That narrows the remaining work to one question — what actually drives
`steps#0:step-execute` in this run, if neither the prompt-like handler
nor the `stepExecute` seam does — and rules out the IR, the foreach
propagation, the threading, and the instrumentation as suspects.
**Next step, now much narrower:** find the handler registration this run
resolves for a foreach instance node (the graph executor's handler map,
not the seam table), then flip the seam, delete the inline handoff, and
update the three ratchets that will correctly fire — PR3's routing pin,
the out-of-band adjacency check, and PR1's ownership ledger
(`runImplementation` 3 → 2; `handleGraphFailure` 0 → 1 for custom graphs
only).
## Verification
- `executor-step-session` + exit-events + ownership ledger +
graph-boundary — **56 tests green**
- `builtin-workflows` + `builtin-coding-workflow-ir` — green. The
layout-completeness contract required a layout entry for the new node in
all four stepwise-derived workflows; placed off the main line, because a
park is an exit and not a stage.
- `pnpm test:gate` green (10 / 309 / 71); `pnpm lint` clean; `tsc
--noEmit` clean
- Changeset included (`patch`, `internal`)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
3cef9c226e |
Drift 1/4: TaskCard planning affordances from traits, not "triage" (8→3 measured; one site needed a new wire fact, one conversion was wrong and the tests caught it) (#2558)
## Drift conversion 1 of 4 — TaskCard.tsx Taking the dashboard surfaces from the drift review. This is the board-card one; ListView, TaskDetailModal and register-task-workflow-routes follow separately so each stays revertable. ### Convergence number `task.column === / !== "todo" | "triage"` in `TaskCard.tsx`: **8 → 3** The three survivors are **one documented fallback**, not scattered checks. `getTaskColumnFlags` (Column.tsx) returns `undefined` when a card's column is absent from the resolved metadata and is not the rendering column — the pre-load window, and a card stranded in a lane its workflow dropped. Converting to bare trait reads would have removed every planning affordance in exactly those states, so the legacy ids survive **once**, at the role helpers, plus the move prompt resolving its own target. They retire with the load window, not with this change. I'd rather report 8 → 3 with the reason than 8 → 0 with a regression behind it. ### Why this file was urgent Every planning affordance was gated on `task.column === "triage"`. Land U11 — merged column keeps id `todo`, `triage` deleted — and each comparison silently becomes false: **delete button, awaiting-approval controls, planner badge, step list all vanish from planning cards.** ### One site needed a new fact on the wire, not a renamed comparison `showStartAction` was `intake === true && column !== "triage"`. That hardcoded id was standing in for *"an intake column that does not auto-triage"* — a distinction that lives in trait **config** (`intake` with `autoTriage: false`) and was invisible to every client. It also **inverts** under U11: with `triage` deleted, `column !== "triage"` is vacuously true, so a Start button would appear on **every planning card**. `describeColumns` now derives `manualIntake` server-side and the gate reads it. Renaming the comparison would have shipped the inversion. ### One conversion was wrong, and the tests caught it I first converted the move-progress prompt to the card's *own* column role. The original tests the move **destination** — moving a card *back* into a pre-implementation lane is what risks discarding step progress. `confirms preserving progress before moving` failed immediately on an `in-progress → todo` move. It now resolves the target column's flags. That is the argument for red-green per site rather than pattern-matching the comparison: the regex looks identical at both sites and means different things. ### Revert-proof New `TaskCard.u11-merged-column.test.tsx` renders cards in the **post-U11 shape** — id `todo`, traits `intake + hold`, no `triage` anywhere — and asserts Delete, the planner badge and the step list still appear; that Start does **not** (auto-triaging); and that a manual-intake lane does get it. Revert any converted site and the matching case fails, because these cards are not in `triage` and never will be again. ### Fixture updates, and why they are not weakening - Start-affordance cases now pass `manualIntake`, which the server supplies for a manual intake lane. - "omits the Start button for the triage column even when intake is flagged" → "for an **AUTO-triaging** intake column". The rule was never about the id; the title said it was. ### Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, both dashboard typechecks green. **No new failures**: `TaskCard.test.tsx` reports the same 2 pre-existing failures with and without the change, verified by diffing failing test *names* against a stashed clean tree rather than comparing counts. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3681a9f9a5 |
test(U9): re-green merge-error-recovery.test.ts (10 stale tests deleted, replacement contract covered) (#2559)
**U9, PR8.** Test-only, two commits (deletion and new coverage deliberately separate). `merge-error-recovery.test.ts` has been **red on main: 10 failed / 23 passed**. Now **24 passed**. ## Commit 1 — the 10 failures test a feature that no longer exists All 10 assert that `ProjectEngine` creates recovery follow-up **tasks** and dedupes them by parent/branch. Evidence this was deliberate, not a regression: - `project-engine.ts` contains **zero** `createTask` calls. - The string the dedupe tests assert on — `"follow-up already exists"` — exists **only in the test file**; no production code emits it. - `project-engine.ts:4801` documents it outright (`FNXC:AutostashRecovery 2026-07-26`): *"This used to file an automated recovery follow-up card via the shared follow-up engine; that engine was deleted ... So the card is replaced by a durable log entry AND an operator comment on the parent."* Deleted rather than repaired — there is nothing left for them to assert. ## Commit 2 — cover the contract that replaced them The production comment is explicit that `record.label` *"must never be dropped from the message or truncated"* — it is the handle `git stash` recovery needs, and the parent may already be `done`, so the notice is the only trace of real uncommitted work. **That invariant had no working assertion.** The file was red, so every claim it made was inert. The new test asserts one log entry + one comment for a `live` orphan (a `subsumed` record stays silent), and that label, short sha, detecting task and source phase all survive into the comment, with the label in both the log message and its detail field. | Mutation | Result | |---|---| | replace the label with `(omitted)` | 1 failed / 23 passed — this test | | notify on non-live orphans too | `NEW-failures=1` — this test | ## A tooling bug this uncovered, which matters beyond this PR The new test originally reported **zero** new failures under mutation while passing normally — i.e. it looked vacuous. It was not. A thrown assertion left the engine running, which **crashed the vitest worker**, and a crashed run emits no parseable `FAIL` lines — so my mutation harness parsed zero failures and printed **NOT COVERED for a guard that had just correctly failed**. Two fixes: - The test stops the engine in a `finally`, so a failure reports as an assertion instead of killing the worker. - The harness now treats *non-zero exit with zero parsed failures* as **INCONCLUSIVE**, never as a coverage verdict, and prints the crash signature. This is the **second** time a blind spot in my own tooling manufactured a false "uncovered" result — after the `|project|` regex that matched nothing for `@fusion/core`. Both had the same shape: the measuring instrument reported success without checking anything, which is precisely the defect class this program is chasing. Worth stating plainly rather than quietly fixing. ## Why this file matters to U9 Its 10 pre-existing failures are what corrupted my own safeguard measurements in #2511 — an absolute-count mutation run credited them to the mutation. **A red file in the merge lane does not merely lack coverage; it poisons the measurement of everything near it.** ## Wider context, measured `engine-default` on clean `main` is **283 failed / 9062 passed across 28 files**. This PR clears one of those files. I did not attempt the rest: most are outside the merge/review lane and plausibly owned by other workers on this program. Also measured and abandoned: extending `check:mock-completeness` to relative intra-package mocks — the naive rule flags **147** factories of which **146 are green**, so it would be almost pure false positives; the barrel heuristic does not transfer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
67904f8a2c |
U11: merge Todo into Planning on the default lineage (+ the migration mechanism, and a measured safety audit that cuts the work list 32%) (#2515)
**Merges Todo into Planning on the operator's real default workflow.** Held from merge pending the `triage` literal audit below — see *Gating*. ## The board change `builtin:coding` → `BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR` → clones `BUILTIN_STEPWISE_CODING_WORKFLOW_IR`. That IR now declares **five** columns, and `plan`, `plan-review`, `plan-replan` and `start` all live in the merged Planning column: ``` columns: todo="Planning", in-progress, in-review, done, archived start -> todo plan -> todo plan-review -> todo plan-replan -> todo parse -> in-progress (first implementation node) ``` The id stays `todo`, the display name becomes "Planning". That is the cheaper half: `todo` was already the hold column, so every trait lookup, task row, stored selection and the 121 `column === "todo"` guards keep their meaning, and **no stored row needs re-homing**. Promoting `triage` instead would have produced the same board while making those guards workflow-*dependent* — live for Coding (Ideas), silently dead for Coding. `builtin:legacy-coding` keeps its six-column shape, per the operator's decision. It exists to be the old thing. ## Entry contract, before and after each IR edit | | result | |---|---| | before the default-lineage edit | **15 passed** | | after the edit | **13 passed, 2 failed** | | after reading both | **15 passed** | Neither failure was routed around. One was a genuine expectation change (two planning entry points became one); the other was my own `mergeTodoIntoPlanning` helper throwing *"source IR is not the split-column shape this merge transforms"* — because production **is** the merged shape now. I **deleted** the helper rather than making it tolerant: a transform that has silently become a no-op asserts nothing. ## The safety argument, proven not asserted Entering at `start` is exactly what dragged cards backward in the three earlier reverted attempts. `merged-planning-start-node-no-move.test.ts` proves against the **real** boundary controller and **real** default IR that entering `start` performs no move (`moveTask` is never *called*), reaches no hold→wip capacity seam, and **still moves on a genuine crossing** so the no-op is same-column rather than a disabled boundary. Removing the controller's same-column short-circuit turns exactly the two no-move tests red. ## The migration mechanism A card can outlive its column. `resolveAllowedColumns` derives targets from graph adjacency, and an undeclared source has none — so it returned `[]` and **every** move was rejected with "Valid targets: none", including the one that would rescue the card. An undeclared source now resolves to the workflow's rebound target. Escape hatch, not relaxation: declared columns are untouched, and it offers the rebound target *only*, so a stranded card gets back **into** the lifecycle rather than a free jump past review. ## A real regression this surfaced `isDefaultWorkflowColumns` matched the legacy **six** ids as a set. The merged default declares five, so the match stopped firing and the default board fell through to neighbor-only adjacency, which **drops legal moves and invents an illegal one**: | edge | effect | |---|---| | `in-progress → done` | **dropped** — the mission-validation cross edge | | `in-review → todo` | **dropped** — review work back to planning | | `todo/done → archived` | **dropped** — the FN-4892 direct-archival edges | | `done → in-review` | **invented** — a backward edge no rule allows | Adjacency now derives from lifecycle **roles**. The load-bearing assertion: the legacy six still reproduce `VALID_TRANSITIONS` **verbatim**. Applied only when a workflow declares the full role set, so custom boards keep neighbor adjacency. ## Failure accounting (core package, vs a 49-failure baseline) | stage | failed | new | |---|---:|---:| | after the merge | 65 | 18 | | after the escape hatch | 52 | 5 | | after role-derived adjacency | 53 | 4 | The 4 remaining are 3 `builtin-workflows` expectations encoding the pre-merge shape and 1 create-intake expectation naming `triage` on `builtin:coding`. Two `schema-applier` and two `workflow-reconciliation-production-shape` failures appeared in intermediate runs and are **not mine** — both files pass in isolation (75/75 and 7/7). I re-ran each before attributing them, which is why the earlier "priority" flag on the reconciliation pair was withdrawn. Gate: **309/309**. Lint clean. ## Gating: the `triage` audit (`docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md`) Program tracking cited **58** `triage` comparisons. Measured with the same pattern: | | count | |---|---:| | raw comparisons | 87 | | inside comments | 1 | | **not a lifecycle column at all** | **15** | | column comparisons | 71 | | OR-paired with `"todo"` in the same expression | 32 | | **exclusive `triage` — the real work list** | **39** | **15 do not compare a column.** `role === "triage"`, `surface === "triage"`, `sessionPurpose === "triage"`, `entry.agent === "triage"` name the planning **agent**. Converting them would be actively wrong, and the failure — a planning agent that can't resolve its prompt template — would look nothing like a column bug. **One site changes an operator-visible affordance**, which is why per-site review beat a sweep: `TaskCard.tsx:1927` — `taskColumnFlags?.intake === true && task.column !== "triage"`. The literal is a **narrowing**, not a match. After the merge a Planning card has `intake === true` and `column === "todo"`, so the narrowing stops applying and **Start begins rendering on default Planning cards where it previously did not.** A sweep would have "converted" the literal and shipped the new affordance silently. These guards do not go **dead**, they go **workflow-dependent** — `triage` stays live for legacy-coding, Ideas, every linear built-in and any user workflow (R11) — which is harder to detect than dead. Work list and ownership are in the audit doc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
82baaa0b67 |
test(U9): give the FN-7720 "no fabricated verdict" invariant a real assertion (#2541)
**U9, PR6.** Test-only, one file, no production change. Found while characterizing the reviewer lane (U9 is "review *and* merge"; PRs 1–5 covered merge). ## A test named for an invariant it does not assert `store-bypass-review.test.ts` has a case called *"rewrites the failed step to skipped with bypass audit metadata **and no fabricated verdict**"*, containing `expect(result?.verdict).toBeUndefined()`. Its fixture sets `verdict: undefined`. **The assertion is vacuous.** Deleting `delete bypassed.verdict;` from `store.ts` leaves the whole suite green. Measured: `NEW-failures=0` across `store-bypass-review`, `task-merge-bypass`, `task-merge`, `legacy-adoption`. I explicitly confirmed the suite **runs rather than skips** — 9 tests via `pgDescribe` against the shared PG harness. A skipped suite produces exactly the same misleading zero, and that is the failure mode I hit earlier in this unit with a regex that matched nothing. ## Why it matters FN-7720 is explicit that a bypass writes status `skipped` and **never fabricates a reviewer verdict**. The invariant only has teeth when the failed step *carries* a verdict — which is the actual risk case: a reviewer says `REVISE`, an operator bypasses, and the verdict rides forward onto a `skipped` step. Every downstream reader then sees a reviewer verdict attached to a step no reviewer passed. The production code is **correct**. It was simply unasserted. ## The added case is two-sided With `verdict: "REVISE"` seeded, it asserts: - the bypassed step has **no** verdict (not carried forward), and - `bypassedFromVerdict` preserves `"REVISE"` (not silently lost from the audit trail) so it fails if the clear is removed *and* if the audit field is dropped. A one-sided version would pass against a bypass that simply discards all verdict history. | Mutation | NEW failures | |---|---| | remove `delete bypassed.verdict` | **1** — this test, and only it | | drop `bypassedFromVerdict` | **1** — this test, and only it | ## Reviewer-lane characterization so far By-name coverage search done **first** this time, per the lesson from #2520: | Invariant | Verdict | |---|---| | FN-8492 orphaned pending results REWRITTEN to failed, never deleted | **covered** — `legacy-adoption.test.ts`, NEW=2; one case is literally named "NEVER deletes an orphaned entry" | | FN-7720 bypass writes status `skipped` | **covered** — NEW=1 | | FN-7720 bypass never fabricates a verdict | **was vacuous** — fixed here | Still to characterize, and stated rather than implied: review verdicts routing as graph outcomes, and provider-outage hold-in-place (no fabricated verdict on outage). Those are the next PR. ## Note on this shared checkout Earlier in this unit I used `git stash` to isolate a measurement and, because my tree was already committed-clean, the `pop` targeted the operator's stash entry. It failed safely on an untracked-file conflict and both entries are intact — but that was luck. I no longer use stash here; isolation is done by editing and restoring files directly, with `git status` asserted clean afterwards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
69790dc3e7 |
test(U9): revive two permanently-red testMode guards in reviewer.test.ts (#2547)
**U9, PR7.** One test file, +15 lines, no production change.
## Two safety tests that could never pass
`reviewer.test.ts`'s `vi.mock("../pi.js")` is missing
`wrapToolsWithOutputBudget`, which `wrapCustomToolsForPluginRuntime`
(`agent-session-helpers.ts:104`) calls as the outermost tool wrapper.
Both test-mode-forcing cases therefore threw:
```
No "wrapToolsWithOutputBudget" export is defined on the "../pi.js" mock
```
They have been **permanently red on main** — dead enforcement on the
invariant that **testMode never issues real AI calls**.
`reviewer.test.ts`: 83 passed | 2 failed → **85 passed**.
Found while characterizing the reviewer lane for U9: they surfaced as
pre-existing baseline failures under an unrelated mutation run. This is
exactly why the delta harness records a baseline — under the old
absolute-count method these two would have been silently credited to
whatever mutation was running.
**Not a product bug.** testMode forcing works correctly; its guard did
not.
## Verified the revived tests actually guard something
A dead test can also be a vacuous one, so passing again is not
sufficient evidence. Mutating `isTestModeActive` in
`model-resolution.ts` to ignore `settings.testMode` fails **exactly
these two** (`NEW-failures=2`). Both assert
`expect(mockedCreateFnAgent).not.toHaveBeenCalled()` — no live agent
spawn.
## Why the existing gate didn't catch it
`pnpm check:mock-completeness` runs in the merge gate and passes. It
inspects only the `@fusion/engine` and `@fusion/dashboard` **barrels**,
under `cli/` and `dashboard/` test dirs — never a relative intra-package
mock like `"../pi.js"`. So the whole class of engine-internal mock drift
is outside it.
**Deliberately not fixed here.** Extending the checker is its own change
and I want the violation count measured before proposing it, rather than
opening a PR that turns out to touch dozens of files. That's the next
PR.
## Also observed, stated rather than buried
Mutating `useMockRuntime` in `agent-session-helpers.ts` produces **no**
failure in this file — the reviewer path routes through model resolution
instead. That downstream seam has its own coverage question which I have
not answered; flagging it rather than implying this PR closes it.
## Scope note
This was initially committed onto #2541's branch. I split it onto its
own branch so each PR stays independently revertable — #2541 is now one
commit (the FN-7720 verdict assertion) and this is one commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
c2705f292f |
U11: delete the dead isRunnableQueuedOverlapCandidate export (scheduler.ts now has zero live todo literals) (#2542)
Based on `main`. **Pure deletion — zero production callers.** `isRunnableQueuedOverlapCandidate`'s only consumer was the legacy pull-from-todo dispatcher deleted in #2505. The three remaining references were all in tests. This was `scheduler.ts`'s **last `"todo"` literal in live code**, so removing it rather than converting it is what actually finishes the file — converting a dead predicate would have added a trait lookup nothing calls, and reported U11 progress for a site that cannot execute. ## Why deleting its tests does not lose coverage Worth checking, because the function carried a real invariant — *"a busy merge lane must not block unrelated dispatch"* — and its own doc comment claims it's a shared contract with self-healing and repair paths. Two facts settle it: 1. **The overlap logic is still live**, implemented inline inside `runHoldReleaseSweepPass` (`activeScopes`, `overlapIgnorePaths`, `getFilteredFileScope`). The behavior didn't die with the predicate; only this copy of it did. 2. **`scheduler-overlap-starvation.test.ts` exercises that live path** through `scheduler.schedule()`, including *"does not defer ready work behind queued overlap blocked by an active lease"* — the same invariant the deleted test asserted, against code that actually runs. The doc comment's claim that self-healing *"must use this same predicate"* is **stale**: no self-healing path imports it. That claim outlived the coupling it described. ## The three test references were not equal Treating them identically would have been wrong: - **Two were incidental trailing assertions** in tests about other subjects (stuck-loop exhaustion parking; transient merge-error classification). Only the assertion line is removed — each test keeps its real subject. - **One test's entire subject was this function** (*"does not block unrelated executor dispatch when merge lane is busy"*), so it goes with it; its invariant is covered on the live path per (2). ## Measured `scheduler.ts` **2,840 → 2,820 = −20**, and it now holds **zero `"todo"` literals in live code**. Combined with #2505's −929, `scheduler.ts` is down **949 lines** across this unit — all genuine removal, not relocation. ## Verification 582 tests green across the reliability-interactions suite and four scheduler suites; merge gate green (414 + 10 + 71); tsc clean; lint clean. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Simplified internal task scheduling logic by removing obsolete overlap coordination checks. * Preserved existing task progress, parking behavior, logging, and review handling. * **Tests** * Updated reliability checks to align with the streamlined scheduler behavior. * Continued validating transient errors, non-progress handling, and correct task dispatch without changing the end-user experience. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
01a75f9edc |
fix(desktop): typecheck streamed model downloads (#2493)
## Summary - make the fetch response-body cast explicit across DOM and desktop TypeScript library definitions - preserve the existing async byte-stream runtime behavior ## Test plan - `pnpm --filter @fusion/dashboard exec vitest run src/stt/__tests__/model-manager.test.ts --reporter=dot` - `pnpm --filter @fusion/desktop typecheck` - `pnpm --filter @fusion/dashboard typecheck` - `node scripts/check-changeset-format.mjs` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved desktop build typechecking for streamed speech-model downloads by refining how streamed response bodies are interpreted for TypeScript. * Preserved runtime behavior, including streaming, integrity/hash checking, file writing, and cancellation handling. * **Maintenance** * Updated the release metadata so this fix is published with the correct patch classification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
f1be80420f |
U12 part 9: make the raw-flag census a ratchet that fails when the last read goes — answer: 2 reads left, key cannot be deleted (#2537)
## U12 part 9 — the flag census now answers itself Independent of the #2530 rebase; adds one test file, no production changes. ## The answer, first: NO, the settings key cannot be deleted yet **Three files reference the raw flag on current main (`3ff98aae5`):** ``` packages/core/src/store.ts ← declares it packages/core/src/task-store/moves.ts:363 ← U2b: `useWorkflow` packages/core/src/task-store/workflow-task-create-ops.ts:351 ← U2b: move-policy preflight ``` Everything else that greps is a comment, a test writing the flag deliberately to reach the dead path, or the unrelated `workflowColumns.*` i18n namespace for the Columns editor panel. **Why I can't remove them.** Both are on the move path and belong to **U2b**, which carries an equivalence-proof obligation because the two move implementations it arbitrates have never both run in production. They are also **not separable from each other**: `workflow-task-create-ops.ts:351` computes the `movePolicyPreflight` that `moves.ts` consumes and validates, so un-gating it alone would start evaluating workflow move policies — with their plugin-gate side effects — while the branch consuming the result stays off. That is a behaviour change with no consumer, which is worse than either end state. **U2b has not landed.** Program history on main runs `#2466 → #2467 → #2468 → #2469 → #2479 → #2500 → #2512 → #2513 → #2525 → #2528 → #2535`. #2468 was Phase A2 **steps 1–2 only** — the differential characterisation. No convergence PR exists. ## Why this is a PR and not another status message You have asked this question three times. I have answered it three times by grepping, and each answer was a number nobody could re-derive later — including me, which is why I re-ran the audit from scratch each time. That is exactly the shape this program keeps finding: a fact everyone believes, maintained by nobody. So the census is now a test. It **fails in both directions**, deliberately: - **A new read appears** → someone re-gated behaviour on a flag that is `false` for every real project, so the feature behind it will not run. That is the defect class U12 spent its length finding (the capacity gate, the U5 guards, the move policies — all looked enforced, none were). - **The last read disappears** → U2b has landed, and the settings key can finally go. The removal steps are written at the assertion. The second case is the one that matters. It converts "remember to delete the settings key someday" into a failing test at the exact moment that becomes possible, instead of a note in a PR body that ages out. ## Verified in both directions, not assumed - Adding a reference in `lifecycle-ops.ts` → fails with `+ "packages/core/src/task-store/lifecycle-ops.ts"`. - Dropping `moves.ts` from the allowlist → fails with `+ "packages/core/src/task-store/moves.ts"`. Equality rather than subset is what makes the second case possible; a subset check would let the last reader vanish silently and leave the key orphaned forever. Two supporting assertions, both there because of failure modes this program has already hit: - **No production code WRITES the key.** That is the premise the entire unit rests on — if a writer appears, every "this branch is unreachable" conclusion in U12 needs revisiting. - **The scan sees >200 files.** A broken path glob would otherwise make every assertion vacuously green: a guard reporting success without checking anything. ## Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, `pnpm verify:fast`, core typecheck green. ## Standing offer If you want U12 actually closed rather than ratcheted, the remaining work is U2b's convergence. I have the inventory and the divergence list its characterisation suite does not yet cover (plugin column gates, the `transitionPending` marker, `workflowId` in `task:move` run-audit, move-policy preflight). I would want the current U2b worker stood down from `moves.ts` first — two writers on the file this whole program pivots on is the one hazard I would not take on my own authority. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added a new automated Vitest “census ratchet” to ensure only an approved, fixed set of production reads is made for the workflow columns compatibility flag. * Added checks that disallow hardcoded `workflowColumns: true/false` assignments in production sources. * Added allowlist validation, including per-file occurrence counts, required rationale text length, and confirmation that referenced files exist. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6721bdc652 |
U12 part 7: the List view never self-healed a card's workflow — extract Board's FN-7591 refetch and wire it up (#2530)
## U12 part 7 — the List view never self-healed a card's workflow **Stacks on #2528.** Merge that first. Paying off something I owed on #2525: greptile pointed out that a task whose `taskWorkflowIds` entry is absent — or present but resolving to a workflow that does not declare the task's stored column — gets no per-workflow move metadata, so its menu falls back to the neighbour approximation and **stays there until some unrelated refresh happens**. Board has forced one board-workflows refetch for exactly this since FN-7591. List had none. So the degraded state persisted longest precisely where it is most likely: a **just-created card**, which is when a workflow was actually chosen. I said there that porting the self-heal deserved its own change rather than riding along in a move-menu fix. This is it. ### Two commits, deliberately separable **1. Extraction — move only.** Board's ~55 lines (refs, suspect-mapping predicate, signature guard, deferred macrotask) become `useUnmappedWorkflowRefetch`. Copying them into ListView would have created a second copy of subtle race-avoidance logic to keep in sync. Evidence it is a move: with comments and the new wrapper signature stripped, the hook's **41 body lines** and the **42 removed from Board** differ by exactly one line — the `}` that closed Board's enclosing scope. Nothing added, removed or reordered. The original FNXC notes travel with the code, since they are the reason each line exists. Board's suite is green with no expectation edits. **2. Wiring — behaviour change.** ListView calls the hook. ### Revert-proof Remove the hook call from ListView and the new case fails: `fetchBoardWorkflows` is never called a second time, so the mapping never resolves. A companion case pins the other half — a fully-mapped board must **not** refetch, so the signature guard cannot turn a healthy list into a loop. It measures calls made *after* the initial load settles, because mount fetch and switcher-open legitimately call the fetcher and counting from zero would measure those instead. ### Two existing tests needed fixture corrections — neither a regression Both because the self-heal now fires **correctly** where the fixture did not expect a fetch: - `refreshes workflow columns when workflow metadata SSE arrives` chained two `mockResolvedValueOnce` payloads. The file-level cache seed maps no tasks, so first paint saw FN-001 as unmapped and the repair fetch ate the payload the test asserts on. Seeded that test's own first-paint cache, and added a trailing default — the SSE swap (`backlog` → `ready`) leaves FN-001 in a column its workflow no longer declares, so a repair fetch there is right, and without a fallback it resolved `undefined` and wiped the payload. Worth stating plainly: both fixtures had quietly depended on List *never* self-healing. That dependency is what the change removes. ### Verification `pnpm test:gate` (309 + 10 + 71), `pnpm lint`, `pnpm verify:fast` (18 steps), dashboard typecheck green. ListView + Board suites: **320 passed, 0 failed, 0 skipped**. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * List and Board views now self-recover when task-to-workflow mappings are missing or incorrect, avoiding degraded workflow UI until a later refresh. * Workflow recovery retries are more robust and coordinated to handle delayed/failed refreshes. * Recovery behavior correctly stops/reset when switching projects or unmounting. * **Tests** * Added comprehensive ListView coverage for unmapped-workflow self-heal, including retry timing, StrictMode effect replay, SSE refresh interactions, and mapped-vs-unmapped scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7fd1c7f124 |
P0 fix: stop reaping worktrees out from under live planners (FN-6756) (#2531)
User-reported: worktrees deleted while a planning agent was still working in them. Small, isolated, ahead of all remaining capacity work. ## Mechanism `clearPhantomExecutorBinding` is documented as *"the last line of defense against pulling a worktree out from under a running agent"*. It computed liveness from four sets — `activeSessions`, `activeStepExecutors`, `activeWorkflowStepSessions`, `activeCliTaskSessions` — **all TaskExecutor-owned**. A triage PLANNING session is owned by `TriageProcessor`, lives in *its own* `activeSessions` map, and registers in the module-level `activeSessionRegistry`. It matched none of the four. Worse: the method **writes** to that registry (unregistering the task’s paths) but never **read** it as a liveness signal. It destroyed the very evidence that proved the planner alive. Under plan-in-place a card is specified while it sits in `todo`/`triage`, and `reapLeakedConcurrencySlots` treats both as reapable on a rationale written *before* planning moved there (“a task waiting to run must not pin a worktree”). Every gate ahead of the last one passes for a planner: | Gate | Saves a planner? | |---|---| | in `listWorktreeHolders()`? | **No** — `ensureTaskWorktreeForPlanning` → `ensureGraphCustomNodeWorktree` → `addActiveWorktree` (`executor.ts:8581`) | | reapable column? | **No** — plan-in-place keeps the card in `todo`/`triage` | | in the executor’s `executing` set? | **No** — a planner is triage-owned | | 60 s `LEAKED_WORKTREE_SLOT_GRACE_MS` | **No** — keyed on `columnMovedAt`, and planning routinely runs for minutes | So the broken guard decided alone. ## This is FN-8600 recurring through a second sweep That fix registered planning paths in the registry and taught the **self-owned-branch reclaim** sweep to consult `isPathActive`. The leaked-slot reaper never got the same signal — fixed at one surface, not enumerated across all. Exactly what the AGENTS.md Surface Enumeration rule exists to prevent. ## Fix The refusal now also fires when `activeSessionRegistry.pathsForTask(taskId)` is non-empty. Keyed on **any** registered path rather than on kind: the point is that a registered surface of any kind means someone is working in that worktree. ## Enumeration — the part that stops a third recurrence The guard is a **chokepoint**, so this covers every caller rather than just the reported one: - `reapLeakedConcurrencySlots` — the reported path - `recoverPausedAbortFailures` — **had the identical executor-only pre-gate** - the `preserveWorktrees: true` reclaim Audited the rest of self-healing’s liveness gates: the self-owned-branch reclaim, worktree-metadata reconcile and PR-branch sweeps already consult `isPathActive`/`lookupByPath`. The three that read only `getExecutingTaskIds` — `checkStuckBudget`, `recoverCompletedTasks`, `recoverStrandedCompletedTodoTasks` — move columns and never destroy a worktree, so they are noted rather than changed. ## Trade-off, stated plainly A leaked registry entry now blocks this sweep instead of a live planner losing its worktree. That is the strictly safer failure and the one the “last line of defense” wording already promises. The registry is process-local and in-memory, so a leak cannot outlive the process, and stale entries have their own reconciler. **A test pins that a genuine phantom — no executor surface AND no registration — still clears**, so this is not a blanket refusal that would trade this bug for a wedged queue. **The 60 s grace is deliberately unchanged.** Raising it would only make the bug rarer and harder to reproduce; the liveness gate was the defect. ## Verification Revert-proof, measured: removing the registry term turns **3 of the 4** new tests red, including the end-to-end sweep case (card in `triage`, past the grace, executor sets empty → asserts the slot is not reaped and the worktree survives). The 4th stays green both ways *by design* — it is the anti-overcorrection guard. `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (309 + 10 + 71) · new suite 4/4. The 2 failures in `self-healing.test.ts` / `-completion-fanout.test.ts` are **pre-existing** — identical with this change stashed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented active planning worktrees from being mistakenly deleted or reclaimed while related planning sessions are still active. * Enhanced session liveness checks so phantom executor bindings are not cleared when a live session is registered. * Updated paused abort recovery to defer or abort safely when a live planning session is detected, avoiding unintended task/worktree mutations. * **Tests** * Added regression coverage for leaked-slot reaping, paused abort recovery behavior, phantom binding refusal, and end-to-end sweep outcomes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b85a5d4531 |
fix(core): bound compound engineering review remediation (#2532)
## Summary - cap Compound Engineering Code Review remediation at two Execute→Review repair passes - enable no-progress detection for the built-in CE workflow - preserve explicit project/workflow overrides while making the authored CE default visible in settings and docs - update stale IR/changeset language that still described Code Review as unbounded when unset ## Why The previous CE default was effectively unbounded. A reviewer that repeatedly returned `REVISE` could consume thousands of remediation cycles without terminally parking the task. The built-in workflow should fail closed after a small, explicit budget while still allowing operators to author a different numeric cap. ## Verification - `FUSION_PG_TEST_SKIP=1 corepack pnpm@10.33.0 --filter @fusion/core exec vitest run src/__tests__/builtin-workflows.test.ts` — 46 passed, 17 skipped - `corepack pnpm@10.33.0 --filter @fusion/core typecheck` - `corepack pnpm@10.33.0 --filter @fusion/dashboard exec vitest run app/components/__tests__/WorkflowSettingsPanel.test.tsx app/components/__tests__/workflow-setting-display.test.ts` — 33 passed - `corepack pnpm@10.33.0 --filter @fusion/dashboard typecheck` - `corepack pnpm@10.33.0 changeset status --since=origin/main` - `git diff --check origin/main...HEAD` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Compound Engineering Code Review now caps remediation attempts at 2; after two unsuccessful attempts, the process parks instead of retrying indefinitely. - Post-restart review recovery now completes in a single maintenance cycle to reduce delays. - Default post-review fix budget increased from 3 to 10. - Review revision limits now consistently honor workflow-authored defaults when settings are left empty, and `0` disables automatic remediation. - **Documentation** - Updated the workflow editor, settings reference, workflow steps, and operator panel text to clarify cap/default/disable semantics (including CE: 2). - **Tests** - Added/updated unit tests to validate the new bounded remediation behavior and messaging. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
72391c90b2 |
fix(engine): route workflow reviews through validator models (#2533)
## Summary - classify review-type workflow steps with the existing review-step classifier - resolve their primary, fallback, and thinking-level settings from the validator model lane - retain per-step model overrides and executor-purpose workflow-step tooling - keep ordinary workflow steps on the execution lane - make missing-fallback diagnostics identify the correct lane ## Why Code Review, Plan Review, verification, and inline-review gates were executed through the implementation model lane merely because they run inside `executeWorkflowStep()`. That defeats configured reviewer-model separation and can make the same model implement and validate its own work. This changes model selection—not the workflow-step session/tooling contract—so review steps remain executor-purpose sessions while using validator lane models. ## Verification - `FUSION_PG_TEST_SKIP=1 corepack pnpm@10.33.0 --filter @fusion/engine exec vitest run src/__tests__/executor-workflow-step-model.test.ts` — 14 passed - `corepack pnpm@10.33.0 --filter @fusion/engine typecheck` - `corepack pnpm@10.33.0 changeset status --since=origin/main` - `git diff --check origin/main...HEAD` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Review-type workflow steps now route through the configured validator model lane (instead of the execution lane). * Validator primary/fallback and thinking-level settings are applied correctly for review steps. * Step/task overrides still take priority over lane-based resolution. * Fallback retry sessions now use the appropriate validator/executor configuration, with lane-specific fallback guidance when fallback settings are missing. * **Tests** * Expanded executor workflow-step model resolution and routing/fallback precedence assertions for validator-lane behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9a8fc409ff |
fix: persist manual task pauses (#2536)
## Summary - persist an explicit `userPaused` latch when operators pause tasks through CLI, MCP, dashboard task routes, or mission stop - keep automatic/internal pauses distinct (`userPaused` remains false unless explicitly requested) - clear the latch on unpause - route the flag through in-memory and PostgreSQL task stores - add contract coverage across core, CLI, MCP, dashboard task routes, and mission stop ## Why A manually paused task could lose the reason for its pause across dashboard/runtime restart. Startup recovery then treated it like an internally interrupted task and reclaimed it, restarting automation against the operator’s intent. Manual pauses must survive restart and remain non-runnable until explicitly unpaused. ## Verification - core pause durability tests: 2 passed - CLI task/extension tests: 150 passed; PostgreSQL integration lane remains active in CI - dashboard route tests: 261 passed - `@fusion/core`, `@runfusion/fusion`, and `@fusion/dashboard` typechecks passed - full workspace build passed with pnpm 10.33.0 - changeset validation and `git diff --check` passed - live aggregate runtime verification also confirmed `paused=true,userPaused=true` survived a normal dashboard restart with zero active tasks <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Manual task pauses now persist across application restarts and recovery. - Pauses initiated via the CLI, dashboard, MCP tools, and mission stop controls are recorded as explicit user actions. - Automatically paused tasks remain eligible for recovery. - Unpausing clears the durable manual-pause state. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
3ff98aae56 |
U12 part 8: delete the lossy normalizeColumn + behaviour ratchet — and the definitive answer on the raw flag (2 reads left, both U2b's) (#2535)
## U12 part 8 — deletes the lossy `normalizeColumn`, and ratchets it shut Independent of the #2525 → #2528 → #2530 stack; touches only `@fusion/core` exports. This closes **one of the two `@deprecated (workflowColumns, U12)` markers** the unit was named for. ### The hazard `normalizeColumn` coerced an arbitrary value to a **legacy** column, rewriting every workflow-defined custom id to `triage`. Silent data loss for any project whose workflow declares a column outside the six built-ins — and it sat one line away from `normalizeColumnId`, which sanitises structurally and passes real ids through. The dashboard picked the wrong one for its entire task-ingest path until that was diagnosed; `useTasks.ts` and `routes-trait-rekey.test.ts` still carry the notes from that fix. So this is not a hypothetical footgun — it already fired once, on the surface where it mattered most. Deleted rather than left deprecated because it has **zero callers anywhere in the workspace**. It was pure exported hazard: a lossy coercion next to its safe twin, waiting to be picked again. ### The ratchet is the point `no-lossy-column-coercion-export.test.ts` bans the **behaviour, not the identifier**: it walks every exported single-argument function whose name mentions "column" and fails if one maps a valid custom id onto a different legacy id. Re-adding `normalizeColumn` under any name trips it. Verified by actually reintroducing the function — **two of the three cases fail, including the name-agnostic one**. That last detail is what stops it being a guard that checks nothing. Coverage stated plainly: deleting an unused export has no behaviour to revert-check. The compile is the proof it had no callers; the ratchet is the proof it cannot return. --- ## Answering the standing question: does anything still read the raw `workflowColumns` flag? **Yes. Exactly two sites, and both are U2b's.** I am not able to close this out, and here is the complete list rather than a summary: ``` packages/core/src/store.ts:38,43 ← the definition packages/core/src/task-store/moves.ts:9,363 ← `useWorkflow` packages/core/src/task-store/workflow-task-create-ops.ts:11,351 ← move-policy preflight ``` That is the whole list in production code. Everything else that greps is a comment, a test that writes the flag deliberately to exercise the dead path, or the unrelated `workflowColumns.*` i18n namespace for the Columns editor panel. **Why I have not deleted the settings key.** It cannot go while those two read it — the key is what they read. And the two are not separable from each other: `workflow-task-create-ops.ts:351` computes the `movePolicyPreflight` that `moves.ts` consumes and validates, and un-gating the preflight alone would start evaluating workflow move policies (with their plugin-gate side effects) while the branch that consumes the result stays off. That is a behaviour change with no consumer, which is worse than either state. **Status of the blocker.** U2b has not landed. `main` at `919f68f9b` still has both reads; the program's merged history goes `#2466 → #2467 → #2468 (characterisation only) → #2469 → #2479 → #2500 → #2512 → #2513`, with no convergence PR. PR #2468 was Phase A2 **steps 1–2 only** — the differential characterisation — and the convergence that deletes one of the two move paths was never merged. So the honest state of the unit: everything U12 owns is done except the two reads that U2b owns, and the settings key that cannot be deleted until they are gone. If you want me to take U2b itself, say so — I have the inventory and the divergence list, and I would want the current U2b worker stood down from `moves.ts` first. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
18d654a5ff |
capacity, part 3: delete the globalMaxConcurrent setting, API and UI (#2529)
Part 3 of the capacity simplification, and the half that removes the **knob**. Enforcement (shared semaphore, runtime wiring) went in #2509; this removes everything an operator or API client can still see, so nothing is left readable-but-ignored. ## Deleted Settings key + schema default · CentralCore’s `getGlobalConcurrencyState` / `updateGlobalConcurrency` / `acquireGlobalSlot` / `releaseGlobalSlot` and the `concurrency:changed` event · the whole Global Concurrency block in `async-central-core` · `PUT /api/global-concurrency` · the Scheduling · Global settings section · the footer and Command Center global sliders · the dead `getGlobalConcurrencyLimit` reader whose only caller went in #2509. ## Kept, deliberately **`GET /api/global-concurrency` survives as telemetry only** — live `currentlyActive` / `projectsActive` from CentralCore’s side-effect-safe source. “How busy is this machine?” is still a real question once the cap that used to answer it is gone. It no longer reports `globalMaxConcurrent`/`queuedCount`: those came from the deleted cap and from slot bookkeeping production code never incremented, so publishing them was publishing zeros dressed as state. **`useGlobalConcurrency` becomes read-only.** Everything that existed to *persist* went with the cap — the 500 ms debounce, the save-state machine, the commit-on-close/unmount flush, the slider clamp, the `interactive` gate. The module-level shared store is **kept**: its original justification (two mounted consumers drift apart with private copies) holds for a polled read exactly as it did for a cap, and one fetch now serves both. The live “N running (all projects)” readout survives in both surfaces, moved onto the per-project row. ## Two sections become one Scheduling · Global existed to host exactly one control. With it deleted the section renders an empty pane, so the Global/Project pair merges back into **“Scheduling”**. An empty nav entry is a promise of settings that are not there. ## One real fix found on the way `SchedulingSection`’s `concurrencyLoading` gated the **project** concurrency inputs on the **global**-concurrency fetch — never the right source, since `maxConcurrent` and `maxWorktrees` come from the settings form. It is repointed at the form’s own load, preserving the invariant it existed for: a concurrency input stays disabled until its live value arrives, so an operator cannot overwrite a resolved limit with a blank fallback. ## Migration A stored `globalMaxConcurrent` is **ignored** — it is a project-blob key nothing reads, so dropping it needs no schema change. The `central.global_concurrency` **table** is dropped in a follow-up; this slice stops seeding and reading it first, so that drop has no live writer to race. ## Verification, and how the wider suite was controlled `pnpm lint` clean · core/engine/dashboard `tsc` clean · `pnpm test:gate` green (309 + 10 + 71) · dashboard settings/footer/command-center/hooks **2237/2237** · core `central-core-backend` 9/9. The broader dashboard suite shows failures, and I checked rather than assumed: running the suspect files on **clean main** reproduces `api-git` (49), `TaskDetailModal.rendering` (28) and `settings-mobile` (17) identically. Two were genuinely mine — `SettingsModal.scheduling-merge` (0 on main, 17 on this branch: my nav rename) and one `settings-mobile` picker case asserting `scheduling` is a scoped pair — and both are fixed. Tests for deleted behaviour are removed with it (footer confirm/cancel/flush/dedupe, global marker geometry, the hook’s PUT case, the CentralCore slot cases), each carrying a note on what it guarded and where the surviving **project-side** equivalent lives. Fixture-only references were updated, not deleted. Nothing booted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7003dc9803 |
U12 part 6: Board re-rendered every column on every state change — one inline arrow, measured with a memo-comparator probe (#2528)
## U12 part 6 — Board re-rendered every column on every state change
**Stacks on #2525.** Merge that first.
`canDropTask` was allocated as a fresh inline arrow, per column, per
render:
```tsx
canDropTask={(taskId) => canDropTask(taskId, columnDef.id, selectedWorkflow.id)}
```
`Column` is `React.memo`, and a new function identity on any prop
defeats that entirely. So **any** Board state change — collapsing
Archived, changing Done sort, opening the workflow switcher —
re-rendered every column and every card beneath it, not just the
affected one.
Bound through a `useMemo` cache keyed by lane + column. After the fix,
collapsing Archived re-renders exactly one column: `archived`.
### Measured, not guessed
I instrumented `React.memo`'s comparator to print which props actually
change identity on a collapse toggle. For every unaffected column the
answer was exactly one:
```
PROBE todo changed: canDropTask
PROBE in-progress changed: canDropTask
PROBE in-review changed: canDropTask
PROBE done changed: canDropTask
PROBE archived changed: canDropTask,collapsed <- the one that should re-render
```
After:
```
PROBE archived changed: collapsed
```
### Why this hid, and why my first attempt failed
Two things worth recording, because both were mistakes I made in this
program:
**The test was pointed at dead code.** "keeps unaffected columns stable"
measured the **legacy single-lane board**, whose props were all stable —
so it passed for a long time while covering nothing operators use.
Deleting that board in part 1 repointed it at the real board, where it
failed 3-vs-2. I skipped it then rather than weaken it to the observed
number, and said it needed its own investigation. This is that
investigation.
**My first fix was wrong and I was right to revert it.** In part 1 I
tried a `useRef` cache invalidated by `useEffect`, it did not fix the
test, and I reverted it as unproven rather than ship it. The reason is
now clear: the effect runs *after* the render that populated the cache,
so it wipes the very bindings that render created and the next render
allocates fresh ones — the invalidation defeated the cache. `useMemo`
keyed on the resolver has no such window; the map lives exactly as long
as the closure owning it.
### Revert-proof
The test is un-skipped **with the fix, not with a new expected number**.
Restore the inline arrow at either call site and it fails 3-vs-2 again.
### Verification
`pnpm test:gate` (309 + 10 + 71), `pnpm lint`, dashboard typecheck
green. Board, Board.canDropTask, workflow-resolved-columns and
board-no-legacy-flash: 132 passed, 0 failed, **0 skipped** — the skip
introduced in part 1 is gone.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Move menus now show exactly the destinations permitted by each custom
workflow, including non-adjacent moves.
- Invalid or hidden destination columns are excluded from move options.
- Older workflow data continues to use a compatible fallback behavior.
- **Performance**
- Improved board responsiveness by preventing unaffected columns and
cards from re-rendering when archived sections collapse or Done sorting
changes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
da0351857e |
U12 part 5: put real workflow adjacency on the wire — custom-workflow move menus were guessing (measured), and the VALID_TRANSITIONS shortcut is gone (#2525)
## U12 part 5 — the move menu was guessing; now it asks the graph **Stacks on #2521** (same file). Merge that first. The context menu had **no adjacency data at all**, so it did two wrong things at once: it approximated move targets from a column's **neighbours in declared order**, and — because that approximation is strictly weaker than the real graph — it kept a `VALID_TRANSITIONS` shortcut for any workflow whose column-id set matched the six built-ins. Measured, the approximation loses real operator moves: | current | workflow graph | neighbour approximation | |---|---|---| | `in-progress` | in-review, todo, triage, done | todo, in-review | | `todo` | in-progress, triage, archived | triage, in-progress | | `done` | todo, triage, archived | in-review, archived | So **every custom workflow has been offering a guess**: menu entries the store would reject, and legal moves it never offered. The built-ins were fine only because the shortcut bypassed the guess entirely. ### The fix `BoardWorkflowColumn` gains `moveTargets`, resolved by `resolveAllowedColumns` — *the same resolver `moveTaskInternal` validates against*. The menu now offers exactly what the store will accept, for any workflow. Threaded through all four metadata builders (Board, Lane, ListView, TaskDetailModal). Optional on the wire, deliberately: a client older than this field keeps the neighbour fallback rather than losing its move menu mid-upgrade. ### Why deleting the legacy shortcut is safe Not an assertion — a measurement, then a pin. `resolveAllowedColumns(BUILTIN_CODING_WORKFLOW_IR, c)` is **identical to `VALID_TRANSITIONS[c]` for all six columns, order included**: ``` triage ["todo","archived"] == VALID SAME todo ["in-progress","triage","archived"] == VALID SAME in-progress ["in-review","todo","triage","done"] == VALID SAME in-review ["done","in-progress","todo","triage"] == VALID SAME done ["todo","triage","archived"] == VALID SAME archived ["done"] == VALID SAME ``` `builtin-adjacency-matches-legacy-transitions.test.ts` pins it so the equivalence cannot drift silently — if the built-in workflow's edges change without `VALID_TRANSITIONS` following, default menus change shape and that test fails first. It compares **order** too, since the menu renders targets in the order it receives them, so a reorder is operator-visible. Default-workflow menus are therefore byte-identical. Custom ones stop guessing. ### What's left of the legacy vocabulary here `COLUMNS` is gone from `TaskContextMenu` — deleting the shortcut removed its last use. `VALID_TRANSITIONS` survives for exactly one thing: the **no-metadata load window**, documented at the site. I measured removing that in #2521 and it left Task Detail with no move options during load, which is a regression rather than a cleanup. It retires when the load window does. ### Revert-proof, two ways - Drop the `declaredTargets` branch → the custom-workflow case fails: the neighbour fallback returns `["backlog","building"]`, missing the legal `shipped` jump **and** offering `backlog`, which that graph forbids. That is exactly the defect class shipped to every custom workflow today. - A second case pins that an adjacency edge into a column the board cannot show is **dropped**, not rendered as a dead menu entry. ### Verification `pnpm test:gate` (309 + 10 + 71), `pnpm lint`, `pnpm verify:fast`, core + dashboard typechecks green. **No new test failures**: five suites report 31 failures with and without the change — an identical, pre-existing set, verified by diffing failing test *names* against a stashed clean tree, not by comparing counts. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Move menus for custom workflows now show only the destinations permitted by that workflow. * Task-specific workflow rules are applied consistently across boards, lists, lanes, and task details. * Invalid or unavailable destinations are excluded from move options. * Existing clients remain supported when workflow destination data is unavailable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
919f68f9bc |
test(U9): cover the two unguarded merge safeguards and admit them to the gate (#2526)
**U9, PR5.** Closes the gap #2520 measured. Tests + gate config only; no production behavior change. ## The gap #2520 found that safeguards **1 (user pause)** and **4 (capacity single-flight)** had **zero test coverage**. Deleting either guard produced no new failure anywhere in the merge, project-engine, self-healing, or concurrency suites. Both guards work correctly today — nothing would have noticed if they stopped. U9 moves merge behind graph nodes, so this is exactly the state not to convert on top of. ## Two tests - **`merge admission excludes a user-paused card`** — safeguard 1, the pause invariant re-ratified in #2486. Without the `paused || userPaused` filter, the admission provider offers a user-paused card to the merge pump. - **`drainMergeQueue is single-flight`** — safeguard 4. Asserted via `reconcileStaleMergeActive`, the first statement *inside* the guard, so the probe isolates the guard rather than dispatching a real merge. (Driving a real drain crashed the vitest worker; probing the guard directly is both safer and more precise.) **Both are two-sided** — they assert the guard blocks *and* permits. A one-sided test would still pass against a guard that rejects everything, which is a real failure mode for a filter. ## Proven by mutation delta Baseline fail-set vs mutated fail-set on the identical selection, NEW failures only: | Mutation | NEW failures | |---|---| | remove the pause filter | **1** — the pause test, and only it | | remove the single-flight guard | **1** — the single-flight test, and only it | | filter rejects *everything* | **1** — proves not one-sided | | drain *always* refuses | **1** — proves not one-sided | ## Gate admission `project-engine.test.ts` joins the `engine-core` allow-list. **One file proves five safeguards** — user pause, `autoMerge:false`, capacity single-flight, the pre-enqueue merge-proof consult, and at-most-once enqueue. Before this, **none of the six safeguards was defended by blocking CI**. A regression surfaced only in non-blocking full-suite, after the merge. Measured, not assumed: | | Files | Tests | Wall (3 runs) | |---|---|---|---| | before | 17 | 309 | 5.19 / 5.51 / 5.19s | | after | 18 | 412 | 6.19 / 6.24 / 6.21s | **+~1.0s against a ~60s ceiling.** **Verified the gate fires**, rather than assuming the allow-list edit took — the failure mode greptile caught in #2494: - remove safeguard 1 → `pnpm test:gate` **exits 1** (1 failed / 411 passed) - remove safeguard 4 → **exits 1** likewise - restored → **exits 0** Deterministic: store, runtime, merger and notifier all mocked; no real git, no network, no real timers in these two cases. ## Reversible calls I made rather than asking - **Added to `project-engine.test.ts` rather than a new file.** A dedicated file would need ~200 lines of duplicated `vi.mock` scaffolding; reusing the existing harness also means one gate admission covers five safeguards instead of two. - **Did not wait for U8.** These guard code that exists today and the conversion needs them in place first. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ebc89310bc |
U12 part 4: derive the move menu's "Back to" label from workflow traits (plus two legacy reads I did NOT delete, with measurements) (#2521)
## U12 part 4 — the move menu's "Back to" label followed hardcoded
column ids
`getTaskMoveTransitions` is shared by Board cards, List rows and Task
Detail. It labelled a backwards move with:
```ts
column === "in-progress" && task.column === "in-review"
? t("taskDetail.move.backToInProgress", "Back to In Progress")
```
Two hardcoded lifecycle ids **and** a hardcoded English column name. On
a workflow that renames those lanes the condition never matched, so the
affordance silently vanished — and had it matched, it would have
announced "In Progress", a column absent from that board. Same
legacy-vocabulary class U10 removed from Board and U12 removed from
ListView, surviving in the context menu all three surfaces render.
Now keyed on the traits it was approximating: the **current** column
carries `mergeBlocker`, the **target** carries `countsTowardWip`, and
the label interpolates the column's own name through a new
`taskDetail.move.backTo` key (added to all six locales).
### Scope I deliberately held back
**The set of moves labelled "Back to" is unchanged.** For
`builtin:coding` the traits resolve to exactly `in-review` and
`in-progress`.
I first generalised this to "any target earlier in the workflow's
declared order" — arguably nicer, and I had it working. Then I measured
it: it relabels moves this change never set out to touch. **18 assertion
sites across three suites** flip from "Move to" to "Back to" (e.g. a
card in In progress gets "Back to Todo", "Back to Planning").
Same-set-different-derivation is the honest scope here; widening which
moves read as backwards is a separate, visible product decision, not a
side effect of a vocabulary fix.
### Two things I chose not to delete, and why
Both are still-live `VALID_TRANSITIONS` reads in this file. Neither is
removable today, and the reason is the same missing wire field —
documented at both sites rather than left as a puzzle.
**1. The default-column-set shortcut.** `TaskContextMenuColumnMetadata`
carries id/label/flags but **no adjacency**, so the workflow branch can
only guess targets from a column's neighbours in declared order.
Measured against the real graph that is a strict loss:
| current | `VALID_TRANSITIONS` | neighbour-derived |
|---|---|---|
| `in-progress` | in-review, todo, triage, done (4) | todo, in-review
(2) |
| `todo` | in-progress, triage, archived (3) | triage, in-progress (2) |
| `done` | todo, triage, archived (3) | in-review, archived (2) |
Deleting that read is not a cleanup — it drops real operator moves
(archive from Todo, straight-to-Done from In progress). Note the guard
keys on the column **id set**, so a workflow that merely renames the six
built-ins still takes this path and still gets correct targets; only
reordering or replacing them falls through to the weaker logic.
**2. The no-metadata fallback.** I removed it first, on principle, and
measured the result: `workflowMoveColumns` is optional at both call
sites (`workflowMoveMetadata?.moveColumns`, `taskMoveColumns`) and
genuinely undefined until board-workflows resolves, so dropping it left
Task Detail with **no move options during load**. That is a live surface
degraded to satisfy a purity rule, so it is not shipped. Unlike Board
and ListView — where the legacy path was provably unreachable — this one
is reachable and useful.
Both retire the same way: put each column's allowed targets on the
board-workflows payload so the load window has real data instead of a
guess. That is a server + wire + client change and belongs in its own
slice.
### Revert-proof
The renamed-workflow fixture declares `signoff` (mergeBlocker) and
`building` (countsTowardWip). Restore the id literals and the new case
fails — `Move to Building` instead of `Back to Building` — which no
relabelling of the old hardcoded string could satisfy, since that string
names a column absent from the board. The same case asserts the forward
move keeps "Move to Shipped", so the rule stays a distinction rather
than a blanket relabel.
### Verification
`pnpm test:gate` (309 + 10 + 71), `pnpm lint`, `pnpm verify:fast`,
dashboard typecheck green.
**No new test failures**, established properly: the three suites this
touches report 30 failures both with and without the change, and I
diffed the failing test *names* against a stashed clean tree rather than
comparing counts — the sets are identical. (An earlier count-only
comparison had me chasing two failures that turned out to be my own new
assertions.)
Also regenerates `packages/i18n/src/resources.d.ts` via `pnpm
i18n:types`. That picks up **~45 lines of pre-existing drift** from
earlier merges that did not regenerate it; the file is generated, and
leaving it stale would omit the new key from the types. Flagged so the
extra lines are not mistaken for scope creep. Note
`packages/dashboard/app/locales/` is gitignored (copied from
`packages/i18n/locales/`), so only the canonical locales are committed.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
99be8e6153 |
docs(U9): correct the safeguard baseline — safeguards 1 and 4 are NOT covered (#2520)
**U9, PR4.** Docs-only correction to a document already on `main` (#2511). No changeset. ## I got #2511 wrong, and it matters #2511's table claimed all six merge safeguards were verified. **Two of them were not**, and the error is the same family this program exists to stamp out: I reported the **absolute** failure count under mutation, with **no baseline**. `merge-error-recovery.test.ts` (10 failures) and `self-healing.test.ts` (1) are **already red on clean `main`**. The "11 failed" I credited to the row 1 mutation *was that pre-existing red*. The mutation added nothing. I even flagged the identical `11 failed` on rows 1 and 3 as "a red flag" in my own notes and then did not chase it. ## Re-measured as deltas Baseline fail-SET vs mutated fail-SET on the identical selection, reporting only NEW failures, each named: | # | Safeguard | Baseline | Mutated | **NEW** | Verdict | |---|---|---|---|---|---| | 1 | user pause | 11 | 11 | **0** | **NOT COVERED** | | 2 | `autoMerge:false` | 0 | 9 | **9** | covered | | 3 | dependency gating | 0 | 5 | **5** | covered | | 4 | capacity single-flight | 10 | 10 | **0** | **NOT COVERED** | | 5a | merge-proof (pre-enqueue) | 0 | 1 | **1** | covered, thin | | 5b | file-scope | 0 | 6 | **6** | covered | | 6 | at-most-once | 0 | 3 | **3** | covered | **Four hold. Two do not.** - **Safeguard 1** is the pause invariant re-ratified in #2486. Removing `task.paused || task.userPaused` from the merge admission provider admits a **user-paused card into the merge pump** — and nothing fails. - **Safeguard 4** is the single-flight guard that serializes merge. Removing it permits concurrent `drainMergeQueue` entry — and nothing fails. Both guards **work correctly today**. What is missing is any test that would notice if they stopped. That is exactly the state U9 must not convert on top of — and #2511 said the opposite. ## Also corrected: nothing here is defended by blocking CI The one gate-admitted file (`merger-merge-lifecycle.test.ts`) is not the file that proves any surviving row. Rows 2/5a/6 rest on `project-engine.test.ts`, row 3 on core's `task-merge.test.ts` — neither is in the gate (core's gate is two PG tests via `test:pg-gate`). ## Three distinct ways the first pass was wrong All recorded in the doc, because each produced a confident wrong answer: 1. **Absolute counts with no baseline** — rows 1 and 4. A mutation run must diff fail-sets and report only new failures. 2. **Too-narrow selection** — an earlier pass measured rows 1 and 3 at zero and I nearly filed two false gaps. Widening fixed row 3 but is also how the pre-existing red crept in. Both directions need the baseline diff. 3. **A harness that silently matched nothing** — the delta harness's regex required a `|project|` segment in vitest's `FAIL` line. `@fusion/core` does not emit one, so it parsed **zero** failures at both baseline and mutation and printed "NOT COVERED" for row 3, which is covered by 5 tests. A verification tool that reports success without checking anything is worse than no tool; it must be tested against a known-failing case first. The harness now aborts on a no-op patch, asserts a clean restore, and I validated its parser against a known-failing run before trusting it. ## Next **PR5 writes the missing tests for safeguards 1 and 4**, then gate admission. Neither should wait for U8 — they guard code that exists today, and the conversion needs them in place first. That is the reversible call I'm making rather than asking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
5de083ef08 |
U8 PR4: declare the pending-review park as a graph node (inert) — and why the behavior move is blocked on the step-session chain (#2519)
Fourth PR of **U8 — the graph owns execution**. This is the IR half of
the pending-review routing move. **Inert: no behavior change.** The
behavior half is deliberately NOT in this PR, for a measured reason
below.
## What lands
A `review-handoff` seam node (`review-pending-handoff`, column
`in-review`) in `BUILTIN_CODING_WORKFLOW_IR`, with:
```
execute --outcome:review-pending--> review-pending-handoff --success--> end
```
An implementation session can end because a step is blocked on a pending
review: the agent cannot continue, and the card belongs in review rather
than in an error bucket (`status: failed` on an `in-review` row
deadlocks the merge queue). Today the **executor** performs that
transition inline, mid-session, and the graph finds out afterwards —
which is why `handleGraphFailure` carries `alreadyFinalizedToReview`, a
classifier whose only job is recognising a move the graph did not make.
Two design points worth recording, both verified against the interpreter
rather than assumed:
- **The edge goes to `end`, not to `review`.** Routing to the ordinary
`review` node would have continued the run into `merge-gate` and
`merge-attempt` on work whose steps are incomplete. "Hand off and stop"
is what the inline handoff does; the edge to `end` is what preserves it.
- **`outcome:` edges match on the node's VALUE and take priority over
generic `success`/`failure` edges** (`shouldTraverseEdge` /
`traverseChildren`). So this claims only the pending-review ending, and
a workflow that does not declare the edge falls through to its generic
`failure` edge — exactly today's behavior. That is what makes the
eventual move safe for user-authored graphs.
## Why the behavior half is not here — a measured finding
I implemented it, and backed it out. The record matters more than the
diff:
1. **`BUILTIN_CODING_WORKFLOW_IR` is not the default workflow.** It
backs `builtin:legacy-coding`; `builtin:coding` uses the
*stepwise-final-review* IR, which has no `execute` node — its
implementation runs as a `foreach` of `step-execute`.
2. **The foreach mechanism would work.** `runForeach` propagates a
failing instance's `value` up as the foreach node's own value, so a
`steps` node could carry an `outcome:review-pending` edge.
3. **But `stepExecute` flattens it first.** The seam returns `value:
result.outcome === "success" ? "step-done" : "step-failed"`, discarding
the exit before it can reach any edge.
So on the default workflow the exit cannot reach an edge, and a compat
classifier in `handleGraphFailure` keyed on the failure value cannot see
it either. **Removing the inline handoff therefore regressed the default
path**: the card stopped reaching `in-review` at all.
`executor-step-session.test.ts`'s FN-5436 case caught it —
```
FAIL FN-5436: pending-review skip on no-fn_task_done exit
> parks in-review when review request has no subsequent verdict
expected "moveTask" to be called with [ 'FN-5436-B', 'in-review' ]
Number of calls: 0
```
I could have made that green by relaxing the assertion. That would have
been appeasement of a test that was telling the truth, so the behavior
commit came out instead.
**Also caught, and worth noting as the ratchets earning their keep:**
the PR1 ownership ledger flagged the change as `runImplementation` 3 → 2
review handoffs and `handleGraphFailure` 0 → 1 — i.e. a *relocation*,
not an elimination, for every non-plain-`execute` shape. That number is
what turned "this move is good" into "this move is only good for one
workflow shape". And PR3's routing-unchanged pin plus its out-of-band
adjacency ratchet both fired, forcing the routing change to be declared
rather than slipping in.
## PR5
Thread the implementation exit through the step-session chain
(`runImplementationPhase` → `graphStepRunOnce` → `runGraphTaskStep` →
`runProjectedGraphTaskStep` → `stepExecute`) so the seam can return
`review-pending` instead of flattening to `step-failed`; add the node +
edge to the stepwise IRs; then flip the execute seam and delete the
inline handoff **in one correct step** for every built-in shape at once.
The compat path for user-authored graphs is then a single named
classifier rather than a call buried two thousand lines into a session
loop.
## Verification
- `builtin-coding-workflow-ir` + `builtin-workflows` — 76 tests green
(the layout-completeness contract required a layout entry for the new
node; it is placed off the main line because the park is an exit, not a
stage)
- `executor-step-session` + ownership ledger + exit events — 50 tests
green, unchanged
- `pnpm test:gate` green (309/10/71); `pnpm lint` clean
- Changeset included (`patch`, `internal`)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
e9bfd0d313 |
test(engine): prove the admission-control agent count on a renamed board — and two of my own cases were vacuous (#2516)
Test-only. Fifth E2E family. Closes three of the four `live-agent-count` classifications. ## Why this one is a scheduler bug, not a display bug `live-agent-count.ts` classifies a card's column, and `persistedTopLevelAgentSlotsFromStore` turns that into **the number admission control compares against the cap**. So a mis-classified column fails in whichever direction hurts: | mis-classification | consequence | |---|---| | wip column not recognised | under-count → **over-admits past the operator's cap** | | complete column not recognised | a finished card counts forever → **board silently stalls** | Both are silent, and both land only on a renamed board. Everything in the path is real: PostgreSQL store, real persisted workflows, cards walked through the real transition policy, and the real counting function resolving each card's own IR. Nothing about counting is reimplemented here. ## Two of my own cases were vacuous — mutation-testing caught it This is the more useful half of the PR. **1. "does not count a card in the COMPLETE column" passed with the terminal classification hardcoded to `done`.** `isRunningAgentTask` rejects that card at the *wip* check anyway, so the test was really asserting "shipped isn't a wip column". `terminalKind` short-circuits **first**, so it only changes the answer for a card whose status would otherwise make it count. Now covered by a complete card carrying a live-looking `planning` status — the state a crashed run leaves behind, which on a renamed board consumes a slot forever. **2. The review/merge lane had no case at all.** A review status is deliberately *not* globally live (a stale `fixing` in wip must not consume capacity), so it is gated on `columnIsReviewOrMerge`. If the renamed review lane isn't recognised, a genuinely-active reviewer stops counting and admission control lets another agent in over the cap. Now covered by a review card with an active merge-pipeline status. Both new cases assert their fixture took effect first, so they can't degrade back into the weaker version silently. ## Mutation-verified independently | classification | mutation | result | |---|---|---| | `countsTowardWip` | → `"in-progress"` | 3 renamed cases fail | | `complete` | → `id === "done"` | exactly the new terminal case fails | | `mergeBlocker` | → `id === "in-review"` | exactly the new review case fails | ## What this does NOT cover, stated plainly The fourth classification, `columnIsIntakeOrHold`, is read only by the **waiting** predicate, which the admission count never calls. It stays in the ledger as unproven rather than being claimed by proximity — the mistake I made last slice with `resolveMergeOrchestrationColumn`. ## Verification - five live-E2E suites green together: **52/52** - engine `tsc --noEmit` clean - `pnpm test:gate` green (309 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added end-to-end coverage for live agent-count admission across workflow lanes and lifecycle states. * Verified slot handling for active, completed, held, and mid-review tasks, including stale statuses. * Confirmed mixed-lane counts and renamed board vocabularies produce consistent results. * **Documentation** * Expanded coverage notes for live agent-count classifications and waiting-state behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
063978c289 |
U12 part 3: make the v1-IR persistence unconditional — after this, every raw-flag read is on the move path (U2b) (#2513)
## U12 part 3 — every remaining raw-flag read is now on the move path **Stacks on #2512** (shares a line in `workflow-ops.ts`). Merge that first. **Behaviour-preserving. Not a single persisted byte changes.** ### What changed The three v1-IR rollback-compat persist sites (#1405) all read `flagOn ? ir : downgradeIrToV1IfPure(ir)`, where `flagOn` came from the retired raw `experimentalFeatures.workflowColumns` key. No production writer sets it, so **every real project has always taken the downgrade arm**. Removing the branch is a runtime no-op; it deletes three flag reads. Sites: `createWorkflowDefinitionImpl`, `updateWorkflowDefinitionImpl`, and `insertWorkflowDefinitionSyncImpl` — whose `flagOn` *parameter* is gone too, along with the plumbing that resolved it in `migrateLegacyWorkflowStepsImpl`. With those gone, **`TaskStore.workflowColumnsFlagOn()` has no callers and is deleted.** Its six readers were the three U5 guards (part 2) and these three persist sites. ### The decision I made, and why I went the other way I had this slice scoped as "retire the v1 downgrade." **I rejected that.** It is a compatibility affordance, not cutover machinery: it fires only for a graph exactly equivalent to pure v1 (default columns, default placements, no v2-only features), and `upgradeV1ToV2` re-reads it into an identical v2 graph, so the runtime never sees a difference. Retiring it would break a binary downgrade for zero benefit — and stale binaries opening these databases is an **observed event** in this project, not a hypothetical. So the slice became the strictly better version of itself: same three flag reads removed, no compat surface touched. ### Why this matters for sequencing `isWorkflowColumnsCompatibilityFlagEnabled` survives. It is still read by `moves.ts:363` and by `workflow-task-create-ops.ts:351`'s move-policy preflight that feeds it. Removing those reads **is** the U2b move-path convergence with its equivalence-proof obligation. The point of deleting the wrapper is that it makes the remainder enumerable: ``` $ grep -rn isWorkflowColumnsCompatibilityFlagEnabled --include=*.ts packages/ | grep -v __tests__ packages/core/src/store.ts:38 <- the definition packages/core/src/task-store/moves.ts:9,363 <- U2b packages/core/src/task-store/workflow-task-create-ops.ts:11,351 <- U2b (feeds moves.ts) ``` **Every surviving read is on the move path.** U2b deletes the definition and the unit closes. ### On coverage — stated honestly This change is behaviour-preserving, so it has **no revert-proof test**, and I am not going to claim one. `flagOn ? ir : downgrade(ir)` with an always-false flag *is* `downgrade(ir)`. What needed a guard is the next edit someone is tempted to make — deleting `downgradeIrToV1IfPure` as dead cutover machinery. New `workflow-ir-v1-rollback-persistence.test.ts` fails if it is removed, and pins the exact boundary: the built-in coding workflow (named columns + traits) stays v2; a pure-v1-equivalent graph stores as v1 without the synthesized `columns`; a downgraded graph re-parses to an **identical** runtime graph (the property that makes unconditional application safe); a graph with a custom column stays v2. ### Verification `pnpm test:gate` (307 + 10 + 71), `pnpm lint`, `pnpm verify:fast` (17 steps), typecheck green. Core workflow-named suites: 383 passed, 1 failed — `workflow-ir-settings.test.ts > moved-key catalog ...` (`expected 10 to strictly equal 3`), which I confirmed fails identically on a stashed clean tree. Pre-existing, unrelated. No Fusion instance booted. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved workflow persistence compatibility by consistently storing pure v1-equivalent workflows in the compatible format. * Preserved v2 workflows and custom column information when they are not v1-equivalent. * Retired obsolete feature-flag checks without changing stored workflow or board behavior. * **Tests** * Added coverage for workflow version preservation, rollback-compatible serialization, and custom columns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |