code-review

Fresh Eyes Review

系统化的提交前质量门禁 → 基于清单的审查 → 发现问题 → 总结。

<when_to_use>

• 提交前代码审查和质量门禁
• 合并前拉取请求审查
• 部署前系统化代码审计
• 关键变更的质量验证
• 请求第二意见审查

不适用于：快速健全性检查、琐碎的拼写错误修复、仅涉及格式的变更

</when_to_use>

<announcement_protocol>

开始审查

审查范围： { 正在审查的文件/区域 } 关注重点： { 具体关注事项或常规质量门禁 } 检查清单： { 完整类别或目标类别 }

审查期间

发现问题时立即报告：

• {严重程度} {FILE_PATH}:{LINE} — { 问题描述 }
• 影响： { 若发布会造成的后果 }
• 修复： { 具体补救措施 }

完成审查

审查完成

问题摘要：

• ◆◆ 严重：{COUNT} — 阻塞性问题
• ◆ 中等：{COUNT} — 合并前应修复
• ◇ 轻微：{COUNT} — 可考虑处理

建议： { 发布 / 修复阻塞问题 / 需要返工 }

{ 如发现问题，在下方提供详细信息 }

</announcement_protocol>

类型安全

• ✓ 没有未附说明注释的 any 类型
• ✓ 明确处理 null/undefined（可选链、空值合并）
• ✓ 对联合类型使用类型守卫
• ✓ 状态机使用可辨识联合类型
• ✓ 在需要时指定泛型约束
• ✓ 公共函数显式声明返回类型
• ✓ 类型断言均附有安全性说明注释

错误处理

• ✓ 处理所有错误路径（没有静默失败）
• ✓ 错误消息有意义且包含上下文
• ✓ 适当地传播或记录错误
• ✓ 对预期失败使用 Result 类型
• ✓ try/catch 块针对特定错误进行处理
• ✓ 处理 Promise 拒绝
• ✓ 在 finally 块中清理资源

安全性

• ✓ 使用用户输入前先进行验证
• ✓ 没有硬编码的密钥或凭据
• ✓ 包含身份验证/授权检查
• ✓ 使用参数化查询（防止 SQL 注入）
• ✓ 防范 XSS（对输出进行净化）
• ✓ 在适用场景下提供 CSRF 保护
• ✓ 对敏感数据进行加密/哈希
• ✓ 对公共端点进行速率限制

测试

• ✓ 新功能有对应测试
• ✓ 覆盖边界情况
• ✓ 测试错误场景
• ✓ 包含实际断言（而非仅执行代码）
• ✓ 没有测试污染（正确设置/清理）
• ✓ 适当使用模拟（不过度使用）
• ✓ 测试名称描述行为
• ✓ 关键路径有集成测试

代码质量

• ✓ 名称能体现意图（函数、变量、类型）
• ✓ 函数 <50 行（遵循单一职责）
• ✓ 文件 <500 行（考虑拆分）
• ✓ 没有魔法数字（使用命名常量）
• ✓ 消除 DRY 违规
• ✓ 嵌套条件不超过 3 层
• ✓ 圈复杂度合理
• ✓ 移除死代码

文档

• ✓ 公共 API 有 JSDoc/TSDoc
• ✓ 说明复杂算法
• ✓ 记录不显而易见的决策
• ✓ 注明破坏性变更
• ✓ TODO 附有上下文和负责人
• ✓ 行为变更时更新 README
• ✓ 为复杂用法提供示例

性能

• ✓ 没有明显的 N+1 查询
• ✓ 使用适当的数据结构
• ✓ 避免不必要的内存分配
• ✓ 耗时操作采用异步/批处理
• ✓ 在有益时使用缓存
• ✓ 考虑数据库索引

Rust 专项（适用时）

• ✓ rustfmt 和 clippy 均通过
• ✓ 优先使用 Result，而不是 panic
• ✓ 在测试/启动流程之外不使用 unwrap/expect
• ✓ 所有权/借用符合惯用写法
• ✓ 遵守 Send/Sync 约束
• ✓ 对不安全代码通过注释说明理由
• ✓ 使用恰当的错误类型（thiserror/anyhow）

1. 宣布（activeForm: 正在宣布审查）

发送开始审查协议：

• 审查范围
• 关注重点
• 检查清单方式（完整或针对性）

2. 检查清单（activeForm: 正在执行清单审查）

系统化核查各类别：

• 类型安全 → 错误处理 → 安全性 → 测试 → 质量 → 文档 → 性能
• 发现违规时立即标记严重程度
• 简要说明检查通过的部分

3. 深入分析（activeForm：正在调查发现的问题）

对于每个发现的问题：

• 确认它确实是个问题（而非误报）
• 评估严重程度和影响
• 确定具体的修复方法
• 检查整个代码库中是否存在类似模式

4. 汇总（activeForm: 正在编写审查摘要）

输出完成协议：

• 按严重程度统计发现数量
• 建议（发布 / 修复阻塞问题 / 返工）
• 详细发现列表
• 可选：注意到的模式、未来建议

加载 maintain-tasks 技能以跟踪审查阶段。

<finding_format>

{严重程度} {文件路径}:{行范围}

问题： { 清晰描述问题 }

影响： { 发布后果——安全风险、运行时错误、维护负担等 }

修复： { 具体的补救步骤 }

模式： { 如果问题多次出现，请注明涉及范围 }

---

示例：

◆◆ src/auth/login.ts:45-52

问题： 使用 == 比较密码，而不是采用恒定时间比较

影响： 存在计时攻击漏洞——攻击者可以通过响应时间推断密码长度和内容

修复： 使用 crypto.timingSafeEqual() 或 bcrypt 内置的比较方法

模式： 单个问题

---

</finding_format>

<severity_guidance>

◆◆ 严重（阻塞）：

• 安全漏洞
• 数据丢失风险
• 常见路径上的运行时崩溃
• 未提供迁移方案的破坏性变更
• 测试失败或缺少关键测试

◆ 中等（应修复）：

• 类型安全违规
• 未处理的错误情况
• 错误消息不佳
• 缺少针对边缘情况的测试
• 重大代码质量问题
• 公共 API 缺少文档

◇ 轻微（考虑处理）：

• 代码风格不一致
• 代码过于复杂但仍能正常工作
• 轻微性能优化
• 文档改进
• 缺少上下文的 TODO
• 命名改进

</severity_guidance>

循环：扫描 → 验证 → 记录 → 检查下一类别

1. 宣布审查 — 范围、重点、方法
2. 运行检查清单 — 系统地验证各个类别
3. 记录发现 — 严重程度、位置、问题、影响、修复
4. 调查模式 — 是否重复出现？是否存在更广泛的问题？
5. 深入检查阻塞问题 — 验证严重程度评估，确保修复方案清晰
6. 编写摘要 — 按严重程度统计数量，给出建议
7. 交付发现 — 按完成协议提供详细列表

对于每项发现：

• 验证它确实是个问题
• 评估发布后的影响
• 确定具体的修复方案
• 注明是否存在跨文件模式

完成审查前：

检查覆盖情况：

• ✓ 是否验证了所有检查清单类别？
• ✓ 是否审查了正常路径和错误路径？
• ✓ 是否检查测试是否包含实际断言？
• ✓ 是否对安全敏感区域进行了额外审查？

检查发现的质量：

• ✓ 严重程度评估是否准确？
• ✓ 是否清楚说明了影响？
• ✓ 修复方案是否可执行且具体？
• ✓ 是否排除了误报？

检查建议：

• ✓ 是否与发现的严重程度一致？
• ✓ 是否明确标记了阻塞问题？
• ✓ 后续处理路径是否明确？

始终：

• 宣布审查开始，并说明范围和重点
• 系统地运行检查清单，不跳过类别
• 发现问题时立即输出，不要等到最后再集中整理
• 如实评估严重程度（倾向于谨慎）
• 提供具体的修复方案，而不只是抱怨
• 以摘要和建议结束
• 如果检查清单项目不适用，标记为误报
• 考虑问题模式（单个问题还是系统性问题？）

绝不：

• 以“快速检查”为由跳过检查清单审查
• 未经验证就假定代码安全
• 将风格偏好标记为阻塞问题
• 提出没有修复指导的模糊发现
• 将严重问题留待以后修复，却仍予以批准
• 未按协议宣布审查就完成审查
• 漏掉对用户输入路径的安全检查
• 忽略测试质量（执行成功 ≠ 验证有效）

核心方法：

• checklist.md — 扩展检查清单详情、示例、严重程度指导

相关技能：

• codebase-recon — 基于证据的调查（审查基础）
• debugging — 结构化 Bug 调查