feat: make Code Review a default-on toggleable optional-group step
Refinement: Code Review is now a DEFAULT-ON but toggleable `optional-group` in the built-in coding and stepwise coding workflows (defaultOn:true), not a standard always-on node. It is part of the existing pre-merge flow (execute → [browser-verification optional] → code-review → review) and runs for every coding task by default, yet an operator can toggle it off per task by removing `code-review` from enabledWorkflowSteps; disabled → byte-inert pass-through. Advisory gateMode keeps it non-blocking (operators can promote to a gate); toolMode readonly. - Restore the optional-group builder (builtin-code-review-node.ts → -group.ts) with config.defaultOn:true; stable group id `code-review`, inner id `code-review-step`. - Wire the default-on optional-group into both built-in coding IRs. - Fix store default-workflow seeding: interpreter-deferred built-ins (which carry optional-group nodes) previously bailed to `undefined` in materializeDefaultWorkflowSteps, dropping default-on group seeding under a project-default workflow. Now they seed resolveDefaultOnOptionalGroupIds, mirroring the explicit-workflow path, so defaultOn:true actually takes effect (the executor enables a group strictly via enabledWorkflowSteps.includes(node.id)). - Update tests + changeset; full @fusion/core suite green (356 files). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -2,6 +2,6 @@
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Add a standard pre-merge Code Review step to the built-in coding workflows.
|
||||
summary: Add a default-on, toggleable pre-merge Code Review step to the built-in coding workflows.
|
||||
category: feature
|
||||
dev: New always-on `code-review` prompt node (toolMode readonly, gateMode advisory, phase pre-merge) on the pre-merge success path (execute → browser-verification → code-review → review) of both the built-in coding and stepwise coding workflows. Runs for every coding task by default with no `enabledWorkflowSteps` gating; advisory so it does not change merge outcomes (operators can promote it to a blocking gate). Reuses the shared prompt-gate verdict machinery (no engine verification code). The `code-review` WORKFLOW_STEP_TEMPLATE is also available in the editor palette.
|
||||
dev: New `code-review` optional-group node (defaultOn:true, toolMode readonly, gateMode advisory, phase pre-merge) on the pre-merge success path (execute → browser-verification → code-review → review) of both the built-in coding and stepwise coding workflows. Runs for every coding task by default (seeded into enabledWorkflowSteps via resolveDefaultOnOptionalGroupIds) but is toggleable off per task; advisory so it does not change merge outcomes (operators can promote it to a blocking gate). Also fixes default-workflow task creation to seed default-on optional groups for interpreter-deferred built-ins (previously dropped). Reuses the shared prompt-gate verdict machinery (no engine verification code). The `code-review` WORKFLOW_STEP_TEMPLATE is also available in the editor palette.
|
||||
|
||||
114
packages/core/src/__tests__/builtin-code-review-group.test.ts
Normal file
114
packages/core/src/__tests__/builtin-code-review-group.test.ts
Normal file
@@ -0,0 +1,114 @@
|
||||
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-15:00:
|
||||
Coverage for the DEFAULT-ON but TOGGLEABLE "Code Review" pre-merge step: the catalog
|
||||
template fields, the `optional-group` node (defaultOn:true) built from it, and its wiring
|
||||
into the coding + stepwise built-ins as a default-on optional group. 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 → non-blocking by default (like the existing review); operators can promote.
|
||||
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-ON 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");
|
||||
// Default-ON (runs by default), but still an optional-group → toggleable per task.
|
||||
expect(node.config?.defaultOn).toBe(true);
|
||||
|
||||
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 as a default-ON optional group", () => {
|
||||
it.each([
|
||||
["builtin coding", BUILTIN_CODING_WORKFLOW_IR],
|
||||
["builtin stepwise", BUILTIN_STEPWISE_CODING_WORKFLOW_IR],
|
||||
])("%s includes the default-ON 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(true);
|
||||
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 a toggle AND seeded into the default-on set", (_name, ir) => {
|
||||
// Advertised as a toggleable optional step (so operators can turn it off per task)…
|
||||
const advertised = resolveWorkflowOptionalSteps(ir).find((s) => s.templateId === "code-review");
|
||||
expect(advertised).toEqual({
|
||||
templateId: "code-review",
|
||||
name: "Code Review",
|
||||
description: "",
|
||||
phase: "pre-merge",
|
||||
defaultOn: true,
|
||||
});
|
||||
// …and in the default-on set, so default-on actually takes effect (new tasks seed it).
|
||||
expect(resolveDefaultOnOptionalGroupIds(ir)).toContain("code-review");
|
||||
});
|
||||
});
|
||||
@@ -1,97 +0,0 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
CODE_REVIEW_NODE_ID,
|
||||
codeReviewStepNode,
|
||||
} from "../builtin-code-review-node.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 { resolveWorkflowOptionalSteps } from "../workflow-optional-steps.js";
|
||||
|
||||
/*
|
||||
FNXC:CodeReviewStep 2026-06-25-13:30:
|
||||
Coverage for the STANDARD, always-on "Code Review" pre-merge step: the catalog template
|
||||
fields, the regular `prompt` node built from it, and its wiring as a default-ON step in
|
||||
the coding + stepwise built-ins (no enabledWorkflowSteps gating). Code review is a
|
||||
WORKFLOW prompt step (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 → non-blocking by default (like the existing review); operators can promote.
|
||||
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("codeReviewStepNode", () => {
|
||||
it("builds a standard advisory readonly prompt node keyed by the catalog id", () => {
|
||||
const node = codeReviewStepNode("in-progress");
|
||||
expect(node.id).toBe(CODE_REVIEW_NODE_ID);
|
||||
expect(CODE_REVIEW_NODE_ID).toBe("code-review");
|
||||
// Standard node, NOT an optional-group toggle.
|
||||
expect(node.kind).toBe("prompt");
|
||||
expect(node.column).toBe("in-progress");
|
||||
expect(node.config?.name).toBe("Code Review");
|
||||
expect(node.config?.toolMode).toBe("readonly");
|
||||
expect(node.config?.gateMode).toBe("advisory");
|
||||
expect(node.config?.defaultOn).toBeUndefined(); // no optional-group toggle semantics.
|
||||
expect(String(node.config?.prompt)).toMatch(/"verdict":"APPROVE\|APPROVE_WITH_NOTES\|REVISE"/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("built-in coding + stepwise workflows wire code-review as a standard always-on step", () => {
|
||||
it.each([
|
||||
["builtin coding", BUILTIN_CODING_WORKFLOW_IR],
|
||||
["builtin stepwise", BUILTIN_STEPWISE_CODING_WORKFLOW_IR],
|
||||
])("%s includes a default-ON code-review prompt node between browser-verification and review", (_name, ir) => {
|
||||
const byId = new Map(ir.nodes.map((n) => [n.id, n]));
|
||||
const node = byId.get("code-review");
|
||||
// Always present as a standard prompt node (not an optional-group, no toggle).
|
||||
expect(node?.kind).toBe("prompt");
|
||||
expect(node?.config?.name).toBe("Code Review");
|
||||
expect(node?.config?.gateMode).toBe("advisory");
|
||||
expect(node?.config?.toolMode).toBe("readonly");
|
||||
|
||||
// Pre-merge wiring: ... → browser-verification → code-review → review; failure → end
|
||||
// (mirrors how the existing review node fails to end — no dead-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 standard 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 NOT advertised as an optional-step toggle (always-on, no gating)", (_name, ir) => {
|
||||
// Standard step → never surfaces in the optional-step toggle list (it is not gated
|
||||
// on task.enabledWorkflowSteps; it runs for every coding task).
|
||||
const toggles = resolveWorkflowOptionalSteps(ir).map((s) => s.templateId);
|
||||
expect(toggles).not.toContain("code-review");
|
||||
});
|
||||
});
|
||||
@@ -600,12 +600,28 @@ describe("built-in workflows", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("create-time branching built-in workflowId records selection without throwing", async () => {
|
||||
it("create-time branching built-in workflowId records selection and seeds the default-on code-review group", async () => {
|
||||
const task = await store.createTask({ description: "explicit builtin coding", workflowId: "builtin:coding" });
|
||||
|
||||
const detail = await store.getTask(task.id);
|
||||
expect(detail.enabledWorkflowSteps ?? []).toEqual([]);
|
||||
expect(store.getTaskWorkflowSelection(task.id)).toEqual({ workflowId: "builtin:coding", stepIds: [] });
|
||||
// FNXC:CodeReviewStep — builtin:coding carries the DEFAULT-ON `code-review`
|
||||
// optional-group, so the explicit-workflow create path seeds it into the task's
|
||||
// enabledWorkflowSteps (and records it in the selection).
|
||||
expect(detail.enabledWorkflowSteps ?? []).toEqual(["code-review"]);
|
||||
expect(store.getTaskWorkflowSelection(task.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] });
|
||||
});
|
||||
|
||||
it("a task can disable code-review by creating with explicit enabledWorkflowSteps excluding it", async () => {
|
||||
// Default-on but TOGGLEABLE: an explicit (non-empty) enabledWorkflowSteps wins over
|
||||
// the workflow's default-on seeding, so omitting `code-review` disables it.
|
||||
const task = await store.createTask({
|
||||
description: "coding without code review",
|
||||
workflowId: "builtin:coding",
|
||||
enabledWorkflowSteps: ["browser-verification"],
|
||||
});
|
||||
const detail = await store.getTask(task.id);
|
||||
expect(detail.enabledWorkflowSteps ?? []).not.toContain("code-review");
|
||||
expect(detail.enabledWorkflowSteps ?? []).toEqual(["browser-verification"]);
|
||||
});
|
||||
|
||||
it("branching built-in project defaults do not throw", async () => {
|
||||
@@ -613,34 +629,35 @@ describe("built-in workflows", () => {
|
||||
description: "implicit builtin default",
|
||||
});
|
||||
|
||||
// U6: builtin:coding now carries the `browser-verification` optional-group
|
||||
// (an interpreter-deferred construct), so its DEFAULT-workflow materialization
|
||||
// falls back to no legacy WorkflowStep rows and records no selection row —
|
||||
// identical to the stepwise built-in below. The group is defaultOn:false, so
|
||||
// enabledWorkflowSteps stays empty.
|
||||
// FNXC:CodeReviewStep — builtin:coding/stepwise are interpreter-deferred (they
|
||||
// carry optional-group nodes), so DEFAULT-workflow materialization records no legacy
|
||||
// WorkflowStep rows. They DO carry the DEFAULT-ON `code-review` optional-group, so
|
||||
// the project-default create path now seeds `code-review` into enabledWorkflowSteps
|
||||
// and records a selection (mirroring the explicit-workflow path) — that is how
|
||||
// default-on actually takes effect. browser-verification stays off (defaultOn:false).
|
||||
await store.setDefaultWorkflowId("builtin:coding");
|
||||
const codingTask = await store.createTask({ description: "default builtin coding" });
|
||||
expect((await store.getTask(codingTask.id)).enabledWorkflowSteps ?? []).toEqual([]);
|
||||
expect(store.getTaskWorkflowSelection(codingTask.id)).toBeUndefined();
|
||||
expect((await store.getTask(codingTask.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]);
|
||||
expect(store.getTaskWorkflowSelection(codingTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] });
|
||||
|
||||
const reservedCodingTask = await store.createTaskWithReservedId(
|
||||
{ description: "reserved default builtin coding" },
|
||||
{ taskId: "reserved-default-builtin-coding" },
|
||||
);
|
||||
expect((await store.getTask(reservedCodingTask.id)).enabledWorkflowSteps ?? []).toEqual([]);
|
||||
expect(store.getTaskWorkflowSelection(reservedCodingTask.id)).toBeUndefined();
|
||||
expect((await store.getTask(reservedCodingTask.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]);
|
||||
expect(store.getTaskWorkflowSelection(reservedCodingTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] });
|
||||
|
||||
await store.setDefaultWorkflowId("builtin:stepwise-coding");
|
||||
const stepwiseTask = await store.createTask({ description: "default builtin stepwise" });
|
||||
expect((await store.getTask(stepwiseTask.id)).enabledWorkflowSteps ?? []).toEqual([]);
|
||||
expect(store.getTaskWorkflowSelection(stepwiseTask.id)).toBeUndefined();
|
||||
expect((await store.getTask(stepwiseTask.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]);
|
||||
expect(store.getTaskWorkflowSelection(stepwiseTask.id)).toEqual({ workflowId: "builtin:stepwise-coding", stepIds: ["code-review"] });
|
||||
|
||||
const reservedStepwiseTask = await store.createTaskWithReservedId(
|
||||
{ description: "reserved default builtin stepwise" },
|
||||
{ taskId: "reserved-default-builtin-stepwise" },
|
||||
);
|
||||
expect((await store.getTask(reservedStepwiseTask.id)).enabledWorkflowSteps ?? []).toEqual([]);
|
||||
expect(store.getTaskWorkflowSelection(reservedStepwiseTask.id)).toBeUndefined();
|
||||
expect((await store.getTask(reservedStepwiseTask.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]);
|
||||
expect(store.getTaskWorkflowSelection(reservedStepwiseTask.id)).toEqual({ workflowId: "builtin:stepwise-coding", stepIds: ["code-review"] });
|
||||
});
|
||||
|
||||
it("rejects selecting the PR lifecycle fragment for a task", async () => {
|
||||
|
||||
@@ -108,12 +108,10 @@ describe("resolveWorkflowOptionalSteps (optional-group nodes)", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("resolves the built-in coding/stepwise browser-verification optional-group (U6)", () => {
|
||||
// U6 migrated both built-ins: `browser-verification` is now an optional-group
|
||||
// node (default OFF), so the resolver advertises exactly one toggle entry per
|
||||
// built-in, keyed by the group node id `browser-verification`. The standard
|
||||
// always-on `code-review` prompt node is NOT an optional-group, so it never
|
||||
// surfaces as a toggle entry here.
|
||||
it("resolves the built-in coding/stepwise browser-verification (off) + code-review (on) optional-groups", () => {
|
||||
// Both built-ins carry two optional-group toggles on the pre-merge path, in node order:
|
||||
// `browser-verification` (default OFF) then `code-review` (default ON — runs by default
|
||||
// but is toggleable off per task).
|
||||
const expected = [
|
||||
{
|
||||
templateId: "browser-verification",
|
||||
@@ -122,10 +120,24 @@ describe("resolveWorkflowOptionalSteps (optional-group nodes)", () => {
|
||||
phase: "pre-merge" as const,
|
||||
defaultOn: false,
|
||||
},
|
||||
{
|
||||
templateId: "code-review",
|
||||
name: "Code Review",
|
||||
description: "",
|
||||
phase: "pre-merge" as const,
|
||||
defaultOn: true,
|
||||
},
|
||||
];
|
||||
expect(resolveWorkflowOptionalSteps(BUILTIN_CODING_WORKFLOW_IR)).toEqual(expected);
|
||||
expect(resolveWorkflowOptionalSteps(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).toEqual(expected);
|
||||
});
|
||||
|
||||
it("seeds code-review (default ON) but not browser-verification (default OFF) for the built-ins", () => {
|
||||
// resolveDefaultOnOptionalGroupIds drives which groups a new task gets enabled by
|
||||
// default: code-review is on, browser-verification is off.
|
||||
expect(resolveDefaultOnOptionalGroupIds(BUILTIN_CODING_WORKFLOW_IR)).toEqual(["code-review"]);
|
||||
expect(resolveDefaultOnOptionalGroupIds(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).toEqual(["code-review"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveDefaultOnOptionalGroupIds (task-creation seeding)", () => {
|
||||
|
||||
@@ -238,7 +238,9 @@ describe("workflow restart durability for explicit selections", () => {
|
||||
const customSelectionBefore = store().getTaskWorkflowSelection(customTask.id);
|
||||
expect(customSelectionBefore?.workflowId).toBe(workflow.id);
|
||||
expect(customSelectionBefore?.stepIds).toHaveLength(2);
|
||||
expect(store().getTaskWorkflowSelection(builtinTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: [] });
|
||||
// FNXC:CodeReviewStep — builtin:coding carries the DEFAULT-ON `code-review`
|
||||
// optional-group, so the create-time workflowId path seeds it into the selection.
|
||||
expect(store().getTaskWorkflowSelection(builtinTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] });
|
||||
|
||||
await reopenAsDiskBackedStore();
|
||||
|
||||
@@ -249,9 +251,9 @@ describe("workflow restart durability for explicit selections", () => {
|
||||
for (const stepId of customSelection?.stepIds ?? []) {
|
||||
expect(await store().getWorkflowStep(stepId)).toBeDefined();
|
||||
}
|
||||
expect(store().getTaskWorkflowSelection(builtinTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: [] });
|
||||
expect((await store().getTask(builtinTask.id)).enabledWorkflowSteps ?? []).toEqual([]);
|
||||
expect((await taskJsonEnabledWorkflowSteps(builtinTask.id)) ?? []).toEqual([]);
|
||||
expect(store().getTaskWorkflowSelection(builtinTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] });
|
||||
expect((await store().getTask(builtinTask.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]);
|
||||
expect((await taskJsonEnabledWorkflowSteps(builtinTask.id)) ?? []).toEqual(["code-review"]);
|
||||
});
|
||||
|
||||
it("fails closed when a selected custom workflow definition is missing without corrupting the dangling selection", async () => {
|
||||
|
||||
80
packages/core/src/builtin-code-review-group.ts
Normal file
80
packages/core/src/builtin-code-review-group.ts
Normal file
@@ -0,0 +1,80 @@
|
||||
import type { WorkflowIrNode } from "./workflow-ir-types.js";
|
||||
import { WORKFLOW_STEP_TEMPLATES } from "./types.js";
|
||||
|
||||
/*
|
||||
FNXC:CodeReviewStep 2026-06-25-15:00:
|
||||
Code Review is a DEFAULT-ON but TOGGLEABLE step in the built-in coding and
|
||||
stepwise-coding workflows: an `optional-group` container node with `defaultOn: true`.
|
||||
It is part of the existing flows and runs for every coding task by default (the
|
||||
default-on resolver seeds `code-review` into a new task's enabledWorkflowSteps), yet an
|
||||
operator can turn it off per task by removing `code-review` from enabledWorkflowSteps —
|
||||
when disabled the group passes through byte-inert, restoring the exact prior flow.
|
||||
|
||||
The group sits on the pre-merge success path (execute → [browser-verification optional]
|
||||
→ code-review → review). The group node id `code-review` is the STABLE per-task enable
|
||||
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` WORKFLOW_STEP_TEMPLATE: a `prompt` node carrying the template's prompt,
|
||||
`toolMode` (readonly — review reads the diff, never mutates), and `gateMode` (advisory —
|
||||
non-blocking, like the existing review; operators can promote to a gate). 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.
|
||||
* `defaultOn: true` makes it run by default while remaining togglable per task. `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,
|
||||
// Default-ON: runs for every coding task by default, but operators can toggle it
|
||||
// off per task (remove `code-review` from enabledWorkflowSteps).
|
||||
defaultOn: true,
|
||||
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: [],
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -1,57 +0,0 @@
|
||||
import type { WorkflowIrNode } from "./workflow-ir-types.js";
|
||||
import { WORKFLOW_STEP_TEMPLATES } from "./types.js";
|
||||
|
||||
/*
|
||||
FNXC:CodeReviewStep 2026-06-25-13:30:
|
||||
Code Review is a STANDARD, always-on step in the built-in coding and stepwise-coding
|
||||
workflows — NOT a default-off optional-group toggle. It is a regular `prompt` node on the
|
||||
pre-merge success path (execute → [browser-verification optional] → code-review → review),
|
||||
so it runs for EVERY coding task by default with no `enabledWorkflowSteps` gating.
|
||||
|
||||
gateMode is "advisory" (sourced from the catalog template): like the existing `review`
|
||||
seam it does not change merge outcomes — it just adds the diff-correctness review to the
|
||||
standard flow. Operators can promote it to a blocking gate later. toolMode is "readonly":
|
||||
review reads the diff/files, it does not mutate the worktree.
|
||||
|
||||
The node id `code-review` is also the catalog template id, so the built-in node stays
|
||||
byte-identical to the `code-review` template a human would insert from the editor palette
|
||||
(prompt/toolMode/gateMode all sourced from the catalog).
|
||||
*/
|
||||
|
||||
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();
|
||||
|
||||
/** Standard pre-merge code-review node id (also the catalog template id). */
|
||||
export const CODE_REVIEW_NODE_ID = "code-review";
|
||||
|
||||
/**
|
||||
* Build the standard, always-on `code-review` prompt node placed on a workflow's
|
||||
* pre-merge success path between browser-verification and review. `column` matches the
|
||||
* pre-merge implementation column (`in-progress`) so the single in-progress → in-review
|
||||
* status transition stays at code-review → review.
|
||||
*
|
||||
* Mirrors `stepTemplateToNode(code-review)`: a `prompt` node whose config carries the
|
||||
* catalog prompt + `toolMode: "readonly"` + `gateMode: "advisory"`.
|
||||
*/
|
||||
export function codeReviewStepNode(column: string): WorkflowIrNode {
|
||||
const tpl = CODE_REVIEW_TEMPLATE;
|
||||
return {
|
||||
id: CODE_REVIEW_NODE_ID,
|
||||
kind: "prompt",
|
||||
column,
|
||||
config: {
|
||||
name: tpl.name,
|
||||
description: tpl.description,
|
||||
prompt: tpl.prompt ?? "",
|
||||
toolMode: tpl.toolMode === "coding" ? "coding" : "readonly",
|
||||
gateMode: tpl.gateMode ?? "advisory",
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -3,7 +3,7 @@ import { parseWorkflowIr } from "./workflow-ir.js";
|
||||
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
||||
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
||||
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
||||
import { codeReviewStepNode } from "./builtin-code-review-node.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
|
||||
@@ -76,11 +76,12 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = {
|
||||
},
|
||||
// Pre-merge optional browser-verification (optional-group, default OFF).
|
||||
browserVerificationOptionalGroupNode("in-progress"),
|
||||
// FNXC:CodeReviewStep 2026-06-25-13:30:
|
||||
// STANDARD always-on pre-merge Code Review prompt node (advisory), on the success
|
||||
// path between browser-verification and review (execute → browser-verification →
|
||||
// code-review → review). Runs for every coding task by default — no enable toggle.
|
||||
codeReviewStepNode("in-progress"),
|
||||
// FNXC:CodeReviewStep 2026-06-25-15:00:
|
||||
// Pre-merge Code Review as a DEFAULT-ON optional-group (advisory), 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.
|
||||
codeReviewOptionalGroupNode("in-progress"),
|
||||
{ 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-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
||||
|
||||
@@ -3,7 +3,7 @@ import { parseWorkflowIr } from "./workflow-ir.js";
|
||||
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
||||
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
||||
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
||||
import { codeReviewStepNode } from "./builtin-code-review-node.js";
|
||||
import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js";
|
||||
|
||||
/**
|
||||
* The built-in **stepwise** coding workflow (KTD-9) — the demonstration of step
|
||||
@@ -135,13 +135,14 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = {
|
||||
// disabled the group passes through inert. Both the normal foreach-success path
|
||||
// and the rework-exhausted manual-release path flow through this node.
|
||||
browserVerificationOptionalGroupNode("in-progress"),
|
||||
// FNXC:CodeReviewStep 2026-06-25-13:30:
|
||||
// STANDARD always-on pre-merge Code Review prompt node (advisory), on the post-foreach
|
||||
// FNXC:CodeReviewStep 2026-06-25-15:00:
|
||||
// Pre-merge Code Review as a DEFAULT-ON optional-group (advisory), 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-
|
||||
// release paths flow through it. Runs for every task by default — no enable toggle.
|
||||
codeReviewStepNode("in-progress"),
|
||||
// release paths flow through it. Runs for every task by default (defaultOn:true) but is
|
||||
// toggleable off per task; disabled → byte-inert pass-through.
|
||||
codeReviewOptionalGroupNode("in-progress"),
|
||||
{ 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-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
||||
|
||||
@@ -16283,7 +16283,20 @@ ${stepsSection}`;
|
||||
try {
|
||||
inputs = compileWorkflowToSteps(def.ir);
|
||||
} catch (err) {
|
||||
if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) return undefined;
|
||||
// FNXC:CodeReviewStep 2026-06-25-15:00:
|
||||
// Interpreter-deferred built-ins (e.g. builtin:coding/stepwise, which carry
|
||||
// optional-group nodes) cannot lower to legacy WorkflowStep rows, but they may
|
||||
// still carry DEFAULT-ON optional groups (e.g. `code-review`) that must be seeded
|
||||
// into the new task's `enabledWorkflowSteps` for default-on to actually take
|
||||
// effect — the executor enables a group strictly via
|
||||
// `enabledWorkflowSteps.includes(node.id)` with no defaultOn fallback. Mirror the
|
||||
// explicit-workflow path (`materializeExplicitWorkflowSteps`) by recording a
|
||||
// selection seeded with the default-on group ids instead of bailing to `undefined`
|
||||
// (which dropped the seeding and silently disabled default-on groups under a
|
||||
// project-default workflow).
|
||||
if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) {
|
||||
return { workflowId, stepIds: resolveDefaultOnOptionalGroupIds(def.ir) };
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
// FNXC:WorkflowOptionalGroup 2026-06-21-14:20: seed `enabledWorkflowSteps`
|
||||
|
||||
Reference in New Issue
Block a user