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
31 changes: 26 additions & 5 deletions bin/fm-spawn.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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() { # <path>
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,
Expand All @@ -641,10 +662,7 @@ validate_spawn_worktree() { # <source> <inspect-target>
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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 10 additions & 3 deletions docs/herdr-backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ You do not need to attach for routine supervision: `bin/fm-peek.sh fm-<id>` 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-<id>` 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

Expand Down Expand Up @@ -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 <real> <link>`, project passed as `<link>/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.
14 changes: 7 additions & 7 deletions tests/fm-backend-autodetect-smoke.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 6 additions & 6 deletions tests/fm-backend-herdr-workspace-per-home-e2e.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
87 changes: 87 additions & 0 deletions tests/fm-backend.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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() { # <dir> <initial-project-path> <worktree-path> -> 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" <<SH
#!/usr/bin/env bash
set -u
{ printf 'tmux'; for a in "\$@"; do printf '\\x1f%s' "\$a"; done; printf '\\n'; } >> "\${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() { # <label> <physical|logical>
local label=$1 first_reply=$2 real_root link_root proj wt id fb data state config log out rc proj_phys initial_path
real_root="$TMP_ROOT/symlink-real-$label"; link_root="$TMP_ROOT/symlink-link-$label"
mkdir -p "$real_root"
ln -s "$real_root" "$link_root"
proj="$link_root/proj"
wt="$TMP_ROOT/symlink-wt-$label"
id="spawnsymlink$label"
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)
case "$first_reply" in
physical) initial_path=$proj_phys ;;
logical) initial_path=$proj ;;
*) fail "unknown symlink first-reply mode: $first_reply" ;;
esac
fb=$(make_spawn_symlink_fakebin "$TMP_ROOT/symlink-fake-$label" "$initial_path" "$wt")
data="$TMP_ROOT/symlink-data-$label"
mkdir -p "$data/$id"
printf 'test brief content\n' > "$data/$id/brief.md"
state="$TMP_ROOT/symlink-state-$label"; config="$TMP_ROOT/symlink-config-$label"
mkdir -p "$state" "$config"
log="$TMP_ROOT/symlink-spawn-$label.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 when the backend reports $first_reply cwd"$'\n'"$out"
assert_contains "$out" "worktree=$wt" \
"fm-spawn.sh did not resolve a symlinked-prefix project to its real worktree when the backend reports $first_reply cwd"

rm -rf "/tmp/fm-$id"
}

test_spawn_symlinked_project_prefix_avoids_false_refusal() {
run_spawn_symlink_case physical physical
run_spawn_symlink_case logical logical
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() { # <dir> -> echoes fakebin dir; logs tmux+treehouse calls
Expand Down Expand Up @@ -964,6 +1050,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
Expand Down