FN-7990: share worktree classifier so Code Review acquires a worktree

Unify write-capability classification so graph preparation acquires a worktree for inline-fix Code Review before runtime runs, eliminating the immediate no-worktree-for-write-node failure.

- Add shared workflowNodeRequiresWorktree helper for preparation and runtime
- Plumb optional-group context and reviewerInlineFixes into graph preparation
- Acquire/reuse/reacquire worktrees for write-capable inline review nodes
- Keep Plan Review and disabled inline fixes read-only
- Add regression tests and a patch changeset

Files changed:
 .changeset/fn-7990-code-review-worktree.md         |  7 ++
 .../__tests__/ce-workflow-step-executor.test.ts    | 97 ++++++++++++++++++++++
 .../workflow-node-execution-needs.test.ts          | 47 +++++++++++
 packages/engine/src/executor.ts                    | 32 +++----
 packages/engine/src/workflow-graph-executor.ts     | 52 ++++++++----
 .../engine/src/workflow-node-execution-needs.ts    | 46 ++++++++++
 6 files changed, 243 insertions(+), 38 deletions(-)

Fusion-Task-Id: FN-7990

Fusion-Task-Lineage: f5d19181-0b98-4827-8adb-069f7dc05c03

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-07-15 15:34:43 -07:00
parent 5ff7a20738
commit dc7bb40948
6 changed files with 243 additions and 38 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Prevent inline Code Review steps from failing before they can run.
category: fix
dev: Shares workflow write-capability classification between graph preparation and runtime.

View File

@@ -357,6 +357,103 @@ describe("CE workflow-step executor integration", () => {
);
});
it.each([
["acquires an absent worktree", undefined, "/tmp/test/.worktrees/acquired-code-review", 1],
["reuses a live worktree", "/tmp/test/.worktrees/live-code-review", "/tmp/test/.worktrees/live-code-review", 0],
["reacquires a stale worktree", "/tmp/test/.worktrees/stale-code-review", "/tmp/test/.worktrees/acquired-code-review", 1],
])("prepares an inline-fix Code Review node when it %s", async (_scenario, existingWorktree, expectedWorktree, acquisitionCount) => {
mockedExistsSync.mockImplementation((path) => path !== "/tmp/test/.worktrees/stale-code-review");
const store = createMockStore();
let live = baseStepTask({
worktree: existingWorktree,
branch: existingWorktree ? "fusion/fn-ce-1" : undefined,
enabledWorkflowSteps: ["code-review"],
});
store.getTask.mockImplementation(async () => live as any);
store.updateTask.mockImplementation(async (_id: string, patch: Record<string, unknown>) => {
live = { ...live, ...patch };
return live as any;
});
const { executor } = makeExecutor(store);
vi.spyOn(executor as any, "createWorktree").mockResolvedValue({
path: "/tmp/test/.worktrees/acquired-code-review",
branch: "fusion/fn-ce-1",
});
vi.spyOn(executor as any, "captureBaseCommitSha").mockResolvedValue(undefined);
const executeStep = vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "APPROVE" });
const requirements: any[] = [];
const codeReview = {
id: "code-review-step",
kind: "gate",
config: { name: "Code Review", prompt: "Review the implementation." },
};
const ir: WorkflowIr = {
version: "v2",
name: "code-review-worktree-test",
columns: [{ id: "in-progress", name: "In Progress", traits: [] }],
nodes: [
{ id: "start", kind: "start" },
{
id: "code-review",
kind: "optional-group",
config: { name: "Code Review", defaultOn: true, template: { nodes: [codeReview], edges: [] } },
},
{ id: "end", kind: "end" },
],
edges: [
{ from: "start", to: "code-review" },
{ from: "code-review", to: "end", condition: "success" },
],
};
const settings = { ...(await store.getSettings()), reviewerInlineFixes: true };
const graph = new WorkflowGraphExecutor({
prepareNodeExecution: (graphNode, task, requirement) => {
requirements.push(requirement);
return (executor as any).prepareGraphNodeExecution(graphNode, task, settings, requirement);
},
runCustomNode: (graphNode, task, context) =>
(executor as any).runGraphCustomNode(graphNode, task, settings, undefined, context),
});
const result = await graph.run(live as any, settings, ir);
expect(requirements).toContainEqual({ requiresWorktree: true, reason: "write-capable-node" });
expect((executor as any).createWorktree).toHaveBeenCalledTimes(acquisitionCount);
expect(executeStep).toHaveBeenCalledTimes(1);
expect(executeStep.mock.calls[0]?.[2]).toBe(expectedWorktree);
expect(result).toMatchObject({ outcome: "success" });
expect(result.context["node:code-review:outcome"]).not.toBe("no-worktree-for-write-node");
});
it("keeps disabled inline fixes and Plan Review read-only during graph preparation", async () => {
const requirements: any[] = [];
const graph = new WorkflowGraphExecutor({
prepareNodeExecution: (_node, _task, requirement) => { requirements.push(requirement); },
handlers: { gate: async () => ({ outcome: "success" }) },
});
const optionalGroup = (id: string, name: string) => ({
id,
kind: "optional-group" as const,
config: { name, defaultOn: true, template: { nodes: [{ id: `${id}-step`, kind: "gate" as const, config: { name } }], edges: [] } },
});
const ir: WorkflowIr = {
version: "v2",
name: "readonly-review-worktree-test",
columns: [],
nodes: [{ id: "start", kind: "start" }, optionalGroup("code-review", "Code Review"), optionalGroup("plan-review", "Plan Review"), { id: "end", kind: "end" }],
edges: [
{ from: "start", to: "code-review" },
{ from: "code-review", to: "plan-review", condition: "success" },
{ from: "plan-review", to: "end", condition: "success" },
],
};
await graph.run(baseStepTask({ enabledWorkflowSteps: ["code-review", "plan-review"] }) as any, {
experimentalFeatures: {},
reviewerInlineFixes: false,
}, ir);
expect(requirements).toEqual([]);
});
it("finalizes a merge-confirmed workflow graph task that is stranded before done", async () => {
const store = createMockStore();
let live = baseStepTask({

View File

@@ -0,0 +1,47 @@
import { describe, expect, it } from "vitest";
import type { WorkflowIrNode } from "@fusion/core";
import { workflowNodeRequiresWorktree } from "../workflow-node-execution-needs.js";
function node(overrides: Partial<WorkflowIrNode> = {}): WorkflowIrNode {
return { id: "node", kind: "prompt", ...overrides };
}
describe("workflowNodeRequiresWorktree", () => {
it.each([
["coding tool mode", node({ config: { toolMode: "coding" } })],
["script node", node({ kind: "script" })],
["named script", node({ config: { scriptName: "validate" } })],
["CLI command", node({ config: { executor: "cli", cliCommand: "pnpm lint" } })],
["CLI agent", node({ config: { executor: "cli-agent" } })],
])("requires a worktree for %s", (_name, workflowNode) => {
expect(workflowNodeRequiresWorktree(workflowNode)).toBe(true);
});
it.each([
["review name", node({ id: "review", config: { name: "Code Review" } }), undefined],
["verification name", node({ id: "verify", config: { name: "Browser Verification" } }), undefined],
["explicit inline fix config", node({ config: { reviewCanFixInline: true } }), undefined],
["code review optional group", node(), "code-review"],
["browser verification optional group", node(), "browser-verification"],
])("requires a worktree for inline fixes from %s", (_name, workflowNode, optionalGroupId) => {
expect(workflowNodeRequiresWorktree(workflowNode, { optionalGroupId })).toBe(true);
});
it("keeps inline-fix reviews read-only when disabled", () => {
expect(workflowNodeRequiresWorktree(node({ config: { name: "Code Review" } }), { reviewerInlineFixes: false })).toBe(false);
expect(workflowNodeRequiresWorktree(node(), {
optionalGroupId: "code-review",
reviewerInlineFixes: false,
})).toBe(false);
});
it.each([
node({ id: "plan-review-step", config: { name: "Code Review" } }),
node({ config: { name: "Plan Review" } }),
node(),
])("keeps Plan Review read-only", (workflowNode) => {
expect(workflowNodeRequiresWorktree(workflowNode, {
optionalGroupId: workflowNode.id === "node" ? "plan-review" : undefined,
})).toBe(false);
});
});

View File

@@ -46,6 +46,7 @@ import {
} from "./workflow-node-handlers.js";
import { MERGE_REGION_KINDS, WORKFLOW_NODE_ENGINE_PAUSE_ABORT_KIND, WORKFLOW_OPTIONAL_GROUP_CONTEXT_KEY } from "./workflow-graph-executor.js";
import type { WorkflowNodePreparationRequirement, WorkflowNodeResult } from "./workflow-graph-executor.js";
import { workflowNodeRequiresWorktree } from "./workflow-node-execution-needs.js";
import type {
AuditPrimitiveInput,
PreparedWorktree,
@@ -7687,34 +7688,23 @@ export class TaskExecutor {
const rawCliCommand = executorKind === "cli" && typeof cfg.cliCommand === "string" && cfg.cliCommand.trim()
? cfg.cliCommand.trim()
: undefined;
const nodeNameForReviewDetection = typeof cfg.name === "string" && cfg.name.trim() ? cfg.name.trim() : node.id;
const isPlanReviewNode =
node.id === "plan-review-step"
|| nodeNameForReviewDetection === "Plan Review"
|| optionalGroupId === "plan-review";
const inlineFixesEnabledForNode = (settings as Settings & { reviewerInlineFixes?: boolean }).reviewerInlineFixes !== false;
const reviewTypeNode =
isPlanReviewNode
|| cfg.reviewCanFixInline === true
|| /(?:^|\b)(?:review|verification)(?:\b|$)/i.test(nodeNameForReviewDetection)
|| optionalGroupId === "code-review"
|| optionalGroupId === "browser-verification";
const inlineFixesMakeNodeWriteCapable =
inlineFixesEnabledForNode
&& executorKind !== "cli"
&& reviewTypeNode
&& !isPlanReviewNode;
// Isolation guard: write-capable nodes must run inside a task worktree, not
// the shared repo root. Before the execute seam runs, live.worktree is unset
// — a coding/script/CLI node falling back to this.rootDir would mutate the
// main checkout and cross-contaminate other tasks. Reject such nodes until a
// worktree exists. Read-only nodes (default toolMode) are safe against root.
/*
FNXC:WorkflowReviewers 2026-07-01-13:28:
Inline-fix Code Review, Browser Verification, and custom review nodes become write-capable even when the workflow definition says `toolMode: readonly`, so the isolation guard must see that before selecting a worktree. Plan Review is excluded because it uses the narrow PROMPT.md writer instead of source-file write tools.
FNXC:WorkflowReviewers 2026-07-15-00:00:
Inline-fix Code Review, Browser Verification, and custom review nodes become
write-capable even when their workflow definition says `toolMode: readonly`.
Use the shared classifier consumed by graph preparation so issue #2075 cannot
leave runtime requiring a worktree that preparation declined to acquire.
Plan Review remains excluded because it uses the narrow PROMPT.md writer.
*/
const writeCapable = cfg.toolMode === "coding" || inlineFixesMakeNodeWriteCapable || node.kind === "script" || Boolean(scriptName) || Boolean(rawCliCommand);
const writeCapable = workflowNodeRequiresWorktree(node, {
optionalGroupId,
reviewerInlineFixes: (settings as Settings & { reviewerInlineFixes?: boolean }).reviewerInlineFixes,
});
const executionTarget = writeCapable ? await this.store.getTask(live.id) : live;
if (writeCapable && !executionTarget.worktree && !this.workspaceConfig) {
return { outcome: "failure", value: "no-worktree-for-write-node" };

View File

@@ -41,9 +41,14 @@ import {
} from "./workflow-graph-foreach.js";
import { runLoop, runOptionalGroup } from "./workflow-graph-loop.js";
import type { WorkflowNodeRunnerRegistry } from "./workflow-node-runner.js";
import { workflowNodeRequiresWorktree } from "./workflow-node-execution-needs.js";
export type WorkflowNodeOutcome = "success" | "failure";
type WorkflowNodeSettings = Pick<Settings, "experimentalFeatures"> & {
reviewerInlineFixes?: boolean;
};
export type WorkflowNodeAbortKind = "engine-pause";
export const WORKFLOW_INTERRUPTED_NODE_ID_CONTEXT_KEY = "workflow:interruptedNodeId";
@@ -79,7 +84,7 @@ export interface WorkflowTaskProjection {
export interface WorkflowNodeExecutionContext {
task: TaskDetail;
settings: Pick<Settings, "experimentalFeatures"> | undefined;
settings: WorkflowNodeSettings | undefined;
context: Record<string, unknown>;
/** Set during concurrent branch execution; fail-fast aborts via this signal.
* Undefined on the sequential path (zero behavior change for linear graphs). */
@@ -339,7 +344,7 @@ export class WorkflowGraphExecutor {
public async run(
task: TaskDetail,
settings: (Pick<Settings, "experimentalFeatures"> & Partial<Pick<Settings, "autoMerge">>) | undefined,
settings: (WorkflowNodeSettings & Partial<Pick<Settings, "autoMerge">>) | undefined,
ir: WorkflowIr = BUILTIN_CODING_WORKFLOW_IR,
): Promise<WorkflowGraphExecutorResult> {
const startNode = ir.nodes.find((node) => node.kind === "start");
@@ -1236,7 +1241,7 @@ export class WorkflowGraphExecutor {
private async executeNodeWithRetries(
node: WorkflowIrNode,
task: TaskDetail,
settings: Pick<Settings, "experimentalFeatures"> | undefined,
settings: WorkflowNodeSettings | undefined,
context: Record<string, unknown>,
workflow: WorkflowIr,
signal?: AbortSignal,
@@ -1255,7 +1260,7 @@ export class WorkflowGraphExecutor {
// Fail-fast cancellation: a branch or top-level graph abort mid-retry stops re-trying.
if (signal?.aborted) return this.withEnginePauseAbortContext(node, { outcome: "failure", value: "aborted" });
try {
await this.prepareNodeExecution(node, task);
await this.prepareNodeExecution(node, task, context, settings);
const progressRecord = recordProgress && this.shouldRecordNodeProgress(node)
? await this.recordNodeProgressStart(task.id, node)
: null;
@@ -1395,23 +1400,36 @@ export class WorkflowGraphExecutor {
});
}
private async prepareNodeExecution(node: WorkflowIrNode, task: TaskDetail): Promise<void> {
const requirement = this.classifyNodePreparation(node);
private async prepareNodeExecution(
node: WorkflowIrNode,
task: TaskDetail,
context: Record<string, unknown>,
settings: WorkflowNodeSettings | undefined,
): Promise<void> {
const requirement = this.classifyNodePreparation(node, context, settings);
if (!requirement.requiresWorktree) return;
await this.deps.prepareNodeExecution?.(node, task, requirement);
}
private classifyNodePreparation(node: WorkflowIrNode): WorkflowNodePreparationRequirement {
const cfg = node.config ?? {};
const executorKind = typeof cfg.executor === "string" ? cfg.executor : "model";
const hasScriptName = typeof cfg.scriptName === "string" && cfg.scriptName.trim().length > 0;
const hasCliCommand = executorKind === "cli" && typeof cfg.cliCommand === "string" && cfg.cliCommand.trim().length > 0;
const requiresWorktree =
cfg.toolMode === "coding"
|| node.kind === "script"
|| executorKind === "cli-agent"
|| hasScriptName
|| hasCliCommand;
private classifyNodePreparation(
node: WorkflowIrNode,
context: Record<string, unknown>,
settings: WorkflowNodeSettings | undefined,
): WorkflowNodePreparationRequirement {
const optionalGroupId = typeof context[WORKFLOW_OPTIONAL_GROUP_CONTEXT_KEY] === "string"
? context[WORKFLOW_OPTIONAL_GROUP_CONTEXT_KEY]
: undefined;
/*
* FNXC:WorkflowExecution 2026-07-15-00:00:
* Graph preparation receives the optional-group context and effective inline-fix
* setting so it applies the same classifier as runtime. Only an explicit false
* disables inline fixes, preserving the default-enabled review worktree contract
* that prevents issue #2075's pre-review no-worktree failure.
*/
const requiresWorktree = workflowNodeRequiresWorktree(node, {
optionalGroupId,
reviewerInlineFixes: settings?.reviewerInlineFixes,
});
return {
requiresWorktree,
reason: requiresWorktree ? "write-capable-node" : undefined,

View File

@@ -0,0 +1,46 @@
import type { WorkflowIrNode } from "@fusion/core";
export interface WorkflowNodeExecutionNeedsOptions {
optionalGroupId?: string;
/** Inline review fixes are enabled unless settings explicitly disable them. */
reviewerInlineFixes?: boolean;
}
/**
* FNXC:WorkflowExecution 2026-07-15-00:00:
* Issue #2075 exposed divergent worktree classifiers: graph preparation treated
* inline-fix reviews as read-only while runtime rejected them without a worktree.
* This pure helper is the single source of truth for write-capable workflow nodes;
* preparation and runtime must both use it before selecting an execution target.
*/
export function workflowNodeRequiresWorktree(
node: WorkflowIrNode,
{ optionalGroupId, reviewerInlineFixes }: WorkflowNodeExecutionNeedsOptions = {},
): boolean {
const cfg = node.config ?? {};
const executorKind = typeof cfg.executor === "string" ? cfg.executor : "model";
const scriptName = typeof cfg.scriptName === "string" && cfg.scriptName.trim()
? cfg.scriptName
: undefined;
const rawCliCommand = executorKind === "cli" && typeof cfg.cliCommand === "string" && cfg.cliCommand.trim()
? cfg.cliCommand
: undefined;
const nodeName = typeof cfg.name === "string" && cfg.name.trim() ? cfg.name.trim() : node.id;
const isPlanReview = node.id === "plan-review-step" || nodeName === "Plan Review" || optionalGroupId === "plan-review";
const isInlineFixReview = reviewerInlineFixes !== false
&& executorKind !== "cli"
&& !isPlanReview
&& (
cfg.reviewCanFixInline === true
|| /(?:^|\b)(?:review|verification)(?:\b|$)/i.test(nodeName)
|| optionalGroupId === "code-review"
|| optionalGroupId === "browser-verification"
);
return cfg.toolMode === "coding"
|| node.kind === "script"
|| executorKind === "cli-agent"
|| Boolean(scriptName)
|| Boolean(rawCliCommand)
|| isInlineFixReview;
}