diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index cfb25f0058..9b99e80201 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -48,7 +48,7 @@ # then tmux. # Spawn-capable backends are the reference tmux adapter and experimental # herdr, zellij, orca, and cmux. Orca owns both the task worktree and -# terminal, so ship/scout Orca spawns do not run treehouse get; cmux is a +# terminal, so ship/scout Orca spawns do not lease a treehouse slot; cmux is a # session provider only, exactly like herdr/zellij, so it does. An # auto-detected herdr or cmux spawn prints a loud stderr notice; # auto-detected tmux stays silent; zellij and orca are never auto-detected. @@ -665,6 +665,8 @@ SPAWN_META_LOCK_HELD=0 SPAWN_META_PUBLISH_STARTED=0 SPAWN_TASK_SET_LOCK= SPAWN_TASK_SET_LOCK_HELD=0 +TREEHOUSE_LEASE_ABORT_CLEANUP=0 +WT_LEASED= RELAUNCH_REPLACEMENT_PENDING=0 RELAUNCH_REPLACEMENT_BUSY_GEN= RELAUNCH_REPLACEMENT_HARNESS= @@ -692,6 +694,15 @@ parse_orca_worktree_result() { spawn_abort_cleanup() { local status=$? + if [ "$TREEHOUSE_LEASE_ABORT_CLEANUP" = 1 ]; then + TREEHOUSE_LEASE_ABORT_CLEANUP=0 + if [ -n "$WT_LEASED" ] && [ -n "$PROJ_ABS" ]; then + if ! ( cd "$PROJ_ABS" && treehouse return --force "$WT_LEASED" >/dev/null 2>&1 ); then + echo "warning: could not release the treehouse lease $WT_LEASED for aborted spawn $ID; the pool slot stays reserved until it is returned by hand" >&2 + fi + fi + WT_LEASED= + fi if [ "$RELAUNCH_REPLACEMENT_PENDING" = 1 ] \ && [ "$SPAWN_META_PUBLISH_STARTED" = 1 ] \ && [ -n "$SPAWN_META_TMP" ] \ @@ -1865,7 +1876,7 @@ case "$BACKEND" in # #134 robustness (tmux): fm_backend_tmux_create_task captures a stable window # id and pins the window name (automatic-rename/allow-rename off) so a captain's # non-default tmux config cannot rename the window away from fm- once - # treehouse cd's into the worktree. WT_TARGET carries that stable id for the + # the pane cds into its leased worktree. WT_TARGET carries that stable id for the # rename-critical worktree-detection steps below; the persisted window= handle # stays $T (the name form), which is safe now that rename is disabled. WID=$(fm_backend_tmux_create_task "$SES" "$W" "$PROJ_ABS") || exit 1 @@ -2097,8 +2108,8 @@ fi # #134 robustness: only tmux needs a worktree-detection target distinct from $T - # its rename-safe stable window id, set as WT_TARGET=$WID in the tmux branch above. # Every other backend addresses its pane/surface by the id already in $T, so default -# WT_TARGET to $T for them (and for any future backend) - the shared treehouse-get + -# worktree-detection steps below must never reference an unbound WT_TARGET under set -u. +# WT_TARGET to $T for them (and for any future backend) - the shared worktree-lease + +# worktree-settle steps below must never reference an unbound WT_TARGET under set -u. : "${WT_TARGET:=$T}" spawn_send_text_line() { # case "$BACKEND" in @@ -2212,36 +2223,58 @@ if [ "$RELAUNCH" -eq 1 ]; then fi [ "$KIND" = secondmate ] || validate_spawn_worktree "relaunch" "$T" elif [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then - spawn_send_text_line "$WT_TARGET" 'treehouse get' + # Durable slot ownership: firstmate itself leases the worktree with + # `treehouse get --lease --lease-holder ` instead of asking the + # pane's shell to run an interactive `treehouse get`. An interactive get + # holds its slot only while processes still run inside it, so a parked + # worker whose occupant processes went quiet could see its slot handed to a + # later spawn while both tasks stayed recorded as live - two workers in one + # worktree (#2754). A lease survives process death until teardown's + # `treehouse return --force` releases it, which is exactly this task's + # ownership lifetime. Run from the project clone: treehouse resolves the + # pool from the working directory. Stderr passes through so an update-check + # banner or allocator error is visible in the spawn output; stdout must be + # only the leased absolute path. + if ! WT=$(cd "$PROJ_ABS" && treehouse get --lease --lease-holder "$ID"); then + echo "error: treehouse get --lease failed for $ID; refusing to launch into an unleased pool slot; inspect window $T" >&2 + exit 1 + fi + WT=${WT%%$'\n'*} + if [ -z "$WT" ]; then + echo "error: treehouse get --lease did not report a worktree path for $ID; refusing to launch; inspect window $T" >&2 + exit 1 + fi + # From here until the task record is published, an abort must release the + # lease again - an unreleased lease would silently shrink the pool, since + # nothing else knows the slot was taken (spawn_abort_cleanup owns it). + WT_LEASED=$WT + TREEHOUSE_LEASE_ABORT_CLEANUP=1 - # Wait for the treehouse subshell: the pane's cwd moves from the project to the worktree. + # Move the pane into the exact slot this task leased, then wait for the + # shell to settle there before anything records or launches against it. + spawn_send_text_line "$WT_TARGET" "cd $(shell_quote "$WT")" # Target the stable window id, not the name: if the name is ever lost (e.g. an # automatic-rename slips through), display-message -t falls back to the # active client's window, which would misread firstmate's OWN pane path as the # worktree and tangle a hook into the primary checkout. The window id never lies. - # 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. # - # A single read that already differs from PROJ_ABS_REAL is not proof the pane - # settled there: on some tmux/WSL setups a brand-new window's pane_current_path - # transiently reports an unrelated stale path (seen live as another real git - # checkout entirely) before the shell catches up with treehouse get's cd. That - # stale path still passes the PROJ_ABS_REAL comparison and validate_spawn_worktree - # below (it resolves to a real, distinct worktree top-level too), so accepting it - # on one read alone silently records the wrong worktree= in state/.meta. Require - # two consecutive reads to agree on the same non-project path before accepting it; - # a mismatch just becomes the new candidate rather than resetting the wait, so a - # pane that is already settled by the first real read only costs the one existing - # inter-poll sleep as confirmation, not a whole extra cycle on top. + # A single read equal to the leased path is not proof the pane settled: + # on some tmux/WSL setups a brand-new window's pane_current_path transiently + # reports an unrelated stale path (seen live as another real git checkout + # entirely) before the shell catches up with the cd. Require two consecutive + # reads to agree on the leased path before accepting it; any other read resets + # the candidate, so a pane that is already settled by the first real read only + # costs the one existing inter-poll sleep as confirmation, not a whole extra + # cycle on top. Physical comparison via real_path_or_raw keeps a symlinked + # project prefix from reading as a different path than the pane's OS-level cwd. + wt_leased_real=$(real_path_or_raw "$WT") candidate="" for _ in $(seq 1 60); do p=$(spawn_current_path "$WT_TARGET" || true) if [ -n "$p" ]; then p_real=$(real_path_or_raw "$p") - if [ "$p_real" != "$PROJ_ABS_REAL" ]; then - if [ -n "$candidate" ] && [ "$p_real" = "$candidate" ]; then - WT="$p" + if [ "$p_real" = "$wt_leased_real" ]; then + if [ -n "$candidate" ] && [ "$candidate" = "$p_real" ]; then break fi candidate="$p_real" @@ -2253,12 +2286,12 @@ elif [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then fi sleep 1 done - if [ -z "$WT" ]; then - echo "error: treehouse get did not enter a worktree within 60s; inspect window $T" >&2 + if [ -z "$candidate" ] || [ "$candidate" != "$wt_leased_real" ]; then + echo "error: pane did not enter its leased worktree '$WT' within 60s; inspect window $T" >&2 exit 1 fi - validate_spawn_worktree "treehouse get" "$T" + validate_spawn_worktree "treehouse lease" "$T" fi if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ]; then freshen_spawn_worktree_base "$WT" || exit 1 @@ -2690,6 +2723,14 @@ preserve_relaunch_meta() { if [ "$RELAUNCH" -eq 1 ]; then SPAWN_META_PUBLISH_STARTED=1 mv -f "$SPAWN_META_TMP" "$STATE/$ID.meta" +fi +# The published record now owns the slot through its teardown's +# `treehouse return --force`, so an abort here must NOT release the lease - +# that would hand a recorded live task's worktree back to the pool (#2754). +TREEHOUSE_LEASE_ABORT_CLEANUP=0 +WT_LEASED= +[ "$BACKEND" = orca ] && ORCA_ABORT_CLEANUP=0 +if [ "$RELAUNCH" -eq 1 ]; then RELAUNCH_REPLACEMENT_PENDING=0 SPAWN_META_PUBLISH_STARTED=0 SPAWN_META_TMP= @@ -2703,7 +2744,6 @@ if [ "$SPAWN_TASK_SET_LOCK_HELD" = 1 ]; then SPAWN_TASK_SET_LOCK_HELD=0 fm_lock_release "$SPAWN_TASK_SET_LOCK" fi -[ "$BACKEND" = orca ] && ORCA_ABORT_CLEANUP=0 sq_brief=$(shell_quote "$BRIEF") sq_turnend=$(shell_quote "$TURNEND") diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index f6b6749ef8..58354600cf 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -204,6 +204,7 @@ family_for_basename() { fm-herdr-session-cleanup.test.sh|fm-send-resolve-key.test.sh|fm-send-strict.test.sh|fm-spawn-batch.test.sh|\ fm-spawn-dispatch-profile.test.sh|\ fm-trace-context-spawn.test.sh|fm-spawn-worktree-settle.test.sh|\ + fm-spawn-worktree-lease.test.sh|\ fm-teardown-endpoint-safety.test.sh) printf '%s\n' backend-dispatch ;; diff --git a/docs/cmux-backend.md b/docs/cmux-backend.md index 8f54d57750..266917b47a 100644 --- a/docs/cmux-backend.md +++ b/docs/cmux-backend.md @@ -87,7 +87,7 @@ A genuinely fresh surface returns an internal error from `read-screen` until som Target readiness therefore uses the structural `list-panes` response instead of a content read. Capture remains bounded and locally trimmed after `read-screen` becomes available. -`current_directory` follows a top-level shell `cd` but not the foreground subshell opened by `treehouse get`. +`current_directory` follows a top-level shell `cd`, which is how spawn moves the pane into its leased worktree; it does not follow nested foreground subshells. Spawn-time worktree discovery sends begin and end markers around `pwd`, captures the marked block, and joins wrapped path lines. Literal send and Enter are separate calls. diff --git a/docs/zellij-backend.md b/docs/zellij-backend.md index 63f8dec7a2..8f87c18b7d 100644 --- a/docs/zellij-backend.md +++ b/docs/zellij-backend.md @@ -66,7 +66,7 @@ A pane can still disappear between verification and the operation; downstream su Every pane operation passes an explicit `--pane-id` because a new session can focus its release-notes plugin pane, whose numeric plugin id is in a separate namespace from terminal pane ids. -`pane_cwd` follows a top-level shell `cd` but not the foreground subshell opened by `treehouse get`. +`pane_cwd` follows a top-level shell `cd`, which is how spawn moves the pane into its leased worktree; it does not follow nested foreground subshells. Worktree discovery therefore sends begin and end markers around `pwd`, captures the marked block, and joins wrapped path lines. This active probe is scoped to spawn-time worktree discovery and is not advertised as a general live-cwd API. diff --git a/tests/fm-backend-herdr-presentation-e2e.test.sh b/tests/fm-backend-herdr-presentation-e2e.test.sh index ac62abb427..be36357426 100755 --- a/tests/fm-backend-herdr-presentation-e2e.test.sh +++ b/tests/fm-backend-herdr-presentation-e2e.test.sh @@ -207,8 +207,25 @@ set -u done printf '\n' } >> "$TREEHOUSE_CALL_LOG" -if [ -d "$POST_CREATE_ABORT_CONTROL" ] && [ "${1:-}" = get ]; then - exit 0 +if [ -d "$POST_CREATE_ABORT_CONTROL" ]; then + case "${1:-}" in + get) + # fm-spawn executes the pool acquire itself since the slot-lease fix: + # answer `get --lease` with exactly the bogus cwd the wrapped pane get + # reports for these tasks, so the armed abort still happens at worktree + # validation ("did not yield an isolated worktree") instead of early at + # an empty acquisition. + if [ "${2:-}" = --lease ]; then + printf '%s\n' "$POST_CREATE_ABORT_CONTROL/not-a-worktree" + fi + exit 0 + ;; + return) + # The post-validation abort releases its lease best-effort; there is no + # real pool slot behind the bogus path to return. + exit 0 + ;; + esac fi exec "$REAL_TREEHOUSE" "$@" SH diff --git a/tests/fm-backend.test.sh b/tests/fm-backend.test.sh index ece981b122..e8055586fd 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -785,7 +785,20 @@ esac exit 0 SH chmod +x "$fb/tmux" - fm_fake_exit0 "$fb" treehouse + cat > "$fb/treehouse" < "$fb/treehouse" < ## Done EOF fakebin=$(fm_fakebin "$home") - fm_fake_exit0 "$fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$home" } @@ -591,7 +592,8 @@ test_secondmate_hold_stays_in_authoritative_home() { ## Done EOF fakebin=$(fm_fakebin "$mate") - fm_fake_exit0 "$fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$fakebin" origin=sample-mate-review mkdir -p "$mate/data/$origin" tasks_in "$mate" add "$origin" "Investigate secondmate sample" --kind scout --repo sample --start >/dev/null diff --git a/tests/fm-gate-refuse.test.sh b/tests/fm-gate-refuse.test.sh index 6f258ae751..0827f60c30 100755 --- a/tests/fm-gate-refuse.test.sh +++ b/tests/fm-gate-refuse.test.sh @@ -152,7 +152,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/fm-grok-harness.test.sh b/tests/fm-grok-harness.test.sh index 957c0f1c77..893382e480 100755 --- a/tests/fm-grok-harness.test.sh +++ b/tests/fm-grok-harness.test.sh @@ -26,7 +26,8 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse gh-axi gh + fm_fake_exit0 "$fakebin" gh-axi gh + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/fm-kimi-harness.test.sh b/tests/fm-kimi-harness.test.sh index 768ee79991..c5dfd4d24f 100755 --- a/tests/fm-kimi-harness.test.sh +++ b/tests/fm-kimi-harness.test.sh @@ -131,7 +131,8 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse gh-axi gh + fm_fake_exit0 "$fakebin" gh-axi gh + fm_fake_treehouse_lease "$fakebin" fm_fake_exit0 "$fakebin" kimi ln -s "$JQ_BIN" "$fakebin/jq" printf '%s\n' "$fakebin" diff --git a/tests/fm-muse-harness.test.sh b/tests/fm-muse-harness.test.sh index 83a0747458..2ce32eb16e 100755 --- a/tests/fm-muse-harness.test.sh +++ b/tests/fm-muse-harness.test.sh @@ -112,7 +112,8 @@ set -u exec "$FM_FAKE_MUSE_VERSIONED" -c 'result=$($FM_FAKE_HARNESS_PROBE); printf "%s" "$result" > "$FM_FAKE_HARNESS_RESULT"' SH chmod +x "$fakebin/muse" - fm_fake_exit0 "$fakebin" treehouse gh-axi gh + fm_fake_exit0 "$fakebin" gh-axi gh + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/fm-public-followup.test.sh b/tests/fm-public-followup.test.sh index fe15e239e2..6cb9bc4dce 100755 --- a/tests/fm-public-followup.test.sh +++ b/tests/fm-public-followup.test.sh @@ -87,7 +87,8 @@ make_home() { # [relay-on|relay-off] EOF [ "$relay" = relay-off ] || printf 'FMX_PAIRING_TOKEN=test-token\n' > "$home/.env" make_fake_curl "$home" >/dev/null - fm_fake_exit0 "$home/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$home/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$home/fakebin" printf '%s\n' "$home" } @@ -718,7 +719,8 @@ test_secondmate_teardown_resolves_parent_from_durable_record_when_env_lost() { child=$(cd "$child" && pwd -P) parent_resolved=$(cd "$parent" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$child/fakebin" assert_local_secondmate_parent_record "$child" "$parent_resolved" @@ -755,7 +757,8 @@ test_secondmate_teardown_durable_record_missing_parent_registration_still_refuse child=$(cd "$child" && pwd -P) parent_resolved=$(cd "$parent" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$child/fakebin" assert_local_secondmate_parent_record "$child" "$parent_resolved" fm_write_meta "$child/state/work-child.meta" \ "window=firstmate:fm-work-child" "endpoint_task_id=work-child" \ @@ -786,7 +789,8 @@ test_secondmate_teardown_durable_record_with_unknown_field_succeeds() { child=$(cd "$child" && pwd -P) parent_resolved=$(cd "$parent" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$child/fakebin" assert_local_secondmate_parent_record "$child" "$parent_resolved" printf 'some_future_field=value\n' >> "$child/.fm-secondmate-parent" parent_alias="$TMP_ROOT/teardown-durable-clean-parent-alias" @@ -821,7 +825,8 @@ test_secondmate_teardown_rejects_conflicting_live_and_durable_parent_bindings() child=$(cd "$child" && pwd -P) parent_resolved=$(cd "$durable_parent" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$child/fakebin" assert_local_secondmate_parent_record "$child" "$parent_resolved" fm_write_meta "$durable_parent/state/mate.meta" "kind=secondmate" "home=$child" fm_git_init_commit "$child/projects/worktree" @@ -853,7 +858,8 @@ test_secondmate_teardown_rejects_unsafe_durable_parent_records() { || fail "real secondmate seeding failed for $case_name" child=$(cd "$child" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$child/fakebin" fm_write_meta "$child/state/work-child.meta" \ "window=firstmate:fm-work-child" "endpoint_task_id=work-child" \ "worktree=$child" "project=$child" "kind=ship" "mode=local-only" @@ -915,7 +921,8 @@ test_secondmate_teardown_rejects_nul_bearing_durable_parent_record() { child=$(cd "$child" && pwd -P) parent_resolved=$(cd "$parent" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi + fm_fake_treehouse_lease "$child/fakebin" assert_local_secondmate_parent_record "$child" "$parent_resolved" fm_write_meta "$parent/state/mate.meta" "kind=secondmate" "home=$child" fm_git_init_commit "$child/projects/worktree" diff --git a/tests/fm-secondmate-harness.test.sh b/tests/fm-secondmate-harness.test.sh index a3fefd8bea..e4a63041f4 100755 --- a/tests/fm-secondmate-harness.test.sh +++ b/tests/fm-secondmate-harness.test.sh @@ -630,7 +630,9 @@ meta_field() { grep "^$2=" "$1" 2>/dev/null | tail -1 | cut -d= -f2-; } # capture technique in fm-spawn-dispatch-profile.test.sh so the constructed # launch command (not just meta) can be asserted on. Also answers the # `#{pane_current_path}` probe from FM_FAKE_PANE_PATH so this same stub works -# for a crew/scout (non-secondmate) spawn's treehouse-worktree wait loop. +# for a crew/scout (non-secondmate) spawn's treehouse-worktree wait loop, and +# drops a lease-aware treehouse stub so that same non-secondmate spawn's +# `treehouse get --lease` call resolves instead of failing on a missing binary. make_launch_capturing_tmux() { local dir=$1 fakebin="$1/fakebin" mkdir -p "$fakebin" @@ -660,6 +662,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" + fm_fake_treehouse_lease "$fakebin" fm_fake_exit0 "$fakebin" pi printf '%s\n' "$fakebin" } diff --git a/tests/fm-shared-captain-inheritance.test.sh b/tests/fm-shared-captain-inheritance.test.sh index 904e8887b4..6c073b1502 100755 --- a/tests/fm-shared-captain-inheritance.test.sh +++ b/tests/fm-shared-captain-inheritance.test.sh @@ -219,7 +219,8 @@ SH # Version-aware stubs so bootstrap's tool floors stay quiet in fixture PATH. add_bootstrap_compatible_tools() { local fakebin=$1 - fm_fake_exit0 "$fakebin" node chrome-devtools-axi gh treehouse + fm_fake_exit0 "$fakebin" node chrome-devtools-axi gh + fm_fake_treehouse_lease "$fakebin" fm_fake_version_tool "$fakebin" lavish-axi FM_FAKE_LAVISH_AXI_VERSION 0.1.46 cat > "$fakebin/gh-axi" <<'SH' #!/usr/bin/env bash diff --git a/tests/fm-spawn-dispatch-profile.test.sh b/tests/fm-spawn-dispatch-profile.test.sh index d1f1effb41..49c3bd19f1 100755 --- a/tests/fm-spawn-dispatch-profile.test.sh +++ b/tests/fm-spawn-dispatch-profile.test.sh @@ -59,7 +59,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" cat > "$fakebin/timeout" <<'SH' #!/usr/bin/env bash shift diff --git a/tests/fm-spawn-pool-base-freshen.test.sh b/tests/fm-spawn-pool-base-freshen.test.sh index 8827e679d6..357997da9c 100755 --- a/tests/fm-spawn-pool-base-freshen.test.sh +++ b/tests/fm-spawn-pool-base-freshen.test.sh @@ -30,7 +30,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/fm-spawn-worktree-lease.test.sh b/tests/fm-spawn-worktree-lease.test.sh new file mode 100644 index 0000000000..4db3c19afa --- /dev/null +++ b/tests/fm-spawn-worktree-lease.test.sh @@ -0,0 +1,251 @@ +#!/usr/bin/env bash +# Regression test for issue #2754: two concurrently-live tasks must never +# receive one treehouse pool slot. +# +# The incident: fm-spawn asked the pane's shell to run an interactive +# `treehouse get`, which holds its slot only while processes still run inside +# it. A parked worker whose occupant processes went quiet left firstmate's +# recorded worktree= pointing at a slot the pool considered free, so the next +# spawn on the same project received the SAME path and did its whole job +# inside another live task's copy. +# +# The fix: fm-spawn itself acquires the slot with `treehouse get --lease +# --lease-holder `, whose durable lease survives process death until +# teardown's `treehouse return --force` releases it, then sends the pane into +# that exact path. +# +# The suite-owned fake tools model treehouse's documented allocation contract: +# a leased slot is never handed out by a later get until an explicit return; +# a plain interactive get's slot is protected only while an occupant process +# lives inside it, which the fixture models by never recording plain-get +# occupancy - exactly the parked-worker condition from the incident, under +# which the old in-pane flow handed both spawns the same slot. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +SPAWN="$ROOT/bin/fm-spawn.sh" +TMP_ROOT=$(fm_test_tmproot fm-spawn-worktree-lease) + +# make_lease_fakebin : builds the fakebin used by every case below. +# +# The treehouse stub keeps durable lease state in $FM_FAKE_TREEHOUSE_STATE: +# `get --lease --lease-holder H` records H against the first free slot and is +# never rehanded; a bare `get` allocates WITHOUT recording occupancy (transient +# process-based protection only); `return [--force] ` releases. The fake +# tmux follows commands like a real pane shell: send-keys text `cd ` +# moves the pane cwd file there, and send-keys text `treehouse get` performs +# the plain acquire before moving the pane to the slot it was handed. +# pane_current_path reads the cwd file, mirroring the OS-level read of a real +# pane. +make_lease_fakebin() { + local dir=$1 fakebin + fakebin=$(fm_fakebin "$dir") + + cat > "$fakebin/treehouse" <&2 + exit "\$FM_FAKE_LEASE_GET_STATUS" + fi + name="" path="" + while IFS='|' read -r n p; do + [ -n "\$n" ] || continue + grep -q "|\$n\$" "\$LEASES" 2>/dev/null && continue + name=\$n + path=\$p + break + done < "\$SLOTS" + if [ -z "\$name" ]; then + echo "fake treehouse: no free slot" >&2 + exit 1 + fi + if [ -n "\$holder" ]; then + printf '%s|%s\n' "\$holder" "\$name" >> "\$LEASES" + fi + printf '%s\n' "\$path" + exit 0 + ;; + return) + target="" + for a in "\${@:2}"; do + case "\$a" in --*) ;; *) target=\$a ;; esac + done + n=\$(printf '%s' "\$target" | sed 's|.*/||') + if [ -f "\$LEASES" ]; then + grep -v "|\$n\$" "\$LEASES" > "\$LEASES.tmp" 2>/dev/null || true + mv "\$LEASES.tmp" "\$LEASES" + fi + exit 0 + ;; +esac +exit 0 +SH + chmod +x "$fakebin/treehouse" + + cat > "$fakebin/tmux" < "\$CWD_FILE" + ;; + cd*) + target=\${text#cd } + case "\$target" in + \'*\') target=\${target#\'} ; target=\${target%\'} ;; + esac + printf '%s\\n' "\$target" > "\$CWD_FILE" + ;; + esac + exit 0 + ;; +esac +exit 0 +SH + chmod +x "$fakebin/tmux" + + printf '%s\n' "$fakebin" +} + +make_lease_case() { + local name=$1 id_a=$2 id_b=$3 case_dir home proj wt1 wt2 state + case_dir="$TMP_ROOT/$name" + home="$case_dir/home" + proj="$case_dir/project" + wt1="$case_dir/pool/1" + wt2="$case_dir/pool/2" + state="$case_dir/thstate" + fakebin=$(make_lease_fakebin "$case_dir/fake") + mkdir -p "$home/data" "$home/projects" "$home/state" "$home/config" "$state" "$case_dir/pool" + printf 'codex\n' > "$home/config/crew-harness" + fm_git_worktree "$proj" "$wt1" "pool-1-$name" + git -C "$proj" worktree add -q --detach "$wt2" >/dev/null 2>&1 + printf '1|%s\n2|%s\n' "$wt1" "$wt2" > "$state/slots" + : > "$state/leases" + printf '%s\n' "$proj" > "$case_dir/pane-cwd" + for id in "$id_a" "$id_b"; do + mkdir -p "$home/data/$id" + printf 'brief for %s\n' "$id" > "$home/data/$id/brief.md" + done + touch "$home/state/.last-watcher-beat" + printf '%s\n' "$case_dir|$home|$proj|$wt1|$wt2|$fakebin|$state|$case_dir/pane-cwd" +} + +read_lease_record() { + IFS='|' read -r CASE_DIR HOME_DIR PROJ_DIR WT1 WT2 FAKEBIN_DIR TH_STATE PANE_CWD <&1 +} + +# Core regression (#2754): spawning task B on the same project while task A is +# still live must never hand B task A's slot. Under the old in-pane +# `treehouse get` flow the fixture's expired transient occupancy makes BOTH +# spawns record pool/1 - the double assignment this test forbids. With the +# durable lease each spawn holds its own exclusive slot until teardown +# returns it. +test_two_live_tasks_never_share_one_slot() { + local rec id_a id_b out status + id_a=lease-live-a-m1 + id_b=lease-live-b-m2 + rec=$(make_lease_case lease-two-live "$id_a" "$id_b") + read_lease_record "$rec" + + out=$(run_lease_spawn "$id_a"); status=$? + expect_code 0 "$status" "spawn A should succeed" + assert_contains "$out" "spawned $id_a" "spawn A did not report success" + assert_grep "worktree=$WT1" "$HOME_DIR/state/$id_a.meta" \ + "spawn A should hold pool slot 1" + assert_contains "$(cat "$TH_STATE/leases")" "$id_a|1" \ + "allocator state should show a durable lease held by spawn A" + + out=$(run_lease_spawn "$id_b"); status=$? + expect_code 0 "$status" "spawn B should succeed while task A is live" + assert_grep "worktree=$WT2" "$HOME_DIR/state/$id_b.meta" \ + "spawn B must receive its own distinct leased slot" + assert_no_grep "worktree=$WT1" "$HOME_DIR/state/$id_b.meta" \ + "spawn B must never be recorded inside spawn A's slot" + assert_contains "$(cat "$TH_STATE/leases")" "$id_b|2" \ + "allocator state should show a durable lease held by spawn B" + pass "two concurrently-live tasks never share one pool slot (durable leases)" +} + +# An allocator refusal must stop the spawn loudly before any worker launches, +# rather than falling back to an unleased or shared slot. +test_lease_refusal_stops_the_spawn() { + local rec id out status + id=lease-refused-n3 + rec=$(make_lease_case lease-refused "$id" unused-b) + read_lease_record "$rec" + + out=$(FM_FAKE_LEASE_GET_STATUS=28 run_lease_spawn "$id"); status=$? + expect_code 1 "$status" "a refused lease must abort the spawn" + assert_contains "$out" "--lease" "refusal error should name the lease acquisition" + [ ! -f "$HOME_DIR/state/$id.meta" ] || fail "refused spawn must not publish meta" + pass "an allocator refusal stops the spawn before launch (fail closed)" +} + +# A spawn that aborts AFTER acquiring its lease (here: the pooled worktree's +# base refresh refuses an unreachable origin) must release the lease again, so +# a failed launch cannot silently shrink the pool. +test_aborted_spawn_releases_its_lease() { + local rec id out status + id=lease-abort-release-p4 + rec=$(make_lease_case lease-abort "$id" unused-b) + read_lease_record "$rec" + git -C "$WT1" remote set-url origin "file://$CASE_DIR/missing-origin.git" + + out=$(run_lease_spawn "$id"); status=$? + expect_code 1 "$status" "an unreachable origin must abort the spawn after the lease" + assert_contains "$out" "could not fetch origin" \ + "abort did not name the base-refresh refusal" + [ ! -s "$TH_STATE/leases" ] || fail "an aborted spawn must release its treehouse lease" + pass "a spawn aborted after acquisition releases its pool slot lease" +} + +test_two_live_tasks_never_share_one_slot +test_lease_refusal_stops_the_spawn +test_aborted_spawn_releases_its_lease + +echo "# all fm-spawn-worktree-lease tests passed" diff --git a/tests/fm-spawn-worktree-settle.test.sh b/tests/fm-spawn-worktree-settle.test.sh index 66f3c837af..43e991e408 100755 --- a/tests/fm-spawn-worktree-settle.test.sh +++ b/tests/fm-spawn-worktree-settle.test.sh @@ -1,17 +1,16 @@ #!/usr/bin/env bash -# Regression test for the fm-spawn.sh treehouse-get worktree-detection settle -# loop (bin/fm-spawn.sh, the `for _ in $(seq 1 60)` loop after `treehouse get`). +# Regression test for the fm-spawn.sh leased-worktree settle +# loop (bin/fm-spawn.sh, the `for _ in $(seq 1 60)` loop after the pane is +# sent into its leased treehouse slot). # # On some tmux/WSL setups a brand-new window's pane_current_path transiently # reports a stale, unrelated-but-real path on the very first poll, before the -# pane actually settles into the worktree treehouse get moved it to. That stale -# path still passes the loop's "differs from the project" check and -# validate_spawn_worktree's "is a real, distinct worktree" check (it IS a real -# git checkout, just the wrong one), so a naive single-read loop silently -# records the wrong worktree= in state/.meta. This test simulates that -# transient-then-settled pane_current_path sequence with a fake tmux and -# asserts the recorded worktree resolves to the real, settled worktree, never -# the stale first read. +# pane actually settles into the leased worktree fm-spawn moved it to. That +# stale path fails the loop's equality check with the leased path, but a naive +# single-read loop could still accept any one lucky read, so this test +# simulates that transient-then-settled pane_current_path sequence with a fake +# tmux and asserts the recorded worktree resolves to the real, settled, +# leased worktree, never the stale first read. set -u # shellcheck source=tests/lib.sh @@ -54,7 +53,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/fm-tangle-guard.test.sh b/tests/fm-tangle-guard.test.sh index 64aabe6400..58c60ca4f4 100755 --- a/tests/fm-tangle-guard.test.sh +++ b/tests/fm-tangle-guard.test.sh @@ -150,8 +150,8 @@ test_brief_assertion_precedes_branch() { # --- GUARD 1b: fm-spawn isolation abort ------------------------------------- -# A fake tmux that reports FM_FAKE_PANE_PATH as the post-`treehouse get` pane cwd -# (so the spawn's worktree-resolution loop resolves to a path we control), names +# A fake tmux that reports FM_FAKE_PANE_PATH as the post-lease pane cwd +# (so the spawn's worktree-settle loop resolves to a path we control), names # the session on '#S', and swallows window ops. Echoes the fakebin dir. make_spawn_fakebin() { local dir=$1 fakebin @@ -170,7 +170,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } @@ -225,8 +225,9 @@ test_spawn_isolation_abort() { # tmux appends at the next free index instead of the active window index, which # collides under base-index 1; # - the window id is captured (-P -F #{window_id}) and automatic-rename/allow-rename -# are disabled so the fm- name survives treehouse cd'ing into the worktree; -# - the treehouse-get send-keys and the worktree wait loop target that stable +# are disabled so the fm- name survives the pane cd'ing into its leased +# worktree; +# - the leased-worktree cd send-keys and the worktree wait loop target that stable # window id, never the (possibly-renamed) name - a lost name would let # display-message fall back to the active client's window and misread firstmate's # OWN pane as the worktree, tangling a hook into the primary checkout. @@ -249,7 +250,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } @@ -293,9 +294,10 @@ test_spawn_tmux_window_construction() { assert_grep "set-window-option -t @spawnwid allow-rename off" "$rec" \ "must disable allow-rename on the spawned window" - # Bug 2 fix (b): treehouse-get and the worktree wait loop target the stable id. - assert_grep "send-keys -t @spawnwid treehouse get Enter" "$rec" \ - "treehouse get must be sent to the stable window id" + # Bug 2 fix (b): the worktree-entry cd and the worktree wait loop target the + # stable id. + assert_grep "send-keys -t @spawnwid cd '$wt' Enter" "$rec" \ + "the leased-worktree cd must be sent to the stable window id" assert_grep "display-message -p -t @spawnwid #{pane_current_path}" "$rec" \ "the worktree wait loop must query the stable window id, not the name" diff --git a/tests/fm-trace-context-spawn.test.sh b/tests/fm-trace-context-spawn.test.sh index 61c88dc3b6..117c6cf947 100755 --- a/tests/fm-trace-context-spawn.test.sh +++ b/tests/fm-trace-context-spawn.test.sh @@ -78,7 +78,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_fake_treehouse_lease "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/lib.sh b/tests/lib.sh index 915741ba0d..2dddabc854 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -187,6 +187,36 @@ SH chmod +x "$fakebin/$tool" } +# fm_fake_treehouse_lease : drop a treehouse stub implementing the +# durable-lease acquire surface bin/fm-spawn.sh depends on. `treehouse get +# --lease --lease-holder ` prints ${FM_FAKE_LEASE_PATH} - falling back to +# ${FM_FAKE_PANE_PATH}, which every spawn fixture already aims at its fake +# worktree - to stdout and exits 0; with FM_FAKE_LEASE_GET_STATUS set nonzero +# it exits with that status and prints nothing, modeling an allocator refusal. +# Every other invocation exits 0. The leased path must equal the path the fake +# pane reports, so the spawn's settle loop can confirm the pane at it. +fm_fake_treehouse_lease() { + local fakebin=$1 + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + get) + shift + [ "${1:-}" = --lease ] || exit 0 + if [ -n "${FM_FAKE_LEASE_GET_STATUS:-}" ] && [ "$FM_FAKE_LEASE_GET_STATUS" != 0 ]; then + echo "fake treehouse: lease refused" >&2 + exit "$FM_FAKE_LEASE_GET_STATUS" + fi + printf '%s\n' "${FM_FAKE_LEASE_PATH:-${FM_FAKE_PANE_PATH:-}}" + exit 0 + ;; +esac +exit 0 +SH + chmod +x "$fakebin/treehouse" +} + # --- deterministic git identity and fixtures -------------------------------- # fm_git_identity [name] [email]: export a fixed author/committer identity so