Plan Review

---
name: plan-review
description: "Pre-ExitPlanMode adversarial plan review loop via Codex MCP. Use when: in plan mode, before presenting a plan to the user; reviewing an in-context plan draft. Not for: .md file review (use doc-review), code review (use codex-code-review), lifecycle spec review (use review-spec). Output: review trail summary + plan gate (✅ Plan Ready / ⛔ Plan Blocked / ⚠️ Plan Needs Human)."
allowed-tools: mcp__codex__codex, mcp__codex__codex-reply, Bash(bash:*), Bash(git:*), Bash(node:*), Read, Grep, Glob, Task, Skill
---

# Plan Review Skill

Adversarial review gate for plan-mode drafts: the plan is challenged by an independent reviewer and revised until convergence **before** `ExitPlanMode` presents it to the user.

## Trigger

- Keywords: plan review, review plan, plan-review, pre-ExitPlanMode review
- Self-invoke: in plan mode, before calling `ExitPlanMode`, when the project opts in via `auto-loop-project.md ## Plan Review: enabled` or the user asks for plan review

## When NOT to Use

- Reviewing `.md` files on disk (use `/codex-review-doc` — different artifact: filesystem path vs in-context plan text)
- Reviewing lifecycle specs `1-requirements.md` / `2-tech-spec.md` (use `/review-spec`)
- Code review (use `/codex-review-fast`)
- Not in plan mode / no plan draft exists

## Boundary Contract (v1 Acceptance Scope)

- Review gate applies **only when this skill is actually invoked** (A1 skill-driven; enabled-but-unexecuted detection is v2).
- Analysis-only: the reviewer surfaces findings; **Claude revises the plan** — the skill never rewrites or deletes plan content itself.
- Review pass ≠ execution approval: the user still arbitrates the final plan after `ExitPlanMode` (FR-14 Won't).
- **Fully behaviour-layer**: no hook parses plan-review output and no state file records the loop. The skill counts its own rounds in conversation, and the sentinels below are prose contracts the model and the human read — nothing mechanical routes on them (hook-lightweighting § 3.3).

## Arguments

| Arg | Behavior |
|-----|----------|
| (none) | tier = `standard` — Codex alone, with the fix → re-review loop |
| `--quick` | Single Codex pass, no loop |
| `--dual` | Adds a secondary reviewer in parallel. **Off unless passed** — for a release or a security-sensitive plan, not routine planning |
| `--deep` | Delegate to `/codex-brainstorm` (Nash equilibrium debate; attack/defense built-in) |
| `--skip-review` | Immediate bypass: emit `[PLAN_REVIEW_SKIPPED]`, present raw plan |
| `--verbose` | Round-by-round trail (default: summary only) |

User escape (NFR-5): any explicit "skip review" / "直接看 plan" instruction — detected at skill entry **and** before each re-review round — exits within ≤1 round, emits `[PLAN_REVIEW_SKIPPED]`, and presents the current plan.

## Workflow

```mermaid
sequenceDiagram
    participant C as Claude (plan mode)
    participant RD as security-redact
    participant CX as Codex MCP
    participant SA as Secondary (Task)

    C->>C: Step 1: tier + round counter (in conversation)
    C->>RD: Step 2 redaction contract
    alt high-confidence secret hit
        C->>C: [PLAN_REVIEW_DEGRADED] → ExitPlanMode (plan NOT sent to reviewer)
    else masked plan
        alt quick
            C->>CX: 1-pass review (references/codex-prompt-plan.md)
        else standard
            C->>CX: Codex review loop (save threadId)
            opt --dual
                C->>SA: Secondary perspective (parallel)
            end
        else deep
            C->>C: Skill("codex-brainstorm", plan challenge)
        end
        loop until ✅ Plan Ready or max_rounds (default 5)
            CX-->>C: findings + ## Plan Review sentinel
            C->>C: revise plan (author-side), increment round
            C->>CX: re-review (codex-reply, references/review-loop-plan.md)
        end
        alt converged
            C->>C: ✅ Plan Ready → trail summary → ExitPlanMode
        else max_rounds reached
            C->>C: ⚠️ Plan Needs Human + residual findings → user arbitrates
        else reviewer unreachable
            C->>C: [PLAN_REVIEW_DEGRADED] → ExitPlanMode
        end
    end
```

### Step 1: Tier + round budget

Determine tier (`standard` default; `--quick` / `--deep` explicit) and the round cap: read
`## Plan Review Max Rounds` from `rules/auto-loop-project.md` directly (unset → **5**). The round
counter lives **in this conversation** — state the current round in each re-review dispatch
("round 2/5") so the count survives in the transcript. There is no state file to open and no
script to run: the loop's bookkeeping is the skill's own.

### Step 2: Secret redaction (NFR-8, fail-closed)

Apply the contract from `scripts/security-redact.js` (verified API — `scanHighConfidence` returns `{name, fingerprint} | null`, it does NOT throw):

```js
const { scanHighConfidence, maskMediumConfidence } = require('./scripts/security-redact.js');
const high = scanHighConfidence(planText);   // {name, fingerprint} | null
if (high) {
  // fail-closed: plan is NOT sent to any reviewer
  // → output [PLAN_REVIEW_DEGRADED]; plan still delivered to user via ExitPlanMode
} else {
  send(maskMediumConfidence(planText));      // medium-confidence → [REDACTED] before send
}
```

Run via `node -e` against the plan text, feeding the text through **stdin with a quoted heredoc — never as an argv literal**:

```bash
node -e '...' <<'PLAN_EOF_<random-hex>'
<plan text>
PLAN_EOF_<random-hex>
```

> `<random-hex>` is a **placeholder to be generated**, not a value to copy. It is written this way
> deliberately: the rationale below is that "a fixed delimiter makes the attack a copy-paste", and
> an example carrying a concrete literal reinstates exactly that for anyone who copies rather than
> generates. Substitute a fresh suffix on every invocation, per the table that follows.

**The delimiter must be freshly randomized per invocation, and you must verify it does not collide.** Before emitting the command:

| Step | Action |
|------|--------|
| 1 | Generate a new random suffix (≥8 hex chars) → `PLAN_EOF_<suffix>` |
| 2 | Scan the plan text for any line whose **entire content** (after stripping trailing whitespace) equals that delimiter |
| 3 | On collision → regenerate and re-check. Never emit a command whose delimiter appears in the body |

Rationale: argv leaks the un-redacted plan into system-wide process listings (`ps`); heredoc stdin never appears in argv. The quoted delimiter prevents shell *interpolation* of plan content — but quoting does **not** stop the plan from **terminating** the heredoc. A plan containing a bare line `PLAN_EOF` ends the here-document early, and every line after it is handed to the shell as commands under this skill's `Bash` permission. That is arbitrary command execution driven by plan text, which in this skill is frequently drafted from untrusted material (issue bodies, PR descriptions, pasted logs). A fixed delimiter makes the attack a copy-paste; a randomized-and-checked one makes it unreachable.

The plan draft already exists in the session transcript (it is in-context text), so the heredoc adds no new exposure surface — and a temp-file path is not viable here because `Write` is unavailable in plan mode. Forbidden anti-pattern: judging high-confidence via `redact(text, {abortOnHigh: false})` return value (high is already masked, indistinguishable from medium).

### Step 3: Review dispatch (tier ladder)

| Tier | Reviewer | Loop |
|------|----------|------|
| quick | Codex MCP ×1 | 1-pass |
| **standard** (default) | Codex MCP | fix → re-review (`codex-reply`) |
| deep | `Skill("codex-brainstorm", ...)` | brainstorm termination (Nash attack/defense) |

- First Codex call: `mcp__codex__codex` with `references/codex-prompt-plan.md`. Config: `sandbox: 'read-only'`, `approval-policy: 'never'`. **Save the threadId.**
- Re-review rounds: `mcp__codex__codex-reply` with `references/review-loop-plan.md`.
- Secondary — **only under `--dual`**: Task agent (`Explore` or `strict-reviewer`), prompt follows the same independent-research mandate; runs in background, does not block the Codex gate; a late secondary P0/P1 re-opens the loop. Without the flag there is no secondary and Codex is the gate.
- **Codex unreachable → fallback carries the gate** (`@rules/auto-loop.md` § Review Dispatch): decide via `scripts/lib/review-dispatch.js` (`contract:'plan'`), record `[REVIEWER_FALLBACK] plane=plan from=codex to=contract-neutral-reviewer reason=<…> | <ISO8601>`, dispatch `contract-neutral-reviewer` via Task with `references/codex-prompt-plan.md` as the governing template (P3 = one retry on a fresh instance), and validate the raw report fail-closed with `node scripts/validate-family-sentinel.js plan` before adopting its verdict. Only when **every** carrier is exhausted does the run degrade: emit `[PLAN_REVIEW_DEGRADED]` and hand the plan to the user — that marker means "no validated verdict exists", never "a fallback reviewed it".
- The plan text is handed over as a **candidate artifact to attack** — never as "Claude's conclusion to confirm" (per `rules/codex-invocation.md`).

### Step 4: Convergence (independent budget)

Decision table applied to the conversation's own round count (never the code/doc review budget):

| # | Condition | Action |
|---|-----------|--------|
| 1 | `current_round >= max_rounds` (default 5; `auto-loop-project.md ## Plan Review Max Rounds`) | `⚠️ Plan Needs Human` + residual findings (never silently pass) — the user arbitrates, so nothing further is owed |
| 2 | No P0/P1 findings this round | `✅ Plan Ready` → trail summary → ExitPlanMode |
| 3 | Findings remain | Revise plan → re-review (continue loop) |

Plateau/fingerprint detection is V2 (OQ-9); v1 relies on the hard cap only.

### Step 5: Trail summary (FR-9 / NFR-4)

Default output before ExitPlanMode (3 columns minimum):

```markdown
## Plan Review

| Rounds | Findings | Modified sections |
|--------|----------|-------------------|
| 2      | 3 (1 P1, 2 P2) | §Approach, §Risks |

✅ Plan Ready
```

`--verbose`: append round-by-round findings. Degraded/skipped runs include the `[PLAN_REVIEW_DEGRADED]` / `[PLAN_REVIEW_SKIPPED]` token in this block.

## Graceful Degradation (NFR-3)

| Source | Action |
|--------|--------|
| Reviewer unreachable (connection error / 401 / timeout) | Max 1 retry → **fallback dispatch first** (Step 3: `contract-neutral-reviewer` + `references/codex-prompt-plan.md`, validated via `validate-family-sentinel.js plan`); only with every carrier exhausted → output `[PLAN_REVIEW_DEGRADED]` → proceed to ExitPlanMode |
| High-confidence secret in plan (Step 2) | NO reviewer send → output `[PLAN_REVIEW_DEGRADED]` → proceed to ExitPlanMode |

Degradation never blocks plan mode: the plan is always delivered to the user in the same turn, with a grep-able degradation marker.

## Sentinel Namespace (prose contracts)

| Sentinel | Meaning |
|----------|---------|
| `## Plan Review` | Section discriminator — MUST precede every plan verdict |
| `✅ Plan Ready` | Converged, no P0/P1 |
| `⛔ Plan Blocked` | P0/P1 present, loop continues |
| `⚠️ Plan Needs Human` | max_rounds reached without convergence |
| `[PLAN_REVIEW_DEGRADED]` | Reviewer unavailable or secret-detected (fail-closed) |
| `[PLAN_REVIEW_SKIPPED]` | User-intent bypass (≠ degraded) |

**No hook parses these.** They are behaviour-layer signals: grep-able in the transcript, read by the model on resume and by the human arbitrating. That is exactly why the namespace still matters —

**Forbidden**: plan-review output must NEVER contain bare `✅ Ready` / `✅ Mergeable` / `## Gate:` / bare `⛔ Blocked`. Those sentinels belong to the code/doc review planes (`rules/auto-loop.md` § Gate Sentinels), and a plan that quotes one publishes a verdict a later reader can mistake for a code or doc gate result. The prompt templates repeat this constraint to the reviewer.

**Verdict precedence when reading reviewer output**: check the machine tokens (`[PLAN_REVIEW_DEGRADED]` / `[PLAN_REVIEW_SKIPPED]`) before verdict markers — degraded/skipped output quoting a verdict in prose must not lose its flags; then `⛔ Plan Blocked` before `✅ Plan Ready`, so output containing both verdict markers reads as blocked.

## Verification

- [ ] Tier and round cap stated before first dispatch (round counter in conversation)
- [ ] Plan text passed redaction contract before any reviewer send
- [ ] Codex prompt used `references/codex-prompt-plan.md` (independent research mandate, candidate-artifact framing)
- [ ] Exactly one terminal sentinel in the final output (Ready / Needs Human / DEGRADED / SKIPPED)
- [ ] Trail summary present in final plan output
- [ ] No bare code/doc sentinels emitted

## References

- Codex first-pass prompt: `references/codex-prompt-plan.md`
- Re-review loop prompt: `references/review-loop-plan.md`
- Rules: @rules/codex-invocation.md, @rules/auto-loop.md (Gate Sentinels)
- Spec: `docs/features/plan-review-loop/2-tech-spec.md`

## Examples

```
Input: /plan-review
Action: tier standard, cap 5 → redact → Codex review → loop → ✅ Plan Ready → trail summary → ExitPlanMode

Input: /plan-review --quick
Action: tier quick → redact → single Codex pass → verdict → ExitPlanMode

Input: /plan-review --deep
Action: tier deep → redact → Skill("codex-brainstorm", plan challenge) → equilibrium → verdict

Input: user says "skip review, show me the plan"
Action: [PLAN_REVIEW_SKIPPED] → raw plan → ExitPlanMode
```