feat(skill): язык ответа ревьюера, батч-пауза, фиксы из ревью

- Зачем: - Ревьюер отвечал на английском вне зависимости от языка сессии — оператор тратил усилия на чтение и перевод. - Скилл паузил оператора непредсказуемо (то спрашивал, то применял молча) — нужен явный критерий, что требует мнения оператора, а что не требует. - Накопились фактические неточности из двух раундов self-review (детекция мёртвого ревьюера, disallowedTools, /agent, грамматика инвокации). - Что: - Step 2: новый блок Language — лид определяет язык сессии и добавляет директиву в брифинг; парсимые литералы (VERDICT, severity tags, заголовки секций) остаются английскими. - Step 4: новый блок «structural vs non-structural» с батч-паузой — одна пауза на раунд только при наличии structural-фиксов; non-structural применяются молча. - reviewer-prompt.md: добавлен плейсхолдер {LANGUAGE_BLOCK} и описание в таблице placeholders. - Фиксы из ревью: детекция мёртвого ревьюера переписана под async-модель Agent Teams; disallowedTools — путь через кастомный subagent; /agent описан как CLI оператора, а не tool лида; bare <file-path> резолвится в mode=plan; добавлены формы code-vs-plan и code-vs-plan <plan-path>; xhigh — orthogonal effort flag; «plan in context» получило конкретные критерии. - Проверка: - git diff main..HEAD -- SKILL.md reviewer-prompt.md — посмотреть итоговый набор правок. - Запустить /claude-team-review на самой ветке и убедиться, что директива Language вставляется и батч-пауза срабатывает только на structural. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-15 15:53:05 +03:00
parent bb8c9cf0e1
commit a9f8d032f3
2 changed files with 157 additions and 23 deletions
@@ -23,20 +23,33 @@ platform-agnostic; the platform decides how to spawn.

 ## When to invoke

+Invocation grammar: `/claude-team-review [<mode-or-path>] [xhigh]` —
+both pieces are independently optional. `xhigh` is an effort flag, not
+a mode, and can be combined with any of the mode arguments below.
+
 - `/claude-team-review` — auto-detect what to review
 - `/claude-team-review plan` — force plan review
 - `/claude-team-review code` — force code review
- `/claude-team-review <file-path>` — review a specific file (argument contains `/` or `.`)
- `/claude-team-review xhigh` — use max reasoning effort for the reviewer
+- `/claude-team-review code-vs-plan` — force code-vs-plan review (uses the plan most recently shown in this conversation)
+- `/claude-team-review code-vs-plan <plan-path>` — force code-vs-plan review against an explicit plan file
+- `/claude-team-review <file-path>` — review a specific file as a **plan** (argument contains `/` or `.`; the file is treated as the plan artifact). For plan-vs-code comparison use `code-vs-plan <plan-path>` explicitly.
+- `xhigh` (anywhere in args) — use max reasoning effort for the reviewer

 ---

 ## Step 1: Determine review mode

-Check in priority order:
+First, strip an `xhigh` token from the args if present (it does not
+consume the mode slot — see Step 2 "Effort override"). Then resolve mode
+in priority order:

-**1. Explicit argument** (`plan`, `code`, file path) → use it. For `plan`,
-skip all git checks and proceed to Step 2.
+**1. Explicit argument** (`plan`, `code`, `code-vs-plan`,
+`code-vs-plan <plan-path>`, or a file path) → use it. For `plan` and
+`code-vs-plan` without an explicit path, skip git for the plan side and
+proceed to Step 2. A bare file path (no preceding `code-vs-plan` token)
+resolves to **plan** mode with that file as the plan artifact; if the
+user wants plan-vs-code, they must invoke `code-vs-plan <plan-path>`
+explicitly.

 **2. Plan Mode active** (Claude Code) — if context contains the system
 message "Plan mode is active" → mode = `plan`, skip git.
@@ -46,7 +59,18 @@ message "Plan mode is active" → mode = `plan`, skip git.
 1. Check for code changes (any non-empty output means changes exist):
   - `git diff --name-only` — unstaged
   - `git diff --cached --name-only` — staged
-2. Check if a plan exists in the current conversation context.
+2. Check if a **structured plan artifact** exists in the current
+   conversation context. This means one of:
+   - The user pasted or referenced a markdown plan document with
+     section headers like `Goals`, `Steps`, `Verification`, `Tasks`,
+     or similar
+   - The user explicitly labeled content as a "plan", "design", "spec",
+     or "RFC"
+   - A previous turn produced a written plan (e.g. via
+     `superpowers:writing-plans` or a saved plan file)
+
+   A casual sentence of intent ("I'll refactor auth next") is **not** a
+   plan in context.

 | Code changes? | Plan in context? | Mode               |
 |---------------|------------------|---------------------|
@@ -55,6 +79,11 @@ message "Plan mode is active" → mode = `plan`, skip git.
 | Yes           | No               | **code**            |
 | No            | No               | Ask the user        |

+If the "Plan in context?" call is genuinely ambiguous (something looks
+plan-ish but doesn't match the criteria above), ask the user rather
+than guessing — silently misclassifying into `code-vs-plan` produces
+contorted reviews.
+
 ---

 ## Step 2: Spawn the reviewer
@@ -69,9 +98,11 @@ pass it the briefing assembled from `reviewer-prompt.md`.
  Task tool spawn is one-shot — Round 2+ has to go through the operator-
  gated fresh-spawn path in Step 6.
 - **Codex:** native subagent spawn (host orchestrates spawn/wait/consolidate).
-  Codex supports intra-session continuation — the lead can send follow-up
-  instructions to a running subagent thread, and `/agent` switches between
-  active threads.
+  Codex supports intra-session continuation — the lead can address a
+  running subagent thread by name in its next instruction and Codex's
+  orchestration routes the message there. (`/agent` is a human-facing
+  CLI command for inspecting/switching threads from the terminal, not a
+  tool the lead invokes.)
 - **Other hosts:** equivalent subagent mechanism. The skill works as long
  as the host can spawn a subagent and (optionally) route follow-up
  messages to it.
@@ -91,10 +122,42 @@ scope exclusions, and required output format.
 **Effort override:** if the user passed `xhigh`, route the spawn through
 a maximum-reasoning configuration if the host supports it.

-**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.
+**Language.** The skill template is intentionally English (stable, host-
+agnostic), but the reviewer's *output* should match the operator's
+session language so verbatim show-back to the operator works without
+translation. Detect the language from recent operator messages (the last
+few human turns in the conversation). If it's English — no extra block
+needed (default). Otherwise, append a `## Language` section to the
+briefing with this content:
+
+```
+## Language
+
+Respond in <detected language>. The following stay in English (they are
+literals parsed by the lead and must not be translated): section headers
+(Summary, Findings, Verdict), severity tags
+(`[severity: critical|high|medium]`), and the final verdict line
+(`VERDICT: APPROVED` or `VERDICT: REVISE`). All other content —
+prose, reasoning, descriptions, recommendations — in
+<detected language>.
+```
+
+The verbatim reply from the reviewer is shown to the operator as-is — do
+not translate it back; it should already be in the operator's language.
+
+**Tool restrictions:** the briefing's "must NOT modify files" rule is
+the primary control on the read path. The built-in `general-purpose`
+subagent on Claude Code has no tool-restriction knob the lead can pass
+at invocation time, so the briefing rule is the only control there. If
+you want enforced sandboxing on top, that requires going off the
+default path:
+
+- **Claude Code:** define a custom subagent (e.g.
+  `~/.claude/agents/adversarial-reviewer.md`) with frontmatter
+  `disallowedTools: Write, Edit`, and dispatch that agent type instead
+  of `general-purpose`.
+- **Codex:** the host's `sandbox_mode = "read-only"` applies to the
+  spawned subagent natively.

 ---

@@ -178,6 +241,51 @@ 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.

+### Decide which fixes need operator sign-off
+
+After the matrix is built and every finding has an action (`accept` /
+`reject with reasoning` / `re-scope`), classify each `accept` / `re-scope`
+fix as **structural** or **non-structural** — this controls whether the
+operator is paused.
+
+**Structural** — pause operator before applying:
+
+- Changes the invocation grammar (new/removed args, renamed mode, changed
+  argument semantics)
+- Changes an external contract: output format, parsed literals
+  (e.g. the `VERDICT: APPROVED|REVISE` line), section header names,
+  named workflow states, public-facing option semantics
+- Adds or removes a workflow step or a terminal state
+- Changes the meaning of a publicly documented option (e.g. what `xhigh`
+  does, what `code-vs-plan` means)
+- Any fix the lead is uncertain how to scope — when in doubt, classify
+  as structural
+
+**Non-structural** — apply without pausing:
+
+- Wording / phrasing changes that don't change semantics
+- Correcting factual inaccuracies in descriptions (e.g. wrong API name,
+  wrong tool mechanic, wrong attribution)
+- Removing outdated comments or examples
+- Adding clarifying sentences or examples that don't change behavior
+- Internal heuristic refinements that don't change observable behavior
+
+**Batch-pause rule.** Do not pause once per fix. Instead:
+
+1. Walk the entire matrix first, classify every accepted/re-scoped fix.
+2. If structural count is **zero** → apply everything, no pause.
+3. If structural count is **≥ 1** → make exactly **one** pause showing
+   the operator:
+   - The list of structural fixes (one-line per fix; ask for go/no-go on
+     each, or as a batch)
+   - The list of non-structural fixes (informational only — auto-applied)
+   - The list of rejects with reasoning (informational only)
+
+**Headless / no operator.** If no operator is reachable, apply structural
+fixes anyway but flag them explicitly in the final summary as
+"structural changes applied without operator sign-off" — the operator
+reviews them after the fact rather than gating them up-front.
+
 ---

 ## Step 5: Apply or push back
@@ -284,7 +392,7 @@ relying on this):**
 |---|---|---|
 | Claude Code + Agent Teams (`CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1`) | yes | `SendMessage` to the teammate inbox |
 | Claude Code without Agent Teams | **no** — Task spawn is one-shot | — |
-| Codex | yes — intra-session | follow-up message to a running thread; `/agent` to switch |
+| Codex | yes — intra-session | address the running subagent thread by name in the next instruction; Codex orchestration routes it (operators can use `/agent` from the CLI to inspect/switch threads — not a lead-side tool) |

 Two routes lead to the "ask operator" step:
 - The host has no continuation mechanism for our use case (Claude Code
@@ -292,12 +400,29 @@ Two routes lead to the "ask operator" step:
 - The host has continuation but the previous reviewer is no longer
  reachable (process ended, thread closed, inbox dead)

-**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.
+**Detecting a dead reviewer (continuation-supporting hosts).** The send
+itself is fire-and-forget — `SendMessage` returns immediately with a
+routing acknowledgment regardless of teammate liveness, and the reply
+arrives asynchronously in a later turn via the host's automatic message
+delivery. **Do not treat the ack as the reply.** After sending, wait for
+the reply to be delivered. Treat the reviewer as unreachable only when
+one of the following is true:
+
+- The host surfaces an explicit shutdown / "teammate exited" / "no such
+  teammate" event. On Claude Code + Agent Teams this includes the
+  documented case where a resumed session no longer has the previous
+  teammates alive, and idle notifications (`TeammateIdle` /
+  message-delivery events) — when present, these are the authoritative
+  liveness signal.
+- Send fails outright (error response, not a success ack).
+- The reply is overdue (no `TeammateIdle` / message-delivery notification
+  in a reasonable window) **and** an operator is present to confirm —
+  surface the wait and let the operator decide whether to abandon. If
+  no operator is present, keep waiting; auto-abandoning on a wall-clock
+  guess is worse than waiting (LLMs don't track wall time reliably and
+  the host's notification channel is the documented signal).
+
+When unreachable is confirmed, fall through to the operator question.

 **Detecting an operator.** Treat the run as **interactive** (operator
 available) only if both conditions hold: