pr-reviewer
Review gate for correctness, reuse, and simplification. Produces a structured verdict with actionable findings. Trigger on any PR before merge, or on any agent-generated code before it is committed.
SKILL.mdskills/pr-reviewer/SKILL.md
GitHubPR Reviewer
Receipts, not polish. A review that says "looks good" is not a review. A review that says "line 47 will panic on a nil pointer and here is the fix" is a review.
This skill runs a structured code review and produces a verdict: PASS or FAIL. Every finding is actionable. No findings without evidence.
When to use
- Before merging any PR
- When reviewing agent-generated code before committing
- When a significant refactor or new feature is complete
- As part of the
release-readinesschecklist
Procedure
Scope the review — fetch the diff. Identify:
- Files changed and their purpose
- The stated intent of the PR (what problem does it solve?)
- Any linked spec, issue, or ADR
Correctness pass — look for bugs, not style:
- Null / nil / undefined access on values that could be absent
- Off-by-one errors in loops or index operations
- Error returns that are silently ignored
- Race conditions if the code is concurrent
- Missing input validation at trust boundaries (user input, external API responses)
- Logic that diverges from the stated intent of the PR
Reuse pass — look for duplication:
- Does this code reimplement something that already exists in the codebase?
- Is there a stdlib or already-installed dependency that does this?
- Can the new code be expressed using an existing abstraction?
Simplification pass — look for unnecessary complexity:
- Can this be fewer lines without losing clarity?
- Is there an abstraction that has only one implementation? (remove it)
- Is there a dependency added for something a few lines of code would do?
- Is there "scaffolding for later" that should not exist yet?
Security and data boundary pass — look for:
- Secrets or credentials in code or logs
- User-controlled input reaching a shell, SQL query, or file path without validation
- Data crossing a tenant or trust boundary without an explicit check
- Missing authentication or authorisation checks on new endpoints
Classify findings:
- BLOCKING: must be fixed before merge (correctness bugs, security issues)
- SUGGESTED: improvement worth making but not a blocker (simplification, reuse)
- NOTE: observation for awareness, no action required
Produce the verdict:
- PASS: no unresolved conditions; optional suggestions are explicitly nonblocking
- FAIL: one or more unresolved conditions
Outputs
- Finding list: location | severity | description | suggested fix
- Verdict: PASS / FAIL; conditions never count as approval
Independent review and Council
The independent review must include an adversarial subagent activity: attempt to refute requirement acceptance, exercise failure/recovery assumptions and identify tests that miss the real user path. One fresh reviewer may cover these lenses; extra agents are not a quota. Give it requirement links and the exact candidate, not the author's desired verdict.
The writer never approves its own work. Use a fresh, read-only reviewer context on a permitted host and retain a substantive verdict bound to the exact candidate. A provider change is optional and never grants authority. Missing review is incomplete.
Read independent Council review before dispatch or when a review loop stalls. It separates the shared contract, host adapters and local subscription policy, preserves unresolved findings across reviewers, and defines bounded escalation. It also documents the optional fail-closed local receipt helper.
Guardrails
- Every BLOCKING finding must have a suggested fix. "This is wrong" is not actionable.
- Style is not a BLOCKING finding. Style enforcement belongs in the linter, not the review.
- Reuse requires evidence. "This might already exist" is not a finding. Find it or drop the comment.
- The review is not a rewrite. Surface the issues; let the author fix them.
See also: REFERENCE.md for a quick lookup of common patterns and their preferred alternatives.