# Review Work

> Adversarially review work just built on this project — strict adherence to CLAUDE.md and CONVENTIONS.md, broken logic, dead ends, invalid paths, regressions, and historical artefacts left in the prose. Dispatches finder agents, verifies what they return, lands fixes as new stack layers or into the PR still under review. Use whenever the user asks to review the work, do a review pass, check for regressions or convention adherence, or dispatch review sub-agents.

- Skill: `leeovery/review-work` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add leeovery/review-work`
- Raw SKILL.md: https://api.skillmd.com/api/skills/leeovery/review-work/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: leeovery (https://skillmd.com/u/leeovery)
- Updated: 2026-09-10
- Page: https://skillmd.com/skills/leeovery/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](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.