Code Review

Code Review Skill

Conduct a thorough code review for quality, security, and maintainability with severity-rated feedback.

When to Use

This skill activates when:

• User requests "review this code", "code review"
• Before merging a pull request
• After implementing a major feature
• User wants quality assessment

GPT-5.6 Guidance Alignment

• Default to outcome-first progress and completion reporting: state the target result, evidence, validation status, and stop condition before adding process detail.
• Treat newer user task updates as local overrides for the active workflow branch while preserving earlier non-conflicting constraints.
• If correctness depends on additional inspection, retrieval, execution, or verification, keep using the relevant tools until the review is grounded; stop once enough evidence exists.
• Continue through clear, low-risk, reversible next steps automatically; ask only when the next step is materially branching, destructive, credentialed, external-production, or preference-dependent.

Delegates to the code-reviewer and architect agents in parallel for a two-lane review:

1. Identify Changes

   • Run git diff to find changed files
   • Determine scope of review (specific files or entire PR)

2. Launch Parallel Review Lanes

   • code-reviewer lane - owns spec compliance, security, code quality, performance, and maintainability findings
   • architect lane - owns the devil's-advocate / design-tradeoff perspective
   • Both lanes run in parallel on a clean context with explicit scope and artifacts, and produce distinct outputs before final synthesis
   • If either lane cannot be launched or does not return evidence, report independent review unavailable; do not substitute the current/authoring lane, and do not approve or mark the review merge-ready.

3. Review Categories

   • Security - Hardcoded secrets, injection risks, XSS, CSRF
   • Code Quality - Function size, complexity, nesting depth
   • Performance - Algorithm efficiency, N+1 queries, caching
   • Best Practices - Naming, documentation, error handling
   • Maintainability - Duplication, coupling, testability

4. Severity Rating

   • CRITICAL - Security vulnerability (must fix before merge)
   • HIGH - Bug or major code smell (should fix before merge)
   • MEDIUM - Minor issue (fix when possible)
   • LOW - Style/suggestion (consider fixing)

5. Architectural Status Contract

   • CLEAR - No unresolved architectural blocker was found
   • WATCH - Non-blocking design/tradeoff concern that must appear in the final synthesis
   • BLOCK - Unresolved design concern that prevents a merge-ready verdict

6. Specific Recommendations

   • File:line locations for each issue
   • Concrete fix suggestions
   • Code examples where applicable

7. Final Synthesis

   • Combine the code-reviewer recommendation and the architect status into one final verdict
   • Approval requires explicit evidence from both independent lanes; missing or failed delegation is a blocking unavailable-review state, not an approval fallback
   • Deterministic merge gating rules:
     • If architect status is BLOCK, final recommendation is REQUEST CHANGES
     • Else if code-reviewer recommendation is REQUEST CHANGES, final recommendation is REQUEST CHANGES
     • Else if architect status is WATCH, final recommendation is COMMENT
     • Else final recommendation follows the code-reviewer lane
   • The final report must make architect blockers impossible to miss

State/HUD Phase Contract

Code-review is a merge-readiness gate and Autopilot child phase, not a standalone tracked mode with a code-review-state.json lifecycle. Keep HUD/current-phase state explicit and minimal:

• Standalone $code-review activation: rely on the hook-owned skill-active-state.json entry (skill:"code-review", phase:"planning") for HUD visibility; do not create an ad-hoc code-review-state.json.
• Inside active Autopilot: before review work starts, keep mode:"autopilot" active and set the supervised phase to current_phase:"code-review" / skill-active phase:"code-review"; do not activate a peer workflow over Autopilot.
• On clean review: persist the review artifact/verdict under Autopilot handoff_artifacts.code_review and transition Autopilot to current_phase:"ultraqa" only after durable independent review evidence exists.
• On non-clean review: persist the review artifact/verdict, set Autopilot current_phase:"rework" for implementation-only fixes or current_phase:"ralplan" when requirements/planning must change, and keep the findings as the scoped handoff.

Minimal Autopilot phase declaration when the review stage begins:

omx state write --input '{"mode":"autopilot","active":true,"current_phase":"code-review"}' --json

Agent Delegation

Do not self-review as a fallback. If the code-reviewer or architect agent path is missing, unavailable, skipped, or fails, emit a clear unavailable-review result and block approval until the independent lane evidence exists.

Respect the user's current model and reasoning/effort selection when launching review lanes. Do not pass model or reasoning_effort overrides in the review-lane task calls unless the user explicitly asks for review-specific overrides; omitting them lets native subagents inherit the active session settings.

task(
  agent_type="code-reviewer",
  prompt="CODE REVIEW TASK

Review code changes for quality, security, and maintainability.

This is the code/spec/security lane. Do not absorb architectural ownership.

Scope: [git diff or specific files]

Review Checklist:
- Security vulnerabilities (OWASP Top 10)
- Code quality (complexity, duplication)
- Performance issues (N+1, inefficient algorithms)
- Best practices (naming, documentation, error handling)
- Maintainability (coupling, testability)

Output: Code review report with:
- Files reviewed count
- Issues by severity (CRITICAL, HIGH, MEDIUM, LOW)
- Specific file:line locations
- Fix recommendations
- Approval recommendation (APPROVE / REQUEST CHANGES / COMMENT)"
)

task(
  agent_type="architect",
  prompt="ARCHITECTURE / DEVIL'S-ADVOCATE REVIEW TASK

Review the same code changes from the architecture/tradeoff perspective.

Scope: [git diff or specific files]

Focus:
- System boundaries and interfaces
- Hidden coupling or long-term maintainability risks
- Tradeoff tension the main reviewer might miss
- Strongest counterargument against approving as-is

Output:
- Architectural Status: CLEAR / WATCH / BLOCK
- File:line evidence for each concern
- Concrete tradeoff or design recommendation"
)

Run both lanes in parallel, then synthesize them with the deterministic rules above.

External Model Consultation (Preferred)

The code-reviewer agent SHOULD consult Codex for cross-validation.

Protocol

1. Form your OWN review FIRST - Complete the review independently
2. Consult for validation - Cross-check findings with Codex
3. Critically evaluate - Never blindly adopt external findings
4. Graceful optional consultation fallback - Never block because optional external consultation tools are unavailable; this does not waive the required independent code-reviewer and architect lanes