Wire review voting, risk tier gate and SDD stages into the engine

.factory/workflows/bug-tdd.yaml

@@ -31,7 +31,12 @@ steps:
       brief: "Strengthen the tests so mutants in the files this run changed are killed. Add tests under specs/<issue>/golden; never change source.",
       checks: [mutation],
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }
   - {
       stage: ci,
       agent: builder,

.factory/workflows/contract.yaml

@@ -25,4 +25,9 @@ steps:
       checks: [contract, lock, gate],
       loop: { until: [contract, lock, gate], max: 3 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }

.factory/workflows/deps.yaml

@@ -24,4 +24,9 @@ steps:
       checks: [manifest-deps, gate],
       loop: { until: [manifest-deps, gate], max: 3 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift, manifest-deps] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift, manifest-deps],
+    }

.factory/workflows/docs.yaml

@@ -17,4 +17,9 @@ steps:
       checks: [prose, gate],
       loop: { until: [prose, gate], max: 3 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }

.factory/workflows/feature-atdd.yaml

@@ -38,7 +38,12 @@ steps:
       checks: [acceptance, holdout, lock],
       loop: { until: [acceptance, holdout, lock], max: 4 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }
   - {
       stage: ci,
       agent: builder,

.factory/workflows/feature-bdd.yaml

@@ -38,7 +38,12 @@ steps:
       checks: [acceptance, holdout, lock],
       loop: { until: [acceptance, holdout, lock], max: 4 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }
   - {
       stage: ci,
       agent: builder,

.factory/workflows/feature-sdd.yaml

@@ -10,6 +10,12 @@ steps:
       writes: intent.md,
       brief: "State the outcome the epic must reach and how we will know, in a few lines. Reply with the intent only.",
     }
+  - {
+      stage: constitution,
+      agent: planner,
+      writes: constitution.md,
+      brief: "List the standing rules this feature must obey, one per line, taken from AGENTS.md and .claude/rules (testing, security, file size, naming). Reply with the rules only.",
+    }
   - {
       stage: spec,
       agent: planner,
@@ -18,6 +24,12 @@ steps:
       startsWith: "R1.",
       brief: "Write EARS requirements, one per line as 'R<n>. When <trigger>, the system shall <response>'. Start your reply with 'R1.' and reply with the requirements only: no commentary, no plan-mode notes, do not run the gate.",
     }
+  - {
+      stage: clarify,
+      agent: planner,
+      writes: clarify.md,
+      brief: "List every requirement in specs/<issue>/spec.md that two readers could build differently, each with the single reading the plan will assume. Reply with that list only, or the single line 'No ambiguity.'",
+    }
   - {
       stage: plan,
       agent: planner,
@@ -26,10 +38,22 @@ steps:
       brief: "Reply with the plan and nothing else: one repo path per line for every file you will create or change, never a script you only run. Do not list files under specs/<issue>/; the runner allows those. A new file needs an owner in levels.json, so list levels.json whenever you add a file. Commentary of any kind makes the plan unusable. The runner saves your reply as specs/<issue>/plan.md, and later stages may touch only the files listed.",
       checks: [plan-drift],
     }
+  - {
+      stage: tasks,
+      agent: planner,
+      writes: tasks.md,
+      brief: "Break the plan into small ordered tasks, one per line as 'T<n>. <task> (R<n>)', each naming the requirement it serves. Reply with the tasks only.",
+    }
+  - {
+      stage: analyze,
+      agent: planner,
+      writes: analyze.md,
+      brief: "Cross-check spec.md, clarify.md, plan.md and tasks.md. List every requirement with no task, every task with no requirement and every plan file no task touches. Reply with the findings only, or the single line 'Consistent.'",
+    }
   - {
       stage: tests,
       agent: builder,
-      brief: "Write a vitest test or fast-check property for each requirement in specs/<issue>/golden. They fail on the base commit. Write no source.",
+      brief: "Write a vitest test or fast-check property for each requirement in specs/<issue>/golden, following specs/<issue>/tasks.md. They fail on the base commit. Write no source.",
       checks: [red, properties, lock],
       loop: { until: [red], max: 2 },
     }
@@ -47,7 +71,12 @@ steps:
       brief: "Strengthen the tests so mutants in the files this run changed are killed. Add tests under specs/<issue>/golden; never change source.",
       checks: [mutation],
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }
   - {
       stage: release,
       agent: builder,

.factory/workflows/incident.yaml

@@ -31,4 +31,9 @@ steps:
       checks: [green, lock, gate],
       loop: { until: [green, lock, gate], max: 3 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }

.factory/workflows/llm-eval.yaml

@@ -25,4 +25,9 @@ steps:
       checks: [eval, lock],
       loop: { until: [eval, lock], max: 4 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }

.factory/workflows/refactor.yaml

@@ -31,4 +31,9 @@ steps:
       checks: [mutation],
       loop: { until: [mutation], max: 3 },
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }

.factory/workflows/security.yaml

@@ -31,4 +31,9 @@ steps:
       checks: [green, lock, gate],
       loop: { until: [green, lock, gate], max: 3 },
     }
-  - { stage: review, agent: security-reviewer, checks: [plan-drift] }
+  - {
+      stage: review,
+      agent: security-reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }

.factory/workflows/ui-visual.yaml

@@ -28,10 +28,15 @@ steps:
       agent: design-reviewer,
       brief: "Review the built UI against the design contract and list what is off. Change nothing.",
     }
+  - {
+      stage: review,
+      agent: reviewer,
+      review: { lenses: [reviewer, security-reviewer, test-reviewer] },
+      checks: [plan-drift],
+    }
   - {
       stage: feedback,
       agent: planner,
       brief: "Summarise what changed for the human who approves, in plain words.",
       gate: human,
     }
-  - { stage: review, agent: reviewer, checks: [plan-drift] }

docs/factory/f5-review.md

@@ -1,6 +1,6 @@
 # F5 Review: lenses, voting verifiers and a capped loop
 
-**Stage:** Review. **Layer:** verification. **Workflows:** every workflow ends with a `review` step; `bug-tdd`, `incident` and the others use the `reviewer` agent with the `plan-drift` check.
+**Stage:** Review. **Layer:** verification. **Workflows:** every workflow ends with a `review` step; `bug-tdd`, `incident` and the others run lenses and voters, then the `plan-drift` check.
 
 ## Goal
 
@@ -11,7 +11,8 @@ A review that reports few findings and is right about them. Several lenses hunt
 Read this before the steps, because the pieces are not all wired the same way.
 
 - Built and tested: `factory/engine/review.ts` (lenses, 3 voters, 2 needed, nit cap, round cap, failed review), `factory/engine/risk.ts`, and `REVIEW.md`. `factory/engine/review.test.ts` proves them.
-- Not yet called by the runner: `factory/engine/interpret.ts` does not import `review.ts`. The `review` step in each workflow YAML is one `reviewer` agent plus `plan-drift`. The voting loop is a tested library you can wire into a workflow as an assignment (F10, "a new review lens").
+- Called by the runner: the `review` step in each workflow YAML carries `review: { lenses: [...] }`, and `factory/engine/interpret.ts` runs `reviewLoop` for it (lenses in parallel, 3 voters, 2 needed, a `builder` fix between rounds, cap of 3) and writes `specs/<n>/review.json`. A surviving Important finding or a lens reply that is not findings stops the run as `needs-human`. The `plan-drift` check still runs after it.
+- Risk at the human gate: `interpret.ts` calls `classify()` on the files changed so far and requires `APPROVALS[tier]` distinct people (docs-only needs none, high needs two) before a `gate: human` step passes.
 - Live on pull requests: `.github/workflows/review.yml` runs the `reviewer` agent, and `design-reviewer` for UI paths, and posts one comment ending in a `VERDICT` line. That review has no voting.
 - The delivery runner (`factory/run.ts`, `make factory ISSUE=n`) has its own loop: `review`, then `build` to fix, at most `reviewLoop.maxRounds` times (3 in `.factory/config.json`), then the `needs-human` label. It reads one `VERDICT: clean` or `VERDICT: blocking`.
 

factory/engine/artifacts.ts

@@ -10,8 +10,20 @@ export const ARTIFACTS = [
   "run.log",
   "incident.md",
   "lock.json",
+  "constitution.md",
+  "clarify.md",
+  "tasks.md",
+  "analyze.md",
+] as const;
+export const AGENT_ARTIFACTS = [
+  "intent.md",
+  "spec.md",
+  "plan.md",
+  "constitution.md",
+  "clarify.md",
+  "tasks.md",
+  "analyze.md",
 ] as const;
-export const AGENT_ARTIFACTS = ["intent.md", "spec.md", "plan.md"] as const;
 export type ArtifactName = (typeof ARTIFACTS)[number];
 
 export const artifactPath = (n: number, name: ArtifactName) => `specs/${n}/${name}`;
@@ -82,6 +94,9 @@ export async function artifactDigest(host: Host, n: number, stage: string): Prom
       ? ["intent.md", "spec.md"]
       : ["intent.md", "spec.md", "plan.md"];
   const names = new Set<string>(docs.map((a) => `specs/${n}/${a}`));
+  // Spec Kit documents join the hash only when a run wrote them, so earlier approvals keep their digest.
+  for (const extra of ["constitution.md", "clarify.md", "tasks.md", "analyze.md"])
+    if ((await host.read(`specs/${n}/${extra}`)) !== null) names.add(`specs/${n}/${extra}`);
   if (stage !== "spec" && stage !== "decide" && stage !== "plan")
     for (const f of await host.changedFiles()) names.add(f);
   // One hash per file, joined as JSON, so no path or content can shift a boundary; a missing file is null, not "".

factory/engine/checks/index.ts

@@ -44,9 +44,14 @@ export const CHECKS: Record<string, Check> = {
     vitestFor(() => "tests/contract"),
   ),
   visual: commandCheck("large", "npx", () => ["playwright", "test", "tests/e2e"]),
-  eval: commandCheck("large", "npx", () => ["tsx", "scripts/eval.ts"]),
+  eval: commandCheck("large", "npx", () => ["tsx", "api/src/eval.ts"]),
   gate: commandCheck("large", "make", () => ["check"]),
-  prepush: commandCheck("small", "bash", () => ["scripts/git/pre-push"]),
+  prepush: commandCheck("small", "npx", () => [
+    "tsx",
+    "scripts/check-affected.ts",
+    "--base",
+    "origin/main",
+  ]),
   ci: commandCheck("large", "gh", () => ["pr", "checks", "--watch"]),
   prose: commandCheck("small", "npx", () => ["tsx", "scripts/check-prose.ts", "docs"]),
 };

factory/engine/fake-host.ts

@@ -45,9 +45,9 @@ export function fakeHost(script: FakeScript = {}) {
     recall: async (k) => state.get(k) ?? null,
     remember: async (k, v) => void state.set(k, v),
     setPhase: (p) => void phases.push(p),
-    approved: async (n, stage, digest) =>
-      script.approvals?.some((a) => a === `${n}:${stage}` || a === `${n}:${stage}:${digest}`) ??
-      false,
+    approved: async (n, stage, digest, need = 1) =>
+      (script.approvals?.filter((a) => a === `${n}:${stage}` || a === `${n}:${stage}:${digest}`)
+        .length ?? 0) >= need,
   };
   return { host, files, calls, phases, prompts };
 }

factory/engine/interpret.ts

@@ -6,6 +6,8 @@ import {
   writeArtifact,
 } from "./artifacts.ts";
 import { CHECKS } from "./checks/index.ts";
+import { reviewLoop } from "./review.ts";
+import { APPROVALS, classify } from "./risk.ts";
 import type { CheckResult, Host } from "./types.ts";
 import type { Step, Workflow } from "./workflow.ts";
 
@@ -34,6 +36,16 @@ async function runChecks(
 
 const failing = (r: Record<string, CheckResult>) => Object.entries(r).filter(([, v]) => !v.ok);
 
+// The tier comes from the paths changed so far, and the tier sets how many people must approve.
+async function gateMet(host: Host, n: number, step: Step, log?: string[]): Promise<boolean> {
+  const tier = classify(await host.changedFiles());
+  const need = APPROVALS[tier];
+  if (log)
+    log.push(`gate ${step.stage} tier ${tier} needs ${need} approval${need === 1 ? "" : "s"}`);
+  if (need === 0) return true;
+  return host.approved(n, step.stage, await artifactDigest(host, n, step.stage), need);
+}
+
 // Runs a workflow's steps in order. A step repeats its agent while its loop checks fail, up to the cap;
 // the evidence of the failing checks is the next round's prompt, so the loop learns from the check, not the model.
 export async function runWorkflow(
@@ -55,10 +67,7 @@ export async function runWorkflow(
   // not run again, because regenerating its artifacts would change what the person signed.
   let resumeAt = 0;
   for (const [i, step] of wf.steps.entries()) {
-    if (
-      step.gate === "human" &&
-      (await host.approved(n, step.stage, await artifactDigest(host, n, step.stage)))
-    ) {
+    if (step.gate === "human" && (await gateMet(host, n, step))) {
       res.log.push(`gate ${step.stage} already approved, skipped`);
       resumeAt = i + 1;
     }
@@ -67,10 +76,7 @@ export async function runWorkflow(
   for (const step of wf.steps.slice(resumeAt)) {
     const stopped = await runStep(host, step, n, ticket, res, wf.manifests);
     if (stopped) return finish("needs-human", step.stage);
-    if (
-      step.gate === "human" &&
-      !(await host.approved(n, step.stage, await artifactDigest(host, n, step.stage)))
-    ) {
+    if (step.gate === "human" && !(await gateMet(host, n, step, res.log))) {
       res.log.push(`gate ${step.stage} awaiting approval`);
       return finish("awaiting-approval", step.stage);
     }
@@ -88,6 +94,7 @@ async function runStep(
   manifests = false,
 ): Promise<boolean> {
   // A reply the runner refuses (chat instead of the artifact) gets the reason and two more tries.
+  if (step.review) return runReview(host, step, n, res);
   const max = step.loop?.max ?? (step.writes ? 3 : 1);
   let feedback = "";
   const where = step.writes
@@ -126,3 +133,31 @@ async function runStep(
   }
   return true;
 }
+
+// Lenses hunt in parallel, three verifiers vote, and a builder fixes what survives, up to the round cap.
+// Returns true when an Important finding is left or a lens reply was not findings; the checks still run.
+async function runReview(host: Host, step: Step, n: number, res: Result): Promise<boolean> {
+  const lenses = step.review?.lenses ?? [];
+  let fixed = false;
+  const fixer = async () => {
+    fixed = true;
+    const findings = (await host.read(artifactPath(n, "review.json"))) ?? "";
+    host.setPhase("fix");
+    const out = await host.agent(
+      "builder",
+      `Fix these verified review findings. Never edit the locked tests.\n${findings}`,
+      { stage: "review-fix" },
+    );
+    host.setPhase(null);
+    res.costUsd += out.costUsd;
+  };
+  const r = await reviewLoop(host, n, lenses, fixer);
+  res.rounds[step.stage] = r.rounds;
+  res.log.push(`${step.stage} ${r.clean ? "clean" : "NOT clean"} after ${r.rounds} round(s)`);
+  // A fixer writes code after the build's checks passed, so the gate runs again on what it left.
+  const names = [...new Set([...(step.checks ?? []), ...(fixed ? ["gate"] : [])])];
+  const results = await runChecks(host, n, names);
+  for (const [name, c] of Object.entries(results))
+    res.log.push(`  check ${name} ${c.ok ? "ok" : "FAIL"}: ${c.evidence.split("\n")[0]}`);
+  return !r.clean || failing(results).length > 0;
+}