stacksmith-review（斯塔克史密斯评论）

Command routing

Parse the user's invocation:

• /stacksmith-review code -> code review mode
• /stacksmith-review design -> design review mode
• /stacksmith-review security -> security audit mode

If no mode is provided, ask which review mode they want.

All timeline logs in this skill use:

• skill: "stacksmith-review"
• mode: "code-review" | "design-review" | "security"

Mode: code

Preamble (run first)

_BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
_REPO=$(basename "$(git rev-parse --show-toplevel 2>/dev/null)" 2>/dev/null || echo "unknown")
echo "BRANCH: $_BRANCH"
echo "REPO: $_REPO"
_BASE=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||')
[ -z "$_BASE" ] && git rev-parse --verify origin/main >/dev/null 2>&1 && _BASE="main"
[ -z "$_BASE" ] && git rev-parse --verify origin/master >/dev/null 2>&1 && _BASE="master"
_BASE="${_BASE:-main}"
echo "BASE: $_BASE"
[ -f Gemfile ] && echo "STACK:ruby"
[ -f package.json ] && echo "STACK:node"
( [ -f requirements.txt ] || [ -f pyproject.toml ] ) && echo "STACK:python"
[ -f go.mod ] && echo "STACK:go"
[ -f Cargo.toml ] && echo "STACK:rust"
git fetch origin $_BASE --quiet 2>/dev/null || true
DIFF_INS=$(git diff origin/$_BASE --stat 2>/dev/null | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff origin/$_BASE --stat 2>/dev/null | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
DIFF_TOTAL=$((DIFF_INS + DIFF_DEL))
echo "DIFF_LINES: $DIFF_TOTAL"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(auth|login|session|token|password|permission|role|access)' && echo "SCOPE_AUTH=true" || echo "SCOPE_AUTH=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '\.(rb|py|go|rs|java|cs|php)$' && echo "SCOPE_BACKEND=true" || echo "SCOPE_BACKEND=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '\.(tsx?|jsx?|vue|svelte|css|scss)$' && echo "SCOPE_FRONTEND=true" || echo "SCOPE_FRONTEND=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(migration|schema|\.sql)' && echo "SCOPE_MIGRATIONS=true" || echo "SCOPE_MIGRATIONS=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(api|routes|endpoints|controllers)' && echo "SCOPE_API=true" || echo "SCOPE_API=false"
_COMMITTERS=$(git log --since="30 days ago" --format="%ae" 2>/dev/null | sort -u | wc -l | tr -d ' ')
[ "${_COMMITTERS:-0}" -gt 1 ] && echo "REPO_MODE=collaborative" || echo "REPO_MODE=solo"

If on the base branch with no diff: "Nothing to review - you're on the base branch. Switch to a feature branch."

Step 1 - Scope drift detection

Before reviewing code quality, check whether they built what was requested:

git log origin/$_BASE..HEAD --oneline 2>/dev/null
cat TODOS.md 2>/dev/null | head -20
cat PLAN.md 2>/dev/null | head -30

Identify stated intent from commit messages, PLAN.md, and TODOS.md. Run git diff origin/$_BASE --stat and compare files changed vs. stated intent.

Output:

Scope Check: [CLEAN / DRIFT DETECTED / REQUIREMENTS MISSING]
Intent: <1-line summary>
Delivered: <1-line summary>
[If drift: each out-of-scope change]
[If missing: each unaddressed requirement]

This is informational only.

Step 2 - Get the diff

git fetch origin $_BASE --quiet
git diff origin/$_BASE

If diff is over 500 lines: ask the user which areas to focus on first.

Step 3 - Critical pass (core review)

Apply against the full diff.

SQL and data safety:

• string interpolation in SQL
• user-controlled input in WHERE/ORDER BY without parameterization
• missing DB transactions on multi-step writes
• unbounded queries
• locking migrations
• non-reversible migrations

Race conditions and concurrency:

• check-then-act without atomic locking
• find_or_create_by without uniqueness constraints
• shared mutable state
• await inside loops where Promise.all is possible
• missing locks

Auth and trust boundaries:

• new routes missing auth middleware
• authorization defaulting to allow
• direct object reference
• token validation without expiration
• untrusted input without validation

Error handling:

• promises without .catch() or try/catch
• catch-all handlers
• swallowed errors
• external API calls without timeout
• missing null/undefined guards

Completeness gaps:

• new enum/status values not handled everywhere
• new API response fields not handled in consumers
• new code paths with no tests

Confidence calibration

Every finding must include a confidence score.

Finding format:

[P0/P1/P2] (confidence: N/10) file:line - description

Step 4 - Specialist dispatch

Always dispatch when DIFF_LINES >= 50:

1. Testing specialist
2. Maintainability specialist

Conditional: 3. Security specialist if SCOPE_AUTH=true or backend diff > 100 lines 4. Performance specialist if backend or frontend scope exists 5. Data migration specialist if migration scope exists 6. API contract specialist if API scope exists

If DIFF_LINES < 50: skip specialists and print Small diff ($DIFF_LINES lines) - specialists skipped.

Dispatch selected specialists in parallel via Agent. Each specialist gets:

• the relevant checklist
• stack context
• JSON-lines output format
• NO FINDINGS when clean

Deduplicate by path:line:category, keep highest confidence, apply gates, compute:

quality_score = max(0, 10 - (critical_count * 2 + informational_count * 0.5))

Testing specialist checklist:

Missing negative-path tests
Missing edge-case coverage
Test isolation violations
Flaky test patterns
New public functions with zero coverage
Changed methods where tests only cover old behavior

Security specialist checklist:

Missing input validation at trust boundaries
Auth/authz checks defaulting to allow
Direct object reference
Role escalation
Weak hashing
Non-constant-time comparison on secrets
Secrets in source code
XSS escape hatches with user data
Command injection
SSRF
Path traversal

Performance specialist checklist:

N+1 queries
Missing DB indexes
O(n^2) patterns
Linear search inside maps
String concatenation in loops
Heavy new production dependencies
Barrel imports
Fetch waterfalls
Unbounded list endpoints
Synchronous I/O inside async handlers

Data migration specialist checklist:

Non-reversible migrations
Table-locking ALTER TABLE
Missing indexes on new foreign keys
Data backfill in migration
Migration assumes specific data state
Old schema removed before old code is gone

API contract specialist checklist:

Response shape changed without version bump
Required/optional contract drift
New required request field without default
Error response shape changed
Pagination contract changed
Auth requirement changed on existing endpoint

Adversarial subagent (always runs):

You are an adversarial reviewer. Read the diff with `git diff origin/<BASE>`.

Think like an attacker and a chaos engineer. Find ways this code will fail in
production - not style issues, not missing tests, actual breakage or security holes.

Look specifically for:
- race conditions
- auth bypasses
- silent data corruption
- resource leaks
- swallowed failures
- trust boundary violations

For each finding: describe the exact failure scenario and classify as
FIXABLE or INVESTIGATE.

Step 5 - Fix-first

Output summary header:

Pre-Landing Review: N issues (X critical, Y informational) - PR Quality: N/10

Classify findings as AUTO-FIX or ASK.

AUTO-FIX examples:

• missing .catch()
• missing null guard where nullability is demonstrated
• missing LIMIT on an unbounded query
• obvious typo in an error message

ASK examples:

• auth checks
• race-condition fixes
• API contract changes
• anything touching payments, sessions, or sensitive data

For AUTO-FIX items:

• apply each fix
• commit with git commit -m "fix: [description] (stacksmith-review code)"
• report [AUTO-FIXED] file:line - problem - what was done

For ASK items: batch them into one AskUserQuestion with recommended fixes.

Step 6 - Documentation staleness check

for doc in README.md ARCHITECTURE.md CONTRIBUTING.md CLAUDE.md; do
  [ -f "$doc" ] && echo "DOC: $doc" || true
done

For each doc found: if the diff changes behavior described there and the doc was not updated, flag it as informational and suggest /stacksmith-release docs.

Final output format

## Code Review [branch] [date]
Scope Check: [CLEAN / DRIFT / MISSING]
Diff: N lines (+N/-N across N files)
Specialists: [list dispatched]
PR Quality Score: N/10

### AUTO-FIXED (N issues)
- file:line - description - what was done

### NEEDS DECISION (N issues)
[batched AskUserQuestion]

### INFORMATIONAL NOTES
- file:line - observation

### Looks good
- [things done well]

### Adversarial review
[Findings or "No additional issues found"]

Log

echo "{\"ts\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\",\"skill\":\"stacksmith-review\",\"mode\":\"code-review\",\"branch\":\"$(git branch --show-current 2>/dev/null || echo 'N/A')\",\"outcome\":\"success\",\"repo\":\"$(basename $(git rev-parse --show-toplevel 2>/dev/null) 2>/dev/null || echo 'N/A')\"}" >> ~/.mystack/timeline.jsonl

Mode: design

Preamble (run first)

_BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
_BASE=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||')
_BASE="${_BASE:-main}"
echo "BRANCH: $_BRANCH"
git diff origin/$_BASE --name-only 2>/dev/null | grep -iE '\.(tsx?|jsx?|vue|svelte|css|scss|html)$'
[ -f DESIGN.md ] && echo "DESIGN_SPEC: DESIGN.md" || echo "DESIGN_SPEC: none"

If DESIGN_SPEC exists: read it and use it as the review standard.

Step 1 - Read the code before auditing

For each changed frontend file, read it fully, not just the diff. Understand:

• component structure and hierarchy
• states rendered
• user interactions handled
• page context

Step 2 - Audit (10 dimensions)

Rate each from 0 to 10. For low scores, write one concrete code-level fix.

AI slop scan

Flag any present:

Step 3 - Interact for judgment calls

For fixes that change user-facing copy or significantly alter layout, ask one AskUserQuestion before changing:

[Re-ground: which component, which screen]

[Problem in plain English]

RECOMMENDATION: A because [one-line reason]

A) [Specific change]
B) [Alternative]
C) Leave as-is

One question at a time.

Step 4 - Fix (atomic commits per dimension)

For each issue:

1. Make the fix
2. Commit atomically:

git commit -m "design: [dimension] - [specific change]"

One commit per dimension. Do not bundle changes.

Step 5 - Report

## Design Review [component/page] [date]

### Scope
Files reviewed: [list]
Changed frontend files: [list]

### Dimension scores
| Dimension | Before | After |
[table]

### AI slop resolved
| Pattern | Where | Fix |
[table]

### Commits
[list of commits]

### Remaining
- [issue] - needs: [what]

Log

echo "{\"ts\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\",\"skill\":\"stacksmith-review\",\"mode\":\"design-review\",\"branch\":\"$(git branch --show-current 2>/dev/null || echo 'N/A')\",\"outcome\":\"success\",\"repo\":\"$(basename $(git rev-parse --show-toplevel 2>/dev/null) 2>/dev/null || echo 'N/A')\"}" >> ~/.mystack/timeline.jsonl

Mode: security

Preamble (run first)

_BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
_BASE=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||')
_BASE="${_BASE:-main}"
echo "BRANCH: $_BRANCH"
git diff origin/$_BASE --name-only 2>/dev/null | head -30
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(auth|login|session|token|password|permission|role|access|oauth|jwt)' && echo "SCOPE_AUTH=true" || echo "SCOPE_AUTH=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(payment|billing|stripe|charge|invoice|subscription)' && echo "SCOPE_PAYMENT=true" || echo "SCOPE_PAYMENT=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(upload|file|attachment|media|s3|blob|storage)' && echo "SCOPE_UPLOAD=true" || echo "SCOPE_UPLOAD=false"
git diff origin/$_BASE --name-only 2>/dev/null | grep -qiE '(api|routes|endpoints|webhook|controller|handler)' && echo "SCOPE_API=true" || echo "SCOPE_API=false"
[ -f Gemfile ] && echo "STACK:ruby"
[ -f package.json ] && echo "STACK:node"
( [ -f requirements.txt ] || [ -f pyproject.toml ] ) && echo "STACK:python"
[ -f go.mod ] && echo "STACK:go"

Zero-noise policy

Report a finding only if all three are true:

1. Confidence >= 8/10
2. Concrete exploit scenario
3. Independently verified at exact file:line

Confidence calibration:

• 10: confirmed vulnerability
• 9: very high
• 8: high
• 6-7: appendix only
• 0-5: do not report

Step 1 - Automated quick scan

grep -rEn "(api_key|api_secret|secret_key|password|passwd|private_key|access_token)\s*[:=]\s*['\"][^'\"]{8,}" \
  --include="*.rb" --include="*.py" --include="*.ts" --include="*.js" --include="*.go" \
  --exclude-dir=node_modules --exclude-dir=.git --exclude-dir=vendor . 2>/dev/null | \
  grep -iv "example\|placeholder\|your_\|<.*>\|ENV\[" | head -20

grep -rEn "\".*\+.*params\|'.*\+.*params\|query.*\+.*user\|sql.*\+.*input\|execute.*\+" \
  --include="*.rb" --include="*.py" --include="*.ts" --include="*.js" . 2>/dev/null | \
  grep -v node_modules | head -10

npm audit --json 2>/dev/null | python3 -c "
import sys,json
try:
  d=json.load(sys.stdin)
  v=d.get('vulnerabilities',{})
  crit=[k for k,v2 in v.items() if v2.get('severity')=='critical']
  high=[k for k,v2 in v.items() if v2.get('severity')=='high']
  print(f'npm audit: {len(crit)} critical, {len(high)} high')
  for c in crit[:5]: print(f'  CRITICAL: {c}')
except: pass
" 2>/dev/null || true

pip-audit -q 2>/dev/null | head -10 || true

grep -rEn "dangerouslySetInnerHTML|\.html_safe|raw\(|v-html|innerHTML\s*=" \
  --include="*.tsx" --include="*.jsx" --include="*.rb" --include="*.vue" --include="*.ts" \
  --exclude-dir=node_modules . 2>/dev/null | head -10

Step 2 - OWASP Top 10

Check relevant changed files and only report findings that pass the zero-noise test.

• A01 Broken Access Control
• A02 Cryptographic Failures
• A03 Injection
• A04 Insecure Design
• A05 Security Misconfiguration
• A06 Vulnerable Components
• A07 Authentication Failures
• A08 Software Integrity
• A09 Security Logging
• A10 SSRF

For each, inspect the actual code paths and document checked-clean areas too.

Step 3 - STRIDE threat model

For each major new component:

Step 4 - Scoped deep dives

If SCOPE_AUTH=true:

• trace every auth decision
• verify deny-on-failure
• look for bypass paths

If SCOPE_PAYMENT=true:

• verify webhook signatures
• validate amount server-side
• require idempotency

If SCOPE_UPLOAD=true:

• validate file type by content
• enforce size limits
• ensure safe serving
• ensure no path traversal

Step 5 - Report

## Security Audit [date] [branch]

### Summary
Files reviewed: N
Scope: [AUTH / PAYMENT / UPLOAD / API]
Automated scans: npm audit [result], secrets scan [N hits / clean]

### Critical (fix before ship)
[finding blocks]

### High (fix this sprint)
[finding blocks]

### Low (track, not blocking)
[finding blocks]

### Checked, no issues found
- [A01] Access Control: [specific check description]
- [A03] Injection: [what was checked]
[list every checked category]

### Appendix - medium confidence
[findings with confidence 6-7]

If there are no critical or high findings: "Audit complete. No critical or high findings. [N low-priority items in appendix.]"

Log

echo "{\"ts\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\",\"skill\":\"stacksmith-review\",\"mode\":\"security\",\"branch\":\"$(git branch --show-current 2>/dev/null || echo 'N/A')\",\"outcome\":\"success\",\"repo\":\"$(basename $(git rev-parse --show-toplevel 2>/dev/null) 2>/dev/null || echo 'N/A')\"}" >> ~/.mystack/timeline.jsonl