Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
shellcheck -x \
install.sh task-init run_tests.sh \
bin/* lib/*.sh tools/*.sh \
tests/helpers/*.bash \
tests/helpers/*.bash tests/setup_suite.bash \
*/install.sh */bin/*

drift-check:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),

### Fixed

- **`./run_tests.sh` no longer wipes the live radio session of whoever ran it, 57 times per run.** `tests/task_done.bats` and `tests/task_done_dispatcher.bats` built their `setup()` without any radio-home helper, so `$TASK_FORCE_HOME` was never overridden and `radio`/`task-done` resolved their mailbox root to the developer's real `~/.task-force`. `$TASK_FORCE_ROLE` is inherited from the tab, so the destructive command those suites exercise — `radio unregister --manual`, which wipes a session file unconditionally by design (#198) — landed on the *runner's own session file*: 51 wipes from `task_done.bats` plus 6 from `task_done_dispatcher.bats`, matching the measured full-suite delta of 57 exactly. An agent worker running the suite (which the pre-PR checklist requires) went unaddressable mid-run — `radio send --to <that worker>` answering *"no session … nobody is listening"* while the agent was plainly alive — and the behaviour was initially misdiagnosed as a regression in #187. It also contaminated this subsystem's own failure data: an unknown share of the 3,165 silent wipes the 2026-09-14 audit attributed to `cmd_unregister` were these bursts of exactly 57, repeating one full-suite run apart, which is precisely the log #191's runbook asks people to trust. Isolation is now the default rather than something each file has to remember: a new `tests/setup_suite.bash` runs once per bats invocation and exports a scratch `$TASK_FORCE_HOME` for the whole run, so a suite that forgets the helper degrades to "isolated anyway" — and because bats resolves `setup_suite.bash` from the folder of the first test file, a bare `bats tests/foo.bats` gets it as well as `./run_tests.sh`. Backing that up, `tests/helpers/common.bash` — which every suite loads — now aborts the run with an explanation if `$TASK_FORCE_HOME` is unset or points at the real `~/.task-force` (skipped only on bats' pre-`setup_suite` test-gathering pass, where an unset value proves nothing). Both suites also got the per-test `setup_task_force_home` they were missing, and `teardown_all` now removes `$TASK_FORCE_HOME` only when the test created it, so a per-test teardown can't delete the run-scoped one. New `tests/radio_home_isolation.bats` is the standing guard: it drives real bats child-runs of both suites under a throwaway `$HOME` and asserts nothing appears under `$HOME/.task-force` — `sessions/`, `mailbox/` and `log` together, with the log pre-created so the check is "nothing was appended", not just "nothing was created" — plus that a subverted `$TASK_FORCE_HOME` fails loudly instead of writing to the live mailbox. Verified: full 1,000-test suite, `./run_tests.sh task_done` and `./run_tests.sh task_done_dispatcher` all now produce delta 0 against `grep -c 'unregister role=' ~/.task-force/radio/log`. Tests only — **no `task-init` re-run needed**, no installed artifact changed. (#203)

- **The Stop hook no longer strands a message that arrives during the drain turn — the loop-breaker compares the unread id set, not the `stop_hook_active` flag.** `cmd_stop_hook` blocks the stop when the inbox is non-empty so the agent drains it (#163), and guarded against an infinite continuation by allowing any `Stop` whose payload carried `stop_hook_active: true`. That test is a boolean, not an inbox: a message queued *during* the forced-continuation turn (the agent is `busy`, so `cmd_send` queues with no wake) hit the same "already had your continuation" branch and the role went idle with unread mail in its own mailbox. For an idle `--auto` worker awaiting `approved-and-merged` that is terminal — no human prompts it, so #164's prompt-hook injection never fires, and no `SessionStart` follows, so #168's drain-on-register never fires either. Caught live on 2026-09-14 (PR #195 merged → `approved-and-merged` queued → `stop-hook: stop_hook_active=true — allowing stop (1 still unread)`), with 9 occurrences in the 2026-05-24 → 2026-09-14 log — every one a handoff that silently didn't happen, recoverable only by a manual re-send that duplicated the message. The hook now records the ids it blocked on in a `BLOCKED_IDS=` field of the session file and, on the next `Stop` with `stop_hook_active: true`, re-reads the inbox: an id that wasn't in that set is a genuine new arrival and earns one more block (the set is reset to the current ids), while an unchanged or shrunken set means the agent declined to drain and the stop is allowed — so the infinite-loop protection is preserved exactly. The record is cleared when the inbox drains, and it lives in the `.info` rather than a sidecar on purpose: post-#188 sidecars survive `unregister` by design, and a set outliving its session could suppress a legitimate block for a later, unrelated session of the same role. With no recorded set to compare against (a wiped or re-seeded session file) the hook falls back to the old flag-only behavior, which is also what keeps a session-file-less role from blocking on every `Stop`. *Upgrading: no re-run of `task-init` needed — `radio` is installed as a symlink to the single canonical script, so the fix is live as soon as this commit is on disk. The workflow steering templates gained a paragraph describing the new behavior; an already-installed `.claude/<loadout>-workflow.md` is yours to edit and is not overwritten.* (#197)

- **`radio unregister` typed at a terminal no longer wipes the session — `-t 0` is demoted from wipe-authority to output channel.** #187 made the command non-destructive by default, but gated the entire guard on `[[ "$manual" != true && ! -t 0 ]]`: with a terminal on stdin it was bypassed wholesale, and the `.info` was `rm -f`'d with neither a `skipping` nor a `proceeding` line in `~/.task-force/radio/log` — the last silent-wipe path in the command, and the reason ~399 wipes in the 2026-09-14 log explained nothing. The tty test was a defensible stand-in for *a human meant this* before `--manual` existed; now that the explicit opt-in is there, nothing decides a wipe except `--manual` or a named real-exit `reason`, and `-t 0` only decides whether a skip is **also announced on stderr**. Simply deleting the test would have been wrong in the other direction — it trades a silent wipe for a silent no-op, since the "use `--manual` to force" guidance goes only to the log, which is not where a person is looking — so an interactive invocation now prints `radio unregister: refusing to wipe <role> without an explicit signal … re-run with --manual to force.` on stderr. Hook-invoked (non-tty) calls stay quiet as before; the skip line records which stdin shape it saw. **Behaviour change:** bare `radio unregister` is no longer cleanup. Scripts that relied on it tearing the session down must pass `--manual` (which still wipes from any stdin shape, so `task-done` is unaffected). Removing the outer gate cannot reintroduce a blocking `cat` on a terminal — `_read_hook_payload` carries its own `! -t 0` test, which was the original reason the gate existed. *Upgrading: re-run `task-init <loadout>` to pick up the refreshed workflow doc; the radio hooks and `settings.json` are unchanged, and `bin/radio` itself ships via the `install.sh` symlink.* (#198)
Expand Down
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,17 @@ git submodule update --init --recursive # first time only
./run_tests.sh task_done # run a single suite
```

**Radio-home isolation.** The suite drives destructive radio commands —
`task-done` calls `radio unregister --manual`, which wipes a session file
unconditionally. `tests/setup_suite.bash` therefore points every run at a
throwaway `$TASK_FORCE_HOME`, so no test can reach the `~/.task-force` of
whoever ran it (#203; before the fix a full run wiped the runner's own live
session 57 times, which made an agent worker unaddressable mid-run). You get
that for free from `./run_tests.sh` *and* from a bare `bats tests/foo.bats`.
If `$TASK_FORCE_HOME` is missing or aimed back at the real home, loading
`tests/helpers/common.bash` aborts the run with an explanation rather than
writing to the live mailbox.

<details>
<summary><b>Test suites</b> (click to expand)</summary>

Expand All @@ -640,12 +651,15 @@ git submodule update --init --recursive # first time only
| `kiro_local_task_init.bats` | `kiro-local/bin/task-init` — `tasks/` scaffolding, `.kiro/steering/local-workflow.md`, agents |
| `task_board.bats` | Shared `task-board` script — frontmatter parsing, sidecar overlay, `_board.md` regen |
| `task_done.bats` | `task-done` across combos — cleanup, PR, guards |
| `radio_home_isolation.bats` | The suite's own radio-home isolation — nothing lands under `$HOME/.task-force` |

</details>

Infrastructure:

- `tests/helpers/common.bash` — `setup_repo`, `setup_stubs`, `setup_worktree`, `teardown_all`, `assert_stub_called`
- `tests/setup_suite.bash` — runs once per bats invocation; exports the run-scoped `$TASK_FORCE_HOME`
- `tests/helpers/radio_home.bash` — `task_force_home_is_isolated`, `require_isolated_task_force_home`
- `tests/helpers/stubs/` — fakes for `zellij`, `gh`, `kiro-cli`, `claude`; every call lands in `$STUB_CALLS_DIR/*.calls`
- `tests/libs/` — bats-core, bats-support, bats-assert as git submodules

Expand Down Expand Up @@ -673,6 +687,9 @@ load helpers/common
setup() { setup_repo; setup_stubs; cd "$MAIN_REPO"; }
teardown() { teardown_all; }

# Anything that shells out to radio or task-done also wants a per-test mailbox:
# setup() { setup_repo; setup_stubs; setup_task_force_home; cd "$MAIN_REPO"; }

@test "description" {
run "$MY_SCRIPT" arg
assert_success
Expand Down
21 changes: 20 additions & 1 deletion tests/helpers/common.bash
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@
# shellcheck disable=SC2034 # path vars are used by the .bats files that load this helper

REPO_ROOT_REAL="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"

# Radio-home isolation guard (#203). Every suite loads this helper, so this is
# the chokepoint that catches a file (or a bats invocation that never picked up
# tests/setup_suite.bash) about to drive the developer's live ~/.task-force.
# shellcheck source=tests/helpers/radio_home.bash
source "$REPO_ROOT_REAL/tests/helpers/radio_home.bash"
# bats sources every file once with BATS_TEST_NAME=source to gather test names,
# and that pass runs *before* setup_suite — so an unset $TASK_FORCE_HOME proves
# nothing there. A home that is set but points at the real one is wrong on any
# pass.
if [[ -n "${TASK_FORCE_HOME:-}" || "${BATS_TEST_NAME:-}" != source ]]; then
require_isolated_task_force_home || exit 1
fi

KIRO_TASK_WORK="$REPO_ROOT_REAL/kiro-notion/bin/task-work"
JIRA_TASK_WORK="$REPO_ROOT_REAL/claude-jira/bin/task-work"
KIRO_TASK_DONE="$REPO_ROOT_REAL/kiro-notion/bin/task-done"
Expand Down Expand Up @@ -133,6 +147,9 @@ reset_radio_env() {
setup_task_force_home() {
TASK_FORCE_HOME=$(mktemp -d)
export TASK_FORCE_HOME
# Flag ownership so teardown_all removes only a home this test created, and
# never the run-scoped one tests/setup_suite.bash hands down (#203).
TASK_FORCE_HOME_OWNED=1
reset_radio_env
}

Expand Down Expand Up @@ -161,7 +178,9 @@ teardown_all() {
[[ -z "${WORKTREE_BASE:-}" ]] || rm -rf "$WORKTREE_BASE"
[[ -z "${STUB_BIN:-}" ]] || rm -rf "$STUB_BIN"
[[ -z "${STUB_CALLS_DIR:-}" ]] || rm -rf "$STUB_CALLS_DIR"
[[ -z "${TASK_FORCE_HOME:-}" ]] || rm -rf "$TASK_FORCE_HOME"
if [[ -n "${TASK_FORCE_HOME_OWNED:-}" && -n "${TASK_FORCE_HOME:-}" ]]; then
rm -rf "$TASK_FORCE_HOME"
fi
}

# Seed the zellij stub with a JSON snapshot of tabs / panes for the radio
Expand Down
47 changes: 47 additions & 0 deletions tests/helpers/radio_home.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env bash
# Radio-home isolation guard (#203).
#
# radio, task-done and friends resolve their mailbox root as
# `${TASK_FORCE_HOME:-$HOME/.task-force}`. A test file that never overrides
# $TASK_FORCE_HOME therefore drives the *developer's live radio home* — and
# since $TASK_FORCE_ROLE is inherited from the tab the suite is run from, the
# damage lands on the runner's own session file. task_done.bats and
# task_done_dispatcher.bats did exactly that: 57 silent
# `radio unregister --manual` wipes per `./run_tests.sh`, which made an agent
# worker unaddressable mid-run and polluted the radio log that #191's runbook
# asks people to trust.
#
# Two layers stop that recurring:
# * tests/setup_suite.bash exports a run-scoped tempdir when the caller
# hasn't supplied one, so isolation is the default rather than something
# each file has to remember;
# * this guard, invoked when tests/helpers/common.bash loads, fails loudly
# if $TASK_FORCE_HOME is missing or points back at the real home.
#
# Sourced by both tests/setup_suite.bash and tests/helpers/common.bash.

# True when $1 is a usable, isolated radio home: non-empty, and neither the
# real ~/.task-force nor anything beneath it.
task_force_home_is_isolated() {
local home="${1:-}" real="${HOME%/}/.task-force"
[[ -n "$home" ]] || return 1
[[ "$home" != "$real" && "$home" != "$real"/* ]]
}

# Fail loudly when the ambient $TASK_FORCE_HOME would point tests at the
# developer's live mailbox. Returns 1 (callers decide whether to exit).
require_isolated_task_force_home() {
task_force_home_is_isolated "${TASK_FORCE_HOME:-}" && return 0
cat >&2 <<MSG
ERROR: refusing to run tests against the real radio home (#203).

\$TASK_FORCE_HOME = ${TASK_FORCE_HOME:-<unset>}
\$HOME/.task-force = ${HOME%/}/.task-force

Tests invoke 'radio unregister --manual' and other destructive commands; with
no isolated \$TASK_FORCE_HOME they would wipe the live session of whoever ran
them. Run the suite via ./run_tests.sh (or any bats invocation that picks up
tests/setup_suite.bash), or export TASK_FORCE_HOME to a scratch directory.
MSG
return 1
}
150 changes: 150 additions & 0 deletions tests/radio_home_isolation.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
#!/usr/bin/env bats
# Radio-home isolation for the test suite itself (#203).
#
# `task-done` calls `radio unregister --manual`, which wipes a session file
# unconditionally by design (#198). Two suites used to run that against the
# *real* ~/.task-force, so `./run_tests.sh` destroyed the live session of
# whoever ran it 57 times per run: an agent worker that ran the suite — which
# the pre-PR checklist requires — went unaddressable mid-run, and the radio log
# #191's runbook asks people to trust filled with wipes no code path explained.
#
# These tests are the standing guard: they drive real bats invocations under a
# throwaway $HOME and assert nothing lands under it.

bats_load_library bats-support
bats_load_library bats-assert

load helpers/common

BATS_BIN="$REPO_ROOT_REAL/tests/libs/bats-core/bin/bats"

setup() {
FAKE_HOME=$(mktemp -d)
export FAKE_HOME
}

teardown() {
[[ -z "${FAKE_HOME:-}" ]] || rm -rf "$FAKE_HOME"
}

# Run one bats suite in a child process with $HOME redirected at $FAKE_HOME and
# $TASK_FORCE_HOME deliberately unset, i.e. exactly the shape that leaked.
# Extra args are passed through to bats (use --filter to keep it quick).
run_suite_under_fake_home() {
local suite="$1"; shift
run env -u TASK_FORCE_HOME \
HOME="$FAKE_HOME" \
BATS_LIB_PATH="$REPO_ROOT_REAL/tests/libs" \
"$BATS_BIN" "$REPO_ROOT_REAL/tests/$suite.bats" "$@"
}

# Everything radio writes lives under one of these three.
assert_real_radio_home_untouched() {
assert [ ! -e "$FAKE_HOME/.task-force/radio/log" ]
assert [ ! -e "$FAKE_HOME/.task-force/radio/sessions" ]
assert [ ! -e "$FAKE_HOME/.task-force/radio/mailbox" ]
assert [ ! -e "$FAKE_HOME/.task-force" ]
}

# ---------------------------------------------------------------------------
# The two suites that leaked
# ---------------------------------------------------------------------------

# The whole of task_done.bats is ~50 tests; the radio-touching subset is what
# leaked, and filtering keeps this suite cheap enough to run every time.
TASK_DONE_RADIO_TESTS='unregister|radio|mailbox'

@test "task_done suite writes nothing under \$HOME/.task-force" {
run_suite_under_fake_home task_done --filter "$TASK_DONE_RADIO_TESTS"
assert_success
assert_real_radio_home_untouched
}

@test "task_done_dispatcher suite writes nothing under \$HOME/.task-force" {
run_suite_under_fake_home task_done_dispatcher
assert_success
assert_real_radio_home_untouched
}

@test "no unregister lands in the real radio log while task_done runs" {
# Pre-create the log so the check is "nothing was appended", not merely
# "nothing created the tree" — the delta the issue measured.
mkdir -p "$FAKE_HOME/.task-force/radio"
: > "$FAKE_HOME/.task-force/radio/log"

run_suite_under_fake_home task_done --filter "$TASK_DONE_RADIO_TESTS"
assert_success

run grep -c 'unregister role=' "$FAKE_HOME/.task-force/radio/log"
assert_output "0"
}

# ---------------------------------------------------------------------------
# Isolation is the default, not something each file has to remember
# ---------------------------------------------------------------------------

@test "setup_suite hands every run an isolated TASK_FORCE_HOME" {
refute [ -z "${TASK_FORCE_HOME:-}" ]
run task_force_home_is_isolated "$TASK_FORCE_HOME"
assert_success
}

@test "a suite that never calls setup_task_force_home is still isolated" {
# Stand up a throwaway bats tree carrying only setup_suite.bash and its
# helper, plus a probe file whose setup() does nothing at all. The probe has
# to come out isolated purely on the strength of the suite-level default.
local dir="$FAKE_HOME/probe"
mkdir -p "$dir/helpers"
cp "$REPO_ROOT_REAL/tests/setup_suite.bash" "$dir/"
cp "$REPO_ROOT_REAL/tests/helpers/radio_home.bash" "$dir/helpers/"
cat > "$dir/probe.bats" <<'PROBE'
@test "probe records its radio home" {
printf '%s' "${TASK_FORCE_HOME:-}" > "$PROBE_OUT"
}
PROBE

run env -u TASK_FORCE_HOME HOME="$FAKE_HOME" PROBE_OUT="$FAKE_HOME/probe-home" \
"$BATS_BIN" "$dir/probe.bats"
assert_success

local seen
seen=$(cat "$FAKE_HOME/probe-home")
refute [ -z "$seen" ]
run task_force_home_is_isolated "$seen"
assert_success
assert_real_radio_home_untouched
}

# ---------------------------------------------------------------------------
# …and when it is subverted, it fails loudly instead of writing to the mailbox
# ---------------------------------------------------------------------------

@test "pointing TASK_FORCE_HOME at the real home fails the run loudly" {
run env HOME="$FAKE_HOME" \
TASK_FORCE_HOME="$FAKE_HOME/.task-force" \
BATS_LIB_PATH="$REPO_ROOT_REAL/tests/libs" \
"$BATS_BIN" "$REPO_ROOT_REAL/tests/task_done.bats" --filter 'unregisters'
assert_failure
assert_output --partial "refusing to run tests against the real radio home"
assert_real_radio_home_untouched
}

@test "a subdirectory of the real home is refused too" {
run task_force_home_is_isolated "$HOME/.task-force/radio"
assert_failure
}

@test "an unset TASK_FORCE_HOME is refused when loading the common helper" {
# Simulates a bats invocation that never picked up tests/setup_suite.bash.
run env -u TASK_FORCE_HOME bash -c \
"source '$REPO_ROOT_REAL/tests/helpers/common.bash'"
assert_failure
assert_output --partial "refusing to run tests against the real radio home"
}

@test "an isolated TASK_FORCE_HOME supplied by the caller is honoured" {
run env TASK_FORCE_HOME="$FAKE_HOME/scratch" bash -c \
"source '$REPO_ROOT_REAL/tests/helpers/common.bash' && printf 'ok'"
assert_success
assert_output "ok"
}
Loading
Loading