--- name: plan-eng-review description: "Eng manager-mode plan review. Lock in the execution plan — architecture, data flow, diagrams, edge cases, test coverage, performance. Walks through issues interactively with opinionated recommendati" --- --- ## CRITICAL RULE — How to ask questions Follow the question format from the Preamble above. Additional rules for plan reviews: * **One issue = one question call.** Never combine multiple issues into one question. * Describe the problem concretely, with file and line references. * Present 2-3 options, including "do nothing" where that's reasonable. * For each option, specify in one line: effort (human: ~X / CC: ~Y), risk, and maintenance burden. If the complete option is only marginally more effort than the shortcut with CC, recommend the complete option. * **Map the reasoning to my engineering preferences above.** One sentence connecting your recommendation to a specific preference (DRY, explicit > clever, minimal diff, etc.). * Label with issue NUMBER + option LETTER (e.g., "3A", "3B"). * **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 — don't waste a question on it. Only use question when there is a genuine decision with meaningful tradeoffs. ## Required outputs ### "NOT in scope" section Every plan review MUST produce a "NOT in scope" section listing work that was considered and explicitly deferred, with a one-line rationale for each item. ### "What already exists" section List existing code/flows that already partially solve sub-problems in this plan, and whether the plan reuses them or unnecessarily rebuilds them. ### TODOS.md updates After all review sections are complete, present each potential TODO as its own individual question. Never batch TODOs — one per question. Never silently skip this step. Follow the format in `.claude/skills/review/TODOS-format.md`. For each TODO, describe: * **What:** One-line description of the work. * **Why:** The concrete problem it solves or value it unlocks. * **Pros:** What you gain by doing this work. * **Cons:** Cost, complexity, or risks of doing it. * **Context:** Enough detail that someone picking this up in 3 months understands the motivation, the current state, and where to start. * **Depends on / blocked by:** Any prerequisites or ordering constraints. Then present options: **A)** Add to TODOS.md **B)** Skip — not valuable enough **C)** Build it now in this PR instead of deferring. Do NOT just append vague bullet points. A TODO without context is worse than no TODO — it creates false confidence that the idea was captured while actually losing the reasoning. ### Diagrams The plan itself should use ASCII diagrams for any non-trivial data flow, state machine, or processing pipeline. Additionally, identify which files in the implementation should get inline ASCII diagram comments — particularly Models with complex state transitions, Services with multi-step pipelines, and Concerns with non-obvious mixin behavior. ### Failure modes For each new codepath identified in the test review diagram, list one realistic way it could fail in production (timeout, nil reference, race condition, stale data, etc.) and whether: 1. A test covers that failure 2. Error handling exists for it 3. The user would see a clear error or a silent failure If any failure mode has no test AND no error handling AND would be silent, flag it as a **critical gap**. ### Completion summary At the end of the review, fill in and display this summary so the user can see all findings at a glance: - Step 0: Scope Challenge — ___ (scope accepted as-is / 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 to user - Failure modes: ___ critical gaps flagged - Outside voice: ran (codex/claude) / skipped - Lake Score: X/Y recommendations chose complete option ## Retrospective learning Check the git log for this branch. If there are prior commits suggesting a previous review cycle (e.g., review-driven refactors, reverted changes), note what was changed and whether the current plan touches the same areas. Be more aggressive reviewing areas that were previously problematic. ## Formatting rules * NUMBER issues (1, 2, 3...) and LETTERS for options (A, B, C...). * Label with NUMBER + LETTER (e.g., "3A", "3B"). * One sentence max per option. Pick in under 5 seconds. * After each review section, pause and ask for feedback before moving on. ## Review Log After producing the Completion Summary above, persist the review result. **PLAN MODE EXCEPTION — ALWAYS RUN:** This command writes review metadata to `~/.gstack/` (user config directory, not project files). The skill preamble already writes to `~/.gstack/sessions/` and `~/.gstack/analytics/` — this is the same pattern. The review dashboard depends on this data. Skipping this command breaks the review readiness dashboard in /ship. ```bash ${GSTACK_OPENCODE_DIR}/bin/gstack-review-log '{"skill":"plan-eng-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"issues_found":N,"mode":"MODE","commit":"COMMIT"}' ``` Substitute values from the Completion Summary: - **TIMESTAMP**: current ISO 8601 datetime - **STATUS**: "clean" if 0 unresolved decisions AND 0 critical gaps; otherwise "issues_open" - **unresolved**: number from "Unresolved decisions" count - **critical_gaps**: number from "Failure modes: ___ critical gaps flagged" - **issues_found**: total issues found across all review sections (Architecture + Code Quality + Performance + Test gaps) - **MODE**: FULL_REVIEW / SCOPE_REDUCED - **COMMIT**: output of `git rev-parse --short HEAD` ## Review Readiness Dashboard After completing the review, read the review log and config to display the dashboard. ```bash ${GSTACK_OPENCODE_DIR}/bin/gstack-review-read ``` Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, adversarial-review, codex-review, codex-plan-review). Ignore entries with timestamps older than 7 days. For the Adversarial row, show whichever is more recent between `adversarial-review` (new auto-scaled) and `codex-review` (legacy). For Design Review, show whichever is more recent between `plan-design-review` (full visual audit) and `design-review-lite` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display: ``` +====================================================================+ | REVIEW READINESS DASHBOARD | +====================================================================+ | Review | Runs | Last Run | Status | Required | |-----------------|------|---------------------|-----------|----------| | Eng Review | 1 | 2026-03-16 15:00 | CLEAR | YES | | CEO Review | 0 | — | — | no | | Design Review | 0 | — | — | no | | Adversarial | 0 | — | — | no | | Outside Voice | 0 | — | — | no | +--------------------------------------------------------------------+ | VERDICT: CLEARED — Eng Review passed | +====================================================================+ ``` **Review tiers:** - **Eng Review (required by default):** The only review that gates shipping. Covers architecture, code quality, tests, performance. Can be disabled globally with \`gstack-config set skip_eng_review true\` (the "don't bother me" setting). - **CEO Review (optional):** Use your judgment. Recommend it for big product/business changes, new user-facing features, or scope decisions. Skip for bug fixes, refactors, infra, and cleanup. - **Design Review (optional):** Use your judgment. Recommend it for UI/UX changes. Skip for backend-only, infra, or prompt-only changes. - **Adversarial Review (automatic):** Auto-scales by diff size. Small diffs (<50 lines) skip adversarial. Medium diffs (50–199) get cross-model adversarial. Large diffs (200+) get all 4 passes: Claude structured, Codex structured, Claude adversarial subagent, Codex adversarial. No configuration needed. - **Outside Voice (optional):** Independent plan review from a different AI model. Offered after all review sections complete in /plan-ceo-review and /plan-eng-review. Falls back to Claude subagent if Codex is unavailable. Never gates shipping. **Verdict logic:** - **CLEARED**: Eng Review has >= 1 entry within 7 days with status "clean" (or \`skip_eng_review\` is \`true\`) - **NOT CLEARED**: Eng Review missing, stale (>7 days), or has open issues - CEO, Design, and Codex reviews are shown for context but never block shipping - If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED **Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale: - Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash - For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review" - For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection" - If all reviews match the current HEAD, do not display any staleness notes ## Plan File Review Report After displaying the Review Readiness Dashboard in conversation output, also update the **plan file** itself so review status is visible to anyone reading the plan. ### Detect the plan file 1. Check if there is an active plan file in this conversation (the host provides plan file paths in system messages — look for plan file references in the conversation context). 2. If not found, skip this section silently — not every review runs in plan mode. ### Generate the report Read the review log output you already have from the Review Readiness Dashboard step above. Parse each JSONL entry. Each skill logs different fields: - **plan-ceo-review**: \`status\`, \`unresolved\`, \`critical_gaps\`, \`mode\`, \`scope_proposed\`, \`scope_accepted\`, \`scope_deferred\`, \`commit\` → Findings: "{scope_proposed} proposals, {scope_accepted} accepted, {scope_deferred} deferred" → If scope fields are 0 or missing (HOLD/REDUCTION mode): "mode: {mode}, {critical_gaps} critical gaps" - **plan-eng-review**: \`status\`, \`unresolved\`, \`critical_gaps\`, \`issues_found\`, \`mode\`, \`commit\` → Findings: "{issues_found} issues, {critical_gaps} critical gaps" - **plan-design-review**: \`status\`, \`initial_score\`, \`overall_score\`, \`unresolved\`, \`decisions_made\`, \`commit\` → Findings: "score: {initial_score}/10 → {overall_score}/10, {decisions_made} decisions" - **codex-review**: \`status\`, \`gate\`, \`findings\`, \`findings_fixed\` → Findings: "{findings} findings, {findings_fixed}/{findings} fixed" All fields needed for the Findings column are now present in the JSONL entries. For the review you just completed, you may use richer details from your own Completion Summary. For prior reviews, use the JSONL fields directly — they contain all required data. Produce this markdown table: \`\`\`markdown ## GSTACK REVIEW REPORT | Review | Trigger | Why | Runs | Status | Findings | |--------|---------|-----|------|--------|----------| | CEO Review | \`/plan-ceo-review\` | Scope & strategy | {runs} | {status} | {findings} | | Codex Review | \`/codex review\` | Independent 2nd opinion | {runs} | {status} | {findings} | | Eng Review | \`/plan-eng-review\` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | \`/plan-design-review\` | UI/UX gaps | {runs} | {status} | {findings} | \`\`\` Below the table, add these lines (omit any that are empty/not applicable): - **CODEX:** (only if codex-review ran) — one-line summary of codex fixes - **CROSS-MODEL:** (only if both Claude and Codex reviews exist) — overlap analysis - **UNRESOLVED:** total unresolved decisions across all reviews - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". ### Write to the plan file **PLAN MODE EXCEPTION — ALWAYS RUN:** This writes to the plan file, which is the one file you are allowed to edit in plan mode. The plan file review report is part of the plan's living status. - Search the plan file for a \`## GSTACK REVIEW REPORT\` section **anywhere** in the file (not just at the end — content may have been added after it). - If found, **replace it** entirely using the Edit tool. Match from \`## GSTACK REVIEW REPORT\` through either the next \`## \` heading or end of file, whichever comes first. This ensures content added after the report section is preserved, not eaten. If the Edit fails (e.g., concurrent edit changed the content), re-read the plan file and retry once. - If no such section exists, **append it** to the end of the plan file. - Always place it as the very last section in the plan file. If it was found mid-file, move it: delete the old location and append at the end. ## Next Steps — Review Chaining After displaying the Review Readiness Dashboard, check if additional reviews would be valuable. Read the dashboard output to see which reviews have already been run and whether they are stale. **Suggest /plan-design-review if UI changes exist and no design review has been run** — detect from the test diagram, architecture review, or any section that touched frontend components, CSS, views, or user-facing interaction flows. If an existing design review's commit hash shows it predates significant changes found in this eng review, note that it may be stale. **Mention /plan-ceo-review if this is a significant product change and no CEO review exists** — this is a soft suggestion, not a push. CEO review is optional. Only mention it if the plan introduces new user-facing features, changes product direction, or expands scope substantially. **Note staleness** of existing CEO or design reviews if this eng review found assumptions that contradict them, or if the commit hash shows significant drift. **If no additional reviews are needed** (or `skip_eng_review` is `true` in the dashboard config, meaning this eng review was optional): state "All relevant reviews complete. Run /ship when ready." Use question with only the applicable options: - **A)** Run /plan-design-review (only if UI scope detected and no design review exists) - **B)** Run /plan-ceo-review (only if significant product change and no CEO review exists) - **C)** Ready to implement — run /ship when done ## Unresolved decisions If the user does not respond to an question or interrupts to move on, note which decisions were left unresolved. At the end of the review, list these as "Unresolved decisions that may bite you later" — never silently default to an option.