---
name: plan-review-eng
description: Use when reviewing the implementation plan for architecture, data flow, edge cases, and test coverage before any code is written.
---
## Triggers
- eng plan review
- review the architecture
- review architecture
- engineering review
- lock in the plan
- tech review
- technical review
- plan engineering review
- arch review
- review implementation plan
# Plan Review — Engineering mode
## Overview
The plan has been through CEO and design review. Scope is locked,
shape is locked. This review's job: catch the architecture issues
that would make the plan painful to build, scary to ship, or
expensive to maintain.
This is the *third* review in the autoplan pipeline (CEO → design
→ eng). Run it after scope and shape are settled, not before — eng
review of a plan that's about to be re-scoped is wasted.
**Do not re-litigate scope.** If you find a fundamental scope
problem at this stage, escalate it to the user as a blocker rather
than silently rewriting it.
## Six dimensions to score (0–10)
For each, one-sentence "what would make it a 10," then the score.
1. **Architecture clarity** — does the plan name every component
and the boundary between them? Could a stranger draw the diagram
from the plan alone?
2. **Data flow** — for each user-facing action, is the path
traced through every layer (request → auth → business logic →
storage → response)? Are state transitions explicit?
3. **Edge cases** — what happens on partial failure, network
timeout, duplicate request, expired auth, race condition,
empty input, oversized input? At least 5 named edge cases per
non-trivial component.
4. **Test coverage** — are there testable acceptance criteria for
each shipped behaviour? What's the smallest reproduction for
the bug that would convince us it's broken?
5. **Reversibility** — can changes be rolled back without data
loss? Is the schema backwards-compatible? Are deploys staged?
6. **Operability** — what would the on-call person need to
diagnose this at 3am? Logs, metrics, runbook, structured
errors with codes?
Below 7 on any dimension ➜ patch the plan before approval.
## What you must add to the plan
If missing, fill these in directly (don't just flag them):
- **Module map** — list of files / new packages and what each
owns. Reference existing files in the repo by path so callers
can be located.
- **Schema diff** — table-level changes, indexes, constraints,
migration order, backfill plan.
- **Failure modes** — bullet list of named failures + the agent's
intended response (retry / fail-loud / silent-skip / escalate).
- **Acceptance criteria** — testable statements that match the
CEO/design layer's "demand reality." Not "the function returns,"
but "the user sees X within Y seconds."
- **Rollback** — exact steps to revert. If "we'd just push another
commit," say so explicitly.
## How to use
1. Read the plan (path, text, or `goal_status` of the active goal).
2. Pull `knowledge_search(scope="system")` for the project's
architecture/conventions docs and any previously-learned
`scope="learned"` lessons (we don't want to repeat known
mistakes).
3. Walk the six dimensions.
4. Either (a) patch the plan in place, or (b) return a structured
diff + list of escalations.
When called from `plan_autoplan`, return JSON matching the schema
the tool expects. When called interactively, narrative is fine.
## Hard rules
- Don't add a new dependency without naming the alternative we
considered and rejected.
- Don't change a public API without a deprecation path.
- Don't propose a schema change without a backfill / rollback
plan.
- Don't paper over a known scope issue with engineering rigor —
escalate to the user.
## What this skill does NOT do
- Re-litigate scope (CEO review's job — escalate instead).
- Critique UI/UX (design review's job).
- Write code or implement.
## Verify
- The deliverable for this phase exists as a concrete artifact (doc, ticket, board, repo) and its location is shared, not described
- Each commitment has an owner name, a due date, and a definition-of-done that someone other than the author could check
- Risks are listed with likelihood/impact and a named mitigation, not as a generic 'risks: TBD' bullet
- Dependencies on other teams/vendors/agents are explicit; an ack from each dependency is recorded or marked 'pending'
- Success criteria for the next phase are numeric or otherwise objectively testable
- A rollback / kill-switch / 'we will stop if X' criterion is written down before work starts