Requesting Code Review

Overview

Systematic review combining verification and code quality checks. Catches issues before they cascade. For OpenSpec projects, validates against proposal requirements.

Core principle: Review early, review often. Evidence before assertions.

When to Use

Use when:

After completing implementation tasks
Before merging branches or creating PRs
Before running /openspec:archive
After significant code changes
When task list shows "done" but not verified

Skip when:

Trivial changes (typo fixes, comments)
Work-in-progress explorations
No code changes (docs only)

The Process

Phase 1: Context Discovery

Detect project type:

# Check for OpenSpec
ls openspec/changes/*/proposal.md 2>/dev/null

# Check for git repo
git status 2>/dev/null

For OpenSpec projects:

Find most recent active change: openspec list
Read changes/<id>/proposal.md, tasks.md, design.md (if exists)
Read spec deltas in changes/<id>/specs/*/spec.md
Understand requirements and scenarios

For non-OpenSpec:

Check git diff for changed files
Identify scope of changes

Phase 2: Verification (Evidence-Based)

CRITICAL: Run actual commands. Never claim "looks good" without evidence.

Basic verification:

# Build check
[Run project build command]
# Examples: npm run build, cargo build, python -m pytest

# Test check
[Run test suite]
# Examples: npm test, pytest, cargo test

# Lint check (if applicable)
[Run linter]
# Examples: eslint ., cargo clippy, flake8

Output analysis:

All tests PASS? (count failures)
Build SUCCESS? (check exit code)
Linter clean? (count warnings/errors)

If ANY fail:

STOP immediately
Report failures with details
Do NOT proceed to Phase 3
Tasks are NOT complete

Phase 3: Task Completion Check

For OpenSpec projects:

Read changes/<id>/tasks.md and verify each task:

Checklist vs Reality:
- How many tasks marked [ ] (pending)?
- How many marked [x] (done)?
- Do file changes match task descriptions?
Spec Delta Validation:
- For each ## ADDED Requirements: implemented?
- For each ## MODIFIED Requirements: changes applied?
- For each ## REMOVED Requirements: code removed?
- For each #### Scenario:: test case exists?
Cross-reference:
- Does code match design.md architecture?
- Are all capabilities in spec deltas addressed?

For non-OpenSpec:

Check git diff against commit messages
Verify stated changes actually present
Look for unfinished TODOs or FIXMEs

Phase 4: Code Quality Review

Scan for common issues:

Security:
- Hardcoded secrets? (API keys, passwords)
- SQL injection risks?
- XSS vulnerabilities?
- Command injection?
Error Handling:
- Uncaught exceptions?
- Silent failures?
- Missing validation at boundaries?
Edge Cases:
- Null/undefined handling?
- Empty array/string handling?
- Boundary conditions?
- Concurrent access issues?
Code Quality:
- Duplicated code blocks?
- Over-complicated logic?
- Missing comments for non-obvious code?
- Inconsistent naming?

For each issue found:

Note file path and line number
Severity: Critical / High / Medium / Low
Recommendation

Phase 5: Review Report

Format:

CODE REVIEW REPORT
==================

PROJECT: [OpenSpec change-id or project name]
DATE: [timestamp]

## Verification Results

Build: [PASS/FAIL with details]
Tests: [X passed, Y failed]
Lint: [Clean / X warnings]

## Task Completion

OpenSpec Tasks: [X/Y complete]
- [x] Task 1
- [ ] Task 2 (INCOMPLETE: [reason])

Spec Requirements: [X/Y implemented]
- [x] Requirement 1
- [ ] Requirement 2 (MISSING: [details])

## Issues Found

### CRITICAL
- [file:line] [description]

### HIGH
- [file:line] [description]

### MEDIUM
- [file:line] [description]

## Recommendations

1. [Action item with priority]
2. [Action item]

## Status

[APPROVED / NEEDS WORK / BLOCKED]

Reasoning: [explanation]

Decision criteria:

APPROVED: All tests pass, no critical/high issues, requirements met
NEEDS WORK: Tests pass but quality issues, or minor requirements missing
BLOCKED: Tests fail, critical issues, or major requirements not implemented

Phase 6: OpenSpec Handoff

If APPROVED and OpenSpec project:

Review APPROVED. Ready for archive.

Mày có thể chạy:
/openspec:archive <change-id>

Hoặc muốn tao làm luôn?

If NEEDS WORK:

Review complete nhưng có X issues cần fix.

Fix trước hay proceed anyway? (Not recommended)

If BLOCKED:

Review BLOCKED. Phải fix [critical issues] trước khi archive.

1. [Critical issue 1]
2. [Critical issue 2]

Muốn tao giúp fix không?

Key Principles

Evidence before assertions - Run commands, never assume
Fail fast - Stop at first verification failure
Be specific - File paths, line numbers, not vague "looks good"
Honest assessment - Don't hide issues to make user happy
Actionable feedback - Each issue has clear fix recommendation

OpenSpec Spec Delta Patterns

When reviewing spec deltas, check:

## ADDED Requirements
### Requirement: [Name]
[Description]

#### Scenario: [Case]
- WHEN [condition]
- THEN [expected behavior]

Verify:

Code implementing the requirement exists
Test case covering the scenario exists
Test passes

Common miss patterns:

Requirement added to spec but no implementation
Implementation exists but no test
Test exists but doesn't match scenario WHEN/THEN

Example: OpenSpec Review

User: "Tao vừa implement xong, check giúp tao"

Assistant (detects openspec):
"Để tao check. Đang detect OpenSpec context..."

[Reads openspec/changes/add-user-auth/]

"Found change: add-user-auth
Đang run verification..."

$ npm run build
✓ Build passed

$ npm test
✓ 15 tests passed

Đang check tasks vs spec requirements...

## CODE REVIEW REPORT

PROJECT: add-user-auth

### Verification: PASS
- Build: PASS
- Tests: 15/15 passed
- Lint: Clean

### Task Completion: 4/4
- [x] Implement JWT auth
- [x] Add login endpoint
- [x] Add password reset
- [x] Write tests

### Spec Requirements: 3/3
- [x] User authentication (spec delta implemented)
- [x] JWT token generation (test covers scenario)
- [x] Password reset flow (test covers scenario)

### Issues: NONE

### Status: APPROVED

Ready for archive. Chạy /openspec:archive add-user-auth?"

requesting-code-review