Gh Pr Review
Overview
Review eligible large or high-risk GitHub pull requests as a findings-first code review. Resolve the active PR with gh, build a local review bundle, run Claude Code's /pr-review-toolkit:review-pr, run lens-specific Codex sub-agents in parallel, then merge every concrete high, medium, and low severity finding into a severity-ordered report.
Eligibility Gate
Apply this gate before building a bundle, starting Claude, or delegating review work. Continue with this skill only when at least one condition is true:
- The user explicitly names
$gh-pr-review or asks for an exhaustive, multi-agent, or multi-lens review.
- The PR is materially large: roughly more than 20 changed files or 1,000 changed lines.
- The PR materially changes a high-risk boundary: cross-cutting architecture, authorization or tenant isolation, destructive migration or data movement, or release/deployment controls.
A file merely belonging to a sensitive area does not satisfy the gate; the diff must materially change that boundary.
When the gate is not satisfied, stop this skill and use a lightweight review:
- Read the changed diff directly.
- Use at most one targeted reviewer when delegation is explicitly authorized and materially useful.
- Run focused validation and the repository's normal exact-head CI gate.
- Do not build the review bundle, start Claude PR Review Toolkit, or wait under this skill's long-running policy.
Do not rerun the full skill after review fixes unless runtime behavior or the high-risk surface materially changes. Test-only, documentation-only, formatting, and review-comment follow-ups require targeted review or validation plus exact-head CI, not another exhaustive pass.
Inputs
repo: repository path. Default ..
pr: PR number or URL. Omit to use the PR for the current branch.
gh authentication with repository access.
Review Modes
- Bundle mode: Default.
prepare_pr_review.py succeeds and provides metadata.json, files.json, lens-hints.json, summary.md, and patch files.
- Manual fallback mode: Use only when bundle generation fails. Build equivalent coverage and lens decisions from
gh pr view --json ..., a saved patch, repository instructions, and direct file reads. Do not claim lens-hints.json was used in this mode.
Both modes must produce the same final output contract and exact changed-file coverage accounting.
Long-Running Wait Policy
This policy applies only after the Eligibility Gate passes. Claude PR Review Toolkit and lens sub-agents can legitimately take longer than 10 minutes on eligible large or high-risk PRs. Do not treat 10 minutes, a single empty wait_agent timeout, or a quiet Claude process as failure.
- For Claude, start the helper as a long-running command and keep polling the same session until it exits, errors, or the user asks to stop. Do not mark Claude unavailable solely because it has run for more than 10 minutes.
- For sub-agents, use long
wait_agent timeouts when supported. If a wait returns an empty timeout while the agent is still running, wait again instead of closing the agent.
- Keep doing coordinator direct reads while slow Claude or sub-agent work runs, but do not finalize until the required Claude/sub-agent outputs return or a real terminal failure occurs.
- Default patience budget: wait at least 45 minutes for eligible large, cross-cutting, migration-heavy, security-sensitive, or all-lens reviews. Waiting longer is appropriate when output is still progressing.
- Close or fallback only when the tool reports a final failure, the user asks to stop, or the process/agent is clearly non-responsive beyond the patience budget and direct coverage can replace it. In that case, report the exact wait time and fallback reason under
Residual risk.
- Send short progress updates during long waits so the user knows the review is still running.
Quick Start
- Apply the Eligibility Gate. Use the lightweight review path and stop this skill when the PR is not eligible.
- Verify GitHub CLI access.
- Read repository instruction files before reviewing.
- Start with repo-root or nearest
AGENTS.md, CLAUDE.md, and contribution docs.
- Treat repo-specific review requirements as hard constraints, not optional context.
- Build the review bundle.
python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "."
- Add
--pr "<number-or-url>" when the review target is not the current branch PR.
- Read
<bundle-dir>/summary.md.
- Read
references/review-lenses.md.
- Run Claude Code's PR Review Toolkit pass and save the raw output in the bundle.
- Launch only the recommended Codex lenses from
lens-hints.json, then merge all concrete findings from Claude and every lens.
Workflow
1. Resolve and materialize the PR
Always start by generating a review bundle with prepare_pr_review.py. The script writes:
metadata.json: PR metadata and counts.
files.json: changed-file list with additions, deletions, and extracted patch paths when available.
lens-hints.json: recommended lenses, selection notes, and per-lens file hints.
summary.md: compact human-readable overview.
diff.patch: full PR patch from gh pr diff --patch.
patches/<path>.patch: file-scoped patch files extracted from the full patch.
Prefer the script over repeated ad hoc gh commands so every sub-agent can point to the same local artifacts.
2. Read the bundle before delegating
Read summary.md first. Then read lens-hints.json and use recommended_lenses as the default sub-agent launch set. Each lens entry also includes reason and focus_files.
Before delegating, read the repository instruction files that govern the target repo, such as AGENTS.md, CLAUDE.md, and repo contribution docs. Convert any review-specific repo rules into explicit checks or lens prompt addenda. Examples include required review language, required architecture-doc updates, testing conventions, and known high-risk stacks such as Supabase or Sentry.
Build a short coordinator review plan before launching sub-agents. Keep it as scratch unless the user asks for it. The plan must include:
- Exact changed-file count and paths from
files.json.
- Repository review rules that affect this PR, such as output language, required tests, migration rules, and doc-update rules.
- Claude PR Review Toolkit configuration, output path, and whether it completed or fell back.
- Recommended lenses from
lens-hints.json, plus any lens added or removed by the coordinator and the reason.
- Exact assigned file scope for each lens.
- Coordinator direct-read targets, especially access-control gates, route handlers, migrations, destructive operations, failure-mode branches, external requests, generated-contract boundaries, and tests that should prove the changed behavior.
- Coverage gaps that must be closed before final output.
Do not launch every available lens by default. When the PR is small and clearly cross-cutting, you can still run all lenses. Otherwise, start with recommended_lenses, narrow each sub-agent to the most relevant file subset, and let it open extra source files only when needed for cross-file reasoning.
Use coverage_gaps from lens-hints.json as a hard stop before finalizing. If any changed file is not covered by the recommended lenses, either assign it to another lens or read it directly as the coordinator. Do not finalize while unread changed files remain.
For large or high-risk PRs, do not rely on sub-agent Findings: none outputs alone. If the PR changes more than roughly 20 files, or touches access control, route handlers, external network calls, migrations, destructive data changes, failure-mode branches, observability, or tenant-scope logic, the coordinator must run a direct hotspot sanity pass before finalizing. At minimum, open the changed helpers or handlers that gate access, the changed files that move/drop data or make external requests, and the changed tests that should prove the new behavior.
For high-risk migration or API compatibility PRs, the coordinator must directly inspect the changed migration/backfill/drop files and compatibility route/client/schema files even when lens-hints.json reports no coverage gaps. For destructive migrations, verify both key coverage and value equality before drop/rename, and treat skipped divergent target rows as reportable data-loss risk. For compatibility APIs, compare old and new permission predicates including self-service carve-outs, and verify response fields preserve semantic sources such as planned versus actual values.
For small PRs, the coordinator's direct read of all changed files is sufficient to count as the sanity pass. You do not need a second separate hotspot sweep when the entire diff is already small enough to read directly.
If the target repository has known high-risk conventions, state them explicitly in the sub-agent prompt. For example, call out Supabase RLS and policy regressions for the access-control lens, destructive migration parity for the data-integrity lens, and Sentry coverage with @sentry/nextjs for the observability-ops lens.
Manual fallback planning
When prepare_pr_review.py cannot produce a bundle, create a synthetic review plan before delegating or reviewing manually:
- Treat
gh pr view --json ... files as the synthetic files.json equivalent for exact changed-file coverage.
- Treat an existing saved patch path, such as
/tmp/pr-88.patch, as the synthetic diff.patch equivalent. Use it to identify changed hunks and line anchors.
- Do not rerun
gh pr diff --patch when a saved patch already exists. If no saved patch exists, try gh pr diff --patch > /tmp/<name>.patch; if that also fails, continue with gh pr view metadata plus targeted source reads and list the missing patch as residual risk.
- Read repository instruction files exactly as in bundle mode.
- Select lenses from changed paths and risk triggers:
access-control: authentication, authorization, roles, RLS/policies, middleware, route handlers, session/cookie/origin/redirect trust boundaries, tenant or object scope, and service-role boundaries.
security: secrets, injection, unsafe file/network handling, uploads, webhooks, CORS, crypto/password/token handling, sanitization, and redaction.
data-integrity: migrations, backfills, canonical data movement, schema changes, destructive drop/rename, repositories, transactions, duplicate/idempotency behavior, and value parity.
failure-modes: dependency failure, partial failure, degraded state, fallback, retry, timeout, broad catch/default branches, fail-open/fail-closed behavior, and required side-effect failures.
api-contract: API routes, client wrappers, schemas, validators, generated/public types, status codes, error payloads, docs for API behavior.
performance: runtime cost, N+1, repeated work, unbounded loops, caching, batching, bundle size, and request/render path regressions.
observability-ops: Sentry, structured logs, audit events, metrics, tracing, alert noise, operational investigation context, jobs, webhooks, and rollout visibility.
tests: changed production behavior without matching tests, weak regression assertions, migrations, or repo-specific coverage requirements.
frontend-ux: visible UI, forms, modals, navigation, responsive/accessibility behavior, client state.
maintainability: docs-only changes, cross-cutting helpers, large refactors, layering and ownership.
correctness: default for changed runtime logic and business behavior not fully covered by the more specific lenses.
- Assign every changed file to at least one selected lens or to coordinator direct read.
- In final
Coverage, write Reviewed sources: Claude pr-review-toolkit, <lenses> (manual fallback) when the Claude pass ran; otherwise write Reviewed sources: <lenses> (manual fallback) and list the Claude fallback reason under Residual risk. Never mention lens-hints.json recommendations in manual fallback mode.
Manual fallback sub-agent prompt template:
Manual fallback is active. Do not read or claim `lens-hints.json`.
## Available Artifacts
- Changed-file list: <exact paths from gh pr view --json ... files>
- Patch source: <saved patch path, or note if patch unavailable>
- Repo rules: <language/testing/docs/migration rules that matter>
## Assigned Scope
- Lens: <lens>
- Files: <exact assigned paths>
- Lens-specific checks: <copy only relevant checks from review-lenses.md>
## Required Method
1. Read the saved patch when available.
2. Open current source files when the patch is insufficient to verify behavior or line numbers.
3. Do not report a finding unless it has a changed-code anchor, violated invariant/contract, concrete failure mode, and plausible fix direction.
4. If no finding remains after false-positive and false-negative passes, write "Findings: none".
## Output
Reviewed files:
- `path/to/file`
Findings:
- `[severity][<lens>] path/to/file.ts:42 Issue summary. Why it matters.`
- `none`
Residual risk:
- Only include unread or unverifiable scope.
Review stability rules
Use these rules to keep outputs stable across runs:
- Treat
files.json as the source of truth for changed-file coverage. Do not infer coverage from area names, lens names, or broad summaries.
- Keep a coordinator finding ledger while merging:
source lens, path:line, severity, risk chain, decision (keep, duplicate, drop), and decision reason.
- Keep a hotspot checklist for every high-risk changed file. Mark each hotspot as
read, covered by lens, finding, or no finding after direct check.
- A finding is admissible only when it has all four parts: changed-code anchor, violated invariant or contract, concrete failure mode, and a plausible fix direction.
- Run a false-positive pass before final output. Drop items whose risk depends on unchanged code alone, missing context contradicted by source, or a line that no longer exists in the current diff.
- Run a false-negative pass before final output. If a high-risk changed file has no finding, explicitly re-check the relevant bug class: access/scope, data loss, contract drift, error handling, fallback behavior, observability, and test coverage.
- Use the fixed severity rubric below. Do not upgrade severity because a finding is interesting, and do not downgrade concrete low-severity findings because the report is long.
Severity rubric:
high: likely data loss, auth/tenant isolation break, secret exposure, destructive migration failure, broadly blocking runtime regression, or contract break that can make a critical user workflow fail.
medium: likely user-visible correctness regression, API/nullability drift, missing failure handling, meaningful test gap for changed critical behavior, performance/operations issue that can hide or amplify production failure.
low: concrete maintainability, test precision, documentation, accessibility, or minor UX issue tied to changed code, with limited immediate blast radius.
3. Run Claude Code PR Review Toolkit
Before or alongside Codex lens sub-agents, run Claude Code's pr-review-toolkit@claude-plugins-official command against the same repository state. Treat this as an additional independent review source, not as a replacement for Codex lens coverage or coordinator hotspot checks.
Default command:
node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>"
For a narrower or more exhaustive Claude pass, pass the same aspect words supported by Claude's slash command:
node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "tests errors"
node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "all parallel"
The helper writes Claude's raw stdout to <bundle-dir>/claude-pr-review.md. Read that file before final merge. Preserve Claude's review result as a source artifact: do not rewrite it in place, do not delete low severity findings, and do not treat a Claude "no findings" result as coverage for changed files unless its output names the files or the coordinator can verify that scope.
The final review must also include Claude's raw report without information loss. After the merged Codex review sections, add Claude PR Review Toolkit Report (verbatim) and paste the complete contents of <bundle-dir>/claude-pr-review.md unchanged. Do not summarize, reorder, deduplicate, translate, omit low-severity items, trim, indent, blockquote, or wrap Claude's raw report in a way that changes its content. If the Claude pass failed and no raw report exists, include Claude PR Review Toolkit Report (verbatim): unavailable with the exact failure reason from stderr or the helper output.
The helper defaults to --permission-mode auto and allows the read/review tools Claude's command needs: Bash(git *), Bash(gh *), Glob, Grep, Read, and Task. Keep it read-only unless the user explicitly asks for fixes.
If Claude Code, the claude-code Codex skill wrapper, or pr-review-toolkit@claude-plugins-official is unavailable, continue with the Codex lens workflow and list Claude pr-review-toolkit unavailable: <reason> under Residual risk. Do not block the PR review solely because the Claude pass failed.
Long Claude runs are not failures. If the helper is still running after 10 minutes, keep the session open and continue polling according to the Long-Running Wait Policy. Do not replace the Claude result with unavailable until the command exits with an error, the user stops the wait, or the explicit patience budget is exhausted.
When merging Claude output with Codex sub-agent outputs:
- Add every concrete Claude finding to the same coordinator finding ledger used for Codex lens findings.
- Use source label
claude-pr-review-toolkit.
- Keep the Claude finding text and severity as close to raw as practical, while normalizing final output to this skill's single
path:line and severity format.
- Merge Claude findings into the top-level
Findings list as Codex-review findings, but still include the complete raw Claude report later in Claude PR Review Toolkit Report (verbatim).
- Deduplicate only exact or semantically identical issues. If Claude and a Codex lens identify different failure modes on the same line, keep both.
- If Claude reports a finding without enough anchor evidence, re-check the patch/source. Keep it if the coordinator can anchor it; otherwise drop it as
line-not-found or speculative in the internal ledger.
- Include Claude in final coverage as
Reviewed sources: Claude pr-review-toolkit, <lenses>.
4. Spawn specialized sub-agents in parallel
Available lenses:
access-control
security
data-integrity
correctness
failure-modes
api-contract
performance
observability-ops
frontend-ux
tests
maintainability
Use sub-agents only when the user explicitly asked for a PR review, asked for this skill, or otherwise gave clear permission for delegated review work. If delegation is unavailable in the current environment, perform the same lens-based reasoning yourself and mention the fallback only when it materially affected coverage or confidence.
Long sub-agent runs are not failures. If an agent takes more than 10 minutes, continue waiting according to the Long-Running Wait Policy. If wait_agent times out with no final status, call wait_agent again instead of closing the agent, unless the user has asked to stop or the explicit patience budget has been exhausted.
Use spawn_agent with these fixed sub-agent settings:
agent_type: "explorer"
model: "gpt-5.4-mini"
reasoning_effort: "xhigh"
Example per lens:
{
"agent_type": "explorer",
"model": "gpt-5.4-mini",
"reasoning_effort": "xhigh",
"message": "Review the PR bundle at <bundle-dir> from the <lens> lens..."
}
Do not omit, downgrade, or vary those model settings between lenses. If the configured model is unavailable in the current environment, state the fallback explicitly in the final review under Residual risk. Pass the minimum context needed: the bundle directory, the review lens, and the output contract. Start with recommended_lenses and add more only if the diff or the user request calls for exhaustive review.
Use prompts shaped like this:
Review the PR bundle at <bundle-dir> from the <lens> lens. Start with summary.md, then inspect the relevant patch files and source files until no unread files remain in your assigned scope. Return a Reviewed files section and a Findings section. Report every concrete finding in your scope, including low severity findings. A finding must have a changed-code anchor, violated invariant or contract, concrete failure mode, and plausible fix direction. Use this severity rubric: high=data loss/access-control/tenant/secret/destructive migration/critical workflow break; medium=likely user-visible regression/API drift/failure handling/test gap/production observability risk; low=concrete maintainability/test precision/docs/accessibility/minor UX issue. Ignore only purely stylistic nits, non-actionable preferences, and speculative concerns that cannot be tied to changed code. If there are no issues, write "Findings: none".
For more stable agent outputs, prefer this expanded prompt template over improvising:
Review the PR bundle at <bundle-dir> from the <lens> lens.
## Assigned Scope
- Files: <exact paths from files.json or lens-hints.json>
- Repo rules: <language/testing/docs/migration rules that matter>
- Lens-specific checks: <copy only relevant checks from review-lenses.md>
## Required Method
1. Read summary.md and lens-hints.json.
2. Read each assigned patch file.
3. Open the current source file whenever the patch alone is insufficient to verify behavior or line numbers.
4. Do not report a finding unless it has a changed-code anchor, violated invariant/contract, concrete failure mode, and plausible fix direction.
5. If no finding remains after a false-positive pass, write "Findings: none".
## Output
Reviewed files:
- `path/to/file`
Findings:
- `[severity][<lens>] path/to/file.ts:42 Issue summary. Why it matters.`
- `none`
Residual risk:
- Only include unread or unverifiable scope.
Good delegation rules:
- Keep each agent focused on one lens.
- Prefer file subsets from
lens-hints.json for large PRs.
- Ask the agent to inspect source files in the repository when the patch alone is insufficient.
- Do not ask sub-agents to propose code changes unless the user asks for fixes.
- Use the union of all agents'
Reviewed files lists to verify full diff coverage before writing the final report.
- For
access-control, explicitly ask for Supabase RLS, policy, service_role, object-scope regressions, self-service carve-outs, CSRF/origin/session/cookie/redirect boundaries, and cross-endpoint guard consistency when the PR touches supabase/**, app/api/**, middleware, auth, or data-access code.
- For
security, explicitly ask for secrets, injection, unsafe file/network handling, webhook trust, upload handling, redaction, crypto/password/token handling, and privacy leakage.
- For
data-integrity, explicitly ask for row coverage and value parity before destructive operations, same-key conflicts, timestamp ties, on conflict precedence, idempotency, rollback, and archive-before-drop verification.
- For
failure-modes, explicitly ask whether dependency failures fail open or fail closed, whether fallback paths hide real failures behind defaults/placeholders, whether partial mutations roll back or compensate, and whether required side effects can fail invisibly.
- For
performance, explicitly ask for N+1 queries, duplicate reads, unbounded pagination/loops, render/request repeated work, caching freshness, and bundle or client-state churn.
- For
observability-ops, explicitly ask for Sentry coverage gaps on Route Handlers, Server Components, Server Actions, and Edge Middleware when the PR touches monitored server paths; also ask for log-level semantics, audit-event reliability, alert noise, and production investigation context.
- For
frontend-ux, explicitly ask for responsive regressions, viewport overflow, touch usability, mobile-friendly behavior, basic accessibility regressions such as keyboard, focus, labels, ARIA, contrast, and status messaging, and whether the UI masks failures with misleading fallback states when the PR touches pages, forms, drawers, modals, or styles.
- For
api-contract, explicitly ask for request and response shape drift, nullability drift, status-code and error-shape drift, permissive fallback parsing or coercion that hides contract breakage, and docs or integration-test update gaps when the PR touches API routes, schemas, or client wrappers.
5. Merge findings into a single review
Combine Claude's raw PR Review Toolkit output and all non-empty Codex sub-agent results into one review. Deduplicate overlapping findings, but do not suppress distinct findings because they are low severity, numerous, or less urgent. Keep the highest-severity version when multiple sources report the same issue.
The top-level merged Findings list is allowed to normalize, deduplicate, and false-positive-check Claude findings. The verbatim Claude section is not: copy the raw Claude report exactly so the user can audit any coordinator decision and recover every detail Claude produced.
Before finalizing, compare the union of sub-agent Reviewed files against files.json. If any changed file is still missing, read it yourself or launch an extra targeted pass. The final review must not leave unread changed files unless the file is opaque, binary, or otherwise not meaningfully inspectable. In that case, call it out explicitly under Residual risk.
During merge, maintain the coordinator finding ledger described in Review stability rules. The ledger is internal scratch unless the user asks for it, but it must drive keep/drop decisions. When dropping a candidate finding, identify whether it was duplicate, speculative, contradicted by source, line-not-found, outside the diff, or style-only.
Treat missing tests or docs as supporting evidence inside the primary behavior or contract finding unless they are independently actionable. Create a separate test/doc finding only when the missing coverage or stale document would remain a concrete problem even after the primary code issue is fixed.
Pressure-test every kept finding before finalizing:
- Can the affected line be found in the current source or patch?
- Is the behavior newly introduced or materially changed by this PR?
- Which invariant, API contract, user workflow, security boundary, or operational expectation is violated?
- What concrete bad outcome follows if the PR ships as-is?
- Would a reasonable maintainer know what to change from the finding?
If the answer to any question is no, rewrite, downgrade, or drop the finding.
Final review requirements:
- Findings first, ordered by severity.
- Include every distinct concrete finding across
high, medium, and low; do not apply a top-N cap or only report the most important findings.
- Each finding must include
severity, path:line, the concrete issue, and why it matters.
- Each finding should be explainable as: changed behavior -> violated invariant/contract -> concrete consequence. If that chain cannot be written in one sentence, drop or downgrade it.
- When a sub-agent reports a line range, normalize it to the first relevant line in the final merged review so every final finding still uses a single
path:line.
Findings: none is allowed only when Claude output, Codex sub-agent outputs, and the coordinator's direct hotspot sanity pass came back clean.
- Treat a finding as reportable when the diff-to-risk chain is concrete enough to explain a user-visible, access-control, security, data-integrity, failure-mode, correctness, contract, test, maintainability, accessibility, or operational consequence. Drop speculative concerns that cannot be tied back to the changed code, but keep low severity findings when they are concrete.
- Keep summary text brief and secondary.
- If every review source returns
no findings, say that explicitly and mention any residual risk such as large untouched areas or inability to run tests.
Coverage must be derived from the exact changed-file paths in files.json, or from the synthetic manual-fallback changed-file list when bundle mode is unavailable, not from broad area summaries. If any file was not read directly or through a lens owner, list that path under Unread areas.
- Before sending the final review, run one final consistency pass: every finding has a single valid
path:line, every severity matches the rubric, every changed file is covered or listed as unread, and the final language follows repository instructions.
- If final
Findings: none, include Residual risk: as a required one-sentence note after Coverage. It must say which hotspot classes were checked and whether any area was unread. Do not use a generic approval phrase.
Docs-only no-findings example:
Findings: none
Coverage:
- Reviewed sources: Claude pr-review-toolkit, maintainability
- Reviewed files: 2/2
- Unread areas: none
Residual risk: Docs-only install-command/link correctness and maintainability/doc-consistency hotspots were checked; no runtime source, tests, configs, API routes, UI, generated files, or unread changed files remained.
Claude PR Review Toolkit Report (verbatim)
<paste the complete contents of claude-pr-review.md here unchanged>
6. Output contract
Use this response shape for the final merged review:
Findings:
[high][access-control] path/to/file.ts:42 Missing authorization check allows ...
[medium][frontend-ux] components/foo.tsx:18 ...
none
Coverage:
Reviewed sources: Claude pr-review-toolkit, access-control, correctness, frontend-ux
Reviewed files: 12/12
Unread areas: none
After Coverage, optionally add:
Open questions: only when something important is ambiguous.
Residual risk: required when Findings: none; otherwise only when the review could not cover an important area, such as opaque or binary files, or when a model/tool fallback materially lowers confidence.
Then always add this section when the Claude pass ran:
Claude PR Review Toolkit Report (verbatim):
<paste the complete raw <bundle-dir>/claude-pr-review.md contents here unchanged>
If the Claude pass failed, add:
Claude PR Review Toolkit Report (verbatim): unavailable
Coverage formatting notes:
- The final merged review does not need to print every reviewed path when the PR is large.
- The coordinator must still reconcile exact paths against
files.json while reviewing.
- In the final response,
Reviewed files: N/N plus explicit Unread areas paths is sufficient.
- Patch-level reconciliation is acceptable for docs, generated files, lockfiles, and similar low-risk artifacts when the diff is human-readable and does not hide runtime behavior. Read the underlying source file directly when the patch alone is insufficient to judge behavior.
Manual Fallback
If the script cannot run, use this fallback:
gh pr view --json number,title,url,baseRefName,headRefName,files
- Use an existing saved patch if one is available. Otherwise run
gh pr diff --patch > /tmp/<name>.patch.
- Build a synthetic review plan from the returned file list, saved patch, repository instructions, and the manual fallback lens-selection rules above.
- Continue with the same findings-first output contract, marking
Reviewed sources: ... (manual fallback) in Coverage.
Bundled Resources
scripts/prepare_pr_review.py
Build a deterministic PR review bundle from gh. Use this first.
Examples:
python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "."
python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "." --pr "164"
python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "." --pr "https://github.com/org/repo/pull/164" --out-dir "/tmp/pr-164-review"
scripts/run_claude_pr_review.mjs
Run Claude Code's /pr-review-toolkit:review-pr through the local $claude-code wrapper and save raw output into the review bundle.
Examples:
node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>"
node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "all parallel"
node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "tests errors" --instruction "Prioritize behavioral regressions."
references/review-lenses.md
Defines the default review lenses, common bug classes, and the expected sub-agent output contract. Read it before spawning sub-agents or when you want to trim the default lens set.
1---2name: gh-pr-review3description: Run an exhaustive, findings-first review of a materially large or high-risk GitHub pull request by combining Claude Code's /pr-review-toolkit:review-pr output with specialized Codex review lenses. Use only when the user explicitly requests `$gh-pr-review` or an exhaustive multi-lens review, or when the PR is materially large or high-risk: roughly more than 20 changed files or 1,000 changed lines, a cross-cutting architecture change, an authorization or tenant-boundary redesign, destructive database migration or data movement, or release/deployment control changes. Do not use for routine PR review, focused bug fixes, review-comment resolution, CI fixes, test-only follow-ups, or merge/closeout; use direct review and targeted validation instead.4---56# Gh Pr Review78## Overview910Review eligible large or high-risk GitHub pull requests as a findings-first code review. Resolve the active PR with `gh`, build a local review bundle, run Claude Code's `/pr-review-toolkit:review-pr`, run lens-specific Codex sub-agents in parallel, then merge every concrete high, medium, and low severity finding into a severity-ordered report.1112## Eligibility Gate1314Apply this gate before building a bundle, starting Claude, or delegating review work. Continue with this skill only when at least one condition is true:1516- The user explicitly names `$gh-pr-review` or asks for an exhaustive, multi-agent, or multi-lens review.17- The PR is materially large: roughly more than 20 changed files or 1,000 changed lines.18- The PR materially changes a high-risk boundary: cross-cutting architecture, authorization or tenant isolation, destructive migration or data movement, or release/deployment controls.1920A file merely belonging to a sensitive area does not satisfy the gate; the diff must materially change that boundary.2122When the gate is not satisfied, stop this skill and use a lightweight review:2324- Read the changed diff directly.25- Use at most one targeted reviewer when delegation is explicitly authorized and materially useful.26- Run focused validation and the repository's normal exact-head CI gate.27- Do not build the review bundle, start Claude PR Review Toolkit, or wait under this skill's long-running policy.2829Do not rerun the full skill after review fixes unless runtime behavior or the high-risk surface materially changes. Test-only, documentation-only, formatting, and review-comment follow-ups require targeted review or validation plus exact-head CI, not another exhaustive pass.3031## Inputs3233- `repo`: repository path. Default `.`.34- `pr`: PR number or URL. Omit to use the PR for the current branch.35- `gh` authentication with repository access.3637## Review Modes3839- **Bundle mode**: Default. `prepare_pr_review.py` succeeds and provides `metadata.json`, `files.json`, `lens-hints.json`, `summary.md`, and patch files.40- **Manual fallback mode**: Use only when bundle generation fails. Build equivalent coverage and lens decisions from `gh pr view --json ...`, a saved patch, repository instructions, and direct file reads. Do not claim `lens-hints.json` was used in this mode.4142Both modes must produce the same final output contract and exact changed-file coverage accounting.4344## Long-Running Wait Policy4546This policy applies only after the Eligibility Gate passes. Claude PR Review Toolkit and lens sub-agents can legitimately take longer than 10 minutes on eligible large or high-risk PRs. Do not treat 10 minutes, a single empty `wait_agent` timeout, or a quiet Claude process as failure.4748- For Claude, start the helper as a long-running command and keep polling the same session until it exits, errors, or the user asks to stop. Do not mark Claude unavailable solely because it has run for more than 10 minutes.49- For sub-agents, use long `wait_agent` timeouts when supported. If a wait returns an empty timeout while the agent is still running, wait again instead of closing the agent.50- Keep doing coordinator direct reads while slow Claude or sub-agent work runs, but do not finalize until the required Claude/sub-agent outputs return or a real terminal failure occurs.51- Default patience budget: wait at least 45 minutes for eligible large, cross-cutting, migration-heavy, security-sensitive, or all-lens reviews. Waiting longer is appropriate when output is still progressing.52- Close or fallback only when the tool reports a final failure, the user asks to stop, or the process/agent is clearly non-responsive beyond the patience budget and direct coverage can replace it. In that case, report the exact wait time and fallback reason under `Residual risk`.53- Send short progress updates during long waits so the user knows the review is still running.5455## Quick Start56571. Apply the Eligibility Gate. Use the lightweight review path and stop this skill when the PR is not eligible.582. Verify GitHub CLI access.59 - `gh auth status`603. Read repository instruction files before reviewing.61 - Start with repo-root or nearest `AGENTS.md`, `CLAUDE.md`, and contribution docs.62 - Treat repo-specific review requirements as hard constraints, not optional context.634. Build the review bundle.64 - `python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "."`65 - Add `--pr "<number-or-url>"` when the review target is not the current branch PR.665. Read `<bundle-dir>/summary.md`.676. Read [`references/review-lenses.md`](references/review-lenses.md).687. Run Claude Code's PR Review Toolkit pass and save the raw output in the bundle.698. Launch only the recommended Codex lenses from `lens-hints.json`, then merge all concrete findings from Claude and every lens.7071## Workflow7273### 1. Resolve and materialize the PR7475Always start by generating a review bundle with `prepare_pr_review.py`. The script writes:7677- `metadata.json`: PR metadata and counts.78- `files.json`: changed-file list with additions, deletions, and extracted patch paths when available.79- `lens-hints.json`: recommended lenses, selection notes, and per-lens file hints.80- `summary.md`: compact human-readable overview.81- `diff.patch`: full PR patch from `gh pr diff --patch`.82- `patches/<path>.patch`: file-scoped patch files extracted from the full patch.8384Prefer the script over repeated ad hoc `gh` commands so every sub-agent can point to the same local artifacts.8586### 2. Read the bundle before delegating8788Read `summary.md` first. Then read `lens-hints.json` and use `recommended_lenses` as the default sub-agent launch set. Each lens entry also includes `reason` and `focus_files`.8990Before delegating, read the repository instruction files that govern the target repo, such as `AGENTS.md`, `CLAUDE.md`, and repo contribution docs. Convert any review-specific repo rules into explicit checks or lens prompt addenda. Examples include required review language, required architecture-doc updates, testing conventions, and known high-risk stacks such as Supabase or Sentry.9192Build a short coordinator review plan before launching sub-agents. Keep it as scratch unless the user asks for it. The plan must include:9394- Exact changed-file count and paths from `files.json`.95- Repository review rules that affect this PR, such as output language, required tests, migration rules, and doc-update rules.96- Claude PR Review Toolkit configuration, output path, and whether it completed or fell back.97- Recommended lenses from `lens-hints.json`, plus any lens added or removed by the coordinator and the reason.98- Exact assigned file scope for each lens.99- Coordinator direct-read targets, especially access-control gates, route handlers, migrations, destructive operations, failure-mode branches, external requests, generated-contract boundaries, and tests that should prove the changed behavior.100- Coverage gaps that must be closed before final output.101102Do not launch every available lens by default. When the PR is small and clearly cross-cutting, you can still run all lenses. Otherwise, start with `recommended_lenses`, narrow each sub-agent to the most relevant file subset, and let it open extra source files only when needed for cross-file reasoning.103104Use `coverage_gaps` from `lens-hints.json` as a hard stop before finalizing. If any changed file is not covered by the recommended lenses, either assign it to another lens or read it directly as the coordinator. Do not finalize while unread changed files remain.105106For large or high-risk PRs, do not rely on sub-agent `Findings: none` outputs alone. If the PR changes more than roughly 20 files, or touches access control, route handlers, external network calls, migrations, destructive data changes, failure-mode branches, observability, or tenant-scope logic, the coordinator must run a direct hotspot sanity pass before finalizing. At minimum, open the changed helpers or handlers that gate access, the changed files that move/drop data or make external requests, and the changed tests that should prove the new behavior.107108For high-risk migration or API compatibility PRs, the coordinator must directly inspect the changed migration/backfill/drop files and compatibility route/client/schema files even when `lens-hints.json` reports no coverage gaps. For destructive migrations, verify both key coverage and value equality before drop/rename, and treat skipped divergent target rows as reportable data-loss risk. For compatibility APIs, compare old and new permission predicates including self-service carve-outs, and verify response fields preserve semantic sources such as planned versus actual values.109110For small PRs, the coordinator's direct read of all changed files is sufficient to count as the sanity pass. You do not need a second separate hotspot sweep when the entire diff is already small enough to read directly.111112If the target repository has known high-risk conventions, state them explicitly in the sub-agent prompt. For example, call out Supabase RLS and policy regressions for the `access-control` lens, destructive migration parity for the `data-integrity` lens, and Sentry coverage with `@sentry/nextjs` for the `observability-ops` lens.113114### Manual fallback planning115116When `prepare_pr_review.py` cannot produce a bundle, create a synthetic review plan before delegating or reviewing manually:117118- Treat `gh pr view --json ... files` as the synthetic `files.json` equivalent for exact changed-file coverage.119- Treat an existing saved patch path, such as `/tmp/pr-88.patch`, as the synthetic `diff.patch` equivalent. Use it to identify changed hunks and line anchors.120- Do not rerun `gh pr diff --patch` when a saved patch already exists. If no saved patch exists, try `gh pr diff --patch > /tmp/<name>.patch`; if that also fails, continue with `gh pr view` metadata plus targeted source reads and list the missing patch as residual risk.121- Read repository instruction files exactly as in bundle mode.122- Select lenses from changed paths and risk triggers:123 - `access-control`: authentication, authorization, roles, RLS/policies, middleware, route handlers, session/cookie/origin/redirect trust boundaries, tenant or object scope, and service-role boundaries.124 - `security`: secrets, injection, unsafe file/network handling, uploads, webhooks, CORS, crypto/password/token handling, sanitization, and redaction.125 - `data-integrity`: migrations, backfills, canonical data movement, schema changes, destructive drop/rename, repositories, transactions, duplicate/idempotency behavior, and value parity.126 - `failure-modes`: dependency failure, partial failure, degraded state, fallback, retry, timeout, broad catch/default branches, fail-open/fail-closed behavior, and required side-effect failures.127 - `api-contract`: API routes, client wrappers, schemas, validators, generated/public types, status codes, error payloads, docs for API behavior.128 - `performance`: runtime cost, N+1, repeated work, unbounded loops, caching, batching, bundle size, and request/render path regressions.129 - `observability-ops`: Sentry, structured logs, audit events, metrics, tracing, alert noise, operational investigation context, jobs, webhooks, and rollout visibility.130 - `tests`: changed production behavior without matching tests, weak regression assertions, migrations, or repo-specific coverage requirements.131 - `frontend-ux`: visible UI, forms, modals, navigation, responsive/accessibility behavior, client state.132 - `maintainability`: docs-only changes, cross-cutting helpers, large refactors, layering and ownership.133 - `correctness`: default for changed runtime logic and business behavior not fully covered by the more specific lenses.134- Assign every changed file to at least one selected lens or to coordinator direct read.135- In final `Coverage`, write `Reviewed sources: Claude pr-review-toolkit, <lenses> (manual fallback)` when the Claude pass ran; otherwise write `Reviewed sources: <lenses> (manual fallback)` and list the Claude fallback reason under `Residual risk`. Never mention `lens-hints.json` recommendations in manual fallback mode.136137Manual fallback sub-agent prompt template:138139```text140Manual fallback is active. Do not read or claim `lens-hints.json`.141142## Available Artifacts143- Changed-file list: <exact paths from gh pr view --json ... files>144- Patch source: <saved patch path, or note if patch unavailable>145- Repo rules: <language/testing/docs/migration rules that matter>146147## Assigned Scope148- Lens: <lens>149- Files: <exact assigned paths>150- Lens-specific checks: <copy only relevant checks from review-lenses.md>151152## Required Method1531. Read the saved patch when available.1542. Open current source files when the patch is insufficient to verify behavior or line numbers.1553. Do not report a finding unless it has a changed-code anchor, violated invariant/contract, concrete failure mode, and plausible fix direction.1564. If no finding remains after false-positive and false-negative passes, write "Findings: none".157158## Output159Reviewed files:160- `path/to/file`161162Findings:163- `[severity][<lens>] path/to/file.ts:42 Issue summary. Why it matters.`164- `none`165166Residual risk:167- Only include unread or unverifiable scope.168```169170### Review stability rules171172Use these rules to keep outputs stable across runs:173174- Treat `files.json` as the source of truth for changed-file coverage. Do not infer coverage from area names, lens names, or broad summaries.175- Keep a coordinator finding ledger while merging: `source lens`, `path:line`, `severity`, `risk chain`, `decision` (`keep`, `duplicate`, `drop`), and `decision reason`.176- Keep a hotspot checklist for every high-risk changed file. Mark each hotspot as `read`, `covered by lens`, `finding`, or `no finding after direct check`.177- A finding is admissible only when it has all four parts: changed-code anchor, violated invariant or contract, concrete failure mode, and a plausible fix direction.178- Run a false-positive pass before final output. Drop items whose risk depends on unchanged code alone, missing context contradicted by source, or a line that no longer exists in the current diff.179- Run a false-negative pass before final output. If a high-risk changed file has no finding, explicitly re-check the relevant bug class: access/scope, data loss, contract drift, error handling, fallback behavior, observability, and test coverage.180- Use the fixed severity rubric below. Do not upgrade severity because a finding is interesting, and do not downgrade concrete low-severity findings because the report is long.181182Severity rubric:183184- `high`: likely data loss, auth/tenant isolation break, secret exposure, destructive migration failure, broadly blocking runtime regression, or contract break that can make a critical user workflow fail.185- `medium`: likely user-visible correctness regression, API/nullability drift, missing failure handling, meaningful test gap for changed critical behavior, performance/operations issue that can hide or amplify production failure.186- `low`: concrete maintainability, test precision, documentation, accessibility, or minor UX issue tied to changed code, with limited immediate blast radius.187188### 3. Run Claude Code PR Review Toolkit189190Before or alongside Codex lens sub-agents, run Claude Code's `pr-review-toolkit@claude-plugins-official` command against the same repository state. Treat this as an additional independent review source, not as a replacement for Codex lens coverage or coordinator hotspot checks.191192Default command:193194```bash195node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>"196```197198For a narrower or more exhaustive Claude pass, pass the same aspect words supported by Claude's slash command:199200```bash201node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "tests errors"202node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "all parallel"203```204205The helper writes Claude's raw stdout to `<bundle-dir>/claude-pr-review.md`. Read that file before final merge. Preserve Claude's review result as a source artifact: do not rewrite it in place, do not delete low severity findings, and do not treat a Claude "no findings" result as coverage for changed files unless its output names the files or the coordinator can verify that scope.206207The final review must also include Claude's raw report without information loss. After the merged Codex review sections, add `Claude PR Review Toolkit Report (verbatim)` and paste the complete contents of `<bundle-dir>/claude-pr-review.md` unchanged. Do not summarize, reorder, deduplicate, translate, omit low-severity items, trim, indent, blockquote, or wrap Claude's raw report in a way that changes its content. If the Claude pass failed and no raw report exists, include `Claude PR Review Toolkit Report (verbatim): unavailable` with the exact failure reason from stderr or the helper output.208209The helper defaults to `--permission-mode auto` and allows the read/review tools Claude's command needs: `Bash(git *)`, `Bash(gh *)`, `Glob`, `Grep`, `Read`, and `Task`. Keep it read-only unless the user explicitly asks for fixes.210211If Claude Code, the `claude-code` Codex skill wrapper, or `pr-review-toolkit@claude-plugins-official` is unavailable, continue with the Codex lens workflow and list `Claude pr-review-toolkit unavailable: <reason>` under `Residual risk`. Do not block the PR review solely because the Claude pass failed.212213Long Claude runs are not failures. If the helper is still running after 10 minutes, keep the session open and continue polling according to the Long-Running Wait Policy. Do not replace the Claude result with `unavailable` until the command exits with an error, the user stops the wait, or the explicit patience budget is exhausted.214215When merging Claude output with Codex sub-agent outputs:216217- Add every concrete Claude finding to the same coordinator finding ledger used for Codex lens findings.218- Use source label `claude-pr-review-toolkit`.219- Keep the Claude finding text and severity as close to raw as practical, while normalizing final output to this skill's single `path:line` and severity format.220- Merge Claude findings into the top-level `Findings` list as Codex-review findings, but still include the complete raw Claude report later in `Claude PR Review Toolkit Report (verbatim)`.221- Deduplicate only exact or semantically identical issues. If Claude and a Codex lens identify different failure modes on the same line, keep both.222- If Claude reports a finding without enough anchor evidence, re-check the patch/source. Keep it if the coordinator can anchor it; otherwise drop it as `line-not-found` or `speculative` in the internal ledger.223- Include Claude in final coverage as `Reviewed sources: Claude pr-review-toolkit, <lenses>`.224225### 4. Spawn specialized sub-agents in parallel226227Available lenses:228229- `access-control`230- `security`231- `data-integrity`232- `correctness`233- `failure-modes`234- `api-contract`235- `performance`236- `observability-ops`237- `frontend-ux`238- `tests`239- `maintainability`240241Use sub-agents only when the user explicitly asked for a PR review, asked for this skill, or otherwise gave clear permission for delegated review work. If delegation is unavailable in the current environment, perform the same lens-based reasoning yourself and mention the fallback only when it materially affected coverage or confidence.242243Long sub-agent runs are not failures. If an agent takes more than 10 minutes, continue waiting according to the Long-Running Wait Policy. If `wait_agent` times out with no final status, call `wait_agent` again instead of closing the agent, unless the user has asked to stop or the explicit patience budget has been exhausted.244245Use `spawn_agent` with these fixed sub-agent settings:246247- `agent_type: "explorer"`248- `model: "gpt-5.4-mini"`249- `reasoning_effort: "xhigh"`250251Example per lens:252253```json254{255 "agent_type": "explorer",256 "model": "gpt-5.4-mini",257 "reasoning_effort": "xhigh",258 "message": "Review the PR bundle at <bundle-dir> from the <lens> lens..."259}260```261262Do not omit, downgrade, or vary those model settings between lenses. If the configured model is unavailable in the current environment, state the fallback explicitly in the final review under `Residual risk`. Pass the minimum context needed: the bundle directory, the review lens, and the output contract. Start with `recommended_lenses` and add more only if the diff or the user request calls for exhaustive review.263264Use prompts shaped like this:265266`Review the PR bundle at <bundle-dir> from the <lens> lens. Start with summary.md, then inspect the relevant patch files and source files until no unread files remain in your assigned scope. Return a Reviewed files section and a Findings section. Report every concrete finding in your scope, including low severity findings. A finding must have a changed-code anchor, violated invariant or contract, concrete failure mode, and plausible fix direction. Use this severity rubric: high=data loss/access-control/tenant/secret/destructive migration/critical workflow break; medium=likely user-visible regression/API drift/failure handling/test gap/production observability risk; low=concrete maintainability/test precision/docs/accessibility/minor UX issue. Ignore only purely stylistic nits, non-actionable preferences, and speculative concerns that cannot be tied to changed code. If there are no issues, write "Findings: none".`267268For more stable agent outputs, prefer this expanded prompt template over improvising:269270```text271Review the PR bundle at <bundle-dir> from the <lens> lens.272273## Assigned Scope274- Files: <exact paths from files.json or lens-hints.json>275- Repo rules: <language/testing/docs/migration rules that matter>276- Lens-specific checks: <copy only relevant checks from review-lenses.md>277278## Required Method2791. Read summary.md and lens-hints.json.2802. Read each assigned patch file.2813. Open the current source file whenever the patch alone is insufficient to verify behavior or line numbers.2824. Do not report a finding unless it has a changed-code anchor, violated invariant/contract, concrete failure mode, and plausible fix direction.2835. If no finding remains after a false-positive pass, write "Findings: none".284285## Output286Reviewed files:287- `path/to/file`288289Findings:290- `[severity][<lens>] path/to/file.ts:42 Issue summary. Why it matters.`291- `none`292293Residual risk:294- Only include unread or unverifiable scope.295```296297Good delegation rules:298299- Keep each agent focused on one lens.300- Prefer file subsets from `lens-hints.json` for large PRs.301- Ask the agent to inspect source files in the repository when the patch alone is insufficient.302- Do not ask sub-agents to propose code changes unless the user asks for fixes.303- Use the union of all agents' `Reviewed files` lists to verify full diff coverage before writing the final report.304- For `access-control`, explicitly ask for Supabase RLS, policy, `service_role`, object-scope regressions, self-service carve-outs, CSRF/origin/session/cookie/redirect boundaries, and cross-endpoint guard consistency when the PR touches `supabase/**`, `app/api/**`, middleware, auth, or data-access code.305- For `security`, explicitly ask for secrets, injection, unsafe file/network handling, webhook trust, upload handling, redaction, crypto/password/token handling, and privacy leakage.306- For `data-integrity`, explicitly ask for row coverage and value parity before destructive operations, same-key conflicts, timestamp ties, `on conflict` precedence, idempotency, rollback, and archive-before-drop verification.307- For `failure-modes`, explicitly ask whether dependency failures fail open or fail closed, whether fallback paths hide real failures behind defaults/placeholders, whether partial mutations roll back or compensate, and whether required side effects can fail invisibly.308- For `performance`, explicitly ask for N+1 queries, duplicate reads, unbounded pagination/loops, render/request repeated work, caching freshness, and bundle or client-state churn.309- For `observability-ops`, explicitly ask for Sentry coverage gaps on Route Handlers, Server Components, Server Actions, and Edge Middleware when the PR touches monitored server paths; also ask for log-level semantics, audit-event reliability, alert noise, and production investigation context.310- For `frontend-ux`, explicitly ask for responsive regressions, viewport overflow, touch usability, mobile-friendly behavior, basic accessibility regressions such as keyboard, focus, labels, ARIA, contrast, and status messaging, and whether the UI masks failures with misleading fallback states when the PR touches pages, forms, drawers, modals, or styles.311- For `api-contract`, explicitly ask for request and response shape drift, nullability drift, status-code and error-shape drift, permissive fallback parsing or coercion that hides contract breakage, and docs or integration-test update gaps when the PR touches API routes, schemas, or client wrappers.312313### 5. Merge findings into a single review314315Combine Claude's raw PR Review Toolkit output and all non-empty Codex sub-agent results into one review. Deduplicate overlapping findings, but do not suppress distinct findings because they are low severity, numerous, or less urgent. Keep the highest-severity version when multiple sources report the same issue.316317The top-level merged `Findings` list is allowed to normalize, deduplicate, and false-positive-check Claude findings. The verbatim Claude section is not: copy the raw Claude report exactly so the user can audit any coordinator decision and recover every detail Claude produced.318319Before finalizing, compare the union of sub-agent `Reviewed files` against `files.json`. If any changed file is still missing, read it yourself or launch an extra targeted pass. The final review must not leave unread changed files unless the file is opaque, binary, or otherwise not meaningfully inspectable. In that case, call it out explicitly under `Residual risk`.320321During merge, maintain the coordinator finding ledger described in Review stability rules. The ledger is internal scratch unless the user asks for it, but it must drive keep/drop decisions. When dropping a candidate finding, identify whether it was duplicate, speculative, contradicted by source, line-not-found, outside the diff, or style-only.322323Treat missing tests or docs as supporting evidence inside the primary behavior or contract finding unless they are independently actionable. Create a separate test/doc finding only when the missing coverage or stale document would remain a concrete problem even after the primary code issue is fixed.324325Pressure-test every kept finding before finalizing:326327- Can the affected line be found in the current source or patch?328- Is the behavior newly introduced or materially changed by this PR?329- Which invariant, API contract, user workflow, security boundary, or operational expectation is violated?330- What concrete bad outcome follows if the PR ships as-is?331- Would a reasonable maintainer know what to change from the finding?332333If the answer to any question is no, rewrite, downgrade, or drop the finding.334335Final review requirements:336337- Findings first, ordered by severity.338- Include every distinct concrete finding across `high`, `medium`, and `low`; do not apply a top-N cap or only report the most important findings.339- Each finding must include `severity`, `path:line`, the concrete issue, and why it matters.340- Each finding should be explainable as: changed behavior -> violated invariant/contract -> concrete consequence. If that chain cannot be written in one sentence, drop or downgrade it.341- When a sub-agent reports a line range, normalize it to the first relevant line in the final merged review so every final finding still uses a single `path:line`.342- `Findings: none` is allowed only when Claude output, Codex sub-agent outputs, and the coordinator's direct hotspot sanity pass came back clean.343- Treat a finding as reportable when the diff-to-risk chain is concrete enough to explain a user-visible, access-control, security, data-integrity, failure-mode, correctness, contract, test, maintainability, accessibility, or operational consequence. Drop speculative concerns that cannot be tied back to the changed code, but keep low severity findings when they are concrete.344- Keep summary text brief and secondary.345- If every review source returns `no findings`, say that explicitly and mention any residual risk such as large untouched areas or inability to run tests.346- `Coverage` must be derived from the exact changed-file paths in `files.json`, or from the synthetic manual-fallback changed-file list when bundle mode is unavailable, not from broad area summaries. If any file was not read directly or through a lens owner, list that path under `Unread areas`.347- Before sending the final review, run one final consistency pass: every finding has a single valid `path:line`, every severity matches the rubric, every changed file is covered or listed as unread, and the final language follows repository instructions.348- If final `Findings: none`, include `Residual risk:` as a required one-sentence note after `Coverage`. It must say which hotspot classes were checked and whether any area was unread. Do not use a generic approval phrase.349350Docs-only no-findings example:351352```text353Findings: none354355Coverage:356- Reviewed sources: Claude pr-review-toolkit, maintainability357- Reviewed files: 2/2358- Unread areas: none359360Residual risk: Docs-only install-command/link correctness and maintainability/doc-consistency hotspots were checked; no runtime source, tests, configs, API routes, UI, generated files, or unread changed files remained.361362Claude PR Review Toolkit Report (verbatim)363<paste the complete contents of claude-pr-review.md here unchanged>364```365366### 6. Output contract367368Use this response shape for the final merged review:369370Findings:371- `[high][access-control] path/to/file.ts:42 Missing authorization check allows ...`372- `[medium][frontend-ux] components/foo.tsx:18 ...`373- `none`374375Coverage:376- `Reviewed sources: Claude pr-review-toolkit, access-control, correctness, frontend-ux`377- `Reviewed files: 12/12`378- `Unread areas: none`379380After `Coverage`, optionally add:381382- `Open questions`: only when something important is ambiguous.383- `Residual risk`: required when `Findings: none`; otherwise only when the review could not cover an important area, such as opaque or binary files, or when a model/tool fallback materially lowers confidence.384385Then always add this section when the Claude pass ran:386387Claude PR Review Toolkit Report (verbatim):388<paste the complete raw `<bundle-dir>/claude-pr-review.md` contents here unchanged>389390If the Claude pass failed, add:391392Claude PR Review Toolkit Report (verbatim): unavailable393<exact failure reason>394395Coverage formatting notes:396397- The final merged review does not need to print every reviewed path when the PR is large.398- The coordinator must still reconcile exact paths against `files.json` while reviewing.399- In the final response, `Reviewed files: N/N` plus explicit `Unread areas` paths is sufficient.400- Patch-level reconciliation is acceptable for docs, generated files, lockfiles, and similar low-risk artifacts when the diff is human-readable and does not hide runtime behavior. Read the underlying source file directly when the patch alone is insufficient to judge behavior.401402## Manual Fallback403404If the script cannot run, use this fallback:4054061. `gh pr view --json number,title,url,baseRefName,headRefName,files`4072. Use an existing saved patch if one is available. Otherwise run `gh pr diff --patch > /tmp/<name>.patch`.4083. Build a synthetic review plan from the returned file list, saved patch, repository instructions, and the manual fallback lens-selection rules above.4094. Continue with the same findings-first output contract, marking `Reviewed sources: ... (manual fallback)` in `Coverage`.410411## Bundled Resources412413### scripts/prepare_pr_review.py414415Build a deterministic PR review bundle from `gh`. Use this first.416417Examples:418419- `python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "."`420- `python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "." --pr "164"`421- `python3 "<skill-path>/scripts/prepare_pr_review.py" --repo "." --pr "https://github.com/org/repo/pull/164" --out-dir "/tmp/pr-164-review"`422423### scripts/run_claude_pr_review.mjs424425Run Claude Code's `/pr-review-toolkit:review-pr` through the local `$claude-code` wrapper and save raw output into the review bundle.426427Examples:428429- `node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>"`430- `node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "all parallel"`431- `node "<skill-path>/scripts/run_claude_pr_review.mjs" --repo "." --bundle-dir "<bundle-dir>" --review-aspects "tests errors" --instruction "Prioritize behavioral regressions."`432433### references/review-lenses.md434435Defines the default review lenses, common bug classes, and the expected sub-agent output contract. Read it before spawning sub-agents or when you want to trim the default lens set.