9ad3a2a93abc2eae3e78108dfb6e2bbf97c63634
279 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3a016b1f17 |
fix(scripts): four FNXC stamps carried hour 26, and main has been red on them (#3010)
## `main` is currently red on `check-fnxc-future-dates` Four stamps read `2026-07-30-26:10` — an hour that cannot exist. They're exactly what #2995 taught this gate to catch. That PR landed the hour validation (`00-23`) *after* #2999 had already merged these four, so the gate started reporting a defect that was already sitting there rather than one introduced afterwards. **The guard is working**; nothing was checking before it. ``` scripts/lib/backend-db.mjs:41 scripts/reconcile-task-state-consistency.mjs:8, :51 scripts/__tests__/reconcile-task-state-consistency.test.mjs:109 ``` Corrected by **literal normalisation** — 26:10 on the 30th *is* 02:10 on the 31st — rather than flattening them to an arbitrary in-range hour. AGENTS.md specifies `yyyy-MM-dd-hh:mm`, and the stamp exists to give a readable why-does-this-exist trail, so the ordering is the part worth preserving. ## The baseline tightening rides along, and it's a date rollover Stamps written yesterday as `2026-07-31` were future *then* and were baselined as such. Today they're past, so **176 files ratchet to zero**. Nobody did anything. The gate rewrites the baseline as a side effect and exits 0, so leaving it uncommitted dirties the tree on every subsequent run **for everyone** — which is why it belongs in this commit rather than a later one. Re-recording on a decrease is the rule this gate and its siblings already state. Worth knowing about the design, since I wrote it: this churn recurs whenever a day boundary passes with future-dated stamps in the baseline, and it shrinks only as people stop writing them — which is the behaviour the gate exists to produce. **93 files still carry a non-zero allowance**, so the drain isn't finished. If it stays noisy once those clear, the gate's fail-on-tighten contract is the thing to revisit, not the stamps. ## Measured | check | result | |---|---| | gate | red before, **exit 0 after**, stable across two consecutive runs | | baseline | −176/+25 entries, all date-rollover | | inert-seam · sql-literal · lane-wiring · census | all green | | reconciler's own suite | green | ## One correction to a claim I made earlier this session While investigating I reported the gate as hanging for 600s. It wasn't — the harness killed the process (exit 144) and the empty output made it look like a stall. The gate completes in seconds. Noting it because I nearly filed a performance bug against a healthy script. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ffe9898710 |
fix(gate): the lane-wiring census could not see three of the shapes it asks for (incl. a false positive it started reporting) (#3013)
**The gate could not see three of the shapes it exists to check** — including the wiring I landed this week in #2990 and #3004. Found by using it: the baseline still listed `useBlockerFanout.ts`, `ExecutorStatusBar.tsx`, and `TaskDetailModal.tsx` as unwired *after* those PRs merged. ### 1. Conditional shapes Passing lanes only when they resolved is the **correct** way to write these — an empty trait index means "not loaded yet", not "nothing is terminal", so the caller must fall through to the documented legacy default rather than fabricate one. Both idioms that produces were invisible: ```ts computeBlockerFanoutMap(tasks, flags ? { columnFlagsByTaskId: flags } : {}) // ConditionalExpression computeBlockerFanoutMapCore(tasks, N, { ...(flags ? { classify } : {}) }) // SpreadAssignment, name === undefined ``` Either branch supplying the lane now counts. **Neither branch supplying it is still unwired** — that negative is tested. ### 2. Vocabulary `columnFlagsByTaskId`, the per-task trait index the dashboard threads, was never added. It answers every lane question at once, so a call site dropping it reverts to the legacy vocabulary wholesale — and the gate would have stayed silent. ### 3. Name collisions — the false positive Declarations were `set` by name, so the **last one parsed won**. Core's `computeBlockerFanoutMap(tasks, n, opts)` and the dashboard wrapper `computeBlockerFanoutMap(tasks, opts)` put their lane options at **different argument indices**, so core's callers were checked against the wrapper's signature: `task-priority.ts:141` passes `terminalColumns` at index 2 and was reported unwired. I caught this because adding the vocabulary entry in (2) made it appear. A ratchet that reports a correctly-wired site is worse than one that misses it — the first person to open one learns the number is noise. Both shapes are now merged; a call satisfying either counts. ### Measured | | | |---|---| | new tests | **6** — every positive paired with its negative | | against the old census | **3 of 6 fail** — exactly the three positives; the negatives pass either way, which is why they exist | | baseline | **23 → 19** — four sites recognized as *already* wired; no site newly excused | | new flags | none | ### Verification `node --test scripts/__tests__/check-lane-wiring.test.mjs` **16 passed** · `pnpm test:gate` 161 + 13 + 487 + 71 · lint · lifecycle census `--strict` · fnxc-dates (TZ=UTC) · changesets — green. Seven other `scripts/__tests__` files fail on main independently of this change (`verify-fast`, `dependency-security-floor`, `engine-vitest-gate-policy`, `plugin-authoring-docs`, `release-prompt-gate`, `ci-test-shard-timings`, `workflow-reliability-release-check`). None are in the merge gate and none are touched here — noting them because I looked, not because this PR affects them. |
||
|
|
f10261f424 |
fix(scripts): the contamination audit scanned four legacy lanes and claimed it had (#3005)
## An audit that scanned four legacy lanes — and claimed it had
Two halves of the same wrong answer.
**The query allowlisted the lanes:**
```sql
WHERE deleted_at IS NULL AND "column" IN ('triage','todo','in-progress','in-review')
```
On a board whose lanes are named anything else that matches **nothing**,
so the audit scans zero rows and reports zero contamination — a clean
bill of health from a scan that never happened. `triage` is in that list
too, a lane U11 (#2515) deleted.
**And the report asserted the coverage it did not have:**
```js
scannedColumns: ["triage", "todo", "in-progress", "in-review"],
```
printed regardless of what the query returned. When I first surveyed
this script I called that field "the one thing keeping it from being
fully silent" — it turns out it was a **claim, not an observation**, so
it was not keeping it honest at all. It is now derived from the rows
that came back.
## Fix: exclude finished lanes instead of allowlisting active ones
Inverted so the default is the safe one — an unrecognised lane is active
work by assumption and **is** audited; only lanes that genuinely mean
finished drop out. An allowlist fails **closed** (skip everything
unknown), a denylist fails **open** (look at it), and for an audit one
extra finished branch is a far smaller error than auditing nothing.
Filtered in JS rather than by building a dynamic SQL exclusion: it keeps
**one** place deciding what "finished" means, and removes the last
raw-SQL lane literal from this file.
## Revert proof
```
✖ scannedColumns reports the board's real lanes, not a fixed legacy claim
✖ reports each scanned lane once, and nothing at all for an empty board
ℹ pass 1 ℹ fail 2
```
## A demonstration of #3000, for free
This PR removes a 4-literal raw-SQL clause, and
`check-sql-column-literals` here reports **22, unchanged and green** —
because this branch predates #3000 and the gate still walks `packages/`
only. That is precisely the blind spot #3000 closes, reproduced a second
time.
## Merge order
This removes the 4 literals #3000 baselines. Landing this **after**
#3000 drops that count and its gate fails on DECREASE — that gate
auto-rewrites the baseline and asks for the commit, unlike
`check-lane-wiring` which needs an explicit `--update-baseline`. Either
order works; one of them needs a re-record, and I am happy to push it.
## Verification (measured)
- `node --test` — **3 passed / 0 failed** (1 pre-existing + 2 new)
- `node --check`, `eslint` — clean
- `lifecycle-column-census --strict`, `check-lane-wiring`,
`check-fnxc-future-dates` — green
No changeset: root `scripts/` is repo tooling, not part of the published
package.
## Territory status
This was the last item I know of in `scripts/`. The four operator
scripts holding lane assumptions — `recover-stale-blocked-by` (#2992),
`reconcile-task-state-consistency` (#2994),
`reconcile-leaked-soft-deletes` (#2999) and this one — are now either
resolved or, where a script genuinely cannot resolve lanes, made loud
rather than silent.
|
||
|
|
52a66297fc |
fix(gate): main is red — normalize #2994's four impossible-hour stamps (#3006)
**`main` is currently red on the FNXC gate.** ``` $ node scripts/check-fnxc-future-dates.mjs # on origin/main scripts/reconcile-task-state-consistency.mjs: 2 future-dated FNXC stamp(s), baseline allows 0 scripts/lib/backend-db.mjs: 1 scripts/__tests__/reconcile-task-state-consistency.test.mjs: 1 exit 1 ``` #2994 carried four `2026-07-30-26:10` stamps. I flagged them on that PR before it merged; #2995 (the hour check) landed first, so the merge order turned the warning into a red gate rather than a red PR. Clamped to `23:10` — same rule as the nine before it: hour to `23`, minutes preserved, so ordering within each file survives. This is a normalization with a stated rule, not a claim about the true minute. **Verified:** FNXC gate exit 0, `reconcile-task-state-consistency` 8 pass / 0 fail. Comment-text only. ### Worth fixing at the source Thirteen impossible-hour stamps across six PRs in two days, and the hours climb — `24:40` → `25:30` → `26:10`. They are being written as a continuing sequence past midnight rather than read off a clock, which is a reasonable instinct and produces an invalid stamp every time. The trap is that the honest spelling does not work either: a genuine post-midnight stamp needs *tomorrow's* date, and the gate compares against the **local** calendar — so `2026-07-31-00:40` written from UTC-7 is future-dated and fails for a different reason. Clamping to `23:xx` is currently the only spelling that satisfies both, which is not obvious and is why this keeps recurring. If it recurs again, the fix is probably in the error message rather than more normalization PRs: the gate could name the valid range and the timezone it compares against, so the next author sees the constraint at the moment they hit it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cd237ae760 |
gate: the SQL column-literal ratchet never scanned scripts/, where the raw SQL actually is (#3000)
## The gate could not see the one place raw SQL is actually written by hand `check-sql-column-literals` walked `packages/` only and took `.tsx?`. Every operator script is a repo-root `.mjs`. I found it by removing a raw-SQL lane literal in #2999 and watching this gate report: ``` [check-sql-column-literals] 22 known SQL column literal(s), none added. ``` Unchanged, and green. Its own header promises the opposite — *"a LOWER count fails too so the baseline is ratcheted down"* — so the silence was the tell. **Two changes, and either alone still sees nothing:** the root and the extension. Adding one without the other scans nothing new and reports a reassuring zero — the same trap #2978 hit when widening the lane-wiring census. ## Newly visible: 6 sites, audited not blind-baselined | site | verdict | | --- | --- | | `audit-branch-cross-contamination.mjs:182` — `"column" IN ('triage','todo','in-progress','in-review')` | **real** — the contamination audit scans only the legacy active lanes, so on a renamed board it scans nothing and reports no contamination. Read-only, and it does print its `scannedColumns`, which is the one thing keeping that from being fully silent. | | `reconcile-leaked-soft-deletes.mjs:53, :73` | already fixed by **#2999** — the PR that exposed this gap | ## Proven able to fail, not just to count A guard that has only ever printed a number is a number. A temporary `.mjs` holding one forbidden comparison: ``` scripts/zz-probe-tmp.mjs: 1 SQL column literal(s), baseline allows 0 ``` and the gate returned to green once removed. ## One claim I withdrew I initially wrote that the `ScriptKind` move to `JS` for `.mjs` was needed because *"TSX treats `<` as JSX and would misparse an ordinary comparison"*. I could not demonstrate it. I tried three JSX-ambiguous shapes — `x <div> y`, `f<b, c>(d)`, and a literal sandwiched between `<` and `>` comparisons — and TSX recovered from all three with counts identical to JS. So `JS` is used because it is the correct kind for the file, **not** because a miss was observed, and the code now says exactly that. The opposite claim would have been easy to make and wrong, and this gate's whole value is that its statements about its own coverage are true. ## Merge order **#2999 removes both literals in `reconcile-leaked-soft-deletes.mjs`.** Landing it *after* this PR drops the count, and this gate fails on DECREASE (by design), needing a re-record. Merge #2999 first, or say the word and I will re-record here. Note the widening is self-protecting afterwards: if someone narrows the walk back to `packages/`, the recorded `scripts/` entries vanish from the scan and the gate goes red on decrease. ## Verification (measured) - gate — green, **28 known / none added** (was 22 across `packages/` only) - its own suite — **32 passed** - `eslint` — clean - `lifecycle-column-census --strict`, `check-lane-wiring`, `check-fnxc-future-dates` — green Gate/tooling only; no product file touched. |
||
|
|
1de0141ab8 |
fix(dashboard): Task Detail's blocking count read the LEGACY lanes (last of the three fan-out surfaces) (#3004)
Third and last of the three surfaces calling the blocker fan-out wrapper, completing the sweep started in #2990 (Board + Executor bar). ## What was wrong, precisely The dependent **list** is lane-independent — core pushes `dependentIds` without consulting lanes — so this section looked broadly right. Two things beside it are not: - `overlapBlockedTodoCount`, rendered as **"FN-X is blocking N todo task(s) via blockedBy overlap"** — counted against the literal `todo`, so on a renamed board it read **0 while cards were genuinely blocked**. - the `stale` marker on each blocking dependent — decided against `terminal`/`review` lanes the operator does not use. A wrong number sitting beside a right list is the easiest kind to miss, which is why I checked what the modal actually consumes before deciding this was worth a PR rather than assuming the whole section was broken. ## Why a prop and not a hook This was the surface I deferred in #2990 because it had no trait index in scope. Two options: - `useBoardWorkflows` inside the modal — rejected. The hook documents that it does **not** dedupe across consumers: each call installs its own visibilitychange/focus listeners and its own SSE subscription. That is a new fetch and subscription per modal open, to answer a question the app has already answered. - **Thread the index that already exists** — `App` builds `footerColumnFlagsByTaskId` for the footer; this forwards it through `AppModals` as an optional prop. Chosen. Optional throughout: a card with no entry keeps the documented legacy fallback, so the remote-node case (where local workflow metadata must never be applied to foreign ids) and the pre-load window stay byte-identical. ## Reverted The new case fails on the rendered text — the modal cannot find `"FN-B is blocking 2 todo task(s) via blockedBy overlap"`. The pre-existing legacy-column case above it passes either way, because `todo` satisfies the literal default; that is exactly why it never caught this. ## Verification TaskDetailModal.rendering + ExecutorStatusBar + useBlockerFanout **206 passed** · dashboard app suite 11986 passed / 5 skipped (581 files) · `pnpm test:gate` 161 + 13 + 487 + 71 · lint · census `--strict` · lane-wiring · fnxc-dates · changesets — green. ## One note for whoever owns the FNXC gate `check-fnxc-future-dates.mjs` **rewrites its baseline as a side effect and still exits 0**. Today's date roll dropped 183 stamps out of "future", so any run dirties `scripts/lib/fnxc-future-dates-baseline.json` in the working tree. It cost me a stash conflict before I noticed. Not bundled here — it is repo-wide midnight drift, not this change — but a check that mutates tracked state on a read is worth a look. |
||
|
|
ac67b8d585 |
fix(scripts): the FN-4000 consistency reconciler failed in BOTH directions on a renamed board (#2994)
## The FN-4000 consistency reconciler failed in *both* directions
`findTaskStateInconsistencies` keyed both checks on legacy lane
literals, and they break in opposite ways:
```js
const hasDoneTransient = task.column === "done" && (status failed || error || worktree || blockedBy || …);
if (task.status === "failed" && task.column !== "in-review") { … }
```
| check | on a renamed board | effect |
| --- | --- | --- |
| `hasDoneTransient` | **never fires** | a finished card still holding
`status:"failed"`, a worktree, a blockedBy or live recovery counters is
never reported and never normalized — precisely the stale state FN-4000
exists to clear |
| `failed-status-outside-in-review` | **fires for every failed card** |
no column equals the literal, so the report lists the whole board |
The second is the more dangerous of the two: a tool that reports nothing
looks broken, but a tool that reports everything looks like it is
working.
## Wiring, and why the resolver is injected rather than built inline
Lanes are resolved **per task** (a board can span workflows) and passed
in. Resolving inside the loop would drag `importCore()` — and therefore
a built `packages/core/dist` — into every unit test of a pure
reconciliation loop.
`main` wires the real resolver whenever it opened a real backend, so
this is **not** the inert optional-parameter shape this migration keeps
finding. A caller injecting its own store (tests) has no staged dist and
falls back to the documented legacy literals, which is exactly today's
behaviour.
`importCore` is now exported from `scripts/lib/backend-db.mjs` so
operator scripts reach core helpers through the **same staged-dist seam
`openBackend` already uses**, rather than each growing its own dist path
— `@fusion/core` is not resolvable from repo-root `scripts/`, which is
what made the obvious import fail.
The normalization move now targets the card's **own** column: naming
`"done"` was only ever a way of spelling *"where it already is"*, since
the move exists to trigger the store's done-normalization.
## One of my test expectations was wrong before the code was
My first version asserted that a card in a renamed complete lane with
`status:"failed"` yields only the transient-state finding. It yields
**both** — and that is correct, because a failed card outside the review
lane genuinely is flagged. I isolated the case (dropping
`status:"failed"`, keeping the worktree) so it pins one behaviour
instead of blurring two, rather than "fixing" the expectation to match
whatever came out.
## Revert proof
Restoring the four literals:
```
✖ reports stale transient state in a RENAMED complete lane
✖ does NOT flag a failed card that is sitting in the board's own review lane
✖ runReconciliation normalizes a renamed complete lane by moving the card to its OWN column
ℹ pass 5 ℹ fail 3
```
The remaining two new cases pass both ways by design — "still flags a
failed card outside the resolved review lane" and "unresolved lanes keep
exactly the legacy behaviour" guard against over-correction, so I am not
counting them as coverage of the defect.
## Verification (measured)
- `node --test` — **8 passed / 0 failed** (3 pre-existing + 5 new)
- sibling script suites (`recover-stale-blocked-by`,
`reconcile-leaked-soft-deletes`) — **7 passed**, unaffected by the
shared-lib export
- `node --check`, `eslint` — clean
- `lifecycle-column-census --strict`, `check-sql-column-literals`,
`check-lane-wiring`, `check-fnxc-future-dates` — green
No changeset: root `scripts/` is repo tooling, not part of the published
package.
## Still not addressed in this territory
`reconcile-leaked-soft-deletes.mjs` carries a raw `UPDATE
project."tasks" SET "column" = 'archived'` — on a renamed board that
writes a column the workflow does not declare, creating the
undeclared-column state this migration keeps repairing elsewhere. It
holds a raw backend rather than a store, so it needs the same
`importCore` seam this PR exports; left for a follow-up rather than
bundled here.
|
||
|
|
65f4e8533e |
fix(dashboard): blocker fan-out classified every board against the LEGACY lanes (finished cards shown as blockers; escalation never fired) (#2990)
The dashboard's `computeBlockerFanoutMap` wrapper called core with **no
lane answers at all**:
```ts
return computeBlockerFanoutMapCore(tasks, MAX_AUTO_MERGE_RETRIES, {
staleHighFanoutAgeThresholdMs: options.staleHighFanoutAgeThresholdMs,
}); // no terminalColumns, no reviewColumns, no holdColumn, no classify
```
So every fan-out surface classified against `todo` / `in-review` /
`done` regardless of what the operator named their columns. Core defines
**active by exclusion — not terminal** — so on a renamed board a
**finished** card never became terminal and stayed an active blocker
forever. The Executor bar's highest-overlap blocker and the task modal's
blocking-dependents list both kept naming work that had already landed.
**Escalation was worse.** `shouldEscalate` requires the blocker to sit
in an escalation lane (wip ∪ review), which unresolved means
`in-progress`/`in-review` only — so a stale blocker holding up many
cards **never escalated**. The fan-out numbers themselves stayed
correct, which is what makes it easy to miss: the metric says there is a
problem and the mechanism that acts on it is switched off.
## Shape
**Per task, not a board-wide union** — the reason `blocker-fanout.ts`
documents on `classify`: an id means something only relative to its own
workflow, and this board renders several at once. `Board` builds the
index exactly as `App.tsx` already does for the footer
(`footerColumnFlagsByTaskId`): task → its own workflow → that workflow's
entry for the column the card rests in.
**Escalation = wip ∪ review**, mirroring `scheduler.ts`'s own
construction. The two must agree — the scheduler decides a blocker
escalates and the dashboard is where an operator sees it.
**An empty trait map means "not resolved yet", not "nothing is
terminal."** The pre-load window and the remote-node case keep the
documented legacy default rather than fabricated lifecycle state.
## Reverted
| case | reverted |
|---|---|
| a finished card in a renamed completion lane is not an active blocker
| **fails** |
| a stale high-fan-out blocker in a renamed wip lane escalates |
**fails** |
| unresolved traits stay byte-identical | passes either way — that is
why it is there |
## Two notes
- The hook call had to move below `useBoardWorkflows` in `Board` (it was
at line 206, the workflows at ~390). `blockerFanoutMap` is consumed only
in JSX, so the hook order change is unconditional and stable.
- The unresolved-card fallbacks are hoisted into three named helpers
with `DELIBERATE-LITERAL` markers on the **declarations** — the census
reads markers from leading comments, so an inline one attaches to the
wrong node and is silently ignored. Census baseline re-recorded in the
same commit (debt did not increase; markers moved 5 sites out of the
guard count).
## Not done
`ExecutorStatusBar` and `TaskDetailModal` call the wrapper directly and
still pass no traits. `ExecutorStatusBar` already receives
`columnFlagsByTaskId` so it is a one-liner; `TaskDetailModal` has no
trait index in scope and needs one threaded. Left out to keep this
reviewable — the ratchet keeps both visible.
## Verification
dashboard app suite **1919 passed (140 files)** · `pnpm test:gate` 161 +
13 + 487 + 71 · lint · census `--strict` · lane-wiring · fnxc-dates ·
changesets — green.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
cfe47b3754 |
chore(plugins): delete the superseded fusion-plugin-even-cards (#2790) (#2988)
Closes #2790 by finishing a decision that was already made and written down. ## The issue's premise was wrong, including as I filed it I raised this as "a package accidentally missing from `pnpm-workspace.yaml`." It wasn't. `CHANGELOG-archive.md:9596`: > Consolidate Even Realities plugin support into `fusion-plugin-even-realities-glasses` and **remove `fusion-plugin-even-cards` from the active workspace package list to avoid duplicate user-facing integrations.** The removal was deliberate, for a stated reason. The directory is what got left behind. That also rules out the option I had been weighting first — adding it back would undo a shipped consolidation and re-create the duplicate integration it was removed to prevent. ## Unreachable by every path | check | result | |---|---| | `pnpm-workspace.yaml` globs | no — never installed or built | | CLI bundle list (`packages/cli/tsup.config.ts`) | no — 0 mentions, while seven other plugins are named | | runtime `plugins/*` directory-scan discovery | none exists — plugins are enumerated explicitly | | `package.json` | `private: true` — never published | | imports outside its own directory | none | | kept as a docs/authoring example | no — zero references in `docs/` or any root `*.md` | | successor in the workspace | yes — `fusion-plugin-even-realities-glasses` | ## It was also polluting two ratchets Dead code in a scanned tree is worse than dead code: both censuses are **source-text scanners**, so they counted debt in files no build or typecheck covers. Nobody could retire those entries through a normally-verified refactor, and they inflated how much of the remaining debt looked actionable. Both baselines regenerated, and I checked each diff rather than trusting the totals: | baseline | change | |---|---| | `lane-wiring` | 26 → 23 sites, 21 → 20 files — **one entry removed**, `board-routes.ts: 3` | | `lifecycle-column-census` | exactly its two `board-cards.ts` entries | Neither regeneration tightened anything unrelated — worth confirming explicitly, because `lifecycle-column-census.mjs --strict` **writes** its baseline as a side effect and could have folded an unrelated drop into this commit. **Verified:** lane-wiring, SQL-literal and FNXC gates all exit 0 after the deletion; lint clean. 15 files removed. ## Why I went ahead I said twice I would not delete this unilaterally. What changed is that the trade-off dissolved — once the consolidation decision turned out to be documented and the "is it a teaching example?" question answered by a docs grep, there was nothing left to decide, only to execute. The deletion is git-reversible and the standing guidance is that reversible calls are mine to make. Fourth time today a thing I filed as "needs someone else's judgement" turned out to have its answer already in the repository. Cheap habit worth keeping: before deferring, grep for whether the judgement has already been made. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
684c324084 |
fix(gate): the lane-wiring census counted { reviewColumns: undefined } as wired (#2984)
## What Follow-up to the finding @gsxdsm left on #2981, taking the direction offered there. Both arms of this census asked whether the lane argument was **present**, not whether it carried anything: ```ts isThing(task, { reviewColumns: undefined }); // property present -> counted as wired isThing(task, undefined); // arity satisfied -> counted as wired ``` The callee receives exactly what it received before: nothing. The seam is still inert, the board still reads the legacy vocabulary — the census just stops saying so, which is the one failure mode a ratchet must not have. Same defect as the positional one #2981 fixes in `check-inert-flag-seams`, one level in. The two gates are complementary by design — this one owns the options-object and default-valued shapes the other is structurally blind to — so the hole had to be closed in **both**. Neither covered it, confirmed by probing each with a control shape. ## The direction I took, since the review raised it as a contract question > *tightening just relocates the dishonesty into whichever spelling survives... especially as I have already spent three attempts learning that heuristic tightening here trades false positives for worse false negatives.* Agreed, which is why this is the narrowest possible reading rather than a heuristic: **Only a literal `undefined` / `void 0` counts as empty.** Shorthand `{ reviewColumns }` forwards a variable whose value is not knowable from syntax, and treating it as unwired would flag every correct forwarding wrapper in the tree — exactly the false-positive wave that trains readers to skip a gate. Same for a call expression, a conditional, or anything else with a value at runtime. That keeps the rule provable from syntax alone. It doesn't relocate the dishonesty so much as remove the one spelling that is *demonstrably* empty; anything ambiguous still counts as wired, so the gate stays conservative in the direction that matters. ## No tests existed for this census `check-lane-wiring` and `lane-wiring-census.mjs` had no unit coverage on `main`, so both rules ship with tests rather than resting on the probe that found them. ## Measured | check | result | |---|---| | clean `main` | exit 0, unchanged — all five gates green | | now caught | property spelled `undefined` · property spelled `void 0` | | correctly **not** flagged | a real value · shorthand forwarding · a call-expression value · a middle `undefined` with a real argument after it | | new suite | **8 tests**; reverting both rules fails **exactly** the 3 positives, negatives hold | ## Not done here, deliberately The second finding on #2981 — `computeBlockerFanoutMap`'s dashboard wrapper dropping all four lane options, so the fanout display reads legacy literals on a renamed board — is **not** in this PR. Confirming the diagnosis: `useBlockerFanout.ts`'s `UseBlockerFanoutOptions` declares only `staleHighFanoutAgeThresholdMs` and forwards only that, and all three dashboard call sites (`Board`, `TaskDetailModal`, `ExecutorStatusBar`) have the same gap. One correction to how it's framed, though: core already has the right seam for it. `classify` and `escalationClassify` are documented there as *"the only correct option on a multi-workflow board"*, precisely because the set-shaped options assume a column id means the same thing everywhere. So the fix should thread **per-task classifiers**, not resolved column-flag sets — otherwise it reproduces the union read that this program's own learnings doc lists as the fourth failure shape. What's genuinely undecided is where a per-task role answer comes from in a sync render path: `Board` holds `columnDef.flags` for the *selected* workflow only, and the dashboard has no per-task resolver hook. That's the design call, and it's dashboard-batch work rather than a mechanical edit — so I've left it for whoever owns that batch rather than guessing at it inside a gate PR. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
16921fc518 |
fix(engine,core): role resolution was half-done in two shared lifecycle predicates (surfacing family + file-scope leases) (#2975)
The three surfacing sweeps stopped reporting anything for a card resting in a board's **second** review or hold column. A lifecycle role is a **trait**, and any number of columns may carry it. The shared runner resolved it with `resolveLifecycleColumns()[role]` — **first match** — then gated on it: ```ts const roleColumn = lifecycle?.[spec.role]; // FIRST column carrying the trait if (task.column !== resolved.roleColumn) continue; // everything else dropped ``` A workflow that splits human sign-off from the merge lane has two review columns; one that parks dependency-blocked cards separately has two hold columns. Cards in the second got **no stale-paused-todo, no stale-paused-review, no in-review-stalled** diagnostic — silently, with no error, on all three sweeps at once. ## The second bug hiding inside the fix for the first Resolving membership but still reading `roleColumns[0]`'s declared `recovery` applies the **merge lane's** threshold to a card sitting in the **sign-off** lane. Each card's policy now comes from its own column, and one of the new cases fails if it doesn't: the first role column declares a policy that suppresses the signal, the card's own column declares one that fires. ## Reverted | | | |---|---| | **6 of 12** new cases fail | `fires for a card in the SECOND column carrying its role` and `reads the recovery policy of the card's OWN role column` — × 3 sweeps | | the other 6 pass either way | non-regression halves: still fires for the FIRST role column, still does **not** fire for a card outside every role column. Membership must widen the gate, not move it. | The pre-existing 45 cases were all green throughout — the single-role-column fixture could not express the case, which is why the table-driven file that exists to stop these three sweeps drifting apart never caught it. ## Verification `pnpm test:gate` 161 + 13 + 487 + 71 · surfacing family 57 · core stale-paused 20 · lint · census `--strict` · sql-literals · fnxc-dates · lane-wiring · changesets — all green. ## Note `holdColumns` was missing from the lane-wiring vocabulary, so the gate could not see that argument dropped. Added in the same commit. While reviewing, I found and measured **two problems in #2974** (comment posted there): six of its newly-visible sites are `satisfies`-wrapped false positives, and baselining them means deleting a real `reviewColumns` argument keeps the count unchanged and the gate green; and its baseline predates #2970, re-opening the slot that PR closed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved stale-card detection across all applicable review and hold columns. * Cards are now surfaced using the policies configured for their specific lifecycle column. * Cards outside matching lifecycle columns are no longer incorrectly surfaced. * Preserved existing fallback behavior when no lifecycle columns are configured. * **Tests** * Added coverage for workflows with split review and hold columns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- ## Second commit: the same predicate, half-converted (`shouldHoldActiveFileScopeLease`) Folded in here rather than stacked — same file, same class, and a stacked PR on an unmerged base is not mergeable. Reversible; say the word and I'll split it. `shouldHoldActiveFileScopeLease` is the **scheduler's** lease predicate, shared with the self-healing repair paths deliberately so the two cannot disagree about who holds a file-scope lease. Its two role answers are optional parameters defaulting to the legacy ids. The scheduler's own call sites were converted to pass resolved answers; self-healing's two were not: ```ts const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; ``` On a renamed board neither branch matches, so the predicate returns `false` for every card. The scheduler kept the lease; self-healing saw none, cleared `overlapBlockedBy`, and **released a dependent to edit files another agent still holds** — the outcome `groupOverlappingFiles` exists to prevent. Membership comes from the wip/review sets each sweep already resolved a few lines above, so this adds no reads. **Reverted:** both new cases fail with `overlapBlockedBy` = `null` — the release itself, not a proxy. The pre-existing legacy-column case in the same file passes either way, because `in-progress` satisfies the literal default; that is exactly why it never caught this. Lane-wiring baseline re-recorded `9 -> 7` in the same commit (the ratchet refused a stale allowance, as intended). **Verification:** gate 161 + 13 + 487 + 71 · surfacing 57 · overlap-seam + scheduler-lease + query-blindness 79 · core stale-paused 20 · lint · census `--strict` · sql-literals · fnxc-dates · changesets — green. |
||
|
|
968af0822c |
gate: the lane-wiring ratchet did not scan plugins, dashboard/app, or any .tsx (#2978)
## The new gate re-opened a blind spot the old one had already learned about `check-lane-wiring.mjs` (#2966) scanned four roots and only `.ts`: ```js const ROOTS = ["packages/core/src", "packages/engine/src", "packages/dashboard/src", "packages/cli/src"]; ``` `unwired-lane-parameter-guard.test.ts` scans **six**, including `packages/dashboard/app` and `plugins`, and its FNXC note records exactly why: > `plugins` is scanned, and its absence was half of a real escape. […] an unwired `completeColumnsByTaskId` sat on `main` unreported: the guard found 0 across 1753 files, and 0 again across 2114 once plugins were added, because the shape was invisible too. **Fixing either alone would still have missed it.** That is the same trap here, and it needed **two** changes. Those trees are overwhelmingly `.tsx`, which the file filter excluded — so adding the roots without the extension would have scanned a handful of files and reported a reassuring near-zero. ## What the widened scan found: 10 sites, in 8 files, audited not blind-baselined | site | verdict | | --- | --- | | `dependency-graph/GraphTaskNode.tsx` (`isTaskStuck`) | **real** — `isTaskStuck` takes an optional 4th `columnFlags`; omitted, `isWipColumnRole` falls back to the literal, so **no card on a renamed board is ever shown stuck** in the graph | | `dashboard/app/Lane.tsx`, `ListView.tsx` (`sortTasksForDisplayColumn`) | **real**, dashboard batch | | `dashboard/app/ModelSelectorTab.tsx` ×2 (`resolveEffectiveExecutor`/`Validator`) | **real**, dashboard batch | | `dashboard/app/TaskDetailModal.tsx` (`isNearDuplicateCanonicalInactive`) | **real**, dashboard batch | | `even-cards/routes/board-routes.ts` ×3 (`boardToDeck`) | **cannot be fixed in place** — deprecated plugin depending on `@fusion/plugin-sdk` alone, with no resolution source | | `even-realities-glasses/routes/board-routes.ts:141` (`boardToDeck`) | **harmless by construction** — the `{ maxCards: 1 }` summary call slices `active` to empty, so `terminalColumns` cannot change its output; documented in `cards.ts` | They are baselined rather than fixed because they span three other batches. I did **not** fix the graph one despite it being my area: wiring it needs the plugin prop contract to carry column flags, and the plugin's own `dashboard-interop.d.ts` declares `isTaskStuck` with only three parameters — so it crosses the dashboard↔plugin API boundary rather than being a local change. ## Merge-order hazard, stated precisely A **decrease** also exits 1 (`process.exit(1)` on the `decreased` branch), and #2976 wires `packages/cli/src/commands/task-lifecycle.ts`, which is present in this baseline. **If #2976 lands after this PR, main's gate goes red** until the baseline is re-recorded. It fails loudly rather than silently, so it is a chore not a risk. Merging #2976 first and letting me re-record here is the cleanest order — say the word and I will push the re-record. ## Verification (measured) - `check-lane-wiring` — green, **19 known / none added** (was 9 across 4 roots) - `unwired-lane-parameter-guard.test.ts` — **9 passed**, the older guard is unaffected - `lifecycle-column-census --strict`, `check-sql-column-literals`, `check-fnxc-future-dates` — green Gate/tooling only; no product file is touched. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Expanded lane-wiring checks to cover dashboard and plugin code. * Added support for scanning `.tsx` files while excluding declarations, tests, specs, and ignored directories. * Updated baseline coverage counts for the additional files. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
41af5e5dbd |
fix(gate): the lane-wiring census could not see its own motivating case (#2956) (#2974)
#2966 shipped a gate that **cannot detect the defect named first in its own header.** `findLaneAcceptingFunctions` matched a lane parameter only when `param.type` was a `TypeLiteralNode` — an inline `{ reviewColumns?: … }`. But the real code declares these as interfaces: ```ts export function getInReviewStallReason( task: Pick<Task, …>, context: InReviewStallContext = {}, // TypeReference — invisible ): InReviewStallSignal | undefined ``` so the function never entered `accepting` and none of its call sites were examined. ### Measured, both directions | | before | after | |---|---|---| | lane-accepting functions detected | 20 | **30** | | `getInReviewStallReason` detected | no | **yes** | | re-introduce #2956 (drop `reviewColumns` from one call site) | `none added` — **passes** | **fails**: `reads.ts: 7 unwired now, baseline allows 6` | The gate now catches the thing it was built for. ### The baseline moves 10 → 24, and that number needs context `10 unwired call site(s) across 8 files` → `24 across 15`. **No entry was removed** — every previously-recorded file kept its count and 14 sites became visible for the first time: ``` core/task-store/reads.ts 0 -> 6 engine/self-healing.ts 2 -> 4 core/task-store/branch-and-pr-entities.ts 0 -> 1 core/task-store/task-update.ts 0 -> 1 engine/scheduler.ts 0 -> 1 dashboard/routes/register-task-workflow-routes 0 -> 1 cli/commands/dashboard-tui/bucket-mapping.ts 0 -> 1 cli/extension.ts 0 -> 1 ``` **These are newly VISIBLE, not newly broken** — they have been unwired all along. I have **not** audited them, and recording them in the baseline is not a claim that they are fine; it is the ratchet doing what its header describes, since the census's own note says roughly half of the original hits were legitimately unwired (identity proven by a stronger means, sentinel columns, dead exports). Someone should walk the 14. Two stand out as worth a look first: **`reads.ts` at 6** is the file #2956 was about, and **`scheduler.ts`** is a dispatch path. Flagging rather than fixing, because wiring a call site that should not be wired is its own defect and each needs the judgement call the census header describes. ### Regression test `packages/engine/src/__tests__/lane-wiring-census-named-types.test.ts` pins the detector's shape — named interface, type alias, inline literal, positional — against fixtures rather than live counts, so it does not churn when someone legitimately wires a call site. Plus one anti-vacuity case asserting the named-type arm is still load-bearing on real source (`getInReviewStallReason` resolves in the live tree), so the fixtures cannot pass while the tool has quietly stopped applying here. **Mutation:** removing the `TypeReference` arm fails **4 of 5**. ### Also worth knowing `findLaneAcceptingFunctions` still only visits `ts.isFunctionDeclaration` at top level, so `export const fn = (ctx) => …` remains invisible. I checked — no exported arrow function currently takes a lane argument, so nothing is missed today, and I left it rather than widen the surface in the same change. Resolved by **name across the corpus** instead of a type-checker `Program`: these are plain source scans and a checker would cost a full type-resolution pass for one lookup. Two same-named types merge, which only ever widens what counts as wired — safe for a ratchet. **Verified:** 5/5 new tests, `check-lane-wiring` clean at the new baseline, lint clean, FNXC gate exit 0. Core suite on main is green (4923 passed / 0 failed) — unrelated, but I had it running. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved lane-wiring analysis to recognize named interfaces and type aliases. * Added support for wrapped configuration expressions and positional parameters when detecting lane information. * **Tests** * Added comprehensive coverage for lane-wiring detection, including named contexts and live-tree validation. * **Chores** * Updated baseline counts to reflect newly recognized application areas and improved self-healing detection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
be79fe0db6 |
fix(cli): PR merges silently never ran on a renamed board — the blocker was asked about in-review (#2976)
## PR merges silently never ran on a renamed board
`processPullRequestMergeTask` called its injected blocker with the task
alone:
```ts
if (getTaskMergeBlocker(task)) return "skipped";
```
So `options.reviewColumns` was undefined and the blocker's identity
check fell back to `task.column === "in-review"`. On a board whose merge
lane is named anything else it returns:
```
task is in 'checking', must be in 'in-review'
```
…which is truthy, so this function returns `"skipped"`. **Silently and
permanently** — nothing logs, nothing fails, the PR simply never merges.
`daemon.ts`, `serve.ts` and `dashboard.ts` all drain PR merges through
here, making this a third instance of the #2963/#2964 class ("merge
entry points unwired — merging was impossible on a renamed board").
Found via the baseline #2966 shipped:
`packages/cli/src/commands/task-lifecycle.ts` was a known-unwired call
site in it.
## Narrow resolution, deliberately
`resolveReviewColumns` is the **broad** set, and its own FNXC note warns
that a caller which admits on it *and then moves the card* will act on
cards the engine does not consider in review. This function merges and
moves to the complete lane — a state-changing admission — so it uses
`resolveMergeOrchestrationColumn`, the single lane the engine acts on.
That matches how `moves.ts` wires the same call.
Degradation is unchanged in both directions: `resolveWorkflowIrForTask`
substitutes the default IR rather than throwing, so a default board
resolves `in-review` and behaves identically; a v1-upgraded IR resolves
every role empty and keeps the documented legacy literal (covered by a
test).
## One shape choice worth flagging
The option is always **passed** and conditionally **valued**:
```ts
getTaskMergeBlocker(task, { reviewColumns: mergeLane ? new Set([mergeLane]) : undefined })
```
rather than making the whole argument conditional. These are identical
at runtime — the blocker treats an undefined `reviewColumns` exactly as
it treats absent options — but **only this shape is visible to
`lane-wiring-census.mjs`**, which matches an object-literal argument and
cannot see a ternary. I wrote the ternary first, and the gate still
reported the site as unwired; wiring a gate cannot check is how this
defect survived in the first place.
The gate then confirmed the fix and asked for the baseline in the same
commit:
```
[check-lane-wiring] unwired call sites decreased:
packages/cli/src/commands/task-lifecycle.ts: 1 -> 0
```
Baseline re-recorded 9 → 8 in this commit, so the allowance cannot be
regrown into.
## Revert proof
**There was no test for this function at all** — that is why it went
unnoticed. Restoring only `task-lifecycle.ts`:
```
AssertionError: expected "vi.fn()" to be called with arguments: [ ObjectContaining{…}, …(1) ]
AssertionError: expected 'skipped' not to be 'skipped'
AssertionError: expected "vi.fn()" to be called with arguments: [ ObjectContaining{…}, undefined ]
Tests 3 failed | 1 passed (4)
```
The one case that passes both ways is "still skips a card that is not in
any merge lane" — it guards against over-admission rather than proving
the fix, and I am not claiming it as coverage of the defect.
## Verification (measured)
- new suite **4/4**; with `pr-automerge-cleanup` **9 passed / 2 files**
- `tsc --noEmit`, `eslint` — clean
- `check-lane-wiring` (8, none added), `lifecycle-column-census
--strict`, `check-sql-column-literals`, `check-fnxc-future-dates` —
green
**Changeset added** (`patch`). `packages/cli` is the published
`@runfusion/fusion` and this changes user-facing merge behaviour, so
AGENTS.md requires one. My first pass hedged and left it to a maintainer
— that was wrong, the rule is not discretionary, and it is now in the
branch.
|
||
|
|
2fd798cb36 |
core: every review card reported a false stall on a renamed board (#2970)
**The failure mode worth distinguishing: the rest of this family went
quiet on a renamed board. This one shouted.**
`getInReviewStallReason` satisfied its **own** lane check from
`context.reviewColumns` — then called `getTaskMergeBlocker` **without**
them. That helper re-ran its column-identity check against the literal
`in-review` and returned, for a perfectly healthy card:
```
task is in 'signoff', must be in 'in-review'
```
…which was surfaced as `{ code: "merge-blocker" }`. **Every in-review
card on a renamed board was flagged as stalled**, each citing a lane the
board does not have. That is how a signal stops being read at all.
## A second symptom, found by the revert rather than by reading
On a **genuinely failed** card, the identity message wins over the real
one. The operator saw the bogus column complaint instead of `task is
marked 'failed': merge verification failed`.
So it did not only invent stalls — it **masked the true reason for real
ones**. I would not have noticed that from the diff; it showed up
because the revert run asserted on the reason text.
## Same shape, last one in the family
The outer question was resolved and the inner one was not — the
half-conversion the helper's own comment records for `moves.ts`, and
#2963/#2964 fixed for the merge entry points. This is the last site the
audit turned up where the lane answer was already in scope and simply
not forwarded.
## Revert results
| | reverted → |
| --- | --- |
| the unforwarded call (what ships today) | **2 of 3 fail** — healthy
card reports a merge-blocker stall; failed card reports the wrong reason
|
**Fixture note worth keeping:** `paused` is deliberately *not* the
genuine-stall case. An earlier guard returns `undefined` for a paused
card before the merge blocker is ever consulted, so that case would pass
whether or not the lanes are forwarded — the vacuous shape this series
has produced eight times.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71; `@fusion/core` full suite **4878
passed** (457 files); `tsc` core clean; lint, lifecycle census
`--strict`, FNXC gate, changesets all clean.
|
||
|
|
19deb42170 |
gate: ratchet call sites that never receive the lane answer (#2966)
**This is the gap that let three defects reach `main` in one day.** `unwired-lane-parameter.mjs` catches a parameter that reaches **no** caller. It is deliberately satisfied by a mention *anywhere*, so **partial** wiring is invisible to it: | | | | --- | --- | | #2956 | `getInReviewStallReason` wired at **0 of 4** call sites while both siblings were wired | | #2963 | both merge entry points unwired — merging was **impossible** on a renamed board | | #2964 | merge-confirmed finalization unwired — **already-landed work parked `failed`** | Every one was a fix that added an optional parameter without the call-site sweep that has to follow it. The existing guard was green throughout, correctly by its own contract. ## A census, not a guard — and that distinction is the whole design Auditing the sites this finds showed **four of seven were legitimately unwired**: `skipColumnIdentityCheck` callers have already proven lane identity by a stronger means, a sentinel-column caller wants the identity check satisfied by construction, and a dead export has no caller to wire at all. A check that failed on those is ~57% false positives. The sibling guard's own header says why that is worse than a miss — *"it teaches people to disable the check"* — and I agree, so this does not do it. Instead it ratchets like the lifecycle census: **36 known unwired sites across 20 files**, allowed to shrink and not to grow. A new unwired caller raises the count and fails; wiring one lowers it and re-records. The recurrence — adding a caller that forgets the lane answer — is precisely what gets caught, and the legitimate sites cost one baseline line each instead of a permanently red gate. ## Detection is AST-based, deliberately It finds exported functions accepting a lane-named argument — directly *or* as an options-bag member — then finds call sites passing none of them. Not regex: the ad-hoc scan I used during the audit produced false negatives on multi-line calls, which is exactly how a caller gets missed in the first place. Using a heuristic to police a defect caused by a heuristic seemed like a poor trade. ## Verified to fail on the recurrence A ratchet that cannot fail is worse than none, so this was measured rather than assumed. Injecting one new unwired caller into `self-healing.ts`: ``` [check-lane-wiring] call sites not passing a resolved lane argument INCREASED: packages/engine/src/self-healing.ts: 9 unwired now, baseline allows 8 ``` exit 1, naming the file and the delta. ## Placement Runs as a named `check:lane-wiring` step in `pr-checks.yml` beside the lifecycle, SQL, inert-seam and FNXC ratchets — same convention, same failure ergonomics, ~1s. Note the baseline records today's state, which still includes the #2963/#2964 sites because those fixes have not merged yet. When they land the count drops and the baseline is re-recorded downward — the ratchet working as intended rather than a conflict. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `tsc` engine clean; lint, lifecycle census `--strict`, FNXC gate, and the new check all clean. |
||
|
|
126cee7e6d |
engine: finalization parked ALREADY-MERGED work as failed on a renamed board (#2964)
**The worst symptom in this family: the branch landed, and the board says the task failed.** `project-engine`'s merge-confirmed finalization spread the task's **real** column into `getTaskHardMergeBlocker` with no `reviewColumns`, so the identity check ran against the literal `in-review`. On a renamed board it returned `task is in 'signoff', must be in 'in-review'`, and the caller parked the card: ``` status: "failed" error: "Merge confirmed but finalization blocked: task is in 'signoff', must be in 'in-review'" ``` For work that had already merged. ## Its sibling had already solved this `auto-merge-finalization.ts` passes the **review-eligible sentinel** instead of the card's own column, with the reasoning recorded at that site: `getTaskHardMergeBlocker` asks *"is this card blocked by anything other than where it sits?"*, and its callers are recovery paths for landed work that a graph crash can leave resting in any column. `project-engine` simply never got the same treatment. ## One name instead of two spellings Rather than write the sentinel a second time, it is exported once as `REVIEW_ELIGIBLE_SENTINEL_COLUMN` next to the helper whose contract gives it meaning, and both recovery paths use it. **Two sites independently spelling a magic value is how one of them came to be missing it** — that is the actual root cause here, not the literal itself. This also answers the census, which flagged the new literal — correctly. Its guidance (which I wrote, in #2909) is to hoist a deliberate literal into a *declaration*, where a `DELIBERATE-LITERAL` marker actually attaches, instead of leaving it mid-expression where the marker is silently ignored. The shared constant is exactly that, and it lowers `auto-merge-finalization`'s literal count too. ## Revert result | | reverted → | | --- | --- | | sentinel replaced by the card's own renamed column | reproduces the shipped string | The middle test asserts that string deliberately — it is what landed in `task.error`, so a regression reports what the operator would actually have seen. A third case checks the sentinel does **not** suppress genuine blockers: incomplete steps still block finalization in any lane. These drive the helper directly; reaching `project-engine`'s finalization end to end needs a live engine, a merge run and a real repo, while the defect is entirely in *what the blocker is asked*. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `project-engine` + `auto-merge-finalization` + the new suite, 207; `tsc` clean on core and engine; lint, census `--strict`, FNXC gate, changesets all clean. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed merge-confirmed tasks being finalized correctly when boards use renamed workflow columns. * Prevented already-merged tasks from being incorrectly marked as failed due to custom review-column names. * Preserved enforcement of genuine incomplete-step blockers. * **Tests** * Added coverage for finalization on renamed lanes and legitimate merge blockers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
189f237a07 |
fix(security): make the approval and permission gates actually enforce (#2457)
## Why
An AI agent autonomously deleted a live task (FN-8609) via
`fn_task_delete`. Investigating that led to two audits, which found the
gate machinery is well-built and its *enforcement* well-tested — but the
*decision boundary* was unguarded and the tools that matter never
reached the gate at all.
The headline defect: all ~76 tools in `packages/cli/src/extension.ts`
are delivered through `hostExtensionPaths` → `DefaultResourceLoader`,
which never passes through `wrapToolsWithActionGate`. `fn_task_delete`
*was* classified as gated in `gating-classifications.ts` — that
classification was dead code. Roughly 15 destructive tools were ungated
even for an operator who had locked their policy all the way down.
Second: `POST /api/approvals/:id/decision` took the approver identity
from the request body, validated for shape only, with no check that the
decider was human or differed from the requester. Combined with a bash
tool whose command string was never inspected (only `params.cwd`), an
agent could read the daemon token from `~/.fusion/settings.json`, find
its own pending request, and approve it.
## Scope
This does **not** tighten defaults. The `unrestricted` preset is
byte-for-byte unchanged — `git diff` on `agent-permission-policy.ts` is
empty — and regression tests assert that an out-of-the-box install
behaves exactly as before. The bug was never "the default is too
permissive"; it was "strict policy doesn't enforce." This makes turning
security up actually work.
The one deliberate exception: the containment that stops an agent
escalating its *own* privileges (reading the daemon token / credentials,
calling the approvals API to self-approve) applies at every preset
including `unrestricted`. That is a privilege-escalation boundary rather
than a permission preference — if it only engaged under strict policy it
would not have prevented the incident that prompted this.
## What changed
8 bisectable commits:
- **Approval lifecycle** — self-approval blocked via server-derived
deciders; same-verdict replay 409s; decide re-reads and re-validates
inside the transaction; expiry TTLs; `markCompleted` ownership check;
session identity registry in core.
- **Engine gates enforce for real** — unclassified tools resolve to a
policy-governed category instead of hardcoded `allow`; missing-policy
fail-open closed; bash containment floor + exact-command approval
binding.
- **Dashboard decision routes** — stop trusting client-supplied actors
(decision, bypass-review, worktrunk → 403 on forged actors).
- **`fn serve` authenticated by default** — auto-mints a token following
the existing `fn dashboard` precedent; `--no-auth` opts out.
- **Sibling entry points closed** — user-sourced hard-cancel moves, ACP
execute-once approvals, plugin task-store gating.
- **pi-extension principal resolution** — the extension resolves the
acting principal and can withhold or policy-gate the previously ungated
destructive tools.
- **Root-cause bonus fix** — `findLatestByDedupeKey` was broken in
PostgreSQL backend mode (already-parsed jsonb fed through a string-only
parser), so approved-grant redemption **never matched in production**,
minting duplicate requests. This explains the live DB state of 17
approved / 0 completed. *(Also cherry-picked to `main` as `a9b30013bb`,
since it is an active production defect on its own.)*
- **Review follow-ups** (`627f1b1fa8`) — operator-configured
provisioning privilege and a configurable grant TTL; see below.
## Review follow-ups
**Provisioning privilege is operator-configured, not role-derived.**
`isCallerPrivileged` had gone from `caller.reportsTo == null` (every
top-level agent privileged — permanent escalation by creating a
manager-less agent) to `caller.role === "ceo"`, which swapped an
implicit rule for a magic string: any agent config can claim that role,
while an operator who genuinely wants a privileged agent had no
supported way to say so. Privilege now derives solely from
`agentProvisioning.trustedAgentIds` / `trustedRoles` and fails closed
when settings are unresolvable.
It is also no longer forwarded to `resolveAgentProvisioningPolicy` as
`isPrivileged`, because that flag short-circuits ahead of
`alwaysApproveDelete` — a trusted caller was bypassing delete approval
entirely. The policy applies the same trusted rules itself, in the right
order. The function now governs only the org-chart escape hatch (acting
outside your own direct reports).
**Grant TTL defaults to 1 hour and is configurable.** Approval →
redemption is not instantaneous: an operator approving from their phone,
an engine restart, a queued lane, or a task waiting on a worktree all
routinely exceeded 15 minutes, after which the grant expired and the
agent silently re-requested. One hour remains far short of the
"redeemable forever" hazard the TTL exists to bound. Override via
`FUSION_APPROVAL_GRANT_TTL_MS` or `configureApprovalRequestTtls()`;
invalid overrides are ignored rather than widening the window to
infinity or collapsing it to zero.
## Behavior changes requiring operator review before rollout
1. `fn serve` requires a bearer token by default (`--no-auth` opts out);
unauthenticated clients get 401.
2. Agents can no longer run withheld destructive tools
(`fn_task_delete`, `fn_task_bypass_review`,
mission/milestone/slice/feature/workflow deletes, `experiment_finalize`,
`skills_install`). Operators keep them via CLI/dashboard. **This is the
incident fix.**
3. Agents get provisioning privilege only when the operator lists them
in `agentProvisioning.trustedAgentIds` / `trustedRoles`; the
provisioning gate is now live in production. Previously-implicit
privilege (top-level position, or a `ceo` role) no longer grants
anything on its own.
4. Decision replay 409s (was 200); pending approvals expire after 24h,
approved grants after 1h (configurable); bash approvals bind per exact
command.
5. Forged/body actors on decision, bypass-review, worktrunk routes →
403; `archive-all-done` requires `{confirm:true}` (external scripts
affected).
6. `fn_secret_get` approvals grant exactly one reveal (previously
granted nothing and looped forever); ACP approvals are execute-once
(previously infinite reuse).
7. Bash containment denies token/credential/approvals-API commands in
all agent sessions at every preset.
## Verification
Independently re-run against the branch, not just self-reported:
- 5 typechecks (core, engine, cli, dashboard `tsconfig.json` +
`tsconfig.app.json`) — clean
- `pnpm lint` — clean
- `pnpm test:gate` — 379 passed
- `pnpm build --force` — green (a plain `pnpm build` skips packages as
unchanged and does **not** compile the branch)
- `pnpm check:changesets` — clean
- ~650 file-scoped tests including new negative-path suites for the
decision boundary, which previously had **zero** test coverage
`packages/engine/src/__tests__/plugin-runner.test.ts` fails 56/80 —
**verified pre-existing**, reproducing identically at base commit
`93a403af67` on `main`. Not in the merge gate.
### A mutation check that failed to fail
Worth recording, because it nearly shipped an untested security fix. The
first mutation check on the provisioning change reintroduced the `ceo`
hardcode and **all 17 tests still passed** — the tests asserted through
the policy path, which can no longer observe `isCallerPrivileged` at
all, precisely because `isPrivileged` is no longer forwarded there.
Org-chart cases that do exercise the function were added; the hardcode
now fails exactly 1 of 19, and restoring is green. A green mutation run
is only meaningful if the test can actually see the code under test.
## Known limitations (stated, not papered over)
- The bash containment floor is string-matching: a cost-raiser, not a
sandbox. Quoting, encoding, `$HOME`, symlinks, or an interpreter
one-liner can evade it. The durable protection is the decision route
refusing agent-originated deciders — the filter is the belt, not the
braces.
- Approval expiry is lazy (evaluated at decide/complete/redeem), not
swept, so an expired pending row stays visible in lists until touched.
- The extension's require-approval path returns a pending message but
cannot suspend a pi session mid-turn; engine-side pause hooks cover
engine lanes only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Security**
* Hardened approval and permission gating with server-side decider
attribution, self-approval blocking, ownership checks, replay/race
protection, and status/TTL enforcement.
* Added fail-closed behavior for sensitive/unclassified tools and
sandbox provisioning approvals.
* Blocked credential/approval access via bash containment; plugin
destructive task operations now require explicit permission.
* **New Features**
* `fn serve` now defaults to bearer-token auth, with `--no-auth` as the
explicit opt-out.
* **Bug Fixes**
* Improved task move-source attribution (`moveSource: "user"`) and
tightened dashboard archive/bypass confirmation and operator attribution
behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
1f0d371228 |
fix(tests): three more portal query-root failures (pr-tab, worktree-terminal, milestone-slice) (#2959)
Three dashboard test files asserted against `render()`'s `container`, but the components under test mount through `createPortal` — so `container` is **empty** and every query returns nothing. Same root cause as the earlier portal batch; these are the three that were still held back. | File | Before | After | |---|---|---| | `TaskDetailModal.pr-tab` | failing | pass | | `TaskDetailModal.worktree-terminal` | failing | pass | | `MilestoneSliceInterviewModal` | failing | pass | **Measured: 39/39 passing**, rebased on current main (`3461ae7a92`). Lint clean, FNXC date gate exit 0. ### Why this stayed hidden The queries were a **mix** of `container.querySelector(...)` and `screen.*`. `screen` queries `document`, so they kept working — a portal-mounted modal makes only the `container` half go blind. The result is a file that looks half-alive rather than obviously broken, and the failures present as five different-looking symptoms (`null`, `undefined`, `+0`, `[]`, `-1`) that don't read as one bug. Grouping candidate files by **`container.querySelector` call count** rather than by symptom is what identified these correctly, and — the part that mattered — correctly *excluded* the neighbouring files that were failing for unrelated reasons. ### One thing to know if you repeat this A blanket `container` → `document` replace is wrong: it also rewrites `renderResult.container.querySelector` into `renderResult.document.querySelector`, which is not a thing. That broke two already-passing tests on my first attempt. This uses two separate passes with a lookbehind so only the bare receiver is rewritten. ### Scope Test-side only — **no product code changes**, so no changeset. This does not fix the *cause* (tests are still free to query the wrong root); a lint rule for that is worth considering separately, but it would need to distinguish portal-mounting components from ordinary ones, and I did not want to guess at that boundary inside a test-fix PR. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved modal and task detail accessibility test reliability by querying rendered elements from the document. * Updated coverage for keyboard navigation, Pull Request status indicators, tab ordering, and onboarding provider cards. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
255741e9ab |
fix(gate): scan every file type that carries an FNXC stamp (#2954)
The walk's extension filter was `/\.(tsx?|m?js|cjs|md)$/` — the file types stamps were **expected** in, rather than the ones they **occur** in. Wherever the convention spread on its own, the gate could not see it. ## How I found it Chasing four stamps dated `2026-10-19` — three months out, so unlike the rest of the population they would not age out on their own. All four were in `packages/core/dist/`, which the gate correctly skips as generated. The *source* they were compiled from is a `.sql` migration, which the gate skips for a different and much worse reason: it was never scanned at all. ## Why `.sql` is the expensive omission A migration's stamp records **when a schema change landed**. That is the case where a wrong date misleads most — it is the file you read to reconstruct the order schema changes happened in. 69 migration files carry stamps; 10 were future-dated and none were visible. `.css` had drifted furthest by volume: **1023 stamps across 123 files**, almost all from the dashboard CSS split. `.html`, `.ya?ml`, `.json`, `.sh` are included too; they add coverage but contribute no baseline entries. ## The 9 new baseline entries are newly VISIBLE, not new 5 `.css` + 4 `.sql`. Every one predates this change and would have been caught had the gate ever looked. Recording them is a **reclassification**, the same distinction the census draws for its DELIBERATE-LITERAL marker — a baseline that grows here is the gate's coverage improving, not the codebase regressing. Reading the rise as a regression would be exactly backwards. ## Verified by mutation, not by reading - A future-dated stamp appended to `ChatView.css` → gate **exit 1**. - A future-dated stamp appended to `0036_chat_session_tags.sql` → gate **exit 1**. - Both reverted → **exit 0**. Without this, both probes pass silently. ## Two notes on the diff - **Zero removals.** My first attempt rewrote the baseline with sorted keys, which turned unmoved lines into add/remove pairs and made it look like entries were being dropped. Rebuilt in walk order so the diff is additions only. - `reads.ts` is deliberately left at `2` here even though it now measures `0`. That drop belongs to #2953; duplicating it across two open PRs is how this queue got tangled before. The gate auto-tightens it at runtime and still exits 0. ## What this does not fix The **478** future-dated stamps still in the tree. They are agent-written (mine included) and most are one or two days out, so the count falls on its own as the clock advances — it should not be read as cleanup progress. This PR only makes the gate able to *see* the SQL and CSS ones, so no new stamp can land there unnoticed. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8e5e1147d2 |
core,engine: the last literal lifecycle query — and the three stall signals that disagreed (#2951)
**This is the last one.** `surfaceInReviewStalls` was the final literal
`listTasks({ column })` in production — I verified it by direct scan,
not by census arithmetic: **1 remaining before this, 0 after.**
It tells an operator that a card is stalled in review. On a renamed
board the stall was real and the board simply never said so.
## It came last on purpose
Converting the read alone would have been **worse than leaving it**.
`getInReviewStallReason` gated on the literal `in-review` itself, so a
widened read hands every renamed-board card to a classifier that drops
it — the missed-pair class, wearing the shape of a clean one-line
conversion.
## What was actually there
Three sibling signals decorate the same row, and they **disagreed about
which lane it is in**:
| signal | before |
| --- | --- |
| `getInReviewStalledSignal` | singular `reviewColumn` — resolved, but
**first-per-role** |
| `getStalePausedReviewSignal` | singular `reviewColumn` — same |
| `getInReviewStallReason` | **no seam at all** — literal |
So one row could be judged in-review by one signal and not by another.
And the singular ones are the **arity trap**:
`resolveLifecycleColumns().review` is the *first* column carrying a
review role, so a board with a separate merge lane beside its
human-review lane had a second review column matching none of them.
All three now take `reviewColumns` (membership), resolved **once per
row** through `resolveReviewColumns` — the union of the three review
roles — so they cannot disagree by construction. The singular/literal
paths remain as the no-metadata fallback, so a caller passing nothing is
byte-identical to today. Ten call sites in `reads.ts` wired from that
one answer; the singular resolver is deleted.
## Revert results
Each applied alone and re-run:
| conversion | reverted → |
| --- | --- |
| the resolved read | fails — the card is never listed |
| `reviewColumns` at the call | fails — the classifier drops the renamed
card the widened read just found |
That second row is the whole point: it proves the pair had to move
together, which is the thing I got wrong twice earlier in this series.
## Second commit: a red on `main`, not from this branch
`check-fnxc-future-dates` landed and **`main` fails it** — verified by
running the script on a clean `origin/main` checkout rather than
inferring. Nine files carry stamps dated after today, so every worker's
gate fails on a check none of their changes caused. Several are mine: I
had been stamping tomorrow's date across this whole series, which is
precisely the out-of-order record the check exists to prevent.
Scope held deliberately: a repo-wide sweep touched **266 files** across
docs, scripts and every package. I ran it, backed it out, and limited
this to the nine files the check actually flags — a mechanical rewrite
that size during a queue freeze would conflict with every in-flight
branch, which is worse than the red it fixes.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71 (green **only** with the stamp
commit); `@fusion/core` full suite **4810 passed**; engine self-healing
+ blindness + both ratchets **758 passed**; `tsc` clean on core and
engine; `pnpm lint`, `check:changesets`, `lifecycle-column-census
--strict`, `check-sql-column-literals` and `check-fnxc-future-dates` all
clean.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Review-stall detection now recognizes renamed and multiple review
columns while retaining support for the legacy review column.
- Paused tasks continue to be excluded from stall detection.
- Self-healing review-stall sweeps now search all configured review
lanes and avoid duplicate task results.
- **Tests**
- Added regression coverage for renamed and legacy review-lane queries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
f49e487d91 |
feat(core): untraited-project lane opt-in — and main was red on the FNXC gate (#2949)
Two things, and the second is why the first does not ship alone. ## The opt-in `resolveProjectColumnsForRoles` gains `untraitedProject: "declared-columns"`. When **no** workflow in the project expresses **any** lifecycle trait, every declared column id joins the answer. This is the three-state rule at **project** scope — the last item on the deferred list, recorded at three self-healing call sites (#2869, #2876). A board that renames its lanes and declares no traits contributes nothing today, so its cards are **absent from every role-keyed query**, and the correct per-card fallback downstream never runs for them. A fallback cannot rescue a card the query never returned. **Not "no workflow declares this role."** A project that expresses traits and has no review lane has *answered*; widening there would invent lanes it deliberately lacks. Mutation-verified both directions — widening unconditionally fails 1 of 12, making the option a no-op fails 1 of 12. **Opt-in, not default**, because the safe direction differs per caller — the finding in `project-union-versus-per-task-lanes.md`: | caller | over-inclusion costs | |---|---| | sweep | nothing — the per-card check discards the extra rows | | aggregator | an inflated number an operator reads (#2864, #2866) | | action site | a card routed or notified under a vocabulary that is not its own (#2852, #2891) | Making it the default moves all three at once, in the one direction two of them must not. Verified byte-identical without the option, so this lands with **no caller changes** and each site adopts it on its own reasoning. ## Main was red, and my own gate caught me first I dated the new comments `2026-07-31` while today is `2026-07-30` — **the exact defect `check-fnxc-future-dates` exists to prevent, committed while writing the feature.** The gate I added yesterday failed my own commit. Correcting mine surfaced that the merged sentinel batch, #2947, and three engine test files carried future-dated stamps too, so **the gate was failing on `main` for everyone**, not just here. All corrected to real dates rather than raising the ceiling. The stamps were simply wrong, and a baseline bump would have recorded the error as permitted — which is the failure mode that ratchet exists to prevent. Core and engine `tsc` clean, `pnpm lint` clean, census `--strict` 0, FNXC gate 0 (469 known, none added), gate green (161/487/13/71). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b1bd571682 |
batch-sql-ratchet: the census / gate-ratchet family — collection branch, fold here (#2941)
## Family branch for consolidation directive item 4 `batch-sql-ratchet` did not exist and ~10 open PRs are waiting for a collection point, so this establishes it. **Fold your census/ratchet commit here and close your own PR as superseded.** ```bash git fetch origin batch-sql-ratchet git checkout -B batch-sql-ratchet origin/batch-sql-ratchet git cherry-pick <your-sha> # verify scoped, not full suite: pnpm --filter @fusion/core exec vitest run src/__tests__/archived-column-gate-parity.test.ts --silent=passed-only --reporter=dot git push origin HEAD:batch-sql-ratchet ``` **Candidates I can see open right now** (owners: please fold + close): | PR | branch | |---|---| | #2938 | `fix/comments-ops-sentinel` | | #2935 | `fix/task-artifacts-sentinels` | | #2933 | `chore/commit-tightened-census-baseline` | | #2931 | `fix/async-comments-sentinels` | | #2928 | `fix/audit-ops-sentinel-marker` | | #2925 | `live-task-column-lanes` | | #2923 | `fix/task-id-integrity-sentinel` | | #2921 | `fix/plugin-store-migration-marker` | | #2894 | `gate/sql-literals-match-census-placement` | That is **10 → 1** once folded. I have not cherry-picked anyone else's commits — folding someone's work without them verifying it is how a batch lands broken. --- ## What is in it so far (mine, from #2924) **Clears a live main red:** `archived-column-gate-parity` fails on `origin/main` today. ``` AssertionError: TypeScript encoding changed. async-comments-attachments.ts: 8 → 5 ``` #2886 fixed a real bug — archived-document guards failing in *opposite* directions on a renamed lane — by replacing three `column === "archived"` comparisons with `isArchivedLane(column, archivedColumns)`. The AST scan counts raw comparisons, so the tally dropped. **What I did not do is record it as three sites converted**, because measured, it is not: ``` grep -rn "archivedColumns:" packages/core/src packages/engine/src --include="*.ts" | grep -v __tests__ → (no matches) ``` No caller passes it. The parameter defaults to `LEGACY_ARCHIVED_LANES = new Set(["archived"])`, so every call resolves to the literal it replaced — byte-identical behaviour, resolved branch dead. That matters for this guard's whole argument: its header warns that converting the TypeScript half while the Drizzle and raw-`sql` halves still compare the string is a split brain *"no test would catch, because every builtin workflow spells the column `archived` so the two halves agree by accident on every board we ship."* **There is no split brain today precisely because the resolved half is unwired** — it becomes one the moment a caller threads real lanes in without the SQL sides moving. Recorded inline so `5` cannot be read as "3 sites done"; flagged on #2886. Verified not a split brain: the Drizzle and raw-sql inventories are unchanged and both pass — worth stating because those assertions run *after* the TypeScript one, so a plain red says nothing about them. Scoped edit to `AUDITED_TS_SITES` by line range: these paths appear in more than one inventory here, and an unscoped replace would quietly edit the raw-sql side too, making the parity guard agree with itself (the trap I hit in #2817). Guard still bites: appending a real `task.column === "archived"` to an audited file fails it. Core **4852 passed / 0 failed**, lint clean, test-only. Closing #2924 as superseded by this. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved task delegation messages when workflow pickup cannot be confirmed. * Delegation results now clearly indicate when a task has not been verified for pickup. * **Quality Improvements** * Added validation checks to catch future-dated markers and inconsistent SQL-column usage. * Refined workflow checks to distinguish stale configuration from incomplete configuration. * **Documentation** * Updated lifecycle conversion guidance with more accurate audit findings and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c3df0f641b |
executor: orphaned tasks were never resumed after a restart on a renamed board (#2947)
`resumeOrphaned` is the only path that recovers tasks after a crash or
restart. On a board with renamed columns it recovered **nothing**.
## A missed pair, not an unconverted read
```ts
const tasks = await this.listWipLaneTasks(); // resolved by role — already converted
const inProgress = tasks.filter(
(t) => t.column === "in-progress" && …, // literal — discards everything the read found
);
```
The read was already resolved. The filter directly beneath it
re-asserted the literal on the rows that read returned, so the sweep
found the orphans and threw them all away.
**This is the worse half of the class, and it hid well:**
- the read *looks* converted, so scanning for `listTasks({ column: "…"
})` finds nothing;
- the census scores only the comparison, so the backlog number moves the
**wrong way** as you convert;
- a **structural test already existed** pinning "the read asks for
resolved lanes" — `executor-resume-query-lanes.test.ts` — and it was
green the entire time the sweep was dead. A test asserting the read
exists says nothing about the filter beneath it.
The failure only surfaces after a crash, when an operator is already
investigating the crash and has every reason to blame that instead.
## The ratchet, generalised
#2944 ratcheted this class inside `self-healing.ts` after review found
one instance and a follow-up audit found five more. This generalises it
to every engine source: a function that resolves lanes **and** compares
a column id in the same body is a pair.
Excluded, deliberately:
- the **fallback arm** of a resolved ternary (`lanes ? lanes.has(c) : c
=== "done"`) — the correct shape;
- four files whose literals are deliberate, each with the reason
recorded: `ephemeral-worker-manager` (unresolvable-workflow default),
`triage` (the U11 orphan case), `scheduler` and `replan-target` (sync
listeners on the inert sync IR reader, already pinned by
`sync-workflow-ir-is-always-default.pg.test.ts`);
- `self-healing.ts`, because it has a **dedicated** ratchet that is
strictly more precise. Two ratchets allowlisting the same site is one
fact with two owners, free to drift — the exact failure mode this
program keeps hitting. One file, one ratchet.
It carries a positive control: a wrong source path would make every case
pass by scanning nothing.
**I swept the rest of the engine with it and executor.ts was the only
genuine hit** — everything else is documented-deliberate or blocked on
the inert sync reader.
## Revert results
Each measured by restoring the literal filter and re-running:
| | reverted → |
| --- | --- |
| behavioural case | fails — the renamed card is dropped and the sweep
returns before touching it |
| the ratchet | fails, naming the site: `resumeOrphaned:
executor.ts:5974` |
A non-vacuous companion (card in the review lane → not resumed) rules
out a filter that matches everything: a card in review has no session to
resume, and re-dispatching it would restart finished work.
**Measured:** `executor.ts` column guards 8 → 7; baseline re-recorded
downward.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71; executor prompt/soft-delete/resume
suites plus the new ratchet, 357 passed; `tsc` engine clean; `pnpm
lint`, `check:changesets`, census `--strict` and
`check-sql-column-literals` clean, each run explicitly.
|
||
|
|
8503a2b12f |
batch-census-sentinels: six sentinel-marker PRs in one (supersedes #2921 #2928 #2931 #2935 #2938 +1) (#2943)
Fifth family, not in the four you listed — it was about to sit while the others consolidated. **Six folded; two need arbitration.** ## Folded (cherry-picked clean) migration marker · async archived check · audited-sentinel missing its marker · five of six `archived` checks in one file · the two artifact/comment read-only guards · the last unmarked `getLiveTaskColumn` sentinel. One root cause, which is why they belong together: **a literal compared against a SENTINEL value is not a lifecycle-lane guard** — the census counts it, and the fix is a marker, not a conversion. ## The baseline conflicted on every cherry-pick All six re-recorded `lifecycle-column-census-baseline.json` independently. I resolved by **regenerating once from the folded tree** rather than merging six hand-edits: the baseline is a derived artifact, so the measured value is the only correct resolution, and hand-merging derived JSON is how a wrong ceiling gets locked in. That is the strongest case for the family model I can give you: six PRs touching one derived file conflict pairwise regardless of merge order — 15 possible pairs — and auto-rebase would have churned them serially. ## NOT folded — one line for arbitration **#2925 (`live-task-column-lanes`) conflicts with #2923 (`fix/task-id-integrity-sentinel`) on `packages/core/src/task-store/task-id-integrity.ts`.** #2923 marks a sentinel there; #2925 converts lanes. Different intents, same file. I did not guess which wins — land one, rebase the other, fold both after. ## Verification `--strict` exit 0 · backlog **158**, reviewed **122** · core typecheck clean · scoped, not full suite. ## Queue **52 → 39** after my two folds (this + #2940 portal). The ~24 "self-healing … on a renamed board" family is still the dominant block. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified lifecycle-state terminology and migration markers throughout task and project management documentation. * Documented the distinction between archived-task sentinels and workflow column identifiers. * Updated lifecycle documentation tracking to reflect the latest coverage. * **Bug Fixes** * No runtime behavior changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8b75a42d22 |
batch: self-healing sweeps were blind on renamed boards (26 sweeps, folds 23 PRs) (#2944)
**Consolidation of 23 open PRs into one.** Every one shared a single root cause and mostly touched a single file; 23 CI runs for that was indefensible. Folds and supersedes: #2867 #2869 #2876 #2879 #2883 #2891 #2899 #2901 #2902 #2905 #2906 #2914 #2916 #2918 #2919 #2920 #2922 #2927 #2929 #2932 #2934 #2937 #2939. (#2865, #2882, #2897, #2909, #2912 already merged and are not re-folded.) ## The root cause A self-healing sweep selects its work with `listTasks({ column: "in-review" })`. On a board whose lanes are renamed that returns **nothing**, so the sweep never runs — no error, no log line, no failed task. Several sweeps had already had their *predicates* converted to resolved lanes, which dropped a census count and changed nothing, because the query above the loop had already returned an empty list. **26 sweeps converted.** Each one: read the project's columns for the role, then decide each card against **its own** workflow, with the legacy ids unioned so a board mid-rename is never skipped. ## What each sweep stops silently failing to do | | | | --- | --- | | stale merger status | one finished card held the **merge queue** for everything behind it | | stale `blockedBy` / completed-task release | dependents stayed blocked on work that had already finished — the board stops moving | | workspace partial lands | a task left with **some repos merged and some not** | | mid-merge retry stamp | the card stalled *and* the operator's manual Retry was gated by the same stamp | | in-progress limbo / no-progress failures | dead cards held a work slot forever | | partial-progress retry | real work parked failed with its **retry budget unspent** | | orphaned-execution signal | visibility only — the one signal pointing at an orphan went silent | | zero-commit audit | went **half-blind**: the error arm kept working, the lane arm did not | Plus: ghost review cards, transient merge failures, misclassified failures, branch misbinding, missing-worktree failures, merged-but-unfinished finalization, done-metadata repair, self-owned branch conflicts, orphan-only scope violations, post-done wedges, idle assigned agents, PR-conflict worktree ownership, and orphaned workspace worktrees. ## Two defects the conversion itself introduced, both caught and fixed 1. **Missed pairs.** Widening a read without converting the guards beneath it is *worse than not converting*: the sweep starts admitting renamed-board cards and then mis-decides every one. Review caught a second guard on a re-read row; the audit that triggered found **five more**, one of which gates the `reviewProof` triple-proof — a renamed review card would have been moved backward with the safety check silently skipped. Column guards 86 → 81. 2. **Duplicate processing.** The literal reads were disjoint by construction; resolved reads are not, so a column carrying two role flags put one card in two buckets — duplicate moves, duplicate audit rows, inflated counts. Both now have ratchets. `self-healing-converted-sweeps-have-no-literal-lane-guards.test.ts` **derives** its sweep list (a sweep counts as converted when its body calls `resolveProjectColumnsForRoles`), so it cannot go stale, and it carries two positive controls because a broken regex finds no offenders and a broken derivation iterates nothing — an empty loop registers no tests and reads green. ## Deliberately unchanged - 22 `moveTask` destinations carrying `recoveryRehome: true` — `moves.ts` exempts these so a card stranded in an undeclared column stays rescuable. - One literal in `clearStaleBlockedBy`'s log-dedup closure (allowed by name in the ratchet, with the reason). - `surfaceInReviewStalls` — hot list-read path, needs a batched prefetch; that is a performance design decision, not a conversion. - `scheduler.ts` and `replan-target.ts` — built on `resolveTaskWorkflowIrSync`, which returns the default IR for every task in production. Converting there produces inert code. ## The fold itself is worth one note All 23 branches appended to the **same test file at the same anchor**, so every automatic strategy — git 3-way, `merge-file --union`, and three hand-written resolvers — interleaved them mid-block. Two attempts committed conflict markers before I caught it. The file is therefore **reconstructed**: head authored once, body assembled as the union of each branch's own intact top-level segments keyed by test title, with the nested `already-merged hard blocker` describe appended whole (flattening it orphaned its helper). Verified by *parsing after every step* rather than trusting the merge — which is how each interleaving was caught. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71. Scoped suites 592 passed (self-healing, the blindness suite at 68 cases, the ratchet, and the notification suite). `tsc` engine clean; `pnpm lint`, `check:changesets`, `lifecycle-column-census --strict` and `check-sql-column-literals` all clean, each run explicitly. Each folded conversion was individually revert-proven on its original branch — the read reverted alone, and the per-card verdict reverted alone — and those measurements are recorded in the commit messages carried into this branch. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b4ed12e9c8 |
batch-u7-lane-fixes: three core/engine renamed-board fixes folded (was #2925, #2930, #2936) (#2925)
**Consolidated per the queue freeze.** Three single-fix PRs of mine folded into this one branch; #2930 and #2936 are closed as superseded. Net effect on the queue: **3 → 1**. All three are the same root cause — a lifecycle lane compared against a legacy id — and all three carry a measured revert proof. Verified scoped (not full suite) on the folded branch: `tsc --noEmit` clean, `pnpm lint` clean, SQL-literal gate green, census `--strict` green, and 61 tests across five suites plus the guard at 9/9. --- ### 1. `getLiveTaskColumn` produced the archived sentinel from a literal (was #2925) `getLiveTaskColumn` **manufactures** the string `"archived"` that a dozen comparisons across five files trust — and it tested `row.column === "archived"`. A live row in a renamed archived lane read as **live**, so the gates hiding an archived card's artifacts and document listings never closed. Fixing those twelve comparisons individually would have been wrong twice over: **they are sentinels, and the defect was in the producer.** One line, once, and all twelve become correct. `resolveArchivedLanes` moved to `project-lane-vocabulary.ts` — three private copies of one fact is how the "write guard says yes, publication guard says no" disagreement happens at scale. *Revert proof (real PostgreSQL):* restore the literal → `expected [ { …(14) } ] to deeply equal []`. **Caught myself shipping the unwired shape here:** I added the parameter to seven functions and wired none of their impl callers — the exact inert-conversion defect this program exists to remove. The failing test is the only reason I noticed. ### 2. Mission delivery repair refused a completed card (was #2930) `getTerminalTaskEvidence` tested only `column === "done"`, so a completed card on a renamed board classified as `nonterminal` and `reconcileFeatureDoneWithTerminalTask` threw `TASK_NOT_TERMINAL: … not shipped`. Valid operator work refused — with the message naming the real column while the check couldn't see it. The **type** blocked the fix from the far end: `TerminalTaskEvidence` pinned `column: "done"` / `"archived"`, so the resolver couldn't report the real column without a compile error. `kind` already carries the role, so `column` is free to carry the truth. *Revert proof (real PostgreSQL):* restore the literal → `TerminalTaskReconciliationError: … not shipped`. I had deferred this twice on the premise that `AsyncMissionStore` "holds a layer, not a store". It holds an **optional `taskStore`**, and the single production construction site supplies it. ### 3. The unwired-lane guard reported two FALSE entries (was #2936) `unwired-lane-parameter-guard.test.ts` has been **red on main** since #2875, flagging two `InReviewDurationLanes` properties as unwired when the impl demonstrably supplies both. Cause: my own owner-scoping rule requires a mention from a file naming the declaring symbol — correct for a function, structurally impossible for an interface passed as an inferred object literal. Fixed at the caller (name the type) after trying the tool three ways: relaxing type-owned properties hid **12** genuine entries; resolving owners to consuming functions hid **6**. Each refinement traded the false positive for false negatives — the sign a co-occurrence heuristic has hit its limit. Recording two *wired* parameters in `KNOWN_UNWIRED` was rejected: that puts non-debt in the debt list, which is how a ratchet starts lying. Guard back to **9/9**, baseline unchanged at 17. **This un-reds main.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c6767cb258 |
self-healing: foreign-only contamination never cleared on a renamed board (fourteenth sweep) (#2891)
`recoverForeignOnlyContaminatedInReviewTasks` classifies a branch carrying **only foreign commits** and clears the contamination park that nothing else clears. Two literal reads meant that on a renamed board it classified nothing, and the task stayed parked indefinitely. ## The two redundant guards were the interesting part Both filters carried a `task.column === …` check that was **redundant** while the query pinned the column. Under a resolved read they stop being redundant and become the per-card verdict — so they convert here rather than being deleted. Deleting them would have silently widened the sweep, which is the failure this whole class is about. ## Dedupe matters more here than elsewhere The concatenated candidate list is deduped (the P1 reviewed on #2879). It bites harder in this sweep because the two filters have **different predicates**: a column carrying both a review role and the wip role could satisfy both and classify one branch twice. Explicit `has` guard rather than `new Map(entries)` — that constructor keeps first insertion *order* but the **last** value for a repeated key, so it reads as first-bucket precedence while doing the opposite. (Corrected in #2879 and #2883 for the same reason.) ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed, so the classifier is never called | | the review verdict | fails — the renamed review lane does not match and the card is filtered out | Observable is **candidacy**: `classifyForeignOnlyContamination` runs once per accepted card and not at all for a rejected one, which is exactly the read-plus-verdict under test. It is a static named import, so it is intercepted with a scoped `vi.mock` (spyOn cannot rebind an already-resolved ESM binding); only that one export is overridden, so the sweeps in this file that use `inspectBranchConflict` are unaffected. A non-vacuous companion (same card in the board's hold lane → never classified) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
60bfebdc98 |
fix(reliability): the duration query hid its lane ids inside a SQL template (#2875)
The Reliability panel's **third and last** blind input — and my own loose end. #2861 fixed the two counts beside it, so the panel went from uniformly wrong to **partially** wrong: entries and bounces populated, duration reporting `no-in-review-entries` forever. Partial blindness is harder to notice than total, which is why finishing it matters more than one site suggests. ```sql metadata->>'to' = 'in-review' OR (metadata->>'from' = 'in-review' AND metadata->>'to' = 'done') ``` ## The class, not just the site **This shape is invisible to every check we have.** The lifecycle census scans `===`/`!==` comparisons; the unwired-lane-parameter guard scans declarations. Neither sees a lane id inside a `sql` template, so this class is **not in the backlog total at all** — the number is a floor for this reason as well as the usual one. `scripts/check-sql-column-literals.mjs` (#2841, in flight) is the detector for exactly this: it freezes the surface at 30 sites rather than converting any, so this one was unowned. That PR and this one are complementary — it stops the surface growing, this shrinks it by one. ## The fix Lanes resolve **once per call** via `resolveProjectColumnsForRoles` and arrive as parameterised equality fragments, one branch per id — no interpolated list, no string building. Resolution lives in `getInReviewDurationEventsImpl` because that is where the store is; `async-audit.ts` takes a bare `db` handle and cannot resolve anything. Best-effort, defaulting to the legacy pair, so a caller that cannot resolve keeps exactly today's query. **The union is correct rather than a widening hack**, for the same reason as #2861: these are *move records*, and a past move recorded the column name as it was at the time. A board renamed last month has rows under both ids, so the honest query covers both — which is precisely what `resolveProjectColumnsForRoles` returns. ## Tested against real PostgreSQL, deliberately This is a **SQL predicate** change. A mocked store would assert the arguments and prove nothing about the query that actually runs — which is the entire risk when the literal lives inside `sql`. The new case inserts real `activity_log` rows on a renamed board and reads them back through the real store method. The legacy-lane case in the same file stays green, which is the compatibility half. **Revert proof, measured:** restore the hardcoded fragments and the new case fails with ``` expected [] to deeply equal [ 'renamed-entered', 'renamed-done' ] ``` ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - `activity-log-parity.pg.test.ts` — 5 passed against real PostgreSQL With this, all three Reliability inputs read the board's own lanes. 🤖 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** * Reliability duration metrics now work correctly with renamed workflow lanes. * Completion tracking recognizes configured completion lanes instead of relying on fixed defaults. * Improved handling of transitions between multiple review lanes and review-to-work-in-progress movements. * Legacy lane behavior remains supported when configured lane information is unavailable. * **Tests** * Added coverage for renamed lanes, historical lane IDs, and transition edge cases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
10f9df1600 |
fix(overseer): the whole oversight loop was inert on a renamed board (#2898)
`resolveWatchedStage` keyed on the literals `in-progress`/`in-review`, so on a board that renames either it returned `null` for **every** card. That is three literals with an outsized blast radius. `observeTask` returns early on a null stage, so: - no `OverseerStageObservation` is recorded, - no `overseer:intervention` entry is emitted, - and `PlannerRecoveryController`, which consumes those observations, has nothing to steer, retry or targeted-fix. **The entire oversight loop was inert and silent about it** — the same shape as the self-healing sweeps whose queries returned empty arrays. ## I deferred this myself, on a cost argument that was wrong The audit note I wrote for this site said resolving inside `observeTask` "buys a workflow read per card per poll". Then I read the caller: the poll **already awaits `resolveEffectiveSettings` per task**. It is a per-task async loop regardless, so with an IR cache keyed by workflow the addition is *(distinct workflows)* resolutions, not *(cards)*. Pricing the fix before checking the caller cost a deferral. Worth recording, because "this needs a cost judgement" is the most comfortable place in this program to leave something. ## The review test is the three-trait union, deliberately `isReviewColumnRole` checks only `mergeBlocker || humanReview`. A board whose review lane carries `merge` (**mergeOrchestration**) — the built-in default's own shape — would classify as *not in review* and be skipped. Reaching for the obvious helper would have reintroduced the bug this change removes, through the helper meant to fix it. There is a case asserting exactly that. ## Wiring Both call sites, because either alone leaves a hole: | site | why it matters | |---|---| | the poll (`project-engine.ts`) | per-poll IR cache — a workflow edit is picked up next tick rather than served stale | | the manual nudge | otherwise a renamed board answers `no-active-stage` to an operator pressing the button | `columnFlags` is in the `unwired-lane-parameter` vocabulary, so the wiring cannot silently rot — the guard reports it if a future change drops the argument. Fail-soft throughout: an unresolvable workflow yields `undefined` and the callee falls back to the legacy ids, which is exactly today's behaviour. A v1 IR declares no columns, so it takes the same path. ## Revert proof (measured) Drop the `columnFlags` branch and **exactly the three renamed-lane cases fail**: ``` expected null to be "executor" expected null to be "merger" (mergeOrchestration lane) expected null to be "merger" (humanReview lane) ``` The legacy-id and neither-role cases stay green — the gate must still gate, and watching every column would be its own defect. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/engine`) — clean - `planner-overseer.test.ts` + `planner-recovery-controller-human-control.test.ts` — 64 passed - unwired-lane guard — 9/9, no new entries Carries the one-line SQL-baseline re-record (`team-analytics.ts: 6 → 3`) that #2864 left behind, same as my other open branches — main is red on it, and identical changes to that line merge without conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
defe48d30f |
fix(core): per-workflow metrics read zero on a renamed board (#2866)
Second of the 14 lane-bound SQL sites from #2839, after #2864. Independent of it — different file, different caller argument. ## The defect `aggregateWorkflowAnalytics` filtered in SQL on `t."column" = 'done'` and `IN ('in-progress','in-review')`. On a renamed board those match nothing, so `tasksCompleted`, `tasksInProgress` and `tasksInReview` come back **zero for every workflow** while the board is busy. Nothing errors. Same shape and same fix as #2864: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, and thread the store from the single Command Center caller so the parameter has a supplier immediately rather than becoming an inert seam. ## What the test caught that I had not **The renamed case still failed with the query fixed.** The bucketing at lines 296–297 already uses `isWipColumnRole` / `isReviewColumnRole` — correctly converted — but those read `query.columnFlagsByName`, which production supplies and my fixture did not. So: - the **SQL** decides *which rows come back*; - the **trait map** decides *which bucket each row lands in*. Both halves have to be right. Fixing only the query would have shipped a "conversion" that still reported zero on a renamed board, and the file would have scored as converted twice over. That is exactly the partial-conversion shape this program keeps re-finding — caught here only because the test asserts `tasksInReview` alongside `tasksCompleted`, since those two paths take **different** resolved sets (complete vs wip+human-review). Asserting the completed count alone would have left the second conversion unproven. ## Measured Reverted, only the renamed case flips: ``` ✓ default vocabulary: completed and in-review work are counted × renamed vocabulary: completed and in-review work are counted ✓ renamed vocabulary: a card in the HOLD lane counts as neither ✓ without a lane store, the legacy ids still answer Tests 1 failed | 3 passed (4) ``` The hold-lane negative is there so resolving real lanes cannot degrade into "every column counts" — trading an undercount for an overcount is harder to notice than the original bug. ## Scope The sync SQLite arm in the same file keeps its literals: it throws in backend mode and has no production caller, the same dead-arm conclusion reached for `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c143327d4b |
fix(core): the archived-document guards failed in OPPOSITE directions on a renamed lane (#2886)
Two of the four convertible sites my own learnings doc **miscounted as sentinels** — the #2877 review corrected "8 of 9 must not be converted" to "5 of 9", and these are two of the three that correction freed. They read `task.column` straight off a row `select`, so they are board lanes by exactly the test that document gives, and a renamed archived column is simply not seen. What makes the pair worth fixing together is that they fail in **opposite directions**: | guard | on a renamed archived lane | consequence | |---|---|---| | `upsertTaskDocument` | fails to **reject** | an archived card's documents stay **writable** — the read-only contract silently does not hold | | `publishArchivedTaskDocumentAddition` | fails to **accept** | a legitimate archived-document publication is refused as `parent-not-archived` | The second is the sharper one: valid operator work refused, and refused with a message that reads as a data-integrity error rather than a lifecycle mismatch. ## Shape Both take an `AsyncDataLayer` and can resolve nothing themselves; their store-level impls hold the store, so the lane set arrives as a parameter resolved once per call — the shape #2875 used for the SQL predicate. **One shared `resolveArchivedLanes` for both paths**, deliberately: if the write guard and the publication guard could disagree about whether a card is archived, a card ends up both read-only *and* un-publishable. ## The revert proof caught my own fixture first My first version set `deletedAt` alongside the renamed column, and **the revert proof passed with the fix removed**. Both guards are `column-is-archived || deletedAt != null`, so a soft-deleted fixture short-circuits the exact comparison under test — the assertion was holding for an unrelated reason. Dropping `deletedAt` isolates it, and is also the *real* shape: a live row in a workflow-declared archived lane is what a renamed board produces, and what `getLiveTaskColumn` was written to catch. Revert proof, measured honestly the second time: restore `task.column === "archived"` and the renamed-lane case fails — the upsert resolves instead of rejecting. ## Real PostgreSQL, deliberately These are row predicates inside a transaction. A mocked store would assert the arguments and prove nothing about the comparison that runs — the same reasoning as #2875. Three cases: the renamed lane rejects, the **legacy** `archived` id still rejects (most boards never rename anything), and a live card is still allowed through (a guard that rejects everything is its own bug). ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - new `archived-document-lanes.pg.test.ts` + existing `artifacts-documents-evals.pg.test.ts` — 12 passed against real PostgreSQL Note: the SQL-literal baseline is untouched here — #2881 owns re-recording it after #2864's conversion left main's gate red. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a453912ddf |
self-healing: merged-but-unfinished tasks never finalized on a renamed board (fifteenth sweep) (#2897)
`recoverMergedReviewTasks` finalizes a task whose merge is **confirmed** but which never reached the complete lane. Two literal reads meant that on a renamed board it was never found, so a card whose commit is already on the base branch sat in review or hold indefinitely — merged work the board still shows as unfinished. ## The two redundant guards convert, they don't get deleted Both `t.column === …` checks were redundant while the query pinned the column. Under a resolved read they become the per-card verdict. Deleting them would have silently widened the sweep — the same trap called out in #2891. ## Carries the two shapes review established earlier in this series - **Narrow when the card can answer, broad when it cannot** (#2891). `resolveWorkflowIrForTask` *substitutes* the built-in IR rather than failing, so a card with an unreadable selection would otherwise be rejected by the very verdict that the project-scoped query had just admitted it under. It falls back to the project sets instead. - **Deduped across the buckets** (#2879), so a column carrying both a review role and the hold role cannot finalize one card twice. Both were review findings on earlier PRs in this series, applied here up front rather than waiting to be caught again. ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed | | the per-card review verdict | fails — the renamed review lane does not match | Observable is `resolveSelfHealingMergeTarget`, a private method called once per candidate, so the assertion sits downstream of both halves without a git fixture. A non-vacuous companion (merge-confirmed card in the wip lane → untouched) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly. |
||
|
|
890e1f87e7 |
fix(core): issue panels reported nothing fixed on a renamed board (#2871)
Fourth and last of the lane-bound analytics sites from #2839, after #2864, #2866 and #2870. ## The defect `aggregateGithubIssueAnalytics` and its GitLab twin filtered their resolved-issue query on `"column" = 'done'`. On a renamed board that matches nothing, so `fixed` is **zero**, the resolved-issue list is empty, and `net` reports every filed issue as still outstanding — while the team closes issues all week. Nothing errors. Same fix as the previous three: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, thread the store from each Command Center caller so the parameters have suppliers immediately. ## Both providers in one change, deliberately These two files are **copies** — same query, only the provider literal differs — and a copy is exactly what gets half-fixed. Converting one and not the other type-checks, passes that provider's test, and leaves the second silently broken with no signal anywhere. The suite runs every case against both, so the pair cannot drift. ## Measured Reverted, exactly the two renamed cases fail — **one per provider** — while both default-vocabulary controls, both WIP-lane negatives, and both omitted-store legacy cases stay green: ``` ✓ github: default vocabulary counts a resolved issue × github: renamed vocabulary counts a resolved issue ✓ github: renamed vocabulary does NOT count an issue still in the WIP lane ✓ github: without a lane store, the legacy id still answers ✓ gitlab: default vocabulary counts a resolved issue × gitlab: renamed vocabulary counts a resolved issue ✓ gitlab: renamed vocabulary does NOT count an issue still in the WIP lane ✓ gitlab: without a lane store, the legacy id still answers Tests 2 failed | 6 passed (8) ``` That the failures are symmetric is itself the check on the copy-paste risk. ## Scope The sync SQLite arms keep their literals: they throw in backend mode and have no production caller, the same dead-arm conclusion as `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · Command Center + GitLab issue analytics suites 10/10 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. --- **This closes the lane-bound half of #2839.** All 14 sites the hand-review identified as genuinely vocabulary-bound are now converted across four PRs. What remains there is the 11 `!= 'archived'` exclusions, which are probably correct as literals — archiving writes `task.column = 'archived'` unconditionally as a state rather than a lane — plus one dead SQLite arm. Those need per-site judgment, not conversion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
216632bd3a |
fix(core): task-duration stats were computed from an empty set on a renamed board (#2870)
Third of the 14 lane-bound SQL sites from #2839, after #2864 and #2866. Independent of both. ## The defect `aggregateProductivityAnalytics` filtered its duration query on `"column" = 'done'`. On a renamed board that matches nothing, so the entire task-duration distribution — median, p90, average, total — is computed from an **empty row set** and reports zeros while the project ships work. Nothing errors. Same shape and fix as the previous two: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, thread the store from the single Command Center caller so the parameter has a supplier immediately rather than becoming an inert seam. ## Measured Reverted, only the renamed case flips: ``` ✓ default vocabulary: a finished task contributes to the duration stats × renamed vocabulary: a task in the RENAMED complete lane contributes ✓ renamed vocabulary: a task still in the WIP lane does NOT contribute ✓ without a lane store, the legacy id still answers Tests 1 failed | 3 passed (4) ``` ## The negative asserts the median, not just the count This fix's failure mode is **worse than the bug it fixes**. Resolving too many lanes would pull unfinished work into the distribution and produce a plausible-but-wrong median — a number nobody questions — where the bug produces an obvious zero. So the WIP-lane case asserts `medianMs` is null as well as `completedTasks` being 0. ## A fixture error worth naming My first version asserted `taskDuration.count`. `TaskDurationSummary` exposes `completedTasks`. Every case failed with `expected undefined to be 1` — **including the controls** — which reads exactly like a broken product until you notice the control is failing too. A control that fails is a fixture bug, not a finding; that asymmetry is the fastest way to tell them apart. ## Scope The sync SQLite arm keeps its literal: it throws in backend mode and has no production caller, the same dead-arm conclusion as `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
995b52d21d |
fix(gate): re-record the SQL baseline — main is red after #2864 (#2878)
**`pnpm test:gate` and both `pretest` hooks fail on `main` right now.** Merge this first. ## What happened #2841 (the SQL gate) merged, then #2864 merged. #2864 removed three legacy comparisons from `team-analytics.ts`, but its baseline entry still allows six — and this gate **fails on a lowered count by design**, so a migrated slot cannot be silently regrown into later. Baseline 30 → 28. ## This is my sequencing error The four analytics conversions were branched and reviewed **before** the gate existed, so none of them carries a baseline update. The gate then landed first, which means **each of them breaks `main` as it merges**. I opened all five without thinking about the order they would land in. The three still open — #2866, #2870, #2871 — will each do this again. I am adding baseline updates to them next so they land clean. ## Note on the downward check The "count went down" failure looks like pedantry until it fires. It exists so a migrated site cannot leave an unused allowance behind for the surface to regrow into — the same rot as an allow-list entry for a deleted function. The real cost is that a conversion and its gate have to land in a known order, which is a coupling I created and did not plan for. ## Verification `pnpm test:gate` green with the re-recorded baseline · lint 0 · `node scripts/check-sql-column-literals.mjs` exit 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ee8ae1eb23 |
fix(census): the header claimed 0 trait-fallback branches while sites of that shape existed (#2874)
The census header has been printing `of the column guards, 0 are trait-fallback branches (already converted)` while sites of exactly that shape exist. I flagged this on #2842 as a suspected classifier gap; this confirms and fixes it. ## The miss Only `cond ? trait : literal` was recognised. The other spelling — a **negative** test with the literal on the **true** branch — is what a caller writes once it hoists its resolved lanes: ```ts complete: completeLanes === undefined ? columnId === "done" : completeLanes.includes(columnId) ``` That is `github-tracking-state.ts:245-246` — a fully converted resolver whose two degraded arms were reported as unconverted debt. **The backlog read higher than the remaining work**, and a reader chasing it was sent to lines that are already correct. Second half of the miss: `completeLanes` matches no hint. Adding `Lanes` to the hint list does **not** work, and the reason is itself a prior fix — hints are word-bounded because the unbounded form once let `hold` match `threshold` and `household`. `\bLanes\b` cannot match inside `completeLanes`, where the boundary does not exist. So resolved-lane identifiers get an explicit suffix rule. ## Both guards on the new rule exist because I broke them while writing it Worth stating, because each failure ran in the **dangerous direction** — marking a *live* line "already converted", which removes a real guard from a backlog people trust: | mistake | what it excused | |---|---| | widened the shared `testsTraitData` | fed the ancestor-walking rules too, which marked `step.status === "done" \|\| step.status === "in-progress"` at `register-task-workflow-routes.ts:941` — a step-**status** comparison, not a column guard — as converted | | let the new rule walk ancestors | excused any literal inside a block governed by a negative lane test | Measured: the count went to **6 with two of them wrong** before I caught it. The rule is now immediate-parent-only with its widened identifier match local to it, and reports exactly the **2 real sites**. ## Verification - Census: **176 guards, 2 trait-fallback** (was 176 / 0). The total is unchanged — this sub-count is diagnostic and does not move the ratchet, so `--strict` exits 0 with no baseline re-record. - 5 cases in `scripts/__tests__/lifecycle-census-inverted-fallback.test.mjs`, including both negatives that pin the mistakes above plus one for the suffix rule not over-reaching (`airplanes` is not a lane test). - `pnpm lint` clean; gate green (161/487/13/71). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved lifecycle analysis accuracy for trait fallback logic, including inverted conditions, legacy fallback syntax, and null or undefined checks. * Added safeguards to avoid misclassifying complex conditions, unrelated identifiers, and nested expressions. * Improved handling of lifecycle lane and column naming patterns. * **Tests** * Expanded coverage for valid and invalid fallback scenarios, identifier boundaries, parent-expression restrictions, and property-path checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b85e5f90e1 |
fix(create): two task-CREATE destinations named a lane the board does not have (#2843)
Both files sat at **census-zero** and both wrote real cards into columns
no workflow declares. The census scores `===` comparisons, so a lane
literal passed as a **call argument** is invisible to it — one of the
four census-blind classes. These are the only two explicit-`column`
creates in production:
```
packages/dashboard/src/routes/register-gitlab.ts:108 column: "triage"
packages/cli/src/extension.ts:5243 column: "todo"
```
## The two defects
**`register-gitlab.ts` — `column: "triage"`, a column U11 DELETED.**
This one is broken on *every* board, not only renamed ones: the default
lineage is now `todo | in-progress | in-review | done | archived`.
`createTask` already resolves the intake column of the workflow it
selects (`resolvedEntryColumn`), and an explicit `column` **overrides**
that resolution — which is why the stale literal survived U11. Nothing
rejects the write and nothing logs it: the route answers `201` with a
task id and the imported card is simply not on the board. Same shape as
the `task-update.ts` triage defect fixed earlier in this program.
Fix: omit `column` and let `createTask` resolve intake.
**`extension.ts` `fn_delegate_task` — `column: "todo"`.** On a workflow
whose ready lane is named anything else, the delegated card goes to an
undeclared column: written, reported to the caller as delegated, never
visible to the agent it was delegated to.
Fix: resolve the selected workflow's `hold` lane. **Deliberately not**
"omit the column like the GitLab route" — the tool's own contract is
*"the task goes to the ready-to-work lane and the target agent picks it
up on its next heartbeat"*, so inheriting intake resolution would park a
delegated card in a manual-intake lane waiting for a human. That would
be a behaviour change; `hold` is the role that names the lane the
literal meant.
## New helper: `resolveWorkflowColumnForRole(store, role, workflowId?)`
The **write**-shaped counterpart to `resolveProjectColumnsForRoles`. The
read helper unions in the legacy ids because an extra id in a query set
is inert; here the same trick is a silent wrong write (post-U12 an
undeclared column is a `TransitionRejectionError` on move, a phantom
lane on create), so it returns one column from one workflow, or
`undefined`.
**A contract I got wrong twice, now pinned by a test.** `undefined`
means *"this workflow declares no such column"* and nothing else.
`resolveWorkflowIrById` never throws and never returns nothing — an
unregistered builtin id, a missing definition row and a failing read all
resolve to the default coding IR (branded via `markFellBack`). So an
unreadable workflow yields the **built-in** hold lane, not `undefined`,
and both call sites' `?? "todo"` fallbacks are narrower than they look.
Two of my first test cases asserted the opposite and failed; the
behaviour is the resolver's, and the write it produces is identical to
the caller's own legacy fallback either way.
## Revert proofs (measured, not asserted)
| revert | failure |
|---|---|
| `column: holdColumn` -> `column: "todo"` | `extension.test.ts`:
`expected 'todo' to be 'queued'` |
| omitted column -> `column: "triage"` | `routes-gitlab.test.ts`:
`expected 'triage' to be undefined` |
The two neighbouring `fn_delegate_task` cases stay green under the first
revert, because the built-in board and the test's `linearWorkflowIr`
both call the lane `todo` — which is exactly why this literal survived
every previous pass.
The GitLab case asserts **absence** of the key rather than a resolved
id: the store there is a fake whose `createTask` echoes its input, so
asserting a resolved value would be testing the fake. Absence is the
property that hands the decision to the real `createTask`.
## Census
| file | before | after |
|---|---|---|
| `packages/dashboard/src/routes/register-gitlab.ts` | 1 | 0 |
| `packages/cli/src/extension.ts` | 1 | 0 |
Baseline tightened. It also picks up
`packages/core/src/task-store/moves.ts` 2 -> 0, which was **already true
on main** — not from this diff.
## Noted, deliberately not changed
- `validateAssignableAgentId`'s synthetic probe a few lines above still
uses `{ id: "<new>", column: "todo" }`. It feeds `isImplementationTask`,
whose `IMPLEMENTATION_TASK_COLUMNS` set an earlier worker documented as
deliberately-not-converted (converting it makes the routing policy async
— an agent-admission behaviour change). On a renamed board the probe is
now *stricter* than the real destination, which is the safe direction
and matches the pre-existing behaviour.
- The third site from this bucket, `workflow-node-handlers.ts:455`
(`transitionTask({ column: "in-review" })` on the `review-handoff`
seam), is a **hard** failure rather than a silent one — `transitionTask`
routes through `moveTask`, which post-U12 throws
`TransitionRejectionError` for an undeclared destination, so the
workflow walk dies at the handoff on any renamed review lane. It is
engine (`batch-engine`) and fixing it properly touches `executor.ts`,
which #2820 is also editing. Left for that batch rather than opened as a
conflicting edit.
## Verification
- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm lint` — clean
- `tsc --noEmit -p tsconfig.json` for `@fusion/core`,
`@runfusion/fusion`, `@fusion/dashboard` — clean
- `node scripts/lifecycle-column-census.mjs --strict` — exit 0
- targeted: `project-lane-vocabulary.test.ts` 14/14,
`routes-gitlab.test.ts` 8/8, `extension.test.ts -t fn_delegate_task` 9/9
🤖 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**
* GitLab-imported cards now appear in the workflow’s configured intake
lane.
* Delegated tasks now move to the workflow’s configured hold lane,
including workflows with custom lane names or separate intake and hold
lanes.
* Delegation reports the task’s final lane and provides an error when it
cannot be moved successfully.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
ef50244234 |
feat(gate): freeze the SQL column-literal surface — 30 sites, none may be added (#2841)
Instruments a surface no existing check can see. Follows #2839, and **corrects the count I reported there** (12 → 14). ## Why it was invisible The lifecycle census parses TypeScript **comparisons**; a legacy id inside a SQL string is string data. The inert-seam gate reasons about parameters and call sites. Neither has ever looked here. **What it cost:** `cleanupStaleMergeQueueRowsImpl` filtered on `t.column != 'in-review'`, so on a renamed board every queued card looked stale, its `merge_queue` row was deleted, and the card became **unleaseable**. The operator found it reviewing #2819 — in SQL I had already read past during that same work. The quieter half is analytics: five sites count `"column" = 'done'`, so throughput, cycle time, and team dashboards report **zero completed work** on a renamed board. Nothing errors, which is why nobody files it. ## What this does, and does not do It does **not** fix the sites. `resolveProjectColumnsForRoles` is the mechanism and its migration has an owner (#2839). This freezes the population so the surface cannot grow underneath that migration: a new file or a higher count fails, **and a lower count fails too** — so the baseline ratchets down as sites migrate rather than leaving slots to silently regrow into. That is the same rot as an allow-list entry for a deleted function, which this repo already hit once. AST-based, deliberately: a line grep for the same pattern reports **37** hits, **25 of them prose** quoting `column === "done"` in explanatory notes. A guard that is 68% false positives trains its readers to skip it — a lesson this program has already paid for. ## Two corrections found by mutation-testing my own gate **1. Clause fragments were missed.** Requiring a SQL keyword *in the same literal* skipped `team-analytics.ts`, which builds `["assignedAgentId IS NOT NULL", `"column" = 'done'`, ...]` and joins them into a `WHERE` later. That fragment is as vocabulary-bound as any full query but contains no keyword. Fixing it took the population **12 → 14**, so the number I put on #2839 was low. **2. My first mutation test proved a direction it had not.** I replaced the first textual occurrence in a file — which was inside a **comment** — and read the unchanged count as the scanner being broken. The scanner was right; my test was wrong. All three directions are now driven against real SQL: | mutation | result | |---|---| | add a full query with a legacy comparison | `3 SQL column literal(s), baseline allows 2` | | add a bare clause **fragment** (no keyword) | caught — same failure | | migrate one away (count drops) | `1 site(s) now, baseline still allows 2 — re-record it` | | restore | exit 0 | I am flagging that second one because it is the exact failure mode this program keeps finding: a green result read as evidence when the experiment was invalid. ## Verification `pnpm test:gate` green with the new check in it · lint 0 · single AST pass. Wired into `test:gate` and both `pretest` hooks. 🤖 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** * Added automated checks to detect increases in legacy SQL column literals. * Added baseline tracking to ensure known SQL literal counts do not regress. * **Tests** * Expanded pre-test and gated verification steps with SQL literal and mock completeness checks. * Updated test validation workflows to enforce the new safeguards. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7784cb1fe8 |
self-healing: six recovery sweeps that never ran on a renamed board — and the guards widening their queries activates (#2838)
**Six self-healing sweeps did not run at all on a renamed board. Each is a recovery path — the thing that unsticks a card when something has already gone wrong.** #2800 measured this class and could not fix it: a read happens *before* any task is in hand, so there is nothing to resolve a per-task lane from. `resolveProjectColumnsForRoles` (landed separately) is the seam that was missing. ## What was silently dead | sweep | what stayed broken on a renamed board | | --- | --- | | `reconcileDoneTaskIntegrity` | a landed card kept **no commit sha**, forever | | `recoverAlreadyMergedReviewTasks` | a card whose merge **succeeded** stayed parked with `status: "failed"` | | `recoverStuckMergeDeadlocks` | **doubly blind** — no candidates *and* no dependents | | `recoverInterruptedMergingTasks` | a task interrupted mid-merge sat in `merging` indefinitely | | `recoverMergeableReviewTasks` | a card ready to merge was never re-enqueued | | `recoverReviewTasksWithFailedPreMergeSteps` | a card parked on a failed review step was never revived | The census scored the `task.column === "..."` re-assertion *inside* each loop, never the query above it. Converting those comparisons would have dropped six counts and changed nothing — the loop bodies were already unreachable. ## The conversion shape — five parts, three of which review taught me Documented in `self-healing-sweeps-are-blind-on-a-renamed-board.md`, because the second sweep **drifted from the first**: I wrote it from the pre-review version and reproduced a flaw already fixed one commit earlier. 1. **Read** — project union, query each column, dedupe by id. Legacy ids unioned so a board mid-rename is not skipped. 2. **Verdict** — per card against **its own** workflow. Widening the read and widening the verdict are different decisions: *a missed row is invisible, a wrong row is a write.* Using the project union as a per-card test claims a card because some **other** board calls its column that role. 3. **Provenance** — the resolver **substitutes** the built-in IR rather than failing, so `length > 0` reads as "this card answered" when nobody did. It does not change the verdict (measured: identical) — it makes the unrepaired card **reportable**. 4. **The log strings** — widening a query invalidates every message naming the old literal. One logged `"stale merging task(s) in in-review"` after its read covered several lanes. 5. **The guards the query ACTIVATES.** ## Part 5 is the one that bites A guard downstream of a literal query is **unreachable** on a renamed board — and unreachable is indistinguishable from correct. That is why these sit unwired indefinitely. `recoverReviewTasksWithFailedPreMergeSteps` filters on `blocker !== "task has failed pre-merge workflow steps"` — an **exact string match**. Unwired, the blocker returns `"task is in 'checking', must be in 'in-review'"`, so widening the query alone would have made the sweep **find every card and reject every card**. Measured: **6 sweeps hold both a literal query and an unwired lane guard**; 30 hold a literal query with no such guard. All six are named in the doc. **One of the six was my own already-converted sweep.** I widened `recoverAlreadyMergedReviewTasks` two commits before noticing its `getTaskHardMergeBlocker` was unwired — so for two commits it found renamed-board cards and declined them. The scan must run **before** widening; I did it after, and only caught it because the next sweep forced the question. `getTaskHardMergeBlocker` was the blind spot for four of the six: a wrapper, no lane parameter at all, every caller behind a literal query. ## Corrections to my own work, kept visible - The project union used as a **per-card verdict** — the flat-set mistake `project-lane-vocabulary.ts` warns about in its own header, which I quoted while writing it. - A **provenance fix that was a no-op**: measured identical verdicts in every state, revert passed its own new test, so it was thrown away rather than shipped with a comment claiming otherwise. - The second sweep **reproducing the first's pre-fix shape**. - Three assertions that were **vacuous until the revert exposed them** — including one where the write needed a real git repo, so `commitSha` could not distinguish accepted from rejected. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71 - `self-healing.test.ts` 412, query-blindness suite 12 - `tsc` on core and engine; `pnpm lint`; `check:changesets`; census `--strict` — all clean, each run explicitly - Every conversion revert-measured, **each direction independently** where a sweep has two (read and guard) ## Scope **42 queries remain**, 5 of the 6 activation-risk sweeps among them. Each is per-sweep work — its own filter semantics, its own downstream guards, its own log strings — so they land one at a time with the pattern proven, never swept. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Self-healing workflows now work correctly on boards with renamed lifecycle columns. - Improved recovery for completed, in-review, interrupted, stalled, and failed-merge tasks. - Prevented tasks from being incorrectly classified using another workflow’s columns. - Added warnings when a task’s workflow lanes cannot be resolved. - **Documentation** - Expanded guidance on renamed-board recovery behavior and related diagnostic limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
92d82b7a17 |
fix(guard): the unwired-lane check reported 0 because its question was too weak (#2852)
A guard nobody has proven can fail is a number, not a check. This one
was returning a clean `[]` while **18** real unwired lane declarations
sat on `main` — including one it was specifically built to catch.
## The escape that found it
`diffSnapshots` in the glasses plugin:
```ts
opts: { notifyOnColumns: ReadonlySet<ColumnId>; completeColumnsByTaskId?: ReadonlyMap<...> }
...
const completeColumns = opts.completeColumnsByTaskId?.get(task.id);
const isComplete = completeColumns ? completeColumns.has(task.column) : task.column === "done";
```
**No file anywhere builds that map.** The conversion was decorative —
the literal decided every real poll, so on a renamed board the wearer is
notified of every column transition *except the card finishing*, the one
they care about. The name was already in the guard's vocabulary list,
the declaration was exported and optional; it satisfied every condition
the guard checks, and the guard said nothing.
## Three independent blind spots
| # | blind spot | why it mattered |
|---|---|---|
| 1 | `SCANNED_PACKAGES` omitted `plugins/` | plugins hold lane logic
like anything else — this one resolves workflow IRs and decides what
"finished" means |
| 2 | inline options-object types were not walked | only bare parameters
and *named* interfaces were. Whether a lane answer arrives as a
parameter, an interface property, or an inline field is a style choice —
**a check evadable by a style choice is decorative** |
| 3 | the mention rule was `source.includes(parameter)` **anywhere** |
satisfied by coincidence for any ordinarily-named parameter |
Fixing 1 or 2 alone would still have missed it: **measured on `main`,
the guard found 0 unwired across 1753 files, and 0 again across 2114
once `plugins` was added**, because the shape was invisible too.
### On (3), I proved it on myself
Renaming the unwired parameter from `completeColumnsByTaskId` to
`completeColumns` — a better name, chosen for good reasons — **silenced
the guard instantly**, because 15 unrelated production files declare a
local called `completeColumns`. The check had not been satisfied; it had
been switched off by a rename. That is exactly the failure the code
comment two lines up condemns, committed one edit later.
The fix is the cause, not a name blocklist: a file that never references
`diffSnapshots` cannot be the thing that wires `diffSnapshots`'s
options. Still deliberately loose — a co-occurrence test, not call-graph
analysis — which keeps the low false-positive rate that makes the guard
bearable while removing a false **negative** that scaled with how
ordinary a parameter's name was.
## The 17 this uncovered
Tightening (3) surfaced 17 further unwired declarations across core,
engine and dashboard. **Spot-checked, not assumed**:
`buildUnblockWeightMap` in `task-priority.ts` declares `terminalColumns`
and `reviewColumns`, and the only files that pass either are its own
tests — the production caller silently uses the built-in `{done,
archived}` default. That is the inert-conversion shape this module
exists to name.
They span three other batches, so they are recorded as a **ratcheted
baseline** in the shape `scripts/lifecycle-column-census.mjs` already
uses here — keyed on `file + parameter` so an unrelated edit above them
cannot manufacture a failure. A new one fails immediately; these can
only leave the list. Listing them beats pretending for another week that
they do not exist.
## The glasses fix
`completeColumnsByTaskId` -> a flat `completeColumns` set, matching its
sibling `notifyOnColumns` in the same options object, resolved **once
per poll** by `notifier.ts` via `resolveProjectColumnsForRoles`.
Project-scoped and not per task because this runs on a polling timer
over the whole board — a per-card workflow read would scale with the
board on every tick. Best-effort: a failed resolve leaves the diff on
its documented legacy default rather than dropping a poll.
Still gated by `alsoNotifyOnDone`, which the production caller passes as
`false`, so it remains unobservable at runtime. Wired anyway: the day
someone enables the flag the resolution must already be right — and now
the guard will say so if the wiring is removed.
## Revert proofs (measured, one per fix)
| revert | failure |
|---|---|
| unwire `notifier.ts` | baseline gains `plugins/…/diff.ts
completeColumns` |
| drop the inline-options walk | "covers an INLINE options-object type"
fails `expected [] to deeply equal [ 'completeColumns' ]`, and the repo
scan loses the glasses entry |
| drop the owner scoping | the repo scan loses **all 17** pre-existing
entries |
| restore `task.column === "done"` | both new `diff.test.ts` cases fail
|
The new diff cases assert **both** directions — a card in the resolved
lane fires, and a card in the legacy `done` does *not* once the caller
resolved other lanes. The second is what proves the resolved set
replaces the default rather than being unioned with it.
## Verification
- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm lint` — clean
- `tsc --noEmit` — clean
- guard suite — 9/9; full glasses plugin — 188 passed across 19 files
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
eed8ca55fc |
batch-dashboard-src: the planner metrics tool froze active runtime on a renamed execution lane (186 → 185) (#2842)
`packages/cli` and the plugin packages are at **zero** lifecycle guards, so this picks up the nearest unowned work: the `packages/dashboard/src/` remainder. ## The defect `activeRuntimeMs` adds the wall-clock since `executionStartedAt` only while the card is accruing work — the **WIP role** — but it was keyed on the literal `in-progress`. On a board whose execution lane is renamed, that live tail was dropped, so `fn_task_planner_get_task_metrics` reported active time frozen at whatever the last completed segment left in `cumulativeActiveMs`. The number stayed plausible, which is why nothing surfaced it. ## The part worth reading: the wiring had no watcher, from either direction I wired the producer (`chat.ts` resolves the task's own lanes via `wipColumnsForTask`) in the same commit, then checked whether that wiring was actually covered. It was not: - **Deleting the `wipColumns:` argument left the entire 3830-test dashboard suite green.** The formatter's own tests inject the set by hand, so they prove the *guard* and are structurally blind to whether production fills it. - **`check-inert-flag-seams.mjs` does not see it either.** It tracks trailing optional **parameters**; this is a property inside an options bag. That is a real gap in the checker — every seam expressed as an options-bag property is currently unguarded. Reported here rather than fixed, because #2822 and #2830 both already modify that script and a third change would guarantee a three-way conflict. So `createTaskPlannerMetricsTool` is exported and a second test drives it, letting it do its **own** resolution against a renamed board. Deleting the argument now fails 1 of 2. ## Census | | before | after | |---|---|---| | COLUMN guards | 186 | **185** | | `packages/dashboard/src/task-planner-chat-metrics.ts` | 1 | **0** | Baseline re-recorded; `--strict` exits 0. ## Two findings I did NOT act on, deliberately **1. `github-tracking-state.ts` keeps 2 counted guards and should.** They are the documented degraded-mode arms of a fully-resolved classifier (`completeLanes === undefined ? columnId === "done" : ...`). Marking them `DELIBERATE-LITERAL` would drop the count by **reclassification rather than conversion** — the exact move the census's own strict-check warns about. Related: the census reports `0 are trait-fallback branches (already converted)`, yet these are precisely that shape, so the trait-fallback classifier appears not to recognise a ternary whose fallback arm is the literal. Worth a look by whoever owns the census. **2. Three pre-existing failures in `packages/dashboard/src/__tests__`, unrelated to this change** — measured identically on `origin/main` before and after: - `planning-browser-e2e.test.ts:353` - `register-model-routes-kimi-k3-supplemental.test.ts:60` - `routes-tasks-near-duplicate.test.ts:274` Flagging rather than touching them; per the standing rule they are quarantine candidates, not appeasement candidates. ## Verification Dashboard `tsc` clean, `pnpm lint` clean, census `--strict` 0, `check-inert-flag-seams` 21/21 supplied, changeset lint clean. Targeted suites: `task-planner-chat-metrics.test.ts` 8/8, `task-planner-metrics-tool-wip-lanes.test.ts` 2/2. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
51934931e1 |
fix(board): the awaitingPlanning badge only ever worked on a lane named "todo" (#2845)
Converts the one site in `register-task-workflow-routes.ts` that a previous pass **deliberately deferred**, and does it in the shape that note asked for. ## What the deferral said ``` FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8, DELIBERATELY NOT CONVERTED): … I converted it and then REVERTED: resolving each task's hold column needs a per-task workflow read, and this is the board-load path whose own comment above exists because unbounded reads here "turn a board load into thousands of reads". Converting it properly needs the hold column resolved per WORKFLOW from data the board payload already carries, not per task from the store. ``` That was the right call and the right diagnosis. `resolveProjectColumnsForRoles(store, ["hold"])` is exactly the project-scoped shape it names: **one** `listWorkflowDefinitions()` read per board load, flat in task count. The expensive part — a PROMPT.md read per row — is untouched and still bounded by `AWAITING_PLANNING_ENRICH_LIMIT`. The test asserts the flatness directly (`listWorkflowDefinitions` called exactly once), so a future per-task regression fails here rather than being discovered as board latency. ## What was broken The filter named `todo`, so on a board whose waiting lane is called anything else **no row was enriched at all** — no error, no log line, just a silent fall back to the client's `steps.length === 0` heuristic. That heuristic is precisely what this enrichment was added to correct, so the card most likely to be mislabelled — real spec, zero parsed steps, already a scheduler dispatch candidate — sat on "Queued to plan" indefinitely. Over-inclusion is the safe direction and is chosen deliberately: a card in some other workflow's hold lane gets annotated as waiting, which is what a waiting card in a waiting lane should show. ## Revert proof (measured) Restore `task.column === "todo"`: ``` FAIL register-task-workflow-routes.awaiting-planning.test.ts > enriches a card in a RENAMED hold lane, not only one literally named todo expected undefined to be false ``` The other 8 cases in the file stay green — their harness store declares no `listWorkflowDefinitions`, so they run the degraded legacy-`todo` path. That compatibility is half the contract, which is why the new case brings its own store rather than widening the shared harness. ## Census | file | before | after | |---|---|---| | `packages/dashboard/src/routes/register-task-workflow-routes.ts` | 3 | 2 | The 2 remaining in that file are documented trait-fallback branches, not unconverted debt. The baseline also picks up `packages/core/src/task-store/moves.ts` 2 -> 0, already true on main and not from this diff. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit -p tsconfig.json` (`@fusion/dashboard`) — clean - `node scripts/lifecycle-column-census.mjs --strict` — exit 0 - targeted: `register-task-workflow-routes.awaiting-planning.test.ts` 9/9 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3a5058edd8 |
chore(census): re-record the baseline after moves.ts reached zero guards (#2844)
## Main is red; this fixes it `packages/engine/src/__tests__/census-baseline-corruption-guard.test.ts` fails on `main`: ``` × the census fails readably on a corrupt baseline > still succeeds against the repo's real baseline → expected 'lifecycle-column-census: scanned 1960…' to contain 'every file matches its baseline exact…' ``` A conversion took `packages/core/src/task-store/moves.ts` from **2 column guards to 0** without re-recording the baseline. `--strict` then reports `TIGHTENED` instead of the exact-match line the guard asserts. ## The whole diff ```diff - "packages/core/src/task-store/moves.ts": 2, ``` One removed allowance. Nothing else moved. ## Why committing it is the prescribed workflow, not a workaround The census says so itself when it tightens: > The baseline file has been rewritten downward. **COMMIT IT** so the allowance cannot be regrown into; > in CI this write is discarded with the runner, which is why the gate is green and not silent. That discard is the reason this recurs: the tightening only ever persists if a human commits the side-effect file, so a conversion PR that does not re-record leaves `main` red for the next person. Leaving the stale `2` in place would also keep an allowance open for guards to regrow into, which is the ratchet's entire purpose. **This cannot hide a regression.** `--strict` fails hard on a *rise*; it only rewrites when counts **drop**. An exit-0 tighten means every change was downward. ## Verification - `census-baseline-corruption-guard.test.ts` — **3/3** - `pnpm check:lifecycle-columns` — **exit 0**, "every file matches its baseline exactly" Not my change to `moves.ts` — found while running the full `engine-default` project (736 files) looking for fallout from my own merged fixes. That sweep also turned up a second red, fixed separately in #2840. No changeset: the baseline is internal tooling state, not published behaviour. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cea9637dfc |
feat(census): split the query class into read-shaped and write — most of the remainder must not be converted (#2837)
Splits the census's query class into **read-shaped** (convertible) and
**write** (must not be converted), reported beside the existing total.
On current `main`:
```
QUERY filters (column: "<legacy>"): 63
of those: 48 read-shaped (convertible), 5 writes (do NOT convert), 10 other
```
## Why the single number misleads
`column:` sits in an options-shaped object for both a source query and a
write, so the existing definition-vs-query rule cannot separate them.
The result reads as "dead reads to convert" — and after #2818 landed,
**48 of the 63 are `self-healing.ts` and the rest are largely not
convertible at all.**
Converting a write in this class is **harmful, not merely pointless**.
`async-persistence.ts` soft-deletes with `.set({ column: "archived",
deletedAt, … })`, and `getLiveTaskColumn` returns `"archived"` as a
**sentinel** for any soft-deleted row — the write and the sentinel have
to agree. A sweep that "finished the query class" by converting all 63
would break live-column resolution for every deleted task.
That is the same shape #2808 flagged for `recoveryRehome` moves. **Two
of the census-invisible classes now have members that must not be
fixed**, and in both cases the count alone cannot tell you which.
## Reported, not ratcheted
`properties.query` and `queryByFile` are byte-identical, so the pinned
baseline does not move and no open PR's Lint changes. The split is one
extra line of output.
**Changing what a ratchet enforces is the owner's call; improving what
it says is not.** Same line I drew when making marker-only failures
legible without loosening them.
## Honest limit
Read-shaped is a better filter than the raw count and **still not a
verdict**. `auto-merge-finalization.ts:242` is classified read-shaped
and must NOT be converted — its own comment records that it is
`getTaskHardMergeBlocker`'s review-eligible sentinel, deliberately not
re-keyed. Nothing mechanical would catch that; only the comment beside
it does. The split narrows a haystack to a readable list; it does not
decide the list.
## Verification
4 new cases — a `listTasks` filter counts read, a `.set()` tombstone
counts write, IR node definitions stay excluded from the class entirely
(the pre-existing rule must keep working), and the pinned total is
unchanged by the split. Revert proof: dropping the write branch fails
the tombstone case.
Census's own suites **87 passed**, gate **161 / 13 / 487 / 71**, lint
clean, `--strict` exits 0.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
2c24966d0c |
fleet: the app-side remainder 18 → 0 — Archive/Revert and diff stats were silently absent on a renamed board (#2731)
Three **genuinely free** clusters in one layer and one idiom —
`Column.tsx` (7), `ListView.tsx` (6), `useTaskDiffStats.ts` (5). I built
the claimed-file set from every open PR's diff before starting, having
duplicated a claimed cluster last round.
## Census
| file | before | after |
|---|---:|---:|
| `Column.tsx` | 7 | **0** |
| `ListView.tsx` | 6 | **0** |
| `useTaskDiffStats.ts` | 5 | **0** |
**16 converted; 2 reclassified with a reason** — the two are accounted
for separately below so the numbers stay honest.
## Three silent failures, not three style nits
- **`ListView` Archive and Revert** were gated on `task.column ===
"done"` / `=== "archived"`, so on a board with renamed terminal lanes
**they did not render at all**. No error, no log — the operator simply
cannot archive or revert from the list.
- **`useTaskDiffStats`** compared a bare `column: string` to
`done`/`in-progress`/`in-review`, so on a renamed board it **fetched
nothing** and the row showed no changes.
- **`ListView` progress display** had the same shape for the WIP lane.
## The `?? {}` is the whole subtlety
Every `Column.tsx` site was `workflowMode ? <trait> : column ===
"<id>"`. One adapter now feeds the shared helpers:
```ts
const columnRoleFlags = workflowMode ? (columnFlags ?? {}) : undefined;
```
`workflowMode` means **traits are the only authority**, so a
workflow-mode column with no resolved flags must answer `false` — which
`Boolean(columnFlags?.archived)` did. Passing `undefined` to a role
helper instead selects its **legacy id fallback**, so a flagless
workflow-mode column would start matching on its id. An empty object
keeps the helper on its trait branch. Legacy mode passes `undefined`
deliberately: there the id fallback *is* the answer, and routing it
through the helpers is the point.
## Two things I deliberately did not do
**`isTodoLikeColumn` keeps its own trait arm.** Adopting
`isPreImplementationColumnRole` would widen its fallback from `todo`
alone to `{todo, triage}`, handing a legacy `triage` column a bulk
replan affordance it does not have today — a behaviour change hiding
inside a de-duplication. Only its *fallback* is routed through a helper.
**The `mode === "done"` pair is reclassified, not converted.** It is the
hook's own `"done" | "active"` discriminant, assigned three lines from
`shouldFetchDoneTask` — not a column id, with no trait to resolve. The
census counts it because the receiver is compared to the string `done`,
which is a classifier limit. Marked deliberate and **recorded in
`deliberateByFile`**, so that file's `byFile` drop is 5 while its
conversion count is 3.
One genuine simplification fell out: `workflowMode ? isReviewColumn :
column === "in-review"`, where `isReviewColumn` is *itself* that same
ternary. Both arms already agreed with it — collapsing is
behaviour-identical.
## Revert proof
Restoring the id comparisons on the ListView row menu fails the new
renamed-lane case with `Unable to find an accessible element with the
role "menuitem" and name "Archive"`.
Driven through the **real `fetchBoardWorkflows` seam** with a renamed
vocabulary — payload → `listColumns` → `columnFlagsById` → row menu —
rather than by injecting flags, so the assertion covers the path the
component actually uses. The DEFAULT-vocabulary path passes either way,
which is exactly why the renamed case has to exist.
## Verification
`pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **375 passed** across
Column / ListView / useTaskDiffStats / role-invariance / columnRoles ·
dashboard `tsc -p tsconfig.app.json` clean · `pnpm lint` clean · census
`--strict` exits 0.
`TaskCard.tsx` is touched only to pass the new optional `columnFlags`
through; its own census count is unchanged at 3. The 2 `TaskCard` reds
in that suite are the known pre-existing CSS-var geometry assertions.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved workflow lane handling when columns are renamed or assigned
roles through workflow settings.
* Archive and Revert actions now remain available for completed and
archived tasks in renamed lanes.
* Corrected task progress and diff-stat behavior across active, review,
completed, and archived lanes.
* Updated bulk actions, sorting controls, and auto-merge controls to
respond consistently to workflow roles.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
109204c590 |
fix: the query class — three sweeps that never ran on a renamed board (#2818)
Three sweeps that **never ran at all** on a renamed board, plus the shared answer the rest of the class needs. Consolidated from three handoff branches so the helper appears once. #2811 merged, so this is my only open PR. `#2800` measured this class and shipped evidence deliberately without conversions: `listTasks({ column: "<literal>" })` filters in the store, so on a renamed board the read returns an **empty array** and the sweep it feeds does nothing. The census scores the comparison *inside* the loop, never the query above it. ## What was broken | file | census count | what actually happened on a renamed board | |---|---|---| | `backlog-pressure-reporter.ts` | **0** | both reads empty, ratio computed as 0/0 — **the alert never fired**, on a board that may be under exactly the pressure it reports | | `stale-task-reporter.ts` | **0** | both reads empty — **no stale-task signal ever raised**, where work is most likely sitting unnoticed | | `restart-recovery-coordinator.ts` | flagged | sweep never ran — **an engine restart left interrupted tasks stuck with no requeue** | Two of the three have a census count of **zero**. They contain no lifecycle comparison at all, so they have never appeared in the backlog, in a per-file list, or in any "N → 0" claim — and were completely inert. **A file at zero is not evidence of anything.** ## The shared answer, and what it is not Every existing resolver answers a **per-task** question. A query has no task in hand, so it needs the project-level one: every column any workflow declares for a role, unioned with the legacy ids so a board mid-rename still finds rows under the old ones. The set is never empty, so a caller cannot accidentally query nothing. The header states what it is **not**: answering a per-card question from the union would mark a card as review because some *other* workflow calls its column review — the flat-set mistake this program has made four times. ## The finding that generalises: the query is rarely the whole defect `stale-task-reporter` **still reported zero after the query was fixed** — `getTaskAgeStalenessSignal` defaults to the legacy pair, so a card the query now returned was refused inside the signal. Converting only the query would have looked like a fix and changed nothing. That is a caveat on #2800's approach, offered as refinement rather than correction: **asserting the query ARGUMENT is right when pinning a known defect** (the outcome is 0 either way) **and insufficient when proving a fix**, because the outcome is the only thing that distinguishes a real conversion from a deeper one. All three conversions here assert outcomes. `restart-recovery` had three layers — query, a redundant re-assertion (deleted; a test pins the `paused` guard it did contribute), and a move destination that was **already** resolved but whose warning comment was stale. A stale warning is its own hazard: it told the next reader a defect existed where none did. ## Verification - helper **8 passed** · three reporter/coordinator suites **29 passed** - `pnpm test:gate` **161 / 13 / 487 / 71** · lint clean · `--strict` exits 0 · four `tsc` targets clean - each conversion revert-proven independently; the failing case is named in each test header ## Two mistakes worth recording **The helper's own test caught a bug in it.** My first draft wrapped the definition loop in one `try`, and `parseWorkflowIr` **validates** rather than parses — one malformed row would have returned legacy-only lanes for *every* workflow, indistinguishable from the bug it exists to fix. Now isolated per definition. **I clobbered the core barrel** by taking `index.ts` wholesale from a handoff branch, dropping two exports `main` had added since; three packages stopped compiling. Taking a file from another branch takes its whole contents, including what is now stale — for a barrel that is nearly always wrong. Re-applied as a single edit on top of `main`. ## Not included `self-healing.ts`'s 49 — actively owned and mid-conversion; an outside refactor there produces conflicting halves of one sweep. `project-engine.ts` (7) and `executor.ts` (2) need their own read of what each sweep does with the rows, which these three are the argument for. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ed83fd6ec3 |
batch-core: node-override guards let a running task be re-routed on a renamed board (45 → 43) (#2821)
## The defect Two guards in `node-override-guard.ts` answered a **role** question with a **column name**: - **`task.column === "in-progress"`** refuses changing a task's node override mid-flight. On a renamed board it never matched, so an operator could re-route a **running** task — precisely what the guard exists to prevent, and the failure is silent because the guard simply returns `allowed: true`. - **`task.column !== "done"`** gates overriding *to* the terminal node. On a renamed board it never matched either, so the override was refused for exactly the tasks that had legitimately reached the end node. Both fail in the direction that looks like normal behaviour rather than an error. ## Why the lanes are injected rather than resolved in place `validateNodeOverrideChange` is **synchronous by design**, and its existing `isTerminalNodeId` option already establishes the pattern: callers with cheap IR access inject, callers without keep a documented literal fallback. **Both production callers now supply the lanes** — `branch-and-pr-entities.ts:594` (which already injected `isTerminalNodeId`) and `task-update.ts:53`. That was the deciding factor: an optional parameter that only tests fill is the inert-injection shape this program keeps finding, where a guard reads as converted, its test passes because the test injects the value, and production keeps the literal. I checked both call sites had a store in scope *before* adding the option. `resolveNodeOverrideLanes` lives beside the guard rather than in the callers, so the two cannot drift about what "executing" and "completed" mean. ## Fallbacks A workflow expressing **no trait on any column** is a v1 upgrade — `synthesizeDefaultColumns` emits `traits: []` everywhere — not a board without these roles, so it keeps the legacy ids. Same for an unresolvable workflow. Both preserve exactly the behaviour the literals already had. ## Verification - **Mutation-verified per guard:** restoring `task.column === "in-progress"` fails a case; restoring `task.column !== "done"` fails a different one. - The suite also pins the paired negative — resolving lanes must not turn the guard into a blanket refusal for a task outside every WIP lane. - `node-override-guard.test.ts` → 27 passed - `pnpm test:gate` → 161 + 487 + 13 + 71 - `--strict` → exit 0; `tsc --noEmit` and `pnpm lint` → 0 errors Census: batch-core scope **45 → 43**; repo total **255**. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Node overrides now correctly recognize workflow-defined in-progress and completed lanes, including renamed columns. - Override validation falls back safely for legacy or unresolved workflows. - Prevented validation from using stale task-column information during updates. - **Tests** - Added coverage for workflow lane resolution, legacy fallbacks, renamed lanes, and override eligibility. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ba40942a10 |
batch-dashboard-app: 75 → 2 across packages/dashboard/app — the last two are deliberate, not missed (#2772)
**Batch branch is live: `batch-dashboard-app`.** Push conversions here as commits rather than opening per-file PRs — that is the CI-run bottleneck this model removes. **One-line ownership note for you to arbitrate:** you have addressed me as U11, U12 and U7 at different points, so the `u12 worker -> batch-dashboard-app` mapping is ambiguous from my side. I claimed it because `dashboard/app` is where I have done the most work this session (TaskContextMenu, Column, TaskCard, TaskDetailModal, columnRoles, taskActivity) and I know which of its guards are load-bearing fallbacks. **If another worker is the intended owner, say so and I will hand the branch over rather than both of us pushing to it** — two workers on one shared branch is exactly what silently discarded a reviewed fix in #2645 today. ## The work order (measured at branch point, tests excluded) **75 guards across 32 files.** Largest: `TaskContextMenu.tsx` 9 · `Column.tsx` 7 · `ListView.tsx` 6 · `TaskDetailModal.tsx` 4 · then a long tail of 3s, 2s and 1s. Full per-file list is in the committed work order so feeders can claim without re-measuring. ## Two rules this surface keeps tripping on **1. A literal after `??`, or in the `else` of a `flags ?` ternary, is a DEGRADED-MODE answer — not an unconverted guard.** Two real states reach it: the **pre-load window** (board renders before the workflows fetch resolves) and a card stranded on an id its workflow no longer declares. In both, `columnFlagsById` has no entry at all. Deleting the fallback does not remove a decision — it substitutes "no role" silently, and affordances vanish during first paint. Those sites reach 0 by **marking**, not deleting. Expect `TaskContextMenu.tsx` and the `utils` files to be **mostly marks**. A "9 → 0" that deleted 9 fallbacks is a regression wearing a green census. **2. A marker excuses ONLY the construct it is attached to** — the statement or function holding the literal, not a sibling declaration. This has cost three passes, two of them mine; my first attempt on `reliability-metrics.ts` scored **1 of 6**. **Verify by the count moving, not by the comment existing.** With the ratchet gate-blocking, a mis-marked batch either wedges the gate or locks the miss into a re-recorded baseline. ## Status Opening commit is the work order only — **0 of 75 converted so far.** I am near the end of my context, so I am establishing the branch and the shared list rather than starting conversions I cannot finish cleanly. Feeders can begin immediately; I will keep the branch rebased. My other PR **#2762** (`live-agent-count.ts` 6 → 0) is green and unconflicted — per your rule it should land rather than fold into a batch, and it is `packages/core` so it belongs to batch-core anyway. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Task UI now resolves workflow “column roles” per task to drive diffs/merge details, routing/steering, progress/runtime visibility, and review badges. * Right-dock/overflow views and dev-server now use per-task column traits for “executing” behavior and dependency-based “Up Next” eligibility. * **Bug Fixes** * Fixed bulk action selection/delete/archive eligibility and prevented cross-workflow role leakage. * Made in-review/stale-paused-review, stuck, and effective executor/validator model logic role-aware. * **Tests** * Added regression coverage for degraded-flag behavior and ensured resolved-flag props aren’t ignored. * Added a static check to fail builds on inert optional flag seams. * **Documentation** * Updated batch work-order and mega-batch branch guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- ## Late addition: the seam gate was masking a real offender `scripts/check-inert-flag-seams.mjs` matched call sites by NAME, so two same-named functions in different modules were conflated. I had documented that as a known false-positive source and moved on — reports mentioning `sortTasksForDisplayColumn` are noise, read past them. That annotation was the damage. Core's `sortTasksForDisplayColumn` genuinely never receives its `columnFlags` argument outside its own tests. The dashboard's separate function of the same name (`app/components/taskSorting.ts`), called with up to five arguments from `Lane`/`Board`/`ListView`, was raising the arg-count max and clearing core's seam. The offender was behind a row everyone had been told to skip. The gate now records the module each callee is imported from and matches it against the seam's declaring module. **Measured, by reverting the change:** the scan prints `17 seams, all supplied` and emits **no row** for the function. With the change, it is reported. Both directions watched. Reported on #2783 rather than fixed from outside — core owns it, and "wire the flags" vs "drop the parameter and let the literal stay counted" is their judgment call. TEMPORARY allow-list entry carries it meanwhile; the existing staleness check fails the moment the site becomes supplied, so the entry cannot outlive the fix. Two known limits remain, both inherent to name matching and both documented in the script: the one-supplier floor, and the `__tests__` exclusion (hence the two permanent `ALLOWED` entries). ## And the one-supplier floor, closed the same way I wrote in the section above that the floor "hasn't cost anything yet." That is verbatim the reasoning that kept the imported-shadow bug alive, so I closed it instead of leaving the note. `best < arity` asked only whether SOME caller supplied the argument. One correct call site cleared the seam while every sibling took the legacy fallback — the `isTaskStuck` defect class, where two of three sites omitted the flags and the gate stayed green because the third was right. Review caught that one. A partially-supplied seam is the harder of the two: wholly-unsupplied is uniformly wrong, this works on the board you tested and degrades on the column you did not. **Measured:** dropping the flags argument at `Column.tsx`'s supplied call site produces `supplied by 5/6 call sites; omitted at packages/dashboard/app/components/Column.tsx:1 (of 2)`; restoring returns `all supplied at every call site`. Red and green both watched. Two real omissions found, both on `isNearDuplicateCanonicalInactive`: - **`TaskDetailModal.tsx`** — deliberate, and it **corrects a note I left at that site**. The old note said hoisting the flags state was "the actual fix." It is not, for this call: the flags in scope describe the *modal's* task, and the canonical is a **different task** on a column this component never resolves. Passing them would type-check, read as a conversion, and answer about the wrong task — exactly what `column-role-degraded-flags.test.ts` exists to catch. Supplying it correctly needs a fetch, which is a data change and out of scope. - **`core/task-store/branch-group-ops.ts`** — genuinely wireable (the impl is async and already holds `store` and `canonicalId`). Reported on #2783, not edited from outside. Exemptions for this class are keyed by **call site** (`<file>::<function>`), not by function name. A name-level entry would waive every site of a partially-supplied seam, which is backwards — its other sites are correct and are the reason the omission is worth reporting. Both entries carry the same staleness check as the name-level list and cannot outlive their fix. Remaining known limit, now the only one: the `__tests__` exclusion, which makes a test-only export read as having no callers. That is what the two permanent `ALLOWED` entries are. ## The `__tests__` exclusion, and two allow-list entries built on false reasons Named as the "last remaining limit" above, so it got closed too. The scan now reads test files for call sites — but counts them **separately**, and a test never clears a seam. That direction is the dangerous one: counting test callers as suppliers would have re-hidden core's `sortTasksForDisplayColumn`, whose only suppliers are its own tests. Measured by lifting its exemption: still reported. Both permanent allow-list entries claimed the scanner couldn't see their callers. **Both reasons were false**, and reading tests is what proved it: - **`evaluateMergeBlockerGuard`** — zero callers in tests either. Its only reference in the repo is its own declaration; never registered as a trait hook; the `evaluateDefaultWorkflowGuards` reader its file header credits does not exist. The `lifecycleColumns` conversion went onto dead code, and its note describes a crossing the guard cannot make. Reported on #2783, including the two things I am explicitly *not* concluding (no `"guard"` hook is registered in production; whether that is residue or a dropped registration needs core's intent). - **`isRecoverableMissingWorktreeReviewFailure`** — 5 test call sites. It wraps `...WithProgress`/`...NoProgress`, the live pair called from `self-healing.ts`, both supplying `reviewColumns`. Entry kept, true reason recorded. ### A wrong turn, recorded because it is the failure mode this PR is about I first classified no-production-caller seams as *informational* when they weren't re-exported from a package index, reasoning that a public export might be called externally. That silently downgraded `sortTasksForDisplayColumn` — a confirmed real offender — from failing to a footnote. Publication status has nothing to do with whether there is production behaviour to be wrong. Reverted to the simple rule: no production caller means inert, and it fails. It is worth stating plainly because it is the exact shape of everything else in this PR: a change that made the gate read *cleaner* while making it catch *less*, and it type-checked, passed every test, and would have reviewed fine. ### Where that leaves the check Every blind spot named in this PR has now been closed, and **each one produced a real defect within minutes of closing it** — imported shadows, the one-supplier floor, the `__tests__` exclusion. Four verified findings went to core, one to engine. I would not read the remaining ~240 guards' green gates as evidence that they are clean; I would read them as untested. ## Two guards for one question, one of them worse Having hardened the script, I checked its older twin rather than assuming it was fine. `resolved-flags-seams-have-suppliers.test.ts` carried its own copy of the trailing-flags-parameter check — written before the script existed — with **all three** holes the script has since closed. **Measured on one reintroduced defect** (dropping the flags argument at `Column.tsx`'s supplied `isNearDuplicateCanonicalInactive` call): | | result | |---|---| | `scripts/check-inert-flag-seams.mjs` | `supplied by 5/6 call sites; omitted at .../Column.tsx:1 (of 2)` | | this test's arity half | **3 passed** | Deleted the arity half. Redundancy between a strong and a weak check isn't redundancy — it's a green result available to whoever runs the weak one, and there was no signal at the call site telling you which you were looking at. The **props-shape half stays**: it has no twin in the script, and I confirmed it still fires by reintroducing the original `PrPanel` defect (outer component stops destructuring `taskColumnFlags`) — it reports `PrPanel declares taskColumnFlags but never takes it`. Dashboard app suite: **113 files / 3921 tests** (was 3922 — the deleted case is the difference). ## The gate started catching defects as they landed Syncing with main brought in three fresh conversions from other workers. The hardened check flagged all three immediately — the first time these guards have fired on someone else's landed code rather than on my own. - **`TaskCard`** — `getRunningOptionalGateBadge(task)` omitted flags while *both* `ListView` sites supplied. Fixed, and `taskColumnFlags` added to the `useMemo` deps: no `exhaustive-deps` rule here, so a memo that reads flags without listing them keeps the first-paint `undefined` answer and reproduces the bug through staleness instead of omission. - **`TaskTokenStatsPanel`** — `getTotalAgentActiveMs` omitted while `TaskCard` supplied, so the same runtime number came from the real column on a card and from legacy ids in the detail modal. Now takes `columnFlags`, supplied from `detailColumnFlags` — correct here because the panel renders the modal's **own** task, unlike the near-duplicate canonical above. - **`ListView` ×2** — passed `columnFlagsById.get(task.column)`, the cross-workflow **union**. A task whose own workflow doesn't declare that column gets a *neighbour workflow's* traits. The landed comment justified it as "this list already owns `columnFlagsById`" — exactly the reasoning `column-role-degraded-flags.test.ts` exists to reject. It failed on merge and is how I found this. Also: the `getTotalAgentActiveMs` exemption I was carrying **self-retired**. Main wired the seam, the staleness check failed the entry, and I removed it. That mechanism has now paid for itself once. ### Pre-existing, NOT from this PR: `App.test.tsx` is red on main `app/components/__tests__/App.test.tsx` fails **10 of 141** identically with my changes, with my changes stashed, and with main's own `App.tsx` restored. Not mine, and not in the merge gate. **Bisected on clean `main` checkouts, so this is measured rather than inferred:** | commit | date | result | |---|---|---| | `main~400` (`41d60f0355`) | 2026-07-25 | **140 passed** (140 tests) | | `main~275` (`74d6513fae`) | 2026-07-27 | 3 failed / 141 | | `main~210` (`d2ce1ba8b5`) | 2026-07-29 | 10 failed / 141 | | `main` (`6fc98fd6c7`) | 2026-07-30 | 10 failed / 141 | So it is **not one regression** — it degraded in two stages across 2026-07-25 → 07-29, and the test file itself changed in that window (140 → 141 tests). Three commits touched it there: `73b2a32e2b`, `f26cbedf4f`, `f157bf7460`. That window overlaps the workflow-owned lifecycle migration, which is suggestive but not something I confirmed. The failures are render-level, not assertion-level — `Unable to find an element with the text: + New Task`, `Unable to find role="dialog"`, `Unable to find ... Back nav task`. The board appears to render nothing. That reads like a real regression or a harness mismatch after the lifecycle migration, not a flake, so I have deliberately **not** quarantined it — quarantine is for flakes, and using it here would hide the signal. Flagging for whoever owns `App.tsx`. My suites: `app/__tests__` **113 files / 3921 tests** green, `tsc` 0, lint 0, census `--strict` 0, seam gate 0. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6fc98fd6c7 |
the third census-invisible class: 51 hardcoded moveTask destinations, measured — and duplicates never archived on a renamed board (#2808)
A third census-invisible class, measured — plus the two worst instances
fixed.
## The shape
```ts
if (task.column !== "in-review") { … return; } // the census counts THIS
await this.store.moveTask(taskId, "in-progress"); // and cannot see THIS
```
The census is an AST scan for **comparisons**. A `moveTask` destination
is a **call argument**, so no backlog entry ever points at one.
Converting the guard alone is *worse than converting neither*: the
handler starts admitting work on a renamed board and then tries to move
the card into a lane that board may not declare.
This bit twice in one week — #2797 (`branch-worktree` requeued into a
lane that may not exist) and #2807 (a GitHub "changes requested" review
dropped, then a move to a hardcoded `in-progress`). Both times it was
found only because the guard *next to it* happened to be under
conversion. So I went looking.
## Measured
Across `core`/`engine`/`dashboard`/`cli`/`plugins`, excluding
`__tests__`/`*.test.*` and comment lines:
| | count |
| --- | ---: |
| hardcoded `moveTask` destinations in production | **51** |
| …passing `recoveryRehome: true` — **deliberate**, not defects | 22 |
| …plain, rejected on a board that does not declare the target | **29**
|
**The 22 must not be "fixed".** `moves.ts` exempts them on purpose
(#1411): a card stranded in an undeclared column has to stay rescuable
to a legacy safe-landing column, or it can never be recovered at all. A
sweep that converts them deletes the rescue path. That distinction is
the reason this is 29 and not 51, and it is why I measured before
writing.
## Why this got sharper recently
The `workflowHasColumn(workflowIr, toColumn)` rejection used to sit
inside a block gated on `isWorkflowColumnsCompatibilityFlagEnabled` — a
settings key **nothing in production writes** — so it never executed and
the legacy `VALID_TRANSITIONS` table decided instead. U12 hoisted it out
of that dead branch and it is now live, proven on a real store by
`live-move-path-undeclared-target.test.ts`:
```
moveTask(card in "todo" -> "triage") now REJECTS: /Unknown column for this workflow/
```
That changed the failure mode of all 29 from *"silently lands the card
in an undeclared column"* to *"throws"*.
**29 is not a crash count.** Whether a throw surfaces or disappears
depends on whether the caller catches, which is per-site and I did
**not** measure it — the doc says so explicitly rather than letting the
number imply severity it hasn't earned.
## Fixed here: 9 of the 29
`duplicate-intake` and `duplicate-guard` both archive a duplicate. On a
renamed archive lane the move is rejected, so **the duplicate is never
archived and keeps sitting on the operator's board as live work** — and
in `duplicate-guard` the row has already been stamped
`deterministicDuplicateOf`, so it is *marked* a duplicate while
occupying an active lane. Half-applied, which is the same trap as
#2797's branch clear.
Both now resolve the `archived`-trait column from the task's own
workflow through one shared helper, unioned with the legacy id.
**`cli/commands/task-lifecycle`** — `finalizePullRequestMerge` and
`finalizeNoOpMergeTask` both move the card to a hardcoded `"done"`, and
both run `updateTask({ status: null, mergeRetries: 0 })` *first*. On a
rejection the merge has already landed and the bookkeeping is already
cleared while the card never reaches its complete lane: the operator
sees a merged branch, a card still sitting in review, and a reset retry
counter. Same half-applied shape as #2797's branch clear. Both now route
through one resolver so they cannot drift.
**`contamination` / `foreign-only-contamination` (×2) /
`restart-recovery-coordinator`** — four recovery requeues to a hardcoded
`"todo"`, none of them a `recoveryRehome` escape. On a board without
that column the move is rejected and **the recovery never completes** —
the card stays contaminated or stranded, which is precisely the state
these paths exist to clear.
**Consolidation.** `resolveReboundTargetForTask` and
`resolveArchiveTargetForTask` now live beside
`resolveTaskLifecycleColumns` in `workflow-lifecycle-traits`, already
the store-dependent resolution seam. My first pass put the archive
helper inside `duplicate-intake` and had `duplicate-guard` import it
from there — wrong home, and it would have grown a copy per caller as
more sites converted. Seven call sites now share two definitions.
**Plain (non-`recoveryRehome`) destinations: 29 → 21.**
**Coverage on the CLI pair is scoped, and I'd rather say so than imply
more:** the test covers the *resolver*, not the two call sites. Both
enclosing functions are private and reachable only through
`processPullRequest`, which needs a live GitHub surface — exporting them
purely to test wiring is a worse trade than stating what is covered.
Three cases: renamed lane resolves, no-workflow falls back to the legacy
id (which also pins that a default board is byte-identical), and a
throwing lookup falls back.
## Revert result (measured)
| conversion | reverted → |
| --- | --- |
| duplicate archive destination | new case fails — `moveTask` called
with `"archived"` on a board whose archive lane is `boxed` |
| CLI complete-lane resolver | replacing the body with a bare `return
"done"` fails the renamed case |
| both move-target resolvers | replacing either body with a bare return
of its legacy id fails 5 cases across the resolver suite and
`duplicate-guard` |
Each resolver has a **non-vacuous companion** asserting it does *not*
return the legacy id on a renamed board — without it, a resolver
returning any string would pass. The fallback cases are load-bearing
rather than padding: `resolveWorkflowIrForTask` degrades to the built-in
IR rather than throwing, and the built-in rebound/archive lanes *are*
`todo`/`archived`, so those cases also pin that a default board is
byte-identical.
The pre-existing case asserting the legacy `"archived"` passes both
ways, which is exactly why it could not detect this and why the new one
supplies a workflow.
## Ownership note
`packages/core` was `batch-core`'s territory and `packages/cli` was
`batch-cli-plugins`'. Both batches have landed, and this is
newly-discovered work in the class documented here rather than leftover
conversion backlog. Four sites, two shared helpers — happy for either
half to move if those owners would rather carry it.
## Verification
- `pnpm test:gate` — 161 + 487 + 13 + 71, green
- `duplicate-guard` + `duplicate-intake` — 40 passed
- `tsc` on core and engine — clean
- `pnpm lint`, `check:changesets`, census `--strict` — all clean (run
explicitly; a clean `pnpm lint` alone is not evidence the CI Lint check
passes)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Duplicate tasks are now archived to each workflow’s configured archive
lane.
- Completed tasks are moved to the workflow-specific completion lane,
with a safe fallback for older workflows.
- Recovery and requeue actions now use each workflow’s configured
rebound lane instead of assuming a fixed destination.
- **Documentation**
- Added guidance on avoiding failures caused by hardcoded workflow
destinations and incomplete lifecycle conversions.
- **Tests**
- Added coverage for renamed workflow lanes, fallback behavior,
duplicate archiving, and recovery destinations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|