--- allowed-tools: - Task - TaskOutput - TaskCreate - TaskUpdate - TaskList - AskUserQuestion - Read - Grep - Glob - LS - Bash(git *) - Bash(gh pr *) - Bash(gh api *) - Bash(rg *) - mcp__plugin_claude-mem_mcp-search__search - mcp__plugin_claude-mem_mcp-search__get_observations argument-hint: '[deep] [team] [external] [architecture]' context: fork description: Code review covering security, quality, tests, implementation, documentation, and architecture / module-depth. Use when the user asks to review code, check changes, audit a PR or diff, find refactoring opportunities, or look for shallow modules and over-abstraction. NOT for fixing the issues found (use fixing-code) or applying refactors (use refactoring-code). name: reviewing-code user-invocable: true --- # Code Review Review changed code for security, quality, test coverage, and architecture. Ground every finding in concrete evidence: a `file:line` reference or tool output. If a task-tracking facility is available, track these phases as tasks. ## Workflow 1. Determine review scope. 2. Detect languages from changed files. 3. Delegate to reviewer agents (one per detected language for a standard review; the thorough set per language for deep coverage). 4. Aggregate findings by severity and report. Track progress through the workflow phases using `TaskCreate` / `TaskUpdate`. If `$ARGUMENTS` is passed, the keywords below tune the workflow: - `deep` — use the thorough-review agent set (full dimension coverage per language). - `team` — run the reviewers as an agent team that challenges each other's findings and converges on consensus. - `external` — additionally spawn external AI reviewers. Only when explicitly requested; never run by default. - `architecture` — pass the Architecture vocabulary into every reviewer's prompt. Without `external`, run only the language reviewers from this skill. Never spawn external reviewers implicitly. ## Determine scope Ask the user which scope to review, with these options: - Uncommitted changes - Branch compared to the default branch - Specific files (user provides paths) Resolve to the appropriate git invocation and use it consistently across phases. If the user already named a scope in their request, use it without asking. Use `AskUserQuestion` with header "Review scope" and the three options spelled out above. ## Detect languages Scan the changed-file extensions and identify which language reviewers are needed. ## Delegate to reviewer agents For each detected language, delegate to the agent(s) below in parallel if the runtime supports parallel sub-agents. Pass each agent: the scope, the project conventions (read `CONTEXT.md`/`docs/adr/` first), and whether architecture focus is requested. Spawn each agent via `Task(subagent_type="", prompt=...)`. Each agent's model is set in its own metadata; do not pin a model from the skill. When `team` is set, spawn the reviewers as an agent team so they challenge each other's findings; report format prefixes findings with `[Flagged by: agent1, agent2]`. When `external` is set, additionally spawn in parallel: - `Task(subagent_type="codex-assistant", prompt="review: code from ")` — security, quality, architecture from a second model. - `Task(subagent_type="gemini-consultant", prompt="review: architecture of ")` — architecture alternatives and trade-offs. ### Standard review — one agent per language - Go → `go-engineer` - Python → `python-engineer` - TypeScript → `typescript-engineer` - Web (HTML / CSS / JS) → `web-engineer` ### Thorough review — full dimension coverage per language - Go: `go-qa`, `go-idioms`, `go-tests`, `go-impl`, `go-docs`, `go-simplify` - Python: `py-qa`, `py-idioms`, `py-tests`, `py-impl`, `py-docs`, `py-simplify` - TypeScript: `ts-qa`, `ts-idioms`, `ts-tests`, `ts-impl`, `ts-docs`, `ts-simplify` - Web: `web-qa`, `web-idioms`, `web-tests`, `web-impl`, `web-docs`, `web-simplify` Dimension meanings (same across languages): - `*-qa` — logic, security, OWASP, race conditions, unchecked errors, resource leaks - `*-idioms` — patterns, conventions, stdlib usage, error handling - `*-tests` — coverage, edge cases, mocking discipline - `*-impl` — requirements match, dependency injection, edge cases - `*-docs` — comments, docstrings, API docs (ARIA labels for web) - `*-simplify` — over-abstraction, dead code, pass-throughs If the runtime cannot spawn subagents, walk the dimensions sequentially as the main agent. ## Review rules (passed to every reviewer) - Cite concrete `file:line` evidence or tool output for every finding. No evidence, no finding. - Findings include severity and a concrete fix. - For security findings, remind the user: keep private code local; do not paste private diffs into web tools. Use web only for external facts (CVE, library docs); cite separately. - Read relevant `CONTEXT.md`, `CONTEXT-MAP.md`, and `docs/adr/` before naming architecture findings. If a candidate contradicts an ADR, flag only when the friction justifies reopening the decision. - Ask one clarifying question at a time; do not batch. ## Architecture vocabulary Apply when the user asks for architecture focus. Pass these terms into reviewer prompts so findings use shared vocabulary: - **Module** — anything with an interface and an implementation: function, class, package, slice. - **Interface** — everything callers must know: types, invariants, ordering, error modes, config, performance. - **Seam** — where an interface lives; a place behavior can change without editing in place. - **Adapter** — a concrete thing satisfying an interface at a seam. - **Depth** — leverage at the interface: lots of behavior behind a small interface. - **Leverage** — caller value from depth. - **Locality** — change, bugs, and verification concentrated in one place. Deletion test: if deleting a module makes complexity vanish, it was a pass-through. If complexity reappears across callers, the module was earning its keep. Seam rule: one adapter means a hypothetical seam; two adapters means a real seam. Do not propose ports without real variation. ## Historical context (optional) If cross-session memory tooling is available, query for prior observations on the files about to be reviewed. Forward relevant findings into reviewer prompts so old issues are not re-litigated. Skip silently if no such tooling is configured. If `mcp__plugin_claude-mem_mcp-search__search` is available, query for past findings on the files being reviewed: ```text search({ query: "", limit: 10 }) ``` Fetch full observations via `get_observations` for relevant hits. Forward key findings into reviewer prompts. Skip silently if the MCP is not installed. ## Report format ```markdown ## Code Review Summary **Scope**: **Languages**: **Reviewers**: ### CRITICAL (must fix) - [] `file:line` — issue. Fix. ### IMPORTANT (should fix) - [] `file:line` — issue. Fix. ### SUGGESTIONS - [] `file:line` — issue. Fix. ### Architecture opportunities (if requested) - Candidate: `module`. Problem: shallow / pass-through / fake seam. Deepening move: . Test benefit: . ### Recommended actions 1. ``` ## Writing style - One sentence per finding. No preamble, no "I noticed that…". - Cut hedging: "potential", "might", "consider". State what is wrong. - Direct: "This leaks memory" not "This could potentially lead to memory issues". - Technical precision: include type names, function signatures, line numbers. ## Edge cases - No changes in scope → "Nothing to review." - Linters missing → say so explicitly; still review by reading. - Tests missing → flag as a finding under the `*-tests` dimension. - Runtime has no subagents → main agent performs the review sequentially using the same rubric.