Review Pr

---
name: review-pr
description: Review a pull request against this harness's rules and the project's own standards, and return findings ranked by cost with a verdict. Reports only, never approves, merges or posts. Use when asked to review a PR, a branch or a diff, "look at PR 42", "is this ready to merge", or "what would you flag in this change".
license: MIT. From the Sanvio Labs harness starter, https://github.com/SanvioLabs/sanvio-harness-starter
---

# Review a pull request

You're the second reader, not the author. Read the change the way the person
who has to maintain it next year would, and say what it costs them. Report
only: a human decides what happens to the PR.

## Inputs

One of: a PR number, a branch name, or "my current changes". If it's unclear
which, ask. Also note which project under [`projects/`](../../projects/) it belongs to, if any.

## Steps

1. **Get the change.** In the project's own checkout:
   - GitHub: `gh pr view <n>` for the description, then `gh pr diff <n>`
   - Azure DevOps or anything else: `git fetch`, then
     `git diff <base>...<branch>` against the branch it merges into
   - Current changes: `git diff` and `git diff --staged`

   If the diff is too big to read properly (roughly over 800 changed lines),
   say so and suggest how it splits. Then review the riskiest part rather than
   skimming all of it.
2. **Read what it's held to.** [`AGENTS.md`](../../AGENTS.md) here, the project's own
   `AGENTS.md` or `CLAUDE.md` if it has one, and the standards they point at.
   If the PR links a ticket, read what the ticket asked for.
3. **Run what checks it.** The project's tests and any gate that covers the
   changed files. Paste the result. A review that didn't run the tests says so
   at the top.
4. **Read the diff for what a script can't catch**, most expensive first:
   - Does it do what the ticket or description says, and nothing else?
   - Security: authorization skipped or done in the client, input trusted,
     secrets in code or logs, a new dependency nobody asked for. When the
     diff touches auth, input handling, secrets, infrastructure or
     dependencies, also hand it to the `security-reviewer` agent
     ([`agents/security-reviewer.md`](../../agents/security-reviewer.md)) and fold its findings in
   - Data: a migration with no way back, a change that breaks callers
   - Error paths: what happens when the call fails, times out or returns
     nothing
   - Tests: is the new behaviour tested, and would the test fail if it broke?
     [`skills/review-tests/`](../review-tests/) goes deeper
   - Standards: where it departs from a written standard, quote the standard
   - The harness itself: a noticeable change with no new entry in
     [`CHANGELOG.md`](../../CHANGELOG.md). Its header says what counts
5. **Decide.** **Ready**, **ready with nits**, or **changes needed**. Changes
   needed means at least one finding that would cost something real if it
   merged.

## Output

- The verdict and the test result, first
- At most ten findings, most expensive first. Each one: `file:line`, what's
  wrong, what it costs, and the smallest fix
- What you didn't review and why, if anything

## On every pull request

To run this skill on every pull request without anyone asking,
[`examples/ci/review-pr.yml`](../../examples/ci/review-pr.yml) is a GitHub Actions workflow to copy into
[`.github/workflows/`](../../.github/workflows/). It runs the skill headlessly and posts the report as one
comment. The workflow posts; the skill still doesn't.

## Rules

- Never approve, merge, close or push. Never post a comment on the PR unless a
  human says so in this session, and then show the exact text first.
- A finding names a consequence. "I'd write it differently" isn't a finding.
- Don't repeat what a linter, a test or a gate already reports. Say it ran.
- If you find a secret in the diff, don't quote it. Name the file and line,
  say it needs rotating, and stop the review there.