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

FN-8648: audit sync workflow resolver test fixtures

Browse Source 
Audit workflow resolver test fixtures so they reflect authoritative async resolution and document intentional sync coverage.

- Remove redundant synchronous resolver fixtures from async workflow tests.
- Preserve and document the scheduler fixture that exposes the known synchronous listener defect.
- Clarify the deliberate default-workflow sync contrast and strengthen recovery fixture intent.

Files changed:
 .../executor-planner-lanes-resolved.test.ts        | 45 +++++-----------------
 .../src/__tests__/planner-lane-resolution.test.ts  | 11 +++---
 .../planner-lanes-async-resolution.test.ts         |  9 ++++-
 .../recover-approved-intake-post-u11.test.ts       | 11 +++---
 .../scheduler-renamed-hold-events.test.ts          |  9 +++++
 .../__tests__/triage-release-renamed-hold.test.ts  |  6 ++-
 .../triage-undeclared-column-rescue.test.ts        | 11 +++---
 packages/engine/src/__tests__/triage.test.ts       | 23 +++++------
 8 files changed, 56 insertions(+), 69 deletions(-)

Fusion-Task-Id: FN-8648

Fusion-Task-Lineage: c75fe9ca-30a3-467e-9268-f8d6d95c49e7

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-07-31 19:26:23 -07:00
parent b65bc0bbb4
commit 408e558f54
8 changed files with 56 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
packages/engine/src/__tests__/executor-planner-lanes-resolved.test.ts
View File

@@ -87,43 +87,18 @@ function completedTaskIn(column: string) {
};
}
/**
* @param syncResolvesIr Feed the SYNC reader the test's IR instead of the default lineage.
*
* Default is `false`, which mirrors production: the sync reader answers with the DEFAULT workflow for
* every task, whatever the card is bound to. Pass `true` ONLY for cases exercising a classifier that
* is still synchronous — there the sync reader is genuinely the input path, so feeding it the IR
* tests the classifier's logic rather than the (separately tracked) fact that the reader does not
* resolve. Those classifiers live in the synchronous `task:moved` listener; converting them needs a
* restructure of that handler's else-if chain, and their production inertness is held by the
* `resolveTaskWorkflowIrSync` call-site allow-list.
*/
function harness(ir: WorkflowIr | undefined, column: string, syncResolvesIr = false) {
function harness(ir: WorkflowIr | undefined, column: string) {
const store = createMockStore();
let task: Record<string, unknown> = completedTaskIn(column);
const moves: Array<[string, string]> = [];
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-23:10 (the sync seam could not see production):
THE AUTHORITATIVE READERS, not just the sync one.
This harness injected ONLY `resolveTaskWorkflowIrSync`, so every case here proved the promotion
LOGIC while being structurally blind to whether production resolves anything at all. It does not:
that reader's selection lookup returns `undefined` unconditionally in PostgreSQL mode, so the real
call site resolved the DEFAULT workflow for every card and the recovery never fired on a renamed
board — with this suite green.
Feeding the async readers makes the suite exercise the path the call site now takes. The sync
reader stays wired so a revert to it is visible as a failure here rather than as silence.
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 REDUNDANT:
Deleting the complete sync-resolver assignment and running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/executor-planner-lanes-resolved.test.ts --silent=passed-only --reporter=dot`
passed 12/12. The harness's async selection and definition readers supply the production path;
its direct classifier cases already pass explicit move lanes, so no sync fixture is required.
*/
/*
The sync reader returns the DEFAULT lineage, which is what it ACTUALLY does in production for every
task regardless of binding. Handing it the test's `ir` — as this harness used to — is the part that
made the suite unable to tell a resolved answer from an unresolved one: it fed the broken reader the
right answer. With this, reverting the call site to the sync resolver fails these cases.
*/
(store as unknown as { resolveTaskWorkflowIrSync: (id: string) => WorkflowIr })
.resolveTaskWorkflowIrSync = () => (syncResolvesIr ? (ir as WorkflowIr) : (BUILTIN_CODING_WORKFLOW_IR as unknown as WorkflowIr));
const workflowId = (ir as { id?: string } | undefined)?.id ?? "builtin:coding";
store.getTaskWorkflowSelectionAsync = vi.fn(async () => (ir ? { workflowId, stepIds: [] } : undefined));
store.getWorkflowDefinition = vi.fn(async () => (ir ? { ir } : undefined));
@@ -154,7 +129,7 @@ function harness(ir: WorkflowIr | undefined, column: string, syncResolvesIr = fa
describe("stranded-completed recovery promotes through the task's OWN planner lanes", () => {
it("re-homes intake -> hold -> wip on a renamed board that separates the two roles", async () => {
const h = harness(RENAMED_SPLIT_IR, "backlog", true);
const h = harness(RENAMED_SPLIT_IR, "backlog");
const recovered = await h.executor.recoverCompletedTask(completedTaskIn("backlog") as never);
@@ -268,7 +243,7 @@ describe("a forward move off a renamed planner lane is not an evacuation", () =>
it("does NOT evacuate a card advancing into the renamed wip/review/complete lanes", () => {
// Pre-fix each of these returned true, so the executor aborted live planning work and
// deleted the pre-execution worktree of a card that was merely advancing.
const h = harness(RENAMED_SPLIT_IR, "backlog", true);
const h = harness(RENAMED_SPLIT_IR, "backlog");
expect(isBackward(h, "backlog", "building", RENAMED_SPLIT_IR)).toBe(false);
expect(isBackward(h, "queued", "checking", RENAMED_SPLIT_IR)).toBe(false);
@@ -278,7 +253,7 @@ describe("a forward move off a renamed planner lane is not an evacuation", () =>
it("DOES evacuate a card withdrawn to a non-lifecycle column", () => {
// The paired positive: the branch must still fire for the case it was written for
// (the reported symptom was todo -> Ideas).
const h = harness(RENAMED_SPLIT_IR, "backlog", true);
const h = harness(RENAMED_SPLIT_IR, "backlog");
expect(isBackward(h, "backlog", "ideas", RENAMED_SPLIT_IR)).toBe(true);
});
@@ -293,7 +268,7 @@ describe("a forward move off a renamed planner lane is not an evacuation", () =>
});
it("never fires for a card that was not in a planner lane", () => {
const h = harness(RENAMED_SPLIT_IR, "building", true);
const h = harness(RENAMED_SPLIT_IR, "building");
expect(isBackward(h, "building", "ideas", RENAMED_SPLIT_IR)).toBe(false);
});

11
packages/engine/src/__tests__/planner-lane-resolution.test.ts
View File

@@ -27,12 +27,11 @@ function storeWith(ir: WorkflowIr | null): TaskStore {
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
getWorkflowDefinition: vi.fn(async () => (ir ? { ir } : null)),
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
The `resolveTaskWorkflowIrSync` stub is REMOVED, and it was redundant: this suite passes without
it. That reader answers with the DEFAULT board for every task in production, so stubbing it with a
working IR feeds the broken reader the right answer — the suite would keep passing even if its
call site stopped resolving. Audited across the 8 files that stubbed it (#3197): four were
redundant like this one, one is legitimately about the sync path, one was masking real inertness.
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 REDUNDANT:
The `resolveTaskWorkflowIrSync` stub remains removed. Re-running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/planner-lane-resolution.test.ts --silent=passed-only --reporter=dot`
passed 7/7. It was redundant because the async readers resolve the test workflow without it.
FN-8648's corrected tally is six redundant, one deliberate DEFAULT-IR contrast, one masking site.
*/
} as unknown as TaskStore;
}

9
packages/engine/src/__tests__/planner-lanes-async-resolution.test.ts
View File

@@ -49,7 +49,14 @@ function createStore(): TaskStore {
getTaskWorkflowSelection: vi.fn(() => undefined),
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
getWorkflowDefinition: vi.fn(async () => ({ ir: RENAMED_IR })),
/* Mirrors the real sync resolver's contract: no selection → the DEFAULT workflow IR. */
/*
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 DELIBERATE-SYNC:
Removing this production-faithful DEFAULT-IR stub and running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/planner-lanes-async-resolution.test.ts --silent=passed-only --reporter=dot`
produced 1 failed / 3 passed: the named SYNC contrast no longer reported
`resolvedFromWorkflow: true`. The stub intentionally proves that an inert sync read can look
resolved, while the authoritative async reader returns the renamed workflow.
*/
resolveTaskWorkflowIrSync: vi.fn(() => BUILTIN_DEFAULT_IR),
} as unknown as TaskStore;
}

11
packages/engine/src/__tests__/recover-approved-intake-post-u11.test.ts
View File

@@ -100,12 +100,11 @@ function createStore(task: Task, workflowIr: WorkflowIr): TaskStore {
conversion does not work" rather than "the fake is incomplete".
*/
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
The `resolveTaskWorkflowIrSync` stub is REMOVED, and it was redundant: this suite passes without
it. That reader answers with the DEFAULT board for every task in production, so stubbing it with a
working IR feeds the broken reader the right answer — the suite would keep passing even if its
call site stopped resolving. Audited across the 8 files that stubbed it (#3197): four were
redundant like this one, one is legitimately about the sync path, one was masking real inertness.
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 REDUNDANT:
The `resolveTaskWorkflowIrSync` stub remains removed. Re-running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/recover-approved-intake-post-u11.test.ts --silent=passed-only --reporter=dot`
passed 6/6. It was redundant because the async readers resolve the test workflow without it.
FN-8648's corrected tally is six redundant, one deliberate DEFAULT-IR contrast, one masking site.
*/
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
getWorkflowDefinition: vi.fn(async () => ({ ir: workflowIr })),

9
packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts
View File

@@ -76,6 +76,15 @@ function createStore(tasks: Record<string, unknown>[] = [], ir: WorkflowIr = ren
getTaskWorkflowSelection: vi.fn(() => selection),
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
getWorkflowDefinition: vi.fn(async () => ({ ir })),
/*
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 MASKING:
Deleting the renamed-IR sync fixture and running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/scheduler-renamed-hold-events.test.ts --silent=passed-only --reporter=dot`
produced 3 failed / 9 passed: renamed-hold wake, dependency lookup, and second-terminal tests.
The fake already supplies the authoritative async selection and IR readers, so this is not an
incomplete fake; synchronous scheduler listeners are inert on renamed boards in production.
Keep this logic fixture explicit while FN-8656 restructures the synchronous listener safely.
*/
resolveTaskWorkflowIrSync: vi.fn(() => ir),
} as unknown as TaskStore;

6
packages/engine/src/__tests__/triage-release-renamed-hold.test.ts
View File

@@ -145,8 +145,10 @@ function createStore(task: Task, ir: WorkflowIr | null): { store: TaskStore; mov
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
getWorkflowDefinition: vi.fn(async () => (ir ? { ir } : null)),
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
THE SYNC READER STUB IS GONE, and its absence is the point.
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 REDUNDANT:
THE SYNC READER STUB IS GONE, and its absence is the point. Re-running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/triage-release-renamed-hold.test.ts --silent=passed-only --reporter=dot`
passed 5/5; its existing mutation check fails 1/4 when the release target reverts to the sync read.
This harness fed `resolveTaskWorkflowIrSync` the test's own IR — the shape this repo's learnings
call out as feeding the broken reader the right answer. In production that reader answers with the

11
packages/engine/src/__tests__/triage-undeclared-column-rescue.test.ts
View File

@@ -106,12 +106,11 @@ function createStore(ir: WorkflowIr, workflowId: string = WF): TaskStore {
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
getWorkflowDefinition: vi.fn(async () => ({ ir })),
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
The `resolveTaskWorkflowIrSync` stub is REMOVED, and it was redundant: this suite passes without
it. That reader answers with the DEFAULT board for every task in production, so stubbing it with a
working IR feeds the broken reader the right answer — the suite would keep passing even if its
call site stopped resolving. Audited across the 8 files that stubbed it (#3197): four were
redundant like this one, one is legitimately about the sync path, one was masking real inertness.
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 REDUNDANT:
The `resolveTaskWorkflowIrSync` stub remains removed. Re-running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/triage-undeclared-column-rescue.test.ts --silent=passed-only --reporter=dot`
passed 7/7. It was redundant because the async readers resolve the test workflow without it.
FN-8648's corrected tally is six redundant, one deliberate DEFAULT-IR contrast, one masking site.
*/
logEntry: vi.fn(),
} as unknown as TaskStore;

23
packages/engine/src/__tests__/triage.test.ts
View File

@@ -7301,23 +7301,14 @@ is NEVER REACHED: `resolvePlannerLanes` reads `resolveTaskWorkflowIrSync`, which
define, so it returns LEGACY_PLANNER_LANES (`intake: "triage"`) and a `triage` card matches the FIRST
arm. Every earlier fixture I wrote passed through that short-circuit and proved nothing.
So all three cases below stub `resolveTaskWorkflowIrSync` with the MERGED DEFAULT shape, which is what
production resolves: `intake` and `hold` both `todo`. That makes the first arm fail for a `triage` card
and leaves the orphan arm as the only thing deciding the outcome.
The fixture must instead rely on the authoritative async readers below. A sync fixture made the
recovery assertion about a mock implementation rather than production's resolved workflow.
The three cases differ ONLY in the workflow readers, so the outcome difference can have no other cause.
Case B is the positive control: without it, "returns false" is unfalsifiable — every case would pass if
the arm were dead.
*/
describe("recoverApprovedTask — the orphan-`triage` arm, with the intake short-circuit disabled", () => {
const MERGED_DEFAULT = {
version: "v2", id: "builtin:coding", nodes: [], edges: [],
columns: [
{ id: "todo", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
{ id: "in-progress", name: "in-progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
{ id: "done", name: "done", traits: [{ trait: "complete" }] },
],
} as never;
const customIr = (id: string, withTriage: boolean) => ({
version: "v2", id, nodes: [], edges: [],
columns: [
@@ -7345,8 +7336,14 @@ describe("recoverApprovedTask — the orphan-`triage` arm, with the intake short
maxConcurrent: 2, maxWorktrees: 4, pollIntervalMs: 10000,
groupOverlappingFiles: false, autoMerge: true, requirePlanApproval: true,
} as Settings),
// Production resolves the MERGED default here, so `lanes.intake` is `todo`, not `triage`.
resolveTaskWorkflowIrSync: vi.fn(() => MERGED_DEFAULT),
/*
FNXC:WorkflowResolvedColumns 2026-08-01-02:07 REDUNDANT:
Deleting the MERGED-default sync stub and running
`pnpm --filter @fusion/engine exec vitest run src/__tests__/triage.test.ts --silent=passed-only --reporter=dot`
passed 232/232. The async selection and workflow-definition readers are the production-faithful
fixture. Mutation replacing `resolvePlannerLanesForTaskAsync` with `resolvePlannerLanes` in
`recoverApprovedTask` produced 1 failed / 231 passed: named case C failed as required.
*/
...workflowReaders,
} as Partial<TaskStore>);
return new TriageProcessor(store, root).recoverApprovedTask({