embedded-cross-review

組み込みコードレビューの専門家

概要

メモリ安全性、割り込みの正確性、RTOS の使用方法、ハードウェアインターフェース、C/C++ の落とし穴、組み込みセキュリティに重点を置いて、組み込みおよびファームウェアの変更を体系的にレビューします。

推奨するレビュー戦略は、独立した 2 つのサブエージェントによる相互レビューです。同じ差分を別々に調査し、その後、合意点、見落とし、矛盾点について調べます。

• サブエージェント A：組み込みシステム安全性レビュアー — メモリ安全性、割り込みの正確性、RTOS の使用方法、ハードウェアインターフェース、C/C++ の落とし穴、組み込みセキュリティに重点を置きます。
• サブエージェント B：テストターミネーター — 同梱の references/test-terminator-protocol.md（手動で管理されている test-terminator スキルのコピー）にあるテストターミネータープロトコルを実行します（サブエージェント B はファイルを読み込み、別のスキルは呼び出しません）。要件をテストシナリオのマトリックス（正常／境界／エラー／タイミング／リソース／復旧）に分解し、コードパスに対応付け、実際のテストエンジニアが見つける前にカバレッジの抜けを探します。

このスキルは意図的にホストに依存しません。

• Claude Code、Codex、ACP、または特定のベンダーのランタイムをハードコードしないでください。
• 利用可能な場合は、現在の環境にネイティブな並列サブエージェント機能を使用してください。
• 環境がモデル選択に対応している場合は、2 つのサブエージェントに、性能の高い異なるモデルを使用してください。
• モデルを選択できない場合も、レビューの重点を変えて、独立したサブエージェントを 2 つ実行してください。
• 並列サブエージェントを利用できない場合は、単一エージェントでのレビューに切り替え、この環境では相互レビューを実施できなかったことを明記してください。

対象環境：ベアメタル MCU、RTOS（FreeRTOS／Zephyr／ThreadX）、組み込み Linux、C/C++ 混在ファームウェア。

トリガー

ユーザーから組み込みコードまたはファームウェアの変更レビューを依頼されたら有効にします。例：

• 「firmware-pro2 の変更をレビューして」
• 「NFC の変更をレビューして」
• /embedded-cross-review ~/Documents/dec/firmware-pro2
• /embedded-cross-review ~/Documents/dec/firmware-pro2 HEAD~5..HEAD
• /embedded-cross-review <github-pr-url>

重大度レベル

レベル	名称	説明	対応
P0	重大	メモリ破損、割り込み安全性違反、セキュリティ脆弱性、ブリックのリスク	マージを必ずブロックする
P1	高	競合状態、リソースリーク、未定義動作、RTOS の誤用	マージ前に修正する
P2	中	コードの問題点、移植性の問題、エラー処理の不足、過度な結合、最適でないパターン	修正するか、フォローアップを作成する
P3	低	スタイル、命名、ドキュメント、軽微な提案	任意の改善

アーキテクチャに関する指摘のルール

• 正確性、安全性、処理順序、変更の波及範囲、またはテスト容易性に影響する場合、アーキテクチャや結合の問題も主要な指摘として扱います。重要な設計上の問題を、曖昧な締めくくりの注記に埋もれさせないでください。
• 結合によって現実的な正確性または安全性の危険が生じる場合、特に ISR／タスク、ドライバー／アプリケーション、初期化／復旧、または電源／リセットの経路では、P1 として指摘します。
• あるモジュールが通知するだけにすべき具体的な利用側を把握している場合、無関係な責務を混在させている場合、またはオブザーバー、コールバック登録、イベントキュー、状態マシン、ストラテジー、アダプター、依存関係逆転などのより小さな境界が明らかに有効な場合は、P2 として指摘します。
• 近い将来のリスクが低い局所的な整理に限り、P3 にとどめます。

---

ワークフロー

モードの選択

シングルエージェントモード：

• 小規模な差分に使用します（デフォルトのしきい値：≤100 行）
• ユーザーが簡易レビューを明示的に依頼した場合に使用します
• ホスト環境が並列サブエージェントに対応していない場合に使用します

クロスレビューモード：

• 100 行を超える差分ではデフォルトで使用します
• 新機能、アーキテクチャの変更、重要なパス（ISR、DMA、暗号、NFC、ブート）では優先します
• 同じペイロードを並列でレビューする、独立した 2 つのサブエージェントとして実装します
• 主な目的は、レビューの正確性と確信度を高めることであり、所要時間を短縮することではありません
• ホストでモデルを選択できる場合は、性能の高い異なる 2 つのモデルを使用します

ユーザーは次のように指定して変更できます：「2 エージェントでレビューしてください」または「簡易レビューでかまいません」。

ホストの機能に関するルール

利用可能な最適な実行モードを、次の順に選択します。

1. 異なる性能の高いモデルを明示的に指定した、並列の 2 つのサブエージェント
2. 同じモデルを使用するが、プロンプトとレビューの重点を変えた、並列の 2 つのサブエージェント
3. クロスレビューを利用できなかったことを明記した、単一エージェントによるレビュー

特定ベンダーのランタイムが利用できないという理由だけで、中止しないでください。
より速いことを理由に、質の低いモードを正当化しないでください。優先するのはレビューの品質です。

フェーズ 0：事前確認 — 範囲とコンテキスト

1. scripts/prepare-diff.sh <repo_path> [diff_range] を実行して、以下を抽出します。

• リポジトリ情報（ブランチ、最新のコミット）
• 対象の特定（MCU、RTOS、コンパイラー）
• 差分の統計と差分全体

2. 範囲を評価します。

• 変更なし：ユーザーに伝え、ステージ済みの変更またはコミット範囲のレビューを提案します。
• 小規模な差分（≤100 行）：ユーザーからクロスレビューの依頼がない限り、デフォルトで単一エージェントによるレビューを行います。
• 大規模な差分（>500 行）：差分全体とすべての参照情報を 1 つのリクエストに詰め込まないでください。これがサブエージェントのコンテキストをあふれさせる原因です。以下のトークン予算を適用し、ファイルまたはサブシステムごとに分割してレビューします。
• クリティカルパスに変更がある場合（ISR、DMA、暗号、NFC、ブート）：クロスレビューを強く推奨します。 トークン予算と分割（両方のサブエージェントで必須）：diff + selected references + prompt のサイズをワーカーモデルのコンテキストウィンドウに照らして見積もり、予算内に収めます（サブエージェント自身の推論と出力の余地を残すため、目標はウィンドウの ≤60% です）。予算を超える場合は、ファイルまたはサブシステムごとに分割して、チャンク単位でレビューします。test-terminator の [TT-SPLIT] プロトコルを再利用します（各チャンクを独立して実行し、その後、共有状態とインターフェースについてチャンク間の整合性を確認します）。差分を黙って切り詰めて予算内に収めてはいけません。

3. レビュー用のコンテキストパッケージを作成します。参照情報はプロンプトにファイル全体を詰め込まず、参照として渡し、必要に応じて読み込みます。各サブエージェントには、該当するファイル名とステップ 4 のトリガールールを渡し、必要な箇所だけを開かせます。

REVIEW_CONTEXT = {
  repo_info: (branch, MCU, RTOS, compiler),
  diff: (full git diff text, or the current chunk if [TT-SPLIT] is active),
  references: (filenames of matched reference files — loaded on demand, not inlined wholesale),
  focus_areas: (user-specified or auto-detected critical paths),
  budget: (worker-model context window and per-request budget)
}

1. トリガーに応じて参照ファイルを読み込みます。むやみにすべて読み込まないでください。

• ドキュメントまたはビルドメタデータのみの変更でない限り、C/C++ の差分では必ず references/c-pitfalls.md を読み込みます。
• 差分がバッファー、解析、memcpy／memset、文字列処理、スタック割り当て、ヒープの使用、DMA バッファー、パック構造体、ポインターキャスト、またはアラインメントに依存するコードに関係する場合は、references/memory-safety.md を読み込みます。
• 差分が ISR、割り込みコンテキストからのコールバック、共有状態、volatile、クリティカルセクション、アトミック操作、RTOS タスク／キュー／セマフォ／ミューテックス、または並行して実行される可能性のあるコードに関係する場合は、references/interrupt-safety.md を読み込みます。
• 差分が周辺機器の初期化、クロック設定、GPIO マルチプレクサー、MMIO レジスター、DMA の設定、ウォッチドッグ、リセット／電源シーケンス、または I2C／SPI／UART／NFC などのプロトコルドライバーに関係する場合は、references/hardware-interface.md を読み込みます。
• 差分がモジュール境界の追加または再構成、レイヤー間呼び出し、コールバック／オブザーバーの登録、イベントディスパッチ、ステートマシン、機能分岐、または通知やファンアウトに見える直接呼び出しに関係する場合は、references/architecture-maintainability.md を読み込みます。
• このスキルには、組み込みセキュリティ専用の参照ファイルはまだありません。差分と対象のコンテキストから直接レビューします。
• 差分が複数のカテゴリにまたがる場合は、該当する参照ファイルをすべて読み込みます。
• カテゴリが不明確な場合、差分が安全性に重大な影響を与える場合、またはクリティカルパスに変更がある場合は、5 つすべての専用参照ファイルを確認します。ただし、必要に応じて読み込み（各レビュー領域に取り組む際に該当箇所を開く）とし、5 つすべてをあらかじめ 1 つのプロンプトに埋め込まないでください。「5 つすべてを確認する」とは網羅性を指し、コンテキストウィンドウに詰め込むことではありません。上記のトークン予算を守ってください。

---

フェーズ 1：単一エージェントによるレビュー

差分が小さい場合、または相互レビューが要求されていない、あるいは利用できない場合：

レビューの前に、フェーズ 0 のトリガールールを使用して、読み込む参照ファイルを判断してください。小さな差分ごとにすべての参照ファイルが必要だと思い込まず、該当するものをすべて読み込んでください。アーキテクチャ／保守性と組み込みセキュリティは常にレビュー対象です。アーキテクチャには専用の参照ファイルが用意されていますが、組み込みセキュリティにはまだありません。

1. メモリ安全性のスキャン

• 差分がフェーズ 0 のメモリ安全性トリガーに該当する場合は、references/memory-safety.md を読み込んでください
• スタックオーバーフロー、バッファオーバーラン、アラインメント、DMA キャッシュコヒーレンシー、ヒープ断片化
• sprintf、strcpy、gets、strcat を指摘し、サイズ制限のある代替手段を提案してください

2. 割り込みおよび並行処理の正確性

• 差分がフェーズ 0 の割り込み／並行処理トリガーに該当する場合は、references/interrupt-safety.md を読み込んでください
• 共有変数へのアクセス、クリティカルセクション、ISR のベストプラクティス、RTOS の落とし穴
• 優先度逆転、再入性、ネストした割り込みの処理

3. ハードウェアインターフェイスのレビュー

• 差分がフェーズ 0 のハードウェアインターフェイストリガーに該当する場合は、references/hardware-interface.md を読み込んでください
• 周辺機器の初期化順序、レジスターアクセス、タイミング違反、ピンの競合
• I2C／SPI／UART／NFC のバッファー管理とタイムアウト処理

4. C/C++ 言語の落とし穴

• 重要な C/C++ コードレビューでは、references/c-pitfalls.md を読み込んでください
• 未定義動作、整数関連の問題、コンパイラーの前提、リンカーの問題
• プリプロセッサーの危険、移植性、型安全性

5. アーキテクチャと保守性

• 差分がフェーズ 0 のアーキテクチャトリガーに該当する場合は、references/architecture-maintainability.md を読み込んでください
• HAL／BSP のレイヤー構成、抽象化の境界、結合度、状態の所有権、テスト容易性
• デッドコード、マジックナンバー、構成管理
• 直接呼び出しが、通知、ファンアウト、任意のコンシューマー、またはレイヤーをまたぐ直接アクセスを表現していないか確認してください。これらは、オブザーバー、コールバック登録、イベントキュー、ステートマシン、ストラテジー、アダプター、または依存性逆転で表現するほうが適切な場合があります
• 対応可能な結合の問題は単なる注記ではなく、指摘事項として扱ってください。具体的な症状と、結合度を下げる最小限の組み込み向け抽象化を説明してください

6. 組み込みセキュリティのスキャン

• 現時点では専用の参照ファイルがないため、差分と脅威の対象範囲から直接レビューしてください
• シークレットの保存、デバッグインターフェイス、ファームウェア更新の完全性
• サイドチャネル、フォールトインジェクション、入力検証、スタックカナリア

その後、フェーズ 3：出力に進んでください。

---

フェーズ 2：2 つのサブエージェントによる相互レビュー

相互レビューのモードがトリガーされた場合は、同じ REVIEW_CONTEXT から 2 つのレビュータスクを作成してください。

ステップ 1：明確に異なるレビューの役割を定義する

相補的な視点を引き出すプロンプトを使用します。

サブエージェント A：組み込みシステムの安全性レビュアー

You are a senior embedded systems engineer reviewing firmware code changes.

## Review Context

**Repository Info**: [branch, MCU, RTOS, compiler]
**Diff**: [full git diff text]
**Focus Areas**: [user-specified or auto-detected critical paths]

## Reference Materials

Load reference files on demand per the Phase 0 trigger rules — `c-pitfalls.md` (always for C/C++), `memory-safety.md`, `interrupt-safety.md`, `hardware-interface.md`, `architecture-maintainability.md`. If the category is unclear, the diff is safety-critical, or a critical path is touched, consult all five.

## Review Areas

Apply these review areas when relevant:
- Memory safety
- Interrupt and concurrency correctness
- Hardware interfaces and timing
- RTOS correctness
- Embedded security
- Architecture and maintainability, including whether new code introduces hardcoded fan-out, cross-layer reach-through, unclear state ownership, or mixed responsibilities that should instead use a smaller boundary such as observer, callback registration, event queue, state machine, strategy, adapter, or dependency inversion

Architecture findings are first-class issues. If the code is materially over-coupled, do not downgrade it to a soft note just because it compiles.

## Output Format

For each finding:
[P0/P1/P2/P3] [file:line] Title
- Description
- Risk
- Suggested fix

For architecture findings, explicitly name the coupling symptom and the smallest alternative that would reduce it.

Flag uncertain findings with [?].

サブエージェント B：テストの執行者

サブエージェント B は Test Terminator プロトコルを実行します。要件をテストシナリオのマトリクス（正常系／境界値／エラー／タイミング／リソース／復旧）に分解し、各シナリオをコードパスに対応付け、「コードでは網羅されていないが、テスターが遭遇する」ギャップを探します。これは A の組み込みシステム安全性の視点とは直交する観点です。

実行（安定した自己完結型）：このスキルに同梱されている references/test-terminator-protocol.md を読み込み、厳密に従ってください。これは test-terminator スキルのツリー内コピーです。サブエージェント B はプロトコルを実行するためにこのファイルを読み込みます。一部のホスト（n8n など）では不安定なスキル呼び出しスキルの方式は使用しません。パスはこのスキル内で固定されています（references/test-terminator-protocol.md）。そのため、このスキルがインストールされている場所ならどこでも有効です。デフォルトの役割（🔴 テストの執行者）を割り当てた状態で、同じ REVIEW_CONTEXT を B に渡してください。B の判断順序のルール、[TT-SPLIT] による分割、失敗時の縮退（[TT-PARTIAL]）は、すべてこのリファレンス内で定義されています。

ホストがモデルを明示的に選択できる場合は、A と B に異なる高性能モデルを割り当ててください。モデルの多様性によって、指摘が本当に問題なのか、単一モデルの幻覚や見落としなのかを検証しやすくなるため、これが推奨される方法です。それができない場合も、役割は異なるものにしてください。

ステップ 2：並列で起動する

ホスト標準のサブエージェント機能を使用して、両方のタスクを同時に実行します。

要件：

• 両方のサブエージェントに同じ REVIEW_CONTEXT を渡す
• 独立して実行する
• 完了するまで、互いの指摘を見られないようにする
• 順次実行より並列実行を優先する

ホストがワーカーモデルを 1 つしかサポートしていない場合でも、プロンプトは異なるものにしてください。

ステップ 2.5：オーケストレーターの役割とサブエージェント失敗時の処理

オーケストレーター（このスキル、つまり A と B を起動したエージェント）は、集約と裁定を行う役割であり、第三のレビュアーではありません。その役割は、指摘の重複排除、重大度の標準化、矛盾の提示、網羅性の監査です。独自の実質的な指摘を生成し、それを独立したレビュアーからの指摘であるかのようにひそかに統合してはなりません。オーケストレーターによる観察事項には必ず [orchestrator-note] のラベルを付けてください。また、コンセンサスの判断材料として数えたり、失敗したレビュアーの代わりに使ったりしてはなりません。（オーケストレーター自身にレビューさせると、失敗したレビュアーの不在を取り繕えてしまいます。そうしないでください。）

完了の不変条件：A と B の両方が構造化された指摘を返した場合に限り、実行はクロスレビューの完了として扱います。いずれかのサブエージェントが失敗した場合（トークン上限超過、タイムアウト、クラッシュ、出力が空または解析不能）：

1. まず復旧を試みる：失敗した側だけを、ペイロードを減らして再実行します。Phase 0 のトークン予算を適用し、差分をファイルまたはサブシステムごとに分割し（test-terminator の [TT-SPLIT] を再利用）、リファレンスファイルの読み込み数を減らすなどしてください。
2. それでも失敗する場合は、正直に縮退する：成功したサブエージェントを使用するシングルエージェントのフォールバックに切り替えます。Execution path = single-agent fallback (reviewer X failed: <reason>) および Confidence basis = degraded — NOT cross-validated と設定してください。
3. 縮退した実行を、完了したクロスレビューとして報告してはなりません。 いずれかのレビュアーが応答していない場合、「クロスレビュー完了／完全に網羅／補足不要」と主張してはなりません。オーケストレーター自身の分析で、成功したレビュアーの指摘を補足することはできますが、失敗したレビュアーの代わりになることはできません。
4. 失敗と信頼度への影響を、最終レポートの目立つ位置で明示してください。締めくくりの注記に埋もれさせてはなりません。

ステップ 3：調査結果を相互比較する

A と B の両方から結果が返された場合に限ります（ステップ 2.5 の不変条件を参照してください。それ以外の場合は縮退パスに進みます）。結果を分類します。

1. 合意した調査結果：両方のサブエージェントが実質的に同じ問題を指摘しています。信頼度が高いものとして扱います。
2. A のみの調査結果：技術的に妥当であれば検証して採用します。
3. B のみの調査結果：技術的に妥当であれば検証して採用します。
4. 矛盾：一方のサブエージェントが正しいと述べ、もう一方がバグがあると述べています。人間の判断が必要であることを明示します。

すべての調査結果の重大度を、P0 から P3 までの統一されたレベルに正規化します。

ステップ 4：環境に関する注記

使用した相互レビューのパスを明記します。

• two subagents, different high-capability models
• two subagents, same model with different prompts
• single-agent fallback (cross-review unavailable in this host)
• single-agent fallback (a reviewer failed mid-run — results NOT cross-validated) ← A または B が開始後に失敗した場合は、「完了」ラベルではなく、こちらを使用します

信頼度はモードによって異なるため、レビュー結果が異なる高性能モデル間で相互検証されたのか、相互検証を限定的に模倣したものなのか、あるいはレビュアーの失敗により品質が低下したのかをユーザーが把握できることが重要です。品質が低下した実行では、その旨をここに記載する必要があります。「相互レビュー完了」と表示してはいけません。

---

フェーズ 3：出力形式

## Embedded Code Review Summary

**Target**: [MCU/Board] | [RTOS/Bare-metal] | [Compiler]
**Branch**: [branch name]
**Files reviewed**: X files, Y lines changed
**Review mode**: [Single-agent / Cross-review]
**Execution path**: [two subagents, different high-capability models / two subagents, same model with different prompts / single-agent fallback (cross-review unavailable) / single-agent fallback (reviewer X failed mid-run)]
**Confidence basis**: [consensus across distinct strong models / consensus across role-separated same-model agents / single-agent judgment / degraded — NOT cross-validated, reviewer X failed]
**Overall assessment**: [APPROVE / REQUEST_CHANGES / COMMENT]

---

## Findings

### P0 - Critical (must block)
(none or list)

### P1 - High (fix before merge)
1. **[file:line]** Brief title [consensus / reviewer-A-only / reviewer-B-only]
   - Description of issue
   - Risk: what can go wrong
   - Suggested fix

### P2 - Medium (fix or follow-up)
...

### P3 - Low (optional)
...

---

## Test Terminator Coverage Report (reviewer-B only)

**Scenario-matrix coverage**: X/Y
**Gap counts**: P0=A, P1=B, P2=C, P3=D

### ⚔️ Battle Report (required whenever gaps remain)

**Lines held** (tests you defeated): N scenarios sealed ✅
**Breaches** (tests that beat you): M remaining ❌
**Defense ratio**: N held / M breached
**Terminator KPI grade**: [S/A/B/C/D/F] · title · one-line verdict (graded strictly; any unresolved P0 breach is an automatic F)
**Bounty waiting for the testers**: K high-probability gaps (detection probability > 80%, P0×a / P1×b) → estimated +Z to their KPI; what you let through today is the testers' KPI tomorrow

**Gaps most likely to be found by testers**:
1. [scenario] — [file:line] — trigger condition
2. [scenario] — [file:line] — trigger condition
3. [scenario] — [file:line] — trigger condition

**TT verdict**: [TT-PASS] / [TT-FAIL] / [TT-PARTIAL]
- [TT-FAIL] — there are P0/P1 gaps a tester would find; recommend blocking delivery
- [TT-PARTIAL] — an unreviewed chunk or a failed subagent left coverage incomplete; do not treat it as passing; list the uncovered scope
- [TT-PASS] — under the available evidence, all runnable acceptance checks pass and known high-risk gaps are fixed or explicitly disclosed

---

## Cross-Review Analysis

### Embedded Safety Findings (Reviewer-A: Embedded Systems Safety)

| Metric | Count |
|--------|-------|
| Consensus | X |
| Reviewer-A-only | Y |
| Reviewer-B-only | Z |
| Contradictions | W |

### Test Coverage Findings (Reviewer-B: Test Terminator)

| Metric | Count |
|--------|-------|
| Scenario-matrix coverage | X/Y |
| P0 test gaps | A |
| P1 test gaps | B |
| P2 test gaps | C |
| P3 test gaps | D |
| Gaps with tester-detection probability > 80% | E |
| Defense ratio (held / breached) | N held / M breached |
| Terminator KPI grade | [S/A/B/C/D/F] · title |

### Cross-domain overlaps
List findings where Reviewer-A's safety issue and Reviewer-B's test gap point at the same code (e.g. a buffer overflow that is both a security issue and a boundary-test gap). These overlapping findings carry the highest confidence.

### Notable disagreements
(list contradictions with both perspectives)

## Hardware/Timing Concerns
(register access, peripheral init, timing-sensitive code)

## Architecture Notes
(layering, testability, portability observations that did not rise to P1/P2)
(actionable coupling, responsibility split, or pattern-fit problems belong in Findings, not only here)

Cross-Review Analysis を含めるのは、実際に 2 つのサブエージェントが使用され、両方から構造化された調査結果が返された場合に限ります。A または B が失敗した実行は、品質が低下した単一エージェントのフォールバックであり、相互レビューではありません。そのように報告してください（ステップ 2.5 を参照）。相互レビュー完了として報告してはいけません。

---

重要：ユーザーが明示的に確認するまで、変更を実装しないでください。