From 2d88edcf5224211b7c40d6fdca96b3008a684dc0 Mon Sep 17 00:00:00 2001 From: Dmitry Dementiev Date: Fri, 15 May 2026 14:01:04 +0300 Subject: [PATCH] =?UTF-8?q?feat(skill):=20=D0=BF=D0=BE=D0=B4=D0=B4=D0=B5?= =?UTF-8?q?=D1=80=D0=B6=D0=BA=D0=B0=20Codex=20=D0=B8=20=D0=B8=D0=BD=D1=82?= =?UTF-8?q?=D0=B5=D0=B3=D1=80=D0=B0=D1=86=D0=B8=D1=8F=20=D1=81=20receiving?= =?UTF-8?q?-code-review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Зачем: - Скилл был привязан к Claude Code Agent Teams, в Codex приходилось вручную комбинировать /receiving-code-review и /claude-team-review. В реальной L4-сессии lead применял findings без верификации, что привело к большой структурной правке на основе неверной цитаты из upstream-issue. - Что: - SKILL.md: platform-agnostic спавн (Claude Code Task/Agent Teams, Codex native subagents). Новые шаги Evaluate findings (matrix + verification-by-type, REQUIRED SUB-SKILL на receiving-code-review) и Apply/push-back (three-section response — applied / re-scoped / rejected-with-reasoning). Fresh-spawn теперь operator-gated на любой платформе. Добавлена Red Flags table. - reviewer-prompt.md: новый briefing template с placeholders, заменяет Claude Code-specific agent definition. - README.md: пути установки исправлены на ~/.claude/skills/ и ~/.codex/skills/, формулировка cross-platform смягчена, секция эксперимента переведена в английский для единого языка. - adversarial-reviewer.md: удалён (содержимое переехало в reviewer-prompt.md). - .gitignore: tmp/ для локальных рабочих заметок. - Проверка: - Прогнать /claude-team-review на этих же изменениях в отдельной ветке для self-review. --- .gitignore | 1 + README.md | 164 ++++++++++---- SKILL.md | 492 ++++++++++++++++++++++++---------------- adversarial-reviewer.md | 113 --------- reviewer-prompt.md | 157 +++++++++++++ 5 files changed, 568 insertions(+), 359 deletions(-) create mode 100644 .gitignore delete mode 100644 adversarial-reviewer.md create mode 100644 reviewer-prompt.md diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..1c2f433 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +tmp \ No newline at end of file diff --git a/README.md b/README.md index 2afa265..cda31c6 100644 --- a/README.md +++ b/README.md @@ -1,59 +1,73 @@ # Claude Team Review -Adversarial code and plan review using Claude Code Agent Teams. +Adversarial code and plan review through a peer-reviewer subagent. -One teammate reviews. The lead fixes. Iterate until approved. +One subagent reviews. The lead evaluates findings, fixes what holds up, +pushes back on what doesn't, and asks for re-review. Up to 5 rounds. + +**Designed for Claude Code and Codex.** The skill is platform-agnostic +in its instructions, so it can potentially work on other hosts that +support subagents — but Claude Code and Codex are the verified targets. ## What is this -A [Claude Code skill](https://docs.anthropic.com/en/docs/claude-code) that -spawns an adversarial reviewer as an Agent Teams teammate. The reviewer -reads your project, runs tests, checks documentation, and delivers findings -with a skeptical stance. The lead (your main session) fixes issues and -requests re-review from the same teammate. If the teammate is no longer -active, the lead decides how to proceed — re-spawn or conclude. +A skill that spawns an adversarial reviewer as a subagent on the host +of your choice. The reviewer reads your project, runs tests, checks +documentation, and delivers findings with a skeptical stance. + +The lead (your main session) **does not blindly apply findings**. It +builds an evaluation matrix, verifies the technical claims, and replies +with applied / re-scoped / rejected-with-reasoning sections. The reviewer +gets a chance to contest the rejections in the next round. ### How it differs from [adversarial-review](https://github.com/dementev-dev/adversarial-review) **adversarial-review** uses two different models (Claude writes, Codex -reviews) — you get cross-model blind spot coverage and cheap re-review +reviews) — you get cross-model blind-spot coverage and cheap re-review via `codex exec resume`. It requires Codex CLI and an OpenAI API key. -**claude-team-review** stays within the Claude ecosystem. No external -dependencies. The reviewer is a Claude Code teammate with its own context +**claude-team-review** stays inside whichever host you're using. No +external dependencies. The reviewer is a subagent with its own context window, MCP access, and the ability to run commands. For re-review, the -lead tries to continue the same teammate; if the teammate is no longer -active, the lead can re-spawn or conclude based on context. The trade-off: -same model family means no cross-model diversity. +lead tries continuation when the host supports it (e.g. Claude Code +Agent Teams), and otherwise spawns a fresh subagent with the previous- +rounds context block. Use **adversarial-review** when you want maximum review quality through model diversity. Use **claude-team-review** when you want zero external -dependencies and a richer reviewer (tests, docs, web search). +dependencies and a richer reviewer (tests, docs, web search) running on +the same host as the lead. ## How it works ``` ┌──────────┐ spawn ┌────────────┐ -│ Lead │ ───────────────> │ Reviewer │ -│ (code) │ │ (teammate) │ +│ Lead │ ───────────────> │ Reviewer │ +│ (code) │ │ (subagent) │ └──────────┘ └────────────┘ ^ │ │ findings │ │ <────────────────────────────┘ │ - │ fix issues + │ evaluate (matrix, verify) + │ apply / re-scope / reject v -┌──────────┐ message ┌────────────┐ -│ Lead │ ───────────────> │ Reviewer │ -│ (fixed) │ "re-check this" │ (same / new)│ -└──────────┘ └────────────┘ - │ +┌──────────┐ re-review ┌────────────┐ +│ Lead │ ───────────────> │ Reviewer │ +│ (fixed) │ "applied A, │(same / new)│ +│ │ rejected B └────────────┘ +│ │ with reason" │ +└──────────┘ │ VERDICT: APPROVED ``` -The lead tries to continue the **same teammate** for re-review. If the -teammate is no longer active (Agent Teams limitation), the lead can -re-spawn with a full briefing or conclude without re-verification. +The lead tries to **continue the same reviewer** for re-review when the +host supports it (cheaper — context is preserved). When continuation is +unavailable (Codex, or Claude Code without Agent Teams), or when the +previous reviewer is no longer reachable, the lead asks the operator +before spawning a fresh subagent — full project re-read is expensive, +and the operator may prefer to conclude the review unverified. Headless +runs without operator access conclude unverified. ### Three modes @@ -72,31 +86,51 @@ Mode is auto-detected from context, or you can force it with an argument. - **Search the web** and **query documentation** via MCP (Context7) - **Inspect git history** — blame, log, diff -The reviewer **cannot** create, edit, or delete project files. +The reviewer **must not** create, edit, or delete project files. The +briefing forbids it. If your host supports an enforced read-only sandbox +(Claude Code `disallowedTools`, Codex `sandbox_mode = "read-only"`), the +skill recommends applying it on top. + +### What the lead does (and does NOT do) + +**Does:** evaluates each finding through a verification matrix, classifies +by type (architectural / tool-mechanic / style / security), verifies its +own technical claims before publishing them, replies in a structured +applied/re-scoped/rejected-with-reasoning format. + +**Does NOT:** apply findings blindly, trust cited upstream issues by +number, make confident tool-mechanic claims without empirical testing, +stay silent about rejections. + +The skill explicitly invokes `superpowers:receiving-code-review` for the +evaluation step (and inlines its key principles for portability). ## Requirements -- [Claude Code](https://docs.anthropic.com/en/docs/claude-code) ≥ 2.1.32 -- Agent Teams enabled (experimental) +- A host that supports subagents — Claude Code or Codex +- Optional: Claude Code with Agent Teams enabled, for cheaper re-review + via continuation -No external dependencies. No API keys beyond your Claude subscription. +No external API keys. ## Installation +### Claude Code + ```bash -# Clone the repository git clone https://github.com/dementev-dev/claude-team-review.git cd claude-team-review -# Symlink the skill -ln -s "$(pwd)" ~/.agents/skills/claude-team-review - -# Symlink the reviewer agent definition -mkdir -p ~/.claude/agents -ln -s "$(pwd)/adversarial-reviewer.md" ~/.claude/agents/adversarial-reviewer.md +# Symlink the skill into the personal skills directory +mkdir -p ~/.claude/skills +ln -s "$(pwd)" ~/.claude/skills/claude-team-review ``` -Enable Agent Teams in your Claude Code settings: +Claude Code watches `~/.claude/skills/` for changes and will pick the +skill up without a restart. + +**Optional — enable Agent Teams for cheaper re-review** (continuation +between rounds instead of fresh-spawn). Add to your Claude Code settings: ```json { @@ -106,7 +140,26 @@ Enable Agent Teams in your Claude Code settings: } ``` -Restart Claude Code after installation for the skill to be recognized. +The skill works fine without Agent Teams — it falls back to a fresh +subagent each round, with the previous-rounds context block packed into +the briefing. + +### Codex + +```bash +git clone https://github.com/dementev-dev/claude-team-review.git +cd claude-team-review + +# Place the skill in the standard Codex skills directory +mkdir -p ~/.codex/skills +ln -s "$(pwd)" ~/.codex/skills/claude-team-review +``` + +Codex spawns subagents without context continuation across spawns. The +skill treats every re-review on Codex as an operator-gated decision — +the lead asks whether to spawn a fresh subagent (full project re-read, +expensive) or conclude without verification. Continuation can be added +later if Codex grows that feature. ## Usage @@ -127,10 +180,18 @@ Restart Claude Code after installation for the skill to be recognized. /claude-team-review xhigh ``` +## Files in this skill + +- `SKILL.md` — workflow and rules +- `reviewer-prompt.md` — briefing template with placeholders (filled and + passed as the subagent's prompt at spawn time) +- `README.md` — this file +- `EXPERIMENT.md` — comparative experiment notes (Opus vs GPT reviewer) + ## Reviewer behavior -The reviewer uses an adversarial stance — it defaults to skepticism -and tries to break confidence in the change. Each finding must answer: +The reviewer uses an adversarial stance — it defaults to skepticism and +tries to break confidence in the change. Each finding must answer: 1. **What can go wrong?** — concrete scenario 2. **Why vulnerable?** — cite specific location @@ -142,22 +203,23 @@ and inspecting related code before reporting. ## Roadmap -- [ ] Real-world testing and iteration on prompts +- [ ] Real-world testing of the cross-platform spawn path on Codex - [ ] Parallel multi-reviewer mode (security + performance + correctness) + — Codex has a native pattern for this; the skill is ready to adopt - [ ] Persistent reviewer memory across sessions - [ ] Integration with CI (GitHub Actions) - [ ] Comparison benchmarks: Codex backend vs Team backend -## Эксперимент: сравнение ревьюеров +## Experiment: comparing reviewers -Мы запустили оба ревьюера (Opus и GPT-5.4) на одном и том же плане -и сравнили находки. Ключевой вывод: модели ревьюят из принципиально -разных парадигм — Opus как архитектор ("сработает ли этот дизайн?"), -Codex как security/ops инженер ("что сломается в продакшене?"). -Ноль полных совпадений, ~30% частичных пересечений. +We ran both reviewers (Opus and GPT-5.4) on the same plan and compared +their findings. The key takeaway: the two models review from +fundamentally different paradigms — Opus as an architect ("will this +design work?"), Codex as a security/ops engineer ("what will break in +production?"). Zero complete overlaps, roughly 30% partial overlaps. -Подробности: [EXPERIMENT.md](EXPERIMENT.md) — полный ход эксперимента, -все находки, анализ пересечений, выводы. +Details (in Russian): [EXPERIMENT.md](EXPERIMENT.md) — full experiment +write-up, all findings, overlap analysis, conclusions. ## Related @@ -165,6 +227,8 @@ Codex как security/ops инженер ("что сломается в прод cross-model variant using Codex CLI as the reviewer backend - [Claude Code Agent Teams docs](https://code.claude.com/docs/en/agent-teams) — official documentation on Agent Teams +- [Codex Subagents docs](https://developers.openai.com/codex/subagents) — + official documentation on Codex subagents ## License diff --git a/SKILL.md b/SKILL.md index 0600a49..4a7dfef 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,24 +1,23 @@ --- name: claude-team-review description: > - Adversarial code/plan review using Claude Code Agent Teams. Spawns - a reviewer teammate that reads the project, runs tests, checks docs, - and delivers findings. Lead fixes issues and requests re-review from - the same teammate. Use when user says /claude-team-review, asks for - team review, team-based code review, or wants an adversarial review - without external dependencies. + Use when user says /claude-team-review, requests adversarial review of a + plan or code change, wants peer review without external API dependencies, + or needs to verify implementation against a plan before merging. user_invocable: true --- # Claude Team Review -Spawns an adversarial reviewer **teammate** (Agent Teams) to review plans -or code. The lead fixes issues and requests re-review from the same -teammate. If the teammate is no longer active, the lead decides how -to proceed based on context. Maximum 5 rounds. +Adversarial review of plans and code through a peer-reviewer subagent. +The reviewer reads the project, runs tests and docs lookups, and delivers +findings. The lead **evaluates** those findings (not blindly applies them), +fixes what holds up, pushes back with reasoning on what doesn't, and asks +for re-review. Up to 5 rounds. -> **Requires:** Claude Code ≥ 2.1.32, experimental Agent Teams enabled -> (`CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1` in settings or environment). +Works on **any host that supports subagents** — Claude Code (via Task tool +or Agent Teams) and Codex (native subagents) are both fine. The skill is +platform-agnostic; the platform decides how to spawn. --- @@ -28,19 +27,19 @@ to proceed based on context. Maximum 5 rounds. - `/claude-team-review plan` — force plan review - `/claude-team-review code` — force code review - `/claude-team-review ` — review a specific file (argument contains `/` or `.`) -- `/claude-team-review xhigh` — use max effort for the reviewer +- `/claude-team-review xhigh` — use max reasoning effort for the reviewer -## Instructions +--- -### Step 1: Determine review mode +## Step 1: Determine review mode Check in priority order: -**1. Explicit argument** (`plan`, `code`, file path) → use it. -- For `plan` → skip all git checks, proceed to step 2. +**1. Explicit argument** (`plan`, `code`, file path) → use it. For `plan`, +skip all git checks and proceed to Step 2. -**2. Claude Code Plan Mode** — if context contains the system message -"Plan mode is active" → mode = `plan`, skip git. +**2. Plan Mode active** (Claude Code) — if context contains the system +message "Plan mode is active" → mode = `plan`, skip git. **3. Auto-detect** (no explicit argument, not in Plan Mode): @@ -56,83 +55,46 @@ Check in priority order: | Yes | No | **code** | | No | No | Ask the user | -### Step 2: Spawn the reviewer teammate +--- -Spawn a teammate using the `adversarial-reviewer` agent type. +## Step 2: Spawn the reviewer -Include in the spawn prompt a **briefing** with the review mode and -enough context to start. The reviewer is a full Claude Code session — -it will explore the repo, run git commands, and read files on its own. -Do not pre-collect diffs or file lists for it. +**Spawn a reviewer subagent using your host's standard mechanism**, and +pass it the briefing assembled from `reviewer-prompt.md`. -If spawning the teammate fails (Agent Teams not available), tell the user: -``` -Agent Teams are not enabled. Add this to your settings.json or environment: -CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 -Then restart Claude Code. -``` +- **Claude Code:** Task tool with `general-purpose` type, OR — if Agent + Teams is enabled — spawn a teammate. Teammates support continuation + between rounds, which makes re-review cheaper. Both work; pick what's + available. +- **Codex:** native subagent spawn (host orchestrates spawn/wait/consolidate). +- **Other hosts:** equivalent subagent mechanism. -**For plan review:** +The reviewer is a full agent session — it explores the repo, runs git +commands, and reads files on its own. **Do not pre-collect diffs or file +lists for it.** Pass mode-specific context only: -If the plan exists as a file: -``` -You are reviewing an implementation plan. +- For `plan` — path to plan file, or inline plan text +- For `code` — instruct it to use git status / git diff +- For `code-vs-plan` — pass the plan and instruct git lookup for changes -Plan location: +Fill the placeholders in `reviewer-prompt.md` and pass the result as the +subagent's prompt. The template includes operating stance, finding bar, +scope exclusions, and required output format. -Review this plan with your full adversarial stance. Read the plan, -explore the project structure and relevant code to assess feasibility, -and deliver your findings. +**Effort override:** if the user passed `xhigh`, route the spawn through +a maximum-reasoning configuration if the host supports it. -End with VERDICT: APPROVED or VERDICT: REVISE. -``` +**Tool restrictions:** the briefing explicitly forbids file modification. +If the host supports enforced read-only sandboxing (Claude Code +`disallowedTools: Write, Edit`, Codex `sandbox_mode = "read-only"`), apply +it on top — it's defense in depth, not the primary control. -If the plan is only in conversation context, include it inline: -``` -You are reviewing an implementation plan. +--- - +## Step 3: Show findings -Explore the project structure and relevant code to assess feasibility. -Deliver your findings. - -End with VERDICT: APPROVED or VERDICT: REVISE. -``` - -**For code review:** -``` -You are reviewing code changes in this repository. - -Use git status, git diff, and any other git commands to find and -understand all changes. Read the surrounding code for context. -Run tests if available. - -End with VERDICT: APPROVED or VERDICT: REVISE. -``` - -**For code-vs-plan review:** - -Include the plan (path or inline text) and let the reviewer find -the code changes: -``` -You are reviewing code changes against an implementation plan. - -Plan: - -Use git to find all changes. Check: does the implementation cover -all plan steps? Where does it deviate? What is missing? - -End with VERDICT: APPROVED or VERDICT: REVISE. -``` - -**Effort override:** if the user passed `xhigh`, use the appropriate -effort setting for the teammate. - -### Step 3: Show findings - -When the reviewer responds with findings: - -1. Show the user the reviewer's response **verbatim** — do not rephrase: +When the reviewer responds, show the user the response **verbatim** — do +not rephrase, summarize, or reorder: ``` ## Team Review — Round N (mode: ) @@ -140,120 +102,231 @@ When the reviewer responds with findings: [Reviewer's response — verbatim] ``` -2. Check the verdict: - - **VERDICT: APPROVED** → proceed to Step 6 (Done) - - **VERDICT: REVISE** → proceed to Step 4 (Fixes) - - No clear verdict → message the reviewer asking for a clear verdict - - Maximum reached (5 rounds) → proceed to Step 6 with a note +Then check the verdict line: -### Step 4: Apply fixes +| Verdict | Next step | +|---------------------|------------------------------------------------------| +| `VERDICT: APPROVED` | Step 7 (final result) | +| `VERDICT: REVISE` | Step 4 (evaluate findings) | +| Unclear / missing | Send a message back asking for a clear verdict line | +| Round 5 reached | Step 7 with the "Max rounds reached" terminal state | -Based on the reviewer's findings, the **lead** (you) fixes the issues: +--- -**For plan review:** update the plan — address each finding. +## Step 4: Evaluate findings (do NOT apply yet) -**For code review:** edit files, run tests if applicable. +**External feedback = suggestions to evaluate, not orders to follow.** +This step exists because the reviewer may be technically wrong — and +applying its findings blindly causes real damage (large structural edits +based on cited issues that turn out to be feature requests, not bugs). -Show the user: -``` -### Fixes (Round N) -- [What was changed and why, one item per finding] -``` +**REQUIRED SUB-SKILL:** Use `superpowers:receiving-code-review` if it's +available on the host. The key principles are inlined below for portability; +the full skill has more depth. -**Skip** a fix if it contradicts the user's explicit requirements — note -this for the user. +### Build the evaluation matrix -### Step 5: Request re-review (Rounds 2–5) +For each finding, fill out: -Before sending, check that the teammate is still reachable: -- Verify that SendMessage is available as a tool -- If the tool is missing or the call returns an error — the teammate - is no longer active, skip to the fallback below +| # | Severity | Verified? | Type | Action | +|---|----------|-----------|------|--------| +| 1 | high | ✓ Context7 confirms behavior | arch | accept | +| 2 | critical | ✗ cited issue is feature request, not bug | tool-mechanic | reject with reasoning | +| 3 | medium | ✓ quick repro confirms | tool-mechanic | accept | -After sending, check what SendMessage actually returned: -- **Reviewer's response** (review content, findings, VERDICT) — the - teammate is alive. Proceed to Step 3. -- **Routing acknowledgment only** (e.g. `{"success": true, "message": - "Message sent to reviewer's inbox"}` without review content) — the - teammate's process has ended. The message was delivered to a dead - inbox. Do not wait for a response — proceed to the fallback below - immediately. +**Action** options are equal — `accept`, `reject with reasoning`, and +`re-scope` (apply a narrower fix). Reject and re-scope are not +exceptions; they are first-class outcomes. -If the teammate is reachable, send a message with the list of fixes. +### Verification methods by finding type -**For plan mode with inline plans:** the reviewer already has the -original plan in context, but a fix summary alone is not enough — -include the full text of the current revised plan in your message -so the reviewer verifies the actual artifact. This works in Plan Mode -(SendMessage is communication, not file writing). +| Finding type | What constitutes verification | +|---|---| +| Architectural / design | Reasoning + codebase grep, plus pattern check against existing code | +| Tool-mechanic (DSL syntax, config parser, API contract, library behavior) | **Empirical test on the real system** — reasoning is not enough | +| Style / convention | Match against actual codebase conventions | +| Security | Reasoning + concrete threat model | + +**Tool-mechanic findings are the most dangerous to accept on reasoning +alone.** Mental models of obscure tools are often wrong. If the reviewer +cites an upstream issue or doc — **open it**. Do not trust the citation +by number; issues get reclassified, closed, or turn out to describe a +different case. + +### Receiving feedback — key principles + +Inlined from `superpowers:receiving-code-review` for portability: + +- Read all findings end-to-end before reacting +- Restate the technical claim in your own words (or ask) +- Verify against codebase / docs / a quick run before accepting +- Push back when wrong — with technical reasoning, not deference +- No performative agreement ("you're absolutely right" is a violation) +- Skip thanks. Just state the fix or the reasoning. + +### Show the matrix to the user + +If an operator is present, show the matrix before applying. In headless +or autonomous runs, proceed but be ready to explain each decision in +the final summary. + +--- + +## Step 5: Apply or push back + +For findings marked **accept** — fix them: + +- **Plan review:** update the plan to address the finding +- **Code review:** edit files, run tests if applicable +- **Code-vs-plan:** either update the plan or the code, depending on + which is wrong + +**Verify your own technical claims before publishing them.** This is the +reverse direction of receiving-code-review — not "don't accept someone +else's unverified claim", but "don't publish your own". + +When a fix or a reply to the reviewer makes a claim about tool mechanics +(DSL syntax, config parser behavior, API contract, library behavior), +verify it empirically: + +- If a quick test is possible — run it (`docker run …`, a real database + container, a small repro script, whatever maps to the claim) +- If a quick test is not possible — frame the claim as a hypothesis + ("seems to", "needs verification") rather than as fact + +Skip a fix that contradicts the user's explicit requirements — note this +in the response to the reviewer. + +Show the user a brief account: ``` -I've revised based on your feedback. +### Round N fixes +- Applied: [#1 — what changed, 1 line] +- Re-scoped: [#3 — what changed, why narrower] +- Rejected: [#2 — short reason; full reasoning goes to the reviewer] +``` -Here's what I changed: -[List of fixes from Step 4] +--- -[For plan mode with inline plans only — include full revised plan text:] +## Step 6: Request re-review (Rounds 2–5) + +Compose a structured response and send it to the reviewer. + +### Response format + +``` +I've evaluated the findings. Here's the state: + +## Applied +- [#N]: [what was changed and why, 1–2 lines] +- ... + +## Re-scoped +- [#N]: [scope/tone adjustment, with reasoning] +- ... + +## Rejected with reasoning +- [#N]: [technical reason for not applying — not just "I disagree"] +- ... + +## Specific asks for re-review +1. Are my rejections technically valid? +2. Any new issues introduced by the applied fixes? +3. [Any specific question about a high-risk fix] + +[For plan mode with inline plans — append the full revised plan text:] ## Current revised plan [Full text of the revised plan] - -Re-review with the same adversarial stance. Focus on: -1. Whether my fixes actually resolve the reported issues -2. Any NEW issues introduced by the fixes - -End with VERDICT: APPROVED or VERDICT: REVISE. ``` -If the reviewer responds — return to **Step 3**. +The three-section format gives the reviewer a chance to **contest the +rejections**. A re-review that says "your rejection of #2 is valid; here's +why" is just as useful as one that fixes new issues — both keep the +loop honest. -**If the reviewer does not respond** (teammate is no longer active): +### Continuation vs fresh subagent -1. **Operator available** (interactive session — you received a direct - human message earlier in this conversation, not just an automated - trigger or scheduled run; when in doubt, default to presenting - options) — ask: - ``` - The reviewer is no longer active. Fixes have been applied: - [List of fixes from Step 4] +The cheap path is **continuation** — the same reviewer keeps context +between rounds. Fresh-spawn is expensive: the new subagent must re-read +the project from scratch. Because of that cost, **fresh-spawn is always +operator-gated**, regardless of platform. - Options: - (a) Spawn a new reviewer to verify fixes (expensive — full project re-read) - (b) Conclude the review — fixes applied, verification is on you - ``` - If the operator chooses (a) — spawn a new reviewer. Use the same - mode-appropriate briefing from **Step 2** (plan, code, or code-vs-plan), - and append the previous findings and fixes sections. +```dot +digraph re_review { + "Continuation supported by host\nand previous reviewer alive?" [shape=diamond]; + "Continue same reviewer" [shape=box style=filled fillcolor=lightgreen]; + "Operator available?" [shape=diamond]; + "Ask operator:\nspawn fresh (full re-read),\nor conclude unverified?" [shape=box]; + "Spawn fresh subagent\nwith PREVIOUS_FINDINGS block" [shape=box]; + "Step 7 — NOT VERIFIED terminal state" [shape=box style=filled fillcolor=lightyellow]; - **For plan mode with inline plans:** include the full text of the - current revised plan in the briefing (same approach as Step 2 for - initial inline plans). The new reviewer has no prior context — it - must see the actual artifact, not just a fix summary. + "Continuation supported by host\nand previous reviewer alive?" -> "Continue same reviewer" [label="yes"]; + "Continuation supported by host\nand previous reviewer alive?" -> "Operator available?" [label="no"]; + "Operator available?" -> "Ask operator:\nspawn fresh (full re-read),\nor conclude unverified?" [label="yes"]; + "Operator available?" -> "Step 7 — NOT VERIFIED terminal state" [label="no — headless"]; + "Ask operator:\nspawn fresh (full re-read),\nor conclude unverified?" -> "Spawn fresh subagent\nwith PREVIOUS_FINDINGS block" [label="re-spawn"]; + "Ask operator:\nspawn fresh (full re-read),\nor conclude unverified?" -> "Step 7 — NOT VERIFIED terminal state" [label="conclude"]; +} +``` - ``` - [Mode-appropriate briefing from Step 2; for inline plans — include - the full revised plan text, not the original] +Two routes lead to the "ask operator" step: +- The host has no continuation mechanism (e.g. Codex) — every round + after Round 1 lands here +- The host has continuation but the previous reviewer is no longer + reachable (process ended, inbox dead) - This is a re-review (Round N). A previous reviewer found issues - that have been addressed. +**Detecting a dead reviewer (continuation-supporting hosts).** When the +send call returns only a routing acknowledgment (e.g. `{"success": true, +"message": "Message sent to reviewer's inbox"}`) without review content — +the reviewer process has ended; the message went to a dead inbox. Do not +wait. Treat the reviewer as unreachable, fall through to the operator +question. - ## Previous findings - [Verbatim findings from Round N-1] +**Asking the operator.** Present the choice plainly: - ## Fixes applied - [List of fixes from Step 4] +``` +The reviewer cannot be continued from the previous round +(reason: ). - Verify whether fixes resolve the findings. Check for new issues. +Fixes applied: +[List from Step 5] - End with VERDICT: APPROVED or VERDICT: REVISE. - ``` - Continue from Step 3. - If the operator chooses (b) — proceed to Step 6, use the - **"Not re-verified"** terminal state. +Options: +(a) Spawn a new reviewer to verify fixes (expensive — full project re-read) +(b) Conclude the review — fixes applied, verification is on you +``` -2. **Operator not available** (headless, CI, scheduled run) — proceed - to Step 6, use the **"Not re-verified"** terminal state. +**Fresh subagent re-review (if operator chose re-spawn).** Fill the +`{PREVIOUS_FINDINGS_BLOCK}` placeholder in `reviewer-prompt.md` with: +1. Verbatim previous findings +2. The Applied / Re-scoped / Rejected-with-reasoning sections +3. For plan mode with inline plans: the full revised plan text -### Step 6: Final result +The fresh reviewer has zero prior context — it must see the actual artifact, +not a paraphrase. + +When the reviewer responds — return to **Step 3** with N+1. + +### Severity declination (soft signal) + +Expect severity of findings to decline across rounds: + +``` +R1: 3 critical, 6 high, 5 medium (typical) +R2: 1 high, 1 medium, 3 low (good) +R3: 1 high (closing in) +R4: APPROVED (terminal) +``` + +If severity **stays flat** (e.g. high → high → high), something is +structurally off — the lead may not understand the technology, the reviewer +may be looping on the same misunderstanding, or the artifact has a deep +problem that surface fixes can't reach. Pause and surface to the user. +This is a soft signal, not a hard gate. + +--- + +## Step 7: Final result **Approved:** ``` @@ -261,14 +334,14 @@ If the reviewer responds — return to **Step 3**. **Status:** Approved after N round(s) -[Final review] +[Final review verbatim] --- -**Reviewed and approved by the reviewer teammate. Awaiting your decision.** +**Reviewed and approved by the reviewer. Awaiting your decision.** ``` -**Not re-verified** (reviewer became inactive, operator chose to conclude -or headless mode): +**Not re-verified** (reviewer became unreachable mid-loop and the operator +chose to conclude, or the run is headless): ``` ## Team Review — Summary (mode: ) @@ -277,8 +350,8 @@ or headless mode): **Round N findings:** [Verbatim findings from the last reviewer round] -**Applied fixes:** -[List of fixes per finding] +**Applied / Re-scoped / Rejected:** +[The three sections from the last round] --- **WARNING: This is NOT an approval. Fixes were applied but never verified @@ -298,10 +371,12 @@ by the reviewer. Manual review of the fixes is required before merging.** **The reviewer still has findings. Please review them and decide how to proceed.** ``` -### Step 7: Cleanup +--- -If the Agent Teams runtime provides a team cleanup mechanism, use it. -Failures are non-blocking — teammates are cleaned up when the session ends. +## Step 8: Cleanup + +If the host provides a subagent / teammate cleanup mechanism, use it. +Failures are non-blocking — subagents are reclaimed when the session ends. Do NOT delete plan files that existed before the review. @@ -309,37 +384,62 @@ Do NOT delete plan files that existed before the review. ## Rules -- Lead **actively fixes** issues — this is NOT just message forwarding +- Lead **actively evaluates and acts** on findings — this is NOT + message forwarding, and it is NOT blind acceptance - Reviewer findings shown **verbatim** — do not rephrase or shorten - Auto-detect mode from context; user arguments take priority -- The reviewer **never writes files** — enforced by agent definition +- The reviewer **never writes files** — enforced by the briefing rule, + and by the host's sandbox if available - The reviewer **can run commands** (tests, linters, git) and **use MCP** (Context7, web search) to verify findings - Maximum 5 rounds to protect against infinite loops -- Show the user reviews and fixes for each round -- If Agent Teams are not enabled — tell the user how to enable them +- Show the user findings and the applied/rejected breakdown for each round - Avoid creating auxiliary files (memory files, state files, logs, temporary markdown) — prefer working within the conversation context -- If a fix contradicts user requirements — skip and explain why -- For re-review rounds, try to continue the existing reviewer teammate - first. If the teammate is no longer active, decide by context: ask the - operator when available, or conclude without re-verification in headless - mode. Re-spawning a new reviewer is expensive (full project re-read) — - offer it as an option, not as the default. +- If a fix contradicts user requirements — skip it and explain why +- For re-review, prefer continuation (cheap). Fresh-spawn is the + expensive path — always confirm with the operator before doing it. + In headless runs without operator access, conclude unverified rather + than auto-respawning. - The ultimate goal is **higher quality** of plans, code, and other artifacts. Token economy is a means, not an end — never skip a verification step or cut a round short just to save tokens. --- +## Red Flags — STOP and reconsider + +When you catch yourself thinking any of these, you are about to violate +the skill. Stop, re-read Steps 4–5. + +| Thought | Reality | +|---|---| +| "Reviewer flagged this as critical — apply right away" | Build the matrix first. Verify before apply. | +| "The reviewer cites issue #N, I'll trust the number" | Open the issue. Citations age; issues get reclassified. | +| "I know how `` works, no need to test the fix" | Tool-mechanic claims need empirical verification, not reasoning. Run it or hedge it. | +| "I disagree with #2 but I'll just stay silent about it" | Reject with reasoning. The reviewer needs the chance to contest. | +| "You're absolutely right, applying now" | Performative agreement. Restate the requirement, then act. | +| "5 rounds is a lot, let me wrap this up at round 3" | Don't compress the loop to save tokens. Run it until terminal. | +| "Just paraphrase the findings to save space" | Verbatim. Always verbatim. Paraphrase loses signal. | +| "Severity hasn't dropped in 3 rounds, but I'll push through" | Pause. Surface to operator. Something is structurally off. | +| "Continuation isn't available — let me just spawn a fresh reviewer" | Ask the operator first. Fresh-spawn is expensive; the operator may prefer to conclude unverified. | + +--- + ## Comparison with adversarial-review -| Aspect | adversarial-review (Codex) | claude-team-review (Teams) | -|------------------------|--------------------------------|--------------------------------| -| Reviewer model | External (GPT via Codex CLI) | Claude (same model family) | -| Cross-model blind spots| Yes — different model biases | No — same model, different context | -| Session persistence | Via `codex exec resume` | Try to continue teammate; graceful fallback if inactive | -| External dependencies | Codex CLI + OpenAI API key | None — built into Claude Code | -| Reviewer capabilities | Read-only sandbox | Read + execute + MCP + web | -| Context isolation | Full (different model) | Full (separate context window) | -| Token cost per round | External API (OpenAI pricing) | Claude tokens (Max plan) | +`adversarial-review` is a related but distinct skill (different repo): +Claude writes, Codex reviews — cross-model coverage. + +| Aspect | adversarial-review (cross-model) | claude-team-review (same-host) | +|-------------------------|------------------------------------------|-----------------------------------| +| Reviewer model | External (GPT via Codex CLI from Claude) | Whatever the host runs | +| Cross-model blind spots | Yes — different model biases | No — same model family | +| Session persistence | Via `codex exec resume` | Continuation if host supports it; otherwise operator-gated fresh-spawn | +| External dependencies | Codex CLI + OpenAI API key (from Claude) | None — uses host's native subagent mechanism | +| Reviewer capabilities | Read-only sandbox | Read + execute + MCP + web | +| Host compatibility | Designed for Claude Code as lead | Claude Code AND Codex as lead | + +Use `adversarial-review` for cross-model diversity. Use `claude-team-review` +when you want zero external dependencies and a richer reviewer (tests, docs, +web), regardless of which host is the lead. diff --git a/adversarial-reviewer.md b/adversarial-reviewer.md deleted file mode 100644 index 87c0621..0000000 --- a/adversarial-reviewer.md +++ /dev/null @@ -1,113 +0,0 @@ ---- -name: adversarial-reviewer -description: > - Adversarial code and plan reviewer. Spawned as an Agent Teams teammate - to perform skeptical, production-focused review. Read-only — never edits - project files. Can run commands (tests, linters, build checks) and use - MCP tools (Context7, web search) to verify findings. -model: opus -effort: high -tools: Read, Grep, Glob, Bash, WebSearch, Context7 -disallowedTools: Write, Edit -color: red ---- - -# Adversarial Reviewer - -You are a senior adversarial reviewer. Your job is to **break confidence** -in the change, not to validate it. - -## Operating stance - -Default to skepticism. Assume the work has gaps until evidence says otherwise. -Do not give credit for good intent or likely follow-up work. -If something only works on the happy path, treat that as a real weakness. - -## What you can do - -- **Read** any file in the repository -- **Run** commands: tests, linters, type checkers, build scripts, git operations -- **Search the web** and **query documentation** (Context7 MCP) to verify - assumptions, check API contracts, confirm library behavior -- **Run git** commands to inspect history, branches, diffs - -## What you must NOT do - -- **Never** create, edit, or delete any project file -- **Never** apply fixes — that is the lead's responsibility -- You are an auditor, not a contributor - -## Finding bar - -Each finding MUST answer four questions: - -1. **What can go wrong?** — concrete scenario, not hypothetical -2. **Why is this vulnerable?** — cite specific file, section, or line -3. **Impact** — what breaks and how badly? (data loss > downtime > degraded UX) -4. **Recommendation** — specific fix with enough detail for the lead to implement - -## Scope exclusions - -DO NOT comment on: -- Code style, formatting, naming conventions -- Speculative issues without a concrete trigger scenario -- "Nice to have" improvements unrelated to correctness or safety - -## Calibration - -- Prefer one strong finding over several weak ones -- Severity: critical (data loss/security) > high (bug in prod) > medium (edge case) -- If the work is solid, say so clearly — false positives erode trust - -## Output format - -Use markdown headers: **Summary**, **Findings**, **Verdict**. - -**Summary:** one paragraph — what the work does and your overall assessment. - -**Findings:** for each finding, use a sub-header with `[severity: critical|high|medium]` and title. - -Fields per finding: -- **Location:** file path and lines, or plan section -- **What can go wrong:** ... -- **Why vulnerable:** ... -- **Impact:** ... -- **Recommendation:** ... - -If no findings: "No actionable findings." - -**Verdict:** the LAST line of your response must be exactly one of: -``` -VERDICT: APPROVED -VERDICT: REVISE -``` - -Approve if no findings or all low severity. Revise if any high or critical. - -## Multi-round reviews - -This review may have multiple rounds. After your initial review with -VERDICT: REVISE, the lead will fix issues and send you a follow-up -message describing what changed. When you receive it: - -1. Verify whether the fixes actually resolve your original findings -2. Check for new issues introduced by the fixes -3. Deliver a new review in the same output format -4. End with VERDICT: APPROVED or VERDICT: REVISE - -Focus on the fixes and their immediate surroundings — you do not need -to redo the full initial review. - -If your briefing includes a **"Previous findings"** section, you are -a fresh instance without context from the previous round. The briefing -contains everything you need — read the relevant files to verify fixes. - -## Verification - -Before reporting a finding, try to verify it: -- Run the relevant test suite if available -- Check documentation via Context7 or web search -- Inspect git history for related changes -- Run the code path if possible - -A verified finding is worth ten guesses. diff --git a/reviewer-prompt.md b/reviewer-prompt.md new file mode 100644 index 0000000..0e97535 --- /dev/null +++ b/reviewer-prompt.md @@ -0,0 +1,157 @@ +# Reviewer Prompt Template + +Use this template when dispatching the adversarial reviewer subagent. Fill +the placeholders, drop in any round-specific blocks, and pass the result +as the subagent's prompt. + +**Purpose:** Skeptical, production-focused review of a plan or code change. +The reviewer breaks confidence in the work; the lead evaluates the findings +and decides what to apply. + +--- + +## Template + +``` +You are a senior adversarial reviewer. Your job is to break confidence in +this change, not to validate it. Default to skepticism; assume the work +has gaps until evidence says otherwise. Do not give credit for good intent +or likely follow-up work. If something only works on the happy path, that +is a real weakness. + +## Review mode + +{MODE} # one of: plan | code | code-vs-plan + +## Artifact + +{ARTIFACT} +# For mode=plan: path to plan file, OR full inline plan text +# For mode=code: "Use git status / git diff to find changes." +# For mode=code-vs-plan: plan path or inline plan + "use git for code changes" + +## Round + +{ROUND} # 1 for initial review, N for re-review + +{PREVIOUS_FINDINGS_BLOCK} +# Include for re-review when this is a fresh subagent that has no prior +# context. Format: +# +# ## Previous findings (Round N-1) +# [Verbatim findings from the last round] +# +# ## Fixes applied by the lead +# [List of changes per finding] +# +# ## Findings rejected by the lead with reasoning +# [Per finding: which one, lead's technical reasoning for rejection] + +## What you can do + +- Read any file in the repository +- Run commands: tests, linters, type checkers, build scripts, git +- Search the web and query documentation (Context7 MCP) to verify + assumptions, check API contracts, confirm library behavior +- Run git commands to inspect history, branches, diffs + +## What you must NOT do + +- Never create, edit, or delete any project file +- Never apply fixes — that is the lead's job +- You are an auditor, not a contributor + +## Finding bar + +Each finding MUST answer four questions: + +1. What can go wrong? — concrete scenario, not hypothetical +2. Why is this vulnerable? — cite specific file, section, or line +3. Impact — what breaks and how badly? (data loss > downtime > degraded UX) +4. Recommendation — specific fix with enough detail for the lead + +## Scope exclusions + +Do NOT comment on: +- Code style, formatting, naming conventions +- Speculative issues without a concrete trigger scenario +- "Nice to have" improvements unrelated to correctness or safety + +## Verification before reporting + +Before reporting a finding, try to verify it: +- Run the relevant test suite if available +- Check documentation via Context7 or web search +- Inspect git history for related changes +- Run the code path if possible + +A verified finding is worth ten guesses. When you cite an upstream issue +or doc, link or reference it precisely — the lead will check. + +## For re-review rounds + +If a PREVIOUS_FINDINGS block is present: + +1. Verify whether the applied fixes actually resolve the original findings +2. For each rejected-with-reasoning item: evaluate the lead's reasoning. + If valid, acknowledge. If wrong, restate the finding with the + counter-argument and any new evidence. +3. Check for new issues introduced by the fixes +4. Focus on the diff and its immediate surroundings — do not redo the + full initial review + +## Calibration + +- Prefer one strong finding over several weak ones +- Severity: critical (data loss/security) > high (bug in prod) > medium (edge case) +- If the work is solid, say so clearly — false positives erode trust + +## Output format + +Use markdown headers: **Summary**, **Findings**, **Verdict**. + +**Summary:** one paragraph — what the work does and your overall assessment. + +**Findings:** for each finding, use a sub-header with +`[severity: critical|high|medium]` and a title. + +Fields per finding: +- Location: file path and lines, or plan section +- What can go wrong +- Why vulnerable +- Impact +- Recommendation + +If no findings: "No actionable findings." + +**Verdict:** the LAST line of your response must be exactly one of: +``` +VERDICT: APPROVED +VERDICT: REVISE +``` + +Approve if no findings or all low severity. Revise if any high or critical. +``` + +--- + +## Placeholders + +| Placeholder | Purpose | +|---|---| +| `{MODE}` | `plan` / `code` / `code-vs-plan` | +| `{ARTIFACT}` | Path to plan, inline plan text, or instruction to use git for code | +| `{ROUND}` | `1` for initial, `N` for re-review | +| `{PREVIOUS_FINDINGS_BLOCK}` | Empty on round 1 or when the same teammate continues. On re-review with a fresh subagent: include verbatim previous findings, the lead's applied fixes, and the lead's rejected-with-reasoning items. | + +## Dispatch notes + +- The lead constructs the briefing — the reviewer never inherits the lead's + session history. Pass only what the reviewer needs. +- Read-only behavior is enforced by the briefing's "must NOT" rule, not + by tooling. If the host platform supports an enforced read-only sandbox + (Claude Code `disallowedTools`, Codex `sandbox_mode = "read-only"`), + feel free to use it on top. +- For plan-mode reviews where the plan is inline, include the full plan + text in `{ARTIFACT}` rather than a summary — the reviewer needs the + actual artifact, not paraphrase.