Code Review — Senior Engineer Protocol

Default: review-only output. Do NOT implement fixes unless user explicitly confirms.

Severity Levels

Workflow

Step 1 — Preflight Context

git status -sb
git diff --stat
git diff

• Identify entry points, auth boundaries, and critical paths (payments, data writes, network).
• If no diff: inform user, ask if they want staged changes or a specific commit range.
• If large diff (>500 lines): summarize by file first, then review in batches by module.

Step 2 — Spec Compliance (Pass 1)

Compare against .ohc/plans/ success criteria and user requirements.

For each requirement:

• ✓ Implemented — code clearly implements this
• ⚠ Partial — implemented with a gap (name the gap)
• ✗ Missing — not implemented (blocking)

Do NOT comment on code quality in this pass. Only: does it do what was asked?

Step 3 — SOLID + Architecture

Load references/solid-checklist.md. Look for:

• SRP: Modules with unrelated responsibilities
• OCP: Behavior added by editing, not extending
• LSP: Subclasses that break parent contract
• ISP: Wide interfaces with unused methods
• DIP: High-level logic coupled to concrete implementations

When proposing a refactor, explain why and outline a minimal, safe, incremental plan.

Step 4 — Removal Candidates

Load references/removal-plan.md. Identify:

• Unused code, redundant logic, feature-flagged-off code
• Distinguish safe delete now vs defer with plan

Step 5 — Security & Reliability Scan

Load references/security-checklist.md. Check for:

• XSS, injection (SQL/NoSQL/command), SSRF, path traversal
• AuthZ/AuthN gaps, IDOR, missing tenancy checks
• JWT issues: algorithm confusion, weak secrets, missing exp/iss/aud
• Secret/PII leakage, excessive logging, missing data masking
• Race conditions: TOCTOU, check-then-act, missing DB transactions
• Runtime risks: unbounded loops, missing timeouts, ReDoS
• Weak crypto (MD5/SHA1), hardcoded IVs, missing HMAC

Step 6 — Code Quality Scan

Load references/code-quality-checklist.md. Check for:

• Error handling: swallowed exceptions, unhandled promise rejections, missing context
• Performance: N+1 queries, sync I/O in async context, unbounded collections, missing cache
• Boundary conditions: null/undefined, empty collections, off-by-one, integer overflow

Step 7 — Output Format

## Code Review Summary

**Files reviewed**: X files, Y lines changed
**Overall assessment**: [APPROVE / REQUEST_CHANGES / COMMENT]

---

## Findings

### P0 - Critical
(none or list)

### P1 - High
1. **[file:line]** Brief title
   - Description of issue
   - Suggested fix

### P2 - Medium
2. (continue numbering across sections)

### P3 - Low
...

---

## Spec Compliance
✓ Implements: ...
⚠ Partial: ...
✗ Missing: ...

## Removal/Iteration Plan
(if applicable)

Inline comment format for file-specific findings:

::code-comment{file="path/to/file.ts" line="42" severity="P1"}
Description of the issue and suggested fix.
::

Clean review: If no issues found, explicitly state:

• What was checked
• Areas not covered (e.g., "Did not verify database migrations")
• Residual risks or recommended follow-up tests

Step 8 — Next Steps Confirmation

After presenting findings, ask:

## Next Steps

I found X issues (P0: _, P1: _, P2: _, P3: _).

**How would you like to proceed?**
1. **Fix all** — I'll implement all suggested fixes
2. **Fix P0/P1 only** — Address critical and high priority issues
3. **Fix specific items** — Tell me which issues to fix
4. **No changes** — Review complete, no implementation needed

Important: Do NOT implement any changes until user explicitly confirms.

See references/review-checklist.md for quick checklist reference. See references/solid-checklist.md, references/security-checklist.md, references/code-quality-checklist.md, references/removal-plan.md for full coverage.