From dd6dcb5c26b4186ece8b82b74f8c8500e39b2f14 Mon Sep 17 00:00:00 2001 From: Younggi Choi <74581798+choiyounggi@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:40:19 +0900 Subject: [PATCH 1/2] fix(orchestrate): reject invalid launch-session.sh inputs before creating a tmux session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three fail-fast guards for issue #123's caller-bug shapes, all firing before any tmux call: a session name outside [A-Za-z0-9_-]+ (a zsh word-splitting bug can hand this a space-containing name), a worktree argument that is not a git work-tree root (a shared parent directory or a non-root subdirectory), and an LO_STATUS_DIR/LO_TASK_ID pair with exactly one set (set-but-empty counts as unset — the incident's exact shape). tests/launch-session.bats: adds a real git-worktree fixture, rewrites existing worktree-arg invocations to use it, and adds normal/error/boundary coverage per guard. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011PK2GZqRmjjctDv4A9X7vp --- skills/orchestrate/scripts/launch-session.sh | 57 +++++++- tests/launch-session.bats | 130 ++++++++++++++----- 2 files changed, 152 insertions(+), 35 deletions(-) diff --git a/skills/orchestrate/scripts/launch-session.sh b/skills/orchestrate/scripts/launch-session.sh index bfb419c..d627b84 100755 --- a/skills/orchestrate/scripts/launch-session.sh +++ b/skills/orchestrate/scripts/launch-session.sh @@ -24,12 +24,14 @@ # # exit 0 launched and the prompt is confirmed submitted # exit 1 wrong argument count -# exit 2 invalid permission mode +# exit 2 invalid permission mode, invalid worker model, invalid session +# name, a worktree argument that is not a git work-tree root, or +# an LO_STATUS_DIR/LO_TASK_ID pair with exactly one set (issue #123) # exit 4 the REPL never became ready (prints the last screen) # exit 5 the prompt was sent but submission could NOT be confirmed — the # session is alive and may hold an unsubmitted prompt (prints the # last screen). Distinct from 4: there the CLI never came up. -# exit 127 tmux or claude not found +# exit 127 tmux, git, or claude not found # # env (also test hooks): # LO_RUN_ID suffix making the session name unique per run @@ -49,8 +51,11 @@ # prompt is confirmed submitted — so dead/stalled-worker # detection covers the session from launch, not from its # first self-report. A seeding failure warns on stderr -# and never changes the exit code. Either var unset: -# exactly the previous behavior, no new output. +# and never changes the exit code. Both unset: exactly +# the previous behavior, no new output. Exactly one set — +# including set-but-empty, e.g. LO_TASK_ID="" — is a +# caller bug (issue #123), rejected at exit 2 before any +# tmux session is touched. set -eu # NOT named TMUX: that is tmux's OWN variable (the server socket). Assigning it # here would overwrite the caller's exported value, and every tmux child would @@ -68,6 +73,15 @@ session="$1"; wt="$2"; perm="$3"; prompt="$4" # sets LO_RUN_ID once per run and reuses the resulting name for later send-keys. [ -n "${LO_RUN_ID:-}" ] && session="${session}-${LO_RUN_ID}" +# whitelist the session name — it is interpolated into tmux commands (new-session, +# has-session, send-keys, capture-pane) and matched by safe-cleanup.sh's sweep, so +# reject anything outside the safe allowlist before it reaches any of them. Catches +# both a bad $1 and a bad LO_RUN_ID suffix (issue #123: a zsh word-splitting bug +# handed this a space-containing name and it was silently accepted). +case "$session" in + ''|*[!A-Za-z0-9_-]*) echo "launch-session: invalid session name '$session'" >&2; exit 2 ;; +esac + # whitelist the permission mode — it is interpolated into a shell command sent to # the pane, so reject anything unexpected (injection guard). case "$perm" in @@ -89,6 +103,41 @@ fi # tmux/claude. Lets the orchestrator (and tests) learn the exact name. if [ -n "${LO_DRY_RUN:-}" ]; then echo "session=$session"; exit 0; fi +# LO_STATUS_DIR / LO_TASK_ID must be set together or not at all. A caller bug +# that sets one and leaves the other empty (issue #123: LO_TASK_ID="") used to +# silently skip status pre-seed below with no error. Set-but-empty counts as +# unset here — that is exactly the incident shape — so reject before any tmux +# session exists. +sd="${LO_STATUS_DIR:-}"; ti="${LO_TASK_ID:-}" +if { [ -n "$sd" ] && [ -z "$ti" ]; } || { [ -z "$sd" ] && [ -n "$ti" ]; }; then + echo "launch-session: LO_STATUS_DIR and LO_TASK_ID must be set together (exactly one is set: LO_STATUS_DIR='$sd' LO_TASK_ID='$ti')" >&2 + exit 2 +fi + +# The worktree argument must be a git work-tree ROOT, not merely a directory +# that exists. Rejects a shared parent directory (issue #123: a coordinator bug +# handed this the worktrees' parent instead of the task's own worktree) and a +# subdirectory of a real checkout, either of which used to be handed straight +# to tmux `-c` with no error. Compare PHYSICAL paths: git already resolves +# symlinks in its own output, and macOS resolves $TMPDIR through a symlink, so +# a naive string compare against the raw argument false-rejects a valid root. +GIT_BIN="$(command -v git 2>/dev/null || true)" +[ -n "$GIT_BIN" ] || { echo "launch-session: git not found" >&2; exit 127; } +if [ ! -d "$wt" ]; then + echo "launch-session: worktree '$wt' does not exist" >&2 + exit 2 +fi +if ! "$GIT_BIN" -C "$wt" rev-parse --is-inside-work-tree >/dev/null 2>&1; then + echo "launch-session: worktree '$wt' is not a git work-tree" >&2 + exit 2 +fi +wt_root=$("$GIT_BIN" -C "$wt" rev-parse --show-toplevel) +wt_physical=$(cd "$wt" && pwd -P) +if [ "$wt_root" != "$wt_physical" ]; then + echo "launch-session: worktree '$wt' is not a git work-tree root (root is '$wt_root')" >&2 + exit 2 +fi + # Pre-seed the status record so dead/stalled-worker detection covers this # worker from launch time, not from its first self-report (plan_ready). # Active only when the orchestrator sets BOTH LO_STATUS_DIR and LO_TASK_ID. diff --git a/tests/launch-session.bats b/tests/launch-session.bats index 5185021..c28dea6 100644 --- a/tests/launch-session.bats +++ b/tests/launch-session.bats @@ -16,10 +16,15 @@ setup() { LO_READY_TIMEOUT LO_READY_INTERVAL LO_READY_EXTRA LO_TRUST_EXTRA \ LO_SUBMIT_TIMEOUT LO_SUBMIT_INTERVAL LO_STATUS_DIR LO_TASK_ID \ DEV_LOOP_WORKER_MODEL GROUNDWORK_ESCALATION_DIR GROUNDWORK_TASK_ID + + # A valid git work-tree root, for every test that needs the worktree guard + # (issue #123) to pass through to the behavior under test. Dry-run-only tests + # may keep a fake path instead — the worktree guard runs after the dry-run exit. + WT="$BATS_TEST_TMPDIR/wt"; mkdir -p "$WT"; git -C "$WT" init -q } @test "rejects an invalid permission mode (injection guard)" { - run bash "$LS" sess "${BATS_TEST_TMPDIR}/wt" "bypassPermissions; rm -rf ~" "prompt" + run bash "$LS" sess "$WT" "bypassPermissions; rm -rf ~" "prompt" [ "$status" -eq 2 ] } @@ -45,6 +50,59 @@ setup() { [ "$status" -eq 2 ] } +# --- Issue #123: session-name guard ---------------------------------------- +# A zsh coordinator's word-splitting bug can hand this a space-containing +# session name; the name is interpolated into tmux commands and matched by +# safe-cleanup.sh's sweep, so it is allowlist-rejected before any tmux call. +# LO_DRY_RUN is used here because the guard runs before the dry-run exit +# (D2) — no tmux/claude stub is needed to prove it fires. + +@test "rejects a session name containing a space (issue #123 zsh word-splitting incident)" { + run env LO_DRY_RUN=1 sh "$LS" "lo-2 t2-lobby" "$WT" bypassPermissions "prompt" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "invalid session name" +} + +@test "rejects an LO_RUN_ID suffix that would corrupt the final session name" { + run env LO_DRY_RUN=1 LO_RUN_ID="r1 evil" sh "$LS" lo-1 "$WT" bypassPermissions "prompt" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "invalid session name" +} + +@test "rejects an empty session name (boundary)" { + run env LO_DRY_RUN=1 sh "$LS" "" "$WT" bypassPermissions "prompt" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "invalid session name" +} + +# --- Issue #123: worktree-root guard ---------------------------------------- +# The worktree argument must be a git work-tree ROOT. A coordinator bug can +# hand this the worktrees' shared parent directory instead of the task's own +# worktree; that and a non-root subdirectory of a real checkout both used to +# be handed straight to tmux `-c` with no error. The guard runs before any +# tmux/claude call, so no stub is needed — a real `tmux` on PATH is enough to +# pass the earlier binary-resolution check (unaffected by this guard's order). + +@test "rejects a non-existent worktree path (boundary)" { + run sh "$LS" lo-1 "$BATS_TEST_TMPDIR/does-not-exist" bypassPermissions "prompt" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "does not exist" +} + +@test "rejects a plain non-git directory as the worktree (issue #123 shared-parent-dir incident)" { + plain="$BATS_TEST_TMPDIR/plain"; mkdir -p "$plain" + run sh "$LS" lo-1 "$plain" bypassPermissions "prompt" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "is not a git work-tree" +} + +@test "rejects a subdirectory of a git repo that is not the work-tree root" { + sub="$WT/sub"; mkdir -p "$sub" + run sh "$LS" lo-1 "$sub" bypassPermissions "prompt" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "is not a git work-tree root" +} + # --------------------------------------------------------------------------- # The tests below drive the real launch path by replacing only the two external # binaries (tmux, claude) via the LO_TMUX / LO_CLAUDE hooks. Assertions are @@ -146,7 +204,7 @@ pane_not_ready() { # matches none of the ready/trust patterns @test "TMUX: the caller's tmux socket reaches the child untouched" { sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; echo 0 > "$sd/has-session-rc" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ - TMUX=SENTINEL-SOCKET,111,0 sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + TMUX=SENTINEL-SOCKET,111,0 sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 0 ] grep -q 'TMUX=SENTINEL-SOCKET,111,0' "$sd/env-log" } @@ -155,7 +213,7 @@ pane_not_ready() { # matches none of the ready/trust patterns sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; echo 0 > "$sd/has-session-rc" stub="$(mk_tmux_stub)" run env STUB_DIR="$sd" LO_TMUX="$stub" LO_CLAUDE=/bin/echo \ - TMUX=SENTINEL-SOCKET,111,0 sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + TMUX=SENTINEL-SOCKET,111,0 sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 0 ] # the assertion that fails on the pre-fix script: ! grep -q "TMUX=$stub" "$sd/env-log" @@ -164,7 +222,7 @@ pane_not_ready() { # matches none of the ready/trust patterns @test "TMUX: a caller with no TMUX set still launches (boundary — masks the bug)" { sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; echo 0 > "$sd/has-session-rc" run env -u TMUX STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 0 ] grep -q 'TMUX=' "$sd/env-log" } @@ -175,7 +233,7 @@ pane_not_ready() { # matches none of the ready/trust patterns sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; pane_not_ready > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=3 LO_READY_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 4 ] [ "$(cat "$sd/captures")" = "3" ] } @@ -184,7 +242,7 @@ pane_not_ready() { # matches none of the ready/trust patterns sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; pane_not_ready > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 4 ] [[ "$output" == *"REPL not ready"* ]] } @@ -193,7 +251,7 @@ pane_not_ready() { # matches none of the ready/trust patterns sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; pane_not_ready > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 4 ] [[ "$output" == *"NOTREADYMARKER"* ]] } @@ -202,7 +260,7 @@ pane_not_ready() { # matches none of the ready/trust patterns sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; pane_not_ready > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=1 LO_READY_INTERVAL=2 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "p" + sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 4 ] [ "$(cat "$sd/captures")" = "1" ] } @@ -217,7 +275,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_submitted > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" [ "$status" -eq 0 ] [[ "$output" == *"submitted"* ]] } @@ -227,7 +285,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_unsubmitted > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD you are the worker" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD you are the worker" [ "$status" -eq 5 ] [[ "$output" != *"ok:"* ]] [[ "$output" == *"NOT confirmed submitted"* ]] @@ -238,7 +296,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_pasted > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD you are the worker" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD you are the worker" [ "$status" -eq 5 ] [[ "$output" == *"NOT confirmed submitted"* ]] } @@ -248,7 +306,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_pasted > "$sd/pane" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" [ "$status" -eq 5 ] [[ "$output" == *"Pasted text"* ]] } @@ -261,7 +319,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_submitted > "$sd/pane-3" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=6 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD you are the worker" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD you are the worker" [ "$status" -eq 0 ] # the initial Enter plus exactly one remedial Enter [ "$(grep -c '^-t lo-1 Enter$' "$sd/keys")" = "2" ] @@ -273,7 +331,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_submitted > "$sd/pane-1" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" [ "$status" -eq 5 ] } @@ -283,7 +341,7 @@ pane_not_ready() { # matches none of the ready/trust patterns start=$(date +%s) run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=1 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" elapsed=$(( $(date +%s) - start )) [ "$status" -eq 5 ] [ "$elapsed" -lt 10 ] @@ -302,12 +360,12 @@ pane_not_ready() { # matches none of the ready/trust patterns run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_RUN_ID=r7 LO_STATUS_DIR="$st" LO_TASK_ID=t3 \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR/wt" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" [ "$status" -eq 0 ] [ -f "$st/t3.json" ] [ "$(jq -r .phase "$st/t3.json")" = "pending" ] [ "$(jq -r .session "$st/t3.json")" = "lo-1-r7" ] - [ "$(jq -r .worktree "$st/t3.json")" = "$BATS_TEST_TMPDIR/wt" ] + [ "$(jq -r .worktree "$st/t3.json")" = "$WT" ] } @test "pre-seed: with both vars unset no status file is written and stdout is unchanged (boundary)" { @@ -315,21 +373,31 @@ pane_not_ready() { # matches none of the ready/trust patterns st="$BATS_TEST_TMPDIR/status" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR/wt" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" [ "$status" -eq 0 ] [ "$output" = "ok: lo-1 launched + prompt submitted (confirmed)" ] [ ! -e "$st" ] } -@test "pre-seed: LO_STATUS_DIR alone (LO_TASK_ID unset) writes nothing (boundary)" { - sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd"; pane_ready_submitted > "$sd/pane" +@test "pre-seed: LO_STATUS_DIR alone (LO_TASK_ID unset) is now rejected (contract updated by issue #123)" { st="$BATS_TEST_TMPDIR/status" - run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ - LO_STATUS_DIR="$st" \ - LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR/wt" bypassPermissions "ZZPROMPTHEAD hello" - [ "$status" -eq 0 ] - [ "$output" = "ok: lo-1 launched + prompt submitted (confirmed)" ] + run env LO_STATUS_DIR="$st" sh "$LS" lo-1 "$WT" bypassPermissions "hello" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "LO_STATUS_DIR and LO_TASK_ID must be set together" + [ ! -e "$st" ] +} + +@test "pre-seed: LO_TASK_ID alone (LO_STATUS_DIR unset) is rejected" { + run env LO_TASK_ID=t9 sh "$LS" lo-1 "$WT" bypassPermissions "hello" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "LO_STATUS_DIR and LO_TASK_ID must be set together" +} + +@test "pre-seed: LO_STATUS_DIR set with LO_TASK_ID as an empty string is rejected (issue #123 incident shape: set-but-empty counts as unset)" { + st="$BATS_TEST_TMPDIR/status" + run env LO_STATUS_DIR="$st" LO_TASK_ID="" sh "$LS" lo-1 "$WT" bypassPermissions "hello" + [ "$status" -eq 2 ] + printf '%s\n' "$output" | grep -qF "LO_STATUS_DIR and LO_TASK_ID must be set together" [ ! -e "$st" ] } @@ -340,7 +408,7 @@ pane_not_ready() { # matches none of the ready/trust patterns run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_STATUS_DIR="$BATS_TEST_TMPDIR/blocker/sub" LO_TASK_ID=t3 \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=2 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR/wt" bypassPermissions "ZZPROMPTHEAD hello" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD hello" [ "$status" -eq 0 ] # grep, not [[ ]]: a false mid-test [[ ]] does not fail a test under this # bats/bash combination, which would make these assertions decoration @@ -353,12 +421,12 @@ pane_not_ready() { # matches none of the ready/trust patterns st="$BATS_TEST_TMPDIR/status" run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_STATUS_DIR="$st" LO_TASK_ID=t3 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR/wt" bypassPermissions "p" + sh "$LS" lo-1 "$WT" bypassPermissions "p" [ "$status" -eq 0 ] printf '%s\n' "$output" | grep -qF "session=lo-1" [ "$(jq -r .phase "$st/t3.json")" = "pending" ] [ "$(jq -r .session "$st/t3.json")" = "lo-1" ] - [ "$(jq -r .worktree "$st/t3.json")" = "$BATS_TEST_TMPDIR/wt" ] + [ "$(jq -r .worktree "$st/t3.json")" = "$WT" ] } # --- DEV_LOOP_WORKER_MODEL -------------------------------------------------- @@ -383,7 +451,7 @@ pane_not_ready() { # matches none of the ready/trust patterns pane_ready_submitted > "$sd/pane-2" run env -u DEV_LOOP_WORKER_MODEL STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=4 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD p" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD p" [ "$status" -eq 0 ] ! grep -q -- '--model' "$sd/keys" } @@ -395,7 +463,7 @@ pane_not_ready() { # matches none of the ready/trust patterns run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ DEV_LOOP_WORKER_MODEL=claude-sonnet-5 \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=4 LO_SUBMIT_INTERVAL=1 \ - sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD p" + sh "$LS" lo-1 "$WT" bypassPermissions "ZZPROMPTHEAD p" [ "$status" -eq 0 ] grep -q -- "--model 'claude-sonnet-5'" "$sd/keys" } From 10e2e94f21ab5d196622279fa86ffd3b442789b0 Mon Sep 17 00:00:00 2001 From: Younggi Choi <74581798+choiyounggi@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:47:11 +0900 Subject: [PATCH 2/2] docs(orchestrate): warn about zsh word-splitting in launch-session.sh calls Issue #123 part B. Documents the root cause behind the incident (zsh does not word-split unquoted parameters, unlike bash) and the exit-2 fail-fast contract t1 shipped, so coordinators write per-task explicit launch-session.sh calls instead of bash-style splitting loops. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AKHgksH77r9sWW9cFHvhDY --- skills/orchestrate/SKILL.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/skills/orchestrate/SKILL.md b/skills/orchestrate/SKILL.md index d81a707..f156396 100644 --- a/skills/orchestrate/SKILL.md +++ b/skills/orchestrate/SKILL.md @@ -267,6 +267,20 @@ defaulted; the effective budget and its source are printed before the wait. same validation as `LO_MAX_SESSIONS` (empty/non-numeric/`<= 0` refused with **exit 4**). +**The coordinator shell may be zsh:** zsh (the macOS default) does not +word-split unquoted parameter expansions the way bash does, so a bash-style +loop such as `for pair in "$name $task"; do launch-session.sh $pair; done` +silently passes the whole `"name task"` string as one argument instead of +splitting it into two. Launch each task with its own explicit +`launch-session.sh` call, arguments spelled out per task, never through a +splitting loop. `launch-session.sh` now fails fast against exactly this class +of bug: it exits **2** before any tmux call when the final session name +(after the `LO_RUN_ID` suffix) is empty or contains a character outside +`[A-Za-z0-9_-]`, when the worktree argument is not a git work-tree root, or +when exactly one of `LO_STATUS_DIR`/`LO_TASK_ID` is set (set-but-empty counts +as unset). A malformed caller now gets an immediate exit 2 instead of a +silently launched broken worker. + **Substrate (the user decided at Gate 1 — do not re-decide here):** the answer was Orca or tmux. **Orca** → spawn **and supervise** workers through it, not raw tmux: it resolves the trust/TUI screen, gives native liveness, and pushes worker events to