review git:20260817.d00a470A

Immutable. This exact content is served forever at /api/v1/blob/115946b114eff565.

---
name: review
description: 审查一批改动时用。缺陷优先，并按这个仓库特有的失效方式去找问题——那些编译通过、类型正确、看起来合理的写法。
---

# 审查改动

## 先定范围

```bash
git diff                    # 未提交的
git diff --stat master...HEAD   # 相对基线分支的整条分支
```

范围大的时候起一个 `explore` 子 agent 去摸背景（它只读、便宜），自己留着
上下文做判断。

## 该找什么

这个仓库的 bug 基本都是**「编译通过、类型正确、看起来合理」**的那种。所以
不要花力气在命名、格式、能不能少写两行上——那些 clippy 和 fmt 管了。按下面
这几类去找：

**1. 静默的错答案。** 结果不完整而没说出来是最典型的：截断了、超时提前收工、
搜索没走完、工具结果被落盘替换成预览——只要模型可能把半份东西当全部，就是
一个 bug，而它不报错不崩，只是结论错。

真实案例：搜索因超时收工且没有匹配时返回「没有找到」，那是把「没搜」说成
「不存在」，模型据此断定这东西不存在。

**2. fail-closed 的方向反了。** 认不出的输入、解析失败、超时、无人应答，
必须落在保守那一侧。逐项问：这条路上「不确定」会变成「放行」吗？

- 权限：`ask` 在无人应答时必须收敛成 **deny**，不是 allow。
- 工具：认不出的类型/参数按「会写」算，不按「只读」算。
- 判危：判不准必须是 Hold，不能是 Safe。

**3. 安全检查被绕过。** 决策链的顺序是有意义的：安全检查排在 bypass 模式
**前面**，工具自己返回的 `Allow` **不是终点**。改动里如果有提前 `return`、
新增的短路、或者把 `Passthrough` 改成 `Allow`，都要问一遍它是不是跳过了
第 4 步。

同类：hook 的 `allow` 不能压过 settings 的 deny/ask；分类器的权力不能超过
bypass 模式（判据是 `yields_to_bypass()`）。

**4. 有实现没测试守着。** 这是最难看出来的一类，而仓库里真出过：决策链
第 3 步拿到工具的 `Allow` 后必须继续走安全检查，实现是对的，但**没有任何
测试守着那一行**。

审查时对每个关键分支问：把这行删掉/改反，会有测试红吗？答不上来就是个发现。
碰上权限层、路径围栏、文件工具、进程执行器，直接建议跑 `mutate`。

**5. 断言的是「失败了」还是「以正确的理由失败」。** 只断言 `!is_ok` 往往
不够。同一个拒绝，理由从「你还没读过」变成「文件内容对不上」，对模型是
实打实的区别——前者让它先读，后者让它以为有人在并发改文件，白跑一轮。

**6. prompt cache 前缀被打碎。** 工具注册顺序变了、system prompt 分段动了、
技能清单顺序不稳定（`read_dir` 的随机顺序）、beta header 集合中途变化——
这些都不报错，只是悄悄变贵。

**7. 注入的边界被绕开。** 内核里直接 `SystemTime::now()` / `std::fs` /
`std::process` / `tokio::time::sleep`，或者给它们加 `#[allow(clippy::disallowed_methods)]`
而理由不成立。黄金回放要成立就靠这条。

**8. 协议改动的两侧。** 改了 `crates/riot-protocol` 的类型：`pnpm gen` 跑了吗？
新字段带 `#[serde(default)]` 吗（不带就读不了老配置/老 transcript，表现为
「我配的东西全没了」）？枚举加了 variant，穷举 match 那几处都重审过吗？

## 怎么报

按**严重程度**排，不按文件顺序。每条给：

- 位置（用 `起始行:结束行:路径` 的代码引用格式，界面上能点开）；
- **这个问题会怎么表现**——具体症状，不是「可能有风险」。说不出症状的
  发现多半不是发现；
- 建议的改法，或者「需要一个测试守着这里」。

最后单独说一句「没发现问题的部分」范围有多大——审了 3 个文件和审了 30 个
文件，同一句「看起来没问题」的含义完全不同。

## 不要做的事

不要顺手改。审查的产出是判断，改不改、怎么改由用户决定。