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

.changeset/fn-7228-code-review-blocks.md

@@ -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.

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

@@ -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(

packages/core/src/__tests__/builtin-workflows.test.ts

@@ -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

packages/core/src/builtin-code-review-group.ts

@@ -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
+The inner node is a blocking gate: a REVISE verdict must stop review/merge until
-`code-review` step: a `prompt` node carrying the prompt, `toolMode` (readonly — review
+the executor remediates the finding or the task exhausts its revision budget.
-reads the diff, never mutates), and `gateMode` (advisory — non-blocking, like the
-existing review; operators can promote to a gate).
 
 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
+name/description/prompt/toolMode/gateMode are now inlined here directly. These built-ins
-to the former `code-review` catalog entry). These built-ins are the parity oracle, so
+are the parity oracle; intentional behavior changes must update this file and the
-the produced node bytes must NOT change.
+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
+ * A single `prompt` node whose config carries the inlined prompt +
- * the inlined prompt + `toolMode: "readonly"` + `gateMode: "advisory"`.
+ * `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",
             },
           },
         ],

packages/core/src/builtin-coding-workflow-ir.ts

@@ -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.

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

@@ -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-

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

@@ -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");
+  });
 });