Plan review

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

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):

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:

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):

## 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