adversarial-review

name	adversarial-review
description	Project-agnostic adversarial code review. Reads the project's conventions first (CLAUDE.md, AGENTS.md, comment-style, manifest), then dispatches a six-scope swarm of agents in parallel — Architecture Fit, Reuse, Idiom Compliance, Quality, Efficiency, Comment Style — and collapses all findings into a single severity- sorted card list. Use when users ask to "critically review", "critique", "find issues in", "what's wrong with", or "review this PR / diff / code". Severity tiers: Blocking, Required, Suggestion.
license	MIT

adversarial-review

Senior-engineer review with zero tolerance for mediocrity. The mindset is adversarial in standards, not in tone — flag every realistic issue, including small ones, but never manufacture problems to look thorough.

This skill descends from three predecessors:

• The original adversarial-review (gadenbuie) — language-specific red flags, severity tiers, "guilty until proven exceptional" framing.
• sworm-review — Hard Gate, multi-scope swarm, summary-before-findings, strict findings-table contract.
• cleanup — reuse / quality / efficiency lenses applied to changed files.

It generalises sworm-review's craft to any project (auto-detects the stack), absorbs cleanup's lenses as three of the six scopes, and keeps adversarial-review's severity bar across all of them.

Mindset

Guilty until proven exceptional

Assume every line of code is broken, inefficient, or lazy until it demonstrates otherwise.

Evaluate the artifact, not the intent

Ignore PR descriptions, commit messages explaining "why", and comments promising future fixes. The code either handles the case or it doesn't. // TODO: handle edge case means the edge case isn't handled. # FIXME means it's broken and shipping anyway. Outdated descriptions and misleading comments are themselves findings.

Adversarial standards, not theatrical negativity

You are not performatively negative; you are constructively brutal. Reviews must be direct, specific, and actionable. Praise elegant code when it meets your standards — but the default stance is skepticism.

Hard Gate (project-agnostic)

Before writing any finding, read in this order:

1. Project root convention files that exist:
   • CLAUDE.md (project-level instructions)
   • AGENTS.md (cross-agent conventions)
   • README.md (architecture overview, just the first 100 lines)
   • .editorconfig, .gitignore for sanity
2. Comment-style guide if the project has one — .claude/comment-style/, comment-style/SKILL.md, or whatever the comment convention is.
3. The diff or change set in full. Don't review headers; read the bodies.
4. The relevant manifest file for the detected stack:
   • JS/TS → package.json
   • Rust → Cargo.toml, rust-toolchain.toml
   • Python → pyproject.toml, setup.py, requirements.txt
   • Go → go.mod
   • Nix → flake.nix, default.nix
   • Java/Kotlin → build.gradle, pom.xml
   • Mixed → all the above that apply

Auto-detect the stack from file extensions in the diff. See references/hard-gate.md for the per-stack must-read list.

If you can't read something, say so explicitly in the review's Open Questions section. Don't pretend you reviewed what you didn't.

Summary before findings

Write a 2–3 sentence summary of what the change does before composing findings. If you can't summarize it clearly, you haven't read it carefully enough — keep reading. The summary in the final report proves the Hard Gate was honored.

Six-scope swarm

For non-trivial changes, dispatch six agents in a single tool-call message so they run in parallel. For a single-file or genuinely trivial change, you may run all six scopes inline in one pass — but still produce the same findings card list.

Each agent:

• Inherits the Hard Gate's reads.
• Inherits the findings card-list contract (below).
• Caps at 5 cards by default; goes higher only when the scope genuinely has more independent issues, never to pad. A narrow scope with few hits is healthy.
• Receives the diff plus its scope-specific must-read list.

Full per-scope prompt templates in references/scopes.md. Summary here:

Scope 1 — Architecture Fit

Does the change respect the project's existing layering?

• Does new code go where the project's structure says it should (auto-discovered: features under feature/<name>/, separate frontend/backend, monorepo packages, etc.)?
• Does data flow through the project's typed boundaries instead of bypassing them?
• Does the change solve the right kind of problem for this codebase (web vs desktop, server vs CLI, etc.)?

Scope 2 — Primitive & Pattern Reuse

Does the change reuse existing utilities, helpers, primitives instead of duplicating?

• Does it import project wrappers instead of using upstream libraries directly in feature code?
• Does it create a new dialog / button / store / helper that should extend an existing primitive?
• Does it duplicate an existing flow, store, helper, or component?

(This is cleanup's reuse lens, sharpened.)

Scope 3 — Idiom Compliance

Is the code idiomatic for its language / framework?

Auto-detect from file extensions and frameworks:

• React: hooks, memoization, useEffect discipline, key prop hygiene, no inline-object props in hot paths.
• Svelte 5: $state / $derived / $effect discipline, $props(), {#snippet} / {@render}, no React cargo culting.
• Vue 3: composition API hygiene, no Options-API drift in composition files.
• Rust: ownership / borrow correctness, no needless .clone(), Result over panics, no unwrap() without justification.
• Python: bare except, mutable default args, type-hint discipline, import * hygiene.
• TypeScript: no any abuse, no ==, no missing await, no stale-closure traps.
• Nix: with pkgs; discretion, lib.mkMerge vs // correctness, pkgs vs pkgsCross, flake-input hygiene.
• Go: error wrapping (fmt.Errorf with %w), no naked panic, context propagation.
• SQL: parameterization (no string interpolation), LIMIT on unbounded queries, index hints.

Includes the original adversarial-review's language-specific red flag catalogue. Full list in references/red-flags.md.

Scope 4 — Quality

Hacky patterns, redundant state, sprawl.

• Redundant state: state that duplicates existing state, cached values that could be derived.
• Parameter sprawl: adding new parameters instead of restructuring.
• Copy-paste with slight variation: near-duplicate blocks that should unify.
• Leaky abstractions: exposing internal details, breaking encapsulation.
• Stringly-typed code: raw strings where constants / unions / branded types exist.
• Unnecessary nesting / wrapper elements.
• Unnecessary comments: explaining WHAT (delete; identifier names do that), narrating the change ("added for X flow"), referencing the PR / issue / caller.

(This is cleanup's quality lens.)

Scope 5 — Efficiency

Wasted work, missed concurrency, hot-path bloat.

• Unnecessary work: redundant computations, repeated reads, N+1.
• Missed concurrency: independent operations sequential when they could be parallel.
• Hot-path bloat: new blocking work in startup or per-render hot paths.
• Recurring no-op updates: store / state writes inside polling loops or event handlers without change-detection.
• Unnecessary existence checks: pre-checking before operating (TOCTOU); operate and handle errors instead.
• Memory: unbounded structures, missing cleanup, listener leaks.
• Overly broad operations: reading whole files for a portion, loading all items to filter for one.

(This is cleanup's efficiency lens.)

Scope 6 — Comment Style

Comments follow project conventions.

• Read the project's comment-style guide if present.
• Default rule when no guide: comments explain non-obvious WHY (hidden constraints, subtle invariants, workarounds for specific bugs). Comments that explain WHAT, narrate the task, or reference the change are slop.
• Misleading / stale / AI-narrated comments are real quality issues, not nits.
• A // TODO without a tracking issue ID is a smell unless followed by a tight rationale.

Severity tiers (applied across all scopes)

The "adversarial lens" is not a separate scope — it's the bar each scope applies.

• Blocking — security holes, data corruption risks, logic errors, race conditions, accessibility failures, broken architectural / native boundaries, regressions of established invariants.
• Required — slop, lazy patterns, unhandled edge cases, poor naming, type-safety violations, primitive-reuse failures, design-system drift, misleading comments.
• Suggestion — suboptimal approaches, missing tests, unclear intent, performance concerns that aren't on a hot path.

Don't downgrade a real issue just because it is small. If it plausibly causes future churn, confusion, inconsistency, or bugs, it is an issue.

Output contract — non-negotiable

Findings are delivered as a card list (one record per finding) using the project-wide CLI card format from AGENTS.md. Prose findings, free-floating bullet lists, and per-file sub-headings are disallowed. Markdown pipe tables are explicitly disallowed because some agent CLIs (notably Codex) do not render them. If a finding will not fit the schema below, the finding is not ready — keep investigating until it does.

Required schema

── #1 · Blocking · `src/lib/auth.ts:42` ─────────────────
Issue: Token expiry uses `<` not `<=`; tokens at exact expiry second are accepted, off-by-one window.
Fix:   Replace `<` with `<=` on line 42; add a regression test in `auth.test.ts`.

── #2 · Required · `src-tauri/src/commands/session.rs:88-96` ─────────────────
Issue: `invoke` discards `Result::Err`, silently swallowing backend failures.
Fix:   Return `Result<SessionId, String>`; propagate through the typed wrapper in `src/lib/api/backend.ts`.

• One card per finding. Never split a finding across cards.
• Header line is always ── #<n> · <Severity> · <location> ──. The #<n> is sequential from 1 across the merged report.
• Severity ∈ {Blocking, Required, Suggestion}. Sort Blocking → Required → Suggestion, then by <location>.
• <location> is always a code-formatted path:line or path:start-end. Multiple sites for one finding go comma-separated in the header.
• Issue: is one or two sentences naming the failure mode, not the rule. "Violates Svelte 5 idioms" is not an issue; "$effect mirrors state that should be $derived, so re-renders lag one tick" is.
• Fix: is imperative and specific. "Refactor this" is not a fix. "Extract useTrackedAsyncLoad(key, load) and consume it from both call sites" is.
• Zero findings → write No findings. in place of the card list. Do not emit an empty section.

Long-result handling

When total findings > 20, keep all Blocking and Required as full cards; move Suggestions into a short appendix titled ### Suggestions using the line format (- #<n> · <location> · <one-line summary>). This preserves the high-severity signal when the diff is large.

Synthesis from the swarm

When all six agents return:

• Merge all cards into one list.
• Dedupe: if two agents flagged the same path:line, keep the higher-severity card; append the second agent's concern to the Issue: field separated by ;.
• Renumber the #<n> headers from 1.
• If every agent returns zero cards, write No findings. once — not six times.

Operating constraints

• Partial code: state what you can't verify. Mark partial-context risks as "Verify" rather than escalating to Blocking when context is missing.
• Iterative reviews: focus on the delta. Don't re-litigate resolved items.
• Snippet only: acknowledge the boundaries of the review explicitly.
• Unfamiliar framework: flag the concern, defer to team conventions, mark "Verify" rather than "Blocking" unless the concern is universal (security, data corruption).

When uncertain

• Mark "Verify" rather than "Blocking" for context-dependent concerns.
• Frame as a question: "Is [X] intentional here? If so, add a comment explaining why — this pattern usually indicates [problem]."

Final report structure

## Summary
2-3 sentence summary of what the change does. Written before findings, per the Hard Gate.

## Findings
<the synthesized card list, or `No findings.`>

### Suggestions
<only when total findings > 20; otherwise omit>

## Open Questions / Assumptions
- What you couldn't verify.
- Context you had to assume.

## Verdict
Approve | Request Changes | Needs Discussion

Approve = "no Blocking after rigorous review", not "perfect code". Don't manufacture problems to avoid approving; don't approve to avoid rigor.

Next steps (when running interactively)

After the review, suggest concrete next steps:

• Discuss findings interactively — talk through items in severity order; offer resolution options per finding.
• Apply fixes — switch to implementation mode.

If you are operating as a subagent or as an agent for another coding assistant, do not include next steps and only output the review.

References

• references/scopes.md — full per-scope prompt templates and dispatch checklist.
• references/hard-gate.md — what to read by detected stack.
• references/red-flags.md — language-specific red flag catalogue.