pr-review-expert

# View diff in terminal
gh pr diff <PR_NUMBER>
Get PR metadata (title, body, labels, linked issues)
gh pr view <PR_NUMBER> --json title,body,labels,assignees,milestone
List files changed
gh pr diff <PR_NUMBER> --name-only
Check CI status
gh pr checks <PR_NUMBER>
Download diff to file for analysis
gh pr diff <PR_NUMBER> > /tmp/pr-<PR_NUMBER>.diff

# View MR diff
glab mr diff <MR_IID>
MR details as JSON
glab mr view <MR_IID> --output json
List changed files
glab mr diff <MR_IID> --name-only
Download diff
glab mr diff <MR_IID> > /tmp/mr-<MR_IID>.diff

PR=123
gh pr view $PR --json title,body,labels,milestone,assignees | jq .
gh pr diff $PR --name-only
gh pr diff $PR > /tmp/pr-$PR.diff

# Find all files importing a changed module
grep -r "from ['\"].*changed-module['\"]" src/ --include="*.ts" -l
grep -r "require(['\"].*changed-module" src/ --include="*.js" -l

Python
grep -r "from changed_module import\|import changed_module" . --include="*.py" -l

# Check if changed files span multiple services (monorepo)
gh pr diff $PR --name-only | cut -d/ -f1-2 | sort -u

gh pr diff $PR --name-only | grep -E "types/|interfaces/|schemas/|models/"

DIFF=/tmp/pr-$PR.diff
SQL Injection — raw query string interpolation
grep -n "query\|execute\|raw(" $DIFF | grep -E '\$\{|f"|%s|format\('
Hardcoded secrets
grep -nE "(password|secret|api_key|token|private_key)\s*=\s*['\"][^'\"]{8,}" $DIFF
AWS key pattern
grep -nE "AKIA[0-9A-Z]{16}" $DIFF
JWT secret in code
grep -nE "jwt\.sign\(.*['\"][^'\"]{20,}['\"]" $DIFF
XSS vectors
grep -n "dangerouslySetInnerHTML\|innerHTML\s*=" $DIFF
Auth bypass patterns
grep -n "bypass\|skip.*auth\|noauth\|TODO.*auth" $DIFF
Insecure hash algorithms
grep -nE "md5\(|sha1\(|createHash\(['\"]md5|createHash\(['\"]sha1" $DIFF
eval / exec
grep -nE "\beval\(|\bexec\(|\bsubprocess\.call\(" $DIFF
Prototype pollution
grep -n "__proto__\|constructor\[" $DIFF
Path traversal risk
grep -nE "path\.join\(.*req\.|readFile\(.*req\." $DIFF

# Count source vs test files changed
CHANGED_SRC=$(gh pr diff $PR --name-only | grep -vE "\.test\.|\.spec\.|__tests__")
CHANGED_TESTS=$(gh pr diff $PR --name-only | grep -E "\.test\.|\.spec\.|__tests__")
echo "Source files changed: $(echo "$CHANGED_SRC" | wc -w)"
echo "Test files changed:   $(echo "$CHANGED_TESTS" | wc -w)"
Lines of new logic vs new test lines
LOGIC_LINES=$(grep "^+" /tmp/pr-$PR.diff | grep -v "^+++" | wc -l)
echo "New lines added: $LOGIC_LINES"
Run coverage locally
npm test -- --coverage --changedSince=main 2>/dev/null | tail -20
pytest --cov --cov-report=term-missing 2>/dev/null | tail -20

# OpenAPI/Swagger spec changes
grep -n "openapi\|swagger" /tmp/pr-$PR.diff | head -20

REST route removals or renames
grep "^-" /tmp/pr-$PR.diff | grep -E "router\.(get|post|put|delete|patch)\("
GraphQL schema removals
grep "^-" /tmp/pr-$PR.diff | grep -E "^-\s*(type |field |Query |Mutation )"
TypeScript interface removals
grep "^-" /tmp/pr-$PR.diff | grep -E "^-\s*(export\s+)?(interface|type) "

# Migration files added
gh pr diff $PR --name-only | grep -E "migrations?/|alembic/|knex/"

Destructive operations
grep -E "DROP TABLE|DROP COLUMN|ALTER.*NOT NULL|TRUNCATE" /tmp/pr-$PR.diff
Index removals (perf regression risk)
grep "DROP INDEX\|remove_index" /tmp/pr-$PR.diff

# New env vars referenced in code (might be missing in prod)
grep "^+" /tmp/pr-$PR.diff | grep -oE "process\.env\.[A-Z_]+" | sort -u

Removed env vars (could break running instances)
grep "^-" /tmp/pr-$PR.diff | grep -oE "process\.env\.[A-Z_]+" | sort -u

# N+1 query patterns (DB calls inside loops)
grep -n "\.find\|\.findOne\|\.query\|db\." /tmp/pr-$PR.diff | grep "^+" | head -20
Then check surrounding context for forEach/map/for loops
Heavy new dependencies
grep "^+" /tmp/pr-$PR.diff | grep -E '"[a-z@].*":\s*"[0-9^~]' | head -20
Unbounded loops
grep -n "while (true\|while(true" /tmp/pr-$PR.diff | grep "^+"
Missing await (accidentally sequential promises)
grep -n "await.*await" /tmp/pr-$PR.diff | grep "^+" | head -10
Large in-memory allocations
grep -n "new Array([0-9]\{4,\}\|Buffer\.alloc" /tmp/pr-$PR.diff | grep "^+"

# Extract ticket references from PR body
gh pr view $PR --json body | jq -r '.body' | \
  grep -oE "(PROJ-[0-9]+|[A-Z]+-[0-9]+|https://linear\.app/[^)\"]+)" | sort -u
Verify Jira ticket exists (requires JIRA_API_TOKEN to be SET in the environment).
Credentials are fed to curl via a config read from stdin (-K -) so the token
never appears in argv — ps aux / /proc/*/cmdline can't see it, and nothing
secret lands in shell history. Never paste the raw token on the command line.
TICKET="PROJ-123"
: "${JIRA_API_TOKEN:?JIRA_API_TOKEN must be set}"
curl -s -K - "https://your-org.atlassian.net/rest/api/3/issue/$TICKET" <<EOF | \
  jq '{key, summary: .fields.summary, status: .fields.status.name}'
user = "[email protected]:$JIRA_API_TOKEN"
EOF
Linear ticket — same pattern: the Authorization header goes through the
stdin config, not a -H flag, to keep the key out of the process list.
LINEAR_ID="abc-123"
: "${LINEAR_API_KEY:?LINEAR_API_KEY must be set}"
curl -s -K - -H "Content-Type: application/json" \
  --data "{\"query\": \"{ issue(id: \\\"$LINEAR_ID\\\") { title state { name } } }\"}" \
  https://api.linear.app/graphql <<EOF | jq .
header = "Authorization: $LINEAR_API_KEY"
EOF

## Code Review Checklist
Scope & Context
[ ] PR title accurately describes the change
[ ] PR description explains WHY, not just WHAT
[ ] Linked Jira/Linear ticket exists and matches scope
[ ] No unrelated changes (scope creep)
[ ] Breaking changes documented in PR body
Blast Radius
[ ] Identified all files importing changed modules
[ ] Cross-service dependencies checked
[ ] Shared types/interfaces/schemas reviewed for breakage
[ ] New env vars documented in .env.example
[ ] DB migrations are reversible (have down() / rollback)
Security
[ ] No hardcoded secrets or API keys
[ ] SQL queries use parameterized inputs (no string interpolation)
[ ] User inputs validated/sanitized before use
[ ] Auth/authorization checks on all new endpoints
[ ] No XSS vectors (innerHTML, dangerouslySetInnerHTML)
[ ] New dependencies checked for known CVEs
[ ] No sensitive data in logs (PII, tokens, passwords)
[ ] File uploads validated (type, size, content-type)
[ ] CORS configured correctly for new endpoints
Testing
[ ] New public functions have unit tests
[ ] Edge cases covered (empty, null, max values)
[ ] Error paths tested (not just happy path)
[ ] Integration tests for API endpoint changes
[ ] No tests deleted without clear reason
[ ] Test names clearly describe what they verify
Breaking Changes
[ ] No API endpoints removed without deprecation notice
[ ] No required fields added to existing API responses
[ ] No DB columns removed without two-phase migration plan
[ ] No env vars removed that may be set in production
[ ] Backward-compatible for external API consumers
Performance
[ ] No N+1 query patterns introduced
[ ] DB indexes added for new query patterns
[ ] No unbounded loops on potentially large datasets
[ ] No heavy new dependencies without justification
[ ] Async operations correctly awaited
[ ] Caching considered for expensive repeated operations
Code Quality
[ ] No dead code or unused imports
[ ] Error handling present (no bare empty catch blocks)
[ ] Consistent with existing patterns and conventions
[ ] Complex logic has explanatory comments
[ ] No unresolved TODOs (or tracked in ticket)

## PR Review: [PR Title] (#NUMBER)
Blast Radius: HIGH — changes lib/auth used by 5 services
Security: 1 finding (medium severity)
Tests: Coverage delta +2%
Breaking Changes: None detected
--- MUST FIX (Blocking) ---
1. SQL Injection risk in src/db/users.ts:42
   Raw string interpolation in WHERE clause.
   Fix: db.query("SELECT * WHERE id = $1", [userId])
--- SHOULD FIX (Non-blocking) ---
2. Missing auth check on POST /api/admin/reset
   No role verification before destructive operation.
--- SUGGESTIONS ---
3. N+1 pattern in src/services/reports.ts:88
   findUser() called inside results.map() — batch with findManyUsers(ids)
--- LOOKS GOOD ---
Test coverage for new auth flow is thorough

DB migration has proper down() rollback method

Error handling consistent with rest of codebase