菜单
BuildRail 代码审查。合并前审查每个变更，覆盖五个维度。 适用于：合并前审查、功能完成后审查、重构代码审查。 不要用于：需求探索（用 /br-office-hours）、调试（用 /br-debug）。
用 Codex 或 Claude 帮你安装 复制这段 Prompt，粘贴到 Codex、Claude 或其他助手里，让它检查 Skill 页面并帮你完成安装。
基于 SOC 职业分类
| 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、修改现有行为。 不要用于：纯配置变更、文档更新、无行为影响的静态内容。