/codereview - Multi-Reviewer Parallel Code Review
Run a comprehensive code review that dispatches 3 specialized perspectives concurrently - architecture, security, and testing - then consolidates into a single actionable verdict.
Target: $ARGUMENTS
Why 3 Perspectives, Not 6
Most multi-reviewer systems use 5-7 overlapping perspectives that waste tokens and produce redundant findings. /codereview uses exactly 3 orthogonal perspectives - each covers a distinct failure mode with zero overlap:
┌──────────────────────────────────────────────────────────────┐
│ PARALLEL REVIEW DISPATCH │
│ │
│ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐ │
│ │ @reviewer │ │ @security │ │ @tester │ │
│ │ Architecture│ │ OWASP/Auth │ │ Coverage │ │
│ │ + Quality │ │ + Data │ │ + Edge Cases│ │
│ └──────┬──────┘ └──────┬──────┘ └──────┬──────┘ │
│ │ │ │ │
│ └────────────────┼────────────────┘ │
│ ▼ │
│ ┌──────────────┐ │
│ │ CONSOLIDATE │ │
│ │ Deduplicate │ │
│ │ Assign Sev. │ │
│ │ Verdict │ │
│ └──────────────┘ │
└──────────────────────────────────────────────────────────────┘
When to Use
- Before merging a pull request
- Before a production release
- After major refactors that touch multiple modules
- After new contributor PRs (unfamiliar with codebase conventions)
- When a change touches auth, payments, or data handling
- NOT for: quick single-file fixes (use
@reviewer directly)
- NOT for: security-only audit (use
@security directly)
- NOT for: test strategy design (use
@tester directly)
Workflow
Step 1: Scope Detection
Determine the review target:
IF $ARGUMENTS is a file/directory path → review that path
IF $ARGUMENTS is a module name → find and review the module
IF $ARGUMENTS is a PR number/URL → extract changed files from PR diff
IF $ARGUMENTS is empty → review staged/uncommitted changes
Gather scope metrics:
# Count files and lines in scope
find $TARGET -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" -o -name "*.vue" -o -name "*.svelte" \) | head -200 | xargs wc -l 2>/dev/null || true
# If PR, get the diff
gh pr diff $PR_NUMBER 2>/dev/null || git diff main...HEAD 2>/dev/null || true
Read the project's docs/ARCHITECTURE.md if it exists - this is the source of truth for architecture review.
Step 2: Parallel Review Dispatch
Dispatch 3 concurrent review perspectives. Each reviewer gets the full file context but applies a different lens.
Perspective 1: Architecture Review (@reviewer focus)
Review the code for structural integrity and quality:
| Check |
What to Look For |
| Layer separation |
Services don't import from components, no cross-module imports |
| Naming conventions |
Files, functions, variables follow project conventions |
| Design patterns |
DRY, SOLID, composition over inheritance |
| API contracts |
DTOs/schemas at boundaries, typed inputs and outputs |
| Dependency direction |
Layers depend inward, not outward |
| Code smells |
Long methods (>40 lines), deep nesting (>3 levels), god objects |
| Type safety |
No any abuse, strict mode, proper generics |
| Error handling |
Try/catch at boundaries, custom error types, no swallowed errors |
| Import hygiene |
No circular dependencies, barrel file discipline |
# Architecture checks
grep -rn ": any\|as any" $TARGET --include="*.ts" --include="*.tsx" 2>/dev/null | head -20
grep -rn "console\.\|debugger" $TARGET --include="*.ts" --include="*.tsx" --include="*.vue" 2>/dev/null | head -20
Perspective 2: Security Review (@security focus)
Review the code for vulnerabilities and data safety:
| Check |
What to Look For |
| Injection |
SQL/NoSQL injection, command injection, template injection |
| XSS |
Unsanitized output, dangerouslySetInnerHTML, v-html, raw HTML |
| Auth/AuthZ |
Missing auth checks, broken access control, IDOR vulnerabilities |
| Input validation |
Missing or insufficient validation, type coercion issues |
| Secrets |
Hardcoded API keys, tokens, passwords, connection strings |
| Data exposure |
Sensitive data in logs, error messages, API responses, URLs |
| CSRF |
Missing CSRF tokens on state-changing operations |
| Cryptography |
Weak algorithms, predictable tokens, missing encryption |
| Headers |
Missing security headers (CSP, HSTS, X-Frame-Options) |
| Dependencies |
Known CVEs in packages |
# Security checks
npm audit --json 2>/dev/null | head -50 || true
grep -rn "password\|secret\|token\|api_key\|apikey" $TARGET --include="*.ts" --include="*.tsx" --include="*.env*" 2>/dev/null | head -20
grep -rn "v-html\|dangerouslySetInnerHTML\|innerHTML" $TARGET --include="*.ts" --include="*.tsx" --include="*.vue" 2>/dev/null | head -20
grep -rn "eval(\|new Function(" $TARGET --include="*.ts" --include="*.tsx" --include="*.js" 2>/dev/null | head -20
Perspective 3: Test Review (@tester focus)
Review the code for test quality and coverage:
| Check |
What to Look For |
| Coverage gaps |
Business logic without corresponding tests |
| Missing edge cases |
Null, empty, boundary values, error paths not tested |
| Test quality |
Tests that test implementation vs behavior |
| Mocking correctness |
Over-mocking, mocking what you own, leaking mocks |
| Assertion strength |
Weak assertions (toBeTruthy vs toEqual), missing assertions |
| Test isolation |
Shared mutable state, order-dependent tests |
| Naming clarity |
Descriptive it('should...') vs vague test names |
| Test types |
Right balance of unit/integration/e2e for the change |
| Regression risk |
Changed code without updated tests |
| Flakiness signals |
Timing dependencies, network calls in unit tests |
# Test checks
find $TARGET -name "*.spec.ts" -o -name "*.test.ts" -o -name "*.spec.tsx" -o -name "*.test.tsx" 2>/dev/null | wc -l
npx vitest run --passWithNoTests --reporter=json 2>/dev/null || npx jest --json --passWithNoTests 2>/dev/null || true
Step 3: Consolidation
Merge findings from all 3 perspectives:
- Deduplicate - Remove findings flagged by multiple perspectives (keep highest severity)
- Cross-reference - A security finding that also lacks tests is escalated one severity level
- Assign severity - Every finding gets a severity level (see table below)
- Sort - CRITICAL first, then HIGH, MEDIUM, LOW
- Group - By file, then by severity within each file
Step 4: Verdict
Apply verdict rules based on the consolidated findings. The verdict is final and non-negotiable - it follows the rules mechanically.
Severity Levels
| Level |
Description |
Action |
| CRITICAL |
Security vulnerability, data loss risk, auth bypass |
Block merge. Fix immediately. |
| HIGH |
Architecture violation, missing validation, untested critical path |
Request changes. Fix before merge. |
| MEDIUM |
Code quality issue, weak test, minor vulnerability |
Approve with notes. Fix in next sprint. |
| LOW |
Style issue, minor improvement, optional optimization |
Approve. Fix when convenient. |
Verdict Rules
| Condition |
Verdict |
Meaning |
| Any CRITICAL finding |
BLOCK |
Do not merge under any circumstances |
| Any HIGH finding (no CRITICAL) |
REQUEST_CHANGES |
Fix HIGH findings, then re-review |
| MEDIUM findings only |
APPROVE WITH NOTES |
Safe to merge, but address MEDIUM items soon |
| LOW findings only (or none) |
APPROVE |
Clean to merge |
Escalation rules:
- A security finding without a corresponding test → severity +1 level
- A finding that recurs from a previous review → severity +1 level
- A finding in auth, payments, or PII handling → minimum severity HIGH
Verification Protocol
Before claiming the review is complete:
- All 3 perspectives were executed (architecture, security, tests)
- Every finding has a severity level assigned
- Every CRITICAL or HIGH finding includes a specific file:line reference
- Every CRITICAL or HIGH finding includes a concrete remediation suggestion
- Automated checks were run where available (
npm audit, tsc, linting, test suite)
- The verdict follows the Verdict Rules mechanically - no exceptions
- Cross-referencing was performed (security findings checked for test coverage)
Anti-Rationalization
| Excuse |
Reality |
| "No security issues found" |
Did you check all OWASP categories? Absence of evidence is not evidence of absence. |
| "Tests look fine" |
Did you check edge cases, error paths, and assertion strength? Or just that tests exist? |
| "Architecture is clean" |
Did you trace actual imports and dependency direction? Or just scan filenames? |
| "It's just a small change" |
Small changes cause production outages. Review scope does not reduce rigor. |
| "The CI passed" |
CI checks syntax, not logic. A green build is not a security audit. |
| "Previous review approved this pattern" |
Patterns can be wrong. Evaluate independently every time. |
| "Too many files to review thoroughly" |
Scope down to critical paths (auth, data, business logic). Never skip security. |
| "The author is senior, they know what they're doing" |
Seniority does not prevent bugs. Review the code, not the author. |
Rules
- All 3 perspectives are mandatory - Skipping one defeats the purpose of multi-reviewer review
- Read-only - Never modify files during a review
- Evidence required - Every finding needs a file:line reference and explanation
- Remediation required - Findings without fix suggestions are useless
- Verdict is mechanical - Follow the Verdict Rules table, no subjective overrides
- No sycophancy - Never say "LGTM", "looks good", or "great job" without evidence
- Cross-reference findings - A security issue without tests is worse than either alone
- Severity must be justified - Every rating needs a specific technical reason
- Run automated tools - Never skip
npm audit, linting, or type checking if available
- Fresh context per perspective - Each reviewer perspective starts from the code, not from another reviewer's output
Output
──── /codereview ────
Target: [path or PR]
Scope: [X files, Y lines]
Architecture: [N findings] - @reviewer
Security: [N findings] - @security
Tests: [N findings] - @tester
Severity:
CRITICAL: N
HIGH: N
MEDIUM: N
LOW: N
Top Findings:
1. [CRITICAL] [description] - [file:line]
2. [HIGH] [description] - [file:line]
3. [HIGH] [description] - [file:line]
Verdict: [APPROVE | APPROVE WITH NOTES | REQUEST_CHANGES | BLOCK]
Reason: [1-line justification]
1---2name: codereview3description: Use when you need a comprehensive code review combining architecture, security, and test perspectives - especially before merging, releasing, or after major changes.4---56# /codereview - Multi-Reviewer Parallel Code Review78Run a comprehensive code review that dispatches 3 specialized perspectives concurrently - architecture, security, and testing - then consolidates into a single actionable verdict.910**Target:** $ARGUMENTS1112## Why 3 Perspectives, Not 61314Most multi-reviewer systems use 5-7 overlapping perspectives that waste tokens and produce redundant findings. `/codereview` uses exactly 3 orthogonal perspectives - each covers a distinct failure mode with zero overlap:1516```17┌──────────────────────────────────────────────────────────────┐18│ PARALLEL REVIEW DISPATCH │19│ │20│ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐ │21│ │ @reviewer │ │ @security │ │ @tester │ │22│ │ Architecture│ │ OWASP/Auth │ │ Coverage │ │23│ │ + Quality │ │ + Data │ │ + Edge Cases│ │24│ └──────┬──────┘ └──────┬──────┘ └──────┬──────┘ │25│ │ │ │ │26│ └────────────────┼────────────────┘ │27│ ▼ │28│ ┌──────────────┐ │29│ │ CONSOLIDATE │ │30│ │ Deduplicate │ │31│ │ Assign Sev. │ │32│ │ Verdict │ │33│ └──────────────┘ │34└──────────────────────────────────────────────────────────────┘35```3637## When to Use3839- Before merging a pull request40- Before a production release41- After major refactors that touch multiple modules42- After new contributor PRs (unfamiliar with codebase conventions)43- When a change touches auth, payments, or data handling44- NOT for: quick single-file fixes (use `@reviewer` directly)45- NOT for: security-only audit (use `@security` directly)46- NOT for: test strategy design (use `@tester` directly)4748## Workflow4950### Step 1: Scope Detection5152Determine the review target:5354```55IF $ARGUMENTS is a file/directory path → review that path56IF $ARGUMENTS is a module name → find and review the module57IF $ARGUMENTS is a PR number/URL → extract changed files from PR diff58IF $ARGUMENTS is empty → review staged/uncommitted changes59```6061Gather scope metrics:6263```bash64# Count files and lines in scope65find $TARGET -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" -o -name "*.vue" -o -name "*.svelte" \) | head -200 | xargs wc -l 2>/dev/null || true6667# If PR, get the diff68gh pr diff $PR_NUMBER 2>/dev/null || git diff main...HEAD 2>/dev/null || true69```7071Read the project's `docs/ARCHITECTURE.md` if it exists - this is the source of truth for architecture review.7273### Step 2: Parallel Review Dispatch7475Dispatch 3 concurrent review perspectives. Each reviewer gets the full file context but applies a different lens.7677---7879**Perspective 1: Architecture Review** (`@reviewer` focus)8081Review the code for structural integrity and quality:8283| Check | What to Look For |84|-------|------------------|85| Layer separation | Services don't import from components, no cross-module imports |86| Naming conventions | Files, functions, variables follow project conventions |87| Design patterns | DRY, SOLID, composition over inheritance |88| API contracts | DTOs/schemas at boundaries, typed inputs and outputs |89| Dependency direction | Layers depend inward, not outward |90| Code smells | Long methods (>40 lines), deep nesting (>3 levels), god objects |91| Type safety | No `any` abuse, strict mode, proper generics |92| Error handling | Try/catch at boundaries, custom error types, no swallowed errors |93| Import hygiene | No circular dependencies, barrel file discipline |9495```bash96# Architecture checks97grep -rn ": any\|as any" $TARGET --include="*.ts" --include="*.tsx" 2>/dev/null | head -2098grep -rn "console\.\|debugger" $TARGET --include="*.ts" --include="*.tsx" --include="*.vue" 2>/dev/null | head -2099```100101---102103**Perspective 2: Security Review** (`@security` focus)104105Review the code for vulnerabilities and data safety:106107| Check | What to Look For |108|-------|------------------|109| Injection | SQL/NoSQL injection, command injection, template injection |110| XSS | Unsanitized output, `dangerouslySetInnerHTML`, `v-html`, raw HTML |111| Auth/AuthZ | Missing auth checks, broken access control, IDOR vulnerabilities |112| Input validation | Missing or insufficient validation, type coercion issues |113| Secrets | Hardcoded API keys, tokens, passwords, connection strings |114| Data exposure | Sensitive data in logs, error messages, API responses, URLs |115| CSRF | Missing CSRF tokens on state-changing operations |116| Cryptography | Weak algorithms, predictable tokens, missing encryption |117| Headers | Missing security headers (CSP, HSTS, X-Frame-Options) |118| Dependencies | Known CVEs in packages |119120```bash121# Security checks122npm audit --json 2>/dev/null | head -50 || true123grep -rn "password\|secret\|token\|api_key\|apikey" $TARGET --include="*.ts" --include="*.tsx" --include="*.env*" 2>/dev/null | head -20124grep -rn "v-html\|dangerouslySetInnerHTML\|innerHTML" $TARGET --include="*.ts" --include="*.tsx" --include="*.vue" 2>/dev/null | head -20125grep -rn "eval(\|new Function(" $TARGET --include="*.ts" --include="*.tsx" --include="*.js" 2>/dev/null | head -20126```127128---129130**Perspective 3: Test Review** (`@tester` focus)131132Review the code for test quality and coverage:133134| Check | What to Look For |135|-------|------------------|136| Coverage gaps | Business logic without corresponding tests |137| Missing edge cases | Null, empty, boundary values, error paths not tested |138| Test quality | Tests that test implementation vs behavior |139| Mocking correctness | Over-mocking, mocking what you own, leaking mocks |140| Assertion strength | Weak assertions (`toBeTruthy` vs `toEqual`), missing assertions |141| Test isolation | Shared mutable state, order-dependent tests |142| Naming clarity | Descriptive `it('should...')` vs vague test names |143| Test types | Right balance of unit/integration/e2e for the change |144| Regression risk | Changed code without updated tests |145| Flakiness signals | Timing dependencies, network calls in unit tests |146147```bash148# Test checks149find $TARGET -name "*.spec.ts" -o -name "*.test.ts" -o -name "*.spec.tsx" -o -name "*.test.tsx" 2>/dev/null | wc -l150npx vitest run --passWithNoTests --reporter=json 2>/dev/null || npx jest --json --passWithNoTests 2>/dev/null || true151```152153### Step 3: Consolidation154155Merge findings from all 3 perspectives:1561571. **Deduplicate** - Remove findings flagged by multiple perspectives (keep highest severity)1582. **Cross-reference** - A security finding that also lacks tests is escalated one severity level1593. **Assign severity** - Every finding gets a severity level (see table below)1604. **Sort** - CRITICAL first, then HIGH, MEDIUM, LOW1615. **Group** - By file, then by severity within each file162163### Step 4: Verdict164165Apply verdict rules based on the consolidated findings. The verdict is final and non-negotiable - it follows the rules mechanically.166167## Severity Levels168169| Level | Description | Action |170|-------|-------------|--------|171| CRITICAL | Security vulnerability, data loss risk, auth bypass | Block merge. Fix immediately. |172| HIGH | Architecture violation, missing validation, untested critical path | Request changes. Fix before merge. |173| MEDIUM | Code quality issue, weak test, minor vulnerability | Approve with notes. Fix in next sprint. |174| LOW | Style issue, minor improvement, optional optimization | Approve. Fix when convenient. |175176## Verdict Rules177178| Condition | Verdict | Meaning |179|-----------|---------|---------|180| Any CRITICAL finding | **BLOCK** | Do not merge under any circumstances |181| Any HIGH finding (no CRITICAL) | **REQUEST_CHANGES** | Fix HIGH findings, then re-review |182| MEDIUM findings only | **APPROVE WITH NOTES** | Safe to merge, but address MEDIUM items soon |183| LOW findings only (or none) | **APPROVE** | Clean to merge |184185**Escalation rules:**186- A security finding without a corresponding test → severity +1 level187- A finding that recurs from a previous review → severity +1 level188- A finding in auth, payments, or PII handling → minimum severity HIGH189190## Verification Protocol191192**Before claiming the review is complete:**1931941. All 3 perspectives were executed (architecture, security, tests)1952. Every finding has a severity level assigned1963. Every CRITICAL or HIGH finding includes a specific file:line reference1974. Every CRITICAL or HIGH finding includes a concrete remediation suggestion1985. Automated checks were run where available (`npm audit`, `tsc`, linting, test suite)1996. The verdict follows the Verdict Rules mechanically - no exceptions2007. Cross-referencing was performed (security findings checked for test coverage)201202## Anti-Rationalization203204| Excuse | Reality |205|--------|---------|206| "No security issues found" | Did you check all OWASP categories? Absence of evidence is not evidence of absence. |207| "Tests look fine" | Did you check edge cases, error paths, and assertion strength? Or just that tests exist? |208| "Architecture is clean" | Did you trace actual imports and dependency direction? Or just scan filenames? |209| "It's just a small change" | Small changes cause production outages. Review scope does not reduce rigor. |210| "The CI passed" | CI checks syntax, not logic. A green build is not a security audit. |211| "Previous review approved this pattern" | Patterns can be wrong. Evaluate independently every time. |212| "Too many files to review thoroughly" | Scope down to critical paths (auth, data, business logic). Never skip security. |213| "The author is senior, they know what they're doing" | Seniority does not prevent bugs. Review the code, not the author. |214215## Rules2162171. **All 3 perspectives are mandatory** - Skipping one defeats the purpose of multi-reviewer review2182. **Read-only** - Never modify files during a review2193. **Evidence required** - Every finding needs a file:line reference and explanation2204. **Remediation required** - Findings without fix suggestions are useless2215. **Verdict is mechanical** - Follow the Verdict Rules table, no subjective overrides2226. **No sycophancy** - Never say "LGTM", "looks good", or "great job" without evidence2237. **Cross-reference findings** - A security issue without tests is worse than either alone2248. **Severity must be justified** - Every rating needs a specific technical reason2259. **Run automated tools** - Never skip `npm audit`, linting, or type checking if available22610. **Fresh context per perspective** - Each reviewer perspective starts from the code, not from another reviewer's output227228## Output229230```231──── /codereview ────232Target: [path or PR]233Scope: [X files, Y lines]234235Architecture: [N findings] - @reviewer236Security: [N findings] - @security237Tests: [N findings] - @tester238239Severity:240 CRITICAL: N241 HIGH: N242 MEDIUM: N243 LOW: N244245Top Findings:2461. [CRITICAL] [description] - [file:line]2472. [HIGH] [description] - [file:line]2483. [HIGH] [description] - [file:line]249250Verdict: [APPROVE | APPROVE WITH NOTES | REQUEST_CHANGES | BLOCK]251Reason: [1-line justification]252```