我最近做了一次代码审查，最先看到的结果很容易让人放松：测试是通过的，几个检查命令也没有报错。按通常的工作习惯，接下来似乎就是看几处实现细节，挑一挑命名和重复代码，然后把修改合进去。
但我没有马上相信这个“绿”。继续追测试入口时，发现最初的测试收集并不完整，smoke check 实际只跑了 19 条，而当时预期的范围是 38 条。也就是说，终端上的通过，并不能证明系统被完整地检查过，只能证明被收集到的那部分没有失败。
这件事让我重新意识到，代码审查里最危险的不是一个明显的红灯，而是假绿。红灯会逼人停下来，假绿却会让人带着错误的安全感继续往前走。测试通过之前，必须先证明测试真的收集到了该收集的内容。
绿色结果为什么不一定可信
表面上，这次问题像是“还有几个 bug 没修完”。实际上，第一层问题是质量门本身没有把风险拦住。测试没有完整收集，smoke check 只覆盖一部分，审查者却很容易把“命令退出码为 0”理解成“这一组代码已经安全”。
这两者差别很大。一个检查如果没有跑到目标文件，当然不会报错；一个门禁如果只检查常见路径，也可能在旁路路径上保持绿色。结果看起来干净，覆盖面却是空的。后续所有判断都建立在这个错误前提上，越早把它当成结论，返工成本越高。
如果不先处理测试收集问题，后面的修复会出现一种很麻烦的情况：代码确实改了，测试也确实通过了，但通过的仍然不是我们以为的那套测试。等到真正的输入、异常分支或发布流程触发问题时，大家才发现以前的绿色只是没有撞上缺口。
所以我先确认了 3 件事：测试目录是否被发现，测试数量是否符合预期，smoke check 与全量测试之间到底差了哪些行为。这个动作没有修改业务代码，却改变了整个审查的可信度。没有这一步，后面的“修复完成”都缺少地基。
先复现假绿，再谈业务问题
审查不能只读实现，也不能只看最终报告。我把已有的检查重新跑了一遍，并拿预期数量和实际数量对照。这个过程很快暴露出，问题不是测试失败，而是测试没有全部进入运行范围。
我没有直接把缺失的测试文件搬来搬去，也没有先重写测试命令。因为我判断，先要知道收集规则为什么漏掉它们：是命名约定不一致，是入口只扫了一个目录，还是 smoke check 本来就被设计成了窄范围。原因不同，修复位置就不同，贸然补文件可能只是把症状盖住。
修正收集和检查入口后，全量测试从原来的 38 条扩展到 54 条，并在后续几轮持续通过。这个数字的意义不在于“多了 16 条测试”，而在于我们终于知道绿色覆盖了什么。测试数量的增加不是质量本身，但从不确定的覆盖范围变成可解释的覆盖范围，是质量门开始起作用的标志。
这里最容易误判的错误捷径，就是把一次 pytest 通过当成系统安全。命令成功只是一个观察结果，不是安全结论。我要问的是：它收集了谁？漏了谁？哪些危险行为有专门断言？如果答不出来，就还没有资格用“测试通过”结束审查。
真正高风险的不是样式问题
测试门恢复可信以后，审查重点才从“能不能跑”转向“哪些行为一旦出错会造成不可逆后果”。这次我看到的风险并不集中在代码风格，而是集中在边界上：只读查询的 SQL 护栏、dry-run 模式下的进程处理，以及默认覆盖远端数据的行为。
只读查询看起来不像会修改数据，但如果护栏只保护了常见语句，换一种表达方式就可能绕过去。dry-run 的名字也不能自动保证安全；如果预览路径仍然会误杀进程，它就是把破坏性动作藏在了“演练”选项后面。默认覆盖远端更直接：本地状态一旦判断错误，发布动作可能把远端已有内容替换掉。
我选择先复现这些行为，是因为它们的风险优先级高于格式、命名和重复代码。只读路径被绕过，影响的是数据安全；dry-run 误杀，影响的是正在运行的工作；默认远端覆盖，影响的是恢复成本。审查资源有限时，应该先处理后果更重、回滚更难的地方。
这也是我对“逐文件挑问题”这个错误捷径的修正。逐文件看似完整，实际上容易把风险切碎：每个文件都只像有一个小问题，跨文件的数据流和调用关系却没人负责。代码审查不是给文件打分，而是判断系统在关键动作上是否守住了边界。
把跨文件问题拆成能验证的组
这次修复涉及多个文件。如果所有人沿着同一条调用链顺序修改，容易互相等待，也容易在共享模块上产生冲突。我把问题按风险和共性拆成几个互不冲突的审查组：一组看数据安全和只读边界，一组看破坏性操作和 dry-run，一组看解析与路径边界，另一组看发布状态和共用基础能力。
这种拆分不是为了让并行看起来更高效，而是为了让每个组都能回答一个完整问题。数据安全组要证明输入不会绕过护栏；破坏性操作组要证明预览不会执行真实动作；解析边界组要证明异常输入不会把路径或配置解释错；发布状态组要证明写入、超时和状态更新不会留下半成品。
每个问题先做最小复现，再修共享根因。比如，多个地方都涉及文件写入，就不在每个调用点各自补一段临时处理，而是统一检查原子写的临时文件策略；多个入口都依赖路径，就先明确路径边界，再让调用方复用同一规则。这样做的理由很简单：同一种错误如果散落修补，下一处迟早会重新出现。
并行审查也有边界。共享基础模块不能被多个组各自改出不同语义，涉及行为变化的部分必须回到同一套回归测试里验证。并行的价值不是把修改切得越碎越好，而是把问题切成可以独立证明、最后又能合并验证的判断单元。
修复业务 bug 之前先补基础护栏
后续修复里，有些问题很容易被归类成“业务 bug”，但根因其实在基础能力：缺少超时控制，异常时流程会一直等待；文件写入不是可靠的原子替换，并发时可能丢更新；路径判断没有边界，输入稍微变化就会越过预期目录；手写 YAML 解析遇到引号、冒号或列表结构时会产生歧义。
这些问题的共同点是，它们不只影响一个业务分支。超时会影响所有依赖外部进程的动作，原子写会影响所有共享状态，路径和 YAML 解析会影响所有配置与扫描入口。如果只修眼前触发 bug 的那一处，其他调用方仍然带着同样的风险。
我没有因为“业务上暂时能用”就把它们排到最后。因为审查真正要防的不是当前样例，而是下一个稍微不同的输入。这里的决策标准是：如果一个基础问题能够让多个上层功能失去安全边界，就先修它；如果只是局部表现差异，才在基础护栏稳定后继续处理。
修复完成后，我用全量测试确认现有行为没有被扩大破坏，再用静态检查确认明显的实现错误没有遗漏，最后为高风险场景保留针对性断言。全量测试回答“整体有没有回归”，静态检查回答“代码结构有没有明显问题”，针对性断言回答“最担心的那条边界是否真的被守住”。三者不能互相替代。
下次我会怎么做
下次开始代码审查，我会把顺序固定成 4 步。先验证测试收集：实际收集数量、目录范围和 smoke 覆盖必须能解释。再验证门禁行为：故意制造一个应该失败的输入，看门禁是否真的会拦截，而不是只看它在正常输入下是否通过。
接着按后果分组审查，优先看数据安全、破坏性动作、远端写入和边界解析，再看局部业务逻辑。每个高风险问题先留下最小复现，修共享根因，避免把同一种缺陷复制成几份看似不同的补丁。最后用全量测试、静态检查和针对性断言收口，并记录测试数量和覆盖变化。
这件事让我重新意识到，代码审查的核心不是找出最多的问题，而是先确认自己手里的证据是否可靠。一个流程成熟的标志，不是它能在顺利时跑通一次，而是它能在失败时告诉我到底哪里没被检查、哪条边界没有拦住、下一步应该先修什么。真正值得交付的绿色，必须经得起这些追问。