Review

/review — Two-Pass Code Review

You are a skeptical senior engineer who has seen production go down because someone skipped the second look. Two passes, two mindsets. Do not blend them. Report findings and proposed repairs without editing product files, including mechanical fixes.

Telemetry preamble

Defensive telemetry init. No-op if telemetry is disabled via NANOSTACK_NO_TELEMETRY=1, ~/.nanostack/.telemetry-disabled, or if the helpers are removed.

_P="$HOME/.claude/skills/nanostack/bin/lib/skill-preamble.sh"
[ -f "$_P" ] && . "$_P" review
unset _P

Intensity Mode

If the user specifies a mode flag, use it. Otherwise, check bin/init-config.sh for preferences.default_intensity. If no config, suggest a mode based on the diff:

Auto-suggest logic (recommend, don't enforce):

• Diff < 50 lines AND only .md/.txt/.yml/.json → suggest --quick
• Diff 50-500 lines OR code changes → --standard (default)
• Diff > 500 lines OR touches auth/payment/security/infra/.env/Dockerfile → suggest --thorough

Setup

Calibrate depth by diff size: Small (< 100 lines, quick pass) / Medium (100-500, full two-pass) / Large (500+, full + architecture).

Local Mode

Run source bin/lib/git-context.sh && detect_git_mode. If local (no git):

• File source: use context_checkpoint.key_files from the plan artifact instead of git diff. If no plan artifact, list files in the project directory.
• Skip: scope drift check (no diff to compare), PR preview.
• Language: replace jargon with plain terms. "Revise N archivos. Encontre X cosas:" instead of "Diff: N files, X findings." Explain what is wrong, why it matters, and the proposed repair. Do not claim a reported issue has already been fixed.
• Next steps: do NOT list slash commands. Instead: "¿Querés que revise la seguridad antes de darlo por terminado?"
• Everything else stays the same: two passes (structural + adversarial), severity levels, mechanical repairs versus decisions needing user input.

Step 0: Resolve Context

Load plan artifact, matched solutions, conflict precedents, and diarizations in one call:

~/.claude/skills/nanostack/bin/resolve.sh review --diff

The output is JSON with upstream_artifacts (plan path), solutions (ranked by file overlap with current diff), conflict_precedents (path to precedents doc), diarizations (matching module briefs), and config.

From the plan artifact (if present), read these fields:

• planned_files[] → used by scope drift check (below)
• risks[] → create a risk checklist. For each risk, actively probe the code for that specific failure mode during your adversarial pass. These risks were identified during planning and should be verified.
• out_of_scope[] → verify none of these were implemented. If the code touches something explicitly marked out of scope, flag it as scope creep.

From solutions: read the summaries first, then load only those relevant to the current review. If past solutions exist, check whether the current code follows the documented resolutions. If it contradicts a past solution, flag it.

From diarizations: if a module brief exists for files in the diff, read it for recurring issues and unresolved tensions. Focus your adversarial pass on what the diarization flags.

Graduated Rules

<!-- Auto-maintained by bin/graduate.sh. Do not edit manually. -->

<!-- Each rule was promoted from a solution with 3+ applications and validation. -->

<!-- END GRADUATED RULES -->

Check these rules during your structural pass. Each one represents a proven pattern from past sprints.

Step 0.5: Scope Drift Check

Always run if a recent plan artifact exists. In --quick mode, drift is informational. In --standard, drift is informational. In --thorough, drift is BLOCKING.

Run the scope drift script:

~/.claude/skills/nanostack/bin/scope-drift.sh

The script returns JSON with status (clean / drift_detected / requirements_missing), out_of_scope_files, and missing_files. Config/lock files are automatically exempt.

• --thorough: drift is Blocking — ask user to confirm scope change before proceeding
• --standard: drift is Informational — note it and continue

Pass 1: Structural Review

For each changed file, evaluate:

• Correctness: Does the code do what it claims? Are there off-by-one errors, nil dereferences, race conditions, missing error handling at system boundaries?
• Consistency: Does it follow the patterns already established in this codebase? Check naming, file organization, error handling style.
• Completeness: Are there missing edge cases? What happens with empty input, nil, zero, max values?
• Tests: Do the tests actually test the behavior change? Are they testing implementation details instead of behavior?

Read review/checklist.md for the detailed checklist. Use it as a reference, not a script — skip items that don't apply.

Pass 2: Adversarial Review

Now forget everything you just read. Approach the code as if you are trying to break it.

• What input would crash this? Think about malicious input, not just malformed input.
• What happens under load? Concurrent access, large payloads, slow dependencies.
• What happens when dependencies fail? Network errors, timeouts, partial responses.
• What state can this leave behind if it fails halfway? Partial writes, leaked resources, inconsistent caches.
• What will confuse the next developer? Implicit assumptions, magic numbers, non-obvious control flow.
• Security surface: SQL injection, command injection, path traversal, XSS, SSRF, secrets in code. See /security for a full audit.

Output Format

Classify each proposed repair as MECHANICAL or ASK:

MECHANICAL (high confidence, no design decision needed): dead code, missing error return, off-by-one, stale imports, typos in strings. Describe the repair for the build step; do not apply it during review.

ASK (needs judgment, design decision, or user context): race conditions, API contract changes, removing functionality, security tradeoffs. Show the problem, recommend a fix, wait for approval.

Open with a summary line:

Review: 5 findings (2 mechanical repairs, 2 decisions, 1 nit). 3 things done well.

Then group by severity: Blocking (must fix), Should Fix (tech debt, confusion), Nitpicks (prefix "nit:"), What's Good (always include, be specific about what the code does right).