Github Pr Review

GitHub PR Review Skill 🔍

Qualified, critical code reviews using GitHub's pending review API.

Use this skill when reviewing pull requests. Produces inline code comments, code suggestions, and a qualified verdict (APPROVE / REQUEST_CHANGES / COMMENT). Always validates that changes match the PR description before assessing quality.

Philosophy

• Be critical. Actively look for problems -- don't rubber-stamp.
• Validate claims. Does the code actually do what the PR description claims?
• Verify impact. Are there side effects, missing edge cases, security issues?
• Inline findings. Every issue goes directly on the affected line, not just in a summary.
• One review, one notification. Always batch via pending reviews.

Trigger Modes

This skill activates in one of these ways:

# Example: find PRs awaiting review (for scheduled mode)
gh pr list --repo <owner>/<repo> --json number,title,reviewDecision \
  --jq '.[] | select(.reviewDecision == "" or .reviewDecision == "REVIEW_REQUIRED")'

Prerequisites

• gh CLI installed and authenticated (gh auth status)
• Collaborator / write access on the target repo

Configuration

Adjust review depth based on model capabilities and budget:

Review Workflow

Step 1: Gather Context

REPO="owner/repo"
PR=42

# PR metadata
gh pr view $PR --repo $REPO --json title,body,state,baseRefName,headRefName,commits,files

# Full diff
gh pr diff $PR --repo $REPO

# Latest commit SHA (needed for review API)
COMMIT=$(gh pr view $PR --repo $REPO --json commits --jq '.commits[-1].oid')

Step 2: PR Hygiene Check

Before reviewing code, validate the PR itself:

Title & Description:

• Does the title follow conventional commits? (feat: / fix: / chore: / docs:)
• Does the description explain Why (context), What (changes), and Impact (outcome)?
• Are there changes NOT mentioned in the description? (scope creep)
• Are there claims in the description NOT reflected in the code? (incomplete)

Metadata:

• Is someone assigned?
• Are appropriate labels set? (enhancement, bug, documentation, etc.)
• Is the base branch correct? (usually main)

Scope:

• Is this a single focused change, or does it mix unrelated things?
• Could this be split into smaller PRs?

If any of these are missing or wrong, flag it in the review. PR hygiene is not optional -- sloppy PRs lead to sloppy repos.

Step 3: Analyze the Diff

Read every changed file. Prioritize checks by severity:

🔴 Always check (blocking):

🟡 Check if relevant (important):

🟢 Nice to have (non-blocking):

Step 4: Create Pending Review with Inline Comments

Always use the 2-step pending review pattern -- even for a single comment.

# Create pending review with inline comments
gh api repos/$REPO/pulls/$PR/reviews \
  -X POST \
  -f commit_id="$COMMIT" \
  -f 'comments[][path]=src/auth.ts' \
  -F 'comments[][line]=25' \
  -f 'comments[][side]=RIGHT' \
  -f 'comments[][body]=Missing null check -- `user` can be undefined when token is expired.

```suggestion
if (!user) {
  throw new AuthError("Invalid or expired token");
}
```' \
  -f 'comments[][path]=src/auth.ts' \
  -F 'comments[][line]=40' \
  -f 'comments[][side]=RIGHT' \
  -f 'comments[][body]=This catches all errors silently. At minimum, log the error.' \
  --jq '{id, state}'

# Returns: {"id": 12345, "state": "PENDING"}

Parameter reference:

Syntax rules:

• Always single-quote 'comments[][...]' parameters
• -f for strings, -F for numbers
• Code suggestions replace the entire specified line range

Step 5: Submit the Review

REVIEW_ID=12345  # from Step 3 response

gh api repos/$REPO/pulls/$PR/reviews/$REVIEW_ID/events \
  -X POST \
  -f event="REQUEST_CHANGES" \
  -f body="## Review Summary

Found 2 issues:
1. Missing null check in auth flow (blocking)
2. Silent error swallowing (blocking)

Please address before merge."

Step 6: Post-Review Action

After submitting:

⚠️ Auto-merge is opt-in per repo/team. Don't auto-merge unless explicitly configured. Check your agent's operational rules (MEMORY.md / AGENTS.md) for repo-specific merge policies.

Step 7: Handle Author Responses

After submitting a review, the PR author may reply to your comments. Follow up:

If the author pushed fixes:

• Re-review the new diff (gh pr diff) -- don't just trust "fixed"
• Reply to your original comments confirming the fix or flagging remaining issues
• Submit a new review with updated verdict

If the author disputes a finding:

• Read their argument carefully -- you might be wrong
• Reply directly on the comment thread (not in a new review)
• If they're right: acknowledge it and resolve
• If you still disagree: explain why with specific reasoning

If the author asks a question:

• Answer directly on the comment thread
• If it reveals you made a wrong assumption, update your review

# Reply to a review comment thread
gh api repos/$REPO/pulls/$PR/comments/<COMMENT_ID>/replies \
  -X POST -f body="Good point -- I missed that the null check happens upstream. Withdrawing this finding."

After confirming a fix: Resolve the conversation thread so the PR shows a clean state before merge.

# List open review threads to find IDs
gh api graphql -f query='
  query {
    repository(owner: "<owner>", name: "<repo>") {
      pullRequest(number: <PR>) {
        reviewThreads(first: 50) {
          nodes { id isResolved comments(first: 1) { nodes { body } } }
        }
      }
    }
  }
' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)'

# Resolve a confirmed thread
gh api graphql -f query='
  mutation {
    resolveReviewThread(input: {threadId: "<THREAD_NODE_ID>"}) {
      thread { isResolved }
    }
  }
'

Event Type Decision Guide

Is there a bug, security issue, or broken behavior?
  → YES → REQUEST_CHANGES

Is something missing (tests, error handling, docs)?
  → YES → REQUEST_CHANGES

Are there only style/naming/optional improvements?
  → YES → APPROVE (with suggestions as non-blocking comments)

Just questions or neutral observations?
  → COMMENT

Review Body Structure

The overall review body (Step 5) should follow this template:

## ✅ / ❌ / 💬 Code Review

**Verdict: APPROVE / REQUEST_CHANGES / COMMENT**

### Validation
- Does the code match the PR description?
- Are the changes correct and complete?
- Are there changes NOT mentioned in the PR description? (scope creep)
- Are there claims in the description NOT reflected in the code? (incomplete)

### Verification
- Side effects? Breaking changes? Missing tests?
- Security or performance concerns?

### Risk Assessment
- Low / Medium / High -- with reasoning

Code Suggestions

Use GitHub's suggestion syntax for concrete fixes:

This variable name is misleading -- it holds a list, not a single item.

```suggestion
const users = await fetchUsers();
```

Clear naming helps the next reader.

Multi-line suggestions use start_line:

-F 'comments[][start_line]=10' \
-F 'comments[][line]=15' \

This replaces lines 10–15 with the suggestion content.

Common Pitfalls

Secret Detection in Diffs

Every diff line must be scanned for potential secrets -- including docs, examples, and markdown.

Heuristics

A string is suspicious if:

1. Length ≥ 20 characters and high entropy (mixed case, digits, no dictionary words)
2. Looks like a token format: Base62, Base64, hex, sk-..., tok-..., ghp_..., clw_...
3. Not an obvious placeholder: your-token-here, xxx, <TOKEN>, example-key, sk-...your-key...
4. Context is auth-related: the PR touches auth docs, token handling, API key usage, or .env examples

What to do

• If suspicious → REQUEST_CHANGES with: "This looks like a real secret. Please replace with a placeholder."
• If the author confirms it's a dummy → they can re-push with a comment explaining why
• Never approve a PR with a suspicious high-entropy string without explicitly verifying it's not real

Elevated sensitivity triggers

When a PR is about auth, tokens, API keys, or secrets handling, assume every token-shaped string is real until proven otherwise. The subject matter itself is the red flag.

Real-world example (incident)

A docs PR added a callout explaining correct X-API-Key usage. The example contained a real agent token (hlFPU...32chars). It was approved and merged because the reviewer classified it as "docs-only, zero risk." The token had to be rotated.

Lesson: "Docs-only" is never "zero risk" when the diff contains strings that look like secrets.

Review Without Inline Comments

For clean PRs with no issues, skip the 2-step pattern:

gh api repos/$REPO/pulls/$PR/reviews \
  -X POST \
  -f commit_id="$COMMIT" \
  -f event="APPROVE" \
  -f body="## ✅ Code Review

**Verdict: APPROVE**

Reviewed all changed files. Clean removal of dead code, no side effects, no orphaned references.

**Risk: Zero.**"

Part of the agentic-foundation skill library.