root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(FN-7225): honor plan review approvals in workflows

Browse Source 
Fusion-Task-Id: FN-7225
This commit is contained in:
gsxdsm
2026-06-29 02:08:08 -07:00
parent 998a7f2745
commit c93217dee0
13 changed files with 278 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/fn-7225-plan-review-approval.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Prevent Plan Review approvals from looping back into triage as failures.
category: fix
dev: Parses explicit reviewer prose verdicts and applies default-on optional workflow steps when no explicit selection exists.

24
packages/core/src/__tests__/workflow-ir-resolver.test.ts
View File

@@ -1,6 +1,6 @@
import { describe, it, expect, vi } from "vitest";
import { getBuiltinWorkflow } from "../builtin-workflows.js";
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../builtin-stepwise-final-review-coding-workflow-ir.js";
import type { WorkflowIr } from "../workflow-ir-types.js";
import {
resolveWorkflowIrForTask,
@@ -73,20 +73,20 @@ describe("resolveWorkflowIrForTask", () => {
defs: { "wf-gone": undefined },
});
const ir = await resolveWorkflowIrForTask(store, "t1");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
});
it("falls back to the default when there is no selection", async () => {
const store = makeStore({ selection: undefined });
const ir = await resolveWorkflowIrForTask(store, "t1");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(store.getWorkflowDefinition).not.toHaveBeenCalled();
});
it("degrades to the default when the selection lookup throws", async () => {
const store = makeStore({ selectionThrows: true });
const ir = await resolveWorkflowIrForTask(store, "t1");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
});
it("caches by workflowId so the definition is fetched once across calls", async () => {
@@ -104,11 +104,11 @@ describe("resolveWorkflowIrForTask", () => {
});
describe("resolveWorkflowIrById", () => {
it("resolves builtin:coding to the canonical authored v2 IR with review column traits", async () => {
it("resolves builtin:coding to the catalog default stepwise final-review IR", async () => {
const store = makeStore({});
const ir = await resolveWorkflowIrById(store, "builtin:coding");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(ir).toBe(getBuiltinWorkflow("builtin:coding")!.ir);
expect(ir.version).toBe("v2");
if (ir.version !== "v2") throw new Error("expected v2");
@@ -123,14 +123,14 @@ describe("resolveWorkflowIrById", () => {
it("resolves an explicit builtin:coding task selection through the canonical IR path", async () => {
const store = makeStore({ selection: { workflowId: "builtin:coding", stepIds: [] } });
const ir = await resolveWorkflowIrForTask(store, "t1");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(store.getWorkflowDefinition).not.toHaveBeenCalled();
});
it("falls back to the canonical IR for an unknown built-in id", async () => {
const store = makeStore({});
const ir = await resolveWorkflowIrById(store, "builtin:missing");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(store.getWorkflowDefinition).not.toHaveBeenCalled();
});
@@ -142,7 +142,7 @@ describe("resolveWorkflowIrById", () => {
const ir = await resolveWorkflowIrById(store, "builtin:coding");
expect(ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(store.getWorkflowSettingsProjectId).toHaveBeenCalledTimes(1);
expect(store.getWorkflowPromptOverrides).not.toHaveBeenCalled();
expect(store.getWorkflowDefinition).not.toHaveBeenCalled();
@@ -162,7 +162,7 @@ describe("resolveWorkflowIrById", () => {
it("keeps project-scoped prompt overrides and cache keys when project identity resolves", async () => {
const store = makeStore({
projectId: "proj-override",
promptOverrides: { planning: "Project-specific plan" },
promptOverrides: { plan: "Project-specific plan" },
});
const cache = new Map<string, WorkflowIr>();
@@ -170,8 +170,8 @@ describe("resolveWorkflowIrById", () => {
const second = await resolveWorkflowIrById(store, "builtin:coding", cache);
expect(first).toBe(second);
expect(first).not.toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(first.nodes.find((node) => node.id === "planning")?.config?.prompt).toBe("Project-specific plan");
expect(first).not.toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(first.nodes.find((node) => node.id === "plan")?.config?.prompt).toBe("Project-specific plan");
expect(cache.get("builtin:coding\u0000proj-override")).toBe(first);
expect(store.getWorkflowPromptOverrides).toHaveBeenCalledTimes(1);
});

5
packages/core/src/__tests__/workflow-restart-durability.test.ts
View File

@@ -3,7 +3,6 @@ import { join } from "node:path";
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../builtin-stepwise-final-review-coding-workflow-ir.js";
import type { TaskStore } from "../store.js";
import type { WorkflowRunStepInstance } from "../types.js";
@@ -287,9 +286,9 @@ describe("workflow restart durability for explicit selections", () => {
expect(store().getTaskWorkflowSelection(selectedTask.id)).toEqual(selectionBefore);
expect(await taskJsonEnabledWorkflowSteps(selectedTask.id)).toEqual(enabledBefore);
// Current hot-path resolution degrades a dangling custom definition to the built-in IR instead of throwing.
// Current hot-path resolution degrades a dangling custom definition to the default built-in Coding IR instead of throwing.
// The explicit materialization APIs below must still fail closed when asked to write that missing id again.
expect(privateStore().resolveTaskWorkflowIrSync(selectedTask.id)).toEqual(BUILTIN_CODING_WORKFLOW_IR);
expect(privateStore().resolveTaskWorkflowIrSync(selectedTask.id)).toEqual(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
await expect(store().selectTaskWorkflow(untouchedTask.id, workflow.id)).rejects.toThrow(
`Workflow '${workflow.id}' not found`,

13
packages/core/src/store.ts
View File

@@ -15697,19 +15697,24 @@ ${stepsSection}`;
private resolveTaskWorkflowIrSync(taskId: string): WorkflowIr {
const selection = this.getTaskWorkflowSelection(taskId);
const workflowId = selection?.workflowId;
if (!workflowId) return this.applyBuiltInPromptOverridesSync("builtin:coding", BUILTIN_CODING_WORKFLOW_IR);
/*
* FNXC:WorkflowBuiltins 2026-06-29-02:18:
* The built-in id `builtin:coding` now points at the stepwise final-review workflow. No-selection tasks must resolve through the built-in catalog, otherwise dashboard/operator defaults say "Coding" while the engine silently executes legacy coding.
*/
const defaultCodingIr = getBuiltinWorkflow("builtin:coding")?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
if (!workflowId) return this.applyBuiltInPromptOverridesSync("builtin:coding", defaultCodingIr);
if (isBuiltinWorkflowId(workflowId)) {
const builtin = getBuiltinWorkflow(workflowId);
return this.applyBuiltInPromptOverridesSync(workflowId, builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR);
return this.applyBuiltInPromptOverridesSync(workflowId, builtin?.ir ?? defaultCodingIr);
}
try {
const row = this.db
.prepare("SELECT ir FROM workflows WHERE id = ?")
.get(workflowId) as { ir: string } | undefined;
if (!row) return BUILTIN_CODING_WORKFLOW_IR;
if (!row) return defaultCodingIr;
return parseWorkflowIr(row.ir);
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
return defaultCodingIr;
}
}

18
packages/core/src/workflow-ir-resolver.ts
View File

@@ -20,6 +20,16 @@ import { parseWorkflowIr } from "./workflow-ir.js";
import { applyPromptOverridesToIr } from "./workflow-prompt-overrides.js";
import type { WorkflowIr } from "./workflow-ir-types.js";
function defaultCodingWorkflowIr(): WorkflowIr {
/*
* FNXC:WorkflowBuiltins 2026-06-29-02:18:
* `builtin:coding` is the operator-facing default workflow id, not the legacy monolithic IR export. Resolve the catalog entry first so no-selection tasks follow the new stepwise default; keep the old IR only as a missing-catalog safety fallback.
*/
const builtin = getBuiltinWorkflow("builtin:coding");
const ir = builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
return typeof ir === "string" ? parseWorkflowIr(ir) : ir;
}
/** Minimal store surface the resolver needs (public APIs only). */
export interface WorkflowIrResolverStore {
getTaskWorkflowSelection(taskId: string): { workflowId: string; stepIds: string[] } | undefined;
@@ -104,7 +114,7 @@ export async function resolveWorkflowIrById(
if (isBuiltinWorkflowId(workflowId)) {
const builtin = getBuiltinWorkflow(workflowId);
const ir = builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
const ir = builtin?.ir ?? defaultCodingWorkflowIr();
const resolved = typeof ir === "string" ? parseWorkflowIr(ir) : ir;
const overrides = projectId ? store.getWorkflowPromptOverrides?.(workflowId, projectId) : undefined;
// FNXC:CustomWorkflows 2026-06-21-19:12:
@@ -116,12 +126,12 @@ export async function resolveWorkflowIrById(
try {
const def = await store.getWorkflowDefinition(workflowId);
if (!def) return BUILTIN_CODING_WORKFLOW_IR;
if (!def) return defaultCodingWorkflowIr();
const ir = typeof def.ir === "string" ? parseWorkflowIr(def.ir) : def.ir;
irCache?.set(cacheKey, ir);
return ir;
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
return defaultCodingWorkflowIr();
}
}
@@ -138,7 +148,7 @@ export async function resolveWorkflowIrForTask(
try {
workflowId = store.getTaskWorkflowSelection(taskId)?.workflowId;
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
return defaultCodingWorkflowIr();
}
if (!workflowId) return resolveWorkflowIrById(store, "builtin:coding", irCache);
return resolveWorkflowIrById(store, workflowId, irCache);

27
packages/engine/src/__tests__/triage.test.ts
View File

@@ -1672,6 +1672,33 @@ describe("requirePlanApproval setting", () => {
expect(store.moveTask).not.toHaveBeenCalled();
});
it("clears stale workflow step instances when a fresh accepted plan replaces existing steps", async () => {
const task = createTriageTask({
id: "FN-7224",
title: "Rebuilt plan task",
status: "planning",
steps: [{ name: "Old step", status: "pending" }],
} as Partial<Task>);
const clearWorkflowRunStepInstances = vi.fn();
const store = createMockStore({
getTask: vi.fn().mockResolvedValue(task),
parseStepsFromPrompt: vi.fn().mockResolvedValue([{ name: "Fresh step", status: "pending" }]),
clearWorkflowRunStepInstances,
} as Partial<TaskStore>);
const processor = new TriageProcessor(store, rootDir);
await (processor as unknown as {
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
}).finalizeApprovedTask(
task,
"# Task: FN-7224 - Rebuilt plan task\n\n## Steps\n\n### Step 1: Fresh step\n- Execute the fresh plan.\n",
{ requirePlanApproval: false } as Settings,
);
expect(clearWorkflowRunStepInstances).toHaveBeenCalledWith("FN-7224");
expect(store.moveTask).toHaveBeenCalledWith("FN-7224", "todo");
});
it.each([
{ mode: "workflow" as const, requirePlanApproval: true, expectedApproval: true },
{ mode: undefined, requirePlanApproval: false, expectedApproval: false },

80
packages/engine/src/__tests__/workflow-graph-optional-group.test.ts
View File

@@ -165,6 +165,28 @@ describe("WorkflowGraphExecutor optional-group", () => {
expect(disabledResult.outcome).toBe("success");
});
it("runs default-on optional groups when the task has no explicit optional-step selection", async () => {
const ir = optionalGroupIr();
const group = ir.nodes.find((node) => node.id === "group");
if (group?.config) group.config.defaultOn = true;
const calls: string[] = [];
const executor = new WorkflowGraphExecutor({
handlers: {
prompt: async (node) => {
calls.push(node.id);
return { outcome: "success" };
},
},
});
const unsetResult = await executor.run(taskWith(undefined), settingsOn(), ir);
const explicitEmptyResult = await executor.run(taskWith([]), settingsOn(), ir);
expect(calls.filter((id) => id === "optstep")).toHaveLength(1);
expect(unsetResult.visitedNodeIds).toContain("group::optstep");
expect(explicitEmptyResult.visitedNodeIds).not.toContain("group::optstep");
});
it("runs an enabled group's template exactly once (single pass, not per-step/looped)", async () => {
const runTemplate = vi.fn<WorkflowNodeHandler>(async () => ({ outcome: "success" }));
const executor = new WorkflowGraphExecutor({
@@ -444,6 +466,64 @@ describe("WorkflowGraphExecutor optional-group", () => {
]));
});
it("does not synthesize a Plan Review replan from advisory malformed output", async () => {
const requestFix = vi.fn(async () => true);
const calls: string[] = [];
const records: unknown[] = [];
const ir: WorkflowIr = {
version: "v2",
name: "plan-review-advisory-malformed",
columns: [{ id: "work", name: "Work", traits: [] }],
nodes: [
{ id: "start", kind: "start" },
{
id: "plan-review",
kind: "optional-group",
config: {
name: "Plan Review",
defaultOn: true,
template: {
nodes: [{ id: "plan-review-step", kind: "prompt", config: { prompt: "review plan" } }],
edges: [],
},
},
},
{ id: "execute", kind: "prompt", config: { prompt: "execute" } },
{ id: "end", kind: "end" },
],
edges: [
{ from: "start", to: "plan-review" },
{ from: "plan-review", to: "execute", condition: "success" },
{ from: "execute", to: "end" },
],
};
const executor = new WorkflowGraphExecutor({
handlers: {
prompt: async (node) => {
calls.push(node.id);
return node.id === "plan-review-step"
? { outcome: "success", value: "advisory_failure", contextPatch: { output: "malformed output — no verdict extracted" } }
: { outcome: "success" };
},
},
recordWorkflowStepResult: async (_taskId, result) => { records.push(result); },
requestPreMergeOptionalStepFix: requestFix,
});
const result = await executor.run(taskWith(["plan-review"]), settingsOn(), ir);
expect(requestFix).not.toHaveBeenCalled();
expect(calls).toContain("execute");
expect(result.outcome).toBe("success");
expect(records).toEqual(expect.arrayContaining([
expect.objectContaining({
workflowStepId: "plan-review",
status: "advisory_failure",
output: "malformed output — no verdict extracted",
}),
]));
});
it("cycles REVISE findings across graph runs until APPROVE, and falls through only after the budget seam declines", async () => {
const verdicts = ["REVISE", "REVISE", "APPROVE"];
const requestFix = vi.fn(async () => true);

15
packages/engine/src/__tests__/workflow-step-verdict-parsing.test.ts
View File

@@ -28,6 +28,21 @@ describe("inferWorkflowStepVerdictFromProse", () => {
expect(inferWorkflowStepVerdictFromProse("looks good")).toEqual({ verdict: "APPROVE", notes: "" });
});
it("infers explicit markdown verdicts from reviewer-style output", () => {
expect(inferWorkflowStepVerdictFromProse("## Spec Review\n\n### Verdict: APPROVE\n\nThe plan is ready.")).toEqual({
verdict: "APPROVE",
notes: "",
});
expect(inferWorkflowStepVerdictFromProse("Status: APPROVE_WITH_NOTES\n\nProceed with notes.")).toEqual({
verdict: "APPROVE_WITH_NOTES",
notes: "",
});
expect(inferWorkflowStepVerdictFromProse("Verdict: REVISE\n\nFix the plan.")).toEqual({
verdict: "REVISE",
notes: "",
});
});
it("returns null for unrelated prose", () => {
expect(inferWorkflowStepVerdictFromProse("lorem ipsum")).toBeNull();
});

88
packages/engine/src/__tests__/workflow-task-runtime.test.ts
View File

@@ -7,6 +7,14 @@ import type { PreparedWorktree, WorkflowRuntimePrimitives } from "../runtime-pri
const task = { id: "FN-9002" } as TaskDetail;
const flagOff = { experimentalFeatures: {} } as unknown as Pick<Settings, "experimentalFeatures">;
const promptWithOneStep = "# Task: FN-9002 - Runtime default\n\n## Steps\n\n### Step 1: Implement runtime default\n- Exercise the default workflow.\n";
const parseStepsDeps = {
readArtifact: async (_task: TaskDetail, key: string) => key === "PROMPT.md" ? promptWithOneStep : undefined,
writeSteps: async (target: TaskDetail, steps: TaskDetail["steps"]) => {
target.steps = steps;
},
};
function selectedIr(): WorkflowIr {
return {
@@ -35,6 +43,7 @@ function recordingPrimitives(
observed: {
prepared?: PreparedWorktree;
executedTasks?: TaskDetail[];
stepTasks?: TaskDetail[];
mergeAttempt?: number;
mergeRunId?: string;
mergeWorkflowId?: string;
@@ -55,7 +64,7 @@ function recordingPrimitives(
: overrides.prepareData ?? prepared,
};
},
readArtifact: async () => undefined,
readArtifact: async (_ctx, _task, key) => key === "PROMPT.md" ? promptWithOneStep : undefined,
writeArtifact: async (_ctx, _task, key) => ({ outcome: "success", data: { key } }),
runPlanningSession: async () => {
calls.push("planning");
@@ -73,7 +82,12 @@ function recordingPrimitives(
data: { taskDone: override?.outcome !== "failure", modifiedFiles: [] },
};
},
runTaskStep: async () => ({ outcome: "success" }),
runTaskStep: async (_ctx, _task, stepIndex) => {
calls.push(`step:${stepIndex}`);
observed.stepTasks?.push(_task);
observed.executedTasks?.push(_task);
return { outcome: "success" };
},
resetTaskStep: async () => ({ ok: true }),
runReview: async (_ctx, _task, input) => {
calls.push(input.stepIndex === undefined ? "review" : "step-review");
@@ -94,7 +108,10 @@ function recordingPrimitives(
data: { allPassed: override?.value !== "remediation-scheduled" },
};
},
updateSteps: async (_ctx, _task, steps) => ({ outcome: "success", data: { count: steps.length } }),
updateSteps: async (_ctx, _task, steps) => {
_task.steps = steps;
return { outcome: "success", data: { count: steps.length } };
},
transitionTask: async () => {
calls.push("schedule");
return { outcome: "success" };
@@ -118,6 +135,7 @@ describe("WorkflowTaskRuntime", () => {
store: {
getTaskWorkflowSelection: () => undefined,
getWorkflowDefinition: async () => undefined,
getTaskDocument: async (_taskId, key) => key === "PROMPT.md" ? { key } : null,
},
runCustomNode: async () => ({ outcome: "success" }),
};
@@ -148,6 +166,7 @@ describe("WorkflowTaskRuntime", () => {
calls.push(`custom:${node.id}`);
return { outcome: "success" };
},
parseStepsDeps,
});
const result = await runtime.run(task, flagOff);
@@ -191,6 +210,7 @@ describe("WorkflowTaskRuntime", () => {
calls.push(`custom:${node.id}`);
return { outcome: "success" };
},
parseStepsDeps,
});
const result = await runtime.run(attachmentTask, flagOff);
@@ -218,20 +238,22 @@ describe("WorkflowTaskRuntime", () => {
store: {
getTaskWorkflowSelection: () => undefined,
getWorkflowDefinition: async () => undefined,
getTaskDocument: async (_taskId, key) => key === "PROMPT.md" ? { key } : null,
},
primitives: recordingPrimitives(calls, undefined, observed),
runCustomNode: async (node) => {
calls.push(`custom:${node.id}`);
return { outcome: "success" };
},
parseStepsDeps,
});
const result = await runtime.run(attachmentTask, flagOff);
expect(result.disposition).toBe("completed");
// U6: the coding built-in no longer runs a `workflow-step` seam; the pre-merge
// browser-verification optional-group is default-OFF and bypassed (no call).
expect(calls).toEqual(["planning", "prepare-worktree", "execute", "review", "merge"]);
// Default Coding is stepwise: planning writes PROMPT.md, parse projects steps,
// then foreach runs `runTaskStep` before merge. No legacy execute/review seam.
expect(calls).toEqual(["planning", "custom:plan-review-step", "step:0", "custom:code-review-step", "merge"]);
expect(observed.executedTasks).toHaveLength(1);
expect(observed.executedTasks[0]?.attachments).toEqual(attachments);
});
@@ -242,9 +264,11 @@ describe("WorkflowTaskRuntime", () => {
store: {
getTaskWorkflowSelection: () => undefined,
getWorkflowDefinition: async () => undefined,
getTaskDocument: async (_taskId, key) => key === "PROMPT.md" ? { key } : null,
},
primitives: recordingPrimitives([], undefined, observed),
runCustomNode: async () => ({ outcome: "success" }),
parseStepsDeps,
});
const result = await runtime.run(task, flagOff);
@@ -284,29 +308,28 @@ describe("WorkflowTaskRuntime", () => {
store: {
getTaskWorkflowSelection: () => undefined,
getWorkflowDefinition: async () => undefined,
getTaskDocument: async (_taskId, key) => key === "PROMPT.md" ? { key } : null,
},
primitives: recordingPrimitives(calls),
runCustomNode: async (node) => {
calls.push(`custom:${node.id}`);
return { outcome: "success" };
},
parseStepsDeps,
});
const result = await runtime.run(task, flagOff);
const defaultTask = { ...task, enabledWorkflowSteps: ["plan-review", "code-review"] } as TaskDetail;
const result = await runtime.run(defaultTask, flagOff);
expect(result.disposition).toBe("completed");
// U6: no `workflow-step` seam; the bypassed browser-verification group node
// sits between execute and review in the visited sequence.
expect(calls).toEqual(["planning", "prepare-worktree", "execute", "review", "merge"]);
expect(result.visitedNodeIds).toEqual([
"start",
"planning",
"execute",
"browser-verification",
"code-review",
"review",
"merge",
]);
expect(calls).toEqual(["planning", "custom:plan-review-step", "step:0", "custom:code-review-step", "merge"]);
expect(result.visitedNodeIds).toContain("plan");
expect(result.visitedNodeIds).toContain("plan-review");
expect(result.visitedNodeIds).toContain("parse");
expect(result.visitedNodeIds).toContain("steps");
expect(result.visitedNodeIds).toContain("code-review");
expect(result.visitedNodeIds).not.toContain("execute");
expect(result.visitedNodeIds).not.toContain("review");
});
it("runs the pre-merge browser-verification optional-group once when enabled, before review", async () => {
@@ -319,37 +342,33 @@ describe("WorkflowTaskRuntime", () => {
store: {
getTaskWorkflowSelection: () => undefined,
getWorkflowDefinition: async () => undefined,
getTaskDocument: async (_taskId, key) => key === "PROMPT.md" ? { key } : null,
},
primitives: recordingPrimitives(calls),
runCustomNode: async (node) => {
calls.push(`custom:${node.id}`);
return { outcome: "success" };
},
parseStepsDeps,
});
const enabledTask = { ...task, enabledWorkflowSteps: ["browser-verification"] } as TaskDetail;
const enabledTask = { ...task, enabledWorkflowSteps: ["plan-review", "browser-verification", "code-review"] } as TaskDetail;
const result = await runtime.run(enabledTask, flagOff);
expect(result.disposition).toBe("completed");
expect(calls).toEqual([
"planning",
"prepare-worktree",
"execute",
"custom:plan-review-step",
"step:0",
"custom:browser-verification-step",
"review",
"merge",
]);
expect(result.visitedNodeIds).toEqual([
"start",
"planning",
"execute",
// The group container node, then its inner template step (run once).
"browser-verification",
"browser-verification::browser-verification-step",
"code-review",
"review",
"custom:code-review-step",
"merge",
]);
expect(result.visitedNodeIds).toContain("plan-review::plan-review-step");
expect(result.visitedNodeIds).toContain("browser-verification::browser-verification-step");
expect(result.visitedNodeIds).toContain("code-review::code-review-step");
expect(result.visitedNodeIds).not.toContain("execute");
expect(result.visitedNodeIds).not.toContain("review");
});
it("fails selected workflow lookup misses instead of running the built-in workflow", async () => {
@@ -426,6 +445,7 @@ describe("WorkflowTaskRuntime", () => {
},
primitives: recordingPrimitives([]),
runCustomNode: async () => ({ outcome: "success" }),
parseStepsDeps,
branchPersistence: {
loadBranchStates: (_taskId, runId) => {
observedRunIds.push(runId);

26
packages/engine/src/executor.ts
View File

@@ -1128,12 +1128,26 @@ export function parseWorkflowStepVerdict(rawOutput: string): { verdict: "APPROVE
return null;
}
export function inferWorkflowStepVerdictFromProse(rawOutput: string): { verdict: "APPROVE" | "REVISE"; notes: string } | null {
export function inferWorkflowStepVerdictFromProse(rawOutput: string): { verdict: "APPROVE" | "APPROVE_WITH_NOTES" | "REVISE"; notes: string } | null {
const trimmed = rawOutput.trim();
const revisionMatch = trimmed.match(/^REQUEST REVISION\s*\n*/i);
if (revisionMatch) {
return { verdict: "REVISE", notes: trimmed.slice(revisionMatch[0].length).trim() || "Revision requested" };
}
/*
* FNXC:PlanReview 2026-06-29-02:05:
* Plan Review runs through reviewer-style agents that often emit a markdown
* section such as `### Verdict: APPROVE` even when the prompt asks for trailing
* JSON. Treat that explicit verdict as authoritative so a real approval does
* not collapse into a synthetic pre-execution plan failure loop.
*/
const explicitVerdictMatch = trimmed.match(/(?:^|\n)\s*(?:#{1,6}\s*)?(?:verdict|status)\s*:\s*(APPROVE_WITH_NOTES|APPROVE|REVISE)\b/i);
if (explicitVerdictMatch) {
return {
verdict: explicitVerdictMatch[1].toUpperCase() as "APPROVE" | "APPROVE_WITH_NOTES" | "REVISE",
notes: "",
};
}
if (/\b(approve|approved|looks good|no issues|out of scope)\b/i.test(trimmed)) {
return { verdict: "APPROVE", notes: "" };
}
@@ -6714,9 +6728,17 @@ export class TaskExecutor {
const contextPatch: Record<string, unknown> = {};
if (typeof stepOutput === "string") contextPatch.output = stepOutput;
if (typeof stepNotes === "string" && stepNotes) contextPatch.notes = stepNotes;
/*
* FNXC:PlanReview 2026-06-29-02:05:
* Advisory graph steps still need a distinct non-pass value when their
* review output is malformed. Returning plain `failed` made optional-group
* recovery synthesize a Plan Review REVISE even when no reviewer requested
* one; `advisory_failure` preserves visibility without inventing feedback.
*/
const advisoryFailureValue = (outcome as { malformed?: boolean }).malformed ? "advisory_failure" : "failed";
return {
outcome: outcome.success || !blocking ? "success" : "failure",
value: verdict ?? (outcome.success ? "passed" : "failed"),
value: verdict ?? (outcome.success ? "passed" : advisoryFailureValue),
...(Object.keys(contextPatch).length > 0 ? { contextPatch } : {}),
};
}

8
packages/engine/src/triage.ts
View File

@@ -1867,6 +1867,9 @@ export class TriageProcessor {
if (parsedSteps.length > 0) {
taskUpdates.steps = parsedSteps;
}
const shouldClearWorkflowRunStepInstances =
parsedSteps.length > 0
&& (options.isReplan === true || (task.steps?.length ?? 0) > 0);
const duplicateLineage = getTaskDuplicateLineage({
id: task.id,
@@ -2200,10 +2203,13 @@ export class TriageProcessor {
return;
}
if (options.isReplan) {
if (shouldClearWorkflowRunStepInstances) {
/*
FNXC:WorkflowReplan 2026-06-29-00:33:
AI spec revision replaces the task's step-source PROMPT.md, so graph foreach instance pins from the previous plan must be discarded before execution reparses steps. Otherwise rebuilt tasks can fail at parse with a stale pin-mismatch even though the new plan is valid.
FNXC:WorkflowReplan 2026-06-29-02:24:
User-triggered spec rebuilds can race an old paused graph run that writes step-instance rows after the route cleared them. Clear again when triage accepts a fresh parsed plan over an existing step projection, even if the task snapshot no longer has status `needs-replan`.
*/
const maybeStore = this.store as unknown as {
clearWorkflowRunStepInstances?: (taskId: string) => void;

21
packages/engine/src/workflow-graph-executor.ts
View File

@@ -557,7 +557,16 @@ export class WorkflowGraphExecutor {
* the enabled one runs the body, the disabled one runs none and
* still reaches the same downstream node.
*/
const enabled = task.enabledWorkflowSteps?.includes(node.id) ?? false;
/*
* FNXC:WorkflowOptionalSteps 2026-06-29-02:05:
* `enabledWorkflowSteps === undefined` means the task has no explicit
* optional-step selection, so default-on workflow nodes must run. An
* explicit empty array still means the operator disabled every optional
* step from Quick Add or task details.
*/
const enabled = Array.isArray(task.enabledWorkflowSteps)
? task.enabledWorkflowSteps.includes(node.id)
: node.config?.defaultOn === true;
if (!enabled) {
// FNXC:WorkflowOptionalGroup 2026-06-21-16:30: record the group's own
// outcome on bypass too (mirrors the enabled path + every other node
@@ -632,6 +641,7 @@ export class WorkflowGraphExecutor {
: undefined;
let stepStatus: WorkflowStepResult["status"];
if (groupResult.outcome === "failure") stepStatus = "failed";
else if (groupResult.value === "advisory_failure") stepStatus = "advisory_failure";
else if (verdict === "REVISE") stepStatus = "advisory_failure";
else stepStatus = "passed";
const exitContextPatch = exitResult?.contextPatch;
@@ -676,10 +686,17 @@ export class WorkflowGraphExecutor {
* seam with a synthesized REVISE verdict so the executor can route it back
* to triage and then let approved replans continue through todo/execution.
*/
/*
* FNXC:PlanReview 2026-06-29-02:05:
* Plan Review should send a task back to triage only for an actual
* REVISE verdict or a hard step failure. A malformed advisory result is
* visible as `advisory_failure`, but it must not fabricate a plan-rewrite
* request after the reviewer already approved or failed to emit JSON.
*/
const shouldRequestPreMergeFix =
stepPhase === "pre-merge"
&& (stepStatus === "advisory_failure" || stepStatus === "failed")
&& (verdict === "REVISE" || node.id === PLAN_REVIEW_GROUP_ID);
&& (verdict === "REVISE" || (node.id === PLAN_REVIEW_GROUP_ID && stepStatus === "failed"));
if (shouldRequestPreMergeFix) {
const feedback = stepOutput?.trim()
|| stepNotes?.trim()

10
packages/engine/src/workflow-task-runtime.ts
View File

@@ -1,6 +1,5 @@
import type { Settings, TaskDetail, WorkflowIr, WorkflowIrArtifact, WorkflowIrNode, WorkflowWorkItem, WorkflowWorkItemState } from "@fusion/core";
import {
BUILTIN_CODING_WORKFLOW_IR,
getBuiltinWorkflow,
isBuiltinWorkflowId,
parseWorkflowIr,
@@ -319,7 +318,14 @@ interface WorkflowRuntimeTarget {
}
function builtinCodingTarget(): WorkflowRuntimeTarget {
return { workflowId: "builtin:coding", ir: BUILTIN_CODING_WORKFLOW_IR };
/*
* FNXC:WorkflowBuiltins 2026-06-29-02:18:
* Runtime defaulting must follow the built-in catalog entry for `builtin:coding`; importing the legacy coding IR here would bypass the renamed default workflow and strand unselected tasks on the old monolithic graph.
*/
const builtin = getBuiltinWorkflow("builtin:coding");
if (!builtin) throw new Error("workflow-missing: builtin:coding");
const ir = typeof builtin.ir === "string" ? parseWorkflowIr(builtin.ir) : builtin.ir;
return { workflowId: "builtin:coding", ir };
}
function buildWorkflowRuntimeSettings(