d5030c55eafe7acbc2ce416bcb8dc7be2f5f0079
3567 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
gsxdsm
|
d5030c55ea |
test(engine): prove BOTH rebound paths on a renamed board — 2 more ledger entries closed (#2510)
Stacked on #2508. Test-only. ## Why this site matters more than most `resolveReboundTarget` answers one question: **where does a recovered card go back to?** Keyed on the literal `todo`, a recovered card on a renamed board is requeued to a column that board **does not declare**. That is not cosmetic — an undeclared column carries no trait flags, so `findColumn` returns undefined and the card becomes invisible to every trait-driven sweep: nothing schedules it, nothing releases it, the board does not draw the column. **The "recovery" strands the card harder than the failure it was recovering from.** One of the two covered paths, `reconcileUndeclaredTaskColumns`, exists *specifically* to repair that state — which makes it the worst possible place for this bug to live. ## Covered, each mutation-verified independently | site | mutation | result | |---|---|---| | `reconcileUndeclaredTaskColumns` | target → `"todo"` | exactly the 2 renamed cases fail | | `autoRecoverWorktreeSessionStartFailure` | rebound → `"todo"` | exactly the renamed requeue fails | Neither needs git — the corrected ledger lens from #2508 (*what the function touches*, not *what family it sits in*) made that obvious rather than assumed. ## Both negatives included "Re-home anything whose column looks wrong" would be a louder failure than the strand it repairs, so: a card whose column **is** declared is left alone, and an operator `userPaused` park is never undone. ## Fixture finding, kept in-file `updateTask({ userPaused: true })` leaves the field `undefined` on both `getTask` and `listTasks({slim:true})`. Seeding it that way produced a card the sweep **correctly** saw as unpaused — a broken fixture that would have read as a broken guard, and would have looked like a real safety hole in the paused-park protection. Found by probing the persisted row rather than trusting the write. Now seeded through the integer column directly, and the test asserts the seed took effect *before* exercising the sweep, so this cannot silently regress into a vacuous pass. ## Verification - three live-E2E suites green together (lifecycle, merge-family, rebound-family) - engine `tsc --noEmit` clean - `pnpm test:gate` green (307 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
4eaa509024 |
test(engine): prove merge finalization on a renamed board — 3 ledger entries closed, and the ledger itself corrected (#2508)
Test-only. Closes three `auto-merge-finalization` entries from the unproven-sites ledger. ## Why this one first It is the **last move a card makes**. Keyed on the literal `done`, a renamed board's proven-merged card is moved to a column its own workflow does not declare — or refused and left stranded in review **with the work already landed**. That is the most expensive failure shape in the lifecycle, and nothing had run it against a renamed workflow. ## The ledger was wrong, and is corrected in this PR My own ledger said this family *"needs a REAL git worktree, branch, and squash … an engine-slow real-git lane, not another table row"*. `finalizeProvenAutoMergeTask` **needs no git at all** — the merge proof is a field on the row. It was reachable the whole time. The inference came from *the family the code sits in* rather than from what the function actually touches, and it parked reachable coverage for a slice. The correction is written into the ledger so the remaining entries get re-checked the same way rather than inheriting the assumption. ## A second correction, from mutation-testing rather than reading I first claimed `resolveMergeOrchestrationColumn` as covered because it sits in the same resolver as the other two. **All cases passed with it hardcoded.** It changes only whether finalization records a column-mismatch *repair* — never where the card lands, which is why the other cases are blind to it. It got its own case. Keyed on `in-review`, a renamed board's card resting in `checking` compares unequal, so **every ordinary finalization would be audited as repairing a mismatch that never existed** — a healthy board reads as one constantly self-healing, and the audit trail operators use to spot real strandings fills with false positives. Sitting next to covered code is not coverage. ## Mutation-verified independently | mutation | result | |---|---| | `completeColumn` → `"done"` | 3 fail — both renamed cases + the differential | | `isCompleteColumn` → `id === "done"` | exactly the already-done case fails | | `mergeColumn` → `"in-review"` | exactly the new audit case fails | ## Shared fixture extracted (pure move) The vocabulary + IR builder moved to `_workflow-vocabulary-fixture.ts` so the two suites cannot drift into testing different workflows — two copies of a differential fixture is precisely how a renamed-workflow test starts passing for reasons unrelated to the code under test. The lifecycle suite is unchanged: **20/20 before and after**. The `mergeOrchestration` trait is an opt-in option so the existing suite's IR stays byte-identical. ## Fixture note worth keeping Seeding needed **completed steps**: task creation parses three pending steps out of the bootstrap PROMPT even with `applyDefaultWorkflowSteps: false`, and `getTaskHardMergeBlocker` refuses on them (`"task has incomplete steps"`). Found by the suite blocking on **both** vocabularies — the signature of a broken fixture rather than a broken guard. ## Verification - 51/51 across the lifecycle, merge-family, ratchet and hold-release suites - engine `tsc --noEmit` clean - `pnpm test:gate` green (307 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8288e4a8ab |
U7 PR3: the specification reaction acts on what finalize DID, not on the fact that planning stopped (#2506)
Completes the pair started in #2498. That landed the outcome; this makes the engine's reaction consume it. ## The bug `onSpecifyComplete` fired on **every** finished specification, because the seam announcing it fired unconditionally. So a card parked at the manual plan-approval gate — finalize writes `status: "awaiting-approval"` and **returns early**, before the release move — was logged as `Specified X → todo` and had a Plan Review run armed for a plan the operator had not approved. #2491 stopped the **seeder** from acting on that, defensively, at the seeder. This removes the reason it was ever asked. Both layers are deliberate and neither is redundant: - the seeder guard covers **every caller**, including self-healing's re-seed; - this one stops the engine doing work nobody asked for, and stops it telling the operator something false about their own board. `released` is the only outcome that licenses arming a run — the only one meaning the card crossed into the hold column (or was already resting there, plan-in-place) and is the graph's now. `parked` belongs to a human; `withheld` belongs to the caller's retry budget. ## The event still fires on every outcome Deliberately. Dropping the reaction for a non-release would also drop the runtime's `recordActivity()` idle signal, and a reaction that silently does not happen is harder to reason about than one that happens with an accurate payload. R5's division of labour: **the seam announces, the subscriber decides what a given outcome licenses.** ## Why there is a new extracted function `reactToSpecificationComplete` is pulled out of the inline `InProcessRuntime` callback for the same reason the continuation drain was in #2491: the callback is built inside a class whose construction attaches to the real central project registry, so no test could distinguish *"the reaction respects the outcome"* from *"the reaction ignores it"*. **Revert proof:** with the outcome gate removed from the reaction, **5 of 8 fail**. ## Two call-site decisions worth naming **`tryFinalizeExplicitDuplicateMarker` reports through a mutable ref, not a widened return type.** Its boolean answers a *different* question — "was this a duplicate marker at all?" — and 16 existing tests assert it directly. I tried the widened return first and it turned all 16 red. Expectation edits are exactly how a behavior change travels disguised as churn, so I backed it out. **This diff touches zero existing test expectations.** **A duplicate-marker redirect reports `parked`**, which is accurate: it deletes, flags, or clears the marker; it never releases the card into the hold column. ## A fixture note — third of this shape on the program My "task vanished between release and reaction" case passed `undefined`, which triggered the harness **default parameter** and silently handed the reaction a live task — making it a duplicate of the control rather than the case it claimed to be. It now passes `null`, with a comment saying why. Running tally of near-false-greens on this unit, all the same family: a fake that ignores its predicate (#2491), a stub that ignores its callback (#2498), a default parameter that swallows the interesting input (here). Each was caught by the test failing for the *wrong reason* and being read rather than fixed. ## Verification | Check | Result | |---|---| | new suite | 8/8 | | 15 triage / planning / continuation suites | 361/361, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (307 + 10 + 71) | | `pnpm check:changesets` | clean | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
a2b4ca76ac |
U11: delete the unreachable legacy todo dispatcher from scheduler.schedule() (-929 lines, pure deletion) (#2505)
Based on `main`. **Pure deletion — no behavior change**, because the
deleted code cannot execute.
## Found while trying to convert it
This started as a U11 slice to make the scheduler's dispatch path
resolve its column by trait. Per the lesson from the dependency-blocked
feature I checked reachability *before* converting:
```ts
function shouldRunWorkflowColumnScheduler(_settings: Settings): boolean {
return true; // parameter UNUSED, body a literal
}
...
if (shouldRunWorkflowColumnScheduler(settings)) {
await this.runHoldReleaseSweepPass(tasks, settings);
...
return; // UNCONDITIONAL, at the block's own depth
}
<929 lines of legacy pull-from-todo dispatcher> // unreachable
```
The guard takes an **unused** parameter and returns a **literal**, so
the branch is statically always taken, and it ends in an **unconditional
`return`**. Everything after it in `schedule()` is unreachable.
`tsc` doesn't flag it because the condition is a function call rather
than a literal — which is exactly why 929 lines survived the U6 cutover.
The replacement was added *in front of* the old dispatcher rather than
*instead of* it, and the in-file comment says so outright:
> the hold/release sweep owns todo→in-progress pickup, so do not fall
through into the legacy pull-from-todo dispatcher after the sweep runs
## Why this matters beyond line count
**4 of the 15 `"todo"` literals in `scheduler.ts` live in this dead
region.** Converting them would have been pure waste — and worse, it
would have reported progress against the U11 critical path while
changing nothing. 11 live sites remain and are the real work.
## Corroborating evidence
Six imports became unused and are removed with it:
`resolveDependencyOrder`, `sortTasksByPriorityFanoutThenAgeAndId`,
`buildUnblockWeightMap`, `TransitionRejectionError`,
`isUnplannedSeedPrompt`, `DEFAULT_WORKFLOW_POOL_ID`.
That the dead region was their **only** consumer in this file is itself
evidence: a live dispatcher would still need dependency ordering and
priority sorting.
## Why no new test
The proof here is **static, not behavioral** — an unconditional `return`
before the code. A test cannot demonstrate absence of execution more
strongly than the control flow already does, and one that passed both
before and after would be theatre.
The evidence that nothing depended on it: **all 100 scheduler tests and
the full merge gate pass unchanged.**
## Measured
`scheduler.ts` **3,726 → 2,797 = −929 lines.**
Unlike every consolidation in this program, this is a **genuine net
reduction** — nothing was moved elsewhere.
## Verification
100 scheduler tests green across all 7 scheduler suites; merge gate
green (307 + 10 + 71); tsc clean; lint clean.
No changeset: `@fusion/engine` is private.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||
|
gsxdsm
|
e4004c8694 |
U9 baseline: pin merge-region IR config as a dead policy authority (test-only) (#2494)
**U9, PR1 of several.** Test-only, no production code touched. This is
the characterization baseline the plan's Execution note asks for before
the merge lane converts.
## The finding
`builtin-coding-workflow-ir.ts` declares merge-region policy that **no
engine code reads**:
| IR declaration | Consumed by |
|---|---|
| `merge-retry` → `{ policy: "merge", maxAttempts: 3 }` | nothing —
`retry-backoff` handler is `async () => ({ outcome: "success" })`
(`workflow-node-handlers.ts:728`) |
| `merge-manual-hold` → `{ release: "manual" }` | nothing — returns a
constant `manual-required` |
| `branch-group-*` → `{ maxReworkCycles: 3 }` | nothing — returns a
constant `success` |
Live merge policy authority is elsewhere, on two separate axes:
- **conflict** retries — `settings.maxAutoMergeRetries` (default 3),
already covered by `auto-merge-retry-cap-settings.test.ts`
- **transient** retries —
`ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES = 5`
(`project-engine.ts:545`)
So the IR is a **third, dead authority**. These are different axes, not
a same-axis contradiction — but a reader looking at the IR would
reasonably take the declared numbers as live, and nothing currently says
otherwise. U9's acceptance criterion is "merge policy changes via IR
config alone, with no code change"; that fails today and this pins why.
## Why characterization rather than a fix
Making these handlers config-driven is a **merge behavior change**, and
the `requestMerge` primitive it routes through lives at
`executor.ts:7383` — inside U8's blast radius. U9 is sequenced behind U8
precisely so the merge lane converts onto an executor that is already
substrate. Landing the behavior change now would change merge semantics
on an executor about to be reshaped. It lands inside U9 proper.
When U9 wires a node kind onto its IR config, the matching case here
goes **red** and the U9 commit must move that kind out of
`CONFIG_BLIND_MERGE_REGION_KINDS`. That is the ratchet working.
## Proof it fails when reverted
A test that passes with the change reverted is not a test. The "change"
here is the test itself, so the honest analogue is mutating the
characterized production behavior. Three independent mutations, each
reverted after measuring:
| Mutation | Result |
|---|---|
| `retry-backoff` honours `config.maxAttempts` (what U9 will do) | **2
failed** / 6 passed |
| `manual-merge-hold` honours `config.release === "external-event"` |
**2 failed** / 6 passed |
| IR declaration drift: `maxAttempts: 3` → `7` | **1 failed** / 7 passed
|
Measured: 8 tests, 4.16s. `pnpm lint` clean. Tree restored to clean
after each mutation.
The assertions are behavioral, not string matches: each handler is
invoked with two contradictory configs (opposite budgets, opposite
release modes, disjoint surfaces) and asserted to return deep-equal
results.
## Six safeguards
This PR changes no production behavior, so no safeguard is altered by
it. The full six-row table with test attribution is the required
artifact for the **conversion** PR, not this one. Baseline located so
far, to be completed and verified by mutation before any conversion
lands:
| # | Safeguard | Consulted at (today) | Test attribution |
|---|---|---|---|
| 1 | user pause | `project-engine.ts:645` (`task.paused \|\|
task.userPaused`) | not yet verified |
| 2 | `autoMerge:false` | `allowsAutoMergeProcessing` —
`project-engine.ts:2797`, `merger.ts:7178` | not yet verified |
| 3 | dependency gating | not yet located | not yet verified |
| 4 | capacity | not yet located |
`workflow-column-boundary-capacity.test.ts` (unverified) |
| 5 | merge-proof | `getTaskMergeBlocker` — `project-engine.ts:2609` |
`merger-file-scope-invariant.test.ts`,
`merger-diff-volume-gate.slow.test.ts` (unverified) |
| 6 | at-most-once merge | `activeMergeTaskId` single-flight —
`project-engine.ts:693`/`:2729` | not yet verified |
Rows 3, 4 and all attributions are honestly incomplete rather than
asserted — I will not present a table I have not earned.
## Also found, for the coordinator
- **Slice statuses are stale.** S02/S03/S04 in
`docs/plans/workflow-owned-merge-stack/` are all marked
`draft-stack-handoff` but S04 has **landed** (the merge-region IR nodes
above), S03's `claimDueWorkflowWorkItem` is implemented and wired via
`workflow-work-processor.ts`, and S02's
`projectMergeRequestToWorkflowWorkItem` is implemented with **zero
production callers**. S06/S07/S08 are genuinely not started. Doc
correction coming as its own small PR.
- **S1 prerequisite verified present, not assumed** — all four store
methods live in `store.ts`, migration `0031` in tree. No S1-completion
gap.
- **Second control plane into the merge lane:** `self-healing.ts:3198`
and `:7200` call `enqueueMerge` directly, bypassing the graph. That
needs to become a recovery-fact/wake (the stack's R6) during U9.
🤖 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 a new test suite to cover U9 merge-region behavior across
supported workflow node types.
* Verified merge-region results are consistent across built-in,
contradictory, and missing configuration inputs.
* Documented current behavior for retry backoff (always succeeds) and
manual merge hold (fails as manual-required).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
eaea082259 |
U8 PR2: the execution-policy ladder resolves its own workflow's columns (the wip literal made retry, escalation and loop protection unreachable) (#2497)
Second PR of **U8 — the graph owns execution**, independent of [#2490](https://github.com/Runfusion/Fusion/pull/2490) and of every other unit. Small, green, independently revertable. ## The defect `handleGraphFailure`'s execution-policy ladder — FN-7863/FN-7926 dispatch-loop terminalization, FN-7996 tool-failure retry, FN-7998 escalation — decided a task's own lifecycle by naming `"todo"` and `"in-progress"` **literally, at 9 sites**. U5b converted the executor's *rebounds* to `resolveReboundColumnFor`; these were left behind, each sitting somewhere an awaited resolver could not reach: inside synchronous `updateTaskAtomic` mutators, inside fire-and-forget resume closures, and in conditions evaluated before any resolution happened. **The severe one is the wip gate, and it fails silently in the worst direction:** ```ts if (live.column !== "in-progress") { // "Workflow graph run ended after task already advanced — no further action needed" return; } ``` Under a workflow that renames the implementation column, that is true of a card sitting in **its own wip column**. So the graph failure was swallowed whole — no terminal park, no status, no error, nothing on the board — and the scheduler re-dispatched the same doomed run. Every later branch sits behind that gate, which is why the retry budgets, the escalation, and the bounded terminalization were **unreachable rather than mistargeted**. This is precisely the failure the program's problem frame predicts: *a guard that stops matching disables a recovery path invisibly and the suite stays green.* I found it because my first renamed-column test for the escalation site could not reach the escalation code at all. Two further sites misbehave once the gate is passable: - **FN-7998 node escalation** wrote `column: "todo"` inside the atomic claim — parking the card where no workflow declares it, which is on the plan's **"Stop implementation if"** list and what R7 exists to clean up after. The scheduler's effective-node resolution, the entire point of a node escalation, never runs. - **FN-7863/FN-7926's `live.column === "todo"` arm** is the classic guard that stops matching. In-process the `executeNodeSelfRequeued` marker covers the same case, so this degrades only on the **durable** arm — after a restart, or for a second `TaskExecutor` instance in the process, where the column read is the only evidence the inner executor requeued. A progressing card then falls through to the terminal sink and is parked `failed`. ## The fix Resolve hold and wip **once per graph failure** through U1's `resolveTaskLifecycleColumns` and thread the pair through the ladder. Both fall back to the legacy literal when the workflow cannot be resolved, so an unresolvable workflow keeps exactly its pre-conversion behavior rather than guessing. One IR read on a terminal recovery path — not an enumeration loop. ## Red-green, measured **3 of the 8 new tests fail with this commit's executor change reverted:** ``` FAIL FN-7998 … > requeues a node escalation to the RENAMED hold column, not the literal todo FAIL FN-7998 … > still does not move the card for a MODEL-target escalation FAIL FN-7863/FN-7926 … > recognises an inner-executor requeue that landed in the RENAMED hold column Tests 3 failed | 5 passed (8) ← reverted Tests 8 passed (8) ← with the fix ``` The other **5 pass both ways by design**, and I am not claiming them as red-green — they are the regression floor: - default coding workflow still resolves hold → `todo`, wip → `in-progress` (byte-identical); - an unresolvable workflow still uses the legacy literals; - the in-process self-requeue marker still works when no workflow resolves; - and a **negative case** proving the dispatch-loop gate stays narrow — a card still in its wip column with no marker is a genuine execute failure and must NOT be swallowed as a benign recovery. Widening that gate to "any column" would have been the easy wrong fix. ## Scope Deliberately the execution-policy ladder only. **20 further column literals remain in the same method's pause-abort, merge, and in-review regions** — they belong to U5's executor slice (B4, not started) and U9's merge lane, and are untouched here. Flagging the overlap: this PR edits `executor.ts`, so whoever takes U5-B4 should rebase onto it rather than converting these 9 sites again. ## Verification - 8 new tests + the preserved-behavior suites (`executor-tool-failure-retry`, `executor-graph-requeue-gate`, `executor-task-done-blocked`, `executor-graph-boundary`, `executor-stuck-requeue-preserve-progress`, `executor-paused-abort-todo-benign`, `executor-abort-provenance`) — **9 files, 112 tests, green** - `pnpm test:gate` — green (2/10, 16/299, 1/71); `pnpm lint` clean; `tsc --noEmit` on `@fusion/engine` clean - Changeset included (`patch`, category `fix`), passes `pnpm check:changesets` 🤖 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** * Fixed execution recovery for workflows with renamed lifecycle columns so retry, escalation, and loop-protection behaviors correctly follow the workflow’s declared hold/WIP columns. * Preserved legacy behavior for default workflows and continued safe handling when lifecycle columns can’t be resolved. * **Tests** * Added a Vitest suite validating execution-policy “ladder” behavior for renamed columns, including node escalation, dispatch-loop gating, and fail-closed scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
2934cccad8 |
U7 PR2: finalize reports what it did with the card — a refused planning handoff is retried, not counted as recovered (#2498)
## The bug `finalizeApprovedTask` has ~25 exit points and returned `void`, so no caller could tell *"the card was handed off"* from *"finalize gave up"*. Both callers assumed success. `recoverApprovedTask` returned `true` **unconditionally** after finalize, and `handleStuckAbortRequeue` treats `true` as "recovery done, stop here". So when the release move was **refused by the planning-stage guard** (FN-8361), or the store could not perform the move at all, recovery reported success and the card's stuck-retry budget was skipped — nothing re-planned it, nothing escalated it, and it sat in the planner column holding a finished spec. The refusal was already logged loudly by FN-8596's visibility work. The return value was the part still lying. ## Three states, not a boolean This is the load-bearing decision in the PR: | Outcome | Meaning | Retry? | |---|---|---| | `released` | crossed into the hold column, or already resting there (plan-in-place) | n/a — handed off | | `parked` | deliberate, terminal-for-now: awaiting manual plan approval, duplicate decision, operator pause, deleted duplicate | **no** — a human owns it | | `withheld` | finalize could not complete the handoff, nothing waiting on a human | **yes** — caller's budget owns it | `recoverApprovedTask` returns `outcome !== "withheld"`, so **`parked` still returns `true`**. Narrowing to `=== "released"` is the tempting simplification and it is wrong: it would send the stuck handler down its draft path and stamp `needs-replan` over a plan a human is mid-review on — a worse bug than the one being fixed. That is asserted, and the assertion fails under exactly that narrowing. ## Why a mutable report, not a return at each exit Threading a return through 25 exits is 25 chances to mis-classify a branch, and mis-classifying turns a truthfulness fix into a lifecycle bug. The report defaults to `parked`, which is equivalent to today's observable behavior at every exit — so the plumbing is **inert everywhere except the three sites explicitly classified**. Adding a state to an exit is then a deliberate, reviewable act rather than a diff-wide judgement call. Only **two** exits are marked `withheld`, both in the release block, both already warning loudly. Deliberately *not* marked: - the `updatePlanningStateIfStillCurrent` guard — FN-8024 says a normal scheduler advance legitimately lands there; the card has moved on, so a retry would be wrong. - `recoverMissingPromptBeforeRelease` — it owns its own recovery budget; retrying would double up. ## Revert proofs (measured) | Reverted | Result | |---|---| | `recoverApprovedTask` back to unconditional `true` | `Tests 2 failed \| 3 passed (5)` | | narrowed to `outcome === "released"` | `Tests 1 failed \| 4 passed (5)` — the approval-park control | The second row is the point: the park case is load-bearing, not decoration. ## A fixture note that nearly produced a false green A `vi.fn()` stub for `updateTaskAtomic` that ignores its callback makes **every** finalize report "no longer in the planning stage" and return before the release — silently collapsing every case into the same uninteresting early exit. My first run was 3 failures for that reason, not the reason I expected. The fake now applies the patch, and the control asserts `moveTaskIf` was actually reached. Same class as the `moveTaskIf` fake caught on #2491; recording it so the next person recognises the shape. ## Scope The other caller — `specifyTask`'s unconditional `onSpecifyComplete` — is **not** gated here. Reaching it needs a live planning session, so gating it without first extracting the reaction would be a change I cannot prove, which is exactly the finding review caught on #2491's deferral. That lands next, on this plumbing. ## Verification | Check | Result | |---|---| | new suite | 5/5 | | 11 triage/planning suites (triage, finalize-duplicate-lineage, stuck-requeue-preserve-draft, explicit-duplicate-marker, preflight, plan-artifact-writeback, refinement-routing, planning-wake, planning-evacuation, …) | 326/326 | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (299 + 10 + 71) | | `pnpm check:changesets` | clean | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8aba310d78 |
U11: resolve the worktree-acquisition requeue column by trait (2 sites, both branches) (#2496)
Based on `main`. First of my U11 conversion PRs — small, green, independently revertable. Both heartbeat worktree-acquisition requeue sites hardcoded `"todo"`. ## Why this is critical path, not a renamed-workflow nicety **U11 deletes the `todo` column from the builtin workflows.** After that, these two sites would requeue every acquisition-failed card into a column that no longer exists. ## Both sites converted together They are different branches of the same failure: - the **bounded-retry** requeue, and - the **retry-cap-exhausted** terminal park. Converting one and not the other would leave the rarer path — which fires only after three consecutive failures, so it's the one least likely to be noticed — still writing the literal. Target is the KTD-10 ordering via `resolveReboundTarget` (hold → intake → first column): the same helper `self-healing` and `mesh-lease-manager` already use for "requeue a recovered card", so the recovery paths cannot drift apart. ## What is deliberately untouched `preserveStatus: true` on the exhausted path. It exists because reopen-to-todo semantics would otherwise wipe the `status: "failed"` written immediately before (FN-7721) — changing the column must not disturb that flag. A test asserts the full options object, not just the column. Fail-soft to the legacy id: a requeue must not be abandoned because a workflow lookup failed, or the card is left holding a worktree it could not acquire. Covered by a regression-floor test. ## Verification - **Mutation-verified:** restoring the literal fails 2 of the 3 new tests - 7 tests green (3 new + the 4 pre-existing worktree tests, unchanged) - tsc clean, lint clean, merge gate green (299 + 10 + 71) ## Measured progress **2 of the 74** code-level `"todo"` sites in my unit (engine recovery/scheduling core) are now trait-resolved. Remaining in-unit: `self-healing` 48, `scheduler` 15, `triage` 8, `replan-target` 1. `stuck-task-detector` needs **no work** — all 4 of its occurrences are comments, not code. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Tasks now return to the workflow’s configured hold column when heartbeat worktree acquisition fails, including workflows that use a renamed hold column. * Retry and retry-limit handling now preserves task progress and, when applicable, status. * Added a safe fallback to the default “todo” column when workflow details cannot be resolved. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
8492278fdd |
U11 PR1: pin the merged intake+hold column contract before the IR moves (a mutation proved the first 7 tests insufficient) (#2495)
First of several small PRs for **U11** (merge Todo into Planning). **Tests only — no production change.** It lands the precondition so the IR edit arrives on proven substrate instead of an assumption. ## Decision taken (reversible, proceeding on it) **The surviving Planning column keeps the id `todo`; `triage` is deleted.** Same board the operator asked for — one column labelled "Planning", no "Todo" — via the cheaper and safer half. Measured, comments excluded, non-test, `packages/*/src` + `dashboard/app`: | | guards | writes | fallbacks | total | |---|---:|---:|---:|---:| | `"todo"` | 121 | 68 | 9 | 323 | | `"triage"` | 90 | 30 | 21 | 304 | Deleting `triage` instead of `todo` also means **no data migration** (every live card in `todo` is already in the surviving column) and **no guard changes meaning** (`column === "todo"` still denotes the hold column). Under the plan's letter the opposite is true, and worse than "dead": because Coding (Ideas) keeps `todo` per R10/R11, a surviving `column === "todo"` guard would stay live for Ideas cards while silently never matching for Coding cards — workflow-dependent, not dead. This is also a proven in-tree pattern rather than a new idea: **`builtin:coding-ideas` already ships this exact merge** — id `todo`, display name "Planning", `hold(capacity)` + `reset-on-entry`, plan-in-place. Consequence worth flagging: **U11 no longer waits on Phase B.** The 121 `todo` guards keep their meaning, so converting them becomes U12 cleanup rather than a U11 blocker. ## What this PR pins Nothing in tree has ever carried `intake` and `hold` on one column. Every built-in splits them. KTD-1 asserts the merged shape works; that assertion was untested. ## The result, reported as found **All 11 assertions passed on the first run against unmodified sources.** The merged column is already supported by trait resolution, the capacity sweep, and the release gate. **I could not make the first seven fail**, so they are a regression floor — not evidence of a fix, and I am not claiming them as one. What makes them worth keeping is that they are *differential*: the same scenario runs against the split-role vocabulary and the merged one and asserts the role-level outcomes are **equal**, so a literal creeping into any path fails the merged half while the split half stays green. ## The finding **The first seven tests were not enough, and proving that is the point of this PR.** A mutation encoding the plausible-but-wrong belief *"an intake column has no releaser"*: ```diff - if (currentFlags.intake !== true && currentFlags.hold !== true) return false; + if (currentFlags.intake === true) return false; + if (currentFlags.hold !== true) return false; ``` left **all seven green**. That belief is not hypothetical — it is stated verbatim in `builtin-plan-review-group.ts`'s own FNXC comment as the reason Plan Review lives in `todo` rather than `triage` today. Under U11 the planning column **is** an intake column, so any code encoding it silently stops holding unplanned cards and they release into implementation with a bootstrap stub for a spec. The gap: nothing reached `isUnplannedForExecution`. The mock store had no `getTasksDir`, so both halves of the gate returned early — the tests were exercising less than they appeared to. The fourth block drives it with a real temp dir and a real bootstrap `PROMPT.md`. **Re-running the same mutation now fails exactly one test — the merged-column one — while its split-shape twin stays green.** That discrimination is what the suite is for. ## Verification 26 tests green across this file plus `hold-release-renamed-columns`, `hold-release-instrumentation`, and `pre-release-plan-review`. Lint clean. No production file touched, so there is nothing to regress. ## Next PRs in this unit 1. Entry-contract test for `start` in a hold-carrying column (the specific interaction the earlier, reverted attempt got wrong). 2. The ~10-line IR change itself — deliberately last, per KTD-7. 3. The intake-lane `triage` conversion: the 21 `?? "triage"` creation defaults are the dangerous ones, since they would silently create cards into a column that no longer exists. `self-healing.ts` (11 triage guards + 4 writes) is the main worker's file — I am not touching it and will hand over the line list rather than race them. 🤖 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 coverage for merged Planning column behavior across split and merged workflow configurations. * Verified intake and hold resolution, rebound targeting, capacity hold/release outcomes, and execution gating. * Confirmed planned cards are released appropriately while cards already in progress are not unnecessarily held. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
fbe7eb5c5a |
U7 PR1: the manual plan-approval gate was bypassable (3 planning-lane surfaces, 8/13 revert-proof) (#2491)
## What this is
The first slice of **U7 — the graph owns planning**. Characterizing the
planning lane's dual ownership turned up a live defect in the exact seam
the unit exists to remove, so this PR fixes that first and reports the
measured map of what U7 still has to move.
## The defect
The manual plan-approval gate parks a card by writing `status:
"awaiting-approval"` and **returning early** from `finalizeApprovedTask`
— before the release move. `specifyTask` then calls `onSpecifyComplete`
**unconditionally** afterwards. Three automated surfaces went on to
advance the parked card, each having re-derived its own weaker "may I
advance this?" check from `paused`/`userPaused` alone.
`isTaskBlockedOnApproval` (`packages/core/src/task-merge.ts`) already
declares itself *"the single shared predicate core and engine code must
consult before rebounding, requeuing, resuming, re-planning, or
otherwise advancing a task"*. **Measured: it had exactly one production
consumer** (`overseer-human-control-policy.ts`). Now four.
Reachable end to end for a **plan-in-place** card — one whose column
already equals the plan-review node's column (Coding (Ideas), or any
`needs-replan` revision resting in the default workflow's `todo`):
```
park at awaiting-approval
→ onSpecifyComplete fires anyway
→ a runnable plan-review continuation is seeded
→ the drain dispatches it
→ Plan Review runs on a plan the operator never approved
→ its evidence satisfies isUnplannedForExecution
→ the capacity sweep releases the card into In progress
```
Blast radius: projects that have manual plan approval switched on.
`planApprovalMode` defaults to auto-approve (FN-7557), so unset projects
have no gate to skip — but the operator who turns it on is precisely the
one who cares.
## Surface enumeration
Per AGENTS.md — fix the invariant, not the repro.
| # | Surface | Fix |
|---|---|---|
| 1 | `issueRelease` — the choke point for the sweep, `promoteHeldTask`,
`releaseHeldTaskByEvent`, and the scheduler's `reserveSlot` guard |
Guarded there rather than inside `isUnplannedForExecution`, because an
approval-held card is not "unplanned". Guarded **again** inside the
`moveTaskIf` predicate so a park landing mid-sweep cannot lose the race
(R6 — only the in-txn check is authoritative). Operator force-promote
(`allowUnplanned`) still waives it: that *is* a human decision about
this card. |
| 2 | **Both** continuation seeders —
`seedPreReleasePlanReviewContinuation` (normal completion) and
`evaluateStrandedHoldContinuation` (FN-8592 self-healing re-seed) |
Guard at the seam, not in the callers: the seeder itself checked
nothing, and its two callers each pre-checked a different subset. |
| 3 | `resolvePlanningContinuationCandidate` (drain classifier) |
**Skip, never orphan.** Cancelling terminalizes the item, so an approval
landing a minute later would have nothing left to resume and would need
a second repair to come back. |
## Measured, not assumed
The two hold shapes `isTaskBlockedOnApproval` accepts were **not equally
broken**. The `paused` + `pausedReason` shape was already refused by the
sweep and the drain — they happen to test `paused` — so it was refused
*for the wrong stated reason*, not advanced. Every genuine advance gap
is on the **status-only** shape, which is exactly what the gate writes.
Both are covered anyway, plus an `ORDINARY_PAUSE` counter-case so the
new check cannot quietly become a catch-all for every operator park.
## Revert proof
With the three production files reverted: **8 of 13 tests fail.** The 5
that still pass are the 3 controls and the 2 pause-shape rows the
pre-existing `paused` checks already covered.
```
·x··xxxxx·xx· → Tests 8 failed | 5 passed (13)
```
## Verification
| Check | Result |
|---|---|
| new suite | 13/13 |
| hold-release (×2) + plan-review (×3) + pre-release-plan-review +
promote-force-unplanned | 43/43 |
| stranded-hold-continuation (×2) + continuation-selection +
planning-finished-wake + planning-service | 27/27 |
| scheduler-trait-dispatch | 9/9 |
| `pnpm --filter @fusion/engine exec tsc --noEmit` | clean |
| `pnpm lint` | clean |
| `pnpm test:gate` | green |
| `pnpm check:changesets` | clean |
## Two findings for the coordinator
**1. `triage.ts` is absent from the Phase B census.** The plan's
per-file table (535 sites) covers `self-healing.ts` (U4), the
executor/scheduler cluster (U5), and the core policy modules (U6).
`triage.ts` appears in none of them, so its lifecycle-column literals
are unowned scope — U7 absorbs them.
Measured with the plan's own methodology (block and line comments
stripped, code lines only): a naive quoted-literal grep of `triage.ts`
reports **50** sites, but **35 of those are the agent *role* string
`"triage"`**, not the column. The genuine lifecycle-column surface is
**15 sites**, of which 12 are planning-lane and 3 are `column !==
"done"` in duplicate search. The 50 figure would over-count by 3.3×.
**2. The graph's planning seam is a rubber stamp, in triplicate.**
`createAuthoritativeWorkflowSeams().planning` returns `{ outcome:
"success", value: "pre-specified" }`;
`WorkflowPlanningService.runPlanningSession` returns the same;
`createNoopLegacySeams().planning` is a bare success. The real
specification is ~1,000 lines of `triage.specifyTask`, entirely outside
the graph. That is the flip U7's remaining slices have to make, and it
is the reason the planning lane has two owners at all.
## Deliberately not in this PR
Triage's unconditional `onSpecifyComplete` call. That is the
**ownership** half — `finalizeApprovedTask` must report whether it
released, and the reaction must key on that outcome — and it belongs
with the seam flip, where finalize's outcome becomes the graph's edge
condition anyway, rather than as a half-measure now. With the three
guards above in place, the downstream damage is already contained; what
remains is a reaction firing for a non-event and an operator-visible log
line (`Specified X → todo`) that is untrue for a parked card.
🤖 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**
* Tasks awaiting manual plan approval are no longer automatically
planned, reviewed, started, or released into active work.
* Approval-held items are consistently skipped across planning
continuations and related workflows.
* Approval-held due work is deferred to prevent starvation while
waiting, and operator force-promotion still bypasses the gate.
* **Tests**
* Added regression coverage to ensure the manual approval hold behavior
remains invariant across multiple continuation scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
319e051c65 |
U8 PR1: pin the execution-lifecycle ownership ledger (measured: 28 executor-owned dispositions vs 3 graph handbacks) (#2490)
First PR of **U8 — the graph owns execution** (plan
`docs/plans/2026-07-26-001-refactor-workflow-owned-lifecycle-plan.md`,
line ~436). The plan states this unit "is expected to land as several
commits; it must not be attempted as one sweep", and
Execution-note-first: **characterization before ownership moves**. This
is that floor. **No behavior change.**
## Why a ledger and not a refactor
U8's goal is "the executor stops deciding *what happens next*" — and
that had no measurable form.
- **Executor line count does not measure it.** A 3,178-line
`runImplementation` can shrink substantially with every lifecycle
decision still exactly where it was.
- **A green suite measures it least of all.** Every disposition counted
below already has passing tests, because each one was *correct behavior*
when it was written. What is wrong is the **owner**, not the behavior.
So the unit needs a number, and the number has to exist *before* the
migration — a ratchet written afterwards cannot prove the migration
happened.
## The measured baseline
Counted from source, comments stripped, method bodies extracted by brace
matching:
| Method | `store.moveTask` | `handoffTaskToReview` | terminal
`status:"failed"` | `graphCompletion` handbacks |
|---|---:|---:|---:|---:|
| `runImplementation` (3,178 lines) | 16 | 3 | 9 | **3** |
| `handleGraphFailure` (~930 lines) | 0 | 0 | 7 | — |
**The implementation phase decides its own lifecycle 28 times and asks
the graph 3 times.**
These are measured, not estimated. My first `handleGraphFailure`
estimate was **wrong** (2 moves / 4 parks); the extractor corrected it
to 0 / 7 — the `moveTask` calls that read as belonging to that method
sit past its closing brace, in the recovery helpers below it. The
correction is in the ledger comment so the next reader does not repeat
the misread.
## The finding this makes concrete
`createAuthoritativeWorkflowSeams.execute` collapses that entire
implementation phase to one boolean:
```ts
if (result.taskDone) return { outcome: "success", value: "implemented" };
```
The graph has no vocabulary for *"the agent stopped because a step is
blocked on a pending review"* or *"the session paused after the work was
already complete"*. So the implementation phase performs those
transitions itself (`executor-exit-while-review-pending`,
`paused-after-completion`) and the graph finds out afterwards.
That is why `handleGraphFailure` carries `alreadyFinalizedToReview` /
`completionFinalized` — **classifiers whose entire job is to recognise a
move the graph did not make.** They are compensation for dual ownership,
and they are U8's acceptance test: they become unreachable, and then
deletable, exactly when the last out-of-band transition is gone. This PR
records that contract in source at the seam (FNXC comment), which is
where the next PR starts.
## Proof the guard fails on the defect
A ratchet that reports success without checking anything is worse than
no ratchet. Both failure modes were injected and observed:
1. **The defect it exists to catch** — injected one `await
this.store.moveTask(task.id, "in-review", {})` into
`runImplementation`'s completion path → ledger fails, `16 -> 17`.
2. **A broken guard** — injected a string literal containing `}` so
naive brace matching ends the body early → the size self-check fails at
**13 lines**, instead of silently reporting a comfortable zero for every
count.
Both injections were reverted; `git diff` against the pre-injection copy
is empty.
## Direction of travel
Executor-owned counts may only go **down**, and a decrement must land
with the disposition visible as a **graph outcome** — not merely
deleted. An increment is a new out-of-graph lifecycle decision and needs
a stated justification in its PR, not a quiet edit to the constant.
This is the precursor to U12's planned
`no-out-of-graph-lifecycle-writes.test.ts`; when the counts reach their
floor the assertion becomes "zero, outside the allowlist", and this file
is where that allowlist grows up.
## Preserved behaviors
Untouched, and re-run green as the regression floor for everything that
follows: FN-8141 honest-blocked exit
(`executor-task-done-blocked.test.ts`), FN-7996/FN-7998 tool-failure
retry + escalation (`executor-tool-failure-retry.test.ts`), FN-7863
dispatch-loop terminalization and FN-7926 completed-blocked parking
(`executor-graph-requeue-gate.test.ts`).
## Verification
- `pnpm --filter @fusion/engine exec vitest run` on the ledger + the
four preserved-behavior suites + `legacy-tombstones` — **6 files, 49
tests, green**
- `pnpm test:gate` — **green** (2/10, 16/299, 1/71)
- `pnpm lint` — clean; `tsc --noEmit` on `@fusion/engine` — clean
No changeset: test-only plus a source comment, no `@runfusion/fusion`
behavior change.
🤖 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 a lifecycle-ownership “source-scanning” test that analyzes the
executor’s task disposition patterns to ensure counts remain consistent
across execution and graph-failure flows.
* Added safeguards to catch unintended changes to lifecycle handling.
* **Documentation**
* Documented the lifecycle-ownership boundary for task disposition
handling, including how completion and failure transitions are
consolidated and how related failure classifiers are affected.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
7871b28766 |
fix(core): bind the in-transaction capacity gate — one shared pool-id convention (NOT user-visible yet — see R2) (#2488)
## The bug `moves.ts` asked `countActiveInCapacitySlotAsync` for occupants of pool `"builtin:coding"`, while the counter buckets selection-less rows under `DEFAULT_WORKFLOW_POOL_ID` (`"__default-workflow__"`). Nothing ever landed in the pool being asked about, so the count came back **0** and a finite limit could never bind. ## Root fix, not a literal swap A shared *constant* would not have prevented this: **`DEFAULT_WORKFLOW_ID` was already imported in `moves.ts` and the code still wrote a literal.** So both sides now call a shared **function**, `resolveCapacityPoolId` — "which pool does a selection-less task belong to" has exactly one answer and no call site is in a position to disagree with it. The one variable serving two masters is split: a capacity **pool key** (a bucketing sentinel that must not collide with a workflow id) and a **workflow id** (telemetry, must stay a real id). The emitted `TaskTransitioned` payload is byte-identical. ## Checked, not assumed: no second copy `scheduler.ts:2514` and `:2536` do carry `?? "builtin:coding"` — but as an **IR resolution key** (`resolveWorkflowIrById`), where a real workflow id is required and the pool sentinel would not resolve at all. Same literal, different concept, correctly used. A blanket replace would have broken it. ## Something did depend on the gate being dead — exactly one thing `move-path-equivalence.pg.test.ts` → *"UNPROVEN: in-transaction column capacity did NOT reject on EITHER path in this fixture"*. It left the cause open — > something further in (`resolveColumnCapacity`'s limit resolution, or what `countActiveInCapacitySlotAsync` counts as an occupant — a task with no session/agent may not count) keeps the check from firing … This suite does not establish which. — and predicted its own obsolescence (*"if a future change makes this reject, that is the capacity gate coming alive"*). **Neither guess was right; it was the pool id.** Updated to assert the divergence with the answer recorded — **not weakened**. Its fixture also had to start each phase from an empty wip column: once the gate binds, the inline phase's leftovers trip the cap on the *holder* move before the contended move under test runs. `schema-applier.test.ts` failed only in the full-suite run and passes in isolation both with and without the fix — cross-file contamination, not mine. ## Before / after — measured, both directions `maxConcurrent: 1`, real PG store, real `moveTask`: | | flagOFF / no selection | flagOFF / selection | flagON / no selection | flagON / selection | |---|---|---|---|---| | **before** | ADMITTED | ADMITTED | **ADMITTED** ← the bug | REJECTED | | **after** | ADMITTED | ADMITTED | **REJECTED** | REJECTED | The E2E acceptance row asserts **held at cap 1 and admitted at cap 2 on the same fixture**, so it cannot pass by simply never admitting anything. **With the fix reverted that row fails**; the `admitted` case still passes, as it should. The Phase A3 ratchet's two flipped assertions also fail with the fix reverted. Ratchet flipped exactly as its author specified: `DEFECT (R1)` becomes a rejection, and `it.fails` on the invariant becomes a plain `it`. ## ⚠️ This is NOT user-visible yet — please read before merging The premise this was approved on ("once it binds, cards that currently slip through will start being held") **does not hold for this change alone.** The whole capacity block sits inside `if (useWorkflow && workflowIr && fromColumn !== toColumn)`, and `useWorkflow` is `experimentalFeatures.workflowColumns === true` — absent from `DEFAULT_GLOBAL_SETTINGS`, with **no writer anywhere outside tests**. That is Phase A3's R2, still live and now retitled `DEFECT (R2, STILL LIVE)` with the measured matrix recorded in it. So on merge: nothing changes for any real project. Making it actually bind means **also** removing the `useWorkflow` condition — a materially larger, genuinely user-visible change that I have not made unilaterally. Escalated for a decision; if that lands, the changeset here should be re-categorised. ## Review follow-up (48e79ffd9): the convention was still duplicated — swept and ratcheted The first pass added the resolver and routed the transactional gate + counters, but **hold-release still derived the pool independently**. Swept the repo: six sites name the sentinel, **five derive the convention** and now call `resolveCapacityPoolId` (`hold-release.ts:116/118/442/576`, `task-store-helpers.ts:290`). The sixth, `scheduler.ts:1558`, names the default pool as a literal in a capacity *diagnostic* — no selection input, nothing to disagree with — so it keeps the constant. **Does this change hold-release behavior? No, and it was never releasing against the wrong pool.** hold-release computed `x ?? DEFAULT_WORKFLOW_POOL_ID`, which is exactly what the counter buckets under; `moves.ts` (`?? "builtin:coding"`) was the sole disagreeing site, and the first commit moved *it* into agreement with hold-release, not the reverse. `resolveCapacityPoolId(x)` **is** `x ?? DEFAULT_WORKFLOW_POOL_ID`, so every routed site computes an identical value for every input. **No second user-visible change rides along with this PR** — the only behavior delta remains the gate binding on the flag-ON path, which per R2 is still not the path production takes. Evidence: hold-release + capacity suites **43/43 identical before and after**. **The resolver is now the only way to compute a pool id, not merely the newest way.** `scripts/check-capacity-pool-id.mjs` fails on any inline `?? DEFAULT_WORKFLOW_POOL_ID` outside `workflow-capacity.ts`, wired into **both `pretest` and the blocking `test:gate`**. A review note would not have sufficed: the original defect landed in a file that *already imported* the canonical constant. Verified both ways — clean run scans 1124 files and passes; reintroducing the old hold-release expression exits 1 and names the line. ## Review follow-up (a5b675503): the ratchet was rebuilt because it would not have caught the bug The first ratchet matched one spelling (`?? DEFAULT_WORKFLOW_POOL_ID`) and the real defect used another (`?? "builtin:coding"`). **Verified: reintroducing the original defect and running the old checker exits 0.** A guard that reports success without checking is worse than no guard — it stops anyone looking. Rebuilt on the TypeScript AST with two rules. **Rule 1 (sink):** a value reaching a capacity counter's `workflowId` must come from `resolveCapacityPoolId`, or a local initialized from it — so it fires on the original defect regardless of which literal was used, on one line or twenty. **Rule 2 (sentinel):** no `??` onto the sentinel at any qualification depth or as its raw value; multiline is one AST node and caught by construction. `?? "builtin:coding"` is deliberately *not* banned outright — it is the legitimate default for a *workflow* id in ~8 places, and is only a bug when it reaches a capacity pool. **Fails closed three ways** that previously reported success without inspecting: unreadable file, unparseable file, and an empty file listing (the old script would have printed a green tick off a broken glob). **Acceptance was not "passes on main".** Each form was reintroduced into the real source and confirmed to fail: the original defect in `moves.ts`, a multiline fallback, and a deeply qualified sentinel. All are pinned in `capacity-pool-id-check.test.ts` (12 cases: 7 must-catch starting with the reduced actual pre-fix `moves.ts`, 4 must-not-flag, 1 fail-closed) so the guard cannot silently narrow again. Also added to `pretest:full`, which had omitted it. ### Follow-up (0be8df6ea): a dead rule found by fixing a test title Splitting the mislabelled fail-closed test surfaced more than a mislabel: **`ts.createSourceFile` is error-tolerant and does not throw on malformed syntax**, so the `try/catch` behind the `unparseable` rule was unreachable and that rule could never fire. The earlier "fails closed three ways" claim was overstated — the guard advertised a capability it did not have. Detection now reads `sf.parseDiagnostics`; a partial AST can silently lack the `??` nodes and sink calls the rules look for, so "did not parse" must not read as "inspected and clean". Mutation-verified: reverting the detection fails that case and only that case. Test-file exclusion also moved to the repo's `{test,spec}.{ts,tsx}` guideline shape — a `.spec.ts` under `packages/<pkg>/src/` was being scanned as production source. Verified both ways: the `.spec.ts` is skipped, and the identical content in a non-test file is still caught, so the exclusion is scoped rather than a hole. ## Verification - engine + core `tsc --noEmit` clean - `pnpm test:gate` green (299 + 10 + 71) - E2E 20/20; capacity + move-path suites 14/14 - full core PG: **1037 passed / 3 failed** — all three reproduce with the fix stashed (pre-existing) - engine-default: **279 failed** vs **280 at baseline** with the fix stashed — pre-existing red lane, no regression - hold-release + capacity suites: **43/43 identical before and after** the resolver routing - `check-capacity-pool-id` ratchet: 14/14 regression cases; clean over 1124 files; exits 1 on the original defect, a multiline fallback, and a deeply qualified sentinel reintroduced into real source 🤖 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** * Fixed capacity-limit accounting when workflow selection is missing by consistently deriving the correct capacity pool id. * Made capacity enforcement align across move and hold/release paths, rejecting over-limit moves with `capacity-exhausted`. * **Tests** * Updated PostgreSQL and added an E2E scenario to verify the corrected in-transaction gating behavior at `maxConcurrent` limits of 1 and 2. * **Chores** * Added an automated guard to detect inconsistent capacity pool id fallback patterns in code. * **Public API** * Exposed `resolveCapacityPoolId` for consistent capacity pool id derivation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
387e836432 |
U4 substrate PR1: extract the git-evidence readers (19/19 bodies byte-identical, self-healing.ts -449) (#2489)
Based on `main`. **PR1 of the substrate decomposition** — deliberately small and boring. ## Mechanical proof (the point of this PR) Every moved body diffed against its pre-move text: ``` BLOCK KIND DIFF-vs-ORIGINAL LandedTaskCommit iface EMPTY commitOwnedByTask helper EMPTY escapeRegex helper EMPTY shellQuote helper EMPTY parseShortstat helper EMPTY findLandedTaskCommit method EMPTY findAlreadyMergedTaskCommit method EMPTY refreshRemoteBaseRef method EMPTY readCommitTaskOwnership method EMPTY branchHasNoUniqueDiff method EMPTY baseHasExplicitTaskOwnership method EMPTY foreignTipRejection method EMPTY branchTipForeignOwnership method EMPTY isCommitReachableFromBranch method EMPTY findWorktreePathForBranch method EMPTY repoBranchExists method EMPTY readShortstatForSha method EMPTY readLandedFilesForSha method EMPTY isBranchTipMisboundToTask method EMPTY RESULT: 19/19 BYTE-IDENTICAL modulo the enumerated deviations; 0 differ. ``` No condition reordered, no signature changed, no rename, no inlining, no "while I am here" cleanup. ## The premise needed correcting — the cheap lesson this cut was meant to buy The brief described **13 pure functions**. They are 13 `private async` **methods** closing over `this.options`, so nothing here is byte-identical in the strict sense. Three deviations were structurally unavoidable: 1. **`private` → `protected`** on the 14 methods and the `options` field — a subclass cannot call a `private` base member. One token per declaration. 2. **Two type annotations** rewritten: `SelfHealingManager["readCommitTaskOwnership"]` → the base class, since the original would be a circular import. 3. **Four module-level helpers moved along** and re-exported. Forced by direction — the new module must not import `self-healing.ts`, so everything the bodies call has to live beside them. `execAsync` and `shellQuote` are imported back because call sites remain. ## The set is 14, not 13 `foreignTipRejection` had to come too, and it's what **closes** the cluster — it depends only on `baseHasExplicitTaskOwnership` and `branchHasNoUniqueDiff`, both already in the set. Without it the extraction isn't self-contained and `this.options` isn't the only external dependency. ## Base class, not free functions — deliberately Converting to free functions would change all 14 signatures: a behavior-adjacent edit riding inside a file move, which is exactly the combination that hid the last four safeguard regressions. An abstract base keeps `this` semantics, so every call site stays `this.<method>(...)` and every body is unchanged text. ## Two pre-existing collisions, preserved exactly - The detector module already exports a **free** `findAlreadyMergedTaskCommit` sharing a name with the protected method; inside the class body the bare identifier resolves to the import. - `SelfHealingOptions` is declared in `self-healing.ts`, imported here **type-only** so it's erased at runtime and creates no module cycle. ## Measured line delta — not an estimate | | lines | |---|---:| | `self-healing.ts` | 13,394 → 12,945 = **−449** | | new module | **+527** (454 moved verbatim, 73 scaffold) | | **NET** | **+78** | Same shape as every consolidation in this program: the target file shrinks, the total grows slightly. At −449 for the first and safest cut, the remaining substrate cuts plausibly take `self-healing.ts` under 11k — but that is **file-size reduction, not code reduction**. ## Verification 464 passed across three engine suites with the single known **pre-existing** `archiveStaleDoneTasks` failure; tsc clean; lint clean; merge gate green (299 + 10 + 71). No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
0021bd363e |
U4: retire the surfacing family onto one policy-driven runner (measured: trim is ~34% of the estimate) (#2487)
Stacked on #2486. Base is `feature/u4-safeguard-scoped-to-mutation` — do not merge before it. Retires the surfacing family — `surfaceStalePausedTodos`, `surfaceStalePausedReviews`, `surfaceInReviewStalled` — onto one policy-driven runner. Migrated **together**, because they were three copies of one skeleton and a fix applied to one had to be remembered for the other two. #2484's characterization suite is the regression floor and **passes unchanged**. ## Two unconverted sites found in core Without these the migration would have been **cosmetic for two of the three**: `getStalePausedReviewSignal` and `getInReviewStalledSignal` both hardcoded `task.column !== "in-review"`. `getStalePausedTodoSignal` gained the equivalent `holdColumn` parameter back in B1 — its two siblings were missed, so they silently stopped matching for any workflow that renames its review column. Both now take `reviewColumn`, defaulting to the legacy id. ## Three bugs this work introduced, and the tests caught Each was silent in the diff: 1. **I dropped the engine activation floor** from all three signal calls. Wall-clock the engine wasn't running for is not quiet time, so every sweep would have reported cards as stale purely because the engine restarted. Caught by the **pre-existing** suite — not by my own characterization floor, which is exactly why that floor wasn't sufficient alone. 2. **I passed `task.column` as the role column**, making the signal's own column check compare a value against itself — tautologically true, so the check was silently deleted. The *resolved* role column is now passed. 3. **The role gate was conditional on the role resolving**, so it was silently absent for exactly the workflows whose role failed to resolve. Now unconditional, with the legacy id as fallback. ## The shared test is a table One row per sweep; every invariant asserted for all three — threshold inheritance, declared-policy override, renamed role column, the negative case outside the role column, at-most-once, activation floor, both pause gates, non-positive-threshold disable, soft-delete. **Adding a fourth surfacing sweep means adding a row.** Its log mock **appends** to the card's log, because the at-most-once dedup reads that log — a call-recording mock cannot observe suppression at all. ## Measured line delta — this corrects the survey estimate downward | | lines | |---|---:| | `self-healing.ts` | **−192 / +95 = −97** | | new runner file | **+209** | | core signal conversions | **+23** | | **NET** | **+135** | Three sweeps migrated **increased** total lines by 135 while shrinking `self-healing.ts` by 97. Per sweep: **−32** in `self-healing.ts`. **Break-even is 6.5 sweeps.** Extrapolated to all 34 POLICY sweeps: **−1,099** in `self-healing.ts`, **−890 net** once the runner is amortized. My survey estimated **−2,612** for that bucket. The measured figure is **34% of it**. The reconciler's size was never the issue — the estimate assumed per-sweep bodies collapse to almost nothing, and they do not: each keeps a real eligibility predicate, signal call, and operator message. ## Verification - 33 shared-family tests green - 471 passed across five engine suites, with the single known **pre-existing** `archiveStaleDoneTasks` failure - 44 core signal tests green - tsc clean in core and engine; lint clean; merge gate green (299 + 10 + 71) No changeset: `@fusion/core` and `@fusion/engine` are private. 🤖 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** - Added support for overriding which workflow column is treated as the relevant “in review” column for stale-paused and stalled-review surfacing. - **Bug Fixes** - Tightened safety safeguards so only lifecycle-mutating recovery actions are blocked when a card is user-paused; observational surfacing remains allowed. - Improved surfacing consistency across stale paused todos, stale paused reviews, and in-review stalled cases, including stronger deduping and cycle-aware behavior. - **Tests** - Expanded and reworked safety and surfacing “family” coverage to verify the new invariants. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
dependabot[bot]
|
af3d78d2cd |
chore(deps-dev): bump vitest from 4.1.8 to 4.1.10 (#2446)
Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 4.1.8 to 4.1.10. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/vitest-dev/vitest/releases">vitest's releases</a>.</em></p> <blockquote> <h2>v4.1.10</h2> <h3> 🐞 Bug Fixes</h3> <ul> <li><strong>browser</strong>: Check fs access in builtin commands [backport to v4] - by <a href="https://github.com/hi-ogawa"><code>@hi-ogawa</code></a>, <strong>Hiroshi Ogawa</strong> and <strong>OpenCode (claude-opus-4-8)</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10680">vitest-dev/vitest#10680</a> <a href="https://github.com/vitest-dev/vitest/commit/5c18dd267"><!-- raw HTML omitted -->(5c18d)<!-- raw HTML omitted --></a></li> <li><strong>vm</strong>: Fix external module resolve error with deps optimizer query for encoded URI [backport to v4] - by <a href="https://github.com/SveLil"><code>@SveLil</code></a> and <a href="https://github.com/hi-ogawa"><code>@hi-ogawa</code></a> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10661">vitest-dev/vitest#10661</a> <a href="https://github.com/vitest-dev/vitest/commit/bae52b511"><!-- raw HTML omitted -->(bae52)<!-- raw HTML omitted --></a></li> </ul> <h5> <a href="https://github.com/vitest-dev/vitest/compare/v4.1.9...v4.1.10">View changes on GitHub</a></h5> <h2>v4.1.9</h2> <h3>🐞 Bug Fixes</h3> <ul> <li>Fix <code>importOriginal</code> with optimizer and query import [backport to v4] - by <strong>Hiroshi Ogawa</strong>, <strong>David Harris</strong>, <strong>Codex</strong>and <strong>Vladimir</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10546">vitest-dev/vitest#10546</a> <a href="https://github.com/vitest-dev/vitest/commit/a5180190c"><!-- raw HTML omitted -->(a5180)<!-- raw HTML omitted --></a></li> <li><strong>browser</strong>: <ul> <li>Wait for orchestrator readiness before resolving browser sessions [backport to v4] - by <strong>Vladimir</strong> and <strong>Séamus O'Connor</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10555">vitest-dev/vitest#10555</a> <a href="https://github.com/vitest-dev/vitest/commit/7fb29651a"><!-- raw HTML omitted -->(7fb29)<!-- raw HTML omitted --></a></li> <li>Wait for iframe tester readiness before preparing [backport to v4] - by <strong>Vladimir</strong> and <strong>Séamus O'Connor</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10497">vitest-dev/vitest#10497</a> and <a href="https://redirect.github.com/vitest-dev/vitest/issues/10556">vitest-dev/vitest#10556</a> <a href="https://github.com/vitest-dev/vitest/commit/fbc626c40"><!-- raw HTML omitted -->(fbc62)<!-- raw HTML omitted --></a></li> </ul> </li> <li><strong>mocker</strong>: <ul> <li>Hoist vi.mock() for vite-plus/test imports [backport to v4] - by <strong>Hiroshi Ogawa</strong>, <strong>LongYinan</strong>, <strong>Claude Opus 4.8</strong> and <strong>Vladimir</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10548">vitest-dev/vitest#10548</a> <a href="https://github.com/vitest-dev/vitest/commit/2c9559c02"><!-- raw HTML omitted -->(2c955)<!-- raw HTML omitted --></a></li> </ul> </li> <li><strong>pool</strong>: <ul> <li>Prevent test run hang on worker crash [backport to v4] - by <strong>Ari Perkkiö</strong> and <strong>Jattioui Ismail</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10543">vitest-dev/vitest#10543</a> and <a href="https://redirect.github.com/vitest-dev/vitest/issues/10564">vitest-dev/vitest#10564</a> <a href="https://github.com/vitest-dev/vitest/commit/934b0f587"><!-- raw HTML omitted -->(934b0)<!-- raw HTML omitted --></a></li> </ul> </li> </ul> <h5><a href="https://github.com/vitest-dev/vitest/compare/v4.1.8...v4.1.9">View changes on GitHub</a></h5> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
dependabot[bot]
|
ff58df22e4 |
chore(deps-dev): bump @vitest/coverage-v8 from 4.1.8 to 4.1.10 (#2445)
Bumps [@vitest/coverage-v8](https://github.com/vitest-dev/vitest/tree/HEAD/packages/coverage-v8) from 4.1.8 to 4.1.10. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/vitest-dev/vitest/releases">@vitest/coverage-v8's releases</a>.</em></p> <blockquote> <h2>v4.1.10</h2> <h3> 🐞 Bug Fixes</h3> <ul> <li><strong>browser</strong>: Check fs access in builtin commands [backport to v4] - by <a href="https://github.com/hi-ogawa"><code>@hi-ogawa</code></a>, <strong>Hiroshi Ogawa</strong> and <strong>OpenCode (claude-opus-4-8)</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10680">vitest-dev/vitest#10680</a> <a href="https://github.com/vitest-dev/vitest/commit/5c18dd267"><!-- raw HTML omitted -->(5c18d)<!-- raw HTML omitted --></a></li> <li><strong>vm</strong>: Fix external module resolve error with deps optimizer query for encoded URI [backport to v4] - by <a href="https://github.com/SveLil"><code>@SveLil</code></a> and <a href="https://github.com/hi-ogawa"><code>@hi-ogawa</code></a> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10661">vitest-dev/vitest#10661</a> <a href="https://github.com/vitest-dev/vitest/commit/bae52b511"><!-- raw HTML omitted -->(bae52)<!-- raw HTML omitted --></a></li> </ul> <h5> <a href="https://github.com/vitest-dev/vitest/compare/v4.1.9...v4.1.10">View changes on GitHub</a></h5> <h2>v4.1.9</h2> <h3>🐞 Bug Fixes</h3> <ul> <li>Fix <code>importOriginal</code> with optimizer and query import [backport to v4] - by <strong>Hiroshi Ogawa</strong>, <strong>David Harris</strong>, <strong>Codex</strong>and <strong>Vladimir</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10546">vitest-dev/vitest#10546</a> <a href="https://github.com/vitest-dev/vitest/commit/a5180190c"><!-- raw HTML omitted -->(a5180)<!-- raw HTML omitted --></a></li> <li><strong>browser</strong>: <ul> <li>Wait for orchestrator readiness before resolving browser sessions [backport to v4] - by <strong>Vladimir</strong> and <strong>Séamus O'Connor</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10555">vitest-dev/vitest#10555</a> <a href="https://github.com/vitest-dev/vitest/commit/7fb29651a"><!-- raw HTML omitted -->(7fb29)<!-- raw HTML omitted --></a></li> <li>Wait for iframe tester readiness before preparing [backport to v4] - by <strong>Vladimir</strong> and <strong>Séamus O'Connor</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10497">vitest-dev/vitest#10497</a> and <a href="https://redirect.github.com/vitest-dev/vitest/issues/10556">vitest-dev/vitest#10556</a> <a href="https://github.com/vitest-dev/vitest/commit/fbc626c40"><!-- raw HTML omitted -->(fbc62)<!-- raw HTML omitted --></a></li> </ul> </li> <li><strong>mocker</strong>: <ul> <li>Hoist vi.mock() for vite-plus/test imports [backport to v4] - by <strong>Hiroshi Ogawa</strong>, <strong>LongYinan</strong>, <strong>Claude Opus 4.8</strong> and <strong>Vladimir</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10548">vitest-dev/vitest#10548</a> <a href="https://github.com/vitest-dev/vitest/commit/2c9559c02"><!-- raw HTML omitted -->(2c955)<!-- raw HTML omitted --></a></li> </ul> </li> <li><strong>pool</strong>: <ul> <li>Prevent test run hang on worker crash [backport to v4] - by <strong>Ari Perkkiö</strong> and <strong>Jattioui Ismail</strong> in <a href="https://redirect.github.com/vitest-dev/vitest/issues/10543">vitest-dev/vitest#10543</a> and <a href="https://redirect.github.com/vitest-dev/vitest/issues/10564">vitest-dev/vitest#10564</a> <a href="https://github.com/vitest-dev/vitest/commit/934b0f587"><!-- raw HTML omitted -->(934b0)<!-- raw HTML omitted --></a></li> </ul> </li> </ul> <h5><a href="https://github.com/vitest-dev/vitest/compare/v4.1.8...v4.1.9">View changes on GitHub</a></h5> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
Phil Larson
|
3822e1081c |
test(engine): remove orphan dependency reporter coverage (#2483)
## Summary - remove the remaining dependency-blocked reporter test after the reporter was deleted in #2477 - prevent Vitest from failing during module collection on a deleted import ## Test plan - `pnpm --filter @fusion/engine typecheck` - `pnpm --filter @fusion/engine build` |
||
|
gsxdsm
|
17fdf2f0ae |
U4: scope the user-pause safeguard to lifecycle MUTATION, not observation (re-ratified) (#2486)
Stacked on #2482. Base is `feature/workflow-vocabulary-u4-override-layer` — do not merge before it. Implements the coordinator's **re-ratification** of the user-pause safeguard with a narrower definition. ## The invariant, written into the code > The user-pause safeguard means **NEVER MUTATE LIFECYCLE STATE** of a user-paused card. It does **NOT** mean never observe one. Respecting a pause exists to stop the engine acting on a card *behind* the operator who paused it — moving, rebounding, archiving, resuming. A read-only diagnostic does the opposite: it tells that same operator what their paused card is doing. Blinding them to their own paused work is not safety; it is the engine deciding they should not be told. The sentence is in the source, because the distinction is the whole point and a future reader will otherwise re-broaden it. ## Why it needed narrowing Ratified broadly first, that reading was caught suppressing the very sweeps it was meant to protect. `surfaceStalePausedTodos` exists to report cards that have sat paused too long — routing it through a reconciler that suppresses paused cards turns a diagnostic into one that **silently reports nothing**. Measured, not argued: #2484 proves that sweep surfaces user-paused cards on current main. ## Scoped by action, never by sweep `OBSERVATIONAL_ACTIONS` is an **allow-list**, so a newly added mutating action is suppressed by default — the scoping fails closed. A sweep cannot opt itself out. `RecoveryActionKind` deliberately names actions the policy vocabulary cannot yet author (`rebound`, `archive`, `requeue`, `resume`). The scoping is only testable if those exist as values, and **a rule that cannot be tested is a rule that erodes**. `parseWorkflowIr` keeps a closed action list, so nothing becomes authorable by being named. ## Which field — chosen, not inherited The gate reads `userPaused`, **not** `paused`. The two diverge (`branch-group-ops.ts:128` says so outright), and `paused` also covers engine-authored automation pauses like dispatch-storm, which carry no operator intent to respect — gating on it would suppress recovery from the engine's own throttles. The safeguard defers to a **human** decision, so it keys on the field that records one. ## Both halves kept The broad case is **narrowed, not deleted**: one test proves mutation is still suppressed, one proves observation is now permitted. A future reader must be able to tell the scoping was *deliberate* rather than eroded by someone who found the broad rule inconvenient. **Mutation-verified in both directions**, since either error is silent: | mutation | result | |---|---| | re-broaden (suppress observation) | **2 tests fail** | | over-narrow (`rebound` treated observational) | **3 tests fail** | ## Verification 46 tests green (28 safety + 18 inheritance); tsc clean; lint clean; merge gate green (299+10+71). No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
89284df85e |
E2E: table-driven converted-sweep coverage, two new sites, and an honest unproven-sites ledger (#2485)
Follow-up to #2475 (merged). Test-only, plus one test-utility seam. ## Why a table #2475 proved one converted sweep. The count has since gone to **three**, twice while this work was open — `surfaceStalePausedTodos` appeared during #2475's review, and #2478 landed `recovery-reconciler.ts` while this branch was open. A suite with a bespoke `describe` per sweep is a coverage claim that quietly becomes false. Replaced with a table of `(seed, run, acted, roles, observability)`. The driver derives four assertions per entry: | | positive | negative | |---|---|---| | **renamed vocabulary** | acts on the card | inert in a non-target column | | **default vocabulary** | acts (regression floor) | inert | Adding a converted sweep is **one entry** — the #2478 site proved that in practice, not in principle. `actsOnRole`/`inertRole` are keys of `Vocabulary`, not column strings, so an entry cannot hardcode `todo` and pass for the wrong reason. ## Two findings, both from mutation rather than reading **1. The census was wrong about `recovery-reconciler.ts:198.`** It was flagged as a `resolveLifecycleColumns` site, so the row was first labelled as covering it. **Destroying that role resolution leaves all 18 tests green** — `decideRecovery` looks policy up by *column id* and never consults a role. The row is relabelled to what it actually proves, and mutation-verified against that instead: keying the reconciler's policy lookup on the `todo` literal fails exactly its renamed test. **2. `resolveRoleRecovery` is an unreachable export.** It is the only use of `resolveLifecycleColumns` in that file and has **no production caller anywhere** in engine, core, or dashboard. So that census line is not a live converted site — it is a helper written ahead of its consumer. **Not fixed here:** it is production code owned by the U4 slice, and whether the consumer is still to land or it should be deleted is its author's call. ## Observability is now explicit in the type `persisted-row` is the strong form. `returned-decision` is recorded as **weaker evidence** and the reconciler row uses it, because `reconcileRecovery` decides and does not apply — there is no row to read. Naming it in the type is what stops a return-value assertion from quietly passing as observed state, and it is what keeps the ledger truthful per site. ## Harness seam `PgTestHarness` now exposes its raw admin SQL client. The store **stamps** `updatedAt`/`columnMovedAt` on every write, so `updateTask` cannot express an aged row at all — the patch is accepted and the value silently replaced with `now`. **Found by the new case failing on BOTH vocabularies**, which is what distinguishes a broken fixture from a broken guard. Seeding only; assertions still read back through the real `getTask` path. ## Mutation verification | Mutation | Result | |---|---| | revert **only** `recoverStrandedCompletedTodoTasks`'s resolution | **exactly** that row's renamed test fails | | revert **only** `surfaceStalePausedTodos`'s resolution | **exactly** its own renamed test fails | | reconciler policy lookup keyed on `todo` | exactly the reconciler row's renamed test fails | | `resolveRoleRecovery` role resolution destroyed | **nothing fails** → finding #2 | | `hold-release` `isHeldTask` keyed on `todo` | 5 of 18 fail; default spine survives | | `markMoveInFlight` dropped | both spine tests fail | Per-site verification matters here: three rows could all be riding one guard. They are not. ## The honest number **Proven end to end: 5** (two self-healing sweeps, the reconciler's policy lookup at the weaker observability, hold-release's capacity release, and the graph boundary + `moveTask` + post-commit bus). **Not proven: 11 call sites** — `merger.ts:324-326`, `merger-ai.ts:1022,1039`, `auto-merge-finalization.ts:20-22`, `executor.ts:1763,6339,6341`, `self-healing.ts:713,6732`, `mesh-lease-manager.ts:61`, `task-agent-sync.ts:59`, `core/task-store/reads.ts:130`, `core/live-agent-count.ts:63-75`, and four dashboard route sites. The ledger lives in the file, not just here, so it stays with the code. ## Where the table does not fit — reported, not papered over The **merge/rebound family** cannot be a table row: those sweeps have no observable persisted effect without a real git repository, so `acted` cannot be written against the row at all. They need an engine-slow real-git lane. The dashboard sites need an HTTP route test with a live store. Both are different lanes, not missing entries. ## Verification - 18/18 green; engine + core `tsc --noEmit` clean; `pnpm test:gate` green (299 + 10 + 71) - full core PG suite run (the harness is shared): 1036 passed, 3 failed in `central-archive-secrets` and `workflow-settings-project-identity` — **reproduce identically with this change stashed**, pre-existing 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
2dfac47917 |
U4: recovery policy as an OVERRIDE LAYER (unset defers to the operator setting) (#2482)
Stacked on #2478. Base is `feature/workflow-vocabulary-u4-reconciler-slice` — do not merge before it. Implements the ratified precedence: **declared explicitly → policy wins; left unset → defer to the project/global setting, exactly as today.** ## The design `resolveEffectiveRecovery(declared, inherited)` composes the two **per field**, so a workflow may declare a threshold while inheriting the action. It mirrors the two-tier merge `effective-settings.ts` already implements for workflow settings (a stored value overrides the base; a declaration default only fills an absent key) rather than inventing a fourth precedence system beside model selection, project settings, and workflow settings. **Absence stays absent.** `??` treats an explicitly-`undefined` field as unset, so a policy is never normalized into a built-in default. The distinction a naive implementation gets wrong: > **equal-to-default is not the same as unset** A declaration whose value happens to equal the legacy literal is a *deliberate choice* and must still override a customized operator setting. Only true absence defers. An effective policy requires **both** halves — a threshold with no action never fires, an action with no threshold has nothing to fire on — so a half-resolved policy yields `undefined` rather than something present but inert. ## The test that matters was written first, and failed > a project with a CUSTOMIZED threshold and the policy key UNSET must observe the customized value This is where a green suite lies. "Read the policy, else use the built-in default" passes every obvious test while silently resetting an operator who tuned `stalePausedTodoThresholdMs` — no error, nothing in any diff, the sweep just starts firing on a schedule nobody chose. **Mutation-verified in both directions:** | mutation | result | |---|---| | substitute a built-in default for the inherited setting | **5 tests fail** | | invert precedence (inherited beats declared) | **3 tests fail** | ## Upgrade guarantee Asserted as a property over several operator values: an undeclared workflow observes *exactly* the operator's value. That is what makes landing the policy table a zero-behavior-change upgrade that touches no project. ## What is NOT here **`surfaceStalePausedTodos` is not retired.** Migrating it surfaced a safeguard-semantics collision I escalated rather than resolved unilaterally: the sweep exists to surface cards that have been **paused** too long, but the reconciler's ratified user-pause safeguard suppresses `surface` on user-paused cards — so migrating it as-is would suppress a large part of what the sweep is for. `paused` and `userPaused` are distinct fields that can diverge (see `branch-group-ops.ts:128`). The sweep is untouched pending that decision. ## Verification - 34 tests green (10 new inheritance + 24 safety) - `tsc --noEmit` clean, `pnpm lint` clean No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Recovery decisions now correctly inherit operator settings when a workflow does not specify a recovery policy. - Workflow-specific recovery settings override inherited values, including when matching built-in defaults. - Recovery settings can now be applied independently by field, allowing thresholds and stale-item actions to inherit separately. - Recovery is suppressed safely when no complete policy is available, preventing unintended recovery actions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
3578903b16 |
U4: characterize surfaceStalePausedTodos before the policy migration (regression floor + safeguard evidence) (#2484)
Based on `main`, independent of #2482 — mergeable on its own. Pins what `surfaceStalePausedTodos` does **today**, so the first real policy migration is judged against *observed* behavior rather than against what the sweep looks like it should do. **No production code changed.** Passes against current main; must still pass after the migration. ## 1. Threshold source — the case you asked to see proven first The sweep reads `settings.stalePausedTodoThresholdMs` directly, so under the override layer an undeclared workflow must keep observing exactly that value. A migration that reaches for a declaration default instead finds the card fresh and **silently stops surfacing it** — resetting a deliberate operator choice with no error and nothing in any diff. Both directions are asserted, deliberately: - a customized **1h** threshold surfaces a 2h-old card; - the **same card is NOT surfaced** under the 24h built-in. Without that discriminator, "always surface" would pass the first test and prove nothing about which threshold was used. ## 2. User-paused cards — evidence for the open safeguard question This replaces my argument with a measurement. `getStalePausedTodoSignal` gates on `paused === true` and **never consults `userPaused`**, so a user-paused card that has sat too long **is surfaced today**. That test passes on current main. This is what blocks the migration: the reconciler's ratified user-pause safeguard suppresses the `surface` action for `userPaused` cards, so routing this sweep through it unchanged would **stop surfacing them** — a silent behavior change dropping a diagnostic operators rely on. `paused` and `userPaused` are distinct fields that can diverge (see the comment at `branch-group-ops.ts:128` about `userPaused` remaining true while legacy `paused` is false), so these are genuinely separable states rather than one condition spelled two ways. **The decision needed:** does the user-pause safeguard mean *"never act on a user-paused card"* or *"never mutate lifecycle state of one"*? It generalizes — `surfaceStalePausedReviews` and `surfaceInReviewStalls` are the same shape. ## 3. Things easiest to lose in a rewrite Also pinned: inert under `globalPause` and `enginePaused`; a non-positive threshold disables it entirely; an unpaused card is never surfaced; automation-paused and user-paused cards are not distinguished today. ## Verification 8 tests green against unmodified main; lint clean. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
b133d521c4 |
U4 vertical slice: recovery-policy reconciler + ratified safety invariant (measured: engine ~780, real cost is a settings migration) (#2478)
Stacked on #2477. Base is `feature/workflow-vocabulary-u4-delete-dep-blocked` — do not merge before it. The smallest end-to-end slice of the U4 reshape, built to **measure** the real cost before committing to the full policy table. The survey's ~900-line reconciler figure was reasoned, not prototyped; this replaces it with numbers. **Everything here is additive and unwired. No behavior changes.** ## What lands | | lines | what | |---|---:|---| | `WorkflowColumnRecovery` (IR) | 42 | one key — `stalenessMs` + `onStale`. Optional and omitted when unset, so existing workflows serialize byte-identically. | | `recovery-reconciler.ts` | 176 | one engine: walks live cards, resolves each card's policy from **its own** workflow (per task, shared `irCache` — a 400-card board across three workflows reads three IRs), returns decisions. Decision and application are separate so the safety boundary is assertable without running an engine. | | `recovery-policy-safety.test.ts` | 156 | one-time. The **ratified invariant**. | ## Measured cost vs the ~900 estimate **Engine + IR types = 222 lines** for one action (`surface`) and one safeguard. Extrapolating the rest — `rebound` (target resolution, attempt budgets, backward-move proof, five more safeguards) ≈ +350, `archive` ≈ +50, the `budgets`/`dependencies` keys ≈ +150 — lands near **780**. So **~900 was a good estimate for the engine**, and the vertical slice does not move it much. That is the answer to the question asked. ## But the estimate's real miss is not lines **16 of the 34 POLICY sweeps read an operator setting today** — ~17 distinct policy-threshold keys, including `stalePausedTodoThresholdMs`, `inReviewStalledThresholdMs`, `taskStuckTimeoutMs`, `doneAutoArchiveDays`, `maxPostReviewFixes`. Moving those sweeps into workflow policy is **not a code refactor — it is a settings migration with operator-visible blast radius**, and it needs three decisions the line estimate never surfaced: 1. Does workflow policy **override** the global setting, or defer to it? 2. What happens to **existing projects** that already configured those settings? 3. Does an **unset** policy inherit the setting, or the built-in default? That is the gating question for the full table — not the reconciler's size. ## Why the sweep is not retired here Retiring `surfaceStalePausedTodos` requires builtin:coding to declare the policy **and** `stalePausedTodoThresholdMs` to migrate — or the behavior silently disappears for every existing project. That is the settings migration above, and it belongs behind its own decision rather than smuggled into a measurement slice. The reconciler is therefore **unwired — deliberately dead code**, for exactly as long as it takes to get that decision. ## The ratified safety invariant The six safeguards (user pause, `autoMerge:false`, dependency, capacity, merge-proof, at-most-once) live **outside** the policy table. A workflow must never be able to author a safety invariant away. Encoded two ways, because either alone is defeatable: - **structural** — the policy exposes only an allow-listed key set; adding a key requires editing the test and re-stating the safety argument (the friction is the point); - **behavioral** — a policy attempting every spelling of "ignore the user pause" has no effect. **Both halves mutation-verified**, because a safety test that cannot fail is worse than none: - making the reconciler honor a policy field that disables the user-pause safeguard → **fails** - adding an unreviewed key to the policy schema → **fails** A third test asserts the reconciler still **acts** on an unpaused card, so a reconciler that suppressed everything cannot pass by doing nothing. ## Scope limits stated rather than implied Only the `surface` action is implemented, so only its relevant safeguard is wired. `surface` mutates no lifecycle state; the other five gate lifecycle-**mutating** actions that do not exist yet, and wiring them now would be untestable dead code. A test records this so the absence reads as deliberate and must be updated when `rebound` lands. ## Verification - `tsc --noEmit` clean in core and engine; `pnpm lint` clean - merge gate green (299 + 10 + 71) - 23 safety tests green; `workflow-lifecycle-traits` green No changeset: `@fusion/core` and `@fusion/engine` are private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
2dce642ccc |
E2E validation: run a RENAMED-column workflow against a live engine (real graph + real PostgreSQL) (#2475)
Stacked on #2472 (`feature/workflow-vocabulary-b3-stranded-todo`). Test-only. No production file is touched. ## Why Every slice of this program has closed with the same caveat: *no renamed workflow was run against a live engine; all evidence is unit-level*. That caveat is load-bearing — eight times this session a test passed without exercising its subject. This PR removes it for the lifecycle spine. ## What actually runs `packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts` drives the REAL pieces: - a **real PostgreSQL `TaskStore`** on a throwaway per-file database (shared PG harness; never the operator's DB, never port 4040), - the **real graph interpreter** (`WorkflowGraphTaskRunner`) with the **real column-boundary controller** wired to the **real `store.moveTask`** — all of its guards, traits, capacity reservation, and post-commit emission, - the **real scheduler release** (`runHoldReleaseSweep`), - the **real post-commit lifecycle bus** (`getWorkflowEventBus`), - the **real converted self-healing sweep** (`SelfHealingManager.recoverStrandedCompletedTodoTasks`, slice B3.1). Only the AI **seams** are scripted — the same boundary `testMode`/`mock` draws in production. **Assertion rule:** every lifecycle claim is asserted on **persisted state** (a fresh `getTask` with the store's task cache defeated, `run_audit_events` rows, `workflow_work_items` rows), never on "a function was called". The one spy — the event-bus subscriber — is asserted on the **received payload**, because the bus silently drops events that fail its shape check, so "emit was called" proves nothing. **Differential design:** the default-vocabulary (`todo`/`in-progress`/`in-review`/`done`) and renamed-vocabulary (`backlog`/`building`/`checking`/`shipped`) workflows come from ONE builder and differ ONLY in their four column ids. Any behavioral delta is attributable to the vocabulary alone. ## Coverage (9 tests, all green) | Scenario | What is proven | |---|---| | Default vocabulary, full spine | planning runs in the hold column, the card parks (graph does not self-promote), the **scheduler** performs hold→wip, the resumed run walks exec → review → merge-gate → end, persisted column is `done` | | **Renamed vocabulary, full spine** | identical, and no leg of the run touches any legacy column id | | Audit differential | the graph-owned boundary crossings are the same crossings node-for-node on both vocabularies; no legacy id appears in the renamed trail | | Event seam | a real subscriber **receives** a well-formed `TaskTransitioned` for the renamed `backlog`→`building` release and for the terminal move; `NodeEntered` arrives for every traversed node including `end` | | Crash / restart | exactly one durable continuation row at `exec`; a brand-new runner resumes from the row and the already-completed `planning` seam does **not** re-run; no duplicate continuation | | Converted sweep (B3.1) | a completed card in a **renamed** hold column is promoted (asserted on its persisted column), a card in the renamed **wip** column is not, and the default `todo` case still works | ## Mutation verification (both directions) Green suites are not evidence in this codebase, so both halves were falsified: 1. Keying `hold-release`'s `isHeldTask` on the `todo` literal → **5 of 6 spine tests fail, and the one that survives is the default-vocabulary one.** That is the exact signature the conversion program cares about. 2. Reverting slice B3.1's per-task hold-column resolution to the literal → **only the renamed stranded-todo test fails**; the default regression floor stays green. ## Findings surfaced by running it 1. **The IR validator refuses a `merge-blocker` column with no reachable merge-class node** ("the gate can never clear without one"). Kept rather than worked around — it means the review column here is genuinely gated. 2. **Entry into the merge region collapses to the legacy `merge` seam** (`MERGE_REGION_KINDS`), so a `merge-gate` node reaches the merge lane. Documented in the fixture. 3. **The transition policy refuses a direct hold → review move**, and it refuses it *workflow-resolved*: on the renamed board the only legal target is its own `building`, not `in-progress`. The recovery callback therefore promotes hold → wip → review rather than bypassing the policy. 4. **`moves.ts` still special-cases the `done` literal** (`if (toColumn === "done") clearNearDuplicateReferencesTo...`) after the post-commit emit. Not converted here and not in this PR's scope — flagged for the Phase B owner. ## Not driven end to end (stated plainly) - **Triage / specification.** The lifecycle starts from a task already bound to a workflow; `triage.ts` was not driven. The `planning` seam is scripted. - **Real merge.** No git worktree, no branch, no squash. `merge-gate` is pure policy; the `merge` seam is scripted. - **Lightweight / self-healing-off workflow.** The Tier 1 policy keys do not exist on this tip — there is no `policies` surface on the IR to set. Not drivable; not substituted with a unit test. - **Process-level crash.** The restart is an in-process one: a brand-new runner resuming from the persisted `workflow_work_items` row with no carried-over memory. No OS process was killed, so this proves durable-state resumption, not signal handling. ## Lane `.pg.test.ts` under the engine-default include glob, gated by `pgDescribe` so it skips cleanly with no PostgreSQL. The merge gate is untouched. Engine `tsc --noEmit` is clean. 🤖 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** * Added comprehensive live PostgreSQL workflow lifecycle coverage, including graph execution, suspension and resume, scheduler capacity release, crash recovery, and durable continuation. * Added validation for renamed workflow column configurations and columnless task movements. * Added event delivery checks for task transitions and node entry events. * Added self-healing recovery for stranded completed tasks in valid hold columns. * **Refactor** * Centralized workflow boundary handling, including task moves, continuation state, audit events, and diagnostics. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
710d56b2db |
U4 trim: delete the dependency-blocked-todo feature (unreachable in production) and revert 5a2de7d (#2477)
Stacked on #2474. Base is `feature/workflow-vocabulary-u4-dead-code` — do not merge before it. Deletes an **entire feature that has never executed in production**, and reverts `5a2de7d`, which only threaded resolved lifecycle columns through it. ## Reachability evidence — the whole basis for this ``` surfaceDependencyBlockedTodos ← in NEITHER sweep registry; no caller in └─ getDependencyBlockedTodoReporter() engine/dashboard/cli — only tests └─ engine/dependency-blocked-todo-reporter.ts ← sole caller of ↓ └─ core/computeDependencyBlockedTodoReport ``` self-healing owns two name-based sweep registries (`runStartupRecovery`, 58 entries; `runMaintenance`, 76). `surfaceDependencyBlockedTodos` is in **neither**, so nothing ever invoked the chain below it. Its four tests passed while proving nothing about production. ## Why delete rather than wire it up Wiring was the tempting option and is the riskier one. Switching on a 450-line path that has never run — whose tests therefore establish nothing about its behavior against real data — is a **behavior change with unquantified blast radius**. This program already refused exactly that move for the **pool-id sentinel**, a one-line change that would switch on dormant enforcement across every project. This is the same class of move at ~450× the size. Deleting is also the recoverable direction: git keeps the feature, and it can be resurrected deliberately — with tests that prove it *runs* — if dependency-blocked reporting is actually wanted. ## The settings keys go with it `dependencyBlockedTodoReportEnabled` defaulted `true` while driving nothing. A schema/API-visible switch that lies about what the system does is worse than no switch. (It had no dashboard UI field — the dashboard test allowlist already recorded it as *"no UI field"*.) Four sibling tuning keys are removed with it. ## Against my own earlier work `5a2de7d` threaded resolved lifecycle roles into `computeDependencyBlockedTodoReport` and its reporter, answering a review finding I confirmed as real. **The code was correct; the impact claim was not**, because the path never executes. Neither the reviewer nor I checked *reachability* before agreeing the defect mattered — only correctness. A correction is posted on that thread in #2470. **Scope limit on that admission:** the same finding also described *incorrect scheduler ordering*. That half runs through `buildUnblockWeightMap` in `task-priority.ts`, which is **live** and was already threading `terminalColumns` (B1, `434b385`). Scheduler ordering was never affected, before or after. ## What survives `blocker-fanout.ts` **stays** — it is live via `task-priority.ts`. Only the plural `holdColumns` option added by `5a2de7d` is reverted, since the deleted report was its sole consumer. `holdColumn` (singular, from B1) remains. ## Net **1,244 deletions / 5 insertions across 15 files** — ~450 production lines, ~684 test lines, 5 settings keys. ## Verification - `tsc --noEmit` clean in **core, engine, and dashboard-app**; `pnpm lint` clean - merge gate green (299 + 10 + 71) - self-healing suite: 411 passed, 1 **pre-existing** failure (`archiveStaleDoneTasks`) - dashboard settings-descriptions suite green - `settings-parity.test.ts` has one **pre-existing** failure (`agentToolOutputMaxChars` overlap) that fails identically with these changes stashed — unrelated to this deletion No changeset: `@fusion/core` and `@fusion/engine` are private. 🤖 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** * Added quiet-window backlog health diagnostics for stalled items in review, with repeat-alert suppression. * Added default thresholds for backlog-pressure alerts. * **Changes** * Removed dependency-blocked todo reporting and related alerts. * Removed the dependency-blocked todo enable/disable setting; remaining tuning options are no longer active. * Updated the workflow hold classification to use a single todo column. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
923a7c0fc9 |
U4 trim: delete the dead resetStepsIfWorkLost duplicate in self-healing (#2474)
Stacked on #2472 (Phase B slice B3.1). Base is `feature/workflow-vocabulary-b3-stranded-todo` — do not merge before it. First output of the **U4 reshape survey**: dead code removed with reachability evidence, not a redundancy argument. ## What is deleted **`SelfHealingManager.resetStepsIfWorkLost`** — 50 lines including its docblock and a section header left with no other member. Evidence: - `private`, with **zero in-file references** beyond its own declaration. - TypeScript already reported it as `declared but its value is never read` — the compiler has been flagging this. - The **live** implementation is `executor.ts:20080`, an independent copy that `executor.ts` actually calls (12667, 14664). The self-healing copy is an orphaned duplicate of it. - Not in either sweep registry, no test of its own, no caller anywhere in engine / dashboard / cli. Safe against every U4 constraint: it enforces none of user pause, `autoMerge:false`, dependency, capacity, merge-proof, or an at-most-once safeguard. ## Why the second approved deletion is NOT here `surfaceDependencyBlockedTodos` was approved alongside this one as a 28-line orphan. It is not. On inspection it is the **tip of an entire unreachable feature**: ``` surfaceDependencyBlockedTodos (in NEITHER registry, no production caller) └─ getDependencyBlockedTodoReporter() ← sole caller └─ engine/dependency-blocked-todo-reporter.ts 223 lines ← sole caller of ↓ └─ core/dependency-blocked-todo-report.ts 184 lines ``` ≈ **450 production lines across three files, plus 684 lines of tests in four files.** The operator-visible setting `dependencyBlockedTodoReportEnabled` defaults `true` in `settings-schema.ts` and drives nothing. Deleting only the approved 28-line tip would be **strictly worse than leaving it** — it orphans the getter and field and strands 407 lines of module with no remaining reference to explain why. Escalated for a decision (delete the subtree / wire the feature up / leave it) rather than resolved unilaterally. ## Verification - `tsc --noEmit` clean, `pnpm lint` clean - self-healing suite: **415 passed**, 1 failure **pre-existing** (`archiveStaleDoneTasks` — fails identically before this change) No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Behavior Changes** * Removed automatic detection and reset of task steps when no unique work is found on a task branch. * Tasks with completed or in-progress steps will no longer be automatically returned to pending based on this condition. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
a4dee1162e |
Phase B slice B3.1 (U4): resolve the hold column in recoverStrandedCompletedTodoTasks — query and guard together (#2472)
Stacked on #2471 (Phase B slice B2). Base is `feature/workflow-vocabulary-b2` — do not merge before it. **First landable slice of U4 (self-healing.ts).** One sweep, one PR, per the phase's sub-split rule. ## The finding: the guard and the query must convert together `recoverStrandedCompletedTodoTasks` promotes a card whose steps are all done/skipped but which is still sitting in the hold column — finished work that never handed off to review. It decided *"is this card in the hold column?"* **twice**, and both were literal: | | was | |---|---| | the QUERY | `listTasks({ column: "todo", slim: true })` | | the GUARD | `task.column !== "todo"` | **Either half alone is a green diff with zero behavior change.** A correct guard behind a literal query never runs; a converted query behind a literal guard rejects every row it just fetched. This is the shape that made B1's stale-paused-todo fix cosmetic, and the phase brief predicted more of it here — correctly. I proved it rather than asserting it: - literal **QUERY** restored (converted guard kept) → **3 tests fail** - literal **GUARD** restored (converted query kept) → **2 tests fail** Neither half passes the suite alone. ## Falsification came first Per the brief I tried to prove the work unnecessary before doing it. It is necessary, and the evidence is empirical, not assumed: the 7 tests were written against unmodified code and 3 failed. Unlike B2's hold-release — which turned out already converted — **self-healing is uniformly unconverted at the query level**: 53 of its sweeps carry a hardcoded `column:` filter (survey in the worker report). ## Negative half, per the brief A completed card resting in a WIP or review column is **not** promoted. Dropping a column filter without a per-task hold check would promote finished cards out of every column — laundering work past review, a louder bug than the silent one being fixed. ## Test-harness hazard (will recur in every remaining U4 slice) The pre-existing self-healing store mock returns its fixture from `listTasks` **regardless of arguments**. A renamed-hold test on that harness passes while the query stays hardcoded, because the mock hands the sweep rows the real store never would. The new harness **honors** the column filter, and one test asserts the query is no longer scoped to the literal. This is documented in the new file's header for whoever writes the next slice. ## Cost The column filter is gone, so the cheap non-column rejections (paused / executing / incomplete steps / errored / no-commits / skip-bypass taint) run **first and synchronously**; only survivors pay an IR resolution, shared through an `irCache`. A board spanning three workflows resolves three IRs regardless of card count. `includeArchived: false` preserves what the column filter did implicitly. The hold column resolves **per task** — a board spans workflows, and a card in *another* workflow's hold column must not be promoted. ## One pre-existing assertion changed, deliberately `self-healing.test.ts` pinned `listTasks` being called with `{ column: "todo", slim: true }`. That query shape changed on purpose; the assertion now pins the new one. The behavioral assertions either side of it (one qualifying card, promoted exactly once) are untouched and still pass. ## Carried A3 questions — both answered **Q1 — does the sync/SQLite counter have the same pool-id mismatch?** **Not applicable: there is no sync counter.** `occupantsByColumnForWorkflowImpl` and `listWorkflowOccupantTaskIds` are async/PG-only and throw without an initialized `AsyncDataLayer`; the sync twin went with the PG cutover. There is no second counter that could mismatch. The surviving pool-id sentinel sites are `project-store-ops.ts:767/819` and `moves.ts` — both parked by operator decision, untouched here. **Q2 — are custom workflows with an explicit numeric limit affected?** **No, by design.** `resolveColumnCapacity` gives `config.limit` top precedence (`configLimit` → `limitSetting` → default-workflow read-through → `Infinity`), and `resolveWipBudgetColumns` documents that a column with an explicit numeric limit is **independent — its budget is itself alone**. Such a column never pools, so there is no pool id to mismatch. Read-only analysis; no code changed for either question. ## Remaining U4 scope (not in this PR) 214 literal occurrences across ~70 methods; **53 sweeps carry a query-level column filter**. Hold-gated sweeps still to convert: `clearStaleBlockedBy`, `reclaimSelfOwnedBranchConflicts`, `reconcileCompletedTask`, `recoverMergedReviewTasks`, `recoverStuckMergeDeadlocks`, plus non-query `todo` guards in `recoverPausedAbortFailures`, `reconcileDependencyBlockingLeases`, and others. `surfaceStalePausedTodos` was already converted (B1 follow-up) and is verified intact on this branch. ## Verification - 7 new tests green; **both mutations kill the suite** - self-healing suite: 415 passed, **1 failure pre-existing** (`archiveStaleDoneTasks` — confirmed identical by stashing my changes) - merge gate green (299 + 10 + 71) - `tsc --noEmit` clean, `pnpm lint` clean No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved recovery of completed tasks stranded in workflow-specific hold columns, including renamed hold columns. * Preserved recovery for built-in workflows while correctly handling boards with mixed workflow configurations. * Prevented recovery for tasks in non-hold columns or with paused, incomplete, or errored states. * Added fallback handling when workflow details cannot be resolved. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
02b0f4f860 |
Phase B slice B2: U5 small movers — 12 literal sites converted, plus a negative result on hold-release (#2471)
Stacked on #2470 (Phase B slice B1). Base is `feature/workflow-vocabulary-conversion` — do not merge before it. ## What this is Phase B slice B2 — the U5 small movers. **12 literal sites converted, plus one negative result.** The plan estimated 36 sites. A survey found 12 genuinely-convertible ones, and separately found that the plan's headline hold-release scenario **was already fixed**. Both are reported below rather than padded into a bigger-looking diff. ## The negative result (commit 1) The plan named hold-release.ts as a target on the scenario *"release readiness must hold and release identically for a RENAMED hold column."* I wrote that test first, to prove it broken. **It is not broken.** All five assertions passed against unmodified `hold-release.ts`. U6/KTD-5 had already converted the module — `isHeldTask`, `resolveReleaseTarget`, and `dependencySatisfied` each resolve the task's IR. **`hold-release.ts` has no production change in this PR.** The tests are kept as a regression floor: the invariant rests on three independent trait resolutions any of which could be "simplified" back to a literal, and nothing else covered a renamed vocabulary end-to-end through the sweep. **I verified the tests can actually fail.** Mutating `isHeldTask` back to `task.column === "todo"` kills all five. Without that check, a green run against unmodified code is indistinguishable from a test asserting something trivially true. Two drafting notes kept in the file: the renamed ids deliberately avoid colliding with any legacy literal, and the first draft's two dependency tests used a `capacity` hold — which never consults dependencies at all, so one passed **vacuously**. Both now use a `dependency` hold. ## The 12 conversions Each was red-green: the renamed-workflow test written first and **observed failing**, then made to pass. | Site | Was | Now | |---|---|---| | `task-agent-sync` CLEAR_COLUMNS | `{done,archived,todo,triage}` | resolved complete+archived+hold+intake | | `task-agent-sync` isParkedTaskColumn | `{todo,triage}` | `parkedColumns` param (hold+intake) | | `task-agent-sync` handler branch | `to === "todo" \|\| "triage"` | resolved parked set | | `mesh-lease` parked guard | `task.column !== "todo"` | resolved rebound column | | `mesh-lease` rebound move | `moveTask(id,"todo")` | resolved rebound column | | `mesh-lease` audit decisionPath | `=== "todo" ? … : …` | same resolved column | | `mesh-lease` audit newColumn | `… : "todo"` | same resolved column | | `merger-ai` already-finalized | `=== "done" \|\| "archived"` | resolved complete+archived | | `merger-ai` ×4 rebounds | `moveTask(id,"todo")` | shared `resolveFinalizeReboundColumn` | Rebound targets all use KTD-10 `resolveReboundTarget` (hold → intake → first column), the helper `self-healing.ts:714` already uses — reused, not invented. ## Three findings worth reading **1. The mesh-lease bug was in the AUDIT, not the move.** The guard and the audit were *independent* `=== "todo"` comparisons, so `newColumn` asserted the card landed in `todo` regardless of what the move did. For a workflow with no `todo` column that produced a lease-recovery trail naming a nonexistent column — and run-audit is the only post-hoc record of a lease recovery. Now resolved once and threaded to both, so they are structurally incapable of disagreeing. **2. The merger-ai failure mode was not what I predicted.** I expected the already-finalized guard to fail open and re-merge a finished card. The red run showed it actually throws `Cannot merge FN-1: task is in 'shipped', must be in 'in-review'` — a hard error blaming the column, on a task whose real state is "already done". The thing preventing the re-merge is *itself* a literal in core's `getTaskMergeBlocker`, outside this slice. Two bugs coinciding, not a design. **3. A fourth site had to move that wasn't on the list.** `evaluateParkedAgentTaskLink` calls `isParkedTaskColumn` internally. Converting only the handler would have left the preservation branch on legacy ids after the caller resolved a renamed workflow — trading a stale-link bug for a **worse** dropped-link bug (a live agent's link cleared mid-run). ## Deliberately NOT converted Both keep their literals with the reason recorded at the site under a greppable `DELIBERATE-LITERAL` tag: - **`hold-release.ts:326` `legacyDependencySatisfied`** — the FN-5719 dual-accept half. Converting makes both halves compute the same answer, deleting the compatibility signal *and* its divergence detector while looking like a cleanup. - **`replan-target.ts` final fallback** — its value is precisely that it is *not* trait-resolved; resolving it against the workflow is the stranded-card bug it was written to fix. ⚠️ **The U12 literal ratchet does not exist in the tree yet.** The brief assumed an allowlist to add entries to; there is none. `grep -rn DELIBERATE-LITERAL packages/*/src` enumerates the sites it must admit. ## What I could NOT verify - **One of the four merger-ai rebound sites is untested.** The `landWorkspaceTask` rebound is verified by inspection and the shared resolver's unit tests only — `landWorkspaceTask` is only ever *mocked* (project-engine.test.ts), never executed. Covering it needs a multi-repo git fixture and a full land run. The **other three are genuinely exercised** by pre-existing merger-ai.test.ts (lines 676/716/777/895 assert `moveTask("FN-1","todo",…)` through a real git repo) and pass unchanged — real wiring proof for those. - **3 of the 9 task-agent-sync tests passed before the conversion too**, vacuously — the literal handler early-returned and cleared nothing. They assert nothing about the old code; they are guardrails against the conversion over-clearing. - **No renamed workflow was run against a live engine.** All evidence is unit-level. ## Call sites outside this slice — NOT converted, byte-identical They keep the legacy defaults: `scheduler.ts:1273`, `agent-heartbeat.ts:1169/3642`, `self-healing.ts:11600/11665` (all `evaluateParkedAgentTaskLink`), and `merger.ts:6585` (the sibling terminal guard). Each is its own Phase C/D surface. ## Behavior changes (not a pure refactor) For a **renamed** workflow: agent links now actually get cleared on terminal moves (they never were); lease rebounds land in the resolved hold column; finalize-blocked rebounds land in the resolved hold column and their operator-facing task-log lines name the real column; already-finalized cards short-circuit cleanly instead of throwing. For **builtin:coding** and any unresolvable workflow: byte-identical. Every new parameter defaults to the legacy set, and both merger-ai resolvers fail *soft* to legacy ids in opposite directions — the terminal guard keeps `done`/`archived` (losing it sends a finished card into the merge path), the rebound keeps `todo` (abandoning it strands the card in the merge lane with no owner). ## Verification - Merge gate **green**: 299 + 10 + 71 tests - Slice suites **green**: 100 tests across 8 files (new + all pre-existing neighbours) - Existing merger suites **green**: 82 tests across 5 files, unchanged - `tsc --noEmit` clean, `pnpm lint` clean No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
gsxdsm
|
5d0f1ef631 |
Phase B slice B1: lifecycle column roles in the U6 policy modules (4 guards, red-green) (#2479)
**Stacked on #2469** → #2468 → #2467. Base is `feature/workflow-capacity-ground-truth`. This is **slice B1 of Phase B, not all of Phase B.** Sizing escalation sent separately; the census is below. ## Why this is a slice Measured census of code lines referencing a lifecycle column literal (comments excluded): | Unit | Files | Sites | |---|---|---:| | U4 | `self-healing.ts` | 203 | | U5 | `executor.ts` 171, `scheduler.ts` 55, `replan-target.ts` 20, `merger-ai.ts` 5, `hold-release.ts` 4, `mesh-lease-manager.ts` 4, `task-agent-sync.ts` 3 | 262 | | U6 | `moves.ts` 34, `default-workflow-hooks.ts` 13, `board-config.ts` 9, `blocker-fanout.ts` 6, `task-priority.ts` 5, `dependency-blocked-todo-report.ts` 2, `stale-paused-todo.ts` 1 | 70 | | | **Total** | **535** | The plan's "~207" counts the guard category only. Under the phase's non-negotiable rule — a test that **fails before** conversion, per guard — that is ~200 red-green cycles. Doing it as one sweep would reproduce exactly the failure this phase exists to prevent: converted guards nobody proved still fire. `moves.ts` and `default-workflow-hooks.ts` stay **parked** per the dispatch constraint (move-path convergence and the pool-id sentinel are on an operator decision). ## Guards converted (4), each red-green Every case below was written **first** and observed failing against the literal implementation. | Module | Guard | Before → After | |---|---|---| | `stale-paused-todo.ts` | stall detection | `column !== "todo"` → resolved **hold** column | | `blocker-fanout.ts` | active | `ACTIVE_COLUMNS.has(col)` → `!terminalColumns.has(col)` | | `blocker-fanout.ts` | hold-wait metric | `col === "todo"` → resolved **hold** column | | `task-priority.ts` | unblock active | `UNBLOCK_ACTIVE_COLUMNS` **deleted**, folded into the terminal set | Three of the seven new cases are **regression floors** that pass before and after. One of them earned its keep immediately: it failed on my own fixture (`activeCount` vs the public `totalCount`), catching a bad test rather than bad code — which is the point of asserting the default path alongside the renamed one. ### The `task-priority` finding `UNBLOCK_ACTIVE_COLUMNS` and `DONE_COLUMNS` encoded **one concept twice**, two lines apart, and disagreed for any custom column: dependency counting treated a `drafting` card as unmet (correct) while the active check treated it as inactive (wrong), zeroing the blocker's unblock weight. The enumeration wasn't just legacy-shaped — it contradicted its own neighbour. ## ⚠️ Behavior change, not a pure refactor Inverting active from enumeration to exclusion means **a card in a column that is neither terminal nor in the legacy enum now counts as active where it previously did not.** That is the plan's stated intent, but it is a real change for any project already using a custom column — **Coding (Ideas)' `ideas` column is the in-tree case.** Fan-out counts and unblock weights for such cards will rise. ## Verification - Four affected suites green (45 tests), each conversion observed red→green. - `pnpm lint`, `tsc --noEmit` (core) green. **Not verified / not done, stated plainly:** - **Call sites are not wired.** These modules now *accept* resolved roles; every parameter still defaults to the legacy set, so at the call sites the vocabulary is unchanged. A caller that cannot resolve a workflow keeps literal behavior. Threading `resolveLifecycleColumns` through `reads.ts` and `self-healing.ts` is follow-on work — until then the guards are *convertible*, not *converted end-to-end*. - `dependency-blocked-todo-report.ts` and `board-config.ts` are untouched in this slice. - 19 core-suite failures exist on this branch; all confirmed **pre-existing** by stashing and re-running on a clean tree (`duplicate-guard`, `log-severity-spam-contract`, `settings-parity`, `task-delete-caller-attribution`, `settings-defaults`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- **Supersedes #2470**, which GitHub force-closed when its base branch was deleted by the merge of #2469 and refuses to reopen. Same head branch, same commits (rebased onto `main`), now based on `main` directly. The two P1 review threads on #2470 were resolved there — one of them with a correction noting the threading half landed in code that was subsequently deleted as a dead feature in #2477. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Dependency and blocker reports now correctly recognize custom hold, active, and terminal workflow columns. * Blockers in renamed terminal columns are no longer incorrectly reported as active. * Stale paused-task badges and self-healing now work with workflow-specific hold columns. * Mixed boards with different workflow column names are handled consistently. * Existing default workflow behavior remains compatible, including fallback handling when workflow details cannot be resolved. * **Enhancements** * Reporting and task-priority calculations now support configurable single or multiple hold and terminal columns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
4158cf1ab7 |
Phase A: workflow-owned lifecycle foundation (U1, U2, U3) (#2467)
Phase A (Foundation) of
`docs/plans/2026-07-26-001-refactor-workflow-owned-lifecycle-plan.md`.
Three units, one commit each. No operator-visible behavior change.
## U1 — Lifecycle-column resolution seam
`resolveLifecycleColumns(ir)` returns `{ intake, hold, wip, review,
complete, archived }` — the first column carrying each trait,
`undefined` for a role no column carries.
`resolveTaskLifecycleColumns(store, taskId, cache?)` is the store-aware
form; the cache is caller-owned so a sweep reads one IR per workflow
rather than one per card.
A v1/column-less IR resolves to `undefined` for the **whole struct**
rather than a struct of undefined roles. A caller must be able to
distinguish "this workflow declares no hold column" (a real shape to
honor) from "no column vocabulary at all" (skip and log) — only the
second licenses conservative fallback.
Nothing consumes the seam yet; Phases B–D convert the ~207 hardcoded
column literals onto it.
## U2 — Delete the pre-cutover parity machinery (delete-only)
**`workflow-columns-settings.ts`** — `isWorkflowColumnsEnabled` had the
body `return true`. Six live call sites branched on it, so every
flag-OFF arm was dead code that read as a supported configuration.
Deleted; surviving side inlined at self-healing's transitionPending
sweep, the scheduler's per-column capacity diagnostic, merge-trait's
policy resolver, the board-workflows payload, two task-workflow routes,
and the CLI TUI's column enrichment.
**`workflow-parity.ts`** — asserted the default workflow's adjacency
*equals* the legacy `VALID_TRANSITIONS`. U11 deliberately breaks that
equality by merging Todo into Planning, so this is not a stale assertion
to update; it is a contract against the target state. Its emitter
(`workflow-parity-observer.ts`) is already a tombstone, so
`getWorkflowParitySummary` and `computeWorkflowColumnsGraduationReport`
aggregated run-audit rows nothing writes and had no caller outside
`TaskStore`. Both store methods go with it.
`flagEnabled` stays on the board-workflows **wire** as a constant `true`
— shipped dashboard clients still branch on it, and changing the
response shape is not a deletion. U10 retires the field once no client
reads it.
The `legacy-tombstones` ratchet is extended to both files plus seven
symbols, each with the reason it is gone.
### ⚠️ Finding: the third listed deletion was NOT dead
The plan also lists "the flag-off inline move path" in
`task-store/moves.ts`. It is **not** deleted, per U2's execution note
("any behavior change found while removing a branch means the branch was
not dead").
That path is gated on `isWorkflowColumnsCompatibilityFlagEnabled`
(`store.ts:38`) — a **different** function from the always-true public
helper. It reads the raw `experimentalFeatures.workflowColumns` setting,
which nothing in production sets (`settings-schema.ts:396` — "no default
flags are emitted"; zero non-test writers; the operator's own
`~/.fusion/settings.json` has no such key). So `useWorkflow` is false
for effectively every real project: the flag-OFF inline side effects are
the **live** default move path and the flag-ON `default-workflow-hooks`
path is the dead one. The code says so itself at `moves.ts:638`.
Deleting that branch would swap every project onto an untravelled code
path — a behavior change, not a deletion.
**Carry this into Phases B and C, stated plainly so the plan's error is
not repeated:**
> **The inline move path in `moves.ts` is LIVE.
`default-workflow-hooks.ts` (the trait-hook path) is DEAD.** KTD-6
asserted the inverse. Until the convergence unit lands, **nothing may
assume trait hooks run** — a guard, sweep, or subscriber written against
`applyDefaultWorkflowMoveEffects` would never fire in production and
would still pass its tests.
Convergence is **not** attempted here. It is its own unit (Phase A2)
with a proper equivalence proof, per operator decision.
### U3's emit point is on the LIVE path — the seam is not born dead
Worth stating explicitly because it is the failure mode that would make
every later subscriber silently never fire: the `TaskTransitioned` emit
is **not** inside the `if (useWorkflow)` branch. That block closes at
`moves.ts:1212`; the emit sits at `:1214`, beside the existing
`store.emit("task:moved", …)`, on the unconditional post-commit path. It
therefore fires on **both** the live inline path and the dead hooks
path, and the convergence unit inherits the obligation to keep it firing
on whichever path survives — same events, same order, same payloads.
The graph-side emitters (`NodeEntered`, `RunSuspended`) carry the same
risk from a different direction: the bus refuses an invalid payload
*silently* by design, so an emitter regression would stop the event with
no test failure. They are asserted end-to-end through the real bus —
"did a subscriber actually receive it", not "was emit called" — because
a spy passes on a refused payload. The `moveTaskInternalImpl` emit does
**not** yet have that end-to-end assertion against a real store move;
that proof belongs to the convergence unit, which has to build the
both-paths fixture anyway.
## U3 — Post-commit event seam with a transactional outbox
**The bus is not a queue, not a transaction participant, and not a
delivery guarantee.** Durable follow-on work uses the transactional
outbox — a `workflow_work_items` row written *inside* the transition
transaction (the shape `createCompletionHandoffWorkflowWork` already
uses). "Emit after commit, let a subscriber enqueue the work" has a
crash window where a process dies between commit and subscriber, leaving
no event *and* no work-item row, so required work is skipped permanently
with nothing to recover from. Post-commit subscribers therefore carry
only losable reactions.
Emission is consequently lossy and isolated by design: a throwing or
rejecting subscriber is caught and logged, cannot roll back the
transition, and cannot stop the others. Deliveries append to one serial
chain, so two transitions on a task deliver in commit order.
The ids/outcomes-only rule is **mechanised, not documented** —
run-audit's equivalent lives only in prose and has been violated
repeatedly. A payload carrying an object body or a prose string is
refused at the emit boundary and never reaches a subscriber or log sink.
It degrades rather than throws: the emitter is post-commit, so a shape
bug must not become a lifecycle failure.
Emit points: `TaskTransitioned` from the single post-commit point in
`moveTaskInternalImpl`; `NodeEntered` and `RunSuspended` from the graph
column boundary, the latter *after* the durable continuation is
persisted so an observed suspension implies a resumable run.
`registerWorkflowEventSubscribers` (engine) is empty on purpose —
U7/U8/U10 move real reactions onto it, each with the characterization
test proving the reaction was non-authoritative first.
## Verification
- `pnpm test:gate` — green (2/10, 16/299, 1/71).
- `pnpm lint`, `pnpm build`, `tsc --noEmit` on core and engine — green.
- U1: 20 tests in `workflow-lifecycle-traits.test.ts`, including the
fully-renamed-workflow case (fails if the resolver falls back to a
literal) and a shared-cache read-count assertion.
- U2: `legacy-tombstones.test.ts` green with the extended ratchet;
`board-workflows`, `merge-trait`, `workflow-graph-executor-parity`, and
move-hook suites green with no expectation edits.
- U3: 20 bus-invariant unit tests (isolation, ordering, the allowed-key
and required-key halves of the ids-only rule, lossiness) plus 3
end-to-end emitter-delivery tests; 5 outbox tests against a **real
PostgreSQL** work-item table (crash survival, rollback, at-least-once
redelivery on lease expiry, idempotent handler → one effect,
dropped-subscriber vs. durable work). A hand-written fake of the lease
predicate would only prove the fake redelivers.
**Not verified:** the `moveTaskInternalImpl` emit is confirmed on the
unconditional post-commit path by structure and by the surrounding
tests, but is *not* yet asserted end-to-end against a real store move on
both flag settings — that is Phase A2's fixture. The engine subscriber
registry ships empty by design, so no production subscriber exercises
the bus end-to-end yet. `settings-defaults.test.ts` has one pre-existing
failure on `main` (a logger-prefix mismatch in the
`mergeIntegrationWorktree=cwd-main` warning) — confirmed present on a
clean tree, unrelated to this branch.
🤖 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**
* Workflow lifecycle columns are now derived from workflow definitions,
supporting renamed and custom workflows.
* Added post-commit lifecycle events for task transitions, node entry,
and run suspend/resume with validated payloads.
* Follow-on processing for lifecycle emissions is now more robust
(rollback-safe, at-least-once delivery, idempotent handling).
* **Bug Fixes**
* Workflow board responses, task enrichment, and promotion no longer
depend on workflow-columns feature-flag gating.
* Subscriber failures no longer impact committed workflow transitions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
gsxdsm
|
8b039a543e |
fix(desktop): advance Pi runtime pin to 0.82.1 for packaging PR lane (#2465)
## Summary - Advance the matched Pi runtime pin (`pi-ai`, `pi-coding-agent`, `pi-agent-core`, `pi-tui`) from **0.82.0 → 0.82.1** so electron-builder's production-dependency walk accepts `pi-agent-core`'s `pi-ai@^0.82.1` requirement. - Fixes the Desktop packaging PR-lane failure: `Production dependency @earendil-works/pi-ai not found for package @earendil-works/pi-agent-core` (required `^0.82.1`). - Keep the workspace override guard; update pin-policy fixtures and CLI package-config expectations. - Tighten the advisory packaging step-order test so it asserts against the real `electron-builder --dir` step (not a missing release-only step name that previously passed via `indexOf === -1`). - Run `pnpm dedupe` so the packaging lane's lockfile dedupe early-warning is clean. ## Context #2439 pinned the full Pi closure at 0.82.0 and made recent main-based packaging runs green. This advances to the current upstream patch so deploy + electron-builder stay aligned with `pi-agent-core@0.82.1`'s declared dependency range. ## Test plan - [x] `node scripts/check-pi-versions-pinned.mjs` - [x] `node --test scripts/__tests__/check-pi-versions-pinned.test.mjs` - [x] `pnpm --filter @runfusion/fusion exec vitest run src/__tests__/package-config.test.ts` - [x] `pnpm --filter @fusion/desktop exec vitest run src/__tests__/release-workflow.test.ts` - [x] `pnpm dedupe --check` - [ ] GitHub: Desktop packaging (should run full packaging walk — lockfile/package.json touched) - [ ] GitHub: PR Checks (Lint, Typecheck, Build, Gate) |
||
|
gsxdsm
|
99c9f14ee0 |
feat: run Plan Review in the planning lane with a Plan Review badge (#2462)
## What Plan Review, planning, and the replan loop move from the implementation column into the **planning lane** (`todo`), so a task under specification never holds a WIP slot. The card crosses into `in-progress` exactly once, at `parse`, released by the scheduler. Operators also finally see a **Plan Review** badge while the gate runs — it was previously invisible on the default workflow. ## The part that made it possible Moving the node is ten lines. It was attempted three times and reverted each time, because a graph run with no durable continuation replayed from `start` and dragged an in-progress card *backward* out of the WIP column, firing `abort-on-exit` and stranding it in a pre-WIP column with no releaser. So this PR adds the graph **entry contract** — `resolveColumnResumeNode`: | Card is in | Resumes at | |---|---| | `triage` | `start` | | `todo` | `plan` | | `in-progress` | `parse` — never re-plans, never moves backward | | `in-review` | first review node — gates are not skipped | `ir.columns` is ordered and that order is the lifecycle order; rework and failure edges are excluded so the entry point is always the main path. The proof it's the right fix: **`executor-task-done-invariant` passes unmodified** after failing every previous attempt. ## Also in here - **Release gate narrowed twice.** `isUnplannedForExecution` applies its pre-release plan-review gate only when the node's column equals the card's column *and* the group is enabled for the task. The enablement check fixes a real deadlock — a task with Plan Review toggled off was held forever waiting for evidence nothing would ever write. - **Badge cleanup.** Gate badge reads "Plan Review" instead of the ambiguous "Reviewing" and no longer hides behind a lane restriction; the status badge stops duplicating it; `planning` renders as "Planning" instead of the raw engine token. - **Coding (Ideas)** renames its planner column to "Planning" (id `todo` unchanged) and loses its private planning-node re-home — the graph it clones is already plan-in-place. - **New sweep** `reconcileUndeclaredTaskColumns` re-homes a row whose column its workflow no longer declares. Written for a follow-up, kept because it makes any column edit survivable. ## Test changes Scheduler and release fixtures now model a card whose Plan Review passed — the state every real card is in when the capacity sweep sees it. A held unreviewed card is the gate working, and that path stays owned by `pre-release-plan-review.test.ts`. New `workflow-graph-entry-contract.test.ts` covers the invariant at every lifecycle position, plus the gap-column and remediation-node cases. ## Verification Gate 299 + 70 + 10, dashboard badge suites 672, engine workflow/entry/executor suites 147, core 122. Lint and typecheck clean. Full engine suite sits at the pre-existing baseline (notifier / plugin-runner / notification-service, untouched by this). ## Follow-up Removing the Todo column entirely is a separate ~207-site lifecycle-vocabulary refactor — planned in `docs/plans/2026-07-26-001-refactor-workflow-owned-lifecycle-plan.md` (companion docs PR). 🤖 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** * Plan Review now runs in the Planning lane before implementation begins. * Cards resume from their current workflow column without replaying earlier steps. * Added automatic recovery for cards stranded in outdated workflow columns. * **Improvements** * Renamed the Coding (Ideas) planner column to “Planning.” * Refined Plan Review gating to respect enabled settings and the card’s current column. * Updated planning and Plan Review badges for clearer, consistent labels across cards and lists. * **Bug Fixes** * Improved workflow transitions and release behavior around planning, review, and execution. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
5ae6332563 |
refactor: collapse dead SQLite dual-path code; keep migration-only readers (#2454)
# Remove dead SQLite dual-path code; keep migration-only readers ## Summary PostgreSQL cutover left hundreds of production dual-path branches (`backendMode ? PG : SQLite/store.db`) whose SQLite arms only hit throwing `Database`/`ArchiveDatabase`/`CentralDatabase` stubs. This change mechanically collapses those unreachable arms so production authority is AsyncDataLayer/PostgreSQL only, while preserving the six authorized read-only migration/recovery `DatabaseSync` seams. ## Dual-path mass removed | Metric | Before | After | |---|---|---| | `if (…backendMode)` (non-test) | ~328 | ~70 | | `store.db` / `this.db` refs in core (non-test) | ~570+ | ~375 (mostly pure legacy MissionStore/eval/insight SQLite classes + thin getters) | | Net diff | — | **~6.7k lines removed** across 41 files | Remaining `backendMode` checks are intentional (incomplete-PG sync safe-defaults, settings-sync disabled-on-PG, symbol-lock PG-only gates, “requires PostgreSQL” config versioning throws), not live SQLite authority. ## Subsystems cleaned - **Core TaskStore / task-store/***: collapsed if/else and early-return dual-path across reads, moves, lifecycle, mutations, workflow, archive, branch/PR, artifacts, comments, audit, project ops, etc. `initImpl` is PostgreSQL-only (SQLite startup tail deleted). - **Satellite stores**: automation, agent, routine, plugin, secrets, approval-request, central-core dual-path arms collapsed. - **Plugins**: reports async methods, compound-engineering pipeline + session stores, CLI Printing Press store — SQLite fallbacks removed; PG required. - **Engine**: no functional dual-path change beyond whitespace (settings-sync / peer-exchange PG-disabled behavior kept). ## Six migration-only readers retained (allowlist unchanged) 1. `packages/core/src/postgres/sqlite-migrator.ts` 2. `packages/core/src/project-identity.ts` 3. `packages/core/src/sqlite-validation.ts` 4. `packages/core/src/postgres/startup-factory.ts` 5. `packages/cli/src/commands/db.ts` 6. `scripts/lib/start-local-project.mjs` Plus low-level `sqlite-adapter` and migrator/startup-import tests. Inventory ratchet still requires exactly these six `new DatabaseSync(` production sites, all `readOnly: true`. ## Not treated as SQLite - `.fusion/project.json`, `task.json`, `agent-log.jsonl` file storage - AsyncDataLayer / Drizzle PG paths - Incomplete-PG sync safe-default stubs (still return empty/false/null under backend without consulting SQLite) ## Verification - `sqlite-production-reader-inventory.test.ts` — 15/15 pass - `incomplete-pg-ports.pg.test.ts` — 6/6 pass - Targeted PG tests (create-task, move, handoff, runtime-persistence, agent, mission, insight, central-core) — green - `tsc --noEmit` for `@fusion/core`, `@fusion/engine`, `@fusion/dashboard` — green - `scripts/check-no-getdatabase.mjs` — clean <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Improved end-to-end consistency by making PostgreSQL/async persistence the standard across core task/workflow, automation, agents, plugins, routines, secrets, approvals, central operations, and session storage. * Unified scheduling, settings, configuration revision writes, run/workflow selection, queues/leases/transitions, and audit/lifecycle updates around consistent async transaction behavior. * **Bug Fixes** * Fixed edge cases for archived/deleted reads, unarchive/recovery flows, not-found handling, and task/artifact/document/log/comment operations, including more reliable emissions and hydration across search/list and lifecycle operations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Phil Larson
|
52d64fa66e |
fix(engine): project CE steps after review handoff (#2464)
## Summary - reconcile successful graph-native workflow results with pending task checklist steps even when review handoff already moved the card into the merge column - preserve terminal, paused, and no-redundant-move behavior - cover the real Compound Engineering post-review-handoff state with a regression test ## Root cause Compound Engineering runs `review-handoff` before `merge`. Review handoff moves the task to `in-review`, which is also the merge column. `ensureWorkflowMergeBoundaryTask()` returned immediately for cards already in that column, before projecting successful `workflowStepResults` onto legacy `Task.steps[]`. The merger then saw `0/N` and rejected approved work with `task has incomplete steps`. ## Verification - RED: regression test failed before the fix because `store.updateTask` was never called - GREEN: `executor-graph-boundary.test.ts` — 6 passed - relevant non-PostgreSQL set — 31 passed, 5 PostgreSQL tests explicitly skipped - `@fusion/engine` typecheck passed - changeset format passed - `git diff --check` passed ## Baseline note `ce-workflow-step-executor.test.ts` currently has three failures on clean `origin/main` after FN-8601 foreach-proof hardening. The same failures reproduce without this patch and are not regressions from this change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reconciliation after review handoff by projecting completed step results onto the legacy checklist when reaching the merge column. * Prevented tasks from being marked approved with incomplete step counts (including “0/N” style states). * Reduced unnecessary merge failures and deadlock/pause scenarios when merge-column progress was already recorded. * **Tests** * Added coverage for execute-and-merge workflows, ensuring merge-boundary resolution updates pending steps without moving the task. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
0e3d2a2265 |
refactor: delete meta-task auto-archive and automated recovery follow-ups (#2461)
Deletes two pieces of automated "meta" machinery that filed and garbage-collected cards restating state already on the task that failed. Net **-1015 lines**. ## Why **Automated recovery follow-ups.** `createAutomatedFollowup` and its dedup engine (289 lines of signature matching, 1h recurrence rate-limiting, 24h supersedes windows) existed to file recovery cards for verification-cap and merge-conflict give-ups. In both cases the parent is *already* parked `failed` with a descriptive `error` and a log entry carrying the failing command, branch, and output — the card was a second copy of that. **Meta-task auto-archive.** The sweeps that garbage-collected those cards were worse than redundant: the regex classifier matched ordinary feature work, and its positional fallback bound cards to unrelated tasks, so **live work could be archived**. They are removed together, because the auto-archive sweeps only existed to clean up after the follow-up engine. ## What changed ### Deleted - `packages/engine/src/verification-followup-dedup.ts` in full — `createAutomatedFollowup`, `decideAutomatedFollowup`, `AutomatedFollowupKind`, `computeVerificationFailureSignature`, `extractFailingTestFiles`. - `findActiveRecoveryFollowUp` — dead code, defined and never called (`tsc` independently flagged it `6133 declared but its value is never read`). - The meta-task auto-archive sweeps `autoArchiveResolvedMetaTasks` / `autoArchiveStalledMetaTasks` and helpers `classifyMetaTask` / `resolveMetaTargetTaskId` / `computeMetaChainDepth` / `archiveMetaTask` / `evaluateMetaAutoArchiveGuards`, plus settings `metaTaskStallAutoCloseMs` and `metaTaskActiveExecutionGraceMs`. - Run-audit types `task:auto-archived-meta-resolved`, `task:auto-archived-meta-stalled`, `task:auto-archive-meta-resolved-skipped`, `task:auto-archive-meta-stalled-skipped`, `verification:followup-created`, `verification:followup-deduped`. The two signature helpers were **deleted rather than relocated** — once the three call sites went they were provably unreachable: `buildVerificationFailureSignature` had exactly one caller, and it was the only caller of `extractFailingTestFiles`. ### Call sites 1 and 2 — park kept, card dropped Verification-cap and merge-conflict give-ups keep their park, audit event, operator comment, and log entry. Site 1's `error` string was reworded off `"See follow-up task for investigation."` (no follow-up will exist) to carry the guidance itself. `autoResolveDisabled` was **kept** — it still drives the outer park guard and the `reason` string; only the inner branch that guarded card creation is gone. ### Call site 3 — autostash orphan, replaced not deleted This one is a genuine data-loss guard, so it keeps a durable trail. A `live`-classified orphan is a merger stash holding **real uncommitted work**, and unlike sites 1–2 there is no parked parent — the parent may already be `done` and merged, so nothing else on the board would ever mention the stash. The card is replaced by a `logEntry` **and** an `addTaskComment` on the parent, preserving every fact the old description carried: the sha, `record.label` (the handle `git stash` recovery needs), `record.detectedByTaskId`, and `sourcePhase`. New truthful run-audit event `task:autostash-orphan-live-detected` replaces the borrowed `verification:followup-*` name, with ids/outcomes-only metadata per AGENTS.md. ### Kept unchanged: the two real product features Eval follow-ups (`eval-followups.ts`) and PR-comment follow-ups (`pr-comment-handler.ts`) only borrowed the shared engine for its dedup pass. Both keep their exact behavior, column, priority, `sourceType`, and log lines, with dedup inlined as a `listTasks` scan on `suggestionId` / `prNumber` respectively. Both fail open (create) if the listing throws, matching the old engine. ## Test changes — read this one Two tests asserted the *deleted* engine's rate-limited `"[verification recurrence]"` logEntry. Those assertions were removed, **not loosened**: both tests still assert no duplicate card is created, and the eval test still asserts the existing id is reported back. No coverage of surviving behavior was weakened. The three `meta-*` test files were deleted along with the sweeps they covered. ## Verification ``` $ pnpm test:gate Test Files 2 passed (2) Tests 10 passed (10) # core Test Files 16 passed (16) Tests 299 passed (299) # engine-core Test Files 1 passed (1) Tests 70 passed (70) # ci-shape GATE_EXIT=0 $ pnpm --filter @fusion/engine --filter @fusion/core exec tsc --noEmit -p tsconfig.json TSC_EXIT=0 (no output) ``` Plus a file-scoped run over the touched surfaces (`eval-followups`, `pr-comment-handler`, `merger-autostash-orphan-surface`, `merger-autostash-cleanup`, `run-audit`, `run-audit-secret-taxonomy`, `project-engine`, `project-engine-manager`): **213/213 passed**. A repo-wide grep confirms no surviving references to any deleted symbol, module, or audit event. 🤖 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** * Failed tasks now retain recovery and verification details directly on the original task instead of generating separate follow-up cards. * Live autostash issues now preserve stash information in task comments and activity logs. * Existing evaluation and pull-request follow-ups continue to be reused when appropriate. * **Changes** * Removed automatic archival of meta-tasks. * Removed obsolete meta-task timing settings. * **Documentation** * Updated architecture and settings documentation to reflect these workflow changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Phil Larson
|
232e17d1cb |
test(engine): complete runtime logger mock (#2458)
## Summary - add the missing `debug` method to the runtime-resolution logger mock - prevent logger calls from short-circuiting runtime selection assertions ## Test plan - `pnpm --filter @fusion/engine exec vitest run src/__tests__/runtime-resolution.test.ts` - `pnpm --filter @fusion/engine typecheck` - `pnpm check:changesets` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated the runtime-resolution test suite’s mocked logger to also support debug-level messages, alongside existing log, warn, and error handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Phil Larson
|
00778cb10f |
test(engine): refresh shellout allowlist (#2451)
## Summary - refresh the engine synchronous-shellout allowlist after recent self-healing and executor source additions shifted audited call sites - keep the guard's path, primitive, and signature checks unchanged ## Test plan - `corepack pnpm --filter @fusion/engine exec vitest run src/__tests__/engine-no-blocking-shellout.test.ts --silent=passed-only --reporter=dot` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated static validation allowlists for audited synchronous shell command operations so matching stays accurate with the latest call-site locations. * Kept safeguards that prevent unapproved blocking shell commands from passing validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> |
||
|
gsxdsm
|
256c64a7bd |
chore(release): v0.74.0-beta.5
Version bump via changesets. |
||
|
gsxdsm
|
beebd270bd |
fix: make Queued to plan / Ready badges agree with the planning lane
TaskCard inferred "unplanned" from steps.length === 0 while triage's todo-discovery and the scheduler's dispatch filter both decide from PROMPT.md seed-ness, so the badges disagreed with the engine in both directions: a real spec that parsed to zero steps read as "Queued to plan" while the scheduler already treated it as a WIP-slot candidate, and a re-seeded card still carrying old steps read as "Ready" while triage was about to plan it. Either way the badge sent operators to the wrong cap. Adds the shared isTaskAwaitingPlanning predicate (replan park, missing spec, seed-vs-real content) used by both triage's discovery and a new best-effort `awaitingPlanning` enrichment on GET /api/tasks. TaskCard derives both badges from that one value — strict complements — and keeps the step count only as a fallback for SSE payloads and older servers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
5ea98f7d4b |
fix: release admission claims when triage evicts a hung planner
evictStaleProcessing cleared `processing` but left the task in `coordinatorAdmittedTaskIds`, which is only cleared by specifyTask's finally — the path a hung promise never reaches. The card stayed eligible (so the throttle branch never logged or emitted `task:plan-admission-throttled`) while admitOldest's refresh filtered it out, leaving it on the "Queued to plan" badge with free slots and no diagnostic until engine restart. Also drop an untransferred pre-held host slot, which otherwise waits out the 600s stale-excess valve. Regression tests assert the invariant on the real production candidate source: an evicted card is re-offered and its host slot returned, while a still-live stale task keeps both claims. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
0022621d22 |
chore(release): v0.74.0-beta.4
Version bump via changesets. |
||
|
gsxdsm
|
2bb8537352 |
FN-8616: make agent tool-output limits configurable
Expose the shared agent tool-output budget as a scoped operator setting with an explicit no-limit option. - Resolve global and project output caps with a safe finite default and zero sentinel. - Propagate configured budgets through PI and plugin runtime tool wrappers. - Add settings controls, localized labels, documentation, and regression coverage. Files changed: .changeset/fn-8616-tool-output-budget-setting.md | 7 ++++ docs/agents.md | 4 +- docs/settings-reference.md | 1 + .../core/src/__tests__/tool-output-budget.test.ts | 23 ++++++++--- packages/core/src/index.gate.ts | 2 + packages/core/src/index.ts | 2 + packages/core/src/settings-schema.ts | 12 ++++++ packages/core/src/tool-output-budget.ts | 31 +++++++++++++-- packages/core/src/types/settings-scope.ts | 8 ++++ .../app/components/settings/save-split.ts | 1 + .../sections/GlobalGeneralSection.search.ts | 20 ++++++++++ .../settings/sections/GlobalGeneralSection.tsx | 26 ++++++++++++ ...lobalGeneralSection.tool-output-budget.test.tsx | 46 ++++++++++++++++++++++ .../settings-default-descriptions.test.tsx | 1 + .../src/__tests__/agent-session-helpers.test.ts | 20 ++++++++++ .../src/__tests__/runtime-resolution.test.ts | 15 +++++++ .../__tests__/tool-output-budget-wrapper.test.ts | 45 ++++++++++++++++----- packages/engine/src/agent-runtime.ts | 2 + packages/engine/src/agent-session-helpers.ts | 18 +++++++-- packages/engine/src/pi.ts | 29 ++++++++++---- packages/engine/src/runtime-resolution.ts | 10 ++++- packages/i18n/locales/en/app.json | 4 ++ packages/i18n/locales/es/app.json | 6 ++- packages/i18n/locales/fr/app.json | 6 ++- packages/i18n/locales/ko/app.json | 6 ++- packages/i18n/locales/zh-CN/app.json | 6 ++- packages/i18n/locales/zh-TW/app.json | 6 ++- packages/i18n/src/resources.d.ts | 4 ++ 28 files changed, 323 insertions(+), 38 deletions(-) Fusion-Task-Id: FN-8616 Fusion-Task-Lineage: 3ca99a61-d6ae-48ff-98d2-f14a153aa2b7 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai> |
||
|
gsxdsm
|
30f81ac0cd |
fix(engine): compact verification failure output
Keep successful verification responses quiet and return bounded, high-signal diagnostics for failures without hiding zero-work or green-while-red warnings. |
||
|
gsxdsm
|
07c8c95b10 |
FN-8614: cap agent tool output
Bound every engine-injected tool result to preserve agent context capacity. - Add shared 16,000-character total text budgets with deterministic truncation markers and validated overrides. - Apply outermost output clamps to Pi and non-Pi plugin tool paths, with semantic caps for high-volume reads. - Cover budget behavior and document the operator-facing configuration contract. Files changed: .changeset/fn-8614-tool-output-budget.md | 7 ++ docs/agents.md | 8 ++ .../core/src/__tests__/tool-output-budget.test.ts | 58 +++++++++++++ packages/core/src/index.gate.ts | 7 ++ packages/core/src/index.ts | 7 ++ packages/core/src/tool-output-budget.ts | 97 ++++++++++++++++++++++ .../src/__tests__/agent-artifact-tools.test.ts | 10 +++ .../src/__tests__/agent-document-tools.test.ts | 10 +++ .../__tests__/agent-task-logs-read-tools.test.ts | 8 ++ .../__tests__/tool-output-budget-wrapper.test.ts | 67 +++++++++++++++ packages/engine/src/agent-session-helpers.ts | 7 +- packages/engine/src/agent-tools.ts | 43 ++++++++-- packages/engine/src/pi.ts | 54 +++++++++++- 13 files changed, 374 insertions(+), 9 deletions(-) Fusion-Task-Id: FN-8614 Fusion-Task-Lineage: b6a76ccd-d7b4-4b43-af7e-cfd16ffb7fc8 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai> |
||
|
gsxdsm
|
15a2fb18cc |
Merge branch 'fix/incomplete-pg-ports'
Wire incomplete PostgreSQL ports for archive, reconcile, health, settings cache, agent cache, and async prompt overrides. |
||
|
gsxdsm
|
ab87d0d803 |
fix(api): return 404 for missing tasks, and make task deletions attributable
Three related fixes, all originating from a `[api:error] Request failed` log line showing a 500 on `GET /api/tasks/FN-8610/runtime-fallback`. 1. Missing/deleted tasks now return 404 instead of 500. `getTaskImpl` signalled a miss with a bare `Error`, and route catches only mapped errno `ENOENT` to 404 — a leftover from the file-backed storage era. In Postgres mode nothing sets an errno code, so every unknown/missing/soft-deleted/wrong-project read returned 500. Adds a typed `TaskNotFoundError` (message byte-identical) plus a shared `task-lookup-error` mapper applied across the task, session-diff, git/GitHub, workflow and file-workspace route registrars. The same bare throw existed on both archive-lifecycle delete paths, so `DELETE /tasks/:id` was affected too. 2. 5xx logs now carry the origin stack. `rethrowAsApiError` constructed a fresh `ApiError` from the message and discarded the original, so the `FNXC:ApiErrorDiagnostics` contract logged the rethrow site rather than the throw site — the reported log entry had no stack at all. Threads `cause` through the error factories and walks the chain (bounded, cycle-guarded). 3. Task deletions are attributable, and non-operator deletes notify. `task:deleted` audit rows recorded `agentId: "system"` for every HTTP delete, making an operator click indistinguishable from a script or an agent; the calling agent's task id was accepted by the store and then never persisted. Adds a `callerKind` union recorded in audit metadata, tags every delete call site, and stamps a self-reported `x-fusion-client` header from the dashboard client. When the caller is `agent-tool` or `api-unattributed`, a best-effort notice is sent to the operator mailbox; operator and engine deletes stay silent. `x-fusion-client` is attribution, not authentication — anything can send it. No delete-blocking, gating or permission logic is added here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
2b55077546 |
fix: wire incomplete PostgreSQL ports for archive, reconcile, health
Replace empty backendMode stubs with real AsyncDataLayer paths: archive ID reservation and isTaskArchivedAsync, orphaned task.json re-import, health snapshots via checkPostgresHealth, settings/agent memory caches for sync readers, async builtin prompt overrides, and self-healing audit/health callers that previously used dead sync SQLite fallbacks. |
||
|
gsxdsm
|
f01461a70e |
feat(engine): thread the node id onto review-gate leases, activating pre-boot reclaim
Completes
|
||
|
gsxdsm
|
3b83282273 |
feat(engine): attribute review-gate leases to a node so dead local leases reclaim fast
Groundwork for FN-8603's remaining ~14-minute wait. Liveness for a pending review gate is judged purely by a 15-minute staleness floor because a lease records WHO took it (`leaseOwner` = run id) but not WHERE, and under multi-node every engine sees every other engine's leases. A fresh-but-unknown lease might be running on a peer, so the floor was the only safe test -- and a lease left by this node's own crashed process is indistinguishable from it. Adds `WorkflowStepResult.leaseNodeId` plus an optional `LocalNodeLeaseIdentity` argument to `classifyReviewLease`. One narrow new case: a lease stamped with the caller's OWN node id whose `startedAt` predates the caller's process boot is provably dead -- the process that could have owned it is gone -- so it classifies as `reclaim` immediately rather than aging out. Deliberately narrow, because widening it is a double-dispatch risk: absent (legacy) or peer node ids keep the floor, and a lease taken by this process after boot is still adopted. InProcessRuntime.start() resolves the local node id from CentralCore (fail-soft; on error it stays undefined and floor-only semantics apply) and passes it to SelfHealingManager. The graph executor stamps the field when deps.localNodeId is set. NOT YET WIRED, so this is inert in production and behavior is unchanged end to end: `localNodeId` is not threaded from WorkflowGraphTaskRunner / WorkflowTaskRuntime down into the executor deps, so no lease actually carries a `leaseNodeId` yet. The reader is ready; the writer needs that pass-through (WorkflowGraphTaskRunnerDeps gains the field, the runner forwards it, and the runtime supplies this.localNodeId). Stopping here rather than half-threading it. Verified: tsc clean on core and engine, pnpm lint clean, pnpm test:gate green (299 + 70), core workflow-step-results suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
00011b0113 |
fix(engine): recover restart-orphaned review steps in one cycle, raise fix budget
FN-8603 sat in-review for ~36 minutes after an engine restart killed its Code Review session 34 seconds in. It did recover on its own; the cost was latency, not a terminal park. Sweep ordering. reconcile-orphaned-pending-step-results PRODUCES the failed results that recover-failed-pre-merge-steps CONSUMES, but in the periodic maintenance list it ran ~15 entries after it. A step orphaned in cycle N was therefore rewritten to failed only after recovery had already scanned, so nothing re-ran it until cycle N+1. Moved it immediately before its consumer and removed the now-duplicated later entry. Startup recovery already ordered the two correctly. Post-review fix budget. Default raised 3 -> 10 per operator request. Three passes is below the observed convergence length for the gates this fallback actually governs -- Browser Verification and custom optional gates -- since Plan Review and Code Review already resolve to "unbounded" when unset, and exhausting the budget parks the card for a human. The declaration default and five inline `settings.maxPostReviewFixes ?? 3` call sites in executor.ts/self-healing.ts had drifted into separate literals, so raising one alone would have left every unset-settings path on the old value; they now share the exported DEFAULT_MAX_POST_REVIEW_FIXES. Not done, and why. Re-dispatching a restart-orphaned lease immediately at startup is the change that would close the remaining ~14-minute wait, but it is unsound as specified: liveness is judged by a 15-minute lease-staleness floor because leases carry no node attribution, so treating a pre-boot lease as dead would let one node orphan another node's genuinely running review. Needs a node id on the lease record first. Left the floor intact. Verified: tsc clean on core and engine, pnpm lint clean, pnpm test:gate green, self-healing orphaned-pending-step-results and optional-step-revision suites green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
cca13737b6 |
FN-8603: reduce steady-state diagnostic log noise
Route routine core, engine, and dashboard diagnostics through debug-gated shared loggers. - Demote steady-state diagnostic sites while preserving warnings and errors for actionable failures. - Add cross-package severity contracts and manifest coverage for demoted log sites. - Document logging severity guidance and add a patch changeset. Files changed: .changeset/fn-8603-log-severity.md | 7 ++ docs/diagnostics.md | 20 ++++-- .../__tests__/log-severity-spam-contract.test.ts | 71 ++++++++++++++++++ packages/core/src/activity-analytics.ts | 5 +- packages/core/src/ai-summarize.ts | 61 +++++++--------- packages/core/src/async-mission-store.ts | 5 +- packages/core/src/async-secrets-store.ts | 7 +- packages/core/src/central-core.ts | 17 ++--- packages/core/src/docker-provisioning.ts | 13 ++-- packages/core/src/index.ts | 1 + packages/core/src/master-key.ts | 9 ++- packages/core/src/memory-compaction.ts | 29 ++++---- packages/core/src/memory-insights.ts | 7 +- packages/core/src/migration-orchestrator.ts | 7 +- packages/core/src/mission-store.ts | 5 +- packages/core/src/node-discovery.ts | 7 +- packages/core/src/notification/dispatcher.ts | 9 ++- .../core/src/plugins/bundled-plugin-install.ts | 11 +-- packages/core/src/reflection-store.ts | 5 +- packages/core/src/secrets-store.ts | 7 +- packages/core/src/task-store/agent-logs.ts | 21 +++--- packages/core/src/task-store/async-events.ts | 5 +- packages/core/src/task-store/async-maintenance.ts | 7 +- packages/core/src/task-store/comments-ops.ts | 7 +- packages/core/src/task-store/task-mutation-ops.ts | 11 +-- packages/core/src/task-store/workflow-integrity.ts | 9 ++- packages/core/src/types/merge-policy.ts | 5 +- packages/core/src/usage-events.ts | 5 +- .../__tests__/log-severity-spam-contract.test.ts | 48 +++++++++++++ packages/dashboard/src/ai-refine.ts | 5 +- packages/dashboard/src/ai-session-diagnostics.ts | 10 +-- packages/dashboard/src/chat.ts | 8 ++- packages/dashboard/src/devserver-manager.ts | 9 ++- packages/dashboard/src/file-service.ts | 5 +- packages/dashboard/src/github-tracking-comments.ts | 7 +- .../dashboard/src/github-tracking-reconciler.ts | 5 +- packages/dashboard/src/github-tracking-state.ts | 5 +- packages/dashboard/src/gitlab-lifecycle.ts | 5 +- packages/dashboard/src/insights-routes.ts | 9 ++- packages/dashboard/src/issue-image-attachments.ts | 5 +- packages/dashboard/src/knowledge-index.ts | 5 +- packages/dashboard/src/plugin-routes.ts | 7 +- packages/dashboard/src/routes/board-workflows.ts | 5 +- packages/dashboard/src/routes/context.ts | 5 +- .../dashboard/src/routes/register-auth-routes.ts | 13 ++-- .../routes/register-docker-provisioning-routes.ts | 7 +- .../dashboard/src/routes/register-git-github.ts | 21 +++--- packages/dashboard/src/routes/register-gitlab.ts | 7 +- .../src/routes/register-session-diff-routes.ts | 9 ++- .../src/routes/register-settings-memory-routes.ts | 7 +- .../src/routes/register-setup-activity-routes.ts | 7 +- .../dashboard/src/routes/register-signal-routes.ts | 5 +- .../src/routes/register-task-workflow-routes.ts | 11 +-- packages/dashboard/src/runtime-logger.ts | 11 +-- packages/dashboard/src/server.ts | 7 +- packages/dashboard/src/sse.ts | 8 ++- packages/dashboard/src/terminal-service.ts | 34 ++++----- packages/dashboard/src/view-chunk-manifest.ts | 5 +- .../engine/src/__tests__/log-severity-manifest.ts | 83 ++++++++++++++++++++++ .../__tests__/log-severity-spam-contract.test.ts | 40 ++++++++++- .../src/__tests__/logger-debug-gating.test.ts | 7 +- packages/engine/src/goal-anchoring-audit.ts | 5 +- packages/engine/src/plugin-runner.ts | 44 ++++++------ packages/engine/src/pty-native.ts | 9 ++- .../engine/src/runtimes/child-process-worker.ts | 4 +- packages/engine/src/self-healing.ts | 12 ++-- packages/engine/src/worktree-hooks.ts | 10 ++- 67 files changed, 632 insertions(+), 250 deletions(-) Fusion-Task-Id: FN-8603 Fusion-Task-Lineage: 53901db6-1af2-4bd7-b5ea-49507e048ef2 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai> |