Files
gstack/plan-eng-review/sections/review-sections.md.tmpl
T
01593aa67c v1.91.2.0 fix: consolidate gstack reliability wave (#2959)
* fix(memory-ingest): --scan-secrets scans the rendered page and fails closed

--scan-secrets ran gitleaks on the raw transcript .jsonl, then imported a
page rendered from it. gitleaks' assignment rules don't match across a
JSON-escaped quote (KEY=\"v\" on disk), so a secret the rendered page
shows as KEY="v" was imported unflagged. And the gate skipped a file only
on scanner "gitleaks" with findings, so a scan that errored (non-zero
exit, 16MB maxBuffer overflow on a file with many findings, unparseable
report) or could not run (gitleaks missing, slow-probe cooldown) imported
the file unscanned.

Scan the rendered page body, the exact bytes writeStaged() writes, via a
new secretScanText() helper, and skip the file whenever the scan did not
complete. Skipped files stay out of the state file, so the next run
retries them. Reword the helper warnings and setup-gbrain/memory.md,
which described the fail-open as intended.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(test): reconcile Bun failure markers and footer counts

* fix(sync-gbrain): verify source-scoped reads without mutation

* fix(test): recognize grounded TTHW target choices structurally

* fix(aside): make the readiness probe work under zsh and report why it failed

The probe built its deadline into `_T` and expanded it unquoted, so
`$_T aside repl …` only worked in a shell that word-splits. zsh does not: it
looked for a command literally named "gtimeout 30", the probe answered
ASIDE_NOT_RUNNING with Aside installed and ready, and every browsing skill
fell back to the bundled Chromium in silence. zsh is the macOS default and
Aside is macOS-only, so on a stock Mac the probe could never report READY.

The deadline becomes a function, `_gs_d`. It receives the command as "$@",
already split, so sh, bash and zsh all behave the same, and the gtimeout →
timeout → perl alarm chain is unchanged. A 4th arm runs the call unbounded
when none of the three is present, which is what the empty `_T` did before.
Not `eval`: it re-parses the string, so the parens and `;` of the perl arm
become syntax and that arm dies in bash *and* zsh — on a stock Mac, the arm
that actually runs.

On failure the probe now prints the CLI's reason after ASIDE_NOT_RUNNING:,
the shape gstack-render already uses: the first line that starts with a
capital letter, i.e. the CLI's own sentence or Node's `Error:` line below its
loader frame. "Not running" covers states with different fixes — no window
open for the profile, a NODE_OPTIONS preload that kills the CLI — and a bare
verdict sent all of them to "open the Aside app". The BROWSER SETUP prose
quotes that reason before asking the user to open the app.

The text pin asserted the broken invocation verbatim, so it now pins the
function and asserts neither `$_T aside repl` nor an eval form comes back. A
second test executes the rendered probe in sh, bash and zsh on each of the
four deadline arms with stubbed binaries on a narrowed PATH, plus two failing
CLIs: one that prints its own sentence, one that crashes like Node with the
useful line below the frame.

The deadline function costs zero bytes against the lines it replaces; the
reason costs 53 per copy of the probe (44 where the reworded BROWSER SETUP
line gives 9 back). That moves four guards by the measured amount:
plan-devex-review's skeleton cap to 68,550 (measured 68,544), plan-ceo-review's
skeleton cap to 80,150 (measured 80,111) and union ratio to 1.081 (measured
1.0803), and plan-eng-review's union ratio to 1.151 (measured 1.1504).

Fixes #2842, #2941.

* Clarify engineering review startup and decision flow

* Fix Windows readiness fixture PATH and command shim

* fix(test): recognize grounded TTHW target choices structurally

* Clarify engineering review startup and decision flow

* fix(test): restrict QA-only fixture tools to its no-Edit contract

* v1.90.0.0 fix(sync-gbrain): guard readiness verdicts and refresh metadata

* fix(browse): validate canonical upload targets

* fix(gbrain): classify structured PGLite busy response

* fix(browse): preserve native extension runtime APIs

* Fix displayless browser handoff ownership

* Accept unique installed autoplan methodology aliases

* fix(skills): preserve positional literals during installation

* fix(browse): checksum installer contents through stdin

* fix(test): normalize Windows checksum fixture paths

* test: emulate unavailable shasum in Windows checksum fixture

* fix(investigate): preserve owned freeze lifecycle

* fix(review): preserve N+1 retry and Red Team completion

* fix: bound Aside readiness and preserve safe fallback

* test: exercise setup and Chromium on native ARM

* fix: preserve install ownership and ARM browser selection

* Fix gbrain ingest scan boundaries and seed observation

* Refresh managed ship hooks and supervise expanded paid census

* Reject resumed gbrain pages excluded by current policy

* Recover zombie agent locks safely and enable CI Python venv

* Repair paid actor declarations and Aside pitch assertions

* Bump consolidated wave to next free minor release

* Clarify CEO review admin choices and option tradeoffs

* Preserve CEO mode handoff anchors in clarified workflow

* Make Windows portability fixtures use shell-native paths

* Restore ARM Bun alias and clarify ship review gates

* Refresh ship workflow golden snapshots

* Fix Windows DX documentation controls without piped stdin

* Decode Codex child pipes without Bun's encoded-stream stall

* Bound DX pre-review audit before product questions

* Clarify trusted review-start read in paid revalidation

* Bump consolidated wave to next free minor release

* Clarify CEO review admin choices and option tradeoffs

* Preserve CEO mode handoff anchors in clarified workflow

* Make Windows portability fixtures use shell-native paths

* Restore ARM Bun alias and clarify ship review gates

* Refresh ship workflow golden snapshots

* Fix Windows DX documentation controls without piped stdin

* Decode Codex child pipes without Bun's encoded-stream stall

* Bound DX pre-review audit before product questions

* Clarify trusted review-start read in paid revalidation

* Reconcile new main planning flow and paid judge census

* fix: reconcile rebased planning and source-bound validation

* test: pin cookie workflow judge to scored Sonnet model

* fix: keep terminal agent boot out of module imports

* fix: preserve pending-question uncertainty in engineering review

* fix: stabilize Windows reliability-wave fixtures

* fix: clarify design consultation research workflow

* fix: preserve independent design consultation inputs

* fix: resolve design taste scope and browser research guidance

* fix: make consultation opt-in preflight unambiguous

* test: await native Edge owner readiness or terminal result

---------

Co-authored-by: Bruce Krysiak <brucek@alum.mit.edu>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Antonio Vitalic <antoninte99@gmail.com>
2026-09-26 18:57:53 -04:00

583 lines
30 KiB
Cheetah
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
## Review preparation
After startup, prepare in this order:
1. Select the report file and permissions under **Review record and write policy**.
2. Run **Prior Learnings** and resolve its configuration question.
3. Run **Retrospective learning** on existing target paths.
4. Read **Confidence Calibration** and **Decision procedure** as rules, not review passes.
Then run **Scope Challenge A → B → C**, followed by Sections 1–4 in order.
## Review record and write policy
- **Target:** the plan, diff or code path selected at the Scope gate. It stays fixed.
- **Working plan:** the proposed work and its current approvals. For a plan target,
start with that plan; for code, build a remedy plan from the findings. This is
review content, not permission to edit implementation or create another file.
- **Report file:** the one destination for the working plan, findings, decision
ledger and final structured report. It may be the selected plan or a separate file.
| Target | Evidence to examine |
|---|---|
| Plan or design document | Proposed paths, checked against existing interfaces and tests |
| Branch diff | Changed behavior and surrounding code, traced from entry points |
| Specific file or directory | Existing behavior and relevant callers/tests |
When Test review or Outside Voice refers to the plan, use the current working
plan and this target evidence. Trace current behavior and proposed changes
separately. Include actual decisions in Outside Voice's bounded input.
Choose the **report file** before any ledger write:
1. Use the output/report path explicitly requested by the user.
2. Otherwise use the selected plan file, if there is one.
3. Otherwise use `$GSTACK_STATE_ROOT/projects/$SLUG/$BRANCH-eng-review-{YYYYMMDD-HHMMSS}.md`, adding a suffix on collision. Obtain assignments from `~/.claude/skills/gstack/bin/gstack-paths` and `~/.claude/skills/gstack/bin/gstack-slug`; failed commands or missing values make this path unavailable.
Name the target in the report header. Never substitute an unrelated active plan
or silently replace a requested destination.
**Check each artifact and parent directory's permission before writing.** Honor
user and host limits, including active-plan-only restrictions. Permission for one
path authorizes no other; implementation edits require explicit authority.
| Artifact | Destination | If writing is forbidden |
|---|---|---|
| Working plan, ledger and complete review report | Selected report file | Ask for a permitted destination if the user can supply one; wait without completion telemetry. If none is permitted, complete the review in chat as **not persisted**, then use **Blocked outcome**. |
| QA Test Plan and task JSONL | Discovery paths below | Present each completely as **not persisted** and continue. |
| TODOS.md | The project's TODO file | Present accepted TODO content as **not persisted** and continue. |
| Required Review Log | The helper's state location | Present its fields as **not persisted**; the final gate cannot pass without this log. |
| Best-effort metadata/learning logs | Helper-defined locations | Skip forbidden writes; otherwise keep their best-effort behavior. |
QA Test Plan/task JSONL keep discovery paths `~/.gstack/projects/{slug}/`:
`{user}-{branch}-eng-review-test-plan-{datetime}.md` and
`tasks-eng-review-{datetime}.jsonl`. Keep their formats; do not relocate.
A failed permitted save uses **Recovery routing → Repairable write/read failure**,
not the forbidden-write branches above. Do not ask from an unsaved record.
Forbidden auxiliary writes allow the review to continue; unrecovered attempted
writes block it. Best-effort logs retain their stated non-blocking behavior.
Apply this policy at every later write.
{{LEARNINGS_SEARCH}}
## Retrospective learning
History paths by review target:
- Plan: named existing paths. Mark named future paths `not available`; missing
history proves nothing about proposed behavior. Never invent paths.
- Branch diff: changed files.
- File/directory: selected path.
Run `git log --oneline -- <paths>` and `git log --grep=revert --oneline -- <paths>`.
Check recurring issues and reversals.
**Plan-review evidence:** Implementation/validation steps are proposals. Calibrate
findings below: quote the motivating plan requirement (file:line) and check existing interfaces
where applicable. Do not require future code or call a proposed regression observed.
Code-specific examples concern existing code.
Bounded probes address named current-behavior/interface uncertainties. Report
evidence, limits, unknowns and future verification. Complete the review without
building proposed code. Keep suppressed findings for the output appendix.
{{CONFIDENCE_CALIBRATION}}
## Decision procedure
For Scope Challenge, Sections 1–4, Outside Voice, late changes and TODOs, finish
one choice at a time through steps 1–6.
Setup gates—Context Recovery/prerequisites, Prior Learnings configuration,
target and Scope Challenge complexity selectors—use local rules without a
pre-answer ledger. Scope Challenge B saves actual selector answers afterward,
outside this remedy loop. These answers approve no engineering remedy.
One question for one choice per AskUserQuestion call. Authorities:
- Preamble: question format, transport/fallback and authorized auto-decisions.
- Steps 1–6: substantive choices/answers; Review record/write policy: persistence.
- Entrypoint: **Paused question** for pending answers; **Blocked outcome** for missing work or failed recovery.
- Finish: Approval readiness → Required outputs → entrypoint verification.
### 1. Establish current state
Read the request, source and actual answers. Give each finding a number, severity,
confidence, file:line and reviewer. Record two separate facts:
- **Plan baseline:** the last approved value, exact scope and answer reference;
if nothing was approved, record the original proposal.
- **Runtime evidence:** what existing code or a probe shows. Mark unverified
behavior unknown.
Approval does not prove deployed behavior, and observed behavior does not grant
approval. Drafts, recommendations and reviewer agreement grant neither. For a
factual correction that changes no behavior, record the correction and evidence;
no question or comparison grid is needed.
If an exact prior approval covers the work, cite its answer and disposition.
Carry its necessary code, tests, documentation and later-discovered required
proof forward without asking again. Otherwise leave the remedy pending. Reopen
an approved choice only for a concrete new risk, contradictory evidence or a
changed assumption. Explain the reason and retain earlier values, complete
briefs and answers in History. Record remaining unknowns and uncertain risks.
### 2. Separate independent choices
Before drafting options, list each current value and proposed change: behavior,
approach, guarantee or bound. Include response timing, resources, lifetimes and
optional verification method or depth. Give each bound a measure and unit.
Give independently selectable changes separate IDs. If the user can accept one
while another stays approved or undecided, they are separate choices even in the
same finding, function or patch. A reopened choice keeps its ID and receives the
next continuous `D<N>` question number.
Keep one behavior with its necessary code, tests and documentation. Alternative
mechanisms for that fixed behavior belong in one question; independently
selectable runtime outcomes do not. Optional depths of one verification form
one choice. Separate instrumentation, follow-ups, guarantees and policies need
their own choices, and their tests wait for approval.
### 3. Compare one choice
Select one pending ID. Prepare its question in this order:
**Draft the native fields:** build `currentDecision`:
- `question`: the complete D-numbered preamble brief, including Project, ELI10,
Stakes, Recommendation and applicable completeness/net fields.
- `header`: the exact native header.
- `options`: every exact label and full description.
Put the problem and file:line in the native fields. Offer 2–3 options, including
do-nothing when reasonable; Outside Voice retains its four-option menu.
Each option must explain human/CC effort, risk and maintenance. Tie the
recommendation to the engineering preferences; prefer complete coverage when
extra CC effort is marginal. Fit headers and labels to host limits now, before
saving. Without stated limits, keep both under 5 words; details go in descriptions.
For one fixed approved contract, coverage choices vary implementation or proof
depth. Apply the preamble's Completeness scores or kind-note accordingly.
Test-review scores rate existing/proposed tests, not answer status.
**Audit the commitments.** Build a separate **comparison grid** for the whole
brief. Give every selectable
behavior, approach, guarantee or bound a row. Show its concrete current value,
each option's value and work, and any approval citation. Include shared, fixed
and pending choices.
Use these three checks for every column:
1. Vary only this choice. Keep other approved values fixed and pending choices
undecided. A value shared by all options still needs approval if it is new.
2. Treat necessary implementation and proof of an approved contract as common
work. Cite its answer instead of creating another approval row. Never cut an
established contract or its required proof.
3. An Investigate/Defer option must bound the investigation and name what stays
unchanged or pending. It approves no implementation, including a conditional
fix. Keep that remedy pending.
**Reconcile before saving.** Compare each option's full label and description
with every row in its grid column. They must make the same commitments and retain
the same conditions. Put all deliberation in the native question/descriptions;
a saved-only Pros/cons block cannot supply missing decision context. Repair
contradictions now. If you discover another independent choice, return to step 2
before sending the question.
For example, jitter and a delay cap can be chosen independently. A menu of “both / cap only / neither” bundles them by omitting “jitter only.” Ask about jitter first:
| Choice | Current | A | B |
|---|---|---|---|
| R1 jitter | unspecified, pending | on | off |
| R2 delay cap | unspecified, pending | unspecified, pending | unspecified, pending |
After the jitter answer, carry that value into both options of the later cap question.
### 4. Save the pending record
Save the record, complete grid and exact `currentDecision` in the report file,
before `## GSTACK REVIEW REPORT`. Include every native field, the recommendation
and all options. A–D record selectors are ledger notation only: if a saved label
already starts `A)`/`B)`/`C)`/`D)`, keep that one prefix; otherwise add it. Compare
the label separately from that notation by removing the selector before matching.
When revising, replace the whole current payload for this record: comparison
grid, question, header, options, state, actual answer and accepted scope. Keep
other choices' headings, content and approvals intact; move superseded payloads
to History. Do not leave duplicate Question, Header or Options fields.
```markdown
## Decision ledger
### R1: <one independently selectable choice>
Finding: <number, severity, confidence, file:line and reviewer>
Plan baseline: <last approved value, exact scope and answer reference; otherwise the original proposal>
Runtime evidence: <observed value and source/probe; unknown if unverified>
Comparison grid: <complete grid from step 3>
Question D2:
<currentDecision.question in full, including its D2 title and recommendation>
Header: <currentDecision.header>
Options:
<first option's exact label, with one A) record selector>
<first option's full description>
<second option's exact label, with one B) record selector>
<second option's full description>
State: <pending, or approved>
Actual answer: <unanswered, or actual option and answer reference>
Accepted scope: <exact approved work; none if no change approved>
History: <earlier values, briefs, answers and reason for reopening>
```
Check the Write/Edit result, then use Read to fetch the entire saved record.
Compare every native field with `currentDecision` and the whole grid with step 3.
Read after the final edit, even if Edit says the content is current in context.
Grep, chat references, summaries and planned writes do not verify the record.
Repair any difference and repeat the complete Read before asking. A failed save
blocks the question; unreadable or unverifiable records use **Recovery routing**.
On the permitted read-only route, present the complete record and grid as **not
persisted** and compare them with `currentDecision`. This can support the chat
review, but cannot pass the saved-report gate.
If any payload field changes, including a shortened label or formatting edit,
repeat step 3, replace the whole saved payload and Read it again. An older
comparison or a critic's advice cannot substitute for this verification.
### 5. Ask and wait
Use the preamble's tool resolution, failure fallback and authorized auto-decision
rules.
Send `AskUserQuestion({ questions: [currentDecision] })` after step 4. Send one
question object for one choice; other IDs wait. Copy the verified question,
header, labels and descriptions literally. Do not add or strip brief paragraphs
or rebuild options. Authorized prose and auto-decisions use this same verified
brief with the preamble's rendering and answer rules.
When Question Tuning is enabled, copying the verified question preserves its
`<gstack-qid:{question_id}>` marker.
**STOP until the actual answer arrives.** Do not apply a remedy, make another
call, start the next section or call ExitPlanMode while the choice awaits an
answer. An obvious fix still needs an answer unless exact prior approval covers it.
### 6. Apply and refresh
Read the selected saved label, full description and grid column together. Carry
all commitments, conditions, unchanged values and pending choices forward. If
they conflict or bundle independent choices, preserve the actual answer, explain
the conflict and repeat steps 2–5 for another answer. Do not reinterpret a caption,
drop a commitment or advance with conflicting approvals.
Replace the whole adjacent `State` / `Actual answer` / `Accepted scope` block
after the options. Use the actual option and answer reference. Set State to
`approved` for accepted scope or `pending` for an unresolved remedy. Each field
must occur once outside History. Preserve the options and move superseded states
to History. If older fields are separated, consolidate all three and remove their
old occurrences in the same edit; never update only the answer/scope tail.
Use a scoped Edit to save this record and only the authorized working-plan
amendments. Leave other choices unchanged. On the read-only route, present both
completely as **not persisted**.
Check the save result, then Read the entire resolution block, including State.
Verify that its unique state, actual answer and accepted scope match the complete
selected option and grid column. An answer-only search or current-in-context hint
cannot replace Read. In read-only mode, verify the presentation instead. Correct
any discrepancy before advancing; apply the write policy to failures.
Return to step 1 with the updated working plan and answer. Keep chosen values
fixed in later questions, and explain when a choice has become irrelevant rather
than asking it again. Start the next section only when no answer is pending in
this section. Keep unresolved risks and verification visible; resolve risk and
safety choices before readiness. /autoplan uses its authorized decisions and
audit trail, leaving User Challenges for its final gate.
## Scope Challenge
### A. Assess the target
Complete these checks before the complexity decision in B. Do not apply scope
changes or write findings into the plan yet.
- **What already solves each sub-problem?** Inspect helpers, libraries, callers and reusable outputs: behavior and dependency/deployment boundaries. Cite authored sources; label proposed callers with their motivating plan requirement and assumptions.
- **What minimum changes achieve the goal?** Flag work deferrable without blocking it; challenge scope creep.
- **Complexity check:** Count files and new classes/services; seek fewer moving parts. Use these counts in B.
- **Search check:** For each new architectural pattern, infrastructure component
or concurrency approach, research built-ins, current practice and pitfalls
through Aside (entrypoint readiness), one read-only request per pattern:
```bash
{{ASIDE_EXEC_PRELUDE}}
_aside_exec "Search the web for {framework} {pattern} built-in, {pattern} best practice {current year}, and {framework} {pattern} pitfalls. Read-only: do not sign in, submit, or change anything. Reply with up to 8 bullets, each with its source URL, then stop."
```
If Aside is unavailable, use host WebSearch for these queries. With neither,
skip and note: "Search unavailable — proceeding with in-distribution knowledge only."
Prefer available built-ins. Label recommendations **[Layer 1]**, **[Layer 2]**,
**[Layer 3]** or **[EUREKA]** per Search Before Building; explain departures
from standard practice.
- **TODOS cross-reference:** Read existing `TODOS.md`: what blocks this plan,
fits this PR without expanding scope, or needs a new TODO?
- **Completeness check:** Full tests, edges and errors cost 10-100x less with AI.
Prefer completeness when a shortcut saves only CC+gstack minutes. Boil the ocean.
- **Distribution check:** For new artifacts, verify build/publish CI/CD, target
OS/architectures and download/install channels. Put deferrals in "NOT in scope".
### B. Resolve complexity selectors
Below both thresholds, skip B's questions and go directly to **C. Resolve findings**.
At 8+ files or 2+ new classes/services, STOP before Section 1. Use the
preamble's decision-brief format for this complexity gate, in this order:
Initial scope selectors need no grid or **pre-answer** ledger write. Ask and
wait before changes.
1. Explain the complexity. Ask each proposed feature cut/deferral separately;
wait before changing scope. With no proposed cuts, keep the feature list and
go directly to the structure question.
2. Always ask the structure question when this gate trips, even with no cuts.
Compare only the file/class arrangement. Use labels `Original arrangement`
and `Smaller arrangement`; put files/classes in each description. Both retain
the same approved feature list, contracts and approved
security/error/test/performance fixes. Include `Pending remedies not decided here: <ids>` in the
question; unapproved fixes stay pending. If no smaller arrangement preserves
these commitments, explain that and offer confirmation of the original
arrangement or a pause to investigate a smaller one. Wait for the answer.
A pause leaves the arrangement undecided: investigate only the agreed question,
then return to this structure selector. Do not continue to C until it is settled.
3. Save the actual feature and structure answers as one scope record: `feature
answers: <refs>; structure: <A/B + ref>; accepted scope: <exact scope>;
pending remedies: <ids or none>`.
This is a post-answer scope summary, not a remedy's pending ledger record.
Save it under the write policy and Read it back against the actual answers;
on the permitted read-only route, present and verify it as **not persisted**.
Do not invent a pre-answer record afterward. A failed save or Read blocks advancement.
After verification, apply only accepted scope changes. Do not re-argue reduction
or skip approved components. Continue to **C. Resolve findings**.
### C. Resolve findings
Run C whether B was completed or skipped.
1. Present numbered Scope Challenge findings with calibrated severity, confidence
and source; use "No issues found" for an empty list.
2. Resolve each remedy through Decision procedure, reusing exact answers.
Findings and scope answers approve no remedies.
3. Report accepted/rejected/deferred/pending dispositions from those answers.
Continue to Section 1 only when no answer is pending.
## Review Sections (after scope is agreed)
Evaluate Architecture → Code Quality → Tests → Performance,
at most 8 top issues each. Never condense, abbreviate or skip a section, including
strategy/spec/infra plans. With zero findings, report "No issues found" and continue.
After each of Sections 1–4, resolve new or reopened choices through Decision
procedure, report findings and dispositions, then continue.
### 1. Architecture review
Evaluate:
* System/component boundaries, dependencies and coupling.
* Data flow, bottlenecks, scaling and single points of failure.
* Security: auth, data access and API boundaries.
* Key flows needing ASCII diagrams in plans/code.
* One realistic production failure per new path/integration; does the plan handle it?
* **Distribution architecture:** New artifacts' build, publish and update paths; included or deferred CI/CD.
### 2. Code quality review
Evaluate:
* Organization and module structure.
* Shared-code opportunities in the target and related callers, using the rubric below. No standalone history/PR sweep or quotas. Check proposed caller assumptions against existing interfaces.
* Explicitly flag error handling gaps and missing edge cases.
* Technical debt, fragility and needless complexity per engineering preferences.
* Accuracy of touched files' ASCII diagrams.
{{SHARED_LIBS_RUBRIC}}
Use Decision procedure for new/reopened extraction choices; scope approval does not approve extraction.
### 3. Test review
For shared-code changes, audit existing/missing shared-contract tests (behavior,
errors, side effects, boundaries) and each migrated caller's integration/differences.
Rejected extractions still need coverage for real duplicated-code defects.
{{TEST_COVERAGE_AUDIT_PLAN}}
After **Add missing tests to the plan** resolves test/eval decisions and the Test Plan Artifact is saved or presented, report the Test review findings and their dispositions and continue to Performance review.
### 4. Performance review
Evaluate:
* N+1/database access, memory, caching, and slow or complex paths.
{{CODEX_PLAN_REVIEW}}
### Continue after Outside Voice
Finish the Outside Voice branch. Only completed reviews enter Cross-model tension. Record the actual coverage, including disabled or unavailable outcomes, in the Completion summary, then continue below.
## Final planning decisions
After Sections 1–4 and Outside Voice, resolve the TODO choices, then check Approval readiness before Required outputs.
### TODOS.md updates
Review every potential TODO. Reuse an exact prior disposition under Decision procedure; ask about each unanswered proposal in its own AskUserQuestion. Never batch TODOs or silently skip them. Use `~/.claude/skills/gstack/review/TODOS-format.md`.
For each TODO, record **What**, **Why**, **Pros**, **Cons** (cost/complexity/risk),
**Context** (motivation, current state, where to start in 3 months), and
**Depends on / blocked by** (prerequisites/order).
Then present options: **A)** Add to TODOS.md **B)** Skip — not valuable enough **C)** Build it now in this PR instead of deferring.
Option C records accepted implementation scope; still do not edit product code.
{{PLAN_REVIEW_APPROVAL_CHECK}}
## Required outputs
After Approval readiness passes, follow this finish sequence using the reference
sections below; those references are not another review cycle.
For recovery or changed outputs, use the entrypoint's **Recovery routing**.
Reuse a successful Review Log only for unchanged saved outputs; changed outputs
must pass steps 1–4 again.
1. **Prepare the review body.** Complete the working plan, Implementation Tasks
and Completion summary below. Leave choices pending according to each record's
current State, actual answer and accepted scope. Save permitted auxiliary artifacts under the write policy.
2. **Save and Read back.** Use Plan File Review Report to save the complete body
and terminal `## GSTACK REVIEW REPORT`; pass its Read-back gate. Forbidden
persistence or an unrecovered save requires **Blocked outcome**, not logging.
3. **Log the saved review.** Run Review Log with saved Completion summary values.
If the required log is forbidden, show fields as not persisted and take **Blocked outcome**;
failures use the write policy's recovery. Neither supplies completion or saved-dashboard credit.
4. **Publish.** Display the Review Readiness Dashboard, then present the saved
Completion summary to the user.
5. **Choose navigation.** Use Next Steps — Review Chaining; wait for its answer.
Navigation grants no implementation authority. A substantive change follows
**Recovery routing → Late change or missing work** before navigation resumes.
6. **Finish.** Run Learning hooks, including gated Brain Calibration Write-Back;
then return to the entrypoint's Section self-check and read-only EXIT PLAN MODE GATE in
every host mode. Only after both pass, run success telemetry and cache refresh;
call ExitPlanMode only in host plan mode.
### Output reference — review body
Place `Suppressed findings` as a body appendix before the terminal
`## GSTACK REVIEW REPORT`; nothing follows that terminal report.
### "NOT in scope" section
List considered work that was explicitly deferred, with one sentence explaining each deferral.
### "What already exists" section
Link existing solutions and distinguish reuse/rebuilding. For accepted shared-code
choices, reference their Code Quality/Test decisions and complete rubric evidence.
Explain safer separation or net growth; never re-ask settled remedies.
### Diagrams
Diagram non-trivial flows, states and pipelines in ASCII. Name files needing inline
diagrams for complex model, service or mixin behavior.
### Failure modes
For each new diagrammed path, name a realistic production failure, its test/error
handling coverage, and whether users 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**.
### Worktree parallelization strategy
Group implementation into parallel git worktrees (`isolation: "worktree"`) or workspaces.
With one primary module or fewer than 2 independent workstreams, write:
"Sequential implementation, no parallelization opportunity."
Otherwise provide each step/workstream's **Dependency table**:
| Step | Modules touched | Depends on |
|------|----------------|------------|
| (step name) | (directories/modules, NOT specific files) | (other steps, or —) |
Use modules, not guessed files. **Parallel lanes:** disjoint modules run together;
shared modules run sequentially, dependencies later. Example:
`Lane A: step1 → step2 (shared models/)` / `Lane B: step3 (independent)`.
**Execution order:** name launch/wait points, e.g. "Launch A + B. Merge both. Then C."
**Conflict flags:** identify cross-lane shared modules; sequence or coordinate them.
{{TASKS_SECTION_EMIT:eng-review}}
### Unresolved decisions
List unanswered/interrupted choices as "Unresolved decisions that may bite you later",
with IDs and missing answers. Never silently default. Count each once, excluding
prior reviews; the terminal report adds those separately.
### Completion summary
From final decisions/outputs; publish after report Read-back and Review Log:
- 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
- Unresolved decisions: ___ in this review
- Outside voice: recorded provider, completed / unavailable / disabled / skipped (reason)
- Parallelization: ___ lanes, ___ parallel / ___ sequential
- Lake Score: X/Y = 10/10 choices / answered coverage choices. Exclude kind choices; N/A if Y=0.
{{PLAN_FILE_REVIEW_REPORT}}
## Review Log
Use these commands in finish step 3, after successful Read-back. Both logs follow the write policy: required review log, best-effort decision log.
```bash
~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-eng-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"issues_found":N,"mode":"MODE","commit":"COMMIT"}' || exit $?
~/.claude/skills/gstack/bin/gstack-decision-log '{"decision":"Eng review (MODE): ARCH_SUMMARY","rationale":"KEY_DECISION","scope":"branch","source":"skill","confidence":8}' 2>/dev/null || true
```
Second command: `ARCH_SUMMARY` = findings/dispositions; `KEY_DECISION` = durable
architecture choice. Omit it when none exists.
- **TIMESTAMP**: current ISO 8601 datetime
- **STATUS**: "clean" if `issues_found=0`, `unresolved=0` and `critical_gaps=0`; else "issues_open". Count resolved findings too; "issues_open" can mean mapped work, not failure.
- **unresolved**: this review's "Unresolved decisions" count; do not include prior reviews
- **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 for the Scope Challenge result "scope accepted as-is"; SCOPE_REDUCED for "scope reduced per recommendation".
- **COMMIT**: output of `git rev-parse --short HEAD`
{{REVIEW_DASHBOARD}}
## Next Steps — Review Chaining
In finish step 5, offer applicable routes from the published dashboard:
- **A) Run /plan-design-review:** unreviewed UI scope (frontend, CSS, views or
interactions in the diagram/findings).
- **B) Run /plan-ceo-review:** optionally, an unreviewed significant product change
(new user-facing features, changed direction or substantial scope expansion).
- **C) Ready to implement — run /ship when done**
Flag stale CEO/design reviews from contradictory assumptions or significant commit
drift. If no further review is needed or `skip_eng_review: true`, state
"All relevant reviews complete. Run /ship when ready."
AskUserQuestion with only applicable options. This is **navigation only**: copy
the working plan's prerequisites, dependencies and execution order without adding
or strengthening them. Do not serialize independent lanes. A next-step answer
approves no implementation change.
## Learning hooks
Keep the working plan/approvals fixed. Use the preamble for
operational learnings, Capture Learnings for other discoveries. Never log twice.
{{LEARNINGS_LOG}}
{{GBRAIN_SAVE_RESULTS}}
**Calibration gate status:** No supported preamble/config produces `BRAIN_CALIBRATION_WRITEBACK`. Skip unless that source explicitly enables it. Personal trust/MCP availability cannot enable it; never set it yourself.
{{BRAIN_WRITE_BACK}}