diff --git a/.claude/gh-workflow.md b/.claude/gh-workflow.md index 16db482..eed6a44 100644 --- a/.claude/gh-workflow.md +++ b/.claude/gh-workflow.md @@ -95,8 +95,10 @@ role transition runs through it. The PM / planner / worker prompts shell out to | Reviewer | review done, no findings | `radio send --to pm --intent review-complete-clean --pr ` | | Reviewer | review done, blockers/nits posted | `radio send --to pm --intent review-complete-with-findings --pr ` | -When a worker finishes its task and has nothing pending, the `radio ready` step -runs automatically via the `Stop` hook — you don't need to invoke it manually. +When a worker's turn ends, the `Stop` hook runs `radio stop-hook` +automatically: with an empty inbox it marks the role idle; if messages queued +up while the worker was busy, it blocks the stop (staying busy) so the agent +drains them immediately — you don't need to invoke it manually. Full command form: diff --git a/.claude/settings.json b/.claude/settings.json index 23a9377..fba0f04 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -25,7 +25,7 @@ "hooks": [ { "type": "command", - "command": "radio ready && radio check" + "command": "radio stop-hook" } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb6b69..dd70422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- **Messages queued while an agent is busy are now delivered at the end of its turn — new `radio stop-hook` replaces `radio ready && radio check` as the claude Stop hook.** `radio send` makes exactly one wake attempt, at send time; a message sent while the recipient was `busy`/`awaiting` (the common case for a working agent) queued with zero redelivery. The old Stop hook couldn't help: Stop-hook stdout goes to the hook subshell, never to the model, so its `radio check` was dead output — live logs showed 101 `busy … queued` entries with no redelivery and 23 messages dangling in the pm inbox. `radio stop-hook` counts the inbox first (empty → mark idle, normal stop — the common fast path spawns nothing); on unread messages it marks the role **busy** (the agent is about to continue) and emits Claude Code's `{"decision": "block", …}` JSON on stdout, which forces the agent to continue and drain the queue immediately. A `stop_hook_active: true` payload never re-blocks (no continue loop; any messages that arrived during the drain turn are logged as stranded — #164/#168 close that path), and a jq-less host degrades to the old queue-only behavior rather than risk blocking blind. The four claude `task-init` installers write the new hook and migrate a byte-exact legacy `radio ready && radio check` Stop entry in place; any customized variant is left untouched with a loud warning telling you what to replace. kiro-* loadouts intentionally keep the legacy wiring — kiro's `agentStop` has no block-JSON mechanism (revisit with #146). Also fixes the `radio ready` usage string, which falsely claimed "(and process pending)". **Upgrading**: re-run `task-init ` in each configured repo — the migration only runs then; without it the repo keeps the dead Stop hook and busy-recipient messages keep stranding. (#163) - **`task-done --remove-worktree` now force-deletes reviewer branches so `task-reviewer` re-dispatch isn't blocked.** The cleanup's `git branch -d` is the safe-delete variant that requires the branch to be fully merged into HEAD — correct for worker branches (don't drop unmerged work), but wrong for reviewer worktrees, whose `task/review-pr` branch is forked from the PR's head ref and is never going to land in main (the PR is). Result: every reviewer cleanup left `task/review-pr` behind as an orphan, and the next `task-reviewer ` refused to dispatch via its per-PR guard with `Error: a review worktree or branch already exists`. The user had to manually `git branch -D task/review-pr` between every re-review round. `task-done` now branches on the `PR_NUMBER=` marker that `task-reviewer` writes into the worktree's `.info` file: when present it does `git branch -D` (scaffold-only, safe to force); when absent worker behavior is unchanged. Lands across the `task-done-std` and `task-done-local` drift groups (7 binaries). (#148) - **Radio session corruption on long-running workers — `LOADOUT` / `AGENT` / `TAB_ID` no longer lost mid-life.** Three-bug cluster surfaced by `--auto` workers running for hours: Claude Code's `SessionEnd` hook fires repeatedly with non-`clear|resume` reasons (the diagnostic added in #150 caught bursts of 144 invocations in ~25s, payload literally a single `y` character, `reason=` — phantom calls upstream of the documented hook contract), each one wiping `~/.task-force/radio/sessions/.info`. The next `busy`/`ready` hook re-seeded the file with `LOADOUT=unknown`, empty `TAB_ID=`, and `AGENT=claude` (silently flipping kiro-* workers), which then broke `radio send`'s tab-id-based wake-up — PM pings landed in the mailbox silently and never woke the worker. Three concrete fixes: **(#140 Fix B)** `_zellij_tab_id_by_name` now matches the emoji-prefixed visible name (`⏸️ ` / `▶️ ` / `❓︎ `) so the re-seed's tab-id lookup survives `_rename_tab`'s repaints; **(#140 Fix C / #150 / #151)** `cmd_register` writes a `.loadout` (and now `.agent`) sidecar atomically BEFORE the `.info` write (so a kill mid-call leaves a recoverable state), `_ensure_session_file` reads from those sidecars on re-seed (TOCTOU-safe via `cat || true`), and `cmd_unregister`'s skip-list now also short-circuits on empty `reason` — treating the cascade pattern the same as `clear` / `resume`. Real-exit reasons (`logout` / `prompt_input_exit` / `other`) still flow through and clean up normally. Net result: a worker that previously degraded after 4–8 hours (or sooner under heavy subagent use) now keeps its full identity through the cascade, and PM `radio send` keeps waking it via the original `TAB_ID`. (#140, #150, #151) - **`task-reviewer`'s spec-issue cross-check now works on `claude-jira` / `claude-notion` / `claude-local`.** The dispatcher and the `/reviewer` prompt were both GitHub-only: `parse_issue_number` rejected anything that wasn't a bare integer or `github.com/.../issues/N`; PR-body auto-detect only matched `Closes #N`; the URL-synthesis step always emitted a GitHub issues URL; and the prompt hardcoded `gh issue view ` for spec lookup. Net result: in any non-gh loadout, `task-reviewer ` silently degraded to a diff-only review. The dispatcher is now loadout-aware (detects from its own file path) — `claude-gh` keeps numeric / URL parsing and PR-body auto-detect; `claude-jira` / `claude-notion` / `claude-local` accept any non-empty 2nd positional arg as an opaque spec identifier (Jira key, Notion URL, local task slug) and pass it through to `/reviewer` unchanged. The 4 dispatcher bodies stay byte-identical (drift-checked). Each loadout's `commands/reviewer.md` now points the spec-lookup step at the right tool: GH stays on `gh issue view`; `claude-jira` uses `mcp__atlassian__getJiraIssue`; `claude-notion` uses `mcp__notion__notion-fetch`; `claude-local` reads `tasks/-.md` via the `Read` tool. With no 2nd arg in a non-gh loadout, the wrapper emits a clear "PR-body auto-detect is GitHub-only — pass the spec id explicitly" advisory and proceeds diff-only. **Upgrading**: re-run `task-init ` after pulling this change so the updated `/reviewer` prompt lands in `.claude/commands/`. (#144) diff --git a/README.md b/README.md index 48abd12..2ac7f57 100644 --- a/README.md +++ b/README.md @@ -387,11 +387,12 @@ Role names are addressable strings, not free-form: the PM is `pm`, and each work | `radio ack ` | Mark it acknowledged (idempotent — no-op if already processed by a prior `read`) | | `radio register` / `radio unregister` | Add/remove this tab's session file (`~/.task-force/radio/sessions/.info`) | | `radio ready` / `radio busy` | Toggle this session's `STATE` field — drives the wake-up vs. queue decision on the sender side | +| `radio stop-hook` | Stop-hook entrypoint: empty inbox → mark idle; unread messages → mark busy and emit Stop-hook block JSON so the agent continues and drains them | | `radio orphans` | List session files whose heartbeat is >1h stale | ### How wake-up works -`radio send` reads the recipient's session file. If `STATE=idle`, it resolves the recipient's tab/pane id via `zellij action list-tabs --json` / `list-panes --json --tab` and writes `radio check\n` straight into that pane with `zellij action write-chars --pane-id` — no focus switch, so the sender's tab stays put. On the recipient's next turn end, the `Stop` hook (`radio ready && radio check`) surfaces the new message. If `STATE=busy`, the message is queued silently — no failed wake attempt, no interrupting the recipient mid-turn. +`radio send` reads the recipient's session file. If `STATE=idle`, it resolves the recipient's tab/pane id via `zellij action list-tabs --json` / `list-panes --json --tab` and writes `radio check\n` straight into that pane with `zellij action write-chars --pane-id` — no focus switch, so the sender's tab stays put. If `STATE=busy`, the message is queued with no wake attempt — no interrupting the recipient mid-turn. Delivery then happens at the end of the recipient's current turn: its `Stop` hook (`radio stop-hook`) sees the non-empty inbox and emits Stop-hook block JSON, which makes Claude Code continue the agent so it drains the queue immediately (`radio check`, then `radio read` each message). A `stop_hook_active` payload never re-blocks, so a drain turn can't loop forever. ### The hooks that make it work @@ -401,7 +402,7 @@ Role names are addressable strings, not free-form: the PM is `pm`, and each work |-------------------|-------------------------------|------------------------------------------------| | `SessionStart` | `radio register` | Claims the role's session file for this tab | | `UserPromptSubmit`| `radio busy` | Marks the session busy while a turn is running | -| `Stop` | `radio ready && radio check` | Marks idle and surfaces any queued messages | +| `Stop` | `radio stop-hook` | Marks idle — or blocks the stop so the agent drains queued messages first | For the kiro loadouts the same logic lives in `.kiro/hooks/` and runs off Kiro's equivalent triggers. @@ -464,7 +465,7 @@ Then bumps the project Status field to `In Review` (or keeps it at `In Progress` radio send --to pm --intent review-requested --pr 42 ``` -PM's zellij tab gets focused; on its next turn end the `Stop` hook's `radio check` surfaces the ping. +If PM is idle, its pane is woken with a `radio check`; if PM is mid-turn, the message queues and PM's `Stop` hook (`radio stop-hook`) forces it to drain the ping at the end of that turn. **6. Worker idles.** The worker stops here. It does **not** run `task-done` yet — cleanup waits for PM's explicit go-ahead in step 8. diff --git a/bin/radio b/bin/radio index b14d0a5..c389019 100755 --- a/bin/radio +++ b/bin/radio @@ -16,9 +16,12 @@ Usage: radio read [--peek|--no-ack] print body; by default also move inbox/.md -> processed/ radio ack move message to processed/ (idempotent: no-op if already processed) radio register --role ROLE --tab TAB [--repo PATH] --agent claude|kiro [--loadout NAME] - radio ready mark this role idle (and process pending) + radio ready mark this role idle radio busy mark this role busy radio awaiting mark this role awaiting user input + radio stop-hook Stop-hook entrypoint: empty inbox -> mark idle; unread + messages -> mark busy + emit block JSON so the agent + continues and drains them (reads hook payload on stdin) radio unregister remove this role's session file radio orphans list session files with stale (>1h) heartbeat @@ -536,6 +539,84 @@ cmd_ready() { _silent_exit_if_no_role; _update_state idle; } cmd_busy() { _silent_exit_if_no_role; _update_state busy; } cmd_awaiting() { _silent_exit_if_no_role; _update_state awaiting; } +# Stop-hook entrypoint (#163). `radio send` makes exactly one wake attempt, at +# send time — a message sent while the recipient is busy/awaiting queues with +# zero redelivery, and the old Stop hook (`radio ready && radio check`) +# couldn't fix that: Stop-hook stdout goes to the hook subshell, never to the +# model. Instead, when the inbox is non-empty we emit Claude Code's +# `{"decision": "block", ...}` JSON on stdout (with exit 0), which forces the +# agent to continue and drain the queue the moment its turn ends. +cmd_stop_hook() { + _silent_exit_if_no_role + + # Count first, pure bash — the common case is an empty inbox, and this + # ordering keeps the cat+jq spawns (below) entirely off that fast path + # (#172 review). It also shrinks the code that runs before the state flip, + # so a set -e failure mid-function is less likely to surface as a non-zero + # exit — which the Stop-hook contract interprets as "block, with raw + # stderr as the reason". + local inbox n=0 + inbox=$(_inbox_dir "$TASK_FORCE_ROLE") + if [[ -d "$inbox" ]]; then + shopt -s nullglob + local msgs=("$inbox"/*.md) + shopt -u nullglob + n=${#msgs[@]} + fi + + if (( n == 0 )); then + _update_state idle + return 0 + fi + + # Fail SAFE without jq (#172 review): we can't read stop_hook_active, and + # blocking blind could re-block on every Stop forever. jq-less hosts never + # get zellij wakes either, so degrading to the old queue-only behavior is + # consistent. + if ! command -v jq >/dev/null 2>&1; then + _log "stop-hook: jq unavailable — not blocking ($n unread) role=$TASK_FORCE_ROLE" + _update_state idle + return 0 + fi + + # Claude Code pipes the Stop-hook JSON payload on stdin. Manual invocations + # from a terminal carry no payload — treat that (and a malformed payload) + # as stop_hook_active=false. `// empty` maps false/null to "", so only a + # literal true suppresses the block. + local payload="" active="" + if [[ ! -t 0 ]]; then + payload=$(cat 2>/dev/null || true) + fi + if [[ -n "$payload" ]]; then + active=$(printf '%s' "$payload" | jq -r '.stop_hook_active // empty' 2>/dev/null || true) + fi + + # stop_hook_active=true means this Stop event was itself caused by a prior + # block from this hook. Blocking again would loop the agent forever — the + # agent already got one forced continuation to drain the inbox; let it + # stop. Known gap: a message that arrives DURING the drain turn (state=busy + # → cmd_send queues silently) strands here until the next wake — logged for + # visibility; #164 (prompt-hook injection) and #168 (drain-on-register) + # close that path from other directions. + if [[ "$active" == "true" ]]; then + _log "stop-hook: stop_hook_active=true — allowing stop ($n still unread) role=$TASK_FORCE_ROLE" + _update_state idle + return 0 + fi + + # We're about to block, so the agent WILL continue — busy is the truthful + # state. Painting idle here would open a window (no UserPromptSubmit fires + # on a hook-forced continuation) where cmd_send sees idle and write-chars + # into a running pane mid-drain (#172 review). + _update_state busy + + # The backticks in the reason are literal markdown for the agent reading + # the block message, not command substitution. + # shellcheck disable=SC2016 + printf '{"decision": "block", "reason": "radio: %s unread message(s) — run `radio check`, process each with `radio read `, act on the intents, then stop."}\n' "$n" + _log "stop-hook: blocked stop — $n unread message(s) role=$TASK_FORCE_ROLE" +} + cmd_send() { local to="" intent="" pr="" issue="" repo="" body="" local body_set=0 @@ -773,6 +854,7 @@ case "$sub" in ready) cmd_ready ;; busy) cmd_busy ;; awaiting) cmd_awaiting ;; + stop-hook) cmd_stop_hook ;; unregister) cmd_unregister ;; orphans) cmd_orphans ;; -h|--help) usage 0 ;; diff --git a/claude-gh/bin/task-init b/claude-gh/bin/task-init index d1cf34d..c3c9e4d 100755 --- a/claude-gh/bin/task-init +++ b/claude-gh/bin/task-init @@ -254,7 +254,12 @@ _radio_install_hooks() { # sessions don't set the env var, so they keep the loadout default ($loadout). local register_cmd="radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent claude --loadout \${TASK_FORCE_LOADOUT:-$loadout}" local busy_cmd="radio busy" - local stop_cmd="radio ready && radio check" + # `radio stop-hook` replaces the old `radio ready && radio check` (#163): + # it marks the role idle AND emits Stop-hook block JSON when the inbox has + # unread messages, so a busy recipient drains its queue at end of turn + # (Stop-hook stdout never reaches the model, so `radio check` there was + # dead output). + local stop_cmd="radio stop-hook" local end_cmd="radio unregister" local awaiting_cmd="radio awaiting" @@ -280,10 +285,21 @@ _radio_install_hooks() { if (arr // []) | any(((.matcher // "") == mtch) and ((.hooks // []) | any((.command // "") | startswith("radio ")))) then arr else (arr // []) + [{matcher: mtch, hooks: [{type: "command", command: cmd}]}] end; + # One-shot migration (#163): rewrite the legacy Stop command in place so + # re-running task-init upgrades existing repos. add_radio alone would see + # a "radio " command and leave the stale one behind. + def upgrade_stop(arr; cmd): + (arr // []) | map( + if has("hooks") then + .hooks = (.hooks | map( + if (.command // "") == "radio ready && radio check" then (.command = cmd) else . end + )) + else . end + ); .hooks //= {} | .hooks.SessionStart = add_radio(.hooks.SessionStart; $sess) | .hooks.UserPromptSubmit = add_radio(.hooks.UserPromptSubmit; $ups) - | .hooks.Stop = add_radio(.hooks.Stop; $stop) + | .hooks.Stop = add_radio(upgrade_stop(.hooks.Stop; $stop); $stop) | .hooks.SessionEnd = add_radio(.hooks.SessionEnd; $endcmd) | .hooks.PermissionRequest = add_radio(.hooks.PermissionRequest; $awaiting) | .hooks.PreToolUse = add_radio_matcher(.hooks.PreToolUse; "AskUserQuestion"; $awaiting) @@ -321,6 +337,25 @@ _radio_install_hooks() { ) ' "$settings_file" >"$tmp" mv -f "$tmp" "$settings_file" + + # Post-merge verification (#172 review): upgrade_stop only rewrites the + # byte-exact legacy command. Any other radio-prefixed Stop command (trailing + # whitespace, `radio ready && radio check && ./notify.sh`, plain + # `radio ready`) is skipped by the upgrade AND blocks add_radio from + # installing the new one — the repo would keep a dead Stop hook while + # task-init prints success. Detect and warn loudly instead. + local stray + stray=$(jq -r --arg stop "$stop_cmd" \ + '[.hooks.Stop // [] | .[] | .hooks // [] | .[] | .command // "" + | select(startswith("radio ") and . != $stop)] | .[]' \ + "$settings_file") + if [[ -n "$stray" ]]; then + { + echo "WARNING: $settings_file has a radio Stop hook task-init could not upgrade:" + printf ' %s\n' "$stray" + echo " Replace it manually with: $stop_cmd" + } >&2 + fi echo "✓ Merged radio hooks + gh read allow-list into $settings_file" } diff --git a/claude-gh/steering/gh-workflow.example.md b/claude-gh/steering/gh-workflow.example.md index e25e875..5a8732f 100644 --- a/claude-gh/steering/gh-workflow.example.md +++ b/claude-gh/steering/gh-workflow.example.md @@ -95,8 +95,10 @@ role transition runs through it. The PM / planner / worker prompts shell out to | Reviewer | review done, no findings | `radio send --to pm --intent review-complete-clean --pr ` | | Reviewer | review done, blockers/nits posted | `radio send --to pm --intent review-complete-with-findings --pr ` | -When a worker finishes its task and has nothing pending, the `radio ready` step -runs automatically via the `Stop` hook — you don't need to invoke it manually. +When a worker's turn ends, the `Stop` hook runs `radio stop-hook` +automatically: with an empty inbox it marks the role idle; if messages queued +up while the worker was busy, it blocks the stop (staying busy) so the agent +drains them immediately — you don't need to invoke it manually. Full command form: diff --git a/claude-jira/bin/task-init b/claude-jira/bin/task-init index 1bd47ca..047d08c 100755 --- a/claude-jira/bin/task-init +++ b/claude-jira/bin/task-init @@ -223,7 +223,12 @@ _radio_install_hooks() { # sessions don't set the env var, so they keep the loadout default ($loadout). local register_cmd="radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent claude --loadout \${TASK_FORCE_LOADOUT:-$loadout}" local busy_cmd="radio busy" - local stop_cmd="radio ready && radio check" + # `radio stop-hook` replaces the old `radio ready && radio check` (#163): + # it marks the role idle AND emits Stop-hook block JSON when the inbox has + # unread messages, so a busy recipient drains its queue at end of turn + # (Stop-hook stdout never reaches the model, so `radio check` there was + # dead output). + local stop_cmd="radio stop-hook" local end_cmd="radio unregister" local awaiting_cmd="radio awaiting" @@ -252,10 +257,21 @@ _radio_install_hooks() { if (arr // []) | any(((.matcher // "") == mtch) and ((.hooks // []) | any((.command // "") | startswith("radio ")))) then arr else (arr // []) + [{matcher: mtch, hooks: [{type: "command", command: cmd}]}] end; + # One-shot migration (#163): rewrite the legacy Stop command in place so + # re-running task-init upgrades existing repos. add_radio alone would see + # a "radio " command and leave the stale one behind. + def upgrade_stop(arr; cmd): + (arr // []) | map( + if has("hooks") then + .hooks = (.hooks | map( + if (.command // "") == "radio ready && radio check" then (.command = cmd) else . end + )) + else . end + ); .hooks //= {} | .hooks.SessionStart = add_radio(.hooks.SessionStart; $sess) | .hooks.UserPromptSubmit = add_radio(.hooks.UserPromptSubmit; $ups) - | .hooks.Stop = add_radio(.hooks.Stop; $stop) + | .hooks.Stop = add_radio(upgrade_stop(.hooks.Stop; $stop); $stop) | .hooks.SessionEnd = add_radio(.hooks.SessionEnd; $endcmd) | .hooks.PermissionRequest = add_radio(.hooks.PermissionRequest; $awaiting) | .hooks.PreToolUse = add_radio_matcher(.hooks.PreToolUse; "AskUserQuestion"; $awaiting) @@ -288,6 +304,25 @@ _radio_install_hooks() { ) ' "$settings_file" >"$tmp" mv -f "$tmp" "$settings_file" + + # Post-merge verification (#172 review): upgrade_stop only rewrites the + # byte-exact legacy command. Any other radio-prefixed Stop command (trailing + # whitespace, `radio ready && radio check && ./notify.sh`, plain + # `radio ready`) is skipped by the upgrade AND blocks add_radio from + # installing the new one — the repo would keep a dead Stop hook while + # task-init prints success. Detect and warn loudly instead. + local stray + stray=$(jq -r --arg stop "$stop_cmd" \ + '[.hooks.Stop // [] | .[] | .hooks // [] | .[] | .command // "" + | select(startswith("radio ") and . != $stop)] | .[]' \ + "$settings_file") + if [[ -n "$stray" ]]; then + { + echo "WARNING: $settings_file has a radio Stop hook task-init could not upgrade:" + printf ' %s\n' "$stray" + echo " Replace it manually with: $stop_cmd" + } >&2 + fi echo "✓ Merged radio hooks + Atlassian read allow-list into $settings_file" } diff --git a/claude-jira/steering/jira-workflow.example.md b/claude-jira/steering/jira-workflow.example.md index 1684b0c..32d9fbc 100644 --- a/claude-jira/steering/jira-workflow.example.md +++ b/claude-jira/steering/jira-workflow.example.md @@ -89,8 +89,10 @@ role transition runs through it. The PM / planner / worker prompts shell out to | PM | review requested changes | `radio send --to --intent changes-requested --pr ` | | PM | PR merged | `radio send --to --intent approved-and-merged --pr ` | -When a worker finishes its task and has nothing pending, the `radio ready` step -runs automatically via the `Stop` hook — you don't need to invoke it manually. +When a worker's turn ends, the `Stop` hook runs `radio stop-hook` +automatically: with an empty inbox it marks the role idle; if messages queued +up while the worker was busy, it blocks the stop (staying busy) so the agent +drains them immediately — you don't need to invoke it manually. Full command form: diff --git a/claude-local/bin/task-init b/claude-local/bin/task-init index 5f703f1..707c633 100755 --- a/claude-local/bin/task-init +++ b/claude-local/bin/task-init @@ -248,7 +248,12 @@ _radio_install_hooks() { # sessions don't set the env var, so they keep the loadout default ($loadout). local register_cmd="radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent claude --loadout \${TASK_FORCE_LOADOUT:-$loadout}" local busy_cmd="radio busy" - local stop_cmd="radio ready && radio check" + # `radio stop-hook` replaces the old `radio ready && radio check` (#163): + # it marks the role idle AND emits Stop-hook block JSON when the inbox has + # unread messages, so a busy recipient drains its queue at end of turn + # (Stop-hook stdout never reaches the model, so `radio check` there was + # dead output). + local stop_cmd="radio stop-hook" local end_cmd="radio unregister" local awaiting_cmd="radio awaiting" @@ -271,10 +276,21 @@ _radio_install_hooks() { if (arr // []) | any(((.matcher // "") == mtch) and ((.hooks // []) | any((.command // "") | startswith("radio ")))) then arr else (arr // []) + [{matcher: mtch, hooks: [{type: "command", command: cmd}]}] end; + # One-shot migration (#163): rewrite the legacy Stop command in place so + # re-running task-init upgrades existing repos. add_radio alone would see + # a "radio " command and leave the stale one behind. + def upgrade_stop(arr; cmd): + (arr // []) | map( + if has("hooks") then + .hooks = (.hooks | map( + if (.command // "") == "radio ready && radio check" then (.command = cmd) else . end + )) + else . end + ); .hooks //= {} | .hooks.SessionStart = add_radio(.hooks.SessionStart; $sess) | .hooks.UserPromptSubmit = add_radio(.hooks.UserPromptSubmit; $ups) - | .hooks.Stop = add_radio(.hooks.Stop; $stop) + | .hooks.Stop = add_radio(upgrade_stop(.hooks.Stop; $stop); $stop) | .hooks.SessionEnd = add_radio(.hooks.SessionEnd; $endcmd) | .hooks.PermissionRequest = add_radio(.hooks.PermissionRequest; $awaiting) | .hooks.PreToolUse = add_radio_matcher(.hooks.PreToolUse; "AskUserQuestion"; $awaiting) @@ -298,6 +314,25 @@ _radio_install_hooks() { ) ' "$settings_file" >"$tmp" mv -f "$tmp" "$settings_file" + + # Post-merge verification (#172 review): upgrade_stop only rewrites the + # byte-exact legacy command. Any other radio-prefixed Stop command (trailing + # whitespace, `radio ready && radio check && ./notify.sh`, plain + # `radio ready`) is skipped by the upgrade AND blocks add_radio from + # installing the new one — the repo would keep a dead Stop hook while + # task-init prints success. Detect and warn loudly instead. + local stray + stray=$(jq -r --arg stop "$stop_cmd" \ + '[.hooks.Stop // [] | .[] | .hooks // [] | .[] | .command // "" + | select(startswith("radio ") and . != $stop)] | .[]' \ + "$settings_file") + if [[ -n "$stray" ]]; then + { + echo "WARNING: $settings_file has a radio Stop hook task-init could not upgrade:" + printf ' %s\n' "$stray" + echo " Replace it manually with: $stop_cmd" + } >&2 + fi echo "✓ Merged radio hooks into $settings_file" } diff --git a/claude-local/steering/local-workflow.example.md b/claude-local/steering/local-workflow.example.md index 35cce05..eb3570c 100644 --- a/claude-local/steering/local-workflow.example.md +++ b/claude-local/steering/local-workflow.example.md @@ -110,8 +110,10 @@ role transition runs through it. The PM / planner / worker prompts shell out to | PM | review requested changes | `radio send --to --intent changes-requested --pr ` | | PM | PR merged | `radio send --to --intent approved-and-merged --pr ` | -When a worker finishes its task and has nothing pending, the `radio ready` step -runs automatically via the `Stop` hook — you don't need to invoke it manually. +When a worker's turn ends, the `Stop` hook runs `radio stop-hook` +automatically: with an empty inbox it marks the role idle; if messages queued +up while the worker was busy, it blocks the stop (staying busy) so the agent +drains them immediately — you don't need to invoke it manually. Full command form: diff --git a/claude-notion/bin/task-init b/claude-notion/bin/task-init index 6b99bd3..cef74ec 100755 --- a/claude-notion/bin/task-init +++ b/claude-notion/bin/task-init @@ -178,7 +178,12 @@ _radio_install_hooks() { # sessions don't set the env var, so they keep the loadout default ($loadout). local register_cmd="radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent claude --loadout \${TASK_FORCE_LOADOUT:-$loadout}" local busy_cmd="radio busy" - local stop_cmd="radio ready && radio check" + # `radio stop-hook` replaces the old `radio ready && radio check` (#163): + # it marks the role idle AND emits Stop-hook block JSON when the inbox has + # unread messages, so a busy recipient drains its queue at end of turn + # (Stop-hook stdout never reaches the model, so `radio check` there was + # dead output). + local stop_cmd="radio stop-hook" local end_cmd="radio unregister" local awaiting_cmd="radio awaiting" @@ -208,10 +213,21 @@ _radio_install_hooks() { if (arr // []) | any(((.matcher // "") == mtch) and ((.hooks // []) | any((.command // "") | startswith("radio ")))) then arr else (arr // []) + [{matcher: mtch, hooks: [{type: "command", command: cmd}]}] end; + # One-shot migration (#163): rewrite the legacy Stop command in place so + # re-running task-init upgrades existing repos. add_radio alone would see + # a "radio " command and leave the stale one behind. + def upgrade_stop(arr; cmd): + (arr // []) | map( + if has("hooks") then + .hooks = (.hooks | map( + if (.command // "") == "radio ready && radio check" then (.command = cmd) else . end + )) + else . end + ); .hooks //= {} | .hooks.SessionStart = add_radio(.hooks.SessionStart; $sess) | .hooks.UserPromptSubmit = add_radio(.hooks.UserPromptSubmit; $ups) - | .hooks.Stop = add_radio(.hooks.Stop; $stop) + | .hooks.Stop = add_radio(upgrade_stop(.hooks.Stop; $stop); $stop) | .hooks.SessionEnd = add_radio(.hooks.SessionEnd; $endcmd) | .hooks.PermissionRequest = add_radio(.hooks.PermissionRequest; $awaiting) | .hooks.PreToolUse = add_radio_matcher(.hooks.PreToolUse; "AskUserQuestion"; $awaiting) @@ -244,6 +260,25 @@ _radio_install_hooks() { ) ' "$settings_file" >"$tmp" mv -f "$tmp" "$settings_file" + + # Post-merge verification (#172 review): upgrade_stop only rewrites the + # byte-exact legacy command. Any other radio-prefixed Stop command (trailing + # whitespace, `radio ready && radio check && ./notify.sh`, plain + # `radio ready`) is skipped by the upgrade AND blocks add_radio from + # installing the new one — the repo would keep a dead Stop hook while + # task-init prints success. Detect and warn loudly instead. + local stray + stray=$(jq -r --arg stop "$stop_cmd" \ + '[.hooks.Stop // [] | .[] | .hooks // [] | .[] | .command // "" + | select(startswith("radio ") and . != $stop)] | .[]' \ + "$settings_file") + if [[ -n "$stray" ]]; then + { + echo "WARNING: $settings_file has a radio Stop hook task-init could not upgrade:" + printf ' %s\n' "$stray" + echo " Replace it manually with: $stop_cmd" + } >&2 + fi echo "✓ Merged radio hooks + Notion read allow-list into $settings_file" } diff --git a/claude-notion/steering/notion-workflow.example.md b/claude-notion/steering/notion-workflow.example.md index 0df5363..58bbbf7 100644 --- a/claude-notion/steering/notion-workflow.example.md +++ b/claude-notion/steering/notion-workflow.example.md @@ -101,8 +101,10 @@ role transition runs through it. The PM / planner / worker prompts shell out to | PM | review requested changes | `radio send --to --intent changes-requested --pr ` | | PM | PR merged | `radio send --to --intent approved-and-merged --pr ` | -When a worker finishes its task and has nothing pending, the `radio ready` step -runs automatically via the `Stop` hook — you don't need to invoke it manually. +When a worker's turn ends, the `Stop` hook runs `radio stop-hook` +automatically: with an empty inbox it marks the role idle; if messages queued +up while the worker was busy, it blocks the stop (staying busy) so the agent +drains them immediately — you don't need to invoke it manually. Full command form: diff --git a/kiro-gh/bin/task-init b/kiro-gh/bin/task-init index 77b30b4..f09dd22 100755 --- a/kiro-gh/bin/task-init +++ b/kiro-gh/bin/task-init @@ -236,6 +236,9 @@ EOF _write_hook "radio-register" "agentSpawn" \ "radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent kiro --loadout \${TASK_FORCE_LOADOUT:-${loadout}}" _write_hook "radio-busy" "userPromptSubmit" "radio busy" + # kiro intentionally keeps the legacy Stop wiring: Claude Code's + # `radio stop-hook` block-JSON mechanism (#163) has no kiro agentStop + # equivalent. Revisit with kiro parity (#146). _write_hook "radio-ready" "agentStop" "radio ready && radio check" # Note: kiro has no session-end trigger (agentStop fires per-turn, not on # session close), so the Claude SessionEnd → `radio unregister` beat has diff --git a/kiro-local/bin/task-init b/kiro-local/bin/task-init index 2a68ce0..83738f2 100755 --- a/kiro-local/bin/task-init +++ b/kiro-local/bin/task-init @@ -233,6 +233,9 @@ EOF _write_hook "radio-register" "agentSpawn" \ "radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent kiro --loadout ${loadout}" _write_hook "radio-busy" "userPromptSubmit" "radio busy" + # kiro intentionally keeps the legacy Stop wiring: Claude Code's + # `radio stop-hook` block-JSON mechanism (#163) has no kiro agentStop + # equivalent. Revisit with kiro parity (#146). _write_hook "radio-ready" "agentStop" "radio ready && radio check" # Note: kiro has no session-end trigger (agentStop fires per-turn, not on # session close), so the Claude SessionEnd → `radio unregister` beat has diff --git a/kiro-notion/bin/task-init b/kiro-notion/bin/task-init index 948d146..427733a 100755 --- a/kiro-notion/bin/task-init +++ b/kiro-notion/bin/task-init @@ -166,6 +166,9 @@ EOF _write_hook "radio-register" "agentSpawn" \ "radio register --role \$TASK_FORCE_ROLE --tab \$ZELLIJ_TAB --repo \$(git rev-parse --show-toplevel 2>/dev/null) --agent kiro --loadout ${loadout}" _write_hook "radio-busy" "userPromptSubmit" "radio busy" + # kiro intentionally keeps the legacy Stop wiring: Claude Code's + # `radio stop-hook` block-JSON mechanism (#163) has no kiro agentStop + # equivalent. Revisit with kiro parity (#146). _write_hook "radio-ready" "agentStop" "radio ready && radio check" # Note: kiro has no session-end trigger (agentStop fires per-turn, not on # session close), so the Claude SessionEnd → `radio unregister` beat has diff --git a/tests/claude_gh_task_init.bats b/tests/claude_gh_task_init.bats index 6d1959b..f1156b4 100644 --- a/tests/claude_gh_task_init.bats +++ b/tests/claude_gh_task_init.bats @@ -398,7 +398,75 @@ EOF assert_output "2" run jq -r '.hooks.Stop[].hooks[0].command' "$TARGET_DIR/.claude/settings.json" assert_output --partial "./scripts/pre-tool-lint.sh" - assert_output --partial "radio ready" + assert_output --partial "radio stop-hook" +} + +@test "Stop hook calls 'radio stop-hook' (#163)" { + run "$CLAUDE_GH_TASK_INIT" + assert_success + run jq -r '.hooks.Stop[0].hooks[0].command' "$TARGET_DIR/.claude/settings.json" + assert_output "radio stop-hook" +} + +@test "legacy Stop hook 'radio ready && radio check' is upgraded in place (#163)" { + mkdir -p "$TARGET_DIR/.claude" + cat > "$TARGET_DIR/.claude/settings.json" <<'EOF' +{ + "hooks": { + "Stop": [ + {"hooks": [{"type": "command", "command": "radio ready && radio check"}]} + ] + } +} +EOF + run "$CLAUDE_GH_TASK_INIT" + assert_success + refute_output --partial "WARNING" + # Rewritten in place — no duplicate entry, no stale command left behind. + run jq -r '.hooks.Stop | length' "$TARGET_DIR/.claude/settings.json" + assert_output "1" + run jq -r '.hooks.Stop[0].hooks[0].command' "$TARGET_DIR/.claude/settings.json" + assert_output "radio stop-hook" +} + +@test "non-byte-exact legacy Stop variant is not upgraded — loud warning instead (#172)" { + mkdir -p "$TARGET_DIR/.claude" + cat > "$TARGET_DIR/.claude/settings.json" <<'EOF' +{ + "hooks": { + "Stop": [ + {"hooks": [{"type": "command", "command": "radio ready && radio check && ./notify.sh"}]} + ] + } +} +EOF + run "$CLAUDE_GH_TASK_INIT" + assert_success + # The variant is preserved untouched (never clobber a user customization)... + run jq -r '.hooks.Stop[0].hooks[0].command' "$TARGET_DIR/.claude/settings.json" + assert_output "radio ready && radio check && ./notify.sh" + # ...but task-init must not pretend everything is fine. + run "$CLAUDE_GH_TASK_INIT" --force + assert_output --partial "WARNING" + assert_output --partial "radio ready && radio check && ./notify.sh" + assert_output --partial "radio stop-hook" +} + +@test "matcher-only user Stop entry is not given an empty hooks array by the upgrade (#172)" { + mkdir -p "$TARGET_DIR/.claude" + cat > "$TARGET_DIR/.claude/settings.json" <<'EOF' +{ + "hooks": { + "Stop": [ + {"matcher": "user-thing"} + ] + } +} +EOF + run "$CLAUDE_GH_TASK_INIT" + assert_success + run jq -c '.hooks.Stop[] | select(.matcher == "user-thing")' "$TARGET_DIR/.claude/settings.json" + assert_output '{"matcher":"user-thing"}' } @test "idempotent re-run does not duplicate radio hook entries" { diff --git a/tests/radio_stop_hook.bats b/tests/radio_stop_hook.bats new file mode 100644 index 0000000..3ed1f77 --- /dev/null +++ b/tests/radio_stop_hook.bats @@ -0,0 +1,161 @@ +#!/usr/bin/env bats +# Tests for `radio stop-hook` (#163) — the Stop-hook entrypoint that replaces +# `radio ready && radio check`. +# +# Background: `radio send` makes exactly one wake attempt, at send time. If +# the recipient is busy/awaiting, the message queues with zero redelivery — +# and the old Stop hook couldn't help, because Stop-hook stdout goes to the +# hook subshell, never to the model. `radio stop-hook` closes that gap: it +# marks the role idle, and if the inbox has unread messages it emits +# `{"decision": "block", ...}` on stdout so Claude Code forces the agent to +# continue and drain the queue at the end of its turn. A payload with +# `stop_hook_active: true` never re-blocks (no infinite continue loop). + +bats_load_library bats-support +bats_load_library bats-assert + +load helpers/common + +setup() { + setup_task_force_home + unset ZELLIJ # no wakeup attempts in unit tests + export TASK_FORCE_ROLE=worker-foo +} + +teardown() { + teardown_all +} + +# Queue one message into worker-foo's inbox without waking anyone +# ($ZELLIJ is unset, so cmd_send just writes the file). +_queue_message() { + TASK_FORCE_ROLE=pm "$RADIO" send --to worker-foo --intent changes-requested --pr 5 --body "${1:-fix the thing}" +} + +# ----- no-role gate (#93 semantics) ------------------------------------------ + +@test "stop-hook with no TASK_FORCE_ROLE is a silent exit 0 (plain claude session)" { + run bash -c "echo '{}' | env -u TASK_FORCE_ROLE '$RADIO' stop-hook" + assert_success + assert_output "" +} + +# ----- idle transition -------------------------------------------------------- + +@test "stop-hook marks the role idle" { + "$RADIO" register --role worker-foo --tab w-foo --agent claude + "$RADIO" busy + run grep "^STATE=" "$TASK_FORCE_HOME/radio/sessions/worker-foo.info" + assert_output "STATE=busy" + run bash -c "echo '{}' | '$RADIO' stop-hook" + assert_success + run grep "^STATE=" "$TASK_FORCE_HOME/radio/sessions/worker-foo.info" + assert_output "STATE=idle" +} + +@test "stop-hook with empty inbox exits 0 with no output (normal stop)" { + "$RADIO" register --role worker-foo --tab w-foo --agent claude + run bash -c "echo '{}' | '$RADIO' stop-hook" + assert_success + assert_output "" +} + +# ----- block on pending mail -------------------------------------------------- + +@test "stop-hook with pending messages emits block JSON and exits 0" { + "$RADIO" register --role worker-foo --tab w-foo --agent claude + _queue_message "first" + _queue_message "second" + run bash -c "echo '{\"stop_hook_active\": false}' | '$RADIO' stop-hook" + assert_success + assert_output --partial '"decision": "block"' + assert_output --partial '2 unread message(s)' + assert_output --partial 'radio check' + assert_output --partial 'radio read' +} + +@test "stop-hook block output is valid JSON with decision=block" { + "$RADIO" register --role worker-foo --tab w-foo --agent claude + _queue_message + run bash -c "echo '{}' | '$RADIO' stop-hook | jq -er '.decision'" + assert_success + assert_output "block" +} + +@test "stop-hook does not consume the inbox — messages stay for radio check/read" { + "$RADIO" register --role worker-foo --tab w-foo --agent claude + _queue_message + run bash -c "echo '{}' | '$RADIO' stop-hook" + assert_success + run bash -c "ls '$TASK_FORCE_HOME/radio/mailbox/worker-foo/inbox'/*.md | wc -l | tr -d ' '" + assert_output "1" +} + +@test "stop-hook marks busy when it blocks — the agent is about to continue" { + # No UserPromptSubmit fires on a hook-forced continuation, so if we painted + # idle here cmd_send could write-chars into the running pane mid-drain + # (#172 review). + "$RADIO" register --role worker-foo --tab w-foo --agent claude + _queue_message + run bash -c "echo '{}' | '$RADIO' stop-hook" + assert_success + run grep "^STATE=" "$TASK_FORCE_HOME/radio/sessions/worker-foo.info" + assert_output "STATE=busy" +} + +@test "stop-hook without jq fails safe: no block, idle, exit 0" { + # jq absent means stop_hook_active is unreadable; blocking blind could + # re-block on every Stop forever, so stop-hook degrades to queue-only. + # Build a PATH of symlinks to everything radio needs, minus jq. + local nojq_bin + nojq_bin=$(mktemp -d) + for cmd in bash cat mkdir mv rm awk grep cut head tr date dirname basename ls sed env; do + ln -s "$(command -v $cmd)" "$nojq_bin/$cmd" + done + "$RADIO" register --role worker-foo --tab w-foo --agent claude + "$RADIO" busy + _queue_message + run bash -c "echo '{}' | env PATH='$nojq_bin' '$RADIO' stop-hook" + rm -rf "$nojq_bin" + assert_success + assert_output "" + run grep "^STATE=" "$TASK_FORCE_HOME/radio/sessions/worker-foo.info" + assert_output "STATE=idle" + run cat "$TASK_FORCE_HOME/radio/log" + assert_output --partial "jq unavailable — not blocking" +} + +@test "stop-hook without a stdin payload still blocks on pending mail (manual invocation)" { + "$RADIO" register --role worker-foo --tab w-foo --agent claude + _queue_message + run "$RADIO" stop-hook