diff --git a/.changeset/fix-workflow-compiler-merge-region.md b/.changeset/fix-workflow-compiler-merge-region.md new file mode 100644 index 0000000000..9ad1839e01 --- /dev/null +++ b/.changeset/fix-workflow-compiler-merge-region.md @@ -0,0 +1,5 @@ +--- +"@fusion/core": patch +--- + +Fix task creation failing with "node 'merge-gate' branches into 2 edges — graphs with branches require the workflow interpreter (deferred)". The built-in coding workflow now models the merge lifecycle as a branching region of merge/retry/branch-group primitives (FN-6035), but the linear workflow compiler still tried to lower those nodes and rejected their fan-out. The compiler now treats the merge-region primitive kinds (merge-gate, merge-attempt, manual-merge-hold, retry-backoff, recovery-router, branch-group-member-integration, branch-group-promotion, pr-merge) as an engine-owned terminal boundary — exempt from the single-edge linearity rule and never lowered to a step — so linear-prefix workflows compile to their pre-merge step list again. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 39a5026f3a..154275dbda 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -138,7 +138,9 @@ describe("built-in workflows", () => { expect(byId.get("execute")?.column).toBe("in-progress"); expect(byId.get("workflow-step")?.column).toBe("in-progress"); expect(byId.get("review")?.column).toBe("in-review"); - expect(byId.get("merge")?.column).toBe("in-review"); + // Merge is the native primitive region (FN-6035), placed in in-review. + expect(byId.get("merge")).toBeUndefined(); + expect(byId.get("merge-attempt")?.column).toBe("in-review"); expect(ir.settings).toEqual(BUILTIN_WORKFLOW_SETTINGS); }); @@ -195,10 +197,11 @@ describe("built-in workflows", () => { const byId = new Map(candidate.nodes.map((node) => [node.id, node])); expect(byId.get("workflow-step")?.config?.name).toBe("Pre-merge workflow steps"); expect(byId.get("review")?.config?.name).toBe("Review"); - expect(byId.get("merge")?.config?.name).toBe("Merge boundary"); expect(byId.get("workflow-step")?.config?.maxRetries).toBeUndefined(); expect(byId.get("review")?.config?.maxRetries).toBeUndefined(); - expect(byId.get("merge")?.config?.maxRetries).toBeUndefined(); + // The merge lifecycle is no longer a single `merge` seam node (FN-6035): it + // is expressed as the merge-gate/merge-attempt/branch-group primitive region. + expect(byId.get("merge")).toBeUndefined(); } }); @@ -320,11 +323,11 @@ describe("built-in workflows", () => { const coding = getBuiltinWorkflow("builtin:coding"); const execute = coding?.ir.nodes.find((node) => node.id === "execute"); const review = coding?.ir.nodes.find((node) => node.id === "review"); - const merge = coding?.ir.nodes.find((node) => node.id === "merge"); expect((execute?.config as { prompt?: string } | undefined)?.prompt).toContain("You are a task execution agent"); expect((review?.config as { prompt?: string } | undefined)?.prompt).toContain("You are an independent code and plan reviewer"); - expect((merge?.config as { prompt?: string } | undefined)?.prompt).toContain("You are a merge agent"); + // No `merge` seam node post-FN-6035 — merge runs as native primitives. + expect(coding?.ir.nodes.find((node) => node.id === "merge")).toBeUndefined(); }); it("rejects editing or deleting a built-in", async () => { diff --git a/packages/core/src/__tests__/workflow-compiler.test.ts b/packages/core/src/__tests__/workflow-compiler.test.ts index accf4e3bd8..4f5a3a1ac8 100644 --- a/packages/core/src/__tests__/workflow-compiler.test.ts +++ b/packages/core/src/__tests__/workflow-compiler.test.ts @@ -120,6 +120,40 @@ describe("compileWorkflowToSteps (U2)", () => { expect(() => compileWorkflowToSteps(ir)).toThrow(/interpreter \(deferred\)/i); }); + it("compiles a workflow whose post-review merge region branches into primitives (FN-6035)", () => { + // Mirrors the builtin:coding shape: review → merge-gate fans out into the + // engine-owned merge/branch-group/retry subgraph. These primitive kinds are a + // terminal boundary, so the graph still compiles to its pre-merge step list + // instead of failing as interpreter-only. + const ir: WorkflowIr = { + version: "v1", + name: "merge-region", + nodes: [ + { id: "start", kind: "start" }, + { id: "spec", kind: "prompt", config: { name: "Spec", prompt: "spec" } }, + { id: "review", kind: "prompt", config: { seam: "review" } }, + { id: "merge-gate", kind: "merge-gate", config: { gate: "auto-merge" } }, + { id: "merge-attempt", kind: "merge-attempt" }, + { id: "merge-hold", kind: "manual-merge-hold" }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "spec", condition: "success" }, + { from: "spec", to: "review", condition: "success" }, + { from: "review", to: "merge-gate", condition: "success" }, + { from: "review", to: "end", condition: "failure" }, + { from: "merge-gate", to: "merge-attempt", condition: "outcome:auto-on" }, + { from: "merge-gate", to: "merge-hold", condition: "outcome:auto-off" }, + { from: "merge-attempt", to: "end", condition: "success" }, + { from: "merge-hold", to: "merge-attempt", condition: "success" }, + ], + }; + expect(validateLinearity(parseWorkflowIr(ir))).toBeNull(); + const steps = compileWorkflowToSteps(ir); + // Only the pre-merge user node lowers; the merge primitives emit no steps. + expect(steps.map((s) => s.name)).toEqual(["Spec"]); + }); + it("rejects a graph missing the start/end nodes via parse", () => { const ir = { version: "v1", name: "x", nodes: [{ id: "p", kind: "prompt" }], edges: [] } as WorkflowIr; expect(() => compileWorkflowToSteps(ir)).toThrow(); diff --git a/packages/core/src/workflow-compiler.ts b/packages/core/src/workflow-compiler.ts index 1b6247b6e9..b6918c2391 100644 --- a/packages/core/src/workflow-compiler.ts +++ b/packages/core/src/workflow-compiler.ts @@ -20,6 +20,30 @@ export class WorkflowCompileError extends Error { * not emitted as steps. */ const SEAM_NAMES = new Set(["planning", "execute", "workflow-step", "review", "merge"]); +/** Workflow-owned merge/retry/recovery policy node kinds (FN-6035). After review, + * the builtin workflows express the merge lifecycle as a branching subgraph of + * these primitives instead of the single legacy `merge` seam node. The linear + * compiler treats the whole region as one engine-owned terminal boundary: it is + * exempt from the single-outgoing-edge rule, never lowered to a WorkflowStep, and + * ends the linear walk (the legacy pipeline runs the merge lifecycle natively, + * the graph interpreter runs the branches). This keeps `builtin:coding` and other + * linear-prefix workflows compilable to their pre-merge step list rather than + * failing as "interpreter (deferred)". */ +const MERGE_REGION_KINDS = new Set([ + "merge-gate", + "merge-attempt", + "manual-merge-hold", + "retry-backoff", + "recovery-router", + "branch-group-member-integration", + "branch-group-promotion", + "pr-merge", +]); + +function isMergeRegion(node: WorkflowIrNode): boolean { + return MERGE_REGION_KINDS.has(node.kind); +} + function seamOf(node: WorkflowIrNode): string | undefined { const seam = node.config?.seam; return typeof seam === "string" && SEAM_NAMES.has(seam) ? seam : undefined; @@ -75,6 +99,11 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { continue; } + // Merge-region primitives are an engine-owned terminal boundary (FN-6035): + // they legitimately branch (e.g. merge-gate's auto-on/auto-off outcome edges) + // and are never lowered to steps, so they are exempt from the linearity rules. + if (isMergeRegion(node)) continue; + const seam = seamOf(node); if (seam) { const failureEdges = outs.filter((edge) => edge.condition === "failure"); @@ -121,10 +150,18 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { const seenSeams = new Set(); let nextExpectedSeamIndex = 0; const visited = new Set(); + // Reaching the engine-owned merge region counts as reaching the terminal + // lifecycle: the linear walk stops there and the branching merge subgraph + // (plus the end node it eventually leads to) is owned by the merge runtime. + let reachedTerminal = false; let cursor: string | undefined = startNode.id; while (cursor && !visited.has(cursor)) { visited.add(cursor); const node = nodesById.get(cursor); + if (node && isMergeRegion(node)) { + reachedTerminal = true; + break; + } const seam = node ? seamOf(node) : undefined; if (seam) { if (seenSeams.has(seam)) { @@ -144,13 +181,21 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { seenSeams.add(seam); nextExpectedSeamIndex += 1; } - if (cursor === endNode.id) break; + if (cursor === endNode.id) { + reachedTerminal = true; + break; + } cursor = mainEdge(outgoing.get(cursor) ?? [])?.to; } - if (!visited.has(endNode.id)) { + if (!reachedTerminal) { return new WorkflowCompileError("workflow main path does not reach the end node"); } - const unreached = ir.nodes.filter((node) => !visited.has(node.id)); + // Merge-region nodes and the end node may be reached only through the branching + // merge subgraph (not the linear walk), so they are not required to appear on the + // pre-merge main path. Every other node must. + const unreached = ir.nodes.filter( + (node) => !visited.has(node.id) && node.kind !== "end" && !isMergeRegion(node), + ); if (unreached.length > 0) { return new WorkflowCompileError( `node '${unreached[0].id}' is not on the main path — disconnected nodes require the workflow interpreter (deferred)`, @@ -236,6 +281,11 @@ export function compileWorkflowToSteps(ir: WorkflowIr): WorkflowStepInput[] { const node = nodesById.get(cursor); if (!node) break; + // The merge region is an engine-owned terminal boundary: it carries no + // lowerable user steps and ends the linear lowering walk (mirrors how the + // legacy `merge` seam terminated the pre-merge chain). + if (isMergeRegion(node)) break; + const seam = seamOf(node); if (seam === "merge") { phase = "post-merge";