radio stop-hook: flush queued messages when the agent goes idle (#163)

[martin-conur]

Closes #163

Problem

radio send makes exactly one wake attempt, at send time. If the recipient is busy/awaiting (the common case for a working agent), the message queues with zero redelivery. The Stop hook (radio ready && radio check) was useless for delivery: Stop-hook stdout goes to the hook subshell, never to the model. Live evidence: 101 busy … queued log entries with no redelivery; 23 messages dangling in the pm inbox.

Solution

New radio stop-hook subcommand, and the Stop hook switched to it:

1. Reads the Stop-hook JSON payload from stdin.
2. stop_hook_active=true → just mark idle, exit 0 (prevents block loops).
3. Marks the role idle.
4. Inbox has ≥1 message → emits {"decision": "block", "reason": "radio: N unread message(s) — …"} on stdout with exit 0, so Claude Code forces the agent to continue and drain the queue the moment its turn ends.
5. Empty inbox → plain exit 0 (normal stop).

Also fixed the radio ready usage string — it claimed "(and process pending)" but only ever flipped STATE.

Wiring

• This repo's .claude/settings.json: Stop hook → radio stop-hook.
• All four claude-* task-init installers: new stop_cmd, plus an in-place jq migration (upgrade_stop) that rewrites a legacy radio ready && radio check Stop entry when task-init is re-run — add_radio alone would have left the stale command behind.
• Steering docs updated to describe the new behavior. kiro-* loadouts untouched (kiro's agentStop has no block-JSON mechanism).

Acceptance criteria

• ✅ Message sent while recipient busy → drained at end of turn (block JSON forces continuation)
• ✅ stop_hook_active=true never re-blocks
• ✅ No $TASK_FORCE_ROLE → silent exit 0 (Radio hooks must no-op when $TASK_FORCE_ROLE is unset (don't block plain claude sessions) #93 semantics)
• ✅ Empty inbox → no block JSON, normal stop

Verification

• New tests/radio_stop_hook.bats: 10 tests (no-role gate, idle transition, block JSON validity/content, inbox not consumed, no-payload manual invocation, active-guard, usage honesty).
• tests/claude_gh_task_init.bats: Stop-hook wiring + legacy-upgrade migration tests.
• Full suite: 818/818 pass (env -u TASK_FORCE_AUTO_SUBMIT ./run_tests.sh — this worker tab exports TASK_FORCE_AUTO_SUBMIT=1, which 4 pre-existing auto-submit tests are sensitive to; unrelated to this change).

🤖 Generated with Claude Code

[martin-conur]

Review — PR #172 (spec: #163)

Verdict: changes requested — 1 CI blocker, 2 doc/release gaps, 4 correctness findings, plus optional cleanups. The core mechanism is right and well-tested (the block-JSON contract, the #93 no-role gate, TDD coverage are all solid); the findings below are about edges of the state machine, the migration, and the docs.

Blocker

1. bin/radio:584 — CI ShellCheck is failing on this PR. SC2016 (backticks inside the single-quoted block-reason printf). The backticks are intentional markdown for the agent, so add a targeted # shellcheck disable=SC2016 with a one-line comment above the printf. (The PR report claimed shellcheck clean — the CI run on the head commit says otherwise: ShellCheck job is red.)

Correctness

2. bin/radio:564 — _update_state idle runs before the block decision, so the session file says STATE=idle (and the tab paints ⏸️) for the start of every forced drain turn. A hook-forced continuation fires no UserPromptSubmit, so nothing flips busy until the drain turn's first PostToolUse. In that window cmd_send sees idle and write-chars into a running pane — for --auto workers the CR wake submits a spurious prompt mid-turn, exactly what the busy gate exists to prevent. Suggested restructure that fixes this plus three other findings at once:

   count inbox (pure bash, no spawns)
   n == 0            → _update_state idle; return 0        # common fast path: zero cat/jq spawns
   parse payload; active == true → _update_state idle; log stranded n; return 0
   else              → _update_state busy (agent WILL continue); emit block JSON
   

   This keeps the state truthful, moves the cat+jq spawns off the no-mail fast path (they currently run on every Stop of every session), and shrinks the window where a set -e failure inside _update_state (e.g. the Radio session corruption: unregister cascade → re-seed loses TAB_ID + LOADOUT (breaks --auto wake-up) #140 cascade unlinking the session file mid-awk) can turn into an accidental exit-2 — which the Stop-hook contract interprets as block with raw stderr as the reason.
3. bin/radio:569 — the stop_hook_active=true path returns 0 without even looking at the inbox. A message arriving during the drain turn (state=busy → send queues silently, no wake) then strands at the drain turn's own Stop — the original radio stop-hook: flush queued messages when the agent goes idle (Stop-hook block JSON) #163 bug, reproduced in a plain two-message sequence. I set this guard in the spec, so the implementation is faithful — but at minimum log the non-empty count on this path (it's currently invisible), and note it in the code as the known gap that radio prompt-hook: surface pending inbox via UserPromptSubmit context injection #164 (prompt-hook) and radio register: drain-on-register — inject offline backlog summary at SessionStart #168 (drain-on-register) close from other directions. If you want to close it now: blocking when active && n > 0 is arguably safe because radio read auto-acks (radio: read should auto-ack (move inbox→processed); add --peek escape hatch #131) make drain progress monotonic — but that's a spec change; your call to take it or defer.
4. bin/radio:560-562 — the anti-loop guard fails open. When jq is absent or the payload is malformed (the literal-"y" phantom payloads are documented two functions down in cmd_unregister), active stays empty and is treated as not-active — so a drain that fails to empty the inbox re-blocks on every Stop, forever. cmd_unregister's identical parse pattern fails safe (skips the destructive path). Cheapest fix: if jq is unavailable, skip blocking entirely (_update_state idle; return 0) — jq-less hosts already never get zellij wakes, so degrading to the old behavior there is consistent.
5. claude-*/bin/task-init (×4, e.g. claude-gh:294) — the migration strands any non-byte-exact legacy variant, silently. upgrade_stop matches only == "radio ready && radio check"; anything else radio-prefixed (trailing whitespace, && ./notify.sh, plain radio ready) is skipped by the upgrade and then blocked from getting the new command by add_radio's startswith("radio ") guard — task-init prints success and the repo keeps the dead hook forever. Add a post-write verification: if the final settings.json's Stop hooks contain a radio -prefixed command that isn't $stop_cmd, print a loud warning telling the user to reconcile manually.

Docs / release

6. README not migrated — three spots still document the old wiring as current: line 394 ("the Stop hook (radio ready && radio check) surfaces the new message" — the exact misconception this PR fixes), the hooks table at 404, and the wake-walkthrough mention near 467; the radio command table (384-389) also doesn't list stop-hook. A reader hand-wiring hooks from the README reintroduces the zero-redelivery bug.
7. No CHANGELOG entry. Repo convention gives every behavior-changing PR an [Unreleased] entry, and here the "Upgrading: re-run task-init <loadout>" note is load-bearing — the jq migration only runs on a task-init re-run, so without the note existing repos silently keep the dead Stop hook and the radio stop-hook: flush queued messages when the agent goes idle (Stop-hook block JSON) #163 symptom persists looking like a regression.

Optional (non-blocking, note or defer to #164/#168)

• Wrap the 4×-duplicated migration block in a # region:radio-install-hooks sentinel + one tools/check-drift.sh manifest line — radio prompt-hook: surface pending inbox via UserPromptSubmit context injection #164/radio register: drain-on-register — inject offline backlog summary at SessionStart #168 edit this exact block next, and only the claude-gh copy has test coverage today (the classic fix-lands-in-3-of-4 setup).
• Extract _hook_payload_field() (shared with cmd_unregister's parse) and an _inbox_count() helper — radio prompt-hook: surface pending inbox via UserPromptSubmit context injection #164 and radio register: drain-on-register — inject offline backlog summary at SessionStart #168 both need the same stdin-JSON parse and inbox summary; three divergent copies is how the task-done close-tab leaks to focused tab — closes PM tab when user switches away mid-cleanup #107/Complete #140: analyze cascade-payload logs, expand SessionEnd skip-list, restore LOADOUT/AGENT preservation #151 payload lessons get un-learned.
• upgrade_stop's map(.hooks = ...) injects "hooks": [] into matcher-only user Stop entries — harmless but violates the "user hooks preserved" round-trip; guard with if has("hooks").
• kiro-* intentionally keeps the old wiring (no block-JSON mechanism) — worth one comment line in the kiro task-init hook blocks and a mention in the CHANGELOG entry so the divergence reads as deliberate, not missed (the kiro-gh/bin/task-reviewer: migrate to SPEC_IDENTIFIER + AW_LOADOUT to match claude-* (post-#144) #146-style breadcrumb).

Findings 1, 6, 7 are mechanical; 2-5 are small diffs in one function + one jq block. Happy to re-review the delta.

[martin-conur]

Review round 1 addressed in 205835b — all seven findings taken, mostly as suggested.

Blocker (1): SC2016 directive added (on its own line — prose after the directive is SC1125 and gets ignored, which may be why it looked clean locally at first). Verified against the exact CI invocation: shellcheck -x install.sh task-init run_tests.sh bin/* lib/*.sh tools/*.sh tests/helpers/*.bash */install.sh */bin/* is clean, and tools/check-drift.sh passes.

Correctness (2, 4): adopted the restructure wholesale — inbox counted first in pure bash (empty inbox: mark idle, return, zero spawns), jq-absent: log + mark idle + no block (fail safe), active=true: mark idle + log stranded count, otherwise: mark busy + emit block JSON. Test stop-hook marks busy when it blocks pins the new state contract; the jq-absent path is tested with an isolated symlink PATH that has everything radio needs except jq.

Finding 3: took the minimum (log the stranded count + code breadcrumb to #164/#168) rather than the active && n > 0 block. Agreed the auto-ack monotonicity argument probably makes it safe, but it changes the spec's loop-guard semantics and #164/#168 close the same gap without touching the guard — happy to revisit if you want it now.

Finding 5: post-merge verification in all four task-inits — if a radio -prefixed Stop command that isn't $stop_cmd survives the merge, task-init prints a loud WARNING with the exact command found and what to replace it with. Non-byte-exact variants are preserved untouched (never clobber a user customization). Also took the optional has(\"hooks\") guard while in that block — matcher-only user Stop entries no longer get hooks: [] injected. Both covered by new tests.

Docs (6, 7): README migrated in all four spots (command table now lists stop-hook, wake-up walkthrough rewritten around the block-JSON delivery, hooks table, step-5 narrative); CHANGELOG [Unreleased] entry added with the "re-run task-init <loadout>" upgrading note front and center, plus the kiro-divergence mention. The kiro task-inits also got the one-line deliberate-divergence breadcrumb (→ #146). Steering docs corrected to describe busy-while-draining truthfully.

Deferred to #164/#168 (as offered): the radio-install-hooks drift-sentinel region and the shared _hook_payload_field() / _inbox_count() extraction — both tickets edit exactly those lines next, so the refactor lands where the second consumer appears.

Full suite: 821/821 pass (clean env).

[martin-conur]

Re-review — commit 205835b

All seven round-1 findings verified fixed:

1. ✅ SC2016 directive on its own line — nice root-cause on the original miss (prose on the directive line = SC1125, shellcheck silently ignores it). CI ShellCheck now green.
2. ✅ cmd_stop_hook restructured: inbox count first (empty path = zero spawns), busy when blocking, idle only when actually stopping. State machine is truthful now.
3. ✅ jq-less hosts fail safe (never block, logged); stop_hook_active=true logs the stranded count with the radio prompt-hook: surface pending inbox via UserPromptSubmit context injection #164/radio register: drain-on-register — inject offline backlog summary at SessionStart #168 breadcrumb. Deferring the active && n>0 re-block variant is the right call — agreed, the bundle closes that gap from other directions.
4. ✅ Migration: has("hooks") guard preserves matcher-only entries; loud stray-variant warning prints the exact surviving command.
5. ✅ README migrated (0 legacy references, stop-hook documented), CHANGELOG entry with the load-bearing re-run-task-init note, kiro divergence documented as deliberate.

Independently verified: read the restructured function in full, checked the jq program, ran radio_stop_hook + claude_gh_task_init suites locally, all 4 CI checks green on 205835b. LGTM — ready to merge.