code-review

name	code-review
description	代码审查助手。用户请求 review 代码、PR 或 commit，或寻找 bug 时使用。以 Spec（需求/规格符合性）与 Standards（项目规范符合性）双轨审查，覆盖 C/C++、嵌入式、BIOS/UEFI、BMC 专项。

Code Review Skill

每条发现须追溯到 Spec（需求/协议/平台规格）或 Standards（项目规范/通用质量）证据；无法证实的行为标注 待确认，未执行的验证保留为 验证缺口——不臆测硬件/协议事实，不虚构命令结果。

分级精度（tiered precision）：当上下文不清、无法证实也无法证伪一个发现时，按严重度分流——🟢 建议（无量化影响的命名/风格/文档）选择不报，不消耗信任；🔴 必须修复与 🟡 强烈建议绝不沉默，标注 待确认 并留验证缺口，交由人类判定。理由：BMC/固件领域沉默可能放过炸硬件的时序/寄存器 bug，精度纪律只下沉到低危层。本规则统摄全文 🔴🟡🟢 分级行为（与“待确认”“验证缺口”“范围外观察”同族，均为“把不确定性显式交给人类”）。

术语衔接：本 skill 新增两个动作性术语，均纳入既有术语体系、不游离——① 证伪自检（falsification self-check，Step 4.5 子步骤）处理 diff 直接反证的确定假阳性，与“待确认”（处理证据不足的不确定项）互补不重叠；② 位置待重锚定（定位失败兜底的发现标注）处理无法精确落点的发现，与“范围外观察”“验证缺口”同属“显式交给人类”族。

Step 0: 锚定审查对象与基线

输入：用户的审查请求（PR/分支/commit/文件/片段/全仓）+ 可用基线　输出：审查模式、固定基线、变更范围证据

区分审查模式，避免把历史遗留、需求猜测和当前变更混在一起。尊重用户指定的审查范围（如仅安全），不强加其他维度。

审查对象	必做动作	审查边界
PR、分支、commit 或工作区改动	要求固定基线；验证 git rev-parse <base>，记录 git diff <base>...HEAD 和 git log <base>..HEAD --oneline，确认 diff 非空	只对该 diff 负责；未改动的遗留问题单列为范围外观察
单个文件、函数或代码片段	明确缺失的调用方、配置、硬件描述或测试上下文	仅基于已提供材料审查，不推断仓库级需求符合性
全仓或模块健康检查	明确扫描路径、时间预算和优先目标	报告中标明这不是增量 PR 审查

🛑 STOP

条件	动作
增量审查未给基线	请求 commit/分支/tag/main；不猜比较对象
基线无效或 diff 为空	报告并停止增量审查；不扩为全仓审查
无 Git 仓库或仅片段	用片段模式，结论标注范围未验证

Step 1: 理解上下文

输入：代码/PR/commit diff（基于 Step 0 基线）+ 上下文描述　输出：审查范围摘要 + 可验证证据来源

读取代码后，列出：① 代码目的与变更范围；② 业务背景与审查目标；③ 语言、技术栈与领域；④ 可验证证据来源（需求/规范/硬件资料/已执行的构建或测试）；⑤ 当 diff 较大或跨文件时（Step 1 读完代码即可判断，不依赖后续 Step 4 的 400 行 gate），列出风险摘要（最可能的风险点位置 + 类别，按 Spec/Standards 自然分流），供 Step 2 双轨证据链建设参考。风险摘要只驱动 🔴🟡，不把 🟢 提级（守分级精度）。

🛑 STOP

条件	动作
代码片段不完整（缺关键函数/文件）	请求补充，标注缺失范围后再继续
无法判断语言/框架	向用户确认技术栈
无明确审查目标	默认全维度（功能→安全→性能→质量→测试→文档→架构）

Step 2: 建立双轨证据

输入：Step 1 上下文摘要 + 已定位的需求/规范来源　输出：Spec 与 Standards 两条独立证据链

检查实现之前，分别建立不可混淆的两条证据链：

• Spec（需求符合性）：按 commit/PR 的 issue 引用、用户提供路径、docs//specs//.scratch/ 需求文档顺序定位来源。BIOS/BMC 变更还要定位对应的协议、平台规格、寄存器定义或硬件勘误。
• Standards（规范符合性）：读取仓库内的 AGENTS.md、CONTRIBUTING.md、编码规范、模块 README、构建规则和静态检查配置；再用 resources/REVIEW-CHECKLIST.md 补足通用检查。
• 优先级（单一来源）：仓库规范及已确认的硬件/协议一手资料 > 通用 checklist > 设计异味启发式。未找到一手资料时，不把硬件行为猜测写成事实，标注 待确认。

🛑 STOP

条件	动作
未找到需求或规格	请求来源；仍不可得则 Spec 轨写“无规格，无法判定需求符合性”，继续做实现质量审查
硬件行为只能依赖猜测	标注 待确认，说明需要的协议/平台/数据表资料
规范相互矛盾或过时	引用冲突来源，取当前模块或项目根目录规范，结论标 待确认

Step 3: 语言/领域专项激活

输入：Step 1 检测到的语言/领域　输出：激活的专项检查清单，或回退通用审查清单

根据 Step 1 检测到的语言和领域，激活对应专项：

语言/领域	专项检查项
C/C++	指针安全（野指针、悬垂、use-after-free）、内存管理（malloc/free 配对、buffer overflow、double-free）、undefined behavior、volatile 正确使用、栈溢出风险；C++ 额外：RAII、智能指针、移动语义、rule-of-five、内存对齐、ABI 兼容性；静态分析：Cppcheck、Clang-Tidy
嵌入式通用	中断安全（ISR 中禁止阻塞调用）、volatile 与 MMIO 正确使用、硬件寄存器访问（原子性、位操作）、内存约束（栈大小、堆碎片）、bare-metal 并发、看门狗喂狗、功耗
BIOS/UEFI	ACPI 表长/校验和与 SMBIOS Type 规范、UEFI Protocol 契约、硬件初始化顺序、SEC/PEI/DXE/TSL 阶段边界和 HOB 生命周期、S3 恢复、Secure Boot/认证变量、SMM 输入验证与 SMRAM 边界
BMC	IPMI 请求长度/权限/完成码、传感器转换/阈值/滞回、Watchdog 失效路径、Redfish Schema/权限/错误映射、D-Bus 异步对象生命周期、systemd 服务状态、I2C/SMBus 超时重试与总线并发、主机电源/复位的安全状态
Python	类型安全、GIL 影响、依赖安全、duck typing 边界
Java/Go/Rust	各语言惯用模式和常见陷阱

激活 BIOS/UEFI、BMC 或嵌入式专项时，按 resources/FIRMWARE-CHECKS.md 检查原始硬件数据解码、通信长度/TOCTOU、时序结论，并将每条结论绑定规格证据；该文件含“BIOS/BMC 与双轨交叉验证”段。未匹配任何专项则用 resources/REVIEW-CHECKLIST.md（回退通用审查清单）。

🛑 STOP

条件	动作
代码涉及不熟悉的硬件架构/芯片手册	坦诚说明局限，审查逻辑与代码质量；硬件特定行为标 待确认
依赖特定 SDK/BSP 且无法获取文档	标注受审查影响范围，跳过 SDK 内部逻辑

Step 4: 结构化审查

输入：Step 2 双轨证据链 + Step 1 审查范围摘要 + Step 3 激活专项清单　输出：🔴/🟡/🟢 分级的发现列表

先分别完成 Spec 与 Standards 两条 pass（子代理可用时并行；不可用时两个独立 pass 并在报告中标明），再汇总发现；两条证据链不得相互替代。逐项过检 resources/REVIEW-CHECKLIST.md。审查须落到逻辑、需求与跨层行为，不做纯格式调整；对看似非安全敏感的代码也查输入验证与数据处理。记录已运行工具的覆盖，人工注意力留给语义、需求与跨层行为。

优先级	维度	说明
🔴 必须修复	功能正确性、安全性、严重 Bug	代码未实现预期功能 / 存在注入漏洞 / 逻辑导致崩溃
🟡 强烈建议	性能、测试覆盖、错误处理、架构	有可量化负面影响：N+1 查询（QPS 下降 >10%）、缺单元测试（覆盖率 <80%）、异常被吞（静默失败）、循环依赖（编译/加载失败）
🟢 建议	代码质量、文档、最佳实践	无量化影响但影响可维护性：命名不清 / 缺注释 / 风格不一致。上下文不清时，对 🟢 选择不报而非报错估（分级精度：🟢 沉默优先，避免消耗信任）

每个发现标注：优先级、类别（Spec/Standards/安全/平台）、行号或函数、证据来源、影响、修复方向。🔴 项必须附完整修复示例或明确替代方案，并说明触发条件或复现路径。只记录有具体证据的优点；没有则省略，不得为满足格式虚构正向结论。能运行验证时，优先执行与改动相符的最窄构建、静态分析、单元测试、QEMU 或硬件验证；无法执行时记为 验证缺口。

定位失败兜底：当一个发现的位置无法锚定到具体行/函数（如引用的代码已改动、跨文件推断无法精确落点），不丢弃该发现，降级处理：在发现标注“位置待重锚定”，或移入“范围外观察”；不得因定位失败而沉默。理由：BMC/固件领域定位漂移的发现仍有价值，需人类复核而非丢弃（与“待确认”“验证缺口”同族——显式交给人类）。

🛑 STOP

条件	动作
PR/diff 超 400 行	建议拆分审查范围（按模块/功能分批）或仅审核心逻辑
单次反馈超 30 条	分两批输出：先 🔴+🟡，再 🟢
代码无法编译	标注“基于静态分析，运行时未验证”，优先修复编译错误

对 BIOS/BMC 改动，将专项检查与 Spec/Standards 交叉验证（见 resources/FIRMWARE-CHECKS.md 末段）。

Step 4.5: 证伪自检（仅 🔴🟡）

输入：Step 4 的 🔴🟡 发现列表　输出：证伪后的发现列表 + “验证状态”段记录

对每条 🔴🟡 发现，检查 diff 本身是否提供直接反证（与发现的核心断言矛盾）。判定遵循证伪不证实——只在 diff 内有直接反证时，将发现自否决降级或移除；diff 之外的信息不可证伪（即使可疑也放行，交由分级精度的“待确认”处理）。🟢 建议不做证伪自检。在报告“验证状态”段如实记录“证伪自检：移除/降级 N 条（自否决理由）”，不静默删除。已知限制：跨文件发现的反证多在 diff 外，自检对它们基本失效（Q6 定位失败兜底已覆盖这类漂流发现）。

Step 5: 最窄验证路径

输入：Step 4.5 证伪后保留的 🔴/🟡 发现列表　输出：每项发现的验证计划（证据来源、最窄动作、可证实行为、状态、限制）

对每个 🔴/🟡 发现制定验证计划。先检查变更目录及父目录中的现有构建文件、测试、CI 配置和 compile_commands.json；项目已有命令优先于通用工具名。每项验证写明 证据来源、最窄动作、能证实的行为、状态（已执行/未执行）、限制。

发现类型	选择顺序与边界
可由现有测试复现的行为	先执行覆盖变更的单个测试文件/用例/target；未发现定向测试时记录该缺口，不以全量构建替代行为验证
编译、类型或内存边界	先从构建元数据或 CI 配置发现包含变更文件的 target；未发现 target 时状态记“需发现 target”，可列读取构建元数据的动作（如 Meson target 列表），不得列 ninja/make/meson compile 或构造的对象文件/target 命令；发现 target 后才可列对应构建命令。compile_commands.json 只提供编译上下文，不证明某静态分析器可用——仅当项目配置或当前环境确认分析器后才提出其命令。静态检查通过 ≠ 协议/状态机/硬件行为正确
BMC D-Bus、Redfish 或主机状态流转	镜像和模型已存在时，QEMU 可验证服务启动、对象状态和接口流转；没有时标为未执行。QEMU 不能证明物理 I2C/SMBus/GPIO 时序、仲裁或异常恢复
BIOS/UEFI/SMM 边界	构建、单元或 host-based 测试只能验证可达路径和输入处理；不得据此宣称 SMRAM 隔离或芯片级 SMI 锁定安全，仍需平台安全审计与物理机专向验证

无法执行时，写明缺失的命令、镜像、模型、工具链、测试或硬件条件，保留为 验证缺口。

Step 6: 输出审查报告

输入：Step 2 双轨证据链 + Step 4 发现列表 + Step 5 验证计划　输出：结构化审查报告（Markdown）

## 代码审查报告 → [文件名/模块名]

### 发现摘要
[按严重度列出数量；没有发现时明确说明“未发现可操作问题”，并保留测试或证据缺口]

### 🔴 必须修复（N 项）
1. **[安全][文件:L42]** buffer overflow：`memcpy` 目标缓冲区仅 64 字节，源数据可达 256 字节
   - 证据：`sizeof(buf)` 与 `src_len` 未比较
   - 影响：可覆盖栈数据并导致固件异常或安全边界失效
   - 修复：`memcpy(buf, src, min(src_len, sizeof(buf)))`

### 🟡 强烈建议（N 项）
1. **[Spec][文件:L78-82]** 传感器轮询间隔与平台规格不一致
   - 证据：平台规格 [来源] 要求的间隔为 [值]
   - 修复：从平台配置读取并为超时路径增加验证

### 🟢 建议（N 项）
1. **[Standards][文件:L15]** 函数名 `read_data` 未表达传感器和单位
   - 修复：改为 `read_sensor_value`

### Spec 结论
- 已核对的需求/协议来源：[路径或链接]
- 缺失、错误或超出需求范围的行为：[发现编号或“无”]
- 无法判定项及原因：[来源缺失、硬件资料待确认等]

### Standards 与平台专项结论
- 已核对的项目规则和专项资料：[路径或链接]
- C/嵌入式/BIOS/BMC 专项发现：[发现编号或“无”]

### 验证状态
- [已执行/未执行] [最窄动作或命令]：证据来源、可证实行为、结果或缺失条件
- 证伪自检：[移除/降级 N 条 🔴🟡 发现，理由：diff 直接反证 — <简述>；未执行则填“无”]
- 验证边界：[不能由当前构建、静态分析、QEMU 或硬件动作证明的事项]

### 下一步
1. 先修复所有 🔴 项并执行对应验证
2. 再处理 🟡 项
3. 提交后以相同基线重新审查

措辞：反馈聚焦代码与证据，不针对作者；不确定的发现用提问式（“此处是否意图 X？”）而非断言。撰写措辞参考 resources/FEEDBACK-GUIDELINES.md。

端到端示例（sensor_read.c，仅 trace 双轨 + 最窄路径）

Step 2 双轨：

• Spec：docs/sensors.md 规定 I2C 读取失败时 Available=false 且不发旧读数，成功读数单位摄氏度，周期 ≥1000ms。
• Standards：项目约定负 errno 须转明确错误路径；D-Bus 更新失败须记录并传播。

Step 5 最窄路径：

• 已执行：pytest tests/test_temp_sensor.py（覆盖 I2C 成功/负 errno/D-Bus 失败）→ 证实错误路径与 Available 状态。
• 未执行：受影响 target 未从 meson.build 确认前不列构建命令；无 QEMU 镜像，物理 I2C 时序留为硬件验证边界。

（发现本身按 Step 4 分级、按 Step 6 模板输出，此处不重复。）

反模式速查（❌ → ✅）

每条反模式都配正确做法；审查时按 ✅ 列执行，❌ 列仅作识别。

❌ 反模式	✅ 正确做法
只写“LGTM / 看起来没问题”就结束	产出分级发现；无可操作问题时明确说明并保留证据缺口
只批评不给修复方向	🔴 项附完整修复示例 + 触发条件/复现路径
对不熟悉领域假装权威	坦诚标注局限；硬件/协议行为标 待确认
非安全敏感代码跳过输入验证	对所有变更查输入验证与数据处理
照抄代码做纯格式化审查	审查落到逻辑、需求与跨层行为
重复报告已运行工具拦截的问题	记录工具覆盖；人工注意力留给语义/需求/跨层行为
把未证实的寄存器/时序写成事实	标 待确认，说明需要的资料
用通用 checklist 推翻项目规范/EDK2/MISRA/硬件资料	以仓库规范与一手资料为准
把删除的代码当审查目标去报问题	删除的代码仅作参考上下文，不对其发表评论（吸收 OCR）
对未改动或正确代码发表评论	未改动代码不评论，审查聚焦新增/修改行（吸收 OCR）

Resources

• resources/REVIEW-CHECKLIST.md（Step 2/4 使用）：8 大类检查项 + 通用设计异味。
• resources/FIRMWARE-CHECKS.md（Step 3 激活固件专项时使用）：原始数据解码、通信长度/TOCTOU、时序结论、BIOS/BMC 双轨交叉验证。
• resources/FEEDBACK-GUIDELINES.md（Step 6 使用）：反馈原则、反馈模式、常见反馈场景措辞示例。