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:
7
.changeset/guard-empty-implementation-plans.md
Normal file
7
.changeset/guard-empty-implementation-plans.md
Normal 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.
|
||||
@@ -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
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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([[]]);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user