agentleFS
liuhean2021liuhean2021/anan-agent-skills/skills/pre-commit-review/SKILL.md

pre-commit-review

Skill1 starsChanged 7 months agoMIT-0View on GitHub

Summarizes modified files in detail and reviews code to ensure changes do not affect other functionality. Use when the user asks for modification summary, code review, pre-commit review, or to ensure changes pass functional testing without introducing new issues. Trigger terms include: 审查代码、审查修改、review代码、代码审查、代码复审、测试代码、修改总结、提交前审查、 不影响其他功能、审查结果不通过、回归测试、功能测试、修改review、审查变更. If the intent is ambiguous (e.g. 检查一下、看一下代码), ask the user to confirm: "是否需要按提交前审查流程,做修改总结 + 场景矩阵 + 代码审查?" If review fails, state the reason clearly.

5 other projects use a version of this.

Adopted by

Most recently taken up by

The file

Files
SKILL.md

提交前修改总结与代码审查

本技能用于在提交前对本次修改做详细总结与代码审查,确保功能测试通过、不引入新问题、不影响需求外的功能,并且开发者对改动有充分理解。

审查的本质:Code Review 两问

代码审查的价值不在「有人看了代码」或「有人读懂了代码」,而在于确认本次 diff 经得起两个核心问题的检验:

一问「做对了吗?」 —— 变更是否满足真实需求、覆盖异常与回归风险、符合前置的技术设计。 二问「放对了吗?」 —— 是否遵守架构边界、避免错误依赖与重复实现、控制长期维护成本。

CR 的本质:确保本次修改符合各方面的质量要求,让人安心发布。CR 是手段而非目的——最终目标是「对发布有信心」,而不是「完成了审查流程」。

实验佐证:曾试验「不 Review 代码,只 Review 需求 + 自动化测试计划与结果」。在测试覆盖足够强的情况下,大量内部功能调整从提交到上生产全自动化,长期运行未出线上事故。结论:测试足够强时 CR 可显著精简;测试不足时 CR 是质量兜底——因此 §3.4 测试质量的权重不低。

触发场景

用户表达以下任一含义时应用本技能(含但不限于):

  • 审查类:审查代码、审查修改、review代码、代码审查、代码复审、修改审查、审查变更、修改review
  • 总结类:详细总结修改、修改总结、变更总结
  • 提交前:提交前审查、提交前检查、commit 前审查、提交前review
  • 质量/测试类:测试代码、回归测试、功能测试、确保不影响其他功能、不引入新问题、审查结果不通过则告知原因

若有疑问:若用户意图不明确(例如仅说「检查一下」「看一下代码」「帮我看下」),先询问确认:「是否需要按提交前审查流程,做修改总结 + 场景矩阵 + 代码审查?」确认后再执行。

严重等级标签

在审查意见中使用以下标签区分优先级:

标签含义处理要求
🔴 [阻塞]必须修复才能提交(安全漏洞、数据丢失、功能中断)修复后重新审查
🟡 [重要]应当修复,有分歧时讨论原则上修复,或说明原因
🟢 [建议]可选优化,不阻塞提交建议项,开发者在后续迭代中酌情处理
💡 [思路]备选方案,供讨论无需操作
📚 [学习]知识分享,无需操作参考了解
🎉 [亮点]值得肯定的做法保持

执行步骤

第一步:收集修改范围(上下文)

  • 使用 git status -s、git diff --cached --stat(或 git diff --stat)确定本次修改的文件与行数。
  • 若文件超过 10 个或改动超过 400 行,建议按模块分批审查。
  • 若用户未指定范围,以当前暂存区或工作区变更为准。
  • 加载此前 Learnings(经验沉淀):读取 docs/review/learnings.jsonl,将之前审查中记录的陷阱/模式载入上下文,避免本次遗漏同类问题。若文件不存在,跳过。
  • Secret 扫描(每次必跑,不受 Diff Scope 影响):

    # gitleaks v8.19+:扫描暂存区(与 pre-commit hook 行为一致)
    gitleaks git --pre-commit --staged --redact --verbose
    # 审查范围含未暂存改动时,追加扫描工作区 diff
    gitleaks git --pre-commit --redact --verbose
    # 旧版本 gitleaks:将 `git --pre-commit` 替换为 `protect`,其余参数不变
    • 按审查范围选择命令:--staged 只扫暂存区,不带 --staged 只扫未暂存 diff;两处都有改动时两条都跑。
    • 以输出中的 scanned ~N bytes 判断是否真正扫到内容:N = 0 而审查范围内有改动时,不得视为通过,应换用另一条命令重扫。
    • 已配置 gitleaks pre-commit hook 的项目,仍须在审查时主动执行一次,而非等提交时才暴露。
    • 未安装 gitleaks 时,按审查范围用 git diff --cached / git diff 人工检索 key、secret、token、password、AKIA、-----BEGIN 等特征,并在审查报告中注明「未执行工具扫描」。
    • 命中真实密钥一律 🔴;已提交过的密钥除删除外还须提醒开发者轮换该密钥。
  • CI/测试状态前置检查:若项目有 CI 或可运行的测试,先确认测试是否通过。测试未通过时先提示开发者修复,再进入审查——在红测基础上做代码审查,审查结论会被污染。

第 1.5 步:审查负载告警(轻率审查预警)

定位:在收集完修改范围后自动评估审查负载,防止花 30 秒给高复杂度改动点 Approve。

基于第一步收集的数据,AI 自动计算以下指标并输出到审查报告顶部:

指标阈值告警
改动行数100-400 行中等负载,> 400 行高负载🔔 高负载建议分批审查或增加审查时间
涉及安全/支付/权限模块改动行数> 50 行🔔 此类改动不应凭直觉通过,建议逐行审查
跨模块数(跨模块规则)≥ 3 模块或服务🔔 跨模块改动容易隐含接口不匹配问题,建议重点验证模块间数据传递的一致性

告警仅是提醒,不阻塞审查。但如果多个告警同时触发,建议 reviewer 仔细检查是否有遗漏后再下结论。

跨模块规则:改动涉及 ≥ 3 个模块或服务即命中。本节、§2.6、§4.3 共用此阈值,改阈值只需改这里。本节三项告警阈值同时作为 §3.12 多视角交叉复核的触发条件。

第二步:详细总结修改

对每个修改文件写出:

  • 文件路径:相对项目根的路径。
  • 修改类型:新增 / 修改 / 删除。
  • 修改要点:用列表说明改了什么(逻辑、条件、顺序、依赖等),可引用关键代码或行号。
  • 修改目的:与需求或问题的对应关系(修复什么、优化什么、限制在什么场景)。

格式示例:

### 文件路径
- **类型**:修改
- **要点**:① …;② …
- **目的**:…

第 2.5 步:Scope Drift(范围漂移)检测

在审查代码质量之前,先判断本次改动是否在应有范围内。

  1. 读取 TODOS.md(如果存在)、commit messages(git log --oneline)、以及用户提供的需求/问题描述
  2. 确定本次改动的原始意图(要解决什么问题、实现什么功能)
  3. 对比实际改动文件列表与原始意图

输出格式(在审查报告中靠前展示):

Scope Check(范围检查): [CLEAN(正常) / DRIFT DETECTED(范围漂移) / MISSING REQUIREMENTS(需求缺失)]
Intent(意图): <1 行总结改动的原始意图>
Delivered(实际改动): <1 行总结 diff 实际在做什么>
[IF drift(范围漂移):逐条列出超出范围的改动]
[IF missing(需求缺失):逐条列出未实现的需求]

此检查为参考信息——不阻塞后续审查,但帮助审查者和开发者意识到范围漂移。

第 2.6 步:Diff Scope(改动范围)自动检测

在进入代码审查前,自动分析改动涉及的领域,决定执行哪些专项清单。

# 检测改动类型(由 AI 基于 diff 判断)
# 输出:SCOPE_AUTH=true/false  SCOPE_FRONTEND=true/false  SCOPE_BACKEND=true/false
#       SCOPE_CONFIG=true/false  SCOPE_API=true/false  SCOPE_MIGRATIONS=true/false

检测结果驱动专项清单:

  • SCOPE_AUTH=true → 必跑 §3.2 安全专项清单(SCOPE_AUTH 覆盖鉴权、支付、隐私、权限、密钥、数据边界等安全敏感改动,不限于登录认证)
  • SCOPE_FRONTEND=true → 必跑 §3.5 技术栈前端、§3.13 浏览器兼容性声明(只声明、不做实际检查;实际检查转交专门的浏览器兼容性审查技能或工具),视情况跑 §3.6 复杂度
  • SCOPE_BACKEND=true → 必跑 §3.3 性能专项、§3.5 技术栈后端
  • SCOPE_MIGRATIONS=true → 必跑 §3.3 性能专项、§3.8 文档同步
  • SCOPE_API=true → 必跑 §3.8 文档同步
  • SCOPE_CONFIG=true → 必跑 §3.8 文档同步
  • 命中跨模块规则(≥ 3 模块或服务) → 必跑 §3.11 架构与归属审查
  • §1.5 负载告警任一项触发(高负载 / 安全敏感改动超阈值 / 跨模块) → SHOULD 执行 §3.12 多视角交叉复核

在审查报告中展示检测结果:Diff Scope(改动范围): auth/frontend/backend/config/API/migration

第 2.7 步:审查前置分析报告(统一产出)

第一步 Secret 扫描结果、第 1.5、2.5、2.6 步的产出及第 2.7 步架构定性检查,应**合并到一份统一的「审查前置分析报告」**中,置于审查报告顶部,分节呈现。避免将每个步骤作为独立输出造成信息碎片化:

## 审查前置分析报告

### 🔔 负载告警
- 改动行数:XXX(→ 建议分批审查 / 正常)
- 安全敏感模块改动:XX 行(→ 建议逐行审查)
- 跨模块数:X(→ 建议关注接口一致性)
...

### 🔍 Scope Check(范围检查)
Scope(范围): [CLEAN(正常) / DRIFT DETECTED(范围漂移) / MISSING REQUIREMENTS(需求缺失)]
Intent(意图): ...
Delivered(实际改动): ...

### 📐 Diff Scope(改动范围)
auth/frontend/backend/config/API/migration

### 🔐 Secret 扫描
- 命令:`gitleaks git --pre-commit [--staged] --redact`(或「未执行工具扫描,已人工检索」)
- 范围:暂存区 / 未暂存 diff,scanned ~N bytes
- 结果:no leaks found / 命中 N 处(仅记规则名与 文件:行号,不写密钥原文)

### 🌐 兼容性基线(SCOPE_FRONTEND=true 时必填)
- 来源:项目声明(兼容声明文档 / browserslist / build.target / legacy `modernTargets` / 需求文档)/ 未声明(写「未声明」,基线由所用的兼容性审查技能或工具确定)
- PC 端:...
- 移动端:...

### 🏛️ 架构定性(先判方向,再逐行)
设计判断: [方案匹配问题 / 存在方向性风险(先修正方向再逐行)]
- 改动落在职责所属模块?[是 / 否→优先处理]
- 依赖方向符合分层(下层不依赖上层、无循环)?[是 / 否→优先处理]
→ 任一为否:先与开发者确认架构方向,再进入 §3.2-§3.10 逐行审查,避免在错误架构上精挑细选

前置分析报告中的信息可以引导审查者关注重点,减少 reviewer 在多个步骤产出间跳转的信息断层。

架构定性是「先抓大问题」的开关:逐行专项(§3.2-§3.10)前的快速方向判断,不替代 §3.11 的完整架构复核——若此处发现方向性风险,§3.11 仍须执行。

第三步:代码审查

逐行审查原则:必须逐行审查分配的每一行代码,不得跳过或假定正确。若某段代码无法理解,应要求开发者澄清——你读不懂的地方,其他人也读不懂。

3.1 影响面分析

对每个修改点做影响面分析:

审查项说明
需求内行为本次修改在「目标场景」下是否按预期工作(条件、分支、顺序、依赖是否正确)。
前置设计符合性实现是否遵循既有技术设计/方案(而非绕过设计另起一套);设计未覆盖处是否有合理决策记录。
需求外行为未改动的场景、平台、入口、用户类型是否仍按原逻辑执行(是否有误判、误跳过、误拦截)。
边界与平台若修改与「平台/环境/登录方式」相关,是否用明确条件(如 isWechat、route.query.xxx)限制,避免其他平台被影响。
依赖与顺序是否依赖未完成的数据(如 userInfo 未拉取)、执行顺序是否会导致竞态或误报。
并发与竞态「先查后写」(check-then-act)是否有锁/唯一约束/事务兜底;重复提交、重复回调、消息重投是否幂等;共享状态(全局变量、缓存、单例)是否存在并发写。
未修改的关联代码路由守卫、请求拦截、401 处理、全局配置等是否未改且与本次逻辑兼容。

对关键分支与条件做场景矩阵(推荐):

  • 列出:场景(平台/登录态/URL/配置) → 预期行为 → 修改后是否一致 → 对抗性检查(极端输入/异常情况)[✅已查 / ⬜未查]。
  • 重点覆盖:非目标平台、未登录、其他登录方式、异常/401、配置未加载等。
  • 对抗性检查示例:对每个场景追问"如果输入是空值/超长值/并发/权限不足/上游超时,这段代码会怎样?"将结果填入对抗性列,并在完成后标记 ✅已查 以确认该场景已做过对抗性评估。

3.2 安全专项清单

涉及认证、输入处理、数据存储时必查;纯 UI/样式改动可跳过。

安全定级规则:安全相关发现最低 🟡;疑似可利用漏洞、可导致数据泄露或越权者,默认 🔴。

  • 信任边界:跨边界进入系统的数据(用户输入、第三方回调、消息队列、LLM 输出、文件上传)是否在边界处校验和过滤(XSS、SQL 注入防护)
  • 认证与鉴权:每个敏感操作前是否有权限校验
  • 接口密钥/Token/密码是否未硬编码在代码中(工具兜底见第一步 Secret 扫描)
  • 错误信息是否避免泄露内部细节(堆栈、数据库结构等)
  • CSRF 防护:状态变更操作是否有 CSRF 保护
  • SSRF:后端发起的对外请求,目标 URL 是否校验协议/主机白名单
  • IDOR / 数据边界:资源访问是否校验归属(owner/租户),而非仅校验「已登录」;列表、导出、批量接口是否带租户/归属条件并限制范围
  • 命令注入:拼接进 shell/子进程的输入是否经转义或参数化
  • 依赖审计:是否运行 npm audit / pnpm audit;新增依赖是否核验过版本与安全
  • 支付/金额:金额、价格、折扣是否由服务端计算与校验;支付回调是否校验签名(幂等见 §3.1 并发与竞态)
  • 隐私数据:手机号、身份证、地址、Token 等敏感字段是否未写入日志、埋点、错误上报或前端存储;对外返回是否脱敏、最小化

3.3 性能专项清单

涉及循环、数据库查询、API 调用时必查;简单逻辑修改可跳过。

  • 是否存在 N+1 查询问题(循环内调用数据库/接口)
  • 循环是否可以提前终止或用 Map/Set 优化
  • 大列表/响应是否有分页或截断
  • 高频调用的结果是否可以缓存
  • 是否在热路径中存在阻塞性同步操作

3.4 测试质量清单

涉及核心业务逻辑变更时必查;文档/配置改动可跳过。

  • 是否测试行为而非实现细节(白盒测试 vs 黑盒测试)
  • 是否覆盖边界情况(空值、空列表、最大值、并发)
  • 是否覆盖失败路径(API 超时、权限拒绝、数据异常)
  • 测试名称是否清晰描述「场景 + 预期结果」
  • 测试是否独立(无共享状态,可任意顺序运行)

3.5 项目技术栈专项(TypeScript / Vue3 / NestJS)

项目涉及对应技术栈时核查。

TypeScript

  • 避免使用 any;若必须使用,是否添加注释说明原因
  • async 函数是否有错误处理(try/catch 或 .catch()),避免 UnhandledPromiseRejection
  • 接口/类型是否准确描述数据结构,未使用宽泛的 object

Vue3 / 前端

  • 是否避免直接修改 props(应通过 emit 通知父组件)
  • 响应式数据依赖是否正确(computed/watch 的依赖项是否完整)
  • v-for 是否有唯一且稳定的 :key

NestJS / 后端

  • 服务注入是否在模块中正确注册(Module providers/imports)
  • 异步操作是否正确使用 async/await,避免忘记 await
  • 错误是否通过 NestJS 异常过滤器或 HttpException 统一抛出

3.6 复杂度审查

涉及新增函数/类/模块、条件分支较多时必查;纯配置/数据改动可跳过。

  • 是否存在过度设计(泛化/抽象/配置化超过了当前需求,在解决还不存在的问题)
  • 函数或方法是否过大、职责单一?能否拆分为更小的单元
  • 是否存在嵌套过深(条件嵌套 > 3 层)或可简化的逻辑分支
  • 是否有死代码(未使用的变量/函数/参数/import)
  • 代码是否对后续读者足够自解释——读一遍能理解核心逻辑,无需反复翻看上下文

3.7 命名与注释审查

  • 变量/函数/类/文件名是否准确传达其含义(名称够长到充分表达,不短到需要猜)
  • 注释是否解释 why(为什么存在)而非 what(代码在做什么)——如果代码不自解释,应优先简化代码而非加注释
  • 是否有已过时的 TODO/FIXME/HACK 残留(已解决的应移除)
  • 若有 reviewer 不理解某段代码,应要求开发者澄清该段代码本身(而非仅在评论串中解释),确保未来读者不再困惑

3.8 文档同步检查

涉及对外接口、构建流程、部署方式、使用方式变更时必查;纯内部逻辑改动可跳过。

  • README、API 文档、使用说明是否随代码同步更新
  • 如果删除了代码或接口,是否已删除或标记过期对应的文档
  • 新增的配置项、环境变量、依赖是否有说明或文档记录

3.9 枚举与状态完整性检查(Cross-Reference)

涉及新增枚举值、状态、类型常量时必查;纯内部逻辑改动可跳过。

  • 新增的枚举值/状态/类型常量是否在对应的 switch/case、if-else 分支或映射表中被处理
  • 使用 grep 或等价方式搜索兄弟值的引用处,检查是否有遗漏处理
  • 数据库中已有的旧数据是否需要迁移或兼容处理
  • API 契约是否同步更新(请求/响应中的可选值列表)

3.10 好代码审查(面向未来)

涉及新增/修改业务逻辑代码时必查;纯配置/纯样式改动可酌情简化。

核心原则:代码要经得起「6 个月后的自己」和「半夜被叫醒的同事」审视——写的时候图快,改的时候真实成本才会显现。

四个优先级(降序,冲突时按此顺序取舍):

  1. 可维护性 > 炫技
    • 可读:命名是否自描述,代码是否即文档,读一遍能否看懂而不必猜
    • 可改:是否单一职责,改一处是否会牵连其他;依赖是否松耦合(避免硬编码依赖,优先依赖注入/参数传入)
    • 可测:核心逻辑是否为纯函数/无副作用,模块能否独立测试而不必搭建整个系统
    • 可扩:新增场景是否只需加代码而非改已有逻辑(开放封闭原则),配置是否与逻辑分离
    • 可复用:是否存在可抽取的重复逻辑(DRY);改一处能否全局生效
    • 反面信号:超长函数(> 100 行)、嵌套 > 3 层、一行代码做三件事——命中即建议重写。宁写三行 if,不写一行晦涩的链式调用/嵌套三元
  2. 稳定性 > 功能丰富
    • Loading / Error / Empty 三态是否先于主流程处理,而非只写「理想路径」
    • 依赖的接口/服务异常时,页面是否白屏或整体崩溃(应有降级展示)
    • 弱网/慢响应场景下是否会卡死或无反馈
  3. 性能体感 > 微观指标
    • 是否关注用户能感知的体感指标(首屏速度、交互响应 < 100ms、滚动是否顺畅),而非孤立堆砌微观指标
    • 是否为解决卡顿滥用了 useMemo/useCallback 等微观优化,而未考虑 useTransition、乐观更新等更直接的手段
  4. 设计意图 > 像素还原
    • 组件是否只满足了当前一种交互路径,未预留后续扩展点(如新增选项/状态时需大改)
    • 状态流转是否完整:是否遗漏某些交互态(如禁用中/提交中),关闭弹窗或切换页面后是否有数据残留未清理

一句话区分优先级:先问「效果垮了会怎样」(稳定性/正确性问题优先处理),再问「改起来要动几行」(可维护性问题决定长期成本,次之权衡)。

3.11 架构与归属审查(放对了吗)

涉及新增模块 / 跨模块调用 / 依赖调整时必查;纯单文件内部逻辑可跳过。

  • 架构边界:是否遵守分层/模块边界(UI 层不直连存储、跨模块不越权访问内部实现、业务层不散落基建逻辑)
  • 依赖方向:依赖是否单向、无循环(下层不依赖上层);新增依赖是否必要、是否引入维护负担
  • 重复实现:是否存在复制粘贴式重复,或与既有实现并存的「双写」逻辑(应复用既有工具函数/组件/服务)
  • 归属正确:改动是否落在职责所属的模块,而非塞进「离得近但不对」的地方
  • 长期维护成本:改动是否让后续修 bug / 加功能更困难;是否引入需要持续同步维护的两处逻辑
  • 接口约定:模块间入参/出参/错误码是否一致——不同模块对同一个错误场景的处理策略是否相同
  • 数据流向:数据流是否符合系统架构设计——是否有绕过了原定路由/中间件/API 网关的数据通道
  • 架构假设:是否引入了「只有一个人知道但团队其他人不知道」的架构假设——如有,需在文档中记录

3.12 多视角交叉复核(大范围/高风险时)

§1.5 负载告警任一项触发时 SHOULD 执行(阈值以 §1.5 为准);其余情况可跳过。

单一视角的审查容易被第一印象锁定。触发时,由专注审查视角的子任务分别独立审查同一份 diff,互不共享结论:

视角关注点对应清单
A:架构 / 类型 / 测试缺口模块边界、依赖方向、类型是否准确、哪些分支没有测试覆盖§3.4、§3.5、§3.11
B:可读性 / UX / 文档读者能否看懂、交互态与异常态是否完整、兼容性声明是否完整(实际兼容检查不在此列)、文档是否同步§3.6、§3.7、§3.8、§3.10、§3.13
  • 每个子任务只拿到 diff、需求描述和对应清单,不拿主审查的结论,避免被带偏。
  • 当前 agent 不支持子任务时,由主审查按视角分轮独立执行,每轮开始前明确声明当前视角。
  • 各视角发现合并进 §4.1 审查结论:重复项合并,保留来源标注;视角间结论冲突时列出双方理由,由开发者裁决。

3.13 浏览器兼容性声明(PC 端 + 移动端,只声明不检查)

SCOPE_FRONTEND=true 时 MUST 执行;纯后端/文档/配置改动标注「不适用」。

本节只做声明,不做实际兼容检查:不逐项审查语法、API、CSS 与 WebView 行为,也不给出兼容修复方案。实际的兼容审查与修复建议由专门的浏览器兼容性审查技能或工具负责(不限定具体技能;其修复须经用户确认后才执行)。

声明内容(写入前置分析报告「🌐 兼容性基线」与审查文档):

  • 兼容基线:来源(项目兼容声明 / browserslist / 构建配置 / 需求文档)与范围(PC、H5 各自的最低版本);项目未声明时写「未声明」
  • 兼容风险面:本次 diff 是否涉及以下任一项,逐项写「涉及 / 不涉及」及涉及位置——
    • 构建目标、转译或 polyfill 配置(build.target、legacy 插件、Babel、core-js、browserslist)
    • 新增或升级前端依赖
    • 新 JS 语法 / 内置 API、正则后行断言、日期字符串解析
    • 新 CSS 特性或全局样式、布局方式
    • index.html head(双核 meta、viewport)
    • H5:视口/安全区/软键盘/触屏交互、App 内 WebView 受限能力(下载、上传、window.open、自动播放、跳转)
  • 兼容审查状态:本次是否已执行专门的兼容性审查(写明所用技能或工具、报告位置 / 结论摘要)

结论规则:

  • 风险面全部「不涉及」→ 声明即可,不产生发现。
  • 任一「涉及」且未执行专门的兼容性审查 → 记 🟡 [重要]〔兼容性〕「涉及兼容风险面,需执行浏览器兼容性审查」,列出涉及点;当前环境没有可用的兼容性审查技能或工具时注明「未执行兼容审查」。
  • 已执行专门的兼容性审查 → 引用其结论(其阻塞级问题同步计入本次审查的 🔴),不重复审查。

第四步:审查结论

纯审查,不改代码:本技能仅提供审查结论、问题定位与修改建议,绝不直接修改代码。所有代码修改应由开发者本人完成。

审查结论是本次审查的最终决策点,分以下步骤执行:

4.1 输出审查结论

  1. 🔴 阻塞:标记问题 + 给出可应用的修改方案。修复前使用「建设性反馈原则」引导讨论。
  2. 🟡 重要:标记问题 + 给出修改方案。
  3. 🟢 建议:标记优化方向,供开发者后续迭代中酌情处理。
  4. 💡 思路:标记备选方案,供讨论。

使用以下结构化格式输出结论。🔴/🟡 发现须标注来源:〔代码审查〕、〔安全专项〕、〔Secret 扫描〕、〔兼容性〕、〔交叉复核-A〕、〔交叉复核-B〕,便于追溯每条发现由哪项检查产生。

## 审查结论

### 亮点
🎉 [描述值得肯定的设计或实现]

### 必须修复(阻塞提交)
🔴 [阻塞]〔来源〕文件路径:行号 — 问题描述
  → 原因:为什么这是问题
  → 建议:具体的修改方案

### 应当修复
🟡 [重要]〔来源〕文件路径:行号 — 问题描述
  → 建议:…

### 优化建议(不阻塞)
🟢 [建议] …
💡 [思路] …

### 问题统计
🔴 阻塞: N | 🟡 重要: N | 🟢 建议: N | 💡 思路: N | 🎉 亮点: N
> 同一分支多次审查时,统计随 review 文档增量更新(§5),形成「问题数递减曲线」,作为该需求可量化的质量证据。

### 审查反思(收尾两问)
🔍 **眼下最没把握的**:<本次审查中置信度最低、最需要开发者额外验证的 1-2 处;没有则写「无」>
👁 **可能遗漏的**:<本次审查未覆盖但可能影响结论的点(未查的调用方/引用、测试盲区);没有则写「无」>

> 结论输出前 MUST 完成此两问并如实作答。即使结论全绿,也须标注置信度边界——它不否定结论,而是把 review 的注意力引向最该验证的地方。问题一源自 Sam Altman,问题二源自 Claude。

### 审查结论
❌ 不通过 — 需修复上述 🔴 问题后重新审查
或
✅ 通过 — 进入 §4.2 触发条件判定

4.2 审查结论判定

不通过:存在 🔴 阻塞问题。必须逐条列出原因,不允许只写「审查不通过」。

通过:无 🔴 阻塞问题。通过前先自查:若改动 > 100 行但结论无任何 🔴/🟡 问题,应反思是否漏审——百行以上改动无严重问题极为罕见。同时完成 §4.1「审查反思(收尾两问)」:如实标注最没把握与可能遗漏之处。确认无漏审、反思两问已作答后,必须检查是否满足 Self-Test 触发条件(见下方),根据检查结果决定下一步:

检查结果下一步
Self-Test 不触发直接进入合并流程
Self-Test SHOULD 执行执行 §4.4 Self-Test 后进入合并
Self-Test MUST 执行必须执行 §4.4,通过后进入合并

4.3 Self-Test 触发条件检查

适用场景
场景建议
大面积重构或重写SHOULD 执行
涉及关键路径、支付、安全等高风险改动SHOULD 执行
改动行数 > 200 行SHOULD 执行
命中跨模块规则(≥ 3 模块或服务)SHOULD 执行
涉及数据库迁移或配置变更SHOULD 执行
AI 自主完成大部分实现且开发者未逐行 reviewSHOULD 执行
开发者对改动中复杂逻辑缺乏充分理解SHOULD 执行
仅修了一个边界值或单行 bug可跳过
用户明确要求「出测试题」或「你自己做题」MUST 执行

Self-Test 触发条件以「适用场景」和「用户要求」两者的较高等级为准,后者始终优先。

4.4 执行 Self-Test(开发者理解度校验)

定位:审查已通过且触发了 Self-Test 条件时,执行开发者理解度校验。不是测试代码,而是测试开发者是否真正理解改动内容。

多数上线事故不是因为代码有 bug,而是改代码的人不理解为什么那些旧逻辑存在。

执行流程

  1. AI 根据本次改动生成一份《改动理解测试题》,包含:
    • 改动的背景与目的(1 问)—— 确保开发者知道为什么改
    • 核心实现逻辑(2-3 问)—— 验证是否理解关键变更
    • 影响范围与潜在风险(1-2 问)—— 验证是否知道可能出问题的地方
    • 回退方案(1 问)—— 验证是否知道出问题了怎么恢复
  2. 开发者逐题作答,AI 逐题检查是否正确。答对有误时 AI 使用引导式提问帮助开发者理解(复用「建设性反馈原则」),而非直接给答案。
  3. 全部答对 → Self-Test 通过,进入合并流程。 答错 → 记录知识盲区,建议开发者返回对应 Phase 重新理解,修复后再测。

通过条件

  • 开发者正确回答了改动的背景、逻辑、影响范围和回退方案
  • 答错的知识盲区已记录(MAY 记入 memory/issues.md 或审查文档中)
  • Self-Test 结论不替代第三步的代码审查——Self-Test 通过不代表代码无问题

第五步:输出审查文档

  • 必须将本次「修改总结 + 场景矩阵 + 审查结论」写入项目 docs/review/ 下的审查文档。
  • 一个分支只需要一个 review 文档:若当前分支已有对应审查文档(如 <需求标识>-review.md),在原有文档基础上更新本次变更的修改总结、场景矩阵与审查结论,不新建重复文档;若无,则新建 <需求或需求标识>-review.md。
  • 文件名建议:<需求或需求标识>-review.md(如 AI-748-review.md),同一分支后续审查只更新该文件,不新增 *-re-review.md、*-code-review.md 等分散文档。
  • 文档内容至少包含:修改文件列表与要点、场景矩阵、审查结论(通过/不通过及原因,发现带来源标注)、专项清单检查结果、Secret 扫描结果、§3.13 兼容性声明(前端改动时:基线、风险面、兼容审查状态)、多视角交叉复核结论(如执行过)、Self-Test 题目与结论(如执行过)。
  • 若由上层工作流调用并指定了输出路径(如 ai-coding-workflow Phase 7 的 specs/<feature-id>/review-findings.md),按调用方指定的路径写入,内容要求不变。
  • 若项目根目录下无 docs/review/,先创建该目录再写入;若已有 docs/review/README.md,在 README 中维护审查文档索引(每个需求/分支对应一条)。

第六步:持久化 Learnings(经验沉淀,可选)

如果本次审查发现了一个非显而易见的模式/陷阱/反模式,记录到 docs/review/learnings.jsonl,下次审查自动加载。

echo '{"ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","type":"TYPE","key":"SHORT-KEY","insight":"DESCRIPTION","confidence":N,"files":["path/to/file"]}' >> docs/review/learnings.jsonl
  • type:pattern(可复用做法)、pitfall(不要做的事)、architecture(结构决策)
  • confidence:1-10。实际观察到的 8-9,推理的 4-5
  • 如果本次审查未发现值得记录的知识,跳过此步

记录前先做质量判定(先排除,再记录)。「值得记录」的唯一标准:下次审查召回后,能改变 Agent 的审查行为——更早发现问题、避免漏检同类陷阱。达不到这条标准的,不记录。判定分三步:

① 排除垃圾特征(命中任一条即不记录):

垃圾特征说明审查场景示例
事实性错误编造不存在的约束/API/方法/配置项「X 方法应返回 Y」但该方法实际不存在
通用常识任何项目都适用的常规规则「记得加错误处理」
对话摘要只复述本次审查过程,未沉淀可复用判断「本次审查了 auth 模块」
一次性 case只对当前 diff 有效,不可迁移「此文件第 N 行不该这样写」
主观偏好个人选择,不代表团队规范「我更喜欢这种写法」
缺少上下文未关联模块/组件/接口/文件,无法定位复用一条孤立的「这里要异步」
粒度混用一条经验里通用规则与 case 特定信息混杂「hook 要异步(且仅 stop hook 场景)」
不可执行只有抽象建议,下次审查不知怎么用「要谨慎处理并发」
证据不足本次 diff 中依据不足,推理成分过多「感觉这会影响其他功能」

② 源码事实校验(涉及具体 API/类/方法时必做):经验中提到的类名、方法名、配置项、路径,先用 rg/grep 在代码库中检索其存在性,再记录。纯文本自洽不等于真实存在——曾有经验声称某类应调用 attachToQBListView(),源码检索发现该类根本没有此方法,正确方法是另一个类的方法。记录一个不存在的方法,比不记录危害更大:下次审查会按错误指引检查,甚至误导 Agent 怀疑正确代码有误。

③ 与历史 learnings 去重/合并:记录前读取 docs/review/learnings.jsonl,按 key/insight 判断与既有经验的关系:

  • 完全重复 → 跳过,不重复写入
  • 同向且有新信息(更具体的场景/触发条件)→ 合并进原有条目,不新增
  • 结论冲突 → 不覆盖旧经验,标注冲突后保留人工裁决(宁严勿宽:误合并造成的边界污染不可逆)

默认策略:宁漏勿错——把握不足的不记录。误记一条泛泛而谈的经验,比漏记一条具体经验对下次审查的干扰更大。

审查不通过时的处理

  1. 明确告知用户:审查不通过,并列出原因(逐条,带文件路径与行号)。
  2. 给出修改建议:使用「情境 + 具体问题 + 建议方案」格式,避免空洞的「请修改」。
  3. 不执行提交:在用户根据建议修改并再次审查通过前,不进行 git commit;若用户坚持提交,提醒风险并记录原因。

建设性反馈原则

使用引导式提问代替直接否定,促进思考而非施压:

❌ 「这里有 bug,必须改。」
✅ 「如果 items 是空数组,这里会发生什么?」

❌ 「你需要加错误处理。」
✅ 「如果这个 API 调用失败,应该如何处理?」

❌ 「这样写性能很差。」
✅ 「用户量到 10 万时,这里每次都循环查询会有什么影响?」

更好的做法(推荐)

  1. 先定范围再改:修改前明确「只动哪些文件、只影响哪些场景」,审查时重点核对是否越界。
  2. 平台/入口显式限制:凡「仅某平台或某入口」才生效的逻辑,用显式条件(如 isWechat、route.query.xxx、配置开关)包裹,避免其他平台误走。
  3. 场景矩阵:对登录、平台、URL、配置等做一张「场景 → 预期 → 结论」表,避免漏掉边界。
  4. 关联代码看一眼:修改了 layout/请求/路由相关逻辑时,顺带确认 permission、request 拦截、401 处理未改且兼容。
  5. 审查文档沉淀:每个分支只维护一个 review 文档,后续审查在原有文档上更新变更部分,避免同一需求多份重复文档。
  6. Self-Test 双保险:代码审查通过不等于开发者完全理解改动。大面积重构、关键路径或 AI 生成的代码,建议在审查通过后追加开发者理解度校验,避免因不理解旧逻辑而埋下后续隐患。

输出约定

  • 总结与审查结论以中文呈现(除非项目约定英文)。
  • 审查不通过时,🔴 阻塞问题单独成段,便于用户一眼看到。
  • 不通过时必须包含具体原因 + 修改建议,使用「情境 + 具体问题 + 建议方案」格式。
  • 如有值得肯定的实现,用 🎉 [亮点] 说明,保持建设性氛围。

小结

  • 本质:CR 确认「做对了吗 + 放对了吗」,目标是让人安心发布——它是手段,不是目的。
  • 目的:确保每次修改在提交前功能测试通过,不引入新问题,不影响需求外的功能,且开发者对改动有充分理解。
  • 通过:无 🔴 阻塞问题,写明符合预期且不影响其他功能,并点出关键保障。
  • 不通过:明确原因(文件/行号/场景)+ 修改建议,不执行提交直至用户修复并再次审查通过(或用户明确坚持提交时提醒风险)。
  • 审查顺序:先架构定性(§2.7)再逐行专项(§3.2-§3.10),避免在错误架构上精挑细选;安全发现按 §3.2 定级规则处理;Secret 扫描每次必跑;前端改动必跑 §3.13 浏览器兼容性声明(只声明不检查,涉及兼容风险面时转交专门的兼容性审查技能或工具);大范围/高风险改动追加 §3.12 多视角交叉复核。

参考:Claude Code 核心工程师爆火攻略:先扫清你的「AI 盲区」

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.