| name | forge-review |
| description | 上线前 PR 审查。分析当前分支与基础分支的 diff，检查 SQL 安全、竞态条件、
LLM 信任边界、枚举完整性等测试捕获不到的结构性问题。发现问题直接修复。
|
| allowed-tools | ["Bash","Read","Edit","Write","Grep","Glob","AskUserQuestion"] |
/forge-review：代码审查
前置脚本（每次先运行）
_BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
echo "当前分支: $_BRANCH"
AskUserQuestion 格式规范
每次提问结构：
- 重新聚焦：当前项目、分支、正在审查的内容
- 通俗解释：高中生能懂的语言，说清楚"做什么"
- 给出建议：
推荐：选择[X]，因为[一句话原因]，标注完整度
- 列出选项：
A) B) C) + 工作量估算
第0步：确定基础分支
按顺序判断此 PR 合并到哪个分支：
gh pr view --json baseRefName -q .baseRefName 2>/dev/null
gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null
打印出基础分支名称，后续所有 git diff、git fetch、git merge 等命令中用实际分支名替换"基础分支"。
第1步：检查分支状态
git branch --show-current
git fetch origin <基础分支> --quiet && git diff origin/<基础分支> --stat
如果在基础分支上，或没有 diff，输出："没有可审查的内容——你在基础分支上或没有变更。" 并停止。
第2步：获取完整 diff
git fetch origin <基础分支> --quiet
git diff origin/<基础分支>
在评论之前先读完完整 diff。 不要标记 diff 中已经修复的问题。
第3步：两轮审查
第一轮（严重问题）
逐项检查以下类别，对每个 diff 文件详细分析：
1. SQL 与数据安全
检查：
- 用户输入是否直接拼入 SQL？（注入风险）
- 原始 SQL 查询是否使用了参数化？
- 批量更新/删除是否有
WHERE 条件？（全表操作风险）
- 事务边界是否正确？（部分成功状态）
- 软删除记录是否在所有查询中被过滤了？
报告格式：[严重] 文件:行号 — 问题描述 → 修复建议
2. 竞态条件与并发
检查：
- 先读后写操作是否有并发安全问题？（检查-再-操作 TOCTOU）
- 数据库操作是否需要乐观锁/悲观锁？
- 共享状态是否在多个请求间安全？
- 缓存失效是否有竞态？
特别注意：状态转换（如 draft → published）必须用数据库级约束，不能只依赖应用层检查。
3. LLM 输出信任边界
检查：
- LLM 生成的内容在写入数据库前是否经过验证/清洗？
- LLM 输出是否直接用于 SQL 构建或系统命令？
- 是否对 LLM 输出的类型和格式做了断言？
- Prompt 中是否包含了敏感数据（密钥、PII）？
4. 枚举与值完整性
检查：
- 新增枚举值/状态/类型时，所有引用了同级值的文件是否都处理了新值？
重要：枚举完整性必须读取 diff 以外的代码。当 diff 新增了枚举值，用 Grep 找出所有引用了同级值的文件，Read 这些文件检查新值是否被处理。
第二轮（信息性问题）
5. 条件副作用
检查：
- 副作用（发邮件、写日志、扣费用）是否在
if 语句的所有分支中都正确处理？
- 删除或禁用功能时，是否清理了对应的副作用触发器？
6. 魔法数字与字符串耦合
检查：
- 是否有应该提取为常量的重复数字/字符串？
- 硬编码的限制值（如
100、1000）是否有注释说明含义？
7. 死代码与一致性
检查：
- 是否有明显无法执行的代码路径？
- 同一逻辑的命名方式是否在整个文件中一致？
8. 测试缺口
检查：
- 新的分支逻辑有没有对应测试？
- 错误路径有没有测试？
- 是否存在只测试了 Happy Path 但跳过了边界情况？
9. 前端/视图层
检查：
- 用户输入是否经过 HTML 转义？（XSS 风险）
- 是否有
dangerouslySetInnerHTML 或 v-html 使用了未清洗的数据？
- 长文本/图片/空状态是否有兜底处理？
第4步：修复优先原则
每个发现都要有行动——不只是报告。
输出摘要头：上线前审查：N 个问题（X 严重，Y 信息性）
自动修复（AUTO-FIX）
机械性、无争议的修复直接应用。每个输出：
[已自动修复] 文件:行号 — 问题 → 修复内容
适合自动修复的典型问题：
- 添加缺失的 WHERE 条件（无歧义的全表操作）
- 移除
outline: none / !important
- 添加明显缺失的类型检查
- 修复 HTML 转义问题
需要确认（ASK）
需要判断或有真实权衡的问题，批量放入一次 AskUserQuestion（≤3个时可单独提问）：
我已自动修复了 5 个问题。2 个需要你的判断：
1. [严重] app/models/order.rb:42 — 状态转换竞态条件
修复：在 UPDATE 中添加 WHERE status = 'pending'
→ A) 按建议修复 B) 跳过
2. [信息性] app/services/ai_writer.rb:88 — AI 输出未经类型验证就写入数据库
修复：添加 JSON Schema 验证
→ A) 按建议修复 B) 跳过
推荐：两个都修——#1是真实竞态，#2防止静默数据损坏。
第5步：TODOS 交叉检查
读取仓库根目录的 TODOS.md（如果存在）：
- 此 PR 是否解决了某个 TODO？如果是，标注出来。
- 此 PR 是否创建了需要变成 TODO 的工作？如果是，标记为信息性发现。
第6步：文档过时检查
对比 diff 与根目录下的文档文件（README.md、ARCHITECTURE.md、CLAUDE.md 等）：
- 如果代码改动影响了文档描述的功能，但文档没有在此分支更新，标记为信息性发现：
"文档可能过时：[文件] 描述了 [功能]，但本分支修改了相关代码。考虑运行
/forge-ship 后更新文档。"
重要规则
- 先读完完整 diff，再评论。 不标记 diff 中已修复的问题。
- 修复优先，不只是报告。 AUTO-FIX 直接应用，ASK 等用户确认。
- 简洁。 一行问题，一行修复建议。
- 只标记真实问题。 正常代码直接跳过。
- 绝不提交、推送或创建 PR——那是
/forge-ship 的工作。
