diff --git a/packages/engine/src/__tests__/executor-test-helpers.ts b/packages/engine/src/__tests__/executor-test-helpers.ts index 0e1a4a7bfe..0dcdc399f8 100644 --- a/packages/engine/src/__tests__/executor-test-helpers.ts +++ b/packages/engine/src/__tests__/executor-test-helpers.ts @@ -1,6 +1,7 @@ import { vi } from "vitest"; import type { Mock } from "vitest"; import { installTaskWorktreeIdentityGuard } from "../worktree-hooks.js"; +import type * as ReviewerModule from "../reviewer.js"; // Mock external dependencies vi.mock("../pi.js", () => ({ @@ -24,9 +25,14 @@ vi.mock("../pi.js", () => ({ } }), })); -vi.mock("../reviewer.js", () => ({ - reviewStep: vi.fn(), -})); +/* + * FNXC:WorkflowReviewers 2026-07-07-08:40: + * Commit 3167dbc83 wired `proseSignalsClearApproval` + `extractJsonObjectCandidates` from reviewer.js into the workflow-step verdict parser (parseWorkflowStepVerdict). A mock that returns only `reviewStep` makes every executeWorkflowStep verdict parse throw `[vitest] No "extractJsonObjectCandidates" export`. Surface the real exports via importOriginal and stub only `reviewStep` (the agent-invoking seam these tests avoid); the verdict-parsing helpers then run for real. + */ +vi.mock("../reviewer.js", async (importOriginal) => { + const actual = (await importOriginal()) as ReviewerModule; + return { ...actual, reviewStep: vi.fn() }; +}); vi.mock("../logger.js", () => { const createMockLogger = () => ({ log: vi.fn(), diff --git a/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts b/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts index 5dd23ea289..0594f7f3ed 100644 --- a/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts +++ b/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts @@ -425,6 +425,8 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { readArtifact: async () => "### Step 1: do it\n", writeSteps: async () => {}, }, + // FNXC:WorkflowGraphCutover 2026-07-07-09:05: stepwise-coding gained a default-on plan-review optional-group (and always-on completion-summary) before the foreach; wire the custom-node runner (success) so those auxiliary nodes pass through and the foreach/step invariant under test is actually reached (mirrors production executor.ts runCustomNode + runStepwiseGraph). + runCustomNode: async () => ({ outcome: "success" }), }); const result = await executor.run(task, settingsOn(), BUILTIN_STEPWISE_CODING_WORKFLOW_IR); @@ -464,6 +466,7 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { readArtifact: async () => "### Step 1: a\n### Step 2: b\n### Step 3: c\n", writeSteps: async () => {}, }, + runCustomNode: async () => ({ outcome: "success" }), }); const result = await executor.run(task, settingsOn(), BUILTIN_STEPWISE_CODING_WORKFLOW_IR); @@ -501,6 +504,7 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { readArtifact: async () => "### Step 1: a\n### Step 2: b\n", writeSteps: async () => {}, }, + runCustomNode: async () => ({ outcome: "success" }), }); const result = await executor.run(task, settingsOn(), BUILTIN_STEPWISE_CODING_WORKFLOW_IR); @@ -581,6 +585,7 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { readArtifact: async () => "no steps here, just prose", writeSteps: async () => {}, }, + runCustomNode: async () => ({ outcome: "success" }), }); const result = await executor.run(task, settingsOn(), BUILTIN_STEPWISE_CODING_WORKFLOW_IR); @@ -620,19 +625,21 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { expect(browserVerificationCalls).toBe(1); expect(result.visitedNodeIds).toContain("browser-verification"); expect(result.visitedNodeIds).toContain(BROWSER_VERIFICATION_STEP_VISITED_ID); - // Ordering: all step instances complete before the group's inner step, which - // precedes review. + // FNXC:WorkflowGraphCutover 2026-07-07-09:10: + // FN-7265 removed the post-foreach `review` node; the pre-merge gate is now the default-on `code-review` optional-group (browser-verification → code-review → completion-summary → merge-gate). The R-3 run-once ordering invariant is therefore: all step instances finish before the browser-verification inner step, which precedes the code-review gate. const groupStepIdx = result.visitedNodeIds.indexOf(BROWSER_VERIFICATION_STEP_VISITED_ID); - const reviewIdx = result.visitedNodeIds.indexOf("review"); + const codeReviewIdx = result.visitedNodeIds.indexOf("code-review"); const lastStepIdx = result.visitedNodeIds.map((id) => id.startsWith("steps#")).lastIndexOf(true); expect(lastStepIdx).toBeLessThan(groupStepIdx); - expect(groupStepIdx).toBeLessThan(reviewIdx); + expect(groupStepIdx).toBeLessThan(codeReviewIdx); }); it("bypasses the browser-verification optional-group (inert) when it is not enabled", async () => { // Disabled (no enabledWorkflowSteps): the group node is traversed but its // template body never runs — the inner prompt node is not visited and the - // custom-node runner is never invoked for it. Routes straight to review. + // custom-node runner is never invoked for it. Routes straight to the + // code-review gate. + // FNXC:WorkflowGraphCutover 2026-07-07-09:10: FN-7265 removed the `review` node; the post-foreach gate this inert path reaches is now `code-review`. let browserVerificationCalls = 0; const { outcome, result } = await runStepwiseGraph(2, [["APPROVE"], ["APPROVE"]], { runCustomNode: async (nodeId) => { @@ -644,6 +651,6 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { expect(browserVerificationCalls).toBe(0); expect(result.visitedNodeIds).toContain("browser-verification"); expect(result.visitedNodeIds).not.toContain(BROWSER_VERIFICATION_STEP_VISITED_ID); - expect(result.visitedNodeIds).toContain("review"); + expect(result.visitedNodeIds).toContain("code-review"); }); }); diff --git a/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts b/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts index 457db672e3..4db53bfbea 100644 --- a/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts @@ -313,7 +313,15 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => { await (executor as any).clearStalePauseAbortBeforeDispatch(liveTask); expect((executor as any).pausedAborted.has("FN-7066")).toBe(false); - expect(store.logEntry).not.toHaveBeenCalled(); + /* + * FNXC:WorkflowLifecycle 2026-07-07-08:35: + * FN-7335 wired a best-effort "Pause abort marked: provenance=… source=…" breadcrumb into markPausedAborted() itself (via safeLogEntry), so the setup markPausedAborted() call above now produces one store.logEntry. clearStalePauseAbortBeforeDispatch() must still clear SILENTLY: it logs via executorLog only and must NOT emit its own store.logEntry (the marker is volatile engine state, not a task event). Assert no "cleared stale pause-abort marker" log reached the store. + */ + expect( + store.logEntry.mock.calls.some(([, message]: [string, string]) => + /cleared stale pause-abort marker/i.test(message), + ), + ).toBe(false); }); it("clears pause-abort provenance for manual retry", () => { diff --git a/packages/engine/src/__tests__/workflow-prompt-overrides-resolution.test.ts b/packages/engine/src/__tests__/workflow-prompt-overrides-resolution.test.ts index 503431f2ca..7cc46b9442 100644 --- a/packages/engine/src/__tests__/workflow-prompt-overrides-resolution.test.ts +++ b/packages/engine/src/__tests__/workflow-prompt-overrides-resolution.test.ts @@ -38,11 +38,13 @@ describe("workflow prompt override resolution", () => { const projectId = store.getWorkflowSettingsProjectId(); const defaultExecutePrompt = resolveSeamPromptFromIr(BUILTIN_CODING_WORKFLOW_IR, "execute"); const beforeStaticIr = JSON.stringify(BUILTIN_CODING_WORKFLOW_IR); - const task = await store.createTask({ description: "uses prompt override", workflowId: "builtin:coding" }); + const task = await store.createTask({ description: "uses prompt override", workflowId: "builtin:legacy-coding" }); // FNXC:CustomWorkflows 2026-06-21-21:04: // Engine seam resolution must consume the same built-in prompt override overlay as dashboard preview and sync store resolution, while reset-to-default must reveal the shipped static prompt again. - store.updateWorkflowPromptOverrides("builtin:coding", projectId, { execute: "Engine execute override" }); + // FNXC:CustomWorkflows 2026-07-07-08:45: + // builtin:coding became the stepwise final-review workflow (commit 6ce0b4405 "make coding stepwise with final review") and no longer carries a top-level `execute` seam prompt node — per-step work runs inside the `steps` foreach, so resolveSeamPromptFromIr(..., "execute") returns undefined there. The execute-seam override/resolution invariant is therefore pinned against builtin:legacy-coding (= BUILTIN_CODING_WORKFLOW_IR), the monolithic workflow that still owns the execute seam node (id "execute", seam "execute"). The override keys by node id and resolves by seam; legacy-coding is the surface where both still coincide. + store.updateWorkflowPromptOverrides("builtin:legacy-coding", projectId, { execute: "Engine execute override" }); expect(await resolveTaskSeamPrompt(store, task.id, "execute")).toBe("Engine execute override"); const syncIr = (store as StoreWithSyncWorkflowResolution).resolveTaskWorkflowIrSync(task.id); @@ -50,7 +52,7 @@ describe("workflow prompt override resolution", () => { expect(syncIr).not.toBe(BUILTIN_CODING_WORKFLOW_IR); expect(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)).toBe(beforeStaticIr); - store.updateWorkflowPromptOverrides("builtin:coding", projectId, { execute: null }); + store.updateWorkflowPromptOverrides("builtin:legacy-coding", projectId, { execute: null }); expect(await resolveTaskSeamPrompt(store, task.id, "execute")).toBe(defaultExecutePrompt); expect(resolveSeamPromptFromIr((store as StoreWithSyncWorkflowResolution).resolveTaskWorkflowIrSync(task.id), "execute")).toBe(