panel-review

# Panel review A workflow skill. The user has just changed code (or explicitly asked for a review), and the change needs to be reviewed across four **orthogonal axes** — spec adherence, project standards, independent test adequacy, and specialist domain expertise — with each axis able to disagree with the others without being silenced. **Proactive trigger discipline.** This skill is meant to auto-fire after implementation, not on every chat message. Fire when there's a plausible code-change checkpoint — a finished feature, a fixed bug, a landed refactor, an "I think that's done" — but not on conversation, questions, planning, or tiny one-line edits the user clearly didn't want reviewed. When in doubt, run a quick `git diff --stat` and ask: "Want me to run a panel review on this?" rather than firing unilaterally on an ambiguous message. Each axis catches a different failure mode: - **Spec** catches *"implements the wrong thing well"*. Standards and Domain both pass; the diff just doesn't do what the issue asked. - **Standards** catches *"implements the right thing the wrong way for this repo"*. Spec passes; the conventions are broken. - **Test adequacy** catches *"the implementation and tests agree on a weaker contract"*. It checks observable AC coverage, negative-test mutation value, boundary cases, and whether the tests exercise the real seam. - **Domain** catches *"implements the right thing with a hidden landmine"*. Spec and Standards pass; a security, K8s, library, or duplication issue lurks. Reporting them separately is deliberate. Synthesised together, one axis masks another. Inspired by Matt Pocock's two-axis `/review` skill (Standards + Spec), generalised with a Domain axis powered by the project's installed specialist reviewer agents. ## Operating modes **Interactive mode (default):** keep the confirmation behavior below: ask when no fixed point/spec is available, the diff is large, or the Domain panel has 5+ agents. **Orchestrator mode:** `/plan-orchestrate` supplies the fixed point and spec. Do not pause for confirmation because the diff or panel is large; announce the scope and proceed. Keep the same bounded briefs and selected panel, record any missing source or reviewer failure, and always finish with the stable `PANEL:` line. All other interactive behavior is unchanged outside this mode. ## Prerequisites - Working directory is a git repository (or files were explicitly named). - At least one of: a spec source (issue / PRD), project standards docs (CLAUDE.md, etc.), or one or more reviewer agents installed at `.claude/agents/` or `~/.claude/agents/`. One axis is enough to run. - `gh` CLI installed if reviewing a PR by number or fetching issue bodies. ## Step 1 — Pin the fixed point The fixed point is what we're diffing **against**. Whatever the user said is the fixed point — a commit SHA, branch name, tag, `origin/main`, `HEAD~5`, a PR number, an explicit file list. Don't be opinionated; pass it through. If the user didn't specify, **ask** before proceeding: > Review against what — a branch (`main`?), a commit, "since I > started this branch", or a PR number? Do not silently auto-detect. The whole review hangs on this and a wrong guess wastes the panel's effort. Once pinned, capture once and reuse: - **Diff command:** `git diff <fp>...HEAD` (three-dot — compares against the merge-base, which is what "what did this branch change?" means). - **Commit list:** `git log <fp>..HEAD --oneline` (used to find issue/spec references). - **File list:** `git diff <fp>...HEAD --name-only`. Print to the user: > Reviewing against **<fp>**: N files, M insertions, L deletions, > K commits. > <file list, capped at 20 with "+N more"> If the diff is **empty**, ask for explicit file paths or a PR number. If the diff is **> 50 files or > 3000 lines**, interactive mode asks whether to split into smaller reviews or proceed. Orchestrator mode proceeds without confirmation and keeps every reviewer brief bounded. ## Step 2 — Read project context In one parallel batch, read the repository's instruction and standards sources: `CLAUDE.md`/`AGENTS.md` (including parents), `.claude/rules/*.md`, relevant architecture/design/ADR indexes, `SECURITY.md`, `CONTRIBUTING.md`, and any `CONTEXT.md`/`CONTEXT-MAP.md` files that exist. ## Step 3 — Detect the spec source The Spec axis needs to know what was *asked for*. Look in this order: 1. **Issue references in commit messages.** Parse `git log <fp>..HEAD --format=%B` for `#123`, `Closes #45`, `Fixes GH-67`, `gitlab !89`, `JIRA-100`, `LIN-1234`. Fetch the issue body via `gh issue view <N>` (or the project's `docs/agents/issue-tracker.md` workflow if present). 2. **A path the user passed as an argument** — "review against spec at docs/specs/foo.md". 3. **PRD / spec files** under conventional locations matching the branch name or feature: `docs/specs/<name>.md`, `docs/prd/<name>.md`, `specs/<name>.md`, `.scratch/<name>.md`, `docs/acceptance/<name>.md`. 4. **If nothing is found**, ask the user: > I don't see a spec or issue reference for this branch. Path > to a spec, or skip the Spec axis? If the Spec axis is skipped, note it in the final output as "Spec axis: no source available — skipped" with the reason. **Do not fabricate a spec.** ## Step 4 — Inventory standards sources The Standards axis needs to know what *conventions* this repo documents. Collect filenames (the Standards subagent will read them): - `CLAUDE.md`, `AGENTS.md`, `CONTRIBUTING.md` - `CONTEXT.md`, `CONTEXT-MAP.md`, per-directory `CONTEXT.md` files - `docs/adr/*.md` (architectural decisions ARE standards) - `STYLE.md`, `STANDARDS.md`, `STYLEGUIDE.md` at repo root or under `docs/` - `.claude/rules/*.md` - `docs/design/principles.md` if present **Explicit skip rule** (inherited from Matt's design): tell the Standards subagent not to re-check anything enforced by detected formatter, linter, type-checker, compiler, or project tooling configs (for example `.editorconfig`, ESLint/Biome/Prettier/TypeScript, `.golangci.yml`, pyproject Ruff/Black/mypy, rustfmt/clippy). Re-flagging tool-enforced rules wastes tokens. If no standards docs are found, note "Standards axis: no project standards docs found — axis returned an empty report" in output; don't skip the axis silently. ## Step 5 — Inventory the available domain panel List `.claude/agents/*.md` and `~/.claude/agents/*.md`. Parse each file's `description` field (frontmatter). Build a table: | Agent | Scope (first line of description) | Defer-to | |---|---|---| This is the **domain panel** of available specialist reviewers. Don't fabricate agents that aren't installed — if a dimension has no matching agent, note the gap rather than skip the dimension silently. ## Step 6 — Classify the diff (Domain axis) For each file in the diff, identify the domain dimensions that apply. The mapping below is a default; **project-specific agents take precedence over generic ones** when their scope matches. | Dimension | File / content signals | Default agent | |---|---|---| | **Security (cross-cutting)** | HTTP handlers, route registration, `Authorization`, `crypto.`, `tls`, `fetch(` / `http.Get` with user input (SSRF), deserialisation, SQL/template assembly, JWT, OAuth, password handling | `secure-code-reviewer` | | **Architecture (cross-language)** | New module / package; new public API; cross-module imports; renamed exported type; new interface; new top-level directory; >3 files touched across distinct modules; new layering boundary; or any non-trivial `*.go` / `*.ts` / `*.tsx` / `*.py` / `*.rs` / `*.java` change with structural shape | `software-architect` | | **K8s in-cluster** | `*.yaml` / `*.yml` with `kind:`, `Chart.yaml`, `values.yaml`, `kustomization.yaml`, `templates/*.yaml` | `kubernetes-deployment-expert` | | **K8s controller / CRD** | `api/v*alpha*` / `api/v*beta*` / `api/v1`, `controllers/`, `internal/controller/`, imports of `sigs.k8s.io/controller-runtime`, `controller-gen` markers | `kubernetes-operator-expert` | | **DevOps / CI / IaC** | `.github/workflows/*`, `.gitlab-ci.yml`, `*.tf`, `*.tfvars`, `Dockerfile`, `*.dockerfile`, `cloudbuild.yaml`, `Jenkinsfile`, `Pulumi.yaml`, `cdk.json`, `Taskfile.yml`, `Makefile` | `devops-expert` | | **Duplication** | Three+ touched files with similar shape; new files that look like copies of existing | `code-duplication-reviewer` | | **Library reuse / dep audit** | `go.mod`, `package.json`, `requirements.txt`, `pyproject.toml`, new top-level dependency, hand-rolled utility shapes | `library-reuse-reviewer` | | **Reinvention / over-build** | Any non-trivial code diff (default-on, NOT signal-gated — see below) | `library-reuse-reviewer` + `code-duplication-reviewer` | | **Project-specific surfaces** | (varies — read each available agent's `description` frontmatter to learn its scope) | Any project-level agent in `.claude/agents/` that names a domain not covered above — typically architects for a specific framework, protocol, API surface, or UI workspace | If no Domain agent is available (including a pure-docs diff), mark `Domain axis: unavailable — no applicable installed specialist` and still fan out Spec, Standards, and Test adequacy. Domain unavailability is not a reviewer failure. Classification rules: - **Always include `secure-code-reviewer`** for diffs touching application code with external trust boundaries. - **`software-architect`** is the default architecture reviewer for any non-trivial code diff — cross-language design plus the language-architecture role when no language-specific architect is installed. - **Project-specific architects** in the repo's `.claude/agents/` compose with `software-architect` rather than replacing it. Both can run on the same diff: the project-specific one carries domain-loaded invariants and ADR knowledge, `software-architect` carries the cross-cutting design lens. - **`code-duplication-reviewer`** and **`library-reuse-reviewer` are DEFAULT-ON for any non-trivial code diff** (any diff touching application logic — not pure-docs / pure-config). Do NOT gate them on "a new dependency was added" or "actual duplication shape is already visible". Their entire job is to FIND the duplication and the reinvented stdlib that ISN'T obvious from the diff surface; gating them on the signal already being visible skips them on exactly the diffs where they add the most value. When in doubt, include both — they default to silence when they find nothing. The reuse pair is cheap insurance against the most common over-build failure mode an agent introduces (reinvented stdlib, speculative abstraction, unrequested layer). See the reuse-ladder brief in Step 8. - **Pure-docs diffs** skip the Domain axis (Spec axis may still run). - **Pure-test diffs**: a single Domain reviewer (project's test-review agent if present, else language architect). ## Step 7 — Announce the four-axis plan ``` Reviewing against <fp>: N files, M insertions, L deletions, K commits. Spec axis: checking against #123 ("Add /preview endpoint") Standards axis: reading CLAUDE.md, .claude/rules/, docs/adr/ skipping tooling: golangci-lint, biome, prettier Test adequacy: independently tracing requirements to assertions and seams Domain axis (running in parallel): - secure-code-reviewer — auth/HTTP/SSRF surface in api/handlers/ - kubernetes-deployment-expert — deploy/staging/ manifests - devops-expert — .github/workflows/release.yml changes - library-reuse-reviewer — default-on: reinvented stdlib / over-build - code-duplication-reviewer — default-on: duplication across the diff Skipping (no diff in scope): - kubernetes-operator-expert Gaps (dimension detected, no matching agent installed): - (none) ``` Wait for user pushback only in interactive mode when the panel is large (5+ agents across the Domain axis) or they asked for a dry-run. Orchestrator mode proceeds immediately. ## Step 8 — Fan out (PARALLEL) In a **single assistant turn**, issue all of: - 1 × `Agent` call for the **Spec** axis (general-purpose subagent with brief). - 1 × `Agent` call for the **Standards** axis (general-purpose subagent with brief). - 1 × `Agent` call for the **Test adequacy** axis (general-purpose subagent with brief). - N × `Agent` calls for the **Domain** panel (named specialist agents). They run concurrently, separate contexts, no order dependencies. ### Spec subagent - `subagent_type`: `general-purpose` - `description`: "Spec adherence check for <scope>" - `prompt`: > Diff to review: `git diff <fp>...HEAD` > Commit list: <list> > Spec source: <path or inline contents of the issue/PRD> > > Read the spec carefully, then read the diff. Report: > > 1. **Missing** — requirements the spec asked for that the diff > doesn't implement, or only partially implements. > 2. **Scope creep** — behaviour added by the diff that the spec > didn't ask for. > 3. **Wrong** — requirements that look implemented but where > the implementation appears incorrect against the spec. > > Quote the specific spec line / requirement for each finding. Classify > **every** finding explicitly as `blocker`, `important`, or `advisory`: > blocker = must fix before merge; important = concrete non-blocking fix; > advisory = judgement/polish. Under 400 words. Default to silence when > uncertain — only flag concrete mismatches. > > Root-cause discipline: when a finding names a symptom, note > whether the diff fixes the root cause or only the path the > ticket names — a sibling caller may still be broken. If no spec source was found in Step 3, **skip this call** and note in final output. ### Standards subagent - `subagent_type`: `general-purpose` - `description`: "Standards conformance check for <scope>" - `prompt`: > Diff to review: `git diff <fp>...HEAD` > Standards source files (read these): <list from Step 4> > Tooling-enforced configs to SKIP (tooling already runs on > every commit; do not re-flag what they cover): <list> > > Read the standards docs, then the diff. Report — per file / > hunk where relevant — every place the diff violates a > documented standard. Cite the standard (file + the rule). > Classify **every** finding explicitly as `blocker`, `important`, or > `advisory`: blocker = must fix before merge; important = concrete > non-blocking fix; advisory = judgement/polish. Under 400 words. Default to > silence when uncertain. ### Test-adequacy subagent - `subagent_type`: `general-purpose` - `description`: "Independent test-adequacy check for <scope>" - `prompt`: > Diff to review: `git diff <fp>...HEAD` > Spec source: <path or inline contents; state unavailable if skipped> > Acceptance plan / verify contract: <path if present> > > Review tests independently of the implementation. Trace each requirement or > AC to an assertion at the lowest adequate layer. Flag missing or weaker > coverage, tests that cannot fail on regression, negative assertions without > a planted violation, fake-only tests that bypass the real seam, and missing > boundary/failure cases. Do not re-report style or implementation findings. > Classify findings as blocker, important, or advisory and cite test paths and > requirement/AC ids. Under 400 words; default to silence when adequate. For a pure-docs or pure-config diff with no executable behavior contract, mark this axis not applicable; that is a deliberate skip, not a reviewer failure. ### Domain agents For each specialist in the panel: - `subagent_type`: the agent's `name` field. - `description`: "<agent-name> review of <scope>". - `prompt`: - The exact diff scope (`git diff <fp>...HEAD -- <paths>`).