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

test(engine): live-PG differential between resolvePlannerLanes and its async twin (#2791)

Browse Source 
## What

One new live-PostgreSQL E2E suite, 5 tests. **No production file is
touched** — evidence, per the E2E worker's remit. Follows #2789 (same
defect class, different site).


`packages/engine/src/__tests__/workflow-planner-lanes-sync-vs-async-live-e2e.pg.test.ts`

## The finding

`replan-target.ts` exports two functions with identical logic and
identical fallbacks, differing only in how they obtain the task's IR:

```
resolvePlannerLanes(store, taskId)              -> store.resolveTaskWorkflowIrSync(taskId)
resolvePlannerLanesForTaskAsync(store, taskId)  -> await resolveWorkflowIrForTask(store, taskId)
```

Under PostgreSQL the sync selection reader answers `undefined` for every
task, so the sync twin resolves the **default** workflow for every card
regardless of the board it is on. **Nine production call sites use it**
(2 × `executor.ts`, 7 × `triage.ts`); one uses the async twin.

The module's own doc comment argues this, and the store-level fact is
proven in `sync-workflow-ir-is-always-default.pg.test.ts`. What had no
executable evidence is the consequence **at this seam** against a real
store with a real persisted workflow. That is this file — a pure
differential: both twins, same store, same task, same call.

### Two harms, different severity

1. **Wrong lanes, labelled authoritative.** `resolvedFromWorkflow`
exists to tell a caller "these came from the workflow, not the
fallback". The sync twin sets it `true` — an IR did come back — while
handing over the default board's ids. A caller that correctly checks the
flag before trusting the lanes is misled *precisely by checking it*,
which is strictly worse than the honest `false` an unresolvable store
would give.
2. **Invented forward lanes.** `wip`/`review`/`complete` are optional so
a caller *refuses* rather than moving a card into a column the board
does not declare (PR #2628's review). The sync twin defeats that
contract without touching it: never having seen the real board, it
reports the default board's forward lanes as present. The optionality is
intact in the type and unreachable in practice. The sharpest arm: a
board declaring **no** review lane gets `undefined` from the async twin
and `"in-review"` from the sync twin.

### A correction worth carrying forward

"It falls back to the legacy lanes" is the wrong mental model **twice
over**. `LEGACY_PLANNER_LANES` (`hold: "todo", intake: "triage"`) is
reached only when no IR resolves at all — under PostgreSQL, never. What
a caller actually receives is the **post-U11 merged default**, whose
intake and hold are one `todo` lane. So the sync twin does not return
`triage` for intake; it returns `todo`, and a caller reading `intake`
gets not merely a wrong id but a lane that is not a dedicated intake at
all.

This is also why the control arm uses `MERGED_VOCAB`: the shape the
twins agree on is the merged one. `DEFAULT_VOCAB`, which splits intake
out as `triage`, already separates them.

## Evidence discipline

- **Observed state.** These are exported pure functions over a live
store; the observation is their return value against a persisted
workflow definition. No spies, no mock IR anywhere in the file. Contrast
the unit coverage in `planner-lanes-async-resolution.test.ts`, which
must supply a mock `resolveTaskWorkflowIrSync` and therefore cannot see
this divergence at all.
- **Control arm.** On the post-U11 default shape the twins agree exactly
— which is why this survived: every default-board test passes and only a
renamed board separates them.
- **Characterization, not endorsement.** The four renamed arms assert
the wrong-but-current values deliberately; they flip when the call sites
move to the async twin, and that flip is the point.

### Mutation-verified, including a round that found weak arms

Replacing the sync twin's whole body with `return LEGACY_PLANNER_LANES`:

| | arms failing |
|---|---|
| first draft | **3 of 5** |
| after strengthening | **5 of 5** |

Two arms originally asserted only `wip`/`review`/`complete`, which are
identical in the merged default IR and in `LEGACY_PLANNER_LANES` — so
they proved the lanes were wrong without proving *why*, and survived the
mutation. Each now also pins `intake` (`todo` merged vs `triage`
legacy), the single field that separates "resolved the wrong board" from
"took the fallback". Recorded in the file next to the assertions.

## Not done, and why

**No fix.** The async twin already exists and is documented as a drop-in
("identical logic and identical fallbacks — the ONLY difference is
awaiting the authoritative resolver"), so the migration is mechanical
*where the caller is already async*. It is not universally so: several
`triage.ts` sites are inside synchronous paths, and `triage.ts:831`
calls `resolvePlannerLanes(this.store, "")` with an empty task id — a
sweep-wide lane read that has no single task to resolve against and
needs a decision, not a mechanical swap. Both are behaviour calls in
files another worker owns; flagging, not smuggling.

## Verification

- new suite — **5/5 passed**, mutation-verified 5/5
- full live-PG E2E surface, 20 suites — **126/126 passed** (was 121/121)
- `pnpm lint` — clean
- `pnpm check:lifecycle-columns` — exit 0

Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is
reachable, so the merge gate is unaffected. Throwaway per-file database;
never port 4040.

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


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Added end-to-end coverage comparing synchronous and asynchronous
workflow lane resolution.
* Validated lane consistency for renamed, non-default, and custom boards
using persisted workflow data.
* Added checks for incorrect fallback lanes, workflow resolution
indicators, and absent review lanes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 10:32:24 -07:00
committed by GitHub
parent 87442b9664
commit dcf9900d61
1 changed files with 219 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

219
packages/engine/src/__tests__/workflow-planner-lanes-sync-vs-async-live-e2e.pg.test.ts Normal file
View File

@@ -0,0 +1,219 @@
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-21:05 (E2E evidence — `resolvePlannerLanes` vs its async twin):
`replan-target.ts` exports two functions with identical logic and identical fallbacks, differing only
in HOW they get the task's IR:
resolvePlannerLanes(store, taskId) -> store.resolveTaskWorkflowIrSync(taskId)
resolvePlannerLanesForTaskAsync(store, taskId) -> await resolveWorkflowIrForTask(store, taskId)
Under PostgreSQL the sync selection reader answers `undefined` for every task, so the sync twin
resolves the DEFAULT workflow for every card no matter what board it is on. NINE production call
sites use it (two in `executor.ts`, seven in `triage.ts`); one uses the async twin.
That is already argued in the module's own doc comment and proven at the store level in
`core/.../postgres/sync-workflow-ir-is-always-default.pg.test.ts`. What had no executable evidence is
the CONSEQUENCE at this seam, against a real store with a real stored workflow — which is what this
file is. It is a pure differential: both twins, same store, same task, same call.
TWO DISTINCT HARMS, and they are not the same severity.
1. WRONG LANES, REPORTED AS RIGHT. `resolvedFromWorkflow` exists precisely to tell a caller "these
came from the workflow, not from the legacy fallback". The sync twin sets it `true` — an IR did
come back — while handing over the DEFAULT board's ids. A caller branching on that flag is not merely
given a wrong answer, it is given a wrong answer LABELLED AUTHORITATIVE, which is strictly worse
than the `resolvedFromWorkflow: false` it would get from an unresolvable store.
2. AN INVENTED FORWARD LANE. `wip`/`review`/`complete` are deliberately optional: PR #2628's review
established that a missing role must stay `undefined` so the caller REFUSES, because
substituting the legacy id makes it `moveTask(..., "in-progress")` into a column the board does
not declare — rejected, and the card can be left half-moved. The sync twin defeats that contract
without touching it: it never sees the real board, so it reports the DEFAULT board's forward
lanes as present. The optionality is intact and unreachable.
WHY A CONTROL ARM. On the post-U11 default shape the two twins AGREE, because the answer the sync twin
always gives happens to be the right one there. That agreement is the reason this survived: every
default-board test passes, and only a renamed board separates them.
Note which shape that is. The sync twin's constant answer is the DEFAULT BUILTIN IR, whose intake and
hold are ONE merged `todo` lane after U11 — NOT the split `todo`/`triage` pair the module's own
`LEGACY_PLANNER_LANES` constant names. That constant is reached only when no IR resolves at all, which
under PostgreSQL is never. So "it falls back to the legacy lanes" is the wrong mental model twice over:
the fallback is unreachable, and the value actually returned is the merged default.
OBSERVED STATE. These are pure exported functions over a live store, so the observation is their
return value against a persisted workflow definition — no spies, and no mock IR anywhere in this
file. Note the contrast with the unit coverage in `planner-lanes-async-resolution.test.ts`, which
must supply a mock `resolveTaskWorkflowIrSync` and therefore cannot see this divergence at all.
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is
unaffected. Throwaway per-file database; never port 4040.
*/
import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest";
import "@fusion/core"; // registers the built-in column traits
import type { TaskStore } from "@fusion/core";
import {
pgDescribe,
createSharedPgTaskStoreTestHarness,
type SharedPgTaskStoreHarness,
} from "../../../core/src/__test-utils__/pg-test-harness.js";
import { resolvePlannerLanes, resolvePlannerLanesForTaskAsync } from "../replan-target.js";
import { MERGED_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
pgDescribe("planner lanes: the sync twin against the async twin, on a live store", () => {
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
prefix: "fusion_planner_twins",
});
beforeAll(h.beforeAll);
afterAll(h.afterAll);
beforeEach(async () => { await h.beforeEach(); });
afterEach(async () => { await h.afterEach(); });
/** Persist a real workflow definition and a task bound to it.
*
* `createWorkflowDefinition` allocates its OWN id and ignores one passed in, so the task binds to
* the id the STORE returned. Binding to the requested id instead silently resolves the default
* builtin IR — a renamed-workflow fixture that passes while testing nothing. */
async function taskOn(
store: TaskStore,
v: Vocabulary,
key: string,
shape: { mergeOrchestration?: boolean } = {},
): Promise<string> {
const created = await store.createWorkflowDefinition({
name: `Twins ${key}`,
kind: "workflow",
ir: lifecycleIr(v, `custom:${key}`, shape),
} as never);
const workflowId = (created as { id: string }).id;
const task = await store.createTask({ description: `twin probe ${key}` });
await store.writeTaskWorkflowSelection(task.id, workflowId, []);
store.taskCache.delete(task.id);
return task.id;
}
it("CONTROL — on the post-U11 DEFAULT shape the two twins agree, which is why this hid", async () => {
/*
MERGED_VOCAB, not DEFAULT_VOCAB. The sync twin's constant answer is the DEFAULT BUILTIN IR, and
after U11 that board has no `triage` column at all — intake and hold are one merged Planning lane
with id `todo`. So the shape the twins agree on is the MERGED one; `DEFAULT_VOCAB`, which splits
intake out as `triage`, already separates them.
Worth stating because the module's own fallback constant still names the split legacy pair
(`hold: "todo", intake: "triage"`). That constant is NOT what the sync twin returns here — it is
only reached when no IR resolves at all. What a caller actually gets is the merged default.
*/
const store = h.store();
/* `mergeOrchestration` because `resolveLifecycleColumns().review` is derived from the
`mergeOrchestration` flag, not from `human-review`. Without the trait the fixture's review
column resolves to NO review lane, and the twins would differ on `review` for a reason that
belongs to the fixture rather than to the subject. (That divergence is real and is asserted
on its own terms in the last case below, where it is the point rather than noise.) */
const taskId = await taskOn(store, MERGED_VOCAB, "wf-merged-twins", { mergeOrchestration: true });
const sync = resolvePlannerLanes(store, taskId);
const async_ = await resolvePlannerLanesForTaskAsync(store, taskId);
expect(sync).toEqual(async_);
expect(async_.hold).toBe(MERGED_VOCAB.hold);
expect(async_.intake).toBe(MERGED_VOCAB.intake);
});
it("CHARACTERIZATION — on a RENAMED board the sync twin returns the DEFAULT-WORKFLOW lanes", async () => {
/*
The async twin resolves the board the card is actually on. The sync twin answers with the ids of
a board this task has nothing to do with — and every one of the nine production call sites is on
the sync side.
*/
const store = h.store();
const taskId = await taskOn(store, RENAMED_VOCAB, "wf-renamed-twins", { mergeOrchestration: true });
const sync = resolvePlannerLanes(store, taskId);
const async_ = await resolvePlannerLanesForTaskAsync(store, taskId);
// What the card's own workflow says.
expect(async_.hold).toBe(RENAMED_VOCAB.hold);
expect(async_.intake).toBe(RENAMED_VOCAB.intake);
expect(async_.wip).toBe(RENAMED_VOCAB.wip);
expect(async_.review).toBe(RENAMED_VOCAB.review);
expect(async_.complete).toBe(RENAMED_VOCAB.complete);
/* What the sync twin says instead: the DEFAULT builtin board's ids, none of which this board
declares. Both lanes read `todo` because that board merged them in U11 — so a caller reading
`intake` gets not just the wrong id but a lane that is not a dedicated intake at all. */
expect(sync.hold).toBe("todo");
expect(sync.intake).toBe("todo");
expect(sync.hold).not.toBe(async_.hold);
expect(sync.intake).not.toBe(async_.intake);
});
it("CHARACTERIZATION — and it labels those default-workflow lanes `resolvedFromWorkflow: true`", async () => {
/*
The sharpest half. `resolvedFromWorkflow: false` would be honest — "no basis to decide, the
fallback names ARE the answer". `true` tells the caller the opposite of the truth, so a caller
that correctly checks the flag before trusting the lanes is misled precisely by checking it.
(Named DEFAULT-WORKFLOW rather than LEGACY throughout, per review: these are the merged builtin
default's ids, and `LEGACY_PLANNER_LANES` is a different shape that is never reached here. The
header already made that distinction; the case titles contradicted it.)
*/
const store = h.store();
const taskId = await taskOn(store, RENAMED_VOCAB, "wf-renamed-flag");
expect(resolvePlannerLanes(store, taskId).resolvedFromWorkflow).toBe(true);
});
it("CHARACTERIZATION — the forward lanes come back populated from the WRONG board", async () => {
/*
`wip`/`review`/`complete` are optional so a caller REFUSES rather than moving a card into a
column the board does not declare. Here every one is populated — with the default board's ids —
so a caller that dutifully checks `if (!lanes.wip) return` sails straight through and attempts
`moveTask(..., "in-progress")` on a board whose WIP column is `building`.
Asserting the wrong-but-current values deliberately; when the call sites move to the async twin
these become the RENAMED ids and this test is what says the behaviour moved.
*/
const store = h.store();
const taskId = await taskOn(store, RENAMED_VOCAB, "wf-renamed-forward", { mergeOrchestration: true });
const sync = resolvePlannerLanes(store, taskId);
expect(sync.wip).toBe("in-progress");
expect(sync.review).toBe("in-review");
expect(sync.complete).toBe("done");
expect(sync.wip).not.toBe(RENAMED_VOCAB.wip);
/* `intake` is the arm's only DISTINGUISHING field, and it is here on purpose. The three lanes
above are identical in the merged default IR and in `LEGACY_PLANNER_LANES`, so without this
the case passes either way — it would prove the lanes are wrong without proving WHY, and a
mutation replacing the whole body with the legacy constant survives it. `todo` (merged) vs
`triage` (legacy) is what separates "resolved the wrong board" from "took the fallback". */
expect(sync.intake).toBe("todo");
});
it("CHARACTERIZATION — a board that declares NO review lane is still handed one", async () => {
/*
The optional-role contract at its sharpest, and the cleanest statement of harm #2. This board
carries `human-review` but no merge orchestration, so it declares no review lane in the sense
`resolveLifecycleColumns` means — and the async twin says so, returning `undefined`, which is the
answer that makes a caller refuse.
The sync twin returns `"in-review"`. Not a renamed column, not a stale one: a column this board
does not contain in any form. The optionality PR #2628's review argued for is fully intact in the
type and fully unreachable through the twin nine call sites use.
*/
const store = h.store();
const taskId = await taskOn(store, RENAMED_VOCAB, "wf-no-review-lane");
expect((await resolvePlannerLanesForTaskAsync(store, taskId)).review).toBeUndefined();
const sync = resolvePlannerLanes(store, taskId);
expect(sync.review).toBe("in-review");
/* Same reason as the case above: pins that this came from the merged default IR rather than
from the legacy fallback constant, which also carries `review: "in-review"`. */
expect(sync.intake).toBe("todo");
});
});