fix(FN-7265): stop duplicate graph plan reviews

This commit is contained in:
gsxdsm
2026-06-29 22:29:22 -07:00
parent 5a6d110483
commit 9e23d6c403
4 changed files with 100 additions and 6 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Prevent workflow tasks from duplicating plan review during implementation.
category: fix
dev: Graph-owned execution sessions no longer receive or prompt for legacy per-step review tools.

View File

@@ -272,4 +272,31 @@ describe("fast mode workflow/runtime invariants", () => {
const tools = mockedCreateFnAgent.mock.calls[0][0].customTools.map((tool: any) => tool.name); const tools = mockedCreateFnAgent.mock.calls[0][0].customTools.map((tool: any) => tool.name);
expect(tools).toContain("fn_review_step"); expect(tools).toContain("fn_review_step");
}); });
it("omits legacy fn_review_step in graph-owned standard execution sessions", async () => {
mockedCreateFnAgent.mockImplementation(async (opts: any) => ({
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
sessionManager: {
getLeafId: vi.fn().mockReturnValue("leaf"),
branchWithSummary: vi.fn(),
navigateTree: vi.fn().mockResolvedValue({ cancelled: false }),
},
navigateTree: vi.fn().mockResolvedValue({ cancelled: false }),
},
capturedTools: opts.customTools,
}));
const store = createMockStore();
const liveTask = task({ id: "FN-TOOLS", executionMode: "standard" });
store.getTask.mockResolvedValue(liveTask);
const executor = new TaskExecutor(store, "/tmp/test") as any;
executor.graphCompletionInterceptors.set("FN-TOOLS", vi.fn());
await executor.execute(liveTask);
const tools = mockedCreateFnAgent.mock.calls[0][0].customTools.map((tool: any) => tool.name);
expect(tools).toContain("fn_task_done");
expect(tools).not.toContain("fn_review_step");
});
}); });

View File

@@ -145,6 +145,38 @@ describe("buildExecutionPrompt", () => {
expect(result).not.toContain("## Attachments"); expect(result).not.toContain("## Attachments");
}); });
it("does not instruct graph-owned execution sessions to request per-step reviews", () => {
const task = createMockTaskDetail({
prompt: [
"# test",
"**Review Level:** 2",
"## Steps",
"### Step 0: Preflight",
"- [ ] check",
"### Step 1: Implement",
"- [ ] change code",
"### Step 2: Delivery",
"- [ ] summarize",
].join("\n"),
});
const result = buildExecutionPrompt(
task,
"/home/user/project",
undefined,
undefined,
undefined,
undefined,
undefined,
{ workflowReviewGatesOwnedByGraph: true },
);
expect(result).toContain("Workflow review gates are handled by the workflow graph");
expect(result).not.toContain("Before implementing each step");
expect(result).not.toContain("After implementing + committing each step");
expect(result).not.toContain("fn_review_step");
});
it("includes Custom fields section listing id/name/type, enum options, required, and current value", () => { it("includes Custom fields section listing id/name/type, enum options, required, and current value", () => {
const task = createMockTaskDetail({ customFields: { severity: "high" } }); const task = createMockTaskDetail({ customFields: { severity: "high" } });
const result = buildExecutionPrompt(task, "/home/user/project", undefined, undefined, undefined, [ const result = buildExecutionPrompt(task, "/home/user/project", undefined, undefined, undefined, [

View File

@@ -9560,8 +9560,11 @@ export class TaskExecutor {
error: (s) => executorLog.warn(s), error: (s) => executorLog.warn(s),
}, },
}), }),
// Skip fn_review_step tool in fast mode — fast mode bypasses automated review gates /*
...(executionMode !== "fast" ? [ FNXC:WorkflowReviewGates 2026-06-29-20:41:
Workflow-graph execution owns plan/code/browser review gates as nodes. Do not expose legacy in-session `fn_review_step` during graph-owned execute seams; otherwise default coding can duplicate Plan Review inside implementation steps after the workflow-level Plan Review has already passed.
*/
...(executionMode !== "fast" && !this.graphCompletionInterceptors.has(task.id) ? [
this.createReviewStepTool(task.id, worktreePath, detail.prompt, codeReviewVerdicts, sessionRef, stepCheckpoints, detail, stuckDetector), this.createReviewStepTool(task.id, worktreePath, detail.prompt, codeReviewVerdicts, sessionRef, stepCheckpoints, detail, stuckDetector),
] : []), ] : []),
this.createSpawnAgentTool(task.id, worktreePath, settings, taskEnv), this.createSpawnAgentTool(task.id, worktreePath, settings, taskEnv),
@@ -9903,6 +9906,7 @@ export class TaskExecutor {
this.options.pluginRunner, this.options.pluginRunner,
customFieldDefs, customFieldDefs,
this.workspaceConfig, this.workspaceConfig,
{ workflowReviewGatesOwnedByGraph: this.graphCompletionInterceptors.has(task.id) },
); );
await promptWithFallback(session, agentPrompt); await promptWithFallback(session, agentPrompt);
} }
@@ -10273,7 +10277,16 @@ export class TaskExecutor {
"Do NOT ask for permission. Do NOT write a summary. Just call a tool and keep working.", "Do NOT ask for permission. Do NOT write a summary. Just call a tool and keep working.",
"", "",
"Original task:", "Original task:",
buildExecutionPrompt(detail, this.rootDir, settings, worktreePath, this.options.pluginRunner, retryCustomFieldDefs, this.workspaceConfig), buildExecutionPrompt(
detail,
this.rootDir,
settings,
worktreePath,
this.options.pluginRunner,
retryCustomFieldDefs,
this.workspaceConfig,
{ workflowReviewGatesOwnedByGraph: this.graphCompletionInterceptors.has(task.id) },
),
].join("\n"); ].join("\n");
} else { } else {
retryPrompt = [ retryPrompt = [
@@ -10283,7 +10296,16 @@ export class TaskExecutor {
"2. If there is remaining work, finish it and then call fn_task_done.", "2. If there is remaining work, finish it and then call fn_task_done.",
"", "",
"Original task:", "Original task:",
buildExecutionPrompt(detail, this.rootDir, settings, worktreePath, this.options.pluginRunner, retryCustomFieldDefs, this.workspaceConfig), buildExecutionPrompt(
detail,
this.rootDir,
settings,
worktreePath,
this.options.pluginRunner,
retryCustomFieldDefs,
this.workspaceConfig,
{ workflowReviewGatesOwnedByGraph: this.graphCompletionInterceptors.has(task.id) },
),
].join("\n"); ].join("\n");
} }
@@ -16980,9 +17002,15 @@ export function buildExecutionPrompt(
pluginRunner?: PluginRunner, pluginRunner?: PluginRunner,
customFieldDefs?: WorkflowFieldDefinition[], customFieldDefs?: WorkflowFieldDefinition[],
workspaceConfig?: WorkspaceConfig | null, workspaceConfig?: WorkspaceConfig | null,
options?: { workflowReviewGatesOwnedByGraph?: boolean },
): string { ): string {
const prompt = scopePromptToWorktree(task.prompt, rootDir, worktreePath, workspaceConfig); const prompt = scopePromptToWorktree(task.prompt, rootDir, worktreePath, workspaceConfig);
const reviewLevel = parseReviewLevelFromPrompt(prompt); const reviewLevel = parseReviewLevelFromPrompt(prompt);
/*
* FNXC:WorkflowReviewGates 2026-06-29-20:41:
* Default Coding and other workflow-graph tasks run review gates as graph nodes, so the executor prompt must not ask implementation agents to call legacy per-step review tools. This keeps Plan Review once-before-execution and Code Review once-before-merge unless a workflow explicitly adds a step-review node.
*/
const workflowReviewGatesOwnedByGraph = options?.workflowReviewGatesOwnedByGraph === true;
// Build co-author trailer arg for git commits based on settings. The user's // Build co-author trailer arg for git commits based on settings. The user's
// configured git identity remains the primary author; Fusion is appended as // configured git identity remains the primary author; Fusion is appended as
@@ -17126,12 +17154,12 @@ ${prompt}
${attachmentsSection}${commandsSection}${memorySection}${progressSection}${steeringSection}${customFieldsSection} ${attachmentsSection}${commandsSection}${memorySection}${progressSection}${steeringSection}${customFieldsSection}
## Review level: ${reviewLevel} ## Review level: ${reviewLevel}
${reviewLevel === 0 ? "No reviews required. Implement directly." : ""} ${workflowReviewGatesOwnedByGraph ? `Workflow review gates are handled by the workflow graph outside this implementation session. Do not request per-step plan review or per-step code review from inside execution; complete the implementation steps and let the graph run enabled Plan Review, Browser Verification, and Code Review nodes at their configured positions.` : `${reviewLevel === 0 ? "No reviews required. Implement directly." : ""}
${reviewLevel >= 1 ? `Before implementing each step (except Step 0 and the final step), call: ${reviewLevel >= 1 ? `Before implementing each step (except Step 0 and the final step), call:
\`fn_review_step(step=N, type="plan", step_name="...")\`` : ""} \`fn_review_step(step=N, type="plan", step_name="...")\`` : ""}
${reviewLevel >= 2 ? `After implementing + committing each step, call: ${reviewLevel >= 2 ? `After implementing + committing each step, call:
\`fn_review_step(step=N, type="code", step_name="...", baseline="<SHA from before step>")\`` : ""} \`fn_review_step(step=N, type="code", step_name="...", baseline="<SHA from before step>")\`` : ""}
${reviewLevel >= 3 ? `After tests, also call fn_review_step with type="code" for test review.` : ""} ${reviewLevel >= 3 ? `After tests, also call fn_review_step with type="code" for test review.` : ""}`}
${pluginTaskContributions ? ` ${pluginTaskContributions ? `
${pluginTaskContributions} ${pluginTaskContributions}