CtrlK
BlogDocsLog inGet started
Tessl Logo

prp-review

Comprehensive PR code review - checks diff, patterns, runs validation, comments on PR. Single-pass by default; pass --agents for a multi-agent specialist fan-out (comments, tests, errors, types, code, docs, simplify). Use when the user wants to review a pull request, wants a thorough/multi-agent review, or invokes the prp-review skill.

69

Quality

85%

Does it follow best practices?

Run evals on this skill

Adds up to 20 points to the overall score

View guide

SecuritybySnyk

Low

Low-risk findings worth noting

SKILL.md
Quality
Evals
Security

Kild lane: you are running inside a kild room, in a workspace (worktree + branch) the kild engine assigned. The driver owns isolation and publishing — SKIP any step below that creates or switches branches or worktrees, pulls or rebases the base branch, pushes, opens PRs, or moves/archives plan artifacts, and never run gh pr checkout. Your job ends at implement → validate → commit in the current workspace, reporting evidence. Where a step spawns subagents, do that analysis inline — or ask the room's orchestrator to invite a helper agent.

Arguments: $ARGUMENTS (and $1, $2, ...) refer to the arguments given when this skill was invoked. Take them from the user's request; if absent, infer them from the conversation.

PR Code Review

Input: $ARGUMENTS


# --- PRP store resolver (canonical; keep byte-identical across skills) ---
_gd="$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null)"
case "$_gd" in */.git) _root="${_gd%/.git}" ;; "") _root="$PWD" ;; *) _root="$_gd" ;; esac
_root="$(cd "$_root" && pwd -P)"
_name="$(basename "$_root" | tr '[:upper:]' '[:lower:]' | tr -cs 'a-z0-9' '-' | sed 's/^-*//;s/-*$//')"
PRP_DIR="${PRP_HOME:-$HOME/.prp}/${_name:-project}-$(printf %s "$_root" | git hash-object --stdin | cut -c1-8)"
mkdir -p "$PRP_DIR"; [ -f "$PRP_DIR/project.json" ] || printf '{"path": "%s", "name": "%s"}\n' "$_root" "${_name:-project}" > "$PRP_DIR/project.json"

Mode Select

This skill has two modes:

ModeTriggerWhat it does
Single-pass (default)no --agents flagOne reviewer performs the full 8-phase review below
Multi-agent fan-out--agents flag present, or aspect keywords (comments/tests/errors/types/code/docs/simplify/all), or the user asks for a "multi-agent"/"thorough" reviewDispatches specialized agents in parallel, one per aspect

If multi-agent mode is selected: follow workflows/agents.md instead of the phases below, then stop. Everything below this section is the single-pass review.

Single-pass is the cheaper default; reach for --agents on large diffs or when explicitly asked.


Your Mission

Perform a thorough, senior-engineer-level code review:

  1. Understand what the PR is trying to accomplish
  2. Check the code against project patterns and constraints
  3. Run all validation (type-check, lint, tests, build)
  4. Identify issues by category (Critical / Important / Suggestions / Strengths)
  5. Report findings as PR comment AND local file

Golden Rule: Be constructive and actionable. Every issue should have a clear recommendation. Acknowledge good work too.


Phase 1: FETCH - Get PR Context

1.1 Parse Input

Determine input type:

Input FormatAction
Number (123, #123)Use as PR number
URL (https://github.com/.../pull/123)Extract PR number
Branch name (feature-x)Find associated PR
# If branch name provided, find PR
gh pr list --head {branch-name} --json number -q '.[0].number'

1.2 Get PR Metadata

# Get comprehensive PR details
gh pr view {NUMBER} --json number,title,body,author,headRefName,baseRefName,state,additions,deletions,changedFiles,files,reviews,comments

# Get the diff
gh pr diff {NUMBER}

# List changed files
gh pr diff {NUMBER} --name-only

Extract:

  • PR number, title, description
  • Author
  • Base and head branches
  • Files changed with line counts
  • Existing review comments

1.3 Checkout PR Branch

# Fetch and checkout the PR branch
gh pr checkout {NUMBER}

1.4 Validate PR State

StateAction
MERGEDSTOP: "PR already merged. Nothing to review."
CLOSEDWARN: "PR is closed. Review anyway? (historical analysis)"
DRAFTNOTE: "Draft PR - focusing on direction, not polish"
OPENPROCEED with full review

PHASE_1_CHECKPOINT:

  • PR number identified
  • PR metadata fetched
  • PR branch checked out
  • PR state is reviewable

Phase 2: CONTEXT - Understand the Change

2.1 Read Project Rules

Read and internalize:

# Project conventions
cat CLAUDE.md

# Check for additional reference docs
ls -la .claude/docs/ 2>/dev/null
ls -la docs/ 2>/dev/null

Extract key constraints:

  • Type safety requirements
  • Code style rules
  • Testing requirements
  • Architecture patterns

2.2 Find Implementation Context

Look for implementation artifacts:

# Find implementation report by branch name. Legacy hits require migration.
ls "$PRP_DIR"/reports/*{branch-name}*.md 2>/dev/null || ls .claude/PRPs/reports/*{branch-name}*.md 2>/dev/null

# Find completed plans. Legacy hits require migration.
ls "$PRP_DIR"/plans/completed/ 2>/dev/null || ls .claude/PRPs/plans/completed/ 2>/dev/null

# Find issue investigations. Legacy hits require migration.
ls "$PRP_DIR"/issues/completed/ 2>/dev/null || ls .claude/PRPs/issues/completed/ 2>/dev/null

If a legacy in-repo path is found, tell the user to run the PRP home-store migration before continuing.

If implementation report exists:

  1. Read the implementation report
  2. Read the referenced plan
  3. Note documented deviations - these are INTENTIONAL, not issues

If no implementation report:

  • PR may not have been created via the prp-implement skill
  • Review normally without plan context
  • Note in review that no implementation report was found

2.3 Understand PR Intent

From PR title, description, AND implementation report (if available):

  • What problem does this solve?
  • What approach was taken?
  • Are there notes from the author?
  • What deviations from plan were documented and why?

2.4 Analyze Changed Files

For each changed file, determine:

  • What type of file? (service, handler, util, test, config)
  • What existing patterns should it follow?
  • Scope of change? (new file, modification, deletion)

PHASE_2_CHECKPOINT:

  • Project rules read and understood
  • Implementation artifacts located (if any)
  • PR intent understood
  • Changed files categorized

Phase 3: REVIEW - Analyze the Code

3.1 Read Each Changed File

For each file in the diff:

  1. Read the full file (not just diff) to understand context
  2. Read similar files to understand expected patterns
  3. Check specific changes against patterns

3.2 Review Checklist

For EVERY changed file, check:

Correctness

  • Does the code do what the PR claims?
  • Are there logic errors?
  • Are edge cases handled?
  • Is error handling appropriate?

Type Safety

  • Are all types explicit (no implicit any)?
  • Are return types declared?
  • Are interfaces used appropriately?
  • Are type guards used where needed?

Pattern Compliance

  • Does it follow existing patterns in the codebase?
  • Is naming consistent with project conventions?
  • Is file organization correct?
  • Are imports from the right places?

Security

  • Any user input without validation?
  • Any secrets that could be exposed?
  • Any injection vulnerabilities (SQL, command, etc.)?
  • Any unsafe operations?

Performance

  • Any obvious N+1 queries or loops?
  • Any unnecessary async/await?
  • Any memory leaks (unclosed resources, growing arrays)?
  • Any blocking operations in hot paths?

Completeness

  • Are there tests for new code?
  • Is documentation updated if needed?
  • Are all TODOs addressed?
  • Is error handling complete?

Maintainability

  • Is the code readable?
  • Is it over-engineered?
  • Is it under-engineered (missing necessary abstractions)?
  • Are there magic numbers/strings that should be constants?

3.3 Categorize Issues

Important: Check implementation report first!

If a deviation from expected patterns is documented in the implementation report with a valid reason, it is NOT an issue - it's an intentional decision. Only flag undocumented deviations.

Finding Categories (shared with --agents mode — see templates/review-report.md):

CategoryIconCriteriaExamples
CriticalREDMust fix before merge - blockingSecurity vulnerabilities, data loss potential, crashes
ImportantORANGEShould fix before mergeType safety violations, missing error handling, logic errors
SuggestionsYELLOWNice to have - considerPattern inconsistencies, missing edge cases, undocumented deviations, style preferences, minor optimizations, documentation
StrengthsGREENWhat's good - acknowledgeGood patterns, clean code, thorough tests

PHASE_3_CHECKPOINT:

  • All changed files reviewed
  • Issues categorized
  • Implementation report deviations accounted for
  • Positive aspects noted

Phase 4: VALIDATE - Run Automated Checks

4.1 Run Validation Suite

# Type checking (adapt to project)
npm run type-check || bun run type-check || npx tsc --noEmit

# Linting
npm run lint || bun run lint

# Tests
npm test || bun test

# Build
npm run build || bun run build

Capture for each:

  • Pass/fail status
  • Error count
  • Warning count
  • Any specific failures

4.2 Specific Validation

Based on what changed:

Change TypeAdditional Validation
New API endpointTest with curl/httpie
Database changesCheck migration exists
Config changesVerify .env.example updated
New dependenciesCheck package.json/lock file

4.3 Regression Check

# Full test suite
npm test || bun test

# Specific tests for changed functionality
npm test -- {relevant-test-pattern}

PHASE_4_CHECKPOINT:

  • Type check executed
  • Lint executed
  • Tests executed
  • Build executed
  • Results captured

Phase 5: DECIDE - Form Recommendation

5.1 Decision Logic

APPROVE if:

  • No Critical or Important issues
  • All validation passes
  • Code follows patterns
  • Changes match PR intent

REQUEST CHANGES if:

  • Important issues exist
  • Validation fails but is fixable
  • Pattern violations that need addressing
  • Missing tests for new functionality

BLOCK if:

  • Critical security or data issues
  • Fundamental approach is wrong
  • Major architectural concerns
  • Breaking changes without migration

5.2 Special Cases

SituationHandling
Draft PRComment only, no approve/block
Large PR (>500 lines)Note thoroughness limits, suggest splitting
Security-sensitiveExtra scrutiny, err on caution
Missing testsStrong recommendation, may not block

PHASE_5_CHECKPOINT:

  • Recommendation determined
  • Rationale clear

Phase 6: REPORT - Generate Review

6.1 Create Report Directory

mkdir -p "$PRP_DIR/reviews"

6.2 Generate Report File

Path: $PRP_DIR/reviews/pr-{NUMBER}-review.md (report the expanded absolute path to the user).

---
pr: {NUMBER}
title: "{TITLE}"
author: "{AUTHOR}"
reviewed: {ISO_TIMESTAMP}
recommendation: {approve|request-changes|block}
---

# PR Review: #{NUMBER} - {TITLE}

**Author**: @{author}
**Branch**: {head} -> {base}
**Files Changed**: {count} (+{additions}/-{deletions})

---

## Summary

{2-3 sentences: What this PR does and overall assessment}

---

## Implementation Context

| Artifact | Path |
|----------|------|
| Implementation Report | `{path}` or "Not found" |
| Original Plan | `{path}` or "Not found" |
| Documented Deviations | {count} or "N/A" |

{If implementation report exists: Brief note about deviation documentation quality}

---

## Changes Overview

| File | Changes | Assessment |
|------|---------|------------|
| `{path/to/file.ts}` | +{N}/-{M} | {PASS/WARN/FAIL} |

---

## Issues Found

### Critical
{If none: "No critical issues found."}

- **`{file.ts}:{line}`** - {Issue description}
  - **Why**: {Explanation of the problem}
  - **Fix**: {Specific recommendation}

### Important
{Issues that should be fixed before merge}

### Suggestions
{Issues worth considering but not blocking, nice-to-haves, and future improvements}

---

## Validation Results

| Check | Status | Details |
|-------|--------|---------|
| Type Check | {PASS/FAIL} | {notes} |
| Lint | {PASS/WARN} | {count} warnings |
| Tests | {PASS/FAIL} | {count} passed |
| Build | {PASS/FAIL} | {notes} |

---

## Pattern Compliance

- [{x}] Follows existing code structure
- [{x}] Type safety maintained
- [{x}] Naming conventions followed
- [{x}] Tests added for new code
- [{x}] Documentation updated

---

## Strengths

{Acknowledge positive aspects - good patterns, clean code, thorough tests, etc.}

---

## Recommendation

**{APPROVE/REQUEST CHANGES/BLOCK}**

{Clear explanation of recommendation and what needs to happen next}

---

*Reviewed by Claude*
*Report: `{expanded absolute path to $PRP_DIR/reviews/pr-{NUMBER}-review.md}`*

PHASE_6_CHECKPOINT:

  • Report file created
  • All sections populated

Phase 7: PUBLISH - Post to GitHub

7.1 Determine Review Action

Based on recommendation and flags:

# If --approve flag AND no Critical/Important issues
gh pr review {NUMBER} --approve --body-file "$PRP_DIR/reviews/pr-{NUMBER}-review.md"

# If --request-changes flag OR Important issues found
gh pr review {NUMBER} --request-changes --body-file "$PRP_DIR/reviews/pr-{NUMBER}-review.md"

# Otherwise just comment
gh pr comment {NUMBER} --body-file "$PRP_DIR/reviews/pr-{NUMBER}-review.md"

7.2 Get Comment URL

# Get the review/comment URL
gh pr view {NUMBER} --json reviews,comments --jq '.reviews[-1].url // .comments[-1].url'

PHASE_7_CHECKPOINT:

  • Review/comment posted to PR
  • Comment URL captured

Phase 8: OUTPUT - Report to User

## PR Review Complete

**PR**: #{NUMBER} - {TITLE}
**URL**: {PR_URL}
**Recommendation**: {APPROVE/REQUEST CHANGES/BLOCK}

### Issues Found

| Category | Count |
|----------|-------|
| Critical | {N} |
| Important | {N} |
| Suggestions | {N} |

### Validation

| Check | Result |
|-------|--------|
| Type Check | {PASS/FAIL} |
| Lint | {PASS/FAIL} |
| Tests | {PASS/FAIL} |
| Build | {PASS/FAIL} |

### Artifacts

- Report: `{expanded absolute path to $PRP_DIR/reviews/pr-{NUMBER}-review.md}`
- PR Comment: {comment_url}

### Next Steps

{Based on recommendation:}
- APPROVE: "PR is ready for merge"
- REQUEST CHANGES: "Author should address {N} Important issues"
- BLOCK: "Fundamental issues need resolution before proceeding"

Critical Reminders

  1. Understand before judging. Read full context, not just the diff.

  2. Be specific. "This could be better" is useless. "Use execFile instead of exec to prevent command injection at line 45" is helpful.

  3. Prioritize. Not everything is critical. Use the categories honestly.

  4. Be constructive. Offer solutions, not just problems.

  5. Acknowledge good work. If something is done well, say so.

  6. Run validation. Don't skip automated checks.

  7. Check patterns. Read existing similar code to understand expectations.

  8. Think about edge cases. What happens with null, empty, very large, concurrent?

  9. Check implementation report. Documented deviations are intentional, not issues.


Success Criteria

  • CONTEXT_GATHERED: PR metadata, diff, and implementation artifacts reviewed
  • CODE_REVIEWED: All changed files analyzed against checklist
  • VALIDATION_RUN: All automated checks executed
  • ISSUES_CATEGORIZED: Findings organized by category
  • REPORT_GENERATED: Comprehensive review saved locally
  • PR_UPDATED: Review/comment posted to GitHub
  • RECOMMENDATION_CLEAR: Approve/request-changes/block with rationale
Repository
Wirasm/prp
Last updated
First committed

Is this your skill?

If you maintain this skill, you can claim it as your own. Once claimed, you can manage eval scenarios, bundle related skills, attach documentation or rules, and ensure cross-agent compatibility.