Review Work

Review Work

The standing review pass for this project, run after a piece of work is built and before the user reviews it by hand.

What it looks for, every time: strict adherence to CLAUDE.md and CONVENTIONS.md; no broken logic; no dead ends or invalid paths; no regressions; no historical artefacts or meta-information left in the prose. Changes are additive or leave the surface better — never worse, never quietly narrowed.

Findings are found by sub-agents, verified by you, and fixed by you. The user reads the chat reply and nothing else.

Step 1: Establish the scope

Three things, before any agent is dispatched.

The diff. If a stack is open, gh pr list --state open for its shape, then diff each PR against its own base — diffing a mid-stack branch against main pulls in the layers below it and reviews them again. Otherwise git log main..HEAD --oneline and git diff main...HEAD --stat.

Include what is not in a PR yet: git status for uncommitted work and git log @{u}..HEAD for unpushed commits. Work built this session is routinely still sitting in the tree, and it is the part most worth reviewing.

Name the files under review explicitly — a review with an unstated scope reports on whatever the agents happened to open.

The intent. The design/ document or ideas/ entry this work came from, plus what was agreed in this session. The review tests the work against what was agreed, not against what reads plausibly. "Everything's implemented as we discussed" is one of the checks, and it cannot be run without the source.

The review state. Which layers has the user already reviewed and signed off? Which are still unread? Is a review live on a PR right now? This decides Step 2 and cannot be guessed from the diff. If the session does not say, ask.

→ Proceed to Step 2.

Step 2: Decide where fixes land

If the user's request already stated it ("new pull requests on top of the stack", "add the fixes to the PR"), follow it and skip to Step 3.

Otherwise route by review state — three regimes:

A review is live on PR N right now. Fixes land as commits on PR N's branch. Never a side PR — it fragments the conversation the user is having about that diff. A live review is also a hard stop on all other work until it finishes.

The layers are already reviewed and signed off. They are frozen. Fixes go in a new branch and PR appended to the top of the stack, one PR per coherent change. Never amend a delivered PR and never cascade-rebase it — that destroys the before/after boundary the stack exists to preserve.

Nothing has been reviewed yet. Fold each fix into the PR that owns the code it corrects, so the user reviews the whole thing once instead of finding a defect in PR 1 that a later PR already fixed. A fix folded into a layer below the top leaves every layer above it behind — the children need restacking onto the moved branch, through the pr-stacked skill, before the stack is coherent again. Say so in the report.

Mixed. A fix belongs to the layer it corrects: into that PR while its layer is unreviewed, onto a new top-of-stack PR once the layer is frozen or the finding is unrelated to any unreviewed layer.

If the state is genuinely unclear, ask with AskUserQuestion before dispatching — the answer changes where every fix goes, and moving them afterwards is expensive. Offer the regime you believe applies first, labelled as recommended, and say which layers you think are signed off.

Stack operations — creating, appending, linking, restacking — go through the pr-stacked skill. Load it rather than hand-rolling the git and gh calls; a mislinked child branch is lost silently.

→ Proceed to Step 3.

Step 3: Dispatch the finders

Read references/dimensions.md and pick the dimensions the diff can actually violate. Dispatch one workflow-reviewer agent per dimension, concurrently, in a single message.

Scope finders to dimensions, never to files — a per-file split makes every agent responsible for the same shallow read, and none responsible for the seam between two files, which is where this project's defects live.

Each agent's prompt carries: the dimension and its checks, the file and commit scope from Step 1, the design source, and the instruction to assume more defects exist and search farthest from whatever it has already found. The agent declares its own model — override it only deliberately, never as a habit, and never downward on a dimension carrying judgement.

Scale the fleet to the blast radius. A one-file prose fix warrants two dimensions; a change touching the engine, the prose, and the tests warrants the full set and a second pass afterwards from a fresh angle.

Never invoke a workflow-* skill and never use the Workflow tool — this project authors the workflow system, it does not run it.

→ Proceed to Step 4.

Step 4: Verify every finding yourself

An agent's finding is a claim, not a fact. Open the file and confirm it before it becomes a fix. Finders routinely misread a conditional, report a line the diff never touched, cite a path that does not exist, or invent a convention the project does not hold. Applying an unverified finding is how a review pass manufactures the regression it was run to catch.

Sort what survives:

• Fix in place — unambiguous, and the correct shape is not in question.
• Bring to the user — anything ambiguous, anything that changes agreed behaviour, anything where two defensible fixes exist, and any convention the project has not already settled. New conventions are agreed, never invented; when the project already has one that covers the case, follow it rather than inventing a variant.

Findings you checked and dismissed are process exhaust. Drop them entirely — they do not reach the report.

Then read the coverage the agents reported back. A dimension that returned nothing was unexamined as often as it was clean, and this is worth saying out loud rather than converting into a clean bill of health.

Present the bring-to-user findings now, before applying anything — one at a time, conversationally. Each finding is its own exchange: the problem and enough context to weigh it, why it exists, your recommendation with its reasoning, the defensible alternative and why you would not take it — then stop and wait.

Ground every finding from zero. Days may separate the user from the design discussion, and none of the session's vocabulary, codenames, or mid-build decisions can be assumed remembered. Each presentation re-establishes the world before naming the defect: what the touched mechanism is and does, in plain words; where this finding sits in it; only then what is wrong. Show, don't describe — render the actual artefact (the menu, the output, the prose) bracketed in ▼/▲ markers, as it is and as each option would make it; where the problem is structural — flows, callers, orderings — draw a compact ASCII diagram built from the real edges. The bar is a reader returning after a week who follows it cold; a presentation the user has to ask to have re-explained has failed, and the re-explanation starts from zero, not from a compressed restatement of the last one. The next finding is not presented until the current one is settled; the discussion of one routinely reshapes the questions behind it, and answers given against a wall of parallel questions are answers to summaries, not to problems.

AskUserQuestion serves the narrow remainder: a small, simple call whose options are fully graspable from the question text alone, with nothing to discuss. Never use it to batch several decisions into one dialog, and never for a finding whose context the user needs explained — that is a conversation, not a form.

An answer can change the routing or the shape of the fixes you thought were settled: a "restore it" turns a prose correction into a revert, a "do it properly" turns a patch into a new layer. Resynthesise the whole fix set against the answers before touching a file. Only when nothing needs the user — every surviving finding unambiguous — does the pass proceed straight to Step 5.

→ Proceed to Step 5.

Step 5: Apply the fixes

A fix is held to the same bar the review ran. Verifying that a finding is real says nothing about the fix a finder proposed alongside it — finders invent guards, defensive checks, and conventions the project does not hold. Before authoring any fix, synthesise the finding into the shape this project already uses (engine-owned state is trusted, never re-checked in prose; patterns come from CONVENTIONS.md and two in-repo siblings, never from the finder's suggestion), and re-check the authored fix against the same dimensions the review dispatched — conventions, historical artefacts, routing. A fix that fails that check is itself a finding: drop it or bring it to the user. Applying reviewer prose verbatim is how a review pass becomes a circle of fixes.

Prose flows are dense conditional graphs with no compiler, and a locally correct edit collides with a return path three sections away.

Before editing any skill or reference section, grep its inbound edges (Proceed to **X, Return to **X, Load, and every mention of its filename) and its outbound edges. After editing, re-walk each inbound path literally against the new text.

Any fix claiming "apply this everywhere" ships with the enumeration of everywhere written into the commit message, each cell checked.

Verify each fix before committing it. Never batch a pile of fixes and discover their interactions a PR later.

Then run the gates the change touches — npm test, npm run test:cli, npm run test:migrations, npm run typecheck — and update the pipeline simulation if the change moved an engine verb, a prose call sequence, a phase ordering, or a manifest field. Where a finding describes a failure the tests would not have caught, add the case that would have caught it.

Commit and push without being asked — but never onto main, and never onto whatever branch happens to be checked out. Where the fix lands was settled in Step 2; when that is a new layer, the branch is created before the first commit, not after it.

If the diff touched skill prose, run node tests/prose/run.cjs select --diff main and suggest the intersecting cases in the report — never run the walks as part of this pass.

→ Proceed to Step 6.

Step 6: Report

In the chat reply. Not Bash output, not a design document, and not a PR body — PR bodies still earn the full reasoning for the repo's record, but the user does not read them, so nothing they must act on can live there.

• Open questions first, before anything else — named in one line each, so the user sees the shape of what is owed, then walked as Step 4 prescribes: the first question's full presentation ends the report's turn, and the rest follow one at a time as each settles. This is what the user asked to be brought to them, and it is the only part of the report that needs an answer.
• What was fixed — one line each, and which PR or branch it landed on.
• What was examined and what was not — the dimensions run, and the ones the fleet did not reach.
• Gate results, quoted. If a gate was not run, that is the headline of the report, not a footnote after it.

Never report the pass as proof that the work is clean. It is a sample — sub-agents satisfice, circling one cluster of findings instead of continuing to search, so an empty report means the net missed, not that the water is empty.

Rules

• Finders never fix. They are read-only by definition; every edit is made by you, after verification, in Step 5.
• Never widen the work. A review pass fixes what it finds and stops. Refactors, cleanups, and improvements noticed along the way are reported, not performed.
• Never move a delivered PR. Amending or force-pushing a layer the user has already read erases the boundary between what they reviewed and what they did not.
• The user's approval of an earlier layer is not approval of a fix landing on top of it. Each new PR is its own review.