From 44b756d927aaec2e4b81faa60161ec648de5c690 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 11 Jun 2026 22:48:59 -0700 Subject: [PATCH] FN-6258: allow deferred built-in workflow selection Allow branching built-in workflows to stay selectable while deferring legacy step materialization.\n\n- Treat interpreter-deferred built-in compile errors as valid zero-step selections or default fallbacks.\n- Update builtin:coding merge-region layout expectations and documentation for workflow-native merge primitives.\n- Cover explicit selection, create-time selection, and project-default fallback cases for deferred built-ins.\n- Add a patch changeset for the published CLI package.\n\nFiles changed:\n .changeset/fuzzy-workflows-branching.md | 5 ++\n docs/workflow-steps.md | 4 +-\n .../core/src/__tests__/builtin-workflows.test.ts | 53 +++++++++++++++--\n packages/core/src/builtin-workflows.ts | 10 +++-\n packages/core/src/store.ts | 43 ++++++++++----\n packages/core/src/workflow-compiler.ts | 66 +++++-----------------\n 6 files changed, 109 insertions(+), 72 deletions(-) Fusion-Task-Id: FN-6258 Fusion-Task-Lineage: 0958f522-029e-4f2a-9292-b408c2a4c208 --- .changeset/fuzzy-workflows-branching.md | 5 ++ docs/workflow-steps.md | 4 +- .../src/__tests__/builtin-workflows.test.ts | 53 +++++++++++++-- packages/core/src/builtin-workflows.ts | 10 ++- packages/core/src/store.ts | 43 +++++++++--- packages/core/src/workflow-compiler.ts | 66 ++++--------------- 6 files changed, 109 insertions(+), 72 deletions(-) create mode 100644 .changeset/fuzzy-workflows-branching.md diff --git a/.changeset/fuzzy-workflows-branching.md b/.changeset/fuzzy-workflows-branching.md new file mode 100644 index 0000000000..09c7f03fdc --- /dev/null +++ b/.changeset/fuzzy-workflows-branching.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix built-in branching workflow selection so interpreter-deferred coding workflows can be selected or used as project defaults without throwing during legacy step materialization. diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index b508da9152..1e4bd5bce9 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -34,9 +34,9 @@ The workflow runtime is the authoritative execution path for task lifecycle work The engine remains the substrate for scheduler dispatch, routing claims, persistence, concurrency limits, process supervision, storage, and audit plumbing. Lifecycle policy belongs in built-in or custom workflows. -The default built-in catalog entry `builtin:coding` is backed by the canonical `BUILTIN_CODING_WORKFLOW_IR`, which is also the resolver/runtime fallback for tasks with no workflow selection or an explicit default selection. Missing/corrupt explicit custom selections fail closed as workflow-resolution failures instead of silently running the default. The built-in IR encodes the legacy lifecycle path as graph stages: +The default built-in catalog entry `builtin:coding` is backed by the canonical `BUILTIN_CODING_WORKFLOW_IR`, which is also the resolver/runtime fallback for tasks with no workflow selection or an explicit default selection. Missing/corrupt explicit custom selections fail closed as workflow-resolution failures instead of silently running the default. The built-in IR encodes the legacy lifecycle path as graph stages, with merge represented by workflow-native policy primitives rather than a single linear merge seam: -- `triage/planning` → `execute` → `workflow-step` → `review` → `merge` → `end` +- `triage/planning` → `execute` → `workflow-step` → `review` → `merge-gate` / branch-group integration / `merge-attempt` / retry or manual hold → `end` `builtin:stepwise-coding` is a separate graph variant backed by `BUILTIN_STEPWISE_CODING_WORKFLOW_IR`; it keeps the same lifecycle columns/traits while modeling per-step parse/execute/review/rework as authored graph structure. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 154275dbda..ecca0a172c 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -20,7 +20,7 @@ const EXECUTE_NODE_MAX_RETRIES = 2; describe("built-in workflows", () => { // Non-compiler built-ins model graph-only node kinds or reusable fragments the // linear compiler cannot lower to a step list. They still must parse as valid IR. - const NON_COMPILABLE_BUILTIN_IDS = new Set(["builtin:stepwise-coding", "builtin:pr-workflow"]); + const NON_COMPILABLE_BUILTIN_IDS = new Set(["builtin:coding", "builtin:stepwise-coding", "builtin:pr-workflow"]); it("every built-in has a valid IR; linear built-ins compile without error", () => { expect(BUILTIN_WORKFLOWS.length).toBeGreaterThanOrEqual(4); @@ -140,7 +140,13 @@ describe("built-in workflows", () => { expect(byId.get("review")?.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-gate")?.column).toBe("in-review"); + expect(byId.get("merge-retry")?.column).toBe("in-review"); + expect(byId.get("merge-manual-hold")?.column).toBe("in-review"); + expect(byId.get("branch-group-member-integration")?.column).toBe("in-review"); + expect(byId.get("branch-group-promotion")?.column).toBe("in-review"); expect(byId.get("merge-attempt")?.column).toBe("in-review"); + expect(byId.get("recovery-router")?.column).toBe("in-review"); expect(ir.settings).toEqual(BUILTIN_WORKFLOW_SETTINGS); }); @@ -202,6 +208,13 @@ describe("built-in workflows", () => { // 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(); + expect(byId.get("merge-gate")?.kind).toBe("merge-gate"); + expect(byId.get("merge-retry")?.kind).toBe("retry-backoff"); + expect(byId.get("merge-manual-hold")?.kind).toBe("manual-merge-hold"); + expect(byId.get("branch-group-member-integration")?.kind).toBe("branch-group-member-integration"); + expect(byId.get("branch-group-promotion")?.kind).toBe("branch-group-promotion"); + expect(byId.get("merge-attempt")?.kind).toBe("merge-attempt"); + expect(byId.get("recovery-router")?.kind).toBe("recovery-router"); } }); @@ -337,10 +350,40 @@ describe("built-in workflows", () => { await expect(store.deleteWorkflowDefinition("builtin:coding")).rejects.toThrow(/cannot be deleted/i); }); - it("a task can select a built-in workflow", async () => { - const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); - await store.selectTaskWorkflow(task.id, "builtin:coding"); - expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe("builtin:coding"); + it("interpreter-deferred built-ins can be selected without compile materialization", async () => { + for (const workflowId of ["builtin:coding", "builtin:stepwise-coding"]) { + const task = await store.createTask({ description: `select ${workflowId}`, enabledWorkflowSteps: [] }); + + await expect(store.selectTaskWorkflow(task.id, workflowId)).resolves.toEqual([]); + + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps ?? []).toEqual([]); + expect(store.getTaskWorkflowSelection(task.id)).toEqual({ workflowId, stepIds: [] }); + } + }); + + it("create-time interpreter-deferred built-in workflowId records selection without throwing", async () => { + const task = await store.createTask({ description: "explicit builtin coding", workflowId: "builtin:coding" }); + + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps ?? []).toEqual([]); + expect(store.getTaskWorkflowSelection(task.id)).toEqual({ workflowId: "builtin:coding", stepIds: [] }); + }); + + it("interpreter-deferred built-in project defaults fall back without throwing", async () => { + await expect(store.createTask({ description: "implicit builtin default" })).resolves.toMatchObject({ + description: "implicit builtin default", + }); + + await store.setDefaultWorkflowId("builtin:coding"); + const codingTask = await store.createTask({ description: "default builtin coding" }); + expect((await store.getTask(codingTask.id)).enabledWorkflowSteps ?? []).toEqual([]); + expect(store.getTaskWorkflowSelection(codingTask.id)).toBeUndefined(); + + await store.setDefaultWorkflowId("builtin:stepwise-coding"); + const stepwiseTask = await store.createTask({ description: "default builtin stepwise" }); + expect((await store.getTask(stepwiseTask.id)).enabledWorkflowSteps ?? []).toEqual([]); + expect(store.getTaskWorkflowSelection(stepwiseTask.id)).toBeUndefined(); }); it("rejects selecting the PR lifecycle fragment for a task", async () => { diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 27b8afd147..bcca6fe7e2 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -110,8 +110,14 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ start: { x: 60, y: 160 }, execute: { x: 230, y: 160 }, review: { x: 400, y: 160 }, - merge: { x: 570, y: 160 }, - end: { x: 740, y: 160 }, + "merge-gate": { x: 570, y: 160 }, + "branch-group-member-integration": { x: 740, y: 80 }, + "branch-group-promotion": { x: 910, y: 80 }, + "merge-attempt": { x: 1080, y: 160 }, + "merge-retry": { x: 1250, y: 80 }, + "recovery-router": { x: 1250, y: 240 }, + "merge-manual-hold": { x: 740, y: 240 }, + end: { x: 1420, y: 160 }, }, createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index ca90c033cd..30f898c534 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -77,7 +77,7 @@ import type { WorkflowDefinitionUpdate, WorkflowNodeLayout, } from "./workflow-definition-types.js"; -import { compileWorkflowToSteps } from "./workflow-compiler.js"; +import { compileWorkflowToSteps, isInterpreterDeferredWorkflowCompileError } from "./workflow-compiler.js"; import { BUILTIN_WORKFLOWS, getBuiltinWorkflow, @@ -14872,8 +14872,16 @@ ${stepsSection}`; // selectable workflow); fall back to no default rather than materializing it. if (def.kind === "fragment") return undefined; // Compile (and validate) before creating any rows so a non-compilable - // default falls back cleanly with nothing written. - const inputs = compileWorkflowToSteps(def.ir); + // default falls back cleanly with nothing written. Interpreter-deferred + // built-ins are valid selectable workflows but not lowerable to legacy + // WorkflowStep rows, so default materialization falls back to legacy defaults. + let inputs: import("./types.js").WorkflowStepInput[]; + try { + inputs = compileWorkflowToSteps(def.ir); + } catch (err) { + if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) return undefined; + throw err; + } const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); return { workflowId, stepIds }; } @@ -14892,16 +14900,23 @@ ${stepsSection}`; if (def.kind === "fragment") { throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); } - const inputs = compileWorkflowToSteps(def.ir); + let inputs: import("./types.js").WorkflowStepInput[]; + try { + inputs = compileWorkflowToSteps(def.ir); + } catch (err) { + if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) return { workflowId, stepIds: [] }; + throw err; + } const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); return { workflowId, stepIds }; } /** - * Select a workflow for a task: compile it, materialize its steps, and write - * their ids into the task's enabledWorkflowSteps. Replaces any prior selection - * (no orphaned steps). Throws WorkflowCompileError for non-linear graphs - * before any state is written. + * Select a workflow for a task: compile it when possible, materialize its + * steps, and write their ids into the task's enabledWorkflowSteps. Replaces + * any prior selection (no orphaned steps). Interpreter-deferred workflow IRs + * record the selection with zero materialized steps; genuinely invalid graphs + * still throw before any state is written. */ async selectTaskWorkflow(taskId: string, workflowId: string): Promise { // Hold the task lock across the whole sequence (materialize → owner write → @@ -14917,8 +14932,16 @@ ${stepsSection}`; if (def.kind === "fragment") { throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); } - // Compile once up front: a non-linear graph aborts before any mutation. - const inputs = compileWorkflowToSteps(def.ir); + // Compile once up front: invalid graphs abort before any mutation, while + // interpreter-deferred graphs keep the selection but materialize no legacy + // WorkflowStep rows. + let inputs: import("./types.js").WorkflowStepInput[]; + try { + inputs = compileWorkflowToSteps(def.ir); + } catch (err) { + if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) inputs = []; + else throw err; + } // Materialize the new steps and point the task at them BEFORE deleting the // prior selection's rows, so a mid-flight failure never leaves the task diff --git a/packages/core/src/workflow-compiler.ts b/packages/core/src/workflow-compiler.ts index b6918c2391..3dcca86094 100644 --- a/packages/core/src/workflow-compiler.ts +++ b/packages/core/src/workflow-compiler.ts @@ -15,35 +15,17 @@ export class WorkflowCompileError extends Error { } } +export const WORKFLOW_INTERPRETER_DEFERRED_SUFFIX = "require the workflow interpreter (deferred)"; + +export function isInterpreterDeferredWorkflowCompileError(error: unknown): boolean { + return error instanceof WorkflowCompileError && error.message.includes(WORKFLOW_INTERPRETER_DEFERRED_SUFFIX); +} + /** Seam anchor kinds, encoded on IR nodes as `config.seam`. These map to the * fixed planning → execute → workflow-step → review → merge pipeline and are * 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; @@ -99,11 +81,6 @@ 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"); @@ -130,12 +107,12 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { return new WorkflowCompileError(`node '${node.id}' has no outgoing edge`); } if (outs.length > 1) { - // NOTE: the `require the workflow interpreter (deferred)` suffix is matched - // by the dashboard editor (WorkflowNodeEditor handleSave, KTD-4) to render - // an info-tone "interpreter-only" banner instead of an error. Keep both - // interpreter-deferred messages carrying this exact suffix in sync. + // NOTE: WORKFLOW_INTERPRETER_DEFERRED_SUFFIX is matched by the dashboard + // editor/routes (KTD-4) to render an info-tone "interpreter-only" banner + // instead of an error. Keep interpreter-deferred messages carrying this + // exact suffix in sync. return new WorkflowCompileError( - `node '${node.id}' branches into ${outs.length} edges — graphs with branches require the workflow interpreter (deferred)`, + `node '${node.id}' branches into ${outs.length} edges — graphs with branches ${WORKFLOW_INTERPRETER_DEFERRED_SUFFIX}`, ); } } @@ -150,18 +127,11 @@ 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)) { @@ -190,15 +160,10 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { if (!reachedTerminal) { return new WorkflowCompileError("workflow main path does not reach the end node"); } - // 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), - ); + const unreached = ir.nodes.filter((node) => !visited.has(node.id) && node.kind !== "end"); if (unreached.length > 0) { return new WorkflowCompileError( - `node '${unreached[0].id}' is not on the main path — disconnected nodes require the workflow interpreter (deferred)`, + `node '${unreached[0].id}' is not on the main path — disconnected nodes ${WORKFLOW_INTERPRETER_DEFERRED_SUFFIX}`, ); } @@ -281,11 +246,6 @@ 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";