pr-review

pr-review (Claude-side)

Goal

Run a comprehensive specialist review of the current branch's committed changes against its base, then render a single actionable report. This is the Claude Code counterpart of the Codex $pr-review skill for environments where Codex is unavailable; both share one gate policy (references/review-criteria.md) and one severity table (references/severity-rules.json). Design: docs/design/claude-pr-review.md (dotfiles repo), decision: ADR 0029.

Architecture note (read first)

gh does NOT work inside workflow subagents (sandbox-pinned). Everything that needs gh or dangerouslyDisableSandbox happens HERE in the main session (Preconditions + Collect). The workflow receives all of it as args and uses only git. Do not move base resolution into the workflow.

Preconditions

Run these in order. If any fails, abort with the indicated error; do not launch the workflow.

Do not change repository or git state to make a failing precondition pass: no git stash or commit on the user's behalf, no .gitignore / .git/info/exclude / git update-index --assume-unchanged / --skip-worktree edits, no deleting untracked files. Report the failing state and stop; the user decides how to resolve it. The ref updates that Base ref resolution itself prescribes (git fetch writing FETCH_HEAD, git remote set-head origin --auto) are part of the procedure, not a workaround, and stay allowed.

1. Workflow tool available — This gate runs entirely inside ~/.claude/workflows/pr-review.js. Treat the Workflow tool as available if it is callable: it appears in your tool list, or ToolSearch("select:Workflow") returns its schema. Abort only when neither holds:

   "Workflow is not exposed in this session, so the gate cannot run. On the Pro plan dynamic workflows are off by default — turn them on from the Dynamic workflows row in /config, then start a new session (disableWorkflows in a settings file and CLAUDE_CODE_DISABLE_WORKFLOWS also switch them off). Otherwise run the Codex CLI $pr-review from a terminal (not from inside Claude Code — nested-bwrap). Do not substitute a manual Agent fan-out of the specialists: that drops the workflow's coverage echo gate and severity normalization, reintroducing exactly the partial-coverage-reported-as-full outcome this gate exists to prevent."

   This check comes first because everything below it — gh pr view and git fetch under dangerouslyDisableSandbox, the diff packet, the reference sentinels — is wasted work on a run that cannot reach the workflow.

   Second check — no project-scoped workflow shadows the gate. The Workflow tool resolves the name pr-review by each script's meta.name: it registers user scope ~/.claude/workflows/ first, then lets any parseable project-scope .claude/workflows/ script with the same name override it. A project copy would run in place of this gate, and the render-side argsContract check cannot tell them apart. The check matches the fixed string pr-review anywhere in any file under those directories. That catches the plain spellings (quoted key, backtick value, any extension, subdirectory, symlinked file), but a name written with string escapes such as 'pr\x2dreview' or assembled at runtime such as 'pr-' + 'review' evades it. The check exists to catch an accidentally placed same-name workflow. It is not a defense against a hostile branch under review, which can also steer the specialists through the diff content itself. Run this block verbatim. It is glob-free, starts from the physical path because git rev-parse --show-toplevel resolves symlinks while $PWD does not, and fails closed when a directory or file cannot be read:

   top=$(git rev-parse --show-toplevel) || exit 1
   d=$(pwd -P)
   while :; do
     if [ -d "$d/.claude" ] && [ ! -x "$d/.claude" ]; then
       echo "ABORT: cannot search $d/.claude for project workflows" >&2; exit 1
     fi
     if [ -e "$d/.claude/workflows" ]; then
       find -L "$d/.claude/workflows" -type f -exec sh -c '
         for f do
           LC_ALL=C grep -qF pr-review "$f"
           case $? in
             0) echo "$f" ;;
             1) ;;
             *) echo "ERROR: cannot read $f" >&2; exit 2 ;;
           esac
         done
       ' sh {} + || { echo "ABORT: shadowing check failed under $d/.claude/workflows" >&2; exit 1; }
     fi
     [ "$d" = "$top" ] && break
     [ "$d" = / ] && break
     d=$(dirname "$d")
   done
   

   If the block exits non-zero, abort with its stderr. If it prints any path, abort with:

   "A project-scoped .claude/workflows/ file mentions pr-review (<printed paths>); a script declaring that name would override the user-scope gate. Remove, rename, or move it out of .claude/workflows/, then retry."

2. Clean worktree — Run git status --porcelain --untracked-files=normal. If the command fails, abort with its output. If output is non-empty, abort with:

   "Worktree has uncommitted changes: <list>. The review covers committed branch diff only; uncommitted changes would be silently excluded. Commit or stash first, then retry."

   Sandbox caveat: in repos whose tracked names collide with sandbox baseline denies (e.g. a chezmoi source dir), sandboxed git status reports ghost char-special entries (crw-rw-rw- nobody nogroup 1, 3 under ls -la) as untracked. Verify with ls -la | grep '^c' and re-run the status check outside the sandbox before trusting a non-empty result.

3. Base ref resolution — Determine $BASE from one of three sources, in priority order:

   • (a) Explicit base from the user prompt (e.g. --base develop, "review against develop"): use it verbatim and skip gh. If it matches a full 40-character hexadecimal commit OID (the same shape the workflow's args validation requires), set $BASE_REF to it. Otherwise validate it as a branch name (git check-ref-format --branch "$BASE"; reject leading -/+, :, or other refspec separators), run git fetch --quiet origin "refs/heads/$BASE" (abort on failure), set $BASE_REF=FETCH_HEAD, and resolve $BASE_COMMIT immediately.
   • (b) --allow-no-pr in the prompt: skip gh. Run git fetch --quiet origin and git remote set-head origin --auto (abort if either fails), then git symbolic-ref --quiet --short refs/remotes/origin/HEAD (abort if it fails or is empty). Strip origin/ for $BASE, keep the full origin/<branch> as $BASE_REF, and add a **Degraded coverage**: no PR base, fell back to default branch line to the final report.
   • (c) Open PR: run gh pr view --json baseRefName,baseRefOid --jq '[.baseRefName,.baseRefOid] | @tsv'. gh typically needs dangerouslyDisableSandbox: true (macOS Seatbelt / hosts.yml restrictions). If it fails with a real credential error, abort with "Run gh auth login and retry." Only an explicit "no pull request found" result is the no-PR case; any other failure aborts loudly. On success: validate the branch name with the same rules as (a), git fetch --quiet origin "refs/heads/$BASE" (abort on failure), verify FETCH_HEAD^{commit} equals the returned baseRefOid exactly (abort with both OIDs if not), then set $BASE_REF=FETCH_HEAD.

   If none of (a)–(c) yields a base, abort with:

   "No PR found for the current branch and no explicit base provided. Create a draft PR first (so the review pins the PR's base ref), provide an explicit base, or pass --allow-no-pr to fall back to the default branch (residual scope-divergence risk acknowledged)."

4. Pin the base commit — BASE_COMMIT=$(git rev-parse --verify "$BASE_REF^{commit}") immediately after assigning $BASE_REF; record the exact output. Abort on failure.

Collect (main session)

1. HEAD_REF=$(git rev-parse HEAD) — record it.

2. Empty diff check + changed files: run git diff --name-only "$BASE_COMMIT...$HEAD_REF" and record the output as changedFiles (one path per array element — this main-session list is the authoritative specialist-routing input; the workflow refuses to re-derive it from an agent echo). If empty, run the Final guard below, then report No committed changes relative to <base>; nothing to review. and stop.

3. Diff packet: diff_packet=$(mktemp "${TMPDIR:-/tmp}/pr-review-diff.XXXXXX") (suffix-free template — suffixed templates collide on BSD mktemp), then git diff "$BASE_COMMIT...$HEAD_REF" > "$diff_packet". Record byte count (wc -c) and SHA-256 (sha256sum / shasum -a 256). Abort if any step fails. The packet is authoritative for all specialists.

4. Deterministic routing flags (grep floor — the workflow OR-composes these with the categorizer agent's semantic judgment, so an agent false negative can never narrow the specialist fan-out; grep false positives only widen it, which is the safe direction). The patterns live in a deployed script so they run identically every time and are covered by tests/bats/test_pr_review_routing_flags.bats:

   sh ~/.claude/skills/pr-review/scripts/routing-flags.sh "$BASE_COMMIT" "$HEAD_REF"
   

   Require exit 0 and stdout exactly one line matching commentChanges=(true|false) typeChanges=(true|false); record both booleans from it. Do not compute or guess the flags any other way. On any other outcome, abort with:

   "Routing-flag script failed at ~/.claude/skills/pr-review/scripts/routing-flags.sh: <exit code, stdout, stderr verbatim>. If the file is missing or its output format differs, run chezmoi apply -v, then retry."

5. Shared references (deploy-skew defense — file existence alone is not enough):

   • Read ~/.claude/skills/pr-review/references/review-criteria.md and verify it contains the sentinel PR_REVIEW_CRITERIA_SHARED_V1. Keep the full contents as criteria.

   • Read ~/.claude/skills/pr-review/references/severity-rules.json, parse it as JSON, and verify sentinel == "PR_REVIEW_SEVERITY_RULES_V1" and version == 1. Keep the parsed object as severityRules.

   • If either check fails, abort with:

     "Shared gate policy missing or stale at ~/.claude/skills/pr-review/references/. Run chezmoi apply -v to redeploy, then retry."

   • Run python3 ~/.claude/skills/pr-review/scripts/validate_finding_evidence.py --help. If it does not exit 0, abort with:

     "Evidence validator missing or not runnable at ~/.claude/skills/pr-review/scripts/validate_finding_evidence.py (needs python3). Run chezmoi apply -v to redeploy, then retry."

     The gate cannot finish without it (see Validate verifier evidence), so a missing validator fails here instead of after a full review.

Launch the workflow

Invoke the Workflow tool by the predefined workflow's name parameter. chezmoi deploys the script at ~/.claude/workflows/pr-review.js, and the tool resolves it from its meta.name. Do not pass a script path: the tool's scriptPath parameter only accepts paths inside the working directory or an added directory, so the deployed file is rejected there (accepted on Claude Code 2.1.247, rejected from 2.1.259 on). Pass every value as real JSON (objects/arrays, not JSON-encoded strings):

Workflow({
  name: "pr-review",
  args: {
    base: "<$BASE>",
    baseCommit: "<$BASE_COMMIT>",
    headRef: "<$HEAD_REF>",
    packetPath: "<$diff_packet>",
    packetBytes: <byte count>,
    packetSha: "<sha256>",
    changedFiles: ["<path>", ...],
    commentChanges: <boolean from the Collect grep floor>,
    typeChanges: <boolean from the Collect grep floor>,
    criteria: "<contents of review-criteria.md>",
    severityRules: <parsed severity-rules.json object>
  }
})

If the Workflow tool itself refuses the call (the name does not resolve, or it reports a tool input error), abort with:

"pr-review workflow launch was refused by the Workflow tool: <tool error verbatim>. If the error says the name is unknown or the script failed to load, run chezmoi apply -v, then retry. Otherwise the args built above are malformed; fix them and retry."

Do not work around a refusal by copying the script elsewhere, launching it through the scriptPath parameter, adding a directory with /add-dir, or fanning out the specialists with Agent by hand. Each of those runs something other than the deployed, reviewed gate.

The workflow validates args (including both sentinels) and fails closed on any coverage mismatch, so a thrown workflow error is a gate failure — report it verbatim and stop; never retry with weakened inputs or partial coverage.

Among those validations are two review size limits, one on the diff packet's byte count and one on the changed-file count; the thrown error names both the limit and the actual value, so the numbers live only in the workflow. They exist because sha256sum succeeds at any size while a specialist's own read of the packet truncates: an oversized packet would satisfy the coverage echo gate and silently return a partial review reported as full coverage. A refusal on either limit is a policy decision, not a bug — split the branch into smaller PRs or narrow the base. Do not work around it by trimming changedFiles, splitting the packet, or re-running with a subset; any of those reintroduces exactly the partial-coverage-reported-as-full outcome the limits prevent.

The workflow runs in the background; wait for its completion notification before rendering. Do not start other work that mutates this repository while it runs.

Final guard (after the workflow returns)

1. Re-run git status --porcelain --untracked-files=normal. If non-empty, abort with:

   "Review subagents or concurrent tooling changed the worktree: <list>. These changes were not part of the reviewed committed diff. Revert, commit, or stash them, then retry."

2. Re-run git rev-parse HEAD and compare with the recorded $HEAD_REF. If it differs, abort with:

   "HEAD changed during review: started at <old>, now <new>. The completed specialist results do not cover the current commit. Re-run the review."

3. Leave the diff packet in place. Do not delete it or any other temp file this skill creates under ${TMPDIR:-/tmp}: an rm matches the Bash(rm:*) ask rule and stops the run for a permission prompt. The files are created with mktemp (owner-only), and the OS cleans up the temp directory; inside the Claude Code sandbox $TMPDIR is a per-user directory.

Result contract

Read the result from the task output file named in the workflow's completion notification (<output-file>). Parse the whole file as JSON and use its result field: the file wraps the workflow's return value with run metadata (summary, logs, agentCount, totalTokens, totalToolCalls, workflowProgress). Never parse the notification's inline <result>, which can be truncated. If the file cannot be read, is not valid JSON, or has no result object, abort with:

"pr-review workflow result is unreadable at <output-file>: <error verbatim>. The run's outcome is unknown; re-run the review."

The workflow returns a structured object (argsContract, critical, important/importantOverflow/importantTotal, suggestions/suggestionsOverflow/suggestionsTotal, strengths, refuted, verifications, tally, specialists, stage2Ran). Before using any of it, verify argsContract is exactly PR_REVIEW_ARGS_V3; if it is missing or different, a stale deployed workflow handled the run and returned no verifier evidence — abort with:

"Stale ~/.claude/workflows/pr-review.js deployed (argsContract mismatch). Run chezmoi apply -v, then re-run the review."

Validate verifier evidence

A verdict decides whether a finding stays in the fix queue, so every cited line must exist in the reviewed commit. The workflow checks only the shape of each citation; this step checks it against the $HEAD_REF tree. Never validate against the worktree.

1. Verify result.verifications.length equals result.tally.critical + result.tally.important + result.tally.refuted (every verified finding, including refuted and capped-out ones). If it differs, abort with:

   "Verifier evidence is incomplete: <length> verifications for <sum> verified findings. Re-run the review."

2. Create a temp directory outside the worktree: evidence_dir=$(mktemp -d "${TMPDIR:-/tmp}/pr-review-evidence.XXXXXX").
3. For each entry v in result.verifications, write the JSON object {"candidate_id": v.candidateId, "evidence": v.evidence} to $evidence_dir/<v.candidateId>.json with the Write tool. Never pass evidence text through a shell command line.
4. For each entry, run:

   python3 ~/.claude/skills/pr-review/scripts/validate_finding_evidence.py \
     --repo-root "$(git rev-parse --show-toplevel)" --head-ref "$HEAD_REF" \
     --result-file "$evidence_dir/<candidateId>.json"
   

   Require exit 0 and stdout exactly EVIDENCE_OK finding-verifier <candidateId> <$HEAD_REF> <number of items in v.evidence>. On any other exit code or output, abort with:

   "Verifier evidence for <candidateId> ([<specialist>] <file>, verdict <verdict>) does not resolve at <$HEAD_REF>: <validator stderr or stdout verbatim>. The verdict is unproven, so the gate result is not trustworthy. Re-run the review."

5. Leave $evidence_dir in place (see Final guard step 3).

Do not drop, edit, or re-verify a failing verdict to make this step pass: a citation that does not resolve means the verdict itself is unproven.

Render the result

Render the validated result as:

# PR Review: <branch> vs <base>

<**Degraded coverage** line if --allow-no-pr was used>

Specialists: <result.specialists, comma-separated> | scope `<result.scope>` | packet `<first 12 chars of packetSha>`

## Critical Issues (N found)
- [<specialist>] <why> [<file>:<line>]
  - Impact scope: <impact_scope>
  - Verified assumptions: <verified_assumptions>
  - Unverified assumptions: <unverified_assumptions; should be empty for Critical>
  - Verdict: <verdict>(<verdictReasoning>)
  - Evidence: <path>:<line> — <observation> (one line per evidence item)
  - Suggested fix: <fix>

## Important Issues (shown X of <importantTotal>, cap 5)
- [<specialist>] <why> [<file>:<line>] — verdict: <verdict><, missing verification: ... if present>

<if importantOverflow is non-empty:>
### Beyond cap (verified but not prioritized)
- [<specialist>] <one-line summary of why> [<file>:<line>] — verdict: <verdict>

## Suggestions (shown X of <suggestionsTotal>, cap 3)
- [<specialist>] <why> [<file>:<line>]

## Refuted by verification (excluded from the fix queue)
- [<specialist>] <why> — <verdictReasoning>
  - Evidence: <path>:<line> — <observation> (one line per evidence item)

## Strengths
- [<specialist>] <note>

## Recommended Action
1. Fix Critical issues first
2. Address Important issues
3. Consider Suggestions
4. Re-run this skill after fixes to verify prior Critical/Important findings
5. Stop when Critical 0 / Important 0; do not re-run for Suggestions only
6. If this is the third or later pass and Critical/Important findings still churn, escalate to human judgment

<!-- pr-review-tally: <result.tally as compact JSON> -->

Emit the trailing tally comment verbatim from result.tally — serialize the object as compact JSON, never recompute the counts from the rendered sections, which are capped and would disagree with it. The counts are pre-cap, so important there can legitimately exceed the 5 shown above; that gap is the standing measure of how much the caps suppress. tally.bySpecialist breaks the same counts down per specialist, with verifier outcomes, for deciding which specialists earn their cost. A CI step or a later pass can read the line with grep -o 'pr-review-tally: .*[}]' | cut -d' ' -f2-.

Omit empty sections (except render ## Critical Issues (0 found) explicitly — the absence of Criticals is the gate's headline). Findings with verdict: needs-verification and non-empty missingVerification stay in the fix queue as Important with their missing verification stated; never silently drop them, but do not render them as Critical until the missing proof exists. Other verdicts carrying missingVerification, or needs-verification without it, are invalid verifier outputs and must fail closed.

Re-review

On a re-run after fixes, prioritize confirming whether each prior Critical/Important finding was fixed, remains, or was intentionally rejected. Raise new findings only when they are clear merge blockers; do not extend the loop with new nits, style feedback, or optional refactors. If the result is Critical 0 / Important 0, stop the gate loop. If only Suggestions remain, do not re-run the gate just to chase them. On the third and later pass, if Critical/Important findings keep appearing or changing without stable blocker evidence, call out possible review churn and return the decision to a human maintainer. (This guidance also reaches the specialists via the bundled criteria.)