test-review

name	test-review
layer	stage
description	Propose the test(s) FIRST and get a human to sign off on the behaviour before writing the code — the test analog of /schema-review and /ui-proposal. For hand-written LOGIC in packages/* (apps opt-in, pocs exempt). Renders the proposed failing test as a reviewable contract, holds for lgtm/approve, then you write the code to make it green. Use when an agent is about to implement packages/* logic, or when asked to "write the test first", "TDD this", "agree on the test", run /test-review.
allowed-tools	Read, Write, Edit, Bash, Glob, Grep

Test Review — agree on the test before you write the code

Turns "I'm about to write some logic" into a reviewable contract: the proposed failing test(s) — the cases being asserted, in plain language plus the actual test code — so a human can sign off on the behaviour before any implementation exists. The agreed test is the definition of done: "done" = that test passes.

This is the third sign-off gate (epic #774), alongside schema-review (#314, the data model) and ui-proposal (#307, the look). Pick the gate by what the change is: a data model → schema gate; how it looks → UI gate; hand-written logic → here. It reuses the same revision/approval loop and signal as the other two (#310/#572).

Why test-first? The test is cheap to change while it's just a proposal and expensive to change after the code is built around the wrong assumption. Agreeing on the cases first ("no double-booking; adjacent slots OK; buffer respected") means the code has one job — go green — instead of being a moving target.

Step 0 — Self-scope (skip when out of scope). This gate is deliberately narrow.

Decide by where the code currently lives (#779), then by what kind of change it is. Skip loudly (one line saying why + which gate, if any, fits instead) rather than gating everything.

By location — resolved from the stage model. Run node scripts/harness-stages.mjs <path> (or gateMode(path, 'test-first')) for the verdict (on / opt-in / off) rather than matching the folder by hand; the canonical source is harness.config.mjs (epic #952). The table below is the default profile rendered for humans:

Current home	Stage	Action
packages/*	package	Proceed (default — what we maintain; every consuming app inherits its correctness)
apps/*	app	Skip unless the app has explicitly opted in (may be another user's app — not ours to impose)
pocs/*	poc	Skip — the incubator must stay fast and safe-to-fail; a POC graduating to packages/* is the checkpoint to backfill its tests

By change kind (only once location is in scope):

• Collection schema / data model → skip, use /schema-review (+ the e2e fixture smoke covers generated CRUD).
• .vue component / layout / theme — how it looks → skip, use /ui-proposal.
• Deterministic generated code (CLI output, templated CRUD) → skip, the e2e smoke harness owns it.
• Hand-written logic — a composable rule, server util, permission check, calculation, layout-engine transform, a generator's own logic → proceed.

Rule of thumb: packages/* logic almost always proceeds; everything else almost always skips. When unsure whether something is "logic", ask: can a human just eyeball it for correctness? If yes, it probably doesn't need a test-first contract.

Step 1 — Pin down the behaviour

Before writing any test, state — in one or two plain sentences — what the logic must do, and enumerate the edge cases that matter. This list IS the thing being agreed on; the test code is just its executable form. Example (a slot-availability rule):

• a booking that overlaps an existing one is rejected
• two back-to-back bookings (end == next start) are both allowed
• a booking inside another's buffer window is rejected
• times are compared in the resource's timezone, not the server's

Step 2 — Write the proposed failing test(s)

Author the test next to the code it will cover, following the package's existing test layout (*.test.ts beside the source, or its __tests__/ dir — match the neighbours; see packages/crouton-core for examples). Vitest is the runner (pnpm test).

• One describe per unit; one it per edge case from Step 1, named in plain language.
• The test must fail first — it asserts against code that doesn't exist yet (or asserts the new behaviour the current code lacks). Red before green is the point; surface that.
• Keep it a general contract (all valid inputs), never assertions reverse-engineered to pass a specific implementation.

Step 3 — Hand off for sign-off (review happens on the test itself)

The proposed test is the review surface. Present both the plain-language case list (Step 1) and the test code, so the reviewer agrees on behaviour, not syntax.

• Interactive session: show the case list + test in chat and hold. The reviewer says lgtm/approve to proceed, or asks for changes (add a case, fix an expectation) → revise and re-show.
• Agent pipeline / PR: commit the failing test (via /commit, scope = the package, e.g. test(crouton-core): …) so it lands in the PR's "Files changed". The reviewer comments inline on any it(...) to change a case. Set the issue to status:blocked while holding. Approval is a comment containing lgtm/approve — not a reaction or label (#572) — and it unblocks implementation.

Iterate Step 1–3 until approved. Do not write implementation code before the test is agreed.

Step 4 — Go green (only after approval)

Write the implementation to make the agreed test pass — and only that. Run pnpm test (and pnpm typecheck) until green. The agreed test is now the regression guard; it stays in the diff and ships with the code. Resume the normal workflow (typecheck → update issue → /commit).

Conventions

• The case list (Step 1) is the contract; keep the test code honest to it — never quietly drop or weaken a case to make implementation easier (that's a re-negotiation → go back to Step 3).
• One concern per test file, mirroring the source it covers.
• Red first, then green. If the proposed test passes before you write any code, the behaviour already exists — say so and skip (nothing to build).
• This is about order (test before code), not enforcement — the CI test job already hard-gates pnpm test.