Complete #140: cascade reasons fully skipped + AGENT sidecar (closes #151)

CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 ### Fixed
 
 - **`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<N>` 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<N>` behind as an orphan, and the next `task-reviewer <N>` 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<N>` 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=<unset>` — phantom calls upstream of the documented hook contract), each one wiping `~/.task-force/radio/sessions/<role>.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 (`⏸️ <slug>` / `▶️ <slug>` / `❓︎ <slug>`) so the re-seed's tab-id lookup survives `_rename_tab`'s repaints; **(#140 Fix C / #150 / #151)** `cmd_register` writes a `<role>.loadout` (and now `<role>.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 <M>` for spec lookup. Net result: in any non-gh loadout, `task-reviewer <pr> <spec-id>` 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/<id>-<slug>.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 <loadout>` after pulling this change so the updated `/reviewer` prompt lands in `.claude/commands/`. (#144)
 
 ### Changed

claude-gh/bin/radio

@@ -167,6 +167,17 @@ _loadout_sidecar() {
   printf '%s/%s.loadout' "$SESSIONS_DIR" "$1"
 }
 
+# Agent sidecar (#151). Same problem as LOADOUT: $TASK_FORCE_AGENT isn't
+# exported into Claude Code's hook subshells, so _ensure_session_file's
+# previous fallback to "${TASK_FORCE_AGENT:-claude}" silently wrote
+# AGENT=claude for kiro-* workers on every re-seed. Parallel sidecar, parallel
+# write/read/sweep semantics — no current runtime consumer, but a kiro worker
+# whose re-seeded session reports AGENT=claude is a footgun if anyone wires
+# AGENT-conditional behaviour later.
+_agent_sidecar() {
+  printf '%s/%s.agent' "$SESSIONS_DIR" "$1"
+}
+
 _inbox_dir()    { printf '%s/%s/inbox'     "$MAILBOX_DIR" "$1"; }
 _processed_dir(){ printf '%s/%s/processed' "$MAILBOX_DIR" "$1"; }
 
@@ -216,13 +227,26 @@ _ensure_session_file() {
   loadout=$(cat "$sidecar" 2>/dev/null || true)
   [[ -n "$loadout" ]] || loadout="${TASK_FORCE_LOADOUT:-unknown}"
 
+  # AGENT recovery (#151): same hook-subshell unset story as LOADOUT — the
+  # cascade wipes .info, and the next re-seed used to default to "claude"
+  # regardless of whether this was a kiro-* worker. Read the agent sidecar
+  # cmd_register persisted; fall back to the env var only for test paths that
+  # exercise _ensure_session_file directly. Same TOCTOU-safe `cat … || true`
+  # pattern as the loadout read above for the same reason (concurrent
+  # unregister race window).
+  local agent=""
+  local agent_sidecar
+  agent_sidecar=$(_agent_sidecar "$TASK_FORCE_ROLE")
+  agent=$(cat "$agent_sidecar" 2>/dev/null || true)
+  [[ -n "$agent" ]] || agent="${TASK_FORCE_AGENT:-claude}"
+
   local repo
   repo=$(git rev-parse --show-toplevel 2>/dev/null || echo "")
 
   {
     printf 'ROLE=%s\nTAB=%s\nTAB_ID=%s\nREPO=%s\nSTATE=idle\nLAST_HEARTBEAT=%s\nAGENT=%s\nLOADOUT=%s\n' \
       "$TASK_FORCE_ROLE" "$ZELLIJ_TAB" "$tab_id" "$repo" "$(_now)" \
-      "${TASK_FORCE_AGENT:-claude}" "$loadout"
+      "$agent" "$loadout"
     # Preserve the --auto opt-in across self-heal so cmd_send keeps using CR
     # for wake-up after an intra-session re-seed (#128). Use `if` rather than
     # `&&` so the brace group exits 0 when the flag is absent — otherwise
@@ -234,7 +258,7 @@ _ensure_session_file() {
   } | _atomic_write "$f"
 
   mkdir -p "$(_inbox_dir "$TASK_FORCE_ROLE")" "$(_processed_dir "$TASK_FORCE_ROLE")"
-  _log "ensure_session: re-seeded $TASK_FORCE_ROLE (file was missing) tab_id=$tab_id loadout=$loadout"
+  _log "ensure_session: re-seeded $TASK_FORCE_ROLE (file was missing) tab_id=$tab_id loadout=$loadout agent=$agent"
 }
 
 # Update STATE=… and LAST_HEARTBEAT=… on the current session file.
@@ -385,29 +409,33 @@ cmd_register() {
     fi
   fi
 
-  # Persist the loadout sidecar BEFORE the session file (#150 round-2 review:
-  # Finding 1). If the process is killed (OOM, task-done --force, host
-  # shutdown) between these two _atomic_write calls, the order matters:
+  # Persist the loadout and agent sidecars BEFORE the session file (#150
+  # round-2 review: Finding 1; extended for AGENT in #151). If the process is
+  # killed (OOM, task-done --force, host shutdown) between these writes, the
+  # order matters:
   #
   #   sidecar-first (this order):
-  #     pre-kill state:  .loadout=claude-gh, .info absent
-  #     re-seed reads:   .loadout → LOADOUT=claude-gh recovered ✓
+  #     pre-kill state:  .loadout=claude-gh, .agent=claude, .info absent
+  #     re-seed reads:   sidecars → LOADOUT/AGENT recovered ✓
   #
   #   info-first (the OLD order — pre-#150-r3):
-  #     pre-kill state:  .info has LOADOUT=claude-gh, .loadout absent
-  #     unregister cascade deletes .info; .loadout never existed
+  #     pre-kill state:  .info has LOADOUT/AGENT, sidecars absent
+  #     unregister cascade deletes .info; sidecars never existed
   #     re-seed reads:   no sidecar, hook subshell has no $TASK_FORCE_LOADOUT
-  #                      → LOADOUT=unknown ✗
+  #                      / $TASK_FORCE_AGENT → LOADOUT=unknown / AGENT=claude ✗
   #
   # The window is microseconds wide, but the cascade in #140 makes it
   # statistically reachable. Reversing the order eliminates the failure mode
-  # entirely. Skipping the sidecar on empty $loadout keeps the sidecar's mere
+  # entirely. Skipping the loadout sidecar on empty $loadout keeps its mere
   # existence equivalent to "we have a real loadout to restore" — otherwise
   # a re-seed could read an empty sidecar and clobber the env-var fallback
-  # that test paths still rely on.
+  # that test paths still rely on. The agent sidecar is always written
+  # because --agent is required (cmd_register exits 2 above if absent), so
+  # $agent is always a real value here.
   if [[ -n "$loadout" ]]; then
     printf '%s' "$loadout" | _atomic_write "$(_loadout_sidecar "$role")"
   fi
+  printf '%s' "$agent" | _atomic_write "$(_agent_sidecar "$role")"
 
   {
     printf 'ROLE=%s\nTAB=%s\nTAB_ID=%s\nREPO=%s\nSTATE=idle\nLAST_HEARTBEAT=%s\nAGENT=%s\nLOADOUT=%s\n' \
@@ -461,14 +489,33 @@ cmd_unregister() {
           _log "unregister: skipping (reason=$reason — intra-session event) role=$TASK_FORCE_ROLE"
           return 0
           ;;
+        "")
+          # Cascade pattern (#151): payload is present but contains no
+          # parseable `.reason` field. The PR-#150 diagnostic showed bursts of
+          # 144 unregister calls in ~25s with payload literally "y" and
+          # reason=<unset> — phantom invocations from somewhere upstream of
+          # Claude Code's documented SessionEnd hook (which always includes
+          # reason ∈ {clear, resume, logout, prompt_input_exit, other}).
+          # Treat these the same as clear/resume: known non-real-exit events
+          # that must not wipe the session file or the sidecars. Real exits
+          # still flow through the logout/prompt_input_exit/other branches
+          # below and clean up normally; manual invocations (no stdin) also
+          # proceed because `[[ ! -t 0 ]]` is false then.
+          payload_oneline=$(printf '%s' "$payload" | tr '\n' ' ' | tr -s ' ')
+          _log "unregister: skipping (reason=<unset> — cascade, no real-exit signal) role=$TASK_FORCE_ROLE payload=$payload_oneline"
+          return 0
+          ;;
       esac
     fi
     # Instrumentation for #140: when the hook fires with a payload that
     # *doesn't* match the skip-list, capture the full payload so we can see
     # what reason (or non-reason) is driving the unregister cascade. Empirically
     # observed as bursts of 20+ unregister calls in ~30s on long-running
     # worker sessions; the root cause needs the payload to investigate. Log
-    # is compacted to one line via tr so it doesn't fragment radio.log.
+    # is compacted to one line via tr so it doesn't fragment radio.log. Post-
+    # #151, an empty reason is already short-circuited above, so this line
+    # only fires for non-empty named reasons (logout, prompt_input_exit,
+    # other, or anything new Claude Code adds in the future).
     if [[ -n "$payload" ]]; then
       payload_oneline=$(printf '%s' "$payload" | tr '\n' ' ' | tr -s ' ')
       _log "unregister: proceeding (reason=${reason:-<unset>}) role=$TASK_FORCE_ROLE payload=$payload_oneline"
@@ -478,10 +525,11 @@ cmd_unregister() {
   local f
   f=$(_session_file "$TASK_FORCE_ROLE")
   rm -f "$f"
-  # Sweep the loadout sidecar too — leaving it behind would let a future
-  # _ensure_session_file resurrect a stale loadout for a role whose tab has
-  # really exited and been re-used (#140).
+  # Sweep the loadout + agent sidecars too — leaving them behind would let a
+  # future _ensure_session_file resurrect a stale loadout / agent for a role
+  # whose tab has really exited and been re-used (#140, #151).
   rm -f "$(_loadout_sidecar "$TASK_FORCE_ROLE")"
+  rm -f "$(_agent_sidecar "$TASK_FORCE_ROLE")"
   _log "unregister role=$TASK_FORCE_ROLE"
 }
 

claude-gh/bin/task-done

@@ -131,8 +131,14 @@ fi
 # `|| true` is a safety net: $TASK_FORCE_ROLE should be set inside a worker
 # tab, but if it isn't (e.g., task-done invoked from an unexpected shell)
 # the no-op must not abort cleanup. `radio unregister` itself already
-# silently exits when $TASK_FORCE_ROLE is unset (#93).
-radio unregister 2>/dev/null || true
+# silently exits when $TASK_FORCE_ROLE is unset (#93). Redirect stdin from
+# /dev/null so `radio unregister` doesn't inherit task-done's stdin — under
+# `--force` the calling shell may have piped `y` (or any other content)
+# to confirm something else, and post-#151 a non-empty stdin without a
+# parseable JSON reason triggers the cascade-skip path. That's correct
+# behaviour for SessionEnd hook invocations but wrong for explicit
+# task-done cleanup, which must always proceed with the unregister.
+radio unregister </dev/null 2>/dev/null || true
 
 # Move out of worktree before removing it
 cd "$MAIN_WORKTREE"