code-review

name	code-review
description	Hit 项目代码审查专用 skill。按 TODO.md 阶段对完成代码做结构化审查， 覆盖 Rust 代码质量、安全审计，按标准报告格式输出审查报告。
license	MIT
compatibility	Hit 项目
metadata	{"version":"2.0.0"}
allowed-tools	Bash(cargo:*) Bash(git:*) Read Write Edit Glob Grep
user_invocable	true
disable_model_invocation	false

Code Review — Hit 项目代码审查

按 TODO.md 中指定的 Phase/模块进行结构化代码审查。

---

⚠️ 重要前提：Hit 是 Windows 专用工具，不跨平台。审查时不要提出"非 Windows 平台无法编译/运行"之类的跨平台问题——这不是项目目标。

Rust 代码审查清单

错误处理

• 库代码使用 thiserror 定义错误类型，CLI 代码使用 anyhow 传播错误
• 库代码优先用 Result<T, AppError>，避免 unwrap()/expect()
• CLI 代码可使用 .context() / .with_context()（来自 anyhow）
• CLI 输出给用户的错误消息使用中文

命名规范与风格

• 函数/变量用 snake_case，类型/trait/枚举用 PascalCase
• 模块层次：hit_xxx::submodule::function
• import 分组：标准库 → 外部 crate → 内部 crate → 当前 crate
• 函数尽量控制在 50 行以内

模块接口一致性

• 跨 crate 类型实现 Debug + Clone + PartialEq
• 公开 API 函数必须写文档注释（///）
• 内部函数使用 pub(crate) 而非 pub

性能

• 优先用迭代器而非索引循环
• 并行文件扫描用 rayon::par_iter()
• 避免对大数据结构调用 clone()，优先用引用
• 借用 vs 所有权不确定时用 Cow<'_, str>

测试

• 单元测试写在同文件 #[cfg(test)] mod tests { ... } 内
• 集成测试放在 tests/ 目录
• 测试命名：fn test_<函数名>_<场景>()
• 覆盖错误路径，不止是 happy path

安全审查（整合自 security-reviewer）

• 路径操作使用 canonicalize() 防止路径穿越
• 子进程调用使用参数数组而非拼接字符串（防命令注入）
• 符号链接/junction 操作前校验目标路径合法性
• 注册表操作限制在 HKCU 范围，避免 HKLM（需提权）
• 临时文件使用 tempfile crate 而非手动构造路径
• 下载 URL 校验 scheme 为 https://，防止 protocol confusion

---

工作流程

Step 0：加载安全审查子 skill

在正式开始之前，先加载 security-reviewer skill 获取安全审查清单：

use_skill name: security-reviewer

Step 1：确认审查范围

读取 docs/TODO.md 中对应的 Phase/模块章节（如 ### 1.2 hit-core/manifest）， 确定：

• 哪些任务标注了 ✅（应该完成）
• 哪些任务标注了 📋（不应审查）
• 依赖关系

只审查标注 ✅ 的任务，不审查未完成的任务。

Step 2：运行基线检查

# 编译检查
cargo check --workspace

# 静态分析（视情况加 -D warnings）
cargo clippy --workspace

# 运行对应 crate 的测试
cargo test -p <crate-name>

记录基线结果用于报告开头：

**基线**：`cargo check` ✅ | `cargo test` ✅ (N/N) | `cargo clippy` ⚠️ N warning(s)

Step 3：阅读对应源码

按 TODO.md 任务清单找到对应源文件，通读代码。重点关注：

• 数据建模是否完整覆盖 Scoop 兼容需求
• 多态字段/特殊反序列化是否正确处理
• 错误处理是否使用 HitError 统一错误类型
• 变量替换/路径操作等逻辑是否正确
• 测试是否覆盖真实 Scoop manifest fixture

Step 4：逐条对照审查清单

在阅读代码过程中，逐条对照上方的审查清单：

• 错误处理 — HitError 使用、unwrap 避免、中文错误消息
• 命名规范 — snake_case / PascalCase、import 分组
• 模块接口 — pub/crate 可见性、doc 注释
• 性能 — 迭代器、rayon、clone 避免
• 测试 — 覆盖率、错误路径
• 安全 — 路径穿越、命令注入、符号链接、注册表

Step 5：输出审查报告

按以下标准格式输出审查报告。注意：

• 报告必须包含 ## 📋 用户意见 章节，内容先留空，等待用户填写
• 顶部必须包含免责声明
• 报告写入 docs/notes/review/REVIEW_YYYY-MM-DD[-<phase>].md

---

报告模板

⚠️ 免责声明：以下"逐项审查"、"问题汇总"、"评分总结"等章节仅代表代码审查者的分析意见， 仅供参考，你可以自行评估决定是否接受意见进行修改或进行其他操作。 但是「用户意见」章节的内容是项目所有者明确的决策，必须遵从。

# 代码审查报告 — Phase <N> <模块名>

**审查者**：AtomCode code-review
**时间**：<日期>
**范围**：仅 TODO.md §<章节>（任务 <编号>）
**文件**：<涉及的文件列表>
**基线**：`cargo check` ✅ | `cargo test` ✅ (N/N) | `cargo clippy` ✅

---

## 📋 用户意见（必须遵从）

> 此章节在审查时由项目所有者填写。审查者先留空，等待用户提出具体决策意见。
> 一旦填写，其内容具有最高优先级，必须遵从。

---

## 任务完成清单

| 序号 | 任务 | 状态 | 代码位置 |
|------|------|:----:|----------|
| N.N.N | 任务描述 | ✅ | `src/.../file.rs` |
| ...用表格列出所有该阶段的任务，标注完成状态... |

**结论：N/N 项任务全部完成，可标记 ✅。**

---

## 模块结构总览

\`\`\`
src/<module>/
├── mod.rs    # 模块入口
├── xxx.rs    # 子模块说明
└── ...
tests/
├── test_xxx.rs
└── ...
\`\`\`

---

## 逐模块审查

### <module>.<file> — 模块名 ⭐⭐⭐⭐⭐

**关键设计**：用文字或表格说明设计亮点。

| 评价 | 说明 |
|------|------|
| ✅ 亮点 | 具体说明 |
| ⚠️ 待改进 | 具体问题 |

### <module>.<file> — 模块名 ⭐⭐⭐⭐

...

---

## 测试覆盖分析

| 测试文件 | 数量 | 覆盖重点 |
|---------|:----:|----------|
| `tests/xxx.rs` | N | 覆盖内容 |
| **总计** | **N** | |

---

## 问题汇总

| # | 任务 | 问题 | 严重度 | 建议 |
|---|------|------|--------|------|
| 1 | N.N.N | 问题描述 | 🔴🟡🟢 | 修复建议 |

严重度定义：
- 🔴 阻塞 —— 必须修复才能继续
- 🟡 中等 —— 建议修复
- 🟢 微小 —— 可忽略或后续优化

---

## 评分总结

| 维度 | 评分 | 说明 |
|------|:----:|------|
| **完成度** | ⭐⭐⭐⭐⭐ | N/N 任务完成 |
| **数据建模** | ⭐⭐⭐⭐⭐ | 视情况 |
| **测试覆盖** | ⭐⭐⭐⭐⭐ | 视情况 |
| **代码质量** | ⭐⭐⭐⭐⭐ | 视情况 |
| **架构设计** | ⭐⭐⭐⭐⭐ | 视情况 |

### 整体结论

**Phase <N>（<模块名>）通过审查，可以关闭。**

一段总结性结论。

---

撰写规范

评分规则

分值	含义
⭐⭐⭐⭐⭐	无可挑剔，生产就绪
⭐⭐⭐⭐	良好，有个别轻微问题
⭐⭐⭐	可用，但有多处待改进
⭐⭐	有严重问题需修复
⭐	不可接受，需重写

问题严重度

标记	含义
🔴	阻塞 —— 必须修复才能继续
🟡	中等 —— 建议在本阶段修复
🟢	微小 —— 可忽略或后续优化

审查原则

1. 只审查标注 ✅ 的任务 — 未完成或进行中的任务不审查
2. 以 TODO.md 为准 — 任务描述中的具体需求是判断完成的标准
3. 不审查无关代码 — 只关注该 Phase 范围，不包括其他模块
4. 先运行测试再下结论 — 编译/测试结果是判断代码质量的基础
5. 用数据说话 — 测试数量、覆盖的 fixture 数量、clippy 警告数等量化指标
6. 中文报告 — 报告正文使用中文，代码片段/类型名称保持英文
7. 用户意见留空 — ## 📋 用户意见 章节先留空，仅保留说明文本，等待用户提出
8. 不要提出跨平台问题 — Hit 是 Windows 专用工具，不计划支持 Linux/macOS。涉及 Windows API（winreg、windows-rs、msiexec 等）的代码跨平台问题不视为问题