code-review

name	code-review
description	Run a comprehensive code review

Code Review Skill

Conduct a thorough, read-only 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

Plain /omagy:code-review is read-only. It does not edit files, run automatic fixes, persist workflow gate state, or promise follow-up implementation. If the user explicitly asks to review and fix, run this review first and then route the findings into /omagy:ralph or /omagy:ultrawork.

GPT-5.5 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. When the scoped files or task are security-sensitive, add a conditional security-reviewer lane instead of using a standalone /omagy:security-review command.

1. Identify Changes

   • Run git diff to find changed files when the current scope is a git repository.
   • Determine scope of review: specific files, current project directory, explicit path, branch, PR, or entire change set.
   • If git diff is unavailable but the current directory is clearly the project under review, use the current project directory as the scope.
   • Do not scan sibling repositories to pick a target implicitly.
   • Stay inside the current review scope. Do not inspect Antigravity brain, transcript, conversation, or generated log directories unless the user explicitly includes them in the review scope.
   • If there is no changed-file or PR scope and the review falls back to the current project directory, treat the result as a broad project audit, not a merge gate for a specific change. In audit mode, report dormant, internal, or future-workflow risks as WATCH unless they break a current entrypoint or create a concrete security, data-loss, or production incident risk.

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
   • security-reviewer lane - conditional lane for auth, secrets, permissions, injection, sandbox, plugin, MCP, subprocess, dependency, supply-chain, PII, or compliance risk
   • Both lanes run in parallel and produce distinct outputs before final synthesis

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
   • Deterministic merge gating rules:
     • If architect status is BLOCK, final recommendation is REQUEST CHANGES
     • If security status is BLOCK, final recommendation is REQUEST CHANGES
     • Else if code-reviewer recommendation is REQUEST CHANGES, final recommendation is REQUEST CHANGES
     • Else if security status is WATCH, final recommendation is COMMENT
     • 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
   • The final report must make security blockers impossible to miss

Agent Delegation

Use agy's native subagent capability directly. The intended call shape is:

{
  "Subagents": [
    {
      "TypeName": "code-reviewer",
      "Role": "code-reviewer",
      "Prompt": "CODE REVIEW TASK\n\nReview code changes for quality, security, and maintainability.\n\nThis is the code/spec/security lane. Do not absorb architectural ownership.\n\nScope: [git diff, current project directory, PR, or specific files]\n\nStay inside the scope. Do not inspect Antigravity brain, transcript, conversation, or generated log directories unless explicitly included.\n\nReview Checklist:\n- Security vulnerabilities (OWASP Top 10)\n- Code quality (complexity, duplication)\n- Performance issues (N+1, inefficient algorithms)\n- Best practices (naming, documentation, error handling)\n- Maintainability (coupling, testability)\n\nOutput: Code review report with:\n- Files reviewed count\n- Issues by severity (CRITICAL, HIGH, MEDIUM, LOW)\n- Specific file:line locations\n- Fix recommendations\n- Approval recommendation (APPROVE / REQUEST CHANGES / COMMENT)"
    },
    {
      "TypeName": "architect",
      "Role": "architect",
      "Prompt": "ARCHITECTURE / DEVIL'S-ADVOCATE REVIEW TASK\n\nReview the same code changes from the architecture/tradeoff perspective.\n\nScope: [same scope as code-reviewer]\n\nStay inside the scope. Do not inspect Antigravity brain, transcript, conversation, or generated log directories unless explicitly included.\n\nFocus:\n- System boundaries and interfaces\n- Hidden coupling or long-term maintainability risks\n- Tradeoff tension the main reviewer might miss\n- Strongest counterargument against approving as-is\n\nBLOCK only when the scoped change or current audited entrypoint is not merge-ready because of a concrete unresolved design risk. In broad project audit mode, dormant/internal/future-workflow risks are WATCH unless they break a current entrypoint or create concrete security, data-loss, or production incident risk.\n\nOutput:\n- Architectural Status: CLEAR / WATCH / BLOCK\n- File:line evidence for each concern\n- 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 another approved model or reviewer surface for cross-validation when one is already available in the agy session.

Protocol

1. Form your OWN review FIRST - Complete the review independently
2. Consult for validation - Cross-check findings with the external reviewer
3. Critically evaluate - Never blindly adopt external findings
4. Graceful fallback - Never block if tools unavailable

When to Consult

• Security-sensitive code changes
• Complex architectural patterns
• Unfamiliar codebases or languages
• High-stakes production code

When to Skip

• Simple refactoring
• Well-understood patterns
• Time-critical reviews
• Small, isolated changes

Tool Usage

Prefer native code-reviewer agent consultation or available CLI-backed ask surfaces when enabled. Optional MCP compatibility ask tools may be used only when already enabled. If consultation tools are unavailable, fall back to the code-reviewer and architect agents.

Output Format

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

Scope Mode: DIFF_REVIEW
Scope: git diff / PR / explicit files
Files Reviewed: 8
Total Issues: 12
Architectural Status: WATCH

CRITICAL (0)
-----------
(none)

HIGH (0)
--------
(none)

MEDIUM (7)
----------
1. src/api/auth.ts:42
   Issue: Email normalization logic is duplicated instead of reusing the shared helper
   Risk: Validation rules can drift between authentication paths
   Fix: Route both paths through the shared normalization helper

2. src/components/UserProfile.tsx:89
   Issue: Derived permissions are recalculated on every render
   Risk: Avoidable work during profile refreshes
   Fix: Memoize the derived permissions list or compute it upstream

3. src/utils/validation.ts:15
   Issue: Form-layer and server-layer validation messages are defined separately
   Risk: User-facing validation guidance can become inconsistent
   Fix: Share one validation message helper across both call sites

LOW (5)
-------
...

ARCHITECTURE WATCHLIST
----------------------
- src/review/orchestrator.ts:88
  Concern: Review result synthesis relies on implicit ordering rather than an explicit blocker contract
  Status: WATCH
  Recommendation: Define deterministic merge gating before expanding reviewers

SYNTHESIS
---------
- code-reviewer recommendation: COMMENT
- architect status: WATCH
- final recommendation: COMMENT

RECOMMENDATION: COMMENT

Address any WATCH concerns before treating the change as merge-ready.

When there is no diff/PR/explicit change set and the review falls back to the current project directory, report audit mode explicitly:

Scope Mode: PROJECT_AUDIT
Scope: current project directory
Files Reviewed: 4 focused files
Scope Note: Broad project audit, not a merge gate for a specific change.

SYNTHESIS
---------
- code-reviewer recommendation: REQUEST CHANGES / COMMENT / APPROVE
- architect status: CLEAR / WATCH / BLOCK
- audit recommendation: REQUEST CHANGES / COMMENT / APPROVE

Audit Recommendation: REQUEST CHANGES / COMMENT / APPROVE
Merge Recommendation: N/A - no specific diff, PR, or change set was reviewed.

In PROJECT_AUDIT mode, never present the final verdict as a direct merge recommendation. Use Audit Recommendation for the project-level quality result and keep Merge Recommendation as N/A unless a specific diff, PR, or explicit change set was reviewed. Do not write final recommendation or bare RECOMMENDATION in PROJECT_AUDIT mode.

Review Checklist

The code-reviewer lane checks:

Security

• No hardcoded secrets (API keys, passwords, tokens)
• All user inputs sanitized
• SQL/NoSQL injection prevention
• XSS prevention (escaped outputs)
• CSRF protection on state-changing operations
• Authentication/authorization properly enforced

Code Quality

• Functions < 50 lines (guideline)
• Cyclomatic complexity < 10
• No deeply nested code (> 4 levels)
• No duplicate logic (DRY principle)
• Clear, descriptive naming

Performance

• No N+1 query patterns
• Appropriate caching where applicable
• Efficient algorithms (avoid O(n²) when O(n) possible)
• No unnecessary re-renders (React/Vue)

Best Practices

• Error handling present and appropriate
• Logging at appropriate levels
• Documentation for public APIs
• Tests for critical paths
• No commented-out code

Architect Lane Checklist

The architect lane checks:

• Boundary or interface changes are explicit
• New coupling/tradeoff risks are surfaced
• Long-horizon maintainability concerns are evidence-backed
• Architectural status is one of CLEAR, WATCH, or BLOCK
• Any BLOCK concern cites the reason merge-ready status should be withheld

Approval Criteria

APPROVE - code-reviewer returns APPROVE and architect status is CLEAR

REQUEST CHANGES - code-reviewer returns REQUEST CHANGES or architect status is BLOCK

COMMENT - code-reviewer returns COMMENT with architect status CLEAR, architect status is WATCH, or only LOW/MEDIUM improvements remain

Scenario Examples

Good: The user says continue after the workflow already has a clear next step. Continue the current branch of work instead of restarting or re-asking the same question.

Good: The user changes only the output shape or downstream delivery step (for example make a PR). Preserve earlier non-conflicting workflow constraints and apply the update locally.

Bad: The user says continue, and the workflow restarts discovery or stops before the missing verification/evidence is gathered.

Use with Other Skills

With Team:

/omagy:team "review recent auth changes and report findings"

Includes coordinated review execution across specialized agents.

With Ralph:

/omagy:ralph code-review then fix all issues

On the explicit Ralph path, review findings should flow into automatic fix follow-up without another permission prompt. Plain code-review itself remains read-only and does not promise auto-fix.

With Ultrawork:

/omagy:ultrawork review all files in src/

Parallel code review across multiple files.

Plain code-review does not write workflow gate state. Structured review gate state belongs to higher-level workflow or pipeline commands, not the default code-review skill path.