From 770638c0d6e2500bdfc501aff1a33e9d51b5f62b Mon Sep 17 00:00:00 2001 From: Param Harrison Date: Fri, 30 Oct 2026 23:28:31 +0200 Subject: [PATCH 1/3] Add factory workshop modules F2 design, F3 build, F4 test Co-Authored-By: Claude Sonnet 5.5 --- docs/factory/F2-design.md | 123 ++++++++++++++++++++++++++++++++++++++ docs/factory/F3-build.md | 110 ++++++++++++++++++++++++++++++++++ docs/factory/F4-test.md | 122 +++++++++++++++++++++++++++++++++++++ 3 files changed, 355 insertions(+) create mode 100644 docs/factory/F2-design.md create mode 100644 docs/factory/F3-build.md create mode 100644 docs/factory/F4-test.md diff --git a/docs/factory/F2-design.md b/docs/factory/F2-design.md new file mode 100644 index 0000000..41dcb59 --- /dev/null +++ b/docs/factory/F2-design.md @@ -0,0 +1,123 @@ +# F2 Design: the spec is the first test + +**Stage:** design. **Layers:** context and verification. **Workflows:** `feature-atdd`, `feature-bdd`, `feature-sdd`. + +## Goal + +Turn an `intent.md` into a `spec.md` that a check can grade, and stop for a human before any code exists. You write the same behaviour as examples, as Gherkin and as EARS requirements, and learn which one fits which ticket. + +## Why the spec comes before the code + +A builder that writes its own tests and its own code grades itself. The spec is the one artifact a person reads and signs, so everything after it (tests, build, review) is measured against words a human approved. + +In the engine the spec is a file, `specs//spec.md`, saved by the runner. The planner agent has only `Read, Grep, Glob` (`.claude/agents/planner.md`), so it cannot write the file. The runner saves its reply, and refuses a reply that is chat. `startsWith` in the workflow decides what counts: `Feature:` for BDD, `R1.` for SDD (`factory/engine/interpret.ts`). + +## Steps + +1. Read the three workflows side by side and find the `spec` step in each. + + ``` + .factory/workflows/feature-atdd.yaml + .factory/workflows/feature-bdd.yaml + .factory/workflows/feature-sdd.yaml + ``` + +2. Write the same small feature three ways. Take "a repeat prompt in the same band costs zero tokens". + + Examples (ATDD, Adzic, Specification by Example). One line per example, real values, as the `feature-atdd` brief asks: + + ``` + Given "Fractions worksheet" for grade 3 was answered once, when the same prompt is sent for grade 3, then the model is called 0 times. + Given the same, when the prompt is sent for grade 7, then the model is called 1 time. + ``` + + Gherkin (BDD, North, the three amigos). Begins with `Feature:`, one behaviour per scenario. Compare `specs/35/spec.md`: + + ``` + Scenario: A repeat prompt in the same band spends zero tokens + Given the prompt "Fractions worksheet" for band "grade 3" was answered once + When the same prompt "Fractions worksheet" is sent for band "grade 3" + Then the model is called 0 times + ``` + + EARS (SDD, the Kiro notation). Begins with `R1.`, one trigger and one response per line. Compare `specs/34/spec.md`: + + ``` + R6. When a workbook is planned, the system shall place no more items on any page than the `maxItemsPerPage` of the workbook's band. + ``` + +3. Pick by the ticket, not by taste. + + | method | write it when | what grades it later | + | --- | --- | --- | + | Examples | a small feature, one person can hold it in their head | one vitest test per example, assertion matches the `Then` | + | Gherkin | user-facing behaviour that product, dev and test must agree on | one vitest test per scenario; the `amigos` step reads it as three roles | + | EARS | a large feature where a rule holds for all inputs | a test or a fast-check property per requirement | + +4. Run the dry run of every workflow and read the order of stages. + + ``` + npx vitest run factory/engine/workflows.test.ts + ``` + +5. Find the human gate. In `feature-sdd.yaml` the `spec` and `plan` steps carry `gate: human`. In `feature-atdd.yaml` only `spec` does. In `feature-bdd.yaml` the gate is on `amigos`. + +## Phase gates + +A gate stops the run with the outcome `awaiting-approval` until a person approves what they read. + +- The approval is a comment on the issue with the exact line `approve `. +- The digest is a hash of `intent.md` and `spec.md` for the `spec` stage, and adds `plan.md` for later stages (`artifactDigest` in `factory/engine/artifacts.ts`). Edit the spec after approving and the old approval no longer matches. +- Only an `OWNER`, `MEMBER` or `COLLABORATOR` counts, and the real run narrows that to the logged-in `gh` user. The agents hold no GitHub token, so they cannot approve (`approvalIn` in `factory/engine/real-host.ts`). +- On resume the runner skips every step up to the last approved gate, so regenerating an artifact cannot change what the person signed. + +## Spec Kit and Kiro compared with this repo + +`feature-sdd.yaml` names both as its source. What the file actually contains: + +| idea | Spec Kit | Kiro | here | +| --- | --- | --- | --- | +| Requirements notation | free prose spec | EARS, "when X the system shall Y" | EARS, `startsWith: "R1."` | +| Approval between phases | per command | per phase | `gate: human` on `spec` and `plan` | +| Plan stays bounded | `plan` and `tasks` | design and tasks | `plan.md` is an allowlist; `plan-drift` fails a stray file (F3) | +| Properties from requirements | not part of it | "for any ... shall" | the `properties` check runs `specs//properties` (F4) | +| Project constitution | `constitution` | steering files | `AGENTS.md` and `.claude/rules/`, no workflow stage | +| `clarify` and `analyze` | yes | no | no stage; a reviewer reads the spec by hand | + +The last row is a gap, not a claim. `feature-sdd.yaml` has no `clarify`, `tasks` or `analyze` stage, so nothing checks that every requirement maps to a test. You close that gap in the exercise. + +## Proof that bites + +The spec-artifact rules are tested in the engine suite. + +``` +npx vitest run factory/engine/engine.test.ts factory/engine/real-host.test.ts +``` + +- A planner reply that is chat ("I drafted the scenarios, but plan mode blocks edits") is refused and nothing is saved as `spec.md`; the step retries with the reason. +- `approvalIn` returns false for a stranger, for another stage, for another digest, and for a line that only contains the approval text. + +Break it once: in `factory/engine/interpret.ts` delete the `else if (step.startsWith && ...)` branch. The engine test that feeds a chat reply to a `startsWith: "Feature:"` step now fails, because the apology is saved as `spec.md`. Revert it. + +## See it on a real run + +- Spec 35, BDD: `specs/35/spec.md` is a Gherkin file with a `Background` and one scenario per behaviour (cache hit, band change, failed generation not cached, template holds slots and no filled content). `specs/35/red.md` maps each scenario to a golden test by title. +- Spec 28, BDD: `specs/28/spec.md` and `features/activity-kinds.feature`. +- Spec 34, SDD: `specs/34/spec.md` is EARS, `R1.` onward. `specs/34/tests.md` lists which requirement each test covers. +- Spec 33, a spike: `specs/33/spec.md` is an ADR, the decision (files) and why, with no code. + +The `.feature` files are read by people and by the `amigos` agent. No Gherkin runner executes them: each scenario is a hand-written vitest test, and the `Then` is the assertion. + +No committed run used `feature-atdd`, so this module shows its workflow file and not a run. + +## Exercise + +Track A: take one scenario from `specs/35/spec.md` and rewrite it as an example line and as an EARS line. Say which of the three you would have asked the planner for, and why. + +Track B: add an `analyze` check that fails when a requirement id in `spec.md` (`R.`) appears in no file under `specs//golden` or `specs//properties`. Give it a failing input first, add it to `CHECK_NAMES` and `CHECKS`, and wire it into `feature-sdd.yaml`. + +## Discussion + +1. A scenario says "the page loads quickly". What would the three methods force you to write instead? +2. The gate is bound to a hash of the documents. Who is hurt if it were bound to the issue number only? +3. Your team already has a product spec in a wiki. Which part of it belongs in `spec.md`, and which part stays out because no check can grade it? diff --git a/docs/factory/F3-build.md b/docs/factory/F3-build.md new file mode 100644 index 0000000..320c0fa --- /dev/null +++ b/docs/factory/F3-build.md @@ -0,0 +1,110 @@ +# F3 Build: explore, plan, code inside a fence + +**Stage:** build. **Layers:** execution and delivery. **Workflows:** every one of them has a `plan` step; the build step differs (`feature-atdd`, `bug-tdd`, `refactor`, `ui-visual`). + +## Goal + +Let a builder change code with the smallest reach. The plan lists the files, the builder works in an isolated worktree with limited tools, and a check fails the run if the diff leaves the plan. + +## Explore, plan, code + +The order comes from Anthropic's Claude Code best practices: explore first, plan second, code third, then commit. The factory splits those into agents with different powers. + +| agent | tools (from `.claude/agents/`) | job | +| --- | --- | --- | +| `planner` | `Read, Grep, Glob` | read the repo and the ticket, reply with the plan | +| `builder` | `Read, Edit, Write, Bash, Grep, Glob`, `isolation: worktree`, `permissionMode: acceptEdits` | change the files the plan lists | +| `verifier` | `Read, Grep, Glob, Bash`, `permissionMode: plan` | run the gate, change nothing | + +The planner cannot write. In the workflow the `plan` step has `writes: plan.md`, and the runner saves the reply as `specs//plan.md`. A reply that is prose instead of paths is refused with the reason and the planner tries again. + +## plan.md is an allowlist + +`plan.md` is one repo path per line (a bullet or backticks are fine). `badPlanLines` in `factory/engine/artifacts.ts` refuses a plan line that is not an ordinary repo path: + +- globs, absolute paths and `..` segments; +- anything under `.github`, `.claude`, `.factory`, `factory` or `scripts`, plus `package.json`, lockfiles, `biome.json`, `vitest` and `tsconfig` files. Only a workflow with `manifests: true` (the `deps` one) may list `package.json` and the lockfile; +- lines under `specs//`, which the runner drops because it always allows them. + +Two plans to read, from the real runs: `specs/25/plan.md` lists the split of `render.ts` plus `levels.json`; `specs/31/plan.md` lists both sides of the SSE contract and `tests/contract/`. + +Why `levels.json` shows up: a new source file needs an owner there, so a plan that adds a file must list it. The workflow brief says so, and `workflows.test.ts` asserts every plan brief contains that sentence. + +## Plan drift + +`plan-drift` is a small check. It reads `plan.md`, asks the host for the changed files, and fails when a changed file is not listed. + +``` +plan-drift fails: files outside plan.md: api/src/agent/template-eval.ts. Update plan.md in the same change. +``` + +- A missing `plan.md` is a failed check, not a crash, so the loop can act on it. +- Editing `plan.md` is not itself a pass: the plan file is in every diff, so the check reads the listed paths and not the file's own change. +- Regenerated golden snapshots under `specs//golden/__snapshots__/` pass, because a human reviews them at the feedback gate. + +## Worktrees + +Two kinds, both real. + +- `builder.md` sets `isolation: worktree`, so an agent run happens in its own checkout and the main tree stays clean. +- The `red` check runs the new tests against the base commit. `real-host.ts` adds a detached `git worktree` at the base inside the repo (`.factory-base/`, gitignored), copies the `specs` files into it, runs the command, then removes the worktree. + +## The builder's fence + +The builder is limited by files the runner does not let it edit. + +| fence | where | what it stops | +| --- | --- | --- | +| `FACTORY_PHASE=fix` | `.claude/hooks/pretooluse-test-lock.mjs` | an edit to any test file (`.test.ts`, `.e2e.ts`, `.feature`, `tests/`) in a fix step; exit 2 | +| `lock` check | `factory/engine/checks/lock.ts` | a test edited by another route, such as `Bash` | +| deny rules | `.claude/settings.json` | reading `.env*` and `.factory/holdout/**`, and `git push *` | +| sandbox | `.claude/settings.json` | `denyRead` on the same two paths | +| plan | `plan-drift` | any file outside the plan | +| per-stage config | `.factory/config.json` | `maxTurns` and `maxBudgetUsd` for the `build` stage | + +A step with `phase: fix` runs with the test lock on from round one. Any retry of a step runs with it on too (`interpret.ts`). + +## Visual iteration + +For a UI change, `ui-visual.yaml` has the builder work against checks `visual` and `gate`, looping up to a cap. The `visual` check runs `playwright test tests/e2e`. See F4 for what those tests assert. A design contract is the first step, so the builder has words to compare a screenshot against. + +## Steps + +1. Read the `plan` step brief in `.factory/workflows/bug-tdd.yaml` and the three agent files above. +2. See the fence refuse an edit: + + ``` + echo '{"tool_input":{"file_path":"api/src/a.test.ts"}}' | FACTORY_PHASE=fix node .claude/hooks/pretooluse-test-lock.mjs; echo $? + ``` + + It prints the refusal and exits 2. Run it again with `FACTORY_PHASE` unset and it exits 0. +3. Run the plan and drift tests. + + ``` + npx vitest run factory/engine/checks/plan-drift.test.ts factory/engine/engine.test.ts + ``` +4. Read `specs/25/plan.md`, then `specs/25/refactor.md`. Every file named in the second is in the first. + +## Proof that bites + +`factory/engine/engine.test.ts` has the case "plan-drift fails on a stray file, even when plan.md is in the diff". It asserts `planDrift("a.ts", ["a.ts", "b.ts"], 1)` returns `["b.ts"]`. To see it fail, change `!listed.has(f)` in `planDrift` to `false` and run the file; the stray file now passes, and the test turns red. Revert it. + +`tests/structure/review-and-test-lock.test.ts` pins the hook: a test edit is blocked in a fix and allowed elsewhere, and the hook is wired in `settings.json`. + +## See it on a real run + +- Spec 35: `specs/35/build.md` records four updates. The last one says plan drift was fixed: `plan.md` had lost its file list and never named `api/src/agent/template-eval.ts`. `specs/35/ci.md` quotes the failing line, `check plan-drift FAIL: files outside plan.md: api/src/agent/template-eval.ts`. The `run.log` it quotes is gitignored, so only the notes are in the repo. +- Spec 25, the refactor: `specs/25/refactor.md` lists the four functions moved out of `render.ts`, with bodies unchanged, and says the golden tests and snapshots were untouched. +- Spec 26, the bug: `specs/26/fix.md` is the smallest change that made the locked test pass. + +## Exercise + +Track A: add a stray file to a scratch branch's diff and watch `plan-drift` name it, then add the path to `plan.md` and watch it pass. + +Track B: the builder has `Bash`, and the test lock hook matches only `Edit|Write|MultiEdit|NotebookEdit`. Name the check that still catches a test changed by `sed -i`, and write the test that proves it. + +## Discussion + +1. The plan is written by a model and used as an allowlist. What stops it from listing everything, and what would you add? +2. A fix needs a file the plan did not list. Who should be allowed to grow the plan, and when? +3. `isolation: worktree` protects your tree from the builder. What does it not protect you from? diff --git a/docs/factory/F4-test.md b/docs/factory/F4-test.md new file mode 100644 index 0000000..b31f050 --- /dev/null +++ b/docs/factory/F4-test.md @@ -0,0 +1,122 @@ +# F4 Test: evidence the model cannot talk its way past + +**Stage:** test. **Layer:** verification. **Workflows:** `bug-tdd`, `feature-atdd`, `feature-bdd`, `feature-sdd`, `refactor`, `contract`, `ui-visual`, `llm-eval`. + +## Goal + +Know which check proves which claim, run it at the right size, and see it fail on purpose. An agent optimises for whatever grades it, so each check below also names the trick it stops. + +## The ladder + +Every check lives in `factory/engine/checks/index.ts` and declares a size. The engine runs them from the `checks` and `loop.until` lists of a workflow step. + +| check | size | claim it proves | what it runs | +| --- | --- | --- | --- | +| `red` | small | the new test fails on the base commit | `vitest run specs/` at the base worktree | +| `lock` | small | no test moved since it was recorded | hashes every test file | +| `plan-drift` | small | the diff stays in `plan.md` | reads the plan and the changed files | +| `green` | small | the locked test passes | `vitest run specs/` | +| `acceptance` | medium | the examples or scenarios pass | `vitest run specs/` | +| `holdout` | medium | cases the builder never saw pass | `vitest run .factory/holdout/` | +| `properties` | medium | a rule holds for generated inputs | `vitest run specs//properties` | +| `behaviour-unchanged` | medium | recorded output is byte-for-byte the same | `vitest run specs//golden` | +| `contract` | medium | both sides agree on the wire | `vitest run tests/contract` | +| `mutation` | large | the tests notice a changed line | Stryker on the changed source files | +| `visual` | large | layout, overflow and accessibility hold | `playwright test tests/e2e` | +| `eval` | large | model behaviour and its token cost hold | `tsx scripts/eval.ts` | +| `gate` | large | the whole repo is green | `make check` | +| `ci` | large | the pull request is green | `gh pr checks --watch` | + +Sizes follow the Google idea of small, medium and large. A small check is fast and hermetic. The size is a field on every check. + +## TDD: red and lock + +`red` is `factory/engine/checks/red.ts`. It runs the spec tests against the base commit and fails if they pass: "already passes on the base commit, so it proves nothing". This stops the test that was written to pass. + +`lock` records a sha256 of every test file (`.test.ts`, `.e2e.ts`, `.feature`, anything under `tests/`) the first time it runs. The record is in runner state outside the worktree, so the builder cannot rewrite it; `specs//lock.json` is only a copy for the audit trail. A later call fails if a test changed or vanished. New tests may be added. + +## Properties + +A property states a rule for all inputs and a library generates them. `specs/34/properties/workbook.test.ts` uses fast-check to build workbooks of random size and band, and asserts what `specs/34/spec.md` says: every item lands on exactly one page, and no page exceeds `maxItemsPerPage`. The property needs a planted mutant to prove it can fail: remove the page split in `planWorkbook` and a counterexample prints. + +## Characterization + +For a refactor, you cannot write the behaviour from a spec, because the code is the spec. `refactor.yaml` records the current output first, with `behaviour-unchanged` and `lock` as the checks and no source change allowed. `specs/25/golden/render.golden.test.ts` records `renderWorksheet` for every band and keeps the HTML in `specs/25/golden/__snapshots__/`. A refactor that moves one byte fails. + +## Mutation + +Line coverage says a line ran, not that a test would notice if it were wrong. `mutation` runs Stryker only on the changed files under `shared/src` (a whole-repo score would punish a run for old gaps). `stryker.config.json` sets `thresholds.break` to 50, so a score below it fails the step. A file name with glob characters is refused so it cannot rewrite the mutate list. Stryker runs the unit tests beside the code and `specs/*/golden`, as `stryker.vitest.config.ts` says. + +## Holdouts + +A builder can overfit to tests it can read. `holdout` runs cases from `.factory/holdout//`, which `.claude/settings.json` denies to `Read` and to the sandbox. A human adds the cases. If the folder is empty the check passes with "nothing graded", because run 28 looped four rounds on "No test files found". An empty folder therefore grades nothing, so read the evidence line. + +## Contract + +When two sides talk, test the wire, not each side alone. `tests/contract/sse-events.test.ts` imports `specs/31/golden/sse-contract.golden.test.ts`, which replays `recorded-events.json` through the api writer and the web reader. A changed event shape becomes one generic error on the web side. + +## Visual and accessibility + +`tests/e2e/layout.e2e.ts` uses Playwright and `@axe-core/playwright`. It asserts no horizontal overflow, no serious accessibility violations, no clipped text and a page count for print. + +## Evals for LLM behaviour + +A prompt change is code, so it needs a test with a token ceiling. `api/src/agent/template-eval.ts` compares output tokens without a template against output tokens with one and returns `ok` only if the second is smaller. `api/src/eval.ts` exits 1 when it is not. Run it with `make eval`, which spends tokens. + +## Steps + +1. See `red` refuse a passing test, and `lock` catch an edit: + + ``` + npx vitest run factory/engine/engine.test.ts -t "red" + npx vitest run factory/engine/engine.test.ts -t "lock" + ``` +2. See holdout and mutation behaviour: + + ``` + npx vitest run factory/engine/checks + ``` +3. Run a real golden test and the property test: + + ``` + npx vitest run specs/25 specs/31 specs/34/properties + ``` + The workbook PDF property lives in `shared/src/workbook-pdf.test.ts` and needs Playwright Chromium installed. +4. Run the structural check on the lock hook: `npx vitest run tests/structure/review-and-test-lock.test.ts`. + +## Proof that bites + +Pick one and revert after. + +- In `factory/engine/checks/lock.ts` change `now[f] !== before[f]` to `false`. The test "lock catches an edited test" turns red. +- In `factory/engine/checks/red.ts` change `r.code === 0` to `false`. The test "red refuses a test that already passes on base" turns red. +- In `shared/src/escape-html.ts` drop the `&` replacement. `specs/25/golden` fails, because the fixtures hold `&` in titles. + +## See it on a real run + +- Spec 35, scenarios: `specs/35/spec.md`, `specs/35/red.md`. The red notes record: 31 of 33 tests fail on `main`, and two pass because they are guards that hold on both sides. The note says the `red` check must run with `ref: base`, since on the branch all pass. +- Spec 31, contract: `specs/31/golden/sse-contract.golden.test.ts`, `specs/31/golden/recorded-events.json`, `specs/31/sides.md`. +- Spec 34, properties: `specs/34/properties/workbook.test.ts`, `specs/34/tests.md`, which lists the interface the tests pin and warns that the PDF property needs a browser. +- Spec 25, characterization: `specs/25/golden/__snapshots__/`. +- Spec 36: `specs/36/intent.md` is a vetting report that says the registry was unreachable, so age and maintainer were not checked. The run reported what it could not verify. + +## Known wiring gaps + +Two checks point at files that are not in this repo, so running them fails. + +- `eval` runs `scripts/eval.ts`, but the eval is `api/src/eval.ts` (`make eval`). +- `prepush` runs `scripts/git/pre-push`, and `make hooks` installs only `pre-commit`. + +No workflow lists either check, so no run hits them. Fixing each is an assignment: wire it, then give it a failing input. + +## Exercise + +Track A: write a characterization test for one pure function in `shared/src`, run it against the unchanged code, then change the function and watch it fail. + +Track B: add a case under `.factory/holdout//` for a finished spec, check that `Read` of it is denied by `.claude/settings.json`, and run `holdout`. + +## Discussion + +1. A test passes on both base and branch. Is it a guard or a mistake? How would you tell? +2. Mutation score is 50 at the floor. What does it cost to push it to 90, and who pays? +3. Which check on the ladder would you drop first for a prototype, and which for a payment flow? From ef6e4d3a3f1dfa254a2b2d67aef912e7389d63ca Mon Sep 17 00:00:00 2001 From: Param Harrison Date: Fri, 30 Oct 2026 23:32:56 +0200 Subject: [PATCH 2/3] Rename factory docs to kebab-case Co-Authored-By: Claude Sonnet 5.5 --- docs/factory/{F2-design.md => f2-design.md} | 0 docs/factory/{F3-build.md => f3-build.md} | 0 docs/factory/{F4-test.md => f4-test.md} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename docs/factory/{F2-design.md => f2-design.md} (100%) rename docs/factory/{F3-build.md => f3-build.md} (100%) rename docs/factory/{F4-test.md => f4-test.md} (100%) diff --git a/docs/factory/F2-design.md b/docs/factory/f2-design.md similarity index 100% rename from docs/factory/F2-design.md rename to docs/factory/f2-design.md diff --git a/docs/factory/F3-build.md b/docs/factory/f3-build.md similarity index 100% rename from docs/factory/F3-build.md rename to docs/factory/f3-build.md diff --git a/docs/factory/F4-test.md b/docs/factory/f4-test.md similarity index 100% rename from docs/factory/F4-test.md rename to docs/factory/f4-test.md From a369070d1ecc08df92cf8a335723f5bbfac6f162 Mon Sep 17 00:00:00 2001 From: Param Harrison Date: Fri, 30 Oct 2026 23:38:08 +0200 Subject: [PATCH 3/3] Format factory docs Co-Authored-By: Claude Sonnet 5.5 --- docs/factory/f2-design.md | 26 +++++++++++++------------- docs/factory/f3-build.md | 24 ++++++++++++------------ docs/factory/f4-test.md | 33 +++++++++++++++++---------------- 3 files changed, 42 insertions(+), 41 deletions(-) diff --git a/docs/factory/f2-design.md b/docs/factory/f2-design.md index 41dcb59..1cc66b3 100644 --- a/docs/factory/f2-design.md +++ b/docs/factory/f2-design.md @@ -48,11 +48,11 @@ In the engine the spec is a file, `specs//spec.md`, saved by the runner. The 3. Pick by the ticket, not by taste. - | method | write it when | what grades it later | - | --- | --- | --- | - | Examples | a small feature, one person can hold it in their head | one vitest test per example, assertion matches the `Then` | - | Gherkin | user-facing behaviour that product, dev and test must agree on | one vitest test per scenario; the `amigos` step reads it as three roles | - | EARS | a large feature where a rule holds for all inputs | a test or a fast-check property per requirement | + | method | write it when | what grades it later | + | -------- | -------------------------------------------------------------- | ----------------------------------------------------------------------- | + | Examples | a small feature, one person can hold it in their head | one vitest test per example, assertion matches the `Then` | + | Gherkin | user-facing behaviour that product, dev and test must agree on | one vitest test per scenario; the `amigos` step reads it as three roles | + | EARS | a large feature where a rule holds for all inputs | a test or a fast-check property per requirement | 4. Run the dry run of every workflow and read the order of stages. @@ -75,14 +75,14 @@ A gate stops the run with the outcome `awaiting-approval` until a person approve `feature-sdd.yaml` names both as its source. What the file actually contains: -| idea | Spec Kit | Kiro | here | -| --- | --- | --- | --- | -| Requirements notation | free prose spec | EARS, "when X the system shall Y" | EARS, `startsWith: "R1."` | -| Approval between phases | per command | per phase | `gate: human` on `spec` and `plan` | -| Plan stays bounded | `plan` and `tasks` | design and tasks | `plan.md` is an allowlist; `plan-drift` fails a stray file (F3) | -| Properties from requirements | not part of it | "for any ... shall" | the `properties` check runs `specs//properties` (F4) | -| Project constitution | `constitution` | steering files | `AGENTS.md` and `.claude/rules/`, no workflow stage | -| `clarify` and `analyze` | yes | no | no stage; a reviewer reads the spec by hand | +| idea | Spec Kit | Kiro | here | +| ---------------------------- | ------------------ | --------------------------------- | --------------------------------------------------------------- | +| Requirements notation | free prose spec | EARS, "when X the system shall Y" | EARS, `startsWith: "R1."` | +| Approval between phases | per command | per phase | `gate: human` on `spec` and `plan` | +| Plan stays bounded | `plan` and `tasks` | design and tasks | `plan.md` is an allowlist; `plan-drift` fails a stray file (F3) | +| Properties from requirements | not part of it | "for any ... shall" | the `properties` check runs `specs//properties` (F4) | +| Project constitution | `constitution` | steering files | `AGENTS.md` and `.claude/rules/`, no workflow stage | +| `clarify` and `analyze` | yes | no | no stage; a reviewer reads the spec by hand | The last row is a gap, not a claim. `feature-sdd.yaml` has no `clarify`, `tasks` or `analyze` stage, so nothing checks that every requirement maps to a test. You close that gap in the exercise. diff --git a/docs/factory/f3-build.md b/docs/factory/f3-build.md index 320c0fa..5e41bd7 100644 --- a/docs/factory/f3-build.md +++ b/docs/factory/f3-build.md @@ -10,11 +10,11 @@ Let a builder change code with the smallest reach. The plan lists the files, the The order comes from Anthropic's Claude Code best practices: explore first, plan second, code third, then commit. The factory splits those into agents with different powers. -| agent | tools (from `.claude/agents/`) | job | -| --- | --- | --- | -| `planner` | `Read, Grep, Glob` | read the repo and the ticket, reply with the plan | -| `builder` | `Read, Edit, Write, Bash, Grep, Glob`, `isolation: worktree`, `permissionMode: acceptEdits` | change the files the plan lists | -| `verifier` | `Read, Grep, Glob, Bash`, `permissionMode: plan` | run the gate, change nothing | +| agent | tools (from `.claude/agents/`) | job | +| ---------- | ------------------------------------------------------------------------------------------- | ------------------------------------------------- | +| `planner` | `Read, Grep, Glob` | read the repo and the ticket, reply with the plan | +| `builder` | `Read, Edit, Write, Bash, Grep, Glob`, `isolation: worktree`, `permissionMode: acceptEdits` | change the files the plan lists | +| `verifier` | `Read, Grep, Glob, Bash`, `permissionMode: plan` | run the gate, change nothing | The planner cannot write. In the workflow the `plan` step has `writes: plan.md`, and the runner saves the reply as `specs//plan.md`. A reply that is prose instead of paths is refused with the reason and the planner tries again. @@ -53,14 +53,14 @@ Two kinds, both real. The builder is limited by files the runner does not let it edit. -| fence | where | what it stops | -| --- | --- | --- | +| fence | where | what it stops | +| ------------------- | ---------------------------------------- | -------------------------------------------------------------------------------------------- | | `FACTORY_PHASE=fix` | `.claude/hooks/pretooluse-test-lock.mjs` | an edit to any test file (`.test.ts`, `.e2e.ts`, `.feature`, `tests/`) in a fix step; exit 2 | -| `lock` check | `factory/engine/checks/lock.ts` | a test edited by another route, such as `Bash` | -| deny rules | `.claude/settings.json` | reading `.env*` and `.factory/holdout/**`, and `git push *` | -| sandbox | `.claude/settings.json` | `denyRead` on the same two paths | -| plan | `plan-drift` | any file outside the plan | -| per-stage config | `.factory/config.json` | `maxTurns` and `maxBudgetUsd` for the `build` stage | +| `lock` check | `factory/engine/checks/lock.ts` | a test edited by another route, such as `Bash` | +| deny rules | `.claude/settings.json` | reading `.env*` and `.factory/holdout/**`, and `git push *` | +| sandbox | `.claude/settings.json` | `denyRead` on the same two paths | +| plan | `plan-drift` | any file outside the plan | +| per-stage config | `.factory/config.json` | `maxTurns` and `maxBudgetUsd` for the `build` stage | A step with `phase: fix` runs with the test lock on from round one. Any retry of a step runs with it on too (`interpret.ts`). diff --git a/docs/factory/f4-test.md b/docs/factory/f4-test.md index b31f050..3e3f4c3 100644 --- a/docs/factory/f4-test.md +++ b/docs/factory/f4-test.md @@ -10,22 +10,22 @@ Know which check proves which claim, run it at the right size, and see it fail o Every check lives in `factory/engine/checks/index.ts` and declares a size. The engine runs them from the `checks` and `loop.until` lists of a workflow step. -| check | size | claim it proves | what it runs | -| --- | --- | --- | --- | -| `red` | small | the new test fails on the base commit | `vitest run specs/` at the base worktree | -| `lock` | small | no test moved since it was recorded | hashes every test file | -| `plan-drift` | small | the diff stays in `plan.md` | reads the plan and the changed files | -| `green` | small | the locked test passes | `vitest run specs/` | -| `acceptance` | medium | the examples or scenarios pass | `vitest run specs/` | -| `holdout` | medium | cases the builder never saw pass | `vitest run .factory/holdout/` | -| `properties` | medium | a rule holds for generated inputs | `vitest run specs//properties` | -| `behaviour-unchanged` | medium | recorded output is byte-for-byte the same | `vitest run specs//golden` | -| `contract` | medium | both sides agree on the wire | `vitest run tests/contract` | -| `mutation` | large | the tests notice a changed line | Stryker on the changed source files | -| `visual` | large | layout, overflow and accessibility hold | `playwright test tests/e2e` | -| `eval` | large | model behaviour and its token cost hold | `tsx scripts/eval.ts` | -| `gate` | large | the whole repo is green | `make check` | -| `ci` | large | the pull request is green | `gh pr checks --watch` | +| check | size | claim it proves | what it runs | +| --------------------- | ------ | ----------------------------------------- | ------------------------------------------- | +| `red` | small | the new test fails on the base commit | `vitest run specs/` at the base worktree | +| `lock` | small | no test moved since it was recorded | hashes every test file | +| `plan-drift` | small | the diff stays in `plan.md` | reads the plan and the changed files | +| `green` | small | the locked test passes | `vitest run specs/` | +| `acceptance` | medium | the examples or scenarios pass | `vitest run specs/` | +| `holdout` | medium | cases the builder never saw pass | `vitest run .factory/holdout/` | +| `properties` | medium | a rule holds for generated inputs | `vitest run specs//properties` | +| `behaviour-unchanged` | medium | recorded output is byte-for-byte the same | `vitest run specs//golden` | +| `contract` | medium | both sides agree on the wire | `vitest run tests/contract` | +| `mutation` | large | the tests notice a changed line | Stryker on the changed source files | +| `visual` | large | layout, overflow and accessibility hold | `playwright test tests/e2e` | +| `eval` | large | model behaviour and its token cost hold | `tsx scripts/eval.ts` | +| `gate` | large | the whole repo is green | `make check` | +| `ci` | large | the pull request is green | `gh pr checks --watch` | Sizes follow the Google idea of small, medium and large. A small check is fast and hermetic. The size is a field on every check. @@ -81,6 +81,7 @@ A prompt change is code, so it needs a test with a token ceiling. `api/src/agent ``` npx vitest run specs/25 specs/31 specs/34/properties ``` + The workbook PDF property lives in `shared/src/workbook-pdf.test.ts` and needs Playwright Chromium installed. 4. Run the structural check on the lock hook: `npx vitest run tests/structure/review-and-test-lock.test.ts`.