From ec050fad96df35146ddf70f9112302a11c823510 Mon Sep 17 00:00:00 2001 From: Paolo Sacconier Date: Sat, 11 Jul 2026 12:24:29 +0200 Subject: [PATCH] fix(spawn): skip treehouse get's transient cwd in worktree-discovery poll The worktree-discovery poll accepted the first non-project cwd, but treehouse get transits / and ~/.treehouse before it settles in the pooled worktree. herdr's foreground_cwd exposes those transients (tmux's pane_current_path reports the settled shell cwd and never did), so a herdr spawn resolved WT to / , failed the isolation gate, and aborted - orphaning the still-initializing treehouse get, which then held the pool's only slot forever with no crewmate (the single-slot wedge). Gate the poll on a new non-fatal is_isolated_worktree predicate (also the one validate_spawn_worktree now delegates to) so it skips the transients until the pane settles in a real worktree. Remember the last non-worktree cwd so a genuine tangle still gets the precise resolved-path refusal instead of a vague timeout, and make the poll budget configurable. No-op for tmux. Regression test drives real fm-spawn against a pane that reports / then the worktree; reproduces the exact 'resolved /' failure pre-fix. --- bin/fm-spawn.sh | 75 +++++++++++++++++++++++++++-------- docs/herdr-backend.md | 8 ++++ tests/fm-backend.test.sh | 70 ++++++++++++++++++++++++++++++++ tests/fm-tangle-guard.test.sh | 1 + 4 files changed, 137 insertions(+), 17 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 9a8a159a8..b8d8adc55 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -668,19 +668,25 @@ real_path_or_raw() { # # herdr-sm-spaces-k4). Both branches converge on the same $T ("target") string # that every downstream operation (send/capture/kill) already treats as opaque # per-backend routing (fm_backend_resolve_selector). +# is_isolated_worktree : non-fatal predicate, true (0) iff is the +# ROOT of a real git worktree physically distinct from the primary project +# checkout. Shared by the treehouse-get discovery poll (to skip transient +# intermediate cwds) and validate_spawn_worktree's fatal assertion, so the two +# use one definition of "a genuine isolated worktree". +is_isolated_worktree() { # + local wt=$1 wt_real wt_top wt_top_real + wt_real=$(cd "$wt" 2>/dev/null && pwd -P) || return 1 + [ -n "$wt_real" ] || return 1 + wt_top=$(git -C "$wt" rev-parse --show-toplevel 2>/dev/null) || return 1 + wt_top_real=$(cd "$wt_top" 2>/dev/null && pwd -P) || return 1 + [ "$wt_real" = "$wt_top_real" ] || return 1 + [ "$wt_real" != "$PROJ_ABS_REAL" ] || return 1 + return 0 +} validate_spawn_worktree() { # - local source=$1 inspect_target=$2 wt_real proj_real wt_top wt_top_real - wt_real= - if ! wt_real=$(cd "$WT" 2>/dev/null && pwd -P); then - wt_real= - fi - proj_real=$PROJ_ABS_REAL - wt_top=$(git -C "$WT" rev-parse --show-toplevel 2>/dev/null || true) - wt_top_real= - if ! wt_top_real=$(cd "$wt_top" 2>/dev/null && pwd -P); then - wt_top_real= - fi - if [ -z "$wt_real" ] || [ -z "$wt_top_real" ] || [ "$wt_real" != "$wt_top_real" ] || [ "$wt_real" = "$proj_real" ]; then + local source=$1 inspect_target=$2 wt_top + if ! is_isolated_worktree "$WT"; then + wt_top=$(git -C "$WT" rev-parse --show-toplevel 2>/dev/null || true) echo "error: $source did not yield an isolated worktree (resolved '$WT'; worktree root '${wt_top:-none}'; primary '$PROJ_ABS'); refusing to launch to avoid tangling the primary checkout. Inspect target $inspect_target" >&2 exit 1 fi @@ -839,17 +845,52 @@ if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then # Compare against PROJ_ABS_REAL (physical), not PROJ_ABS: a symlinked project # prefix would otherwise make the pane's OS-level cwd read differ from # PROJ_ABS on the very first poll, before the pane has actually moved. - for _ in $(seq 1 60); do + # + # Accept only a cwd that is a GENUINE isolated worktree, not merely the first + # non-project cwd: `treehouse get` transits intermediate cwds while it sets the + # worktree up (it is momentarily at `/` and at the treehouse root ~/.treehouse + # before it creates/enters the pooled worktree), and herdr's foreground_cwd + # exposes those transients (tmux's pane_current_path reports the settled + # interactive-shell cwd and never showed them). Grabbing a transient here made + # fm-spawn resolve WT to `/` or ~/.treehouse, fail the isolation gate below, + # and abort - orphaning the still-initializing treehouse get, which then held + # the pool slot forever with no crewmate (the herdr single-slot wedge). Gating + # on is_isolated_worktree keeps polling past the transients until the pane + # settles in the real worktree; it is a no-op for tmux, whose first observed + # non-project cwd already is that worktree. + # + # moved_to remembers the last non-project cwd seen that was NOT (yet) a valid + # worktree: if the pane leaves the project but never reaches an isolated + # worktree within the budget (a genuine tangle, or treehouse get landing the + # pane somewhere invalid and staying), that path is handed to + # validate_spawn_worktree below so its precise "did not yield an isolated + # worktree (resolved '')" refusal still fires, rather than the vaguer + # never-moved timeout. A single point-in-time read cannot tell a settling + # transient apart from a permanent tangle, so we wait out the budget rather + # than risk a false refusal on a slow cold setup. + poll_tries=${FM_SPAWN_WORKTREE_POLL_TRIES:-60} + case "$poll_tries" in ''|*[!0-9]*|0) poll_tries=60 ;; esac + moved_to="" + for _ in $(seq 1 "$poll_tries"); do p=$(spawn_current_path "$WT_TARGET" || true) if [ -n "$p" ] && [ "$(real_path_or_raw "$p")" != "$PROJ_ABS_REAL" ]; then - WT="$p" - break + if is_isolated_worktree "$p"; then + WT="$p" + break + fi + moved_to="$p" fi sleep 1 done if [ -z "$WT" ]; then - echo "error: treehouse get did not enter a worktree within 60s; inspect window $T" >&2 - exit 1 + if [ -n "$moved_to" ]; then + # Pane left the project but never reached an isolated worktree; let the + # gate emit its precise resolved-path refusal. + WT="$moved_to" + else + echo "error: treehouse get did not enter a worktree within ${poll_tries}s; inspect window $T" >&2 + exit 1 + fi fi validate_spawn_worktree "treehouse get" "$T" diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index 2ab3088d6..93937b4a8 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -657,6 +657,14 @@ It now also attempts a configurable active alert independent of the supervisor b Verified with GNU bash 5.3.9(1)-release (aarch64-apple-darwin25.3.0) and git 2.53.0 on macOS (Darwin 25.5.0): added `tests/fm-backend.test.sh:test_spawn_symlinked_project_prefix_avoids_false_refusal`, which drives the real `bin/fm-spawn.sh` against fake-tmux panes whose first `pane_current_path` poll returns both the project's `pwd -P`-resolved physical path and its logical symlink-preserving path while `PROJ_ABS` is reached through a synthetic symlinked prefix (`ln -s `, project passed as `/proj`). Confirmed the test reproduces the original bug against the pre-fix script (`git stash` the `bin/fm-spawn.sh` change and rerun: `not ok - fm-spawn.sh should succeed for a project reached through a symlinked prefix` / `error: treehouse get did not yield an isolated worktree ...`), and passes against the fix (`bash tests/fm-backend.test.sh` reports `ok - fm-spawn.sh: a project reached through a symlinked prefix (e.g. macOS /tmp -> /private/tmp) does not trip the isolation guard's false refusal`, with the rest of that suite's assertions unaffected). `shellcheck bin/*.sh bin/backends/*.sh tests/*.sh` passes clean on the changed scripts. +- **RESOLVED: the worktree-discovery poll grabbed `treehouse get`'s transient intermediate cwd (the herdr single-slot wedge).** Symptom (reproduced live 2026-07-11 against real herdr 0.7.3 + treehouse on a single-slot pool, backlog `herdr-fix-r5`): a herdr spawn aborted with `treehouse get did not yield an isolated worktree (resolved '/'; ...)` and left a live, stuck `treehouse get` holding the pool's only slot with no crewmate attached, so the next spawn found the pool exhausted. + Root cause is NOT process reaping: `treehouse get` transits intermediate cwds while it sets the worktree up - it is momentarily at `/` and at the treehouse root (`~/.treehouse`) before it creates and enters the pooled worktree (confirmed live: a cold `pane run 'treehouse get'` polled `foreground_cwd` = the project dir, then `/`, then the real `~/.treehouse//1/`; owner_pid is recorded only once it settles). + `fm-spawn.sh`'s poll accepted the FIRST cwd that merely differed from the project, so it captured `/` (or `~/.treehouse`), handed that transient to `validate_spawn_worktree`, and aborted - while the still-initializing `treehouse get` completed and held the slot forever. + herdr's `foreground_cwd` exposes those transients because it reads the actively-running foreground process's cwd; tmux's `pane_current_path` reports the settled interactive-shell cwd and never surfaced them, which is why only the herdr path wedged (verified reference backend stayed reliable). + Fix (`bin/fm-spawn.sh`): the worktree-discovery poll now gates on a new non-fatal `is_isolated_worktree ` predicate (a real git-worktree root, physically distinct from the primary checkout - the same rule `validate_spawn_worktree` now delegates to), so it skips `/` and `~/.treehouse` and keeps polling until the pane settles in the actual worktree. + It is a no-op for tmux, whose first observed non-project cwd already is that worktree. + Verified: added `tests/fm-backend.test.sh:test_spawn_worktree_poll_skips_treehouse_get_transient_cwd`, which drives the real `bin/fm-spawn.sh` against a fake-tmux pane whose polls return the project dir, then a transient `/`, then the real worktree; it reproduces the exact `resolved '/'` failure against the pre-fix poll (`not ok`) and passes against the fix, with the rest of that suite unaffected. + `shellcheck bin/*.sh bin/backends/*.sh tests/*.sh` passes clean. - **RESOLVED: a restart's restored-layout husk no longer needs a manual pane close before respawn.** See "Respawn idempotency: a restored task tab is a husk, not a duplicate" above for the fix (`fm_backend_herdr_pane_agent_state`, `fm_backend_herdr_create_task`'s close-and-replace). Left over from that fix: the `dead` (`pane_not_found`) husk classification is exercised only at the unit level, never against the real binary - killing a pane's process on a live server was observed to make herdr reap the whole tab immediately (never leaving a dead-but-still-listed pane for the duplicate check to find), and a real session restart was never observed to produce one either. It remains a conservative, defensively-coded path for a herdr failure mode (e.g. a restored process that fails to start) nobody has reproduced against the real binary yet. diff --git a/tests/fm-backend.test.sh b/tests/fm-backend.test.sh index c5fbf9045..d9ca470fc 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -871,6 +871,75 @@ test_spawn_symlinked_project_prefix_avoids_false_refusal() { pass "fm-spawn.sh: a project reached through a symlinked prefix (e.g. macOS /tmp -> /private/tmp) does not trip the isolation guard's false refusal" } +# --- the worktree poll must skip treehouse-get's transient intermediate cwds -- +# +# Regression for the herdr single-slot wedge (task herdr-fix-r5): `treehouse get` +# transits intermediate cwds while it sets the worktree up - it is momentarily at +# `/` and at the treehouse root before it creates/enters the pooled worktree - and +# herdr's foreground_cwd exposes those transients (tmux's pane_current_path +# reported the settled cwd and never did). The old poll accepted the FIRST cwd +# that merely differed from the project, so it grabbed `/`, failed the isolation +# gate, and aborted - orphaning the still-initializing treehouse get, which then +# held the pool slot forever with no crewmate. The poll now gates on +# is_isolated_worktree, so it must skip a `/` transient and settle on the real +# worktree. make_spawn_transient_fakebin's stub returns the unmoved project path, +# then a transient `/`, then the real worktree, on successive polls. +make_spawn_transient_fakebin() { # -> echoes fakebin dir + local dir=$1 proj=$2 transient=$3 wt=$4 fb="$1/fakebin" counter="$1/poll-count" + mkdir -p "$fb" + : > "$counter" + cat > "$fb/tmux" <> "\${FM_TMUX_LOG:?}" +case "\${1:-}" in + display-message) + for a in "\$@"; do case "\$a" in *pane_current_path*) + printf x >> "$counter" + n=\$(wc -c < "$counter") + if [ "\$n" -le 1 ]; then + printf '%s\\n' "$proj" + elif [ "\$n" -le 2 ]; then + printf '%s\\n' "$transient" + else + printf '%s\\n' "$wt" + fi + exit 0 + ;; esac; done + printf 'firstmate\\n'; exit 0 ;; + list-windows) exit 0 ;; +esac +exit 0 +SH + chmod +x "$fb/tmux" + fm_fake_exit0 "$fb" treehouse + printf '%s\n' "$fb" +} + +test_spawn_worktree_poll_skips_treehouse_get_transient_cwd() { + local proj wt id fb data state config log out rc + proj="$TMP_ROOT/transient-proj" + wt="$TMP_ROOT/transient-wt" + id="spawntransient" + fm_git_worktree "$proj" "$wt" "fm/$id" + # First non-project poll returns `/` (the exact transient the real bug hit); + # the isolated-worktree gate must reject it and keep polling to the real wt. + fb=$(make_spawn_transient_fakebin "$TMP_ROOT/transient-fake" "$proj" "/" "$wt") + data="$TMP_ROOT/transient-data"; mkdir -p "$data/$id" + printf 'test brief content\n' > "$data/$id/brief.md" + state="$TMP_ROOT/transient-state"; config="$TMP_ROOT/transient-config" + mkdir -p "$state" "$config" + log="$TMP_ROOT/transient-spawn.log" + + out=$(run_spawn_case "$ROOT" "$fb" "$log" "$state" "$data" "$config" "$proj" -- "$id" "$proj" claude 2>&1) + rc=$? + expect_code 0 "$rc" "fm-spawn.sh should skip treehouse get's transient '/' cwd and settle on the real worktree"$'\n'"$out" + assert_contains "$out" "worktree=$wt" \ + "fm-spawn.sh resolved the worktree to a transient cwd instead of the real pooled worktree" + rm -rf "/tmp/fm-$id" + pass "fm-spawn.sh: the worktree-discovery poll skips treehouse get's transient intermediate cwds (/ , treehouse root) and resolves the real isolated worktree" +} + # --- old vs new: fm-teardown.sh ---------------------------------------------- make_teardown_fakebin() { # -> echoes fakebin dir; logs tmux+treehouse calls @@ -1082,6 +1151,7 @@ test_backend_of_selector_matches_explicit_target_meta test_send_conformance_old_vs_new test_peek_conformance_old_vs_new test_spawn_symlinked_project_prefix_avoids_false_refusal +test_spawn_worktree_poll_skips_treehouse_get_transient_cwd test_teardown_conformance_old_vs_new test_spawn_refuses_unknown_backend_flag test_spawn_refuses_codex_app_backend_flag diff --git a/tests/fm-tangle-guard.test.sh b/tests/fm-tangle-guard.test.sh index a6f24eaa5..4c5397d8f 100755 --- a/tests/fm-tangle-guard.test.sh +++ b/tests/fm-tangle-guard.test.sh @@ -181,6 +181,7 @@ run_spawn() { FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \ FM_SPAWN_NO_GUARD=1 FM_FAKE_PANE_PATH="$pane" TMUX="fake,1,0" \ + FM_SPAWN_WORKTREE_POLL_TRIES=2 \ PATH="$fakebin:$PATH" \ "$ROOT/bin/fm-spawn.sh" "$id" "$proj" codex 2>&1 }