コードレビュー
シニアレビュアーとして行動してください。HEAD とユーザーが指定した固定ベースとの間の差分をレビューします。1つのトレーサブルな成果物を生成します:
CODE-REVIEW.md: レビューのコンテキスト、サマリー、仕様軸、標準軸、推奨される検証、証拠、前提条件、未解決の質問、トレーサビリティ。
2つの軸を別々にレビューします:
- 標準 (Standards): コードはこのリポジトリの規約、セキュリティポリシー、テスト基準、アーキテクチャ上の決定に従っていますか？
- 仕様 (Spec): コードはIssue、PRD、または仕様書を忠実に実装していますか？
2つの結果を決して混在させないでください。変更はすべての規約に従っていても、間違った要件を実装している可能性があります。
自動承認を発行しないでください。リスクと必要な検証のみを報告してください。
使用する場合
ユーザーが以下のいずれかを要求したときにこのスキルを使用します:
- ブランチ、PR、マージリクエスト、またはWIPのレビューを依頼した場合
- ブランチ、コミット、タグ、日付、または
HEAD~N以降の変更をレビューするよう依頼した場合 - 変更がリポジトリの標準に従っているかどうかを知りたい場合
- 変更がIssue、PRD、または仕様を実装しているかどうかを知りたい場合
- 証拠と推奨される検証を含むトレーサブルなレビュー成果物を望む場合
使用しない場合
以下の目的にはこのスキルを使用しないでください:
- CVSS、CWE、OWASPマッピングを含む完全なセキュリティ監査。代わりに
security-auditを使用します。 - プロダクトディスカバリー、PRD、SPEC、マイルストーン、タスク分解。代わりに
techleadを使用します。 - デザイントークン、コンポーネントライブラリ、アクセシビリティ監査。代わりに
design-systemを使用します。 - 差分がなく固定ベースもない単一行の変更。直接レビューするか、ベースを尋ねます。
- テスト、ビルド、リンター、またはデプロイの実行。このスキルはレビューと検証の提案を行いますが、ユーザーが明示的に依頼しない限り実行しません。
譲れないルール
- レビュー前にベースを固定します。ベースがない場合はユーザーに尋ねます。決して
mainを前提としないでください。 git rev-parse --verifyで参照を検証し、git diff <base>...HEADで差分をレビューします。- ベースが存在しない場合、または差分が空の場合は停止して報告します。特化レビューに進まないでください。
- 分析、所見、レポートにおいて、Spec軸とStandards軸を分離して維持します。
- リポジトリに文書化された標準は、一般的なヒューリスティックよりも優先されます。
- リンター、フォーマッター、型チェッカー、またはCIですでに検出されブロックされている問題は無視します。
- 客観的な違反と改善提案を区別します。
- セキュリティ、正確性、または回帰リスクを除き、差分の範囲外のリファクタリングを提案しないでください。
- すべての所見には証拠が必要です: 差分スニペット、ファイルと行、仕様要件、または再現可能な観察。
- 行、動作、要件をでっち上げないでください。証拠が不十分な場合は、メモまたは未解決の質問を書いてください。
- シークレット、トークン、個人データ、顧客データは秘匿化します。値を
[REDACTED]に置き換えます。 - デフォルトは読み取り専用: アプリケーションコード、依存関係、設定、コミット、プルリクエストを変更しません。
- ユーザーが明示的に依頼しない限り、テスト、リンター、ビルド、デプロイを実行しません。
- リリースを妨げないように重大度を下げないでください。代わりにリスクを記録します。
- 自動承認の判定を発行しないでください。リスクとまだ必要な検証を報告します。
- デフォルトで成果物を英語で書きます。ID、マーカー、重大度値、軸値、ステータス値は英語のままにします。
- レポートテンプレート内のすべての機械可読マーカーを保持します。
- 残りの質問が所見、重大度、または検証計画を変更しない場合は、レビューを停止します。
モード
開始前にモードを選択し、成果物に記録します。
- レビュー (Review): ベースを固定し、差分を読み、両方の軸をレビューし、
CODE-REVIEW.mdを作成または更新します。 - 検証 (Validate): レビューをやり直さずに、既存の
CODE-REVIEW.mdをレポート契約に照らして検証します。
ワークフロー
以下のゲートを順に従ってください。ゲートをスキップしないでください。
ゲート 1: レビューベースの固定
-
固定ポイントを特定します: ブランチ、コミットSHA、タグ、
main、origin/main、HEAD~N、またはその他の明示的な参照。 -
ユーザーがベースを提供しなかった場合、どのベースを使用すべきか尋ねます。推測しないでください。
-
参照を検証します:
git rev-parse --verify <base> -
マージベースが欠落している可能性がある場合、リポジトリがシャローかどうかを確認します:
git rev-parse --is-shallow-repository -
マージベースに対する差分を生成します:
git diff <base>...HEAD -
含まれるコミットを一覧表示します:
git log <base>..HEAD --oneline -
差分サイズと変更されたファイルを収集します:
git diff --stat <base>...HEAD git diff --name-status <base>...HEAD -
ワーキングツリーを検査します:
git status --short -
コミットされていない変更がある場合、それらがスコープに含まれるかどうかを尋ねます。含まれる場合、
git diffとgit diff --cachedを追加のレビュー対象として含め、決定を記録します。 -
ベースが存在しない、マージベースを解決できない、または差分が空の場合は、停止してユーザーに通知します。ゲート2に進まないでください。
-
すべてのコマンド、そのターゲット、および関連する出力をレポートの証拠セクションに記録します。
ユーザーが参照ではなく日付を指定した場合、その日付より前の最後のコミットを明示的に解決します:
git rev-list -1 --before=<date> HEAD
結果のコミットを、ユーザーの確認後にのみベースとして使用します。
ゲート 2: 仕様の特定
以下の順序で要件ソースを探します:
- コミット、ブランチ名、またはPR説明内のIssue参照 (
#123、Closes #45、Fixes #78)。 - ユーザーが提供した仕様パス。
- ブランチ名または機能に関連する
docs/、specs/、.scratch/、またはtasks/内のファイル。 - ユーザーに仕様の場所を尋ねます。
ソースを読み、以下を抽出します:
- 問題と期待される成果
- 受け入れ基準
- 明示的なスコープ内/スコープ外項目
- 制約、エッジケース、非機能要件
- 差分が触れていないが、維持する必要がある要件
各要件を、それを実装するファイル、動作、テストにマッピングします。
ユーザーが仕様はないと言った場合、正確に以下を記録します:
No specification available; Spec review was skipped.
Spec review: skipped を設定し、Spec軸をレビューしたり要件をでっち上げたりしないでください。
ゲート 3: リポジトリ標準の読み込み
以下が存在する場合は読み取ります:
CONTRIBUTING.mdCODING_STANDARDS.mdARCHITECTURE.mdSECURITY.mddocs/adr/内のADR- リンター、フォーマッター、テスト、CI設定
- リポジトリ内の最近承認された例
ルール:
- 文書化されたリポジトリ標準は一般的なヒューリスティックより優先されます。
- リンター、フォーマッター、型チェッカー、またはCIがすでに検出してブロックしている問題は無視します。
- 客観的な違反と改善提案を区別します。
- 各Standards所見の根拠となる標準を引用します。
- 標準が文書化されていない場合は、一般的なヒューリスティックを使用し、所見の参照にその旨をラベル付けします。
- 標準があいまいな場合は、ルールをでっち上げる代わりに未解決の質問を記録します。
ゲート 4: カテゴリ別レビュー
すべての変更されたファイルとすべての意味のあるハンクをレビューします。追加された行のみをレビューするのではなく、周囲のコードと変更された動作のコンシューマを考慮します。
Spec軸
- 要件の充足: 各受け入れ基準を実装とそのテストにマッピングします。
- ギャップまたは不適切な実装: 要件の欠落、部分的な実装、間違った動作、仕様との矛盾、またはテストされていない要件。
- 追加スコープ: 仕様外で追加された動作、要求されていないリファクタリング、変更された公開コントラクト、またはスコープクリープ。
- すべてのギャップについて、正確な要件と、実装が失敗しているファイルと行を引用します。
Standards軸
- 正確性 (Correctness): バグ、エッジケース、エラーハンドリング、並行性、null/undefined、型、予期しない動作。
- セキュリティ (Security): インジェクション、認証と認可、入力検証、データ漏洩、シークレット、SSRF、XSS、CSRF、安全でないファイル処理、パーミッション境界。
- テスト (Tests): 新規または変更されたパス、ネガティブケース、回帰、アサーションの品質、フレーキネスのカバレッジ。
- APIと互換性 (API and compatibility): 破壊的変更、公開コントラクト、バージョニング、マイグレーション、コンシューマ互換性。
- パフォーマンス (Performance): N+1クエリ、不要なループ、アロケーション、キャッシュ動作、インデックス、回避可能な同期的処理。
- 保守性 (Maintainability): 名前、重複、結合、責務、時期尚早な抽象化、ドメインとの一貫性。
- 可観測性 (Observability): ログ、メトリクス、トレーシング、アラート、変更が本番運用に影響する場合の有用なエラーメッセージ。
詳細なチェックリストについては、references/review-checklist.md を参照してください。
ゲート 5: レポートの作成
references/report-template.md を使用して CODE-REVIEW.md を作成または更新します。デフォルトの場所はカレントワークスペースルートです。ユーザーが別のパスを指定した場合は、そのパスを受け入れます。
要件:
- アーティファクトマーカーとすべての必須セクションマーカーを保持します。
- ベース、HEAD、マージベース、コミット数、変更ファイル、標準ソース、仕様ソースを記録します。
- SpecセクションとStandardsセクションを分離して維持します。Spec所見をStandardsに移動したり、その逆をしたりしないでください。
- 安定した所見IDを使用します:
CR-001、CR-002など。変更を記録せずに承認済み所見の番号を振り直さないでください。 - 所見ごとに1つの重大度を使用します:
blocking、recommended、またはnote。references/severity-baseline.md に従います。 - すべての所見に、軸、カテゴリ、場所、問題、証拠、提案、参照、検証手順を記載します。
- カウントマーカーが所見と正確に一致するようにします。
- サマリーカウントがカウントマーカーと一致するようにします。
- すべての所見を証拠と推奨される検証にトレーサビリティセクションでマッピングします。
- Specレビューが実行されたかスキップされたかを明記します。スキップされた場合、ゲート2の正確な文章を含めます。
- カテゴリに所見がない場合は
No findingsと書き、空のセクションを残さないでください。 - シークレット、個人データ、顧客データを含めないでください。それらを
[REDACTED]として秘匿化します。 - 承認判定、
LGTM、Ship itを含めないでください。
ゲート 6: バリデーション
この SKILL.md からの相対パスでバリデータを解決し、実行します:
python3 skills/code-review/scripts/validate_review.py CODE-REVIEW.md
オプション:
--strict 警告を失敗として扱います。
--json 機械可読レポートを出力します。
--min-findings N 少なくともN個の所見を要求します。デフォルトは0です。
--require-spec Specレビューがスキップされた場合に失敗します。
--require-standards-sources 標準ソースがリストされていない場合に失敗します。
すべてのエラーを修正し、バリデータを再実行します。バリデーションに失敗したレポートを提供しないでください。
Pythonが利用できない場合は、references/validation-checklist.md の手動チェックリストを実行します。
ゲート 7: ハンドオフ
報告:
CODE-REVIEW.mdが書き込まれた場所- レビューしたベース、マージベース、コミット数、変更ファイル
- 重大度と軸別の所見数
- Specレビューステータスと最もリスクの高いSpecギャップ
- 最もリスクの高いStandards所見とその影響
- バリデーション結果
- 制限、除外ファイル、実行されなかったチェック
- アプリケーションコード、依存関係、設定は変更されていないこと
修正や修復計画の提案を提供しますが、ユーザーが明示的に依頼しない限り、コード、コミット、プッシュ、プルリクエストのオープンは行いません。
2軸ルール
各軸は異なる質問に答えるため、分離して維持する必要があります:
| 軸 | 質問 | 典型的な証拠 |
|---|---|---|
| Spec | 変更は要求された要件を実装していますか？ | Issue、PRD、仕様書、受け入れ基準 |
| Standards | 変更はリポジトリの規約とエンジニアリング標準に従っていますか？ | CONTRIBUTING、ADR、lint/test/CI設定、コードレビュー |
根本原因が異なり、修正も異なる場合にのみ、所見は両方の軸に存在しても構いません。それ以外の場合は、根本原因を所有する軸の下に1回記録し、参照フィールドで他の軸を相互参照します。
例:
- 変更はすべての規約に従っているが、受け入れ基準を1つ省略している: 1つのSpec
blocking所見、Standards所見なし。 - 変更は要件を実装しているが、認可バイパスを導入している: 1つのStandards
blocking所見、および要件は実装されているが安全ではないというSpecノート。 - 変更は要件を文書化されているが貧弱なパターンで実装している: 1つのStandards
recommended所見、および「要件充足」エントリ。
成果物契約
CODE-REVIEW.md は以下で始まります:
<!-- code-review:artifact type=review version=1 -->
必須セクションはマーカーを使用します:
<!-- code-review:section id=summary -->
<!-- code-review:section id=context -->
<!-- code-review:section id=spec -->
<!-- code-review:section id=standards -->
<!-- code-review:section id=validation -->
<!-- code-review:section id=evidence -->
<!-- code-review:section id=assumptions -->
<!-- code-review:section id=traceability -->
Specセクションはステータスマーカーも使用します:
<!-- code-review:spec status=performed -->
<!-- code-review:spec status=skipped -->
カウントマーカーは所見と一致する必要があります:
<!-- code-review:counts blocking=1 recommended=2 note=0 -->
各所見は、その見出しの直前にマーカーを使用します:
<!-- code-review:finding id=CR-001 axis=standards severity=blocking category=security status=open -->
### CR-001: 短い所見のタイトル
各所見の本文には以下が含まれます:
Axis:SpecまたはStandardsSeverity:Blocking、Recommended、またはNoteCategory:spec-gap、scope、correctness、security、tests、api、performance、maintainability、またはobservabilityLocation:path/to/file.ext:42、path/to/file.ext、またはコードロケーションなしのSpec所見の場合はn/aProblem: 欠陥、リスク、またはギャップEvidence: 差分スニペット、ファイルと行、仕様要件、または再現可能な観
... [翻訳のため省略]