/requesting-code-review

name: requesting-code-review
description: >
  Structured code review against requirements, quality, and security
  standards. Invoke after meaningful code changes or before merge.
  Triggers on: "review my code", "code review", "check this before
  merge", "security review", "is this secure", "look over my changes",
  "second pair of eyes", "check the diff". Routed by
  using-superpowers or executing-plans after implementation.

Requesting Code Review

Request review early to catch issues before they spread.

When

• After completing a plan task or batch
• After major refactor/feature work
• Before merge or PR finalization

How

1. Determine review range (`BASE_SHA` -> `HEAD_SHA`). Getting this wrong silently reviews the wrong thing:

• **Whole branch:** `BASE_SHA=$(git merge-base origin/main HEAD)` (substitute your default branch). **Not** a bare `origin/main` — once main moves past your branch point, main's newer files appear in the diff as phantom deletions your branch never made, and the reviewer spends its pass on them.
• **A single task or batch:** the commit recorded *before* the work started. **Not** `HEAD~1`, which silently drops all but the last commit of a multi-commit task.
• No remote, or `origin/main` not fetched: fall back to `git merge-base main HEAD`.
• Confirm the range is non-empty before dispatching — `git diff --stat BASE..HEAD`. An empty range produces a confident "no issues found" review of nothing.

2. The review scope is always `git diff --name-only BASE_SHA..HEAD_SHA`. Never take it from `context-snapshot.json`: its `changed_files` are the commits since the last session start (or the last commit), not your review range.

• The snapshot may add callers. If it exists and its `git_hash` equals `git rev-parse HEAD`, then for each file in the review range that has a `blast_radius` entry, add those callers to the reviewer prompt: *"Changed files: [range list]. Also referenced by: [blast_radius callers]."* If the hashes differ, ignore `blast_radius`.

**What `blast_radius` means.** Each entry lists files containing a reference that *resolves* to the changed file's path — a relative import, or the repo path written out. It is deliberately incomplete: references that cannot be resolved (package-style imports, dynamic paths, aliases from `tsconfig`) are dropped rather than guessed, so an empty list means "nothing was proven," not "nothing depends on this." Never widen review scope on a name match alone; if you need dependents the snapshot does not list, grep for them and say you did.

`blast_radius_method` records how the edges were derived. If it is missing, the snapshot came from a version that matched basenames as words — ignore `blast_radius` entirely and scope from the diff. 3. If a plan is in play, write the diff to a file first: `bash ../subagent-driven-development/scripts/review-package PLAN_FILE BASE HEAD`. It refuses empty and non-descendant ranges outright, so the reviewer never receives a range that cannot mean what it claims. Pass the printed path to the reviewer. 4. Dispatch `superpowers-optimized:code-reviewer` using `requesting-code-review/code-reviewer.md`. 5. Provide:

• What changed (from context snapshot or git diff)
• Scoped file list (changed files + blast radius callers if fresh snapshot available, or broad if not)
• Requirement or plan reference
• SHA range
• Short summary

Security Review (Built-In)

When changes touch security-relevant areas, the code review **must** include a security pass. This is not a separate step — it's part of every review where applicable.

**Triggers automatically when changes touch:**

• Authentication or authorization flows
• Input validation or output encoding
• API endpoints handling user data
• Secrets management or credential handling
• Cryptography, key management, or token generation
• Infrastructure, deployment, or CI/CD configs

**Security checklist:**

• OWASP Top 10 and CWE vulnerability scan
• OWASP API Security Top 10: broken object/function-level authorization, unrestricted resource consumption, SSRF, mass assignment, improper inventory management
• Input validation and injection risk (SQL, XSS, CSRF, command injection)
• Auth flow correctness (session handling, token expiry, privilege escalation, rate limiting on auth endpoints)
• Secrets handling (no hardcoded credentials, proper env var usage)
• Dependency vulnerabilities (known CVEs in imported packages)
• API hardening (security headers, CORS configuration, error message sanitization, rate limiting)
• Logging hygiene (no secrets in logs, adequate audit trail)

**Severity enforcement:**

• Critical/High security findings **block merge** until addressed or the user explicitly accepts the risk with documented rationale.
• Medium security findings should be fixed before merge unless explicitly deferred.

Adversarial Red Team (Optional)

For changes involving complex logic, concurrency, state management, or critical data paths, dispatch `superpowers-optimized:red-team` in parallel with the code reviewer.

**Triggers when changes touch:**

• State machines or multi-step workflows
• Concurrent access to shared resources
• Complex business logic with branching conditions
• Data transformation pipelines
• Retry/recovery/rollback logic
• Performance-critical paths handling large inputs

The red team agent finds concrete failure scenarios (specific inputs, race conditions, state corruption, resource exhaustion) that checklist-based review misses. It does NOT duplicate the security review — its focus is adversarial logic analysis, not OWASP/CWE compliance.

**Red team critical findings block merge** alongside security critical findings.

Auto-Fix Pipeline

When the red team report contains Critical or High findings, run the auto-fix pipeline. The pipeline is **ASI-guided and iterative** — fix one finding at a time, starting from the red team's designated ASI, then re-assess before proceeding. This prevents fixe