PR Review Expert

PR审核专家

Tier: POWERFUL Category: Engineering Domain: Code Review / Quality Assurance

等级： POWERFUL 类别： 工程 领域： 代码审核 / 质量保障

Overview

概述

Structured, systematic code review for GitHub PRs and GitLab MRs. Goes beyond style nits — this skill performs blast radius analysis, security scanning, breaking change detection, and test coverage delta calculation. Produces a reviewer-ready report with a 30+ item checklist and prioritized findings.

针对GitHub PR和GitLab MR的结构化、系统化代码审核。不止于格式细节——该技能可执行影响范围分析、安全扫描、破坏性变更检测以及测试覆盖率差异计算。生成包含30+项检查清单和优先级发现的审核就绪报告。

Core Capabilities

核心能力

Blast radius analysis — trace which files, services, and downstream consumers could break
Security scan — SQL injection, XSS, auth bypass, secret exposure, dependency vulns
Test coverage delta — new code vs new tests ratio
Breaking change detection — API contracts, DB schema migrations, config keys
Ticket linking — verify Jira/Linear ticket exists and matches scope
Performance impact — N+1 queries, bundle size regression, memory allocations

影响范围分析 — 追踪哪些文件、服务和下游消费者可能受影响而中断
安全扫描 — 检测SQL injection、XSS、身份验证绕过、密钥泄露、依赖漏洞
测试覆盖率差异 — 新代码与新测试的比例
破坏性变更检测 — API契约、数据库Schema迁移、配置键
工单关联验证 — 验证Jira/Linear工单是否存在且与范围匹配
性能影响评估 — N+1查询、包大小回退、内存分配

When to Use

适用场景

Before merging any PR/MR that touches shared libraries, APIs, or DB schema
When a PR is large (>200 lines changed) and needs structured review
Onboarding new contributors whose PRs need thorough feedback
Security-sensitive code paths (auth, payments, PII handling)
After an incident — review similar PRs proactively

在合并任何涉及共享库、API或数据库Schema的PR/MR之前
当PR规模较大（变更超过200行）且需要结构化审核时
新贡献者入职时，其PR需要详尽反馈
安全敏感的代码路径（身份验证、支付、个人可识别信息处理）
事件发生后 — 主动审核类似PR

Fetching the Diff

获取代码差异

GitHub (gh CLI)

bash

undefined

bash

undefined

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

undefined

gh pr diff <PR_NUMBER> > /tmp/pr-<PR_NUMBER>.diff

undefined

GitLab (glab CLI)

bash

undefined

bash

undefined

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

---

glab mr diff <MR_IID> > /tmp/mr-<MR_IID>.diff

---

Workflow

工作流程

Step 1 — Fetch Context

步骤1 — 获取上下文

bash

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

bash

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

Step 2 — Blast Radius Analysis

步骤2 — 影响范围分析

For each changed file, identify:

Direct dependents — who imports this file?

bash

undefined

针对每个变更文件，识别：

直接依赖项 — 哪些文件引入了该模块？

bash

undefined

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


2. **Service boundaries** — does this change cross a service?
```bash

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


2. **服务边界** — 变更是否跨服务？
```bash

Check if changed files span multiple services (monorepo)

gh pr diff $PR --name-only | cut -d/ -f1-2 | sort -u


3. **Shared contracts** — types, interfaces, schemas
```bash
gh pr diff $PR --name-only | grep -E "types/|interfaces/|schemas/|models/"

Blast radius severity:

CRITICAL — shared library, DB model, auth middleware, API contract
HIGH — service used by >3 others, shared config, env vars
MEDIUM — single service internal change, utility function
LOW — UI component, test file, docs

gh pr diff $PR --name-only | cut -d/ -f1-2 | sort -u


3. **共享契约** — 类型、接口、Schema
```bash
gh pr diff $PR --name-only | grep -E "types/|interfaces/|schemas/|models/"

影响范围严重程度：

严重 — 共享库、数据库模型、身份验证中间件、API契约
高 — 被3个以上服务使用的模块、共享配置、环境变量
中 — 单一服务内部变更、工具函数
低 — UI组件、测试文件、文档

Step 3 — Security Scan

步骤3 — 安全扫描

bash

DIFF=/tmp/pr-$PR.diff

bash

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

undefined

grep -nE "path.join(.*req.|readFile(.*req." $DIFF

undefined

Step 4 — Test Coverage Delta

步骤4 — 测试覆盖率差异

bash

undefined

bash

undefined

Count source vs test files changed

echo "Source files changed: $(echo "$CHANGED_SRC" | wc -w)" echo "Test files changed: $(echo "$CHANGED_TESTS" | wc -w)"

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


**Coverage delta rules:**
- New function without tests → flag
- Deleted tests without deleted code → flag
- Coverage drop >5% → block merge
- Auth/payments paths → require 100% coverage

npm test -- --coverage --changedSince=main 2>/dev/null | tail -20 pytest --cov --cov-report=term-missing 2>/dev/null | tail -20


**覆盖率差异规则：**
- 新增函数无测试 → 标记
- 删除测试但未删除对应代码 → 标记
- 覆盖率下降超过5% → 阻止合并
- 身份验证/支付路径 → 要求100%覆盖率

Step 5 — Breaking Change Detection

步骤5 — 破坏性变更检测

API Contract Changes

API契约变更

bash

undefined

bash

undefined

OpenAPI/Swagger spec changes

grep -n "openapi|swagger" /tmp/pr-$PR.diff | head -20

REST route removals or renames

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) "

undefined

grep "^-" /tmp/pr-$PR.diff | grep -E "^-\s*(export\s+)?(interface|type) "

undefined

DB Schema Changes

数据库Schema变更

bash

undefined

bash

undefined

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

undefined

grep "DROP INDEX|remove_index" /tmp/pr-$PR.diff

undefined

Config / Env Var Changes

配置 / 环境变量变更

bash

undefined

bash

undefined

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

undefined

grep "^-" /tmp/pr-$PR.diff | grep -oE "process.env.[A-Z_]+" | sort -u

undefined

Step 6 — Performance Impact

步骤6 — 性能影响评估

bash

undefined

bash

undefined

N+1 query patterns (DB calls inside loops)

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 "^+"

---

grep -n "new Array([0-9]{4,}|Buffer.alloc" /tmp/pr-$PR.diff | grep "^+"

---

Ticket Linking Verification

工单关联验证

bash

undefined

bash

undefined

Extract ticket references from PR body

Verify Jira ticket exists (requires JIRA_API_TOKEN)

TICKET="PROJ-123" curl -s -u "user@company.com:$JIRA_API_TOKEN"
"https://your-org.atlassian.net/rest/api/3/issue/$TICKET" |
jq '{key, summary: .fields.summary, status: .fields.status.name}'

Linear ticket

LINEAR_ID="abc-123" curl -s -H "Authorization: $LINEAR_API_KEY"
-H "Content-Type: application/json"
--data "{"query": "{ issue(id: \"$LINEAR_ID\") { title state { name } } }"}"
https://api.linear.app/graphql | jq .

---

LINEAR_ID="abc-123" curl -s -H "Authorization: $LINEAR_API_KEY"
-H "Content-Type: application/json"
--data "{"query": "{ issue(id: \"$LINEAR_ID\") { title state { name } } }"}"
https://api.linear.app/graphql | jq .

---

Complete Review Checklist (30+ Items)

完整审核清单（30+项）

markdown

undefined

markdown

undefined

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

PR标题准确描述了变更内容
PR描述解释了变更原因，而非仅说明变更内容
关联的Jira/Linear工单存在且与变更范围匹配
无无关变更（范围蔓延）
破坏性变更已在PR正文中记录

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)

已识别所有引入变更模块的文件
已检查跨服务依赖
已审核共享类型/接口/Schema是否存在中断风险
新增环境变量已在.env.example中记录
数据库迁移支持回滚（包含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

无硬编码密钥或API密钥
SQL查询使用参数化输入（无字符串插值）
用户输入在使用前已验证/ sanitize
所有新端点均有身份验证/授权检查
无XSS风险（innerHTML、dangerouslySetInnerHTML）
已检查新增依赖是否存在已知CVE
日志中无敏感数据（个人可识别信息、令牌、密码）
文件上传已验证（类型、大小、内容类型）
新端点的CORS配置正确

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

新增公共函数有单元测试
已覆盖边缘情况（空值、null、最大值）
已测试错误路径（而非仅正常路径）
API端点变更有集成测试
无无理由删除测试的情况
测试名称清晰描述验证内容

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

无未提前通知就删除API端点的情况
无在现有API响应中新增必填字段的情况
无未采用两阶段迁移计划就删除数据库列的情况
无删除可能在生产环境中设置的环境变量的情况
对外部API消费者保持向后兼容

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

未引入N+1查询模式
为新查询模式新增了数据库索引
无针对潜在大型数据集的无界循环
无无理由引入的重型依赖
异步操作已正确使用await
已考虑对昂贵的重复操作进行缓存

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)

---

无死代码或未使用的引入
有错误处理（无空catch块）
与现有模式和约定保持一致
复杂逻辑有解释性注释
无未解决的TODO（或已在工单中追踪）

---

Output Format

输出格式

Structure your review comment as:

undefined

将审核评论组织为：

undefined

PR Review: [PR Title] (#NUMBER)

PR审核：[PR标题] (#编号)

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) ---

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) ---

Missing auth check on POST /api/admin/reset No role verification before destructive operation.

--- SUGGESTIONS ---

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

---

影响范围：高 — 变更了被5个服务使用的lib/auth模块安全：1项发现（中等严重程度）测试：覆盖率差异 +2% 破坏性变更：未检测到

--- 必须修复（阻塞合并） ---

src/db/users.ts:42存在SQL注入风险 WHERE子句中使用了原始字符串插值修复方案：db.query("SELECT * WHERE id = $1", [userId])

--- 建议修复（非阻塞） ---

POST /api/admin/reset缺少身份验证检查执行破坏性操作前无角色验证

--- 优化建议 ---

src/services/reports.ts:88存在N+1模式 findUser()在results.map()内调用 — 改用findManyUsers(ids)批量查询

--- 良好实践 ---

新增身份验证流程的测试覆盖率全面
数据库迁移有正确的down()回滚方法
错误处理与代码库其他部分一致

---

Common Pitfalls

常见误区

Reviewing style over substance — let the linter handle style; focus on logic, security, correctness
Missing blast radius — a 5-line change in a shared utility can break 20 services
Approving untested happy paths — always verify error paths have coverage
Ignoring migration risk — NOT NULL additions need a default or two-phase migration
Indirect secret exposure — secrets in error messages/logs, not just hardcoded values
Skipping large PRs — if a PR is too large to review properly, request it be split

重格式轻实质 — 让代码检查工具处理格式问题；重点关注逻辑、安全性和正确性
忽略影响范围 — 共享工具中5行的变更可能导致20个服务中断
批准未测试错误路径的PR — 始终验证错误路径是否有覆盖
忽略迁移风险 — 添加NOT NULL约束需要默认值或两阶段迁移
间接密钥泄露 — 密钥出现在错误消息/日志中，而非仅硬编码
跳过大型PR — 如果PR过大无法妥善审核，要求拆分

Best Practices

最佳实践

Read the linked ticket before looking at code — context prevents false positives
Check CI status before reviewing — don't review code that fails to build
Prioritize blast radius and security over style
Reproduce locally for non-trivial auth or performance changes
Label each comment clearly: "nit:", "must:", "question:", "suggestion:"
Batch all comments in one review round — don't trickle feedback
Acknowledge good patterns, not just problems — specific praise improves culture

查看代码前先阅读关联工单 — 上下文可避免误判
审核前检查CI状态 — 不要审核构建失败的代码
优先关注影响范围和安全性，而非格式
对于非 trivial 的身份验证或性能变更，本地复现验证
清晰标记每条评论：“格式细节:”、“必须修复:”、“疑问:”、“建议:”
一轮审核中批量提交所有评论 — 不要零散反馈
认可良好模式，而非仅指出问题 — 具体的表扬有助于改善团队文化

pr-review-expert

Original

Translation

PR Review Expert

PR审核专家

Overview

概述

Core Capabilities

核心能力

When to Use

适用场景

Fetching the Diff

获取代码差异

GitHub (gh CLI)

GitHub (gh CLI)

View diff in terminal

View diff in terminal

Get PR metadata (title, body, labels, linked issues)

Get PR metadata (title, body, labels, linked issues)

List files changed

List files changed

Check CI status

Check CI status

Download diff to file for analysis

Download diff to file for analysis

GitLab (glab CLI)

GitLab (glab CLI)

View MR diff

View MR diff

MR details as JSON

MR details as JSON

List changed files

List changed files

Download diff

Download diff

Workflow

工作流程

Step 1 — Fetch Context

步骤1 — 获取上下文

Step 2 — Blast Radius Analysis

步骤2 — 影响范围分析

Find all files importing a changed module

Find all files importing a changed module

Python

Python

Check if changed files span multiple services (monorepo)

Check if changed files span multiple services (monorepo)

Step 3 — Security Scan

步骤3 — 安全扫描

SQL Injection — raw query string interpolation

SQL Injection — raw query string interpolation

Hardcoded secrets

Hardcoded secrets

AWS key pattern

AWS key pattern

JWT secret in code

JWT secret in code

XSS vectors

XSS vectors

Auth bypass patterns

Auth bypass patterns

Insecure hash algorithms

Insecure hash algorithms

eval / exec

eval / exec

Prototype pollution

Prototype pollution

Path traversal risk

Path traversal risk

Step 4 — Test Coverage Delta

步骤4 — 测试覆盖率差异

Count source vs test files changed

Count source vs test files changed

Lines of new logic vs new test lines

Lines of new logic vs new test lines

Run coverage locally

Run coverage locally

Step 5 — Breaking Change Detection

步骤5 — 破坏性变更检测

API Contract Changes