/review-code

name: review-code
description: |
  Code review of current git changes with an expert senior-engineer lens. Detects SOLID violations, security risks, and proposes actionable improvements.
  Use when performing code reviews.

Code Review

Overview

Review whether the current changes achieve their intended behavior, preserve affected contracts and meet applicable delivery constraints. Apply SOLID, architecture, security, quality and removal guidance alongside that reasoning. Default to review-only output unless the user asks to implement changes.

Conventions

Load these through [packet-preparation.md](packet-preparation.md)'s preparation sequence. It supplies complete original instruction text in fewer Read calls; the direct-read fallback uses the same sources.

• **Read capy knowledge base conventions** at [shared-capy-knowledge-protocol.md](shared-capy-knowledge-protocol.md).
• **Always read change context** at [shared-change-context.md](shared-change-context.md) and the common method at [functional-review.md](functional-review.md), even when no profile matches.
• **Read task scope** at [shared-review-scope-protocol.md](shared-review-scope-protocol.md); pending work does not excuse regressions in current flows.
• **Read profile detection** at [shared-profile-detection.md](shared-profile-detection.md). Use its records to select and load domain guidance before investigation.

Required Outputs

Before declaring the review complete, verify all outputs are delivered:

• [ ] Review report presented to user
• [ ] Intent, scope/baselines, inspected behavior, applicable compatibility conclusions and evidence limits reported
• [ ] P0/P1 systemic findings indexed as `kk:review-findings` (skip if no qualifying findings)
• [ ] Required next steps handled, or clean review closed without proposing new work

Indexing is owned by this skill — callers (e.g., `/kk:implement`) do NOT duplicate it.

Review Modes

Standard Mode (`/kk:review-code`)

Reviews code in the main conversation context. Fast, single-pass review using the workflow below.

Isolated Mode (`/kk:review-code:isolated`)

Delegates detection to independent reviewers that did not write the code, then annotates their findings with author context. One internal `code-reviewer` sub-agent plus external review chosen **once at discovery**: `pal codereview` when an API model is available, otherwise one `pal clink` CLI review per eligible non-authoring provider using its configured model (variable count; on the clink roster the authoring provider is excluded; PAL's shipped clients get default provider mappings that the report labels as assumed, and an operator declaration, for example in the project's `CLAUDE.md` or `AGENTS.md`, overrides them and is required for any other client name). Produces a report organized by agreement level with corroborated findings highlighted.

• **Cost**: Higher (sub-agent + one or more external reviews + annotation); clink cost scales with the roster size
• **Isolation**: True — reviewers have zero authorship bias or session context; each external reviewer gets a fresh conversation
• **Degradation**: Graceful — a failed reviewer is disclosed and the others proceed; an API runtime failure never switches to clink; if every reviewer fails, suggests standard mode fallback
• **Requires**: a PAL build whose `listmodels` exposes CLI client observations in every mode for the clink path; unresolved discovery keeps the internal review with an explicit external coverage limit. Operator setup (the minimum PAL build, CLI read access outside the project, provider declarations) is described in the plugin README under "Isolated reviews with external models"
• **Best for**: When extra rigor is worth the cost (pre-merge, high-stakes changes)

See [review-isolated.md](./review-isolated.md) for the isolated workflow.

Severity Levels

| Level | Name | Description | Action | | ------ | -------- | ---------------------------------------------------------------- | ---------------------------------- | | **P0** | Critical | Demonstrated critical security, integrity or availability impact needing immediate mitigation | Must block merge | | **P1** | High | Logic error, significant SOLID violation, performance regression | Should fix before merge | | **P2** | Medium | Code smell, maintainability concern, minor SOLID violation | Fix in this PR or create follow-up | | **P3** | Low | Style, naming, minor suggestion | Optional improvement |

Workflow

Mandatory ordering — methodology before evidence

The standard workflow below is strictly sequential. **Load the process, shared protocols, common functional method and every resolved profile checklist before source investigation, `capy_search`, findings, edits or tests.** After basic process instructions load, the sole early-content exception is bounded inspection to resolve declared detection/conditional-load predicates: at most approximately 16 KiB per candidate file; log predicate/path, and conservatively load an instruction when its conditional is undecidable within the bound. No behavioral analysis or full-diff investigation is permitted during routing.

This ordering is load-bearing, not stylistic. Reviewing from a diff before loading profile checklists is the known failure mode this skill is designed to prevent: the LLM has enough from the diff to produce plausible findings, and optimizes away the methodology if the workflow permits.

**Phases** (same order as [review-process.md](./review-process.md)):

1. Prepare instruction packets — read [packet-preparation.md](packet-preparation.md), run the helper below for `bootstrap`, and Read every returned part; this loads process, shared protocols, common method and all known detection rules 2. Select the diff — use status/filenames only;