fix(FN-7265): stop duplicate graph plan reviews
This commit is contained in:
7
.changeset/graph-review-gates-own-execution.md
Normal file
7
.changeset/graph-review-gates-own-execution.md
Normal 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.
|
||||||
@@ -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");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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, [
|
||||||
|
|||||||
@@ -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}
|
||||||
|
|||||||
Reference in New Issue
Block a user