/pr-review

name: pr-review
description: Use when the user wants to deeply review a pull/merge request or a local diff — "review this PR", "grill the MR", "stress-test this diff", "deep-dive code review", "code review before merge", or when they name a PR/MR URL, number, branch, commit range (`A..B`/`A...B`), tag or SHA. Works on GitHub (gh), GitLab (glab), or a hostless local branch or range. Review-only — never edits code.

PR Review — Two-Axis, Stack-Agnostic Deep Code Review

**Commands.** Run each command as one plain command line with literal values (real refs and paths): no shell variables you set yourself, no `;` or `&&` chains, no `$?`, and no pipes or redirects (`|`, `>`, `2>&1`): the one exception is the `<<'REQUEST'` heredoc that hands `run start` its request. The sandbox's auto-allow refuses compound commands, and `sandbox.excludedCommands` only matches a bare call.

Philosophy

The purpose of reviewing a change is to build **shared understanding** of what the change is trying to do, then systematically hunt for the bugs and edge cases that intent reveals. Every diff carries implicit assumptions, unstated invariants, and ambiguous decisions. The review walks through them one by one.

When neither the codebase nor the user can resolve a question deterministically — when it's a real unknown about author intent or non-local context — that question itself becomes a review comment for the author to answer. The goal is **zero unexamined assumptions** before the review is finalized.

The two review axes (Standards ⊥ Spec)

The diff is checked along **two deliberately separate axes** by parallel, context-isolated sub-agents (Phase 3.5):

• **Standards** — does the code conform to *this repo's* documented conventions (discovered in Phase 0)?
• **Spec** — does the code faithfully implement the originating spec / issue / story / PRD?

A change can pass one axis and fail the other:

• Code that follows every standard but implements the wrong thing → **Standards pass, Spec fail.**
• Code that does exactly what the issue asked but breaks the project's conventions → **Spec pass, Standards fail.**

The two axes run as **separate sub-agents** so neither pollutes the other's context, and their findings are reported **side by side, never merged or reranked across axes** — separation stops one axis from masking the other.

Prime Directives

1. **Review only — never modify code.** No edits, fixes, commits, pushes, or merges. If the user asks for a fix mid-review, stop and confirm they want to leave the review before touching code. 2. **Exhaust the codebase before asking.** Every question to the user (or the author) must include an `Already checked:` line citing what you grepped/read and why it didn't answer it. If you can't write that line, you haven't explored enough. See [references/exploration-checklist.md](references/exploration-checklist.md). 3. **Use AskUserQuestion for every judgment question.** Never plain-text a question. Lead with your recommendation (first option, `(Recommended)`). Batch up to 4 same-domain questions per call. **When there is no user to ask** — a subagent, `claude -p`, a workflow step — do not plain-text the question to nobody: resolve it from the code, or promote it to an Open Question for the author, per the interactivity table in *Inputs & VCS Auto-Detection*. 4. **Don't surface what the code already answers.** Anti-pattern: asking something that 30 seconds of `grep` resolves. Reserve user questions for genuine judgment calls and author-intent for genuine non-local unknowns. 5. **Stay concrete.** Anchor every question and finding to a specific `file:line` and a specific failure mode. 6. **Promote real unknowns to review comments.** When neither code nor user can answer, record as an Open Question for the Author (defaults to post = `[x]`). 7. **Verdict and confidence are mandatory.** Every finding gets both before the user sees the triage table.

Non-Goals

• ❌ Editing the change's code, tests, or docs
• ❌ Running migrations, fixing bugs, applying suggestions
• ❌ Merging, closing, or approving the PR/MR
• ❌ Pushing commits
• ❌ Re-printing the findings table in chat after writing it to the file (unless asked)

The only host side-effects this skill performs are **posting review comments** (Phase 7) and **updating the PR/MR title verdict icon** (Phase 8), both only on explicit user confirmation, and skipped entirely in local-branch mode.

---

Inputs & VCS Auto-Detection

Resolve the target, the host and the **interactivity mode** once, in Phase 0. Pick the cheapest mode that works — never require a host.

| Input | Mode | How to fetch | |-------|------|--------------| | PR/MR **URL** | host from the URL domain | `github.com` → `gh`; `gitlab.*` → `glab` | | PR/MR **number** | host from `git remote get-url origin` | github.com → `gh`; gitlab.* → `glab` | | **commit range** `A..B` / `A...B` | **local** (no host) | `git diff A..B` / `git diff A...B`, `git log A..B` | | **tag or SHA** `X` | **local** (no host) | `git diff <base>...X`, or `git diff X^..X` for a single commit already on the base | | **empty / "current branch"** | **local** (no host) | `git diff <base>...HEAD`, `git log <base>..HEAD` |

**Which diff form — say it out loud.** On a range the choice changes the answer, so name the exact command in the findings-file header:

• **Three dots** (`git diff A...B`) is the **merge-base** diff — B against `git merge-base A B`, i.e. only what

B added since it diverged. It is the deliberate default for branch-vs-base review (`git diff <base>...HEAD`): it never blames the branch for commits that landed on base afterwards.

• **Two dots** (`git diff A..B`) is a plain endpoint-to-endpoint diff. When the user hands you `A..B`, honour it literally — never silently rewrite it to `A...B`.
• `git log` is the mirror image: the commit list is `git log A..B` (two dots) in both cases.
• A **tag or bare SHA** `X` is ambiguous alone: use `<base>...X` when `X`