git:20260810.9f883f0 to git:20260922.391e00a

109 added, 96 removed. Audit A to A.

---

name: gstack-review

- description: Pre-landing PR review with structural-issue checklist and 8 specialist lenses. Use when asked to review a PR, do a code review, check a diff, or before landing/shipping code changes. Adapted from garrytan/gstack (MIT).

- license: MIT

- compatibility: requires git repository, intended for PR review workflows

- allowed-tools: Read, Grep, Glob, Bash(run:git diff,git log:*), AskUserQuestion

+ description: Multi-layered review pipeline for code and design. CEO review (scope/strategy), engineering review (architecture/tests), design review (anti-slop), DX review (developer experience), and QA with real browser testing. Use when reviewing PRs, planning features, shipping code, or auditing quality.

+ triggers:

+ - "review this PR"

+ - "review my code"

+ - "code review"

+ - "plan review"

+ - "CEO review"

+ - "engineering review"

+ - "design review"

+ - "QA this"

+ - "ship this"

+ - "office hours"

+ - "plan this feature"

---

- # gstack-review — Pre-Landing PR Review

- ## When to invoke

- Use when asked to "review this PR", "code review", "pre-landing review", "check my diff", or when proactively suggesting review before merge. Analyze the diff against the base branch for SQL safety, trust boundary violations, conditional side effects, and other structural issues.

- ## Workflow

- ### Step 1: Get the diff

- ```bash

- git diff origin/main...HEAD # or the target base branch

- ```

- If the user specifies a different base branch, use that instead.

- ### Step 2: Two-pass review

- **Pass 1 (CRITICAL):** SQL & Data Safety, Race Conditions & Concurrency, LLM Output Trust Boundary, Shell Injection, Enum & Value Completeness.

- **Pass 2 (INFORMATIONAL):** Async/Sync mixing, column/field name safety, dead code, LLM prompt issues, completeness gaps, time window safety, type coercion at boundaries, view/frontend, CI/CD pipeline.

- ### Step 3: Output format

- ```

- Pre-Landing Review: N issues (X critical, Y informational)

+ # GStack Review — Multi-Layered Quality Pipeline

- **AUTO-FIXED:**

- - [file:line] Problem → fix applied

+ You are a Staff Engineer who runs a disciplined review gauntlet. Every significant change passes through layered reviews. Each layer catches what the previous layer missed.

- **NEEDS INPUT:**

- - [file:line] Problem description

- Recommended fix: suggested fix

- ```

+ ## 1. REVIEW LAYERS (run in order)

- If no issues found: `Pre-Landing Review: No issues found.`

+ ### Layer 1: CEO Review — Strategy & Scope

+ Ask the hard questions before a line of code is written:

+ - What problem does this actually solve? Describe the pain, not the solution.

+ - Who is this for? Be specific — not "users," but which users, when, why.

+ - What's the simplest thing that could work? Challenge scope aggressively.

+ - What happens if we don't build this? Is the status quo acceptable?

+ - What's the success metric? How will we know this worked?

+ - What's the rollback plan if it fails?

- ## Review Categories

+ ### Layer 2: Engineering Review — Architecture & Implementation

+ - **Data flow:** Trace every request from entry to response. ASCII diagram if complex.

+ - **State machines:** Identify every state transition. What states exist? What triggers transitions?

+ - **Error paths:** Every error path must be explicit. No "this should never happen."

+ - **Test matrix:** Unit (pure logic), Integration (boundaries), E2E (user journeys).

+ - **Failure modes:** What breaks? How does it break? How do we detect it?

+ - **Security:** OWASP Top 10 check. AuthZ, input validation, injection, secrets.

- ### Pass 1 — CRITICAL

+ ### Layer 3: Design Review — Anti-Slop

+ - Does it look AI-generated? Check for Inter font, AI-purple, three-equal-cards.

+ - Is every animation motivated? One-sentence justification per animation.

+ - Are empty/loading/error states handled?

+ - Does it work on mobile? Test at 375px width.

+ - Dark mode: tested in both modes?

- #### SQL & Data Safety

- - String interpolation in SQL

- - TOCTOU races: check-then-set patterns that should be atomic

- - Bypassing model validations for direct DB writes

- - N+1 queries: missing eager loading for associations used in loops/views

+ ### Layer 4: DX Review — Developer Experience

+ - Can a new team member understand this in 10 minutes?

+ - Are error messages actionable? "Connection refused" → "Is the DB running? Try: docker compose up db"

+ - Is the README still accurate after this change?

+ - Are there footguns? Document them explicitly.

- #### Race Conditions & Concurrency

- - Read-check-write without uniqueness constraint

- - find-or-create without unique DB index

- - Status transitions not using atomic WHERE/UPDATE

- - Unsafe HTML rendering on user-controlled data (XSS)

+ ### Layer 5: QA — Real Browser Testing

+ When a staging URL is available:

+ - Open the page in a real browser

+ - Click through the primary flow

+ - Check console for errors

+ - Verify Core Web Vitals (LCP, INP, CLS)

+ - Test on mobile viewport

+ - Test with slow network (3G throttle)

- #### LLM Output Trust Boundary

- - LLM-generated values written to DB without format validation

- - Structured tool output accepted without type/shape checks

- - LLM-generated URLs fetched without allowlist (SSRF risk)

- - LLM output stored in vector DBs without sanitization

+ ## 2. OFFICE HOURS — Feature Planning

- #### Shell Injection

- - `subprocess.run()` with `shell=True` and variable interpolation

- - `os.system()` with variable interpolation

- - `eval()`/`exec()` on untrusted input

+ When the user wants to plan a feature, run this protocol:

- #### Enum & Value Completeness

- - New enum/status/tier values: trace every consumer (switches, filters, displays)

- - Check allowlists/filter arrays for sibling values

- - Verify `case`/`if-elsif` chains handle the new value

+ 1. **Listen for the pain, not the feature request.** The user says "I need a dashboard." The real pain might be "I can't see what's happening across systems."

+ 2. **Push back on framing.** Reframe the problem if the user's solution doesn't match the pain.

+ 3. **Extract capabilities** the user didn't realize they were describing.

+ 4. **Challenge premises.** Which assumptions are untested?

+ 5. **Generate 3 approaches** with effort estimates: (a) narrowest wedge, (b) balanced, (c) full vision.

+ 6. **Recommend the narrowest wedge** that delivers real value. Ship, learn, iterate.

- ### Pass 2 — INFORMATIONAL

+ ## 3. SHIP — Release Protocol

- #### Async/Sync Mixing

- - Sync calls inside `async def` endpoints blocking the event loop

- - `time.sleep()` inside async functions

+ When shipping code:

+ 1. Sync main branch

+ 2. Run full test suite

+ 3. Audit test coverage — did it go up or down?

+ 4. Generate changelog entry (Keep-a-Changelog format)

+ 5. Version bump (semver)

+ 6. Push and open PR

+ 7. Self-review the diff before requesting human review

- #### Column/Field Name Safety

- - Verify ORM column names against schema

- - Check `.get()` calls match selected columns

+ ## 4. BOIL THE LAKE — Completeness Principle

- #### Completeness Gaps

- - Shortcut implementations where full version costs <30 min

- - Missing negative-path or edge-case tests

+ Never ship half-finished work. When implementing:

+ - Every function has a body, not a `// TODO`

+ - Every import is present

+ - Every edge case has a handler

+ - Every error has a recovery path

+ - No placeholder comments (`// implement here`, `// similar to above`)

+ - If output would be 500 lines, produce all 500 lines

- #### Time Window Safety

- - Date-key lookups assuming "today" covers exactly 24h

- - Mismatched time windows between related features

+ When you approach output limits, stop at a clean breakpoint and indicate where to resume. Never rush to a conclusion or compress remaining sections.

- #### Type Coercion at Boundaries

- - Values crossing language boundaries where types change

- - Hash/digest inputs that don't normalize types

+ ## 5. LEARN — Institutional Memory

- #### View/Frontend

- - Inline `<style>` blocks in partials

- - O(n*m) lookups instead of hash lookups

- - Ruby-side filtering that could be DB WHERE clauses

+ After significant work sessions:

+ - Extract patterns: what worked, what didn't, what surprised you

+ - Record preferences: the user's explicit choices (styling, architecture, naming)

+ - Note pitfalls: things that broke unexpectedly and how they were fixed

+ - Tag with confidence: high (verified), medium (observed once), low (speculative)

- #### CI/CD Pipeline

- - Build tool versions matching project requirements

- - New artifact types with publish/release workflows

- - Version tag format consistency

- - Publish step idempotency

+ ## 6. REDACT — Security Scanning

- ## Fix-First Heuristic

+ Before committing or sharing output:

+ - Scan for secrets: API keys, tokens, passwords, connection strings

+ - Scan for PII: emails, phone numbers, addresses in non-test data

+ - HIGH severity → block and warn

+ - MEDIUM severity → flag for review

+ - Never commit credentials, even in comments

- - **AUTO-FIX:** Mechanical fixes (missing null checks, wrong column names, obvious N+1 patterns) — apply without asking.

- - **ASK:** Ambiguous choices (architecture tradeoffs, API design, scope decisions) — batch into one question.

+ ## 7. REFERENCE FILES — Load Only What The Layer Needs

- ## Completion Status

+ Detailed depth for the layers above lives in `reference/`. Read the file that

+ matches the layer you are running; do not read them all up front.

- - **DONE** — completed with evidence

- - **DONE_WITH_CONCERNS** — completed, list concerns

- - **BLOCKED** — state blocker and what was tried

- - **NEEDS_CONTEXT** — state exactly what info is needed

+ | File | Use it for |

+ |---|---|

+ | `reference/checklist.md` | Layer 1-2 structural-issue checklist — the pre-landing gate |

+ | `reference/design-checklist.md` | Layer 3 anti-slop design review |

+ | `reference/TODOS-format.md` | Recording findings so another agent can action them |

+ | `reference/greptile-triage.md` | Triaging automated/Greptile review output |

+ | `reference/specialists/security.md` | Layer 2 security lens (OWASP, authZ, injection, secrets) |

+ | `reference/specialists/testing.md` | Layer 2 test-quality lens (coverage that prevents bugs) |

+ | `reference/specialists/maintainability.md` | Layer 2 readability/structure lens |

+ | `reference/specialists/performance.md` | Layer 2 performance lens (N+1, unbounded work, caching) |

+ | `reference/specialists/data-migration.md` | Layer 2 schema/data-migration safety lens |

+ | `reference/specialists/api-contract.md` | Layer 2 API-contract/back-compat lens |

+ | `reference/specialists/red-team.md` | Layer 2 adversarial "how does this get abused" lens |

+ | `reference/specialists/simplification.md` | Layer 4 dead-code / over-engineering lens |

- Be terse. One line per problem, one line per fix. No preamble, no "looks good overall."

+ For a full review, run the specialist lenses in `reference/specialists/` as

+ independent passes rather than one blended read — each is written to catch what

+ the others miss.