Review an existing AccessFlow pull request — check the head out into an isolated worktree, fan out the read-only agents (change summary, conventions, security, frontend, cross-cutting drift), explain what the PR actually does, then merge the findings into one numbered list and let you pick which ones to keep. Never posts to GitHub; renders the selected findings as markdown for you to paste. Trigger when the user says "review PR
Review an existing AccessFlow pull request — check the head out into an isolated worktree, fan out the read-only agents (change summary, conventions, security, frontend, cross-cutting drift), explain what the PR actually does, then merge the findings into one numbered list and let you pick which ones to keep. Never posts to GitHub; renders the selected findings as markdown for you to paste. Trigger when the user says "review PR
Review a GitHub pull request
CLAUDE.md is the authoritative rulebook and the agents under
.claude/agents/ do the actual reviewing. This skill only adds the
workflow that gets a PR onto disk, fans them out, and turns their reports into something you can
choose from.
Three properties define it:
It explains the PR before it critiques it. The first thing you get is what the change
actually does — read from the diff, not from the description — so the findings that follow have
somewhere to land.
Your working tree is never touched. The PR head goes into a throwaway git worktree. You can
review a PR in the middle of your own work, on a dirty tree, without stashing.
It never writes to GitHub. Not a comment, not a review, not an API call. The selected
findings come back as markdown you paste yourself.
Inputs
A PR number (735), a #-prefixed number (#735), or a GitHub pull-request URL. Optional flag:
Flag
Effect
--verify
Also dispatch af-verifier to run the real build gates locally. Slow — a backend PR pays a full mvn verify -Pcoverage. Without it, report gh pr checks instead, since CI already ran them.
If the user omits the number, ask for it once and stop. Do not guess from gh pr list — the
wrong PR reviewed silently is worse than a question.
Workflow
1. Resolve the PR
gh pr view <n> --json number,title,body,url,state,author,headRefName,headRefOid,baseRefName,isCrossRepository,additions,deletions,files
gh pr checks <n>
gh api repos/<owner>/<repo>/pulls/<n> --jq .base.sha # the PR's base commit — see below
gh pr checks exits non-zero when checks are failing or pending — that is data, not an error;
capture the output and carry on.
base.sha is load-bearing — do not substitute origin/<baseRefName> for it. For a merged
PR the head is already an ancestor of the base branch, so git merge-base HEAD origin/main returns
HEAD itself and the diff comes back empty — every agent would then report "no files touched,
VERDICT: approve", which is the worst possible failure: a silent false pass. base.sha is the
commit the PR was actually opened against, and it gives the right diff for open and merged PRs
alike. Sanity-check it: git diff --shortstat <base.sha>...<headRefOid> must match the
additions/deletions from gh pr view. If it does not, stop and say so rather than reviewing a
diff you cannot account for.
(Note gh pr view --json files caps at 100 entries, so on a large PR its file list is short of the
real count. Use it to pick which agents to dispatch, but take counts from git diff.)
If state is MERGED or CLOSED, say so and ask whether to continue before spending the agents.
Reviewing merged code is legitimate (post-mortem, learning the codebase) but rarely what someone
means by "review this PR".
2. Check the head out into an isolated worktree
git fetch origin <baseRefName>
git fetch origin pull/<n>/head # works for fork PRs too — do not add a fork remote
git worktree add --detach .claude/worktrees/pr-<n> <headRefOid>
git -C .claude/worktrees/pr-<n> cat-file -e <base.sha>^{commit} || git fetch origin <base.sha>
Detached HEAD is correct and sufficient: the worktree shares the parent .git, so both the head
and base.sha resolve in it. Confirm the diff is non-empty before spending any agent:
An empty result here means something is wrong with the refs — never dispatch on it.
If .claude/worktrees/pr-<n> already exists from an earlier run, remove it and recreate — a stale
worktree pinned to an older head silently reviews the wrong code.
Nevergh pr checkout, and never git checkout in the user's tree. Switching their branch is
the one thing this skill exists to avoid.
3. Fan out the reviewers
Pick from .files[].path in the step-1 JSON, and dispatch all of them in a single message so
they run concurrently.
Condition
Agent
always
af-pr-summarizer — what the change does, per-area breakdown, interface surface (endpoints, migrations, enums, config, events), and where the risk concentrates. Describes; does not judge.
af-java-reviewer — CLAUDE.md backend rules + the backend/engine pattern checklists.
any path under frontend/ or e2e/
af-frontend-reviewer — the frontend non-negotiables + the frontend/e2e pattern checklists.
--verify only
af-verifier — maps touched paths onto the gates that apply and runs them.
Every agent prompt must carry these two overrides verbatim, because each agent's body assumes it
is reviewing a local branch in the primary working tree:
Your review root is <absolute worktree path>. Begin every Bash call with
cd <absolute worktree path> and treat every relative path in your instructions as relative to
it. Wherever your instructions say $(git merge-base HEAD origin/main), use the literal commit
<base.sha> instead — this is PR #<n> against <baseRefName>, and on a merged PR the
merge-base ... origin/main form silently yields an empty diff. Your diff is
git diff <base.sha>...HEAD, and it covers <n> files.
Also pass the PR title and body, so a reviewer can tell whether an omission is deliberate.
If you get Agent type '…' not found, the session predates the agent file — agents load at session
start. Tell the user to restart; do not silently drop that reviewer from the run.
Say which agents you dispatched and which you skipped, and why. A frontend-only PR should
produce "af-java-reviewer not dispatched — no backend/ or engines/ paths", not silence.
4. PR hygiene pass
Do this yourself from the step-1 metadata — no agent has it, and none of it needs the code:
CI: the gh pr checks result. A failing required check outranks every style finding below.
Branch name matches (feature|fix|chore)/AF-<n>-description per CLAUDE.md → Git Workflow.
Body references the issue (Closes #<n> or an AF-<n> token).
Size: flag anything over ~800 changed lines as hard to review in one pass, and say which files
dominate it.
Drift declared: if the diff touches user-facing surface, does the body list the touched
docs/ and website/ files, per impl-gh-issue's definition of done? (Whether they should
have been touched is af-reviewer's call — you only check whether the description says so.)
5. Merge the reports
Collate everything into one set:
Two reviewers on the same path:line → keep one. The specialist beats af-reviewer;
af-security-reviewer beats the specialist when both claim it, because the security framing
is the one that decides severity.
The overall verdict is the strictest returned: revise > approve-with-concerns > approve.
An agent's NOT CHECKED items are not clean. Carry them into your summary as unverified.
af-pr-summarizer contributes no findings. Its output is the briefing in step 6, never an
entry in the numbered list. If it noted something that smells wrong, that is a lead for a
reviewer, not a finding of its own.
6. Lead with what the PR does
Relay af-pr-summarizer's briefing before any finding — what it does, why, the per-area
breakdown, the interface surface, notable decisions, and where the risk concentrates. This is the
part someone reads to decide whether they even agree with the change; findings about a change
nobody understands are noise.
Two things from it are load-bearing and must not be dropped when you relay it:
DESCRIPTION ACCURACY. If the PR body claims something the diff does not do, or the diff
does something the body never mentions, that goes near the top in its own right. Every human
reviewer after you is anchored on that description.
INTERFACE SURFACE. Endpoints, migrations, enum values, config knobs, events, engine pins.
These are what break other people, and an explicit "no migrations, no new endpoints" is as useful
as a list.
Scale it to the change: a three-file PR gets a short paragraph, not a filled-in template with
"none" six times.
7. Present the findings
One numbered list, numbering continuous across severities so a selection like 3,7-9 is
unambiguous. Group Blockers → Concerns → Nits, hygiene findings included and numbered like any
other. Each entry: the path:line, what is wrong, the reviewer that raised it, and the one-line
consequence.
Close with the merged verdict, the CI status, and anything left unverified.
8. Let the user select
Ask for a selection and accept any of:
1,3,7-9 blockers blockers+concerns all none
Follow-up edits are part of this step — "drop 5", "keep 3 but reword it", "merge 8 and 9". Iterate
until they are happy.
Say plainly, once: nothing has been sent anywhere, and nothing will be.
9. Render — do not post
Open with two or three sentences of the what it does summary, so the pasted comment stands on
its own for anyone reading it on GitHub, then the selected findings only, grouped by severity.
Include DESCRIPTION ACCURACY if it flagged a real mismatch — that is a comment worth making even
when every other finding is dropped. Link each finding to its exact line at the reviewed commit:
Use the commit SHA, not the branch name — a branch-relative link rots the moment the author pushes.
Then do all three:
write it to the scratchpad as pr-<n>-review.md and send it with SendUserFile,
pipe it to pbcopy so it is on the clipboard ready to paste,
print it in the transcript as one fenced block.
Non-negotiable: this skill never runs gh pr comment, gh pr review, gh api -X POST, or any
other GitHub write. Not when asked mid-run either — if the user wants it posted, say that this
skill does not post and let them paste, or let them ask for a posting step as a separate,
explicitly-confirmed action.
10. Clean up
Ask once whether to keep the worktree — after a review with real Blockers, digging around in the
actual code is often the next thing wanted. Otherwise:
Never leave a worktree behind without saying where it is. If the user keeps it, tell them the
path and the cleanup command.
Acceptance checklist
PR resolved by number/URL; merged or closed state surfaced before spending agents.
Head checked out into .claude/worktrees/pr-<n>; the user's branch and working tree untouched.
af-pr-summarizer, af-reviewer and af-security-reviewer dispatched; path-matched
specialists dispatched; af-verifier only under --verify; all in one message.
Summary relayed before the findings, carrying DESCRIPTION ACCURACY and
INTERFACE SURFACE.
base.sha resolved and its git diff shortstat reconciled against gh pr view's totals.
Every agent prompt carried the review-root and <base.sha> overrides.
NOT CHECKED items carried through as unverified, not dropped.
User selected; selection honoured exactly.
Markdown rendered to scratchpad + clipboard + transcript. Nothing posted to GitHub.
Worktree removed, or its path and cleanup command handed to the user.
Anti-patterns
Reviewing from gh pr diff alone. Most real findings come from repo-wide greps and reading
full files — the fan-out sweeps, the @JsonIgnore sweep, the permitAll ↔ docs diff. A diff
cannot produce any of them. The worktree is the point.
gh pr checkout in the user's tree. Switches their branch, needs a clean tree, and strands
them somewhere else if the skill errors mid-run.
Posting anything. Including "just a quick summary comment". The user chose to paste.
Padding the list. An empty review is a real result. Say "no Blockers" and mean it.
Reporting NOT CHECKED as clean. A gate that was not run is NOT_RUN, never PASS — the
same rule the agents live by applies to your summary of them.
Reviewing a stale worktree from a previous run of the same PR number.
Diffing against origin/main on a merged PR. The head is already an ancestor, the diff is
empty, and every agent returns a confident approve on nothing. Always base.sha.
Letting a failing CI check get buried under nits. It goes at the top.
Opening with findings. A list of defects about a change the reader cannot yet describe is
unreadable. The summary comes first, always.
Relaying the PR description as the summary. The description is a claim; the diff is the
evidence. Reporting the two agree, when they do, is a real result — asserting it without checking
is not.
