code-review

/code-review — MCP 驱动的代码审查

内容

结合 Roslyn MCP 分析和结构化人工审查，进行多维度代码审查。审查精力遵循 80/20 规则：对造成 80% 事故的 20% 代码（数据访问、安全性、并发、集成边界）进行彻底审查；风格和格式问题交由工具处理。

审查维度：正确性（逻辑、边界情况、空值处理、异步陷阱）、安全性（身份验证缺口、注入、机密信息、CORS）、性能（N+1、内存分配、缺少取消机制）、架构合规性（层级违规、边界越界）、测试覆盖率（针对变更类型的行为测试）。

适用场景

• “审查一下这个”“代码审查”“PR 审查”，或合并拉取请求前
• 重大重构后，检查是否存在回归或设计偏离
• “我应该审查什么？”——决定大型变更中审查精力的分配
• 熟悉不熟悉的代码并希望评估其质量时

操作方式

第 1 步：确定范围并评估影响范围

识别变更文件（git diff main...HEAD、指定文件或模块）。 为每项变更评分，以确定审查深度——审查深度由影响范围决定，而非代码行数。一行的中间件变更，其优先级高于 300 行的重命名。

影响范围	示例	深度
关键	中间件、身份验证、数据库迁移、共享内核、CI/CD	彻底——检查每条代码路径
高	公共 API 变更、消息使用者、EF 配置、新模块	聚焦——检查使用方和行为
中	遵循现有模式的新功能、错误修复、新端点	标准——逐项检查清单
低	文档、格式调整、重命名、日志语句	快速浏览——确保构建和测试通过

第 2 步：MCP 分析（读取任何文件前）

detect_antipatterns(projectFilter: "affected-project")   → async void, DateTime.Now, new HttpClient(), broad catch
get_diagnostics(scope: "project", path: "affected-project") → new warnings, nullability issues

区分新引入的问题和已有问题——重点关注新问题。

第 3 步：验证影响范围

对每个已修改的公共 API：

find_references(symbolName: "ModifiedType")              → count consumers; high count = high risk
get_dependency_graph(symbolName: "ModifiedMethod", depth: 2) → ripple effects

检查调用方是否处理了变更后的返回类型和新增错误情况。

第 4 步：架构合规性

通过 get_project_graph 和 detect_circular_dependencies 验证依赖方向（Domain → 无依赖；Infrastructure → Application → Domain）。根据架构要求：VSA 功能之间不能交叉引用；Clean Architecture 的 Domain 不得引用任何项目；Modular Monolith 模块只能通过集成事件通信——对某个模块的 DbContext 使用 find_references，结果应仅限于该模块内部。

第 5 步：人工审查——优先级顺序

审查工具无法发现的问题，先检查风险最高的领域：

优先级	领域	检查项
1	数据访问	N+1（缺少 Include/投影）、包含用户输入的原始 SQL、缺少 CancellationToken
2	安全性	每个端点都有明确的 [Authorize]/[AllowAnonymous]，输入经过验证，代码中不含机密信息，日志中不含 PII
3	并发	令牌端到端传递，不使用 .Result/.Wait()，共享状态具备线程安全性
4	集成	外部调用具备重试/超时机制，使用者具备幂等性，不吞掉异常
5	正确性	业务逻辑、边界情况（空值/null/并发）、实体在边界处映射为 DTO
6	测试	测试行为（而非实现）；覆盖正常路径和主要错误情况
—	风格/命名	仅在上述问题之后提及；格式化工具和分析器负责处理

第 6 步：生成审查结果

每条发现都应说明问题所在、重要性以及修复方法。绝不能让安全漏洞淹没在命名方面的小问题中。

## Code Review: [Scope]

### Summary
[1-3 sentences: scope, risk level, recommendation]

### Critical (must fix before merge)
- **[Title]** — [file:line] [What's wrong. Why it matters. How to fix.]

### Warnings (should fix, creates tech debt)
- **[Title]** — [file:line] [...]

### Suggestions (nice to have)
- **[Title]** — [file:line] [...]

### Architecture Compliance
[PASS/WARN with boundary-violation notes]

### Test Coverage
[Which changed types have tests; specific scenarios to add]

### What's Good
- [Always include — reinforce good patterns]

快速审查（1–2 个文件、影响范围低）：运行 detect_antipatterns 和 get_diagnostics，检查正确性，并输出 Summary + Issues + What's Good。

示例

User: /code-review the changes in this PR

Claude: 7 changed files across 3 projects. CreateOrder touches data access
and a public endpoint — High blast radius. Running MCP analysis...

## Code Review: Order Processing Feature

### Summary
Adds CreateOrder/GetOrder endpoints with EF Core persistence. Well-structured
VSA feature. Two issues need attention before merge.

### Critical (must fix before merge)
- **Missing CancellationToken propagation** — CreateOrder.cs:38
  SaveChangesAsync() called without the token. Client disconnects keep
  burning server resources. Pass `ct` from the handler parameter.

### Warnings (should fix, creates tech debt)
- **N+1 query in GetOrder** — GetOrder.cs:25
  Order loaded without `.Include(o => o.Items)`; one lazy load per item
  during serialization. Eager-load or use a projection.

### Suggestions (nice to have)
- **Seal the handler** — CreateOrderHandler.cs:10
  Not designed for inheritance; `sealed` enables devirtualization.

### Architecture Compliance
PASS — all changes within Features/Orders/, no layer violations.

### Test Coverage
Happy path covered. Add tests for validation failure and not-found.

### What's Good
- Clean command/query separation; FluentValidation covers edge cases
- Response DTOs are records, no entity leaks

相关内容

• /de-sloppify — 清理审查中跳过的风格/格式问题
• /verify — 自动化验证流水线（与人工审查互为补充）
• /health-check — 对整个项目进行更全面的健康状况评估，而非只评估单个 PR