# Review Changes

> 当一段改动已经写完、要在收尾或合并前对它做一次独立审查时使用——如"审一遍这条分支、review 这次改动、这批代码有没有问题、帮我把关再合"。先把 BASE..HEAD 生成一份审查包一次读完，再按 correctness / readability / architecture / security / performance 五轴加测试质量逐项判定，输出带 file:line 的 Critical / Important / Minor 分级 findings。composable 层能力单元，通常由 execute-task 在整体验收时调用。不要用于：改动还没写完、只想跑测试、要的是动手修复而不是审查、评审需求或技术方案本身。

- Skill: `rollrollroll/review-changes` (Agent Skill, multi-file: 5 files)
- Install (CLI): `npx skillmds@latest add rollrollroll/review-changes`
- Raw SKILL.md: https://api.skillmd.com/api/skills/rollrollroll/review-changes/raw
- Safety review: PASS (external: skill-scanner PASS, skillspector PASS)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: RollRollRoll (https://skillmd.com/u/rollrollroll)
- Updated: 2026-08-19
- Page: https://skillmd.com/skills/rollrollroll/review-changes

---


# Review Changes — 整体改动审查 Skill（composable 层）

## 你的任务

对一段**已完成的改动**（默认 `BASE..HEAD` 整条开发线）做一次**独立审查**：
把改动生成一份审查包一次读完，按**六关**逐项判定，输出带 `file:line` 的分级 findings。

你只**看**，不**改**：不动工作区、不动 git 状态、不修 findings。修复由调用方决定谁来做、修不修。

> 一句话：一次读完整包，六关逐项判定，问题分级带位置，只审不改。

## 调用契约

**传入**

- **审查范围**：`BASE` 起点 commit（本 skill 自己生成包），或调用方已生成好的**审查包路径**。
- **需求依据**（可选）：spec / design / tasks 路径——correctness 轴按它判"验收标准真满足吗"。
- **执行期疑虑**（可选）：实现方留下的结构问题清单，architecture 轴优先看这些点。
- **回执落盘路径**（可选）：缺省 `.review-changes/`（自忽略）。

**取回**

- 分级 **findings**：每条带 `file:line` + 什么问题 + 为什么要紧（能给修法就给）。
- **六关各一句判定**。
- 先说做得好的（要具体），再列问题。

**本 skill 不做**

- 不改代码、不 commit、不动 git 状态、不跑修复。
- 不做**系统性覆盖核对回扫**（design 每个组件 / spec 每条 MUST 是否都有落点）——
  那要上游产物与任务清单，是调用方（execute-task）的闸门。
- 不评审需求或技术方案本身：方案有问题就作为 finding 报上去，让调用方退回 **make-design / write-spec**，
  不在这里重做设计。

**独立性要求（调用方保证）**

审的人不能是写的人。调用方参与过实现时，必须**派一个 fresh 上下文的 subagent 加载本 skill 执行**，
并显式指定**最强档**模型——fresh 上下文正是这道门的价值。平台无 subagent 能力时，
明确记录降级原因，由调用方以 fresh 视角完整走一遍六关，**不删掉这道门**。

## 招牌机制：一次读完整包 + 六关 ⭐

### 一次读完整包

审查前先把范围凝固成一份包：`scripts/review-package.sh <BASE> [输出目录]` 生成
**commit 清单 + 变更统计 + `BASE..HEAD` 完整 diff（`-U10`）**，并打印路径。

- **读一次即可**——上下文行就是改动后的文件，不必自己爬库推导改动。
- 只有为核实一个**能点名的具体风险**才看包外代码，并在回执里写明查了什么。
- 脚本会先**拒绝任何非忽略的 dirty 状态**、确认 `BASE` 是 `HEAD` 的祖先：
  审一个漏掉未提交改动的包等于没审。报错就先处理再重跑，不绕过。

### 六关

五轴 + 测试质量，逐关表态，每关给一句判定（判据见 [references/axes.md](references/axes.md)）：

| 关 | 问什么 |
|---|---|
| **correctness** 正确 | 行为对不对，边界 / 错误路径处理了吗，验收标准真满足吗 |
| **readability** 可读 | 命名 / 结构清晰吗，后人能看懂吗 |
| **architecture** 架构 | 模块边界 / 职责合理吗，有没有该抽的重复、该隔的耦合 |
| **security** 安全 | 输入校验、权限、敏感数据、注入面有没有问题 |
| **performance** 性能 | 有没有明显热点 / N+1 / 不必要开销（按项目实际规模判，别过早优化） |
| **测试质量** | 验的是真行为还是实现细节 / mock，期望值有没有按代码算法重算，边界覆盖了吗 |

**测试质量是第六关，不是附加题**：红绿循环里没有 reviewer 审过测试本身，这里是唯一关口。
一批测试全绿但全是同义反复，等于没有测试。

## 工作流程

1. **定范围**：拿到 `BASE` 或包路径。`BASE` 不明确 → 问调用方，不自己猜一个起点。
2. **生成包**：跑 `scripts/review-package.sh <BASE> [输出目录]`，拿打印路径。
   （调用方已给包路径就跳过。）
3. **读一遍包**：commit 清单看意图脉络，变更统计看范围，diff 看实现。
4. **六关逐项判定**：每关都要有结论，"看着没问题"也要说出依据。
5. **写回执**：先具体地说做得好的，再列分级 findings，最后六关各一句判定。

## findings 分级

- **Critical**：行为错 / 违反验收标准 / 安全漏洞——不修不能合。
- **Important**：不修不能信任——坏味道、回归风险、需求之外的多余改动、测试验的是实现细节。
- **Minor**：雕琢项——记录，不阻塞。

分级要能站住：把 Minor 报成 Critical 会让调用方的修复轮空转，把 Critical 压成 Minor 则等于没审。

## 回执格式

```text
## 做得好的
- [具体到 file:line 或具体做法，不要"整体不错"]

## Findings
- [Critical] path/to/file.py:42 —— [什么问题] · [为什么要紧] · [修法（若有）]
- [Important] ...
- [Minor] ...

## 六关判定
- correctness：[一句]
- readability：[一句]
- architecture：[一句]
- security：[一句]
- performance：[一句]
- 测试质量：[一句]

（包外核实：[查了什么、为什么；没有则写"无"]）
```

## 反例（不要这样做）

❌ 自己爬库推导改动，不生成审查包 —— 漏掉的改动永远审不到。
❌ 工作区 dirty 就硬审 —— 未提交的改动不在 `BASE..HEAD` 里，审的是残缺范围。
❌ 顺手把发现的问题改掉 —— 本 skill 只读；改了就没人知道审的是哪个版本。
❌ 跳过测试质量关，看到"测试全绿"就通过 —— 同义反复测试按构造必然通过。
❌ finding 不带 `file:line`、不说为什么要紧 —— 调用方没法判、没法修。
❌ 把雕琢项报成 Critical，或把行为错误压成 Minor。
❌ 写的人自己审 —— 独立性没了，这道门就只是走个形式。
❌ 在这里重做需求 / 技术方案 —— 报成 finding，退回上游。
❌ 只回一句"看起来没问题" —— 六关每关都要有可解释的判定。

## 自检清单

- [ ] 审查范围是**包**（commit 清单 + 统计 + 完整 diff），不是自己爬出来的？
- [ ] 生成包时工作区无非忽略改动、`BASE` 是 `HEAD` 的祖先？
- [ ] **六关都表态**了，包括测试质量关？
- [ ] 每条 finding 都有 `file:line` + 问题 + 为什么要紧？分级站得住？
- [ ] 全程**只读**：没改代码、没 commit、没动 git 状态？
- [ ] 看包外代码的地方都写进回执了？
- [ ] 先具体说了做得好的，不是只列问题？

## 相关参考

- [references/axes.md](references/axes.md) —— 六关逐项判据：每关看什么、什么算过、常见漏判。
- `scripts/review-package.sh` —— 拒绝 dirty 工作区并校验祖先关系后，生成
  `BASE..HEAD` 审查包（commit 清单 + 变更统计 + 完整 diff），按轮次自动递增命名。

## 分层与调用方

- **composable 层**能力单元：通常由 workflow 层的 **execute-task**（阶段 3 整体验收）调用，
  也可被用户直接触发（"审一遍这条分支再合"）。
- 调用方负责**独立性与档位**（fresh subagent + 最强档）、**修复轮次**与**覆盖核对回扫**；
  本 skill 只负责审得准、报得清。
- 修复完再审时，调用方 commit 后**重新生成包**，本 skill 读新一轮的包，不复用旧包。