Review

---
name: review
description: Senior-level review against the PR. Per SPEC-019, delegates correctness + security passes to runtime /code-review and /security-review (primary path), or falls back to the kit's degraded reviewer agent (degraded path) when those runtime skills are not exposed. Layers kit-distinctive checks (over-engineering minimality axis, Figma 3.5-3.10, ui-reviewer, design-auditor, a11y-auditor, tech-debt ledger, native-memory review-lessons learning) on top.
argument-hint: '[ISSUE-number]'
disable-model-invocation: true
allowed-tools: Task, SlashCommand, Read, Glob, Grep, Write, Edit, Bash(bash scripts/checkpoint.sh *), Bash(bash scripts/wt_setup.sh *), Bash(bash scripts/wt_cleanup.sh *), Bash(bash scripts/registry_edit.sh *), Bash(bash scripts/flock_edit.sh *), Bash(bash scripts/worktree.sh *), Bash(python3 scripts/*), Bash(git *), Bash(gh *), Bash(pytest *), Bash(npm *), Bash(bash ${CLAUDE_PLUGIN_ROOT}/scripts/*), Bash(python3 ${CLAUDE_PLUGIN_ROOT}/scripts/*)
---
<!-- AUTO-GENERATED by scripts/gen_skills.py — DO NOT EDIT. Edit SKILL.md.tmpl instead. -->

## Kit Preamble — review

### Kit Script Root
Kit root: `${CLAUDE_PLUGIN_ROOT}`
- Absolute path above → plugin install (substituted at load time; no project
  `scripts/` dir): prefix every kit script command with it, e.g.
  `bash <kit-root>/scripts/checkpoint.sh …`. Absolute paths also work from worktrees.
- Literal `${…}` placeholder above → standalone layout: run commands as written.

### Project Context Detection
Run these checks silently at the start. Use results to adapt behavior:
- `[ -f issues.md ]` — if true, this project uses the sprint system. Respect issue numbering and STATUS.md.
- `[ -f docs/sprint_state.md ]` — if true and Status shows `running`, a sprint is active. Be aware of parallel work in worktrees.
- `[ -f docs/prd_digest.md ]` — if true, read it for quick project context before starting.

### Kit Rules
- Verify `gh auth status` before any GitHub operation.

### Checkpoint Verification Pattern
Every phase has a checkpoint. Run the verification command and check the exit code.
- Exit non-zero (blocking gate): **STOP immediately**, report failure, do NOT proceed.
- Exit 0 with an `ADVISORY:` line (advisory gate): report the gap, self-correct, continue.
Standard prefix:
```
bash scripts/checkpoint.sh
```
Append `--skill <name> --phase <phase> --issue <ID>` for the specific check.
`checkpoint.sh` resolves the main repo root internally, so the command stays
a single prefix-matchable form (safe to allowlist as `Bash(bash scripts/checkpoint.sh *)`).

### Worktree Setup Pattern
Pipeline skills operate in git worktrees to isolate changes from main.
- Create + freeze: `WT="$(bash scripts/wt_setup.sh <branch>)"` — creates the
  worktree via `scripts/worktree.sh create` and writes `.claude-kit/freeze-dir.txt`
  inside it in a single step.
- Resolve main root: `bash scripts/worktree.sh root`
- Remove safely: `bash scripts/wt_cleanup.sh <branch>` — cd's to main root
  inside a subshell, then removes the worktree (never leaves CWD dangling).
All file operations happen inside `$WT/`. Shared files live on main only.

### Registry Update Pattern
Shared files (`issues.md`, `STATUS.md`, `CHANGELOG.md`) are managed on main only.
Always use `registry_edit.sh` for concurrent-safe writes — it resolves the
main repo root internally and delegates to `flock_edit.sh`:
```bash
bash scripts/registry_edit.sh issues.md -- bash -c '<update command>'
```
Never commit these files to feature branches.
Steps:
1) Find PR from issues.md (PR field) or `gh pr status`.
2) Checkout the PR branch in a worktree + auto-freeze in one step:
   ```bash
   BRANCH="$(gh pr view <pr_number> --json headRefName -q .headRefName)"
   WT="$(bash scripts/wt_setup.sh "$BRANCH")"
   ```
   `wt_setup.sh` creates the worktree and writes the freeze marker inside
   `.claude-kit/freeze-dir.txt` atomically. All subsequent file operations
   happen inside `$WT/`.

> **CHECKPOINT — ADVISORY (report & continue)**
> Run: `bash scripts/checkpoint.sh --skill review --phase checkout --issue $ARGUMENTS`
> Advisory: exits 0 even on failure, printing an `ADVISORY:` line — report the gap, self-correct, then continue.

3) Gather review context:
   **Read all applicable context files via parallel Read tool calls in a single message.**
   - **Recalled review lessons** — recurring patterns recorded by past reviews auto-recall from native memory into this session; apply them (the legacy `docs/review_lessons.md` is retired, ISSUE-033).
   - `docs/architecture.md` — if exists (for conformance checks)
   - For UI issues, also read in parallel: `docs/design_system.md` (or `design_system_mobile.md`), `docs/copy_guide.md`, `docs/interactions.md` (or `interactions_mobile.md`), `docs/wireframes.md` (or `wireframes_mobile.md`).

3.1) **Runtime probe** (per SPEC-019 platform-first delegation):
   ```bash
   python3 scripts/has_skill.py code-review;       CR_EXIT=$?
   python3 scripts/has_skill.py security-review;   SR_EXIT=$?
   ```
   Branch per dimension:
   - `CR_EXIT` in {0, 2} → primary path for code dimension (invoke runtime `/code-review`).
   - `CR_EXIT` == 1 → degraded path for code dimension (invoke `reviewer` agent with `--dimension code`).
   - Same for `SR_EXIT` with `/security-review` / `--dimension security`.

   Mixed mode (one runtime + one degraded) is supported. Run code-quality and security in parallel where possible; capture outputs to `docs/.review/code-review.md` and `docs/.review/security-review.md` (the synthesizer expects these paths). On the primary path, emit telemetry events `review_delegated_to_code_review` and/or `review_delegated_to_security_review`; on the degraded path, emit `review_degraded_path_used` with the dimension name.

3.2) **Over-engineering audit (minimality axis)** — kit-distinctive, runs on BOTH paths:
   Scan the diff for unnecessary complexity only — not correctness (that is the code dimension above). Tag each finding `delete` / `stdlib` / `native` / `yagni` / `shrink`, one line per finding (`path:line: <tag> <what to cut> → <replacement>`), ending with net removable lines — or `Lean already. Ship.` if nothing to cut. This axis never overrides safety (validation, error handling, security, a11y, explicitly-requested work).
   - On the degraded code path, this is folded into the same `reviewer` agent invocation (the agent's Over-Engineering section covers it) — do not run it twice.
   - On the primary path, ask the `reviewer` subagent to run ONLY this axis; capture output to `docs/.review/minimality.md`.
   - When building `docs/.review/findings.json` (step 3.14), put each minimality finding into the `minimality_findings` array with its tag prefixed in the title (e.g. `[yagni] …`); the synthesizer renders these under the always-on **Over-Engineering** section (the kit's SSOT review-notes contract).
   - Pass gathered context (recalled review lessons, architecture) so recurring patterns are recognized. Because separate-context subagents do not receive auto-recall, include the relevant recalled lessons verbatim in the subagent prompt.

_(Synthesis + merge-audit happen AFTER all kit-distinctive checks 3.5–3.13 complete — see steps 3.14 and 3.15 below. This sequencing lets the synthesizer merge runtime findings, kit-distinctive findings, and any degraded-path findings in a single pass.)_

3.5) **Figma compliance check** (auto-skips if no Figma data) — **runs BEFORE UI review to ensure Figma fidelity first**:

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase figma-compliance --issue $ARGUMENTS`
> Compares ALL implementation style properties (24 categories) against `figma-export/design_data.json`.
> Auto-passes when no Figma data exists. Fails if implementation deviates from Figma design tokens.
> If exit code ≠ 0: STOP immediately. Fix all violations before proceeding.
> **Figma is the absolute source of truth.** Any deviation is a blocking violation.

3.6) **Computed style verification** (auto-skips if no Figma data):

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase computed-styles --issue $ARGUMENTS`
> Renders the implementation in Chromium, extracts ACTUAL computed CSS values from every visible element, and compares against Figma tokens. Catches CSS cascade conflicts, specificity overrides, and inheritance issues that static source analysis (figma-compliance) misses.
> If exit code ≠ 0: STOP immediately. Fix the rendered style mismatches.

3.7) **Structural match** (auto-skips if no Figma data):

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase structural-match --issue $ARGUMENTS`
> Verifies all Figma elements exist per viewport, with responsive CSS.
> If exit code ≠ 0: STOP immediately. Add the missing elements.

3.8) **Layout verification** (auto-skips if no Figma data):

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase layout --issue $ARGUMENTS`
> Verifies element spatial relationships (row/column, sibling ordering).
> If exit code ≠ 0: STOP immediately. Fix layout.

3.9) **Same-renderer visual diff** (blocking — prototype HTML vs implementation, both Chromium):

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase visual-diff --issue $ARGUMENTS`
> Renders both prototype HTML and implementation in the SAME Chromium engine — eliminates font hinting and anti-aliasing differences. Threshold: 1%.
> Falls back to Figma render PNG comparison (5% threshold) if no prototype HTML.
> If exit code ≠ 0: STOP immediately. Fix visual discrepancies.

3.10) **Visual diff debug images** (for reviewer reference):
   Diff images saved to `figma-export/visual-diff/` — red = real diff, yellow = AA.
   Reviewer should inspect these alongside the diff percentage.
   Run: `python3 scripts/verify_visual_diff.py --project-path $WT --threshold 5`
   This is a **non-blocking advisory signal**, NOT a gate. Pixel comparison across different renderers (Figma vs Chromium) inherently produces false positives due to font hinting, anti-aliasing, and sub-pixel rendering differences.
   - Generates diff images in `figma-export/visual-diff/` (red = real diff, yellow = AA)
   - Log the diff percentage in `docs/review_notes/$ARGUMENTS.md` for the reviewer to assess
   - If diff > 5%: flag as a **warning** in review notes — reviewer should inspect the diff images
   - Do NOT block the review based on pixel diff alone. The **figma-compliance checkpoint** (step 3.5) is the authoritative Figma fidelity gate.

3.11) IF the issue involves UI/frontend work (check `UI: true` field first; fall back to Track/title keywords: "UI", "screen", "component", "prototype"):
   Ask ui-reviewer subagent to perform UI state review:
   - Pass the UI context files gathered in step 3 plus any recalled review lessons (inject them into the subagent prompt — subagents get no auto-recall).
   - Reviewer checks state coverage, copy compliance, token usage, accessibility, interaction fidelity.
   - Output: `docs/ui_review_notes/$ARGUMENTS.md` with severity-classified findings.

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase ui-review --issue $ARGUMENTS`
> The script auto-detects UI issues via Track field/title keywords. Non-UI issues pass automatically.
> If exit code ≠ 0: STOP immediately and report the failure. Do NOT proceed.

3.12) IF UI issue AND design system docs exist (`docs/design_system.md`, `design_system_mobile.md`, or `design_system_desktop.md`):
   Ask design-auditor subagent to audit the design system:
   - Pass all design context files gathered in step 3.
   - **Also pass `figma-export/design_data.json`** (if exists) so auditor can verify design system covers all Figma tokens.
   - Auditor checks: token consistency, component completeness, cross-platform alignment, philosophy compliance, copy coverage, **Figma token coverage**.
   - Output: `docs/design_audit.md` with severity-classified findings.

3.13) IF UI issue:
   Ask a11y-auditor subagent to perform WCAG 2.1 AA accessibility audit:
   - Pass design context files + source code files from the PR diff.
   - Auditor checks all 4 WCAG principles: Perceivable, Operable, Understandable, Robust.
   - Output: `docs/a11y_audit.md` with findings and fix suggestions.

3.14) **Synthesize** runtime code/security outputs (from step 3.1) + kit-distinctive findings (Figma 3.5–3.10, UI review 3.11, design audit 3.12, a11y audit 3.13) into `docs/review_notes/$ARGUMENTS.md`:
   - Build the structured intermediate `docs/.review/findings.json` with `code_findings` and `security_findings` (extracted verbatim from runtime outputs or the degraded reviewer agent), plus optional `ui_findings` / `design_findings` / `a11y_findings` / `figma_findings` arrays from whichever kit-distinctive checks ran.
   - Run: `python3 scripts/synthesize_review_notes.py --input docs/.review/findings.json --out docs/review_notes/$ARGUMENTS.md`
   - The synthesizer preserves severity verbatim from upstream, sorts findings within each section by severity (Critical → Low), and renders empty runtime sections with the `_No findings._` literal. Kit-distinctive sections appear only when they ran.

4) Apply minimal fixes for Critical/High findings (code + UI + design audit + a11y audit + Figma compliance); re-run tests inside `$WT/`.

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase test --issue $ARGUMENTS`
> If exit code ≠ 0: STOP immediately and report the failure. Do NOT proceed.

4.5) **Test quality verification**: Validate that all test files in the branch contain real assertions (no hollow tests). This catches test files that exist to satisfy checkpoints but don't actually verify behavior.

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase test-quality --issue $ARGUMENTS`
> If exit code ≠ 0: STOP immediately and report the failure. Do NOT proceed.

4.7) **Tech-debt ledger** (advisory — harvests `KIT-DEBT` markers):

> **CHECKPOINT — advisory (non-blocking)**
> Run: `bash scripts/checkpoint.sh --skill review --phase debt --issue $ARGUMENTS`
> Always exits 0. Lists every `KIT-DEBT(ceiling=…, trigger=…)` marker; flags markers missing a `trigger=` as **silent-rot risk**.
> Record any `no-trigger` markers as Low-severity `code_findings` entries (title prefixed `[debt]`) in `docs/.review/findings.json` so deferrals don't become permanent. Does NOT block the review.

5) `docs/review_notes/$ARGUMENTS.md` is **already produced by step 3.14** (synthesizer) with the canonical 2-section format (Code Review + Security Findings, minimality/debt findings folded into Code Review with their tags) plus any kit-distinctive sections (UI Review / Design Audit / Accessibility Audit / Figma Compliance) that ran. If fixes applied in step 4 changed the underlying findings, or step 4.7 added `[debt]` entries, re-run `python3 scripts/synthesize_review_notes.py` with the updated `docs/.review/findings.json`. Do not edit `docs/review_notes/$ARGUMENTS.md` by hand — the kit's SSOT contract (`/ship`, `/sprint` consumers) depends on the synthesizer-produced shape.

5.5) **Learning Extraction → native memory** (kit-distinctive, not duplicated by runtime). Identify findings in `docs/review_notes/$ARGUMENTS.md` that could have been prevented earlier (at kickoff or implementation time), and record each preventable pattern as a **review lesson in Claude Code's native persistent memory** so it auto-recalls into future sessions — replacing the retired `docs/review_lessons.md` registry (ISSUE-033).
   - Persist to the project memory topic file `review-lessons.md` (in the native auto-memory directory — `${autoMemoryDirectory}` if set, else `~/.claude/projects/<project>/memory/`; use your memory capability rather than a hard-coded path). Add a one-line pointer under a `## Review Lessons` heading in `MEMORY.md` so the index surfaces it at session start.
   - Each lesson is one entry: a short title, the classification (Code Quality / Security / Testing / Architecture / Over-Engineering), **Why** it matters, and **How to apply** (the prevention step). Include the observed PR/issue.
   - **Dedup, don't version-count**: before writing, check the `MEMORY.md` index and `review-lessons.md`; if the pattern already exists, update that entry (append the new observed issue) instead of creating a duplicate. No numeric lesson IDs, no Frequency counter — the native convention is one fact per entry, updated in place.
   - **Do not record a lesson that teaches a better document guard.** A lesson whose "How to apply" amounts to *keep this doc in sync with the code* — regenerate a copied number, tighten a phrasing blacklist, assert a constant is documented — institutionalizes the `[record]` class: it widens what future reviews check, and each new drift becomes triage work. Record the lesson one level up instead: *don't state the derived fact, or generate it from its source*. Same test as the triage gate — if the pattern never recurring would break nothing for a user, it is not a lesson.
   - Do **not** write `docs/review_lessons.md` or use `registry_edit.sh` for lessons; native memory is the store now.
5.7) **Update test plan** (if `docs/test_plan.md` exists):
   - Ask qa-designer subagent to review and update `docs/test_plan.md` based on the PR changes.
   - Pass: PR diff summary, list of changed files, and current `docs/test_plan.md` content.
   - qa-designer evaluates: new flows requiring test coverage, risk level changes, E2E gap updates.
   - Update via `bash scripts/registry_edit.sh docs/test_plan.md -- bash -c '<update command>'` for safe concurrent modification.
   - If no updates needed: skip silently.

> **CHECKPOINT — MANDATORY — NEVER SKIP**
> Run: `bash scripts/checkpoint.sh --skill review --phase review --issue $ARGUMENTS`
> If exit code ≠ 0: STOP immediately and report the failure. Do NOT proceed.

6) Commit + push from `$WT/`.

> **CHECKPOINT — ADVISORY (report & continue)**
> Run: `bash scripts/checkpoint.sh --skill review --phase push --issue $ARGUMENTS`
> Advisory: exits 0 even on failure, printing an `ADVISORY:` line — report the gap, self-correct, then continue.

7) If PR is draft and ready: `gh pr ready`.

## Shared Registry Files
**IMPORTANT**: Never commit `issues.md`, `STATUS.md`, or `CHANGELOG.md` to the feature branch.
These are registry files managed only on main. Always use `bash scripts/registry_edit.sh <file> -- bash -c '<update command>'` — the wrapper resolves the main repo root internally.

## Error Handling
- If PR not found (issues.md has no PR field and `gh pr status` returns nothing): stop and report; suggest running `/implement` first.
- If reviewer subagent fails: retry once; if still failing, skip automated review and log a warning in docs/review_notes/$ARGUMENTS.md.
- If applied fixes break tests:
  1. Revert the fix commits: `git checkout -- <files>` for unstaged or `git revert HEAD` for committed changes (inside `$WT/`).
  2. Re-run tests to confirm the branch is back to a passing state.
  3. Log the failed fix attempt in docs/review_notes/$ARGUMENTS.md as a follow-up item.
- If `gh pr ready` fails: report the error but do not block — the PR can be manually marked ready.

## Rollback
- Review changes are commits on the existing PR branch.
- If review fixes must be fully undone: `git revert` the review commits (do not force-push).
- docs/review_notes/$ARGUMENTS.md is append-only; no rollback needed for notes.
- Clean up worktree when done: `bash scripts/wt_cleanup.sh "$BRANCH"`.