gstack-review: Multi-Perspective Code Review
Review Framework
When asked to review code, follow this three-perspective framework. Run all steps systematically.
Step 0: Detect What to Review
Priority order for review scope:
- Uncommitted changes (
git diff HEAD) — if working directory is dirty
- Specific files mentioned by user
- Branch diff vs main — if on a feature branch
- Recent commits — if no clear scope
# Check working tree status
git status --short
# Get uncommitted changes
git diff HEAD
# Get committed changes on current branch vs origin/main
BRANCH=$(git branch --show-current 2>/dev/null || git rev-parse --abbrev-ref HEAD 2>/dev/null)
MAIN_BRANCH=$(git main-branch 2>/dev/null || echo "main")
git log ${MAIN_BRANCH}..${BRANCH} --oneline 2>/dev/null | head -20
git diff ${MAIN_BRANCH}..${BRANCH} 2>/dev/null | head -500
# List changed files
git diff --name-only ${MAIN_BRANCH}..${BRANCH} 2>/dev/null
Step 1: Gather Context
Before reviewing, collect:
# Project type and language
ls *.json *.toml *.yaml *.gradle *.xml Makefile package.json 2>/dev/null | head -5
cat package.json 2>/dev/null | grep '"name"\|"scripts"' | head -5
# Run tests (silently, capture exit code)
TEST_OUTPUT=$(npm test 2>&1 || pytest 2>&1 || cargo test 2>&1 || true)
TEST_EXIT=$?
# Type check / lint
LINT_OUTPUT=$(npm run lint 2>&1 || npx tsc --noEmit 2>&1 || true)
# Build check
BUILD_OUTPUT=$(npm run build 2>&1 || cargo build 2>&1 || true)
BUILD_EXIT=$?
Step 2: Read Changed Files
For each changed file, read the full content to understand context. Don't just look at the diff — read the surrounding code.
For each file in the diff:
1. Read the full file (not just changed lines — context matters)
2. Identify what the code actually does vs. what the diff claims
3. Note any files that are only binary/generated (skip detailed review)
Step 3: CEO / Product Perspective
Ask: Does this code serve the business and users?
Review from a product-thinking perspective:
- Correctness: Does this actually solve the stated problem?
- Complexity: Is this 10x simpler than the previous approach? Or did we add an abstraction layer that solves nothing?
- Product signal: Is this a feature users asked for, or engineer-invented complexity?
- Scope creep: Did the diff grow beyond its original purpose?
- Tech vs business: Is this engineering for its own sake, or does it genuinely ship value?
Key question: "If I had to explain this change to the CEO in 30 seconds, would they be excited or confused?"
Step 4: Engineering Perspective
Ask: Is this code sound, maintainable, and safe?
- Architecture: Does this fit the existing patterns? Or did we invent a new framework within a framework?
- Error handling: Are all failure modes handled? (network, disk, invalid input, timeouts)
- Resource management: Connections closed? Memory leaked? Background tasks cancelled?
- Dependencies: Did we add a new heavy dependency when a stdlib call would suffice?
- Security: SQL injection, XSS, auth bypass, secrets in code, overly permissive CORS?
- Performance: N+1 queries? Unindexed queries in loops? Memory-allocating operations in hot paths?
- API design: Are interfaces clean and composable, or did we leak internal state?
Code quality signals:
- Functions under 30 lines? Exceptions are allowed but need strong justification.
- Meaningful variable/function names? No
temp2_final_v3 patterns.
- Comments that explain why, not what? (the code shows what)
- Tests that test behavior, not implementation?
Step 5: QA / Testing Perspective
Ask: Would this pass a senior engineer's gut check for correctness?
- Test coverage: Are the changed paths actually tested? Not just "tested" but validated?
- Edge cases: Empty input, max length, null bytes, unicode edge cases, concurrent access
- Happy path: Does the main user flow actually work end-to-end?
- Failure modes: What breaks when this code is wrong? Can a user detect it?
- Smoke tests: Does it at least import/parse/load without crashing?
- Test quality: Are tests asserting on behavior or mocking everything and asserting on internals?
Step 6: Assemble the Review
Present a structured review with three clearly labeled sections.
Review Output Template
# Code Review: [BRANCH_NAME] — [DATE]
═══════════════════════════════════════════════
## Summary
[One paragraph: what changed and why. If this were a commit message, is it a good one?]
**Files changed:** N files | **Lines:** +N -N
**Tests:** [PASS/FAIL/NONE] | **Build:** [PASS/FAIL/NONE] | **Lint:** [PASS/FAIL/NONE]
---
## 🏛️ CEO / Product Review
[Bulleted findings. Flag concerns in 🔴, praise good decisions in ✅]
**Verdict:** [Clear statement — ship it, rework it, or discuss with the team]
---
## ⚙️ Engineering Review
[Bulleted findings. Group by category: Architecture, Security, Performance, Code Quality]
**Verdict:** [Clear statement]
---
## 🧪 QA / Testing Review
[Bulleted findings. Group by: Coverage, Edge Cases, Correctness]
**Verdict:** [Clear statement]
---
## Action Items
- [ ] [Priority] [Specific actionable item — who should fix it and how]
- [ ] [Priority] ...
**Overall:** ✅ APPROVED TO SHIP / ⚠️ REVISIONS NEEDED / ❌ BLOCKED
═══════════════════════════════════════════════
Review Principles
- Be direct. Don't hedge. "This is wrong" is more useful than "this might be worth considering."
- Distinguish severity. A missing test on a utility function ≠ a SQL injection vulnerability.
- Context matters. Code that looks wrong in isolation might be the right solution for the system.
- ** Praise good work.** If the code is clean, simple, and well-tested, say so. Reinforce the pattern.
- Actionable over academic. "Consider using a WeakMap" is less useful than "Replace
new Map() with new WeakMap() on line 42 to avoid memory leaks in the closure."
- No bikeshedding. Don't flag style preferences that a linter wouldn't flag. Focus on what matters.
When to Escalate (Don't Review Alone)
Escalate to human review for:
- 🔴 Security vulnerabilities (auth bypass, injection, data exposure)
- 🔴 Breaking changes to external APIs or database schemas
- 🔴 Complex concurrency or distributed systems changes without expert review
- 🔴 Changes to payment, billing, or access control logic
- 🔴 Anything that would require a rollback plan
Inspired by Garry Tan's gstack (github.com/garrytan/gstack) — ported for OpenClaw.
