skills/pr-review/SKILL.md
Comprehensive PR review focusing on code quality, test coverage, security, backward compatibility, and what CI cannot check. Use when reviewing PRs, when asked to review code changes, or when the user mentions "review PR", "code review", or "check this PR".
npx skillsauth add harshitsinghbhandari/domain-expansion pr-reviewInstall this skill globally with one command. Works with Claude Code, Cursor, and Windsurf.
3 of 9 scanners reported clean
Some scanners were skipped, did not run, or reported a non-clean status. Review each row below.
Review pull requests focusing on what CI cannot check: code quality, design decisions, test coverage adequacy, security vulnerabilities, and backward compatibility.
If the user invokes /pr-review with no arguments, do not perform a review. Instead, ask the user what they would like to review:
What would you like me to review?
- A PR number or URL (e.g.,
/pr-review 12345)- A local branch (e.g.,
/pr-review branch)
The user provides a PR number or URL:
/pr-review 12345
/pr-review https://github.com/owner/repo/pull/12345
For a detailed review with line-by-line specific comments:
/pr-review 12345 detailed
/pr-review https://github.com/owner/repo/pull/12345 detailed
Use gh CLI to fetch PR data:
# Get PR details
gh pr view <PR_NUMBER> --json title,body,author,baseRefName,headRefName,files,additions,deletions,commits
# Get the diff
gh pr diff <PR_NUMBER>
# Get PR comments and reviews
gh pr view <PR_NUMBER> --json comments,reviews
# Get line-level inline review comments (not included in comments,reviews)
gh api repos/{owner}/{repo}/pulls/<PR_NUMBER>/comments
Review changes in the current branch that are not in main:
/pr-review branch
/pr-review branch detailed
Use git commands to get branch changes:
# Get current branch name
git branch --show-current
# Get list of changed files compared to main
git diff --name-only main...HEAD
# Get full diff compared to main
git diff main...HEAD
# Get commit log for the branch
git log main..HEAD --oneline
# Get diff stats (files changed, insertions, deletions)
git diff --stat main...HEAD
For local branch reviews:
When invoked via @claude /pr-review on a GitHub PR, the action pre-fetches PR
metadata and injects it into the prompt. Detect this mode by the presence of
<formatted_context>, <pr_or_issue_body>, and <comments> tags in the prompt.
The prompt already contains:
Use git commands to get the diff and commit history. The base branch name is in the
prompt context (look for PR Branch: <head> -> <base> or the baseBranch field).
# Get the full diff against the base branch
git diff origin/<baseBranch>...HEAD
# Get diff stats
git diff --stat origin/<baseBranch>...HEAD
# Get commit history for this PR
git log origin/<baseBranch>..HEAD --oneline
# If the base branch ref is not available, fetch it first
git fetch origin <baseBranch> --depth=1
Do NOT use gh CLI commands in this mode -- only git commands are available.
All PR metadata, comments, and reviews are already in the prompt context;
only the diff and commit log need to be fetched via git.
Your job is not to read the diff and check boxes. Your job is to investigate — trace every change through the system, experience it as a user would, and find the bugs that only surface when you look beyond the lines that changed.
A single line of code can have deep cross-cutting implications: a missing null check causes silent failures, a missing validation opens security holes, a manual implementation instead of using framework utilities creates maintenance burden. Treat every line as potentially load-bearing.
The review checklist is large. You cannot hold the full context of every system in your head. Spawn sub-agents to investigate: read surrounding code, trace call chains, check what infrastructure the PR should be using, verify tests that should exist. Spawn them in parallel for independent areas. A typical medium PR should spawn 3-8 sub-agents.
The workflow is structured as investigate first, then verify, then synthesize. Steps 1-4 are investigation — context-building, flow tracing, delegated boundary verification, and consumer-UX inspection. Steps 5-9 are verification — checklist sweep, merge-conflict residue, PR discussion, BC, edge cases. Step 10 synthesizes findings into the review.
Before reviewing, build understanding of what the PR touches and why:
For each significantly changed function, method, or class, follow the execution path in both directions. The diff shows what changed — this step reveals what the change affects.
Trace callers (up the call chain):
Trace callees (down the call chain):
Map the blast radius:
Spawn sub-agents in parallel to trace each significant change.
Data flow integrity at seams — schema drift, dual-path divergence, sanitizer strips, writer-reader mismatches, crash-mid-sequence, schema versioning, shared mutable state — is the domain of the boundary-bug-hunter skill. Do not attempt this analysis inline; boundary-bug-hunter does it more rigorously and produces a refusable gate.
Required action: Invoke boundary-bug-hunter in aggressive mode on this diff. Read the resulting boundary-report.md and boundary-status.json. Skip only if the diff is confined to a single file with no callers and no externally visible output (rare; verify before skipping).
Incorporate into review:
boundary-status.json.status == "fail", the recommendation cannot be Approve. The unverified boundaries become must-fix items in the final review.What this step does NOT cover (handled elsewhere):
Boundary-bug-hunter (Step 3) verifies that the contract is met. This step asks a different question: is the contract the right thing to be exposing? For every changed output surface — API response field, CLI output, UI state, event payload, webhook body, error message, log line — read it as the human receiver would.
For each output surface:
title will be displayed as a title. Is the value actually a title — or is it a fallback summary, a default, an internal key? Misleading names corrupt every downstream display.idempotency-key header — see API docs / retry with new key" is actionable.Any "yes" to question 7, or any "no" to questions 2 / 3 / 5 / 6, is a finding.
This step is about judgment, not contracts. Contract verification is Step 3's job.
Go through every changed line in the diff and evaluate it against the review checklist in review-checklist.md. This is a systematic sweep to catch anything the flow-tracing steps missed.
If the PR includes a merge commit or conflict resolution:
git diff main -- <file>.-X theirs resolution.Before formulating your review, read what others have already said on the PR:
gh api repos/{owner}/{repo}/pulls/<PR_NUMBER>/comments and PR-level reviews via gh pr view <PR_NUMBER> --json comments,reviews<comments> section already in the prompt contextEvaluate BC implications per bc-guidelines.md. For non-trivial BC questions, spawn a sub-agent to search for existing callers of the modified API.
If the PR touches high-risk code (state machines, parsers, dependency resolution, auth flows, financial calculations, concurrency, or configuration merging), invoke /edge-case-hunter on the changed files in those areas. Include the top findings in the Edge Cases section of your review.
Skip this step only if the PR is purely additive (new files, new tests) with no modifications to existing complex logic.
Structure your review with actionable feedback organized by category. Every finding should be traceable to a specific line in the diff.
Structure your review as follows. Omit sections where you have no findings — don't write "No concerns" for every empty section. Only include sections with actual observations.
## PR Review: #<number>
<!-- Or for local branch reviews: -->
## Branch Review: <branch-name> (vs main)
### Summary
Brief overall assessment of the changes (1-2 sentences).
### Code Quality
[Issues and suggestions]
### Architecture & Design
[Flag any checklist items from the Infrastructure section that apply.
Reference the specific patterns or utilities the PR should be using.]
### Testing
[Testing adequacy findings — missing edge cases, non-device-generic tests, etc.]
### API Design
[Flag new patterns, internal-access flags, or broader implications if any.]
### Security
[Issues if any]
### Thread Safety
[Threading concerns if any]
### Backward Compatibility
[BC concerns if any]
### Boundary Verification
[Summary of the boundary-bug-hunter run. Always present when boundary-bug-hunter
was invoked in Step 3.
Required content:
- Status (PASS / FAIL) and unverified-boundary count from boundary-status.json.
- For FAIL: each unverified boundary listed as producer:line → consumer:line,
with the contract and why it's unverified. These are must-fix.
- Deferrals applied this run: each one named with its recorded reason and a
one-line confirmation that the reason still holds. Reasons that no longer
hold are review findings.
- Stale or invalidated deferrals: surface as cleanup items.
A FAIL status here makes the overall recommendation Request Changes regardless
of what other sections say.]
### Consumer Impact
[Output-UX findings from Step 4 that are NOT contract verification (which lives
in Boundary Verification). Examples: misleading field names, error messages
that don't tell the operator what to do, log lines missing correlation IDs,
indistinguishable fallback representations, dishonest transitional states.
Only present when the PR changes output surfaces (APIs, events, CLI output,
UI state, logs).]
### Edge Cases
[High-risk edge cases found by targeted analysis of critical changed files.
Only present when the PR touches state machines, parsers, dependency resolution,
auth flows, financial calculations, or similarly complex logic.]
### Performance
[Performance concerns if any]
### Recommendation
**Approve** / **Request Changes** / **Needs Discussion**
[Brief justification for recommendation]
Only include this section if the user requests a "detailed" or "in depth" review.
Do not repeat observations already made in other sections. This section is for additional file-specific feedback that doesn't fit into the categorized sections above.
When requested, add file-specific feedback with line references:
### Specific Comments
- `src/module.py:42` - Consider extracting this logic into a named function for clarity
- `test/test_feature.py:100-105` - Missing test for error case when input is None
- `src/utils/helper.ts:78` - This allocation could be moved outside the loop
When reviewing, consult these project files for context — read them rather than relying on memory, as they change frequently:
CLAUDE.md or AGENTS.md - Coding style philosophy and project-specific patternsCONTRIBUTING.md - PR requirements and review processThis skill delegates parts of the review to specialized skills:
boundary-bug-hunter (Step 3) — seam analysis, round-trip verification, deferral audit. A fail status from its run is blocking.edge-case-hunter (Step 9) — input-side edge cases inside high-risk functions.Do not duplicate their findings inline; reference and incorporate them. If either skill is unavailable in the running environment, note that explicitly in the review under "Coverage gaps in this review".
development
Aggressive user-flow and boundary-bug analysis on a diff or branch. Auto-detects entry points, traces flows through changed code, finds every seam (cross-module calls, serialization, file I/O, shared state, schema versioning, network/IPC), and refuses to mark the work complete until each unverified boundary has a real round-trip test or an explicit written out-of-scope record persisted in an audit file. Use whenever the user says "boundary check", "seam check", "round-trip check", "flow boundaries", "user-flow check", "before merge", "is this safe to ship", "pre-merge gate", "boundary bugs", "verify the joins", or asks to validate cross-module joins, producer/consumer contracts, or end-to-end coverage of a change. Also use as a final gate from pr-review on any diff that touches more than one file, module, or process. Be pushy. Most surviving production bugs live at seams, not inside units — if the diff crosses any boundary, this skill almost certainly applies.
development
Run existing work through 5 specialist craftspeople who each produce an improved version, then peer-review and synthesize the best into a single improved artifact. Use when the user says "forge this", "improve this", "make this better", "level this up", "refine this", or asks for multi-angle improvement on code, copy, strategy, plans, designs, or any artifact where the current version works but could be significantly better. Do NOT use for decisions (use llm-council), simple edits, or creation from scratch.
development
Expert skill for maintaining a Keep a Changelog formatted CHANGELOG.md file. Use this skill whenever you add features, fix bugs, or release a new version. You MUST use this skill to record any changes that have a user-facing impact. It handles categorization (Added, Changed, Fixed, etc.), semantic versioning, and reverse-chronological ordering with surgical precision.
development
Comprehensive test suite audit that combines ruthless analysis with a solution-focused roadmap. Reads test suites (unit, integration, e2e) and source code, produces a brutal audit report of test quality and gaps, and generates prioritized testing improvements.