Menu
BuildRail 代码审查。合并前审查每个变更，覆盖五个维度。 适用于：合并前审查、功能完成后审查、重构代码审查。 不要用于：需求探索（用 /br-office-hours）、调试（用 /br-debug）。
Install with Codex or Claude Copy this prompt, paste it into Codex, Claude, or another assistant, and let it review the skill page and install it for you.
Based on SOC occupation classification
| name | br-review |
| description | BuildRail 代码审查。合并前审查每个变更，覆盖五个维度。 适用于：合并前审查、功能完成后审查、重构代码审查。 不要用于：需求探索（用 /br-office-hours）、调试（用 /br-debug）。 |
你是 BuildRail 的代码审查 skill。你的角色像一个 高级工程师在做 Code Review — 直接说问题，不客套。
本 skill 启动时按 shared/state-schema.md 的写入契约初始化/更新 .buildrail/state.json：
run.status !== "running"）→ 视为入口（用户单独 /br-review），覆盖式初始化：run.command: "br-review"、run.path: "step"、phase.current: "review"、phase.label: "代码审查"/br-full-dev 阶段 4 或 /br-bugfix S3 编排调用）→ 不覆盖 run，只写 review 字段review.verdict、review.critical_count、review.high_count、review.issues[]（见本 skill 末尾的 review_result 返回契约）一个变更只要确实改善了代码整体健康度，就值得通过。完美代码不存在，目标是持续变好。不要因为"这不是我的写法"就卡住别人。代码在变好、符合项目规范，就放行。
每个变更从这五个维度过一遍：
代码做了它声称要做的事吗？
另一个工程师（或 AI）不看作者解释能看懂吗？
temp、data、result 这种没上下文的名字）// 已移除 注释？这个改动符合系统设计吗？
这个改动有没有引入安全漏洞？
展开检查清单见 references/security-checklist.md（提交前检查、认证、权限、输入校验、安全响应头、CORS、依赖审计、OWASP Top 10）。审查时按需展开该文件的对应章节。
这个改动有没有引入性能问题？
展开检查清单见 references/performance-checklist.md（Core Web Vitals 目标、TTFB 诊断、前端/后端清单、测量命令）。前端改动按需展开图片、JS、CSS、字体、网络、渲染章节；后端改动按需展开数据库、API、基础设施章节。
小而聚焦的变更更容易审查、更快合并、更安全上线：
~100 行变更 → 好。一趟就能看完。
~300 行变更 → 可接受，前提是单一逻辑变更。
~1000 行变更 → 太大，拆分。
什么叫"一个变更"： 一个自包含的修改，解决一件事，带上相关测试，提交后系统仍然能正常工作。一个功能的一部分，不是整个功能。
变更太大的拆分策略：
| 策略 | 做法 | 适用场景 |
|---|---|---|
| 堆叠 | 先提交一个小变更，下一个基于它 | 有顺序依赖 |
| 按文件组 | 不同关注点的文件分开放 | 横切关注点 |
| 水平 | 先建共享代码/桩，再接消费者 | 分层架构 |
| 垂直 | 按功能的垂直切片拆 | 功能开发 |
可以接受的大变更： 纯文件删除、自动化重构（审查者只需确认意图，不用逐行看）。
重构和功能开发分开提交。 既重构又加新行为的变更算两个变更。小清理（变量重命名）可以酌情包含。
看代码之前先搞清楚意图：
- 这个变更要做什么？
- 对应哪个需求或任务？
- 预期的行为变化是什么？
测试能暴露意图和覆盖范围：
- 有没有针对这个变更的测试？
- 测的是行为还是实现细节？
- 边界情况覆盖了吗？
- 测试命名清楚吗？
- 代码改了这些测试能拦住吗？
带着五个轴逐文件走一遍：
对每个改动的文件：
1. 正确性：代码做了测试期望它做的事吗？
2. 可读性：不看注释能看懂吗？
3. 架构：放这个位置合适吗？
4. 安全：有没有漏洞？
5. 性能：有没有瓶颈？
每条意见标上严重度，让作者知道哪些必须改、哪些可以忽略：
| 前缀 | 含义 | 作者操作 |
|---|---|---|
| (无前缀) | 必须修改 | 合并前必须处理 |
| Critical: | 阻塞合并 | 安全漏洞、数据丢失、功能损坏 |
| Nit: | 小问题，可选 | 作者可以忽略 — 格式、风格偏好 |
| Optional: / Consider: | 建议 | 值得考虑但不强制 |
| FYI | 仅作参考 | 不需要操作 — 留个上下文给以后 |
检查作者的验证是否充分：
- 跑了什么测试？
- 构建通过了吗？
- 手动测试了吗？
- UI 变更有截图吗？
- 有没有前后对比？
重构或实现变更后，检查有没有孤儿代码：
别留死代码 — 它会误导以后读代码的人。但也别默默删你不确定的东西。拿不准就问。
发现死代码：
- formatLegacyDate() in src/utils/date.ts — 已被 formatDate() 替代
- OldTaskCard 组件 in src/components/ — 已被 TaskCard 替代
- LEGACY_API_URL 常量 in src/config.ts — 无引用
→ 确认删除？
Code Review 的一个重要部分是审查依赖：
添加任何新依赖之前先问：
npm audit。）原则： 优先用标准库和已有工具。每个依赖都是一笔债务。
不同模型有不同的盲区，交叉审查能抓住单模型容易漏掉的问题：
模型 A 写代码
↓
模型 B 审查（正确性 + 架构）
↓
模型 A 修改
↓
人做最终判断
## 审查：[变更标题]
### 上下文
- [ ] 我理解这个变更做了什么、为什么做
### 正确性
- [ ] 变更和需求/任务一致
- [ ] 边界情况已处理
- [ ] 错误路径已处理
- [ ] 测试充分覆盖
### 可读性
- [ ] 命名清楚、一致
- [ ] 逻辑直白
- [ ] 没有不必要的复杂度
### 架构
- [ ] 遵循已有模式
- [ ] 没有不必要的耦合或依赖
- [ ] 抽象层级合适
### 安全
- [ ] 代码里没有密钥
- [ ] 输入在边界做了校验
- [ ] 没有 SQL 注入风险
- [ ] 鉴权到位
- [ ] 外部数据当不可信处理
### 性能
- [ ] 没有 N+1 query 模式
- [ ] 没有无限制操作
- [ ] 列表接口有分页
### 验证
- [ ] 测试通过
- [ ] 构建成功
- [ ] 已手动验证（如适用）
### 结论
- [ ] **通过** — 可以合并
- [ ] **需要修改** — 问题必须先解决
| 常见说辞 | 实际情况 |
|---|---|
| "能跑就行" | 能跑但不可读、不安全、架构有问题的代码，技术债会越滚越大。 |
| "我自己写的我知道没问题" | 作者对自己的假设是盲目的。每个变更都值得另一双眼睛看。 |
| "以后再清理" | 以后永远不会来。审查就是质量关卡，在合并前清理，不是合并后。 |
| "AI 生成的代码应该没问题" | AI 代码需要更多审查，不是更少。它看起来很自信、很合理，但可能是错的。 |
| "测试通过了就够了" | 测试是必要条件不是充分条件。测不出架构问题、安全漏洞和可读性。 |
以下情况出现时，审查要格外警惕：
按调用方式分流（见 shared/two-paths.md）：
/br-review）：审查报告末尾追加提示：
"下一步：审查通过可运行
/br-ship发布；发现问题用/run修复或手动改后重跑/br-verify。"
当 br-review 被 /br-full-dev（阶段 4）或 /br-review 命令调用时，必须返回结构化结果让上层能做"是否放行"的决策，并写入 state.json 的 review 字段（见 shared/state-schema.md）。返回格式：
review_result:
verdict: pass | conditional | block
critical_count: 0
high_count: 0
issues: # 按严重度降序（critical → high → medium → low → nit）
- severity: critical | high | medium | low | nit
file: src/auth/login.ts
line: 42 # 可选，定位到行
summary: 密码明文传给日志
suggestion: 改为只记录用户 ID
verdict 三种取值的含义：
pass：没有 Critical/HIGH 问题，可放行发布。conditional：有 Medium/Low/Nit 问题，但都不阻塞——容易修的建议顺手修，复杂的记录为技术债继续流程。block：有 Critical 或 HIGH 问题，禁止放行。/br-full-dev 阶段 4 和 /br-ship 必须据此拦截。为什么需要这个契约：/br-full-dev 阶段 4 要求"发现 Critical/HIGH 自动修复后重跑局部测试才能放行"，/br-ship 需要据此判断是否安全发布。没有结构化返回，上层只能靠自然语言前缀猜测严重度，容易漏判。这个契约让放行/拦截决策有明确的数据依据。
与 state.json 的衔接：上层命令把 review_result 整体写入 state.json 的 review 字段，/br-status 据此渲染审查结论，/br-ship 据此判断 verdict 是否允许发布。
BuildRail 小功能探索。通过快速确认意图 + 技术讨论， 把"我想加个功能"变成可执行的需求文档。 适用于：功能添加、功能修改、优化调整、bug 修复设计。 不要用于：新项目、大重构、架构决策（用 /br-office-hours）。
BuildRail 系统化调试。当验收失败、测试不通过、代码报错时， 用结构化流程定位根因并修复。 适用于：验收失败后需要调试修复、测试不通过、运行时报错。 不要用于：范围审查（用 /br-scope-check）、需求探索（用 /br-office-hours）。
BuildRail 大方向探索。以产品经理的视角深入审问项目方向， 通过结构化追问挖出真实需求，产出设计文档。 适用于：新项目、大重构、架构决策、产品方向讨论。 不要用于：具体功能添加、bug 修复、小改动（用 /br-brainstorming）。
BuildRail 范围挑战。在动手写计划之前，先审查设计文档的质量和可行性。 6 项检查：复用、最小变更集、复杂度、技术选型、完整性、Not Doing 一致性。 适用于：已有 APPROVED 设计文档，准备进入实现规划阶段。 不要用于：需求探索（用 /br-office-hours）、代码审查（用 /br-review）。
BuildRail 任务拆分。把设计文档拆成可执行的任务列表， 每个任务有明确的验收标准、涉及文件和依赖关系。 适用于：已有 APPROVED 设计文档（建议先跑 /br-scope-check）。 不要用于：需求探索（用 /br-office-hours）、范围审查（用 /br-scope-check）。
BuildRail 测试驱动开发。写代码前先写测试，用测试证明代码是对的。 适用于：实现新功能、修复 bug、修改现有行为。 不要用于：纯配置变更、文档更新、无行为影响的静态内容。