fix(FN-1320): block incomplete plans before execution

Require executable steps before recovering stuck planning tasks or advancing the built-in coding workflow. Preserve explicitly authorized no-commit tasks and custom zero-step workflow behavior.
This commit is contained in:
gsxdsm
2026-07-20 16:08:00 -07:00
parent 606c320c52
commit 9ad97317cb
7 changed files with 221 additions and 19 deletions

View File

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

View File

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

View File

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

View File

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

View File

@@ -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> = {}): 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<ParseStepsHandlerDeps> = {}): {
return { deps, written, audits };
}
async function runParse(ir: WorkflowIr, deps: ParseStepsHandlerDeps) {
async function runParse(ir: WorkflowIr, deps: ParseStepsHandlerDeps, taskOverrides: Partial<TaskDetail> = {}) {
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([[]]);

View File

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

View File

@@ -24,7 +24,11 @@ export class ParseStepsNodeRunner implements WorkflowNodeRunner {
public constructor(private readonly deps: ParseStepsHandlerDeps) {}
public async run(node: WorkflowIrNode, ctx: WorkflowNodeRunnerContext): Promise<WorkflowNodeResult> {
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" };
}