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

fix(FN-7228): make built-in code review blocking

Browse Source
This commit is contained in:
gsxdsm
2026-06-29 10:16:17 -07:00
parent 6fc50d8d9e
commit 6239b2acb3
7 changed files with 112 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/fn-7228-code-review-blocks.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Make built-in Code Review block merge when it requests revisions.
category: fix
dev: Generic built-in code-review optional groups now use gateMode: gate; Browser Verification remains advisory.

14
packages/core/src/__tests__/builtin-coding-workflow-ir.test.ts
View File

@@ -8,6 +8,7 @@ import {
serializeWorkflowIr,
} from "../index.js";
import { BROWSER_VERIFICATION_GROUP_ID, BROWSER_VERIFICATION_STEP_NODE_ID } from "../builtin-browser-verification-group.js";
import { CODE_REVIEW_GROUP_ID, CODE_REVIEW_STEP_NODE_ID } from "../builtin-code-review-group.js";
import type { WorkflowIrV2 } from "../workflow-ir-types.js";
const EXECUTE_NODE_MAX_RETRIES = 2;
@@ -21,6 +22,15 @@ function browserVerificationInnerConfig(ir: WorkflowIrV2): Record<string, unknow
return inner?.config ?? {};
}
function codeReviewInnerConfig(ir: WorkflowIrV2): Record<string, unknown> {
const group = ir.nodes.find((node) => node.id === CODE_REVIEW_GROUP_ID);
expect(group?.kind).toBe("optional-group");
const template = group?.config?.template as { nodes?: Array<{ id: string; config?: Record<string, unknown> }> } | undefined;
const inner = template?.nodes?.find((node) => node.id === CODE_REVIEW_STEP_NODE_ID);
expect(inner).toBeDefined();
return inner?.config ?? {};
}
function executeNodeConfig(ir = BUILTIN_CODING_WORKFLOW_IR): Record<string, unknown> {
const executeNodes = ir.nodes.filter((node) => node.id === "execute" && node.config?.seam === "execute");
expect(executeNodes).toHaveLength(1);
@@ -69,6 +79,10 @@ describe("builtin coding workflow ir", () => {
gateMode: "advisory",
requiresBrowser: true,
});
expect(codeReviewInnerConfig(BUILTIN_CODING_WORKFLOW_IR)).toMatchObject({
toolMode: "readonly",
gateMode: "gate",
});
// execute → browser-verification → code-review → review on the success path; the
// pre-merge code-review optional-group sits next to browser-verification. failure → end.
expect(BUILTIN_CODING_WORKFLOW_IR.edges).toEqual(

12
packages/core/src/__tests__/builtin-workflows.test.ts
View File

@@ -101,6 +101,18 @@ describe("built-in workflows", () => {
}
});
it("all built-in Code Review optional groups are blocking gates", () => {
for (const workflow of BUILTIN_WORKFLOWS) {
const codeReview = workflow.ir.nodes.find((node) => node.id === "code-review");
if (!codeReview) continue;
expect(codeReview.kind, workflow.id).toBe("optional-group");
const template = codeReview.config?.template as { nodes?: Array<{ id: string; config?: Record<string, unknown> }> } | undefined;
const inner = template?.nodes?.find((node) => node.id === CODE_REVIEW_STEP_NODE_ID);
expect(inner, workflow.id).toBeDefined();
expect(inner?.config?.gateMode, workflow.id).toBe("gate");
}
});
it("built-in workflow layouts cover every authored node", () => {
for (const workflow of BUILTIN_WORKFLOWS) {
const missingLayoutNodes = workflow.ir.nodes

25
packages/core/src/builtin-code-review-group.ts
View File

@@ -14,16 +14,21 @@ The group sits on the pre-merge success path (execute → [browser-verification
key; the inner template node carries a DISTINCT id (`code-review-step`) because a template
node id may not collide with the group/top-level node id (U1 validation).
The inner node mirrors the dashboard's `stepTemplateToNode` projection of the canonical
`code-review` step: a `prompt` node carrying the prompt, `toolMode` (readonly — review
reads the diff, never mutates), and `gateMode` (advisory — non-blocking, like the
existing review; operators can promote to a gate).
The inner node is a blocking gate: a REVISE verdict must stop review/merge until
the executor remediates the finding or the task exhausts its revision budget.
FNXC:CodeReviewStep 2026-06-25-00:00:
U6 deleted the built-in step-template catalog; the inner node's literal
name/description/prompt/toolMode/gateMode are now inlined here directly (byte-identical
to the former `code-review` catalog entry). These built-ins are the parity oracle, so
the produced node bytes must NOT change.
name/description/prompt/toolMode/gateMode are now inlined here directly. These built-ins
are the parity oracle; intentional behavior changes must update this file and the
built-in workflow tests together.
FNXC:CodeReviewStep 2026-06-29-10:42:
Code Review is not advisory. FN-7228 reached merge after Code Review returned REVISE
because the generic built-in was authored as advisory and the graph continued after the
remediation budget was exhausted. Keep browser verification advisory, but make Code
Review a gate so REVISE records a blocking failed workflow step and cannot advance to
review or merge.
*/
/** Stable per-task enable key + group node id. */
@@ -70,8 +75,8 @@ Be specific: cite \`file:line\` for every finding and explain the concrete failu
* matches where the browser-verification group sits (in-progress) so the editor renders
* the group in the implementation column.
*
* Mirrors `stepTemplateToNode(code-review)`: a single `prompt` node whose config carries
* the inlined prompt + `toolMode: "readonly"` + `gateMode: "advisory"`.
* A single `prompt` node whose config carries the inlined prompt +
* `toolMode: "readonly"` + blocking `gateMode: "gate"`.
*/
export function codeReviewOptionalGroupNode(
column: string,
@@ -96,7 +101,7 @@ export function codeReviewOptionalGroupNode(
description: CODE_REVIEW_DESCRIPTION,
prompt: CODE_REVIEW_PROMPT,
toolMode: "readonly",
gateMode: "advisory",
gateMode: "gate",
},
},
],

2
packages/core/src/builtin-coding-workflow-ir.ts
View File

@@ -79,7 +79,7 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = {
// Pre-merge optional browser-verification (optional-group, default OFF).
browserVerificationOptionalGroupNode("in-progress"),
// FNXC:CodeReviewStep 2026-06-25-15:00:
// Pre-merge Code Review as a DEFAULT-ON optional-group (advisory), on the success path
// Pre-merge Code Review as a DEFAULT-ON optional-group (blocking gate), on the success path
// between browser-verification and review (execute → browser-verification →
// code-review → review). Runs for every coding task by default (defaultOn:true) but is
// toggleable off per task; disabled → byte-inert pass-through.

2
packages/core/src/builtin-stepwise-coding-workflow-ir.ts
View File

@@ -143,7 +143,7 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = {
// and the rework-exhausted manual-release path flow through this node.
browserVerificationOptionalGroupNode("in-progress"),
// FNXC:CodeReviewStep 2026-06-25-15:00:
// Pre-merge Code Review as a DEFAULT-ON optional-group (advisory), on the post-foreach
// Pre-merge Code Review as a DEFAULT-ON optional-group (blocking gate), on the post-foreach
// success path between browser-verification and review (steps → browser-verification →
// code-review → review). It sits after the foreach so it runs EXACTLY ONCE pre-merge
// (never per step-instance); both the foreach-success and rework-exhausted manual-

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

@@ -723,7 +723,11 @@ describe("WorkflowGraphExecutor optional-group", () => {
calls.push(node.id);
if ((groupId === "code-review" && node.id === "code-review-step")
|| (groupId === "browser-verification" && node.id === "browser-verification-step")) {
return { outcome: "success", value: "REVISE", contextPatch: { output: `${groupId} finding` } };
return {
outcome: groupId === "code-review" ? "failure" : "success",
value: "REVISE",
contextPatch: { output: `${groupId} finding` },
};
}
return { outcome: "success" };
},
@@ -767,7 +771,11 @@ describe("WorkflowGraphExecutor optional-group", () => {
prompt: async (node) => {
if ((groupId === "code-review" && node.id === "code-review-step")
|| (groupId === "browser-verification" && node.id === "browser-verification-step")) {
return { outcome: "success", value: "REVISE", contextPatch: { output: `stepwise ${groupId} finding` } };
return {
outcome: groupId === "code-review" ? "failure" : "success",
value: "REVISE",
contextPatch: { output: `stepwise ${groupId} finding` },
};
}
return { outcome: "success" };
},
@@ -790,4 +798,56 @@ describe("WorkflowGraphExecutor optional-group", () => {
expect(stepwiseResult.context[`node:${groupId}:fixScheduled`]).toBe(true);
}
});
it("blocks builtin coding review and merge when Code Review requests revision and no remediation is scheduled", async () => {
const requestFix = vi.fn(async () => false);
const calls: string[] = [];
const executor = new WorkflowGraphExecutor({
handlers: {
"parse-steps": async () => ({ outcome: "success", value: "no-steps" }),
prompt: async (node) => {
calls.push(node.id);
if (node.id === "code-review-step") {
return {
outcome: "failure",
value: "REVISE",
contextPatch: { output: "blocking code review finding" },
};
}
return { outcome: "success" };
},
},
requestPreMergeOptionalStepFix: requestFix,
});
const result = await executor.run({
...taskWith(["plan-review", "code-review"]),
id: "FN-7228-regression",
steps: [],
workflowStepResults: [
{
workflowStepId: "plan-review",
workflowStepName: "Plan Review",
phase: "pre-merge",
status: "passed",
startedAt: "2026-06-29T17:00:00.000Z",
completedAt: "2026-06-29T17:00:01.000Z",
},
],
} as TaskDetail, settingsOn(), BUILTIN_CODING_WORKFLOW_IR);
expect(requestFix).toHaveBeenCalledWith("FN-7228-regression", expect.objectContaining({
stepName: "Code Review",
feedback: "blocking code review finding",
nodeId: "code-review",
status: "failed",
verdict: "REVISE",
}));
expect(result.outcome).toBe("failure");
expect(result.visitedNodeIds).toContain("code-review::code-review-step");
expect(result.visitedNodeIds).not.toContain("review");
expect(result.visitedNodeIds).not.toContain("merge-gate");
expect(result.visitedNodeIds).not.toContain("merge-attempt");
expect(calls).not.toContain("review");
});
});