Files
gstack/plan-design-review/sections/review-sections.md.tmpl
86091dd7f5 v1.91.109.0 fix: wave 5 — pregate secrets false negatives (#3211), Windows gstack-upgrade, env.kv schema keys (#3208), browse rule 3 (#3216), review summaries (#3219), CSO flake (#3207) (#3225)
* fix(upgrade): accept a Git Bash checkout whose toplevel Git spells C:/…

bin/gstack-upgrade compared `git rev-parse --show-toplevel` (C:/x from Git
for Windows) with `pwd -P` (/c/x in Git Bash) as strings, so every Windows
Git Bash install was refused as "not a gstack checkout", and the doctor's
recovery line points at this command. Compare the two through
gstack_path_same (bin/gstack-path-spelling.sh); a tree nested inside another
repository is still refused.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(redact): a catalog field named <schema word>_key is not a credential (#3208)

env.kv accepted every separated name ending in _key, so a catalog entry
`"input_key": "expected_media_rate_bits_per_second_ingress"` failed
pregate's never-downgradable secrets check as MEDIUM env.kv [secret]. The
camelCase twins (cacheKey, sortKey, partitionKey, idempotencyKey) were already
rejected for having no credential prefix; the snake/kebab/dotted forms of
those data-model words now are too. ALL-CAPS env names (INPUT_KEY=) and
credential-word names (signing_key, stripe_key, input_secret) still fire.

Reported by @brentyates-swx.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(cso): build the git-hardening fixture repo once per file

Each test built its fixture repo with five /usr/bin/git spawns inside
its own 5 s budget. On macOS that binary is the Xcode shim, and on a cold
runner those spawns cost far more than the code under test: in run
38800592772 the first test timed out at 5 s before readGitMetadata got
past its first lstat, and afterEach then tore the fixture down under it.

Build the repo once in beforeAll, under the same 30 s bound each fixture
spawn already declares, and give every test a fresh copy. The assertions
and the code under test are unchanged, and each test keeps its full 5 s
for the code under test.

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

* fix(browse): rule 3 treats wizard steps as writes and reads back sets before saving (#3216)

On a non-local target a wizard or dialog step (Next, Step N, a menu item
that opens a dialog) now counts as mutating until its handler or network
effect shows otherwise, and its Cancel proves nothing. Before any Save or
Submit, every programmatic set is read back and the flow stops on a
mismatch. The cookbook's silent-failures list adds the skip-path
precondition probe. Redundant clauses in rules 2, 3, 7, 8, 9, 10 and
READY are tightened so the shared setup grows by 3 bytes and every
browsing skill stays under its parity cap.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(plan-design-review): keep the Completion Summary out of the regenerated report (#3219)

The design review saved its Completion Summary inside the terminal
GSTACK REVIEW REPORT section, which the next plan review deletes from
its heading to the next "## " heading. The summary, deferred list and
unresolved decisions now go in their own "## Design Review Completion
Summary" section above the report, and the template says everything
under the report heading is regenerated by the next review.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(redact): exempt a schema field only when its name and value both say so (#3208)

Review (GPT-6.1 Sol outside voice): the name-only exemption also silenced
real secrets, e.g. Azure's cosmosdb_primary_key and any longer name such as
api_cache_key. Now the whole name must be one data-model word plus _key
(primary/secondary dropped) and the value two or more lowercase identifier
words; an entropic value, a longer name or a credential name still fires in
.env, YAML, JSON and TOML.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(upgrade): the Git shim delegates with the unshimmed PATH

A PATH-searching git wrapper found the shim again and recursed until the
test timed out. The shim now restores the host PATH before running git.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(pregate): secrets scans each file with its type and never reads an emptied diff as clean (#3211)

addedLines() flattened every file into one added.txt, so gstack-redact never
saw a .ts path and the #3048 source-expression exemption never ran: TS
properties such as `token: credentials.token,` failed the gate as MEDIUM
env.kv. Two false negatives predate this wave: the `+++` header filter
dropped added content lines that begin with "++", and `git diff` honoured
diff.external/textconv, so an external driver that prints nothing made a
branch carrying a real AWS key pass with "no added lines".

Added lines are now parsed hunk-aware per file (the pre-push hook's rules),
with --no-ext-diff --no-textconv, and each file is scanned as
<tmp>/<i>/<basename> so its type survives; findings name the file.

Reported by @brentyates-swx.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(plan-devex-review): keep the DX Scorecard out of the regenerated report (#3219)

The DX Scorecard had the same placement gap as the design Completion
Summary: with no rule, it could land inside GSTACK REVIEW REPORT, which the
next plan review deletes. It now goes in its own "## DX Review Scorecard"
section above the report.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* v1.91.109.0 fix: wave 5 — pregate secrets false negatives (#3211), Windows gstack-upgrade, env.kv schema keys (#3208), browse rule 3 (#3216), review summaries (#3219)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
Co-authored-by: Scott Gigante <scott@gethubflow.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-17 00:06:43 +00:00

286 lines
21 KiB
Cheetah

## Review Sections (7 passes, after scope is agreed)
**Anti-skip rule:** Never condense, abbreviate, or skip any review pass (1-7) regardless of plan type (strategy, spec, code, infra). Every pass in this skill exists for a reason. "This is a strategy doc so design passes don't apply" is always wrong — design gaps are where implementation breaks down. If a pass genuinely has zero findings, say "No issues found" and move on — but you must evaluate it.
**Context:** This section continues `plan-design-review/SKILL.md`. If its setup
is no longer in context, Read `~/.claude/skills/gstack/plan-design-review/SKILL.md`
for the System Audit, Design Philosophy, Step 0, Step 0.5 mockup setup (`$D`),
and Section self-check. Use their existing results; do not restart the review.
**Anti-shortcut clause:** Complete one decision cycle per unresolved finding:
explain the gap, recommend options, obtain its individual decision, then apply
the selected fix. Scope, focus, setup, and next-step choices approve no remedies.
Never use the final next-step AskUserQuestion to satisfy the issue-approval loop.
With no unresolved findings, no issue question is required.
**Carry decisions across passes.** An issue is one unresolved design requirement
or tradeoff, even when it appears in several plan locations. Before each pass,
compare the plan, DESIGN.md, and the decisions already made:
| Situation | Required action |
|-----------|-----------------|
| The exact fix already has an individual user decision or a preamble-authorized per-issue auto-decision. | Reuse that decision. Apply it to all affected references and matching tokens; do not ask again. |
| An accepted requirement needs to be copied unchanged into a required artifact, such as the journey storyboard. | Create the artifact without a separate format question. This records the requirement; it approves no new remedy. |
| The plan violates DESIGN.md or has a gap, and no individual decision has approved its fix. | Ask about that issue and wait before fixing it, even if the input names the gap or DESIGN.md prescribes the exact token. Keep the proposed remedy pending meanwhile. |
| New evidence introduces a missing requirement, a conflict, or a new tradeoff. | Name the new issue, offer alternatives, and obtain its individual decision before changing the plan. |
Writing a report, mapping a token, creating a mockup, or listing a task does not
approve a remedy. If findings exist but only navigation was answered, the review
is still waiting for its first issue decision.
{{LEARNINGS_SEARCH}}
Evaluate every pass using the plan and referenced existing contracts. An explicitly excluded artifact is not a missing requirement: assess the same dimension with permitted evidence. Reopen that exclusion only for a concrete design gap, and ask before changing it.
**Pass protocol (1-6):** Record the initial 0-10 score. Every `FIX TO 10` below is a proposal: ask about each issue, wait for approval, then edit the plan and re-rate that pass with the reason for the score change. Never edit first and ask afterward.
A declined fix remains documented: the unresolved gap keeps the rating below 10. Carry each pass's before/after scores and remaining gaps into the Completion Summary below. Compute the overall score as specified in the Completion Summary; do not treat litmus answers or Pass 7's decision counts as numeric ratings. Pass 7 is the unscored decision register, not a seventh rating.
A faithful restatement of an existing or already approved contract, such as a wireframe of the unchanged layout, is review evidence rather than a new design choice. Record it directly; ask if producing it would introduce or change a design decision. Finish with the Required Outputs and terminal Plan File Review Report below, including any unanswered decisions.
### Pass 1: Information Architecture
Rate 0-10: Does the plan define what the user sees first, second, third?
FIX TO 10: Add information hierarchy to the plan. Include ASCII diagram of screen/page structure and navigation flow. Apply "constraint worship" — if you can only show 3 things, which 3?
**STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY. If no issues, say so and move on. Do NOT proceed until user responds.
### Pass 2: Interaction State Coverage
Rate 0-10: Does the plan specify loading, empty, error, success, partial states?
FIX TO 10: Add interaction state table to the plan:
```
FEATURE | LOADING | EMPTY | ERROR | SUCCESS | PARTIAL
---------------------|---------|-------|-------|---------|--------
[each UI feature] | [spec] | [spec]| [spec]| [spec] | [spec]
```
For each state: describe what the user SEES, not backend behavior.
Empty states are features — specify warmth, primary action, context.
**STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY.
### Pass 3: User Journey & Emotional Arc
Rate 0-10: Does the plan consider the user's emotional experience?
FIX TO 10: Render the accepted journey as the required storyboard; do not ask whether to create it:
```
STEP | USER DOES | USER FEELS | PLAN SPECIFIES?
-----|------------------|-----------------|----------------
1 | Lands on page | [what emotion?] | [what supports it?]
...
```
Apply time-horizon design: 5-sec visceral, 5-min behavioral, 5-year reflective.
**STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY.
### Pass 4: AI Slop Risk
**Pass 4 evaluation:** Rate 0-10: Does the plan describe specific, intentional UI, or generic patterns? Record each hard-rejection hit and litmus YES/NO with evidence. An unresolved hard rejection caps this pass below 8 (not design-complete); it does not automatically set the score to 0. Litmus answers support findings, not a separate numeric score.
Use plan text and any available mockups as evidence for the rules below.
{{DESIGN_HARD_RULES}}
FIX TO 10: Rewrite vague UI descriptions with specific alternatives:
- "Cards with icons" → what differentiates these from every SaaS template?
- "Hero section" → what makes this hero feel like THIS product?
- "Clean, modern UI" → meaningless. Replace with actual design decisions.
- "Dashboard with widgets" → what makes this NOT every other dashboard?
If visual mockups were generated in Step 0.5, evaluate them against the AI slop blacklist above. Read each mockup image using the Read tool. Does the mockup fall into generic patterns (3-column grid, centered hero, stock-photo feel)? If so, flag it and offer to regenerate with more specific direction via `$D iterate --feedback "$(cat "$FEEDBACK_FILE")"` (the feedback in a private file, never inline); use its printed `outputPath` for any later view, check or board.
**STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY.
### Pass 5: Design System Alignment
Rate 0-10: Does the plan align with DESIGN.md?
If DESIGN.md is absent, rate the plan's explicit token and component specifications. Missing specifications remain findings; do not skip the score or assume alignment.
FIX TO 10: If DESIGN.md exists, annotate with specific tokens/components; when it has YAML front matter (the open DESIGN.md format), cite tokens by path (`{colors.primary}`, `{rounded.md}`) so the plan and the file share one vocabulary. If no DESIGN.md, flag the gap and recommend `/design-consultation`.
Flag any new component — does it fit the existing vocabulary?
Before offering a token-alignment fix, check whether an earlier pass already approved that outcome. If so, apply the established tokens and update every stale gap/reference under that decision; changing the plan location or spelling out the same fix is not a new issue. Ask again only if new evidence exposes an unresolved requirement or tradeoff, and name it. An unapproved violation still needs its first individual decision.
**STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY.
### Pass 6: Responsive & Accessibility
Rate 0-10: Does the plan specify mobile/tablet, keyboard nav, screen readers?
FIX TO 10: Add responsive specs per viewport — not "stacked on mobile" but intentional layout changes. Add a11y: keyboard nav patterns, ARIA landmarks, touch target sizes (44px min), color contrast requirements.
**STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY.
### Pass 7: Unresolved Design Decisions
Start from unresolved choices recorded in earlier passes. For each new item, cite an actual in-scope element and the missing decision in the plan, source, DESIGN.md, or approved mockup. Page/section names and outside-review suggestions do not establish that a control exists. Check the available artifacts first; if its existence is unknown, keep the item conditional and state what must be verified. Do not invent controls or reopen accepted treatments for a hypothetical element. Surface real missing decisions and concrete conflicts; approval of one treatment does not settle a different known element.
Preserve accepted user-facing outcomes. Choosing implementation mechanics does not
reopen them; ask only if a concrete constraint exposes a new design requirement
or tradeoff. Surface the remaining ambiguities that will haunt implementation:
```
DECISION NEEDED | IF DEFERRED, WHAT HAPPENS
-----------------------------|---------------------------
What does empty state look like? | Engineer ships "No items found."
Mobile nav pattern? | Desktop nav hides behind hamburger
...
```
If visual mockups were generated in Step 0.5, reference them as evidence when surfacing unresolved decisions. A mockup makes decisions concrete — e.g., "Your approved mockup shows a sidebar nav, but the plan doesn't specify mobile behavior. What happens to this sidebar on 375px?"
Each decision = one AskUserQuestion with recommendation + WHY + alternatives. Edit the plan with each decision as it's made.
**STOP.** Wait for each answer before editing or advancing. Record unanswered decisions as unresolved.
### Post-Pass: Update Mockups (if generated)
After Pass 7: offer the mockup update below when applicable, resolve deferred TODO proposals, reconcile approvals, then synthesize tasks and the Completion Summary.
If mockups were generated in Step 0.5 and review passes changed significant design decisions (information architecture restructure, new states, layout changes), offer to regenerate (one-shot, not a loop):
AskUserQuestion: "The review passes changed [list major design changes]. Want me to regenerate mockups to reflect the updated plan? This ensures the visual reference matches what we're actually building."
If yes, use `$D iterate` with feedback summarizing the changes, or `$D variants` with an updated brief; either text goes in a brief or feedback file, never inline. Save to the same `$_DESIGN_DIR` directory; nothing is overwritten, so do round accounting and carry the printed paths into any new board (rebuild it with the board block, which rewrites `board-images.json`) or check.
## How to ask questions
Follow the AskUserQuestion format from the Preamble above. Additional rules for plan design reviews:
* **One issue = one AskUserQuestion call.** Never combine multiple issues into one question. Each call contains exactly one question; multiple question tabs in one call are also batching. If a user could choose either remedy without choosing the other, ask separately, even when both affect the same component or token file. Shared implementation work does not make independent design choices inseparable.
* Describe the design gap concretely — what's missing, what the user will experience if it's not specified.
* Present 2-3 options. For each: effort to specify now, risk if deferred.
* **Map to Design Principles above.** One sentence connecting your recommendation to a specific principle.
* Label with issue NUMBER + option LETTER (e.g., "3A", "3B").
* **Zero findings:** if a section has zero findings, state "No issues, moving on" and proceed. Otherwise, use AskUserQuestion for each gap — a gap with an "obvious fix" is still a gap and still needs user approval before any change lands in the plan.
* **NEVER use AskUserQuestion to ask which variant the user prefers.** Always create a comparison board first (`$D compare --serve`) and open it in the browser. The board has rating controls, comments, remix/regenerate buttons, and structured feedback output. Use AskUserQuestion ONLY to notify the user the board is open and wait for them to finish — not to present variants inline and ask "which do you prefer?" That is a degraded experience.
## Required Outputs
### "NOT in scope" section
Design decisions considered and explicitly deferred, with one-line rationale each.
### "What already exists" section
Existing DESIGN.md, UI patterns, and components that the plan should reuse.
### TODOS.md updates
Put implementation and verification of approved fixes in the plan tasks. Do not
make in-scope verification an optional follow-up. Reserve deferred TODO proposals
for unresolved/out-of-scope debt or a new scope decision or tradeoff. After the
passes, ask about each such TODO individually; never batch. Honor explicit user
deferrals. If none remain, say so.
For design debt: missing a11y, unresolved responsive behavior, deferred empty states. Each TODO gets:
* **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.
* **Depends on / blocked by:** Any prerequisites.
Then present options: **A)** Add to TODOS.md **B)** Skip — not valuable enough **C)** Build it now in this PR instead of deferring.
Before synthesizing tasks or the completion summary, perform the approval
reconciliation from the Section self-check in `~/.claude/skills/gstack/plan-design-review/SKILL.md` (Read it if no longer in context). Export only agreed implementation work; retain unapproved remedies as pending findings.
Count only individually approved new decisions in "Decisions made" and the
review log; a proposed remedy or next-step answer contributes zero.
{{TASKS_SECTION_EMIT:design-review}}
### Completion Summary
Prepare this for the saved review; announce completion after the Read-back gate below.
In the plan file, write it as its own `## Design Review Completion Summary` section
(the box, the deferred list and the Unresolved Decisions below) above the terminal
`## GSTACK REVIEW REPORT`, replacing this review's earlier one. Never write it inside
the report: the next plan review deletes and regenerates everything under that heading.
**Overall design score:** use the lowest of the six rated pass scores (1-6),
separately before and after approved fixes. Pass 7 is unscored. Keep Step 0's
initial impression in its own row. An overall 8+ therefore means every rated
pass is 8+; unresolved findings still prevent a clean review log.
```
+====================================================================+
| DESIGN PLAN REVIEW — COMPLETION SUMMARY |
+====================================================================+
| System Audit | [DESIGN.md status, UI scope] |
| Step 0 | [initial rating, focus areas] |
| Pass 1 (Info Arch) | ___/10 → ___/10 after fixes |
| Pass 2 (States) | ___/10 → ___/10 after fixes |
| Pass 3 (Journey) | ___/10 → ___/10 after fixes |
| Pass 4 (AI Slop) | ___/10 → ___/10 after fixes |
| Pass 5 (Design Sys) | ___/10 → ___/10 after fixes |
| Pass 6 (Responsive) | ___/10 → ___/10 after fixes |
| Pass 7 (Decisions) | ___ resolved, ___ deferred |
+--------------------------------------------------------------------+
| NOT in scope | written (___ items) |
| What already exists | written |
| TODOS.md updates | ___ items proposed |
| Approved Mockups | ___ generated, ___ approved |
| Decisions made | ___ added to plan |
| Decisions deferred | ___ (listed below) |
| Overall design score | ___/10 → ___/10 |
+====================================================================+
```
After Read-back, if all passes 8+: "Plan is design-complete. Run /design-review after implementation for visual QA."
If any below 8: note what's unresolved and why (user chose to defer).
### Unresolved Decisions
List every unresolved finding here, including a finding not yet asked or an unanswered AskUserQuestion. Never silently default to an option.
### Approved Mockups
If visual mockups were generated during this review, add to the plan file:
```
## Approved Mockups
| Screen/Section | Mockup Path | Direction | Notes |
|----------------|-------------|-----------|-------|
| [screen name] | [absolute $_DESIGN_DIR]/[filename].png | [brief description] | [constraints from review] |
```
Include the full path to each approved mockup (the variant the user chose), a one-line description of the direction, and any constraints. The implementer reads this to know exactly which visual to build from. These persist across conversations and workspaces. If no mockups were generated, omit this section.
{{PLAN_FILE_REVIEW_REPORT}}
## Review Log
When a plan/report file is in scope, persist only after its successful write and Read-back
above. On failure, report the error and stop; do not log completion or an accepted decision.
**PLAN MODE EXCEPTION — ALWAYS RUN after verification:** these commands write review
metadata to `~/.gstack/`; the following dashboard reads the saved result.
```bash
~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-design-review","timestamp":"TIMESTAMP","status":"STATUS","initial_score":N,"overall_score":N,"unresolved":N,"decisions_made":N,"commit":"COMMIT"}'
```
Substitute values from the Completion Summary:
- **TIMESTAMP**: current ISO 8601 datetime
- **STATUS**: "clean" if overall score 8+ AND 0 unresolved; otherwise "issues_open"
- **initial_score**: initial overall design score before fixes (0-10)
- **overall_score**: final overall design score after fixes (0-10)
- **unresolved**: number of unresolved design decisions
- **decisions_made**: number of design decisions added to the plan
- **COMMIT**: output of `git rev-parse --short HEAD`
{{REVIEW_DASHBOARD}}
{{LEARNINGS_LOG}}
{{GBRAIN_SAVE_RESULTS}}
{{BRAIN_WRITE_BACK}}
{{BRAIN_CACHE_REFRESH}}
## Next Steps — Review Chaining
After displaying the Review Readiness Dashboard, recommend the next review(s) based on what this design review discovered. Read the dashboard output to see which reviews have already been run and whether they are stale.
**Recommend /plan-eng-review if eng review is not skipped globally** — check the dashboard output for `skip_eng_review`. If it is `true`, eng review is opted out — do not recommend it. Otherwise, eng review is the required shipping gate. If this design review added significant interaction specifications, new user flows, or changed the information architecture, emphasize that eng review needs to validate the architectural implications. If an eng review already exists but the commit hash shows it predates this design review, note that it may be stale and should be re-run.
**Consider recommending /plan-ceo-review** — but only if this design review revealed fundamental product direction gaps. Specifically: if the overall design score started below 4/10, if the information architecture had major structural problems, or if the review surfaced questions about whether the right problem is being solved. AND no CEO review exists in the dashboard. This is a selective recommendation — most design reviews should NOT trigger a CEO review.
**If both are needed, recommend eng review first** (required gate).
**Recommend design exploration skills when appropriate** — /design-shotgun and /design-html
produce design artifacts (mockups, HTML previews), not application code. They belong in
plan mode alongside reviews. If this design review found visual issues that would benefit
from exploring new directions, recommend /design-shotgun. If approved mockups exist and
need to be turned into working HTML, recommend /design-html.
Use AskUserQuestion to present the next step. Always include the manual/stop
option E; offer only applicable follow-on skills. If the user chooses manual,
finish without starting another skill:
- **A)** Run /plan-eng-review next (required gate)
- **B)** Run /plan-ceo-review (only if fundamental product gaps found)
- **C)** Run /design-shotgun — explore visual design variants for issues found
- **D)** Run /design-html — generate Pretext-native HTML from approved mockups
- **E)** Skip — I'll handle next steps manually
## 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.
* Pause for each unresolved issue. If a pass has none, say so and continue; do not manufacture a question.
* Rate before and after each pass for scannability.