code-review

/code-review — MCP によるコードレビュー

内容

Roslyn MCP による分析と、体系的な手動レビューを組み合わせて、多角的なコードレビューを実施します。作業量は 80/20 の法則に従います。インシデントの 80% の原因となるコードの 20%（データアクセス、セキュリティ、並行処理、統合境界）を徹底的にレビューし、スタイルやフォーマットはツールに任せます。

レビューの観点：正確性（ロジック、エッジケース、null の処理、非同期処理の落とし穴）、セキュリティ（認証の不備、インジェクション、シークレット、CORS）、パフォーマンス（N+1、メモリ割り当て、キャンセル処理の欠落）、アーキテクチャへの準拠（レイヤー違反、境界の侵害）、テストカバレッジ（変更した型の振る舞いテスト）。

タイミング

• 「これをレビューして」「コードレビュー」「PR レビュー」、プルリクエストのマージ前
• 大規模なリファクタリングの後、リグレッションや設計の逸脱がないことを確認したいとき
• 「何をレビューすればよいか」—大きな変更で、どこにレビューの労力をかけるか判断するとき
• 不慣れなコードに参加し、品質評価を行いたいとき

方法

ステップ 1：対象範囲と影響範囲の評価

変更されたファイル（git diff main...HEAD、指定されたファイル、またはモジュール）を特定します。各変更のレビュー深度を決めるために、影響範囲を評価します。深度を決めるのは行数ではなく、影響範囲です。1 行のミドルウェア変更が、300 行の名前変更より優先されます。

影響範囲	例	深度
重大	ミドルウェア、認証、DB マイグレーション、共有カーネル、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 のドメインにプロジェクト参照がないこと、モジュラーモノリスのモジュール間通信は統合イベント経由に限ること — モジュールの 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 を実行し、正確性を確認して、概要 + 問題点 + 良い点を出力します。

例

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 にとどまらない、プロジェクト全体の健全性評価