From d3a45fcf9e15fb3fce72caf1d04ea47d44e2b4e1 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Mon, 6 Jul 2026 10:50:59 -0700 Subject: [PATCH 1/3] fix(spawn): canonicalize worktree-isolation guard against symlinked project prefixes fm-spawn.sh compared a logical PROJ_ABS against the physically-resolved pane cwd every backend reports, so a project reached through a symlinked prefix (e.g. macOS's /tmp -> /private/tmp) could trip the isolation guard's false refusal before treehouse ever moved the pane. Canonicalize once into PROJ_ABS_REAL and compare against that everywhere instead. --- bin/fm-spawn.sh | 22 +++++++++--- docs/herdr-backend.md | 6 ++-- tests/fm-backend.test.sh | 78 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index bd760060861..d57fd77649d 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -627,6 +627,18 @@ else fi [ -f "$BRIEF" ] || { echo "error: no brief at $BRIEF" >&2; exit 1; } +# PROJ_ABS can still carry a symlinked path component (e.g. macOS's /tmp -> +# /private/tmp) when it came from the ship/scout branch's logical `pwd` above. +# Every backend's own current-path read (tmux's pane_current_path, herdr's +# foreground_cwd, zellij/cmux's active pwd probe against the live shell) can +# report the OS-level, physically-resolved cwd, so comparing it against a +# still-symlinked PROJ_ABS can misfire both ways: false-negative (the poll +# below never notices the pane left the project) or false-positive (the +# isolation guard refuses a spawn that never actually tangled). Canonicalize +# once here so every downstream comparison uses the same physical form +# (docs/herdr-backend.md "Known gaps"). +PROJ_ABS_REAL=$(cd "$PROJ_ABS" 2>/dev/null && pwd -P) || PROJ_ABS_REAL="$PROJ_ABS" + # Session-provider container-ensure + task creation. tmux stays exactly as P1 # left it (same session-name / new-window sequence, see bin/backends/tmux.sh); # a herdr spawn goes through the version-gated, workspace-per-HOME, @@ -641,10 +653,7 @@ validate_spawn_worktree() { # if ! wt_real=$(cd "$WT" 2>/dev/null && pwd -P); then wt_real= fi - proj_real= - if ! proj_real=$(cd "$PROJ_ABS" 2>/dev/null && pwd -P); then - proj_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 @@ -789,9 +798,12 @@ if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then spawn_send_text_line "$T" 'treehouse get' # Wait for the treehouse subshell: the pane's cwd moves from the project to the worktree. + # 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 p=$(spawn_current_path "$T" || true) - if [ -n "$p" ] && [ "$p" != "$PROJ_ABS" ]; then + if [ -n "$p" ] && [ "$p" != "$PROJ_ABS_REAL" ]; then WT="$p" break fi diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index 21645fc36c4..99af60c1d83 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -377,8 +377,10 @@ This is a test-harness-only concern - `fm_backend_herdr_composer_state` and `fm_ A genuine `events.subscribe`-driven push is a reasonable follow-up, not implemented here. - **`bin/fm-bootstrap.sh`'s required-tools list is unchanged.** It still unconditionally requires `tmux`, and does not yet conditionally add `herdr` and `jq` when a backend selection resolves to herdr. The version/tool gate happens at spawn time instead and refuses loudly, so this is bootstrap-detection polish, not a functional gap. -- **Worktree-discovery isolation guard is symlink-fragile for a project path under a symlinked prefix (e.g. macOS's `/tmp` -> `/private/tmp`).** Discovered while building the runtime-backend-auto-detection real smoke test (`tests/fm-backend-autodetect-smoke.test.sh`), which needed a scratch project. `fm-spawn.sh`'s `PROJ_ABS` is a LOGICAL `cd && pwd` (symlink components kept), while herdr's `foreground_cwd` (and real tmux's `pane_current_path`, on the same OS-level cwd primitive) report the PHYSICALLY resolved path. - When the project itself lives under a symlinked directory, the very first worktree-discovery poll sees two different strings for the identical starting directory and the isolation guard false-refuses the spawn as "not isolated" before `treehouse get` ever moves the pane - backend-agnostic, not specific to herdr. Worked around in the test by resolving its scratch `TMP_ROOT` through `pwd -P` before use; the underlying `fm-spawn.sh` path-comparison gap (worth resolving `PROJ_ABS` physically, or comparing physically-resolved forms in the isolation guard) is unfixed and worth a dedicated follow-up. +- **RESOLVED: worktree-discovery isolation guard's symlinked-project-prefix false refusal.** Originally discovered while building the runtime-backend-auto-detection real smoke test (`tests/fm-backend-autodetect-smoke.test.sh`), which needed a scratch project. `fm-spawn.sh`'s `PROJ_ABS` was a LOGICAL `cd && pwd` (symlink components kept), while herdr's `foreground_cwd` (and real tmux's `pane_current_path`, on the same OS-level cwd primitive) report the PHYSICALLY resolved path. + When the project itself lived under a symlinked directory (e.g. macOS's `/tmp` -> `/private/tmp`), the very first worktree-discovery poll saw two different strings for the identical starting directory and the isolation guard false-refused the spawn as "not isolated" before `treehouse get` ever moved the pane - backend-agnostic, not specific to herdr. + Fixed 2026-07-06 (backlog `fm-spawn-symlink-guard-s8`): `bin/fm-spawn.sh` now canonicalizes once into `PROJ_ABS_REAL` (`cd "$PROJ_ABS" && pwd -P`) right after `PROJ_ABS` is resolved, and both the worktree-discovery poll's "has the pane left the project" comparison and `validate_spawn_worktree`'s own primary-vs-worktree comparison compare against that physical form instead of the still-symlinked `PROJ_ABS`. This removes both failure directions: a symlinked prefix can no longer false-refuse an isolated spawn, and, since `PROJ_ABS_REAL` is always the OS-level resolved form regardless of input, a genuinely tangled spawn (worktree resolves to the same physical directory as the project) still correctly refuses. + 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 a fake-tmux pane whose first `pane_current_path` poll returns the project's `pwd -P`-resolved physical 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 20 assertions unaffected). `shellcheck bin/*.sh bin/backends/*.sh tests/*.sh` passes clean on the changed scripts. - **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 8254cfb8816..c80c6d6865d 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -764,6 +764,83 @@ test_spawn_conformance_old_vs_new() { pass "fm-spawn.sh: tmux command log and printed summary line are byte-identical old vs new for a ship-task claude spawn" } +# --- symlinked project prefix must not false-refuse the isolation guard ----- +# +# docs/herdr-backend.md "Known gaps": a real backend's pane_current_path read +# (tmux, herdr) reports the OS-level PHYSICALLY-resolved cwd. When the project +# itself lives under a symlinked prefix (e.g. macOS's /tmp -> /private/tmp), +# fm-spawn.sh's PROJ_ABS - a logical `cd && pwd` - differs string-for-string +# from that physical read even before treehouse moves the pane at all, so the +# worktree-discovery poll used to mistake an UNMOVED pane for one that had +# already left the project, handing validate_spawn_worktree the project's own +# directory as "the worktree" and tripping its false isolation refusal. +# make_spawn_symlink_fakebin's tmux stub returns the PHYSICAL project path on +# the first pane_current_path poll (reproducing that unmoved-but-resolved +# read), then the real worktree path from the second poll onward (reproducing +# treehouse actually moving the pane), so this test fails loudly if the +# PROJ_ABS/PROJ_ABS_REAL canonicalization in bin/fm-spawn.sh ever regresses. +make_spawn_symlink_fakebin() { # -> echoes fakebin dir + local dir=$1 proj_phys=$2 wt=$3 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" + if [ "\$(wc -c < "$counter")" -le 1 ]; then + printf '%s\\n' "$proj_phys" + 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_symlinked_project_prefix_avoids_false_refusal() { + local real_root link_root proj wt id fb data state config log out rc proj_phys + real_root="$TMP_ROOT/symlink-real"; link_root="$TMP_ROOT/symlink-link" + mkdir -p "$real_root" + ln -s "$real_root" "$link_root" + proj="$link_root/proj" + wt="$TMP_ROOT/symlink-wt" + id="spawnsymlink1" + fm_git_worktree "$real_root/proj" "$wt" "fm/$id" + # TMP_ROOT itself can already sit behind an OS-level symlink (e.g. macOS's + # /var -> /private/var), so resolve the fakebin's "physical" reply with + # pwd -P rather than string concatenation - it must match exactly what + # fm-spawn.sh's own PROJ_ABS_REAL computes, including any symlink layers + # ABOVE this test's own synthetic real_root/link_root pair. + proj_phys=$(cd "$real_root/proj" && pwd -P) + fb=$(make_spawn_symlink_fakebin "$TMP_ROOT/symlink-fake" "$proj_phys" "$wt") + data="$TMP_ROOT/symlink-data" + mkdir -p "$data/$id" + printf 'test brief content\n' > "$data/$id/brief.md" + state="$TMP_ROOT/symlink-state"; config="$TMP_ROOT/symlink-config" + mkdir -p "$state" "$config" + log="$TMP_ROOT/symlink-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 succeed for a project reached through a symlinked prefix"$'\n'"$out" + assert_contains "$out" "worktree=$wt" \ + "fm-spawn.sh did not resolve a symlinked-prefix project to its real worktree (isolation-guard false-refusal regression)" + + rm -rf "/tmp/fm-$id" + 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" +} + # --- old vs new: fm-teardown.sh ---------------------------------------------- make_teardown_fakebin() { # -> echoes fakebin dir; logs tmux+treehouse calls @@ -964,6 +1041,7 @@ test_backend_of_selector_matches_explicit_target_meta test_send_conformance_old_vs_new test_peek_conformance_old_vs_new test_spawn_conformance_old_vs_new +test_spawn_symlinked_project_prefix_avoids_false_refusal test_teardown_conformance_old_vs_new test_spawn_refuses_unknown_backend_flag test_spawn_refuses_unknown_fm_backend_env From 38bfc9bbce2d62a227eae29b473e9e0091939058 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Mon, 6 Jul 2026 11:01:00 -0700 Subject: [PATCH 2/3] no-mistakes(review): Canonicalize spawn cwd comparisons --- bin/fm-spawn.sh | 11 +++++++++- docs/herdr-backend.md | 10 ++++++--- tests/fm-backend.test.sh | 47 ++++++++++++++++++++++++---------------- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index d57fd77649d..ea59454ab5b 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -639,6 +639,15 @@ fi # (docs/herdr-backend.md "Known gaps"). PROJ_ABS_REAL=$(cd "$PROJ_ABS" 2>/dev/null && pwd -P) || PROJ_ABS_REAL="$PROJ_ABS" +real_path_or_raw() { # + local path=$1 real + if real=$(cd "$path" 2>/dev/null && pwd -P); then + printf '%s\n' "$real" + else + printf '%s\n' "$path" + fi +} + # Session-provider container-ensure + task creation. tmux stays exactly as P1 # left it (same session-name / new-window sequence, see bin/backends/tmux.sh); # a herdr spawn goes through the version-gated, workspace-per-HOME, @@ -803,7 +812,7 @@ if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then # PROJ_ABS on the very first poll, before the pane has actually moved. for _ in $(seq 1 60); do p=$(spawn_current_path "$T" || true) - if [ -n "$p" ] && [ "$p" != "$PROJ_ABS_REAL" ]; then + if [ -n "$p" ] && [ "$(real_path_or_raw "$p")" != "$PROJ_ABS_REAL" ]; then WT="$p" break fi diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index 99af60c1d83..db791fc713f 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -377,10 +377,14 @@ This is a test-harness-only concern - `fm_backend_herdr_composer_state` and `fm_ A genuine `events.subscribe`-driven push is a reasonable follow-up, not implemented here. - **`bin/fm-bootstrap.sh`'s required-tools list is unchanged.** It still unconditionally requires `tmux`, and does not yet conditionally add `herdr` and `jq` when a backend selection resolves to herdr. The version/tool gate happens at spawn time instead and refuses loudly, so this is bootstrap-detection polish, not a functional gap. -- **RESOLVED: worktree-discovery isolation guard's symlinked-project-prefix false refusal.** Originally discovered while building the runtime-backend-auto-detection real smoke test (`tests/fm-backend-autodetect-smoke.test.sh`), which needed a scratch project. `fm-spawn.sh`'s `PROJ_ABS` was a LOGICAL `cd && pwd` (symlink components kept), while herdr's `foreground_cwd` (and real tmux's `pane_current_path`, on the same OS-level cwd primitive) report the PHYSICALLY resolved path. +- **RESOLVED: worktree-discovery isolation guard's symlinked-project-prefix false refusal.** Originally discovered while building the runtime-backend-auto-detection real smoke test (`tests/fm-backend-autodetect-smoke.test.sh`), which needed a scratch project. + `fm-spawn.sh`'s `PROJ_ABS` was a LOGICAL `cd && pwd` (symlink components kept), while herdr's `foreground_cwd` (and real tmux's `pane_current_path`, on the same OS-level cwd primitive) report the PHYSICALLY resolved path. When the project itself lived under a symlinked directory (e.g. macOS's `/tmp` -> `/private/tmp`), the very first worktree-discovery poll saw two different strings for the identical starting directory and the isolation guard false-refused the spawn as "not isolated" before `treehouse get` ever moved the pane - backend-agnostic, not specific to herdr. - Fixed 2026-07-06 (backlog `fm-spawn-symlink-guard-s8`): `bin/fm-spawn.sh` now canonicalizes once into `PROJ_ABS_REAL` (`cd "$PROJ_ABS" && pwd -P`) right after `PROJ_ABS` is resolved, and both the worktree-discovery poll's "has the pane left the project" comparison and `validate_spawn_worktree`'s own primary-vs-worktree comparison compare against that physical form instead of the still-symlinked `PROJ_ABS`. This removes both failure directions: a symlinked prefix can no longer false-refuse an isolated spawn, and, since `PROJ_ABS_REAL` is always the OS-level resolved form regardless of input, a genuinely tangled spawn (worktree resolves to the same physical directory as the project) still correctly refuses. - 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 a fake-tmux pane whose first `pane_current_path` poll returns the project's `pwd -P`-resolved physical 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 20 assertions unaffected). `shellcheck bin/*.sh bin/backends/*.sh tests/*.sh` passes clean on the changed scripts. + Fixed 2026-07-06 (backlog `fm-spawn-symlink-guard-s8`): `bin/fm-spawn.sh` now canonicalizes once into `PROJ_ABS_REAL` (`cd "$PROJ_ABS" && pwd -P`) right after `PROJ_ABS` is resolved, canonicalizes each observed pane cwd for the worktree-discovery comparison, and uses `PROJ_ABS_REAL` in `validate_spawn_worktree`'s own primary-vs-worktree comparison instead of recomputing from the still-symlinked `PROJ_ABS`. + This removes both failure directions: a symlinked prefix can no longer false-refuse an isolated spawn, and, since both sides are physically resolved for comparison, a genuinely tangled spawn (worktree resolves to the same physical directory as the project) still correctly refuses. + 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: 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 c80c6d6865d..ef1f620034b 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -774,13 +774,12 @@ test_spawn_conformance_old_vs_new() { # worktree-discovery poll used to mistake an UNMOVED pane for one that had # already left the project, handing validate_spawn_worktree the project's own # directory as "the worktree" and tripping its false isolation refusal. -# make_spawn_symlink_fakebin's tmux stub returns the PHYSICAL project path on -# the first pane_current_path poll (reproducing that unmoved-but-resolved -# read), then the real worktree path from the second poll onward (reproducing -# treehouse actually moving the pane), so this test fails loudly if the -# PROJ_ABS/PROJ_ABS_REAL canonicalization in bin/fm-spawn.sh ever regresses. -make_spawn_symlink_fakebin() { # -> echoes fakebin dir - local dir=$1 proj_phys=$2 wt=$3 fb="$1/fakebin" counter="$1/poll-count" +# make_spawn_symlink_fakebin's tmux stub returns an unmoved project path on the +# first pane_current_path poll, then the real worktree path from the second poll +# onward, so this test fails loudly if the PROJ_ABS/PROJ_ABS_REAL +# canonicalization in bin/fm-spawn.sh ever regresses. +make_spawn_symlink_fakebin() { # -> echoes fakebin dir + local dir=$1 initial_path=$2 wt=$3 fb="$1/fakebin" counter="$1/poll-count" mkdir -p "$fb" : > "$counter" cat > "$fb/tmux" <> "$counter" if [ "\$(wc -c < "$counter")" -le 1 ]; then - printf '%s\\n' "$proj_phys" + printf '%s\\n' "$initial_path" else printf '%s\\n' "$wt" fi @@ -808,14 +807,14 @@ SH printf '%s\n' "$fb" } -test_spawn_symlinked_project_prefix_avoids_false_refusal() { - local real_root link_root proj wt id fb data state config log out rc proj_phys - real_root="$TMP_ROOT/symlink-real"; link_root="$TMP_ROOT/symlink-link" +run_spawn_symlink_case() { #