21ef60047efcb5993ee255ca3ec5ae0feef836b1
2728 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
998d75da3b |
refactor(core): resolve the hand-off archive guard by role (fleet) (#3054)
## Census | | column guards | |---|---| | before (this branch) | **122** | | after | **118** | Baseline re-recorded in the same commit, as the ratchet requires. (Four of the delta land with #3052; this PR carries `moves.ts`.) ## What changed `handoffToReviewImpl` refuses a hand-off from an archived card. Against the literal `archived`, a board whose archive lane is renamed **never matched** — so an archived card could be handed to review, and the invariant `HandoffInvariantViolationError` exists to protect was silently unenforced. The IR is resolved at the guard rather than 28 lines below where `handoffTarget` already reads it; the later read now **reuses** it instead of resolving twice. The hoist is safe because this function has already awaited `readTaskRowAsync` above — no new tick boundary. That's the specific hazard blocking the scheduler cluster, so I checked it here rather than assuming. Absent or trait-free IR keeps the legacy id: unconverted boards are byte-identical. ## Fleet intelligence: the backlog is now essentially fully triaged I worked down the census top-files list and verified each before writing. **Every remaining cluster is claimed, fallback-by-design, or documented-blocked:** | cluster | guards | status | |---|---|---| | `self-healing.ts` | 51 | **claimed** — checked out in another worktree (`convert/self-healing-lane-cluster-u7`) | | `scheduler.ts` | 12 | **blocked**, documented at line 907 — `task:moved` prologue is synchronous; hoisting reorders this listener against every other subscriber | | `notification-service.ts` | 5 | **blocked**, documented — needs the wedge-episode contract serialised first; the second site needs gate-placement judgement in `handleTaskUpdated` | | `executor.ts` | 4 | **claimed** (`fleet/executor-lifecycle-roles`) | | `restart-recovery-coordinator.ts` | 4 | **trait-fallback arms** — the census counts these as already converted | | `taskRevert.ts` | 2 | **blocked**, documented — would classify a *neighbour* task with the modal's own flags (the wrong-row shape, worse than the literal) | | `project-store-ops.ts` | 2 | **blocked**, documented — the dead SQLite twin; its first statement throws under PostgreSQL | | `github-tracking-state.ts`, `planner-overseer.ts`, `async-mission-store-queries.ts`, `register-task-workflow-routes.ts` | 2 each | **trait-fallback arms** | | `auto-merge-finalization.ts` | 2 | one is the `catch`-block degraded fallback; the other is a reason string | So the mechanical conversions are done. What's left needs either a design change (scheduler's event payload, notification's episode contract) or per-row lane data that doesn't exist at the call site yet (`taskRevert`). **That's the useful signal for the fleet**: further census reduction isn't a matter of more conversion passes. Forcing these would produce exactly the "conversions that break the code and improve the number" the learnings doc is named for. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
107a1e790a |
fix(core): the review lane was resolved for two stall signals and literal for the other two (#3053)
## Claim Largest **unclaimed** census cluster. `self-healing.ts` (56) is the capacity worker's file and `scheduler.ts` (12) is blocked (below), so I took **@fusion/core** — 16 bare guards across 12 files, untouched by any open PR. ## Census before / after ``` before: COLUMN guards (the backlog): 126 after: COLUMN guards (the backlog): 126 ``` **Unchanged, and that is the honest result — not a failed conversion.** The repo's sanctioned device is an optional *resolved* parameter whose default stays the legacy literal (the exemplar is `restart-recovery-coordinator.ts`, watched by the unwired-lane-parameter guard). The literal survives as the default arm, so the counter cannot see the conversion. **This matters for the fleet phase.** The census is not a progress meter for this pattern. A worker driving the number down has only two ways to move it, and both are wrong: 1. **Delete the fallback** (make the parameter required) — prior review explicitly argued against this; `cli-active-count-lanes.test.ts` deliberately covers the no-argument path. 2. **"Convert" with `resolveTaskWorkflowIrSync`** — that reader returns `undefined` unconditionally under PostgreSQL, the shipped backend. It drops the count while behaving *exactly* like the literal. That is the inert-conversion class, and `merge-queue-ops-2.ts:53` already carries a flag note saying so. I measured the split across all 126: **18 are fallback arms of already-converted seams; 108 are bare guards.** The headline number conflates them. ## What changed `reads.ts` states the invariant in its own words — > RESOLVED BEFORE THE FIRST SIGNAL, because two adjacent signals must not disagree. — and then called two of the four stall signals with the literal: | signal | before | |---|---| | `getInReviewStallReason` | resolved (`reviewColumns`) | | `getInReviewStalledSignal` | resolved (`reviewColumns`) | | `detectStalledReview` | **literal `"in-review"`** | | `hasFreshAgentLogActivitySinceTaskUpdate` | **literal `"in-review"`** | On a renamed board `stalledReview` returned `undefined` for every card, and the fresh-activity gate answered `false` — so `executingTaskIds` stayed empty and the board showed Stalled / Merge stalled *while a merger was visibly streaming*, the precise regression that function's own FNXC note says it was restored to prevent. Both now take an optional resolved `reviewColumns`. All four hydration passes pass the set **they already had in scope one line away**; two needed only a hoist, one reused the per-row map, one was resolving the same set inline twice. ## Mutation evidence | Mutant | Result | |---|---| | baseline | 11 passed | | revert the detector guard to the literal | **2 failed** | | make the parameter a widening (`reviewColumns ? true`) | **2 failed** | The second matters: it proves the new parameter is a real gate and not a change that merely makes every card eligible. Both arms are asserted, since the literal default is load-bearing for every caller outside `reads.ts`. ## Flagged — do not guess - **`scheduler.ts` (12 guards).** All 12 sit inside *synchronous* listeners (`task:moved`'s sync prologue; `task:updated` is sync outright). The only sync resolver available, `resolveTaskParkedColumnsSync`, is already used at lines 929/1130/1157 and is **inert under PostgreSQL** — my own live-PG E2E proves it always returns the default board. "Converting" these with it would drop the census by 12 and change nothing. The existing note at 908–926 names the real unblock: carry resolved lanes on the event payload so no listener resolves at all. Left alone. - **`restart-recovery-coordinator.ts` (4)** — already the optional-parameter device with all three production callers passing resolved answers. Not backlog. - **`reads.ts:358`, `audit-ops.ts:208`, `task-id-integrity.ts:444`** — `"archived"` here is the *cold-storage tier*, not the board column. Trait resolution would be wrong. ## Verification `test:gate` exit 0 · full `@fusion/core` unit suite **4880 passed** · typecheck exit 0 · `pnpm lint` clean · lifecycle-column census exit 0 · FNXC date ratchet exit 0 · lane-wiring census exit 0. **One unrelated failure to report, not appeased:** `src/__tests__/postgres/pg-test-harness-template-concurrency.pg.test.ts` fails under the full suite and **passes in isolation on both my tree and the untouched baseline** — a pre-existing full-suite concurrency flake in the PG harness. Not mine, not in the merge gate. I did not quarantine it: it is another worker's harness, and AGENTS.md warns that quarantining a concurrency test can mask a real product race. Flagging for its owner. |
||
|
|
4878bda197 |
fix(core): the mission bootstrap duplicate was archived into a lane the board does not declare (#3046)
## Invisible to both censuses
`archiveDefinedFeatureBootstrapDuplicate` writes `tasks.column`
**directly** rather than through `moveTask`:
```ts
.set({ column: "archived", updatedAt: … })
```
- the **lifecycle census** reads comparisons — an assignment isn't one
- the **move-target census** reads `moveTask` call arguments — this
never calls it
So on a board whose archive lane is renamed, the duplicate landed in a
column that workflow doesn't declare: a card in a lane the board can't
render, from a path that runs during ordinary feature bootstrap.
## Reuses the helper this class already has
`archivedLanesFor(taskId)` was added for the guards further up the same
file. It returns the legacy id when the task has no resolvable workflow,
so an **unconverted board is byte-identical**. No new resolution
machinery — the two `<> 'archived'` guards become `notInArray(column,
[...lanes])` and the write targets the resolved lane.
A board declaring several archive lanes is arbitrated by taking the
first, the same choice `resolveLifecycleColumns` makes. Multiple archive
lanes aren't a shape the builtin lineages produce.
## Measured
| check | result |
|---|---|
| mission-store PG suite | **36 → 38**, all green |
| new pair | differential — `filed` collides with no legacy id, and the
default-lineage control still lands in `archived` |
| mutation (hardcode the target back) | fails the renamed case |
| SQL literal gate · `tsc` | green |
## How this was found
Measuring the literal-column-**write** population for #2839: 51 raw
sites, of which 20 are the four builtin workflow IRs declaring their own
columns (correct by definition) and several more are archive-*entry
record* fields rather than board columns. This is the one I verified is
a real board write on a live path.
Worth noting the measurement itself was wrong twice first — my glob was
`packages/*/src/**/*.ts`, which requires a subdirectory and silently
skipped every top-level file in `src/` (including this one), and my
script printed only the first 14 findings so the grouping was over a
truncated list. Same scope-blindness class as #3000 and #3002, this time
in a throwaway scanner.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
8b82e77fbf |
chore(core): delete liveParentFilter — no caller, and it carried a legacy lane literal (#3042)
Found while enumerating archive-exclusion sites for #3041. ## Unambiguously dead `liveParentFilter` has exactly **one** reference in the repo: its own definition. - not exported from `index.ts` or `index.gate.ts` - no test imports it - no production code calls it It nonetheless contained `column != 'archived'`, so it was one of the 22 sites the SQL column-literal gate tracks. ## Why delete rather than convert Converting it would mean adding lane resolution to code nothing runs — risk with no behaviour. That's the same argument #3041 makes for *not* converting the other two dead sites; deleting is the version of it that also removes the literal. ## The gate it documents is not being deleted Its docblock describes the document/artifact visibility gate (VAL-CROSS-015). That gate is real and still enforced — by the inline conditions inside `listLiveTaskDocuments` and `listLiveArtifacts`, which is presumably why this helper was never wired up in the first place. Only the unused composition goes. ## Measured | check | result | |---|---| | SQL literal population | **22 → 21**; the gate ratcheted its own baseline down and asked for the commit, included here | | `taskstore-remaining.test.ts` (archive-lineage suite) | **27 tests green** | | six gates + `tsc` | green | ## Not deleted, deliberately `listLiveTaskDocuments` and `listLiveArtifacts` are referenced **only** by that test file. That's a weaker signal than zero references — someone may have written them ahead of a consumer. Their literals stay counted, which is the honest state for code whose intent I can't read from the repo. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
511f5b7e2b |
fix(core): archived tasks leaked into the live feed on a renamed board (#3041)
From my own #2839, re-measured today. Of that issue's SQL-literal sites, this is the one that decides what a **live view** shows. ## The defect `listTasksModifiedSinceImpl` backs the SSE watcher and modified-since polling — the incremental feed the dashboard applies to its task list. Its `includeArchived: false` branch excluded the literal `archived`: ```ts conditions.push(sql`${schema.project.tasks.column} != 'archived'`); ``` On a board whose archive lane is named anything else, that predicate matches **every** row and excludes nothing. Archived cards arrive in the live feed and reappear on the board. Nothing errors, and a full refetch filters archived rows by another path — so the symptom is archived work that comes back until the next reload. That gets reported as *"the board is flaky"*, not as a bug. ## The fix `resolveProjectColumnsForRoles` seeds the legacy ids before adding resolved ones, so the set is never empty and an **unconverted board excludes exactly `archived` as before**. The literal stays as the resolution-failure fallback, where excluding nothing would be worse than excluding the legacy id. ## Surface enumeration — three of four sites are dead Four sites share this invariant. Verified rather than assumed: | site | status | |---|---| | `reads.ts:558` (SSE / modified-since) | **live** — converted here | | `liveParentFilter` | **no references anywhere** in `packages/` or `plugins/` | | `listLiveTaskDocuments`, `listLiveArtifacts` | referenced **only** by `taskstore-remaining.test.ts` | That's why this PR converts one site rather than four — the other three are production-dead, and converting dead code would add risk for no behaviour. ## Measured | check | result | |---|---| | new PG suite | **4 cases** — legacy control, the renamed defect, a live-lane negative, and the forensic `includeArchived: true` read | | mutation (force the legacy fallback) | fails **exactly** the renamed case; the other three hold | | six gates + `tsc` | green | The negative case is the one that matters most: resolving the archive role must not start excluding **live** work, or the board silently stops updating for real tasks — a worse failure than the leak this fixes. ## One process note I corrupted this file mid-session by mutation-testing it while uncommitted: a failed restore left a half-applied block, and a later `git checkout --` discarded the fix entirely. Both were caught by re-grepping for the symbol rather than trusting the restore. The reliable pattern is **commit first, then mutate, then `git checkout` to restore** — which is how the proof above was actually run. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f8155cafd7 |
fix(cli): the node-override guard saw only the FIRST wip lane (#3023)
Follow-up to #3019, which merged with an incomplete fix. I found this while sitting down to write the test that PR was missing. ## The guard still never fired, one lane over #3019 wired `fn_task_update`'s guard like this: ```ts const nodeOverrideLifecycle = await resolveTaskLifecycleColumns(store, task.id); wipColumns: nodeOverrideLifecycle?.wip ? new Set([nodeOverrideLifecycle.wip]) : undefined, ``` `resolveTaskLifecycleColumns` → `resolveLifecycleColumns`, whose per-role accessor is **first match** (`workflow-lifecycle-traits.ts:353`): ```ts const first = (flag) => resolved.find((c) => c.flags[flag] === true)?.id; ``` The guard's contract is **every** column carrying the trait — its own resolver uses `columnsWithFlag(ir, "countsTowardWip")`. So on a board with a build lane beside a verify lane, a task sitting in the **second** wip lane still slipped the mid-flight check, and an operator could still repoint the node of a running task. That is the defect #3019 set out to close. Interchangeable on any single-wip-lane board, which is exactly why it read as correct — the same arity trap #2975 removed from the surfacing family. ## The fix Use `resolveNodeOverrideLanes`, the guard's own resolver, which `task-update.ts` and `branch-and-pr-entities.ts` already call. All three callers now resolve identically and the V1/unresolvable fallback lives in one place. Needed a one-line re-export from `@fusion/core`. **Mutation:** forcing the resolver to first-match (`.slice(0, 1)`) fails the new case, 1 of 32. The new test names **two** wip lanes, because that is the only shape that separates the two resolutions — a single-wip-lane test passes against both, which is why #3019's gap was invisible and why I would have written a useless test if I had not read the implementation first. ## A gate constraint worth recording My first version passed the resolved object straight through: ```ts validateNodeOverrideChange(task, normalizedNodeId ?? null, overrideLanes) ``` Identical at runtime, and it turned the lane-wiring gate **red**: `check-lane-wiring` matches an object-literal argument and cannot see through a variable, so the correct call reads as UNWIRED. #3019's header records hitting the same constraint — and it is what pushed that PR toward resolving the lanes inline, which is where the first-match bug entered. So the gate's shape requirement steered a correct instinct into a subtly wrong implementation. The fix here spells both keys explicitly, satisfying the gate without the bespoke resolution. Worth someone deciding whether the census should follow a variable to its initializer — but that is a change to a shared ratchet, and I have noted it at the call site rather than making it. **Verified:** 32/32 core guard suite, `tsc` 0 errors for both packages, lane-wiring gate exit 0, FNXC gate exit 0, lint clean. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
16921fc518 |
fix(engine,core): role resolution was half-done in two shared lifecycle predicates (surfacing family + file-scope leases) (#2975)
The three surfacing sweeps stopped reporting anything for a card resting in a board's **second** review or hold column. A lifecycle role is a **trait**, and any number of columns may carry it. The shared runner resolved it with `resolveLifecycleColumns()[role]` — **first match** — then gated on it: ```ts const roleColumn = lifecycle?.[spec.role]; // FIRST column carrying the trait if (task.column !== resolved.roleColumn) continue; // everything else dropped ``` A workflow that splits human sign-off from the merge lane has two review columns; one that parks dependency-blocked cards separately has two hold columns. Cards in the second got **no stale-paused-todo, no stale-paused-review, no in-review-stalled** diagnostic — silently, with no error, on all three sweeps at once. ## The second bug hiding inside the fix for the first Resolving membership but still reading `roleColumns[0]`'s declared `recovery` applies the **merge lane's** threshold to a card sitting in the **sign-off** lane. Each card's policy now comes from its own column, and one of the new cases fails if it doesn't: the first role column declares a policy that suppresses the signal, the card's own column declares one that fires. ## Reverted | | | |---|---| | **6 of 12** new cases fail | `fires for a card in the SECOND column carrying its role` and `reads the recovery policy of the card's OWN role column` — × 3 sweeps | | the other 6 pass either way | non-regression halves: still fires for the FIRST role column, still does **not** fire for a card outside every role column. Membership must widen the gate, not move it. | The pre-existing 45 cases were all green throughout — the single-role-column fixture could not express the case, which is why the table-driven file that exists to stop these three sweeps drifting apart never caught it. ## Verification `pnpm test:gate` 161 + 13 + 487 + 71 · surfacing family 57 · core stale-paused 20 · lint · census `--strict` · sql-literals · fnxc-dates · lane-wiring · changesets — all green. ## Note `holdColumns` was missing from the lane-wiring vocabulary, so the gate could not see that argument dropped. Added in the same commit. While reviewing, I found and measured **two problems in #2974** (comment posted there): six of its newly-visible sites are `satisfies`-wrapped false positives, and baselining them means deleting a real `reviewColumns` argument keeps the count unchanged and the gate green; and its baseline predates #2970, re-opening the slot that PR closed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved stale-card detection across all applicable review and hold columns. * Cards are now surfaced using the policies configured for their specific lifecycle column. * Cards outside matching lifecycle columns are no longer incorrectly surfaced. * Preserved existing fallback behavior when no lifecycle columns are configured. * **Tests** * Added coverage for workflows with split review and hold columns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- ## Second commit: the same predicate, half-converted (`shouldHoldActiveFileScopeLease`) Folded in here rather than stacked — same file, same class, and a stacked PR on an unmerged base is not mergeable. Reversible; say the word and I'll split it. `shouldHoldActiveFileScopeLease` is the **scheduler's** lease predicate, shared with the self-healing repair paths deliberately so the two cannot disagree about who holds a file-scope lease. Its two role answers are optional parameters defaulting to the legacy ids. The scheduler's own call sites were converted to pass resolved answers; self-healing's two were not: ```ts const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; ``` On a renamed board neither branch matches, so the predicate returns `false` for every card. The scheduler kept the lease; self-healing saw none, cleared `overlapBlockedBy`, and **released a dependent to edit files another agent still holds** — the outcome `groupOverlappingFiles` exists to prevent. Membership comes from the wip/review sets each sweep already resolved a few lines above, so this adds no reads. **Reverted:** both new cases fail with `overlapBlockedBy` = `null` — the release itself, not a proxy. The pre-existing legacy-column case in the same file passes either way, because `in-progress` satisfies the literal default; that is exactly why it never caught this. Lane-wiring baseline re-recorded `9 -> 7` in the same commit (the ratchet refused a stale allowance, as intended). **Verification:** gate 161 + 13 + 487 + 71 · surfacing 57 · overlap-seam + scheduler-lease + query-blindness 79 · core stale-paused 20 · lint · census `--strict` · sql-literals · fnxc-dates · changesets — green. |
||
|
|
2fd798cb36 |
core: every review card reported a false stall on a renamed board (#2970)
**The failure mode worth distinguishing: the rest of this family went
quiet on a renamed board. This one shouted.**
`getInReviewStallReason` satisfied its **own** lane check from
`context.reviewColumns` — then called `getTaskMergeBlocker` **without**
them. That helper re-ran its column-identity check against the literal
`in-review` and returned, for a perfectly healthy card:
```
task is in 'signoff', must be in 'in-review'
```
…which was surfaced as `{ code: "merge-blocker" }`. **Every in-review
card on a renamed board was flagged as stalled**, each citing a lane the
board does not have. That is how a signal stops being read at all.
## A second symptom, found by the revert rather than by reading
On a **genuinely failed** card, the identity message wins over the real
one. The operator saw the bogus column complaint instead of `task is
marked 'failed': merge verification failed`.
So it did not only invent stalls — it **masked the true reason for real
ones**. I would not have noticed that from the diff; it showed up
because the revert run asserted on the reason text.
## Same shape, last one in the family
The outer question was resolved and the inner one was not — the
half-conversion the helper's own comment records for `moves.ts`, and
#2963/#2964 fixed for the merge entry points. This is the last site the
audit turned up where the lane answer was already in scope and simply
not forwarded.
## Revert results
| | reverted → |
| --- | --- |
| the unforwarded call (what ships today) | **2 of 3 fail** — healthy
card reports a merge-blocker stall; failed card reports the wrong reason
|
**Fixture note worth keeping:** `paused` is deliberately *not* the
genuine-stall case. An earlier guard returns `undefined` for a paused
card before the merge blocker is ever consulted, so that case would pass
whether or not the lanes are forwarded — the vacuous shape this series
has produced eight times.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71; `@fusion/core` full suite **4878
passed** (457 files); `tsc` core clean; lint, lifecycle census
`--strict`, FNXC gate, changesets all clean.
|
||
|
|
126cee7e6d |
engine: finalization parked ALREADY-MERGED work as failed on a renamed board (#2964)
**The worst symptom in this family: the branch landed, and the board says the task failed.** `project-engine`'s merge-confirmed finalization spread the task's **real** column into `getTaskHardMergeBlocker` with no `reviewColumns`, so the identity check ran against the literal `in-review`. On a renamed board it returned `task is in 'signoff', must be in 'in-review'`, and the caller parked the card: ``` status: "failed" error: "Merge confirmed but finalization blocked: task is in 'signoff', must be in 'in-review'" ``` For work that had already merged. ## Its sibling had already solved this `auto-merge-finalization.ts` passes the **review-eligible sentinel** instead of the card's own column, with the reasoning recorded at that site: `getTaskHardMergeBlocker` asks *"is this card blocked by anything other than where it sits?"*, and its callers are recovery paths for landed work that a graph crash can leave resting in any column. `project-engine` simply never got the same treatment. ## One name instead of two spellings Rather than write the sentinel a second time, it is exported once as `REVIEW_ELIGIBLE_SENTINEL_COLUMN` next to the helper whose contract gives it meaning, and both recovery paths use it. **Two sites independently spelling a magic value is how one of them came to be missing it** — that is the actual root cause here, not the literal itself. This also answers the census, which flagged the new literal — correctly. Its guidance (which I wrote, in #2909) is to hoist a deliberate literal into a *declaration*, where a `DELIBERATE-LITERAL` marker actually attaches, instead of leaving it mid-expression where the marker is silently ignored. The shared constant is exactly that, and it lowers `auto-merge-finalization`'s literal count too. ## Revert result | | reverted → | | --- | --- | | sentinel replaced by the card's own renamed column | reproduces the shipped string | The middle test asserts that string deliberately — it is what landed in `task.error`, so a regression reports what the operator would actually have seen. A third case checks the sentinel does **not** suppress genuine blockers: incomplete steps still block finalization in any lane. These drive the helper directly; reaching `project-engine`'s finalization end to end needs a live engine, a merge run and a real repo, while the defect is entirely in *what the blocker is asked*. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `project-engine` + `auto-merge-finalization` + the new suite, 207; `tsc` clean on core and engine; lint, census `--strict`, FNXC gate, changesets all clean. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed merge-confirmed tasks being finalized correctly when boards use renamed workflow columns. * Prevented already-merged tasks from being incorrectly marked as failed due to custom review-column names. * Preserved enforcement of genuine incomplete-step blockers. * **Tests** * Added coverage for finalization on renamed lanes and legitimate merge blockers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
dd930c8d7d |
fix(cli): qualify cross-fork PR heads (#2377)
## Summary - resolve the repository receiving pushes through `git remote get-url --push origin` - qualify pull-request head branches with the fork owner when the push owner differs from upstream - preserve the existing unqualified head for same-repository workflows ## Root cause Fusion correctly resolved the PR target from origin's fetch URL, but assumed the pushed branch lived in that same repository. With an upstream fetch URL and a fork push URL, GitHub requires `fork-owner:branch`; the unqualified branch is rejected. ## Validation - CLI task lifecycle tests: 48 passed - `@fusion/core` typecheck - `@runfusion/fusion` typecheck - strict changeset validation <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Pull requests created from branches pushed to contributor forks now correctly qualify the PR head with the fork owner when the push remote differs from the upstream owner. * Improved PR head handling across both group/shared-branch and per-task pull request creation paths. * **Tests** * Updated and expanded lifecycle tests to cover “origin push to fork” scenarios using push URL–based repo resolution. * **Documentation** * Added a patch release note for the fork-aware PR head fix. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: v <v@v.speedport.ip> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> |
||
|
|
189f237a07 |
fix(security): make the approval and permission gates actually enforce (#2457)
## Why
An AI agent autonomously deleted a live task (FN-8609) via
`fn_task_delete`. Investigating that led to two audits, which found the
gate machinery is well-built and its *enforcement* well-tested — but the
*decision boundary* was unguarded and the tools that matter never
reached the gate at all.
The headline defect: all ~76 tools in `packages/cli/src/extension.ts`
are delivered through `hostExtensionPaths` → `DefaultResourceLoader`,
which never passes through `wrapToolsWithActionGate`. `fn_task_delete`
*was* classified as gated in `gating-classifications.ts` — that
classification was dead code. Roughly 15 destructive tools were ungated
even for an operator who had locked their policy all the way down.
Second: `POST /api/approvals/:id/decision` took the approver identity
from the request body, validated for shape only, with no check that the
decider was human or differed from the requester. Combined with a bash
tool whose command string was never inspected (only `params.cwd`), an
agent could read the daemon token from `~/.fusion/settings.json`, find
its own pending request, and approve it.
## Scope
This does **not** tighten defaults. The `unrestricted` preset is
byte-for-byte unchanged — `git diff` on `agent-permission-policy.ts` is
empty — and regression tests assert that an out-of-the-box install
behaves exactly as before. The bug was never "the default is too
permissive"; it was "strict policy doesn't enforce." This makes turning
security up actually work.
The one deliberate exception: the containment that stops an agent
escalating its *own* privileges (reading the daemon token / credentials,
calling the approvals API to self-approve) applies at every preset
including `unrestricted`. That is a privilege-escalation boundary rather
than a permission preference — if it only engaged under strict policy it
would not have prevented the incident that prompted this.
## What changed
8 bisectable commits:
- **Approval lifecycle** — self-approval blocked via server-derived
deciders; same-verdict replay 409s; decide re-reads and re-validates
inside the transaction; expiry TTLs; `markCompleted` ownership check;
session identity registry in core.
- **Engine gates enforce for real** — unclassified tools resolve to a
policy-governed category instead of hardcoded `allow`; missing-policy
fail-open closed; bash containment floor + exact-command approval
binding.
- **Dashboard decision routes** — stop trusting client-supplied actors
(decision, bypass-review, worktrunk → 403 on forged actors).
- **`fn serve` authenticated by default** — auto-mints a token following
the existing `fn dashboard` precedent; `--no-auth` opts out.
- **Sibling entry points closed** — user-sourced hard-cancel moves, ACP
execute-once approvals, plugin task-store gating.
- **pi-extension principal resolution** — the extension resolves the
acting principal and can withhold or policy-gate the previously ungated
destructive tools.
- **Root-cause bonus fix** — `findLatestByDedupeKey` was broken in
PostgreSQL backend mode (already-parsed jsonb fed through a string-only
parser), so approved-grant redemption **never matched in production**,
minting duplicate requests. This explains the live DB state of 17
approved / 0 completed. *(Also cherry-picked to `main` as `a9b30013bb`,
since it is an active production defect on its own.)*
- **Review follow-ups** (`627f1b1fa8`) — operator-configured
provisioning privilege and a configurable grant TTL; see below.
## Review follow-ups
**Provisioning privilege is operator-configured, not role-derived.**
`isCallerPrivileged` had gone from `caller.reportsTo == null` (every
top-level agent privileged — permanent escalation by creating a
manager-less agent) to `caller.role === "ceo"`, which swapped an
implicit rule for a magic string: any agent config can claim that role,
while an operator who genuinely wants a privileged agent had no
supported way to say so. Privilege now derives solely from
`agentProvisioning.trustedAgentIds` / `trustedRoles` and fails closed
when settings are unresolvable.
It is also no longer forwarded to `resolveAgentProvisioningPolicy` as
`isPrivileged`, because that flag short-circuits ahead of
`alwaysApproveDelete` — a trusted caller was bypassing delete approval
entirely. The policy applies the same trusted rules itself, in the right
order. The function now governs only the org-chart escape hatch (acting
outside your own direct reports).
**Grant TTL defaults to 1 hour and is configurable.** Approval →
redemption is not instantaneous: an operator approving from their phone,
an engine restart, a queued lane, or a task waiting on a worktree all
routinely exceeded 15 minutes, after which the grant expired and the
agent silently re-requested. One hour remains far short of the
"redeemable forever" hazard the TTL exists to bound. Override via
`FUSION_APPROVAL_GRANT_TTL_MS` or `configureApprovalRequestTtls()`;
invalid overrides are ignored rather than widening the window to
infinity or collapsing it to zero.
## Behavior changes requiring operator review before rollout
1. `fn serve` requires a bearer token by default (`--no-auth` opts out);
unauthenticated clients get 401.
2. Agents can no longer run withheld destructive tools
(`fn_task_delete`, `fn_task_bypass_review`,
mission/milestone/slice/feature/workflow deletes, `experiment_finalize`,
`skills_install`). Operators keep them via CLI/dashboard. **This is the
incident fix.**
3. Agents get provisioning privilege only when the operator lists them
in `agentProvisioning.trustedAgentIds` / `trustedRoles`; the
provisioning gate is now live in production. Previously-implicit
privilege (top-level position, or a `ceo` role) no longer grants
anything on its own.
4. Decision replay 409s (was 200); pending approvals expire after 24h,
approved grants after 1h (configurable); bash approvals bind per exact
command.
5. Forged/body actors on decision, bypass-review, worktrunk routes →
403; `archive-all-done` requires `{confirm:true}` (external scripts
affected).
6. `fn_secret_get` approvals grant exactly one reveal (previously
granted nothing and looped forever); ACP approvals are execute-once
(previously infinite reuse).
7. Bash containment denies token/credential/approvals-API commands in
all agent sessions at every preset.
## Verification
Independently re-run against the branch, not just self-reported:
- 5 typechecks (core, engine, cli, dashboard `tsconfig.json` +
`tsconfig.app.json`) — clean
- `pnpm lint` — clean
- `pnpm test:gate` — 379 passed
- `pnpm build --force` — green (a plain `pnpm build` skips packages as
unchanged and does **not** compile the branch)
- `pnpm check:changesets` — clean
- ~650 file-scoped tests including new negative-path suites for the
decision boundary, which previously had **zero** test coverage
`packages/engine/src/__tests__/plugin-runner.test.ts` fails 56/80 —
**verified pre-existing**, reproducing identically at base commit
`93a403af67` on `main`. Not in the merge gate.
### A mutation check that failed to fail
Worth recording, because it nearly shipped an untested security fix. The
first mutation check on the provisioning change reintroduced the `ceo`
hardcode and **all 17 tests still passed** — the tests asserted through
the policy path, which can no longer observe `isCallerPrivileged` at
all, precisely because `isPrivileged` is no longer forwarded there.
Org-chart cases that do exercise the function were added; the hardcode
now fails exactly 1 of 19, and restoring is green. A green mutation run
is only meaningful if the test can actually see the code under test.
## Known limitations (stated, not papered over)
- The bash containment floor is string-matching: a cost-raiser, not a
sandbox. Quoting, encoding, `$HOME`, symlinks, or an interpreter
one-liner can evade it. The durable protection is the decision route
refusing agent-originated deciders — the filter is the belt, not the
braces.
- Approval expiry is lazy (evaluated at decide/complete/redeem), not
swept, so an expired pending row stays visible in lists until touched.
- The extension's require-approval path returns a pending message but
cannot suspend a pi session mid-turn; engine-side pause hooks cover
engine lanes only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Security**
* Hardened approval and permission gating with server-side decider
attribution, self-approval blocking, ownership checks, replay/race
protection, and status/TTL enforcement.
* Added fail-closed behavior for sensitive/unclassified tools and
sandbox provisioning approvals.
* Blocked credential/approval access via bash containment; plugin
destructive task operations now require explicit permission.
* **New Features**
* `fn serve` now defaults to bearer-token auth, with `--no-auth` as the
explicit opt-out.
* **Bug Fixes**
* Improved task move-source attribution (`moveSource: "user"`) and
tightened dashboard archive/bypass confirmation and operator attribution
behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
fd795883c5 |
feat(missions): per-mission taskPrefix override for triaged task ids (#2347)
## Summary Maintainer re-land of [#2334](https://github.com/Runfusion/Fusion/pull/2334) (fork `flexi767:feat/per-mission-task-prefix`) after resolving merge conflicts with current `main`. Fork push was unavailable despite `maintainerCanModify`, so this branch carries the conflict resolution. ### Feature - Optional per-mission `taskPrefix` for triaged task ids (inherits project prefix when unset) - Dashboard MissionManager + routes + store/triage plumbing - Postgres migration for `project.missions.task_prefix` ### Conflict resolution - Main claimed migration **0026** (bigint counters) and **0027** (workflow IR pin) - Mission task-prefix migration renumbered **0026 → 0028** - Baseline `0000_initial.sql` includes `task_prefix` on missions - `legacy.ts` keeps code-org re-exports; `missions.ts` carries `taskPrefix` on create/update types ## Test plan - [ ] CI green (lint/typecheck/build/gate) - [ ] Create mission with custom prefix; triage feature → task ids use that prefix - [ ] Clear mission prefix via PATCH null; new tasks inherit project prefix Closes / supersedes #2334 once this lands (or re-point the fork PR). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Missions can now set an optional per-mission task ID prefix (overriding the project default). * Added task prefix support to mission create/edit UI and dashboard APIs, including normalized uppercase values and validation. * **Bug Fixes** * Improved commit hook generation for custom prefixes and special characters, with safer shell handling to prevent unsafe interpretation. * **Chores** * Added PostgreSQL migration and schema-applier support to persist and propagate mission task prefixes, including upgrade/backfill coverage. * **Tests** * Added backend and UI/API test coverage for task-prefix creation, clearing, and ID minting behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
6c1f074773 |
fix(core): the in-review stall signal never got the board's review lanes — 0 of 4 call sites (#2956)
## Main is red, and the red is pointing at a real defect `unwired-lane-parameter-guard` fails on `origin/main` after #2951. This is not a stale allow-list — the parameter genuinely never reaches the function. #2951 added `reviewColumns?: ReadonlySet<string>` to three signal modules and wired two of them completely. **`getInReviewStallReason` was wired at none of its four call sites.** Measured by brace-matching each call's option literal: ``` getInReviewStallReason L227=NO L390=NO L599=NO L729=NO getInReviewStalledSignal all 4 wired getStalePausedReviewSignal both wired ``` ## The user-visible consequence `reads.ts` computes two adjacent signals for the same card. On a board declaring a **separate merge lane beside its human-review lane**, `inReviewStall` read the *first* review column only, while `inReviewStalled` — three lines below — read the *set*. **The same card is "in review" for one signal and not the other.** Two signals disagreeing is worse than both being legacy, and it is invisible on every builtin board because there the review set has exactly one element. At three of the four sites the resolve sat *below* the call, which is why the parameter could not be passed. Those are hoisted. ## I have to correct my own earlier report On #2951 I said *"3 of 4 call sites wired, `reads.ts:227` is the gap."* **That was wrong.** I had measured with a 12-line proximity grep, which bled into the adjacent `getInReviewStalledSignal` call and counted its `reviewColumns:` as the first call's. Brace-matching the literal shows 0 of 4. The defect was four times larger than I reported, and the cause was exactly the anti-pattern I have spent this session filing against other people's guards — a proximity window standing in for structure. ## Naming the context types The guard keys an interface member to its **owner symbol** and only counts a mention from a file that also names that owner, so passing the property inline reads as unwired even when every site supplies it. `satisfies InReviewStalledContext` / `satisfies StalePausedReviewContext` on the option literals is real type-checking, not a decorative import — lint rejected the decorative version, correctly. ## New test, because the existing guard cannot see this Measured: **deleting the `reviewColumns:` line from a fixed call site leaves `unwired-lane-parameter-guard` at 9/9 green**, because the file still names the type. So the wiring I just fixed had no coverage at all. The new ratchet brace-matches each call site's option literal: | mutation | result | |---|---| | remove lanes from one call site | **1 failed** — *"1 of 4 getInReviewStallReason call sites omit reviewColumns"* | It also asserts it **found** call sites before checking them — a parse that matched nothing would be vacuous, which is the failure mode this guard family keeps producing. (It caught me mid-change too: an earlier scripted edit left the file syntactically invalid and the source-text test still passed 3/3. It is a wiring ratchet, not a substitute for `tsc`.) ## Verification Core **4861 passed / 0 failed** · guard **9/9** with `KNOWN_UNWIRED` **unchanged** · `pnpm test:gate` **exit 0** · lint clean · core `tsc` **0 errors**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8e5e1147d2 |
core,engine: the last literal lifecycle query — and the three stall signals that disagreed (#2951)
**This is the last one.** `surfaceInReviewStalls` was the final literal
`listTasks({ column })` in production — I verified it by direct scan,
not by census arithmetic: **1 remaining before this, 0 after.**
It tells an operator that a card is stalled in review. On a renamed
board the stall was real and the board simply never said so.
## It came last on purpose
Converting the read alone would have been **worse than leaving it**.
`getInReviewStallReason` gated on the literal `in-review` itself, so a
widened read hands every renamed-board card to a classifier that drops
it — the missed-pair class, wearing the shape of a clean one-line
conversion.
## What was actually there
Three sibling signals decorate the same row, and they **disagreed about
which lane it is in**:
| signal | before |
| --- | --- |
| `getInReviewStalledSignal` | singular `reviewColumn` — resolved, but
**first-per-role** |
| `getStalePausedReviewSignal` | singular `reviewColumn` — same |
| `getInReviewStallReason` | **no seam at all** — literal |
So one row could be judged in-review by one signal and not by another.
And the singular ones are the **arity trap**:
`resolveLifecycleColumns().review` is the *first* column carrying a
review role, so a board with a separate merge lane beside its
human-review lane had a second review column matching none of them.
All three now take `reviewColumns` (membership), resolved **once per
row** through `resolveReviewColumns` — the union of the three review
roles — so they cannot disagree by construction. The singular/literal
paths remain as the no-metadata fallback, so a caller passing nothing is
byte-identical to today. Ten call sites in `reads.ts` wired from that
one answer; the singular resolver is deleted.
## Revert results
Each applied alone and re-run:
| conversion | reverted → |
| --- | --- |
| the resolved read | fails — the card is never listed |
| `reviewColumns` at the call | fails — the classifier drops the renamed
card the widened read just found |
That second row is the whole point: it proves the pair had to move
together, which is the thing I got wrong twice earlier in this series.
## Second commit: a red on `main`, not from this branch
`check-fnxc-future-dates` landed and **`main` fails it** — verified by
running the script on a clean `origin/main` checkout rather than
inferring. Nine files carry stamps dated after today, so every worker's
gate fails on a check none of their changes caused. Several are mine: I
had been stamping tomorrow's date across this whole series, which is
precisely the out-of-order record the check exists to prevent.
Scope held deliberately: a repo-wide sweep touched **266 files** across
docs, scripts and every package. I ran it, backed it out, and limited
this to the nine files the check actually flags — a mechanical rewrite
that size during a queue freeze would conflict with every in-flight
branch, which is worse than the red it fixes.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71 (green **only** with the stamp
commit); `@fusion/core` full suite **4810 passed**; engine self-healing
+ blindness + both ratchets **758 passed**; `tsc` clean on core and
engine; `pnpm lint`, `check:changesets`, `lifecycle-column-census
--strict`, `check-sql-column-literals` and `check-fnxc-future-dates` all
clean.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Review-stall detection now recognizes renamed and multiple review
columns while retaining support for the legacy review column.
- Paused tasks continue to be excluded from stall detection.
- Self-healing review-stall sweeps now search all configured review
lanes and avoid duplicate task results.
- **Tests**
- Added regression coverage for renamed and legacy review-lane queries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
f49e487d91 |
feat(core): untraited-project lane opt-in — and main was red on the FNXC gate (#2949)
Two things, and the second is why the first does not ship alone. ## The opt-in `resolveProjectColumnsForRoles` gains `untraitedProject: "declared-columns"`. When **no** workflow in the project expresses **any** lifecycle trait, every declared column id joins the answer. This is the three-state rule at **project** scope — the last item on the deferred list, recorded at three self-healing call sites (#2869, #2876). A board that renames its lanes and declares no traits contributes nothing today, so its cards are **absent from every role-keyed query**, and the correct per-card fallback downstream never runs for them. A fallback cannot rescue a card the query never returned. **Not "no workflow declares this role."** A project that expresses traits and has no review lane has *answered*; widening there would invent lanes it deliberately lacks. Mutation-verified both directions — widening unconditionally fails 1 of 12, making the option a no-op fails 1 of 12. **Opt-in, not default**, because the safe direction differs per caller — the finding in `project-union-versus-per-task-lanes.md`: | caller | over-inclusion costs | |---|---| | sweep | nothing — the per-card check discards the extra rows | | aggregator | an inflated number an operator reads (#2864, #2866) | | action site | a card routed or notified under a vocabulary that is not its own (#2852, #2891) | Making it the default moves all three at once, in the one direction two of them must not. Verified byte-identical without the option, so this lands with **no caller changes** and each site adopts it on its own reasoning. ## Main was red, and my own gate caught me first I dated the new comments `2026-07-31` while today is `2026-07-30` — **the exact defect `check-fnxc-future-dates` exists to prevent, committed while writing the feature.** The gate I added yesterday failed my own commit. Correcting mine surfaced that the merged sentinel batch, #2947, and three engine test files carried future-dated stamps too, so **the gate was failing on `main` for everyone**, not just here. All corrected to real dates rather than raising the ceiling. The stamps were simply wrong, and a baseline bump would have recorded the error as permitted — which is the failure mode that ratchet exists to prevent. Core and engine `tsc` clean, `pnpm lint` clean, census `--strict` 0, FNXC gate 0 (469 known, none added), gate green (161/487/13/71). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b1bd571682 |
batch-sql-ratchet: the census / gate-ratchet family — collection branch, fold here (#2941)
## Family branch for consolidation directive item 4 `batch-sql-ratchet` did not exist and ~10 open PRs are waiting for a collection point, so this establishes it. **Fold your census/ratchet commit here and close your own PR as superseded.** ```bash git fetch origin batch-sql-ratchet git checkout -B batch-sql-ratchet origin/batch-sql-ratchet git cherry-pick <your-sha> # verify scoped, not full suite: pnpm --filter @fusion/core exec vitest run src/__tests__/archived-column-gate-parity.test.ts --silent=passed-only --reporter=dot git push origin HEAD:batch-sql-ratchet ``` **Candidates I can see open right now** (owners: please fold + close): | PR | branch | |---|---| | #2938 | `fix/comments-ops-sentinel` | | #2935 | `fix/task-artifacts-sentinels` | | #2933 | `chore/commit-tightened-census-baseline` | | #2931 | `fix/async-comments-sentinels` | | #2928 | `fix/audit-ops-sentinel-marker` | | #2925 | `live-task-column-lanes` | | #2923 | `fix/task-id-integrity-sentinel` | | #2921 | `fix/plugin-store-migration-marker` | | #2894 | `gate/sql-literals-match-census-placement` | That is **10 → 1** once folded. I have not cherry-picked anyone else's commits — folding someone's work without them verifying it is how a batch lands broken. --- ## What is in it so far (mine, from #2924) **Clears a live main red:** `archived-column-gate-parity` fails on `origin/main` today. ``` AssertionError: TypeScript encoding changed. async-comments-attachments.ts: 8 → 5 ``` #2886 fixed a real bug — archived-document guards failing in *opposite* directions on a renamed lane — by replacing three `column === "archived"` comparisons with `isArchivedLane(column, archivedColumns)`. The AST scan counts raw comparisons, so the tally dropped. **What I did not do is record it as three sites converted**, because measured, it is not: ``` grep -rn "archivedColumns:" packages/core/src packages/engine/src --include="*.ts" | grep -v __tests__ → (no matches) ``` No caller passes it. The parameter defaults to `LEGACY_ARCHIVED_LANES = new Set(["archived"])`, so every call resolves to the literal it replaced — byte-identical behaviour, resolved branch dead. That matters for this guard's whole argument: its header warns that converting the TypeScript half while the Drizzle and raw-`sql` halves still compare the string is a split brain *"no test would catch, because every builtin workflow spells the column `archived` so the two halves agree by accident on every board we ship."* **There is no split brain today precisely because the resolved half is unwired** — it becomes one the moment a caller threads real lanes in without the SQL sides moving. Recorded inline so `5` cannot be read as "3 sites done"; flagged on #2886. Verified not a split brain: the Drizzle and raw-sql inventories are unchanged and both pass — worth stating because those assertions run *after* the TypeScript one, so a plain red says nothing about them. Scoped edit to `AUDITED_TS_SITES` by line range: these paths appear in more than one inventory here, and an unscoped replace would quietly edit the raw-sql side too, making the parity guard agree with itself (the trap I hit in #2817). Guard still bites: appending a real `task.column === "archived"` to an audited file fails it. Core **4852 passed / 0 failed**, lint clean, test-only. Closing #2924 as superseded by this. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved task delegation messages when workflow pickup cannot be confirmed. * Delegation results now clearly indicate when a task has not been verified for pickup. * **Quality Improvements** * Added validation checks to catch future-dated markers and inconsistent SQL-column usage. * Refined workflow checks to distinguish stale configuration from incomplete configuration. * **Documentation** * Updated lifecycle conversion guidance with more accurate audit findings and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8503a2b12f |
batch-census-sentinels: six sentinel-marker PRs in one (supersedes #2921 #2928 #2931 #2935 #2938 +1) (#2943)
Fifth family, not in the four you listed — it was about to sit while the others consolidated. **Six folded; two need arbitration.** ## Folded (cherry-picked clean) migration marker · async archived check · audited-sentinel missing its marker · five of six `archived` checks in one file · the two artifact/comment read-only guards · the last unmarked `getLiveTaskColumn` sentinel. One root cause, which is why they belong together: **a literal compared against a SENTINEL value is not a lifecycle-lane guard** — the census counts it, and the fix is a marker, not a conversion. ## The baseline conflicted on every cherry-pick All six re-recorded `lifecycle-column-census-baseline.json` independently. I resolved by **regenerating once from the folded tree** rather than merging six hand-edits: the baseline is a derived artifact, so the measured value is the only correct resolution, and hand-merging derived JSON is how a wrong ceiling gets locked in. That is the strongest case for the family model I can give you: six PRs touching one derived file conflict pairwise regardless of merge order — 15 possible pairs — and auto-rebase would have churned them serially. ## NOT folded — one line for arbitration **#2925 (`live-task-column-lanes`) conflicts with #2923 (`fix/task-id-integrity-sentinel`) on `packages/core/src/task-store/task-id-integrity.ts`.** #2923 marks a sentinel there; #2925 converts lanes. Different intents, same file. I did not guess which wins — land one, rebase the other, fold both after. ## Verification `--strict` exit 0 · backlog **158**, reviewed **122** · core typecheck clean · scoped, not full suite. ## Queue **52 → 39** after my two folds (this + #2940 portal). The ~24 "self-healing … on a renamed board" family is still the dominant block. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified lifecycle-state terminology and migration markers throughout task and project management documentation. * Documented the distinction between archived-task sentinels and workflow column identifiers. * Updated lifecycle documentation tracking to reflect the latest coverage. * **Bug Fixes** * No runtime behavior changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7b68f20501 |
batch(docs): fold the three workflow-learnings / annotation PRs into one (#2942)
## Family batch — replaces #2926, #2892, #2887 Per the consolidation directive: the u9/e2e **docs family**, folded into one branch and one CI run. Three PRs, five commits, **five files, comment and markdown only**. | folded PR | commits | |---|---| | #2892 `docs/union-vs-per-task` | the project union and the per-task answer are not ranked; date correction | | #2926 `docs/date-my-measured-claims` | date the measured claims (one was wrong); date the grep-vs-AST measurement in the SQL gate header | | #2887 `docs/archived-state-literals` | mark the three archived STATE literals as deliberate | Cherry-picked in original order with authorship preserved; all five applied clean, no conflicts. ## Scope is provably comment-only ``` docs/solutions/workflow-learnings/lifecycle-conversions-that-score-as-wins.md docs/solutions/workflow-learnings/project-union-versus-per-task-lanes.md packages/core/src/task-store/async-maintenance.ts ← FNXC DELIBERATE-LITERAL annotation packages/core/src/task-store/workflow-definitions.ts ← FNXC DELIBERATE-LITERAL annotation scripts/check-sql-column-literals.mjs ← header prose only ``` Every added line in `packages/` and `scripts/` is inside a comment — checked by filtering the diff for declarations, conditionals and returns, which returns nothing. The two core files gain `DELIBERATE-LITERAL` markers explaining that `'archived'` is a **state** marker there, not a lane: the sweep collects rows Fusion itself archived or soft-deleted, so widening to the resolved archived set would pull live cards into a cleanup pass. ## Verification (scoped, per the directive — not the full suite) - `pnpm lint` — clean - `check-sql-column-literals` — exit 0 (the file it annotates) - `check:lifecycle-columns` — exit 0 (the markers it adds are census-visible) - `sync-workflow-ir-callsite-allowlist.test.ts` — 3/3 ## A correction worth recording Mid-fold I saw a changeset, `self-healing.ts` and a test file in `git diff origin/main..HEAD` and nearly reported the batch as impure. They were **main's own commits** — `origin/main` advanced between branch creation and the diff, so the comparison was against a stale base. Rebasing onto current `main` reduced it to the five files above. Worth flagging for anyone else folding a family today: with `main` moving this fast, diff the branch **after** rebasing or the file list will lie to you. ## Closing the originals #2926, #2892 and #2887 are superseded by this and are being closed. I hold no PRs of my own in this family — all mine merged — so this fold is on behalf of the family rather than a rollup of my own work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b4ed12e9c8 |
batch-u7-lane-fixes: three core/engine renamed-board fixes folded (was #2925, #2930, #2936) (#2925)
**Consolidated per the queue freeze.** Three single-fix PRs of mine folded into this one branch; #2930 and #2936 are closed as superseded. Net effect on the queue: **3 → 1**. All three are the same root cause — a lifecycle lane compared against a legacy id — and all three carry a measured revert proof. Verified scoped (not full suite) on the folded branch: `tsc --noEmit` clean, `pnpm lint` clean, SQL-literal gate green, census `--strict` green, and 61 tests across five suites plus the guard at 9/9. --- ### 1. `getLiveTaskColumn` produced the archived sentinel from a literal (was #2925) `getLiveTaskColumn` **manufactures** the string `"archived"` that a dozen comparisons across five files trust — and it tested `row.column === "archived"`. A live row in a renamed archived lane read as **live**, so the gates hiding an archived card's artifacts and document listings never closed. Fixing those twelve comparisons individually would have been wrong twice over: **they are sentinels, and the defect was in the producer.** One line, once, and all twelve become correct. `resolveArchivedLanes` moved to `project-lane-vocabulary.ts` — three private copies of one fact is how the "write guard says yes, publication guard says no" disagreement happens at scale. *Revert proof (real PostgreSQL):* restore the literal → `expected [ { …(14) } ] to deeply equal []`. **Caught myself shipping the unwired shape here:** I added the parameter to seven functions and wired none of their impl callers — the exact inert-conversion defect this program exists to remove. The failing test is the only reason I noticed. ### 2. Mission delivery repair refused a completed card (was #2930) `getTerminalTaskEvidence` tested only `column === "done"`, so a completed card on a renamed board classified as `nonterminal` and `reconcileFeatureDoneWithTerminalTask` threw `TASK_NOT_TERMINAL: … not shipped`. Valid operator work refused — with the message naming the real column while the check couldn't see it. The **type** blocked the fix from the far end: `TerminalTaskEvidence` pinned `column: "done"` / `"archived"`, so the resolver couldn't report the real column without a compile error. `kind` already carries the role, so `column` is free to carry the truth. *Revert proof (real PostgreSQL):* restore the literal → `TerminalTaskReconciliationError: … not shipped`. I had deferred this twice on the premise that `AsyncMissionStore` "holds a layer, not a store". It holds an **optional `taskStore`**, and the single production construction site supplies it. ### 3. The unwired-lane guard reported two FALSE entries (was #2936) `unwired-lane-parameter-guard.test.ts` has been **red on main** since #2875, flagging two `InReviewDurationLanes` properties as unwired when the impl demonstrably supplies both. Cause: my own owner-scoping rule requires a mention from a file naming the declaring symbol — correct for a function, structurally impossible for an interface passed as an inferred object literal. Fixed at the caller (name the type) after trying the tool three ways: relaxing type-owned properties hid **12** genuine entries; resolving owners to consuming functions hid **6**. Each refinement traded the false positive for false negatives — the sign a co-occurrence heuristic has hit its limit. Recording two *wired* parameters in `KNOWN_UNWIRED` was rejected: that puts non-debt in the debt list, which is how a ratchet starts lying. Guard back to **9/9**, baseline unchanged at 17. **This un-reds main.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
aacd18e847 |
docs(lanes): audit the last three files the census points at with no reason attached (#2908)
Second pass of #2873's sweep, over the files that still carry lifecycle guards and **zero** audit notes. No source change — every literal stays counted, none gets an exemption marker. ## `project-store-ops.ts` (1) — dead sync path, do **not** convert The literal would leak a merge-queue entry on a renamed board: a card leaving review would never be dequeued. Except the function cannot run — it reaches for `store.db.prepare`, which throws in PostgreSQL backend mode. The live path is `dequeueMergeQueueOnColumnExitInTransaction` (`async-merge-coordination.ts`, called from `moves.ts`), and it is **already converted** — it takes `moveReviewColumns` and the caller supplies them. Recorded so the census entry is not mistaken for unconverted debt, and so it can be deleted alongside the rest of the sync SQLite residue. ## `task-id-integrity.ts` (2) — one real, one sentinel, and the real one must not go alone ```ts return cached?.column === "archived"; // ← board lane: real if (live === "archived") return true; // ← getLiveTaskColumn's manufactured value: sentinel ``` Converting the first while `getLiveTaskColumn` still keys on the literal would leave the two disagreeing about what "archived" means. It waits for that one, which is the single highest-leverage line in this cluster — fixing it makes five downstream sentinel checks correct without touching any of them. ## `auto-merge-finalization.ts` (3) — one real but diagnostic-only, two non-defects `task.column === "done"` selects which **reason string** is reported; both arms return `{ ok: false }`. So a renamed board is refused with the generic `missing-merge-confirmation` instead of the specific `done-without-merge-confirmation`. Real, and worth less than the signature change required to fix it — the resolver two functions up already computes `isCompleteColumn`, but this function does not receive it. The other two are **not** defects and it is worth saying so explicitly: the `columnId === "done"` near the top is the resolver's documented degraded fallback (the live arm calls `columnHasFlag`), and the `step.status` comparison is a **step status**, not a column. ## The pattern across both passes Of **6 files and 15 guards** audited: **2** were live defects worth converting, **4** were sentinels or dead paths that would have *broken* a renamed board if converted, and the rest were diagnostics or misfiled step statuses. That ratio is the argument for these notes existing. A file's census count is an upper bound on convertible sites, not a work estimate — and in this cluster the naive reading of the number would have made things worse more often than better. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`, `@fusion/engine`) — clean - census `--strict` — exit 0, counts unchanged (that is the point) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
60bfebdc98 |
fix(reliability): the duration query hid its lane ids inside a SQL template (#2875)
The Reliability panel's **third and last** blind input — and my own loose end. #2861 fixed the two counts beside it, so the panel went from uniformly wrong to **partially** wrong: entries and bounces populated, duration reporting `no-in-review-entries` forever. Partial blindness is harder to notice than total, which is why finishing it matters more than one site suggests. ```sql metadata->>'to' = 'in-review' OR (metadata->>'from' = 'in-review' AND metadata->>'to' = 'done') ``` ## The class, not just the site **This shape is invisible to every check we have.** The lifecycle census scans `===`/`!==` comparisons; the unwired-lane-parameter guard scans declarations. Neither sees a lane id inside a `sql` template, so this class is **not in the backlog total at all** — the number is a floor for this reason as well as the usual one. `scripts/check-sql-column-literals.mjs` (#2841, in flight) is the detector for exactly this: it freezes the surface at 30 sites rather than converting any, so this one was unowned. That PR and this one are complementary — it stops the surface growing, this shrinks it by one. ## The fix Lanes resolve **once per call** via `resolveProjectColumnsForRoles` and arrive as parameterised equality fragments, one branch per id — no interpolated list, no string building. Resolution lives in `getInReviewDurationEventsImpl` because that is where the store is; `async-audit.ts` takes a bare `db` handle and cannot resolve anything. Best-effort, defaulting to the legacy pair, so a caller that cannot resolve keeps exactly today's query. **The union is correct rather than a widening hack**, for the same reason as #2861: these are *move records*, and a past move recorded the column name as it was at the time. A board renamed last month has rows under both ids, so the honest query covers both — which is precisely what `resolveProjectColumnsForRoles` returns. ## Tested against real PostgreSQL, deliberately This is a **SQL predicate** change. A mocked store would assert the arguments and prove nothing about the query that actually runs — which is the entire risk when the literal lives inside `sql`. The new case inserts real `activity_log` rows on a renamed board and reads them back through the real store method. The legacy-lane case in the same file stays green, which is the compatibility half. **Revert proof, measured:** restore the hardcoded fragments and the new case fails with ``` expected [] to deeply equal [ 'renamed-entered', 'renamed-done' ] ``` ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - `activity-log-parity.pg.test.ts` — 5 passed against real PostgreSQL With this, all three Reliability inputs read the board's own lanes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Reliability duration metrics now work correctly with renamed workflow lanes. * Completion tracking recognizes configured completion lanes instead of relying on fixed defaults. * Improved handling of transitions between multiple review lanes and review-to-work-in-progress movements. * Legacy lane behavior remains supported when configured lane information is unavailable. * **Tests** * Added coverage for renamed lanes, historical lane IDs, and transition edge cases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d1ea33ee79 |
docs(lanes): three measured claims of mine had gone stale — date them or delete them (#2904)
Comment-only. No source change, no behaviour change. #2903 corrected a note that named a caller which had since been converted. This applies the same check to my own notes, and all three measured claims I wrote are now wrong: | claim | where | actual | |---|---|---| | "`self-healing.ts` alone issues **49** such reads" | `project-lane-vocabulary.ts` + its test | **37** | | "**51** such destinations exist in production" | `workflow-lifecycle-traits.ts` | ~34 | | "**22** deliberately pass `recoveryRehome: true`" | same | ~18 | All were accurate when measured. The shq fleet has been converting `self-healing.ts` since, and this program has been converting `moveTask` destinations all day. The repo-wide read-shaped total is now 37 *in total*, so "49 in one file" could not have remained true regardless. ## Two different repairs, because the claims differ in one way that matters **The self-healing figure has a reproduction.** `node scripts/lifecycle-column-census.mjs --json` reports `queryByFile` and `queryRoles`. So the number is kept, marked explicitly as a dated measurement, and the reader is pointed at the command rather than asked to trust the figure. **The `moveTask` counts have none.** Nothing regenerates them — the census cannot see call arguments, which is the very point the note is making. So they are **deleted** rather than refreshed, with the grep that approximates them inlined and labelled approximate. Refreshing an un-reproducible number just resets the clock on the same failure. The shape of the finding is what the note is for; the count was decoration that decays. ## Two process notes worth recording **Notes that assert facts about other files are a decay class with no detector.** The census counts literals; the unwired-lane guard counts declarations; neither reads prose. Two of these corrections in a row (#2903 and this) came from *reading a note and checking its claim*, which is not something the toolchain will ever do for us. The durable form is: cite a command, or state the shape without the number. **I nearly published a wrong replacement figure.** My first probe against the census AST returned `0` because I wired `summarize()` incorrectly — the third time this session a probe has been wrong before the product was. That is why the corrected note cites `--json` output rather than another hand count. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - `project-lane-vocabulary.test.ts` — 9 passed - census `--strict` — exit 0, counts unchanged 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
defe48d30f |
fix(core): per-workflow metrics read zero on a renamed board (#2866)
Second of the 14 lane-bound SQL sites from #2839, after #2864. Independent of it — different file, different caller argument. ## The defect `aggregateWorkflowAnalytics` filtered in SQL on `t."column" = 'done'` and `IN ('in-progress','in-review')`. On a renamed board those match nothing, so `tasksCompleted`, `tasksInProgress` and `tasksInReview` come back **zero for every workflow** while the board is busy. Nothing errors. Same shape and same fix as #2864: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, and thread the store from the single Command Center caller so the parameter has a supplier immediately rather than becoming an inert seam. ## What the test caught that I had not **The renamed case still failed with the query fixed.** The bucketing at lines 296–297 already uses `isWipColumnRole` / `isReviewColumnRole` — correctly converted — but those read `query.columnFlagsByName`, which production supplies and my fixture did not. So: - the **SQL** decides *which rows come back*; - the **trait map** decides *which bucket each row lands in*. Both halves have to be right. Fixing only the query would have shipped a "conversion" that still reported zero on a renamed board, and the file would have scored as converted twice over. That is exactly the partial-conversion shape this program keeps re-finding — caught here only because the test asserts `tasksInReview` alongside `tasksCompleted`, since those two paths take **different** resolved sets (complete vs wip+human-review). Asserting the completed count alone would have left the second conversion unproven. ## Measured Reverted, only the renamed case flips: ``` ✓ default vocabulary: completed and in-review work are counted × renamed vocabulary: completed and in-review work are counted ✓ renamed vocabulary: a card in the HOLD lane counts as neither ✓ without a lane store, the legacy ids still answer Tests 1 failed | 3 passed (4) ``` The hold-lane negative is there so resolving real lanes cannot degrade into "every column counts" — trading an undercount for an overcount is harder to notice than the original bug. ## Scope The sync SQLite arm in the same file keeps its literals: it throws in backend mode and has no production caller, the same dead-arm conclusion reached for `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c143327d4b |
fix(core): the archived-document guards failed in OPPOSITE directions on a renamed lane (#2886)
Two of the four convertible sites my own learnings doc **miscounted as sentinels** — the #2877 review corrected "8 of 9 must not be converted" to "5 of 9", and these are two of the three that correction freed. They read `task.column` straight off a row `select`, so they are board lanes by exactly the test that document gives, and a renamed archived column is simply not seen. What makes the pair worth fixing together is that they fail in **opposite directions**: | guard | on a renamed archived lane | consequence | |---|---|---| | `upsertTaskDocument` | fails to **reject** | an archived card's documents stay **writable** — the read-only contract silently does not hold | | `publishArchivedTaskDocumentAddition` | fails to **accept** | a legitimate archived-document publication is refused as `parent-not-archived` | The second is the sharper one: valid operator work refused, and refused with a message that reads as a data-integrity error rather than a lifecycle mismatch. ## Shape Both take an `AsyncDataLayer` and can resolve nothing themselves; their store-level impls hold the store, so the lane set arrives as a parameter resolved once per call — the shape #2875 used for the SQL predicate. **One shared `resolveArchivedLanes` for both paths**, deliberately: if the write guard and the publication guard could disagree about whether a card is archived, a card ends up both read-only *and* un-publishable. ## The revert proof caught my own fixture first My first version set `deletedAt` alongside the renamed column, and **the revert proof passed with the fix removed**. Both guards are `column-is-archived || deletedAt != null`, so a soft-deleted fixture short-circuits the exact comparison under test — the assertion was holding for an unrelated reason. Dropping `deletedAt` isolates it, and is also the *real* shape: a live row in a workflow-declared archived lane is what a renamed board produces, and what `getLiveTaskColumn` was written to catch. Revert proof, measured honestly the second time: restore `task.column === "archived"` and the renamed-lane case fails — the upsert resolves instead of rejecting. ## Real PostgreSQL, deliberately These are row predicates inside a transaction. A mocked store would assert the arguments and prove nothing about the comparison that runs — the same reasoning as #2875. Three cases: the renamed lane rejects, the **legacy** `archived` id still rejects (most boards never rename anything), and a live card is still allowed through (a guard that rejects everything is its own bug). ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - new `archived-document-lanes.pg.test.ts` + existing `artifacts-documents-evals.pg.test.ts` — 12 passed against real PostgreSQL Note: the SQL-literal baseline is untouched here — #2881 owns re-recording it after #2864's conversion left main's gate red. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
890e1f87e7 |
fix(core): issue panels reported nothing fixed on a renamed board (#2871)
Fourth and last of the lane-bound analytics sites from #2839, after #2864, #2866 and #2870. ## The defect `aggregateGithubIssueAnalytics` and its GitLab twin filtered their resolved-issue query on `"column" = 'done'`. On a renamed board that matches nothing, so `fixed` is **zero**, the resolved-issue list is empty, and `net` reports every filed issue as still outstanding — while the team closes issues all week. Nothing errors. Same fix as the previous three: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, thread the store from each Command Center caller so the parameters have suppliers immediately. ## Both providers in one change, deliberately These two files are **copies** — same query, only the provider literal differs — and a copy is exactly what gets half-fixed. Converting one and not the other type-checks, passes that provider's test, and leaves the second silently broken with no signal anywhere. The suite runs every case against both, so the pair cannot drift. ## Measured Reverted, exactly the two renamed cases fail — **one per provider** — while both default-vocabulary controls, both WIP-lane negatives, and both omitted-store legacy cases stay green: ``` ✓ github: default vocabulary counts a resolved issue × github: renamed vocabulary counts a resolved issue ✓ github: renamed vocabulary does NOT count an issue still in the WIP lane ✓ github: without a lane store, the legacy id still answers ✓ gitlab: default vocabulary counts a resolved issue × gitlab: renamed vocabulary counts a resolved issue ✓ gitlab: renamed vocabulary does NOT count an issue still in the WIP lane ✓ gitlab: without a lane store, the legacy id still answers Tests 2 failed | 6 passed (8) ``` That the failures are symmetric is itself the check on the copy-paste risk. ## Scope The sync SQLite arms keep their literals: they throw in backend mode and have no production caller, the same dead-arm conclusion as `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · Command Center + GitLab issue analytics suites 10/10 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. --- **This closes the lane-bound half of #2839.** All 14 sites the hand-review identified as genuinely vocabulary-bound are now converted across four PRs. What remains there is the 11 `!= 'archived'` exclusions, which are probably correct as literals — archiving writes `task.column = 'archived'` unconditionally as a state rather than a lane — plus one dead SQLite arm. Those need per-site judgment, not conversion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
216632bd3a |
fix(core): task-duration stats were computed from an empty set on a renamed board (#2870)
Third of the 14 lane-bound SQL sites from #2839, after #2864 and #2866. Independent of both. ## The defect `aggregateProductivityAnalytics` filtered its duration query on `"column" = 'done'`. On a renamed board that matches nothing, so the entire task-duration distribution — median, p90, average, total — is computed from an **empty row set** and reports zeros while the project ships work. Nothing errors. Same shape and fix as the previous two: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, thread the store from the single Command Center caller so the parameter has a supplier immediately rather than becoming an inert seam. ## Measured Reverted, only the renamed case flips: ``` ✓ default vocabulary: a finished task contributes to the duration stats × renamed vocabulary: a task in the RENAMED complete lane contributes ✓ renamed vocabulary: a task still in the WIP lane does NOT contribute ✓ without a lane store, the legacy id still answers Tests 1 failed | 3 passed (4) ``` ## The negative asserts the median, not just the count This fix's failure mode is **worse than the bug it fixes**. Resolving too many lanes would pull unfinished work into the distribution and produce a plausible-but-wrong median — a number nobody questions — where the bug produces an obvious zero. So the WIP-lane case asserts `medianMs` is null as well as `completedTasks` being 0. ## A fixture error worth naming My first version asserted `taskDuration.count`. `TaskDurationSummary` exposes `completedTasks`. Every case failed with `expected undefined to be 1` — **including the controls** — which reads exactly like a broken product until you notice the control is failing too. A control that fails is a fixture bug, not a finding; that asymmetry is the fastest way to tell them apart. ## Scope The sync SQLite arm keeps its literal: it throws in backend mode and has no production caller, the same dead-arm conclusion as `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ab15e5f9f7 |
docs(lanes): audit the three files the census points at with no reason attached (#2873)
**No source change.** Every literal stays counted and none gets an exemption marker. What changes is that the census now points at these three with the analysis attached, instead of making each worker who reaches them re-derive it. Peers have already done this well for `notification-service.ts` — converted it, *measured* a real delivery regression, reverted, and left it counted with the reason. These three had nothing at all, and one of them is the highest-impact unowned site I found. ## `planner-overseer.ts` (3 guards) — REAL, and larger than three literals suggest On a renamed board `resolveWatchedStage` returns `null` for every card. `observeTask` returns early on a null stage, so **no observation is recorded**, no `overseer:intervention` entry is emitted, and `PlannerRecoveryController` — which consumes those observations — has nothing to steer, retry, or targeted-fix. **The entire oversight loop is inert and silent about it**, exactly like the self-healing sweeps whose queries returned empty arrays. Not mechanical, which is why it is flagged rather than converted. `resolveWatchedStage` is a pure sync function over a `Partial<OverseerTaskRef>` with no store and no task id, so the lane answer has to arrive as a parameter. Its only production caller, `observeTask`, *is* async and the monitor *does* hold a store — but it runs **once per task per poll**, so resolving inside it buys a workflow read per card on a timer. The shape that works is the one the board-load enrichment landed on in #2845: resolve at the **poll**, once, with an IR cache keyed by workflow, and pass the flags down. That makes it a change to `project-engine.ts`'s poll as much as to this file — a cost judgement about a periodic engine loop, not a rename. `columnFlags` is in the unwired-lane-parameter vocabulary, so whoever adds the parameter cannot leave it unwired. ## `async-mission-store-queries.ts` (1 of 3) — REAL `getTerminalTaskEvidence` tests only `column === "done"` for its `done` verdict, so a completed card on a renamed board falls through every branch to `{ kind: "nonterminal" }`. The caller is mission **terminal evidence repair**, so a finished feature reads as unfinished — a wrong *verdict*, not an error. The `archived` test beside it has the same defect, masked for soft-deleted rows by its `deletedAt` companion, which is why only the `done` half bites in practice. Takes a bare `QueryHandle`: no store, no task object, no workflow. The fix is a resolved terminal-lane set threaded in by the caller — the same shape `getLiveTaskColumn` needs, and it should land *with* it so the two cannot disagree about what "finished" means. ## `audit-ops.ts` (2) — one sentinel, one real, and they look identical ```ts if (state === "archived") // ← getLiveTaskColumn's MANUFACTURED value: do NOT convert if (pgRow.column === "archived") // ← a real board lane: convertible ``` The first compares against a string `getLiveTaskColumn` *fabricates* for an archived-or-soft-deleted parent, so converting it to `isArchivedColumnRole` would keep passing on the built-in board and start **failing** on a renamed one — a soft-deleted task's log would become writable. The second reads the task row, so a renamed archived column keeps accepting log writes; its `deletedAt` companion masks that in practice. Two lines that look the same and need opposite treatment is precisely the reason these notes are worth more than the count. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`, `@fusion/engine`) — clean - `planner-overseer.test.ts` — 50 passed - census `--strict` — exit 0, **counts unchanged** (that is the point) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ea477f3ada |
fix(core): team analytics reported an idle project on a renamed board (#2864)
First of the 14 genuinely lane-bound SQL sites from #2839. I had been deferring these as "the owner is active in those files" — then checked, and **no open PR touches them**. The batch-core commit had already landed, so there was nothing in flight to collide with. The deferral was an assumption I could have tested two turns earlier. ## The defect `aggregateTeamAnalytics` filtered in SQL on `"column" = 'done'` and `IN ('in-progress','in-review')`. On a board whose lanes are renamed those match nothing, so per-agent completed counts, the project total, and the in-flight breakdown all came back **zero**. Nothing errors. A dashboard reading *"0 tasks completed"* for a team that shipped all week looks like an idle project, not a bug — which is why this survives review and never gets filed. **Why the sweep missed it:** the lifecycle census parses TypeScript comparisons; these ids live inside SQL strings, which are string data. The batch-core conversion fixed this file's TS guards **today** and left the queries untouched. The file scored as converted. ## Resolved per project, not per task `resolveProjectColumnsForRoles` gives the union of a role's columns across the project's workflows, which is the right set here because analytics aggregates a whole project — so a bound `IN` list is sufficient. The merge-queue cleanup needed the *superset-then-decide-in-JS* shape instead, because its lanes are genuinely per task and SQL cannot know a task's workflow. Same program, two correct answers; worth not copying the wrong one. ## Measured Reverted, exactly one case flips: ``` ✓ default vocabulary: a completed task is counted × renamed vocabulary: a task in the RENAMED complete lane is counted ✓ renamed vocabulary: a task in the WIP lane is NOT counted as completed ✓ without a lane store, the legacy ids still answer Tests 1 failed | 3 passed (4) ``` The three controls are deliberate: the default vocabulary (a generally broken aggregator cannot hide behind the renamed case), a WIP task that must **not** count as completed on the renamed board (resolving real lanes must not degrade into "every column counts" — an undercount turned overcount is harder to notice), and an omitted lane store that must keep the legacy answer byte-identical. ## Two mistakes the first attempt made, both caught by running it - **`= ANY(${array})` does not work.** Drizzle expands a JS array in a template into a comma tuple, so PostgreSQL rejected `(($1,$2,$3))` with *"op ANY/ALL (array) requires array on right side"*. An `IN` list of individual bound parameters is the working shape. Each id stays a parameter — these come from operator-authored workflow definitions and are never interpolated as SQL text. - The fixture's `ON CONFLICT (id)` had no matching constraint on `project.agents`; the sibling suite seeds with explicit `created_at`/`updated_at`. ## Scope One production caller (`register-command-center-routes.ts`), threaded in this same change so the parameter has a supplier from the start rather than becoming another inert seam of exactly the class this program keeps finding. The sync SQLite arm in the same file keeps its literals: that path throws in backend mode and has no production caller, the same dead-arm conclusion reached for `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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).
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
74cba4b46d |
batch-core: one shared landed-lane helper for the source-issue surfaces (75 → 72) (#2783)
## batch-core continued — the source-issue cluster Follow-on to #2780 (merged). Scope is still `packages/core` + `packages/dashboard/src`. ### The defect Five places asked the same question — *has this task landed?* — and all five compared against the literal `done`: | surface | consequence on a renamed board | |---|---| | GitHub source-issue commenter | never comments on or closes the source issue | | GitLab source-issue commenter | same | | GitLab `closedAt` backfill reconciler | finds nothing, reports a clean scan | | session-diff boundary | finished tasks diff against an already-merged branch | | tracking-comment transition | (already converted; left alone) | The commenters are the sharpest case: they returned **before reading a single setting**, so on a renamed board the feature looked *disabled* rather than broken — an operator checking `githubCommentOnDone` would see it enabled and still get nothing. The backfill is the quietest: `scanned: N, filled: 0` reads as "nothing to do", so the failure was indistinguishable from success. ### The fix One home: `packages/dashboard/src/task-lifecycle-lanes.ts`. Callers now only ask. Five copies of one question is exactly how the halves drift apart — the motivating incident is FN-6115 → FN-6118 → FN-6123, where the same affordance was fixed three times because it lived in two components. This also folds in the duplicate landed-lane helper I had left in `register-session-diff-routes.ts` in the previous PR, which was the sixth copy waiting to happen. Two helpers, and the difference is deliberate: - **`landedColumnsForTask`** — `complete ∪ archived`. Membership, since a board may declare more than one column carrying either role, and `columnsWithFlag(...)[0]` would silently ignore the second. - **`completeColumnsForTask`** — complete only. The GitLab backfill's own FNXC note records that archived tasks live in `archiveDb` and are *intentionally* excluded, so it must not widen to the archived role just because the shared helper offers it. Today it lists with `includeArchived: false` and would see no archived rows either way — but that is an incidental property of the query, not the contract. The test pins the difference so the two are not later "simplified" into one, which would change that caller's behaviour without touching it. Both treat an **empty** resolved set as *unexpressed*, not absent — the v1 hazard: `synthesizeDefaultColumns` upgrades a v1 graph with `traits: []` on every column, so reading empty as "no complete lane" would stop these surfaces firing on every pre-v2 project. The reconciler is two-stage on purpose: the cheap provider and `closedAt` tests run first and reject almost everything, so a workflow read only happens for real candidates, and it shares one IR cache across the scan — one read per distinct workflow rather than per task. ### Census `batch-core` scope **75 → 72**; repo total **338**. ### Verification - `pnpm --filter @fusion/dashboard exec tsc --noEmit -p tsconfig.json` → 0 errors - `pnpm lint` → 0 errors - commenter + reconciler suites → **63 passed**; helper suite → **5 passed** - **Mutation-verified:** making the helper ignore its resolved set fails 1 of 5. --- ## Round 2 — server.ts, chat.ts, and a correction **Census: 75 → 67** across this PR. ### The correction (see the review thread above) My first pass gated the source-issue commenters on `landedColumnsForTask` (`complete ∪ archived`), which **widened** the trigger — `to === "done"` never fired on archival, and the landed set does. Both commenters now use `completeColumnsForTask`, and the unused `hasTaskLanded` wrapper is gone. The ratchet for it is pinned on the **default** board, deliberately: a widening is visible exactly where the legacy names still apply, so no renamed-board fixture would catch it. ### `chat.ts` — three sites, and a pair that had to move together - **Chat verification** required `column === "in-progress"`, so on a renamed board every chat-driven verification was refused with a message naming a column the board does not have. - **The planner refinement pair.** Two separate guards decide this feature: `createSession` *registers* the tool only for a finished task, and the tool's own `execute()` *refuses* a non-finished source. Both compared `done`. Converting only one half would have offered the tool and then had it refuse itself — the half-converted-pair shape. The new test asserts **both** halves in one case (tool present *and* refinement created), and each half reverted independently fails it. Existing `chat-manager` coverage caught neither revert, which is why the case exists rather than relying on the suite that was already there. Complete-only again, not the landed set: an archived task is off the board and is not a refinement source. ### `server.ts` - **Planner-chat retention** — the archival cutoff was a literal, so on a renamed board task-planner chat sessions were retained forever; the rule this listener exists to enforce never fired. Resolved, and awaited inside the existing fire-and-forget chain rather than by making the listener `async` — `task:moved` has synchronous subscribers whose ordering is load-bearing elsewhere, and a chat-row delete is not the right place to introduce a microtask boundary into that emit. - **`isBadgeEligibleTask` — deliberately NOT converted, and marked as backlog.** On a renamed board it is genuinely wrong: an archived card stays badge-eligible, its snapshot is never evicted, and the cache grows for the daemon's lifetime — the exact memory leak the predicate was added to fix, back under a different column name. What blocks it is measured, not assumed: both callers are synchronous `task:updated` / `task:created` listeners whose next statement is documented as *"Update local cache immediately"*, so awaiting lets a second event for the same task interleave between the eligibility check and the cache write. I did **not** add an optional `archivedColumns` parameter, because nothing could fill it — the callers are the sync listeners. That is the inert-injection shape this PR's own review caught twice on #2780: the predicate would read as converted, its test would pass by injecting the value, and production would keep the literal. The unblocking change (a resolved-archived-lane cache on the badge-snapshot scope, keeping the predicate synchronous) is recorded at the site. ### Verification - `tsc --noEmit` → 0 errors; `pnpm lint` → 0 errors - `chat-manager` → 101 passed; commenter/reconciler/helper/badge suites → 55 passed - Mutation-verified per fix, including each half of the refinement pair separately <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved task lifecycle handling for renamed workflow lanes, including completed, archived, landed, and in-progress states. * Task lists now exclude completed tasks regardless of the completion lane’s name. * Chat verification and refinement actions now recognize configured workflow lanes. * GitHub and GitLab completion comments trigger only for genuinely completed tasks, not archived tasks. * Knowledge index refreshes and GitLab metadata updates now support custom completion lanes. * **Tests** * Added regression coverage for renamed completion lanes and archived-task behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e84e9d7f60 |
fix: the caller audit — five unwired parameters, five defects in their callers (#2803)
Seven fixes that were sitting on separate handoff branches with no owner while `main` moved. Consolidated, rebased onto current `main`, and verified **together** rather than only per-branch. The individual branches remain if a subset is preferred. This is the same consolidation that got `batch-core` and #2787 adopted. **Close it if it breaks queue policy** — the branch keeps the work safe either way. ## Where these came from #2787's review found an optional parameter whose production caller never passed it. That is a class, so I ran it against everything I had landed and found five more. **All five turned out to have their real defect in the CALLER, not the parameter** — in four of them the parameter was unreachable: | unwired parameter | what was actually wrong | |---|---| | `blocker-fanout.escalationColumns` | the hold default made the count zero — **no bottleneck warning was emitted at all** | | analytics `columnFlagsByName` | routes never built a map — **0 in-progress / 0 in-review beside correct cost totals** | | `isLegacyAutoMergeStampCandidate` | the read **queried a column a renamed board does not have**, so the backfill iterated nothing | | `rankAssignedTasksForWakeDelta` | `getTasksByAssignedAgent`'s `excludeArchived` used the literal — **archived cards returned as open work** | | `duplicate-intake.columnFlagsByColumnId` | intake could **archive or soft-delete a newly created task** as a duplicate of finished work | The heuristic worth keeping: **an optional parameter no production caller fills is a marker pointing at an unexamined caller.** The census cannot see any of these five — every gate is a `Set`/array literal or a query filter, i.e. a definition rather than a comparison. ## Also included - **`executor.ts`** — the stale-spec guard did the exact thing its own comment forbids: on a renamed board it ran on a LIVE task and pulled it out of execution into replan. `activeMergeStatuses` protected merging cards *by accident*, which is why the symptom looked arbitrary. - **`register-project-routes.ts`** — project health reported **0 active tasks**; its list also still contained `triage`, dead since U11. - **`dashboard/app/utils/taskTiming.ts`** — a **second copy** of `getTotalAgentActiveMs`. Core's was converted; the card chip imports this one, so the census counted the site as done while the rendered number stayed keyed on `"in-progress"`. ## Verification Verified as a set: `pnpm test:gate` **161 / 13 / 487 / 71** · core suites **15 passed** · engine **7** · dashboard **12** · four `tsc` targets clean · lint clean · census `--strict` exits 0. Each fix is revert-proven individually; the specific case that fails is named in each test header. ## Two honesty notes **Three guards here are structural, not behavioural, and say so in their headers.** `sanitizeAgentTaskLinks` is a closure inside `createApiRoutes`; the analytics aggregators need a live `AsyncDataLayer`; the stale-spec guard sits deep inside `execute()`. Each ratchet fails on revert — verified — but none is an end-to-end proof, and the headers state which half they cover. **One of my behavioural test sets would have lied.** The intake-dedup cases drive `findSameAgentDuplicates` directly; I removed the wiring to measure the revert and **they stayed green**, because they pin the predicate and not the caller. That is the exact illusion this audit was chasing, reproduced in my own file. The forward now has its own structural check. ## Deliberately not included `worktree-pool.ts:1205` — it **fails safe** (a missed match protects a branch from cleanup rather than deleting it) and sits in the merger's branch-reaping path where the opposite error destroys work. That deserves its owner's judgement, not a drive-by conversion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
02d0f80068 |
fix(tests): update the archived-gate inventory after #2780's conversions (#2786)
## New red on main `archived-column-gate-parity.test.ts` fails with **"TypeScript encoding changed"** after batch-core (#2780). It is the only failure in a full `@fusion/core` run (`1 failed / 4748 passed`). ## The conversions are right — this is their missing half #2780 moved four files off raw `column === "archived"` comparisons onto `isTerminalColumnRole`, exactly the intended role-based pattern: | file | before → after | |---|---| | `assigned-task-ranking.ts` | 1 → 0 | | `duplicate-intake.ts` | 1 → 0 | | `near-duplicate-canonical.ts` | 1 → 0 | | `store.ts` | 2 → 1 | (The literal still in `duplicate-intake.ts` is a `moveTask` **destination**, not a gate comparison — a different question, like the planner-lane move targets.) The guard's own failure text asks for the inventory to be updated **in the same commit** as a conversion. That did not happen, so the ratchet went red on main. This PR is only that bookkeeping. ## 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, so on a renamed board one says a task is archived and the others return it as live. The Drizzle and raw-sql inventories are **unchanged and both pass**. Worth stating explicitly because those two assertions run *after* the TypeScript one: a plain red tells you nothing about them, they had to be re-run green to know. ## The guard still bites Appending a real `task.column === "archived"` to an audited file fails it immediately. **Recorded because it nearly fooled me:** my first two mutation attempts *passed*, which looked like a guard blind to new comparisons — a much worse finding than a stale inventory. Both had been inserted at line 2 of a file whose line 1 opens a JSDoc block, so my "code" was comment text and was never compiled. **A mutation that does not compile is not evidence of anything.** Appended at end of file instead, the guard fails on the first run. Gate **732 green** · lint clean. Test-only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
31b9fafe11 |
fix(tests): the last core red — assert the funnel COUNTS the move, not which stage owns it (#2781)
## What was red
A full `@fusion/core` run on `origin/main` reports **1 failed / 4700
passed**. This is that one: the SDLC funnel case in
`agent-logs-and-monitor.pg.test.ts`.
```ts
expect(result.funnel.stages.find(({ stage }) => stage === "todo")?.entered).toBe(2);
// expected 2, received 0
```
## The move is not lost
Post-U11 the default Planning column is **one** column carrying
`["intake","hold","reset-on-entry"]`. `stageForTraits` prefers the
earliest stage in flow order, so `intake` wins and a move to `todo` is
attributed to the **`triage`** stage. Analytics is working correctly;
the column vocabulary underneath it merged.
## Why not just re-point the assertion
Which stage the merged Planning column *should* report is an open
product question — I flagged it on #2669 while adding the `hold`
mapping, and it is visible to users: **the funnel shows a phantom 100%
drop between Triage and Todo on every default board since U11.**
- Re-pointing the **test** at `"triage"` quietly blesses the phantom
drop.
- Re-pointing the **mapping** retroactively changes how historical
analytics read — not a reversible call.
Neither belongs in a change whose job is clearing a red.
So the assertion now pins what is true under **either** resolution: both
moves are counted exactly once, in the single pre-implementation stage
the Planning column resolves to. When #2669 is decided, this test does
not need rewriting.
## Evidence
**6/6 passed.** Mutation-proved for the failure that actually matters:
| mutation | result |
|---|---|
| unmap every pre-implementation trait (move falls to `OTHER`) |
**fails** — `expected +0 to be 2` |
| unmap `intake` alone (attribution shifts triage → todo) | **passes, by
design** — that is the open question, not a defect |
The second row is the point of the rewrite: the test is indifferent to
the unsettled question and strict about the invariant. It still catches
a dropped, double-counted, or split move.
Gate **732 green** · lint clean. Test-only — no production file touched
(the mutations above were reverted; `git diff` clean).
## Ownership
`packages/core` belongs to the **batch-core** owner under the mega-batch
split. This is fix-forward on a red rather than a conversion, so it is
deliberately confined to one assertion in one test file and touches no
production code — it should not conflict with the batch.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
8c9b84ae38 |
batch-core: packages/core + dashboard/src lifecycle conversion (129 → 92) (#2780)
## batch-core — `packages/core` + `packages/dashboard/src` Shared branch: two workers are converting into it. Opening the PR because the branch was green with none, and a branch without a PR merges nothing. ### Census Measured with `node scripts/lifecycle-column-census.mjs --json`. | | guards | |---|---| | batch-core scope at branch point | 129 | | batch-core scope now | **92** (51 files) | | repo total now | 358 | Files closed so far: `store.ts` 11→0, `task-merge.ts` 6→0, `live-agent-count.ts` 6→0 (marked, not converted — see #2762), `task-update.ts` 3→0, display-ordering + Wake Delta ranking 5→0, `register-git-github.ts` 4→0. ### The `register-git-github.ts` slice Three PR routes — `pr/create`, `pr/push-branch`, `pr/resolve-conflicts` — plus the `CHANGES_REQUESTED` handler each compared `task.column !== "in-review"`. On a renamed board **none** of them matched, so every PR affordance the dashboard offers was refused for a card sitting in the lane that board calls review, and the refusal named a column that does not exist there. All four now share one helper, `reviewColumnsForTask`, which gets two things right that this program has repeatedly gotten wrong: - **Membership, not a single id.** It takes the broad review set (`mergeOrchestration ∪ mergeBlocker ∪ humanReview`). `resolveLifecycleColumns` returns the *first* column per trait, so a single-id answer silently ignores a board that declares a merge lane **and** a separate human sign-off lane. These guards only refuse or permit — they never move the card — so over-admitting costs nothing while under-admitting refuses a request that should have worked. - **An empty resolved set means UNEXPRESSED, not absent.** `synthesizeDefaultColumns` upgrades a v1 graph by emitting every default column with `traits: []`, so a v1-upgraded workflow resolves to an empty review set while its `in-review` column plainly exists and holds the card. Reading empty as "this board has no review lane" would refuse these routes on **every pre-v2 project** — a worse regression than the one being fixed, and invisible to any v2 test. This is the dashboard twin of the `fn pr create` guard in `packages/cli/src/commands/pr.ts` (#2775). The two surfaces answer the same question and now agree — FN-5893 surface enumeration. ### Testing note: why the seam and not the routes I wrote route-level HTTP tests first and **deleted them**. An express fixture over `registerGitGitHubRoutes` hangs — every case, including the pure refusals, times out at 4s, because registering the router starts background work the fixture never satisfies. Making it run would mean mocking git, the GitHub client, and the pollers: a mock-the-world shell, which is what the project's do-not-add-slow-tests rule (FN-5048) says to avoid in favour of a narrow seam. `reviewColumnsForTask` *is* the narrow seam — it holds the entire decision, and the four call sites now do nothing but ask it and render its answer. Six cases pin it: the renamed lane is returned and `in-review` is not, a two-lane board returns both, a v1-upgraded board falls back, an unresolvable workflow falls back, and the refusal renders lanes an operator can act on. **Mutation-verified, both directions:** reverting the helper to the legacy literal fails 2 of 6; treating an empty set as an answer fails 1 of 6. One fixture bug worth recording, since it would have made the two-lane case vacuous: the trait id is kebab-case `human-review`, not `humanReview`, and the built-in traits must be registered via `import "@fusion/core"` before flags resolve. ### Verification - `pnpm --filter @fusion/dashboard exec tsc --noEmit -p tsconfig.json` → 0 errors - `pnpm lint` → 0 errors - `register-git-github.review-lanes.test.ts` → 6 passed --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |