/review

name: review
description: "Reviews code changes for bugs with P0-P2 prioritized feedback. Uses parallel subagents for thorough analysis, then creates fix plans. Use when reviewing code, finding bugs, checking quality, or before merging. Use review fix to implement fixes."
argument-hint: "[staged | branch] | fix [P0 | P0-P1 | all]"

**Arguments:** $ARGUMENTS

| Command | Behavior | |---------|----------| | `review` | Review branch diff, synthesize findings, create fix plan | | `review staged` | Review staged changes only | | `review fix` | Implement all fixes from saved plan | | `review fix P0` | Implement only P0 (critical) fixes | | `review fix P0-P1` | Implement P0 and P1 fixes |

| Scope | Git Command | |-------|-------------| | `branch` (default) | `git diff main...HEAD` | | `staged` | `git diff --cached` |

---

Review Mode

Step 1: Categorize Changes

Get the diff and categorize files by change type:

**Architectural changes** → assign to subagent role `architect-reviewer` (persona in `agents/architect-reviewer.md`):

• API routes, endpoints, controllers
• Database schemas, migrations
• Service interfaces, dependency injection
• Configuration files (docker, CI/CD)
• Directory structure changes

**Implementation changes** → assign to subagent role `senior-code-reviewer` (persona in `agents/senior-code-reviewer.md`):

• UI components, styling
• Business logic within existing patterns
• Bug fixes, refactoring
• Test files
• Utility functions

If a file fits both categories, assign to both reviewers.

Step 2: Delegate to Review Subagents

**Scale based on change size, delegating to subagents in parallel:**

• 1-4 files: 1 subagent
• 5-10 files: 2-3 subagents grouped by directory/component
• 10+ files: 3-5 subagents grouped by directory/component

Each subagent runs in its own forked, isolated context with the persona for its assigned role (`agents/architect-reviewer.md` or `agents/senior-code-reviewer.md`).

**Handoff to reviewer subagents:**

• File list to review
• Diff command: `git diff main...HEAD -- <files>` (or `--cached` for staged)
• Scope context (branch or staged)

Subagents return prioritized findings (P0-P2) in consistent format with Files, Problem, Fix, and Done when.

Wait for all subagents to complete.

Step 3: Synthesize Findings

1. **Collect** all findings from subagents 2. **Deduplicate** overlapping findings 3. **Sort** by priority (P0 first) 4. **Present** findings table for review:

## Code Review Findings

| P | Problem | Fix Approach | Files | Done When |
|---|---------|--------------|-------|-----------|
| P0 | Null pointer in user lookup | Add early return with 404 | `users.ts:47` | Returns 404 for missing user |
| P1 | Race condition in cache | Use mutex lock | `cache.ts:23` | Concurrent requests don't corrupt |
| ... | ... | ... | ... | ... |

**Verdict: CORRECT | NEEDS FIXES**

**If no findings:** Report "No issues found" and stop.

Step 4: Validate Findings

Skip this step if there are no findings (verdict is already CORRECT).

Delegate to a subagent with role `false-positive-validator` (persona in `agents/false-positive-validator.md`), passing:

• The full findings list from Step 3
• Scope context (branch or staged)
• Diff command used

**Process results:** 1. **Replace** findings list with validated results (KEPT + DOWNGRADED findings only) 2. **Re-sort** by priority (P0 first) 3. **Append** a collapsed details section showing what was removed or changed:

<details>
<summary>Validation: {N} removed, {M} downgraded</summary>

| Finding | Verdict | Reason |
|---------|---------|--------|
| [P1] Title | FALSE POSITIVE | Already handled — `file.ts:32` has null check |
| [P0 → P2] Title | DOWNGRADED | Context negates severity — `api.ts:15` validates input |

</details>

4. **If all findings removed** → verdict becomes CORRECT, report "No issues found after validation" and stop 5. Otherwise proceed to Step 5

Step 5: Resolve Ambiguous Fixes

**If any fix requires a decision** (contains "Either...OR", "Option A/B", or similar patterns), interview the user to choose:

[P0] Plugin discovery limited to 3 hardcoded paths

The fix has multiple options:
A) Restore two-pass file fetching (more complete, adds complexity)
B) Remove dead countCommands/countSkills functions (simpler, less data)

Which approach?

**Process multiple ambiguous fixes** in a single interview when possible:

• Group related decisions together
• Show context for each choice
• Update fixes with chosen approaches

**If no ambiguous fixes**, skip to Step 6.

Step 6: Approve Fix Plan

**Ask for approval:**

• "Approve and save plan"
• "Request changes" — revise based on feedback
• "Open in editor" — save to `.otto/reviews/fix-plan-draft.md` for editing

**On approval**, write fix plan to `.otto/reviews/fix-plan.json`:

{
  "version": 1,
  "created": "{timestamp}",
  "scope": "{scope}",
  "branch": "{branch}",
  "commit_sha": "{HEAD}",
  "summary": { "p0": 0, "p1": 0, "p2": 0, "p3": 0 },
  "verdict": "NEEDS FIXES",
  "fixes": [
    {
      "id": "f1",
      "priority": "P0",
      "title": "Null pointer dereference in user lookup",
      "problem": "user.profile accessed without null check",
      "fix": "Add early return with 404 when user is null",
      "files": [
        { "path": "src/auth/users.ts", "line": 47, "role": "primary" },
        { "path": "src/auth/users.test.ts", "role": "add test" }
      ],
      "done_when": "Returns 404 for missing user; test covers case",
      "status": "pending",
      "depends_on": []
    }
  ]
}

Report: `Fix plan saved. Run review fix to implement.`

---

Fix Mode

Step 1: Load and Filter

1. Check `.otto/reviews/fix-plan.json` exists

• If missing or stale (code changed): run `review` first

2. Filter by priority argument:

• `fix` or `fix all`: P0-P2
• `fix P0`: P0 only
• `fix P0-P1`: P0 and P1

3. If no matching fixes: report "No {priority} issues to fix"

###