feat: add built-in Code Review pre-merge workflow step
Add a configurable "Code Review" diff-review step to the built-in coding and stepwise coding workflows as a default-OFF optional-group prompt gate. It reuses the existing workflow-step machinery and the shared trailing-verdict convention (REVISE blocks, APPROVE/APPROVE_WITH_NOTES pass) — no engine verification code. - New `code-review` WORKFLOW_STEP_TEMPLATE (toolMode readonly, gateMode advisory, phase pre-merge) focused on the correctness value tests miss: logic bugs, edge cases, intent-vs-implementation drift, regressions, error handling, contracts. - New builtin-code-review-group.ts mirroring builtin-browser-verification-group.ts (stable group id `code-review`, distinct inner node id `code-review-step`). - Wired into builtin-coding-workflow-ir.ts and builtin-stepwise-coding-workflow-ir.ts on the pre-merge path next to browser-verification, default OFF / opt-in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/code-review-workflow-step.md
Normal file
7
.changeset/code-review-workflow-step.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": minor
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Add an optional built-in "Code Review" pre-merge step to the coding workflows.
|
||||||
|
category: feature
|
||||||
|
dev: New `code-review` WORKFLOW_STEP_TEMPLATE (toolMode readonly, gateMode advisory, phase pre-merge) plus a default-OFF `optional-group` node (group id `code-review`, inner `code-review-step`) wired into the built-in coding + stepwise coding workflows next to browser-verification. Opt-in via task `enabledWorkflowSteps`; reuses the shared prompt-gate verdict machinery (no engine verification code).
|
||||||
115
packages/core/src/__tests__/builtin-code-review-group.test.ts
Normal file
115
packages/core/src/__tests__/builtin-code-review-group.test.ts
Normal file
@@ -0,0 +1,115 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import {
|
||||||
|
CODE_REVIEW_GROUP_ID,
|
||||||
|
CODE_REVIEW_STEP_NODE_ID,
|
||||||
|
codeReviewOptionalGroupNode,
|
||||||
|
} from "../builtin-code-review-group.js";
|
||||||
|
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
|
||||||
|
import { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "../builtin-stepwise-coding-workflow-ir.js";
|
||||||
|
import { WORKFLOW_STEP_TEMPLATES } from "../types.js";
|
||||||
|
import { parseWorkflowIr, serializeWorkflowIr } from "../workflow-ir.js";
|
||||||
|
import {
|
||||||
|
resolveDefaultOnOptionalGroupIds,
|
||||||
|
resolveWorkflowOptionalSteps,
|
||||||
|
} from "../workflow-optional-steps.js";
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:CodeReviewStep 2026-06-25-12:00:
|
||||||
|
Coverage for the built-in "Code Review" pre-merge workflow step: the catalog template
|
||||||
|
fields, the default-OFF optional-group node built from it, and its wiring into the coding
|
||||||
|
+ stepwise built-ins. Mirrors the browser-verification group/IR suite — code review is a
|
||||||
|
WORKFLOW prompt-gate (shared verdict machinery), not engine verification code.
|
||||||
|
*/
|
||||||
|
|
||||||
|
describe("code-review WORKFLOW_STEP_TEMPLATE", () => {
|
||||||
|
const template = WORKFLOW_STEP_TEMPLATES.find((t) => t.id === "code-review");
|
||||||
|
|
||||||
|
it("exists with the expected catalog fields", () => {
|
||||||
|
expect(template).toBeTruthy();
|
||||||
|
expect(template!.name).toBe("Code Review");
|
||||||
|
expect(template!.toolMode).toBe("readonly");
|
||||||
|
// Advisory default → non-blocking until an operator promotes it to a gate (parity
|
||||||
|
// with browser-verification).
|
||||||
|
expect(template!.gateMode).toBe("advisory");
|
||||||
|
expect(template!.phase).toBe("pre-merge");
|
||||||
|
expect(template!.description.length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ends with the shared trailing verdict convention and reads the diff", () => {
|
||||||
|
const prompt = template!.prompt;
|
||||||
|
expect(prompt).toMatch(/"verdict":"APPROVE\|APPROVE_WITH_NOTES\|REVISE"/);
|
||||||
|
expect(prompt).not.toContain('"verdict":"PASS"');
|
||||||
|
expect(prompt).not.toContain('"verdict":"FAIL"');
|
||||||
|
// Focused on the value tests miss + reads the diff against the base.
|
||||||
|
expect(prompt).toMatch(/git diff/);
|
||||||
|
expect(prompt).toMatch(/out of scope/i);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("codeReviewOptionalGroupNode", () => {
|
||||||
|
it("builds a default-OFF optional-group with the stable group id and distinct inner id", () => {
|
||||||
|
const node = codeReviewOptionalGroupNode("in-progress");
|
||||||
|
expect(node.id).toBe(CODE_REVIEW_GROUP_ID);
|
||||||
|
expect(CODE_REVIEW_GROUP_ID).toBe("code-review");
|
||||||
|
expect(CODE_REVIEW_STEP_NODE_ID).toBe("code-review-step");
|
||||||
|
expect(node.id).not.toBe(CODE_REVIEW_STEP_NODE_ID); // U1: inner id ≠ group id.
|
||||||
|
expect(node.kind).toBe("optional-group");
|
||||||
|
expect(node.column).toBe("in-progress");
|
||||||
|
expect(node.config?.name).toBe("Code Review");
|
||||||
|
expect(node.config?.defaultOn).toBe(false);
|
||||||
|
|
||||||
|
const template = node.config?.template as { nodes: { id: string; kind: string; config?: Record<string, unknown> }[] };
|
||||||
|
expect(template.nodes).toHaveLength(1);
|
||||||
|
const inner = template.nodes[0];
|
||||||
|
expect(inner.id).toBe(CODE_REVIEW_STEP_NODE_ID);
|
||||||
|
expect(inner.kind).toBe("prompt");
|
||||||
|
expect(inner.config?.toolMode).toBe("readonly");
|
||||||
|
expect(inner.config?.gateMode).toBe("advisory");
|
||||||
|
expect(String(inner.config?.prompt)).toMatch(/"verdict":"APPROVE\|APPROVE_WITH_NOTES\|REVISE"/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("built-in coding + stepwise workflows wire code-review default-OFF on the pre-merge path", () => {
|
||||||
|
it.each([
|
||||||
|
["builtin coding", BUILTIN_CODING_WORKFLOW_IR],
|
||||||
|
["builtin stepwise", BUILTIN_STEPWISE_CODING_WORKFLOW_IR],
|
||||||
|
])("%s includes the code-review optional-group and still parses/round-trips", (_name, ir) => {
|
||||||
|
const byId = new Map(ir.nodes.map((n) => [n.id, n]));
|
||||||
|
const group = byId.get("code-review");
|
||||||
|
expect(group?.kind).toBe("optional-group");
|
||||||
|
expect(group?.config?.name).toBe("Code Review");
|
||||||
|
expect(group?.config?.defaultOn).toBe(false);
|
||||||
|
expect(group?.column).toBe("in-progress");
|
||||||
|
|
||||||
|
// Pre-merge wiring: ... → browser-verification → code-review → review; failure → end.
|
||||||
|
expect(ir.edges).toEqual(
|
||||||
|
expect.arrayContaining([
|
||||||
|
expect.objectContaining({ from: "browser-verification", to: "code-review", condition: "success" }),
|
||||||
|
expect.objectContaining({ from: "code-review", to: "review", condition: "success" }),
|
||||||
|
expect.objectContaining({ from: "code-review", to: "end", condition: "failure" }),
|
||||||
|
]),
|
||||||
|
);
|
||||||
|
|
||||||
|
// The built-in still compiles/validates with the new node (parse round-trips).
|
||||||
|
const reparsed = parseWorkflowIr(serializeWorkflowIr(ir));
|
||||||
|
expect(reparsed).toEqual(parseWorkflowIr(ir));
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["builtin coding", BUILTIN_CODING_WORKFLOW_IR],
|
||||||
|
["builtin stepwise", BUILTIN_STEPWISE_CODING_WORKFLOW_IR],
|
||||||
|
])("%s: code-review is advertised as an opt-in toggle but never default-seeded", (_name, ir) => {
|
||||||
|
// Enabling it (task.enabledWorkflowSteps includes `code-review`) surfaces it on the
|
||||||
|
// pre-merge path; the resolver advertises the toggle keyed by the group id.
|
||||||
|
const advertised = resolveWorkflowOptionalSteps(ir).find((s) => s.templateId === "code-review");
|
||||||
|
expect(advertised).toEqual({
|
||||||
|
templateId: "code-review",
|
||||||
|
name: "Code Review",
|
||||||
|
description: "",
|
||||||
|
phase: "pre-merge",
|
||||||
|
defaultOn: false,
|
||||||
|
});
|
||||||
|
// Default OFF → byte-inert pass-through: never auto-seeded into a new task.
|
||||||
|
expect(resolveDefaultOnOptionalGroupIds(ir)).not.toContain("code-review");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -53,11 +53,13 @@ describe("builtin coding workflow ir", () => {
|
|||||||
expect(group?.kind).toBe("optional-group");
|
expect(group?.kind).toBe("optional-group");
|
||||||
expect(group?.config?.name).toBe("Browser Verification");
|
expect(group?.config?.name).toBe("Browser Verification");
|
||||||
expect(group?.config?.defaultOn).toBe(false);
|
expect(group?.config?.defaultOn).toBe(false);
|
||||||
// execute → browser-verification → review on the success path; failure → end.
|
// 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(
|
expect(BUILTIN_CODING_WORKFLOW_IR.edges).toEqual(
|
||||||
expect.arrayContaining([
|
expect.arrayContaining([
|
||||||
expect.objectContaining({ from: "execute", to: "browser-verification", condition: "success" }),
|
expect.objectContaining({ from: "execute", to: "browser-verification", condition: "success" }),
|
||||||
expect.objectContaining({ from: "browser-verification", to: "review", condition: "success" }),
|
expect.objectContaining({ from: "browser-verification", to: "code-review", condition: "success" }),
|
||||||
|
expect.objectContaining({ from: "code-review", to: "review", condition: "success" }),
|
||||||
expect.objectContaining({ from: "browser-verification", to: "end", condition: "failure" }),
|
expect.objectContaining({ from: "browser-verification", to: "end", condition: "failure" }),
|
||||||
]),
|
]),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -108,10 +108,11 @@ describe("resolveWorkflowOptionalSteps (optional-group nodes)", () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("resolves the built-in coding/stepwise browser-verification optional-group (U6)", () => {
|
it("resolves the built-in coding/stepwise browser-verification + code-review optional-groups", () => {
|
||||||
// U6 migrated both built-ins: `browser-verification` is now an optional-group
|
// U6 migrated both built-ins: `browser-verification` is an optional-group node
|
||||||
// node (default OFF), so the resolver advertises exactly one toggle entry per
|
// (default OFF). The pre-merge `code-review` optional-group (also default OFF) sits
|
||||||
// built-in, keyed by the group node id `browser-verification`.
|
// next to it on the same success path, so the resolver advertises exactly these two
|
||||||
|
// toggle entries per built-in, in node order (browser-verification → code-review).
|
||||||
const expected = [
|
const expected = [
|
||||||
{
|
{
|
||||||
templateId: "browser-verification",
|
templateId: "browser-verification",
|
||||||
@@ -120,6 +121,13 @@ describe("resolveWorkflowOptionalSteps (optional-group nodes)", () => {
|
|||||||
phase: "pre-merge" as const,
|
phase: "pre-merge" as const,
|
||||||
defaultOn: false,
|
defaultOn: false,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
templateId: "code-review",
|
||||||
|
name: "Code Review",
|
||||||
|
description: "",
|
||||||
|
phase: "pre-merge" as const,
|
||||||
|
defaultOn: false,
|
||||||
|
},
|
||||||
];
|
];
|
||||||
expect(resolveWorkflowOptionalSteps(BUILTIN_CODING_WORKFLOW_IR)).toEqual(expected);
|
expect(resolveWorkflowOptionalSteps(BUILTIN_CODING_WORKFLOW_IR)).toEqual(expected);
|
||||||
expect(resolveWorkflowOptionalSteps(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).toEqual(expected);
|
expect(resolveWorkflowOptionalSteps(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).toEqual(expected);
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ const TARGET_IDS = [
|
|||||||
"performance-review",
|
"performance-review",
|
||||||
"accessibility-check",
|
"accessibility-check",
|
||||||
"browser-verification",
|
"browser-verification",
|
||||||
|
"code-review",
|
||||||
"frontend-ux-design",
|
"frontend-ux-design",
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
|
|||||||
76
packages/core/src/builtin-code-review-group.ts
Normal file
76
packages/core/src/builtin-code-review-group.ts
Normal file
@@ -0,0 +1,76 @@
|
|||||||
|
import type { WorkflowIrNode } from "./workflow-ir-types.js";
|
||||||
|
import { WORKFLOW_STEP_TEMPLATES } from "./types.js";
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:CodeReviewStep 2026-06-25-12:00:
|
||||||
|
The built-in coding and stepwise-coding workflows express the optional `code-review`
|
||||||
|
step as an `optional-group` container node on the pre-merge path (default OFF),
|
||||||
|
mirroring `builtin-browser-verification-group.ts` exactly. Enabled (the task's
|
||||||
|
`enabledWorkflowSteps` includes the group id) → the diff-review step runs ONCE
|
||||||
|
pre-merge between implementation and review. Disabled → the group passes through
|
||||||
|
(byte-inert), so the step is purely opt-in.
|
||||||
|
|
||||||
|
The group node id `code-review` is the STABLE per-task enable key (KTD-2). 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` WORKFLOW_STEP_TEMPLATE: a `prompt` node carrying the template's prompt,
|
||||||
|
`toolMode` (readonly — review reads the diff, never mutates), and `gateMode` (advisory
|
||||||
|
default — non-blocking until an operator promotes it). Sourcing prompt/toolMode/gateMode
|
||||||
|
from the catalog keeps the built-in byte-identical to the template a human would insert
|
||||||
|
from the palette (KTD-5).
|
||||||
|
*/
|
||||||
|
|
||||||
|
function resolveCodeReviewTemplate() {
|
||||||
|
const tpl = WORKFLOW_STEP_TEMPLATES.find((t) => t.id === "code-review");
|
||||||
|
if (!tpl) {
|
||||||
|
throw new Error("code-review WORKFLOW_STEP_TEMPLATE is missing");
|
||||||
|
}
|
||||||
|
return tpl;
|
||||||
|
}
|
||||||
|
|
||||||
|
const CODE_REVIEW_TEMPLATE = resolveCodeReviewTemplate();
|
||||||
|
|
||||||
|
/** Stable per-task enable key + group node id. */
|
||||||
|
export const CODE_REVIEW_GROUP_ID = "code-review";
|
||||||
|
|
||||||
|
/** Inner template node id — distinct from the group id (template-node-id collision rule, U1). */
|
||||||
|
export const CODE_REVIEW_STEP_NODE_ID = "code-review-step";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build the `code-review` optional-group node placed on a workflow's pre-merge path.
|
||||||
|
* `column` 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 catalog prompt + `toolMode: "readonly"` + `gateMode: "advisory"`.
|
||||||
|
*/
|
||||||
|
export function codeReviewOptionalGroupNode(column: string): WorkflowIrNode {
|
||||||
|
const tpl = CODE_REVIEW_TEMPLATE;
|
||||||
|
return {
|
||||||
|
id: CODE_REVIEW_GROUP_ID,
|
||||||
|
kind: "optional-group",
|
||||||
|
column,
|
||||||
|
config: {
|
||||||
|
name: tpl.name,
|
||||||
|
defaultOn: false,
|
||||||
|
template: {
|
||||||
|
nodes: [
|
||||||
|
{
|
||||||
|
id: CODE_REVIEW_STEP_NODE_ID,
|
||||||
|
kind: "prompt",
|
||||||
|
config: {
|
||||||
|
name: tpl.name,
|
||||||
|
description: tpl.description,
|
||||||
|
prompt: tpl.prompt ?? "",
|
||||||
|
toolMode: tpl.toolMode === "coding" ? "coding" : "readonly",
|
||||||
|
gateMode: tpl.gateMode ?? "advisory",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
edges: [],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@ import { parseWorkflowIr } from "./workflow-ir.js";
|
|||||||
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
||||||
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
||||||
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
||||||
|
import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The built-in default workflow as a v2 IR. Its six columns have ids that are
|
* The built-in default workflow as a v2 IR. Its six columns have ids that are
|
||||||
@@ -75,6 +76,12 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = {
|
|||||||
},
|
},
|
||||||
// Pre-merge optional browser-verification (optional-group, default OFF).
|
// Pre-merge optional browser-verification (optional-group, default OFF).
|
||||||
browserVerificationOptionalGroupNode("in-progress"),
|
browserVerificationOptionalGroupNode("in-progress"),
|
||||||
|
// FNXC:CodeReviewStep 2026-06-25-12:00:
|
||||||
|
// Pre-merge optional Code Review (optional-group, default OFF), placed next to the
|
||||||
|
// browser-verification group on the same success path (execute → browser-verification
|
||||||
|
// → code-review → review). Disabled → passes through inert; enabled → diff-reviews the
|
||||||
|
// change once pre-merge.
|
||||||
|
codeReviewOptionalGroupNode("in-progress"),
|
||||||
{ id: "review", kind: "prompt", column: "in-review", config: builtinPromptConfig("review", "Review") },
|
{ id: "review", kind: "prompt", column: "in-review", config: builtinPromptConfig("review", "Review") },
|
||||||
{ id: "merge-gate", kind: "merge-gate", column: "in-review", config: { gate: "auto-merge" } },
|
{ id: "merge-gate", kind: "merge-gate", column: "in-review", config: { gate: "auto-merge" } },
|
||||||
{ id: "merge-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
{ id: "merge-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
||||||
@@ -101,7 +108,10 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = {
|
|||||||
// execute → browser-verification (optional-group) → review. When the group is
|
// execute → browser-verification (optional-group) → review. When the group is
|
||||||
// disabled it passes through with outcome=success and routes straight to review.
|
// disabled it passes through with outcome=success and routes straight to review.
|
||||||
{ from: "execute", to: "browser-verification", condition: "success" },
|
{ from: "execute", to: "browser-verification", condition: "success" },
|
||||||
{ from: "browser-verification", to: "review", condition: "success" },
|
// browser-verification → code-review → review. Each optional-group passes through with
|
||||||
|
// outcome=success when disabled, so a task with both off routes straight to review.
|
||||||
|
{ from: "browser-verification", to: "code-review", condition: "success" },
|
||||||
|
{ from: "code-review", to: "review", condition: "success" },
|
||||||
{ from: "review", to: "merge-gate", condition: "success" },
|
{ from: "review", to: "merge-gate", condition: "success" },
|
||||||
{ from: "merge-gate", to: "branch-group-member-integration", condition: "outcome:auto-on" },
|
{ from: "merge-gate", to: "branch-group-member-integration", condition: "outcome:auto-on" },
|
||||||
{ from: "merge-gate", to: "merge-manual-hold", condition: "outcome:auto-off" },
|
{ from: "merge-gate", to: "merge-manual-hold", condition: "outcome:auto-off" },
|
||||||
@@ -118,6 +128,7 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = {
|
|||||||
{ from: "planning", to: "end", condition: "failure" },
|
{ from: "planning", to: "end", condition: "failure" },
|
||||||
{ from: "execute", to: "end", condition: "failure" },
|
{ from: "execute", to: "end", condition: "failure" },
|
||||||
{ from: "browser-verification", to: "end", condition: "failure" },
|
{ from: "browser-verification", to: "end", condition: "failure" },
|
||||||
|
{ from: "code-review", to: "end", condition: "failure" },
|
||||||
{ from: "review", to: "end", condition: "failure" },
|
{ from: "review", to: "end", condition: "failure" },
|
||||||
{ from: "merge-attempt", to: "end", condition: "failure" },
|
{ from: "merge-attempt", to: "end", condition: "failure" },
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import { parseWorkflowIr } from "./workflow-ir.js";
|
|||||||
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
||||||
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
||||||
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
||||||
|
import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The built-in **stepwise** coding workflow (KTD-9) — the demonstration of step
|
* The built-in **stepwise** coding workflow (KTD-9) — the demonstration of step
|
||||||
@@ -134,6 +135,14 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = {
|
|||||||
// disabled the group passes through inert. Both the normal foreach-success path
|
// disabled the group passes through inert. Both the normal foreach-success path
|
||||||
// and the rework-exhausted manual-release path flow through this node.
|
// and the rework-exhausted manual-release path flow through this node.
|
||||||
browserVerificationOptionalGroupNode("in-progress"),
|
browserVerificationOptionalGroupNode("in-progress"),
|
||||||
|
// FNXC:CodeReviewStep 2026-06-25-12:00:
|
||||||
|
// Pre-merge optional Code Review (optional-group, default OFF), placed next to the
|
||||||
|
// browser-verification group on the post-foreach success path (steps → browser-
|
||||||
|
// verification → code-review → review). Same R-3 run-once guarantee: it sits after the
|
||||||
|
// foreach so an enabled task runs it EXACTLY ONCE pre-merge, never per step-instance;
|
||||||
|
// disabled → inert pass-through. Both the foreach-success and rework-exhausted manual-
|
||||||
|
// release paths flow through it.
|
||||||
|
codeReviewOptionalGroupNode("in-progress"),
|
||||||
{ id: "review", kind: "prompt", column: "in-review", config: builtinPromptConfig("review", "Review") },
|
{ id: "review", kind: "prompt", column: "in-review", config: builtinPromptConfig("review", "Review") },
|
||||||
{ id: "merge-gate", kind: "merge-gate", column: "in-review", config: { gate: "auto-merge" } },
|
{ id: "merge-gate", kind: "merge-gate", column: "in-review", config: { gate: "auto-merge" } },
|
||||||
{ id: "merge-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
{ id: "merge-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
||||||
@@ -172,8 +181,12 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = {
|
|||||||
// KTD-5: bounded rework exhaustion → manual hold; release re-enters the group.
|
// KTD-5: bounded rework exhaustion → manual hold; release re-enters the group.
|
||||||
{ from: "steps", to: "rework-hold", condition: "outcome:rework-exhausted" },
|
{ from: "steps", to: "rework-hold", condition: "outcome:rework-exhausted" },
|
||||||
{ from: "rework-hold", to: "browser-verification", condition: "success" },
|
{ from: "rework-hold", to: "browser-verification", condition: "success" },
|
||||||
{ from: "browser-verification", to: "review", condition: "success" },
|
// browser-verification → code-review → review; each optional-group passes through
|
||||||
|
// (outcome=success) when disabled, so a task with both off routes straight to review.
|
||||||
|
{ from: "browser-verification", to: "code-review", condition: "success" },
|
||||||
|
{ from: "code-review", to: "review", condition: "success" },
|
||||||
{ from: "browser-verification", to: "end", condition: "failure" },
|
{ from: "browser-verification", to: "end", condition: "failure" },
|
||||||
|
{ from: "code-review", to: "end", condition: "failure" },
|
||||||
{ from: "steps", to: "end", condition: "failure" },
|
{ from: "steps", to: "end", condition: "failure" },
|
||||||
{ from: "review", to: "merge-gate", condition: "success" },
|
{ from: "review", to: "merge-gate", condition: "success" },
|
||||||
{ from: "review", to: "end", condition: "failure" },
|
{ from: "review", to: "end", condition: "failure" },
|
||||||
|
|||||||
@@ -1102,6 +1102,51 @@ Use these agent-browser commands for verification:
|
|||||||
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}
|
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}
|
||||||
|
|
||||||
Note: Refs (@e1, @e2) are invalidated after page navigation. Re-snapshot after clicking links or form submissions.`,
|
Note: Refs (@e1, @e2) are invalidated after page navigation. Re-snapshot after clicking links or form submissions.`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
/*
|
||||||
|
FNXC:CodeReviewStep 2026-06-25-12:00:
|
||||||
|
Built-in "Code Review" catalog template: a configurable pre-merge prompt-gate that
|
||||||
|
diff-reviews the task's changes for the correctness value automated tests miss
|
||||||
|
(logic bugs, broken edge cases, intent-vs-implementation drift, regressions in
|
||||||
|
touched paths, error handling, contract/signature breaks). This is the WORKFLOW-layer
|
||||||
|
code review — it reuses the shared prompt-gate verdict machinery, NOT engine
|
||||||
|
verification code. gateMode defaults to "advisory" (non-blocking) exactly like
|
||||||
|
browser-verification, so it is opt-in/non-blocking until an operator promotes it to a
|
||||||
|
blocking gate. phase "pre-merge" places it before merge. toolMode "readonly": review
|
||||||
|
reads the diff/files, it does not mutate the worktree.
|
||||||
|
*/
|
||||||
|
id: "code-review",
|
||||||
|
name: "Code Review",
|
||||||
|
description: "Diff-review the task's changes for correctness bugs, regressions, and intent mismatches that tests miss",
|
||||||
|
category: "Quality",
|
||||||
|
icon: "git-pull-request",
|
||||||
|
toolMode: "readonly",
|
||||||
|
gateMode: "advisory",
|
||||||
|
phase: "pre-merge",
|
||||||
|
prompt: `You are a senior code reviewer. Review the task's diff for the correctness value automated tests do NOT catch.
|
||||||
|
|
||||||
|
## Step 1: Read the change
|
||||||
|
1. Read the full diff against the base branch: \`git diff <base>...HEAD\` (or \`git diff <base>\`). Determine the base from the task context / merge target.
|
||||||
|
2. Read the changed files in full where the diff is non-trivial, so you see the surrounding code paths the change touches — not just the hunks.
|
||||||
|
|
||||||
|
## Step 2: Review focus (the value tests miss)
|
||||||
|
1. **Correctness / logic bugs** — wrong conditions, inverted boolean/comparison logic, off-by-one, incorrect operator precedence, mishandled return values.
|
||||||
|
2. **Broken edge cases** — empty/undefined/null inputs, zero/duplicate/boundary values, concurrency and ordering assumptions.
|
||||||
|
3. **Intent vs implementation** — does the code actually do what the task/PROMPT.md describes? Flag silent scope drift or partial implementations.
|
||||||
|
4. **Regressions in touched code paths** — does the change break or weaken an existing behavior in the files it edits or their callers?
|
||||||
|
5. **Error handling** — swallowed errors, unhandled rejections/exceptions, missing validation at trust boundaries, misleading error messages.
|
||||||
|
6. **Contract / signature changes** — changed function/exported-type signatures, API request/response shapes, or serialization that breaks existing callers.
|
||||||
|
|
||||||
|
Be specific: cite \`file:line\` for every finding and explain the concrete failure it causes.
|
||||||
|
|
||||||
|
## Output Requirements
|
||||||
|
- Fast-bail: if the diff is trivial, generated, or out-of-scope for code review (e.g. pure docs/config/formatting with no logic), output {"verdict":"APPROVE","notes":"out of scope: code review"} immediately and stop.
|
||||||
|
- APPROVE: no correctness concerns; use empty or brief notes.
|
||||||
|
- APPROVE_WITH_NOTES: shippable, but include non-blocking advisories (with file:line) in notes.
|
||||||
|
- REVISE: a correctness bug, regression, or contract break requires changes; include file:line and the concrete failure plus remediation in notes.
|
||||||
|
- Final output: output exactly one trailing JSON object on the final line (no markdown fences, no surrounding prose):
|
||||||
|
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}`,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
id: "frontend-ux-design",
|
id: "frontend-ux-design",
|
||||||
|
|||||||
Reference in New Issue
Block a user