pr-review

name

pr-review

description

Autonomous local code review. Reviews either an open PR's diff (via `gh`) or the local working diff against a base branch (`--local`, auto-selected when no PR exists yet), spawns a dedicated review subagent with universal + stack-specific severity criteria, posts the review as a comment on the PR (audit trail; skipped in local mode), and asks for a decision. Stack-aware via sibling PATTERNS.md (node-ts, python, swift in v1; agnostic body fallback for others). Severity rules configurable via `team-settings.json` `prReviewSeverity` (Option β); hard-coded universal defaults when absent (Option α). Default model is sonnet; `--deep` escalates to opus. Skill never modifies code, never auto-merges — merge is always the user's decision.

Run an autonomous PR review locally. Classify findings by severity, post the review as a comment for audit trail, and surface a merge decision to the user. Read-only: the skill orchestrates a review; it does not modify code, does not push, does not merge.

Step 0 — Resolve repo + parse args

# Detect the GitHub repo from the local clone (no hard-coded org/repo).
REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)

If gh repo view fails: respond "This skill requires the gh CLI authenticated against a GitHub repo. Run \gh auth login` and ensure the project has a GitHub remote."` and stop.

Parse $ARGUMENTS:

Arg	Behavior
<PR_NUMBER> (positional, optional)	Integer. If absent, detect from current branch in Step 1.
--local	Review the local working diff (git diff <base>...HEAD) instead of a PR. Use before the PR exists to catch findings pre-push. Auto-selected in Step 1 when no PR is found for the branch, so an explicit flag is only needed to force local mode while an open PR also exists.
--base <branch>	Base branch for --local diff (default: the repo's default branch, resolved in Step 1). Ignored in PR mode (the PR carries its own base).
--deep	Escalate the review subagent from sonnet to opus. Use for changes touching auth, money, migrations, or shared utilities. Adds ~30-60s latency.
--with-context	Pass the active session file (.claude/session/block-*.md if present) to the review subagent. Default OFF — review stays diff-pure and unbiased by prior decisions.

Examples:

• /pr-review 122
• /pr-review 122 --deep
• /pr-review --deep (resolves PR from current branch; falls back to local diff if none open)
• /pr-review --local (review the working diff against the default base, before opening a PR)
• /pr-review --local --base staging

Step 1 — Resolve PR number, or select local-diff mode

Mode selection:

• If --local was passed → local mode (skip PR resolution entirely).

• Else if PR_NUMBER was passed → PR mode with that number.

• Else detect a PR for the current branch:

  BRANCH=$(git rev-parse --abbrev-ref HEAD)
  gh pr list --repo "$REPO" --head "$BRANCH" --state open --json number -q '.[0].number'
  

  If empty, retry with --state merged to catch a recently-merged PR. If a number is found → PR mode. If still nothing → auto-fall back to local mode (do NOT stop): there is no PR yet, so review the local diff. Announce: No open PR for branch <X> — reviewing the local diff instead (run again after opening the PR for the audit-trail comment).

Local mode — resolve the base branch and confirm there is a diff:

BRANCH=$(git rev-parse --abbrev-ref HEAD)
# --base <branch> if given; otherwise the repo default branch.
BASE="${BASE_ARG:-$(gh repo view --repo "$REPO" --json defaultBranchRef -q .defaultBranchRef.name)}"
git rev-parse --verify "$BASE" >/dev/null 2>&1 || BASE="origin/$BASE"
git merge-base --is-ancestor "$BASE" HEAD 2>/dev/null; git rev-list --count "$BASE...HEAD"

If the branch IS the base branch, or git diff "$BASE"...HEAD is empty: respond "No local changes vs <BASE> to review. Commit or stage work first, or pass --base <branch>." and stop. The diff uses three-dot <BASE>...HEAD (changes introduced on HEAD since it diverged from base) — the same set a PR against <BASE> would show.

Step 2 — Fetch metadata + diff

PR mode:

gh pr view "$PR_NUMBER" --repo "$REPO" \
  --json title,state,headRefName,baseRefName,additions,deletions,changedFiles \
  > /tmp/pr-review-${PR_NUMBER}-meta.json

gh pr diff "$PR_NUMBER" --repo "$REPO" > /tmp/pr-review-${PR_NUMBER}.diff

Local mode (no PR number — key the temp files off the branch name):

SLUG=$(echo "$BRANCH" | tr '/' '-')
git diff "$BASE"...HEAD > /tmp/pr-review-local-${SLUG}.diff
git diff "$BASE"...HEAD --stat | tail -1        # summary line for the stats field

Build the metadata fields from git rather than gh pr view: title = latest commit subject (git log -1 --format=%s), state = LOCAL (no PR yet), headRefName = $BRANCH, baseRefName = $BASE, additions/deletions/changedFiles from git diff "$BASE"...HEAD --shortstat.

In both modes, <DIFF_PATH> is the file written above.

If the diff is over 50 000 lines: warn the user and ask whether to proceed. A very large diff produces shallow review; default proceed but flag in the report.

Step 3 — Load review context

Three sources, layered:

1. Universal severity defaults (Option α): always loaded from this SKILL.md (see "Severity criteria — universal defaults" below).

2. Stack-specific severity additions: load sibling PATTERNS.md if the project's stack matches an entry there. PATTERNS.md adds critical/major/minor patterns specific to the stack (node-ts, python, swift in v1).

3. Project override (Option β): if .claude/team-settings.json contains a prReviewSeverity section, its arrays override / extend the universal + stack defaults. Schema:

   {
     "prReviewSeverity": {
       "critical": ["paths/to/auth/**", "src/migrations/**"],
       "major": ["src/api/**"],
       "minor": []
     }
   }
   

   File path globs marked critical always escalate findings on those paths; same for major. The minor array is an explicit "downgrade" list — findings in those paths default to Minor.

Also read CLAUDE.md (project conventions). Pass it to the subagent so project-specific intentional patterns are not flagged as issues (e.g., language conventions, framework-specific idioms, intentional service-role usage).

Step 4 — Spawn the review subagent

PR mode — reuse a same-commit pre-PR review (confirmatory short-circuit). Before spawning, check whether a local pre-PR review already covers this exact commit and base:

read -r PR_HEAD_SHA HEAD_REF PR_BASE < <(gh pr view "$PR_NUMBER" --repo "$REPO" \
  --json headRefOid,headRefName,baseRefName -q '"\(.headRefOid) \(.headRefName) \(.baseRefName)"')
SLUG=$(echo "$HEAD_REF" | tr '/' '-')
LOCAL_REPORT=/tmp/pr-review-local-${SLUG}-output.md

# Read provenance from the HEADER BLOCK ONLY (the first lines, up to the blank line that
# closes it) — never a body/diff line that happens to quote "Reviewed-SHA:". `sed` quits at
# the first blank line, and `^Reviewed-…:` is anchored to the line start.
if [ -f "$LOCAL_REPORT" ]; then
  REVIEWED_SHA=$(sed -n '/^$/q; s/^Reviewed-SHA: *//p'   "$LOCAL_REPORT")
  REVIEWED_BASE=$(sed -n '/^$/q; s/^Reviewed-Base: *//p'  "$LOCAL_REPORT")
  REVIEWED_MODEL=$(sed -n '/^$/q; s/^Reviewed-Model: *//p' "$LOCAL_REPORT")
fi

Reuse the local report (skip the subagent) only when all of these hold:

• $LOCAL_REPORT exists and $REVIEWED_SHA == $PR_HEAD_SHA (head unchanged since the local pass);
• $REVIEWED_BASE == $PR_BASE (same three-dot diff scope — a local review taken against a different base covers a different change set and must not be reused);
• the report has zero Critical and zero Major findings — key the skip on the finding counts, never on the verdict line: a soft or mis-generated ✅ LGTM that still carries a Major must not ride the skip (count the entries under the report's ### Critical (N) / ### Major (N) sections; N=0 on both is the only pass);
• the current invocation is not --deep, unless $REVIEWED_MODEL is already opus (an explicit deep request is never satisfied by a shallower pass).

When all hold, write the reused body to the PR output path with a reuse banner prepended, carry the local report's verdict to Step 6, and skip the rest of Step 4:

{ printf '> Reused from same-commit pre-PR local review — second review run skipped (head and base unchanged since the local pass).\n\n'; cat "$LOCAL_REPORT"; } \
  > /tmp/pr-review-${PR_NUMBER}-output.md

Otherwise re-review (spawn below). This is the conservative default: no local report, a Reviewed-SHA/Reviewed-Base mismatch, a Critical/Major in the local report, a --deep request against a shallow report, or a maintainer choosing a fresh pass after seeing CI at Step 7 — all lead here. The skill does not query CI status itself; that judgment is the maintainer's at Step 7.

Use the Agent tool (subagent_type: general-purpose). Model: sonnet by default, opus if --deep.

Compose the subagent prompt by substituting these placeholders into the template under "Review subagent prompt" below: <REPO>, <N>, <TITLE>, <HEAD_REF>, <BASE_REF>, <DIFF_PATH> (do NOT inline the diff content; pass the path so the subagent reads it from disk), <META_JSON>, <CLAUDE_MD>, <STACK_PATTERNS> (contents of PATTERNS.md or "(no stack-specific patterns for this project)"), <TEAM_SEVERITY> (parsed prReviewSeverity JSON or null), and <SESSION_FILE_CONTENT> (only if --with-context).

The subagent returns a structured markdown review.

Step 5 — Post the review as a PR comment

PR mode:

gh pr comment "$PR_NUMBER" --repo "$REPO" --body-file /tmp/pr-review-${PR_NUMBER}-output.md

Always post — even on LGTM clean reviews, and even on the Step 4 reuse path — to maintain a permanent audit trail. Reuse skips the second review run, never this comment: it posts the reused report written to the same /tmp/pr-review-${PR_NUMBER}-output.md path. The comment is the canonical record; the in-conversation summary in Step 6 is for the maintainer's terminal.

Local mode: there is no PR to comment on, so skip gh pr comment. Write the report to /tmp/pr-review-local-${SLUG}-output.md, then prepend a provenance header so a later PR-mode run can safely detect a same-commit pre-PR review:

{
  printf 'Reviewed-SHA: %s\n'  "$(git rev-parse HEAD)"
  printf 'Reviewed-Base: %s\n' "${BASE#origin/}"      # normalized: strip origin/ so it matches a PR's baseRefName
  printf 'Reviewed-Model: <sonnet|opus>\n\n'          # the model chosen in Step 4 (opus when --deep)
  cat /tmp/pr-review-local-${SLUG}-output.md
} > /tmp/pr-review-local-${SLUG}-output.md.tmp \
  && mv /tmp/pr-review-local-${SLUG}-output.md.tmp /tmp/pr-review-local-${SLUG}-output.md

Tell the user the audit-trail comment is deferred — re-run /pr-review <N> once the PR is open to post the canonical record. The local pass is a pre-push gate, not the audit trail; the provenance header lets the PR-mode run reuse this review only if the PR's head commit and base branch both match (see Step 4).

Step 6 — Synthesize for the user

Parse the subagent output. Produce a compact in-terminal summary.

PR mode:

PR #<N> — <TITLE>
Status: CI <state> · Review model: <sonnet|opus, or "reused (same-commit pre-PR)"> · Comment posted: <URL>

Critical (N)
- <file:line> — <finding> → <action>

Major (N)
- <file:line> — <finding> → <action>

Minor (N) — to append to docs/refactoring-backlog.md if user proceeds
- <one-liner>

Decision needed (default: **integrate** for a fix on this open PR): integrate fix · fix branch · proceed merge?

If zero Critical and zero Major: omit those sections, list Minor in ≤ 2 lines, recommend proceed merge directly.

Local mode: replace the header with Local review — branch <BRANCH> vs <BASE> · Review model: <sonnet|opus> · Comment: deferred until PR opens. Keep the Critical/Major/Minor sections identical. Replace the decision line with Decision needed (default: **fix** before opening the PR): fix now · open PR as-is? — see Step 7.

Step 7 — Wait for user decision

Three valid responses (integrate is the default for fixing findings on the PR under review):

1. integrate (default) — apply the fix in the current branch, run /commit, push, re-run gh pr checks --watch, then re-invoke /pr-review <N> to confirm. This is the right choice for a Critical/Major found on this PR: the fix lands on the same branch/PR.
2. fix branch — open fix/<short-desc> from the base branch, apply, full pipeline. Reference the original PR in the description. Not for fixing this PR: a branch cut from the base cannot contain this open PR's own changes, so it only makes sense as a deliberately deferred, decoupled follow-up (e.g. a separate refactor), never as the default for resolving a finding on the current PR.
3. proceed — user runs gh pr merge themselves. Persist unresolved Minor findings via the backlog write-once protocol (.claude/rules/backlog-protocol.md) — session scratch in an active block, direct to docs/refactoring-backlog.md standalone — with the appropriate ID prefix (PERF-, DEV-, SEC-, DB-, A-, S-, T-, N-).

Local mode — no PR exists yet, so the choice is whether to fix before opening it:

1. fix (default) — apply the fix on the current branch, run /commit. No push/PR round-trip is spent on the finding. This is the whole point of the local pass: resolve Critical/Major before CI and reviewers see the PR.
2. open PR as-is — the user opens the PR themselves; unresolved Minor findings are persisted via the backlog write-once protocol (.claude/rules/backlog-protocol.md; session scratch in a block, direct standalone) with the appropriate ID prefix. Re-run /pr-review <N> after the PR opens to post the audit-trail comment.

Never open, push, or merge from this skill in either mode — those are the user's actions. The canonical merge gate stays pipeline.md Phase 8 (Tier M/L) or FL-2 (Tier S).

Never call gh pr merge from this skill — merge is always the user's decision. The pipeline.md Phase 8 (Tier M/L) or FL-2 (Tier S) is the canonical merge location, and it's a human gate.

---

Severity criteria — universal defaults (Option α)

These apply to any project. PATTERNS.md (when present for the detected stack) adds stack-specific entries. team-settings.json prReviewSeverity (when present) overrides + extends.

Critical — blocks merge

• Missing auth/authz check in an API route or RPC handler before mutating data
• Secret / token leaked in response body, error message, log line, or test fixture
• Direct SQL or shell injection vector (raw user input → query string / shell argument)
• User input written to a persistent store without runtime validation on a write path
• Cross-tenant / cross-user data leak (entity A's record returned to user B)
• Hard-coded credentials in source (password = "...", apiKey = "...")
• A migration that drops / renames a column without a documented rollback path
• Privileged client (admin / service role / sudo) used in a context that should not have privilege
• Authentication state ignored or bypassed (unsigned JWT accepted, expired token honored)

Major — should be resolved before merge

• API route missing input validation on a write path
• Race condition / TOCTOU on a mutating path
• Error path silently swallowed (catch {}) where the error should be reported / logged / surfaced
• Type unsafety on a production path (as any, unsafe cast on data from external source without runtime validation)
• Unbounded query / pagination missing on a list endpoint
• N+1 query pattern on a request-handling path
• New persistent table without access-control policy declaration
• Console / debugger / TODO / FIXME / @ts-ignore / similar suppression left in committed code
• An async fire-and-forget without a .catch() that should report
• Test asserts state synchronously where a wait-for-condition is required (flake)

Minor — append to refactoring backlog

• Naming inconsistency, comment typo, doc gap
• Opportunistic refactor that could simplify but isn't broken
• Unused import, unused variable, dead code
• Stylistic preference (variable name length, ordering)
• Minor performance pitfall on a cold path

---

Review subagent prompt (template)

Pass this prompt verbatim to the review subagent in Step 4. Substitute the bracketed placeholders. The subagent has read access only — it must NOT post the comment, edit files, or run any mutation command. That is the orchestrator's job.

You are an autonomous code reviewer for a pull request on the <REPO> repository. Your output goes verbatim into a comment on the PR — write it as a markdown review.

## PR metadata
- Repo: <REPO>
- Number: #<N>
- Title: <TITLE>
- Branch: <HEAD_REF> → <BASE_REF>
- Diff path: <DIFF_PATH>
- Stats: <META_JSON>

## Project conventions (from CLAUDE.md — do NOT flag these as issues)
<CLAUDE_MD>

## Severity criteria

### Universal (always apply)
[Insert "Severity criteria — universal defaults" section verbatim from SKILL.md]

### Stack-specific additions
<STACK_PATTERNS>

### Project overrides
<TEAM_SEVERITY>

## Output format — MANDATORY

Produce a single markdown document. This is what gets posted as a PR comment.

```markdown
## /pr-review — autonomous local review

**PR**: #<N> — <TITLE>
**Branch**: `<HEAD_REF>` → `<BASE_REF>`
**Stats**: +<additions> −<deletions> across <changedFiles> files
**Model**: <sonnet|opus>

### Verdict
<one of: ✅ LGTM clean — safe to merge | ⚠ Findings present — see below | 🛑 BLOCKING issues — do not merge>

### Critical (N)
<numbered list: file path:line, finding in 1-2 sentences, recommended action. If N=0, write "None.">

### Major (N)
<same format. If N=0, write "None.">

### Minor (N)
<one-line bullets. If N=0, write "None.">

### Notes
<optional, ≤3 sentences: context useful for the merger that doesn't fit above>

---
*Generated locally by `/pr-review` skill (<ISO timestamp>)*

Constraints

• Be conservative: if you're not sure something is a problem, surface it in Notes, not as Critical/Major.
• Cite line numbers from the diff when possible (the diff has +/- line markers).
• Never invent issues to fill quota — "0 findings" is a valid output.
• Length cap: total output ≤ 200 lines. If the diff is enormous, summarize patterns rather than enumerating every nit.
• You may grep / read project files to verify a concern, but do NOT edit anything.
• You must NOT call gh pr comment. Return the markdown report only.
• If <SESSION_FILE_CONTENT> is provided: use it to understand architectural decisions made during the block, but do NOT defer to it on security/correctness findings. Your job is to spot issues the implementer may have rationalized away.

---

## Hard rules

- **Never call `gh pr merge`**. Merge is the user's decision; this skill only reviews.
- **Never modify code**. The subagent has read-only access; the orchestrator never patches.
- **Always post the comment in PR mode**. Even on LGTM clean reviews — the audit trail is the value. Local mode has no PR to comment on: it writes the report to disk and defers the canonical comment to the later PR-mode pass.
- **Reuse only a same-commit, same-base pre-PR review**. The Step 4 short-circuit skips the second review run only when head SHA and base branch both match and no Critical/Major was found; it still posts the audit-trail comment, and it never reuses across a moved HEAD, a different base, or an explicit `--deep` request.
- **Never trust `--with-context` to absolve findings**. Session context informs the review; it does not override security or correctness signals.
- **Never paste 50 000-line diffs into the subagent prompt**. Pass the path; the subagent reads from disk.

## Out of scope (v1)

- CI workflow that auto-runs `/pr-review` on every PR. Defer to v2 once observed adoption signal supports it.
- Auto-merging based on the review verdict. Always human-in-the-loop.
- Multi-language reviewer comments. English only in v1.
- Streaming the review back to the user as it's generated. v1 returns the full report once.

## Stack adaptation

PATTERNS.md (sibling file) provides per-stack severity additions for the top 3 stacks in v1: node-ts, python, swift. Other stacks fall back to the universal defaults in this body. PATTERNS.md is loaded conditionally only when the detected stack matches one of the documented entries.