code-review

name	code-review
description	Use this skill when verifying that implementation work meets acceptance criteria, reviewing code for security vulnerabilities, assessing code quality and stability, or producing a formal review report. Load when conducting QA, security audits, or acceptance testing against a defined spec.
license	Proprietary. LICENSE.txt has complete terms

Code Review — QA & Security Methodology

Purpose

Verify that implementation work meets every acceptance criterion, is secure, is stable, and is ready for production. The verdict is binary: APPROVED or REQUIRES CHANGES. No middle ground.

Review Process

Follow this exactly. Do not skip steps. Do not reorder.

Step 1: Extract Acceptance Criteria

List every acceptance criterion from the requirements document. Number them. Each one will receive an explicit PASS or FAIL — no "partially met," no ambiguity.

Step 2: Review the Changes

Run git log --oneline -20 to understand scope. Run git diff to see the full changeset. Read the diff carefully. Understand what was added, modified, and removed. Form a mental model of the implementation before beginning verification.

Step 3: Verify Each Criterion

For each acceptance criterion:

1. Run the verification command if one is specified.
2. If no command exists, inspect the code and verify manually.
3. Record: PASS or FAIL.
4. Record evidence: command output, file:line reference, or specific observation.

Do not infer that a criterion passes because a related one passed. Each stands alone.

Step 4: Run the Full Quality Checklist

Beyond acceptance criteria, evaluate every applicable category below.

Step 5: Produce the Review Report

Write a structured report using the format in the Output section.

Review Checklist

Spec Compliance

• Every acceptance criterion satisfied with evidence
• Implementation does not exceed scope — no unrequested features, no gold-plating

Code Quality

• Linting passes (run the project's lint command)
• Type checking passes (run the project's typecheck command)
• Tests written for new functionality
• Existing tests still pass
• Build succeeds

Security

• No hardcoded secrets, API keys, tokens, or credentials in source code
• No secrets in logs, error messages, or client-facing responses
• All user inputs validated and sanitized
• Authentication enforced on protected routes
• Authorization checks present — users cannot access resources they do not own
• No SQL injection, XSS, CSRF, or command injection vectors
• No PII leaked in logs, analytics, or error reporting
• Dependencies with known vulnerabilities flagged
• Sensitive data encrypted at rest and in transit where applicable

Error Handling

• Errors caught and handled gracefully — no unhandled rejections or uncaught exceptions
• Error messages are user-safe — no stack traces or internal paths exposed
• Failures do not leave the system in an inconsistent state
• No silent error swallowing — caught errors are logged or reported
• Retry logic has backoff and maximum attempt limits

Edge Cases

• Empty states: empty collections, blank inputs, missing data
• Boundary values: zero, negative, maximum lengths, overflow, Unicode
• Concurrent access: simultaneous operations on the same resource
• Malformed input: unexpected types, missing fields, extra fields, extreme lengths
• Network failures: timeouts, partial responses, connection drops
• Null and undefined: nullable values handled without throwing

Performance

• No N+1 query patterns
• No unnecessary re-renders or re-computations
• No unbounded loops, unguarded recursion, or memory leaks
• Large datasets paginated, streamed, or lazily loaded
• Expensive operations cached with clear invalidation

Accessibility (Frontend Changes Only)

• Semantic HTML elements used correctly
• Interactive elements keyboard-navigable
• Focus management correct after dynamic content changes
• ARIA attributes present where semantic HTML is insufficient
• Color is not the sole means of conveying information
• Text meets WCAG AA contrast requirements

Output Format

# Review Report: <Title>

## Acceptance Criteria

| #   | Criterion | Verdict   | Evidence                                    |
| --- | --------- | --------- | ------------------------------------------- |
| 1   | <title>   | PASS/FAIL | <command output, file:line, or observation> |

## Security Findings

<If none: "No security issues identified.">

### <Finding Title>

- **Severity**: Critical / High / Medium / Low
- **Location**: `<file>:<line>`
- **Description**: <What the issue is>
- **Impact**: <What could go wrong>
- **Remediation**: <Specific fix>

## Stability Concerns

<If none: "No stability concerns identified.">

## Edge Case Issues

<If none: "No edge case issues identified.">

## Code Quality

- Lint: PASS/FAIL — <summary>
- Typecheck: PASS/FAIL/N/A — <summary>
- Tests: PASS/FAIL — <summary>
- Build: PASS/FAIL — <summary>

## Overall Verdict: APPROVED | REQUIRES CHANGES

### Remediation Required (if REQUIRES CHANGES)

1. **<Issue>** — `<file>:<line>` — <What must change and why>

Verdict Rules

• ANY acceptance criterion FAIL → REQUIRES CHANGES
• ANY Critical or High security finding → REQUIRES CHANGES
• All criteria pass + no critical security findings → APPROVED (note medium/low findings as observations)
• APPROVED means confident the work is correct, secure, and production-ready

Mindset

• Start adversarial. Assume bugs exist until you prove otherwise.
• Follow the evidence. Every verdict backed by command output, file reference, or concrete observation.
• Think like an attacker. Every input: what if malicious? Every endpoint: what if unauthorized? Every operation: what if it fails halfway?
• Think about edges. The happy path is the easy part. What happens at boundaries?
• Be specific, not vague. Actionable feedback with file paths and line numbers.
• Scope your findings. Distinguish spec violations (failures) from security risks (findings) from quality observations (notes).