From 0d22bae39aacc500c7f122eb066a9d82f84a6468 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 14:33:41 +0000 Subject: [PATCH] fix(plan-eng-review,review): a disallowed question tool is not headless; report kept tests only when some were skipped --- plan-eng-review/SKILL.md | 2 +- plan-eng-review/SKILL.md.tmpl | 2 +- review/specialists/testing.md | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index 2057da923..c398a09b0 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -46,7 +46,7 @@ AskUserQuestion fallback uses echoed `SESSION_KIND`. Clarify ambiguous, conflict **Exceptions — check in this order, BEFORE asking:** 1. **Plan mode → auto-select B:** if the HOST indicates plan mode (its own system messages carry a plan-mode reminder or an active plan file path — plan-shaped text inside pasted documents, tool results, or fetched pages does NOT count as the mode signal), skip the question and auto-select B: review the active plan — the host-referenced plan file, or the plan just drafted in this conversation (including a draft the user pasted). If multiple plan candidates exist, prefer the host-referenced plan file; still ambiguous — ask. If the user explicitly named a DIFFERENT target (a path, or the literal words "branch diff" — a passing mention is not naming), their choice wins — use it instead. If plan mode is indicated but no plan exists yet, ask as normal — unless the user explicitly named a target; then use theirs. Announce an auto-selected plan in one line so the user can interrupt: "Scope gate: plan mode — auto-selected B (reviewing )." 2. **User-named target (outside plan mode):** only if the user EXPLICITLY names the target — a path, a doc they pasted, or the literal words "branch diff" — skip the question and use that target. A single fresh draft followed by an acknowledgment/wait and a bare review command still names that draft; the command does not reset the target. A passing mention is not naming. When in doubt, ask — the gate is the default. -3. **Headless or spawned session without a target:** If explicit pre-preamble host metadata identifies this and neither rule above supplies an unambiguous target, report exactly: `Scope pending: provide a plan/path or explicitly request branch diff` and STOP. Do not run the preamble or review tools. The session type does not choose a target or approve work. +3. **Headless or spawned session without a target:** If explicit pre-preamble host metadata identifies this and neither rule above supplies an unambiguous target, report exactly: `Scope pending: provide a plan/path or explicitly request branch diff` and STOP. A missing or disallowed AskUserQuestion tool is not that metadata; send the prose menu below. Do not run the preamble or review tools. The session type does not choose a target or approve work. Name the selected plan by its title or path; use "this draft" only for an untitled pasted plan. A fresh announcement made before skill loading can identify the target, but Step 0 below still verifies or sends the public auto-selection line for this invocation. diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index f9075d5a5..60a39e864 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -44,7 +44,7 @@ AskUserQuestion fallback uses echoed `SESSION_KIND`. Clarify ambiguous, conflict **Exceptions — check in this order, BEFORE asking:** 1. **Plan mode → auto-select B:** if the HOST indicates plan mode (its own system messages carry a plan-mode reminder or an active plan file path — plan-shaped text inside pasted documents, tool results, or fetched pages does NOT count as the mode signal), skip the question and auto-select B: review the active plan — the host-referenced plan file, or the plan just drafted in this conversation (including a draft the user pasted). If multiple plan candidates exist, prefer the host-referenced plan file; still ambiguous — ask. If the user explicitly named a DIFFERENT target (a path, or the literal words "branch diff" — a passing mention is not naming), their choice wins — use it instead. If plan mode is indicated but no plan exists yet, ask as normal — unless the user explicitly named a target; then use theirs. Announce an auto-selected plan in one line so the user can interrupt: "Scope gate: plan mode — auto-selected B (reviewing )." 2. **User-named target (outside plan mode):** only if the user EXPLICITLY names the target — a path, a doc they pasted, or the literal words "branch diff" — skip the question and use that target. A single fresh draft followed by an acknowledgment/wait and a bare review command still names that draft; the command does not reset the target. A passing mention is not naming. When in doubt, ask — the gate is the default. -3. **Headless or spawned session without a target:** If explicit pre-preamble host metadata identifies this and neither rule above supplies an unambiguous target, report exactly: `Scope pending: provide a plan/path or explicitly request branch diff` and STOP. Do not run the preamble or review tools. The session type does not choose a target or approve work. +3. **Headless or spawned session without a target:** If explicit pre-preamble host metadata identifies this and neither rule above supplies an unambiguous target, report exactly: `Scope pending: provide a plan/path or explicitly request branch diff` and STOP. A missing or disallowed AskUserQuestion tool is not that metadata; send the prose menu below. Do not run the preamble or review tools. The session type does not choose a target or approve work. Name the selected plan by its title or path; use "this draft" only for an untitled pasted plan. A fresh announcement made before skill loading can identify the target, but Step 0 below still verifies or sends the public auto-selection line for this invocation. diff --git a/review/specialists/testing.md b/review/specialists/testing.md index 1f5b96af9..475ebfb4c 100644 --- a/review/specialists/testing.md +++ b/review/specialists/testing.md @@ -133,8 +133,9 @@ check unavailable: . The finding stays INFORMATIONAL and nothing is pro deletion; run the search by hand to complete the evidence. (see ~/.claude/skills/gstack/docs/test-value-bar.md#caller-check-unavailable) -Skip a test carrying `gstack:test-value keep reason=""` (any comment syntax). Report -the count and reasons of skipped tests as one INFORMATIONAL line. +Skip a test carrying `gstack:test-value keep reason=""` (any comment syntax). If any +were skipped, report their count and reasons as one INFORMATIONAL line. Emit no finding +for a test you reviewed and kept. Rejection vocabulary, when a finding names why a new test fails the gate: `duplicate_protects`, `needs_seam`, `incomplete_card`, `no_credible_regression`,