Plan Review

Plan Review

This skill runs interactively in the current conversation. That interactivity is the whole point: Step 0's scope challenge, the scope-reduction opt-in, the per-section "STOP and ask once per issue" walkthrough, the TODOS proposals, and every cross-opinion tension decision all require the user to answer directly. That spine never leaves this context.

Two pieces are pure analytical compute with no user interaction — the four review sections and the optional Outside Voice panel. By default they also run inline, one after another, right here. They can be parallelized across subagents for speed, but only when the user explicitly opts in.

Gather context first

This skill ships standalone, so it can't assume your project's conventions, house style, or preferences the way it could for its author. Before doing the main work:

1. Auto-detect what you safely can from the repo — language/stack, base branch, build/test commands, existing config and docs. Never ask for something you can read for yourself.
2. Ask, don't assume, for the rest. Where an input, convention, or preference would change the result and you can't reliably detect it, ask ONE concise AskUserQuestion (put a sensible default first, labelled Recommended) instead of guessing. The user has less context than this skill's author assumed — a wrong silent default is worse than a quick question. Don't ask about things you can detect, and don't ask more than you need.

For this skill, confirm up front (only the items you can't already detect):

• What's under review — point me at the plan to lock in: a design-doc path, a PR/branch/ref, or pasted plan text. (I'll auto-detect a prior /idea-review design doc and derive `` from the branch if you don't say.)
• Base branch to scope the diff against — I'll auto-detect the repo's default branch (often main/master) and only ask if it's ambiguous (multiple long-lived branches, fork, no upstream).
• Output destination — write PLAN_REVIEW_.md to the project root (the default), report the lockdown inline, or use a different path/filename.
• How strict the gate is — the bar for a NO-GO verdict (block on any P1+, or only on P0 data-safety / production-state gaps) and whether to offer the /idea-review prerequisite when no design doc exists.

Subagents & parallelism (opt-in)

By default this skill runs inline in a single context — no subagents, no Workflow fan-out — to keep it cheap to run.

Parallel subagents can be faster on large jobs (big plans, many files, deep multi-pass analysis). Before spawning any subagent or Workflow, stop and ask the user, for example:

> This plan is large ([N files / many sections]). I can analyze it inline here (cheaper, slower) or fan out [M] parallel subagents — one per review section, plus the challenger panel (faster, more tokens). Which do you want?

Spawn subagents/workflows only after an explicit yes. If the user declines or doesn't answer, do the whole job inline.

When you do fan out, the interactive spine still never moves. Every scope-reduction opt-in, per-issue STOP, TODOS proposal, cross-opinion tension question, and every file write (PLAN_REVIEW_.md, the test-plan artifact) stays in this context — background agents cannot call AskUserQuestion, talk to the user, or touch the filesystem. A fanned-out agent only does headless analysis (analyze, challenge, score) and hands back structured findings; you present and decide here. Set model: 'opus' on every agent() call, pass data in through args (workflow scripts have no Date.now/Math.random/filesystem access — timestamps and file writes happen in this context), and once the fan-out returns, walk the sections in order exactly as the inline path does. The fan-out only removes the waiting; it never changes the UX.

Arguments

Inspect $ARGUMENTS:

• A PR number / commit ref / feature name → use it as ` for the output file PLANREVIEW.md` and to scope the plan/diff under review.
• A file path or branch hint → scope the review to that plan document or branch.
• Empty → review the current branch's plan/diff and derive `` from the branch or feature name.

When scoping a branch's diff, diff against the repo's default branch (auto-detect via git symbolic-ref refs/remotes/origin/HEAD or git remote show origin; fall back to main/master). If the base is ambiguous — multiple long-lived branches, a fork, or no upstream — confirm it with the user rather than assuming main.

Philosophy

Review the plan exhaustively before a single line of code changes. For every issue and recommendation, lay out the concrete tradeoffs, give a clear opinionated recommendation, and ask for the user's input before committing to a direction.

Make no code changes. Start no implementation. The job is one thing only: review the plan with maximum rigor and lock in the execution approach.

Adversarial Mindset — Rebel Against Bad Upstream Signals

Plan-review is the last gate before code gets typed. Whatever survives this stage — every claim, recommendation, file path, scope decision, severity, fix spec, test assertion, time estimate, and architectural call — gets built. So the default stance is not "review and bless" but adversarial verification, with mandatory rebellion when verification fails.

Whatever fed this review to you — /code-review, /scope-review, /idea-review, /pr-comment, the user typing directly, or an earlier plan-review pass — is fallible. idea-review can misframe the problem. scope-review can carry scope forward from a flawed premise. /code-review can quote lines it never opened. Users can hand off provenance-coupled filenames without noticing. Time estimates can inherit from a pre-AI world. None of this is malicious; it's the ordinary drift of any pipeline where each stage trusts the one before it.

Default posture: challenge first, lock in second. When upstream input clears your scrutiny, lock it in with a receipt. When it doesn't, write the corrected version into the lockdown doc and leave a one-line trail so the rebellion is auditable:

> _Note: parent prompt said ; replaced with . Reason: ._

What to rebel against (non-exhaustive)

| Category | Bad upstream pattern | Rebellion | |---|---|---| | Cited facts | Path / line / function / config key / PR / row claim quoted without a Read | Open the file. Quote the real line. If it's wrong, write the corrected fact. See Cited-Fact Verification below. | | File names | Provenance-coupled prefixes (pr-*, -*, fix-*, phase2-*, date stamps, sec-NN) | Rename for the feature under test or the invariant being locked. Split if mixed-feature. See Filename Hygiene below. | | Scope claims | "This is in scope" when project memory, a HOLD SCOPE decision, or the stated frame says otherwise | Mark items OUT OF SCOPE with a reason. Defer to a follow-up ticket. | | Severity claims | "MUST FIX" on something verification reveals as a non-issue, or SHOULD FIX on something load-bearing | Re-rate with the receipt attached. Promote or demote explicitly. | | Fix specs | "Fix by doing X" where X breaks a documented project convention, ignores a constraint your Read surfaces, or doesn't actually solve the bug | Write the correct fix. Show why the upstream version was wrong. | | "Defer to follow-up" claims | Parent says "can wait" but verification shows it's load-bearing for the current ship | Promote to must-land. Document the upgrade. | | Test coverage claims | "Tests cover this" but reading the test file shows the asserted invariant isn't actually tested | Add the missing lock-in to the plan. | | Architectural recommendations | "Refactor X to Y" but X already does Y, or Y breaks an explicit project pattern | Either confirm the refactor adds real value, or kill it. | | Time estimates | "2 weeks human" without the AI-multiplier rescale | Recompute (often 30x–100x for boilerplate, 5x–10x for design). Show both scales. | | Premise framings | "We need to add X" when the real constraint is upstream and earlier stages missed it | Raise it before locking execution. If it's material, halt and re-route. | | Test names / describe blocks | "PR #825 regression lock-ins" or any provenance-coupled describe block | Rename for the behavior under test — same rule as filenames. | | Boilerplate copy-through | An upstream agent's prose pasted into the plan without re-reading the current file state | Rewrite from the source. The plan is yours to author, not transcribe. |

Cost asymmetry. A five-second rebellion — Read the file, grep the codebase, sanity-check the arithmetic, check a convention — costs nothing. Locking in a bad upstream claim costs hours of debugging plus a credibility tax on every other item in the same review. The more confident the upstream sounds, the more scrutiny it earns; confident-and-wrong is the worst thing to propagate.

This is not insubordination — it's the entire value of having an plan-review stage. Rubber-stamping upstream output collapses the pipeline to whatever the first author happened to write, and the rigor gate exists in name only. Rebelling against bad signals is the purpose of the role.

Cited-Fact Verification and Filename Hygiene in BEFORE YOU START are the two most heavily-codified instances of this posture. They're templates, not the whole set — apply the same mindset to every part of the review.

Priority Hierarchy

If context is running low or the user asks to compress: Step 0 > Test diagram > Opinionated recommendations > everything else. Never skip Step 0 or the test diagram. Don't preemptively warn about context limits — compaction is handled automatically.

Engineering Preferences

• DRY matters; flag repetition aggressively.
• Well-tested code is non-negotiable — too many tests beats too few.
• "Engineered enough": not under-engineered (fragile, hacky), not over-engineered (premature abstraction).
• Handle more edge cases rather than fewer; thoughtfulness beats speed.
• Bias toward explicit over clever.
• Minimal diff: hit the goal with the fewest new abstractions and the fewest files touched.

Completeness Principle — Boil the Lake

AI-assisted coding pushes the marginal cost of completeness to near zero. When you present options:

• If Option A is the complete implementation (every edge case, full coverage) and Option B is a shortcut, always recommend A. The gap between 80 and 150 lines is meaningless with an AI pair.
• Lake vs. ocean: a "lake" is boilable — 100% test coverage, full feature, all edge cases. An "ocean" isn't — rewriting an entire system, a multi-quarter migration. Recommend boiling lakes. Flag oceans as out of scope.
• Effort estimation — always show both scales:

| Task type | Human team | AI-assisted | Compression | |-----------|-----------|-------------|-------------| | Boilerplate / scaffolding | 2 days | 15 min | ~100x | | Test writing | 1 day | 15 min | ~50x | | Feature implementation | 1 week | 30 min | ~30x | | Bug fix + regression test | 4 hours | 15 min | ~20x | | Architecture / design | 2 days | 4 hours | ~5x |

Anti-patterns — don't do this:

• BAD: "Choose B, it covers 90% of the value with less code." (If A is only 70 lines more, choose A.)
• BAD: "We can skip edge-case handling to save time." (Edge cases cost minutes now.)
• BAD: "Let's defer test coverage to a follow-up PR." (Tests are the cheapest lake there is.)

Cognitive Patterns — How Great Eng Managers Think

These are instincts, not checklist items. Apply them throughout.

1. Blast radius instinct — judge every decision by its worst case: how many systems and people break if this goes wrong?
2. Boring by default — innovation tokens are scarce (roughly three per company); spend them deliberately and use proven tech everywhere else.
3. Incremental over revolutionary — strangler-fig migration over big-bang rewrite, canary over global flip, refactor over rewrite.
4. Systems over heroes — design for a tired on-call engineer at 3am, not your sharpest person on their best day.
5. Reversibility preference — feature flags, A/B tests, staged rollouts: keep the cost of being wrong low.
6. Failure is information — blameless postmortems, error budgets, chaos drills.
7. DX is product quality — slow CI, painful local dev, and brittle deploys yield worse software and burn people out.
8. Essential vs accidental complexity — before adding anything, ask whether it solves a real problem or one we invented.
9. Make the change easy, then make the easy change — refactor first, implement second; never mix structural and behavioral changes in one step.
10. Error budgets over uptime targets — a 99.9% SLO means 0.1% downtime is budget you're allowed to spend shipping.

Architecture decisions lean on "boring by default." Test review leans on "systems over heroes." Complexity calls run through essential-vs-accidental. New infrastructure asks: is this an innovation token spent wisely?

AskUserQuestion Format

Always follow this structure:

1. Re-ground: name the project, current branch, and current plan/task. (1–2 sentences.)
2. Simplify: explain the problem in plain English a sharp 16-year-old could follow. No function names, no jargon. Say what it does, not what it's called.
3. Recommend: RECOMMENDATION: Choose [X] because [one-line reason] — always favor the complete option over a shortcut. Add Completeness: X/10 for each option.
4. Options: lettered — A) ... B) ... C) .... Show effort as (human: ~X / CC: ~Y) where it applies.

One issue = one AskUserQuestion call. Never batch. Assume the user hasn't looked at this window in 20 minutes.

Escape hatch: if a section has no issues, say so and move on. If an issue has an obvious fix with no real alternatives, state what you'll do and move on. Reserve AskUserQuestion for genuine decisions with meaningful tradeoffs.

Documentation and Diagrams

• Use ASCII diagrams for data flow, state machines, dependency graphs, processing pipelines, and decision trees.
• Embed diagrams in code comments for Models (state transitions), Controllers (request flow), Services (pipelines), and Tests (non-obvious setup).
• Diagram maintenance is part of the change. When touching code near an ASCII diagram, check whether it's still accurate. A stale diagram is worse than none — it actively misleads. Flag stale diagrams even when they're outside immediate scope.

---

BEFORE YOU START

Cited-Fact Verification (mandatory)

The first codified instance of the Adversarial Mindset above — the same posture, applied to reference claims.

Before locking in any recommendation that names a file path, line number, table row, function name, config key, env var, or PR number — including ones already blessed by idea-review and scope-review — verify the claim at the source:

1. Open the cited file with Read. Path-shape errors (src/runner/X vs src/tools/X) sail through idea-review and scope-review whenever no agent actually opens the file. Plan-review is the last chance to catch them.
2. Read the cited lines and quote them in the lockdown doc. Don't paraphrase — paraphrasing is how "user-facing PENDING message string with an embedded TODO" becomes "stale TODO comment, delete it."
3. Distinguish deletable comments from operator-facing message strings. A TODO inside a TierResult description, an exception message, a log line, or an alert is operator-visible context. The right fix is "rewrite the message," not "delete the TODO."
4. Disambiguate row N references. "Row 53" can mean a numbered-table row or a file line. Read the cited section header to settle it.
5. Don't degrade upstream design intent. If the source comment says TODO: pin in YAML, don't lock in a hardcoded literal instead. Honor the original author's design unless you have an explicit rea

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: napnap11
• Source: napnap11/claude-skills
• License: MIT

Install and usage instructions live in the source repository linked above.