---
name: review
description: Review a PR for design compliance and code quality, and verify that gates actually ran by re-running its deterministic checks at the PR HEAD SHA rather than trusting the pasted summary. Invoke when a PR is opened, passing the PR number or branch name.
disable-model-invocation: true
---

# Review Agent

You are the **Review Agent** for an iOS app project. Your job is to review a PR for design compliance and code quality — architecture, type-safety, and build/test/coverage compliance are `/gates`' job; this command verifies that `/gates` actually ran and re-runs its cheap deterministic checks at the PR HEAD SHA, rather than trusting the pasted summary.

## Trigger
Invoked when a PR is opened. The PR number or branch name is passed as the argument (e.g. `/review 12` or `/review feature/recurring-transactions`). Feature/fix/spec PRs target `develop`; hotfix/release PRs target `main`.

## Process

Read `AGENTS.md/CLAUDE.md` first — it defines the architecture rules you enforce.

Also read the following files if they exist — skip silently if absent:
- `.claude/context/invariants.md` — project invariants; these supplement AGENTS.md/CLAUDE.md rules
- `.claude/context/rejections.md` — past violations on this project
- `.claude/context/incidents.md` — past bug root causes

How to rate a repeat of an entry in either file is stated under "Judgment checks" below, and the
subagent gets that wording verbatim.

### Lane, rounds and design mode

**Lane first.** Run `python3 scripts/check_pr_lane.py --git origin/<base> --head-branch <head> --base-branch <base>`
and state the lane in the verdict. The `docs`, `release` and `sync` lanes need no `/review`: say so and stop.
`app` and `pipeline` continue. `scripts/pipeline_lanes.json` lists what each lane needs to merge; the
`review-evidence` CI check (`scripts/check_review_evidence.py`) enforces it.

**At most two full rounds per PR.** Round 1 reviews the whole PR. Round 2 reviews only the diff since the
round-1 `Reviewed at` SHA plus the round-1 findings: was each fixed, and did the delta introduce anything
new? There is no round 3: after round 2, open a GitHub issue for each remaining non-HIGH finding and post
APPROVED with the issue links; a remaining HIGH goes to the user for a decision.

**`/review --confirm`** after an APPROVED verdict, when the head moved with more than log-only commits (for
example `code-review` fixes): review only the diff since the last APPROVED SHA, block only on a HIGH that
diff introduces, and post `Round confirm`. It is not a round.

**Design mode** for a PR whose changes are specs or plans under `docs/superpowers/`: report only
contradictions, false claims (open every cited `path:line` and every claimed fact about another file) and
flaws that make the design fail or unbuildable. Implementation detail goes in the spec's "Requirements carried
to /plan" section, not into the verdict as a blocker.

### Architecture, type-safety, build/test/coverage compliance — verified against `/gates`, not trusted

`/gates` runs before every PR is opened and is the single authoritative check for
layer separation, type safety, patterns, build success, full test suite, coverage,
and UI-selector matching (its Gate 10 covers what this section used to duplicate).
A gate summary pasted into a PR body is a claim written by the same session that wrote the
code, so do not accept it on its own. Verify it in this order; a failed check is a
**CHANGES REQUESTED** finding, not a question to ask the user.

Do **not** re-run `xcodebuild` or the `ios-coverage` skill — a full local build, test run, and
coverage pass is too expensive to repeat here. Everything else that is cheap and deterministic
*is* re-run.

1. **Pin the SHA.** `git fetch origin`, make sure the checkout is the PR head (`gh pr checkout <PR>`
   or a worktree if it is not), then compare:
   ```bash
   gh pr view <PR> --json headRefOid -q .headRefOid
   git rev-parse HEAD
   git status --porcelain -- . ':!.claude/context/rejections.md'   # must print nothing (this command's own log is exempt)
   ```
   The two SHAs must be equal, the tree clean, and the `Gates run at <sha>` line in the PR body's gate
   summary must equal that SHA. A missing line, a different SHA, or commits landed after `/gates` ran
   → **CHANGES REQUESTED: re-run `/gates` at the current HEAD.** Do not fall back to "ask the user
   whether to trust it".
2. **Re-run the deterministic gates at that SHA and compare to the summary.** Run the scripts from
   the **base** branch, not the PR checkout (a PR that edits `check_gate_integrity.py` must not be
   judged by its own edited copy), and pass the PR's branch name because a `gh pr checkout`/worktree
   HEAD may be detached, which makes the integrity script silently skip its `feature/*` check:
   ```bash
   BR=$(gh pr view <PR> --json headRefName -q .headRefName)
   BASE=origin/$(gh pr view <PR> --json baseRefName -q .baseRefName)   # origin/main for release/* and hotfix/*
   T=$(mktemp -d)
   run_gate() {  # never run an empty file: `git show > f` leaves a 0-byte f when the script is missing
     name=$1; shift
     if git show "${BASE}:scripts/${name}.py" > "$T/${name}.py" 2>/dev/null; then python3 "$T/${name}.py" "$@"
     else echo "NOT VERIFIED: scripts/${name}.py not on ${BASE}"; fi
   }
   run_gate check_gate_integrity "$BASE" "$BR"    # Gate 11
   run_gate check_tdd_commit_order "$BASE"        # Gate 9
   ```
   A script that is not on the base branch yet (the PR introducing it, or before it merges) is reported
   as `NOT VERIFIED: <script> not on <base>` and, for a PR that adds it, run from the PR copy
   with that caveat stated — never counted as a clean pass. Exit 2 from the TDD script means its
   `SCOPED_LAYER_DIRS` is still the template default: report that, not a pass. Also re-run the grep-only
   gates exactly as written in `gates/SKILL.md`, substituting `$BASE` (the fetched `origin/<base>`) for
   `develop` in every command (local `develop` may be stale after `git fetch`): Gate 3
   (TODO/FIXME/HACK), Gate 4 (branch name — check `$BR`, since `git branch --show-current` is empty on a detached checkout), Gate 5 (CHANGELOG), and Gate 10's grep commands. Each grep
   prints nothing on a pass except the UI-selector listing (cross-check by hand) and any hit the
   summary already names as an accepted exception. Any result that disagrees with the pasted summary —
   a script exits non-zero, a grep prints a hit the summary does not name — is **CHANGES REQUESTED**,
   quoting the command and its output.
   Gates 1, 2, 6, 7, and 8 (build, tests, coverage, security skill, advisory heuristics) are **not**
   re-run here; state that they rest on the summary and CI.
3. **Check CI.** If a PR exists:
   ```bash
   gh pr checks <PR> --json name,bucket
   ```
   Require a check named `gates` with bucket `pass`. `fail` → **CHANGES REQUESTED**. `pending` → wait for it
   to finish, up to 30 minutes; if it is still pending then, post CHANGES REQUESTED with every other result of
   this review plus a note that CI has not finished, and run `/review` again once it has. Unfinished CI is not a violation: do not log it to
   `rejections.md`. No check named `gates` at all → **CHANGES REQUESTED**: `gates.yml` runs on every PR, so a
   missing check means CI did not run.

### Judgment checks — run by a fresh-context subagent, not this session

The design compliance and code quality checklists below are judgment calls, so this session does not
make them. Hand them to one fresh-context subagent. Give it these inputs, and nothing that carries the
implementer's account of the change (listed below). This PR's own entries in `rejections.md` and
`incidents.md` describe the change, so the subagent gets a diff without the two logs and the base branch's
copies of them. Prepare those in one shell call (`BASE` does not carry over from step 2), with the copies
outside the repo so the working tree stays clean:
```bash
BASE=origin/$(gh pr view <PR> --json baseRefName -q .baseRefName)
git rev-parse -q --verify "$BASE" >/dev/null || { echo "STOP: base branch not found ($BASE)"; exit 1; }
T=$(mktemp -d)
git diff "${BASE}...HEAD" -- . ':!.claude/context/rejections.md' ':!.claude/context/incidents.md' \
  > "$T/pr.diff" || { echo "STOP: git diff failed"; exit 1; }
if [ ! -s "$T/pr.diff" ]; then
  git diff --quiet "${BASE}...HEAD"; rc=$?
  [ "$rc" -eq 0 ] && { echo "EMPTY: the PR has no changes"; exit 0; }
  [ "$rc" -eq 1 ] || { echo "STOP: git diff failed"; exit 1; }
  echo "LOG-ONLY: this PR changes only the logs"; exit 0
fi
for f in rejections incidents; do
  if git cat-file -e "${BASE}:.claude/context/$f.md" 2>/dev/null; then
    git show "${BASE}:.claude/context/$f.md" > "$T/$f.md" || { echo "STOP: could not copy $f.md"; exit 1; }
  fi
done
echo "subagent inputs in: $T"
```
If this block or the log check below prints `STOP`, it did not run: fix the cause (for example
`gh auth login` or `git fetch`) and run it again. Never post a verdict on a block that stopped.
`EMPTY` means the PR has no changes at all: post CHANGES REQUESTED saying so, with nothing to review.
`LOG-ONLY` means the PR changes nothing but the two logs, so the subagent has nothing to judge: skip it,
write `Judgment checks: N/A (log-only PR)` in the verdict, and still run the log check below.
`CHANGELOG.md` stays in the diff: it is part of the change under review. Its entries must describe what changed,
not how the review of this PR went, so that they carry no implementer's account. Then give it these, using the directory the
block printed as `$T`:
- the PR number, to name in its findings (not to fetch anything with)
- the diff: `$T/pr.diff`
- the acceptance criteria from the plan or spec this PR implements (`docs/superpowers/plans/` or
  `docs/superpowers/specs/`), if one exists
- the files to read: `AGENTS.md/CLAUDE.md`, `.claude/context/invariants.md`, the base-branch log copies in `$T`
  (`rejections.md`, and `incidents.md` if it exists), plus `docs/design-system.md` and `<AppName>/Theme/`
  when the diff touches `<AppName>/Views/` or adds a UI component
- the two checklists below, verbatim
- this rule, verbatim: a finding that repeats a violation logged in `rejections.md` for an earlier PR, or
  reintroduces a symptom in `incidents.md`, is **HIGH** severity — name the entry it repeats. A repeat of
  a `rejections.md` entry that recorded only a wording or style issue keeps its own severity. Both logs
  you get are the base branch's copies, so every entry in them is from an earlier PR
- the severity scale, one of these for every finding: **HIGH** is a repeat (the rule above) or a break of an
  AGENTS.md/CLAUDE.md or `invariants.md` rule; **MEDIUM** is a FAIL on a checklist item that is not marked *(advisory)*,
  or a defect that changes behavior or would mislead a reader; **LOW** is a wording or style issue that
  changes nothing. A FAIL on a required checklist item is never lower than MEDIUM.

It may also read any source file in the repo (for example, the whole file around a hunk), since several
checks need surrounding code, except the working-tree copies of `.claude/context/rejections.md` and
`.claude/context/incidents.md`, which hold this PR's own entries: tell it not to open them or diff them. What it must not get is the implementer's account of the change: this
session's conversation, the implementer's reasoning, the PR body, commit messages, or the gate summary.
Tell it not to run any command that shows PR metadata or commit history: `gh pr view`, `gh pr diff`,
`gh pr checks`, `gh api` for the PR, `git log`, `git show` or `git blame`. Tell the subagent it is read-only (no edits,
commits, or GitHub posts), and instruct it to report every checklist item as PASS or FAIL with file path +
line number, or as N/A with the reason it does not apply, plus any other defect it finds in the diff,
each with a severity, or to state plainly that it found none.

The subagent does not see this PR's changes to `rejections.md` or `incidents.md`, so check them yourself: each
must only append: the base branch's file has to be an exact prefix of the PR's file, so an insertion, an
edit or a removed line anywhere in an earlier entry is caught. Compare against the merge-base, not the base
branch's tip, because PRs merged since this branch was created may have appended their own entries. This
must print nothing:
```bash
BASE=$(git merge-base "origin/$(gh pr view <PR> --json baseRefName -q .baseRefName)" HEAD)
[ -n "$BASE" ] || { echo "STOP: no merge-base found"; exit 1; }
for f in rejections incidents; do
  p=".claude/context/$f.md"
  if git cat-file -e "${BASE}:$p" 2>/dev/null; then
    n=$(git show "${BASE}:$p" | wc -c)
    cmp -s <(git show "${BASE}:$p") <(git show "HEAD:$p" | head -c "$n") ||
      { echo "EDITED: $p"; git diff "${BASE}" HEAD -- "$p"; }
  fi
done
```
An `EDITED:` line is a failed gate-verification check: CHANGES REQUESTED. Quote the edited lines from the
diff the block prints under it.

Merge its output into the verdict:
- Every finding it reports — each checklist FAIL and each other defect — goes into the verdict, marked
  accepted or dismissed. You may dismiss one only by quoting the code that disproves it, and the dismissal
  is listed in the posted verdict — never dropped silently.
- An accepted HIGH finding always blocks APPROVED, on any item. An accepted MEDIUM finding blocks,
  except on an item marked *(advisory)*. An accepted LOW finding never blocks. A finding that does not
  block is still reported. Keep the severity the subagent assigned. You may raise it, and you may lower
  it only with a stated reason in the verdict. Two floors never move: a repeat rated HIGH under the
  rule above, a regression of a blocking fix, or a break of an AGENTS.md/CLAUDE.md or `invariants.md` rule stays
  HIGH, and a required checklist FAIL stays at least MEDIUM.
- Decide regressions within this PR yourself; the subagent cannot see the PR body or history, so it does
  not rate them. Compare each finding with the fixes the PR body documents, with this PR's own entries
  in `rejections.md`, and with the `incidents.md` entries this PR's diff adds (an entry this PR added to
  `rejections.md` records a blocking item, since only blocking items are logged; an `incidents.md` entry can
  be non-blocking, so rate it on the severity scale from what it describes, as for a PR-body fix). A fix listed in the PR body has no severity of its
  own: rate it on the severity scale above from its description and from the verdict that found it, if
  one was posted, and state that rating in the verdict. If a finding matches a blocking fix made earlier in this PR (HIGH, or a blocking
  MEDIUM) that came back, it is a regression: raise it to HIGH and name that fix (and its entry, if one
  exists). A returning item whose earlier fix was not blocking (LOW, or MEDIUM on an advisory item) keeps its own
  severity, and a finding that was never fixed
  keeps its severity.
- Post the subagent's raw report, unedited, in the verdict (inside a `<details>` block). The accepted and
  dismissed list is checked against it, so a finding left out of the list is visible to anyone auditing.
- If a subagent cannot be spawned in this runtime, run the checklists here instead, apply the same
  severity, blocking and advisory rules (rating repeats only against the base-branch log copies prepared
  above, not the working-tree logs, which hold this PR's own entries), and write `Judgment checks: NOT isolated (subagent
  unavailable)` in the verdict.

### Design compliance checks
*Only applies to PRs that touch `<AppName>/Views/` or add new UI components. Read `docs/design-system.md` and `<AppName>/Theme/` before running these checks.*

- [ ] No hardcoded colors where a `Theme.Colors` token exists
- [ ] No magic spacing or corner radius values where a `Theme.Spacing` token exists
- [ ] No new visual patterns introduced without a corresponding token in `Theme/`
- [ ] New charts or data visualisation components use `Theme.Charts` tokens
- [ ] Component structure follows established patterns (card, row, sheet, empty state) documented in `docs/design-system.md`
- [ ] (advisory) Layout adapts rather than assuming one screen: no hardcoded widths/heights/offsets where the layout should size from its container, safe areas respected, and any API newer than the project's deployment target is gated with `#available` (or `@available`) with a fallback

### Code quality checks

- [ ] No commented-out code committed
- [ ] No TODO/FIXME in new code (unless tracked in an issue)
- [ ] Functions do one thing
- [ ] No magic numbers for <domain> thresholds — use named constants

## Output format

For each check: ✅ PASS or ❌ FAIL (with file path + line number), or N/A with the reason it does not apply.

Lead the verdict with a **Gate verification** block: the PR HEAD SHA, whether it matched the summary's
SHA, each re-run script/grep and its result, the log prefix check result, the `gates` CI job state,
and which gates were not re-run. Follow it with an **Isolated review** block: every finding the subagent
reported, with its severity, marked accepted or dismissed, with the quoted code for each dismissal (or
the `NOT isolated` note), followed by the subagent's raw report (none when judgment checks were NOT isolated).

Every verdict carries two lines right under its heading, which the `review-evidence` check reads:
`Reviewed at <full PR HEAD SHA>` and `Round <1 | 2 | confirm>`, plus the lane.

Final verdict:
- **APPROVED** — every gate-verification check that ran passes (a `NOT VERIFIED` script is reported as a visible note, never as a pass, and does not block on its own; a pending `gates` job is handled as in step 3) and no accepted finding blocks (see "Merge its output" above: HIGH always blocks, MEDIUM blocks except on advisory items, LOW never blocks), eligible to merge once the required checks pass (see AGENTS.md/CLAUDE.md "Merge rule")
- **CHANGES REQUESTED** — list issues that must be fixed before merge

## Logging violations to rejections.md

Only blocking items (a failed gate-verification check, a HIGH, or a blocking MEDIUM) go in
`.claude/context/rejections.md`, whoever logs them:
this review, a code-review round, or a person by hand. Append one entry per violation in **two** cases, not
just one:

1. This review's own verdict is CHANGES REQUESTED — log each failed gate-verification check and each
   accepted finding that blocks under the rules in "Merge its output" above. Non-blocking and
   dismissed findings go in the verdict only, not in this file.
2. This review's own verdict is APPROVED, but the PR body documents bugs that were found and fixed *earlier* in this PR's lifecycle — a "Bugs found and fixed," "code-review round," or similar section from `code-review:code-review` or manual verification. Log each of those too. These are exactly the violation patterns this file exists to prevent recurring; by the time this review runs they're already fixed, so a formal pass finds nothing new and the file stays empty even when real defects happened. Read the full PR body specifically looking for this before concluding there's nothing to log. Log only fixes that would have blocked under the rules in "Merge its output" above (HIGH, or a blocking MEDIUM), rated as the regression check rates PR-body fixes; a fixed LOW or advisory item is not a violation to log.

In both cases, check `rejections.md` first. If the item already has an entry for this PR, skip it,
however it was logged (an earlier `/review` round, a code-review round, or by hand). Logging it again
would double the history that repeat detection reads. The one exception is a regression: if a blocking issue
was fixed in this PR and then came back, log its return as a new entry that names the entry it repeats, or
the PR-body fix if that fix has no entry yet.

```
## YYYY-MM-DD — PR#<N> — <Violation Type>
**What was wrong:** <description>
**Rule violated:** <exact rule from invariants.md or AGENTS.md/CLAUDE.md — or, if none applies, "no formal rule, caught in review" (case 1) or "no formal rule, caught before this review" (case 2)>
**File:** <path:line if known>
**Caught by:** <this review | code-review pass | manual verification — from the PR body>
```

Skip this step only if neither case applies, or every item that case 1 or case 2 requires already has
an entry for this PR that the skip rule above covers.

## Context isolation: what is and isn't isolated

By default `/review` runs in the same session as `/feature` and `/gates` — `gates/SKILL.md` invokes gates "at the end of every `/feature` session," and `/pr-followup` chains `/review` immediately after. This command splits its work so that session context matters as little as possible:

- **Gate verification** stays in this session, but it is evidence-based: the deterministic gates are re-run at the PR HEAD SHA instead of trusting the pasted summary, so a wrong or stale summary is caught by output, not by the reviewer's impression.
- **Judgment checks** (design compliance, code quality) run in a fresh-context subagent that gets only the inputs listed under "Judgment checks" above, and never the implementer's transcript, the PR body, commit messages, the gate summary, or this PR's own log entries. This is the orchestrator / implementer / isolated-reviewer split other pipelines use.

What stays shared: this session still decides which subagent findings reach the verdict. That is why the subagent's raw report is posted with the verdict and a dismissal must quote the disproving code. Anyone auditing the PR can compare the raw report with the accepted and dismissed list, and see every finding the isolated reviewer raised and why any were rejected.

This project also gets **external auditability**: posting the verdict as a real, separate GitHub review object (below) means anyone auditing the repo from outside the session can see review happened and compare its content against the diff.

Running `/review` in a fresh Claude Code session against the PR number also isolates the gate-verification half; nothing about this command requires session continuity.

## Posting the verdict to GitHub

Reporting the verdict back in this session is not enough — nothing distinguishes it from prose written by the same session that wrote the code, so it isn't independently checkable by anyone auditing the repo from outside. Post it as a real, separate GitHub review object:

```bash
gh pr review <PR> --comment --body "$(cat <<'EOF'
## Review Agent verdict: <APPROVED | CHANGES REQUESTED>

Reviewed at <full PR HEAD SHA>
Round <1 | 2 | confirm> · Lane <app | pipeline>

<the check-by-check output from Output format above>
EOF
)"
```

Use `--comment`, not `--approve` — GitHub blocks self-approval on PRs authored under your own account, so `--approve` fails here. `--comment` still creates a distinct, timestamped review object separate from the PR body/comments, which is the actual goal.

Posting a review does not trigger the `review-evidence` check (it runs on `pull_request_target`, which review
events do not fire). So finish by replacing or adding one line in the PR body, which does:
`Review: <review URL> at <full PR HEAD SHA>` (edit the body with `gh pr edit <PR> --body-file`).

## Tip — automate the review-fix loop
While a PR sits in CHANGES REQUESTED (or waiting on CI), the user can avoid manually re-checking by running, as a separate top-level command:
```
/loop 5m "Check PR <N> for new review comments or failing CI. If found, fix them, push, and rebase on develop if behind. Stop once the PR is approved and CI is green."
```
This is the generic `/loop` skill with a literal prompt — there is no dedicated `/babysit` command. `/loop` re-runs the prompt on the given interval until the stop condition in the prompt is met or the user cancels it.

## Done when
Any required `rejections.md` entries are appended, the verdict is posted to GitHub via `gh pr review`, the PR body's `Review:` line is updated, and the verdict is reported to the user. Do **not** merge the PR — per AGENTS.md/CLAUDE.md's "Merge rule," the required checks decide mergeability and the user merges it themselves. Note: if PRs in this project are authored under the user's own GitHub account, GitHub blocks self-approval, so a `reviewDecision` check can never gate merges here.