diff --git a/.changeset/guard-empty-implementation-plans.md b/.changeset/guard-empty-implementation-plans.md new file mode 100644 index 0000000000..6aa3008070 --- /dev/null +++ b/.changeset/guard-empty-implementation-plans.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Prevent unfinished prose-only plans from advancing into implementation and merge. +category: fix +dev: Requires explicit no-commits authorization before a parsed workflow may continue with zero implementation steps. diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts index 2f93c37090..6532c78e61 100644 --- a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -94,7 +94,11 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { id: "parse", kind: "parse-steps", column: "in-progress", - config: { artifact: "PROMPT.md", parser: "step-headings" }, + config: { + artifact: "PROMPT.md", + parser: "step-headings", + requireStepsUnlessNoCommits: true, + }, }, // KTD-3: runtime-expanding per-step region. Sequential + shared isolation is // the default baseline physics (one step at a time in the task's worktree). @@ -194,11 +198,12 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { from: "plan-review", to: "plan-replan", condition: "failure" }, { from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" }, { from: "parse", to: "steps", condition: "success" }, - // parse-steps no-steps defaults to success; route it explicitly to the foreach - // (zero steps → foreach no-ops through its success edge, KTD-8/R8). + // Only explicitly authorized no-commit tasks return no-steps; they may no-op + // through foreach. Missing implementation steps fail before execution/review. { from: "parse", to: "steps", condition: "outcome:no-steps" }, { from: "parse", to: "end", condition: "failure" }, { from: "parse", to: "end", condition: "outcome:parse-error" }, + { from: "parse", to: "end", condition: "outcome:missing-implementation-steps" }, // Implementation complete → pre-merge browser-verification optional-group → // review. Both the normal foreach-success path and the rework-exhausted // manual-release path flow through the group so an enabled task runs the step diff --git a/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts b/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts index 0594f7f3ed..0886d257b0 100644 --- a/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts +++ b/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts @@ -562,9 +562,10 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { // ── Zero-step task (R8) ──────────────────────────────────────────────────── - it("zero-step task on stepwise merges without step work (no-steps outcome path)", async () => { + it("explicit no-commits zero-step task on stepwise merges without step work", async () => { let stepExecuteCalls = 0; const task = taskWithSteps(0); + task.noCommitsExpected = true; const seams: WorkflowLegacySeams = { planning: async () => ({ outcome: "success" }), execute: async () => ({ outcome: "success" }), @@ -598,6 +599,38 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { expect(result.visitedNodeIds).toContain("merge"); }); + it("does not reach foreach or merge when an implementation plan has zero steps", async () => { + let mergeCalls = 0; + const task = taskWithSteps(0); + const executor = new WorkflowGraphExecutor({ + seams: { + planning: async () => ({ outcome: "success" }), + execute: async () => ({ outcome: "success" }), + review: async () => ({ outcome: "success" }), + merge: async () => { + mergeCalls++; + return { outcome: "success" }; + }, + schedule: async () => ({ outcome: "success" }), + stepExecute: async () => ({ outcome: "success", value: "step-done" }), + }, + getTaskSteps: () => [], + parseStepsDeps: { + readArtifact: async () => "prose-only planning draft", + writeSteps: async () => {}, + }, + runCustomNode: async () => ({ outcome: "success" }), + }); + + const result = await executor.run(task, settingsOn(), BUILTIN_STEPWISE_CODING_WORKFLOW_IR); + + expect(result.outcome).toBe("failure"); + expect(result.context["node:parse:value"]).toBe("missing-implementation-steps"); + expect(result.visitedNodeIds).not.toContain("steps"); + expect(result.visitedNodeIds).not.toContain("merge"); + expect(mergeCalls).toBe(0); + }); + // ── Pre-merge browser-verification optional-group (U6, R-3 run-once) ──────── const BROWSER_VERIFICATION_STEP_VISITED_ID = "browser-verification::browser-verification-step"; diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index b2eac6f92d..e14be75b88 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -2411,8 +2411,8 @@ describe("requirePlanApproval setting", () => { * awaiting-approval a second time; the fix must move straight to todo instead. */ describe("FN-7569: plan approval fingerprint idempotency", () => { - const planText = "# Task: FN-IDEMPOTENT - Idempotent plan\n\n## Mission\n\nDo the thing.\n\n## File Scope\n\n- a.ts\n"; - const changedPlanText = "# Task: FN-IDEMPOTENT - Idempotent plan\n\n## Mission\n\nDo the thing, differently.\n\n## File Scope\n\n- a.ts\n- b.ts\n"; + const planText = "# Task: FN-IDEMPOTENT - Idempotent plan\n\n## Mission\n\nDo the thing.\n\n## File Scope\n\n- a.ts\n\n## Steps\n\n### Step 1: Implement\n\nDo the thing.\n"; + const changedPlanText = "# Task: FN-IDEMPOTENT - Idempotent plan\n\n## Mission\n\nDo the thing, differently.\n\n## File Scope\n\n- a.ts\n- b.ts\n\n## Steps\n\n### Step 1: Implement differently\n\nDo the changed thing.\n"; /* FNXC:PlanApproval 2026-07-15-14:05: @@ -2885,6 +2885,81 @@ describe("specified triage recovery", () => { ); }); + it("does not recover a prose-only partial planning draft into execution", async () => { + await writeFile( + join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"), + "# Refinement: unfinished plan\n\nOperator request.\n\n## Original Description\n\nOperator request.\n", + ); + const store = createMockStore({ + getSettings: vi.fn().mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 10000, + groupOverlappingFiles: false, + autoMerge: true, + requirePlanApproval: false, + } as Settings), + parseStepsFromPrompt: vi.fn().mockResolvedValue([]), + }); + const processor = new TriageProcessor(store, rootDir); + + const recovered = await processor.recoverApprovedTask({ + id: "FN-001", + title: "Refinement: unfinished plan", + description: "Operator request.", + column: "triage", + status: "planning", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:02:00.000Z", + }); + + expect(recovered).toBe(false); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo"); + expect(store.logEntry).toHaveBeenCalledWith( + "FN-001", + "Planning recovery withheld: PROMPT.md has no executable steps and does not declare no commits expected", + ); + }); + + it("recovers a structured implementation plan without a no-commits marker", async () => { + await writeFile( + join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"), + "# Task: FN-001 - Implement change\n\n**Size:** M\n\n## Steps\n\n### Step 1: Implement\n\nMake the change.\n", + ); + const store = createMockStore({ + getSettings: vi.fn().mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 10000, + groupOverlappingFiles: false, + autoMerge: true, + requirePlanApproval: false, + } as Settings), + parseStepsFromPrompt: vi.fn().mockResolvedValue([{ name: "Implement", status: "pending" }]), + }); + const processor = new TriageProcessor(store, rootDir); + + const recovered = await processor.recoverApprovedTask({ + id: "FN-001", + description: "Implement change", + column: "triage", + status: "planning", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:02:00.000Z", + }); + + expect(recovered).toBe(true); + expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo"); + }); + /* FNXC:TriageStuckKill 2026-07-18-22:30: Null status alone is not proof of an approved plan (Greptile P1). Only recover null-status @@ -3165,6 +3240,12 @@ Forbidden paths / non-goals: - Do not edit Atlas files: \`AtlasNotes.xcodeproj/**\`, \`Tests/AtlasNotesMobileUITests/**\`, \`Packages/MobileApp/**\`. - Evidence only: \`.fusion/fusion.db\`, \`.fusion/tasks/*/task.json\`, \`Packages/*/Package.resolved\`. - Conditional only: \`.changeset/*.md\`. + +## Steps + +### Step 1: Fix poisoned scope + +Apply the scoped implementation changes. `, ); @@ -3226,7 +3307,7 @@ Forbidden paths / non-goals: it("updates malformed metadata title from prompt heading when task ID matches", async () => { await writeFile( join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"), - "# Task: FN-001 - Experimental AI Agent Onboarding Flow\n\n**Size:** M\n\n## Review Level: 2\n\nRecovered specification", + "# Task: FN-001 - Experimental AI Agent Onboarding Flow\n\n**Size:** M\n\n## Review Level: 2\n\nRecovered specification\n\n## Steps\n\n### Step 1: Implement onboarding flow\n\nMake the change.", ); const store = createMockStore({ @@ -3265,7 +3346,7 @@ Forbidden paths / non-goals: it("does not overwrite title when heading task ID does not match", async () => { await writeFile( join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"), - "# Task: FN-999 - Wrong Task\n\n**Size:** M\n\n## Review Level: 2\n\nRecovered specification", + "# Task: FN-999 - Wrong Task\n\n**Size:** M\n\n## Review Level: 2\n\nRecovered specification\n\n## Steps\n\n### Step 1: Implement change\n\nMake the change.", ); const store = createMockStore({ @@ -3315,7 +3396,7 @@ Forbidden paths / non-goals: it("preserves imported GitHub issue titles during planning recovery", async () => { await writeFile( join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"), - "# Task: FN-001 - Different AI-generated planning title\n\n**Size:** M\n\n## Review Level: 2\n\nRecovered specification", + "# Task: FN-001 - Different AI-generated planning title\n\n**Size:** M\n\n## Review Level: 2\n\nRecovered specification\n\n## Steps\n\n### Step 1: Implement issue fix\n\nMake the change.", ); const store = createMockStore({ @@ -4529,7 +4610,10 @@ describe("taskCreate tool model inheritance", () => { await new TriageProcessor(store, "/test/root", { pollIntervalMs: 100_000 }).specifyTask(task); expect(liveTask).toMatchObject({ column: "in-review", status: "reviewing", worktree: "/tmp/FN-7977-MODEL", steps: [{ id: "1" }] }); - expect(store.updateTask).toHaveBeenCalledTimes(1); + expect(store.updateTask).toHaveBeenCalledWith(task.id, { status: "planning" }); + expect(store.updateTask).toHaveBeenCalledWith(task.id, { + planningStartedAt: expect.any(String), + }); }); it("keeps advanced worktree and steps after deterministic validation recovery", async () => { @@ -4559,7 +4643,10 @@ describe("taskCreate tool model inheritance", () => { await new TriageProcessor(store, "/test/root", { pollIntervalMs: 100_000 }).specifyTask(task); expect(liveTask).toMatchObject({ column: "in-progress", status: "executing", worktree: "/tmp/FN-7977-VALIDATION", steps: [{ id: "1" }] }); - expect(store.updateTask).toHaveBeenCalledTimes(1); + expect(store.updateTask).toHaveBeenCalledWith(task.id, { status: "planning" }); + expect(store.updateTask).toHaveBeenCalledWith(task.id, { + planningStartedAt: expect.any(String), + }); }); it("escalates to error state when triage retries are exhausted via specifyTask", async () => { diff --git a/packages/engine/src/__tests__/workflow-parse-steps.test.ts b/packages/engine/src/__tests__/workflow-parse-steps.test.ts index 62a2e1295f..d93465a6a5 100644 --- a/packages/engine/src/__tests__/workflow-parse-steps.test.ts +++ b/packages/engine/src/__tests__/workflow-parse-steps.test.ts @@ -15,8 +15,8 @@ import { const settingsOn = () => ({ experimentalFeatures: { workflowGraphExecutor: true } }); -function task(): TaskDetail { - return { id: "FN-PARSE", title: "t", steps: [] as TaskStep[] } as unknown as TaskDetail; +function task(overrides: Partial = {}): TaskDetail { + return { id: "FN-PARSE", title: "t", steps: [] as TaskStep[], ...overrides } as unknown as TaskDetail; } /** start → parse → end, with optional outcome edges off the parse node. */ @@ -58,9 +58,9 @@ function makeDeps(over: Partial = {}): { return { deps, written, audits }; } -async function runParse(ir: WorkflowIr, deps: ParseStepsHandlerDeps) { +async function runParse(ir: WorkflowIr, deps: ParseStepsHandlerDeps, taskOverrides: Partial = {}) { const exec = new WorkflowGraphExecutor({ seams: createNoopLegacySeams(), parseStepsDeps: deps }); - return exec.run(task(), settingsOn(), ir); + return exec.run(task(taskOverrides), settingsOn(), ir); } describe("parse-steps node handler (U12, KTD-12)", () => { @@ -146,12 +146,41 @@ describe("parse-steps node handler (U12, KTD-12)", () => { expect(audits.some((a) => a.reason === "parse-error")).toBe(true); }); - it("clean empty parse → no-steps outcome (success), writes empty list", async () => { + it("explicit no-commits empty parse → no-steps outcome (success), writes empty list", async () => { const { deps, written } = makeDeps({ readArtifact: async () => "no headings here" }); const ir = parseIr("step-headings", undefined, [ { from: "parse", to: "end", condition: "outcome:no-steps" }, ]); + ir.nodes.find((node) => node.id === "parse")!.config!.requireStepsUnlessNoCommits = true; + const result = await runParse(ir, deps, { noCommitsExpected: true }); + expect(result.outcome).toBe("success"); + expect(result.context["node:parse:value"]).toBe("no-steps"); + expect(written).toEqual([[]]); + }); + + it("empty implementation plan without no-commits authorization fails before foreach", async () => { + const { deps, written, audits } = makeDeps({ readArtifact: async () => "planning draft without executable headings" }); + const ir = parseIr("step-headings", undefined, [ + { from: "parse", to: "end", condition: "outcome:missing-implementation-steps" }, + ]); + ir.nodes.find((node) => node.id === "parse")!.config!.requireStepsUnlessNoCommits = true; + const result = await runParse(ir, deps); + + expect(result.outcome).toBe("failure"); + expect(result.context["node:parse:value"]).toBe("missing-implementation-steps"); + expect(written).toEqual([[]]); + expect(audits).toContainEqual(expect.objectContaining({ reason: "missing-implementation-steps" })); + }); + + it("preserves custom workflow zero-step behavior when the implementation guard is not enabled", async () => { + const { deps, written } = makeDeps({ readArtifact: async () => "no headings here" }); + const ir = parseIr("step-headings", undefined, [ + { from: "parse", to: "end", condition: "outcome:no-steps" }, + ]); + + const result = await runParse(ir, deps); + expect(result.outcome).toBe("success"); expect(result.context["node:parse:value"]).toBe("no-steps"); expect(written).toEqual([[]]); diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index e485650df1..d18aa9f9fb 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -31,6 +31,8 @@ import { compareTaskIdNumeric, resolveAgentMemoryInclusionMode, resolvePlanApprovalRequired, + resolveWorkflowIrForTask, + getStepParser, computePlanApprovalFingerprint, extractIntentSignature, findNearDuplicates, @@ -807,6 +809,31 @@ export class TriageProcessor { return false; } + /* + FNXC:TriageStuckRecovery 2026-07-20: + A stuck planner may leave a partially edited seed that no longer matches the + byte-exact unplanned-seed detector. For step-heading workflows, non-empty prose + is not executable proof: require parsed steps unless the plan explicitly opts + into the legitimate zero-work contract. Otherwise recovery would release the + task to parse-steps, whose empty foreach could advance toward merge. + */ + const workflow = await resolveWorkflowIrForTask(this.store, task.id).catch(() => undefined); + const requiresPromptImplementationSteps = workflow?.nodes.some((node) => + node.kind === "parse-steps" + && (node.config?.artifact === undefined || node.config.artifact === "PROMPT.md") + && node.config?.parser === "step-headings" + && node.config?.requireStepsUnlessNoCommits === true + ) === true; + if (requiresPromptImplementationSteps && !promptDeclaresNoCommitsExpected(written)) { + const parsedSteps = getStepParser("step-headings")?.parse(written).steps ?? []; + if (parsedSteps.length === 0) { + const message = "Planning recovery withheld: PROMPT.md has no executable steps and does not declare no commits expected"; + planLog.warn(`${task.id} ${message}`); + await this.store.logEntry(task.id, message); + return false; + } + } + await this.finalizeApprovedTask(task, written, settings, { recoveryLogAction: approvalRequired ? "Auto-recovered specified task stuck in planning — awaiting manual approval" @@ -2638,8 +2665,7 @@ export class TriageProcessor { the row's reviewLevel from the specified prompt. */ - const noCommitsExpectedMatch = written.match(/^\*\*No commits expected:\*\*\s*(true|yes)\b/im); - if (noCommitsExpectedMatch) { + if (promptDeclaresNoCommitsExpected(written)) { taskUpdates.noCommitsExpected = true; } @@ -3066,6 +3092,10 @@ function parseFileScopeFromPrompt(text: string): string[] { return extractEffectiveWriteScopeFromPrompt(text); } +function promptDeclaresNoCommitsExpected(text: string): boolean { + return /^\*\*No commits expected:\*\*\s*(true|yes)\b/im.test(text); +} + function extractPromptDeclaredTitle(prompt: string, taskId: string): string | null { const headingMatch = prompt.match(/^#\s+Task:\s+([A-Z]+-\d+)\s+-\s+(.+)$/m); if (!headingMatch) return null; diff --git a/packages/engine/src/workflow-node-runners/parse-steps-runner.ts b/packages/engine/src/workflow-node-runners/parse-steps-runner.ts index 02e0e0e1fb..54a9b72e03 100644 --- a/packages/engine/src/workflow-node-runners/parse-steps-runner.ts +++ b/packages/engine/src/workflow-node-runners/parse-steps-runner.ts @@ -24,7 +24,11 @@ export class ParseStepsNodeRunner implements WorkflowNodeRunner { public constructor(private readonly deps: ParseStepsHandlerDeps) {} public async run(node: WorkflowIrNode, ctx: WorkflowNodeRunnerContext): Promise { - const cfg = (node.config ?? {}) as { artifact?: unknown; parser?: unknown }; + const cfg = (node.config ?? {}) as { + artifact?: unknown; + parser?: unknown; + requireStepsUnlessNoCommits?: unknown; + }; const parserId = typeof cfg.parser === "string" ? cfg.parser : ""; const artifactKey = typeof cfg.artifact === "string" && cfg.artifact.trim() !== "" @@ -96,6 +100,13 @@ export class ParseStepsNodeRunner implements WorkflowNodeRunner { ); return { outcome: "failure", value: "parse-error" }; } + if (cfg.requireStepsUnlessNoCommits === true && ctx.task.noCommitsExpected !== true) { + this.audit( + "missing-implementation-steps", + `parse-steps node '${node.id}' found no executable steps for task ${ctx.task.id} without explicit no-commits authorization`, + ); + return { outcome: "failure", value: "missing-implementation-steps" }; + } return { outcome: "success", value: "no-steps" }; }