5cb731420cd8d5a948c9c7ebcd2dbdff7dbf7190
12670 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
gsxdsm
|
5cb731420c |
test(engine): re-point the stale-spec ratchet at the improved guard (#2863)
## Main is red; this fixes it
`executor-stale-spec-active-lanes.test.ts` — 2 failures on `main`:
```
× resolves the task's lifecycle columns before deciding the skip
→ expected source to contain 'const activeLifecycle = resolveLifecy…'
× adds the wip, review and complete lanes to the active set
```
**Nothing regressed — the guard got better and the ratchet didn't
follow.**
## What changed in the product
The stale-spec skip used to build its active set from
`resolveLifecycleColumns(...)`, taking `?.wip`, `?.review` and
`?.complete`. That returns the **first** column carrying each trait, so
a board with two wip lanes — or a review lane plus a second
merge-blocking one — had only one of each recognised as active. A card
in the other read as **inactive**, and its prompt file was treated as
reclaimable.
It now resolves the IR once and unions `columnsWithFlag` over five
flags, which returns **every** column carrying each:
```ts
const activeIr = await resolveWorkflowIrForTask(this.store, task.id);
const activeColumns = new Set<string>(["in-progress", "in-review", "done"]);
if (activeIr) {
for (const flag of ["countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview", "complete"] as const) {
for (const lane of columnsWithFlag(activeIr, flag)) activeColumns.add(lane);
```
That is a real fix to an arity bug, and the legacy trio stays unioned in
for the documented reason: under-reporting active is the destructive
direction.
## Why re-point rather than loosen
This is a **source ratchet** in the `engine-no-blocking-shellout` style,
and its entire value is that it fails on a revert. A substring loose
enough to match both the old and new shapes would keep the file green
through exactly the regression it exists to catch.
So the assertions now name the new shape precisely, including the **flag
list**, so dropping one of the five is caught here too.
**Mutation-verified:** replacing the IR resolution with `undefined`
fails the ratchet. It still does its job.
## Scope
The file's own header notes this is *not* a behavioural proof — the
guard sits inside `execute()` behind worktree and session setup a unit
test has no business standing up, and it asks whoever next touches that
scaffolding to add the end-to-end case. Re-pointing is maintenance; I
have not taken on that harness here, and the note still stands.
## Verification
- `executor-stale-spec-active-lanes.test.ts` — **4/4**, fails on revert
- `pnpm lint` — clean
Test-only; no changeset. Found by running the full `engine-default`
project after #2855 merged — the remaining `main` red is my own audit
case, fixed by the pending #2857.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
9e1deffa72 |
fix(reliability): the review-failure headline read 0% because both inputs were zero (#2861)
The Reliability panel's headline metric is computed from two queries
that name lanes:
```ts
scopedStore.getTaskMovedCountsByDay({ …, toColumn: "in-review" }),
scopedStore.getTaskMovedCountsByDay({ …, fromColumn: "in-review", toColumn: "in-progress" }),
…
const headline = inReviewFailureRate7d(enteredByDay, bouncedByDay, nowMs);
```
On a board that renamed either lane, **both return `{}`**. Every per-day
count is zero, and the headline then divides one zero by another and
reports a healthy rate.
**This is the worst shape a lifecycle defect takes.** It produces a
*number*, not an error, and the number is reassuring. An operator
reading a 0% review-failure rate beside a populated audit event list has
no reason to suspect the metric is blind — the same failure mode as the
analytics `tasksInProgress: 0` sitting next to correct cost totals.
## Why a union is correct here, not a compromise
`getTaskMovedCountsByDay` takes **one** column per side, so the lanes
are resolved to sets and the query is issued per `(from, to)` pair and
summed.
The important part is *why* summing over a union is the right answer
rather than a widening hack. These read **move history**, and a past
move recorded the column name as it was at the time — the same reasoning
that keeps `tasksEnteredInReviewPerDay` in this module matching recorded
values verbatim, marked DELIBERATE-LITERAL. A board renamed last month
therefore has old rows under the old id and new rows under the new one,
so the correct query covers **both**. That is exactly the set
`resolveProjectColumnsForRoles` returns, with the legacy id always
unioned in.
Asking for either name alone is the bug — not a choice between them.
No double-counting: a move event has exactly one `(from, to)` pair, so
the queries partition the events rather than overlapping.
**The common path does not get more expensive.** On the built-in board
this issues the same two queries as before, and there is a test
asserting the call count so a future change cannot quietly turn one
query into N.
## Revert proof (measured)
Collapse the sets back to the single literals:
```
AssertionError: expected { '2026-07-01': 1 } to deeply equal { '2026-07-01': 2, '2026-07-02': 3 }
AssertionError: expected { '2026-07-01': 1 } to deeply equal { '2026-07-01': 2, '2026-07-03': 5 }
```
The helpers live in `reliability-metrics.ts` rather than inline in
`server.ts` specifically so they are testable without booting an express
app.
## Verification
- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm smoke:boot` — PASS (`fn --help`, real `serve` `/api/health` 200,
clean shutdown)
- `pnpm lint` — clean
- `tsc --noEmit` (`@fusion/dashboard`) — clean
- `reliability-metrics.test.ts` — 15 passed
- census `--strict` — exit 0 (unchanged: query filters are not
comparisons)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
5792452f0a |
docs(workflow-learnings): mutation testing has one blind spot — your own imagination (#2858)
The most transferable thing this lane produced, and it is a correction to advice I wrote earlier in the same document. ## The gap Every other section here says *"watch the guard go red before you trust it."* That rule is necessary and **not sufficient**, and the way it fails cost the most. Three instruments were written during this program. Each was mutation-tested in both directions before shipping. Each was green. Reviewers then found, in those same instruments: - a **file-level pre-filter** that skipped whole files, so a forbidden site added to a file with no other SQL was invisible; - an **anchored pattern** that missed qualified and compound fragments (`t."column" = 'done'`); - a scan over **SOURCE text**, where a double-quoted TS string still spells `\"column\"` with the backslashes in it; - an operator list of `= != <>` that never considered **`IN (...)`**; - and worst, a template scan that joined only the **static spans** — so a Drizzle query, which puts the COLUMN in the interpolation hole and the legacy id in the static text, matched nothing. **That gate was blind on the exact files it was built to freeze.** Enabling that one shape took the population from 14 to 31 and revealed five previously invisible files. ## Why the mutation tests could not catch any of them All five are **false negatives**, and the reason is structural rather than sloppy: > A mutation you write is a mutation you already imagined, so it lands inside the space your scanner understands. Reintroducing a defect the checker was designed around proves the checker still handles that defect. It says nothing about shapes you never modelled. ## What does find them 1. **Run the instrument against the code it was written for and read the hits by hand.** The Drizzle blindness was obvious the moment someone asked *"why is the merge-queue query — the reason this exists — not in the output?"* 2. **Prefer one unanchored pattern over a fast pre-filter plus a precise one.** Every false negative above came from two patterns disagreeing about whether to run the real check at all. A pre-filter is a second, weaker specification of the thing you are testing. 3. **Treat a guard's own count as a claim to verify, not a result.** "14 sites" read as coverage for days; it was the subset one scanner happened to model. A false positive is loud and gets fixed. **A false negative prints a baseline and reads as coverage.** Docs only — no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b9785ec10f |
fix(tests): stop the census guard reddening main on every legitimate conversion (#2856)
## This is the cause of four main reds today, not a fifth instance of them I have now fixed the census baseline on `main` three times (#2814, plus a withdrawn branch, plus watching #2811 and #2844 do the same). Rather than do it a fourth time, here is why it keeps happening. `census-baseline-corruption-guard` asserted: ```ts expect(result).toContain("every file matches its baseline exactly"); ``` That demands the **committed baseline be byte-in-step with the tree at all times**. **It is not, by design.** A conversion PR that removes guards leaves the tree holding *fewer* than the baseline allows, and the CLI treats that as the good case — it tightens the pin and exits 0. Measured directly: ``` simulated drop → EXIT ON DROP: 0 "The baseline file has been rewritten downward. COMMIT IT … in CI this write is discarded with the runner, which is why the gate is green and not silent." ``` So the ratchet was already happy while this test went red. Every conversion that did not *also* re-record the baseline turned `main` red for a condition that was never a defect. That is the mechanism behind **#2783's markers, #2837's query split and two more** — plus three collisions between workers racing to re-record the same file (#2811/#2814, #2844, and a branch of mine I deleted rather than open as a duplicate). ## The fix matches the guard's own stated intent Its comment says: *"a guard that always fails is no guard"* — its job is to prove the **corruption** diagnosis does not false-positive on a healthy file. **A tightened baseline is healthy.** So it now asserts what that needs: - the run **succeeds** — `execFileSync` throws on a non-zero exit, so a **rise still fails before any assertion runs**; a rise is real debt and must stay loud - the corruption diagnosis is **absent** - the outcome is one of the two healthy shapes the CLI can report ## Measured discrimination — all four cases | scenario | before | after | |---|---|---| | **drop** (legitimate conversion) | ❌ 1 failed — *the false red* | ✅ 3 passed | | **rise** (real debt) | ❌ fails | ❌ **still fails** | | **corrupt JSON** | ❌ fails | ❌ still fails (case unchanged) | | **unreadable file** | ❌ fails | ❌ still fails (case unchanged) | Only *"somebody converted guards and has not re-recorded the pin yet"* stops being a red. ## Scope Engine **11022 passed / 0 failed** · gate **732 green** · lint clean. Test-only. **Does not change** the CLI, the ratchet, or what `--strict` reports. Re-recording the baseline on a drop is still the right thing to do — it just stops being an emergency that reddens main and blocks everyone else while three people race to fix it. The baseline file is restored byte-clean after every simulation above (`git diff` verified). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Expanded baseline validation coverage to detect unreadable or invalid baseline data. - Added support for both exact baseline matches and successfully tightened baselines. - Improved health checks for baseline verification outcomes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
9d103fb171 |
test(engine): record the partial planning-continuation conversion the audit caught (#2857)
## The audit fired, as designed `workflow-planning-continuation-terminal-gap-live-e2e.pg.test.ts` went red on `main`: ``` × AUDIT — one of the two classifier call sites is converted, and the inner predicate is not → expected 2 to be 1 ``` That is the alarm working. #2799 built this case to fail **in both directions** — a new unconverted caller *or* a conversion of an existing one — precisely so a change here cannot land silently. Someone converted the inner half: ```ts if (!isPlanningContinuationTaskDispatchable(task, terminal)) { ``` ## What actually changed, and what did not | site | before | now | |---|---|---| | `drainDuePlanningContinuations` | passes `{ terminalColumns }` | unchanged ✅ | | `resolvePlanningContinuationCandidate` → inner predicate | **not threaded** | **threaded** ✅ | | `selectActionablePlanningContinuations` | passes nothing | **still passes nothing** ❌ | **The behavioural case is untouched and still passes.** A card in a renamed COMPLETE lane still comes back `actionable` and still re-enters plan-review, because `selectActionablePlanningContinuations` is the site that decides that. That combination is the useful shape of a **partial conversion**: the code reads more converted than before, and the operator-visible defect is exactly where it was. Without the behavioural case sitting next to the audit, the arity change would have looked like the fix. The fixer's own comment agrees, and names the narrow case the inner half reaches — a board declaring `done` as a *non-terminal* column id, where the outer check passes and the inner one skipped the continuation as "paused". Real, and not the case the characterization covers. ## The change Arity assertion updated `1` → `2`, with the reasoning recorded at the assertion and in the header. **Updated deliberately, not loosened** — arity is still the property asserted, so the next change to either site lands here again. Nothing else moved. ## Verification - suite — **4/4** - full live-PG E2E surface — **171/171** - `pnpm lint` — clean Test-only; no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
32b6041149 |
fix(linear): every imported issue was created into a column U11 deleted (#2860)
The **same** defect as the GitLab importer fixed in #2843, in a plugin written from the same template — found only because I re-grepped my own area with a wider pattern after declaring it clean. ```ts export function buildLinearTaskCreateInput(issue: LinearIssue): TaskCreateInput { return { title, description, column: "triage", … }; } ``` `triage` was **deleted by U11**; the default board's lanes are `todo | in-progress | in-review | done | archived`. An explicit `column` **overrides** the intake column `createTask` resolves for the workflow it selects — which is precisely how the literal survived the deletion. Nothing rejects the write and nothing logs it: the route answers with a task id, and the card is not on the board. Fix: omit `column`, exactly as #2843 did for GitLab. ## Two tests were pinning the bug ```ts expect(input.column).toBe("triage"); // import-linear.test.ts expect(createTask).toHaveBeenCalledWith(objectContaining({ column: "triage" })); // routes.test.ts ``` That is how this survived a lifecycle sweep that *did* reach the GitLab importer. The census cannot see a lane literal passed as a **call argument**, and the tests asserted the behaviour was intended — so both the automated check and the human check said this file was fine. Both now assert the column is **absent**, which is the property that hands the decision to `createTask` and the one that fails on revert. ## The finding worth carrying forward "We fixed the import path" was true of the forge everybody uses and false of the other one. Two importers, one template, one of them audited. When a defect is found in a file that had a sibling, the sibling is the next place to look — and it is not something the census will tell you, because this whole class is invisible to it. ## Revert proof (measured) Restore `column: "triage"`: ``` AssertionError: expected 'triage' to be undefined AssertionError: expected "vi.fn()" to be called with arguments: [ ObjectNotContaining{…} ] ``` ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion-plugin-examples/linear-import`) — clean - full plugin suite — 35 passed across 5 files - census `--strict` — exit 0 (unchanged: this class is invisible to it) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
4ce3ff75b6 |
fix(ce): the sync queue recorded "done" for a card that finished somewhere else (#2859)
A lane literal the census cannot see — it is a **call argument**, not a
comparison — and the two sibling hooks disagree about how to record the
same kind of fact:
```ts
onTaskMoved: async (task, fromColumn, toColumn, ctx) => {
await store.enqueueSyncAsync({ …, toColumn }); // the real column
},
onTaskCompleted: async (task, ctx) => {
await store.enqueueSyncAsync({ …, toColumn: "done" }); // ← a guess
},
```
On a board whose complete lane is named anything else, the sync-queue
row names a column that board does not have.
## Why fix a field nothing reads
`toColumn` is written to `ce_pipeline_sync_queue` and **never consumed
by any logic** — I checked every reference; it appears only in the
schema, the store's insert, and the row type. It is audit metadata.
That is both why it went unnoticed and why it is worth one token: the
single thing a wrong audit row costs you is the ability to reconstruct
what happened after the fact. A queue that says a card went to `done` on
a board with no `done` is worse than a queue with no column at all,
because it reads as authoritative.
## Structural ratchet, and I would rather say so than imply otherwise
`getCePipelineStore` requires a live PostgreSQL `AsyncDataLayer` and
throws without one, so driving the hook means standing up PG to
re-assert a one-token substitution — against the standing rule on slow
tests. The repo already takes this trade in the same shape
(`packages/core/src/__tests__/analytics-timing-roles-resolved.test.ts`,
whose analytics aggregators have the identical problem).
The test comment states plainly what it does and does not prove: it pins
that the hook reads the card's own column and holds no completion
literal; it does not exercise the write.
Comments are stripped before the negative assertion — the test's own
explanation names the old literal, and a ratchet that matches its own
prose passes forever without checking anything. (That mistake is already
in this program's history, which is why it is guarded here.)
## Revert proof (measured)
Restore `toColumn: "done"` — **both** assertions fail (the positive one
on the missing `task.column`, the negative one on the literal).
## Verification
- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm lint` — clean
- `tsc --noEmit` (`@fusion-plugin-examples/compound-engineering`) —
clean
- full plugin suite — 319 passed across 32 files
🤖 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**
* Sync audit records now capture the task’s actual completion lane
instead of assuming it is named “done.”
* Improved audit accuracy for boards with custom completion lane names.
* **Tests**
* Added coverage to verify completion records use the task’s real
destination lane.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
68fd781eb8 |
test(engine): stop engine-default exiting 1 with zero failing tests (#2855)
## The problem
On `main`, the whole `engine-default` project exits **1 while reporting
736 files passed and 0 failed**:
```
Test Files 736 passed | 1 skipped (737)
Tests 9944 passed | 14 skipped | 1 todo (9959)
Errors 9 errors
```
Nine unhandled rejections — `TypeError: this.store.listTasks is not a
function`, all from `self-healing-db-corruption.test.ts`, raised *after*
its tests had passed.
That is worse than a plain failure: the lane is red, **nothing names a
test**, and a genuine regression landing later arrives in an already-red
lane where it reads as more of the same noise.
## Why the existing isolation didn't hold
The suite stubs every other maintenance sweep so only the corruption
path runs. But `runMaintenance` invokes the surfacing family like this:
```ts
{ name: "surface-in-review-stalled", fn: () => this.surfaceInReviewStalled(maintenanceSurfacing()) },
```
`maintenanceSurfacing()` lazily calls `openSurfacingCycle()`, which does
`await this.store.listTasks({ slim: false })`. **Spying on
`surfaceInReviewStalled` replaces the method, but the call site still
evaluates its argument** — so the shared cycle opened regardless,
against a mock store that deliberately implements only what corruption
surfacing needs.
## Two fixes that look right and are not
Both tried, both reverted — recorded because each is the obvious next
move:
| attempt | result |
|---|---|
| add `listTasks` to the mock | **3 tests fail** — the sweeps then run
far enough to record audit events the assertions don't expect |
| stub the 27 maintenance methods missing from the suite's lists | **5
tests fail, and all 9 errors remain** — `openSurfacingCycle` isn't among
`runMaintenance`'s own calls, so the drift was a real but unrelated gap
|
The second result is what located the fault: stubbing everything
`runMaintenance` calls does not silence the rejections, so they don't
originate there. The stack confirmed it —
`SelfHealingManager.openSurfacingCycle` at `self-healing.ts:8015`. I
should have read that stack before guessing twice.
## The fix
Stub the cycle itself. `null` is exactly what `openSurfacingCycle`
returns when the engine is paused, so every consumer already handles it,
and the corruption-surfacing path this suite exists to test is
untouched.
## Verification
- `self-healing-db-corruption.test.ts` — **6/6, 0 errors, exit 0**
- full `engine-default` project — **736 files passed, 9944 tests, 0
errors, exit 0** (was exit 1)
- `pnpm lint` — clean
Test-only change; no production file is touched, so no changeset.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
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>
|
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
5a3541315c |
fix(gate): pnpm test is red on main — stale exemptions, plus two false positives they were masking (#2851)
**`pnpm test` and `pnpm test:full` fail on `main` right now** (commit `51934931e1`). Found by running the gates against `main` after my earlier PRs landed, not by CI telling me. > **Correction to this PR's first version.** I originally wrote that `pnpm test:gate` was failing. It is not: `check-inert-flag-seams` runs in the `pretest` and `pretest:full` hooks, not in `test:gate`. So the blocking merge gate (Lint / Typecheck / Build / Gate) is unaffected and PRs are not blocked — what is broken is every local `pnpm test` run, which fails before a single test executes. Lower urgency than I claimed, still worth fixing promptly, and I would rather correct the scope than leave an overstated one standing. Three causes, each surfaced by fixing the one before it. ## 1. Stale exemptions — the mechanism working #2819 and #2823 merged, so the two `ALLOWED_OMISSIONS` entries covering those call sites became stale and the staleness check failed them. Removed. This is my cleanup: the entries were designed so they could not outlive their fixes, and they didn t. ## 2. Renamed imports were not resolved Removing the first entry surfaced: ``` enqueueMergeQueue() — best call passes 2 of 5 ``` Its only production caller passes all five — through `import { enqueueMergeQueue as enqueueMergeQueueAsync }`. Call sites were recorded under the **local** name, so an aliased supplier was invisible and the seam read as unsupplied. The local name is now mapped back to the exported one. ## 3. Method calls were conflated with module functions That fix then surfaced two engine sites as omitting — but `store.enqueueMergeQueue(taskId, opts)` is a **2-arg `TaskStore` method** that resolves the review columns internally (#2819), not the 5-arg module function sharing its name. Property-access calls are no longer attributed to module-level seams. **Tradeoff, stated at the site:** a genuine `namespace.fn(...)` call is now skipped. This codebase calls module functions as bare identifiers, and aliases are resolved by fix 2, so that shape does not currently occur. Recorded rather than left for someone to discover. ## Both directions re-verified A fix that quietly disarms the gate would be worse than the red, so I re-ran the defects it exists to catch: | mutation | result | |---|---| | drop `Column.tsx`'s flags argument (partial supply) | `supplied by 10/11 call sites; omitted at .../Column.tsx:1 (of 2)` | | drop the aliased 5-arg supplier (wholly unsupplied) | `best call passes 3 of 5` | | restored | exit 0 | My first attempt at the second row grepped for the wrong message shape and printed nothing. **I re-ran it rather than reading silence as success** — which is the failure this gate exists to prevent, and one I have made in this same file before. ## Verification `pnpm test:gate` green (it was never affected) · `node scripts/check-inert-flag-seams.mjs` exit 0 · lint 0 · gate reports `21 lane/flag seams, all supplied at every production call site`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
89aaf341d0 |
the unwired-seam audit: 9 defects the census cannot see, incl. a reviewed card that cannot merge (#2820)
**Nine operator-visible defects in a class the census cannot see, plus
the audit method that found them.**
The census scans for lifecycle-column **comparisons**. This PR is about
guards that have no literal to find: a helper takes an optional
*resolved* lane set, its own test passes it, the census entry is gone —
and the callers pass nothing. **A resolved seam nobody wired is
indistinguishable from no seam at all.**
## What was broken
| defect | operator sees |
| --- | --- |
| `getTaskMergeBlocker` unwired in `mergeTaskImpl` | `Cannot merge FN-1:
task is in 'checking', must be in 'in-review'` — **a reviewed card
cannot merge** |
| …and in the completion move | `Cannot move FN-1 to done: …` — **and
cannot complete** |
| `isParkedTaskColumn` unwired ×2 (`agent-heartbeat`) | a durable agent
keeps claiming a parked card; **Health Check renders it RUNNING** |
| `resolveLinkSyncColumnRoles` first-per-role | link hygiene skips a
**second hold lane** entirely |
| `executor` active-task predicate first-per-role | a card in a **second
wip lane reads as INACTIVE**; its prompt file becomes reclaimable |
| `isPlanningContinuationTaskDispatchable` partially threaded | a board
declaring `done` as *non-terminal* stalls its cards — **stalled by a
lane name** |
| `default-workflow-hooks:72`, `executor:2404` | resolved gate admits
the move, unresolved blocker refuses it |
## The recurring shape, which is sharper than "a caller forgot an
argument"
Four sites resolve the lane and then re-ask with the literal, **a few
lines apart in the same function**:
- `task-artifacts-ops` resolves `completeColumn`, then asks the blocker
with the literal.
- `default-workflow-hooks:72` gates on `lifecycleColumns?.review`, then
the literal.
- `executor:2404` compares `resolveResumeLanes(…).review`, then the
literal.
- `resolvePlanningContinuationCandidate` applies the caller's terminal
set, then delegates without it.
**Grep for the helper, not the literal.** The literal is one function
away, correctly annotated as a fallback — which is exactly why the
census is blind to all of it.
## The arity trap, named and measured (six occurrences, one caught by
review here)
`resolveLifecycleColumns` answers *"which column is **the** hold
lane?"*. A `.includes()`/`.has()` test asks *"is this **any** hold
lane?"*. Nothing distinguishes them — same types, no literal.
**A default-vs-renamed differential cannot catch it**, because the
default board declares one column per role and therefore cannot express
the failing shape. It needs a *structurally* different fixture. That is
a sharper rule than "test both vocabularies", and it would have caught
all six.
Scanned: 12 candidate sites. **4 fixed · 3 blocked (2 on the inert sync
IR reader; `triage:833` also query-shaped) · 1 needs a hook-contract
change · 3 not defects (a returned tuple; an ordering-sensitive
precedence list) · 1 false positive of my own scan.**
A sweep over all twelve would have broken the ordering-sensitive pair,
delivered nothing at the sync-blocked ones, and "fixed" a site that was
already correct.
## Two traps in fixing this class — I hit both here
1. **The legacy id is a FALLBACK, not a member.** Pre-seeding
`"in-review"` admits a board that *declares* `in-review` as its WIP
column — a card mid-implementation merges prematurely. A real resolved
answer must **replace** the default. (Caught by review; it is the same
unscoped-legacy-acceptance the glasses plugin's review caught earlier,
which I had read and reintroduced.)
2. **Two guards, one assertion.** `toContain("must be in")` passed with
`mergeTaskImpl` reverted, because the *completion* guard caught the card
instead. The assertion now names the site (`Cannot merge` vs `Cannot
move … to done`) so the two fail independently.
## Corrections I made to my own work, recorded rather than quietly fixed
- My first PG test was **vacuous three ways**:
`saveWorkflowDefinition?.()`/`setTaskWorkflowSelection?.()` do not exist
(the `?.` swallowed both, so the task kept the builtin workflow),
`updateTask({column})` does not move a card, and a two-node IR made
every setup move illegal. Premise is now **asserted**, not assumed.
- My doc claimed the audit was complete. It enumerated **helpers**, not
every **caller** — `getTaskMergeBlocker` alone has 13 call sites.
Corrected in place, with the still-unwired ones listed by file and line
and a note to distrust any "audit complete" claim including mine.
- A severity correction to another worker's E2E:
`selectActionablePlanningContinuations` has **no production caller**, so
its stated consequence is latent, not live.
## Verification
- `pnpm test:gate` — 161 + 487 + 13 + 71
- `tsc` on core and engine; `pnpm lint`; `check:changesets`; census
`--strict` — all clean, each run explicitly
- Every fix revert-measured; each has a non-vacuous companion. The
two-hold-lane and repurposed-`in-review` cases exist because the default
board cannot express those shapes.
## Deliberately not done, with reasons in
`resolved-seams-nobody-wired.md`
`isTaskReadyForMerge` (dead in production — wiring it would be the
anti-pattern itself); `getTaskHardMergeBlocker` (3 of 4 callers are
query-gated sweeps); `getInReviewStallReason` (needs a **batch
prefetch**, not a per-task resolve — its callers decorate every task on
every list read; the in-review stall badge is wrong on renamed boards
until then); `default-workflow-hooks` planning/live-work sets (needs
`DefaultWorkflowMoveContext` to carry the IR — a shared contract
change).
|
||
|
gsxdsm
|
aca25494a5 |
test(dashboard): App.test.tsx is fully green — an incomplete mock was crashing NewTaskModal (#2846)
Closes #2829. `App.test.tsx` has been red since **2026-07-25**; #2833 took it 10 failures → 2, and this takes it to **0**. ## The cause, and it was not what I guessed `vi.mock("../../hooks/useViewportMode")` did not export `isShortViewport`, which `NewTaskModal` imports. An incomplete module mock **does not fail at the mock**. It throws inside whichever component imports the missing export, and the nearest `ErrorBoundary` swallows that into *"This section encountered an error"* — so the test failed on a **missing heading** with a DOM that looks perfectly healthy. Structurally the same presentation as the workflow-skeleton bug in #2833: the real failure sits two layers below what the assertion reports. Both remaining tests had this one cause. ## Found by probing, not theorising I had already been wrong twice on this file (`renders nothing`, then i18n), and my standing note said the modal was "plausibly FN-8620's FloatingWindow rework, **unverified**". That would have been a third wrong guess. Instead I dumped what was actually in the DOM at the assertion point: ``` headings: ["Fusion"] | dialogs: 0 newtask-ish: [..., "error-boundary error-boundary--modal"] ``` The `error-boundary--modal` class was the entire answer; reading its text gave the exact missing export. Same technique that cracked #2833 in one shot — the difference between the two halves of this investigation is that I stopped guessing. **App.test.tsx: 141 passed (141).** ## Also patched, and one deliberately not - **`Header.mobile-project-favorites.test.tsx`** mocks the same module with an inline factory and lacks the export. It is **latent, not broken** — its component does not import `isShortViewport` yet, and the day it does, the failure would be this same swallowed crash. One line. - **`FloatingWindow.touch-geometry.test.tsx`** greps as missing but is **not** — it uses `{ ...(await vi.importActual(...)) }`, so its surface is complete by construction. My grep-based classification was a false positive. That spread is the pattern that prevents this class outright, and it is the better fix **where it is available**. It is not available here: App.test.tsx's mock exists precisely so tests need no `window.matchMedia` in jsdom, and spreading the real module would reintroduce that dependency for every unstubbed export. So the scoped one-line stub is the right fix for this file, and the spread is the recommendation for new mocks. A guard asserting "a module mock exports everything the real module does" would close this class repo-wide. I did not build it — 34 files mock this module and only one was genuinely wrong, so the population does not yet justify another instrument, by the same measure-first rule that killed two other candidate gates this week. ## Verification App.test.tsx 141/141 · the two touched siblings 9/9 · `tsc` 0 · lint 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
7927c7b58a |
docs(testing): probe the DOM before theorising about a missing element (#2850)
Earned the hard way this week: three separate causes in `App.test.tsx`
and `board-mobile-view-switch.test.tsx` all presented **identically** as
a missing element, each with a DOM that looked healthy.
| what the test said | what was actually wrong |
|---|---|
| `Unable to find "+ New Task"` | `ListView` rendered its workflow
**skeleton**, which carries the same `list-view` class as the real body
— so the preceding `waitFor(".list-view")` passed |
| `Unable to find role="heading" "New Task"` | `NewTaskModal` **threw**
— an incomplete `vi.mock` was missing `isShortViewport` — and an
`ErrorBoundary` swallowed it |
| `Unable to find [data-testid="switch-to-board"]` | an uncaught render
error **unmounted the entire React root**; the DOM was already empty
three lines earlier |
The part worth recording is the hit rate. **Three theories were offered
before any probe — "the board renders nothing", "i18n is returning
keys", "the FloatingWindow rework" — and all three were wrong.** Three
probes each landed the cause on the first try. Those wrong theories cost
days; the probe is four lines.
The doc records the snippet, what each signal means (`error-boundary` in
the DOM = a swallowed throw, and its text names a missing mock export
exactly; empty DOM with no boundary = unmounted root, so trust the
*first* failing assertion not the reported one; container present but
contents absent = a skeleton standing in), and the corollary for writing
assertions:
> Wait on a marker only the real thing has.
A class shared with a loading or empty state turns *"the list rendered"*
into *"something rendered"*, and the test then fails one line later
against a DOM that looks fine. That is why `list-view-body` exists
(#2834).
Placed under the dashboard testing sections in `docs/testing.md`. Docs
only — no changeset.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
21e688e1f9 |
fix(glasses): ?columns= silently returned the WHOLE board on a renamed board (#2849)
A lane-literal defect the census cannot see — the literals are
**Set/Array members**, not comparisons — in a live plugin, with **no
test coverage on the filter at all**. That absence is how the inversion
survived.
## The bug
```ts
const ALLOWED_COLUMNS = ["triage", "todo", "in-progress", "in-review", "done", "archived"];
function parseColumns(raw) {
const parsed = raw.split(",").filter(v => ALLOWED_COLUMNS.includes(v));
return parsed.length ? new Set(parsed) : null; // ← `null` ALSO means "no filter requested"
}
```
On a board whose lanes are named anything else, every requested id is
discarded, `parsed.length` is `0`, and the function returns `null` —
**the same value it returns when no filter was requested**. The caller
then does `columns ? all.filter(...) : all`, so the route answers `200`
with the **entire board**.
Asking for one column returns all of them. Nothing in the response says
the filter was dropped. The list also still named `triage`, a column U11
deleted, so it described a board that no longer exists in either
direction.
## The fix
No allow-list can be correct here and none is needed. Valid ids are
whatever the project's workflows declare, and `Task["column"]` is
already `ColumnId = Column | (string & {})` — open by construction.
Filtering directly on the requested ids needs **no resolution source at
all**, which is why this literal, unlike the display ordering in
`cards.ts` (documented DELIBERATE-LITERAL: this package depends on
`@fusion/plugin-sdk` only, so there is no IR or store to resolve from),
is a defect rather than a deferral.
**Deliberate behaviour change:** `?columns=nonsense` now returns an
**empty deck** instead of the whole board. "Show me column X" answered
with every column is not a lenient default — it is the bug wearing a
200.
## Revert proof (measured)
Restore the allow-list and **both** new cases fail:
```
FAIL > filters on a RENAMED lane instead of silently returning the whole board
expected [ { id: 'summary', …(5) }, …(2) ] to have a length of 2 but got 3
FAIL > answers an unknown column with an EMPTY deck, not with everything
expected [ { id: 'summary', …(5) }, …(2) ] to have a length of 1 but got 3
```
Both directions are asserted on purpose: a filter that matched *nothing*
would satisfy the renamed-lane case alone while being equally broken.
## Not changed, and why
`plugins/fusion-plugin-even-cards` carries the **identical** bug — I
wrote the fix there first. It was removed from the pnpm workspace
(`858bab2`, "remove `fusion-plugin-even-cards` from the active workspace
package list to avoid duplicate user-facing integrations") and its
README names this plugin as its replacement, so it is not built, tested,
or shipped by anything. Fixing it would only imply it still runs.
Reverted and left alone.
## Verification
- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm lint` — clean
- `tsc --noEmit` (`@fusion-plugin-examples/even-realities-glasses`) —
clean
- full plugin suite — 188 passed across 19 files
- census `--strict` — exit 0 (unchanged: these literals are Set members,
invisible to it)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
d252c4e0cf |
fix(glasses): finished cards were crowding live work off the deck on a renamed board (#2854)
Not a labelling bug. `boardToDeck` filters finished cards out of a deck that is **capped at `maxCards`**, so on a board whose complete lane is named anything but `done`/`archived`, every finished card **consumes a slot and displaces live work**. The wearer sees fewer live tasks the more the team finishes — which reads as "nothing is happening", not as a defect. ```ts const active = tasks.filter((task) => task.column !== "archived" && task.column !== "done") .sort(...) .slice(0, Math.max(0, maxCards - 1)); // ← the cap is what makes this bite ``` ## Correcting an earlier DELIBERATE-LITERAL call This site was marked **DELIBERATE-LITERAL — no resolution source in this package**. That reasoning was inherited from the **deprecated** `fusion-plugin-even-cards`, which depends on `@fusion/plugin-sdk` alone. **This** package lists `@fusion/core` as a runtime dependency and already calls `resolveWorkflowIrById` / `resolveLifecycleColumns` in `quick-capture.ts` and `agent-actions.ts`. The half that *was* right: `cards.ts` genuinely cannot resolve anything — it takes plain `Task` rows. So the lane answer becomes a parameter and the **route** supplies it: one `listWorkflowDefinitions()` read per request regardless of board size, matching how `?columns=` already treats the board as a single pool. Best-effort, so a failed resolve leaves the deck on its documented default rather than failing the request — a slightly-wrong deck beats no deck on a pair of glasses. `terminalColumns` is in the `unwired-lane-parameter` vocabulary, so an unwired version of this parameter fails the build instead of sitting here looking converted. (That guard only learned to see this class in #2852.) ## Revert proof (measured) ``` FAIL > does not let a card in a RENAMED complete lane displace live work expected [ 'summary', 'FN-SHIPPED' ] to deeply equal [ 'summary', 'FN-LIVE' ] ``` `maxCards: 2` in the fixture is load-bearing — one summary card plus exactly one task slot, so an unfiltered finished card **displaces** the live one rather than merely joining it. A larger cap would let both through and the case would pass either way. The second case pins the degraded default (legacy `done`/`archived` still filtered when the caller resolved nothing), since most boards never rename anything. ## Not changed `cards.ts:187` — `task.column === "in-review" ? "In review" : "Moved in"` in the notification title. Genuinely cosmetic: a review card on a renamed board reads "Moved in" instead of "In review". No slot is lost and no decision is made from it, and `notificationCard` has no options object to thread a lane answer through, so converting it means widening a signature for a label. Left with the finding stated rather than silently swept in. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion-plugin-examples/even-realities-glasses`) — clean - full plugin suite — 188 passed across 19 files - unwired-lane guard — 6/6, no new entries - census `--strict` — exit 0 Touches `board-routes.ts`, which #2849 also edits (different hunk — `parseColumns` vs the handlers), so the two merge cleanly in either order. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
c0e647f320 |
test(dashboard): name the skeleton in board-mobile-view-switch instead of the shared class (#2848)
Closes the last item I left open on #2834 — the two cases that were passing against a skeleton. ## The weakness They asserted `.list-view` to mean *"we are in list view"*. The workflow **skeleton** carries that same class, so they matched it and passed **without the list ever drawing**. I found this while adding the `list-view-body` marker in #2834, tried to fix it, hit a dead end, and reverted rather than leave two green tests red. ## Fixed by making the assertion honest, not by forcing the real body This harness renders `<ListView>` with a deliberately small `../../api` mock and no workflows, so the skeleton **is** what appears — and that is fine, because the subject of these cases is the **board's** structure after switching, not the list's contents. Naming the skeleton says what the test actually observes. ## Why not the real body — measured, and it explains the earlier dead end Supplying the shared `DEFAULT_BOARD_WORKFLOWS` fixture makes ListView render its full body, which **throws** under this file's api mock. The harness has no `ErrorBoundary`, so React unmounts the entire root: the DOM goes completely empty and every later query fails with a misleading `Unable to find switch-to-board`. That is precisely the failure I could not explain on #2834. A probe placed after the await settled it: ``` PROBE after-await testids: [] | boundary: none ``` Empty DOM, no boundary — an uncaught render error taking the root down, presenting as a missing button three lines later. Upgrading this to the real body means expanding an api mock inside a file about board CSS structure, which is a bigger change than the assertion is worth. The reasoning is recorded at the site so the next person does not rediscover it the way I did. ## The assertion is load-bearing, not just renamed Mutating the skeleton's own `data-testid` in `ListView.tsx`: ``` TestingLibraryElementError: Unable to find an element by: [data-testid="list-workflows-skeleton"] (x2) ``` Restoring it: `Tests 3 passed`. ## Verification 3/3 · `tsc` 0 · lint 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
bebbdf9083 |
fix(tests): main red — archive restore returns to the ARCHIVED lane now (#2832) (#2847)
## Red on main ``` store-archive-reads > TaskStore archived read parity (PostgreSQL) > rebuilds a missing live row before consuming its archive snapshot AssertionError: expected 'done' to be 'todo' ``` ## The product change is a real fix **#2832** found that `preArchiveColumn` has no database column — it exists on the `Task` type and in the archive snapshot and nowhere else — so the old code fell through to a literal and decided the destination the same way for **every unarchive that has ever run**. On a custom board that meant a card archived mid-implementation came back marked *finished*. That PR flipped its own characterization cases. This one, in a different file, was missed. The fixture creates the card in `done`, so `done` is the answer now. ## I measured a second sample before encoding a rule The behaviour is **narrower than "restores to the lane it came from"**. With the fixture changed to `in-progress`, restore returns **`todo`** — not `in-progress`: | archived from | restored to | |---|---| | `done` | `done` | | `in-progress` | **`todo`** | A terminal lane is preserved; a WIP lane is re-queued. That is plausible product behaviour — a card cannot resume mid-execution after a restore — but it is **not what #2832's summary describes**, so I have flagged it there rather than encoding it here. If re-queueing WIP is deliberate it deserves its own case; if it is not, this snapshot-rebuild path still carries the defect #2832 fixed elsewhere. ## An honest limitation, recorded in the test This assertion is **weaker than it looks and cannot be strengthened here**: `done` is also the complete lane, which is exactly what the pre-#2832 *"no usable history"* branch returned. A card archived from `done` therefore reads identically under both the fixed and the broken implementation. My first attempt "strengthened" it by moving the fixture to `in-progress` — that is what surfaced the second behaviour above, and shipping it would have encoded a rule inferred from two samples. Reverted; the limitation is documented instead. ## Scope Only the line-205 case is touched. Line 218 asserts `todo` for a card genuinely archived from `todo` and still passes — the two are not the same claim. Core **4813 passed / 0 failed** · gate **732 green** · lint clean. Test-only. ## Pre-flight results for the current queue Merged-with-main, engine + core on each: | PR | result | |---|---| | #2822, #2819, #2823, #2818 | only the 2 inherited `workflow-ir-resolver` failures (fixed in #2836, now merged) | | #2805 | clean | | #2828 | inherited only, once this and #2836 land | | #2830 | inherited only | | #2820, #2803, #2808 | **conflict** with main — census baseline; told the owners to regenerate rather than hand-merge | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
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>
|
||
|
gsxdsm
|
f53c9dbd39 |
fix(core): merge re-enqueue threw on every board with a renamed review column (#2819)
The single most consequential finding from the u12 seam-gate work, picked up because batch-core (#2783) merged without it and it is now unowned. ## The defect `enqueueMergeQueueInTransaction` gates on the task's column being a review column, and takes the board's review columns as an optional trailing argument. - `moves.ts:487` and `moves.ts:1153` — the automatic handoff-to-review path — resolve and pass them. - The public `enqueueMergeQueue` wrapper (`async-merge-coordination.ts:246`), reached through `store.enqueueMergeQueue`, **did not**, so it fell back to `new Set(["in-review"])`. This is not the quiet legacy-id degradation most of these seams produce. The reject branch records `mergeQueue:enqueue-rejected` and **throws** `MergeQueueInvalidColumnError`. Its production callers are `merger.ts:7251` and `self-healing.ts:10329` — so on any board whose review lane is renamed, the merge and recovery re-enqueue paths failed outright while the handoff path kept working. ## Measured, not asserted With the fix reverted, the renamed case fails with the exact predicted error and the controls stay green: ``` × renamed vocabulary: a task in the RENAMED review lane enqueues for merge MergeQueueInvalidColumnError: Task KB-001 is in column 'checking', not 'in-review'; cannot enqueue ✓ default vocabulary: a task in the review lane enqueues for merge ✓ renamed vocabulary: a task in the WIP lane is still REJECTED ✓ default vocabulary: a task in the WIP lane is still REJECTED Tests 1 failed | 3 passed (4) ``` With it: `Tests 4 passed (4)`. ## About the suite **Differential.** The fixture is the builtin coding workflow with only its column ids renamed, so the sole difference between the two runs is vocabulary — a hand-built graph would test the fixture's own transition table as much as the code. It asserts the rename actually landed (`checking` present, `in-review` absent), so a surviving literal cannot pass by luck, and it walks the graph rather than jumping, because moves are transition-validated. **Both negatives included.** A WIP-lane task must still be REJECTED under each vocabulary. Supplying the real columns must not degrade into "every column is a review column", which would let work merge straight out of the WIP lane — the failure mode a careless version of this fix would introduce. ## Why nothing caught it Partial supply. Two of three call sites passed the argument, so a check asking "does SOME caller supply this?" reported the seam as satisfied, and the lifecycle-column census counted the conversion as done. Closing that one-supplier floor in `scripts/check-inert-flag-seams.mjs` (on #2772) is what surfaced it. ## Verification - `pnpm test:gate` green - new suite 4/4; neighbouring merge-queue suites (`taskstore-lifecycle`, `store-in-review-stall`, `runtime-lifecycle-async`) 29/29 - `tsc -p packages/core` 0, lint 0 - changeset included (`@runfusion/fusion` patch) Note: #2772 still carries a TEMPORARY per-call-site exemption for this seam. Once this lands, that exemption's staleness check will fail and I will remove it there — it cannot outlive the fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
be12ca905b |
docs(workflow-learnings): the fifth shape — converted consumer, literal-passing producer (#2835)
Found by the operator reviewing **my own** shipped fix (#2823). It belongs in this document because it is the only shape so far that every instrument in the program reported as done. ## The defect `clearNearDuplicateReferencesTo` was converted to resolve the canonical's column flags, and my test proved that by supplying `column: "shipped"`. Both production call sites in `moves.ts` gated on the **resolved** complete lane and then passed the literal `column: "done"`. The consumer was correct; the producer was never converted; and the hand-supplied fixture value is exactly what hid it. Why nothing caught it: - the **census** counts comparisons — a value passed as an argument is not a comparison; - the **seam check** asks whether callers *supply* the argument — these did, with a literal; - the **test** supplied the interesting value itself, so it exercised the consumer and never the producer. And the part I would have got wrong: driving the flow end to end still does **not** distinguish them. The consumer looks the passed column up in the canonical's IR, finds nothing for `done` on a renamed board, and falls through to the legacy predicate where `done` *is* terminal — right answer, wrong reason. It only bites on a board that declares a `done` column **without** the complete trait. ## The rule > A differential test must vary the value the **production** code computes, not one the test hands in. If the fixture passes the lane name, it has tested the consumer. Who computes that argument in production, and do they compute it or spell it, is a separate question — and the one that was wrong here. ## Measured, so nobody builds the wrong instrument An AST probe for call arguments shaped `{ column: "<legacy id>" }` finds **79 sites** across `packages/`. It correctly flags the two real `moves.ts` offenders — but most of the rest are legitimate: `set({ column: "archived" })` writing the archive state, `listTasks({ column: "todo" })` filtering a query. So a blocking gate on this shape needs a curated list of consumers that interpret a column as a **role**, as opposed to storing or filtering it — a per-consumer judgment call, not a mechanical check. **Recorded rather than built**, and deliberately not attempted on top of five unmerged PRs in packages I do not own. I also verified one nearby call site that a naive version of this probe flags and which is **not** a defect: `archive-lifecycle-2.ts:353` passes `column: "archived"`, but that path sets `task.column = "archived"` unconditionally, so it is passing the column the card actually reached. Exactly what the operator's fix is about. Docs only — no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
4c5080c1cf |
fix(tests): main red — the IR fallback is a BRANDED COPY, so identity can't hold (#2836)
## Red on main
```
workflow-ir-resolver > resolveWorkflowIrForTask > falls back to the built-in default when the definition is missing
workflow-ir-resolver > resolveWorkflowIrById > falls back to the canonical IR for an unknown built-in id
AssertionError: expected { version: 'v2', …(6), …(1) } to be { version: 'v2', …(6) }
Received: serializes to the same string
Compared values have no visual difference.
```
That message is the signature of an **identity-only** break, and that is
exactly what it is.
## Why identity can never hold again
Both asserted `toBe` against the exported builtin constant. **#2815**
added `markFellBack`:
```ts
function markFellBack(ir: WorkflowIr): WorkflowIr {
const copy = { ...ir } as WorkflowIr;
Object.defineProperty(copy, FELL_BACK_TO_DEFAULT, { value: true, enumerable: false, configurable: true });
return copy;
}
```
It brands the fallback so a caller can tell a **resolved** workflow from
a **guessed** one — the provenance contract #2618 introduced. Copying
*is* the mechanism, so these two paths cannot return the shared object.
Worth noting: the sibling `toBe` assertions in the same file **still
pass**. The no-selection and throwing-selection paths return the
constant unbranded, so only the two `markFellBack` paths changed — which
is why this presents as two failures rather than five, and why it is a
genuine contract change rather than a blanket refactor.
## Not just loosening to `toEqual`
Swapping `toBe` → `toEqual` alone would delete a real assertion. The
**brand is asserted too**, via the public provenance API rather than by
reaching for the private symbol:
```ts
expect(ir).toEqual(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
const provenance = await resolveWorkflowIrForTaskWithProvenance(store, "t1");
expect(provenance.source).toBe("default");
```
This is **stronger** than the identity check it replaces:
| mutation | result |
|---|---|
| fallback no longer branded | **fails** — `expected 'selection' to be
'default'` |
| fallback returns a different IR entirely | **fails** structurally |
The first is the case the old `toBe` could not articulate: an unbranded
fallback still equals the constant structurally, so a caller asking
*"was this actually resolved?"* would get **yes for a guess** —
precisely the lie #2815 exists to prevent.
Core **4792 passed / 0 failed** (was 2 failed) · gate **732 green** ·
lint clean. Test-only; the resolver is restored clean after the
mutations.
## How it was found
Pre-flighting **#2822, #2819, #2823 and #2818** merged-with-main. All
four reported the *same two* failures — the signature of an inherited
red rather than four independent regressions. Confirmed directly on
`origin/main`. Each of those four is otherwise green (engine 11003
passed on all of them); I have noted that on the PRs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
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>
|
||
|
gsxdsm
|
fe8f3af751 |
fix(gate): barrel imports were silently dropped from the inert-seam check, hiding 5 engine omissions (#2822)
**Self-reported regression from #2772, which has already merged.** The seam gate on main currently prints a clean bill of health while five real omissions sit in front of it. ## What broke Closing the imported-shadow hole in #2772 taught the check to record which module each callee was imported from, and to exclude a call site whose module basename does not match the seam's declaring module. That works for relative imports. It does not work for barrel imports. Engine and CLI reach core through `import { ... } from "@fusion/core"`. That specifier's basename is `core`, which never matches a module name like `near-duplicate-canonical` — so **every barrel-imported call site was classified as "a different function of the same name" and dropped.** The check stopped seeing engine's and cli's calls into core entirely, which is most of the cross-package surface it exists to watch. ## Measured On `main` today: ``` [check-inert-flag-seams] 21 lane/flag seams, all supplied at every production call site. ``` With this fix: ``` packages/core/src/near-duplicate-canonical.ts: isNearDuplicateCanonicalInactive() — supplied by 6/11 call sites; omitted at packages/engine/src/self-healing.ts (x2), packages/engine/src/triage.ts (x3) ``` Those five were always there. Earlier I reported this seam as "supplied by 5/6" — that number was wrong for this reason, and the engine sites were invisible to me when I said it. ## The rule now Only a **relative** specifier identifies a module well enough to exclude a call site on. Anything else is unresolved, and unresolved must mean **counted**: an over-counted seam produces a false report somebody investigates, an under-counted one produces silence. I had this backwards, and it is the second time in this lane a change made the gate read cleaner while catching less. ## Both directions verified - **Barrel imports counted** — the five engine sites appear. - **Relative shadows still excluded** — lifting the `sortTasksForDisplayColumn` entry still reports it unsupplied, so `Lane`/`Board`/`ListView` calling the dashboard twin through `"./taskSorting"` does not clear core's seam. That was the entire point of the original fix and it still holds. ## The five engine omissions Not fixed here — engine-owned, reported on #2785. Same shape as the merge-queue bug in #2819: a canonical resting in a **renamed active column** reads as *inactive*, so duplicate markers get cleared against live work. They carry TEMPORARY per-file exemptions so this PR is green and self-announcing. Noted at the entry: the key is `<file>::<function>`, so a file with two omitting calls is exempted for both — coarser than I want, recorded rather than left to be discovered. ## Verification `pnpm test:gate` green, lint 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
7bf3df9477 |
chore(core): delete the dead merge-blocker guard, and catch exemptions for deleted seams (#2830)
Closes the last of the five findings I reported to batch-core (#2783), which merged without them. This one I held back twice on purpose; the reason is now resolved. ## The dead code `evaluateMergeBlockerGuard` had **exactly one reference in the repo: its own declaration.** Never called, never re-exported from `index.ts`/`index.gate.ts`, never registered as a trait hook. Its `lifecycleColumns` conversion was applied to dead code, and the census counted it as progress. ## Why I would not delete it earlier, and what changed I twice declined this, because no production `"guard"` trait hook is registered anywhere in the codebase — the only `registerTraitHookImpl(..., "guard", ...)` is in a test. If a registration had been dropped, that would be a real product bug and this function would be its evidence, so deleting it would have destroyed the breadcrumb. **It is not missing.** Merge blocking is enforced inline in `task-store/moves.ts` (~645 and ~821) via `getTaskMergeBlocker`, gated on the **resolved trait flags** (`toFacts.flags.complete` + `fromFacts.flags.mergeBlocker`) rather than on column ids — a better implementation than the one being deleted. The logic moved; this function, and a file-header line crediting a never-existent `evaluateDefaultWorkflowGuards` reader, were left behind. The header now records where the guard actually lives, so the next person does not repeat the investigation I just did. Also removed: `GuardVerdict` (its return type, used nowhere else) and the orphaned `getTaskMergeBlocker` import — the latter caught by lint, not by me. ## The gap this exposed The allow-list staleness check only fired for a seam the scan still **finds** — it asks *"is this site supplied now?"*. Delete the declaration and the name is never iterated, so its entry sits in the list forever, exempting nothing and misreporting what is tolerated. I found this by deleting the function and watching the gate stay **silent** about its leftover entry. **Measured:** an `ALLOWED` entry naming a non-existent seam now fails with `no such seam declared any more; remove its ALLOWED entry`; removing the probe returns exit 0. The failure header is reworded, since "the sites are supplied now" no longer covers both reasons. This is the fourth blind spot closed in this check, and like the others it was found by exercising the gate rather than reading it. ## Verification `pnpm test:gate` green · `tsc -p packages/core` 0 · lint 0 · gate now reports **20** seams (was 21 — the drop is this deletion). No changeset: deleting unreachable internal code with no exported surface is behaviour-preserving. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
c601a22c0c |
chore(gate): the sortTasksForDisplayColumn exemption is permanent, not pending — with evidence (#2831)
The last of my five batch-core findings, resolved by **not** changing code. Recording why, because "leave it" is the kind of decision that rots into "nobody looked". ## What was wrong with the entry It read `TEMPORARY: core-owned; reported on #2783`. That PR merged without picking it up, so the entry implied an owner who was going to act, and none exists. A stale exemption misreports what the project tolerates — exactly the rot the staleness checks exist to prevent, and one they cannot catch: the site is genuinely still unsupplied, so nothing fires. ## Evidence, gathered rather than assumed - **Zero callers anywhere.** The three dashboard call sites (`Lane`, `Board`, `ListView`) bind to a **different function of the same name** in `app/components/taskSorting.ts`. Only its own tests pass the flags. - **Not reachable externally.** `@fusion/core` is private; `@runfusion/fusion` declares no `exports` map and re-exports nothing from core wholesale; it is absent from `plugin-sdk`. So there is no production behaviour to be wrong, and no supplier to wire it from. ## Why I did not "fix" it This check's usual remedy is *wire a supplier, or delete the parameter and leave the literal counted*. Neither is right here: - **Deleting the parameter** trades a dormant-but-correct implementation for a dormant legacy-only one. The parameter is optional and harmless; the only thing it costs is that the census once counted it as a conversion when it changed nothing — and that is now recorded rather than hidden. - **Deleting the function** discards a well-documented ordering (the note enumerates three real renamed-board defects) that someone may plausibly wire later. Both are churn with no product effect, in a package I do not own. The actual resolution is **consolidating the duplicate with the dashboard twin** — that name collision has already cost real time twice in this lane, including one "fix" `tsc` rejected and one bad cross-batch report. But that is a refactor with a design question attached, and it needs an owner rather than a seam-gate cleanup smuggling it in. ## Verification Gate exit 0 · lint 0. No code change, so no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
3603da731f |
fix(core): duplicate markers were never cleared on a board with renamed terminal columns (#2823)
Third unowned finding picked up after batch-core (#2783) merged without addressing its reports. Same class as #2819, opposite failure direction. ## The defect `clearNearDuplicateReferencesTo` runs on every complete/archive/delete of a canonical task and clears the `nearDuplicateOf` markers pointing at it. It first asks `isNearDuplicateCanonicalInactive` whether the canonical really is finished — a safety check, so a live canonical's markers are not cleared out from under an operator. That call omitted the canonical's resolved column flags, so it fell back to the legacy `done`/`archived` ids. On a renamed board a just-completed canonical (`shipped`, `filed`) read as **still active**, the guard early-returned, and the markers were **never cleared**. The flagged duplicates stayed parked behind a user decision that could never arrive — the exact stranding the predicate's own FNXC note says it was written to prevent. ## I had the direction backwards, and it matters My first report of this seam described it as *markers cleared against a live canonical*. That is wrong. The legacy fallback errs toward "still active", so the failure is the opposite: markers that never clear at all. Same seam, same missing argument, entirely different symptom to look for — which is why the direction is worth pinning in a test rather than reasoning about. ## Measured With the fix reverted, exactly one case flips: ``` ✓ default vocabulary: completing the canonical clears the duplicate's marker × renamed vocabulary: completing the canonical clears the duplicate's marker ✓ renamed vocabulary: a canonical still in the WIP lane does NOT clear the marker ✓ default vocabulary: a canonical still in the WIP lane does NOT clear the marker ✓ a soft-deleted canonical clears the marker under a renamed board Tests 1 failed | 4 passed (5) ``` With it: `Tests 5 passed (5)`. ## Both negatives included A canonical still in the WIP lane must **not** clear its duplicates' markers, under each vocabulary. Resolving the real flags must not degrade into "every column is terminal", which would clear markers out from under an operator who has not made the duplicate decision yet. The soft-deleted path is covered too, since that branch never consults column flags at all. ## A fixture trap worth keeping `sourceMetadata` is `jsonb` and must be seeded as an **object**. Seeding a stringified value reads back fine through `getTask` — it parses either shape — while the production query's `source_metadata->>'nearDuplicateOf'` matches nothing. The fixture looks correctly seeded and the code under test can never find the row. My first version had this, and the self-check in the seed helper is what caught it. ## Scope Five of this predicate's six production call sites already resolved flags. This was the sixth, and the one that runs on every archive/complete transition. Not touched here: the same function's SQL predicate excludes duplicates by literal `ne(column, "archived")` / `ne(column, "done")`. That is a per-row question across many rows in one statement, not a one-line supply, so it is flagged rather than guessed at. The five **engine** call sites of this predicate are separately reported on #2785 and surfaced only via #2822. ## Verification `pnpm test:gate` green, new suite 5/5, `tsc -p packages/core` 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> |
||
|
gsxdsm
|
a6af3188df |
fix(core): startup recovery deadlocked on its own per-task lock (#2809)
## The bug `recoverStaleTransitionPendingImpl` runs its whole per-task body inside `store.withTaskLock(id, …)`. On the PostgreSQL arm it then read the task with `store.getTask(id)` — and `getTaskImpl` opens with `store.withTaskLock(id, …)` too. **The per-task lock is non-reentrant.** This codebase states that invariant in prose in two other files: > "nesting inside `withTaskLock` would deadlock since the lock is non-reentrant" — `branch-and-pr-entities.ts:561` > "because the per-task lock is non-reentrant" — `workflow-ops.ts:464` So the sweep waited forever on a lock its own frame was holding. **PostgreSQL-only — which is every production install.** The SQLite arm on the very next line reads through `readTaskFromDb`, a lock-free row read. The backend-mode port swapped only the PostgreSQL arm to `getTask`. The fix restores a lock-free read (`readTaskRow`) on that arm; nothing else changes. ## Why it survived until now The branch is entered **only** when a stale marker names a plugin hook the trait registry still knows (`hasSurvivingPluginHook`). Three nearby cases all miss it: | marker | path | |---|---| | none | the row is never scanned | | only `default-workflow:postCommit` | `hasSurvivingPluginHook` false — marker just cleared | | names an **uninstalled** plugin hook | reconciled away as degraded; nothing survives to re-run | | names a **registered** plugin hook | **reaches the in-lock read → deadlock** | Those first three are what the existing tests cover. The fourth is precisely the state a crash mid-hook leaves behind. All four are asserted in the new suite so the path cannot be re-narrowed and called covered. ## Impact This sweep runs at **startup**. A task left with such a marker deadlocks startup recovery — and because it deadlocks *while holding the task's lock*, that task is also left permanently unlockable. ## How it was found, including a correction By **bisection**, not by reading. An earlier attempt of mine to drive this recovery reported that "the sweep never returns". That was wrong in a way worth recording: the sweep returns fine in three of the four cases, and generalising the one hang to the whole function is what hid the actual trigger across several sessions. Narrowing case by case — empty store, plain task, default-only marker, unknown-hook marker, registered-hook marker — put the fault on one line. ## Verification - **Mutation-verified against the real defect.** With the fix reverted, the regression case fails by name — `recoverStaleTransitionPendingImpl did not settle within 8000ms — deadlock` — while the other three stay green. That is the actual pre-fix behaviour, not a simulation of it. - Every case is **timeboxed** on purpose: a deadlock otherwise surfaces as a suite-level timeout naming no case, which is useless for locating the fault. The deadline is not a flake knob — the fixed code settles in ~150 ms and the broken code never settles, so there is no value in between to tune. - A **vacuity guard** (no markers → scans nothing) so a change that stopped listing marked rows can't leave the other cases green. - `pnpm test:gate` — **exit 0** - full live-PG E2E surface — **152/152** - `pnpm lint` — clean Changeset included (`patch`, category `fix`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
1824c04584 |
fix(core): restore an archived card to the lane it came from (#2832)
## What Fixes the defect #2824 measured. That PR's characterization cases — merged and asserting the wrong-but-real behaviour — are flipped here to the correct lanes, which is what they were written to do. ## The bug ```ts // archive-lifecycle-2.ts const preArchiveColumn = task.preArchiveColumn ?? "todo"; ``` **`preArchiveColumn` has no database column.** It exists on the `Task` type and in the archive snapshot, and nowhere else — so the in-place restore cannot carry it, `store.getTask(id)` reads a live row that never had it, and the literal decided the destination for **every unarchive that has ever run**. | board | what happened | |---|---| | **default** | `todo` is declared, so the resolver returned it. Restores landed in the queue and **looked right**. | | **custom** | `todo` is declared nowhere, so the resolver took its "no usable history" branch and returned the **complete** lane. A card archived mid-implementation came back marked **finished**. | That coincidence is why this survived **three** separate fixes to `resolveUnarchiveTargetColumnImpl` — a `?? "done"` that invented a column, an `isColumn` legacy-enum gate, and the same gate one function over. Every one was correcting how the resolver interprets a value that never arrived. ## The fix is two halves, and either alone does nothing 1. **Capture** — `taskToArchiveEntryImpl` records `task.column` into the snapshot. That is the last place the original is still in hand, since the entry's own `column` is set to `"archived"` on the line above. 2. **Read** — `unarchiveTaskImpl` reads the **snapshot it already loaded**, not the restored row. I shipped half of this first and watched the destination stay wrong, which is how I found that the field has no row to live on. Mutation matrix: | state | result | |---|---| | both halves | **5/5 pass** | | capture only (read reverted) | **3 fail** | | read only (capture reverted) | **3 fail** | I also tried carrying it through `restoreTaskFromArchive`'s row update — that fails to typecheck, which is the proof that no such column exists and the snapshot is the only source. ## Behaviour changes, deliberately - **Custom boards** — a card returns to the lane it was archived from instead of appearing finished. - **Default board** — a card archived from `done` restored to `todo` under the literal and now restores to `done`. Returning finished work to the queue was the fallback showing through, not a rule anyone chose; the resolver's own branches say a card archived from a declared column goes back to it. ## One expectation of mine was wrong, and the resolver was right I expected a card archived from the review lane to return to **hold**. It returns to the review lane, and that is correct: `.review` is derived from the `mergeOrchestration` flag, **not** from `human-review`. The fixture's review column declares `human-review` + `merge-blocker` only, so it is not a `.review` lane to the resolver — just a declared column with usable history. The case now asserts that with the reasoning attached, so the next reader does not "fix" it back to hold. ## Verification - unarchive suite — **5/5**, mutation matrix above - `pnpm test:gate` — **exit 0** - full live-PG E2E surface — **164/164** - `pnpm lint` — clean Changeset included (`patch`, category `fix`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
9f61180dfc |
test(dashboard): restore 8 of 10 red App tests — the fixture went stale at the U12 flag cutover (#2833)
Fixes most of #2829. `App.test.tsx` has been red on `main` since 2026-07-25 — deterministically, on every commit since, identically with and without any batch branch applied. ## Root cause `ListView` early-returns a workflow skeleton when the board has no workflows: ```ts if (boardWorkflows === null || boardWorkflows.workflows.length === 0) { return renderListWorkflowSkeleton(boardWorkflows !== null); } ``` That skeleton carries the **same `list-view` class** as the real body. So every test that waits on `.list-view` and then reaches for a control inside it passed its `waitFor` and failed on the control — presenting a DOM that looks like a perfectly healthy list. That is why this read as "the board renders nothing" and then as an i18n problem; both were wrong. The probe that settled it: ``` cluster: false | listview: true | buttons: [] ``` The fixture returned `workflows: []` with `flagEnabled: false`. That **used to be correct** — the flag-off path rendered legacy columns and needed no workflow. **U12 deleted that path**, so an empty `workflows` array now means "this board has no lanes". The fixture went stale, not the product. The mock now mirrors the builtin coding lanes with the trait flags the board actually reads. ## Measured | | before | after | |---|---|---| | App.test.tsx | 10 failed / 131 passed | **2 failed / 139 passed** (141) | `tsc` 0 · lint 0 · rest of the `dashboard-app-quality-app` project unaffected. ## Two remain — different root cause, deliberately not chased here - **`opens the NewTaskModal from the list view new-task button`** now gets *past* the button (that was the skeleton bug) and fails on `role="heading"` name `"New Task"`. The heading exists as `<h3>{t("newTaskModal.title", "New Task")}</h3>`, so the modal is not rendering at all — plausibly FN-8620's FloatingWindow rework, **unverified**. - **`keeps the 'subtask' background-session route unchanged`** — uncharacterised. Two of my earlier root-cause guesses on this file were wrong, so I am not offering a third. #2829 stays open for these two with the evidence attached. ## Not quarantined `test-quarantine.json` is for flakes. This failed deterministically, and quarantining would have started a 14-day deletion clock on 141 tests covering deep-link handling, view switching, board branch filters, and the FN-5817 mobile auto-merge shell — while hiding a fixture that had silently stopped exercising the list view at all. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
654d28c104 |
test(engine): live-PG coverage for the timing role — and it is correct (#2828)
## What One new live-PostgreSQL E2E suite, 2 tests. **No production file is touched** — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-timing-trait-live-e2e.pg.test.ts` ## This one is a clean bill of health Coverage for the last lifecycle role that had none — and the seam turns out to be **correct**. Recorded deliberately: a working conversion with no test is one refactor away from a silent regression, and this program should be able to say which seams are right, not only which are broken. `cumulativeActiveMs` accrues while a card sits in the WIP lane, and both halves of the segment boundary resolve that lane by role: ```ts const isWip = (column) => inRole(column, ctx.lifecycleColumnSets?.wip, ctx.lifecycleColumns?.wip, "in-progress"); ``` Note the literal at the end. That is the **same optional-parameter shape** this series found broken at four other seams — `shouldHoldActiveFileScopeLease` (#2795), `evaluateParkedAgentTaskLink` (#2798), `resolvePlanningContinuationCandidate` (#2799), `hasAutoHealableVerificationBufferFailure` (#2802) — where a caller failed to supply the resolved answer and the literal silently took over. Here the move path **does** supply it. Measured on a live store: ``` default: after exit cumulativeActiveMs=84 renamed: after exit cumulativeActiveMs=75 <- accrues on `building`, not just `in-progress` ``` ## What it guards If the move path ever stops populating the resolved columns, the literal takes over and a renamed board's cards accrue **exactly zero** active time — silently, because zero is a legitimate value for a card that has not run. The operator sees no error, just wrong numbers on every custom board. **Mutation-verified against precisely that regression:** blinding the resolved answer (`inRole(column, undefined, undefined, "in-progress")`) fails the renamed case and leaves the control green. This is a guard with demonstrated power, not a test that happens to pass. ## Both boundaries are asserted `cumulativeActiveMs` is `0` while the card is still in WIP and only accrues when it leaves; `firstExecutionAt` is stamped on entry. A test that checked only the final number would pass against an implementation that accrued at the wrong boundary, so both are pinned. ## Verification - new suite — **2/2 passed**, mutation-verified - full live-PG E2E surface — **161/161** - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
d56c635c77 |
test(dashboard): cover the three lane resolvers nobody was testing (#2826)
## Why this exists #2821's review found a bug that lived entirely in a lane **builder** while every test drove the **guard** that consumed it. That is a structural blind spot, not a one-off: injecting a resolved value into a synchronous guard makes the guard testable and the resolver invisible. So I audited every lane helper I added this session. Three had **no direct coverage at all** — `archivedColumnsForTask`, `wipColumnsForTask`, `preWipColumnsForTask`. Their callers were tested; the functions were not. ## They shared the defect that review named Each read `resolved.length > 0 ? resolved : legacyId`, which conflates two different boards: - a **v1 upgrade** — `synthesizeDefaultColumns` emits `traits: []` on every column, so the legacy id is the only vocabulary that exists, and falling back is correct; - a **v2 board that expresses traits** and declares no lane of that role — where the legacy id names a column the board may still *have* and deliberately did not give the role. Falling back there widens the guard onto a role the board explicitly withheld. `declaresAnyLifecycleTrait` separates them, matching the shape #2821's review established for `resolveNodeOverrideLanes`. ## The fixture trap, which is the part worth reading **My first fixture could not see the bug.** It traited the role under test — and where the role *is* traited, the two shapes agree: both return the traited lane. Mutating a helper back to the old shape left all 15 cases green. The shapes diverge only when the resolved set is **empty while traits are expressed**. Each helper now has that case explicitly, with a fixture that traits something *other* than the role under test. **Mutation-verified per helper:** all three reverted independently now fail. Before the extra case, none did. This is the second time this session a fixture built with the production path normalised away the very thing under test. Worth stating as a rule: a renamed-lane fixture proves the resolver reads traits; only a *traits-expressed-but-role-absent* fixture proves what it does when the answer is legitimately nothing. ## Verification - `task-lifecycle-lanes.test.ts` → 18 passed (was 15, none covering these three) - consumer suites (`github-issue-comment`, `planning-board-tools`, `register-git-github.review-lanes`) → 64 passed together - `pnpm test:gate` → 161 + 487 + 13 + 71 - `--strict` → 0; `tsc --noEmit` and `pnpm lint` → 0 errors Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
e5c9ea3870 |
fix(core): resolve the task's own terminal node in the node-override guard (#2812)
## What
Fixes the defect **#2793 measured** but deliberately did not fix. That
PR's two characterization tests are flipped here to assert the correct
behaviour — which is what they were written to do.
## The bug
`updateTask({ nodeId })` passes through `validateNodeOverrideChange`
**twice**, and both calls were wrong in different ways:
| call | how it answered "is this terminal?" |
|---|---|
| `branch-and-pr-entities.ts:568` | `resolveTaskWorkflowIrSync` — the
**default** workflow for every task under PostgreSQL |
| `task-update.ts:53` | no options at all → `defaultIsTerminalNodeId`,
the bare literal `nodeId === "end"` |
On a board whose terminal node is not named `end`, the FN-7641 guard
inverted in both directions:
- an override to a **non-terminal** node that happens to be named `end`
was **rejected** with a merge-proof error about finalizing a card the
operator was not finalizing;
- an override to the board's **real** terminal node was **written
verbatim**, no error, card unadvanced — the silent no-op FN-7641 exists
to prevent.
## The fix
A new `isTaskTerminalNodeIdAsync` resolves the task's own workflow, with
the **identical** literal fail-soft for an unresolvable one. Both call
sites use it — pre-resolved, because `validateNodeOverrideChange`'s
callback is synchronous and it asks the question at most once.
**Nothing forced the sync call at either site**: both frames are already
`async` and already awaiting. That is the same finding as #2809's
review, one file over.
The sync helper is **deleted, not kept as a fallback**. Keeping both
would re-create the half-conversion this program keeps finding — one
caller resolved, one not, and no way to tell from a call site which it
got. `branch-and-pr-entities.ts` also leaves the sync-resolver call-site
allow-list (ratchet green, 3/3), the **second** of the six allow-listed
sites to close.
## Why both halves were needed — and how that is proven
#2793's mutation matrix showed the rejected-`end` case is
**over-determined**: both guards independently called it terminal, so
correcting either one alone changed nothing an operator could see. That
is why fixing only the allow-listed sync site would have looked like
progress and delivered none.
Re-measured here, on the fixed tree:
| state | result |
|---|---|
| both guards fixed | **3/3 pass** |
| inner guard reverted to no-options | **1 fails** |
| outer guard reverted to the literal | **1 fails** |
## Tests
#2793's two cases now assert the fixed behaviour and keep their
reasoning:
- the non-terminal `end` override is **written**, and the card stays in
review — a routing change, not a finalize;
- the real terminal `finish` override is **refused** without merge
proof, **and** the field is not written on the way to refusing.
The fixture-integrity case (`finish` is the end node, `end` is not) is
unchanged — it is what stops both assertions passing for the wrong
reason.
## Verification
- terminal-node suite — **3/3**, both-halves matrix above
- sync-resolver call-site allow-list ratchet — **3/3**
- `pnpm test:gate` — **exit 0**
- full live-PG E2E surface — **151/151**
- `pnpm lint` — clean
Changeset included (`patch`, category `fix`).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
7c30b54255 |
test(engine): live-PG evidence that restore lands every custom-board card in DONE (#2824)
## What One new live-PostgreSQL E2E suite, 5 tests. **No production file is touched** — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-unarchive-target-live-e2e.pg.test.ts` ## The finding I went looking for coverage of the one lifecycle role that had none — `archived` — in the function with the worst track record in this program. `resolveUnarchiveTargetColumnImpl`'s own comments record **three** separate defects fixed on those few lines: a `?? "done"` that invented an undeclared column, an `isColumn` legacy-enum gate that rejected every renamed id, and the same gate one function over that dropped a renamed board's stored history on read. All three were reasoned from source. **None had a live-store test.** With one, the path is still broken — and the cause is upstream of everything those fixes touched: ```ts // archive-lifecycle-2.ts:441 const preArchiveColumn = task.preArchiveColumn ?? "todo"; ``` **`preArchiveColumn` is never written.** Across `packages/core`, every occurrence *reads* it or copies it through — into the archive entry, back out of it, through serialization. Nothing ever sets it from `task.column` when a card is archived. Measured on both boards: ``` default: after archive column=archived preArchiveColumn=undefined -> resolver target=todo renamed: after archive column=archived preArchiveColumn=undefined -> resolver target=shipped ``` So the fallback fires for **every restore that has ever happened**, and the two boards diverge on what `"todo"` means to each: | board | what happens | |---|---| | **default** | `todo` is declared, so the resolver returns it. Every restore lands in the queue — right for a card archived mid-implementation, wrong for one archived from `done`, and it **looks** right. | | **renamed** | `todo` is declared nowhere, so the resolver takes its "no usable history" branch and returns the **complete** lane. Archive a card mid-implementation, restore it, and it comes back marked **finished**. | That coincidence on the default board is why this survived three rounds of fixes to the resolver: they were correcting how it interprets a value that never arrives. ## Evidence discipline - **Observed state** — the persisted `column` after a real `archiveTask` + `unarchiveTask` against a real store and real stored workflows. - **Characterization**: the cases assert today's wrong-but-real behaviour so the defect is executable rather than argued, and they are written to flip when the write is added. - **The default-board control** is what shows the coincidence, not just the failure. - **Mutation-verified**: changing the hardcoded fallback from `"todo"` to the renamed hold id fails **all five** cases — every assertion binds to that literal, which is the claim. ## The fix, proposed rather than included One line: persist the card's column when archiving, so `preArchiveColumn` carries real history and the resolver — already correct — can use it. I did not include it because **it also changes default-board behaviour**: a card archived from `done` currently restores to `todo` (the fallback), and with real history it would restore to `done`. That is the better outcome, but it is a visible placement change for every existing install, and that decision belongs to whoever owns archive semantics rather than to an evidence PR. The five cases above are the acceptance criteria for it — the three renamed expectations become the lane the card was actually in. ## Verification - new suite — **5/5 passed**, mutation-verified - full live-PG E2E surface — **159/159** - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
9a5155c71a |
fix(tests): main red — a census source guard that a COMMENT could invert (#2825)
## Red on main
```
lifecycle-column-census > the baseline can always be re-recorded
> writes the baseline BEFORE the rise check can exit
AssertionError: expected 19345 to be greater than 26374
```
Read literally, that says the CLI now runs its rise check *before* the
`--update-baseline` write — which would break the one command whose
entire job is re-recording, and would be a genuine bug worth stopping
for.
**It does not.** The order in code is correct and unchanged:
| | line |
|---|---|
| `if (updateBaseline) { … writeBaseline() … process.exit(0)` |
`scripts/lifecycle-column-census.mjs:487` |
| `"column-guard count ROSE"` + `process.exit(1)` | `:510` |
## What actually moved was a comment
Line **359** explains this exact failure mode and quotes the marker
verbatim:
> …`"column-guard count ROSE"`, which is the opposite of what happened
and sends the reader looking for…
So `cli.indexOf("column-guard count ROSE")` found the **prose**, 7000
characters before the branch it was meant to locate.
A guard that a comment can invert is not measuring control flow. And the
honest-looking fix — reword the comment — silently re-arms the same trap
for whoever explains this next.
`cliSource()` now strips comments before indexing. The same defence is
already used by `archived-column-gate-parity.test.ts`, for the same
reason: notes documenting *why* a literal is dangerous have to mention
the literal.
## Kept, not deleted
The end-to-end block below these does cover the contract — it drives the
real CLI and asserts exit code, baseline content and printed output, and
its own comment names the ordering bug. It would have been defensible to
delete the two source-text cases as redundant.
I kept them because two guards at different levels is the point: **e2e
proves the behaviour, these locate the branch that provides it.** They
only needed to stop being defeated by prose.
## Evidence
The real ordering bug — make a rise exit before the update branch writes
— fires **all three**:
| guard | failure |
|---|---|
| source order | `expected 9454 to be greater than 9505` |
| slice / uniqueness | `expected 10433 to be -1` |
| end-to-end | `expected 1 to be +0` (exit code) |
**My first mutation attempt was invalid** and I nearly reported it as
evidence: it moved the block by line range, mangled the file, and both
markers disappeared — the resulting `-1`s look like a firing guard but
prove nothing. A mutation that corrupts its target is not evidence that
a guard works.
Engine **10991 passed / 0 failed** · gate **732 green** · lint clean.
Test-only; the CLI is restored clean.
## Note on duplicated effort
#2811 and my #2814 both re-recorded the census baseline for #2783's
rise, concurrently. No harm done — but this file is now a fleet-wide
contention point, and the per-file baseline shape exists precisely to
avoid that. Worth one owner for census/ratchet fixes rather than whoever
notices first.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
8c79da364c |
fix(engine): FN-8356's duplicate-marker cleanup was inert on a renamed board (5 call sites) (#2827)
Fourth unowned finding picked up after both batch PRs (#2783 core, #2785 engine) merged without addressing their reports. Completes the `isNearDuplicateCanonicalInactive` seam alongside #2823, which fixed the sixth (core) site. ## The defect All five engine call sites — `self-healing.ts` ×2, `triage.ts` ×3 — called the predicate without the canonical's resolved column flags, so it fell back to the legacy `done`/`archived` ids. A canonical resting in a renamed complete column (`shipped`) read as **still active**, so every *"the canonical is inactive, so clear the marker"* branch failed to fire. The user-visible result is precisely the stranding FN-8356 was written to remove: a card keeps its **"Needs your decision"** duplicate badge pointing at work that shipped days ago, and no decision can resolve it — the detail banner deliberately offers none for an inactive canonical. ## Measured With the fix reverted, exactly one case flips: ``` ✓ clears the FN-8353-shaped hidden decision for every inactive canonical state × renamed vocabulary: clears the decision for a canonical resting in a RENAMED complete column ✓ renamed vocabulary: leaves the decision alone while the canonical is still in the WIP lane ✓ leaves active canonical decisions, user pauses, unrelated reasons, non-marker sources untouched Tests 1 failed | 3 passed (4) ``` With it: `Tests 4 passed (4)`. ## Wiring is proven separately from behaviour A behaviour test on one call site says nothing about the other four — that is the failure this whole lane keeps re-finding, so I did not rely on it. With #2822's barrel-import fix applied locally, the seam gate's staleness check **fails both engine allow-list entries as "now supplied"** — it can no longer find an omitting call site in either file. That is the proof for all five. Consequence worth flagging: **when the second of #2822 / this PR lands, the two engine entries in #2822 must be deleted.** CI fails until they are, by design — the exemption cannot outlive its fix. ## Both negatives included A canonical still in the WIP lane must **not** have its decision cleared, under each vocabulary. Resolving real flags must not degrade into "every column is terminal", which would dismiss a duplicate decision the operator has not made yet. ## Design note The helper is module-private in each file rather than shared. `findColumn` is already duplicated exactly this way in `hold-release.ts`, `merge-trait.ts`, and `workflow-capacity.ts` — following the established shape beat adding a cross-module abstraction for a bug fix. ## Verification `pnpm test:gate` green · 27 triage suites / 387 tests green · engine `tsc` 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> |
||
|
gsxdsm
|
bcc8bf17c1 |
fix(core): brand the unregistered built-in workflow fallback (#2815)
> **Corrected after opening.** The first version of this PR claimed the
id cross-check reported `"default"` for *every* authored workflow and
stranded cards in triage recovery forever. That was wrong, and the
correction is below in full. The reachable defect is the branding hole;
removing the id check is correctness, not a live bug fix.
## The bug (reachable)
`resolveWorkflowIrById` has four ways to substitute the default coding
IR. Three brand the result via `markFellBack`. The fourth did not:
```ts
if (isBuiltinWorkflowId(workflowId)) {
const builtin = getBuiltinWorkflow(workflowId);
const ir = builtin?.ir ?? defaultCodingWorkflowIr(); // <- unmarked substitution
```
An id that *looks* built-in but is not registered — a workflow removed
between releases, a typo'd selection — lands there and silently gets the
default coding IR. `resolveWorkflowIrForTaskWithProvenance` then reports
**`source: "selection"`** for it, handing a caller the default board's
graph under the selected workflow's name. That is precisely the lying
signal the API exists to prevent.
Found by review on this PR (thanks — see the thread), not by me.
## The correction to my own claim
I also deleted an id cross-check that ran after the marker check and
reported `"default"` when the resolved IR's `id` differed from the
requested workflow id. I justified that by saying it misfired for every
authored workflow, because `createWorkflowDefinition` stores an IR
verbatim while minting `WF-NNN` separately:
```
store workflow id = WF-001 stored ir.id = custom:prov
PROVENANCE source = default resolved ir.id = custom:prov <- the CORRECT IR, called a guess
```
**That measurement is real but it came from this suite's own fixture.**
Neither `WorkflowIrV1` nor `WorkflowIrV2` declares an `id` field. An
editor-authored workflow carries none, so `resolvedId` is `undefined`
and the check passed it as `"selection"` — the misfire never reached
`triage.ts`'s post-U11 intake recovery, the one production consumer. My
"declined forever, not deferred" claim does not hold.
Removing the check is still right: it is **unreliable** (it interrogates
a property the IR types do not declare, and when one is present it is
the author's id, unrelated to the store-minted row id) and **redundant**
(all four substitutions are now branded, and the marker is checked
first). But it is a cleanup, not a fix — and saying otherwise is how a
narrow change gets backported as a critical one.
## Tests
| case | asserts |
|---|---|
| unregistered **built-in** id | **`source: "default"`** — the reachable
defect |
| missing definition | still `"default"` |
| no selection | still `"default"` |
| IR carrying its own id | `"selection"`, with the id mismatch asserted
explicitly so it can't pass for the wrong reason |
| the resolved IR is the task's own board | contains the renamed hold
column, not a default-board id |
**Mutation-verified:** removing only the branding fails the
unregistered-builtin case and nothing else — which shows it covers that
specific hole rather than overlapping the others. Restoring the id check
fails only the carries-its-own-id case, leaving both fallback cases
green, which shows the deletion did not widen trust.
## Blast radius
`triage.ts:1211` is the only production consumer of the provenance
result across core, engine, dashboard and CLI. Every other `.source ===`
hit is an unrelated field on an unrelated type; the two index hits are
re-exports.
## Verification
- new suite — **5/5**, mutation-verified in both directions
- `pnpm test:gate` — **exit 0**
- `pnpm lint` — clean
Changeset included (`patch`, category `fix`), rewritten to describe the
branding fix rather than the overstated one.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
ed6d54485b |
glasses plugin: the review actions could never resolve a review lane (4 guards + 3 invisible destinations) (#2816)
Four agent actions still keyed on literals, with three census-invisible `moveTask` destinations between them. `agent-actions.ts` already had `laneContext`/`destination` from an earlier partial conversion — these were simply never migrated. ## Census | file | main | here | | --- | ---: | ---: | | `plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts` | 4 | **0** | Plus 3 hardcoded `moveTask` destinations the census cannot see (`requestReview` → `in-review`, `returnToAgent` → `todo`, `retryTask` → `todo`). ## The real finding: this plugin could never resolve a review lane `resolveLifecycleColumns` keys its `review` role on the **`mergeOrchestration` trait alone**. A board whose review column carries only `merge-blocker` and/or `human-review` — the common custom shape, since `merge` is opt-in — resolves **no review lane at all**. So every review-gated action here (`requestReview`, `acceptReview`, `returnToAgent`, `retryTask`) compared against `undefined` and **refused every card**, and `requestReview` had nowhere to move one. This is not a regression from converting them; it is why they *could not* be converted with `lanes.review` as-is. Converting the four guards without noticing would have shipped four actions that fail closed on exactly the boards this program exists to support — a conversion that looks complete, passes its suite, and makes the plugin useless on a custom board. **Widened in `laneContext`, not in the shared resolver.** `resolveLifecycleColumns` is consumed well beyond this plugin, and its `review` role deliberately means "the merge-orchestration column" for the merge queue. The gap is already recorded in `notification-renamed-lifecycle-columns.test.ts` and in #2807 — reconciling the two definitions is a core-level decision, not one to take from a plugin. `mergeBlocker` is preferred over `humanReview` because a card cannot leave a merge-blocking column until the gate clears, which is the closer analogue of the legacy `in-review`. ## The suite caught an over-reach of mine My first version put a blanket `if (degraded) conflict(...)` at the top of `retryTask`, which broke a pinned invariant the test names outright: **"a degraded workflow does not block retries that move nothing."** The status-only retry just clears fields; refusing it because the workflow could not be read breaks a recovery that needs no lane at all. Degraded now blocks only the branches that actually **move**. Same reasoning applied to `acceptReview`, which also moves nothing. The existing `startWork` convention — conflict on degraded — is right precisely *because* it moves. ## Ordering `returnToAgent` and `retryTask` now resolve their destination **before** the field clear. Both cleared first, so a rejected move left the assignee and status — or the worktree, branch and base refs — nulled with the card exactly where it was. That is the fifth instance of this half-applied shape in the audit, and it is rule 3 in the class doc. ## Revert results (measured, each independently) | conversion | reverted → | | --- | --- | | `requestReview` destination | 1 failed — moves to the literal `in-review`, which this workflow does not declare | | `returnToAgent` destination | 1 failed — moves to the literal `todo`, same | Plus a non-vacuous companion: a renamed card *not* in the wip lane must still be refused by `requestReview`, so a gate admitting everything would not pass. ## Verification - Plugin suite — **186/186** - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `tsc` on the plugin — clean - `pnpm lint`, `check:changesets`, census `--strict` — all clean (run explicitly) |
||
|
gsxdsm
|
d2f47acedd |
fix(tests): the core half of #2783's bookkeeping — archived-gate inventory (#2817)
## The 6th red from #2783 #2814 cleared the 5 **engine** reds #2783 left on `main`. This is the sixth, in **core** — I found it after #2814 was already open, and it merged before I could fold this in. ``` archived-column-gate-parity > all three encodings of the archived gate stay in lockstep with the audited inventory AssertionError: TypeScript encoding changed. ``` ## Same cause, same shape as #2786 #2783 converted three more sites off raw `column === "archived"` comparisons and did not update the inventory in the same commit — which the guard's own failure text explicitly asks for. | file | before → after | |---|---| | `async-mission-store.ts` | 2 → 0 | | `task-store/symbol-locks.ts` | 1 → 0 | | `task-store/archive-lifecycle-2.ts` | 2 → 1 | **Verified each is a real conversion, not a dropped gate.** All three now seed a legacy lane set and extend it from the workflow: ```ts const lanes = new Set<string>(["done", "archived"]); … for (const id of columnsWithFlag(ir, "archived")) lanes.add(id); ``` The literal still in `archive-lifecycle-2.ts:46` is `column: "archived"` as a **move destination**, not a gate comparison — the same distinction the planner-lane move targets get. ## Verified NOT a split-brain That is the thing this file exists to catch — TypeScript moving to the resolved role while the SQL sides keep comparing the raw string. The **Drizzle and raw-sql inventories are unchanged and both pass**. Worth stating explicitly because those assertions run *after* the TypeScript one: a plain red tells you nothing about them, so they had to be re-run green to know. ## One thing I nearly got wrong My first edit was a whole-file string replace and it threw on an assertion count. That turned out to be load-bearing: **these paths appear in more than one inventory in this file** (`AUDITED_TS_SITES` and the raw-sql inventory both list `async-mission-store.ts`). An unscoped replace would have silently edited the raw-sql inventory too — making the parity guard agree with itself and defeating the exact cross-encoding check it exists for. The edit is now scoped to `AUDITED_TS_SITES` by line range. ## Evidence - Guard still bites: appending a real `task.column === "archived"` to an audited file → **fails**. - Core **4773 passed / 0 failed** · engine **10988 passed / 0 failed** · gate **732 green** · lint clean. Test-only; `agent-store.ts` restored clean after the mutation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
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> |
||
|
gsxdsm
|
2ccd78abbc |
fix: main is red on the lifecycle ratchet — re-record the census baseline (#2811)
**`main` is RED on the lifecycle ratchet right now.** `node scripts/lifecycle-column-census.mjs --strict` exits **1** on pristine `origin/main`, which is the `Lint` job's *Lifecycle-column ratchet* step — so **every open PR fails Lint** until this lands, regardless of its own contents. Verified on a detached checkout of `origin/main`, not on a branch of mine. ## Cause Eight `DELIBERATE-LITERAL` markers were added across seven files without re-recording the baseline: ``` packages/core/src/task-move-disposer.ts (in-progress, todo) packages/core/src/task-store/archive-lifecycle-2.ts (archived) packages/dashboard/src/github-tracking-comments.ts (done) packages/dashboard/src/gitlab-tracking-comments.ts (in-progress) packages/dashboard/src/server.ts (archived) packages/dashboard/src/task-planner-chat-context.ts (done) packages/dashboard/src/test/mockCoreEngine.ts (in-review) ``` Adding a marker RECLASSIFIES a site (column-guard → deliberate), so the tracked deliberate totals move and `--strict` fails until the baseline records the new shape. It is the same mechanism that turned #2775 red earlier today — a marker landing without its baseline — which is worth noting because it has now happened twice from different PRs. ## The fix Baseline re-recorded, nothing else. Zero source changes; the diff is one derived file. - `--strict` exits **0** - `pnpm test:gate` — **161 / 13 / 487 / 71** - `pnpm lint` clean ## Worth a follow-up by whoever owns the ratchet The failure is structural rather than careless: a PR that adds a marker is *doing the right thing*, and the baseline requirement is only discovered when CI goes red — after merge, for everyone else. Two options, neither of which I am taking unilaterally on a red-main fix: 1. have `--strict` treat a marker-only reclassification as an accepted rise (it is not new debt — the count of unconverted guards goes **down**); 2. or fail the PR that adds the marker, by comparing against the base ref rather than the recorded baseline — the machinery for that already exists in this script. I would take (1): a marker is the documented way to close a site, and requiring a second mechanical step to record it is a trap that catches good behaviour. 🤖 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 lifecycle census error messages to distinguish genuine increases in column-guard debt from reclassified deliberate literals. * Added clearer remediation guidance for reclassified results, including when to update the baseline. * Updated lifecycle census baseline mappings to reflect current classifications. * **Tests** * Added coverage for unchanged baselines, genuine guard-count increases, and marker-only reclassification scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
9fab32e1d9 |
fix(tests): 5 engine reds from #2783 — a stale census baseline and a turn-counting test (#2814)
## Red on main #2783 (batch-core) landed and put **5 failures** on `main`. Both causes are the bookkeeping half of correct changes, not defects in them. ## 1. The census baseline — 4 failures `census-baseline-corruption-guard` plus 3 `lifecycle-column-census` ratchet cases, all downstream of one thing: ``` lifecycle-column-census --strict: column-guard count ROSE packages/core/src/task-move-disposer.ts (DELIBERATE-LITERAL: in-progress): 0 -> 1 packages/core/src/task-move-disposer.ts (DELIBERATE-LITERAL: todo): 0 -> 1 packages/core/src/task-store/archive-lifecycle-2.ts (DELIBERATE-LITERAL: archived): 0 -> 1 packages/dashboard/src/github-tracking-comments.ts (DELIBERATE-LITERAL: done): 0 -> 1 packages/dashboard/src/gitlab-tracking-comments.ts (DELIBERATE-LITERAL: in-progress): 0 -> 1 packages/dashboard/src/server.ts (DELIBERATE-LITERAL: archived): 0 -> 1 ``` **The rise is legitimate.** #2783 *annotated* documented fast-path literals — e.g. `task-move-disposer.ts`'s *"a fast path, not the guard … the actual lane decision is the RESOLVED membership test inside this block"* — and the census tracks marked literals per file. Re-recorded with `--strict --update-baseline`; the same run also **tightened 20 entries whose counts dropped**, so this moves the ratchet down as well as up. ## 2. The disposal-order test — 1 failure ``` executor-user-cancel > re-dispatch (task:moved → in-progress) awaits prior disposal before execute() AssertionError: expected -1 to be greater than 2 ``` `-1` reads like the re-dispatch was **dropped**. It was not — that would be a real cancel-race bug, so I checked before touching the test: ``` PROBE_MICROTASK callOrder=["abort-started","abort-resolved","dispose","execute"] PROBE_AFTER_TIMER callOrder=["abort-started","abort-resolved","dispose","execute"] ``` Correct order, reached once drained, unchanged after a real 50ms timer. The test drained exactly **two** microtask turns and #2783's disposer refactor added await hops, so `execute` had not been recorded yet. A fixed turn count encodes today's await depth into the test: any added `await` on the product path fails it for a reason that has nothing to do with the invariant. It now waits on the **outcome** via `vi.waitFor`. The ordering assertion is untouched and is still the point. ## Evidence | mutation | result | |---|---| | `execute` never recorded (stands in for a dropped re-dispatch) | **fails** — `waitFor` times out | | `execute` observed *before* `dispose` | **fails** — `expected 'execute' to be 'dispose'` | | baseline: fresh `--strict` run | *"every file matches its baseline exactly"* | Engine **10985 passed / 0 failed** (was 5 failed) · gate **732 green** · lint clean. ## Method note, against myself I pre-flighted #2783 and **reported it clean** — but I ran only `@fusion/core` and the dashboard `api` group, because that is what the diff touches. The census and disposal tests live in `packages/engine`, which batch-core does not modify at all. **The suite that breaks is not always the suite the diff points at.** A cross-package ratchet like the census is exactly the case where scoping pre-flight to the changed packages produces a confident "clean" that is wrong. Pre-flight needs the engine suite regardless of which package a batch touches; I have adjusted accordingly for the remaining queue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
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>
|
||
|
gsxdsm
|
240a6be0aa |
fix(core): dependency update deadlocked on a self-blocked task (#2810)
## The bug `updateTaskDependenciesImpl` wraps its whole body in `store.withTaskLock(id, …)`, then reads the current blocker with `readDepTask(task.blockedBy)` → `store.getTask()`. `getTaskImpl` opens with `withTaskLock(id, …)` too, and the per-task lock is **non-reentrant**. So when `blockedBy` is the task's **own id**, the call waits forever on a lock its own frame holds — and holds that lock while doing so, leaving the row permanently unlockable. ## Found by generalising #2809, not by luck #2809 removed one `getTask`-inside-`withTaskLock`. An AST scan for the same shape across `packages/core` and `packages/engine` returned **exactly three sites**: | site | verdict | |---|---| | `lifecycle-ops.ts:1049` | the deadlock fixed in #2809 | | `update-task-deps.ts:233` (`assertTaskExists`) | **safe** — a self-dependency is rejected 15 lines earlier | | `update-task-deps.ts:344` (`readDepTask`) | **this bug** | Both surviving sites carry the same `FNXC:SqliteDualPathCleanup` note — *"In backend mode, readTaskFromDb uses store.db (SQLite) which is unavailable. Replace with async store.getTask() calls."* That port is the common cause across the whole class: it swapped a **lock-free** read for a **lock-acquiring** one. ## Why `blockedBy === id` is reachable The dependencies list rejects self-reference explicitly (*"Task X cannot depend on itself"*) — and that guard is precisely why the sibling `assertTaskExists` read on this same lock is safe, so it is left unchanged. **`blockedBy` has no such guard:** `updateTask({ blockedBy })` accepts the task's own id. The first test asserts that rather than assuming it. The whole regression rests on that state being reachable, so it is proven, not stipulated — and it also pins the asymmetry, so a future guard on `blockedBy` will show up here as a deliberate change. ## The fix Return the in-lock copy already in scope instead of re-reading. One line, no new read path, and **strictly more correct than a re-read**: it is the state this mutation is reasoning about, rather than whatever a concurrent writer left behind. ## Verification - **Mutation-verified against the real defect.** With the fix reverted the regression case fails by name — `updateTaskDependencies did not settle within 8000ms — deadlock` — while the precondition and the ordinary-path cases stay green. That is the actual pre-fix behaviour. - **A differential** covering the ordinary case (blocked by *another* task). Without it, a fix that short-circuited *every* blocker read would pass everything else. - Timeboxed for the same reason as #2809: a deadlock otherwise surfaces as a suite-level timeout naming no case. Not a flake knob — the fixed path settles in ~0.5 s and the broken one never settles. - `pnpm test:gate` — **exit 0** - `pnpm lint` — clean Changeset included (`patch`, category `fix`). Independent of #2809 — different file, no overlap — but the same class, and the scan above is the argument that the class is now closed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b7288572a1 |
engine: a GitHub "changes requested" review was silently dropped on a renamed board (1 → 0) (#2807)
A human reviewer's feedback was being thrown away.
`PrCommentHandler.handleChangesRequested` gated on `task.column !==
"in-review"` and returned early. On any board whose review lane is
renamed, a GitHub **"changes requested"** review produced **no steering
comment** and the card **never went back to work** — the feedback
vanished behind a log line nobody reads. No error, no audit row.
## Census
| file | main | here |
| --- | ---: | ---: |
| `packages/engine/src/pr-comment-handler.ts` | 1 | **0** |
## Two literals, only one countable — again
```ts
if (task.column !== "in-review") { … return; } // counted
…
await this.store.moveTask(taskId, "in-progress"); // INVISIBLE to the census
```
The census scores comparisons. The requeue **destination** is a call
argument, so nothing in the backlog pointed at it — the same pairing as
the branch-worktree auto-requeue in #2797, and the same trap: converting
the gate alone would make the handler *admit* the review and then
attempt a move into a lane the board may not declare, which `moveTask`
rejects. A half-conversion here turns a silent drop into a thrown
rejection. They convert together or not at all.
That is now the second confirmed instance of this shape. The pattern to
look for is a **counted guard whose body performs a hardcoded
`moveTask`** — the guard is the visible half and the move is the
dangerous one.
## Revert results (measured, each run independently)
| conversion | reverted → |
| --- | --- |
| review-lane gate | RENAMED case fails — `updateTask`/`moveTask` never
called; the review is dropped |
| requeue destination | RENAMED case fails — `moveTask` called with
`"in-progress"` instead of the board's wip lane |
The legacy case passes both ways, which is why both vocabularies run. A
non-vacuous companion (renamed board, card sitting in the hold lane)
keeps a gate that admits everything from passing.
## Verification
- `pnpm test:gate` — 161 + 487 + 13 + 71, green
- `pr-comment-handler.test.ts` — 34 passed
- `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean
- `pnpm lint` — clean
- `node scripts/lifecycle-column-census.mjs --strict` — exit 0
(Running the census explicitly, not just `pnpm lint`: CI's Lint job runs
both, and a clean local `pnpm lint` is **not** evidence the Lint check
passes — that cost a round-trip on #2797.)
|