diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index bd760060861..ea59454ab5b 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -627,6 +627,27 @@ 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" + +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, @@ -641,10 +662,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 +807,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" ] && [ "$(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 21645fc36c4..30b7f3067ca 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -35,7 +35,8 @@ You do not need to attach for routine supervision: `bin/fm-peek.sh fm-` read Verify it works by spawning a trivial task with `--backend herdr` and confirming the task's meta records `backend=herdr` plus `herdr_session=`, `herdr_workspace_id=`, `herdr_tab_id=`, and `herdr_pane_id=`; the workspace for your home should show the new `fm-` tab. -Limitations: herdr is experimental, not yet used for `bin/fm-bootstrap.sh`'s required-tools list (the version/tool gate happens at spawn time instead), and carries the known gaps documented below (a small-`--lines` capture bug with a built-in workaround, and a `pane_cwd`-adjacent worktree-discovery symlink fragility) - see "Known gaps and follow-up notes" at the end of this document. +Limitations: herdr is experimental, not yet used for `bin/fm-bootstrap.sh`'s required-tools list (the version/tool gate happens at spawn time instead), and still carries the open gaps documented below. +Resolved backend evidence, including the 2026-07-06 symlinked-project-prefix isolation fix, is kept in the same follow-up log for auditability. ## Status: experimental @@ -377,8 +378,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. -- **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, 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-autodetect-smoke.test.sh b/tests/fm-backend-autodetect-smoke.test.sh index f538ce75ebf..2d7aa0c1403 100755 --- a/tests/fm-backend-autodetect-smoke.test.sh +++ b/tests/fm-backend-autodetect-smoke.test.sh @@ -45,13 +45,13 @@ command -v treehouse >/dev/null 2>&1 || { echo "skip: treehouse not found (requi # shellcheck source=tests/herdr-test-safety.sh . "$ROOT/tests/herdr-test-safety.sh" -# Physically-resolved (mktemp -d "$(pwd -P)"-relative), not the logical -# ${TMPDIR:-/tmp} path: on macOS /tmp is a symlink to /private/tmp, and -# fm-spawn.sh's PROJ_ABS uses a logical `cd && pwd` while herdr's own -# foreground_cwd reports the OS-resolved physical path. A project rooted on -# the logical side of that symlink makes the very first worktree-discovery -# poll see two different STRINGS for the same directory and trip the -# isolation guard's false-refusal before treehouse ever moves the pane. +# TMP_ROOT is physically resolved (mktemp -d "$(pwd -P)"-relative) to keep this +# real-herdr smoke fixture free of unrelated OS symlink noise. +# The old fm-spawn bug that originally motivated this fixture shape was fixed in +# fm-spawn-symlink-guard-s8: fm-spawn.sh now normalizes PROJ_ABS and observed +# backend cwd reads before the worktree-discovery comparison. +# The dedicated regression is +# tests/fm-backend.test.sh:test_spawn_symlinked_project_prefix_avoids_false_refusal. TMP_ROOT=$(mktemp -d "$(cd "${TMPDIR:-/tmp}" && pwd -P)/fm-backend-autodetect-smoke.XXXXXX") SESSION="fm-autodetect-smoke-$$" export HERDR_SESSION="$SESSION" diff --git a/tests/fm-backend-herdr-workspace-per-home-e2e.test.sh b/tests/fm-backend-herdr-workspace-per-home-e2e.test.sh index 93d7dd0ef99..f30857292e3 100755 --- a/tests/fm-backend-herdr-workspace-per-home-e2e.test.sh +++ b/tests/fm-backend-herdr-workspace-per-home-e2e.test.sh @@ -54,12 +54,12 @@ command -v treehouse >/dev/null 2>&1 || { echo "skip: treehouse not found (requi # shellcheck source=tests/herdr-test-safety.sh . "$ROOT/tests/herdr-test-safety.sh" -# Physically-resolved TMP_ROOT (mktemp -d "$(pwd -P)"-relative), not the -# logical ${TMPDIR:-/tmp} path - see tests/fm-backend-autodetect-smoke.test.sh -# for why: fm-spawn.sh's worktree-discovery poll compares a logical cd&&pwd -# against herdr's physically-resolved foreground_cwd, and a project rooted on -# the logical side of a symlinked /tmp trips the isolation guard's false -# refusal before treehouse ever moves the pane. +# TMP_ROOT is physically resolved (mktemp -d "$(pwd -P)"-relative) for the same +# low-noise scratch fixture shape used by +# tests/fm-backend-autodetect-smoke.test.sh. +# fm-spawn no longer needs this as a symlink workaround: fm-spawn-symlink-guard-s8 +# canonicalized project and backend cwd comparisons in the worktree-discovery +# poll. TMP_ROOT=$(mktemp -d "$(cd "${TMPDIR:-/tmp}" && pwd -P)/fm-herdr-e2e.XXXXXX") SESSION="fm-herdr-e2e-$$" export HERDR_SESSION="$SESSION" diff --git a/tests/fm-backend.test.sh b/tests/fm-backend.test.sh index 8254cfb8816..ef1f620034b 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -764,6 +764,92 @@ 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 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" <> "\${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' "$initial_path" + 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" +} + +run_spawn_symlink_case() { #