---
name: review-plan
description: >
  Review a plan file against best practices: API design, LLM code security, spec-driven
  development, test-driven development, and security architecture. Flags gaps, missing
  considerations, and anti-patterns before implementation begins. Invoke with the plan
  filename, e.g. /review-plan M2-auth-PLAN.md
allowed-tools: Read, Glob, Grep, Bash(cat *), Bash(ls *), Bash(find *)
---

# Plan Review Skill

You are reviewing the plan file **$ARGUMENTS** against established best practices.

## Pre-gathered context

### Best practices index
!`cat ~/dev/claude/BESTPRACTICES.md 2>/dev/null || echo "BESTPRACTICES.md not found"`

### Plan file to review
(Use the Read tool to read the plan file specified in $ARGUMENTS. If no filename is given, look for *-PLAN.md files in the project root and ask which one to review.)

## Best practice files to load

Read ALL of the following best practice files before starting the review:

1. `~/dev/claude/projects/best-practices/api-design.md` -- Transport security, auth, API patterns, input validation, zero-trust
2. `~/dev/claude/projects/best-practices/llm-code-security.md` -- LLM-generated code vulnerabilities, review checklists
3. `~/dev/claude/projects/best-practices/spec-driven-development.md` -- Spec structure, requirements, scenarios
4. `~/dev/claude/projects/best-practices/test-driven-development.md` -- Test derivation, edge cases, property testing
5. `~/dev/claude/projects/best-practices/security-architecture.md` -- Server boundary rule, credential proxying

Also read any additional best practice files relevant to the plan's technology stack (check the index for Kubernetes, Helm, Docker, secrets management, etc.).

## Review checklist

Evaluate the plan against each area below. For each area, report one of:
- **Covered** -- the plan explicitly addresses this
- **Partially covered** -- mentioned but lacks detail or has gaps
- **Missing** -- not addressed and should be
- **N/A** -- not relevant to this plan

### 1. Security by design

- [ ] Authentication model defined (who authenticates, how, what protocol)
- [ ] Authorization model defined (who can do what, how enforced)
- [ ] Transport security specified (TLS, mTLS, or explicit justification for plaintext)
- [ ] Secrets handling defined (how injected, never in payloads/URLs/logs, rotation plan)
- [ ] No credentials crossing server boundary to clients (server boundary rule)
- [ ] Input validation strategy at system boundaries
- [ ] Error responses don't leak internals (stack traces, paths, SQL)

### 2. API design (if the plan involves APIs)

- [ ] API versioning strategy
- [ ] Pagination on list endpoints
- [ ] Idempotency for state-changing operations
- [ ] Rate limiting considered
- [ ] Structured error responses with stable codes
- [ ] Health checks split into liveness and readiness
- [ ] Request size limits
- [ ] CORS policy (explicit origins, not wildcard)

### 3. LLM code security awareness

- [ ] Plan acknowledges that LLM-generated code needs security review
- [ ] Input validation is planned at all external boundaries (not deferred)
- [ ] Dependency versions will be verified from live sources (not LLM memory)
- [ ] Infrastructure manifests include security defaults (securityContext, NetworkPolicy, resource limits)
- [ ] No over-permissive defaults (0.0.0.0 binding, CORS *, chmod 777, verify=False)
- [ ] Secrets never hardcoded -- plan specifies how they're injected

### 4. Spec-driven development

- [ ] Plan references or will produce specs before implementation
- [ ] Subsystem boundaries identified (what gets its own spec)
- [ ] Data models and interfaces described (not just "we'll figure it out")
- [ ] Requirements are testable and unambiguous
- [ ] Scenarios included or planned (given/when/then)
- [ ] Spec dependencies mapped (which specs need to be read together)

### 5. Test-driven development

- [ ] Test strategy defined (what's tested, how, what tools)
- [ ] Tests derived from spec requirements (requirement IDs in test names)
- [ ] Edge cases and failure modes considered (not just happy path)
- [ ] Integration test plan (not just unit tests)
- [ ] Verification script planned for milestone completion

### 6. Operational readiness

- [ ] Logging and observability considered
- [ ] Deployment strategy (how it gets deployed, rollback plan)
- [ ] Configuration via environment variables (not baked in)
- [ ] Health checks and monitoring
- [ ] Backward compatibility considered (existing consumers)

## Output format

Structure your review as:

### Summary
One paragraph: overall assessment of the plan's readiness.

### Scorecard

| Area | Status | Notes |
|------|--------|-------|
| Security by design | Covered/Partial/Missing | ... |
| API design | Covered/Partial/Missing/N/A | ... |
| LLM code security | Covered/Partial/Missing | ... |
| Spec-driven development | Covered/Partial/Missing | ... |
| Test-driven development | Covered/Partial/Missing | ... |
| Operational readiness | Covered/Partial/Missing | ... |

### Critical gaps
Numbered list of issues that should be addressed before implementation begins. Include the specific best practice being violated and a concrete suggestion.

### Recommendations
Numbered list of improvements that would strengthen the plan but aren't blockers.

### What's done well
Brief acknowledgment of areas the plan handles correctly -- reinforces good patterns.