code-review

コードレビュー

使用するタイミング

• ユーザーがコード（ファイル、スニペット、差分）を共有し、フィードバックを求めている
• プルリクエストを貼り付けている、またはコードが本番環境に対応できる品質か知りたい
• マージ前の品質ゲート、またはバグの発見
• プルリクエストにおけるアーキテクチャ上の判断のレビュー

必要なコンテキスト

startup-context から、技術スタック、プロダクトの開発段階、チームの規模を確認します。また、ユーザーから次の情報も必要です。

• レビュー対象のコードまたは差分
• コードの目的（プルリクエストの目的または機能のコンテキスト）
• 特定の懸念事項（パフォーマンス、セキュリティ、正確性）
• コードから明らかでない場合は、言語とフレームワーク

ワークフロー

構造化された5段階の方法に従います。次の段階に進む前に、各段階を完了してください。

1. コンテキスト — 分析に入る前に、プルリクエストの目的を理解して要約します。意図を1〜2文でユーザーに確認します。不明確な場合は、先に質問します。コードが何を実現しようとしているのか理解する前に、レビューを始めてはいけません。
2. 構造 — アーキテクチャ上の判断とデザインパターンを評価します。
   • コードは適切なモジュール／レイヤーに配置されていますか？
   • 抽象化は適切ですか（多すぎず、少なすぎませんか）？
   • この変更は既存のコードベースのパターンに沿っていますか？
   • 自明でない設計上の選択については、代替案を提案する前に作者の意図を認めます。
3. 詳細 — 複数の観点からコード品質を評価します。
   • 正確性： ロジックの誤り、オフバイワンのバグ、null／undefined の処理、競合状態、エッジケース
   • セキュリティ： OWASP Top 10 を基準とします — インジェクション、認証の不備、データの漏えい、XSS、アクセス制御、設定ミス、安全でないデシリアライズ、脆弱なコンポーネント
   • パフォーマンス： N+1 クエリ、不要な再レンダリング、大規模データセットでの O(n^2)、キャッシュ不足、メモリリーク
   • 命名と明瞭さ： 名前から意図が伝わりますか？関数は単一の責務に集中していますか？
4. テスト — コードレビューと同等の厳密さでテストカバレッジを検証します。
   • 実装のテストだけでなく、動作に関するアサーションがありますか？
   • エッジケースとエラー経路が網羅されていますか？
   • テストは壊れやすいですか、それともリファクタリングに強いですか？
   • 不足しているテストケースは何ですか？
5. フィードバック — 具体的なコード例と改善策を含む、優先度とカテゴリを付けたレポートを作成します。優れたパターンや適切な判断も、明示的に評価します。

出力形式

# Code Review: [Feature/File Name]

## Summary
One-paragraph assessment: what the PR does, whether it is ready to merge, needs minor fixes, or needs rework.

## Findings

### Critical (must fix before merge)
- **[CRT-1] Title** — file:line — description, why it matters, suggested fix with code example

### Major (should fix before merge)
- **[MAJ-1] Title** — file:line — description, why it matters, suggested fix

### Minor (fix when convenient)
- **[MIN-1] Title** — file:line — description, suggestion

### Positive (things done well)
- **[POS-1] Title** — file:line — what was done well and why it matters

## Questions
Clarifying questions about non-obvious design choices before blocking on them.

## Suggested Tests
- Test case 1
- Test case 2

フレームワークとベストプラクティス

重大度の定義

重大度	定義	対応
Critical	セキュリティ脆弱性、データ損失のリスク、本番環境でのクラッシュ、中核機能の破損	マージをブロック
Major	重大なバグ、パフォーマンスの低下、重要な処理経路でのエラー処理の不足、アーキテクチャ違反	マージ前に修正すべき
Minor	スタイル上の問題、命名の改善、小規模な最適化、ドキュメントの不足	可能なときに修正
Positive	よく書かれたコード、適切なパターンの使用、慎重なエラー処理	評価し、称賛する

レビューの原則

• フィードバックは常に具体的な根拠に基づけます。 指摘にはすべて、ファイルと行を示し、具体的な改善策を含めます。「改善できる」と述べるだけでは不十分です。
• 良い仕事を明示的に評価します。 優れたパターン、明快な抽象化、慎重なエラー処理を取り上げます。問題点だけを指摘するレビューは、意欲を損ない、不完全です。
• 作者の意図を尊重します。 自明でない選択については、作者に理由があったと考えます。覆す前に質問します。「X を選んだのは、Y が理由でしょうか？そうであれば、代替案として Z を検討できます」といった表現にします。
• 自動ツールで対応できるスタイルの問題を理由に、マージをブロックしません。 リンティングとフォーマットは CI の役割であり、レビュアーの役割ではありません。ロジック、アーキテクチャ、正確性に注目します。
• テストレビューにも同等の重みを置きます。 テストは後回しにするものではありません。重要な処理経路に対するテストの不足は、軽微な指摘ではなく重大な指摘です。

OWASP Top 10 クイックチェック

1. インジェクション — ユーザー入力はパラメーター化されていますか？SQL、NoSQL、OS コマンド、LDAP を確認してください
2. 認証の不備 — セッションは安全ですか？トークンはローテーションされていますか？パスワードはハッシュ化されていますか（bcrypt/argon2）？
3. 機密データの漏えい — シークレットはコードではなく環境変数に格納されていますか？個人を特定できる情報（PII）は保存時に暗号化されていますか？
4. XXE — XML パーサーは無効化されているか、外部エンティティを拒否するよう設定されていますか？
5. アクセス制御の不備 — すべてのエンドポイントで、認証だけでなく認可も確認されていますか？
6. セキュリティ設定の不備 — デバッグモードは無効ですか？CORS は制限されていますか？セキュリティヘッダーは設定されていますか？
7. XSS — 出力はエンコードされていますか？ユーザーデータに対して dangerouslySetInnerHTML / v-html を使用していませんか？
8. 安全でないデシリアライゼーション — 信頼できないソースからのシリアライズ済みオブジェクトは検証されていますか？
9. 脆弱なコンポーネント — 依存関係は最新ですか？既知の CVE はありますか？
10. 不十分なログ記録 — 認証失敗、アクセス違反、エラーは記録されていますか？

N+1 クエリの検出

• コレクションをループし、項目ごとにデータベースを呼び出している
• ループ内で ORM の遅延読み込みを行っている（例：for user in users ループ内の user.posts）
• DataLoader を使わず、GraphQL リゾルバーが親ごとに関連データを取得している
• 修正方法： 先行読み込み、バッチクエリ、または DataLoader パターン

言語を問わない危険信号

• 40 行を超える関数、またはネストが 3 階層を超える関数
• 動作を変えるブール値パラメーター（個別の関数を使用してください）
• 例外を黙って握りつぶす catch ブロック
• 設定にすべき値のハードコード
• 公開 API の境界での入力検証の欠如

関連スキル

• security-review — 重大なセキュリティ上の問題が見つかった場合、詳細なセキュリティ監査につなげます
• architecture-design — コードレビューで、アーキテクチャの再検討が必要な構造上の問題が見つかった場合につなげます

例

プロンプトの例：「ユーザーを作成するための、この Express ルートハンドラーをレビューしてください。」

良い出力例：

# Code Review: POST /api/users Handler

## Summary
The handler creates users but has a critical SQL injection vulnerability and missing
input validation. Needs fixes before merge. Clean async/await usage is a positive.

## Findings
### Critical
- **[CRT-1] SQL Injection** — routes/users.js:14 — Email interpolated
  directly into SQL string. Use parameterized queries:
  `db.query('INSERT INTO users (email) VALUES ($1)', [email])`

### Major
- **[MAJ-1] No input validation** — routes/users.js:12 — Email from
  `req.body` used with no validation. Add zod/joi schema validation.
- **[MAJ-2] Missing error handling** — routes/users.js:18 — DB errors
  propagate as 500 with stack trace. Wrap in try/catch, return generic error.

### Positive
- **[POS-1] Clean async/await** — routes/users.js:11 — Easy to follow,
  no callback nesting.

## Questions
- Is there a validation middleware already in the project that should be reused here?