gstack-plan git:20261106.eaf1701A

Immutable. This exact content never changes and is served at /api/v1/blob/9dc375055b0efe23.

---
name: gstack-plan
description: "Use before building a feature: challenge the scope, then review architecture, code quality, tests and performance, and end with a plan to build. (gstack)"
---
<!-- Generated by scripts/gen-qm-edition.ts from hosts/qm.ts and plan-eng-review/SKILL.md.tmpl, plan-eng-review/sections/review-sections.md.tmpl. Do not edit. -->

# Plan a feature: scope first, then the engineering review

Review a plan, not code: do not build the feature. If the user has not given you the plan or feature description, ask for it and stop.

**Start here.** In the repository (`cd` into it first: the shell starts in the workspace root), run with `execute` to see what already exists:

```sh
git log --oneline -10
ls
```

Then read the files the plan touches. Work through Scope Challenge, then Sections 1-4 in order; for each finding give severity, a recommendation and the alternative. Ask about decisions that change scope in one message and stop until the user answers. Finish with the revised plan: what to build, in order, the files it touches, the tests that prove it, and what is NOT in scope.

## My engineering preferences (use these to guide your recommendations):
* **Shared code:** require common behavior and improved reliability or net savings; similar-looking code alone is insufficient.
* **Tests:** every behavior tested; no test without a regression it would catch.
* **Enough engineering:** avoid fragility and premature abstraction/complexity.
* **Edge cases:** thorough handling over speed.
* **Explicit over clever.**
* **Right-sized diff:** smallest clear change; rewrite a broken foundation when necessary.
* **Data model:** storage follows the data's queries, integrity rules and update boundaries, not file count: fields queried, constrained or updated on their own get explicit columns or tables; JSON suits a document read and written whole.

## A. Assess the target

Complete these checks before proposing scope changes. Do not 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 the selected work, not files read only as evidence:
  for a plan, its proposed changed files and new classes/services; for a diff,
  changed files and classes/services introduced by that diff; for a file/directory,
  files in that selected scope and any explicitly proposed new classes/services.
  Count each once, label estimates, and seek fewer moving parts. At 8+ files or 2+ new classes or services, propose a smaller arrangement and ask before changing scope.

- **Distribution check:** For new artifacts, verify build/publish CI/CD, target
   OS/architectures and download/install channels. Put deferrals in "NOT in scope".

## Review Sections (after scope is agreed)

Evaluate Architecture → Code Quality → Tests → Performance. Never condense,
abbreviate or skip a section, including strategy/spec/infra plans.

### 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.

<!-- "Design it twice" and the red flags derive from pstack's architect skill (Lauren Tan, MIT), rewritten for gstack. See NOTICE.md. -->
### Two shapes before one (design-twice)

Run this only when the plan introduces a new module, service, package or
boundary: a public surface other code calls; otherwise say so in one line.

**Ask for the second shape.** A plan with one design usually took the first
shape that compiled in the author's head. Before judging it, require a second
shape that is structurally different: another owner for the state, another
split of responsibilities, another place for the boundary. A point fix inside
the first shape is not a second candidate. When the plan has none, sketch one
in five lines (types and signatures, bodies `not implemented`) and record it
as the alternative. Compare the candidates on interface depth: how much
behavior and policy each hides behind how small a public surface, and prefer
the one that hides more behind less. A deep module is not a deep call chain:
the chain scatters the understanding across layers, the module concentrates
it behind one interface.

**The eight red flags.** Screen the chosen shape before Section 1 closes.
Assume the next contributor is an agent that sees only the files it opened,
copies the nearest example and takes the shortest path that compiles; prefer
the design where a change that looks right from one file is right for the
whole repo. A flag is a finding with a file or symbol and a reason to revise
the shape, not a comment asking for care.

1. **Shallow module.** A large interface over little hidden complexity:
   several calls per operation, options that expose internal stages.
2. **Information leakage.** One decision (a representation, a policy, a wire
   or storage detail) lives in more than one module. Re-exported transport
   types are leakage; parse external data into domain types at the boundary.
3. **Temporal decomposition.** Modules split by execution order (load,
   validate, transform, save) instead of by the knowledge they own.
4. **Pass-through method.** A function that forwards the same arguments to
   another with the same shape; remove it or move the responsibility.
5. **Split ownership.** Two modules write the same state or each keep a copy;
   an agent editing one cannot see the other. One owner per piece of state.
6. **Two ways to do one task.** An agent copies whichever it meets first.
   Keep one; move the callers and delete the rest in the same change.
7. **Importable internals.** The shortest path that compiles imports them and
   they become interface. Make an outside import fail the build.
8. **Hand-synced list.** The same items in two places; an agent that sees one
   list edits one. Derive the second, or fail the build when they disagree.

Record under Architecture: both shapes, the pick, the depth argument for it,
and each flag with its fix.

### 2. Code quality review
Evaluate:
* Organization and module structure.
* Shared-code opportunities in the target and related callers. 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.

### 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.

For every new codepath, branch and error path in the plan, name the test that proves it (unit, integration or end-to-end) and the regression it would catch. Missing tests become plan items.

### 4. Performance review
Evaluate:
* N+1 queries and database access patterns.
* Memory usage.
* Caching opportunities.
* Slow or complex paths.

On per-request or looped paths, check queries in loops, unbounded
queries or caches, missing indexes, whole-file loads and blocking calls without
timeouts. Give each finding's scale (rows, requests/s, bytes) or mark it
unknown; never invent benchmarks.