pr-reviewer

Files (3)

• REFERENCE.md
• SKILL.md
• independent-council.md · references/

SKILL.md (first lines)

# PR 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-readiness` checklist

## Procedure

1. **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

2. **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

3. **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?

4. **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?

5. **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

6. **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

7. **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](references/independent-council.md) 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`](./REFERENCE.md) for a quick lookup of common patterns and their preferred alternatives.

From jpantsjoha/ai-native-developer-experience, Apache-2.0.

More from jpantsjoha/ai-native-developer-experience