Code Review
Run a structured review of branch changes covering security, language strictness, performance, and conventions.
Workflow
Step 1: Detect changes
# Default base branch is main. User can override: /code-review --base staging
BASE=${1:-main}
git fetch origin "$BASE" --quiet 2>/dev/null || true
MERGE_BASE=$(git merge-base "origin/$BASE" HEAD)
BRANCH=$(git rev-parse --abbrev-ref HEAD)
# Changed files and stats
git diff --name-status "$MERGE_BASE"..HEAD
git diff --stat "$MERGE_BASE"..HEAD
git log --oneline "$MERGE_BASE"..HEAD
Use $MERGE_BASE..HEAD for ALL diffs. Never use origin/main..HEAD directly -- it includes changes from other merged branches.
Step 2: Auto-detect languages
Map file extensions to rule sets:
| Extension | Rule set | Skill |
|---|---|---|
.ts, .tsx |
TypeScript | typescript-strict |
.rs |
Rust | rust-strict |
.swift |
Swift | swift-strict |
.go |
Go | go-strict |
.js, .mjs, .cjs |
JavaScript | javascript-strict |
Only apply rules for languages present in the diff.
Step 3: Run checks
Run in this order. For each finding, record file, line number, description, and rule ID.
3a. Security scan (security-audit-standard)
Scan the diff output (not the full repo) for:
- Secrets: API keys, tokens, passwords, connection strings (
sk-,ghp_,AKIA,-----BEGIN PRIVATE KEY) - Input validation: unvalidated user input reaching SQL, HTML, shell commands, file paths
- Injection risks: string concatenation in queries,
eval(),innerHTML, unsanitized template literals - Auth issues: missing auth checks on endpoints, weak JWT config, secrets in client-side code
- File patterns:
.env,*.pem,credentials.jsoncommitted
3b. Language strictness checks
Apply the top rules for each detected language (see Quick Reference Tables below). Read the changed lines and flag violations.
3c. Performance scan (performance-audit-standard)
Look for these anti-patterns in the diff:
- O(n) membership checks where Set/Map should be used
- O(n^2) nested loops (
.filter()inside.map(),includes()inside loop) - Sort to find min/max (O(n log n) vs O(n) single pass)
- Sync file I/O in async context (
writeFileSync,std::fs::writeunder async) - Lock held across
.awaitor disk I/O (Rust) - Per-request client/regex creation instead of singleton
- Unbounded caches without capacity or TTL
- Multiple array passes where a single pass works
3d. Style and conventions (github-standards)
- Commit messages follow
type(scope): subjectformat - No
console.log/print/println!debugging left in code - No commented-out code blocks
- No TODO/FIXME without a ticket reference
- Error handling present (no empty catch blocks, no swallowed errors)
- Naming conventions match language idioms
Step 4: Generate review summary
Output Format
## Code Review: [branch-name]
**Base**: [base-branch] | **Files changed**: N | **Commits**: N
### CRITICAL (must fix before merge)
- [file:line] Finding description (RULE-ID)
### HIGH (should fix)
- [file:line] Finding description (RULE-ID)
### MEDIUM (consider fixing)
- [file:line] Finding description (RULE-ID)
### LOW (nitpick)
- [file:line] Finding description (RULE-ID)
### Passed checks
- No secrets detected in diff
- Error handling present
- Input validated at boundaries
- Async operations properly awaited
- Resources cleaned up
- Commit messages follow conventions
Omit empty severity sections. Always show "Passed checks" to confirm what was verified.
Quick Reference: Top 5 Rules Per Language
TypeScript
| ID | Rule | What to flag |
|---|---|---|
| TS-01 | No any |
any type annotation without justification comment |
| TS-02 | No as assertions |
as Type without preceding type guard |
| TS-04 | No @ts-ignore |
Use @ts-expect-error with explanation instead |
| TS-12 | Narrow catch errors | catch (err) without instanceof Error check |
| TS-13 | No silent catch | Empty catch {} or catch without logging |
Rust
| ID | Rule | What to flag |
|---|---|---|
| -- | No .unwrap() |
.unwrap() in non-test code without // BUG IF: comment |
| -- | No .expect() |
.expect() outside static init (LazyLock, OnceLock) |
| -- | SAFETY comments | unsafe block without // SAFETY: comment |
| -- | Lock across await | RwLock/Mutex guard held across .await |
| -- | Bounded caches | HashMap used as cache without capacity limit |
Swift
| ID | Rule | What to flag |
|---|---|---|
| SW-01 | No force unwrap | ! on optionals in production code |
| SW-03 | No silent try? |
try? on critical operations (file creation, auth, data save) |
| SW-09 | @MainActor on VMs |
ViewModel missing @MainActor annotation |
| SW-12 | Task cancellation | Task { } without [weak self] or cancellation check |
| SW-15 | [weak self] |
Async closures with strong self capture |
Go
| ID | Rule | What to flag |
|---|---|---|
| GO-01 | Wrap errors | return err without fmt.Errorf("context: %w", err) |
| GO-02 | Check errors | Ignored error return value |
| GO-03 | Drain body | resp.Body.Close() without io.Copy(io.Discard, ...) on error paths |
| GO-07 | RWMutex | Write lock where read lock suffices, or missing double-check |
| GO-17 | No hardcoded secrets | const apiKey = "..." or similar |
JavaScript
| ID | Rule | What to flag |
|---|---|---|
| JS-01 | No var |
var declaration anywhere |
| JS-03 | No silent catch | Empty catch block or catch without action |
| JS-05 | Bounded retries | Recursive retry without max attempt limit |
| JS-08 | Async file I/O | writeFileSync, readFileSync in server code |
| JS-17 | No eval() |
eval() or new Function() with dynamic input |
Common Anti-Patterns Checklist
Quick-scan checklist applied to ALL languages:
- No hardcoded secrets or API keys in diff
- All errors handled (not swallowed with empty catch)
- No
console.log/print/println!/NSLogdebugging left - No commented-out code blocks
- No TODO/FIXME without ticket reference
- Input validated at system boundaries
- No force-unwrap / force-cast /
anytype escape - Async operations properly awaited (no fire-and-forget)
- Resources cleaned up (listeners, subscriptions, file handles, DB connections)
- No unbounded collections in hot paths (use bounded cache, Set, or Map)
Branch Comparison Options
Review against a different base branch
/code-review --base staging
/code-review --base develop
Override the default main base branch.
Review specific files only
/code-review --files src/auth.ts src/middleware.ts
Limits the review to listed files. Use git diff $MERGE_BASE..HEAD -- <file> per file.
Review a PR by number
# Fetch PR metadata and diff
gh pr view <NUMBER> --json title,body,headRefName,baseRefName,files
gh pr diff <NUMBER>
Use gh pr diff output as the diff source instead of git diff.
Review a specific commit range
/code-review --range abc123..def456
Uses the provided range instead of computing merge-base.