root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tests): 4 engine reds on main — each was green for a reason the fleet removed (#2778)

Browse Source 
## Context

A full `@fusion/engine` run on `origin/main` (`9b61d795c9`) reports **39
failures / 10835 passed**. 32 are the notifier harness, fixed in #2776.
This PR takes 4 of the remaining 7.

All three files share one shape: **each case was passing off something
the lifecycle conversions have since correctly taken away.** In every
one, the product is fine and a good change landed as a red test.

---

### 1. `executor-graph-failure-lanes-resolved.ts` — an equality that
fails on its own fix

The guard forbids resolving a lifecycle *guard* through the synchronous
`resolvePlannerLanes` (a no-op under the shipped PostgreSQL backend, so
the census counts the site as converted while it behaves like the
literal). It asserted `expect(callSites).toBe(3)`.

#2764 converted the promotion-path site to
`resolvePlannerLanesForTaskAsync` — exactly the direction this guard
wants. Count went **3 → 2** and the assertion failed.

The guard's own comment states the invariant as *"Any FOURTH is a new
sync resolution"* — one-directional. Coded as equality, it fails on
removal, which is the change it exists to encourage. Now
`toBeLessThanOrEqual(2)`.

**Mutation:** adding a third sync call site → `expected 3 to be less
than or equal to 2`. Still load-bearing.

### 2. `restart.integration.test.ts` — a fixture matching a fallback
constant

`recoverCompletedTask` re-homes intake → hold → wip only when the origin
is the board's **intake** lane; otherwise it hands straight to review.
The failure showed the 1st move as `in-review` with no re-home.

Nothing regressed. The fixture put the card in `triage` and resolved
lanes through the sync resolver, so it fell through to
`LEGACY_PLANNER_LANES` — where `intake` is literally `"triage"`. **It
was matching a hardcoded fallback, not a declared lane.** #2764 made the
site await the real resolver; the mock selects `builtin:coding`, and
**U11 merged intake and hold onto one Planning column (`todo`)**, so
`triage` is not a lane on that board and the two-hop correctly
collapses.

The invariant the test is named for — completed work in a distinct
intake lane is re-homed along a legal path, not moved intake → review,
which role adjacency rejects — is still real. So the fixture now
**declares** a board with intake separate from hold, the only shape
where the two-hop is reachable.

**Mutation:** removing the re-home hop from the product → fails with the
expected `todo` first-move. Load-bearing.

### 3. `executor-abort-provenance.test.ts` — a call one argument short

Both provenance cases returned `false` for a clean completed in-review
row. This reads as an FN-6796 regression stranding rows that are already
handed off for review.

It is not. #2703 added a 7th `reviewLane` parameter so the lane is
resolved by the caller. **The call goes through `as any`, so the missing
argument was not a type error** — it arrived `undefined`, `live.column
!== reviewLane` held for every row, and the classifier answered false
for everything.

Passed explicitly rather than defaulted inside the classifier: a default
would restore the literal the parameter exists to remove. Added a
**differential** — a card resting in a *renamed* review lane classifies
the same, a mismatched one does not — so the parameter cannot be
re-literalized while still looking converted.

**Mutation:** `live.column !== "in-review"` → the differential fails.
The other cases pass, which is precisely why it was worth adding.

---

## Evidence

| file | result |
|---|---|
| `executor-graph-failure-lanes-resolved` | **24 passed** |
| `restart.integration` | **48 passed** |
| `executor-abort-provenance` | **16 passed** |

Gate **732 green** · `pnpm lint` clean · engine `tsc --noEmit` **0
errors**. Test-only — no product file is touched by this PR (the
mutations above were run and reverted; `git diff` confirms clean).

## Deliberately NOT fixed here

3 cases in `executor-prompt.test.ts` ("global pause behavior") remain
red on main: **a user-paused todo task now reaches `createFnAgent`**.
That is a safety invariant rather than a stale fixture, and neither
`executeCore` nor the graph executor holds a pause gate — the refusal
#2371 documented is not where its note implies. It gets its own change;
editing the fixture to match current behaviour would hide it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 09:40:49 -07:00
committed by GitHub
parent 9da2674fa0
commit 698bded476
3 changed files with 93 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
packages/engine/src/__tests__/executor-abort-provenance.test.ts
View File

@@ -256,10 +256,51 @@ describe("pause-abort provenance truthfulness (KB-PROV)", () => {
provenance,
true,
false,
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-21:45:
THE REVIEW LANE, which this call was silently omitting.
#2703 added a seventh parameter so the lane is resolved by the caller instead of through the
sync resolver (a no-op under the shipped backend). The call goes through `as any`, so the
missing argument was not a type error — it arrived `undefined`, `live.column !== reviewLane`
was true for every row, and the classifier returned false for BOTH provenances. That reads as
"FN-6796 regressed and clean in-review rows are being stranded again" when the product is
fine and the call is short one argument.
Passed explicitly rather than defaulted inside the classifier: a default would restore the
literal this parameter exists to remove.
*/
"in-review",
);
expect(benign).toBe(true);
},
);
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-21:45:
The lane must be USED, not merely accepted. Without this, passing the argument could be reverted to
a hardcoded "in-review" inside the classifier and every assertion above would still pass — the
parameter would look converted while behaving like the literal, which is the exact failure the
census counts as a win. A card resting in a RENAMED review lane is the differential.
*/
it("honours the caller's review lane: a renamed lane classifies the same, a mismatched one does not", () => {
const { executor } = makeExecutor();
const result = { disposition: "failed", outcome: "failure", visitedNodeIds: ["plan", "execute"], context: {} };
const classify = (column: string, reviewLane: string) =>
(executor as any).isBenignInReviewPauseAbort(
makeTask({ column, steps: [{ name: "Implement", status: "done" }] }),
result,
"engine-abort",
true,
false,
reviewLane,
);
// Resting in the board's declared review lane, whatever it is called.
expect(classify("validating", "validating")).toBe(true);
// Resting somewhere else: not a benign in-review pause-abort.
expect(classify("validating", "in-review")).toBe(false);
});
});
});

17
packages/engine/src/__tests__/executor-graph-failure-lanes-resolved.test.ts
View File

@@ -393,11 +393,20 @@ describe("no lifecycle GUARD resolves its lane synchronously", () => {
const callSites = [...code.matchAll(/resolvePlannerLanes\s*\(/g)].length;
/*
Three call sites are the move/promotion destinations documented above (two in the planner-column
helpers, one in the promotion path), plus the import. Any FOURTH is a new sync resolution and must be
justified — if it is a guard, it is a no-op on PostgreSQL and the census will claim it is converted.
A CEILING, not an equality.
The invariant this guard exists for is one-directional: no NEW synchronous resolution may appear.
Removing one is always safe — it is the fix this test is trying to encourage — so an exact count
fails on exactly the change it wants. That is what happened: #2764 converted the promotion-path
site to `resolvePlannerLanesForTaskAsync`, the count went 3 -> 2, and a correct improvement
landed as a red test on main with nothing wrong in the product.
What remains are the two planner-column move destinations documented above (`PlannerLanes` exists
so a caller refuses rather than inventing a column), which is a different question from "which
lane is this card in". A THIRD is a new sync resolution and must be justified — if it is a guard,
it is a no-op on PostgreSQL and the census will claim it is converted.
*/
expect(callSites).toBe(3);
expect(callSites).toBeLessThanOrEqual(2);
});
it("does not compare a column against a synchronously-resolved lane on the same line", async () => {

39
packages/engine/src/__tests__/restart.integration.test.ts
View File

@@ -1101,8 +1101,47 @@ describe("In-progress task resume after restart", () => {
}));
});
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-21:30:
THIS FIXTURE NOW DECLARES THE BOARD IT DEPENDS ON, instead of inheriting a lane by accident.
It went red on main after #2764 with the FIRST move landing as `in-review` and no re-home at all.
Nothing regressed in the product — the test had been passing for the wrong reason. The card sits
in `triage`, and recovery only re-homes when the origin is the board's INTAKE lane. It used to
resolve lanes through the SYNC `resolvePlannerLanes`, whose selection reader is a no-op under the
shipped backend, so it fell through to LEGACY_PLANNER_LANES — where `intake` is literally
"triage". The fixture was therefore matching a hardcoded fallback constant, not a declared lane.
#2764 correctly made the site await the real resolver. The mock selects `builtin:coding`, and
U11 MERGED intake and hold onto one Planning column (`todo`), so post-U11 `triage` is not a lane
on that board at all and the two-hop correctly collapses.
The invariant this test is NAMED for is still real and still worth pinning: on a board with a
DISTINCT intake lane, completed work stranded there is re-homed along a legal path rather than
moved intake -> review, which role adjacency rejects. So the workflow is declared explicitly here
with intake separate from hold. That is the only shape under which the two-hop is reachable, and
saying so in the fixture means the next vocabulary change fails loudly instead of silently
selecting a different code path.
*/
it("recoverCompletedTask() legally re-homes a completed triage zombie before review handoff", async () => {
/* Distinct intake ("triage") and hold ("todo") — pre-U11 / custom-lineage shape. */
const distinctIntakeIr = {
version: "v2",
id: "custom:distinct-intake",
nodes: [],
edges: [],
columns: [
{ id: "triage", label: "Triage", traits: [{ trait: "intake" }] },
{ id: "todo", label: "Planning", traits: [{ trait: "hold", config: { release: "capacity" } }] },
{ id: "in-progress", label: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
{ id: "in-review", label: "Review", traits: [{ trait: "humanReview" }, { trait: "mergeBlocker" }] },
{ id: "done", label: "Done", traits: [{ trait: "complete" }] },
],
};
const store = createMockStore();
store.getTaskWorkflowSelectionAsync = vi.fn().mockResolvedValue({ workflowId: "custom:distinct-intake", stepIds: [] });
store.getTaskWorkflowSelection = vi.fn().mockReturnValue({ workflowId: "custom:distinct-intake", stepIds: [] });
store.getWorkflowDefinition = vi.fn().mockResolvedValue({ ir: distinctIntakeIr });
const task = makeTask("FN-TRIAGE-ZOMBIE", "triage", {
worktree: "/tmp/wt/FN-TRIAGE-ZOMBIE",
steps: makeSteps("done"),