Embedded Code Review Expert

# Embedded Code Review Expert

Overview

Perform structured code review of embedded/firmware projects using dual-model cross-review: Claude Code and Codex independently review the same diff, then findings are cross-compared to catch blind spots that single-model review misses.

Target environments: bare-metal MCU, RTOS (FreeRTOS/Zephyr/ThreadX), Linux embedded, mixed C/C++ firmware.

Trigger

• Activate when user asks to review embedded/firmware code changes. Examples:
• "review firmware-pro2 的改动"
• "review the NFC changes"
• `/embedded-review ~/Documents/dec/firmware-pro2`
• `/embedded-review ~/Documents/dec/firmware-pro2 HEAD~5..HEAD`
• `/embedded-review <github-pr-url>`

Severity Levels

| Level | Name | Description | Action | |-------|------|-------------|--------| | P0 | Critical | Memory corruption, interrupt safety violation, security vulnerability, brick risk | Must block merge | | P1 | High | Race condition, resource leak, undefined behavior, RTOS misuse | Should fix before merge | | P2 | Medium | Code smell, portability issue, missing error handling, suboptimal pattern | Fix or create follow-up | | P3 | Low | Style, naming, documentation, minor suggestion | Optional improvement |

---

Workflow

Mode Selection

• Single-model mode (default for small diffs ≤100 lines):
• One review pass using the current session's model
• Faster, lower cost
• Suitable for trivial changes, config tweaks, documentation

• Dual-model cross-review (default for diffs >100 lines, or when explicitly requested):
• Claude Code + Codex review independently via ACP
• Cross-compare findings
• Higher quality, catches heterogeneous blind spots
• Use for: new features, architecture changes, critical paths (ISR, crypto, NFC, DMA)

User can override: "用双模型 review" or "quick review 就行"

---

Phase 0: Preflight — Scope & Context

1. Run `scripts/prepare-diff.sh <repo_path> [diff_range]` to extract:
2. - Repository info (branch, last commit)
3. - Target identification (MCU, RTOS, compiler)
4. - Diff stat and full diff content

1. Assess scope:
2. - No changes: Inform user; offer to review staged changes or a commit range.
3. - Small diff (≤100 lines): Default to single-model review.
4. - Large diff (>500 lines): Summarize by file/module first, then review in batches by subsystem.
5. - Critical path touched (ISR, DMA, crypto, NFC, boot): Always recommend dual-model.

1. Build review context package:
2. ```
3. REVIEW_CONTEXT = {
4. repo_info: (branch, MCU, RTOS, compiler),
5. diff: (full git diff text),
6. references: (relevant checklist sections from references/),
7. focus_areas: (user-specified or auto-detected critical paths)
8. }
9. ```

---

Phase 1: Single-Model Review

For small diffs or when dual-model is not requested:

• #### 1) Memory safety scan
• Load `references/memory-safety.md` for detailed checklist.
• Stack overflow, buffer overrun, alignment, DMA cache coherence, heap fragmentation
• Flag `sprintf`, `strcpy`, `gets`, `strcat` — suggest bounded alternatives

• #### 2) Interrupt & concurrency correctness
• Load `references/interrupt-safety.md` for detailed checklist.
• Shared variable access, critical sections, ISR best practices, RTOS pitfalls
• Priority inversion, reentrancy, nested interrupt handling

• #### 3) Hardware interface review
• Load `references/hardware-interface.md` for detailed checklist.
• Peripheral init ordering, register access, timing violations, pin conflicts
• Communication protocols: I2C/SPI/UART/NFC buffer management, timeout handling

• #### 4) C/C++ language pitfalls
• Load `references/c-pitfalls.md` for detailed checklist.
• Undefined behavior, integer issues, compiler assumptions, linker issues
• Preprocessor hazards, portability, type safety

• #### 5) Architecture & maintainability
• HAL/BSP layering, abstraction, coupling, testability
• Dead code, magic numbers, configuration management

• #### 6) Security scan (embedded-specific)
• Secret storage, debug interfaces, firmware update integrity
• Side channels, fault injection, input validation, stack canaries

→ Skip to Phase 3: Output for single-model results.

---

Phase 2: Dual-Model Cross-Review (ACP)

When dual-model review is triggered:

#### Step 1: Prepare review payloads

Build two independent review tasks from the same REVIEW_CONTEXT:

Claude Code task: ``` You are a senior embedded systems engineer reviewing firmware code changes.

[REVIEW_CONTEXT: repo info, diff, focus areas]

• Review checklist (apply all that are relevant):
• Memory safety (references/memory-safety.md)
• Interrupt & concurrency (references/interrupt-safety.md)
• Hardware interfaces (references/hardware-interface.md)
• C/C++ pitfalls (references/c-pitfalls.md)
• Architecture & security

• Output format: For each finding, provide:
• [P0/P1/P2/P3] [file:line] Title
• Description
• Risk
• Suggested fix

Be thorough. Flag everything you find, even if uncertain — mark uncertain items with [?]. ```

Codex task: ``` You are an independent code reviewer for embedded/firmware projects. Your job is to find bugs, security issues, and correctness problems.

[REVIEW_CONTEXT: repo info, diff, focus areas]

1. Focus on:
2. Memory corruption risks (buffer overflow, use-after-free, stack overflow)
3. Concurrency bugs (race conditions, missing volatile, ISR safety)
4. Hardware interface errors (timing, register access, peripheral init)
5. Logic errors and edge cases
6. Security vulnerabilities

• Output: List every issue found as:
• [SEVERITY: critical/high/medium/low] [file:line] Issue title
• What's wrong
• What could happen
• How to fix

Do NOT skip low-severity items. Report everything. ```

#### Step 2: Spawn parallel ACP sessions

``` sessions_spawn(runtime="acp", agentId="claude-code", task=claude_task) sessions_spawn(runtime="acp", agentId="codex", task=codex_task) ```

Both run simultaneously. Wait for both to complete.

#### Step 3: Cross-compare findings

After both complete, analyze results:

1. Consensus findings (both flagged same issue): HIGH CONFIDENCE — these are real bugs
2. Claude-only findings: Review for validity — may be false positive or genuine catch
3. Codex-only findings: Review for validity — heterogeneous perspective may catch Claude's blind spots
4. Contradictions: Flag for human judgment — one says it's fine, other says it's a bug

Map to unified severity levels (P0-P3).

---

Phase 3: Output Format

```markdown ## Embedded Code Review Summary

Target: [MCU/Board] | [RTOS/Bare-metal] | [Compiler] Branch: [branch name] Files reviewed: X files, Y lines changed Review mode: [Single-model / Dual-model (Claude Code + Codex)] 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 / 🔵 Claude-only / 🟢 Codex-only] - Description of issue - Risk: what can go wrong - Suggested fix

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

⚪ P3 - Low (optional) ...

---

Cross-Review Analysis (dual-model only)

| Metric | Count | |--------|-------| | 🤝 Consensus (both found) | X | | 🔵 Claude-only | Y | | 🟢 Codex-only | Z | | ⚠️ Contradictions | W |

Notable disagreements (list any contradictions with both perspectives)

---

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

Architecture Notes (layering, testability, portability observations) ```

Phase 4: Next Steps

```markdown --- ## Next Steps

Found X issues (P0: _, P1: _, P2: _, P3: _).

1. How would you like to proceed?
2. Fix all — implement all suggested fixes
3. Fix P0/P1 only — address critical and high priority
4. Fix specific items — tell me which issues to fix
5. Re-review with dual-model — run cross-review (if single-model was used)
6. No changes — review complete
7. ```

Important: Do NOT implement changes until user explicitly confirms.

---

Resources

references/

| File | Purpose | |------|---------| | `memory-safety.md` | Buffer, stack, heap, DMA, alignment checklist | | `interrupt-safety.md` | ISR, concurrency, RTOS, atomic operations checklist | | `hardware-interface.md` | Peripherals, registers, timing, protocol checklist | | `c-pitfalls.md` | UB, integer, compiler, preprocessor, portability checklist |

scripts/

| File | Purpose | |------|---------| | `prepare-diff.sh` | Extract git diff and build review context |