requesting-code-review · diff
git:20261015.96a89a5 to git:20261015.b377f6b
12 added, 1 removed. Audit A to A.
---
name: requesting-code-review
description: 用户要求审查当前代码差异或 PR 时使用,默认只读;处理已收到的评审意见用 receiving-code-review。
compatibility: 本地差异审查需要 git。读取 GitHub PR 或发布评审需要可用的 GitHub 连接器或已认证的 gh CLI。
---
# 系统性代码审查
## 边界
- 审查当前本地差异或指定 GitHub PR,默认只分析和报告。
- `receiving-code-review` 处理别人已经给出的意见。
- 审查阶段不修改文件、不提交、不推送;用户已明确授权后续工作时,审查完成后继续对应步骤。
- 发布远端评审仍须用户明确要求;同一范围内已有授权时不重复请求。
- 不要求特定客户端插件。GitHub 连接器可用时优先使用,否则回退 `gh`;两者都不可用时仍可审查本地差异,远端操作明确报告未执行。
## 1. 固定审查快照
先确认仓库、工作区和审查范围:
```bash
git rev-parse --git-dir
git status --short
git branch --show-current
git log --oneline -10
```
+ 工作区中的修改、暂存和未跟踪文件都是用户的工作:可以读取并纳入审查范围,但不移动、隐藏、覆盖或清理。审查 PR 时不切换当前检出,也不用 `git switch`、`git checkout`、`git stash`、`git reset --hard`、`git clean` 或 `gh pr checkout` 准备环境;优先使用 `gh pr diff`、`git fetch origin pull/<n>/head:refs/tmp/pr-<n>`、`git merge-tree` 或临时 worktree。
+
PR 模式通过当前可用的 GitHub 能力读取编号、基准分支和来源分支、状态、更新时间、完整差异和检查。本地模式读取 `git diff HEAD` 或明确的 `<base>...HEAD`。记录实际范围,不基于旧差异或截断差异块下结论。
完整差异、逐文件覆盖、可信上下文、行号定位和反证规则遵循 [评审证据协议](references/evidence-protocol.md)。标准或深入审查先执行 [范围与完整性预判](references/review-preflight.md)。
## 1.1 场景契约与验收范围
如果变更目录或目标项目已有 `test-scenarios.md`,在深入审查前读取它,把它当作本次行为范围和验收标准的输入,而不是把测试数量当作覆盖证明。
检查:
- 每个受影响的行为是否能映射到场景 ID;
- `covered` 场景是否有真实测试命令、日志、运行结果或其他证据;
- `skipped` 场景是否说明原因、影响和后续动作;
- P0/P1、错误路径、兼容性、权限和人工 E2E 场景是否被遗漏;
- 实现、测试和场景契约之间是否出现互相矛盾的预期。
没有场景契约时,不要为了补文档而擅自创建文件。对于非平凡或行为变化的差异,在摘要中记录 `scenario contract: none`,将其作为覆盖与验收缺口,而不是自动判定为代码缺陷。
审查仍然是只读流程;场景状态和证据的补充由实现者或用户授权的后续步骤完成。
## 2. 确定深度与专项
- 快速:100 行以内且 1-5 个文件
- 标准:100-500 行,或 6-10 个文件
- 深入:500 行以上、10 个以上文件,或涉及认证、支付、数据写入等高风险路径
只根据本轮变更加载专项:
- 通用正确性、边界、错误处理、测试与复杂度:[代码质量](references/code-quality.md)
- 所有审查都做轻量复杂度检查;标准、深入审查或用户明确关注简化时,读取 [简化审查](references/simplification-review.md)
- 明确热点、I/O/网络放大、锁或分配风险:[性能](references/performance.md)
- 信任边界、鉴权、输入或密钥风险:[安全](references/security.md)
- 行为、API、配置或示例变化:[文档](references/documentation.md)
- `.go` / `go.mod`:[Go](references/go-specialist.md)
- `.java`:[Java](references/java.md)
- `.py`:[Python](references/python.md)
- CI、YAML/JSON 配置或依赖清单:[CI 与配置](references/ci-config.md)
+ - 差异命中以下形状时读取 [条件审查模式](references/review-patterns.md) 的对应小节:复发或难以观察的缺陷、按输出文本分支或异步完成、删除与精简、恢复刚删除的内容或把例外改成与同类一致、覆盖用户文件、批量删除或递归匹配、同一规则多处计算、测试是否覆盖真实入口、未发布功能的迁移代码、引用新标识符、依赖与锁文件变化
TS/JS 变更执行轻量语言检查,不因仓库存在某语言就加载无关清单。
## 3. 架构分路
用户明确要求架构评审,或差异改变模块分层、公共接口、适配器或中间件、状态或任务生命周期、存储、跨服务调用、权限、错误体系、并发与资源管理时,加载 [架构审查](references/architecture.md)。
架构结论必须基于当前差异和必要相邻实现。区分必须修复与可选优化,不把理想化分层、个人偏好或未触及历史问题包装成缺陷。
+ 发现集中在同一个根因时(同类缺陷反复打补丁、权限或状态问题源于架构本身、简单问题被做复杂),先给出路线结论:保留、调整、替换或信息不足,再列具体问题。只有替代方案能消除这类问题且迁移代价可接受时才比较;用户已声明路线确定或设计文档已记录该取舍时不重新讨论。
+
## 3.1 简化审查
每次审查都检查当前差异是否增加了可以删除的概念、分支、状态、层级或依赖,以及缺陷修复是否只在共享机制上为单个调用方叠加特例而没有修到根因。标准、深入审查或用户明确要求评估复杂度时,按 [简化审查](references/simplification-review.md) 完成专项检查。
简化建议必须落到当前代码:指出复杂度来源,给出适配现有类型和调用方式的替代实现,并说明如何验证错误语义、调用顺序、副作用和兼容性不变。只能描述“可以更优雅”或无法通过当前接口实现的方案,不作为 finding 输出。
## 4. 证据过滤与汇总
对每个候选 finding:
1. 读取定义、调用方、配置和测试等必要上下文;差异块不足时查真实代码。
2. 找能推翻关键事实的反证,有直接反证就删除。
3. 确认问题由当前变更引入或暴露,并定位到当前快照最小连续范围。
4. 内部评估置信度:`80+` 才作为问题;`50-79` 放入待确认;低于 50 不输出。机制真实、只是触发条件取决于时序、环境或配置的候选,例如并发竞态、罕见但可达路径上的空值、把零值当作缺失、边界 off-by-one、重试风暴或部分失败,不因“推测性”删除;至少放入待确认并写明确认方法。
+ - P0/P1 还要说明现有防护(校验、类型系统、上游捕获、框架默认行为)为什么拦不住;说明不了就降级或删除。
+ - 判定为回归前先读对应提交信息和作者意图;维护者有意的改动是决定,不是 finding。
+ - 深入审查的 P0/P1 候选做一次独立反驳:重新读取引用的代码,确认问题真实存在、没有在别处处理、不是设计如此、不是只在理论上可达;客户端支持子代理时可以每条交给独立代理反驳。
5. 按文件和根因去重,再按 P0/P1/P2 排序。
- 6. 标准或深入审查在去重后再做一轮补漏:带着已有清单重读差异和所在函数,只找清单之外的问题,重点看移动或抽取后丢失的校验与锚点、锁范围缩小、带副作用的判断函数、测试 setup 与 teardown 不对称、配置默认值被翻转。没有新问题时不凑数。
+ 6. 深入审查额外从四个角度找问题:代码默认成立的假设(格式、顺序、范围)不成立时会怎样;与现有系统在并发或部分失败下组合会怎样;哪串合法操作会走到非法状态;第 1000 次请求、发布过程中或两人同时编辑同一资源时会怎样。这些候选同样按第 4 步分流。
+ 7. 标准或深入审查在去重后再做一轮补漏:带着已有清单重读差异和所在函数,只找清单之外的问题,重点看移动或抽取后丢失的校验与锚点、锁范围缩小、带副作用的判断函数、测试 setup 与 teardown 不对称、配置默认值被翻转。没有新问题时不凑数。
若要指出回归由哪个提交或 PR 引入,先用 `git log -S/-G`、`git blame` 和关联记录定位候选,再核对该提交的父提交与实际差异是否改变了相关行为。浅克隆、历史裁剪或缺少父提交导致无法核实时,写明“来源未确认”,不要仅凭 blame、提交标题或时间归因;区分代码作者、PR 作者与合并者。
简化候选还需通过可用性检查:替代方案能在当前依赖、类型和接口下实现;示例覆盖关键分支与错误路径;验证命令能够证明行为保持不变。缺少其中任何一项时,先补充上下文或把它降为待确认问题。
对影响范围较大的变更,找出“该变更安全所依赖的一个关键事实”,并尽量用真实代码、最小脚本或针对性测试证明它。只能从源码推理或无法运行验证时,明确标为待确认,不把未证明的安全假设写成结论。
判断兼容性时,以 PR 目标分支在本次变更前已有的行为、数据和调用方为基线。若接口、配置或功能由本 PR 首次引入,同一 PR 内对它的调整不算破坏性变更,也不要求兼容 PR 的中间版本;按最终快照检查正确性、用法和文档。若调整同时影响基线已有的行为、数据或调用方,仍按实际回归评估。
对可能改变开发者或既有功能使用方式的差异,额外检查环境变量和 secret 读取、端口与网络、必需脚本、构建入口、feature gate 和内部权限条件。研究尚未完成时保留未确认项,不用条件猜测替代对后端、调用方或 PR 讨论的核实。
测试相关差异额外检查:
- 关键主流程、失败路径、边界输入、兼容性、权限和并发/异步状态是否有与风险匹配的验证;
- 新增或修复的 bug 是否有可复现的回归场景,而不是只增加无关覆盖率;
- 测试是否依赖执行顺序、共享状态、未提交事务、固定资源名或外部环境;
- 删除、收缩或改写测试时,原有关键约束是否仍被保留;
- 单元测试使用 mock/fake 时,集成/E2E 是否仍覆盖真实协议、数据和最终状态;
- 测试通过、编译通过或中间资源状态是否被错误地报告成完整运行面验证。
严重度:
- P0:确定的正确性、安全或严重并发缺陷
- P1:健壮性、关键测试缺口或明确性能退化
- P2:有实际维护成本的设计、文档或规范问题
不为凑数量报告问题。默认检查是否存在可删除的抽象、依赖和配置层,但安全校验、错误处理、数据保护、可访问性和用户明确需求不算无效复杂度。
+
+ 审查过程中如果代码被修改或 HEAD 移动,之前的结论作废:重新读取 HEAD、工作区和完整差异,重跑暴露问题的检查,再给出结论。
## 5. 发布评审
只有用户明确要求把结果发布到 GitHub 时:
1. 完成所有适用检查和去重后再发布,避免中途制造多次通知。
2. 使用当前可用的连接器;不支持写入时回退 `gh api`。
3. 只发布一次不阻塞合并的 `COMMENT`,不擅自使用 `REQUEST_CHANGES`。
4. 发布后读回评审,验证目标 PR、正文和事件类型。
5. 没有远端写能力时输出完整评审正文和人工执行说明,不声称已经发布。
## 输出
```text
## 问题
[P0/P1/P2] <标题> - <文件:行> [置信度: 90]
证据: <触发条件、当前代码和影响>
建议: <最小可执行修复>
## 待确认问题
- <缺少的关键证据>
## 可行的简化(存在时才输出)
[P2/建议] <标题> - <文件:行>
复杂度来源: <多余概念、分支、状态、层级或依赖>
替代实现: <适配当前代码的代码片段或具体步骤>
行为证明: <保持的语义与验证方法>
## 摘要
- diff 快照:<range>
- 文件覆盖:<已审查 / 上下文 / 跳过 / 失败>
- 范围与需求:<结论>
- 场景契约:<path / none>; 覆盖:<结论>
- 可拆分性与文档债务:<结论>
- 架构专项:<未执行或基于证据的结论>
- 发布:<未请求 / connector / gh / 未执行>
```
没有高置信问题时明确写“未发现明确问题”,并说明验证和上下文边界。
## 统一约束
- 验收标准:遵循 `../_shared/common-acceptance.md`
- 场景契约只作为审查输入;没有用户授权,不在审查流程中创建或修改它。