Radio session corruption: unregister cascade → re-seed loses TAB_ID + LOADOUT

claude-gh/bin/radio

@@ -111,13 +111,29 @@ _strip_tab_prefix() {
 # Resolve a zellij tab's stable id from its visible name. Empty stdout means
 # "lookup failed" (zellij not reachable, jq absent, name not found) — callers
 # must treat that as a hard skip so we never operate on the wrong tab (#102).
+#
+# Emoji-prefix awareness (#140): _rename_tab paints the visible tab name to
+# "⏸️ <slug>" / "▶️ <slug>" / "❓︎ <slug>" reflecting STATE. Callers always
+# query by the bare slug ($ZELLIJ_TAB / cmd_register's --tab arg) — that's
+# the stable identifier captured at pane creation and never mutates. So a
+# literal name match against the painted name misses on every flip past
+# idle. We compare against both the bare name AND any of the three emoji-
+# prefixed variants so the lookup survives _rename_tab's repaints. This is
+# the symmetric fix for #117 (cmd_register) AND the same loss in
+# _ensure_session_file's re-seed path (Bug B in #140).
 _zellij_tab_id_by_name() {
   local target="$1"
   [[ -n "$target" ]] || return 0
   command -v jq >/dev/null 2>&1 || return 0
   zellij action list-tabs --json 2>/dev/null \
-    | jq -r --arg n "$target" \
-        '[.[] | select(.name == $n) | .tab_id] | .[0] // empty' 2>/dev/null
+    | jq -r --arg n "$target" '
+        [.[] | select(
+          .name == $n
+          or .name == ("⏸️ " + $n)
+          or .name == ("▶️ " + $n)
+          or .name == ("❓︎ " + $n)
+        ) | .tab_id] | .[0] // empty
+      ' 2>/dev/null
 }
 
 # Pick a writable pane on the given tab. Prefer the tab's own focused pane
@@ -139,6 +155,18 @@ _session_file() {
   printf '%s/%s.info' "$SESSIONS_DIR" "$1"
 }
 
+# Sidecar for the per-role loadout name (#140). cmd_register writes it from
+# its --loadout CLI arg; _ensure_session_file reads from it during a re-seed.
+# Separate from the main session file so it survives cmd_unregister's `rm -f`
+# of <role>.info — by definition, we re-seed because the .info file was just
+# wiped, so we can't read the previous LOADOUT= line out of it. The bare env
+# var ($TASK_FORCE_LOADOUT) isn't available either: hook subshells inherit
+# Claude Code's env, not task-work's launch-time exports — that fallback to
+# the literal string "unknown" was Bug C in #140.
+_loadout_sidecar() {
+  printf '%s/%s.loadout' "$SESSIONS_DIR" "$1"
+}
+
 _inbox_dir()    { printf '%s/%s/inbox'     "$MAILBOX_DIR" "$1"; }
 _processed_dir(){ printf '%s/%s/processed' "$MAILBOX_DIR" "$1"; }
 
@@ -163,13 +191,38 @@ _ensure_session_file() {
     tab_id=$(_zellij_tab_id_by_name "$ZELLIJ_TAB" || true)
   fi
 
+  # LOADOUT recovery (#140 Bug C): the hook subshell doesn't inherit
+  # $TASK_FORCE_LOADOUT, so the previous fallback to "${TASK_FORCE_LOADOUT:-unknown}"
+  # wrote the literal string "unknown" on every re-seed. Read from the
+  # sidecar that cmd_register persisted at register time. Fall back to the
+  # env var only as a last resort (covers test harnesses that exercise
+  # _ensure_session_file without going through register first).
+  local loadout=""
+  local sidecar
+  sidecar=$(_loadout_sidecar "$TASK_FORCE_ROLE")
+  # TOCTOU-safe read (#150 review): the sidecar can be unlinked by a
+  # concurrent cmd_unregister between any pre-check and the read — the
+  # unregister cascade documented in #140 is exactly that race window.
+  # `cat … 2>/dev/null || true` collapses three failure modes into "empty
+  # string" without tripping set -e: file absent, file unlinked mid-read,
+  # file unreadable. The env-var fallback below handles the empty case.
+  #
+  # Why not the obvious `loadout=$(<"$sidecar" || true)` the reviewer
+  # suggested: bash's `<file` is a command-substitution-only shorthand, not
+  # a real command, and `<file || true` short-circuits such that the read
+  # never returns the file's contents — so the sidecar value would be lost
+  # even on the happy path. Verified with `bash -c 'echo x>/tmp/f; v=$(</tmp/f || true); echo "[$v]"'`
+  # printing `[]`. cat is the unambiguous form.
+  loadout=$(cat "$sidecar" 2>/dev/null || true)
+  [[ -n "$loadout" ]] || loadout="${TASK_FORCE_LOADOUT:-unknown}"
+
   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}" "${TASK_FORCE_LOADOUT:-unknown}"
+      "${TASK_FORCE_AGENT:-claude}" "$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
@@ -181,7 +234,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"
+  _log "ensure_session: re-seeded $TASK_FORCE_ROLE (file was missing) tab_id=$tab_id loadout=$loadout"
 }
 
 # Update STATE=… and LAST_HEARTBEAT=… on the current session file.
@@ -332,6 +385,30 @@ 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:
+  #
+  #   sidecar-first (this order):
+  #     pre-kill state:  .loadout=claude-gh, .info absent
+  #     re-seed reads:   .loadout → LOADOUT=claude-gh 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
+  #     re-seed reads:   no sidecar, hook subshell has no $TASK_FORCE_LOADOUT
+  #                      → LOADOUT=unknown ✗
+  #
+  # 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
+  # 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.
+  if [[ -n "$loadout" ]]; then
+    printf '%s' "$loadout" | _atomic_write "$(_loadout_sidecar "$role")"
+  fi
+
   {
     printf 'ROLE=%s\nTAB=%s\nTAB_ID=%s\nREPO=%s\nSTATE=idle\nLAST_HEARTBEAT=%s\nAGENT=%s\nLOADOUT=%s\n' \
       "$role" "$tab" "$tab_id" "$repo" "$(_now)" "$agent" "$loadout"
@@ -348,6 +425,7 @@ cmd_register() {
   } | _atomic_write "$f"
 
   mkdir -p "$(_inbox_dir "$role")" "$(_processed_dir "$role")"
+
   _log "register role=$role tab=$tab tab_id=$tab_id agent=$agent loadout=$loadout"
 
   # First paint of the idle prefix without waiting for a Stop hook cycle (#95).
@@ -374,7 +452,7 @@ cmd_unregister() {
   # in that case. We only filter when both (a) stdin is not a terminal and
   # (b) a parseable `reason` is present.
   if [[ ! -t 0 ]]; then
-    local payload reason
+    local payload reason payload_oneline
     payload=$(cat 2>/dev/null || true)
     if [[ -n "$payload" ]] && command -v jq >/dev/null 2>&1; then
       reason=$(printf '%s' "$payload" | jq -r '.reason // empty' 2>/dev/null || true)
@@ -385,11 +463,25 @@ cmd_unregister() {
           ;;
       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.
+    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"
+    fi
   fi
 
   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).
+  rm -f "$(_loadout_sidecar "$TASK_FORCE_ROLE")"
   _log "unregister role=$TASK_FORCE_ROLE"
 }
 

claude-jira/bin/radio

@@ -111,13 +111,29 @@ _strip_tab_prefix() {
 # Resolve a zellij tab's stable id from its visible name. Empty stdout means
 # "lookup failed" (zellij not reachable, jq absent, name not found) — callers
 # must treat that as a hard skip so we never operate on the wrong tab (#102).
+#
+# Emoji-prefix awareness (#140): _rename_tab paints the visible tab name to
+# "⏸️ <slug>" / "▶️ <slug>" / "❓︎ <slug>" reflecting STATE. Callers always
+# query by the bare slug ($ZELLIJ_TAB / cmd_register's --tab arg) — that's
+# the stable identifier captured at pane creation and never mutates. So a
+# literal name match against the painted name misses on every flip past
+# idle. We compare against both the bare name AND any of the three emoji-
+# prefixed variants so the lookup survives _rename_tab's repaints. This is
+# the symmetric fix for #117 (cmd_register) AND the same loss in
+# _ensure_session_file's re-seed path (Bug B in #140).
 _zellij_tab_id_by_name() {
   local target="$1"
   [[ -n "$target" ]] || return 0
   command -v jq >/dev/null 2>&1 || return 0
   zellij action list-tabs --json 2>/dev/null \
-    | jq -r --arg n "$target" \
-        '[.[] | select(.name == $n) | .tab_id] | .[0] // empty' 2>/dev/null
+    | jq -r --arg n "$target" '
+        [.[] | select(
+          .name == $n
+          or .name == ("⏸️ " + $n)
+          or .name == ("▶️ " + $n)
+          or .name == ("❓︎ " + $n)
+        ) | .tab_id] | .[0] // empty
+      ' 2>/dev/null
 }
 
 # Pick a writable pane on the given tab. Prefer the tab's own focused pane
@@ -139,6 +155,18 @@ _session_file() {
   printf '%s/%s.info' "$SESSIONS_DIR" "$1"
 }
 
+# Sidecar for the per-role loadout name (#140). cmd_register writes it from
+# its --loadout CLI arg; _ensure_session_file reads from it during a re-seed.
+# Separate from the main session file so it survives cmd_unregister's `rm -f`
+# of <role>.info — by definition, we re-seed because the .info file was just
+# wiped, so we can't read the previous LOADOUT= line out of it. The bare env
+# var ($TASK_FORCE_LOADOUT) isn't available either: hook subshells inherit
+# Claude Code's env, not task-work's launch-time exports — that fallback to
+# the literal string "unknown" was Bug C in #140.
+_loadout_sidecar() {
+  printf '%s/%s.loadout' "$SESSIONS_DIR" "$1"
+}
+
 _inbox_dir()    { printf '%s/%s/inbox'     "$MAILBOX_DIR" "$1"; }
 _processed_dir(){ printf '%s/%s/processed' "$MAILBOX_DIR" "$1"; }
 
@@ -163,13 +191,38 @@ _ensure_session_file() {
     tab_id=$(_zellij_tab_id_by_name "$ZELLIJ_TAB" || true)
   fi
 
+  # LOADOUT recovery (#140 Bug C): the hook subshell doesn't inherit
+  # $TASK_FORCE_LOADOUT, so the previous fallback to "${TASK_FORCE_LOADOUT:-unknown}"
+  # wrote the literal string "unknown" on every re-seed. Read from the
+  # sidecar that cmd_register persisted at register time. Fall back to the
+  # env var only as a last resort (covers test harnesses that exercise
+  # _ensure_session_file without going through register first).
+  local loadout=""
+  local sidecar
+  sidecar=$(_loadout_sidecar "$TASK_FORCE_ROLE")
+  # TOCTOU-safe read (#150 review): the sidecar can be unlinked by a
+  # concurrent cmd_unregister between any pre-check and the read — the
+  # unregister cascade documented in #140 is exactly that race window.
+  # `cat … 2>/dev/null || true` collapses three failure modes into "empty
+  # string" without tripping set -e: file absent, file unlinked mid-read,
+  # file unreadable. The env-var fallback below handles the empty case.
+  #
+  # Why not the obvious `loadout=$(<"$sidecar" || true)` the reviewer
+  # suggested: bash's `<file` is a command-substitution-only shorthand, not
+  # a real command, and `<file || true` short-circuits such that the read
+  # never returns the file's contents — so the sidecar value would be lost
+  # even on the happy path. Verified with `bash -c 'echo x>/tmp/f; v=$(</tmp/f || true); echo "[$v]"'`
+  # printing `[]`. cat is the unambiguous form.
+  loadout=$(cat "$sidecar" 2>/dev/null || true)
+  [[ -n "$loadout" ]] || loadout="${TASK_FORCE_LOADOUT:-unknown}"
+
   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}" "${TASK_FORCE_LOADOUT:-unknown}"
+      "${TASK_FORCE_AGENT:-claude}" "$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
@@ -181,7 +234,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"
+  _log "ensure_session: re-seeded $TASK_FORCE_ROLE (file was missing) tab_id=$tab_id loadout=$loadout"
 }
 
 # Update STATE=… and LAST_HEARTBEAT=… on the current session file.
@@ -332,6 +385,30 @@ 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:
+  #
+  #   sidecar-first (this order):
+  #     pre-kill state:  .loadout=claude-gh, .info absent
+  #     re-seed reads:   .loadout → LOADOUT=claude-gh 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
+  #     re-seed reads:   no sidecar, hook subshell has no $TASK_FORCE_LOADOUT
+  #                      → LOADOUT=unknown ✗
+  #
+  # 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
+  # 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.
+  if [[ -n "$loadout" ]]; then
+    printf '%s' "$loadout" | _atomic_write "$(_loadout_sidecar "$role")"
+  fi
+
   {
     printf 'ROLE=%s\nTAB=%s\nTAB_ID=%s\nREPO=%s\nSTATE=idle\nLAST_HEARTBEAT=%s\nAGENT=%s\nLOADOUT=%s\n' \
       "$role" "$tab" "$tab_id" "$repo" "$(_now)" "$agent" "$loadout"
@@ -348,6 +425,7 @@ cmd_register() {
   } | _atomic_write "$f"
 
   mkdir -p "$(_inbox_dir "$role")" "$(_processed_dir "$role")"
+
   _log "register role=$role tab=$tab tab_id=$tab_id agent=$agent loadout=$loadout"
 
   # First paint of the idle prefix without waiting for a Stop hook cycle (#95).
@@ -374,7 +452,7 @@ cmd_unregister() {
   # in that case. We only filter when both (a) stdin is not a terminal and
   # (b) a parseable `reason` is present.
   if [[ ! -t 0 ]]; then
-    local payload reason
+    local payload reason payload_oneline
     payload=$(cat 2>/dev/null || true)
     if [[ -n "$payload" ]] && command -v jq >/dev/null 2>&1; then
       reason=$(printf '%s' "$payload" | jq -r '.reason // empty' 2>/dev/null || true)
@@ -385,11 +463,25 @@ cmd_unregister() {
           ;;
       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.
+    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"
+    fi
   fi
 
   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).
+  rm -f "$(_loadout_sidecar "$TASK_FORCE_ROLE")"
   _log "unregister role=$TASK_FORCE_ROLE"
 }