f5cc416ae4ebc755d8b8ebfaee1253dc7c0aef70
12480 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
gsxdsm
|
f5cc416ae4 |
U7 item 3: the replan no-match fallback named a column no lineage declares (#2659)
**Item 3 from the closing bar.** Behaviour change, own commit. ## The defect `resolveReplanTargetColumn` fell back to the literal `"triage"` when a workflow declared neither legacy planner id. That names a column the workflow doesn't declare — and since #2515 the **default lineage doesn't declare it either**, so the fallback pointed at a column that exists nowhere. The replan move then either failed outright or put the card somewhere no sweep owns. Resolved through `resolveReboundTarget` (KTD-10: hold → intake → first declared) — the same helper every other rebound path uses, so replan lanes and rebound lanes stay consistent instead of drifting. ## The catch path keeps its literal, deliberately It's reached only when resolution **throws** — not when it silently falls back to the default IR, which returns a real workflow and takes the `todo` branch above. With no IR there's nothing to resolve, and swapping one arbitrary literal for another changes behaviour without evidence about the workflow. Documented at the site so the asymmetry reads as a decision, not an oversight. ## Test Written first and observed **red**. It asserts the target is a column the workflow actually declares: ```ts expect(workflowHasColumn(ir, target)).toBe(true); ``` rather than pinning a specific id — so it can't pass by naming a *different* wrong column, which is how the previous version of this test stayed green while the fallback was broken. **Mutation-verified:** restoring the literal fails it. ## Verification 40 replan-target tests green, engine tsc clean, lint clean, merge gate green (487 + 132 + 10). No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
be63e72f10 |
U11 [E2E evidence]: live-PG proof for the stranded-column rescue and the planner-lane asymmetry (8 tests, test-only) (#2629)
**Completion bar #3 for my phases.** Test-only, no production changes, no guard-count movement — the two live-PG E2E suites I held during the freeze. ## Why these exist Every U11 slice I shipped closed with the same caveat: *all evidence is unit-level*. Three claims in particular were argued from reading code, and each is the kind a mock would happily confirm: 1. #2515 left `triage` a legal id but removed it from the default lineage. 2. #2603 — `createTask` resolves the workflow's intake column, and an explicit `column` **overrides** it. Nine write sites were removed on that reasoning. 3. #2591 — a card stranded on a legacy planner id is admitted by planning discovery, which is what lets it heal with no data migration. Both suites drive a **real PostgreSQL TaskStore** (per-file throwaway database) and the **real shipped workflows**, not fixture IRs. Claim 3 goes through the real `discoverReadyPlanningTasks` — the method the poll calls. Every assertion is on **observed persisted state** (fresh `getTask` after clearing the task cache), the rule inherited from `workflow-lifecycle-live-e2e.pg.test.ts`, because "a function was called" is exactly what has passed falsely on this program before. ## Two things the E2E found that unit tests did not **The shared fixture's "merged" shape was not #2515's.** Omitting `separateIntake` leaves the hold column with *no* intake trait, so the resolver reports `undefined` — "I have no intake to name" — whereas the shipped merged lineage carries intake **and** hold on one column and reports `[]` — "intake exists and *is* the hold column". Callers treat those differently: `undefined` keeps their legacy default, `[]` positively asserts no dedicated planner lane. Assuming the plain shape was the merged shape is how a test appears to cover #2515 while covering something else. Added an opt-in `mergedIntake` to model the real thing; the third shape is now asserted explicitly. **`insertWorkflowDefinitionSync` throws in backend mode** — it's the SQLite path. The suites use `createWorkflowDefinition` + `writeTaskWorkflowSelection` like the other live E2Es, including binding to the id the *store* allocated rather than the one passed in, which the lifecycle suite documents as a way a renamed-workflow fixture silently resolves to the default IR. ## Fixture changes are opt-in Both new options follow the existing `mergeOrchestration` precedent: seven suites build on this builder and a shared fixture must not silently change an existing suite's subject. ## Naming `workflow-planner-lane-**resolution**-live-e2e` deliberately, to stay distinguishable from #2611's `workflow-planning-lane-live-e2e`. Different subjects — that one drives the real hold-release sweep, this one drives the resolvers the lane guards consume. Near-identical names would invite someone to delete one as a duplicate. ## Verification - 8 new tests green against a real PG store - **Mutation-verified:** disabling the #2591 rescue in `discoverReadyPlanningTasks` fails claim 3, and only claim 3 - Merge gate green (482 + 132 + 10), engine tsc clean, lint clean **Pre-existing failures, not from this PR:** the full live-E2E sweep is 82 tests / 2 failed, both in `workflow-lifecycle-live-e2e.pg.test.ts`. Verified by swapping main's `_workflow-vocabulary-fixture.ts` in and re-running: 2 failed either way, identical. They are main's, and they appeared since my earlier clean run of that suite — worth a look against bar #2. ## What this does not cover Neither suite runs a planning **session** — that lane is the AI, substituted here as `testMode` does in production. So this proves a card is *admitted* and re-homable, not that a full plan-and-release round trip happens. The release half is covered by the existing lifecycle E2E. No changeset: `@fusion/engine` is private and this is test-only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
5481c27729 |
docs(solutions): finding 6 — read the implementation before claiming its output is wrong (#2649)
Completes `proving-a-code-path-actually-runs.md` (merged as #2642) with the rule its own author broke three times while writing it. **Docs only.** ## Why this belongs in that document rather than a new one Findings 1-5 are about proving **your own** claim: does this path run, can this test fail, is this negative result observable. Finding 6 is the mirror image — the claims we make against **other people's** work — and it is the same underlying error pointed outward. Splitting them would let a reader take the first five as "be rigorous about my code" and miss that the identical discipline applies when reviewing someone else's. ## The three cases, all mine, all in one day | What I claimed | What was actually true | |---|---| | The census undercounts triage guards, 13 vs 10 | `summarize()` counts `byColumnId` only for `kind === "column"`. My patched counter summed `role`, `status` and `deliberate` too. The three "missing" ones were exactly the ones it classifies correctly — and I reported this against the instrument the program had just adopted as authoritative. | | `resolvePlannerLanesForTask` silently disables two recovery paths for legacy cards — escalated across four messages | The file's own header had already reasoned it through and documented why that answer is correct. And `TaskStore` implements `getTaskWorkflowSelectionAsync`, which the resolver prefers — so real projects never take the path my `{ getTask }`-only probe forced. | | `executor.ts` is clean of triage guards | A receiver-specific grep missed three under `from` and `originColumn`. Same error one step earlier: trusting a reconstruction of the thing instead of the thing. | Every one was: reconstruct behaviour from outside → compare to actual output → find a difference → report a defect, **without reading the implementation.** ## The rules it adds - Read the implementation and its header comment before reporting anything as wrong. On this codebase the reasoning is usually already written down, and the FNXC note frequently answers the exact objection — twice today it answered mine verbatim. - **A fixture is not a measurement of production.** When a probe and the real system disagree, suspect the probe: ask what it had to stub, and whether production ever supplies that shape. - Retract precisely and immediately. A false defect report against shared infrastructure costs more than the bug would have — it sends people to verify something already correct, and spends the credibility needed for the next report that is real. Also updates the count in the intro (five → six) and adds an `applies_when` entry so the doc surfaces for "about to report a tool as defective", which is when it is needed and not when someone is already debugging. `pnpm lint` clean. No changeset — internal documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
177c2309d9 |
consolidate/u11: a hold column is a planner lane only if it precedes wip (real defect in merged code) + funnel aliases (triage 11 -> 10) (#2645)
**Consolidation branch for u11/u7.** Supersedes #2624. Contents changed substantially while it sat unmerged — this body reflects what is actually in it now. ## Measured with the authoritative census, not grep `node scripts/lifecycle-column-census.mjs` — **triage guards 11 → 10.** ## 1. A real defect in merged code: a hold column is only a planner lane if it precedes implementation Found by greptile on #2616, verified by me, fixed here at the source because that PR cannot land. `resolveLifecycleColumns` returns `hold` as the **first** hold-trait column in declared order, with no positional constraint relative to wip (`workflow-lifecycle-traits.ts`: `hold: first(LIFECYCLE_ROLE_FLAGS.hold)`). A workflow using a hold trait for a **mid-pipeline wait** — a pause after implementation starts — therefore had that column returned as its planner lane, and `reconcileMissionFeatureState` demoted the feature to `triaged`. The mission board reported started work as not-yet-started: silent, and wrong in the direction that makes a roadmap lie. This is my defect, introduced in #2610. **Why it survived:** every lineage anyone has tested puts the hold *in front* of wip, so the default and Ideas boards are unaffected and no existing test could see it. **The fix is positional, with a deliberate asymmetry.** A hold column counts only when it appears before wip in declared order. When wip cannot be located the hold is left **out** rather than guessed — including it wrongly demotes live work on the roadmap, while excluding it wrongly costs only a `triaged` transition the next reconcile re-applies. Mutation-verified: dropping the positional test fails the mid-pipeline case and nothing else. ## 2. MissionControlPanel funnel aliases Assessed and **deliberately not trait-converted**. These are heuristic *name aliases* for a canonical SDLC stage — the matcher already accepts `signal`/`backlog`/`ready`/`shipped` because it buckets arbitrary boards, with an `other` fallback. Post-#2515 a default board's planning cards sit in `todo` and count at the Todo stage, leaving Planning at zero: the funnel reporting where cards *are*, not a guard that stopped firing. Hoisted to a named set so it stops reading as unconverted. This is the **DISPLAY-ALIAS** class the census still lacks — receiver *is* a column id, purpose is presentation rather than a lifecycle decision. `DocumentsView`'s status dot is the other one. Without that bucket a ratchet will keep demanding conversions that make the product worse. ## What I dropped, because main's version was better The original #2624 carried a `TaskContextMenu` conversion. #2626 landed `isPureIntakeColumn` — intake **without** hold — while mine treated any intake-flagged column as intake. That's wrong for a **merged Planning column**: it carries both traits, cards there wait for capacity and have real actions, so I would have suppressed the menu where it belongs — a new regression in place of the one I was fixing. Theirs is correct. Mine is gone, along with its now-invalid test and a helper nothing else used. ## Verification - merge gate green (482 + 132 + 10), engine tsc clean, dashboard tsc clean, lint clean - 7 planner-lane tests green, mutation-verified No changeset: `@fusion/engine` and `@fusion/dashboard` are private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
3e8f604848 |
test(engine): census the UNCONVERTED lifecycle surface — 417 legacy column literals, ratcheted (#2557)
Test-only, no production change. Independent of my other open PRs.
## The number nobody was counting
This program has two censuses, and **both count converted things**: the
unproven-sites ledger (callers of the lifecycle-role resolvers) and
`raw-workflow-columns-flag-census` (reads of the `workflowColumns`
flag).
Neither counts what is still keyed to a legacy column id — **which is
where every defect this program has found actually lived**:
| defect | the literal |
|---|---|
| pool-id sentinel (capacity gate never bound) | `?? "builtin:coding"`
vs the counter's sentinel |
| agent-link leak (slot consumed forever) | terminal column matched
against a fixed id set |
| stale-paused badge silent on renamed boards | `task.column !== "todo"`
|
| merge chokepoint threw on a finished card | the `done`/`archived` pair
|
| recovered card stranded harder | `?? "todo"` |
Every one was found **by hand, one at a time, by whoever happened to
look.**
## Measured
**438 lifecycle decisions keyed to a legacy column name** (417
comparisons + 31 `??` column fallbacks, minus 8 agent-id false positives
and 2 lines carrying both shapes), across 85+ production files — 94 in
`self-healing.ts`, 70 in `executor.ts`, 26 in the dashboard
task-workflow routes.
That is the real size of the remaining surface. It dwarfs the 15-site
resolver census I've spent this unit closing, which is worth knowing
before anyone calls the vocabulary work finished.
## A hit is not a bug
Many are correct — documented legacy fallbacks, the legacy-adoption
path, code genuinely about the built-in workflow. The census claims only
that each site decides by **name** rather than by **role**, and
therefore needs a human judgment. Reporting 417 as a bug count would be
exactly the overclaiming this program keeps correcting.
## A ceiling, not an equality — deliberate
The sibling flag census fails in both directions. That number moves only
when two units touch it. **This** one moves whenever any of a dozen
concurrent conversion slices lands, and an exact-equality assertion
would go red on work heading the *right* way.
A test that's red for good reasons gets suppressed, and a suppressed
ratchet is worse than none — the failure mode AGENTS.md's quarantine
rule exists to prevent. So the count may fall freely and may never rise;
when it falls, the failure message says to lower the pin.
## Verified in both directions
- green at 417
- adding **one** literal to `replan-target.ts` → `census ROSE to 418
(ceiling 417)`
- the regex is unit-tested to count a **decision**, not a mention: a
column id in a fixture, a log line, or a `moveTask` argument is not
counted — inflating the number into noise is how a census stops being
acted on
- unreadable sources **fail closed** rather than silently shrinking the
count
## Follow-up (a8c150b12): the census was blind to three of the five
defects it cites
I ran the census against its own header. It lists five motivating
defects; the comparison-only regex counted **two**. The pool-id
sentinel, the rebound strand and the terminal fallback are all `??`
**defaults** — invisible to a `.column === "x"` pattern.
A census that cannot see three of the five bugs it names as its reason
to exist is worse than none: it reports a number that *feels* like
coverage. That is precisely the overclaim this unit keeps catching in
other people's work — caught here in mine, and only because the header
wrote the examples down somewhere they could be tested against.
It now counts two shapes — deciding **by** a name (`===`/`!==`) and
**defaulting** to one (`??`) — and pins the five motivating examples as
a test case, so the pattern cannot narrow back without failing.
**Measured: 417 comparisons + 31 fallbacks, of which 2 lines carry both
shapes → 446 lines.** Ceiling raised 417 → 446 to cover the missing
shape, not to excuse new debt.
`?? "builtin:coding"` stays deliberately uncounted: it defaults a
*workflow* id rather than a column and is legitimately correct at most
sites. It already has a stronger guard —
`scripts/check-capacity-pool-id.mjs` bans it only where the value
reaches a capacity counter, which is the only place it's wrong.
Verified both directions: green at 446; adding one fallback of the
newly-counted shape → `census ROSE to 447 (ceiling 446)`.
## Follow-up 2 (98f4264fd): 8 false positives removed — 446 → 438
Then I checked the census against real source instead of trusting the
pattern. Its top-scoring fallback file was `triage.ts` with 8 hits — and
**every one is `agentId: task.assignedAgentId ?? "triage"`**, an *agent*
id, not a column. `"triage"` is both a column id and the synthetic agent
id triage stamps on its audit rows.
Eight of ~34 fallbacks is a quarter of that shape: enough to make the
number **wrong** rather than merely imprecise. A census with known false
positives is one people learn to discount — the same end state as not
having one, which is exactly what its own header warns about.
Excluded, and the exclusion is **pinned as a test case** so it can't
creep back: the three agent-id spellings must match the raw shape *and*
be filtered, while a genuine column fallback that also mentions triage
(`first("intake") ?? "triage"`) must still count.
**Residual imprecision is stated rather than tuned away.** A couple of
counted lines are display defaults (a column rendered in CLI output).
They stay: the census claims each site *needs a human judgment*, and a
display default passes that judgment in seconds. Chasing them costs more
than the precision buys and makes the pattern too clever to trust.
Agent-ids were excluded because they're a quarter of the shape — not
because any false positive is intolerable.
Ceiling 446 → **438**. Verified both directions: green at 438; one new
fallback → `census ROSE to 439`.
## Verification
- census 3/3; engine `tsc --noEmit` clean; `pnpm test:gate` green (414 +
10 + 71)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
2a4013b723 |
consolidate/u9 — review+merge lane: E2E evidence, re-greens, and the conversion blocker (#2646)
**U9's consolidation branch.** Supersedes nothing — #2637 and #2643 are green with zero threads and left for your sweep per rule 3. ## Contents | File | Change | Before → After | |---|---|---| | `__tests__/executor-step-numbering-zero-based.test.ts` | isolate the review-handoff `moveTask` call so the assertion is attributable | **1 failed / 3 passed → 4 passed** | | `__tests__/ce-workflow-step-executor.test.ts` | re-green against the block-first merge boundary | **3 failed / 48 passed → 51 passed** | | `__tests__/goal-anchoring-audit.test.ts` | swallow path reports at debug, not `console.warn` | **1 failed / 6 passed → 7 passed** | Triage-guard counts: **no change**. My lane has no remaining column receivers — the rest belong to the capacity/U7/U8/U11/U12 workers, or are deliberate compat retentions I verified individually (`spec-staleness.ts` carries its own "U11 proof" block; `live-agent-count.ts`'s literal fallback is reachable by flag-less callers). Commits kept small and separated: signature fix, then attribution fix, then the boundary re-green, then the debug-channel fix. **Census reconciliation:** `node scripts/lifecycle-column-census.mjs` reports **11** triage guards on main, and **none are in the review/merge lane** — they are the `moves.ts` flag-OFF branch plus the dashboard cluster. Nothing in this branch moves that number, and I am not chasing the 779 non-triage guards per your instruction. ## 1. The review-handoff assertion (and a lesson) The handoff gained a third argument (workflow move provenance), so a two-arg `toHaveBeenCalledWith` failed on the extra options object while the card moved correctly. My first fix used `expect.anything()` — and I *documented in the comment* that six mutations couldn't make it fail, then shipped it anyway. Greptile (P2) correctly called that out: this flow records two `moveTask` calls, so the assertion is satisfied by the boundary move even if the handoff regresses. **Documenting a weakness is not removing it.** Now the test selects the handoff call by its own marker (`workflowMoveMetadata.reason === "workflow-review-handoff"`), asserts exactly one such call, and asserts its target column: | Mutation | Before | After | |---|---|---| | change the seam's `reason` | green | **NEW=1**, this test only | | retarget the seam to `"done"` | green | **NEW=1**, this test only | ## 2. The merge boundary changed shape `ensureWorkflowMergeBoundaryTask` (`executor.ts:7808`) now **refuses** a foreach step-execute region with incomplete pre-merge node proof — logging `"Workflow merge boundary blocked: <reason>"` and returning **without moving**. The move-then-check sequence this file pinned is gone: `"Workflow merge boundary moved task to in-review before requesting merge"` no longer exists anywhere in production. Three fixes, one per failure: 1. **negative case** pinned the retired move-first log. Now pins the *stronger* property the new order gives: an unproven card is **not moved into review at all**. The old assertion could only say "it was moved, then blocked". Log text asserted by stable prefix — the reason clause enumerates missing instance ids, which is legitimately volatile. 2. **"moves direct-to-merge tasks into in-review"** got zero calls: its fixture recorded no node results, so the gate blocked it. Added one `steps#0:step-execute` pre-merge result. 3. **"completes graph-native checklist projection"** also got zero calls. Its existing `plan` result proves *some* pre-merge node ran but not the per-instance work; the gate additionally requires an instance per foreach step-execute. Added the two matching its two steps. (2) and (3) are the same class as the lifecycle E2E `seedTask` fix in #2634: a fixture that never modelled completed work, asking the engine to advance it, and reading the correct refusal as a failure. Proof shape matched to the evaluator (`source: "node"`, `phase: "pre-merge"`, terminal = `passed`/`skipped`) rather than guessed. Verified the gate is what these fixtures exercise: disabling the boundary proof check fails the negative case (`NEW=1`, that test only). `pnpm test:gate` green, `pnpm lint` clean. ## Where U9 actually stands The conversion (S06/S07/S08) is **not** done, and is now precisely characterised rather than "blocked on U8": `workflow-graph-executor.ts:310` short-circuits every `MERGE_REGION_KINDS` entry to the legacy merge seam, so `merge-gate`, `merge-attempt`, `manual-merge-hold`, `retry-backoff`, `recovery-router` and both `branch-group-*` handlers **never execute**. `createMergeGateHandler` does read `task.autoMerge` and emit auto-on/auto-off — and is never called. The builtin IR's `outcome:auto-*` edges are unreachable. **U9's conversion, concretely: stop short-circuiting `MERGE_REGION_KINDS` and let those nodes run.** S06/S07/S08 all hang off that one change. Safeguard 2 has no node-level representation today, so enabling the region without carrying the `autoMerge` contract into it would let an `autoMerge:false` card merge on PR-readiness alone. Full write-up in `docs/plans/workflow-owned-merge-stack/u9-safeguard-baseline.md` (#2634). ## 3. A recurring class worth a shared helper `goal-anchoring-audit`'s swallow path now reports via `log.debug` (a deliberate demotion of log noise), and `debug` is FUSION_DEBUG-gated so vitest emits nothing — the test asserted a channel that was both wrong *and* disabled. I kept both halves of the contract (swallowed **and** reported) by enabling the flag for that case, rather than deleting the awkward assertion. **This is the third instance this session** — `worktree-pool`, `self-healing`'s auto-archive line, and now this. If a fourth appears it deserves a shared test helper rather than three bespoke fixes. ## Two failing files I could NOT responsibly take — flagged, not touched **`executor-prompt.test.ts` (3 failures) — I ESCALATED THIS AND I WAS WRONG. Retracting.** I flagged these as a possible real pause-contract violation: an agent session spawning while an operator has globally paused the engine. I then finished the diagnosis, and the evidence goes the other way. Recording the retraction with the same detail as the alarm, because a false alarm aimed at another unit costs them a chase. **The discriminator I asked for, resolved.** Six tests in that file assert `expect(mockedCreateFnAgent).not.toHaveBeenCalled()` during global pause; 3 fail. Splitting them by what they drive: | Assertions | Drives | Result | |---|---|---| | `does not resume unpaused in-progress task while global pause is active` (+2 siblings) | no executor method — `task:updated` / resume paths | **pass** | | `parks todo tasks in in-progress when fn_task_done…` (+2 siblings) | `executor.execute(...)` **directly** | **fail** | So the guard holds on every event-driven path and is absent only from the direct `execute()` entry. **And `execute()` is not the guard site — the scheduler is.** `scheduler.ts:1491` is an explicit hard stop (*"Global pause (hard stop): halt all scheduling activity"*), with a second gate at `:1055`, and the scheduler never calls `.execute(` at all — dispatch routes through the runtime. In production a global pause halts scheduling before anything reaches the executor. **Conclusion: the pause contract is intact in production.** The 3 failing tests call `execute()` directly, bypassing the upstream gate, and assert a defence-in-depth check *inside* `execute()` that is not there. They are testing a path production does not take during a pause. What that leaves is a real but much smaller question, and a design one rather than a defect: should `execute()` carry its own pause check as defence-in-depth, given non-scheduler callers exist (self-healing, manual retry)? If yes, add the guard and all six assertions pass. If no, the 3 direct-`execute` assertions are asserting a guarantee the architecture places elsewhere and should be retired. **I have not changed either the code or the tests** — but nobody needs to hunt a pause-contract regression, because there isn't one. **`executor-fast-mode-workflows.test.ts` (1 failure) — mechanism not isolated.** `visitedNodeIds` is `['review']` where the test expects `['start','review']`. Three probes failed to explain it: giving the review node an explicit `column: "in-progress"` changed nothing (so it is not column-based entry resolution), and swapping `seam: "review"` for a plain prompt config did not isolate it either. Two structurally identical sibling tests in the same file still pass with `['start', ...]`, so something in graph traversal distinguishes them that I did not find. That is U8/graph-executor territory; I am not asserting a `visitedNodeIds` shape I cannot explain. ## Still open and green - **#2637** — `task-delete-notice` 21 failed → 34 passed. - **#2643** — shellout allowlist re-pin. **Merge early:** it re-drifts whenever `executor.ts`/`self-healing.ts` line counts shift, with no git conflict to warn you. It already drifted once while open (`executor.ts:17106 → 17198`) and I re-pinned it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
6ed284f36a |
drop the dead semaphore parameter from dropPreHeldExecutorSlot (#2574)
Small follow-on to the cross-project cap removal. `dropPreHeldExecutorSlot(taskId, semaphore?)` released a cross-project semaphore slot. That semaphore is deleted, and **all 16 production call sites passed `this.options.semaphore`**, which nothing wires any more — so the release was a no-op on an always-undefined value: an optional parameter that reads as if it does something. ## What is *not* deleted Pre-held slots are **dual-purpose**: a cross-project semaphore slot **and** the FN-8453 per-project coordinator reservation. Only the first is gone. The reservation is the half that matters — every rejection path funnels through this helper so an early scheduler/triage return cannot permanently consume a project slot — and it stays. That is why this is a parameter change, not a helper deletion. Sites that still hold a semaphore reference release it **explicitly** next to their drop, so behaviour is unchanged for any caller that supplies one. Nothing wires one in production today, but silently leaking a slot for a caller that does is not a trade a cleanup is allowed to make. ## One real leak fixed — found by a failing test, not by reading `ProjectAdmissionCoordinator.admitOldest`’s release lambda took the pre-held branch and **returned**, relying on the deleted parameter to hand the host slot back. With the parameter gone, that branch unwound the registration and the reservation while **leaking the host slot** the attempt had acquired. The release is now unconditional across both branches. Worth noting how it surfaced: the test that caught it (`drops a declined candidate’s pre-held executor slot`) asserted `semaphore.activeCount`, which I had initially assumed was just coupling to the deleted half. It was not — it was pinning a real invariant. ## Tests Five cases in `concurrency.test.ts` pinned `sem.activeCount` through a drop. Each is re-pointed at the surviving contract — registration and reservation unwound, nothing left for a later pass to “take” — with the semaphore assertions moved to the sites that now own the release. ## Verification `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green · `concurrency.test.ts` **56/56**. The 8 `triage.test.ts` failures are **pre-existing** — reproduced identically with this branch’s `triage.ts` replaced by main’s. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
3aa942ee5f |
capacity: spawned agents count against the project agent count (#2579)
Two configurable numbers per project. `maxSpawnedAgentsPerParent` (5) and `maxSpawnedAgentsGlobal` (20) were a **third and fourth** limiter with private budgets invisible to both. ## This closes a hole, not just knobs A spawned child **is** an agent and gets **its own git worktree** (branched from the parent’s — the tool’s own description says so), but children were counted by **neither** capacity gate. A fan-out could put up to 20 extra worktrees on disk while the scheduler believed the project was at its configured limit. The operator’s two numbers were simply wrong about what was running. ## The old caps also measured the wrong thing `totalSpawnedCount` decrements on child cleanup, but the per-parent **set** is cleared only when the **parent task** ends. So `maxSpawnedAgentsPerParent` throttled *cumulative* spawns across a task’s life rather than *concurrent* ones — a long-running task could exhaust its budget with five children that had all long since finished, and the operator had no way to see why. ## Fix `fn_spawn_agent` gates on the same project agent count every other lane uses (`computeTopLevelConcurrencyClaimedFromStore`) plus live children. One number, one answer, no private budget that can disagree with the board. The refusal names **Max Concurrent Tasks** — a control the operator actually has. The old messages pointed at settings that no longer exist, which is worse than no message: it sends someone hunting for a knob that is not there. ## Verification **Revert-proof, measured:** restoring the private budgets turns **3 of the 4** new cases red — a project at 1/1 could still spawn, which is precisely the hole. `executor.ts` restored byte-identical. `pnpm lint` clean · core + engine `tsc` clean · `pnpm test:gate` green (414 + 10 + 71) · new suite 4/4 · `settings-default-descriptions` 4/4. There was no spawn-capacity test before this; the file is new. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Spawned agents now count toward the project’s **Max Concurrent Tasks** capacity. * Agent spawning is blocked when capacity is reached, including concurrent spawn attempts. * **Bug Fixes** * Prevented over-allocation during simultaneous agent spawns. * Restored available capacity when agent creation fails. * **Changes** * Removed separate per-parent and global spawned-agent limits. * Updated settings to reflect the revised capacity controls. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
152fedbd32 |
record the detector audit: gridlock and stuck-task are keeps, with evidence (#2581)
Answering the review question *“does gridlock detection still have a job?”* — with evidence rather than assumption, and recording it so the question is not re-opened by someone reading the name. **No behaviour change.** Comments only. ## Gridlock detector — KEEP `GridlockEvent.reasons` is typed `"dependency" | "overlap"`. It detects **dependency deadlock** and **file-scope overlap deadlock** via the scheduler’s `pathsOverlap` / `filterPathsByIgnoreList`. That has nothing to do with limiters arbitrating against each other — two tasks can still block on a dependency cycle or a shared file scope no matter how many agents the operator allows. The hypothesis that gridlock ≈ competing limiters deadlocking was reasonable from the name, and wrong. ## Stuck-task detector — KEEP Detects a stuck **agent** — a live session repeating the same tool call, or emitting no activity signal — via tool fingerprints and inactivity windows. Orthogonal to how many agents may run: a single agent on an unlimited board can still wedge. ## Evidence Measured for both: **zero** references to `maxConcurrent` / `maxWorktrees` / `semaphore` / `capacity` / `slot`. Both are live and wired — gridlock via `project-engine.ts → notifier.notifyGridlock`, stuck-task via `in-process-runtime.ts`. The note lives in each file because the natural reading of “gridlock” is “limiters deadlocking”, and deleting a live detector on that reading would remove real coverage silently. Each note states the question a future cleanup should actually ask — *is dependency/overlap deadlock still possible?* — rather than *is capacity simpler now?* `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green · detector suites **108/108**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Phil Larson
|
15b21dead1 |
fix(dashboard): reconcile task state through live API (#2595)
## Summary - add a project-scoped live API route for updating individual task checklist steps - add an atomic live API route for resolving stale durable wedge episodes - prevent operator repair tooling from opening a second embedded store that can diverge from the running dashboard backend ## Why Legacy graph-native workflow runs can retain successful `workflowStepResults` while their narrative checklist remains at 0/N. The existing `fn task update` fallback may open a separate embedded store, producing split-brain writes that do not accumulate in the live dashboard backend. There was also no API surface for the existing atomic wedge-episode resolver. ## Verification - `pnpm exec vitest run src/routes/__tests__/register-task-workflow-routes.step-update.test.ts` — 5/5 passing - `pnpm build` in `packages/dashboard` — passing - full managed runtime workspace build — passing - deployed to the managed local runtime and used to reconcile six legacy review-deadlock tasks - live board audit: zero `in-review-stall-deadlock` paused reasons - exact local and Tailscale dashboard roots: HTTP 200 with 16,926-byte bodies <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added live API endpoints to update individual task checklist steps with validation (step index and allowed status values). * Added an endpoint to reconcile/resolve stale task “wedge” episodes, resolving only the matching active episode and returning conflicts on mismatches. * **Tests** * Expanded route tests for step updates and wedge resolution, including consistent 404 behavior for soft-deleted and missing tasks, plus conflict and invalid-input cases. * Expanded PostgreSQL coverage for wedge resolution persistence and concurrent episode replacement scenarios. * **Bug Fixes** * Improved task-lookup error handling so soft-deleted tasks are consistently treated as “not found” (HTTP 404). <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
0c07584d51 |
U11 fallout: disprove the coding-ideas column collapse, and correct a U11 note that recorded the merge backwards (#2651)
Two findings, no behavior change. Both are about **recorded reasoning that was wrong** — the kind that sends the next person the wrong way. ## 1. The coding-ideas column collapse does not work (IR change reverted) I implemented it — deleted `ideas`, moved its `intake`/`autoTriage: false` onto Planning, repointed the `start` anchor, updated the IR suites to the merged shape (they went green, 44/44). Then the wider suites failed and showed why it cannot work. **The manual gate IS the column boundary.** `replan-target.ts` names the discriminator in its own comment: *"The real discriminator is which lane the triage service SCANS, which depends on the intake column's `autoTriage` config."* So `ideas` is unscanned, `todo` is scanned, and "promote" means moving the card from one into the other. Merge them and one column must be both: | if… | consequence | |---|---| | `autoTriage: false` wins | never scanned → nothing is ever planned → the capacity hold releases an **unplanned** card into `in-progress`, violating FN-7648 | | scanning wins | `autoTriage: false` is meaningless → the manual gate is gone → the preset duplicates the default Coding workflow | **8 tests fail, and they are not fixtures** — they encode the promotion flow itself, e.g. `store-create-intake-column.test.ts` › *"promotes an Ideas-parked task to todo without planning it (still bootstrap-stub PROMPT.md)"*. Rewriting them would have meant inventing what "promote" means with no destination column, which is how a broken flow gets blessed by a green suite. **What it would actually take:** a promoted flag the triage scan reads, so one column can hold both "not yet promoted" and "being planned". That is a new lifecycle signal, not a column merge — the same shape as the deferred `needs-replan` follow-up. Happy to scope it. **I also corrected my own earlier checklist** in this doc, which said to delete the now-dead `isUnplannedStartCreate` arm. Wrong: `autoTriage` is a general trait field (`builtin-traits.ts`), so any custom workflow can declare a manual intake with `intake !== hold`. The arm is dead only for this preset. ## 2. `replan-target.ts` recorded the U11 merge backwards The note claimed U11 deletes `todo` and keeps `triage`. It is the reverse — Shape B kept the id `todo` and deleted `triage`, precisely so the ~120 `column === "todo"` guards kept their meaning and no data migration shipped. The default lineage now declares `todo, in-progress, in-review, done, archived`. The lookups are correct today, but **for the opposite reason to the one recorded**: the default lineage falls *through* the `triage` lookup and lands on `todo`, its merged planning column. `triage` still matches the workflows that genuinely declare it (Lead generation, PR review). Also flagged without changing (it would be a behavior change): the `return "triage"` fallbacks on the no-match and throw paths name a column the default lineage no longer declares, so a workflow with neither `triage` nor `todo` gets a nonexistent target. ## Census **Unchanged: 781 total, triage 5.** This PR adds no guards and converts none — `workflowHasColumn(ir, "triage")` is a call argument, not a comparison, so it is outside what the census counts either way. ## Verification 41/41 engine replan-target suites (including the existing `replan-target-merged-planning-column` suite that covers the corrected behavior) · engine typecheck clean · the reverted IR restores the tree to main's content for those three files, verified by `git checkout --`. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
22a66c3a51 |
fix(test): re-pin the blocking-shellout allowlist after source lines moved (5 lines, 0 new sanctions) (#2643)
**A failing ratchet, fixed without widening it.** 5-line diff, no
production change. `pnpm test:gate` green.
`engine-no-blocking-shellout.test.ts` was red on main, reporting **5
unaudited synchronous shellouts** (1 in `executor.ts`, 4 in
`self-healing.ts`).
## No new violation — the allowlist went stale
The allowlist is keyed by `(file, LINE, primitive, signature)`.
`self-healing.ts` shrank during the U4 extractions, so the recorded
lines drifted. Every flagged signature was **already sanctioned**:
self-healing's three sat at 4445/4451/4488 and are now at
4127/4133/4170. The file carries an FNXC note for precisely this case —
*"Re-pin all audited shellouts after current main moved source lines
without changing the sanctioned short-git-plumbing calls."*
**Re-pinned by signature, not by hand:** for each of the 33 entries,
find the line whose trimmed text equals the recorded signature and take
the occurrence closest to the old line — which stays stable when a
signature repeats, e.g. `merger.ts`'s six identical `git reset --merge`
calls. Result: **5 lines moved, 0 signatures unfound**, so nothing
became sanctioned that was not sanctioned before.
## A wrong turn worth recording
I first read the guard's *"only after proving timeout and maxBuffer
bounds"* as applying to every sanctioned site. I checked all 5, found
none had `timeout` or `maxBuffer`, and was about to add bounds to
`executor.ts` and `self-healing.ts` — an unnecessary production change
in two files other workers are actively editing.
Re-reading the guard's own comment corrected it: the bounds criterion
belongs to `BOUNDED_GIT_DIFF` (data-dependent diff output, where the
buffer can grow with the repo), not to `SHORT_GIT_PLUMBING`. All 5 are
`rev-parse` / `merge-base --is-ancestor` / `rev-list --count` / `branch
--list` — fixed-size output, already the sanctioned category.
## Verified the ratchet still bites
A re-pin could silently widen a guard, so I checked rather than assumed:
injecting an unaudited `execSync("git log --all")` into
`integration-branch.ts` **fails** the ratchet and it names the offending
signature; reverting returns it to green.
## Why this one was worth taking
A red ratchet is the worst failure mode for a guard — it stops being a
signal and starts being noise someone silences. This one had already
caught a real class of defect (unbounded sync shellouts on the shared
event loop), and it was red for a purely mechanical reason.
`workflow-lifecycle-live-e2e` (#2634, merged) and
`executor-review-verdicts` (#2641) clear two more of main's 13 failing
engine-default files; this is a third.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
8d3b8262c0 |
U2b: the second move-path divergence — legal targets differ, not just message shape (blocks the useWorkflow flip) (#2638)
Tests only. Advances U2b's equivalence proof **without touching `moves.ts`**, whose edit order is still being agreed between U12 and MAIN. ## Why this one decides the sequencing The equivalence suite already records one divergence: rejection **type and message** differ. That is a shape difference and easy to absorb. This second one is a difference in **which moves are legal**, and it is workflow-dependent. U11 removed `triage` from the default coding lineage, so rows left there sit in a column their own workflow no longer declares. #2515 added an escape hatch to `resolveAllowedColumns` so such a card has a legal move — its workflow's rebound target — instead of `Valid targets: none`. **That hatch lives inside the `useWorkflow` block, so it only runs on the hooks path.** Mutation-verified in #2597: stubbing it back to `[]` left an operator-move test green, because the inline path answers from the legacy `VALID_TRANSITIONS` map instead, whose `triage` row happens to permit the move for unrelated reasons. So **flipping `useWorkflow` changes move validation for every stranded card**, not just side-effect routing. ## What that means for "flip the flag, then delete the flag-OFF branch" That plan is not the mechanical cleanup it looks like, and KTD-6's Phase A escalation correction already ruled on this exact shape once: > Deleting the inline branch would have swapped every project onto an untravelled code path and called it a cleanup. > > The convergence is its own unit with an equivalence proof (**U2b**), and it **blocks Phase B**. Nothing downstream may assume the trait-hook path runs until it lands. Flipping the flag and deleting the other branch *is* that deletion, reached from the other side. The tests won't catch a divergence because both paths have tests and only one of them runs — which is precisely why U2b was scoped as a proof rather than a refactor. **Recommended order:** U2b's equivalence proof completes → U12 flips `useWorkflow` → the flag-OFF branch and its 5 guards go away wholesale. That still gets the "strictly less work" outcome, just after the proof instead of instead of it. ## A note on how this is asserted Deliberately a **positive assertion about the inline path**, not a comparison of two target lists. The two paths do not reach the same rejection — inline enumerates the legacy table and reports it in the message; hooks throws the typed unknown-column rejection first. That asymmetry **is** the divergence. Comparing two lists would hide it behind two empty arrays and read as equivalence, which is the failure mode this suite exists to prevent. 11 tests green; lint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
14c73ab727 |
U11 [tool-availability + skill-resolver + cli/task]: name the 3 literals that are NOT columns (48 -> 45) (#2619)
**Taking: `engine/tool-availability.ts`, `engine/skill-resolver.ts`, `cli/commands/task.ts`** — the three census hits that are not board columns. ## Census | file | before | after | |---|---:|---:| | `packages/engine/src/tool-availability.ts` | 1 | **0** | | `packages/engine/src/skill-resolver.ts` | 1 | **0** | | `packages/cli/src/commands/task.ts` | 1 | **0** | | **repo total (comment-stripped)** | **48** | **45** | ## These are not lifecycle guards — converting them would have been wrong - **`tool-availability`** — `surface: "triage" | "executor"` is an **agent lane**. The lane that writes specs keeps its name whatever the board calls its planning column. Resolving it from a workflow IR would make an agent's prompt depend on board configuration. - **`skill-resolver`** — `sessionPurpose === "triage"` is an **agent role**. Same argument: a role doesn't move when a board renames a column. - **`cli task list`** — the glyph chain distinguished **active** columns from the rest and nothing else; all four active ids mapped to the same `●`. Each is now named (`AgentResearchSurface`, `ROLE_FALLBACK_SESSION_PURPOSES`, `ACTIVE_COLUMN_GLYPH_IDS`) so the next person working the census sees at a glance that they're out of scope, rather than re-deriving it as I had to. ## A real divergence my own equivalence test caught I first wrote the glyph as the tempting inverse: ```ts const dot = col === "done" || col === "archived" ? "○" : "●"; ``` That is equivalent across all six lifecycle ids and **not** equivalent for anything else — the original chain fell through to `"○"` for an unrecognised id, while the inverse renders it as **active**. The loop only walks the six `COLUMNS` today, so nothing would have caught it in practice; a renamed workflow reaching this code later would have silently changed how its columns render. Shipped as an explicit ACTIVE set that mirrors the fallthrough exactly. The test asserts equivalence over the six ids **and** over unknown ids, which is where the difference lives. That's the point of testing a "pure rename" at its edges rather than only where it's currently exercised. ## Verification - 71 tests green across skill-resolver / heartbeat-skills / tool-availability / the new equivalence suite - merge gate green (482 + 132 + 10), engine + CLI tsc clean, lint clean ## Note on the remaining count Of the 45 left, `replan-target.ts` (2), `board-workflows.ts` (2) and `archive-planning.ts` (1) show up in a **raw** grep but are **0** real — every hit is inside a comment. A raw grep reports 53; comment-stripped is 45. Real remaining work concentrates in `self-healing.ts` (11), `register-task-workflow-routes.ts` (6), and the parked `moves.ts` / `default-workflow-hooks.ts` (9). No changeset: `@fusion/engine` is private; the CLI change is display-identical. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## ⚠️ Read before merging — these are ROLE renames, not column conversions The coordinator's hand classification says several literals in this PR "must be left exactly as they are" because they compare an **agent role**, not a task column, and resolving them to a column trait would be a bug. **I agree, and this PR does not do that.** What it does: replaces a bare `=== "triage"` with a **named role predicate** — `isPlanningAgentLane`, `AgentResearchSurface`, `ROLE_FALLBACK_SESSION_PURPOSES`. Behaviour is **byte-identical** for every input. No IR is consulted, no trait is resolved, no column is involved. The reason to keep it rather than revert: the danger isn't the literal, it's that nothing at the call site tells the next person `"triage"` here means a *lane*. A list of exceptions maintained elsewhere only helps someone who finds the list; a call named `isPlanningAgentLane` helps whoever is reading the line. It also shrinks what the #2630 ratchet's ignore list has to carry. Reversible: if the preference is to leave the literals untouched, say so and I'll strip these hunks — but then the ratchet's ignore list must carry **all twelve** role sites or it can never reach zero, because those six are correct code. ## Classification finding Bucketing by the **receiver** of the comparison (not the literal) mechanically separates guards from roles, and it found **six role sites currently listed as "real column guards"**: | site | receiver | what it actually is | |---|---|---| | `usage-limit-detector.ts:144, 207` | `agentType` | agent type — the column test one line above is *already* trait-driven | | `skill-resolver.ts:432` | `sessionPurpose` | session purpose | | `tool-availability.ts:32` | `surface` | agent surface (`"triage" \| "executor"`) | | `effective-model-resolution.ts:148` | `entry.agent` | agent-log lane | | `useTasks.ts:162` | `entry.agent` | agent-log lane | So the real bar is roughly **39**, not 45. The rule that found all twelve without judgement calls: `column`/`toColumn`/`taskColumn`/`c` are guards; `role`/`agent`/`agentType`/`surface`/`sessionPurpose` are not. Worth teaching #2630's ratchet directly. |
||
|
gsxdsm
|
f91b8a4178 |
TAKING mission-feature-sync.ts: roadmap reconciliation resolves lifecycle roles (unowned drift site) (#2602)
> **Taking `packages/engine/src/mission-feature-sync.ts`** from the shared backlog — announced in the title per the collision protocol. Based on `main`, no dependencies. It is in **no unit's file list**: absent from the plan's per-file census *and* from the drift review's ownership split (self-healing, dashboard, triage/replan-target, core, executor). It is a planning-lane reader. ## What was broken `reconcileMissionFeatureState` maps a task's lifecycle **position** onto its mission feature's roadmap status, and read five column literals: `done`, `archived`, `in-progress`, `in-review`, `triage`/`todo`. On a renamed workflow **every branch answers "no"**, so the function collapses to a permanent `noop`. **What an operator sees:** a mission roadmap frozen at whatever status it last held, while the tasks underneath it run to completion. Nothing errors, nothing retries. Worse than a wrong status, because a stale roadmap reads as a stable one. ## Guard counts (per the reporting requirement) | Metric | Before | After | |---|---:|---:| | `column === / !== "triage"` in this file | **1** | **1** | | role comparisons converted | — | **5** | **The metric does not move here, and I am not claiming it does.** The five role comparisons are converted; the one literal that remains is the deliberate scoped migration acceptance this change *adds*. That is the third time on this program the real fix has been invisible to the convergence count — the count finds the site, it does not define done. Worth knowing while the shared backlog is being tracked by that number: repo-wide it currently reads **29** triage comparisons (including 4 in `plugins/`, which are also unowned). ## Fallback direction matters **Unresolvable workflow falls back to the legacy ids, not to `noop`.** A mission whose workflow cannot be read should keep tracking on the default vocabulary rather than go silent — going silent *is* the failure being fixed, so the fallback must not reproduce it. **The planner-lane branch also accepts an orphaned legacy id.** A pre-existing test asserted a card in `triage` returns its feature to `triaged`; that stopped holding for the default lineage after #2515 — the migration-window population again. Accepting `triage`/`todo` additively keeps those rows tracked, **scoped to ids the workflow does not declare**, for the reason greptile gave on #2593: a custom workflow may legitimately name its **review** lane `triage`, and mapping a card there to `triaged` would walk the roadmap backwards while the task is awaiting merge. ## A test of mine that proved nothing until fixed The scoping case first used a `triaged` feature. The planner-lane branch only fires for an **in-progress** feature, so the fixture fell through to the review branch and **passed under both implementations**. It discriminates only once the feature status lets the wrong branch win — verified by reverting the scoping and watching exactly that case fail. ## Revert proofs | Reverted | Result | |---|---| | all five literals restored | **5 of 15 fail** — every renamed case; every default case passes | | legacy acceptance unscoped | **1 of 15 fails** — the custom-`triage`-as-review case | ## Verification | Check | Result | |---|---| | new suite | 15/15 | | pre-existing mission-feature-sync + mission-autopilot + scheduler-trait-dispatch | 94/94, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (482 + 10 + 71) | | `pnpm check:changesets` | clean | ## Next from the shared backlog Taking `plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts` (3) and `plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx` (1) next — 4 sites in `plugins/`, which no unit owns and which the #2587 ratchet now scans. Shout if anyone is already there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
347107d8e4 |
Phase B — TaskContextMenu.tsx: intake by ROLE (2 → 1), and two conversions I dropped rather than force (#2626)
**Claimed:** `packages/dashboard/app/components/TaskContextMenu.tsx`
| file | before | after |
|---|---:|---:|
| `TaskContextMenu.tsx` | **2** | **1** |
## The real bug
`shouldShowActionsMenu: task.column !== "triage"` meant *"a bare card in
a pure intake lane has no actions worth showing yet."*
Post-U11 the literal does not go dead — it **inverts**. A default
Planning card is `todo`, so the condition is true and the menu shows
unconditionally. That is right for the hold half (cards waiting for
capacity do have actions), but the guard has stopped distinguishing
anything — and it would show a full action menu on a bare Coding (Ideas)
capture, which is the case it existed to suppress.
Resolved to `intake AND NOT hold` — a *pure* intake lane — which
reproduces all four shapes rather than picking a winner:
| workflow | column traits | menu |
|---|---|---|
| legacy `triage` | intake only | suppressed *(as before)* |
| legacy `todo` | hold only | shown *(as before)* |
| merged Planning | intake + hold | shown *(matches the Todo half, where
cards wait)* |
| Ideas `ideas` | intake only | suppressed *(a bare captured idea)* |
Its degraded arm now defers to `isIntakeColumnRole`, so the legacy
intake id lives in `utils/columnRoles.ts` only.
## The remaining site is audited, not overlooked
I routed `isPreExecutionHoldColumn` through
`isPreImplementationColumnRole` — same question, one definition — **and
then reverted it.** Its degraded-mode answer is wider: its legacy set is
`{todo, triage}`, this predicate's was `{triage}` alone.
They differ **for a reason.** That helper drives the preserve-progress
prompt, where a flagless `todo` *should* prompt because losing steps is
unrecoverable. This one drives the Plan affordance, where a flagless
`todo` must **not** offer to re-plan a card that may already be planned.
Consolidating added `plan` to flagless `todo` cards — caught by
*"exposes Plan only for pre-execution hold columns"*. Identical trait
path, non-interchangeable fallbacks. Kept separate with the difference
recorded rather than made to look shared.
## Two conversions I dropped rather than force
**1. A `ListView.tsx` 5 → 0 conversion.** Main changed underneath it:
the U12 worker centralized the same fallbacks into
`utils/columnRoles.ts`. Their approach is on main and other files
already call it, so I took theirs and dropped mine rather than fight for
my version through a rebase conflict.
**2. A `strandedColumnFlags.ts` seam** that resolved an undeclared
column's role from the workflow's **rebound target**, so the degraded
arms could be *deleted* rather than documented.
I built it, tested it, wired it into ListView — and then their
`columnRoles.ts` identified a state my seam cannot serve: the **pre-load
window**, where the board renders before the workflows fetch resolves
and there are no columns at all, hence no rebound target to borrow from.
Their analysis is more complete than mine, the fallback is genuinely
undeletable, and shipping an unused module is worse than shipping
nothing.
Worth recording because I twice reported these arms as permanently
unconvertible, then thought I had a way to convert them, and was wrong
for a reason worth knowing: **there are two degraded states, not one.**
The stranded-card half is resolvable; the pre-load half is not.
## Verification
10 of 11 green in this suite. The one failure — `"Back to in-progress"`
vs `"Back to In Progress"` — is **pre-existing**, verified by stashing
this change and re-running against clean `main`.
Dashboard app typecheck and lint clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
1d0f21b428 |
U12 R12: lifecycle-column literal ratchet — and the raw count's floor is not zero (#2630)
The anti-regression ratchet U12 R12 calls for. Counts lifecycle-column **literal comparisons** in production source and fails when the count rises. ## Two jobs **1. Ratchet.** A converted guard cannot silently come back as a literal. Ceilings only go down. **Mutation-verified both ways:** adding one `t.column === "triage"` fails with *"rose to 49 (ceiling 48)"*; lowering the ceiling to 47 fails with *"rose to 48 (ceiling 47)"*. The number is exact, not approximately right. **2. Honest denominator — the finding.** `triage` is overloaded in this codebase: a column id, an **agent role**, a **session purpose**, a **prompt-template family**, and a **CLI glyph key**. A raw grep counts them together, which makes "reach zero" unreachable by construction — converting `role === "triage"` in `agent-prompts.ts` would break the planning agent's prompt-template resolution, and the failure would look nothing like a column bug. | | count | |---|---:| | raw `triage` matches | 72 | | **not a column at all** | **10** | | genuine column comparisons | **48** | The 10: `agent-prompts.ts` ×3 (`role`), `usage-limit-detector.ts` ×2 (`agentType`), `skill-resolver.ts` (`sessionPurpose`), `tool-availability.ts` (`surface`), `cli/commands/task.ts` (a glyph key), plus two in comments. Ceilings recorded for all four ids — **`in-progress` (133) and `in-review` (200) were untracked entirely.** ## A measurement error of mine that writing this caught A grep over `packages/<pkg>/src` **misses `packages/dashboard/app`**, where the board components live. That undercounted `triage` as 43 in an earlier audit of mine when it was 62. The source roots are now listed explicitly in code so the number cannot drift with someone's glob. ## The classifier is under test, not trusted A ratchet that matched nothing would pass forever while measuring nothing — the failure mode this program keeps finding. Three self-tests prevent it: - it asserts **positively** that `agent-prompts` / `skill-resolver` / `tool-availability` are excluded, so a classifier change that swallowed them would fail rather than quietly shrink the number; - it asserts the classifier still **matches** real column comparisons; - it pins `live-agent-count.ts`'s two sites as **permanent** no-flags fallbacks, with the reason, so a future edit that deletes them has to argue with it rather than silently drop stranded cards from the footer's queued total. The classifier keys on the **left-hand side naming a column** — deliberately syntactic, so a reader can audit it against the source without running anything, and conservative: an unrecognised shape counts **as** a column comparison, erring toward demanding conversion rather than excusing it. 7 tests green; lint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
bcb9782d3f |
fix(test): re-green task-delete-notice after the SQLite-arm deletion (21 → 0) (#2637)
**Unowned work, picked up.** `task-delete-notice.test.ts` was **21 failed / 13 passed** on main → now **34 passed**. Test-only. `pnpm test:gate` green, `pnpm lint` clean. ## Cause `deleteTaskImpl` and `deleteTaskIfImpl` are now **thin delegators**. The SQLite arms were deleted in the PG cutover (`FNXC:SqliteDualPathCleanup 2026-07-26`) and both forward unconditionally to `store.deleteTaskBackend` / `store.deleteTaskIf` — `deleteTaskImpl` is literally *"throw if self-delete; return `store.deleteTaskBackend()`"*, with **no `backendMode` branch left**. The suite drove them against a `makeSqliteStore` fake providing neither method, so every case threw `store.deleteTaskBackend is not a function` **before reaching any notice logic**. All 21 failures were measuring a crash, not a decision. ## Fixes - the PG fake gains `deleteTaskBackend` / `deleteTaskIf`, wired to the **real backend impls** rather than stubbed — so the delegating paths still prove the delegation preserves the notice decision instead of asserting against a mock; - plus `withTaskLock` (`deleteTaskIf` wraps the conditional delete in the per-task lock), running the body inline so the predicate and short-circuit paths execute for real; - every remaining `makeSqliteStore` call site retargeted, and the now-dead factory **deleted** so it cannot rot back in. ## Corrected a claim the file was making The header's Surface Enumeration said the three paths prove *"the behavior cannot depend on backend mode"*. **There is one backend now.** The enumeration is still worth driving — a caller reaching the public entry point must get the same notice as one reaching the backend directly, and these paths prove exactly that — but it is a different claim, and the file now states it, with the two paths renamed from `(SQLite)` to what they actually are. ## A bug in my own patch, caught by re-running My first edit inserted the method assignments **after** the `return`, so they were unreachable and the symptom didn't change. I only found it because the failure count stayed identical and I checked the file instead of assuming the edit had landed. Worth noting because "the patch applied" and "the patch took effect" are different facts, and this session has now produced three variants of that same mistake. ## Deliberately not fixed here `task-delete-caller-attribution` (13 failed) and `task-delete-nonblocking-cleanup` (2 failed) share the root cause, but their `makeDeleteStore` fake carries `backendMode: false` and lacks the PG surface the real backend impl needs (`asyncLayer` / `transactionImmediate` / `rowToTask` …). Wiring them means either building that surface out or re-pointing the suites at `deleteTaskBackendImpl` directly — a judgement about what those suites are *for*, and worth making deliberately rather than folding into this fix. Whoever owns the PG cutover cleanup will know which; the diagnosis above is the whole of it. Verified no collateral: `task-merge` and `legacy-adoption` unaffected (166 passed across the three files). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
9edc746f96 |
E2E evidence: the MERGED board (third completion criterion) — plus a RETRACTION of my #2613 escalation (#2632)
This is the merged-board half of the evidence assignment. **It is red, deliberately, and the red is the finding.** Do not merge it to make the red go away — the assertions are correct and `main` is broken. ## Escalation first: #2613 broke the default board, and the gate did not notice `6a33d8f8c` — *"Phase B — TAKING task-creation.ts: intake classification by trait (4 sites → 0)"* (#2613) — regressed four E2E cases, including **the default-vocabulary full lifecycle**, which is scenario 1 of the whole E2E assignment. Attribution is a clean single-file revert, not a guess: ``` HEAD (main): 4 failed | 39 passed HEAD with ONLY 6a33d8f8c's task-creation.ts reverted: 28 passed (both files fully green) ``` Failing: 1. `scenario 1 — DEFAULT vocabulary … persists the card in the expected column at every stage` 2. `scenario 2 — RENAMED vocabulary … writes the same column-transition audit trail as the default` 3. `releases a card out of the merged lane on capacity — the release is not a self-move` 4. `does not re-release a card that already left the merged lane` **`pnpm test:gate` is green on this branch — exit 0, 695 tests.** #2613 merged through a green gate, and its own tests pass. This is the eighth time this program a test has passed without exercising its subject, and the first one an E2E family caught rather than review. ### Mechanism `isIntakeColumn` in `task-creation.ts` decides whether a new card gets a **bootstrap** prompt (freeform, "triage will plan this later") or a **specified** prompt (planned, executable). #2613 rewrote it as: ```ts const isIntakeColumn = (intakeFacts.intake !== undefined && task.column === intakeFacts.intake) || … ``` where `intakeFacts.intake` falls back to the **default workflow's** intake when the create supplies no `workflowId`. Post-U11 the default workflow's intake **is `todo`**. So any card created directly in `todo` is now classified as intake and gets a bootstrap prompt — unplanned. Unplanned cards do not advance through the graph (no `NodeEntered` audit rows → failures 1 and 2) and hold-release will not release them (FN-7648: no unplanned card enters a processing column → failures 3 and 4). Before U11 this was safe: `triage` was intake and `todo` was a distinct lane, so creating in `todo` meant "planned work". The merge deleted that distinction. ### Why this is the exact trap you warned about You said you did not want *"a conversion that swaps the literal for a trait lookup WITHOUT checking what the guard was for."* The old `task.column === "triage"` guard meant **"is this card unplanned?"** On a merged board, intake-vs-hold **cannot answer that question at all** — one column is both. The distinguishing fact is not the column; it is whether the caller supplied a spec. Resolving the role faithfully still gets the wrong answer, because the question was never really about the column. Not fixing it from here: `task-creation.ts` is #2613's owner's file, and the fix is a design call about which fact replaces the column test. ## What the evidence itself adds Three families extended to the U11 shape — one column carrying **both** intake and hold. That breaks a class of guard renamed boards structurally cannot reveal: | shape | consequence | |---|---| | `intake && !hold` | **unsatisfiable** — silent | | hold → intake release | **self-move**, re-fires every poll — loops | | `intake && column !== "triage"` | inverts to **always-true** — silent | Two are silent and one loops, so every case sweeps **twice** and asserts no re-release; a single pass cannot tell a no-op from a self-move. ### A fixture that could not fail My first merged row used `MERGED_VOCAB`, which is *faithful* to U11 — it reuses the legacy ids, because that is what the default lineage has. That fidelity **destroyed its discriminating power**: its hold column *is* `todo`, so a guard falling back to the `todo` literal returns the same answer as one resolving the role. The "hold but not intake" mutation left all 23 green. Added `MERGED_RENAMED_VOCAB` — merged *structure*, renamed *vocabulary* — the only combination where the collapse is observable **and** the literal is wrong. Same mutation now fails exactly 1 of 23. Both vocabularies stay: one asks *"does the collapse break the release path"*, the other *"is the role actually resolved"*. One rebound mutation was **genuinely unobservable** rather than undetected — `hold` is also the first column in that fixture, so the fallback chain lands there regardless. Pointing rebound at `complete` instead fails 9 of 15. Recorded rather than papered over. ## For the CAPACITY worker before `self-healing.ts` is marked done `self-healing.ts:2952` and `:9134` query `listTasks({ column: "triage" })`. Converting the 10 guards leaves those sweeps **blind** — they never see a renamed card, so the guard is correct and unreachable. Query and guard convert together or not at all. There are **137** such `column: "<legacy id>"` sites repo-wide, 52 in that one file, and the 45→0 grep counts none of them because they are object properties, not comparisons. **The bar can reach zero with sweeps still unable to fire.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
7ab6506c0f |
docs(solutions): proving a code path actually runs — the five ways U8 shipped code that never executed (#2642)
Durable write-up of U8's verification findings. **Docs only — no code change, no CI risk beyond lint.** These currently exist only in PR bodies, which nobody greps. `docs/solutions/` is where this project keeps exactly this kind of thing, and every one of the five will recur: the handler-pair shape and the resolved-vs-guessed fork both have more call sites than U8 touched. ## The five 1. **Two prompt-node handlers exist; only one runs.** `createDefaultNodeHandlers` prefers the primitives handler whenever `deps.primitives` is set, and `executeWorkflowGraph` always sets it — so every seam entry in `createAuthoritativeWorkflowSeams` is unreachable for prompt nodes. A lifecycle announcement sat there through two PRs. It type-checked and its unit tests passed, because a seam-level test calls the seam object directly and therefore always can. 2. **A negative instrumentation result is worthless without a control.** No output from an instrumented seam is only evidence once you have shown writes from that module are visible under the harness. One `process.stderr.write` at module load separates "never ran" from "output swallowed" — opposite conclusions. 3. **Source-string ratchets prove syntax, not behavior.** Three were torn down in review. The sharpest guarded a never-executed-code bug with a source search, reproducing the bug one level up; measured, the behavioural version fails an inverted dispatch and the textual one passes it. Includes the sub-rules paid for the hard way: use the AST not regex (a brace in a string truncated an extraction to 13 lines and every count read a *passing* zero), guard the guard, anchor by index rather than a character window. 4. **A green test on first try, on a path with no prior coverage, is a warning.** Two conversions were reverted in one day because their tests passed with the change reverted. Negative assertions succeed trivially when the method returns early — `recoverCompletedTask` has seven guards before the converted line, and the fixture has to satisfy all of them. 5. **A named workflow selection is not a resolved one.** Provenance cannot be inferred from the returned value, because a fallback IR and a valid id-less IR are structurally identical — the resolver that knows has to report it. This is the fork every remaining lifecycle-column conversion hits. ## Why this rather than another conversion Everything left in my area is now owned and further along than I could take it: `executor.ts` → #2628 (which solved the `recoverCompletedTask` fixture I could not), `self-healing.ts` → #2560 (independently hit all three traps I catalogued), the dashboard cluster → #2625/#2626/#2636. Duplicating that would be motion, not progress. Turning findings that cost real cycles into something greppable is the useful thing I can still add. `pnpm lint` clean. No changeset — internal documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a best-practices guide for verifying that workflow code paths actually execute. * Covers reliable behavioral assertions, instrumentation controls, regression-proof tests, source validation, and detection of fallback behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
7397cea2dc |
U12: pin the move-path flag blast radius (6 seams, not 1) before flipping it (#2639)
Per the sequencing agreed in-thread — **U12 resolves the flag first, then the flag-OFF branch is deleted wholesale** — this is the step before the flip, not the flip. ## The flag is six switches, not one `moves.ts:363` reads the raw compatibility flag nothing in production source writes, and gates the hottest lifecycle path in the system. Every summary so far has under-scoped it, mine included: I described it as the `789`/`837` pair. Measured, it is six decision points: | seam | what flipping turns on | |---|---| | 392 | resolves the task's workflow IR — `undefined` when off, so **every IR-dependent guard below is inert** | | 489 | typed **rejections**: unknown-column and adjacency validation | | 789 | column side effects route through the trait hooks instead of the inline legacy block (timing, reset-on-entry, abort-on-exit, `merge.onEnter`) | | 1092 | writes the transition-pending marker that **capacity counting reads** | | 1330 | runs **plugin hooks** on column change | | 1395 | records `workflowId` on the emitted move payload | ## The risk is seam 2, and it is not an equivalence question With the flag off there is **no target-column validation on the move path at all**. Flipping introduces new refusals for moves that succeed today, on the path every engine lane uses. That is not "do the two implementations agree" — it is new behaviour, and a green suite is not evidence about it. `recoveryRehome` already carves out legacy targets (#1411); nothing proves the other callers are covered. ## What this PR asserts - **The seam count.** Mutation-checked, not assumed: replacing one gate with `if (true)` fails with `expected 5 to be 6`. (My first mutation attempt silently didn't apply — `str.replace` with no assert — so the anchor is verified now.) - **All six read ONE flag.** If a seam were rewritten to consult settings directly, a flip would move five behaviours and leave one behind, and nothing else in the suite would notice because both states are individually valid. - **The flag-OFF branch is still inline**, so the delete-with-the-branch step has a test naming the plan if someone converts its guards instead. - **The atomically-coupled second reader is named.** `workflow-task-create-ops.ts` computes the `movePolicyPreflight` that `moves.ts` consumes, so un-gating either alone either evaluates workflow move policies whose result is ignored, or validates against a preflight never computed. Comments cannot inflate the count — it is an AST walk. Parse failure fails loudly via `parseDiagnostics` rather than a try/catch, since `createSourceFile` is error-tolerant and a partial tree would undercount and read as "seams were removed". ## Preconditions for the flip, recorded in the file header 1. An equivalence proof for seam 3 across timing, reset-on-entry, abort-on-exit and `merge.onEnter`. **Neither implementation is the observed baseline** — they have never both run in production. 2. A census of the moves seam 2 would newly reject. 3. Both raw-flag readers flipped atomically. ## Why not just flip it here Because I cannot honestly claim the equivalence proof from a source read, and the flip is on every task move. Landing the blast radius as a test first means the flip PR has something to be proven against, and it means the next person cannot under-scope it the way this has been under-scoped four times. ## Verification `pnpm lint` clean, `pnpm test:gate` green (10 / 482 / 71), `tsc -p packages/core/tsconfig.json` clean, suite 5/5. No changeset: test-only, no behaviour change. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
50ebf3c543 |
TAKING cli/project.ts (fn project reported 0 running agents) + two test fixes — dashboard conversions WITHDRAWN in favour of #2626 and #2636 (#2631)
Three app-cluster conversions plus the evidence that they behave on a renamed AND a merged board. ## Per-file guard counts | file | before | after | note | |---|---|---|---| | `packages/cli/src/commands/project.ts` | 0 | 0 | not a comparison site — see below | | `packages/dashboard/app/components/TaskContextMenu.tsx` | 2 | 2 | **count does not move — deliberate, see below** | | `packages/dashboard/app/components/Column.tsx` | 2 | 2 | **count does not move — deliberate, see below** | **Read this before scoring the PR against the bar.** You said a claim that does not move your number is not done, so I am telling you up front that *this PR does not move it*, and why. Both dashboard conversions are **fallback-preserving**: ```ts const isIntakeColumn = columnFlags ? columnFlags.intake === true : column === "triage"; ``` The literal survives as the no-flags branch, so the grep still counts it. That is the shape the sibling code already uses (`isPreExecutionHoldColumn`, same file, converted earlier in the program), and dropping the fallback would make an unresolved-column render *lose* the affordance a second way. What changes is the **behaviour when flags exist** — which is what the mutation results below measure. If you want these to zero out the count, the fallback has to go, and that is a separate decision about whether an unresolved column should fail open or closed. Say the word and I will do it as a follow-up; I did not make that call unilaterally because it is not reversible from a rendering standpoint. `cli/project.ts` was never a comparison site at all — it fed **raw rows** to `isRunningAgentTaskShape`, so the helper's own internal legacy fallback kicked in and `fn project` reported **0 running agents** on any renamed board. Fixed by resolving the IR per task before counting. Nothing to subtract. ## Two of the three had a test that looked like coverage and was not - **`Column.tsx`** — the quick-create gate is `workflowMode || isIntakeColumn`. Every pre-existing intake case in `Column.test.tsx` *also* passes `workflowMode`, so the `||` short-circuited and **none of them ever reached the trait lookup**. Added cases that omit `workflowMode`, the only path where the conversion changes the answer. - **`TaskContextMenu.tsx`** — the intake suppression was asserted only for the legacy `triage` id, the one board shape where a broken conversion still returns the right answer. Mutation-verified rather than asserted: | mutation | result | |---|---| | `isIntakeColumn` → `column === "triage"` | **2 of 88 fail** (exactly the renamed and merged cases) | | menu suppression → `task.column !== "triage"` | **1 of 12 fail** | ## A pre-existing red I fixed on the way past `uses VALID_TRANSITIONS and in-review back-to-progress labels` was **already failing on origin/main**. #2521 correctly moved the "Back to X" label onto the host's `columnLabel` function; this file's stub is `(column) => column`, so the hardcoded `"Back to In Progress"` expectation was left over from the pre-#2521 hardcode and nothing had updated it. Matching the raw id would have made it pass while proving nothing, so instead that one case gets a display-like label function — the assertion now fails both if the "Back to" prefix regresses **and** if the label stops routing through `columnLabel`. Strengthened, not relaxed. Counts against completion criterion #2. ## Verification - `Column.test.tsx` + `TaskContextMenu.test.tsx`: **100 passed** - `tsc -p tsconfig.app.json` (the root config does not cover `app/`) and the CLI typecheck: clean - `pnpm test:gate`: green 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8e211d1870 |
TAKING scripts/: parse instead of grep — an AST classifier for the lifecycle-column bar, cross-checked by a second implementation (#2633)
The program's completion bar is "`column === "triage"` reaches zero".
This measures what that bar actually covers, and checks the measurement
in so it cannot drift.
## The number, measured by the checked-in tool
```
lifecycle-column-census: scanned 1956 source files
COLUMN guards (the backlog): 1031
ROLE comparisons (not guards): 10
DELIBERATE-LITERAL (reviewed): 4
by column id:
313 done
217 in-review
201 in-progress
177 archived
83 todo
40 triage
top files:
151 packages/engine/src/executor.ts
136 packages/engine/src/self-healing.ts
50 packages/dashboard/app/components/TaskCard.tsx
44 packages/core/src/task-store/moves.ts
34 packages/dashboard/app/components/TaskDetailModal.tsx
```
**`triage` is under 4% of the class.** Every one of those 1031 sites is
the same defect: a lifecycle decision made by column NAME, which stops
matching the moment a board renames a column. The bar can be met in full
while 991 identical guards remain — and two files hold a quarter of
them.
## The tracked count is wrong in three directions at once
Each of these cost real work this week, which is why this is a PR and
not a comment.
1. **Vocabulary.** It measures one of six legacy ids.
2. **Receiver.** It is anchored on locals named
`column`/`toColumn`/`fromColumn`, so it never saw the three real guards
in `executor.ts` written against `from` and `originColumn`. One of those
meant completed-but-stranded work was never recovered on a renamed
board, with nothing else owning that state (converted in #2628).
3. **Collision.** `role === "triage"`, `agentType === "triage"`,
`entry.agent === "triage"` compare an **AGENT ROLE**. The planner *lane*
is named `triage` and keeps that name — U11 removed the *column*. Ten
such sites were counted as backlog, and the "obvious" fix (renaming the
role) silently empties the planner's prompt template and mis-binds its
model markers.
A count that is too high and too low simultaneously sends work to the
wrong files while hiding the files that need it. So the census reports
**three separate numbers** and never nets them.
## Proven to fail on the original defect
Not asserted — exercised:
```
$ # reintroduce `task.column === "triage" || task.column === "todo"` into live-agent-count.ts
$ node scripts/lifecycle-column-census.mjs --strict; echo "exit=$?"
packages/core/src/live-agent-count.ts: 10 -> 12
exit=1
$ # restore the file
$ node scripts/lifecycle-column-census.mjs --strict >/dev/null; echo "exit=$?"
exit=0
```
The CLI also exits 1 when its own file list comes back empty — a guard
that reports success without checking anything is worse than no guard.
## 12 regression cases, split by what they defend
Must catch: all six ids; a guard on a local named `from`/`originColumn`
(verbatim the executor.ts shape); single quotes; negation; several
comparisons on one line.
Must **not** catch: role comparisons; comment prose (two tracked
"guards" in `replan-target.ts` were prose about a filter that lives in
another file); a trailing `// … === "triage"` on a code line; sites
carrying a `DELIBERATE-LITERAL` marker.
Plus: **one marker cannot launder a distant guard in the same file** —
that is how allowlists rot.
## Report-only, deliberately
`--strict` compares per-file counts against
`scripts/lib/lifecycle-column-census-baseline.json` and fails when any
file's count **rises**. It is **not** wired into the merge gate: a
thousand-site backlog cannot be a blocking check the day it is first
measured, and a guard nobody can pass is a guard everyone disables.
Owners tightening their own area re-record the baseline in the PR that
lowers it. This is the ratchet shape the `DELIBERATE-LITERAL` markers
scattered through the program already anticipate.
## Stated limitation
Classification is by receiver **name**, so a future field named `agent`
that holds a column would be misclassified as a role comparison.
Recorded at the site, and it is precisely why the two classes are
reported separately instead of netted into one figure.
## Verification
- 12/12 new cases
(`packages/engine/src/__tests__/lifecycle-column-census.test.ts`)
- `pnpm test:gate` **71/71**; `pnpm lint` clean
- `pnpm census:lifecycle-columns`, `--json`, and `--strict` all
exercised end to end
- documented in `docs/testing.md`; no production code touched
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
6d10683dbd |
docs(solutions): store fakes that lie — six fixture defects that each looked like a production bug (#2534)
Six consecutive slices of U7 produced **six test-fixture defects, and every one first presented as a bug in the code under test.** Not one was real. Each cost 15–60 minutes debugging the wrong file. **Two would have shipped a false green** — a test passing while asserting nothing — if the failure had happened to look plausible rather than implausible. This is not a story about carelessness. Every one of these fakes was modelled on an existing fixture in this repo, and the repo's fixtures are inconsistent about exactly the things that matter. ## The catalogue | # | Defect | How it presented | Real cause | |---|---|---|---| | 1 | `moveTaskIf` ignores its predicate | Test passed; in-txn guard untested and indistinguishable from absent | Fake never invoked the callback | | 2 | `updateTaskAtomic: vi.fn()` never invokes its callback | *Every* finalize bailed before the branch under test | Success is derived from whether the callback ran | | 3 | Harness default parameter swallows the input | "Task vanished" case became a duplicate of the control | `harness(undefined)` triggers the default | | 4 | `logEntry: vi.fn()` returns `undefined` | Sweep appeared to match only one column | `.catch` on a non-promise throws, aborting the loop after item one | | 5 | Harness lets `poll()` reach the real `specifyTask` | **exit 1 with every test green** | Real agent path threw *asynchronously*, after assertions passed | | 6 | `updateTask: vi.fn()` returns `undefined` | Branch "did not run" | Same as #4 | **4 and 6 are the same shape, found a week apart, because nothing prevented the second.** That is the argument for writing this down. ## The three rules 1. **Every store method a fake exposes returns what the real one returns** — overwhelmingly a promise. Production writes `await store.m(...).catch(h)` as a fail-soft idiom; `.catch` on `undefined` throws a `TypeError` that unwinds into a broad *"never let housekeeping break the poll"* handler and vanishes. Symptom is never "your fake is wrong" — it is *"the loop only processed the first item"*. 2. **A fake handed a predicate or callback must invoke it.** Ignoring it makes the guarded and unguarded implementations *indistinguishable*, so a test named for the guard cannot detect the guard's removal. Includes the `onLockedRead` hook, without which an in-transaction recheck stays untestable even once the predicate is invoked. 3. **Stub the agent-dispatch boundary.** `poll()` ends in "start an agent", which in a unit test throws *after* the test resolved — `17 passed`, exit code 1, which on CI reads as infrastructure noise. > Never accept a non-zero exit on a green run. It is the only signal that something escaped your assertions entirely. ## Also covered - **How to spot a fixture defect fast** — the tell is *failing for the wrong reason*. Three concrete checks before you open the production file. - **Why differential tests earn their keep** even when they feel redundant: the default-vocabulary half doubles as a fixture self-check, because it asserts behavior that is by definition already shipping. On this program, "both halves failed" was the signal that found three of the six. - **The connection to guards that cannot fire** — six of those on this program too, including a ratchet I wrote that matched only a double-quoted literal (#2527). Same discipline either way: *prove the check fails on the thing it claims to catch before trusting that it passes.* Including the warning that one ratchet injection silently failed to apply, leaving a green run that would have "proven" the ratchet worked. ## The concrete next step, stated plainly A shared `createTaskStoreFake({ tasks, workflowIr })` with promise-resolving, callback-invoking defaults would remove this whole class in one small PR. **It is not built here** because it is cross-unit and needs adopters — building it inside U7 and hoping others find it is how conventions die. The doc says: if you are about to hand-roll a seventh store fake, build the helper instead and link it. Docs-only; no changeset (AGENTS.md excludes internal docs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added guidance on six store-fake defect patterns that can resemble production bugs during testing. * Documented best practices for creating reliable store fakes, including promise handling, callback invocation, and async dispatch isolation. * Added diagnostic techniques for distinguishing fixture issues from genuine application defects. * Included guidance for validating production guards and links to related documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
26c82ebc18 |
ratchet the planner-liveness gate so a fourth door fails CI (FN-6756) (#2540)
Test-only follow-up to the merged P0 (#2531). No production change, no changeset (internal). ## Why This bug reached users **three times**, each as the same mistake in a new place: | | What happened | |---|---| | FN-8600 | the reclaim sweep removed a worktree a live **planner** was using — fixed by registering planning paths and teaching *that* sweep `isPathActive` | | FN-6756 | the leaked-slot reaper never got the same signal; its last line of defense computed liveness from four TaskExecutor-owned maps, so a triage planner matched none of them | | (same PR) | fixing that was not enough — `recoverPausedAbortFailures` **discarded** the refusal and still logged `"Auto-recovered…"`, audited and counted it. The whole bug again, while reporting success | The shared cause is not any one sweep: **“liveness” was re-derived per call site**, so closing one door left the next open and nothing failed. Every one of those fixes was found by review, not by CI. This makes the next one a CI failure. ## Four properties, each written to fail on the exact defect that got through 1. **Every `clearPhantomExecutorBinding?.(` call site consumes its return** — a bare expression statement (including `void`/`await`-prefixed) is the signature of the pause-abort defect. 2. **The destructive path delegates to `hasLiveSessionSurface`** rather than inlining the session-map disjunction — a second copy can drift from the one callers gate on, which is precisely how each sweep got “fixed” without fixing the next. 3. **The probe is wired** in `in-process-runtime`. `self-healing.ts` already records `releaseExecutorWorktreeOwnership` as a declared-but-never-wired option that silently no-opped; an unwired *probe* is worse, since `?.() === true` is `false` when unwired and every gate would quietly stop deferring with nothing failing. 4. **The probe counts registered session paths**, not just executor maps — a triage planner appears in no executor-owned map, so that term is the only thing that sees it. Grep-level, comment-stripped, production source only; no engine boot and no fixtures (FN-5048). Fails closed on an empty/moved source file so a rename cannot make it silently check nothing. ## Proven, one injection at a time **The first draft of property 1 was worthless** — its filter chain was convoluted enough to discard every candidate, so the injected bare call passed. Caught by actually running the injection instead of trusting the green, and rewritten as a single “is this a bare expression statement” rule. | Injection | Result | |---|---| | discard the return value | fails, naming the call site | | re-derive liveness inline | fails on the delegation assertion | | unwire the probe | fails, naming `in-process-runtime` | | drop the registry term | fails, naming `activeSessionRegistry` | Clean tree passes 4/4; all three sources restored byte-identical (`git status` shows only the new file). **Verified:** `pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (414 + 10 + 71). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added safeguards to ensure liveness checks remain consistently enforced. * Verified phantom executor cleanup uses shared session-liveness detection. * Added coverage for registered session paths to prevent false inactive states. * Added fail-closed checks when required runtime source is unavailable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
55ce01335c |
self-healing.ts: resolve pre-WIP columns by role — 11 → 0 receiver-agnostic (largest single item) (#2560)
Taken ahead of my capacity slice, per the drift review.
U11 merges the two pre-implementation columns into one that **keeps the
id `todo`** and **deletes `triage`**. Every `column === "triage"` here
is live breakage the moment that IR lands — and it does not throw, it
simply **stops matching**, so the sweep never fires again and the suite
stays green. That is the Problem Frame’s measured failure mode, landing
on self-healing, where a silently-dead recovery is least likely to be
noticed.
## Count for tracking convergence
`self-healing.ts`, code only, `column === / !== "todo" | "triage"`:
| | before | after |
|---|---:|---:|
| `"triage"` comparisons | **10** | **0** |
| `todo` + `triage` combined | 24 | 15 |
The 15 remaining are all `"todo"`, whose id **survives** U11 — not
breakage, and deliberately left for the hold-column conversion rather
than mixed in here.
## Ten sites, converted by role (intake / hold)
advanced-triage recovery (3, one sweep) · dependency-deadlock blocked
dependents · parked-agent task link · orphaned-approved planning ·
orphaned planning · duplicate-decision candidates · refine-source sweep
· leaked-slot reaper
## Two literals the grep did not count — and they would have silently
killed their sweeps
`listTasks({ column: "triage" })` in both orphaned-planning sweeps.
Converting only the predicate would have left the **query** returning
nothing once the id is gone; the sweep would have looked converted and
done nothing. Both now query the board and filter by role.
All ten route through one seam (`resolvePreWipColumns` /
`filterByPreWipRole`) with a caller-owned per-sweep cache, so 400 cards
over three workflows read three IRs, not 400.
## Two judgement calls, stated
**Unresolvable workflows fall back to the legacy literals, not to
nothing.** These are *recovery* sweeps: a card whose IR cannot be read
must keep its current behaviour rather than silently drop out of every
sweep. That is the conservative direction *here*, and deliberately
differs from conversions whose failure mode is a destructive move.
Pinned by test.
**The leaked-slot reaper’s predicate is left as-is and flagged in
place.** It is arguably too *wide* under plan-in-place — a card being
specified sits in the hold column while a planner works in its worktree,
so “waiting to run must not pin a worktree” no longer holds. What stops
that being live is the FN-6756 liveness gate (already merged). Narrowing
it is a behaviour change and gets its own commit; this PR is vocabulary
only.
The dependency-deadlock site needed restructuring rather than
substitution: its filter is synchronous and role resolution reads the
IR, so membership is precomputed once per sweep.
## Verification
**Revert-proof, measured:** making the resolver return the literals
regardless of workflow turns **4 of the 6** new cases red — the
renamed-workflow ones, precisely the case a literal cannot serve.
`self-healing.ts` restored byte-identical.
`pnpm lint` clean · engine `tsc` clean · `pnpm test:gate` green (414 +
10 + 71) · self-healing suites **436 passed / 2 failed** — the same 2
pre-existing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Re-verified against current `origin/main` (2026-07-29)
Per the definitive-list instruction, re-measured rather than re-asserted
— comment-stripped, so FNXC prose quoting a removed literal does not
inflate the count.
- `origin/main` `self-healing.ts`: **10** code sites (lines 2964, 2984,
3019, 9218, 10703, 11282, 12173, 12218, 12321, 12494 — the same ten,
line numbers shifted only).
- This branch: **0**.
- Main touched this file after my branch point (#2600, the R7 dead-guard
fix). Checked: its diff adds **no** `"triage"` literal, and git reports
this PR `MERGEABLE`, so the merge result stays at 0 — the branch being
behind does not hide a new site.
So merging this moves the tracked number **45 → 35**.
Both stated constraints hold: every site resolves to the **intake/hold
ROLE** (not a renamed literal), and the count is zero across the whole
file rather than per-branch — there is no surviving guard in another
branch of the same function. The two `listTasks({ column: "triage" })`
**queries** are converted too; predicate-only conversion would have left
these sweeps looking converted while returning nothing.
## Re-measured RECEIVER-AGNOSTICALLY (2026-07-30, after the revised bar)
The revised count matches any receiver, not just `.column`. Re-measured
with that pattern, comment-stripped, this file is **11 → 0**, not 10 →
0.
| pattern | origin/main | this branch |
|---|---:|---:|
| `<anything> === / !== "triage"` (any receiver, both quote styles,
line-splits) | **11** | **0** |
The eleventh is the one your list attributes separately to
`engine/self-healing.ts`:
```
origin/main:3017 if (!resumeColumn || resumeColumn === "triage") continue;
```
`resumeColumn` is a bare local holding `live.workflowIrPinColumnId`, so
a `.column`-anchored pattern cannot see it. This branch already converts
it — line 3098 reads `resumeColumn === liveColumns.intake`, resolved
from the same per-sweep role cache as the other ten. It was converted
because the sweep was rewritten around roles rather than by
pattern-matching on receivers, which is why it did not slip.
**Merging this therefore moves the revised 56 by 11, to 45.**
### The 5 literal mentions that remain, and why each is not a guard
Nothing above is a comparison. For completeness, since "a file is not
done because the pattern is gone from it":
- **3 legacy fallbacks** (`?? "triage"`) at 2978, 2980, 10821 — the
unresolvable-workflow path in `resolvePreWipColumns`. These are
*recovery* sweeps: a card whose IR cannot be read must keep its current
behaviour rather than silently drop out of every sweep. Pinned by test.
- **2 union reads** at 6044 (`["triage", "todo"]`) and 9291
(`listTasks({ column: "triage" })` alongside a `todo` read) — unions
covering both vocabularies, with the role filter deciding membership.
Neither replaces nor disables anything.
Known residual gap, stated rather than hidden: those unions do not cover
a **renamed** intake (Coding (Ideas)'s `ideas`). That gap **pre-dates
U11** — the same unions missed `ideas` before the merge — so it is not a
regression here, and closing it needs a cross-workflow lane union rather
than a vocabulary edit.
## AST-VERIFIED, replacing the grep-derived figure (2026-07-30)
Since no grep-derived number is authoritative, I re-measured this file
by PARSING it — `ts.createSourceFile`, walking binary expressions,
classifying on the left-hand side. Not a pattern match.
| | origin/main | this branch |
|---|---:|---:|
| lifecycle-column comparisons (AST-classified) | **11** | **0** |
All eleven classify as `COLUMN`; none is an agent role, session purpose
or surface name, so all eleven are real guards and every one is
converted:
```
2964 task.column 9218 dep.column 12218 task.column
2984 live.column 10703 task.column 12321 task.column
3017 resumeColumn 11282 linkedTask.column 12494 t.column
3019 current.column 12173 t.column
```
`3017` is `resumeColumn` — a bare local holding `workflowIrPinColumnId`,
which is exactly the receiver class the `.column`-anchored greps missed.
It is converted here to `resumeColumn === liveColumns.intake`, resolved
from the same per-sweep role cache as the other ten. It was caught
because the sweeps were rewritten around roles rather than
pattern-matched on receivers.
The classifier is on #2623 as `scripts/lib/lifecycle-column-ast.mjs` and
is offered to #2630 to import. My earlier "11 → 0" was correct — but it
was a regex reading, and this is the same number arrived at by parsing.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
833f379fbd |
U7: ratchet the planning claim to a single writer (extracted from #2517 so it can land) (#2587)
> Test-only, based on `main`, no dependencies. **Extracted from #2517**,
which has been overtaken by events — see the bottom.
U7's stated verification is *"a grep-level assertion that planning
status literals have one writer module"*. This is that assertion, and
the plan names **FN-8504** as the acceptance case: a store-open sweep
cleared a live planner's status because two owners wrote it.
## What it asserts
1. **`status: "planning"` — the claim on a card — is written by exactly
one production module** (`triage.ts`).
2. That single write goes through `updatePlanningStateIfStillCurrent`,
never a bare `store.updateTask`. FN-7977 and FN-8361 are both the latter
bug.
## What it deliberately does not assert
Stated in the file so the guard is not oversold:
- **Who clears the status.** Eleven modules write `status: null` for
unrelated reasons — "one clearer" would be *false*, and the only way to
make it pass is to weaken it into meaninglessness.
- **`needs-replan`.** Post-U3 it is the graph's own durable replan
signal with multiple writers **by design**.
- **Mission `status: "planning"`.** A different entity, excluded by
*path* rather than by pattern — a pattern loose enough to tell them
apart is loose enough to miss a real task write.
## Writes and bindings are separate contracts
Adding constant-indirection detection (`const CLAIM = "planning"`
defeats every shape pattern) flagged `replan-target.ts` — which binds
the literal only to **exclude** it from a status set and writes no task
status anywhere.
Calling that a second writer would have been a false accusation;
dropping the binding rule would have reopened the hole. So the **write**
list stays at one module and **binding** is its own allowlist.
## Proven to fail — twice
A guard that cannot be shown to fail is not a guard.
**1. The real scan function is re-run over a fixture tree** by four of
its own tests — a re-implementation would prove only that the copy
works. Covers every evasion form (single quotes, template literal,
whitespace, plain assignment, computed key), the indirection route, a
comparison-only counter-case, and a brand-new package.
**2. End-to-end against real source**, in a form the *original* detector
missed on **both** axes — a single-quoted writer in `packages/desktop`,
a package the first version never scanned. Re-verified on post-U11
`main` just now:
```
FAIL ... + "packages/desktop/src/bundled-plugin-dirs.ts"
Tests 1 failed | 13 passed (14)
```
Both holes — double-quote-only matching, and a hardcoded four-package
scope — were found by greptile on #2527, and are why the detector now
**discovers** its roots by enumerating `packages/<name>/src` rather than
listing them.
## Why this is a separate PR
**#2517 has been overtaken.** Main absorbed a better discovery
conversion from another worker (it handles U11's merged column, which
mine did not); #2515 then changed the column vocabulary underneath it;
and what remains there is entangled with 14 U11 fixture updates.
This file is a source scan with no dependency on any of that, so it
lands on its own. My recommendation on #2517: **close it** and re-land
its remaining unique content — the sweeps, the handler snapshot,
stuck-abort, `recoverApprovedTask`'s intake gate — as focused PRs
against post-U11 main. Holding a 1262-line PR open through three
vocabulary changes has cost more than it has delivered. That is a
reversible call and it is yours if you disagree; say so and I will
rebase it instead.
## Verification
| Check | Result |
|---|---|
| suite | 14/14 |
| `pnpm lint` | clean |
| `pnpm test:gate` | green (482 + 10 + 71) |
Cheap by construction (FN-5048): grep-level over production source, no
engine boot.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Tests**
* Added automated safeguards to ensure planning task cards have exactly
one valid writer.
* Added checks covering direct and indirect status updates, supported
syntax variations, package discovery, and comment or test-file
exclusions.
* Added validation that planning updates occur only through the approved
guarded workflow.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Phil Larson
|
a54d60ee70 |
fix(core): restore standalone central backend initialization (#2596)
## Summary - restore the owned PostgreSQL backend bootstrap for layer-less `CentralCore.init()` callers - fix node, mesh, and project CLI commands returning empty state and logging `backendHandle is only available in backend mode` during cleanup - add a hermetic regression test for standalone backend ownership and shutdown PR #2454 accidentally added an unconditional early return immediately before the existing standalone bootstrap. Runtime pool sharing remains unchanged: `attachBackendLayer()` releases the central-only connections before adopting the project store layer. ## Verification - RED: regression failed because `createCentralBackendLayer` had zero calls - GREEN: focused regression passes - `pnpm --filter @fusion/core typecheck` - `pnpm --filter @fusion/core build` - `pnpm test` with `FUSION_PG_TEST_SKIP=1`: 482 engine + 132 Core gate + 71 CLI shape + changed regression passed; isolation clean - changeset format check passed The local PostgreSQL merge-gate harness is unavailable without credentials (`empty password returned by client`), so its 10 tests were explicitly skipped rather than misreported. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Restored PostgreSQL central registry access for standalone Node/mesh/project CLI commands without marking the host offline on shutdown. * Improved CentralCore lifecycle handling: concurrent `init()` coalesces, and operations are blocked once `close()` is requested/in progress. * Refined embedded PostgreSQL runtime shutdown: owner stop is coordinated with lease release, registrations are rejected while stopping, and shutdown/teardown uses lease lifecycle consistently. Embedded start failures now treat stopping as retryable. * **Tests** * Expanded coverage for CentralCore close/init/attach races and embedded PostgreSQL lease/shutdown coordination scenarios. * **Documentation** * Updated Changeset notes to clarify CLI and shutdown semantics. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
76b513e028 |
comments-ops.ts: user comments stopped invalidating spec approval (guards 3 → 0) (#2606)
Taking **`packages/core/src/task-store/comments-ops.ts`** from the shared 48-guard backlog. | file | before | after | |---|---:|---:| | `packages/core/src/task-store/comments-ops.ts` | 3 | **0** | ## One of the three was a live defect The awaiting-approval branch read: ```ts task.column === "triage" && task.status === "awaiting-approval" ``` #2515 merged the two pre-implementation columns into one with id `todo`, so a card awaiting spec approval now sits in `todo` and **that condition can never match**. A user comment on such a card silently stopped invalidating the approval — the operator types a correction, the spec stays approved, and the task proceeds on the very spec they were correcting. No error, no log line, nothing to notice. This is exactly the failure mode the census exists to eliminate, and it is user-visible: the operator’s correction is accepted into the comment thread and then ignored by the pipeline. The other two guards survived by luck — their `column === "todo"` arm still matched the merged column, so only the dead `triage` arm was inert. ## Fix All three resolve the **intake/hold roles** from the task’s own workflow. Unresolvable workflows fall back to the legacy pair: this is a best-effort re-triage path whose failure mode is a *missed* re-spec, so degrading to the old vocabulary beats dropping the card out of the branch entirely. ## Verification Regression test drives the **real store** on the merged column and asserts the approval is invalidated. **Revert-proof, measured:** restoring the `triage` literal fails with `expected awaiting-approval not to be awaiting-approval`. `comments-ops.ts` restored byte-identical. `pnpm lint` clean · core `tsc` clean · `pnpm test:gate` green (482 + 132) · `store-comments` 15/15. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8beba5f543 |
U7 E2E evidence: the planning lane, real PostgreSQL + real hold-release sweep (3/7 red without the guard) (#2611)
> Completion-bar **item 3** for my phase. Test-only, based on `main`, no
dependencies, no changeset.
## The gap this closes
The planning lane was the **one lifecycle lane with no E2E coverage**.
The existing live-E2E files cover the lifecycle spine, agent count,
agent link, lease rebound, the merge family and the rebound family —
**not one exercises a planning decision.** Every U7 fix shipped with the
caveat the other units already removed for their lanes: *"all evidence
is unit-level."*
That caveat matters more here than anywhere, because the planning fixes
are **guards that refuse things**, and a refusal is what unit tests are
worst at proving. Nine times on this program a planning test passed
without exercising its subject: a fake that ignored its predicate, a
store stub returning a non-promise into `.catch`, a fixture that
silently resolved to the default IR, a control that passed when it
should have failed.
## What is real
- a per-file **throwaway PostgreSQL** TaskStore (never the operator's)
- the **real `runHoldReleaseSweep`** — every guard, trait resolution,
reservation ordering, and the in-transaction `moveTaskIf` predicate
- **persisted rows** read back with the store's task cache defeated, so
an assertion can only have come from the row
Nothing about the AI is substituted, because none of these decisions
involve it — there is no seam here to script.
## The proof, which is the point
With **#2491's two approval guards removed** from `hold-release.ts`,
this file goes **3 of 7 red against real PostgreSQL**:
```
FAIL does NOT release a card blocked on manual plan approval on a default board
FAIL does NOT release a card blocked on manual plan approval on a renamed board
FAIL holds a card parked for approval MID-SWEEP, after the snapshot was read
Tests 3 failed | 4 passed (7)
```
Restored: **7/7**. So the file demonstrably exercises the guard rather
than merely observing that a sweep works — which the two control cases
(an ordinary held card **is** released, on both vocabularies) exist to
keep falsifiable.
## The case no unit test could honestly make
The **mid-sweep** case needs the in-transaction predicate enforced by a
real store. The hand-built fake that shipped with #2491 originally **did
not honour the predicate at all** — exactly what greptile caught. Here
PostgreSQL enforces it, and the sweep's own log confirms the refusal:
```
[scheduler] Hold release for FN-RACE skipped — task became paused or left todo
```
It parks the card inside `reserveSlot`, which runs *after* the snapshot
and *before* the move — the precise window the in-txn half exists for.
## Coverage
Both approval hold shapes (`status: "awaiting-approval"` and `paused` +
`pausedReason`), on **both** vocabularies, so no assertion can pass by
matching a legacy id. The renamed run's log shows the real sweep
releasing the control card to `building`, not `in-progress`.
## Lane
`.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable,
so **the merge gate is unaffected**. Throwaway per-file database, never
port 4040, no temp-root walk.
## Verification
| Check | Result |
|---|---|
| E2E suite (real PostgreSQL) | 7/7 |
| same suite with #2491's guards reverted | **3/7 fail** |
| `tsc --noEmit` (engine) | clean |
| `pnpm lint` | clean |
| `pnpm test:gate` | green (482 + 10 + 71) |
## Still owed on the completion bar for my lane
E2E for the other three U7 fixes — approved-plan recovery (#2593), the
spec-staleness exemption (#2583), and the discovery advancement guard
(#2576) — is **not** in this PR. Those need a driver for triage's own
poll/recovery path rather than the sweep, which is a different harness
shape; adding it here would have made this PR a harness project rather
than evidence. Taking that next.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
51e108b4f0 |
core/workflow-ir-resolver: let callers tell a RESOLVED workflow from a GUESSED one (unblocks the triage census) (#2618)
Shared-backlog infrastructure, not a single-file conversion. This is the blocker I hit on three separate census files and flagged twice; landing it once beats working around it five more times. ## The problem `resolveWorkflowIrForTask` returns the default coding IR in two cases that are **not** the same as knowing which workflow governs a task: - the selection read threw; - the store reported no selection at all — the synchronous PostgreSQL path does exactly this, deliberately. Callers cannot distinguish either from a genuine selection. **For lifecycle-column work that difference decides correctness.** Post-merge the default coding lineage declares `todo` as its single Planning column and **no `triage`**. So a call site converting a `column === "triage"` guard to trait resolution silently stops firing for `builtin:legacy-coding` cards whenever the store cannot name the workflow — it is handed the default's vocabulary with no signal that it was a guess. ## Why this is the census blocker, with receipts Every conversion I have landed has hit it and worked around it the same way: | Site | Workaround forced | |---|---| | `usage-limit-detector.ts` (#2572) | narrowed to intake, three separate corrections | | `mission-feature-sync.ts` (#2609) | legacy ids unioned, then position-ordered to stop over-claiming | | `live-agent-count.ts` (#2604) | not converted at all — left as a documented finding | That is why the count stalls around a dozen rather than converging on zero: the honest conversion is unavailable, so each site keeps the literal "just in case". With provenance a caller can finally say what it means — **trust the resolved columns when the workflow was selected; fall back to legacy compat only when it was guessed.** ## What lands `resolveWorkflowIrForTaskWithProvenance` returning `{ ir, source: "selection" | "default", workflowId? }`. **Additive by construction:** `resolveWorkflowIrForTask` delegates to it and drops the provenance, so the two answers cannot drift and no existing caller changes behaviour. ## Red-green Mislabelling the no-selection guess as a selection fails its test (`1 failed | 5 passed`). One test deliberately pins the underlying *fact* rather than assuming it — the default guess really does lack `triage` and does have `todo`. If that lineage ever regains the column the hazard changes, and the callers relying on provenance should be revisited; this is what will tell them. Another asserts `resolveWorkflowIrForTask` returns exactly the provenance form's IR across all three paths, so the delegation cannot silently diverge. ## Not done here I have **not** converted any call site onto it. Each one is a behaviour decision for its owner — `comments-ops.ts`, `task-creation.ts`, `archive-planning.ts`, plus revisiting the three above — and bundling them would make this unrevertable. The enabler is the shared part. ## Verification - 6 new tests green; `pnpm test:gate` green (132 / 10 / 482 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Additive API on a private package (`@fusion/core`), no behaviour change, so no changeset 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
f14059e8d3 |
Retry still refuses cards parked mid-planning on 5 builtins (survives #2614; count 0 → 0, defect-only) (#2621)
**Rebased onto main after #2614 landed this file.** That PR's conversion already took the tracked count for `register-task-workflow-routes.ts` to **0**, so this PR does **not** move your number and I am not claiming it does. | file | before | after | |---|---:|---:| | `packages/dashboard/src/routes/register-task-workflow-routes.ts` | 0 (post-#2614) | 0 | What it fixes is a **live 400** that #2614 left in place. Measured on current main: **9 of this file's 14 retry tests fail** without the change below. ## The defect `POST /api/tasks/:id/retry` must answer *"does this card sit where its workflow **plans**?"*, because the yes-branch is **destructive** — it stamps `needs-replan` **and deletes PROMPT.md**. Two predicates stood in for that question and neither answered it: - **#2614** resolved the **intake** column. Correct for the merged lineage; wrong wherever intake and the planning column differ. - The older arm asked `!workflowHasColumn(ir, "triage")`. **Measured across all 12 builtins:** *not one* plans in `triage`, while **seven** still declare that column. So for the five that declare `triage` **and** run every plan node in `todo` — `quick-fix`, `review-heavy`, `compound-engineering`, `design`, `legacy-coding` — the predicate is `false` and a `planning`/`needs-replan` card sitting in **its own planning column** is refused outright: ``` 400 — "Task is not in a retryable state (current status: needs-replan)" ``` The operator has no button at all on a card parked mid-planning. The mirror-image fault is destructive rather than obstructive: a workflow that plans anywhere other than `todo` had a `todo` card's PROMPT.md deleted for a re-plan nobody asked for. ## Fix `workflowPlansInColumn` asks the graph. Planning nodes are recognised by the **semantic markers** the builtins carry — `config.seam === "planning"` and an **exact** `workflowAction` set (measured vocabulary: `plan-replan`, `code-review`, `pre-merge-remediation`) — with node ids as a backstop. Deliberately **not** a `startsWith("plan")` prefix. That was my first attempt and greptile was right to kill it: it matched in the **destructive** direction, classifying a custom `plan-execute` column as a planning column, which deletes a specification. An unlisted planning action costs a replan (recoverable, card stays retryable); a wrongly-listed one costs a spec (not). Hence opt-in. ### Second concern, split out Narrowing the destructive branch must not narrow **retryability** — those were one boolean and are two questions. A card parked outside its planning column would otherwise fail the gate and answer 400: that trades *a card which loses its spec* for *a card nothing can rescue*. It stays retryable via the non-destructive branch, scoped to pre-WIP columns so no `in-progress`/`in-review` status gains a path it lacked. A **v1 IR** declares neither columns nor nodes, so placement is **unanswerable** rather than answered "no". `workflowDeclaresColumnModel` distinguishes the two — reading that silence as "past planning" is exactly what 400'd a v1 planning card. ## Verification All three greptile P1s on the earlier revision were real and are fixed with revert-proof tests (bespoke planning-node ids; the v1 regression I introduced; my own loose action prefix). `pnpm lint` clean · dashboard `tsc` clean · `pnpm test:gate` green (132 + 10 + 482 + 71) · core 11/11 · dashboard 119/119 (`retry-planning-column` + `stale-merge-status` + `routes-tasks`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
31e49b684a |
TAKING default-workflow-hooks.ts + executor.ts + live-agent-count.ts + 6 dashboard files: reopen semantics by role, and the census's blind spot in both directions (13 sites) (#2628)
Batched conversion of every lifecycle-column guard I hold, plus the three the census could not see. **Six files to zero, repo-wide 60 → 49 by a comment-stripped unanchored sweep.** Each conversion has an isolated revert proof and a paired negative case, and the one code move is a separate commit from the behavior changes. ## Per-file before → after Counts from a comment-stripped, unanchored `(===|!==) ["']triage["']` sweep over `packages/*/src` + `plugins/*/src`, excluding tests. | file | before | after | note | |---|---:|---:|---| | `core/default-workflow-hooks.ts` | 4 | **0** | | | `core/task-store/moves.ts` | 5 | **4** | only the flag-ON mirror converted; the flag-OFF inline block is the parity reference and stays | | `engine/executor.ts` | 3 | **0** | **absent from the 45-guard list** — see below | | `core/live-agent-count.ts` | 2 | **0** | duplication removed; answer deliberately unchanged | | `engine/replan-target.ts` | 2 | **0** | both were comment prose, not guards | | `core/agent-prompts.ts` | 3 | **0** | ROLE comparisons, never column guards | | `engine/usage-limit-detector.ts` | 2 | **0** | ROLE comparisons | | `dashboard/app/components/DocumentsView.tsx` | 1 | **0** | real column guard | | `dashboard/app/components/TaskChatTab.tsx` | 2 | **0** | ROLE | | `dashboard/app/components/AgentLogViewer.tsx` | 1 | **0** | ROLE | | `dashboard/app/components/effective-model-resolution.ts` | 1 | **0** | ROLE | | `dashboard/app/hooks/useTasks.ts` | 1 | **0** | ROLE | | `dashboard/…/command-center/MissionControlPanel.tsx` | 1 | 1 | alias table, marked `DELIBERATE-LITERAL` with its reason | ## The census errs in BOTH directions This is the finding I would most like carried into the remaining work. - It **flagged 10 sites that were never column guards.** `role === "triage"` / `agentType === "triage"` compare an **AGENT ROLE**. The planner *lane* is named `triage` and keeps that name — U11 removed the *column*. Worse than noise: the obvious "finish the migration" edit is to rename the role, and that silently empties the planner's prompt template and mis-binds its model markers. `PLANNER_AGENT_ROLE` now names it, so the two vocabularies are distinguishable by grep and a rename fails loudly (revert proof: 4 tests, two of them pre-existing). - It **missed 3 real guards in `executor.ts`**, because the pattern matches `column`/`toColumn`/`fromColumn` and those locals are named `from` and `originColumn`. A census keyed on variable names will keep missing guards wherever a local was named for its role in the function. ## Two real defects, not tidying **1. A renamed board could merge with its re-review never run.** `default-workflow-hooks.ts` is named for the default workflow, but the store runs it on the flag-ON path for *every* workflow — the trait registry resolves hooks by trait id, not by workflow. Its reopen predicates listed the default lineage's column names, so on a renamed board **no reopen effect fired at all**. One of them clears `workflowStepResults`, which `getTaskMergeBlocker` reads: a card bounced out of review carried its old `passed` result back in, and that satisfies the merge gate. Same regression the graph-owned-crossing carve-out exists to prevent, arriving through the other door. (Two smaller ones rode along: failure state never cleared on a renamed reopen, and an operator dragging a card back to the queue never parked it, so the scheduler re-dispatched what they had just pulled back.) **I forgot the carve-out on my first pass, and that was worse than not converting.** A role-resolved clear plus a *name*-matched exemption means a renamed board takes the clear and never the exemption, destroying the remediation input the graph had just written. My own paired negative test caught it. **2. The last-resort recovery for completed-but-stranded work did not exist off the default lineage.** In `recoverCompletedTask`, `promotedFromPlannerColumn` was false on a renamed board, so finished work resting in the planning lane was never promoted — the code fell through to `handoffTaskToReview` straight from the planning column, and role adjacency has no planning → review edge, so the handoff was rejected and the card stayed stuck with its work complete. I converted the promotion **target** too: resolving the lane and then moving to a literal `in-progress` is the half-conversion I have already been burned by twice this program, where the guard starts admitting cards and the move then sends them to a column the board does not declare. ## E2E evidence `renamed-board-reopen.pg.test.ts` drives a **real PostgreSQL store** and a real `moveTask` on a workflow whose columns carry the standard traits under non-default names. The unit tests cannot show this: if `moves.ts` passed `undefined`, every unit case still passes via the no-basis fallback while the real board keeps the old behavior. **Proof it is load-bearing: forcing `moveLifecycleColumns` to `undefined` fails 2 of 3.** The executor suite covers both the split-role and the MERGED post-U11 shape. ## Revert proofs, isolated per site | change reverted | result | |---|---| | reopen predicate → literal names | 4 of 10 fail | | reopen field clears → literal names | 2 of 10 fail | | `userPaused` hold lane → literal `todo` | 1 of 10 fail | | graph carve-out → literal names | 1 of 10 fail | | store passes `undefined` lifecycle columns | 2 of 3 fail (real PG) | | `promotedFromPlannerColumn` → literals | 3 of 7 fail | | two-hop condition → `=== "triage"` | 1 of 7 fails | | promotion target → `"in-progress"` | 3 of 7 fail | | `isPlannerColumnFor` → literals | 1 of 7 fails | | live-agent-count: one arm dropped | 2 of 11 fail | | DocumentsView: trait branch removed | 3 of 7 fail | | planner role renamed to `"planner"` | 4 fail (2 pre-existing) | Every conversion is paired with a negative case (a forward move, a not-a-planner-lane card, a default-lineage card, a renamed column with no traits), so neither "always fire" nor "never fire" can pass for "resolve the role". ## Deliberately NOT converted, with reasons - **`moves.ts` flag-OFF inline block (4).** That branch *is* the legacy path, kept verbatim so the two can be parity-checked. Converting it erases the reference implementation. - **`live-agent-count.ts`'s no-flags fallback.** Reachable, and there is nothing to resolve from — `enrich…FromFlags` exists for callers with board flags rather than an IR, so a column missing from that map is the renamed case. "Not intake" is as much a guess as "todo is intake", and Running/Waiting are complements, so a card matching neither arm is reported as neither and the footer's queued total under-reports it. The real fix is at the caller; four new cases pin that flags override the legacy answer **in both directions**. What did change is the duplication: two hand-written copies of one rule now call one named function. - **`MissionControlPanel`'s `FUNNEL_STAGES`.** An alias table of column *names* where `triage` sits beside `signal` and `backlog`. Command Center aggregates across projects, so there is no single workflow to resolve traits from — the honest conversion is a data change, not a predicate change. - **`DocumentsView` with no traits.** Same no-basis rule; the documents list is full of historical columns absent from the current board. A case asserts a renamed column with no traits still reads as "working", documenting the gap rather than hiding it. ## Fixture findings Each cost a red run that looked like the code under test: - a `merge-blocker` column needs a reachable merge-class node, or `parseWorkflowIr` rejects the workflow; - a back-edge must be `kind: "rework"`, and a rework edge is legal only **into** a node with `config.reworkRegion: true`; - a workflow gets role-level transitions only when it declares wip + review + complete + **archived** plus a planning lane — without the archived column, adjacency falls back to order-derived neighbours and `checking -> queued` is not a legal move at all; - `recoverCompletedTask` only *reaches* the promotion seam when nothing is left to gate; without passed `plan-review`/`code-review` rows it re-enters the workflow graph and returns first, so a naive fixture silently tests the wrong branch and every assertion reads "no moves happened" for an unrelated reason. ## Verification - `pnpm test:gate` **71/71** - new suites: 10/10 reopen-semantics, 3/3 renamed-board-reopen (real PG), 7/7 executor-planner-lanes, 7/7 documents-status-dot, 4/4 planner-role-is-not-a-column - neighbours: 132 + 10 + 482 (gate shards), 350/351 engine planning/replan suites, 64/64 agent-prompts, 51/51 usage-limit-detector, 11/11 live-agent-count, 11/11 dashboard hook/log suites - the single engine failure (`executor-fast-mode-workflows.test.ts` › "raw fast mode still invokes non-executable review seam nodes") **reproduces with my changes stashed** — pre-existing on `origin/main` - typechecks clean for core, engine, and dashboard-app (`tsconfig.app.json`; `tsconfig.json` checks nothing under `app/`); `pnpm lint` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
c9f117bcd9 |
fix(test): dep-abort rebound asserts the resolved column, not the retired triage literal (#2641)
**Review-lane test, 1 failed → 21 passed.** Test-only. `pnpm test:gate`
green, `pnpm lint` clean.
## Production was right; the test was stale
```
expected moveTask("FN-DEP", "triage")
received moveTask("FN-DEP", "todo")
```
`handleDepAbortCleanup` no longer hardcodes a column. It moves the card
to `resolveReboundColumnFor(store, taskId)` (`executor.ts:16576`), which
resolves the task's **own** workflow rebound target by trait — hold,
else intake, else first column — with a `todo` fallback. For this
fixture's default workflow that resolves to `todo`, which post-U11
**is** the merged Planning column. `triage` is not declared on the
default lineage at all, so the old expectation was asserting a column
the workflow does not have.
## Why the concrete value, not the resolver
I asserted `"todo"` rather than re-calling `resolveReboundColumnFor` in
the test. Deriving the expectation from the code under test makes the
assertion agree with whatever the resolver happens to return — the exact
anti-pattern `task-delete-notice.test.ts` documents for its notify table
("deriving the expectation from the value under test makes the suite
agree with whatever the production constant happens to say").
That's only legitimate because **per-workflow resolution already has its
own coverage** — `replan-target-merged-planning-column.test.ts` and
`replan-target-renamed-planner.test.ts`. I checked they exist rather
than assuming; without them, pinning a concrete id here would be hiding
the interesting behaviour.
Also added the negative: the move must **not** be `triage`, so a
regression that reinstates the literal fails instead of quietly passing
on a column the default lineage no longer declares.
**Red-green:** reinstating `moveTask(taskId, "triage")` in production
fails exactly this test (`NEW-failures=1`).
## Context — main's engine-default census
Measured on `origin/main` just now: **66 failed / 9382 passed across 13
files**. This clears one. `workflow-lifecycle-live-e2e.pg.test.ts` is
another and is already fixed in #2634 (pending merge), which takes it
from 2 failures to 0 and adds two new scenarios.
Of the remaining 11, none are in the review/merge lane:
`agent-tools-intake-column`, `builtin-workflows-lifecycle`,
`workflow-graph-optional-step-fix` and
`workflow-settings-fallback-alignment` are column-vocabulary drift
belonging to the U11/U12 owners (the optional-step-fix one expects
`triage` where the replan rebound now resolves `todo` — same class as
this fix, different owner's file); the rest are
executor/CE/goal-anchoring.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
890d588891 |
Phase B — Column.tsx 2→0 and taskActivity.ts 2→0 (U11's cluster to zero) (#2636)
**Claimed:** `Column.tsx`, `taskActivity.ts` — both to zero.
| file | before | after |
|---|---:|---:|
| `packages/dashboard/app/components/Column.tsx` | **2** | **0** |
| `packages/dashboard/app/utils/taskActivity.ts` | **2** | **0** |
## `Column.tsx` — two different fixes, because the two sites are
different problems
**The preserve-progress prompt** routed through
`isPreImplementationColumnRole`. This is the *same* question that helper
was written for — ListView asks it about a move target, Column asks it
about itself — and the degraded id sets are identical (`{todo,
triage}`), so the consolidation is exact.
I verified the sets matched **before** consolidating, because the
sibling case is not interchangeable: `isPreExecutionHoldColumn` in
`TaskContextMenu` drives the Plan affordance and its degraded set is
`{triage}` alone. Routing *that* through this helper added `plan` to
flagless `todo` cards, caught by an existing test. **Same shape,
identical trait path, non-interchangeable fallbacks.**
**The legacy-board arm** (`workflowMode || column === "triage"`) —
deleted, on the third attempt.
I deleted it twice before and reverted both times because four Column
tests render without `workflowMode`. That was the delete-only rule
working, but **my conclusion from it was wrong**: a behaviour change
means the branch was not dead *for those callers*, and the callers are
**fixtures, not production**. Board is Column's only consumer and passes
`workflowMode` at all three render sites. Defending an unreachable arm
so four tests keep passing preserves the tests, not the behaviour.
Two notes for anyone converting the remaining dashboard files:
- I did **not** default `workflowMode` to `true`, which was the tempting
one-liner. `isArchived`, `isHoldColumn` and `isWipProcessingColumn` all
switch on that same flag, so a global default would silently reinterpret
every other fixture in an 85-test file.
- **"Four tests break" was itself an underestimate.** Two more FN-770
fixtures surfaced only after the first two were fixed, because they
render their own explicit `column="triage"` block instead of using
`defaultProps`. The blast radius only became accurate by fixing it in
waves.
## `taskActivity.ts` — composed, not copied
The degraded arm now composes `utils/columnRoles`' predicates instead of
naming ids. **No local copy** — which is the failure mode #2625 hit from
the other direction.
Equivalent *by construction*:
| lane | composition | resolves to |
|---|---|---|
| intake | `isIntakeColumnRole(undefined, col)` | `triage` |
| hold | `isPreImplementationColumnRole(...)` **and not** intake |
`todo` |
reproducing `col === "triage" || (col === "todo" && isReplanning)`
exactly, since the shared pre-implementation set is `{todo, triage}` and
the shared intake id is `triage`.
Deliberately phrased as *"pre-implementation and not intake"* rather
than a second id list: if either shared set changes, this composition
follows it instead of silently disagreeing with the file next door. That
disagreement is precisely what bit the `TaskContextMenu` consolidation
above.
**I previously reported this site as blocked on `TaskCard.tsx` (U12's)**
— on the theory that the arm could only die once every caller supplied
resolved flags. Wrong framing: the arm doesn't need to become
*unreachable*, it needs to stop *naming ids*. Composing the shared
predicates does that without touching any caller.
## Verification
**1139 of 1141** green across `app/utils`, `Column` and `TaskCard`
suites. The two `TaskCard` failures are **pre-existing** — verified by
stashing this change and re-running, where they fail identically.
Dashboard app typecheck and lint clean.
Takes U11's cluster to zero except `TaskContextMenu.tsx`, whose
remaining site is covered in **#2626** and whose second site is a
documented non-consolidation.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
6ca7cc94ec |
triage census — core/types/archive-planning.ts 1→0, plus the self-healing.ts audit (10 guards, 2 traps a mechanical conversion would miss) (#2622)
Batched: one conversion plus the audit for the largest remaining file,
so the CAPACITY worker inherits the analysis instead of redoing it.
## Per-file counts
| File | Before | After |
|---|---:|---:|
| `packages/core/src/types/archive-planning.ts` | 1 | **0** |
Verified with the raw pattern (`column [!=]== "triage"`), which is what
the coordinator greps — including checking that my own explanatory
comment did not reintroduce the literal. It did, on the first attempt;
caught and removed before pushing.
## The conversion: a doc that manufactures dead guards
There is **no executable guard** in this file — raw 1, code 0. I fixed
it anyway, because the documentation was wrong in the way that
propagates: it told consumers to derive running agents via a hardcoded
intake-column comparison. Post-merge the default lineage declares one
Planning column and no `triage`, so anyone implementing from that
sentence writes a comparison that matches nothing — **a dead guard
authored on purpose, from an instruction we left lying around.** A doc
handing out a dead predicate is worse than a dead guard, because it
manufactures more of them.
Now describes roles. Also disambiguates the neighbouring line where
"triage agent" is a lane/role id, not a column — the same conflation
that accounts for 23 of the original broad 48.
## Audit: `self-healing.ts` (10 guards, unclaimed at time of writing)
45% of the remaining bar, and every one sits in a recovery sweep. **6 of
the 10 are sole-`triage` and already dead for default-workflow cards.**
| Line | Guard | Fires for default cards? | What silently stops |
|---|---|---|---|
| 2964 / 2984 / 3019 | advanced-triage recovery: filter, live re-check,
`moveTaskIf` CAS | **No** | stranded specification work never recovered
|
| 12173 / 12494 | orphaned-approved + orphaned-planning sweeps | **No**
| orphaned planning sessions never reaped |
| 12321 | `task_refine` candidates | **No** | refinement tasks never
recovered |
| 9218, 10703, 11282, 12218 | paired with `todo` | Yes, via the `todo`
arm | — (legacy-compat arms) |
**Two traps a mechanical conversion walks straight into:**
1. **`listTasks({ column: "triage" })` at 12172 and 12493 is a dead
QUERY, not just a dead filter.** Convert only the `t.column ===
"triage"` predicate and both sweeps scan an empty result set — the file
counts as converted while the sweeps stay exactly as dead. This is the
coordinator's rule #2 in its most literal form: a guard surviving in
another branch of the same function.
2. **Lines 2964 / 2984 / 3019 are one transaction** — filter, live
re-verify, and a `moveTaskIf` compare-and-set. Convert them
independently and you get a filter matching the resolved intake column
against a CAS still demanding the literal, so **every move refuses**.
Silently: `moveTaskIf` returning false is indistinguishable from a lost
race.
Both need the resolved-vs-guessed distinction from **#2618** — a
`builtin:legacy-coding` card in `triage` must still be recovered when
the store cannot name its workflow.
## Related live finding, not fixed here
`resolvePlannerLanesForTask` (merged in #2610; used by
`executor.ts:11978`, `scheduler.ts:1843`/`:2414`,
`mission-autopilot.ts:973`) cannot tell a resolved workflow from a
guessed one. Probe on main against a `{ getTask }`-only store:
```
PROBE lanes: ["todo"] dedicated: []
```
So a `builtin:legacy-coding` card in `triage` is not recognised as a
planner lane — mission-feature rollback stops firing and the
spec-staleness planner skip never fires. Neither errors. #2618 is the
fix; ~3 lines per resolver in `planner-lane-resolution.ts`.
## Verification
`tsc --noEmit` on `@fusion/core` clean; `pnpm lint` clean. Comment-only
change, no behaviour change, no changeset.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Clarified terminology for archived task planning overrides.
* Updated project health documentation to better explain how active
agent counts are calculated across workflow stages.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
73338502e5 |
fix(test) + E2E: re-green main's lifecycle release leg, and prove the MERGED board + REVISE rework (#2634)
**Second batch.** Three commits, no production code. `pnpm test:gate`
green, `pnpm lint` clean, all three E2E suites together **3 files / 41
tests, exit 0**.
## 1. main's lifecycle E2E is RED right now — this fixes it
Independently of my work, on a detached `origin/main`: **2 failed / 18
passed**. Scenarios 1 and 2 fail with `sweep.released` **empty**.
**Cause:** `seedTask` relied on task creation's PROMPT.md, which is a
bootstrap seed (`"# <id>\n\n<description>"`). FN-7648's
`isUnplannedForExecution` reads that file for any card resting in an
intake- **or** hold-trait column and refuses to move an unplanned card
into a processing column. The sweep reported `held: [{ reason:
"move-rejected-or-no-slot" }]`.
**That is the gate working.** The fixture was asking the scheduler to
release a card that had never been specified. The fix is the one the
graph-entry contract doc already prescribes: *"Scheduler/release test
fixtures must model a card that cleared the gate ... A held unreviewed
card is the gate working."* `seedTask` now writes a planned PROMPT.md.
**Verified it repairs main, not just this branch:** applying only that
file to a detached `origin/main` leaves scenarios 1 and 2 **passing**,
with the 4 residual failures being scenarios 3 and 6 — which need the
fixture-options commit main does not have.
### I was wrong in #2627 and this corrects it
In #2627 I named the in-transaction capacity gate (#2488/#2499) as the
likely cause. **It was not.** Two hypotheses died, both recorded in the
code comment so nobody re-runs them:
| Hypothesis | Result |
|---|---|
| E2E settings lack `maxConcurrent` → capacity gate rejects the move |
added `maxConcurrent`/`maxWorktrees` → **still 2 failed**. Not the
cause. |
| the move itself is refused | a direct `moveTask(id, wip)` →
**succeeded**. Never the blocker. |
Only then did probing the two release gates give
`isTaskBlockedOnApproval=false`, `isUnplannedForExecution=true`, and
dumping the file show the stub. I've flagged the wrong lead on #2627 too
— a plausible-sounding cause pointed at another worker's PR is worse
than no lead.
## 2. E2E evidence: the MERGED intake+hold board
U11's shape — one column carrying intake **and** hold — had no
end-to-end coverage; every prior E2E drove intake and hold as separate
columns.
- shared fixture gains opt-in `mergedIntakeAndHold`, plus `MERGED_VOCAB`
(legacy ids, so a failure is attributable to the **role** merge alone)
and `MERGED_RENAMED_VOCAB` (ids move too).
- lifecycle scenario 3 drives the full spine: planning runs **in place**
on the dual-role column, the real `runHoldReleaseSweep` releases
**from** it, the graph runs to complete.
- 4 merge-safeguard cases on the merged board (finalize, proofless
refusal with the same reason, merged+renamed landing no legacy id,
at-most-once).
## 3. E2E evidence: a REVISE routes back through rework
The plan's `InReview → InProgress: review requests changes` had **no**
live-engine evidence on any board — the fixture's review seam always
succeeded.
Two things the engine taught me, both corrected here:
- the **IR validator refused** my rework edge: it is only legal into a
node with `config.reworkRegion: true`. A real contract, and the
validator catching it is the system working. `exec` now declares it (the
shape the builtin uses on `merge-attempt`).
- my first assertion was wrong. A REVISE does **not** leave the card in
wip — rework re-enters `exec` within the same run, review approves on
its second call, and the card finishes at complete. The evidence is the
**seam sequence**
`["planning","execute","review","execute","review","merge"]`, not an
intermediate column the run has already passed. Asserting the final
column alone would have been satisfied by a graph that ignored the
REVISE entirely.
## Both families are mutation-attributed
| Scenario | Mutation | Result |
|---|---|---|
| 3 — merged intake+hold | `isHeldTask` treats intake/hold as exclusive
| **exactly its 2 tests** fail |
| 6 — REVISE → rework | disable rework re-entry in
`workflow-graph-executor` | **exactly its 2 tests** fail |
Both fixture options are opt-in; the two pre-existing suites are
behaviourally unchanged (27 → 29 → 41 passed across the additions, no
existing assertion touched).
## Still not shipped: safeguard 2's graph E2E
Attempted twice, deleted both times. Attempt 1 passed and then survived
mutating `merge-gate` to ignore `task.autoMerge` — the card parked on
the review column's `merge-blocker` trait, not the gate. Attempt 2
removed that trait to isolate the gate, and the **control** case parked
too. Isolating it needs a merge path mirroring the builtin (`merge-gate
→ merge node → end`) rather than a direct edge to `end` — a real
redesign, not a speculative edit. The enforcement that holds today is
`allowInReviewMergeProcessing` in `project-engine` (unit-mutation
verified, NEW=9; gated via #2526).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
eb874f3da3 |
convert(cli/commands/task.ts): triage guard 1 → 0 (+ a live main regression in the lifecycle E2E release path) (#2627)
**Batched push, gate open.** One conversion; the two E2E commits are held back and the reason is below — it is the more important half of this PR body. ## Conversion | File | triage column comparisons before | after | |---|---|---| | `packages/cli/src/commands/task.ts` | **1** | **0** | `pnpm test:gate` green, `pnpm lint` clean. All four non-terminal columns rendered the **same** glyph, so the four id comparisons were only ever asking "is this column terminal?". Naming `triage` made it a lifecycle-vocabulary site for no behavioural reason — the merged Planning column dropped that id, so the comparison silently stopped matching while the output stayed correct **by accident** (the fallthrough gave it the same glyph). Behaviour-identical **only** because the loop iterates the legacy `COLUMNS` constant (`types/board.ts:27` — exactly the six ids), so `col` can never be a custom id. Stated because the forms **diverge** outside that set: the old chain fell through to the terminal glyph for an unrecognised id, the new form returns the non-terminal one. If this ever iterates workflow-resolved columns that difference becomes live, and the right answer is a trait lookup, not this. **Deeper bug deliberately untouched, for U12:** because the loop iterates the legacy enum, a card in a workflow-renamed column **is not rendered at all**. That is R8's surface change, far bigger than this glyph. *(Note: this file is not on the 45-guard list, so it will not move your count. Flagging so the numbers reconcile.)* --- ## ⚠️ Live regression on origin/main — the lifecycle E2E release path While rebasing to push, the flagship lifecycle E2E went red. **I verified it on `origin/main` alone, with none of my commits: 2 failed / 18 passed.** ``` scenario 1 — DEFAULT vocabulary → AssertionError: expected [] to include 'FN-E2E-1' (r.sweep.released is EMPTY) scenario 2 — RENAMED vocabulary → audit trail differential broken: renamed produced [{end},{review}], default produced [] ``` Both are pre-existing tests I have never touched. **The capacity release sweep is releasing nothing.** **Likely cause, from reading rather than bisecting** — so treat it as a lead, not a verdict: `moves.ts:1059` now resolves a capacity pool id and enforces `enforcePooledColumnCapacity` **inside the move transaction** (#2488 "bind the in-transaction capacity gate", made user-visible by #2499 "make the capacity gate actually bind for real projects"). The E2E drives with `settings = { experimentalFeatures: { workflowGraphExecutor: true } }` — **no `maxConcurrent`** — while the fixture's wip column declares `{ trait: "wip", config: { limitSetting: "maxConcurrent", countPending: true } }`. If the resolved limit is finite and the pooled count meets it, the hold→wip move is rejected on capacity and the sweep correctly reports nothing released. If that is right, it is a **test-harness/production interaction, not a product break** — but it means the program's primary end-to-end evidence for the capacity boundary is currently inert on main, which matters for completion criterion 3. It needs the capacity worker's eyes, since #2488/#2499 are theirs and I would be guessing at the intended pool/limit contract. ## Why my two E2E commits are held They add scenario 3 (merged intake+hold board) and scenario 6 (REVISE → rework), both of which **depend on the same release leg**. On current main they fail for main's reason, taking the file from 2 failures to 4. Pushing them would add red to the count you are tracking and obscure whose regression it is. Both are complete, mutation-attributed, and green against the commit I wrote them on: | Scenario | Proves | Mutation that fails it | |---|---|---| | 3 — merged intake+hold | capacity release works from a dual-role column | `isHeldTask` treating intake/hold as exclusive → exactly its 2 tests | | 6 — REVISE → rework | `InReview → InProgress` on renamed *and* merged boards | disabling rework re-entry → exactly its 2 tests | They go out in the next batch the moment the release path is green. ## Also not shipped, twice attempted, deleted both times Safeguard 2 (`autoMerge:false` terminal-until-human) still has **no** graph-level E2E. Attempt 1 passed and then survived mutating `merge-gate` to ignore `task.autoMerge` — the card was parking on the review column's `merge-blocker` trait, not the gate. Attempt 2 removed that trait to isolate the gate, and then the *control* case parked too, so the flag still was not the discriminator. A fixture that can isolate it needs a merge path mirroring the builtin (`merge-gate → merge node → end`) rather than a direct edge to `end` — a real redesign, not a speculative edit. The enforcement that actually holds today is `allowInReviewMergeProcessing` in `project-engine` (unit-mutation verified, NEW=9; gated via #2526). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
6a33d8f8cc |
Phase B — TAKING task-creation.ts: intake classification by trait (4 sites → 0) (#2613)
**Taking:** `packages/core/src/task-store/task-creation.ts` | file | guards before | after | |---|---:|---:| | `packages/core/src/task-store/task-creation.ts` | **4** | **0** | (4 remaining pattern matches in that file are inside the new explanatory comments, not code.) ## What the literals meant, and why they had stopped meaning it ``` resolvedEntryColumn !== "triage" ×2 "this workflow has a MANUAL intake" task.column === "triage" ×2 "created into the intake column" ``` The first named the **default workflow's intake id** to express *"not the default workflow"*. Post-U11 the default's intake **is** `todo`, so the comparison became vacuously true for the default workflow and the guard stopped separating the two shapes it exists to separate. The real fact is the intake trait's `autoTriage: false`, which `resolveWorkflowIntakeFacts` now reads from the IR alongside the intake column id. The second was the last-resort clause for a card whose workflow could not be resolved. `intakeFacts.intake` covers that properly — it falls back to `DEFAULT_WORKFLOW_ID` rather than to a bare id — so an explicit `column: "triage"` create on a workflow that still declares `triage` (R11) is matched through the *resolved* intake instead of a coincidence of naming. `isUnplannedStartCreate` is also restated in terms of what it actually detects — *"the card landed past its workflow's manual intake"*, which is what quick-add Start does by submitting the workflow id and the post-intake column together. That replaces `&& task.column === "todo"`, another id standing in for a relationship. ## Two expectations the conversion legitimately inverted Both read before changing, neither retargeted blindly. **1. *"keeps generateSpecifiedPrompt for a direct create into todo (not bootstrap)"*** `todo` **is** the default's intake now, so a card created there with no spec **must** get the bootstrap seed — triage admits a card for planning only when its `PROMPT.md` reads as a seed. Keeping the old expectation would have pinned the FN-8587 stall: a boilerplate spec that reads as "already planned" and is never planned. The old behaviour survived only by **accident of resolution failing** in the harness, which left the `=== "triage"` literal as the sole deciding clause. Removing that literal is what surfaced it — which is the point of the conversion. Split into two tests so the contract it was really protecting (an explicit **non**-intake column stays a specified create) keeps its own case. **2. `store-reservation-atomicity`'s file-scope rollback test** It stubs `generateSpecifiedPrompt` to inject a bad `## File Scope`, so it needs a create that actually **calls** that generator. A `todo` create now gets the bootstrap seed instead, and bootstrap intake prompts deliberately skip the file-scope hard-fail because their body is freeform operator prose where a stray `## File Scope` token is not a real declaration. Moved to a non-intake column so validation still runs. Left as-is the test would have been **vacuous** — no throw, no rollback exercised — while still reporting green. ## Verification Core package vs the 47-failure post-merge main baseline: **47 failed — zero new.** Two files reported failures in the wide run and pass in isolation (`create-task-reserved-id` 4/4, `schema-applier` 75/75) — the known contention pattern in this suite; re-run before attributing. Gate **482 + 10 + 71** green. Lint and core typecheck clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
88df46bedb |
test: re-green executor-workspace onto FN-6756's contract (+ flag a dead branch) (#2617)
**Test-only.** One file. No production changes. `executor-workspace.test.ts`: **2 failed / 10 passed → 13 passed**. (This commit was pushed earlier and I failed to open its PR — the work was finished and sitting on a dangling branch, which is why `executor-workspace` still shows in main's failure census.) ## Why it was red Both cases asserted that `clearPhantomExecutorBinding` **succeeds** while session-registry paths are held. PR #2531 (FN-6756, P0: *"stop reaping worktrees out from under live planners"*) inverted that. `hasLiveSessionSurface` now includes `activeSessionRegistry.pathsForTask(taskId).length > 0`, and that guard runs **before** both branches — so any registered path refuses the clear. The FNXC note at `executor.ts:2729` says the kind-blind guard is deliberate: *"A leaked entry now blocks THIS sweep rather than a live planner losing its worktree — the strictly safer failure."* So the old expectations describe the pre-#2531 contract. Rewritten to the current one, which had **no direct coverage**: a refusal leaves `activeWorktrees` and the registry entries untouched. The FN-6736 KTD2 invariant ("every held path, not one") is kept, exercised with no registry paths so the guard permits it. **Verified the new tests guard:** removing the registry term from `hasLiveSessionSurface` fails 2 of them (`NEW-failures=2`), and only them. ## Flagged, not fixed — a possible dead branch The guard appears to make **both branches it precedes** unreachable for their stated purpose: - the default branch exists to unregister every held registry path (FN-6736); - `preserveWorktrees: true` exists to **keep** those paths so a `moveTask(preserveWorktree: true)` re-dispatch reattaches to the same worktree (FN-7249) — and its **only** production caller is the self-healing reclaim at `self-healing.ts:3565`. Both need registered paths to do anything, and the guard rejects exactly that case. With none registered, one sweeps nothing and the other preserves nothing. The third new test pins this so the conflict is **executable rather than prose**: `preserveWorktrees: true` returns `false` while the path it exists to preserve is registered. I did not "fix" it by rewriting the assertion to match production — that would bury a possible regression in FN-7249's reattach path. Resolving it (exempting the non-destructive `preserveWorktrees` path, or narrowing the guard by kind) is a product decision for the FN-6756 owner. ## Main's engine-default census, measured just now | Point | Failed | Files | |---|---|---| | when I started this sweep | 283 | 28 | | after the logger-mock fix (#2573) | 106 | 23 | | **now** | **65** | **12** | This PR clears one of the remaining 12. `executor-review-verdicts.test.ts` is newly red and in my lane — taking that next. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
d438cd1d13 |
U12 drift: register-task-workflow-routes.ts — resolve the intake column (7 -> 1) (#2614)
**File claimed: `packages/dashboard/src/routes/register-task-workflow-routes.ts`.** Per-file lifecycle-column guard count: **7 → 1**, and the 1 is comment prose (line 3758), so this file is done for completion-bar item 1. ## The bug this fixes `retrySpecification` decided "this Retry is a re-plan, not a generic retry" with `task.column === "triage"`. `status: "planning"` is retryable **only** through that flag — it is not in the generic `failed`/`stuck-killed` set. So on any lineage whose intake column is not literally named `triage`, a card visibly sitting in planning got `400 Task is not in a retryable state`. The operator's Retry button did nothing, with no error to explain why. Post-#2515 that includes the **default** workflow: `columnsWithFlag(resolveDefaultWorkflowIr(), "intake")` is `["todo"]` and the default's columns are `[todo, in-progress, in-review, done, archived]` — `triage` is not declared at all. The pre-existing `todo` fallback below it papered over the default case (it fires when the workflow has no `triage`), which is why this did not show up as a total outage; custom and renamed lineages had no such cover. Now: `const retryIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id)`. ## Red-green, measured `packages/dashboard/src/__tests__/plan-approval-intake-column.test.ts` — new case, custom lineage with intake `backlog`, card in `backlog` with `status: "planning"`: - with the change: `200` - with `task.column === "triage"` restored: **`AssertionError: expected 400 to be 200`** The fixture uses `planning` deliberately. A `failed` fixture would pass either way through the generic retryable set and prove nothing. ## What is NOT tested, and why not This PR also removes four `&& task.column !== "triage"` disjuncts I added earlier while widening the P0 approve/reject guard. **Those are untestable by construction** and I am not claiming coverage for them: removing an extra acceptance only shrinks what the guard accepts, and no case can feed these routes a `triage` card now that no shipped lineage declares one. I re-widened one guard and confirmed the suite stays green — i.e. nothing depends on the disjunct in either direction. That is the honest result, not a passing test. ## Three fixtures updated, not guards re-widened `stranded-refinements-routes.test.ts` failed with three `expected 400 to be 200` — the same failures that made me widen in the first place. This time I probed instead: `BASE_TASK` had `column: "triage"`, a column the default workflow no longer declares, so a 400 is **correct** and the fixtures were pre-merge artifacts describing a board shape the product stopped shipping. Changed to `column: "todo"` with the resolver output recorded in the file. ## Observation, deliberately not fixed here The `todo` fallback at ~2642 (`retrySpecification = !workflowHasColumn(workflowIr, "triage")`) is now near-dead: for the merged default the first branch already fires. It survives only for a lineage that has a `todo` column, no `triage`, and some *other* intake column — where treating a `todo` card as planning is arguably wrong. Deleting it is a behaviour change with its own blast radius, so it does not ride along in a conversion commit. ## Verification `pnpm lint` clean. `pnpm test:gate` green (10 / 482 / 71). Target suites: `plan-approval-intake-column.test.ts` 8/8, `stranded-refinements-routes.test.ts` 12/12. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
1469d57477 |
U12 drift: ListView.tsx — one tested column-role helper (5 -> 0) (#2620)
**File claimed: `packages/dashboard/app/components/ListView.tsx`.** Per-file lifecycle-column guard count: **5 → 0** (3 live, 2 in comment prose that described the deleted code). ## What was actually wrong All three live sites were *already* flags-first. The defect was that each carried its own inline copy of the same fallback: ```ts targetFlags ? Boolean(targetFlags.intake || targetFlags.hold) : column === "todo" || column === "triage" ``` Three copies, none reachable from a test, each reading like a lifecycle rule rather than the degraded mode it is. A fourth copy was the natural next step. ## Why the fallback survives instead of being deleted `columnFlagsById` is legitimately empty in two states: the pre-load window before the workflows fetch resolves, and a card stranded in a column its workflow no longer declares. A bare `flags.intake === true` returns false in both, and **both failures are silent** — the Planning badge stops appearing, and a backwards move stops asking whether to preserve step progress, so the operator loses completed steps with no prompt and no error. Deleting the fallback is not the cleanup it looks like. So it is kept, named (`isPreImplementationColumnRole`, `isIntakeColumnRole` in `app/utils/columnRoles.ts`), defined once, and documented with that reason at the definition. The legacy ids now live in a named `LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS` set — a last-resort guess, not a comparison masquerading as a rule. ## Tests, and the case that never had one `app/__tests__/columnRoles.test.ts` (6). The degraded branch is now covered for the first time — it was unreachable while inline inside two `handleMove` closures and a `useCallback`. It also pins the **inversion** a fourth copy would eventually get wrong: a resolved column whose traits say it is *not* pre-implementation must not be overridden by an id that happens to be `todo` or `triage`. That is the direction that trains operators to dismiss the prompt. Mutation-checked, measured: | mutation | result | |---|---| | ignore the flags argument (`return LEGACY_….has(columnId)`) | **4 failed / 2 passed** | | ignore the id fallback (`return Boolean(flags?.intake \|\| flags?.hold)`) | **4 failed / 2 passed** | ## Behaviour preservation `ListView.test.tsx` + `workflow-resolved-columns.test.tsx`: **260 passed**, unchanged. The extraction is a pure move — the two helper bodies are the inline expressions verbatim, with the id set hoisted. `pnpm lint` clean. `tsc -p tsconfig.app.json` clean (the app config, not the root one that silently skips `app/`). No changeset: behaviour-preserving refactor. ## Backlog measured on `origin/main` at time of writing 48 total. `self-healing.ts` (10) is the capacity worker's; `register-task-workflow-routes.ts` (7) is my #2614. Remaining unowned in this area after this PR: `TaskCard.tsx` 4, `TaskDetailModal.tsx` 3, `TaskContextMenu.tsx` 2, `Column.tsx` 2, `taskActivity.ts` 2. Several of those hold the *same* fallback pattern and can now call this helper rather than grow another copy. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
89d6d76d60 |
Unowned: the R7 sweep guessed with another workflow's columns — its "do not guess" guard was unreachable dead code (#2600)
## Unowned: the R7 sweep's "do not guess a column" guard could not fire Picked up from my own #2543 finding. Independent of my other PRs. ### The guard existed in comment form only `reconcileUndeclaredTaskColumns` wraps IR resolution in a try/catch whose comment reads: > An unresolvable workflow is its own fault path; do not guess a column. But `resolveWorkflowIrById` catches **every** failure and returns `defaultCodingWorkflowIr()`, and `resolveWorkflowIrForTask` does the same for a failed selection read. The resolver never rejects, so that catch is **dead code**. What actually happened to a card whose workflow could not be loaded: it was judged against the **default** workflow, and if its column was not one the default declares, the sweep re-homed it to the **default's** rebound target. It guessed, using a workflow that is not the card's own — the precise outcome the guard was written to prevent, in a **startup recovery path that runs against every task**. ### How it was found, which is the part worth keeping By being **unable to make a test of the guard fail**. Three separate mutations all passed — deleting the `continue`, deleting the try/catch, and simulating a whole-sweep abort at that very catch. I had written that off once as "this case pins the outcome, not the mechanism". The inability was the signal, not a limitation of the assertion: the branch is unreachable. This is the seventh instance of the program's core shape, and the first I found in a guard I had just finished writing coverage for. ### The fix The sweep now **proves the resolved IR belongs to the task** before moving its card: it reads the task's workflow selection and confirms that id resolves to a real definition (built-in or stored). - A task with **no** selection legitimately resolves to the default workflow — not treated as unresolvable. - An unreadable selection **read** is itself grounds not to guess. Placed at the **move site**, not at resolution, deliberately: it costs one definition read only for a card already about to be moved — a healthy board reaches that line for nobody — and it keeps the fix inside the sweep instead of changing a resolver whose soft-failure many other callers depend on. Changing `resolveWorkflowIrById` to reject would have been the tidier-looking fix and a much wider blast radius. ### Revert-proof, both directions - Remove the proof → the case fails `expected 2 to be 1`: the unloadable card is re-homed on a guess. - The same case asserts the neighbour **is** still repaired, so the fix cannot be mistaken for letting one bad card disable the sweep for everyone else. That is the per-task isolation property, and a single-task fixture cannot distinguish it from a whole-sweep abort — verified by injecting a throw at the loop head (`expected 0 to be 2`). ### Verification `pnpm test:gate` (482 + 10 + 71), `pnpm lint`, engine typecheck green. Sweep suite + `legacy-tombstones`: 13 passed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented startup recovery from moving cards into incorrect columns when their workflow cannot be loaded or resolved. * Cards with unreadable workflow information now remain in place, while other recoverable cards continue to be repaired correctly. * Added safeguards to avoid guessing a fallback workflow during column reconciliation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
534798dea0 |
test(U9): E2E evidence for the merge safeguards on a real PG store (completion-bar item 3) (#2615)
**U9 E2E evidence.** One new `.pg.test.ts`, 6 tests, green. No
production changes. `pnpm test:gate` green — `pgDescribe`-skipped
without PostgreSQL, so the gate is unaffected.
## What this closes
The U9 safeguard baseline verified all six merge safeguards by
**mutation at unit level**. The sibling `workflow-merge-family-live-e2e`
covers exactly **one** end-to-end. This drives
`finalizeProvenAutoMergeTask` — the last move a card makes — against a
real PostgreSQL `TaskStore`, asserting on the **persisted column** read
back after clearing the task cache. Never on "a function was called".
Only the merge **proof** is seeded (`mergeDetails.mergeConfirmed`),
which is what a real merger writes; there's no git and none is needed.
Column resolution, blocker evaluation, the move and its guards, and
persistence are all real. Includes the rename differential, where a
guard keyed on a literal goes silent.
## Three things I expected and measured wrong
Corrected in the file rather than worked around — each is a claim I
would otherwise have shipped:
**1. Dependency gating does not reach this seam.** My first draft
asserted a refusal. A proven-merged card with a live `blockedBy`
finalizes to the complete column anyway. That's coherent: dependency
gating lives in `getTaskCompletionBlocker` and gates whether work may be
*called* complete, while this seam runs after `mergeConfirmed` —
refusing would strand a merged card in review and misreport the
repository without un-merging anything. Now pinned as designed behavior
*with* that reasoning, not filed as a hole.
**2. The at-most-once outcome is `already-done`**, not the
`already-complete` I guessed.
**3. `expect(outcome).toBe("blocked")` cannot attribute a refusal.** The
finalizer has **three layered refusal gates**, and the two proof gates
emit the *same* reason (`missing-merge-confirmation`, also returned by
`validateWorkflowDoneMergeProof`). So removing either one left my
original assertion **green**:
| Mutation | Result |
|---|---|
| remove the durable-proof gate | 6 passed — invisible |
| remove the main-path proof gate | 6 passed — invisible |
| remove **both** | **2 failed** / 4 passed |
Fixed by pinning the **reason**, not just the refusal. The lesson
generalises: single-gate mutation cannot detect redundant
defense-in-depth from outside, so the unit-level attribution in the
baseline doc and this E2E are **complementary**, not duplicative. I
nearly labelled these tests as proving a specific gate they don't.
## Flagged, not changed — safeguard 1 at this seam
Written as open questions and answered by running them. **Both a
`paused` and a `userPaused` proven-merged card are moved to the complete
column.**
For `paused` that's documented design — `auto-merge-finalization.ts:243`
evaluates hard blockers with `paused: false` because the branch already
landed.
For `userPaused` it sits against the invariant re-ratified in #2486:
*never MUTATE lifecycle state of a user-paused card.* The mitigating
argument is the same one — the merge is durable, so the move is
bookkeeping that reflects reality, and refusing would leave an
operator's card permanently misfiled in review.
**Either reading may be right. What was not acceptable is that it was
untested.** Both are now explicit named assertions with the tension in
the comment, so tightening the pause contract becomes a decision rather
than a discovery. Resolution belongs to whoever owns the pause contract
— I'm not quietly changing merge behavior on a paused card.
## Safeguard coverage after this PR
| # | Safeguard | Unit (mutation) | E2E |
|---|---|---|---|
| 1 | user pause | ✅ | ✅ pinned as an exception at this seam — flagged
above |
| 2 | autoMerge:false | ✅ | ✗ gate lives upstream in `project-engine`,
not this seam |
| 3 | dependency gating | ✅ | ✅ pinned as *not* applying here, with
rationale |
| 4 | capacity single-flight | ✅ | ✗ in-memory pump, no store seam to
observe |
| 5 | merge-proof | ✅ | ✅ both vocabularies, reason-attributed |
| 6 | at-most-once | ✅ | ✅ second finalize classifies `already-done`, no
second move |
The two gaps are stated rather than implied: safeguard 2's gate is
`allowInReviewMergeProcessing` in `project-engine`, which needs an
engine harness rather than a store one, and safeguard 4 is an in-memory
single-flight latch with nothing persisted to assert on. Both are
covered by mutation at unit level and both are in the gate as of
#2526/#2569.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
592fd5c0c6 |
U11 [mission-feature-sync + spec-staleness]: convert the last two planner-lane guards (48 -> 46) (#2610)
**Taking: `engine/mission-feature-sync.ts`, `engine/spec-staleness.ts`** — the last two planner-lane guards in my area. ## Census (comment-stripped, `=== "triage"` / `!== "triage"` in `packages/*/src`, tests excluded) | file | before | after | |---|---:|---:| | `packages/engine/src/mission-feature-sync.ts` | 1 | **0** | | `packages/engine/src/spec-staleness.ts` | 1 | **0** | | **repo total** | **48** | **46** | ## Both are real conversions, not seams Each guard takes its vocabulary from the **caller**, which holds the store — so unlike a defaulted parameter nothing passes, these can actually be driven. **`reconcileMissionFeatureState`** — a card back in a planner lane returns the mission feature to `triaged`. Keyed on literals, a renamed workflow left the feature reading `in-progress` forever: the roadmap claims work is underway while the card waits to be re-planned. Nothing errors; the rollup is just wrong. The vocabulary arrives via `MissionFeatureSyncContext` rather than by widening this module's deliberately narrowed `Pick<TaskStore, "getTask">`. **`shouldSkipSpecStalenessForPreservedProgress`** — returning `false` for a planner-lane card is what *keeps* staleness evaluation on. Miss the lane and it falls through to the preserved-progress branch, so a card with progress skips staleness and keeps a spec that should have been re-validated. ## The two take different defaults — and I got it wrong first I defaulted **both** to the `triage`/`todo` pair and broke the pre-existing U11 proof in `spec-staleness.test.ts`, which states the reason exactly: > same column, different status, opposite correct answer - **mission-feature-sync → the PAIR.** It asks "is this card waiting to be planned?", true in either lane. - **spec-staleness → the DEDICATED planner column only.** On a merged lineage `todo` is *also* the hold lane, so the planner distinction there is carried by **status** (`planning` / `needs-replan`), not by the column. Treating the merged column as a planner lane stops a parked card with preserved progress from skipping staleness. Its default is now the single legacy id — byte-identical to the literal it replaced. That asymmetry is now pinned by its own test rather than left for the next reader to rediscover. ## Findings on the remaining census, from measuring it Two of the 46 are **not lifecycle-column guards** and converting them would be wrong: - `tool-availability.ts:32` — `surface === "triage"` where `surface: "triage" | "executor"` is an **agent lane**, not a column. - `skill-resolver.ts:432` — `sessionPurpose === "triage"`, a **session purpose**. Also worth noting for the count: `replan-target.ts` reads as 2 in a raw grep but is **0** — both hits are inside comments. `board-workflows.ts` (2) and `archive-planning.ts` (1) are likewise comment-only. A raw grep says 52; comment-stripped says 46. ## Not wired at the call sites yet `scheduler.ts` / `mission-autopilot.ts` (mission sync) and `executor.ts` / `scheduler.ts` (staleness) still omit the new option, so behaviour is byte-identical today. Deliberate: `executor.ts` belongs to u8's active slice and I would rather not create a textual collision for a pass-through. The seam is proven by tests and the count is real; wiring is a follow-up. ## Verification - **Mutation-verified:** restoring either literal fails a test - 35 tests green across the three suites, merge gate green (482 + 132 + 10), tsc clean, lint clean No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
9a11e0b136 |
U2b reproduction: the live move path accepts the column U11 deleted (characterized, not patched) (#2601)
Found while proving U11's caveat 2. **Characterization plus guard-rails
— no production change, deliberately.**
## The defect
A default-workflow card in Planning can be moved **into `triage`** — a
column its workflow no longer declares — re-creating exactly the
stranded state `reconcileUndeclaredTaskColumns` exists to repair.
Measured on a fresh store:
```
experimentalFeatures.workflowColumns null ← no production writer
createTask(...) column = "todo"
moveTask("todo" → "triage") ACCEPTED
moveTask("todo" → "bogus-column") REJECTED: "Valid targets: in-progress, triage, archived"
```
The second rejection is the tell. Validation is real — but it is the
**legacy `VALID_TRANSITIONS`** table talking, and that table does not
know the card's workflow. Its `todo` row still lists `triage`.
## Why the workflow-aware check does not run
`moves.ts` gates its adjacency block — including
`workflowHasColumn(workflowIr, toColumn)` — on
`isWorkflowColumnsCompatibilityFlagEnabled`, which reads the raw
`experimentalFeatures.workflowColumns` key. Nothing writes it, so the
block is dead on the path every real project takes.
**Corollary, already reported:** U11's undeclared-source escape hatch in
`resolveAllowedColumns` also does not run in production. It was added
with #2515 so a stranded card would have a legal move instead of `Valid
targets: none`; on the live path that rescue comes from the legacy table
instead. Mutation-verified — stubbing the hatch back to `[]` leaves the
operator-move test green.
## Why I did not fix it
PR #2499 un-gated the capacity check and **explicitly scoped validation
out**:
> SCOPE, deliberately narrow: only the CAPACITY check is un-gated.
`workflowIr` stays flag-gated so transition VALIDATION keeps its current
behavior — the inline path's bare-Error/"Valid targets:" contract is
unchanged, and none of the Phase A2 divergences are flipped here.
That is a considered decision by the owner of this function, and several
suites pin the contract it protects. Overriding it from outside would
flip an error shape I do not own.
**What has changed since that decision is U11:** the legacy table now
offers a target the default workflow does not declare, which it never
did before. That is new input to the scoping call, not licence to ignore
it — so this lands as a reproduction for U2b rather than a patch.
U2b's branch (`feature/workflow-move-path-convergence`) is stale — HEAD
predates several merged PRs, clean tree — so nothing is being raced.
## What ships
The defect is **characterized, not asserted-as-correct**: the test pins
today's behaviour so it is visible and measurable, and an `it.todo`
states the intended behaviour. Writing it as a passing "refuses" test
would have required the fix; writing it as a failing test would redden
CI; asserting the current behaviour as *correct* would be a lie.
Characterization plus `it.todo` is the honest third option.
Four guard-rails pin what a fix must **not** break:
- every declared lifecycle move (`todo → in-progress → in-review →
done`)
- archiving
- a `recoveryRehome` deliberately reaching an undeclared column — the
path that rescues already-stranded cards, and the one a careless fix
would break
- a premise test asserting the compatibility flag really is unset, so
the suite fails loudly if that ever changes rather than silently testing
a different code path
## Exposure
Narrow but real. U10 already fixed the dashboard move menu to offer only
workflow-declared targets, so the board does not present this. The
**write path** does — REST API, CLI, plugins, any stale client — which
is why the guard belongs in `moves.ts` rather than only in the UI.
5 passed + 1 todo; lint clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Added coverage for task moves involving workflow-declared and
undeclared columns.
* Documented a known issue where tasks can currently be moved into the
deleted `triage` column.
* Preserved valid moves, archiving, and recovery re-homing behavior.
* **Documentation**
* Added reproduction steps, affected move paths, and guardrails for
addressing the issue.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
f47fc167ee |
convert(core/task-store/comments-ops.ts): triage guards 3 → 1, and the dead approval-invalidation it hid (#2608)
**Taking `packages/core/src/task-store/comments-ops.ts`** (announced for collision avoidance). Two commits: a behaviour-identical extraction, then the conversion. | File | triage column comparisons before | after | |---|---|---| | `packages/core/src/task-store/comments-ops.ts` | **3** | **1** | `pnpm test:gate` green. ## The bug the literal was hiding `builtin:coding` → `BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR`, whose merged Planning column keeps the id **`todo`** and declares **no `triage` column**. So `task.column === "triage" && task.status === "awaiting-approval"` never matched a default card. The damage was graded: - **with a real spec** — the card fell through to the re-triage arm. Same `needs-replan` write, but audited as *"requested re-specification of planned task"* instead of *"invalidated spec approval"*. - **with a bootstrap-stub spec** — `hasRealPrompt` was false and **neither arm fired**, so a user comment on a card awaiting spec approval invalidated **nothing**. The approval silently stood. That second case is the real regression; the wording is cosmetic. I checked both rather than assuming the first one was the whole story. ## The conversion The column was never the discriminator. Callers reach this only after establishing the card sits in a pre-implementation column, so re-testing it inside was redundant before U11 and wrong after. **Status carries the distinction** — the same conclusion `spec-staleness.test.ts` already reached for its sibling guard. **Red-green:** the 3 new cases fail with the literal reinstated (**3 failed / 4 passed**) and pass without it. Two assert the merged-Planning card is now invalidated; the third uses a `planning`-named column to show no column id remains in the decision at all. **The 1 remaining literal is deliberate:** the caller's gate `column === "todo" || column === "triage"` names *both* vocabularies, so it still fires for default cards, and narrowing it to traits needs an IR the caller doesn't have. Commit 1 is move-only — the extracted body is the inlined expression verbatim, `triage` literals included, so the moved logic diffs empty apart from field renames. Behaviour change is entirely in commit 2. --- ## Census correction — the 48 is 41, and "reach ZERO" is wrong as stated I re-measured before picking a file, and the shared number needs three corrections. Same-scope method: `packages/*/src`, `.ts`, tests excluded, **comments stripped**. | Measurement | Count | |---|---| | raw `=== "triage"` / `!== "triage"` | 54 | | …comments stripped | **48** ← matches your figure | | …of those, genuine **column** comparisons | **41** | | …non-column identifiers that must NOT be converted | **7** | The 7 are `role === "triage"` ×3 (`agent-prompts.ts`), `agentType === "triage"` ×2 (`usage-limit-detector.ts`), `sessionPurpose === "triage"` (`skill-resolver.ts`), `surface === "triage"` (`tool-availability.ts`). **The triage service keeps its name; only the column id was merged away.** Converting these would break the triage lane, so the bar cannot be literal zero — it's zero *column* comparisons, with those 7 documented as permanent. Two I nearly misclassified and hand-checked: `col === "triage"` (`cli/commands/task.ts`, indexes `COLUMN_LABELS`) and `from === "triage"` (`executor.ts`, a `moveTask` from-column) **are** columns despite their names. ## Of the 41, which are actually dead Splitting by whether a `todo` companion arm sits in the same condition: - **27 have one** → still fire for default cards. Real but lower priority. - **14 have none** → candidates for silently-dead. But on inspection that set shrinks further: - `register-task-workflow-routes.ts` ×5 compare against a *resolved* `approveIntakeColumn`/`refineIntakeColumn` variable **plus** a legacy `"triage"` fallback, so they still fire via the variable; - `spec-staleness.ts:40` is a **deliberate R11 compat retention** — `spec-staleness.test.ts` already carries a "U11 proof" block concluding the guard is carried by status, not column, and that other workflows still declare `triage`. Converting it would be wrong; - `self-healing.ts` ×7 is U4's file; - `comments-ops.ts` ×1 was genuinely dead — this PR. **So the actionable dead set is far smaller than 14, and `self-healing.ts` holds most of it.** I'd suggest whoever takes `self-healing.ts` starts from that 7 rather than its 11 total. ## Files I evaluated and did NOT convert - **`replan-target.ts`** — my first pick, then both its "sites" turned out to be **comment text**. Zero real sites; already trait-resolved via `workflowHasColumn`. - **`mission-feature-sync.ts:88`** — `(column === "triage" || column === "todo")` still fires via the `todo` arm. The genuine gap is a custom-named planning column, but `reconcileMissionFeatureState`'s store is narrowed to `Pick<TaskStore,"getTask">`, so trait resolution means plumbing through `scheduler.ts` — **U5's file**. Left to avoid the collision, per KTD-2's warning that most sites have no IR in scope. - **`tool-availability.ts` / `skill-resolver.ts` / `usage-limit-detector.ts`** — non-column identifiers, see above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
45e8b5f7ac |
U8: pin the completion-finalize ordering invariant before moving the last out-of-band exit (#2599)
Groundwork for moving `paused-after-completion`, the **last** out-of-band exit. Stacked on #2590. ## What lands 1. **An indentation defect I introduced.** My bulk edit when the exit vocabulary landed left the second `paused-after-completion` site mis-indented inside a `finally` block. Cosmetic, but misleading indentation in a `finally` is how a future reader misjudges scope. 2. **The adjacency ratchet now requires `markCompletionFinalized` before the handoff, at every reporting site.** It previously checked only the first occurrence, and only for the handoff itself. That ordering is the invariant `handleGraphFailure` depends on and **cannot check for itself**: `alreadyFinalizedToReview` / `completionFinalized` exist to recognise this out-of-band move when a later teardown re-marks the abort as `hard-cancel`. Without the durable marker set first, a completed no-commit task is re-parked `failed` — FN-6644/FN-6641. It is asserted **structurally, and labelled as such in the test**. Both call sites sit in pause and `finally` paths that cannot be driven without mocking an entire agent session; presenting a source assertion as behavioural coverage would repeat the overclaim I have been correctly pulled up on twice in this unit. Red-green: removing `markCompletionFinalized` from either site fails the ratchet. ## Why the move itself is not in this PR `paused-after-completion` is structurally harder than the pending-review ending that #2590 moved, and the difference is worth recording before someone assumes it is a copy-paste: - it does **four** things, not one — `markCompletionFinalized`, `handoffTaskToReview`, `clearCompletedTaskWatchdog`/`signalTaskComplete`. Only the handoff is lifecycle; the rest is substrate that must stay put. - one of the two sites is inside a **`finally`**. Moving a transition out of a `finally` is not the same operation as moving one out of a branch: the graph may already be unwinding, so "report and let the graph route" needs a defined answer for a run that is already ending. - there is **no behavioural coverage of either site today** — the closest tests only exercise the exit vocabulary. The pending-review move succeeded on the fourth attempt precisely because FN-5436 existed to catch each wrong version; this exit has no equivalent, so the move needs that floor built first, and building it means real session mocking rather than a shortcut. ## Verification - exit-events + primitive-exit-events + step-session + ownership ledger — green - `pnpm lint` clean; `tsc --noEmit` clean - No user-facing behaviour change, so no changeset 🤖 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 handling of workflow steps that pause for review. - Tasks now remain in review when a review request has no subsequent decision. - Added clearer completion events for primitive prompt steps. - Preserved correct failure handling when later workflow steps fail. - **Workflow Improvements** - Built-in workflows now route pending reviews through a dedicated review handoff. - User-authored workflows retain compatible review parking behavior when routing is unavailable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
3f763cba87 |
U8: the graph owns the pending-review park — ownership ledger 28 → 27 (#2590)
The routing move this unit has been building toward, landing on the path the engine actually runs. **Includes #2578's commit** (the live-path fix it depends on) — merge that first, or this supersedes it. ## What changes Three things together, because a half-routed move is a card that silently does not advance: 1. The **live** implementation primitive (`runCodingSession`) returns `{outcome: "failure", value: "review-pending"}` for that ending. 2. The primitive step handler stops flattening every ending to `step-done`/`step-failed`, so the value survives the foreach — `runForeach` propagates a failing instance's value as the node's own — and reaches an edge. 3. The inline `handoffTaskToReview` in `runImplementation` is **deleted**. The phase reports and stops, which is all an implementation phase should do. Built-in workflows route to the `review-pending-handoff` node added in #2519/#2546, which performs the handoff and ends the run: the same two effects in the same order, with the graph as the owner. ## Proof, end to end FN-5436 — the test that blocked this move twice and was right both times — now passes, with a **stronger** assertion than it had: ```ts expect(store.moveTask).toHaveBeenCalledWith("FN-5436-B", "in-review", expect.objectContaining({ workflowMoveSource: "workflow-graph", workflowMoveMetadata: expect.objectContaining({ nodeId: "review-pending-handoff" }), })); ``` The old two-argument `moveTask(id, "in-review")` could not distinguish a graph-owned park from an out-of-band one — which is the entire distinction this unit exists to make. The invariant (park in review, never `failed`) is unchanged; the owner is now proven. ## Every ratchet fired, and each records a real change | Ratchet | Before | After | Why | |---|---|---|---| | Ownership ledger — `runImplementation` review handoffs | 3 | **2** | the handoff left the phase | | Ownership ledger — `handleGraphFailure` | 0 | **1** | the named compat classifier | | Ledger headline — executor-owned dispositions | 28 | **27** | first decrement of the unit | | Out-of-band exit list | 2 | **1** | pending-review is graph-owned now | | Primitive routing pin | "must not reroute" | routes *only* the moved ending | declared, not discovered | None was relaxed. The `handleGraphFailure` 0 → 1 is the honest one: for a user-authored graph without the edge this is a **relocation, not an elimination** — the transition is still executor-performed, but from one named classifier in the failure ladder rather than a call buried two thousand lines into a session loop. The ledger says so rather than letting the headline number imply more progress than there is. ## Why it took four attempts Recorded because the reason is reusable: the value was being produced on `createAuthoritativeWorkflowSeams`, a handler that never runs (#2578). Every earlier attempt was correct code on a dead path, and the only thing that showed it was instrumenting until a negative result was proven observable rather than assumed. ## Verification - step-session + exit-events + primitive-exit-events + ownership ledger + graph-requeue-gate + task-done-blocked — **83 tests green** - `pnpm test:gate` green (10 / 482 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `internal`) 🤖 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 handling of tasks awaiting review so they are correctly routed to the review workflow. * Tasks now remain in review instead of being marked as failed when no follow-up review route is configured. * Review handoffs now include workflow ownership and provenance details. * Preserved standard failure handling for tasks that are not awaiting review. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
d5f1ce7abd |
U11 [writes]: stop CREATING cards into a column the workflow no longer declares (9 -> 0, engine+cli) (#2603)
**Taking: `engine/triage.ts`, `engine/pr-comment-handler.ts`, `engine/eval-followups.ts`, `cli/commands/task.ts`, `cli/extension.ts`** (write class — no collision with the comparison backlog). ## A class the census does not count The 48-guard work list tracks `=== "triage"` **comparisons**. These are `column: "triage"` **writes** — and post-#2515 every one creates a card directly into the state STALL 3 was about, except **manufactured continuously** rather than left behind by the upgrade. ## Why they bite `createTaskImpl` resolves the column as: ```ts column: input.column || options?.resolvedEntryColumn || fallbackIntakeColumn || "triage" ``` `input.column` **wins**, so an explicit `column: "triage"` overrides the workflow's resolved intake column entirely. `store-create-intake-column.test.ts` already pins that a create with **no** column lands in the default workflow's intake (now `todo`) — these callers opted out of it. The sharpest is `triage.ts`'s `fn_task_create` agent tool: it passed `workflowId: params.workflow_id` **and** `column: "triage"` in the same call. The caller chose a workflow and the column ignored it — a Coding (Ideas) create landed in `triage` instead of `ideas`. ## Counts **Comparison guards: unchanged by this PR.** This is the write class; conflating the two would misreport convergence toward the zero bar. | file | `column: "triage"` writes before | after | |---|---:|---:| | `packages/engine/src/triage.ts` | 1 | **0** | | `packages/engine/src/pr-comment-handler.ts` | 1 | **0** | | `packages/engine/src/eval-followups.ts` | 1 | **0** | | `packages/cli/src/commands/task.ts` | 3 | **0** | | `packages/cli/src/extension.ts` | 3 | **0** | | **total** | **9** | **0** | ## A test that pinned the defect `pr-comment-handler.test.ts` asserted `column: "triage"` in the createTask call — so it would have **failed the fix and passed the bug**. Rewritten to assert the invariant (the caller passes no column, so the workflow's intake wins) plus an explicit `Object.hasOwn(arg, "column") === false`, which is what actually catches a reintroduction. ## Interaction with #2591 My merged #2591 rescues these cards once created — they sit on a legacy planner id their workflow doesn't declare and are still in planning stage. So this isn't a *visible* stall today; the rescue absorbs it. **That's the reason to fix it rather than leave it:** a self-healing path silently absorbing a steady stream of malformed creates is exactly how the underlying defect stays invisible. ## Deliberately not touched - `{ id: "start", kind: "start", column: "triage" }` in the builtin coding / PR / lead-generation IRs — workflow-internal **node declarations** for workflows that still legitimately declare a `triage` column, not lifecycle writes. - Left for their owners: `core/task-store/project-store-ops.ts:210`, `core/task-store/update-task-deps.ts:111` (main worker), `dashboard/src/routes/register-gitlab.ts:108` (u12). Same defect, same one-line shape. ## Verification - 304 engine/CLI tests green across the affected suites - merge gate green (482 + 132 + 10), engine + CLI tsc clean, lint clean No changeset: `@fusion/engine` and `@fusion/core` are private; the CLI change is a bug fix with no user-facing API change — happy to add one if you'd rather it appear in release notes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |