| name | code-review |
| description | 代码审查规范 Skill。对代码进行系统化审查,按 MUST/SHOULD/NIT 三级分类输出审查意见,覆盖正确性、安全性、可维护性、性能、规范性五大维度。触发条件:(1) 代码审查/Code Review,(2) PR Review/Pull Request 审查,(3) 审查代码质量,(4) MUST/SHOULD/NIT 分级审查,(5) 代码走查,(6) 审查这个 PR。 |
代码审查规范
对代码进行系统化审查,按 MUST / SHOULD / NIT 三级分类输出结构化审查意见。覆盖正确性、安全性、可维护性、性能、规范性五大维度。
外部 Skill 依赖
| Skill | 路径 | 何时加载 |
|---|
security-audit | .agents/skills/security-audit/SKILL.md | 发现安全类 finding 需要深度分析时 |
coding-standards | .agents/skills/coding-standards/SKILL.md | 需要语言特定编码规范参考时(若存在) |
严重等级定义
| 等级 | 标记 | 含义 | 合并影响 |
|---|
| 🔴 MUST | [MUST] | 阻断合并,必须修复 | → REQUEST_CHANGES |
| 🟡 SHOULD | [SHOULD] | 强烈建议修复,不阻断 | → COMMENT |
| 🟢 NIT | [NIT] | 建议优化,可忽略 | → COMMENT |
等级判定标准
| 等级 | 适用场景 |
|---|
[MUST] | 逻辑错误导致功能异常、安全漏洞(注入/XSS/认证绕过)、数据丢失或损坏风险、并发竞态导致不一致、API 契约破坏(Breaking Change) |
[SHOULD] | 性能问题(N+1 查询、内存泄漏、不必要渲染)、可维护性问题(高圈复杂度、深嵌套、重复逻辑)、缺少错误处理或日志、不符合项目最佳实践、缺少必要测试覆盖 |
[NIT] | 命名不够清晰、注释缺失或过时、代码格式/排版、可用更简洁的写法、文档不完整 |
审查维度
1. 正确性(Correctness)
- 逻辑是否正确实现了需求(对照 Issue 验收标准)
- 边界条件是否覆盖(空值、零值、最大值、负数、空集合)
- 错误处理是否完整(异常捕获、错误传播、用户友好的错误信息)
- 并发安全(竞态条件、死锁风险、原子操作)
- 状态管理(状态一致性、状态泄漏、状态清理)
2. 安全性(Security)
- 输入验证与净化(SQL 注入、XSS、命令注入、路径遍历)
- 认证与授权(权限检查、越权风险、会话管理)
- 敏感数据处理(密钥硬编码、日志泄露、加密方式)
- 依赖安全(已知漏洞依赖、供应链风险)
发现安全类 finding 时,可加载 security-audit Skill 进行 OWASP Top 10 深度检查。
3. 可维护性(Maintainability)
- 命名清晰度(变量/函数/类名是否表达意图)
- 圈复杂度(单函数 > 10 标记
[SHOULD],> 20 标记 [MUST])
- 重复代码(DRY 原则,3+ 处相似逻辑应抽取)
- 模块耦合度(依赖方向、循环依赖、接口隔离)
- 单一职责(函数/类是否职责过多)
4. 性能(Performance)
- 数据库查询效率(N+1 查询、缺少索引、全表扫描)
- 内存使用(泄漏、不必要的大对象拷贝、缓存策略)
- 前端渲染(不必要的重渲染、大列表未虚拟化、bundle 体积)
- 算法复杂度(O(n²) 可优化为 O(n log n) 等)
- I/O 效率(串行可改并行、缺少批量操作、连接池配置)
5. 规范性(Standards)
- 编码风格一致性(与项目现有代码一致)
- API 设计一致性(RESTful 规范、错误响应格式、分页模式)
- 错误处理模式(与项目约定一致)
- 测试覆盖(新功能是否有对应测试、测试是否有效)
- 提交规范(commit message 格式、PR 描述完整度)
约束
- 不修改代码 — 审查 Agent 仅输出审查意见,不直接修改源码
- 不跳过文件 — PR diff 中的每个变更文件都必须审查
- 每个 finding 必须包含:严重等级
[MUST/SHOULD/NIT]、文件路径、行号(或行范围)、问题描述、修复建议
- 不编造问题 — 只报告实际存在的问题,不为凑数量而虚构
- 上下文感知 — 审查时考虑代码的业务上下文,不机械套用规则
- 一致性 — 同类问题给出相同等级,不在文件间出现等级漂移
工作流
步骤 1:确定审查范围
- 确认审查来源:
- PR 模式:读取 PR diff,获取变更文件列表
- 文件模式:用户指定的文件或目录
- 模块模式:用户指定的模块(通过
module:{slug} 标签定位)
- 统计变更规模:文件数、新增/删除行数
- 标记重点关注区域:新文件、核心业务逻辑文件、安全敏感文件(认证/支付/权限)
步骤 2:检测技术栈
- 识别语言和框架(读取
package.json、pom.xml、go.mod、pyproject.toml、*.csproj 等)
- 识别已有 lint/format 配置(
.eslintrc、prettier、pylint、golangci-lint 等)
- 加载对应的审查规则集(若存在
coding-standards Skill 则引用)
步骤 3:逐文件审查
对每个变更文件,按 5 大维度逐项检查:
- 阅读文件完整上下文(不仅看 diff,还要看周围代码)
- 按维度顺序检查:正确性 → 安全性 → 可维护性 → 性能 → 规范性
- 对每个发现的问题,记录为 finding:
### [MUST] {问题简述}
**文件**:`{file_path}`
**行号**:L{start}-L{end}
**维度**:{正确性/安全性/可维护性/性能/规范性}
**问题**:{详细描述}
**建议**:
```{language}
// 建议的修复代码
### 步骤 4:跨文件分析
1. **模块耦合**:检查新增的模块间依赖是否合理
2. **API 一致性**:新增的 API 风格是否与已有 API 一致
3. **重复模式**:跨文件是否存在可抽取的公共逻辑
4. **依赖引入**:新增的依赖是否必要、版本是否安全
### 步骤 5:生成审查报告
输出格式:
```markdown
# 📝 代码审查报告
> **审查范围**:{PR #N / 文件列表 / 模块}
> **审查日期**:{当前日期}
> **变更规模**:{N} 文件,+{adds} / -{deletes} 行
---
## 审查摘要
| 等级 | 数量 | 关键问题 |
|------|------|---------|
| 🔴 MUST | {count} | {最重要的 1-2 项} |
| 🟡 SHOULD | {count} | {最重要的 1-2 项} |
| 🟢 NIT | {count} | — |
**审查建议**:{APPROVE / REQUEST_CHANGES / COMMENT}
---
## 审查详情
{按严重等级排列的 finding 列表,MUST 在前}
---
## 亮点
{代码中做得好的地方,1-3 项}
步骤 6:提交审查(可选)
如果用户需要将审查结果写入 GitHub PR:
- 建议调用
pr_review_submit Agent,传入 finding 列表和最终决策
- 决策映射:
- 有
[MUST] → REQUEST_CHANGES
- 无
[MUST] 且质量良好 → APPROVE
- 其他 →
COMMENT
语言特定检查清单
JavaScript / TypeScript
| 维度 | 检查项 |
|---|
| 正确性 | === vs ==、async/await 错误处理、类型断言安全 |
| 安全性 | eval()、innerHTML、dangerouslySetInnerHTML、SQL 拼接、new Function() |
| 性能 | useEffect 依赖数组、useMemo/useCallback 合理性、bundle 拆分 |
| 规范性 | ESLint 规则遵循、TypeScript strictNullChecks |
Python
| 维度 | 检查项 |
|---|
| 正确性 | 可变默认参数、except 过于宽泛、生成器/迭代器正确使用 |
| 安全性 | exec()/eval()、pickle.loads() 不可信输入、SQL format string |
| 性能 | 列表推导 vs 循环、生成器 vs 列表、GIL 相关并发 |
| 规范性 | PEP 8、类型注解完整性、docstring 格式 |
Java
| 维度 | 检查项 |
|---|
| 正确性 | NPE 风险、equals/hashCode 一致性、资源泄漏(try-with-resources) |
| 安全性 | PreparedStatement vs 字符串拼接、反序列化安全、权限注解 |
| 性能 | Stream 滥用、String 拼接(StringBuilder)、连接池配置 |
| 规范性 | Lombok 使用规范、日志级别、异常处理层级 |
Go
| 维度 | 检查项 |
|---|
| 正确性 | error 未检查、goroutine 泄漏、channel 死锁、defer 顺序 |
| 安全性 | sql.DB 参数化查询、html/template vs text/template、TLS 配置 |
| 性能 | slice 预分配、sync.Pool 使用、context 传递 |
| 规范性 | golint/staticcheck 规则、error wrapping(%w)、interface 最小化 |
快速命令
- "审查这个 PR" → 完整 6 步审查流程
- "审查 {文件路径}" → 指定文件的单文件审查
- "只看安全问题" → 仅执行安全性维度审查
- "快速审查" → 仅输出 MUST 级别 finding