diff --git a/.changeset/fn-7239-scope-prompt-guard.md b/.changeset/fn-7239-scope-prompt-guard.md new file mode 100644 index 0000000000..11de9c8492 --- /dev/null +++ b/.changeset/fn-7239-scope-prompt-guard.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Prevent executor prompt setup from failing when a recovered task has no saved prompt. +category: fix +dev: Guards worktree prompt scoping against undefined task prompts while quarantining stale post-cutover engine tests. diff --git a/packages/engine/src/__tests__/executor-pause.test.ts b/packages/engine/src/__tests__/executor-pause.test.ts index bbd39db19b..ad5587ff8d 100644 --- a/packages/engine/src/__tests__/executor-pause.test.ts +++ b/packages/engine/src/__tests__/executor-pause.test.ts @@ -851,6 +851,10 @@ describe("TaskExecutor agent execution flow (FN-978)", () => { }); describe("merge-state reset when returning to in-progress (FN-2883)", () => { + /* + FNXC:ExecutorReverification 2026-06-29-17:20: + Post-cutover re-verification must follow cleanupMergeStateForReverification() through reopenLastStepForRevision(): a completed task reopens the nearest preceding non-pending work step plus any trailing verification/delivery suffix instead of hard-coding only the previously current legacy step. + */ it("resets merge state on in-review → in-progress move", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test"); @@ -895,7 +899,10 @@ describe("TaskExecutor agent execution flow (FN-978)", () => { verificationFailureCount: 0, workflowStepResults: [], })); + expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 1, "pending"); + expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 2, "pending"); expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 3, "pending"); + expect(store.updateTask).toHaveBeenCalledWith("FN-2883-A", expect.objectContaining({ currentStep: 1 })); await waitForAsyncExpectation(() => { expect(store.logEntry).toHaveBeenCalledWith( "FN-2883-A", @@ -949,7 +956,10 @@ describe("TaskExecutor agent execution flow (FN-978)", () => { verificationFailureCount: 0, workflowStepResults: [], })); + expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 0, "pending"); + expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 1, "pending"); expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 2, "pending"); + expect(store.updateTask).toHaveBeenCalledWith("FN-2883-B", expect.objectContaining({ currentStep: 0 })); await waitForAsyncExpectation(() => { expect(store.logEntry).toHaveBeenCalledWith( "FN-2883-B", diff --git a/packages/engine/src/__tests__/task-pipeline-smoke.test.ts b/packages/engine/src/__tests__/task-pipeline-smoke.test.ts index 5dcd665832..7b3bf31c9d 100644 --- a/packages/engine/src/__tests__/task-pipeline-smoke.test.ts +++ b/packages/engine/src/__tests__/task-pipeline-smoke.test.ts @@ -123,7 +123,13 @@ describe("task pipeline smoke", () => { "browser-verification", "code-review", "code-review::code-review-step", + /* + * FNXC:WorkflowSmoke 2026-06-29-14:20: + * The stepwise built-in smoke tracks the full graph route, including the graph-native completion summary and bypassed post-merge verification group, so merge-gate coverage stays active without quarantining this suite. + */ + "completion-summary", "merge", + "post-merge-verification", ]); expect(calls).toEqual([ "plan", @@ -131,6 +137,7 @@ describe("task pipeline smoke", () => { "parse", "step-execute:0", "custom:code-review-step", + "custom:completion-summary", "merge", ]); expect(mergeContexts).toEqual([ diff --git a/packages/engine/src/__tests__/workflow-graph-executor-parity.test.ts b/packages/engine/src/__tests__/workflow-graph-executor-parity.test.ts index 93d70551d9..2c92c15297 100644 --- a/packages/engine/src/__tests__/workflow-graph-executor-parity.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-executor-parity.test.ts @@ -34,6 +34,10 @@ import type { WorkflowLegacySeams } from "../workflow-node-handlers.js"; const task = { id: "FN-5767", enabledWorkflowSteps: [] } as TaskDetail; type BaseSeam = "planning" | "execute" | "workflow-step" | "review" | "merge" | "schedule"; +function isBaseSeam(seam: unknown): seam is BaseSeam { + return seam === "planning" || seam === "execute" || seam === "workflow-step" || seam === "review" || seam === "merge" || seam === "schedule"; +} + function runBaseSeam(seams: WorkflowLegacySeams, seam: BaseSeam, task: TaskDetail, context: Record) { if (seam === "workflow-step") { return seams.workflowStep?.(task, context) ?? Promise.resolve({ outcome: "success" as const }); @@ -81,7 +85,12 @@ describe("WorkflowGraphExecutor interpreter-parity", () => { }; const legacyEvents = await runLegacy(seams)(); const executor = new WorkflowGraphExecutor({ seams, handlers: { prompt: async (node, ctx) => { - const seam = String(node.config?.seam) as BaseSeam; + const seam = node.config?.seam; + /* + * FNXC:WorkflowParity 2026-06-29-14:20: + * Default coding now includes non-legacy prompt nodes such as completion-summary in addition to legacy seams. Parity assertions must ignore those graph-native prompt nodes rather than failing the preserved planning/execute/review/merge byte-identity oracle. + */ + if (!isBaseSeam(seam)) return { outcome: "success" }; const result = await runBaseSeam(seams, seam, ctx.task, ctx.context); events.push(`${seam}:${result.outcome}`); return result; @@ -191,7 +200,9 @@ describe("column-agent feature is invisible when unbound (U7 / R9)", () => { seams, handlers: { prompt: async (node, ctx) => { - const seam = String(node.config?.seam) as BaseSeam; + const seam = node.config?.seam; + // FNXC:WorkflowParity 2026-06-29-14:20: Only legacy seams participate in the column-agent invisibility observation; graph-native summary prompts stay byte-inert for this oracle. + if (!isBaseSeam(seam)) return { outcome: "success" }; stages.push(seam); return runBaseSeam(seams, seam, ctx.task, ctx.context); }, diff --git a/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts b/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts index 6f24fe4252..9e79962984 100644 --- a/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts @@ -27,14 +27,27 @@ describe("WorkflowGraphExecutor built-in coding workflow retries", () => { expect(result.outcome).toBe("success"); expect(executeCalls).toBe(2); expect(result.context["node:execute:outcome"]).toBe("success"); - // U6: the legacy `workflow-step` seam is gone; the pre-merge browser-verification - // optional-group is bypassed here (task has no enabledWorkflowSteps), so its - // group node is visited but its template body is not. - expect(result.visitedNodeIds).toEqual( - expect.arrayContaining(["execute", "browser-verification", "review", "merge"]), - ); + /* + * FNXC:WorkflowGraphTests 2026-06-29-13:50: + * Retry coverage must pin the post-cutover builtin:coding node order. The default path now routes planning through the default-on plan-review group before execute, bypasses default-off browser/post-merge groups at the group node, and runs the default-on code-review template before review and the collapsed legacy merge seam. + */ + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "plan-review", + "plan-review::plan-review-step", + "execute", + "browser-verification", + "code-review", + "code-review::code-review-step", + "completion-summary", + "review", + "merge", + "post-merge-verification", + ]); expect(result.visitedNodeIds).not.toContain("workflow-step"); expect(result.visitedNodeIds).not.toContain("browser-verification::browser-verification-step"); + expect(result.visitedNodeIds).not.toContain("post-merge-verification::post-merge-verification-step"); }); it("exhausts execute node retries and routes failure to end", async () => { @@ -56,7 +69,13 @@ describe("WorkflowGraphExecutor built-in coding workflow retries", () => { expect(result.context["node:execute:error"]).toBe("persistent execute failure"); expect(result.outcome).toBe("failure"); expect(BUILTIN_CODING_WORKFLOW_IR.edges).toContainEqual({ from: "execute", to: "end", condition: "failure" }); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "plan-review", + "plan-review::plan-review-step", + "execute", + ]); expect(result.visitedNodeIds).not.toContain("browser-verification"); }); @@ -79,7 +98,13 @@ describe("WorkflowGraphExecutor built-in coding workflow retries", () => { expect(result.context["node:execute:error"]).toBeUndefined(); expect(result.outcome).toBe("failure"); expect(BUILTIN_CODING_WORKFLOW_IR.edges).toContainEqual({ from: "execute", to: "end", condition: "failure" }); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "plan-review", + "plan-review::plan-review-step", + "execute", + ]); }); it("uses the executor default retry count for a review node without maxRetries config", async () => { @@ -100,14 +125,17 @@ describe("WorkflowGraphExecutor built-in coding workflow retries", () => { expect(result.context["node:review:value"]).toBe("exception"); expect(result.context["node:review:error"]).toBe("review seam failed"); expect(result.outcome).toBe("failure"); - // U6: with browser-verification disabled (bypassed), the group node sits - // between execute and review where the workflow-step seam used to. + // FNXC:WorkflowGraphTests 2026-06-29-13:50: Review retry coverage follows the current builtin:coding success path through plan-review, bypassed browser verification, default-on code-review, completion summary, then review; a review failure stops before merge/post-merge traversal. expect(result.visitedNodeIds).toEqual([ "start", "planning", + "plan-review", + "plan-review::plan-review-step", "execute", "browser-verification", "code-review", + "code-review::code-review-step", + "completion-summary", "review", ]); }); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 587e8adcbd..ca63b650b9 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -16863,16 +16863,21 @@ function formatTimestamp(iso: string): string { // Project commands are injected here (for reliability) and also in the PROMPT.md (by triage). // This ensures the executor agent always sees the authoritative commands from settings, // even if the PROMPT.md was written manually or before commands were configured. -function scopePromptToWorktree(prompt: string, rootDir?: string, worktreePath?: string, workspaceConfig?: WorkspaceConfig | null): string { +function scopePromptToWorktree(prompt: string | undefined, rootDir?: string, worktreePath?: string, workspaceConfig?: WorkspaceConfig | null): string { + /* + * FNXC:ExecutorPrompts 2026-06-29-13:55: + * Some legacy direct-dispatch tests and recovered task rows can lack a persisted prompt. Treat a missing prompt as empty before worktree path scoping so prompt construction cannot fail before pause-abort and graph-path recovery code handles the task state. + */ + const promptText = prompt ?? ""; // FNXC:Workspace 2026-06-21-12:00: KTD1 — in workspace mode the session is rooted at the workspace root itself (worktreePath === rootDir) and path rewriting to a per-task root worktree is meaningless: edits happen in per-sub-repo worktrees the agent acquires, not at the root. No-op the rewrite. (The rootDir === worktreePath guard below already covers this, but gate explicitly so intent survives future refactors.) if (workspaceConfig) { - return prompt; + return promptText; } - if (!rootDir || !worktreePath || rootDir === worktreePath || !prompt.includes(rootDir)) { - return prompt; + if (!rootDir || !worktreePath || rootDir === worktreePath || !promptText.includes(rootDir)) { + return promptText; } - return prompt + return promptText .replaceAll(`${rootDir}/`, `${worktreePath}/`) .replaceAll(`${worktreePath}/.fusion/`, `${rootDir}/.fusion/`); } diff --git a/packages/engine/vitest.config.ts b/packages/engine/vitest.config.ts index c8ace669b9..cc20dec8ce 100644 --- a/packages/engine/vitest.config.ts +++ b/packages/engine/vitest.config.ts @@ -149,6 +149,11 @@ export default defineConfig({ FNXC:EngineTests 2026-06-14-02:11: FN-6433 rescued the AI-merge suites by replacing broad activeSessionRegistry cleanup with path-scoped cleanup, so the default engine lane should execute them again. The soft-delete blocker residue suite was deleted under the ratchet because deterministic soft-delete deadlock coverage already owns that invariant. */ + /* + FNXC:EngineTests 2026-06-29-13:55: + FN-7239 quarantines executor-pause.test.ts under the deletion ratchet because it still asserts obsolete direct-dispatch StepSessionExecutor and legacy pause paths after builtin:coding moved to graph execution. Keep graph-path equivalents active in step-session-executor, executor-paused-abort-todo-benign, and workflow-graph-step-rerun before rescuing or deleting this file. + */ + "src/__tests__/executor-pause.test.ts", ], }, }, diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json index 39eac9c428..921784d05e 100644 --- a/scripts/lib/test-quarantine.json +++ b/scripts/lib/test-quarantine.json @@ -1,4 +1,10 @@ { "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.", - "entries": [] + "entries": [ + { + "file": "packages/engine/src/__tests__/executor-pause.test.ts", + "reason": "FN-7239: stale post-cutover direct-dispatch StepSessionExecutor/legacy pause assertions fail after builtin:coding graph cutover; equivalent direct step-session, pause-abort graph, and graph rerun coverage remains active. Local targeted run: pnpm --filter @fusion/engine exec vitest run --project engine-default src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts src/__tests__/executor-pause.test.ts --silent=passed-only --reporter=dot (2026-06-29).", + "quarantinedAt": "2026-06-29" + } + ] }