comprehensive-review

全面审查

概述

在将代码视为完成之前，依据 7 项标准进行审查。

核心原则： 自我审查可在问题影响他人之前发现问题。

硬性要求： 必须将审查记录发布到 GitHub issue。此要求由钩子强制执行。

开始时宣布：“我正在进行全面代码审查。”

审查记录要求

这不是可选项。 创建 PR 之前：

1. 按照全部 7 项标准完成审查
2. 记录所有发现
3. 使用下方的确切格式，将记录发布为 issue 评论
4. 处理所有发现（修复，或推迟并创建跟踪 issue）
5. 更新记录，显示“未处理：0”

如果没有此记录，review-gate 技能和 PreToolUse 钩子将阻止创建 PR。

7 项标准

1. 盲点

问题： 我遗漏了什么？

检查项	自问
边缘情况	在边界条件下会发生什么？输入为空时呢？达到最大值时呢？
错误路径	外部服务故障时会怎样？出现网络问题时呢？
并发	多个用户或线程同时操作时呢？是否存在竞态条件？
状态	以错误顺序调用时会怎样？状态无效时呢？
依赖项	依赖项的行为发生变化时会怎样？

// Blindspot example: What if items is empty?
function calculateAverage(items: number[]): number {
  return items.reduce((a, b) => a + b, 0) / items.length;
  // Blindspot: Division by zero when items is empty!
}

// Fixed
function calculateAverage(items: number[]): number {
  if (items.length === 0) {
    throw new Error('Cannot calculate average of empty array');
  }
  return items.reduce((a, b) => a + b, 0) / items.length;
}

2. 清晰度／一致性

问题： 别人能看懂这些内容吗？

检查项	自问
名称	名称是否能说明事物的作用或含义？
结构	代码的组织是否合乎逻辑？
复杂度	能否进一步简化？
模式	是否与现有模式一致？
意外之处	是否有任何地方会让读者感到意外？

3. 可维护性

问题： 能否安全地修改这些内容？

检查项	自问
耦合	是否与其他代码紧密绑定？
内聚	是否能把一件事做好？
重复	是否有重复的逻辑？
测试	测试是否充分覆盖？
可扩展性	能否轻松添加新功能？

4. 安全风险

问题： 这些内容能否被利用？

检查项	自问
输入验证	是否对所有输入进行了验证和清理？
身份验证	是否妥善控制了访问？
授权	是否检查了权限？
数据泄露	是否保护了敏感数据？
注入	是否可能发生 SQL、XSS 或命令注入？
依赖项	依赖项是否安全且已更新？

注意： 如果更改了安全敏感文件（如 auth、api、middleware 等），请调用 security-review 技能进行深入分析。

5. 性能影响

问题： 这些内容能否扩展以应对更大规模？

检查项	自问
算法	复杂度是否合适？能用 O(n) 时是否用了 O(n²)？
数据库	是否存在 N+1 查询？是否缺少索引？是否全表扫描？
内存	是否在内存中存放了大型对象？是否存在内存泄漏？
网络	是否有不必要的请求？载荷是否过大？
缓存	是否应缓存结果？

6. 文档

问题： 文档是否充分？

检查项	自问
公共 API	所有公共函数是否都有文档？
参数	参数的类型和用途是否清楚？
返回值	是否记录了返回值？
错误	是否记录了抛出的错误？
示例	是否展示了复杂用法？
原因	是否解释了不明显的决策？

有关文档标准，请参阅 inline-documentation 技能。

7. 标准与风格

问题： 这些内容是否遵循项目约定？

检查项	自问
命名	是否遵循项目命名约定？
格式	是否符合项目格式要求？
模式	是否采用既定模式？
类型	是否进行了完整的类型标注（没有 any）？
语言	是否使用了包容性语言？
IPv6 优先	网络代码是否默认使用 IPv6？是否仅在文档说明的遗留场景中使用 IPv4？
代码检查	是否通过了所有代码检查工具？

请参阅 style-guide-adherence、strict-typing、inclusive-language、ipv6-first 技能。

审查流程

步骤 1：准备

# Get list of changed files
git diff --name-only HEAD~1

# Get full diff
git diff HEAD~1

# Check for security-sensitive files
git diff --name-only HEAD~1 | grep -E '(auth|security|middleware|api|password|token|secret)'
# If matches found, security-review skill is MANDATORY

第 2 步：审查每项标准

针对 7 项标准中的每一项：

1. 审查所有已更改的代码
2. 记录发现的问题
3. 确定严重程度（Critical/Major/Minor）

第 3 步：检查安全敏感内容

如果有任何安全敏感文件被更改：

1. 调用 security-review skill 或 security-reviewer 子 Agent
2. 在产物中包含安全审查结果
3. 在产物中标记“Security-Sensitive: YES”

第 4 步：记录发现的问题

## Code Review Findings

### 1. Blindspots
- [ ] **Critical**: No handling for empty array in `calculateAverage()`
- [ ] **Minor**: Missing null check in `formatUser()`

### 2. Clarity/Consistency
- [ ] **Major**: Variable `x` should have descriptive name

### 3. Maintainability
- [x] No issues found

### 4. Security Risks
- [ ] **Critical**: SQL injection possible in `findUser()`

### 5. Performance Implications
- [ ] **Major**: N+1 query in `getOrdersWithUsers()`

### 6. Documentation
- [ ] **Minor**: Missing JSDoc on `processOrder()`

### 7. Standards and Style
- [x] Passes all checks

第 5 步：处理所有发现的问题

使用 apply-all-findings skill 处理每个问题。

对于无法修复的问题：

1. 使用 deferred-finding skill 创建跟踪问题
2. 在产物中链接跟踪问题
3. 不允许“延后但未创建跟踪问题”

第 6 步：将产物发布到 Issue（必须）

将审查产物作为评论发布到 GitHub Issue：

ISSUE_NUMBER=123
gh issue comment $ISSUE_NUMBER --body "$(cat <<'EOF'

## Code Review Complete

| Property | Value |
|----------|-------|
| Worker | `[WORKER_ID]` |
| Issue | #123 |
| Scope | [MINOR|MAJOR] |
| Security-Sensitive | [YES|NO] |
| Reviewed | [ISO_TIMESTAMP] |

### Criteria Results

| # | Criterion | Status | Findings |
|---|-----------|--------|----------|
| 1 | Blindspots | [✅ PASS|✅ FIXED|⚠️ DEFERRED] | [N] |
| 2 | Clarity | [✅ PASS|✅ FIXED|⚠️ DEFERRED] | [N] |
| 3 | Maintainability | [✅ PASS|✅ FIXED|⚠️ DEFERRED] | [N] |
| 4 | Security | [✅ PASS|✅ FIXED|⚠️ DEFERRED|N/A] | [N] |
| 5 | Performance | [✅ PASS|✅ FIXED|⚠️ DEFERRED] | [N] |
| 6 | Documentation | [✅ PASS|✅ FIXED|⚠️ DEFERRED] | [N] |
| 7 | Style | [✅ PASS|✅ FIXED|⚠️ DEFERRED] | [N] |

### Findings Fixed in This PR

| # | Severity | Finding | Resolution |
|---|----------|---------|------------|
| 1 | [SEVERITY] | [DESCRIPTION] | [HOW_FIXED] |

### Findings Deferred (With Tracking Issues)

| # | Severity | Finding | Tracking Issue | Justification |
|---|----------|---------|----------------|---------------|
| 1 | [SEVERITY] | [DESCRIPTION] | #[ISSUE] | [WHY] |

### Summary

| Category | Count |
|----------|-------|
| Fixed in PR | [N] |
| Deferred (with tracking) | [N] |
| Unaddressed | 0 |

**Review Status:** ✅ COMPLETE

EOF
)"

重要：“Unaddressed”必须为 0。“Review Status”必须为“COMPLETE”。

严重程度等级

严重程度	描述	操作
Critical	安全问题、数据丢失、崩溃	合并前必须修复
Major	重大错误、性能问题	合并前必须修复
Minor	风格、清晰度、小幅改进	合并前应修复

检查清单

每次代码审查都要完成：

• 盲点：检查边缘情况、错误和并发
• 清晰度：审查命名、结构和复杂度
• 可维护性：评估耦合、内聚和测试
• 安全性：检查输入、身份验证、注入和暴露（安全敏感文件必须检查）
• 性能：审查算法、查询和内存
• 文档：为公共 API 编写文档
• 风格：遵循规范
• 记录所有发现的问题
• 处理所有发现的问题，或延后处理并创建跟踪问题
• 将审查产物发布到 Issue（格式必须准确）
• 产物中的“Unaddressed: 0”
• 产物中的“Review Status: COMPLETE”

集成

此 skill 由以下内容调用：

• issue-driven-development - 第 9 步

此 skill 使用：

• review-scope - 确定审查范围
• apply-all-findings - 处理问题
• security-review - 用于安全敏感的更改
• deferred-finding - 用于创建跟踪问题

此 skill 由以下内容强制执行：

• review-gate - 在 PR 前验证产物
• PreToolUse hook - 没有产物时阻止 PR

此 skill 引用：

• inline-documentation - 文档标准
• strict-typing - 类型要求
• style-guide-adherence - 风格要求
• inclusive-language - 包容性语言要求
• ipv6-first - 网络代码要求（优先使用 IPv6，IPv4 用于兼容旧版）