Review that runs

Written for the post Six weeks of AI code review: what it caught and what it let through, which tells what these rules caught and missed over 79 real PRs.

A first-pass PR review for a human to read, cut and post. The agent checks; the human decides. Nothing here approves or blocks a PR.

Structure borrowed from Matteo Collina's reviewing-prs.md (skill nodejs-core, https://github.com/mcollina/skills). The difference: this one executes.

Hard rules

1. Verify before you claim. Anything you cannot prove against the code on the branch does not become a comment. A killed false positive is the system working.
2. Never touch the user's working tree. Fetch the PR and git worktree add --detach into <state>/<pr|mr|change>-<N>/worktree/ (see Output template), never inside the repo.
3. Nothing reaches the code host. No gh pr review, glab mr note, az repos pr comment etc. The deliverable is the notes file. Posting it is the user's call.
4. Detect, don't assume. Forge, language, package manager and gates come from the repository, never from habit. A guessed npm test that fails in a Ruby repo gets blamed on the PR.
5. Frontend changes get run in Chrome. The real app, styled, built from the PR's own worktree. A passing test, a screenshot of unstyled markup and a careful read of the diff are not validation: layout and overflow are invisible to all three. If the app genuinely cannot run, say so and name the reason.
6. Every finding ends with a proof line. *Verified:* when you tested it, *Inferred:* when you only read it. No proof line, no comment.
7. Mark what you changed to make it run. Any edit to mocks, fixtures or env to boot the app carries a // REVIEW SCRATCH: not part of the PR comment, and the overview names it, so mock behaviour is never read as the PR's.
8. Leave nothing running. The worktree, every container, volume, network, dev server and Chrome page the review started is torn down before the notes path is reported, also when the review is aborted or fails halfway (see Cleaning up). Only the notes file and the profile survive a review.

Flow

Track progress in a todo list with exactly these steps, in order:

1. Detect: identify the forge, then fetch the head and build the project profile (below) before running anything.
2. Target: read the PR's metadata and diff with the forge's own tool, and fetch its head. Note the head SHA and the base branch.
3. Spec: find the ticket in the branch, title or description. Unreachable? The PR description is the spec. Say which one you used.
4. Big picture: read CLAUDE.md / AGENTS.md, the project's docs and ADRs, and the modules around the change. A diff can be locally correct and still wrong for the system.
5. What changed: one business-level paragraph. "Fixes the duplicate image on the product page", never "renamed a to b and added an if".
6. Scope check: the spec against the diff. Matches, misses pieces, or does things nobody asked for.
7. Find problems: correctness (edge cases, wrong data, unhandled errors, races) and design (consistency with sibling code, reinventing what already exists, silent breaking changes). Open every changed file at the PR's revision. When a function, route or event changes, find its other callers including the ones the diff does not touch.
8. Verify: test each finding before writing it up. Prefer a failing test that reproduces it. Frontend goes through Chrome (below). Discard what does not hold.
9. Write: fill the template.
10. Tighten: cut filler, restated findings and praise adjectives.
11. Check anchors: every anchor must land on a line the PR diff shows.
12. Clean up: tear down everything the review started (below), then report the notes path.

Detecting the project

The forge comes from the remote and decides how to fetch. Everything else comes from the PR's head, since a PR can change the lockfile or CI config.

• Forge: from git remote get-url origin. Decides how to fetch the head (pull/<N>/head, merge-requests/<N>/head, refs/changes/...), the name of the thing (PR, MR, change), anchor format and suggestion syntax. Self-hosted and unrecognised? Ask. CLI missing or logged out? Fall back to plain git fetch and ask for the description.
• Stack: manifests and lockfiles name the language, package manager, toolchain pins and frozen-install command. Monorepo? Note which packages the diff touches.
• Gates: read the CI config first (.github/workflows/, .gitlab-ci.yml, bitbucket-pipelines.yml, Jenkinsfile...). What CI runs is what will judge the PR. Makefile, bin/, script/ and package.json scripts usually wrap the same steps.
• Runtime: how the app boots (compose, Procfile.dev, bin/dev), what services it needs, which mock layer exists, and whether the diff renders in a browser.

Diff against the merge base (git diff <base>...<head>, three dots). Two dots against a moved base shows other people's commits as the PR's.

Toolchain pins to copy: .tool-versions, mise.toml, .nvmrc, .node-version, .ruby-version, .python-version, rust-toolchain.toml, global.json, the packageManager field. A CI step that needs secrets or deploy access does not run locally: list it under what could not be checked. Monorepo (pnpm-workspace.yaml, turbo.json, nx.json, Cargo or Go workspaces): run the gates for the changed packages and their dependents.

Save the profile to <state>/profile.md (see Output template) with the head SHA it was built from. Reuse it on the next review unless a lockfile, toolchain pin or CI file changed since that SHA. Copy it as the first line of the validation run, e.g. "GitLab MR, Rails 7 + Hotwire, Ruby 3.3, gates from .gitlab-ci.yml: rspec, rubocop, brakeman."

Running the PR

• Copy the project's toolchain pins into the worktree and install with the lockfile frozen, using the command the profile names.
• Run the project's own gates, the ones CI runs. Report what ran and what did not.
• Serve the frontend on a port the user is not using. Their dev server is serving their branch, not the PR's.
• No backend? Use the project's own mock layer (MSW handlers, fixtures, cassettes, factories) instead of starting services against the user's local database. Compose stacks run under their own project name (-p review-<N>) and ports, or not at all. Seed what the changed flow needs, and mark it (rule 7).
• Name everything you start after the review, so the cleanup can find it without guessing: compose with -p review-<N>, docker run with --name review-<N>-<service> and --label review-that-runs=<N>, test databases and schemas as review_<N>. Never reuse a container, volume or database the user already has.

• Anything not in Docker (dev server, test runner, watcher, tunnel) starts in its own process group and is written to <state>/<pr|mr|change>-<N>/pids the moment it is up: one line per process, <pgid> <port or -> <command>. Memory of what was started does not survive a crashed or interrupted session; the file does.

  setsid pnpm dev --port 4173 > "$STATE/pr-<N>/dev-server.log" 2>&1 &
  echo "$! 4173 pnpm dev --port 4173" >> "$STATE/pr-<N>/pids"

Cleaning up

Runs as the last step, and also when the review aborts, errors out or is interrupted by the user: do the teardown first, then explain what went wrong. Nothing waits for the user to ask.

Keep the notes file and <state>/profile.md. Then check: git -C <repo> worktree list shows only the user's worktrees, docker ps -a --filter label=review-that-runs=<N> and docker compose -p review-<N> ps are empty, the pids file is gone and every port it listed is free. A pids file found at the start of a review is a previous run that did not get this far: tear it down first. If a teardown step fails, the validation run names what was left and the exact command to remove it.

Frontend validation with Chrome DevTools MCP

Server-rendered templates (ERB, Jinja, Blade, HEEx...) count as frontend.

• new_page on the served app, log in, navigate to the changed flow.
• take_snapshot to find elements; screenshots are evidence, not discovery.
• Measure layout with evaluate_script over the real DOM: innerWidth, the container's clientWidth, the table's scrollWidth. State the viewport you measured at, and resize_page when the finding is about a narrower screen.
• Try the change as each affected role, including one that should not see it.
• list_console_messages at the end. New errors are findings.

Severity

Open each comment with its severity as a plain sentence:

Output template

Write the notes to <state>/<pr|mr|change>-<N>/review-notes.md, using the forge's term, where <state> is ${XDG_STATE_HOME:-$HOME/.local/state}/review-that-runs/<host>/<path>, the host and path of the origin URL without .git (a local remote has no host: use its path alone). Nothing the review writes goes inside the reviewed repo; tell the user the notes path at the end:

# <PR|MR|Change> #<N> review notes (<forge>)

## Overview

<Business-level description, 2-5 sentences. Name any REVIEW SCRATCH edits.>

*Verdict: <does what was asked / misses X / also does unrelated Y>*

## Validation run

<The project profile. Then one line per pass that actually ran, one line for what
could not be checked and why, and one line naming what was torn down, or what was left
behind and the command that removes it.>

## Comments

<!-- anchor path="path/to/file.ext" start_line=N line=M side=RIGHT -->

<Lead-in. One short paragraph: what breaks and what it costs. At most one evidence
block: a suggestion, a diff, or input -> expected vs got.>

*Verified: <how you confirmed it>*

---

<!-- anchor path="path/to/other.ext" line=N side=RIGHT -->

<next comment>

*Inferred: <what you read, and why you could not test it>*

Line numbers are head-branch numbers on lines the diff shows. side=RIGHT is GitHub's term; on other forges it is the new-file line. Use the forge's suggestion syntax (```suggestion on GitHub, ```suggestion:-0+0 on GitLab, a plain ```diff elsewhere) only when the fix is exactly the anchored lines rewritten: Apply replaces the whole anchored range. When posted, the review goes out as a plain comment, with no approval, vote or change request. Those are a human's call.

When the notes are posted, the review body ends with this line, verbatim:

First pass by review-that-runs. Approving is a human's call.