fix(FN-7271): keep fast coding tasks from skipping implementation
This commit is contained in:
7
.changeset/fast-coding-executes-before-merge.md
Normal file
7
.changeset/fast-coding-executes-before-merge.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Prevent fast Coding tasks from merging before implementation runs.
|
||||
category: fix
|
||||
dev: Fast mode now requires implementation proof at the workflow merge boundary.
|
||||
@@ -287,6 +287,8 @@ describe("resolveAgentPrompt", () => {
|
||||
expect(BUILTIN_SEAM_PROMPTS["planning-fast"]).toBe(fastTemplate?.prompt);
|
||||
expect(fastPrompt).toBe(fastTemplate?.prompt);
|
||||
expect(fastPrompt).toContain("This task is running in **fast mode**");
|
||||
expect(fastPrompt).toContain("### Step N: <name>");
|
||||
expect(fastPrompt).toContain("Do not write bare `### Preflight` / `### Implementation` headings");
|
||||
expect(fastPrompt).not.toContain("## Review Level");
|
||||
expect(fastPrompt.length).toBeLessThan(standardPrompt.length / 3);
|
||||
expect(fastPrompt.length).toBeLessThan(6000);
|
||||
|
||||
@@ -101,6 +101,49 @@ describe("step-parsers registry (U12, KTD-12)", () => {
|
||||
{ name: "Preflight", status: "pending" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("parses plain third-level headings inside the Steps section when legacy step headings are absent", () => {
|
||||
const content = `# Task
|
||||
|
||||
## Mission
|
||||
|
||||
### Not a task step
|
||||
|
||||
## Steps
|
||||
|
||||
### Preflight
|
||||
|
||||
- [ ] inspect
|
||||
|
||||
### Implementation
|
||||
|
||||
### Testing & Verification
|
||||
|
||||
## Do NOT
|
||||
|
||||
### Also not a task step
|
||||
`;
|
||||
expect(headings().parse(content).steps).toEqual([
|
||||
{ name: "Preflight" },
|
||||
{ name: "Implementation" },
|
||||
{ name: "Testing & Verification" },
|
||||
]);
|
||||
expect(parseStepHeadings(content)).toEqual([
|
||||
{ name: "Preflight", status: "pending" },
|
||||
{ name: "Implementation", status: "pending" },
|
||||
{ name: "Testing & Verification", status: "pending" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps legacy Step N headings authoritative when both styles appear", () => {
|
||||
const content = `## Steps
|
||||
|
||||
### Preflight
|
||||
|
||||
### Step 1: Implementation
|
||||
`;
|
||||
expect(headings().parse(content).steps).toEqual([{ name: "Implementation" }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("json-steps built-in", () => {
|
||||
|
||||
@@ -232,7 +232,7 @@ Write a lean, executable PROMPT.md quickly. Preserve safety-critical gates, but
|
||||
Before writing a spec, call \`fn_task_list\` for active work, then call \`fn_task_search\` with 2-4 targeted keyword phrases from the title/description, such as file paths, symptoms, and symbols. For any likely match in \`done\` or \`archived\`, call \`fn_task_show\` and inspect it before deciding. If an existing task covers the same work, do not write PROMPT.md; write exactly \`DUPLICATE: {existing-task-id}\`.
|
||||
|
||||
## Required PROMPT.md shape
|
||||
Write a real PROMPT.md to the requested path using the write tool. Keep sections lean, but include Mission, Dependencies, Context to Read First, File Scope, Steps with Preflight / Testing & Verification / Documentation & Delivery, Documentation Requirements, Completion Criteria, Git Commit Convention, and Do NOT. Do not add a review-level heading, triage subtask breakdown, or proactive subtask breakdown.
|
||||
Write PROMPT.md with Mission, Dependencies, Context to Read First, File Scope, Steps, Documentation Requirements, Completion Criteria, Git Commit Convention, and Do NOT. In \`## Steps\`, every executable heading MUST use \`### Step N: <name>\` (for example, \`### Step 1: Preflight\`); Do not write bare \`### Preflight\` / \`### Implementation\` headings. Do not add review-level, triage subtask, or proactive subtask headings.
|
||||
|
||||
## Surface Enumeration
|
||||
For bug fixes and UI-affordance add/remove tasks, the spec MUST include a \`## Surface Enumeration\` section. The workflow Plan Review gate validates this before execution when plan review is enabled.
|
||||
@@ -434,7 +434,7 @@ files with assertions that run via a test runner. Typechecks and builds are NOT
|
||||
tests. Manual verification is NOT a test.
|
||||
|
||||
- Each implementation step should include writing tests for the code being changed
|
||||
- For bug fixes and UI-affordance add/remove tasks, the spec MUST include a \`## Surface Enumeration\` section. The workflow Plan Review gate validates this before execution when plan review is enabled.
|
||||
- For bug fixes and UI-affordance add/remove tasks, the spec MUST include a \`## Surface Enumeration\` section. The workflow Plan Review gate validates this before execution when plan review is enabled; missing coverage is a blocking REVISE.
|
||||
- For bug fixes and UI-affordance add/remove tasks, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers; every component that renders the affordance; leftover shells after removal.
|
||||
- For bug fixes and UI-affordance add/remove tasks, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, empty/undefined/populated data states, and for UI-affordance changes every component rendering the affordance plus leftover shells after removal — not just the reported repro (see FN-5787/FN-5789/FN-5803, FN-5751, and FN-6115/FN-6118/FN-6123)
|
||||
- For bug-class/bug-fix tasks, the spec MUST include a \`## Symptom Verification\` section with **Original symptom**, **Exact reproduction**, and **Assertion it is gone**. The final verification step must perform symptom-based acceptance: reproduce the original failure and prove it is gone with a real automated test. Green build/tests alone are insufficient. Feature/docs/non-bug tasks are not required to carry \`## Symptom Verification\`.
|
||||
|
||||
@@ -5,8 +5,9 @@
|
||||
* a parser id to an implementation that reads an artifact's content and yields a
|
||||
* canonical step list. Built-ins:
|
||||
* - `step-headings` — the extracted `parseStepsFromPrompt` logic (the
|
||||
* `### Step N:` regex + `(depends: …)` annotation from U1); legacy callers
|
||||
* in `store.ts` delegate to this exact function (byte-identical parity).
|
||||
* `### Step N:` regex + `(depends: …)` annotation from U1), plus a bounded
|
||||
* fallback for plain `### Heading` entries inside `## Steps`; legacy callers
|
||||
* in `store.ts` delegate to this exact function.
|
||||
* - `json-steps` — a structured `[{ name, depends? }]` JSON document for
|
||||
* workflows that plan in JSON.
|
||||
*
|
||||
@@ -228,9 +229,31 @@ export function parseStepHeadings(content: string): TaskStep[] {
|
||||
if (fallbackName) steps.push({ name: fallbackName, status: "pending" });
|
||||
}
|
||||
}
|
||||
if (steps.length > 0) return steps;
|
||||
|
||||
const stepsSection = extractStepsSection(content);
|
||||
if (!stepsSection) return steps;
|
||||
const plainHeadingRegex = /^###\s+(?!Step\s+\d+\b)(.+?)\s*$/gm;
|
||||
while ((match = plainHeadingRegex.exec(stepsSection)) !== null) {
|
||||
const name = match[1].trim();
|
||||
if (name) steps.push({ name, status: "pending" });
|
||||
}
|
||||
return steps;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowSteps 2026-06-30-00:54:
|
||||
Default Coding parses PROMPT.md before step execution. FN-7260/FN-7271 specs used plain `### Preflight`/`### Implementation` headings under `## Steps`; the previous parser returned zero steps, so fast-mode tasks reached merge with no implementation session. Accept plain third-level headings only inside the Steps section, and only when no legacy `### Step N:` headings were found, so unrelated spec sections do not become executable work.
|
||||
*/
|
||||
function extractStepsSection(content: string): string | undefined {
|
||||
const sectionMatch = /^##\s+Steps\s*$/gim.exec(content);
|
||||
if (!sectionMatch) return undefined;
|
||||
const start = sectionMatch.index + sectionMatch[0].length;
|
||||
const rest = content.slice(start);
|
||||
const nextSection = /^##\s+(?!#)/gm.exec(rest);
|
||||
return nextSection ? rest.slice(0, nextSection.index) : rest;
|
||||
}
|
||||
|
||||
/** Parse a `depends:` value list (1-indexed step numbers) into 0-indexed,
|
||||
* deduped, sorted indices. Returns null if any token is not a positive integer. */
|
||||
function parseDependsList(raw: string): number[] | null {
|
||||
|
||||
@@ -131,20 +131,32 @@ describe("fast mode workflow/runtime invariants", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// U6: the coding built-in's pre-merge browser-verification optional-group is
|
||||
// default-OFF (the task sets no enabledWorkflowSteps), so it is bypassed — its
|
||||
// group node is visited but its body never runs and runWorkflowSteps is not
|
||||
// called. Fast mode is irrelevant to a bypassed group; the seam is simply gone.
|
||||
it("graph executor with builtin:coding selection bypasses the disabled browser-verification group", async () => {
|
||||
const { executor } = makeExecutorForTask(task({ executionMode: "fast", worktree: "/tmp/wt" }));
|
||||
// U4 (KTD-2): runWorkflowSteps + the workflow-step seam were removed; workflow
|
||||
// gates run as graph optional-group nodes only.
|
||||
it("fast builtin:coding still parses and executes steps while disabled optional groups stay inert", async () => {
|
||||
const calls: string[] = [];
|
||||
const prompt = "# Task\n\n## Steps\n\n### Step 1: Do the work\n- [ ] edit files";
|
||||
const taskSteps = [{ name: "Do the work", status: "pending" }];
|
||||
const seams = {
|
||||
planning: vi.fn(async () => ({ outcome: "success", value: "planned" })),
|
||||
execute: vi.fn(async () => ({ outcome: "success", value: "implemented" })),
|
||||
review: vi.fn(async () => ({ outcome: "success", value: "approved" })),
|
||||
merge: vi.fn(async () => ({ outcome: "success", value: "merged" })),
|
||||
planning: vi.fn(async () => {
|
||||
calls.push("plan");
|
||||
return { outcome: "success", value: "planned" };
|
||||
}),
|
||||
execute: vi.fn(async () => {
|
||||
calls.push("legacy-execute");
|
||||
return { outcome: "success", value: "implemented" };
|
||||
}),
|
||||
review: vi.fn(async () => {
|
||||
calls.push("review");
|
||||
return { outcome: "success", value: "approved" };
|
||||
}),
|
||||
merge: vi.fn(async () => {
|
||||
calls.push("merge");
|
||||
return { outcome: "success", value: "merged" };
|
||||
}),
|
||||
schedule: vi.fn(async () => ({ outcome: "success", value: "scheduled" })),
|
||||
stepExecute: vi.fn(async (_task, context) => {
|
||||
calls.push(`step-execute:${context["foreach:active"]?.stepIndex}`);
|
||||
return { outcome: "success", value: "step-done" };
|
||||
}),
|
||||
};
|
||||
const runner = new WorkflowGraphTaskRunner({
|
||||
store: {
|
||||
@@ -152,16 +164,134 @@ describe("fast mode workflow/runtime invariants", () => {
|
||||
getWorkflowDefinition: vi.fn(async (id: string) => getBuiltinWorkflow(id)),
|
||||
},
|
||||
seams,
|
||||
runCustomNode: vi.fn(async () => ({ outcome: "failure", value: "unexpected-custom-node" })),
|
||||
parseStepsDeps: {
|
||||
readArtifact: async (_target, key) => key === "PROMPT.md" ? prompt : undefined,
|
||||
writeSteps: async (target) => {
|
||||
calls.push("parse");
|
||||
target.steps = taskSteps;
|
||||
},
|
||||
},
|
||||
runCustomNode: vi.fn(async (node) => {
|
||||
calls.push(`custom:${node.id}`);
|
||||
return { outcome: "success", value: "custom-ok" };
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await runner.run(task({ id: "FN-6226", executionMode: "fast" }), { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
const result = await runner.run(task({
|
||||
id: "FN-6226",
|
||||
executionMode: "fast",
|
||||
enabledWorkflowSteps: [],
|
||||
prompt,
|
||||
}), { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
|
||||
expect(result.disposition).toBe("completed");
|
||||
expect(result.visitedNodeIds).toContain("parse");
|
||||
expect(result.visitedNodeIds).toContain("steps#0:step-execute");
|
||||
expect(result.visitedNodeIds).toContain("browser-verification");
|
||||
expect(result.visitedNodeIds).not.toContain("browser-verification::browser-verification-step");
|
||||
expect(result.visitedNodeIds).toContain("code-review");
|
||||
expect(result.visitedNodeIds).not.toContain("code-review::code-review-step");
|
||||
expect(result.visitedNodeIds).not.toContain("workflow-step");
|
||||
expect(seams.review).toHaveBeenCalledTimes(1);
|
||||
expect(calls).toContain("parse");
|
||||
expect(calls).toContain("step-execute:0");
|
||||
expect(calls).not.toContain("legacy-execute");
|
||||
expect(seams.review).not.toHaveBeenCalled();
|
||||
expect(seams.merge).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks fast builtin:coding merge when parsed implementation proof is missing", async () => {
|
||||
const liveTask = task({
|
||||
id: "FN-7271",
|
||||
executionMode: "fast",
|
||||
enabledWorkflowSteps: [],
|
||||
column: "in-progress",
|
||||
steps: [],
|
||||
prompt: "# Task\n\n## Steps\n\n### Step 1: Do the work\n- [ ] edit files",
|
||||
});
|
||||
const store = createMockStore();
|
||||
store.getTask.mockResolvedValue(liveTask);
|
||||
store.getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "builtin:coding", stepIds: [] }));
|
||||
store.getWorkflowDefinition = vi.fn(async (id: string) => getBuiltinWorkflow(id));
|
||||
store.moveTask.mockResolvedValue({ ...liveTask, column: "in-review" });
|
||||
const executor = new TaskExecutor(store, "/tmp/test") as any;
|
||||
const mergeRequester = vi.fn(async () => ({ merged: true }));
|
||||
executor.setMergeRequester(mergeRequester);
|
||||
|
||||
const result = await executor.createAuthoritativeWorkflowPrimitives({ autoMerge: true }).requestMerge(
|
||||
{
|
||||
run: { runId: "FN-7271:builtin:coding", taskId: "FN-7271", workflowId: "builtin-stepwise-final-review-coding" },
|
||||
node: { node: { id: "merge" } },
|
||||
},
|
||||
liveTask,
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
outcome: "failure",
|
||||
value: "implementation-incomplete",
|
||||
data: { reason: "implementation-incomplete" },
|
||||
});
|
||||
expect(mergeRequester).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-7271",
|
||||
expect.stringContaining("Workflow merge blocked before requester: implementation did not run"),
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it("fast builtin:coding executes plain Steps-section headings from fast triage specs", async () => {
|
||||
const calls: string[] = [];
|
||||
const prompt = `# Task
|
||||
|
||||
## Steps
|
||||
|
||||
### Preflight
|
||||
- [ ] inspect
|
||||
|
||||
### Implementation
|
||||
- [ ] edit
|
||||
|
||||
### Testing & Verification
|
||||
- [ ] test
|
||||
`;
|
||||
const seams = {
|
||||
planning: vi.fn(async () => ({ outcome: "success", value: "planned" })),
|
||||
execute: vi.fn(async () => ({ outcome: "success", value: "implemented" })),
|
||||
review: vi.fn(async () => ({ outcome: "success", value: "approved" })),
|
||||
merge: vi.fn(async () => ({ outcome: "success", value: "merged" })),
|
||||
schedule: vi.fn(async () => ({ outcome: "success", value: "scheduled" })),
|
||||
stepExecute: vi.fn(async (_task, context) => {
|
||||
calls.push(`step-execute:${context["foreach:active"]?.stepIndex}`);
|
||||
return { outcome: "success", value: "step-done" };
|
||||
}),
|
||||
};
|
||||
const runner = new WorkflowGraphTaskRunner({
|
||||
store: {
|
||||
getTaskWorkflowSelection: () => ({ workflowId: "builtin:coding", stepIds: [] }),
|
||||
getWorkflowDefinition: vi.fn(async (id: string) => getBuiltinWorkflow(id)),
|
||||
},
|
||||
seams,
|
||||
parseStepsDeps: {
|
||||
readArtifact: async (_target, key) => key === "PROMPT.md" ? prompt : undefined,
|
||||
writeSteps: async (target, steps) => {
|
||||
target.steps = steps;
|
||||
},
|
||||
},
|
||||
runCustomNode: vi.fn(async () => ({ outcome: "success" })),
|
||||
});
|
||||
|
||||
const result = await runner.run(task({
|
||||
id: "FN-7260",
|
||||
executionMode: "fast",
|
||||
enabledWorkflowSteps: [],
|
||||
prompt,
|
||||
}), { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
|
||||
expect(result.disposition).toBe("completed");
|
||||
expect(result.visitedNodeIds).toContain("steps#0:step-execute");
|
||||
expect(result.visitedNodeIds).toContain("steps#1:step-execute");
|
||||
expect(result.visitedNodeIds).toContain("steps#2:step-execute");
|
||||
expect(calls).toEqual(["step-execute:0", "step-execute:1", "step-execute:2"]);
|
||||
expect(seams.merge).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
|
||||
@@ -5811,7 +5811,24 @@ export class TaskExecutor {
|
||||
/*
|
||||
FNXC:WorkflowMerge 2026-06-29-23:18:
|
||||
FN-7261 reached the merge node in fast mode with every legacy implementation step still pending, producing a no-op merge proof for work that never ran. A graph-native workflow may project its checklist at the merge boundary only when node workflow results prove implementation completed; otherwise incomplete legacy steps are authoritative and merge must fail before the merger can create stale no-op proof.
|
||||
|
||||
FNXC:WorkflowMerge 2026-06-30-00:38:
|
||||
Fast default Coding tasks must still execute implementation work. FN-7260/FN-7271 reached merge with no parsed task steps, no foreach instances, and no implementation proof, then finalized through no-op merge. The workflow merge boundary must fail before requesting merge when a coding workflow has not produced implementation evidence; fast mode only bypasses review/verification gates.
|
||||
*/
|
||||
const missingImplementationProof = await this.getWorkflowMergeImplementationProofFailure(mergeTask);
|
||||
if (missingImplementationProof) {
|
||||
await this.store.logEntry(
|
||||
mergeTask.id,
|
||||
`Workflow merge blocked before requester: ${missingImplementationProof}`,
|
||||
undefined,
|
||||
this.getRunContextFor(mergeTask.id),
|
||||
);
|
||||
return {
|
||||
outcome: "failure",
|
||||
value: "implementation-incomplete",
|
||||
data: { status: "failed", reason: "implementation-incomplete" },
|
||||
};
|
||||
}
|
||||
if (hasNonTerminalWorkflowSteps(mergeTask)) {
|
||||
await this.store.logEntry(
|
||||
mergeTask.id,
|
||||
@@ -5962,6 +5979,50 @@ export class TaskExecutor {
|
||||
return { ...live, column: "in-review" };
|
||||
}
|
||||
|
||||
private async getWorkflowMergeImplementationProofFailure(task: TaskDetail): Promise<string | undefined> {
|
||||
if (task.noCommitsExpected === true) return undefined;
|
||||
|
||||
let ir: WorkflowIr | undefined;
|
||||
try {
|
||||
ir = await resolveWorkflowIrForTask(this.store, task.id);
|
||||
} catch {
|
||||
ir = undefined;
|
||||
}
|
||||
if (!ir) return undefined;
|
||||
|
||||
const usesParsedSteps = ir.nodes.some((node) => node.kind === "parse-steps");
|
||||
const usesExecuteSeam = ir.nodes.some((node) => node.kind === "prompt" && node.config?.seam === "execute");
|
||||
if (!usesParsedSteps && !usesExecuteSeam) return undefined;
|
||||
|
||||
const steps = Array.isArray(task.steps) ? task.steps : [];
|
||||
const hasTerminalParsedSteps =
|
||||
steps.length > 0 && steps.every((step) => step.status === "done" || step.status === "skipped");
|
||||
const hasModifiedFiles = (task.modifiedFiles?.length ?? 0) > 0;
|
||||
const hasGraphNativeImplementationProof = (task.workflowStepResults ?? []).some((result) =>
|
||||
result.source === "node"
|
||||
&& (result.phase ?? "pre-merge") === "pre-merge"
|
||||
&& (result.status === "passed" || result.status === "skipped")
|
||||
);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowMerge 2026-06-30-00:38:
|
||||
Stepwise Coding proves implementation through parsed task steps/foreach projection. Legacy monolithic Coding may prove through modified files or explicit no-op completion. Do not accept an empty step list as success for parse-step workflows; a valid PROMPT.md with unparsed steps must resume execution, not no-op merge.
|
||||
*/
|
||||
if (usesParsedSteps) {
|
||||
return hasTerminalParsedSteps || hasGraphNativeImplementationProof
|
||||
? undefined
|
||||
: "implementation did not run: parsed coding steps are missing or incomplete";
|
||||
}
|
||||
|
||||
if (usesExecuteSeam) {
|
||||
return hasTerminalParsedSteps || hasModifiedFiles || hasGraphNativeImplementationProof
|
||||
? undefined
|
||||
: "implementation did not run: execute seam has no completion proof";
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
private shouldCompleteChecklistAtWorkflowMerge(task: TaskDetail): boolean {
|
||||
if (!Array.isArray(task.steps) || task.steps.length === 0) return false;
|
||||
if (task.steps.every((step) => step.status === "done" || step.status === "skipped")) return false;
|
||||
@@ -6051,6 +6112,16 @@ export class TaskExecutor {
|
||||
workflowId: "legacy-seams",
|
||||
runId: this.getRunContextFor(seamTask.id)?.runId ?? "legacy-seam",
|
||||
});
|
||||
const missingImplementationProof = await this.getWorkflowMergeImplementationProofFailure(mergeTask);
|
||||
if (missingImplementationProof) {
|
||||
await this.store.logEntry(
|
||||
mergeTask.id,
|
||||
`Workflow merge blocked before requester: ${missingImplementationProof}`,
|
||||
undefined,
|
||||
this.getRunContextFor(mergeTask.id),
|
||||
);
|
||||
return { outcome: "failure", value: "implementation-incomplete" };
|
||||
}
|
||||
// Bound the wait: a wedged merge queue must not strand the graph walk
|
||||
// holding the routing claim. On timeout the run fails cleanly and the
|
||||
// task is parked for human review; the queue can still finish later.
|
||||
|
||||
Reference in New Issue
Block a user