fix: degrade SendMessage shutdown handshake gracefully outside agent teams (#645)#646
Conversation
…gent teams The teammate shutdown_request/shutdown_response handshake requires the SendMessage tool, which only exists inside a live Claude Code agent team (CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1). The six team agent prompts carried an unconditional "you MUST call the SendMessage tool" mandate, so in subagent mode (e.g. prefer_teams=never) or when agent teams are disabled, agents and readers saw an instruction that cannot be followed (issue swt-labs#645). Make the handshake conditional on actually being in a team, delivered through channels that know the runtime mode: - agents/vbw-{dev,qa,scout,lead,debugger,docs}.md: remove the static "Shutdown Handling" section entirely. The teammate-side response protocol is no longer baked into static prompts (SubagentStart is unreliable for team-spawned teammates, so a hook injector would miss them). - references/execute-protocol.md: the orchestrator now appends the teammate shutdown-response protocol to each teammate's spawn prompt in the true-team-mode branch only — gated by construction, reliably delivered (it is the spawn prompt). Also add a hard precondition that team mode is never requested unless CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1. - scripts/compaction-instructions.sh: the post-compaction shutdown reminder is now gated on a live team marker (delegated-workflow.sh status-json: live && delegation_mode=team && team_name) instead of firing for any team role. Non-team runs get zero shutdown tokens. - references/handoff-schemas.md: point the conditional-refusal note at the new protocol location. Also fixes a latent PreCompact bug uncovered here: snapshot-resume.sh's stdout (the snapshot path) leaked into the hook JSON whenever .execution-state.json existed (every live execute), corrupting the PreCompact additionalContext output. Its stdout is now discarded. tests/shutdown-protocol.bats: rewritten to assert the new design — no static SendMessage mandate in agent prompts, protocol present in execute-protocol's team-spawn block, and compaction injecting only with a live team marker (with subagent/no-marker negative cases). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
QA Round 1 |
QA round 1 (Claude Opus 4.8) found a major regression and two stale-doc issues, all confirmed: - [major] /vbw:map and /vbw:debug spawn teammates into real TeamCreate teams with their own inline spawn payloads and do not read execute-protocol.md, so they relied on the now-removed static "## Shutdown Handling" agent sections for the teammate-side handler. Their orchestrator teardown gate waits for a tool-call shutdown_response from teammates that were no longer told to send one. Fix: append the teammate shutdown handler to the map.md Scout communication section (duo/quad, team-mode only) and to the debug.md Path A hypothesis-investigator task body (team-mode only). - [minor] execute-protocol.md enforcement-limitation note still claimed enforcement relies on "mechanical SendMessage instructions in all 6 agent prompts" — removed in this PR. Updated to describe the spawn-prompt + gated compaction mechanism. - [minor] handoff-schemas.md Delivery format prose read as if the handler is an intrinsic agent obligation. Clarified it is delivered at spawn in team mode only (and mirrored in map/debug team-spawn prompts), not in static prompts. Verified: shutdown-protocol.bats 71/71, verify-commands-contract 521/521, verify-execute-delegation-routing pass, two-phase/typed-protocol/event-type bats pass, lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
QA round 2 (Claude Opus 4.8) verdict: clean (approve). Team-spawn coverage confirmed complete (only debug.md/map.md/vibe.md use TeamCreate; vibe.md delegates to execute-protocol.md which carries the gated block; research/fix/qa use non-team subagents only). Two minor items, both optional: - [minor, addressed] Over-escaped JSON quotes inside inline code spans in the round-1 map.md/debug.md handler insertions (backslashes are literal inside a code span). Unescaped to clean JSON — functionally equivalent, cleaner to read and to copy into the spawned task body. - [minor, left as-is] map.md places the handler under the shared "Scout communication" section rather than inline like debug.md. The reviewer confirmed this is consistent with map.md's existing pattern of deferring all Scout task-body content to that later section; no functional risk. Verified: verify-commands-contract 521/521, shutdown-protocol.bats 71/71. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
QA Round 2Round-2 fixes applied: unescaped the JSON quotes (item #5). Item #4 left as-is per reviewer (consistent with map.md structure). |
QA round 3 (Claude Opus 4.8) verdict: CLEAN — no critical, major, or minor findings. Empty evidence commit per CONTRIBUTING.md (clean round still records an evidence commit in this format). Confirmed by the reviewer: - All three handler copies (execute-protocol.md, map.md, debug.md) specify the same fields (type, approved, request_id, final_status, complete|idle|in_progress) and the "call SendMessage / plain text not sufficient" mandate — no drift. - Round-2 quote-unescaping is a 1-line change per file; the JSON parses; no markdown/contract regression. - Team-spawn coverage complete; no non-team path received the handler. - compaction gate fail-closed; SCRIPT_DIR single-defined before use; snapshot stdout leak fixed. - bats sanity: live/non-team gate tests form a real positive+negative pair; schema-field test derives fields dynamically (not vacuous). - Serial runs: shutdown-protocol 71/71, verify-commands-contract 521/521, verify-bash-scripts-contract 669/669, verify-lsp-first-policy 49/49, verify-plugin-root-resolution 19/19, verify-claude-bootstrap 58/58, agent-shutdown-integration 26/26, two-phase-completion 20/20, crash-recovery 20/20, event-type-validation 5/5; shellcheck/bash -n clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
QA Round 3 (final) |
The branch is current with main and the full suite passes locally (3583 BATS, 51/51 contract, lint). The prior CI failure predated a fix now present in both the branch and main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dpearson2699
left a comment
There was a problem hiding this comment.
Verdict: REQUEST_CHANGES
Blocking findings are inline where GitHub could anchor them.
Unanchored blockers:
- None.
Additional non-blocking finding:
- VBW-PR-003: README.md:118 still says all six team-participating agents have explicit shutdown handlers with mechanical SendMessage instructions, but this PR moves those handlers to true-team spawn prompts. Reason not inline: README.md is not in the changed diff.
Evidence:
- Linked issue: #645.
- Tests:
bash testing/run-all.shpassed locally: lint 1/1, contract 53/53, BATS 3583 passed / 0 failed. - Blind baseline: complete; expected
/vbw:debugPath A to fall back or gate team mode when SendMessage is unavailable. - QA: complete; independent QA verdict FAIL for the same
/vbw:debugPath A gap. - QA evidence comments: stale. QA rounds are present and align with earlier QA-round commits, but they predate later
ci:and merge commits/current head.
| ``` | ||
| Replace `{bug description from Step 1}` with the actual parsed bug description. If `RESEARCH_CONTEXT` is non-empty, include it in each Debugger task prompt below. If empty, omit the `<standalone_research_context>` block entirely. | ||
| - Create 3 tasks via TaskCreate, each with: bug report, standalone research context (include ONLY if RESEARCH_CONTEXT was non-empty: `<standalone_research_context>Prior research findings from /vbw:research. Advisory — verify all claims against the current codebase before relying on them.\n{RESEARCH_CONTEXT}</standalone_research_context>`), extended context from todo detail only when the detail has actual signal from Step 1 (include `detail.context` and `detail.files` — omit this section entirely when detail was absent or structurally empty), and if detail did not have signal but bounded sparse-context enrichment from Step 1 returned `ok`, include the enrichment summary plus matched files and markers as a compact hint. Omit this enrichment section entirely if the helper returned `no_signal` or `no_match`. Include ONE hypothesis only (no cross-contamination), working dir, codebase bootstrap instruction ("If `.vbw-planning/codebase/META.md` exists, read ARCHITECTURE.md, CONCERNS.md, PATTERNS.md, and DEPENDENCIES.md (whichever exist) from `.vbw-planning/codebase/` to bootstrap codebase understanding before investigating"), instruction to report via `debugger_report` schema (see `{plugin-root}/references/handoff-schemas.md`) including explicit `resolution_observation=already_fixed|needs_change|inconclusive`, instruction that `resolution_observation` is analysis-scoped only (teammates do not own the final command outcome or session status), and instruction that accepted-process-exception/backlog metadata alone is not enough for `already_fixed`; `already_fixed` requires fresh current evidence that the underlying issue no longer reproduces or the branch contains a real fix. Explicit report-only contract: "You are a hypothesis investigator, not the implementation owner. Do NOT edit files, apply fixes, run mutating Bash, commit, request implementation approval, or claim ownership of the final session outcome. Stop after diagnosis plus evidence reporting via `debugger_report`." Also include instruction: "If investigation reveals pre-existing failures unrelated to this bug, list them in your response under a 'Pre-existing Issues' heading with test name, file, and failure message." **Include `[analysis-only]` in each task subject** (e.g., "Hypothesis 1: race condition in sync handler [analysis-only]") so the TaskCompleted hook skips the commit-verification gate for report-only tasks. Keep `[analysis-only]` as an advisory hook marker only — it does NOT replace the report-only contract above. | ||
| - Create 3 tasks via TaskCreate, each with: bug report, standalone research context (include ONLY if RESEARCH_CONTEXT was non-empty: `<standalone_research_context>Prior research findings from /vbw:research. Advisory — verify all claims against the current codebase before relying on them.\n{RESEARCH_CONTEXT}</standalone_research_context>`), extended context from todo detail only when the detail has actual signal from Step 1 (include `detail.context` and `detail.files` — omit this section entirely when detail was absent or structurally empty), and if detail did not have signal but bounded sparse-context enrichment from Step 1 returned `ok`, include the enrichment summary plus matched files and markers as a compact hint. Omit this enrichment section entirely if the helper returned `no_signal` or `no_match`. Include ONE hypothesis only (no cross-contamination), working dir, codebase bootstrap instruction ("If `.vbw-planning/codebase/META.md` exists, read ARCHITECTURE.md, CONCERNS.md, PATTERNS.md, and DEPENDENCIES.md (whichever exist) from `.vbw-planning/codebase/` to bootstrap codebase understanding before investigating"), instruction to report via `debugger_report` schema (see `{plugin-root}/references/handoff-schemas.md`) including explicit `resolution_observation=already_fixed|needs_change|inconclusive`, instruction that `resolution_observation` is analysis-scoped only (teammates do not own the final command outcome or session status), and instruction that accepted-process-exception/backlog metadata alone is not enough for `already_fixed`; `already_fixed` requires fresh current evidence that the underlying issue no longer reproduces or the branch contains a real fix. Explicit report-only contract: "You are a hypothesis investigator, not the implementation owner. Do NOT edit files, apply fixes, run mutating Bash, commit, request implementation approval, or claim ownership of the final session outcome. Stop after diagnosis plus evidence reporting via `debugger_report`." Also include instruction: "If investigation reveals pre-existing failures unrelated to this bug, list them in your response under a 'Pre-existing Issues' heading with test name, file, and failure message." **Include `[analysis-only]` in each task subject** (e.g., "Hypothesis 1: race condition in sync handler [analysis-only]") so the TaskCompleted hook skips the commit-verification gate for report-only tasks. Keep `[analysis-only]` as an advisory hook marker only — it does NOT replace the report-only contract above. **Teammate shutdown handler (Path A team only):** because these investigators run inside a real team and the teardown gate below waits for a tool-call response, also include this in each task body: "When you receive a message containing `"type":"shutdown_request"` (or `shutdown_request` in the text), finish any in-progress tool call, then **call the `SendMessage` tool** with `{"type": "shutdown_response", "approved": true, "request_id": "<id from shutdown_request>", "final_status": "complete|idle|in_progress"}`, then STOP. Plain text acknowledgement does NOT satisfy the handshake — you MUST call `SendMessage`." |
There was a problem hiding this comment.
VBW-PR-001 (blocker): Path A still reaches team creation and teardown based only on prefer_teams/effort/ambiguity. The issue explicitly includes /vbw:debug --competing; in an environment without SendMessage, this prompt still appends a SendMessage-only shutdown handler and later waits for SendMessage responses. Gate Path A on real team messaging availability or fall back to Path B/serialized non-team before emitting this task body.
| echo "$section" | grep -qi 'STOP' | ||
| } | ||
|
|
||
| @test "execute-protocol.md shutdown protocol is gated to team mode only" { |
There was a problem hiding this comment.
VBW-PR-002 (blocker): The focused shutdown tests now prove the execute-protocol team-mode gate, but they do not cover the /vbw:debug Path A reproduction path. Add a contract test that Path A does not use TeamCreate/SendMessage when Agent Teams/SendMessage are unavailable and falls back to non-team mode.
…labs#645) Addresses PR swt-labs#646 review (VBW-PR-001, VBW-PR-002, VBW-PR-003). VBW-PR-001: Path A (Competing Hypotheses) is a real agent team — it calls TeamCreate, spawns teammates with team_name, and its teardown HARD GATE waits for SendMessage shutdown_response tool calls. The routing decision selected Path A purely on prefer_teams/effort/ambiguity, so in an environment without agent teams / SendMessage (the default — CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS is 0/unset) it would emit a TeamCreate/SendMessage-dependent body and then hang at teardown waiting for responses that can never arrive. Add a hard team-tooling precondition evaluated before the prefer_teams tree: require the Agent Teams flag plus TeamCreate/SendMessage in the live tool set, else force Path B (single debugger) with a "⚠ Agent Teams not enabled" notice. Surfaced the Agent Teams flag in the command Context (mirrors map.md) and tightened the Path A header. VBW-PR-002: added three contract tests in tests/shutdown-protocol.bats asserting debug.md surfaces the flag, gates Path A on the precondition (requiring SendMessage/TeamCreate and forcing Path B when unavailable), and that the Path A header carries the precondition. VBW-PR-003: README.md "Shutdown coordination" said all 6 agents carry explicit SendMessage shutdown handlers — stale after this PR moved handlers to spawn-time (team-mode-only) delivery. Reworded to match. Tests: bash testing/run-all.sh green (3586 BATS, 0 failed); the 3 new gate tests pass. (A flaky parallel-shard failure in the untouched phase-detect.bats appeared once and cleared on re-run — passes 3/3 in isolation.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFFmV2TntcmvJRjvcv3gAD
|
Addressed the review feedback in VBW-PR-001 — gate VBW-PR-002 — contract coverage for the Path A reproduction. Added three tests in VBW-PR-003 — stale README.
|
What
The teammate
shutdown_request/shutdown_responsehandshake requires theSendMessagetool, which only exists inside a live Claude Code agent team (CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1). This makes that dependency conditional on actually being in a team, delivered through channels that know the runtime mode.agents/vbw-{dev,qa,scout,lead,debugger,docs}.md— removed the static "Shutdown Handling" section (the unconditional "you MUST call the SendMessage tool" mandate).references/execute-protocol.md— the orchestrator now appends the teammate shutdown-response protocol to each teammate's spawn prompt in the true-team-mode branch only (gated by construction, reliably delivered). Added a hard precondition that team mode is never requested unlessCLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1.scripts/compaction-instructions.sh— the post-compaction shutdown reminder is now gated on a live team marker (delegated-workflow.sh status-json:live && delegation_mode=team && team_name). Also fixes a latent bug wheresnapshot-resume.sh's stdout (the snapshot path) corrupted the PreCompact hook JSON whenever.execution-state.jsonexisted.references/handoff-schemas.md— repointed the conditional-refusal note to the new protocol location.tests/shutdown-protocol.bats— rewritten to assert the new design.Why
Reported in #645: in subagent mode (e.g.
prefer_teams=never) or when agent teams are disabled,SendMessageis not in the toolset, so the agent prompts' mandatory "you MUST call the SendMessage tool" instruction could not be followed. VBW's orchestrator already degrades to subagent mode correctly; the gap was the static agent prompts (and the unconditional compaction reminder) issuing an un-followable directive.Chosen approach is option (b) from the issue — graceful degradation — implemented so the instruction is only ever issued when a real team exists, rather than adding no-op conditional text to every spawn.
How
status-json:live+delegation_mode=team+team_name), which subsumes the env flag (VBW only writes a team marker afterTeamCreatesucceeds). The env flag is additionally checked up front as a hard precondition before team mode is even requested.SubagentStartis known-unreliable for team-spawned teammates) and re-injected after compaction only when the live team marker is present.Testing
bash testing/run-all.sh: lint 1/1, contract 51/51.tests/shutdown-protocol.bats71/71.agent-shutdown-integration,hooks-bash-classifier,hooks-isolation-lifecycle,role-isolation-runtime) — each passes 0 failures when run serially.shellcheck -S warning+bash -nclean oncompaction-instructions.sh.Fixes #645