Reviewing Pull Requests

What this skill does

# Pull Request Review Workflow

This skill guides comprehensive PR reviews using the GitHub CLI and local code analysis.

## 0. Determine Starting Point

**When this skill is invoked:**

1. **If user is already in a PR worktree** (mentions "already in worktree", "skip to step 3", or "skip worktree setup"):
   - Skip directly to **step 3** (Gather PR Context)
   - Assume worktree is set up correctly
   - Proceed with the review

2. **If user provides a PR reference** (URL, org/repo#number, etc.) and is NOT in a worktree:
   - Parse the PR reference (see formats below)
   - Confirm with user before creating worktree
   - Proceed to **step 1** (Create worktree)

**Supported PR reference formats:**
- `<github_org>/<github_repo>/pull/<pull_request_number>`
- `<github_org>/<github_repo>#<pull_request_number>`
- Full github.com URL pointing to a specific pull request

**Note**: For setting up worktrees, you can also use the `/pr-review` slash command which handles the setup workflow and provides handoff instructions.

## 1. Creating a Git Worktree for the PR

**Determine the repository location:**

First, check if the repository exists locally on this machine:
- Look in common source code locations (~/src/, ~/repos/, etc.)
- Use the user's machine-specific path configuration if available
- If the repository doesn't exist locally, ask the user if they want to clone it first or provide the path

**Create a new worktree with the PR checked out:**

```bash
# Navigate to the repository (if not already there)
cd /path/to/<repo>

# Create worktree and check out the PR
# Use format: <repo_name>-pr-<pr_number> for the branch and directory
git worktree add ../<repo_name>-pr-<pr_number> -b <repo_name>-pr-<pr_number>
cd ../<repo_name>-pr-<pr_number>
gh pr checkout <pr_number>
```

**Example for vLLM project PR #12345 where vllm is at /Users/alice/repos/vllm:**
```bash
cd /Users/alice/repos/vllm
git worktree add ../vllm-pr-12345 -b vllm-pr-12345
cd ../vllm-pr-12345
gh pr checkout 12345
```

**Share .claude configuration across worktrees:**

After creating the worktree, set up `.claude/` configuration sharing if needed:

```bash
cd ../<repo_name>-pr-<pr_number>

# Check if .claude already exists (from git or previous setup)
if [ ! -e .claude ]; then
  # Get main worktree path
  MAIN_WORKTREE=$(git worktree list --porcelain | awk '/^worktree/ {print $2; exit}')

  # If .claude exists in main worktree, create symlink
  if [ -d "$MAIN_WORKTREE/.claude" ]; then
    ln -s "$MAIN_WORKTREE/.claude" .claude
    echo "Created .claude symlink to share configuration"
  fi
fi
```

**Why symlink .claude?**
- Ensures project-local configuration (review criteria, skills, commands) is available in the PR worktree
- Maintains consistency across all worktrees for the same repository
- If `.claude/` is committed to git, it's already available; if not, the symlink shares it

**CRITICAL SAFETY**: Never run code from the PR. It may contain untrusted code. Only read and analyze files.

## 2. Launch Claude Code in the Worktree

After creating the worktree, **STOP HERE**. Do NOT continue to step 3 in this session.

**CRITICAL HANDOFF POINT**: You must provide the user with instructions to launch a NEW Claude Code session in the worktree. The review MUST happen in a fresh session in the worktree directory, NOT in the current session.

**Why this matters:**
- Fresh context focused only on PR changes
- Correct working directory for running tests and examining code
- Isolation from the original repository/marketplace

**IMPORTANT**: Include ALL relevant skills that were loaded in the current session in the handoff prompt. This ensures the new Claude Code session has the full context needed for the review.

### Determining Relevant Skills

Before providing handoff instructions, identify which skills were loaded for this review:

1. **pr-review skill**: Always relevant (this skill)
2. **Repository-specific skills**: Any skills matching the repository being reviewed (e.g., llama-stack, vllm)
3. **Domain-specific skills**: Any skills relevant to the PR content (e.g., auth-security for authentication/authorization code)

**Example**: For a llama-stack PR, both `pr-review` and `llama-stack` skills would be relevant.

### Handoff Instruction Template

**IMPORTANT**: Only provide the plain text prompt below. Do NOT invent or reference non-existent slash commands (like `/continue-pr-review`). The only real slash command is `/bbrowning-claude:pr-review` which was already used to set up the worktree.

```
I've created a git worktree for PR #<pr_number> (<github_org>/<github_repo>) at: <worktree_path>

To continue the review in an isolated environment:

1. Open a new terminal
2. Navigate to the worktree: cd <worktree_path>
3. Launch Claude Code: claude
4. In the new Claude Code session, provide this prompt:

   > Review PR #<pr_number> for <github_org>/<github_repo>. I'm already in a git worktree with the PR checked out. Use [list ALL relevant skills: pr-review, <repo-specific>, <domain-specific>] and skip directly to step 3 (Gather PR Context) since the worktree is already set up.

This ensures we're reviewing the correct code in isolation with all necessary context.
```

**Why this matters:**
- Repository-specific skills contain critical domain knowledge (e.g., llama-stack's recordings/ handling, distributed systems concerns)
- Domain-specific skills provide specialized security or technical guidance
- Without all skills, the review loses important context and may miss critical issues

---

**⚠️ STOP: The steps below (3-9) are ONLY performed in the NEW Claude Code session within the worktree.**

**If you just created a worktree in step 1-2, DO NOT proceed to step 3. Provide handoff instructions and wait for the user to start a new session.**

**If you're in a fresh session and the user says "already in worktree" or "skip to step 3", then proceed with step 3.**

---

**IMPORTANT**: Remember to clean up the worktree after completing the review (see section 9 below).

## 3. Gather PR Context

**Fetch PR details:**
```bash
gh pr view <pr_number> --json title,body,commits,comments,reviews,files
```

Extract and note:
- PR title and description
- Number of commits and commit messages
- Files changed
- Existing comments and reviews
- Any unaddressed review comments

**Identify unaddressed comments:**
Look for review comments that:
- Have no replies from the PR author
- Requested changes that weren't made
- Raised concerns not acknowledged
- Are marked as unresolved

Flag these prominently in your review.

## 4. Analyze Code Changes

**Get the diff:**
```bash
gh pr diff <pr_number> > pr_changes.diff
```

For large PRs (>500 lines changed), break the review into logical sections (e.g., by file, by functionality).

Reference the local pr_changes.diff as you need to find changes in the PR over repeated calls to `gh pr diff`. And remember that you are already in a directory that has the PR cloned and checked out, so you can also look at local files.

**Review each changed file systematically:**

Use Read, Grep, and Glob tools to examine:
- Changed files and surrounding context
- Related files that might be affected
- Test files for the changed code
- Documentation for updated features

**Apply review checklist:**

For comprehensive criteria, see `reference/review-checklist.md`. Key areas:

1. **Code Quality**
   - Readability and maintainability
   - Follows project conventions
   - Appropriate abstraction levels
   - Error handling

2. **Correctness**
   - Logic is sound
   - Edge cases handled
   - No obvious bugs
   - Changes align with PR description

3. **Testing**
   - Tests included for new functionality
   - Tests cover edge cases
   - Existing tests still pass
   - Test quality is adequate

4. **Security**
   - No security vulnerabilities
   - Input validation present
   - No exposed secrets or credentials
   - Safe handling of user data
   - **CRITICAL**: Check for `pull_request_target` + checkout of untrust