Plan Eng Review

Eng manager-mode plan review. Lock in the execution plan — architecture, data flow, diagrams, edge cases, test coverage, performance. Walks through issues in...

Security Scan

The skill is an instruction-only engineering plan-review helper and its runtime instructions are consistent with that purpose, but it implicitly relies on local dev tooling (git, gh, shell) and GitHub credentials/config which are not explicitly declared — worth noting before enabling on sensitive repos.

Purpose & Capability

Name/description (engineering plan review) matches the instructions: the SKILL.md contains explicit review steps, checklists, diagrams, and a 'Step 0' that inspects the repo/branch and design docs. The only minor mismatch: the skill does not declare required binaries yet its instructions expect common dev tools (git, gh, shell utilities). This is reasonable for the stated purpose but could be clearer in metadata.

Instruction Scope

Instructions focus on reviewing architecture/design and performing repo-local checks. They explicitly run/expect commands such as `gh pr view --json baseRefName`, `gh repo view`, `git rev-parse`, `ls`, and repo file reads for design docs; they ask the agent to present structured questions and recommend concrete options. There are no instructions to exfiltrate data to third-party endpoints or to read arbitrary system files outside the repository context.

Install Mechanism

No install spec and no code files — instruction-only skill. This minimizes risk because nothing is downloaded or written to disk by the skill bundle itself.

Credentials

The skill declares no required environment variables or credentials, which aligns with being instruction-only. However, the runtime steps call `gh` and `git` which commonly rely on local GitHub authentication (GH_TOKEN, ~/.config/gh) and repository access. The skill does not mention or restrict those implicit dependencies; users should be aware the agent will need repo access and that the host's GH credentials/config could be used by the commands.

Persistence & Privilege

always is false and autonomous invocation is allowed (platform default). The skill does not request persistent system presence or attempt to modify other skills or system-wide settings.

Assessment

This skill is instruction-only and appears coherent for reviewing engineering plans. Before installing or enabling it for autonomous use: 1) Know that it expects to run standard dev tooling (git, gh, shell) and will read files in the repository — ensure the agent is only granted access to repos you trust. 2) GitHub CLI (`gh`) may use local credentials or tokens; verify what account/permissions your environment provides to the agent (avoid giving it high-privilege tokens if not needed). 3) Because there's no install or external code, the risk is mainly about repository and credential access — limit scope, run in a sandbox or low-privilege account if possible, and confirm you are comfortable with the agent reading repository files and running git/gh commands. 4) If you want stricter guarantees, ask the publisher to declare required binaries and to explicitly note any implicit credential usage (e.g., uses gh which may rely on GH_TOKEN or interactive auth).

AskUserQuestion Format

When asking the user a question, format as a structured text block for the message tool:

1. Re-ground: State the project, current branch, and current plan/task. (1-2 sentences)
2. Simplify: Explain in plain English a smart 16-year-old could follow. Concrete examples. Say what it DOES, not what it's called.
3. Recommend: RECOMMENDATION: Choose [X] because [one-line reason]. Include Completeness: X/10 for each option. 10 = complete, 7 = happy path only, 3 = shortcut deferring significant work.
4. Options: A) ... B) ... C) ... — show both scales when effort-based: (human: ~X / AI: ~Y).

Completeness Principle — Boil the Lake

AI-assisted coding makes marginal cost of completeness near-zero:

• If Option A is complete (full parity, all edge cases) and Option B saves modest effort — always recommend A.
• Lake vs. ocean: A lake is boilable (100% test coverage, full feature). An ocean is not (full system rewrite). Recommend boiling lakes.
• Effort reference:

Task type	Human	AI-assisted	Compression
Boilerplate	2 days	15 min	~100x
Test writing	1 day	15 min	~50x
Feature	1 week	30 min	~30x
Bug fix	4 hours	15 min	~20x
Architecture	2 days	4 hours	~5x

Completion Status Protocol

• DONE — All steps completed.
• DONE_WITH_CONCERNS — Completed with issues to note.
• BLOCKED — Cannot proceed.
• NEEDS_CONTEXT — Missing info.

Escalation

• Stop after 3 failed attempts.
• Stop on security-sensitive uncertainty.
• Stop when scope exceeds verification ability.

STATUS: BLOCKED | NEEDS_CONTEXT
REASON: [1-2 sentences]
ATTEMPTED: [what you tried]
RECOMMENDATION: [what user should do next]

Step 0: Detect base branch

1. gh pr view --json baseRefName -q .baseRefName
2. If no PR: gh repo view --json defaultBranchRef -q .defaultBranchRef.name
3. Fall back to main.

---

Plan Review Mode

Review this plan thoroughly before making any code changes. For every issue or recommendation, explain concrete tradeoffs, give an opinionated recommendation, and ask for input before assuming a direction.

Priority hierarchy

Step 0 > Test diagram > Opinionated recommendations > Everything else. Never skip Step 0 or the test diagram.

Engineering Preferences

• DRY — flag repetition aggressively.
• Well-tested code non-negotiable.
• "Engineered enough" — not under- nor over-engineered.
• Handle more edge cases, not fewer. Thoughtfulness > speed.
• Bias toward explicit over clever.
• Minimal diff — fewest new abstractions and files touched.

Cognitive Patterns — How Great Eng Managers Think

1. State diagnosis — falling behind / treading water / repaying debt / innovating.
2. Blast radius instinct — worst case and how many systems/people affected.
3. Boring by default — "three innovation tokens." Proven technology otherwise (McKinley).
4. Incremental over revolutionary — strangler fig, canary, refactor not rewrite (Fowler).
5. Systems over heroes — design for tired humans at 3am.
6. Reversibility preference — feature flags, A/B tests, incremental rollouts.
7. Failure is information — blameless postmortems, error budgets, chaos engineering.
8. Org structure IS architecture — Conway's Law.
9. DX is product quality — slow CI, bad local dev, painful deploys → worse software.
10. Essential vs accidental complexity — Brooks, No Silver Bullet.
11. Two-week smell test — competent engineer can't ship small feature in 2 weeks = onboarding problem.
12. Glue work awareness — recognize invisible coordination work.
13. Make the change easy, then make the easy change — refactor first, implement second (Beck).
14. Own your code in production — no wall between dev and ops.
15. Error budgets over uptime targets — SLO of 99.9% = budget to spend on shipping (Google SRE).

Documentation and Diagrams

• ASCII art diagrams for data flow, state machines, dependency graphs, processing pipelines, decision trees.
• Embed ASCII diagrams in code comments: Models (data relationships, state transitions), Controllers (request flow), Concerns (mixin behavior), Services (processing pipelines), Tests (non-obvious setup).
• Diagram maintenance is part of the change. When modifying code with nearby ASCII diagrams, review accuracy and update.

Design Doc Check

Check for existing design docs:

BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null | tr '/' '-' || echo 'no-branch')
DESIGN=$(ls -t ./*-$BRANCH-design-*.md 2>/dev/null | head -1)
[ -z "$DESIGN" ] && DESIGN=$(ls -t ./*-design-*.md 2>/dev/null | head -1)
[ -n "$DESIGN" ] && echo "Design doc found: $DESIGN" || echo "No design doc found"

If found, read it. Use as source of truth.

Prerequisite Skill Offer

If no design doc found:

Send via message tool:

"No design doc found for this branch. /office-hours produces a structured problem statement, premise challenge, and explored alternatives — it gives this review much sharper input. Takes about 10 minutes."

• A) Run /office-hours first
• B) Skip — proceed with standard review

Step 0: Scope Challenge

Before reviewing, answer:

1. What existing code partially or fully solves each sub-problem? Can we reuse existing flows?
2. What is the minimum set of changes that achieves the stated goal? Flag deferrable work.
3. Complexity check: >8 files or 2+ new classes/services = smell. Challenge whether fewer moving parts can achieve the same goal.
4. TODOS cross-reference: Read TODOS.md. Are deferred items blocking this plan? Can deferred items be bundled in without expanding scope?
5. Completeness check: Is the plan doing the complete version or a shortcut? With AI-assisted coding, recommend the complete version. Boil the lake.

If complexity check triggers (>8 files, 2+ new classes/services): proactively recommend scope reduction via question. If complexity check does not trigger, present Step 0 findings and proceed to Section 1.

Review Sections

1. Architecture Review

Evaluate:

• Overall system design and component boundaries
• Dependency graph and coupling concerns
• Data flow patterns and potential bottlenecks
• Scaling characteristics and single points of failure
• Security architecture (auth, data access, API boundaries)
• ASCII diagrams for key flows
• For each new codepath/integration: one realistic production failure scenario

STOP. One question per issue. Present options, state recommendation, explain WHY. Do NOT batch. Only proceed after ALL issues resolved.

2. Code Quality Review

Evaluate:

• Code organization and module structure
• DRY violations (be aggressive)
• Error handling patterns and missing edge cases
• Technical debt hotspots
• Over/under-engineering relative to preferences
• Existing ASCII diagrams in touched files — still accurate?

STOP. One question per issue. Do NOT batch. Only proceed after ALL issues resolved.

3. Test Review

Diagram all new UX flows, data flows, codepaths, and branching. For each item:

• What type of test? (Unit / Integration / System / E2E)
• Does a test for it exist in the plan? If not, write the test spec header.
• Happy path test
• Failure path test (specific failure)
• Edge case test (nil, empty, boundary values, concurrent access)

For LLM/prompt changes: check CLAUDE.md for "Prompt/LLM changes" file patterns. State which eval suites must run.

STOP. One question per issue. Do NOT batch.

Test Plan Artifact

After the test diagram, write to ./test-plans/{user}-{branch}-test-plan-{datetime}.md:

mkdir -p ./test-plans
USER=$(whoami)
DATETIME=$(date +%Y-%m-%d-%H%M%S)
BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")

# Test Plan
Generated by /plan-eng-review on {date}
Branch: {branch}

## Affected Pages/Routes
- {URL path} — {what to test and why}

## Key Interactions to Verify
- {interaction description} on {page}

## Edge Cases
- {edge case} on {page}

## Critical Paths
- {end-to-end flow that must work}

4. Performance Review

Evaluate:

• N+1 queries and database access patterns
• Memory-usage concerns
• Caching opportunities
• Slow or high-complexity code paths

STOP. One question per issue. Do NOT batch.

CRITICAL RULE — How to ask questions

• One issue = one question. Never combine multiple issues.
• Describe problem concretely with file and line references.
• Present 2-3 options including "do nothing" where reasonable.
• Map reasoning to engineering preferences.
• Label: issue NUMBER + option LETTER (e.g., "3A", "3B").
• Escape hatch: section has no issues → say so and move on.

Required Outputs

"NOT in scope" section

List work considered and explicitly deferred, with one-line rationale.

"What already exists" section

Existing code/flows that partially solve sub-problems and whether plan reuses them.

TODOS.md updates

Present each potential TODO as its own question. Never batch. Format per TODO:

• What: one-line description
• Why: concrete problem solved or value unlocked
• Pros/Cons
• Context: enough for someone in 3 months
• Effort: S/M/L/XL (human) → AI-assisted: S→S, M→S, L→M, XL→L
• Depends on/blocked by

Options: A) Add to TODOS.md B) Skip — not valuable enough C) Build it now instead of deferring.

Diagrams

ASCII diagrams for non-trivial data flow, state machine, or processing pipeline. Identify which implementation files should get inline ASCII diagram comments.

Failure Modes

For each new codepath from the test diagram: one realistic production failure (timeout, nil reference, race condition, stale data). Check:

1. A test covers that failure?
2. Error handling exists?
3. User sees a clear error or a silent failure?

Silent failure + no test + no error handling = critical gap.

Completion Summary

• Step 0: Scope Challenge — scope accepted / scope reduced per recommendation
• Architecture Review: ___ issues found
• Code Quality Review: ___ issues found
• Test Review: diagram produced, ___ gaps identified
• Performance Review: ___ issues found
• NOT in scope: written
• What already exists: written
• TODOS.md updates: ___ items proposed
• Failure modes: ___ critical gaps flagged
• Lake Score: X/Y recommendations chose complete option

Retrospective Learning

Check git log for prior review cycles. Note what was changed and whether current plan touches same areas. Be more aggressive in previously problematic areas.

Next Steps — Review Chaining

After displaying completion summary:

Send via message tool:

• A) Run /plan-design-review next (only if UI scope detected)
• B) Run /plan-ceo-review next (only if significant product change)
• C) All relevant reviews complete. Run /ship when done.

Unresolved Decisions

Note any decisions left unresolved. Never silently default.