diff --git a/.factory/workflows/bug-tdd.yaml b/.factory/workflows/bug-tdd.yaml index c0d3488..54c19f1 100644 --- a/.factory/workflows/bug-tdd.yaml +++ b/.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//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, diff --git a/.factory/workflows/contract.yaml b/.factory/workflows/contract.yaml index 2dc269e..ff22632 100644 --- a/.factory/workflows/contract.yaml +++ b/.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], + } diff --git a/.factory/workflows/deps.yaml b/.factory/workflows/deps.yaml index 0d64811..3708921 100644 --- a/.factory/workflows/deps.yaml +++ b/.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], + } diff --git a/.factory/workflows/docs.yaml b/.factory/workflows/docs.yaml index 3a6fb10..792eee7 100644 --- a/.factory/workflows/docs.yaml +++ b/.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], + } diff --git a/.factory/workflows/feature-atdd.yaml b/.factory/workflows/feature-atdd.yaml index 7a49d54..6b1f0fa 100644 --- a/.factory/workflows/feature-atdd.yaml +++ b/.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, diff --git a/.factory/workflows/feature-bdd.yaml b/.factory/workflows/feature-bdd.yaml index fbb6afc..8558e0a 100644 --- a/.factory/workflows/feature-bdd.yaml +++ b/.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, diff --git a/.factory/workflows/feature-sdd.yaml b/.factory/workflows/feature-sdd.yaml index b0a87b3..597bb44 100644 --- a/.factory/workflows/feature-sdd.yaml +++ b/.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. When , the system shall '. 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//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//; 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//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. (R)', 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//golden. They fail on the base commit. Write no source.", + brief: "Write a vitest test or fast-check property for each requirement in specs//golden, following specs//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//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, diff --git a/.factory/workflows/incident.yaml b/.factory/workflows/incident.yaml index e61ec68..43d1adb 100644 --- a/.factory/workflows/incident.yaml +++ b/.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], + } diff --git a/.factory/workflows/llm-eval.yaml b/.factory/workflows/llm-eval.yaml index 348d932..3f59b7e 100644 --- a/.factory/workflows/llm-eval.yaml +++ b/.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], + } diff --git a/.factory/workflows/refactor.yaml b/.factory/workflows/refactor.yaml index 42cb998..af62c3a 100644 --- a/.factory/workflows/refactor.yaml +++ b/.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], + } diff --git a/.factory/workflows/security.yaml b/.factory/workflows/security.yaml index e43de61..de06843 100644 --- a/.factory/workflows/security.yaml +++ b/.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], + } diff --git a/.factory/workflows/ui-visual.yaml b/.factory/workflows/ui-visual.yaml index 8e01510..9d10db9 100644 --- a/.factory/workflows/ui-visual.yaml +++ b/.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] } diff --git a/docs/factory/f5-review.md b/docs/factory/f5-review.md index 2512a38..ef321fc 100644 --- a/docs/factory/f5-review.md +++ b/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//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`. diff --git a/factory/engine/artifacts.ts b/factory/engine/artifacts.ts index 0dcd0a1..2a797d6 100644 --- a/factory/engine/artifacts.ts +++ b/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(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 "". diff --git a/factory/engine/checks/index.ts b/factory/engine/checks/index.ts index f347985..5c9f85c 100644 --- a/factory/engine/checks/index.ts +++ b/factory/engine/checks/index.ts @@ -44,9 +44,14 @@ export const CHECKS: Record = { 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"]), }; diff --git a/factory/engine/fake-host.ts b/factory/engine/fake-host.ts index 731c177..84ff847 100644 --- a/factory/engine/fake-host.ts +++ b/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 }; } diff --git a/factory/engine/interpret.ts b/factory/engine/interpret.ts index 3caed36..15a4142 100644 --- a/factory/engine/interpret.ts +++ b/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) => 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 { + 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 { // 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 { + 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; +} diff --git a/factory/engine/real-host.ts b/factory/engine/real-host.ts index 63b9359..ab5be4b 100644 --- a/factory/engine/real-host.ts +++ b/factory/engine/real-host.ts @@ -39,14 +39,21 @@ export function approvalIn( stage: string, digest: string, approvers?: string[], + need = 1, ): boolean { const line = `approve ${stage} ${digest}`; - return comments.some( - (c) => - ["OWNER", "MEMBER", "COLLABORATOR"].includes(c.authorAssociation) && - (!approvers || approvers.includes(c.author?.login ?? "")) && - c.body.split("\n").some((l) => l.trim() === line), + // Distinct people: one person commenting twice is still one approval. + const who = new Set( + comments + .filter( + (c) => + ["OWNER", "MEMBER", "COLLABORATOR"].includes(c.authorAssociation) && + (!approvers || approvers.includes(c.author?.login ?? "")) && + c.body.split("\n").some((l) => l.trim() === line), + ) + .map((c) => c.author?.login ?? ""), ); + return who.size >= need; } export type RealHostOptions = { @@ -250,11 +257,11 @@ export function realHost(o: RealHostOptions): Host { setPhase(p) { phase = p; }, - async approved(n, stage, digest) { + async approved(n, stage, digest, need = 1) { // Runner-side call with the real environment: gh needs the person's login, the agents never see it. const r = sh("gh", ["issue", "view", String(n), "--json", "comments"], o.cwd, process.env); if (r.status !== 0) return false; - return approvalIn(JSON.parse(r.stdout).comments ?? [], stage, digest, o.approvers); + return approvalIn(JSON.parse(r.stdout).comments ?? [], stage, digest, o.approvers, need); }, }; } diff --git a/factory/engine/risk.ts b/factory/engine/risk.ts index 12a27c2..05ae986 100644 --- a/factory/engine/risk.ts +++ b/factory/engine/risk.ts @@ -21,7 +21,7 @@ const norm = (f: string) => posix.normalize(f.replaceAll("\\", "/")).replace(/^\.\//, "").toLowerCase(); const RUN_ARTIFACT = - /^specs\/\d+\/(intent\.md|spec\.md|plan\.md|run\.log|review\.json|lock\.json|incident\.md)$/; + /^specs\/\d+\/(intent\.md|spec\.md|plan\.md|constitution\.md|clarify\.md|tasks\.md|analyze\.md|run\.log|review\.json|lock\.json|incident\.md)$/; // The tier comes from the paths in the diff, never from who wrote it. The worst path sets it, and // HIGH is checked before LOW. specs// holds the run's own artifacts and says nothing about risk. diff --git a/factory/engine/types.ts b/factory/engine/types.ts index 29d4637..c192ed9 100644 --- a/factory/engine/types.ts +++ b/factory/engine/types.ts @@ -20,7 +20,8 @@ export type Host = { setPhase(phase: string | null): void; // A human gate: true once a person approved this stage's artifacts. `digest` is the hash of what they // read, so an approval of one version never covers another. - approved(n: number, stage: string, digest: string): Promise; + // `need` is how many distinct people must have approved (from the risk tier); omitted means one. + approved(n: number, stage: string, digest: string, need?: number): Promise; }; export type CheckResult = { ok: boolean; evidence: string }; diff --git a/factory/engine/wiring.test.ts b/factory/engine/wiring.test.ts new file mode 100644 index 0000000..4f32ebd --- /dev/null +++ b/factory/engine/wiring.test.ts @@ -0,0 +1,110 @@ +import { existsSync, readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { CHECKS } from "./checks/index.ts"; +import { fakeHost } from "./fake-host.ts"; +import { runWorkflow } from "./interpret.ts"; +import { loadWorkflow } from "./workflow.ts"; + +const wf = (steps: string) => + loadWorkflow(`name: t\nmethod: m\nsource: s\nfor: f\nsteps:\n${steps}`); +const finding = { file: "a.ts", line: 3, severity: "important", claim: "bad", evidence: "x=null" }; +const reviewStep = + " - {stage: review, agent: reviewer, review: {lenses: [reviewer, test-reviewer]}}"; + +describe("the review step runs lenses and voters", () => { + it("runs every lens, three voters per finding, and writes review.json", async () => { + const { host, calls, files } = fakeHost({ + byAgent: (name) => ({ + text: name === "verifier" ? "yes" : name === "reviewer" ? JSON.stringify([finding]) : "[]", + }), + }); + // The fixer cannot clear the finding, so the run stops with the finding on file. + const r = await runWorkflow(host, wf(reviewStep), 4, "t"); + expect(calls).toContain("agent reviewer review"); + expect(calls).toContain("agent test-reviewer review"); + expect(calls.filter((c) => c === "agent verifier review-vote").length).toBeGreaterThanOrEqual( + 3, + ); + expect(JSON.parse(files.get("specs/4/review.json") ?? "{}").findings).toHaveLength(1); + expect(r.outcome).toBe("needs-human"); + expect(r.stoppedAt).toBe("review"); + }); + it("passes when no finding survives the vote", async () => { + const { host } = fakeHost({ + byAgent: (name) => ({ + text: name === "verifier" ? "no" : name === "reviewer" ? JSON.stringify([finding]) : "[]", + }), + }); + expect((await runWorkflow(host, wf(reviewStep), 4, "t")).outcome).toBe("done"); + }); + it("stops when a lens replies with something that is not findings", async () => { + const { host } = fakeHost({ agents: [{ text: "looks fine" }] }); + expect((await runWorkflow(host, wf(reviewStep), 4, "t")).outcome).toBe("needs-human"); + }); +}); + +describe("the human gate takes its approval count from the risk tier", () => { + const gate = wf(" - {stage: spec, agent: planner, gate: human}"); + it("needs one approval for a medium change and two for a high one", async () => { + const one = fakeHost({ approvals: ["5:spec"], changed: ["specs/5/spec.md"] }); + expect((await runWorkflow(one.host, gate, 5, "t")).outcome).toBe("done"); + const high = fakeHost({ approvals: ["5:spec"], changed: [".github/workflows/ci.yml"] }); + const r = await runWorkflow(high.host, gate, 5, "t"); + expect(r.outcome).toBe("awaiting-approval"); + expect(r.log.join("\n")).toContain("tier high needs 2"); + const two = fakeHost({ + approvals: ["5:spec", "5:spec"], + changed: [".github/workflows/ci.yml"], + }); + expect((await runWorkflow(two.host, gate, 5, "t")).outcome).toBe("done"); + }); + it("needs no person for a docs-only change", async () => { + const { host } = fakeHost({ changed: ["docs/a.md"] }); + expect((await runWorkflow(host, gate, 5, "t")).outcome).toBe("done"); + }); +}); + +describe("a check names a script that exists", () => { + it("prepush and eval run files in this repo", async () => { + for (const name of ["prepush", "eval"]) { + const { host, calls } = fakeHost(); + await CHECKS[name]?.run({ host, n: 1 }); + const paths = calls + .flatMap((c) => c.split(" ")) + .filter((a) => a.endsWith(".ts") && a.includes("/")); + expect(paths.length, name).toBeGreaterThan(0); + for (const p of paths) expect(existsSync(p), `${name}: ${p}`).toBe(true); + } + }); +}); + +describe("feature-sdd carries the Spec Kit stages", () => { + const sdd = loadWorkflow(readFileSync(".factory/workflows/feature-sdd.yaml", "utf8")); + it("orders constitution, spec, clarify, plan, tasks, analyze before the tests", () => { + const order = sdd.steps.map((s) => s.stage); + const at = (s: string) => order.indexOf(s); + for (const s of ["constitution", "clarify", "tasks", "analyze"]) + expect(at(s)).toBeGreaterThan(-1); + expect(at("constitution")).toBeLessThan(at("spec")); + expect(at("spec")).toBeLessThan(at("clarify")); + expect(at("clarify")).toBeLessThan(at("plan")); + expect(at("plan")).toBeLessThan(at("tasks")); + expect(at("tasks")).toBeLessThan(at("analyze")); + expect(at("analyze")).toBeLessThan(at("tests")); + }); + it("saves each new stage as its own artifact", () => { + for (const s of ["constitution", "clarify", "tasks", "analyze"]) + expect(sdd.steps.find((x) => x.stage === s)?.writes).toBe(`${s}.md`); + }); +}); + +describe("every workflow review step runs lenses", () => { + it("names at least two lenses and uses the voting path", () => { + for (const f of ["bug-tdd", "feature-sdd", "security", "deps"]) { + const w = loadWorkflow(readFileSync(`.factory/workflows/${f}.yaml`, "utf8")); + expect(w.steps.find((s) => s.stage === "review")?.review?.lenses.length, f).toBeGreaterThan( + 1, + ); + } + }); +}); diff --git a/factory/engine/workflow.ts b/factory/engine/workflow.ts index b54d3e5..2459deb 100644 --- a/factory/engine/workflow.ts +++ b/factory/engine/workflow.ts @@ -36,6 +36,8 @@ const stepSchema = z.object({ loop: z .object({ until: z.array(z.enum(CHECK_NAMES)).min(1), max: z.number().int().min(1).max(5) }) .optional(), + // Parallel lenses hunt, three verifiers vote (2 of 3 keep a finding), a builder fixes what survives. + review: z.object({ lenses: z.array(z.string()).min(1) }).optional(), gate: z.enum(["human", "auto"]).default("auto"), // Fix steps run with the test lock on. phase: z.enum(["build", "fix"]).default("build"), diff --git a/factory/engine/workflows.test.ts b/factory/engine/workflows.test.ts index c9db0c1..749811b 100644 --- a/factory/engine/workflows.test.ts +++ b/factory/engine/workflows.test.ts @@ -7,7 +7,15 @@ import { loadWorkflow } from "./workflow.ts"; const dir = ".factory/workflows"; const files = readdirSync(dir).filter((f) => f.endsWith(".yaml")); // The planner has no Write tool, so these stages only produce an artifact if the runner saves the reply. -const ARTIFACT_STAGES = new Set(["intent", "spec", "plan"]); +const ARTIFACT_STAGES = new Set([ + "intent", + "spec", + "plan", + "constitution", + "clarify", + "tasks", + "analyze", +]); const agents = new Set(readdirSync(".claude/agents").map((f) => f.replace(/\.md$/, ""))); describe.each(files)("workflow %s", (file) => { @@ -52,6 +60,8 @@ describe.each(files)("workflow %s", (file) => { "specs/1/a.test.ts": "t", ...(wf.manifests ? { "package.json": bumped } : {}), }, + // Lenses find nothing, so the review step is clean. + byAgent: (name) => (name.endsWith("reviewer") ? { text: "[]" } : undefined), changed: ["specs/1/plan.md", "specs/1/a.test.ts", ...(wf.manifests ? ["package.json"] : [])], exec: [ { match: "npx vitest", ref: "base", result: { code: 1, output: "red" } },