diff --git a/packages/engine/src/__tests__/executor-planner-lanes-resolved.test.ts b/packages/engine/src/__tests__/executor-planner-lanes-resolved.test.ts index ea72a5563e..dc8b6d53d1 100644 --- a/packages/engine/src/__tests__/executor-planner-lanes-resolved.test.ts +++ b/packages/engine/src/__tests__/executor-planner-lanes-resolved.test.ts @@ -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 = 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); }); diff --git a/packages/engine/src/__tests__/planner-lane-resolution.test.ts b/packages/engine/src/__tests__/planner-lane-resolution.test.ts index c0abf9b4c9..54dd1fc0c9 100644 --- a/packages/engine/src/__tests__/planner-lane-resolution.test.ts +++ b/packages/engine/src/__tests__/planner-lane-resolution.test.ts @@ -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; } diff --git a/packages/engine/src/__tests__/planner-lanes-async-resolution.test.ts b/packages/engine/src/__tests__/planner-lanes-async-resolution.test.ts index 48130cf6c1..31d8c17b88 100644 --- a/packages/engine/src/__tests__/planner-lanes-async-resolution.test.ts +++ b/packages/engine/src/__tests__/planner-lanes-async-resolution.test.ts @@ -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; } diff --git a/packages/engine/src/__tests__/recover-approved-intake-post-u11.test.ts b/packages/engine/src/__tests__/recover-approved-intake-post-u11.test.ts index b19ff0ecf2..15e71b8ff2 100644 --- a/packages/engine/src/__tests__/recover-approved-intake-post-u11.test.ts +++ b/packages/engine/src/__tests__/recover-approved-intake-post-u11.test.ts @@ -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 })), diff --git a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts index 880f5003d2..3d8d3cb01a 100644 --- a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts +++ b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts @@ -76,6 +76,15 @@ function createStore(tasks: Record[] = [], 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; diff --git a/packages/engine/src/__tests__/triage-release-renamed-hold.test.ts b/packages/engine/src/__tests__/triage-release-renamed-hold.test.ts index 2924dd9623..ef1078acb1 100644 --- a/packages/engine/src/__tests__/triage-release-renamed-hold.test.ts +++ b/packages/engine/src/__tests__/triage-release-renamed-hold.test.ts @@ -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 diff --git a/packages/engine/src/__tests__/triage-undeclared-column-rescue.test.ts b/packages/engine/src/__tests__/triage-undeclared-column-rescue.test.ts index 1061cf30d3..5b75837ab9 100644 --- a/packages/engine/src/__tests__/triage-undeclared-column-rescue.test.ts +++ b/packages/engine/src/__tests__/triage-undeclared-column-rescue.test.ts @@ -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; diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index de9810793f..0c2fd3c06a 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -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); return new TriageProcessor(store, root).recoverApprovedTask({