diff --git a/.changeset/fast-coding-executes-before-merge.md b/.changeset/fast-coding-executes-before-merge.md new file mode 100644 index 0000000000..b07c815257 --- /dev/null +++ b/.changeset/fast-coding-executes-before-merge.md @@ -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. diff --git a/packages/core/src/__tests__/agent-prompts.test.ts b/packages/core/src/__tests__/agent-prompts.test.ts index 97d8c9b2bb..716a2af683 100644 --- a/packages/core/src/__tests__/agent-prompts.test.ts +++ b/packages/core/src/__tests__/agent-prompts.test.ts @@ -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: "); + 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); diff --git a/packages/core/src/__tests__/step-parsers.test.ts b/packages/core/src/__tests__/step-parsers.test.ts index 251cd44d4d..bd9273e0c2 100644 --- a/packages/core/src/__tests__/step-parsers.test.ts +++ b/packages/core/src/__tests__/step-parsers.test.ts @@ -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", () => { diff --git a/packages/core/src/agent-prompts.ts b/packages/core/src/agent-prompts.ts index 5fb2fb407e..a4cc5031be 100644 --- a/packages/core/src/agent-prompts.ts +++ b/packages/core/src/agent-prompts.ts @@ -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: \` (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\`. diff --git a/packages/core/src/step-parsers.ts b/packages/core/src/step-parsers.ts index bea91f2aa5..8c9dd655e1 100644 --- a/packages/core/src/step-parsers.ts +++ b/packages/core/src/step-parsers.ts @@ -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 { diff --git a/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts b/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts index 6a00c72ddd..cf9defc2f9 100644 --- a/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts +++ b/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts @@ -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); }); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 96d5fc4d2e..007c086775 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -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 { + 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.