test(engine): spread real reviewer exports in executor-test-helpers mock (3167dbc83); align stepwise/graph/prompt-override tests (FN-7265/7335)

This commit is contained in:
gsxdsm
2026-07-07 08:54:49 -07:00
parent a017b53e85
commit 58d085efab
4 changed files with 36 additions and 13 deletions

View File

@@ -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(),

View File

@@ -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");
});
});

View File

@@ -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", () => {

View File

@@ -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(