From 30754d5ecf549085e545353fb97d152bd9a7e685 Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Wed, 2 Sep 2026 17:11:47 +0530 Subject: [PATCH 1/6] fix(bin): lease and refresh the task copy before any worker shell enters it fm-spawn.sh created the task pane, typed `treehouse get` into it, and only then refreshed the copy. The new shell's git prompt and the refresh both took the worktree's index.lock under the shared .git/worktrees/, so on a large repository the refresh was refused and the spawn failed every time. Spawn now leases the copy from its own process with `treehouse get --lease`, refreshes it with no shell inside, and creates the pane already in that path on every session-provider backend (tmux, herdr, zellij, cmux). A refusal before any create call returns a clean lease to the pool; from the first create call on the lease is retained and reported, because `treehouse return` terminates every process still in the copy. A fresh spawn whose record still names a copy (a quarantined Herdr recovery) refreshes that copy instead of leasing another, and refuses when its HEAD holds commits no remote has. A reclaimed Herdr pane is sent one plain `cd` into the copy after the refresh. fm-teardown.sh closes the endpoint before its worktree process reap and the copy's return, since both would now kill the pane's own shell and skip the planned focus-preserving Herdr close. The presentation e2e audits the emptying-close plan's pane-death removal and asserts copy reuse on reclaim. --- bin/backends/herdr.sh | 18 +- bin/backends/tmux.sh | 21 +- bin/backends/zellij.sh | 10 +- bin/fm-spawn.sh | 239 +++++++++++++---- bin/fm-teardown.sh | 129 ++++----- docs/architecture.md | 5 +- docs/cmux-backend.md | 4 +- docs/zellij-backend.md | 7 +- tests/fixtures.sh | 7 +- .../fm-backend-herdr-presentation-e2e.test.sh | 44 ++- tests/fm-backend.test.sh | 16 +- tests/fm-backlog-atomicity.test.sh | 3 +- tests/fm-bootstrap-network-parallel.test.sh | 3 +- tests/fm-captain-hold-lifecycle.test.sh | 6 +- tests/fm-kimi-harness.test.sh | 3 +- tests/fm-muse-harness.test.sh | 3 +- tests/fm-public-followup.test.sh | 27 +- tests/fm-secondmate-harness.test.sh | 6 +- tests/fm-shared-captain-inheritance.test.sh | 3 +- tests/fm-spawn-lease-before-enter.test.sh | 252 ++++++++++++++++++ tests/fm-spawn-worktree-settle.test.sh | 20 +- tests/fm-tangle-guard.test.sh | 16 +- tests/fm-trace-context-spawn.test.sh | 2 +- tests/lib.sh | 21 ++ 24 files changed, 674 insertions(+), 191 deletions(-) create mode 100755 tests/fm-spawn-lease-before-enter.test.sh diff --git a/bin/backends/herdr.sh b/bin/backends/herdr.sh index 8728b356cc0..713622449e3 100644 --- a/bin/backends/herdr.sh +++ b/bin/backends/herdr.sh @@ -2520,17 +2520,15 @@ fm_backend_herdr_target_ready() { # } # fm_backend_herdr_current_path: the live FOREGROUND process's cwd, or empty on -# any error. Mirrors tmux's pane_current_path poll used for worktree-path -# discovery after `treehouse get`. +# any error. Mirrors tmux's pane_current_path poll fm-spawn.sh uses to confirm +# the pane's shell sits in the leased worktree it was created in. # # Verified pitfall: `pane get`'s `.result.pane.cwd` is the pane's cwd AT # CREATION TIME - the top-level shell's cwd - and does NOT update when that -# shell `cd`s or enters a subshell (as `treehouse get` does). Reading it here -# would make fm-spawn.sh's worktree-discovery poll never see the pane "leave" -# the project directory, since `cwd` stays frozen at the original path forever. -# `.result.pane.foreground_cwd` tracks the ACTUALLY RUNNING foreground -# process's cwd instead, which is what changes when `treehouse get` enters its -# worktree subshell - confirmed live against a real treehouse acquisition. +# shell `cd`s or enters a subshell. `.result.pane.foreground_cwd` tracks the +# ACTUALLY RUNNING foreground process's cwd instead - confirmed live against a +# real treehouse subshell - so it is the read that reports where the shell +# really is rather than where the pane was opened. fm_backend_herdr_current_path() { # fm_backend_herdr_target_ready "$1" || return 0 fm_backend_herdr_cli "$FM_BACKEND_HERDR_SESSION" pane get "$FM_BACKEND_HERDR_PANE" 2>/dev/null \ @@ -2539,8 +2537,8 @@ fm_backend_herdr_current_path() { # # fm_backend_herdr_send_text_line: send one line of TEXT then submit, # ATOMICALLY - mirrors tmux's `send-keys -t T text Enter`. Used for the fixed -# spawn-time commands (treehouse get, the GOTMPDIR export). `pane run` types -# the command and submits it in one call (verified). +# spawn-time commands (the GOTMPDIR export). `pane run` types the command and +# submits it in one call (verified). fm_backend_herdr_send_text_line() { # fm_backend_herdr_target_ready "$1" || return 1 fm_backend_herdr_cli "$FM_BACKEND_HERDR_SESSION" pane run "$FM_BACKEND_HERDR_PANE" "$2" >/dev/null 2>&1 diff --git a/bin/backends/tmux.sh b/bin/backends/tmux.sh index 9eed5f3ec3e..76566985f3c 100644 --- a/bin/backends/tmux.sh +++ b/bin/backends/tmux.sh @@ -8,10 +8,10 @@ # default (tmux, `backend=` absent) path stays byte-identical. Sourced only # through bin/fm-backend.sh's fm_backend_source, never directly. # -# Worktree acquisition (running `treehouse get` inside the pane, and polling -# its cwd) is unchanged by this extraction: P1 scopes only the session -# provider, not the worktree provider, so fm-spawn.sh still drives that part -# inline with these same send/current-path primitives. +# Worktree acquisition is not this adapter's concern: bin/fm-spawn.sh leases +# and refreshes the copy itself, creates the window with that path as its +# starting directory, and confirms the pane's cwd through the current-path +# primitive here (fm-spawn.sh's header owns that order). # # The verified composer/busy-detection and verify-and-retry-submit primitives # already live in bin/fm-tmux-lib.sh, shared with the away-mode daemon @@ -82,11 +82,12 @@ fm_backend_tmux_container_ensure() { # at the next free index by targeting the session with a trailing colon # ("$ses:"), so a non-default base-index (e.g. base-index 1) cannot collide. # - PIN the window name by disabling automatic-rename and allow-rename on the -# new window: the captain's tmux may rename the window away from fm- once -# treehouse cd's into the worktree, which would break name-based targeting. +# new window: the captain's tmux may rename the window away from fm- as +# soon as its shell starts in the worktree, which would break name-based +# targeting. # The returned window id lets callers target the window even if its name is ever # lost, so worktree discovery cannot fall back to the active client's window. -fm_backend_tmux_create_task() { # -> prints window id +fm_backend_tmux_create_task() { # -> prints window id local ses=$1 wname=$2 proj_abs=$3 wid if tmux list-windows -t "$ses" -F '#{window_name}' | grep -qx "$wname"; then echo "error: window $ses:$wname already exists" >&2 @@ -106,9 +107,9 @@ fm_backend_tmux_current_path() { # } # fm_backend_tmux_send_text_line: send one line of TEXT then Enter, with no -# composer verification - used for the fixed spawn-time commands -# (`treehouse get`, the GOTMPDIR export) that already ran this exact sequence -# inline in fm-spawn.sh. Mirrors `tmux send-keys -t "$T" "" Enter`. +# composer verification - used for the fixed spawn-time commands (the GOTMPDIR +# export) that already ran this exact sequence inline in fm-spawn.sh. Mirrors +# `tmux send-keys -t "$T" "" Enter`. fm_backend_tmux_send_text_line() { # tmux send-keys -t "$1" "$2" Enter } diff --git a/bin/backends/zellij.sh b/bin/backends/zellij.sh index 56478f7db35..b05c4d080f5 100644 --- a/bin/backends/zellij.sh +++ b/bin/backends/zellij.sh @@ -387,15 +387,15 @@ fm_backend_zellij_target_ready() { # [expected-label] } # fm_backend_zellij_current_path: the live pane's cwd, or empty on any error. -# Mirrors tmux's pane_current_path poll used for worktree-path discovery after -# `treehouse get`. +# Mirrors tmux's pane_current_path poll fm-spawn.sh uses to confirm the pane's +# shell sits in the leased worktree it was created in. # # Verified pitfall (docs/zellij-backend.md "Worktree-path discovery: pane_cwd # does not track a subshell"): `list-panes --json`'s `pane_cwd` DOES reflect a # `cd` run directly in the pane's own top-level shell, but stays FROZEN at -# whatever directory the pane's shell was in when it launched `treehouse get` -# as a foreground command - it never follows that command's own internal `cd` -# into the acquired worktree, even after the subshell is fully interactive and +# whatever directory the pane's shell was in when it launched a foreground +# command such as `treehouse get` - it never follows that command's own internal +# `cd` into the acquired worktree, even after the subshell is fully interactive and # a `pwd` typed into it prints the correct live path on screen. Zellij's CLI # exposes no per-pane pid and no live-process cwd field to read instead # (unlike herdr's `foreground_cwd`), so passive JSON polling cannot solve diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 9158fce64df..5940b621285 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -48,8 +48,8 @@ # 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 -# session provider only, exactly like herdr/zellij, so it does. An +# terminal, so ship/scout Orca spawns never lease a treehouse copy; cmux is a +# session provider only, exactly like tmux/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. # codex-app is not a known backend yet; docs/codex-app-backend.md owns that @@ -138,6 +138,35 @@ # origin, resolves the current remote default branch, and resets to its tip. # An unreachable origin, unresolved default branch, or non-clean worktree # refuses the spawn rather than risking a PR based on stale history. +# This script is the one owner of the ORDER of that sequence on every +# session-provider backend (tmux, herdr, zellij, cmux): the copy is leased +# from this process with `treehouse get --lease` (no pane, no shell), the +# refresh above runs on that leased path, and only then is the task pane +# created with the leased path as its starting directory. No `treehouse get` +# is ever typed into the pane; the one exception is a reclaimed Herdr pane +# that already existed, which is sent a plain `cd` into the copy only after +# the refresh. A fresh spawn whose durable record still names a pooled copy +# (a quarantined Herdr recovery) refreshes that same copy instead of leasing +# another, and refuses when its HEAD holds commits no remote has, so a +# retry never grows the pool or resets a prior worker's unlanded commits. +# The old order (create the pane, enter the copy +# from its shell, then refresh) raced a fresh shell's git prompt against the +# refresh for the worktree's index.lock under the shared .git/worktrees/, and +# refused the spawn every time on a large repository; the worker shell is +# simply not in the copy yet when the refresh runs. The pane is then polled +# once a second until its live cwd resolves to the leased path before launch; +# FM_SPAWN_SETTLE_POLLS bounds that wait (default 60). +# When a fresh spawn stops after the lease but BEFORE any backend endpoint +# creation has begun with the leased path as its cwd, a clean leased copy is +# returned to the pool so a refused launch cannot shrink the pool; a copy +# holding uncommitted work stays leased, untouched, and its path is reported. +# From the first create call onward the lease is retained: `treehouse return` +# terminates every process still inside the copy, so returning it after a +# pane, tab, or workspace was opened there would kill a shell the refusal +# deliberately left in place (a quarantined Herdr projection, an +# inspectable endpoint). A refusal in that window reports the retained path +# and its release command, and bin/fm-teardown.sh's `treehouse return` +# releases a launched task's lease. # A slot whose only deviation is a stale submodule gitlink is refused by that # same clean check, but is reported as a stale checkout naming each submodule # and both pins; nothing is converged or removed, and no remedy is suggested. @@ -727,6 +756,9 @@ RELAUNCH_REPLACEMENT_STATE= RELAUNCH_REPLACEMENT_WT= CONFIG_INHERIT_LOCK= CONFIG_INHERIT_LOCK_HELD=0 +SPAWN_LEASE_RETURN_PENDING=0 +SPAWN_LEASE_WT= +HERDR_RECLAIMED_PANE=0 spawn_fresh_commit_rollback() { if fm_backlog_atomic_transition rollback "$STATE/$ID.meta" \ @@ -838,6 +870,12 @@ spawn_abort_cleanup() { fi fi fi + if [ "$SPAWN_LEASE_RETURN_PENDING" = 1 ]; then + SPAWN_LEASE_RETURN_PENDING=0 + spawn_lease_return_on_abort "$SPAWN_LEASE_WT" || true + elif [ "$status" -ne 0 ] && [ -n "$SPAWN_LEASE_WT" ]; then + echo "warning: leased copy '$SPAWN_LEASE_WT' is retained because an endpoint may already sit inside it; inspect it, then release it with: (cd '$PROJ_ABS' && treehouse return --force '$SPAWN_LEASE_WT')" >&2 + fi if [ "$SPAWN_TASK_LOCK_HELD" = 1 ]; then SPAWN_TASK_LOCK_HELD=0 fm_lock_release "$SPAWN_TASK_LOCK" || true @@ -1938,6 +1976,93 @@ freshen_spawn_worktree_base() { # fi } +# Lease a pooled copy of the project from THIS process, with no pane and no +# shell anywhere near it. `treehouse get --lease` prints exactly the absolute +# path on stdout; its banners go to stderr and are shown only on failure. +spawn_lease_pool_worktree() { # -> prints the leased worktree path + local out err_file lines + err_file=$(mktemp "${TMPDIR:-/tmp}/fm-spawn-lease.XXXXXX") || return 1 + if ! out=$(cd "$PROJ_ABS" && treehouse get --lease --lease-holder "$W" 2>"$err_file"); then + echo "error: treehouse get --lease failed for project '$PROJ_ABS'; no worker was started" >&2 + cat "$err_file" >&2 2>/dev/null || true + rm -f "$err_file" + return 1 + fi + rm -f "$err_file" + lines=$(printf '%s\n' "$out" | grep -c . || true) + case "$out" in + /*) ;; + *) lines=0 ;; + esac + if [ "$lines" != 1 ] || [ ! -d "$out" ]; then + echo "error: treehouse get --lease did not report exactly one existing absolute worktree path for project '$PROJ_ABS' (got '${out:-nothing}'); no worker was started" >&2 + return 1 + fi + printf '%s\n' "$out" +} + +# A fresh spawn that stops after leasing but before any endpoint exists hands +# a CLEAN copy straight back so a refused launch cannot shrink the pool. A copy +# holding uncommitted work is never reset here: it stays leased (prune cannot +# recycle it) and its path is reported. The primary checkout is never returned. +spawn_lease_return_on_abort() { # + local worktree=$1 wt_real status + [ -n "$worktree" ] || return 0 + wt_real=$(real_path_or_raw "$worktree") + if [ "$wt_real" = "$PROJ_ABS_REAL" ]; then + echo "warning: leased path '$worktree' resolves to the primary checkout; not returning it" >&2 + return 1 + fi + status=$(git -C "$worktree" -c core.quotePath=false status --porcelain 2>/dev/null) || { + echo "warning: could not inspect leased copy '$worktree'; leaving it leased - release it with: (cd '$PROJ_ABS' && treehouse return '$worktree')" >&2 + return 1 + } + if [ -n "$status" ]; then + echo "warning: leased copy '$worktree' holds uncommitted work; leaving it leased and untouched - inspect it, then release it with: (cd '$PROJ_ABS' && treehouse return '$worktree')" >&2 + return 1 + fi + if ! (cd "$PROJ_ABS" && treehouse return --force "$worktree") >/dev/null 2>&1; then + echo "warning: could not return leased copy '$worktree' to the pool; release it with: (cd '$PROJ_ABS' && treehouse return --force '$worktree')" >&2 + return 1 + fi +} + +# A fresh spawn for a task id whose durable record still names a pooled copy +# (a quarantined Herdr recovery, a relaunch-shaped resume) reuses that copy +# rather than leasing a second one per attempt: the record's copy is already +# this task's lease. Prints the path only when it is a real directory distinct +# from the primary checkout. +spawn_prior_record_worktree() { # -> prints the record's worktree path + local prior + prior=$(herdr_projection_meta_field_exact "$STATE/$ID.meta" worktree 2>/dev/null) || return 1 + [ -n "$prior" ] && [ -d "$prior" ] || return 1 + [ "$(real_path_or_raw "$prior")" != "$PROJ_ABS_REAL" ] || return 1 + printf '%s\n' "$prior" +} + +# A reused copy is refreshed under the same clean check as a leased one, and +# additionally refuses when its HEAD carries commits that no remote holds: +# the refresh would reset those away, and they may be the prior worker's +# unlanded work. +spawn_refuse_unpushed_head() { # + local worktree=$1 unpushed + unpushed=$(git -C "$worktree" log --format=%H --max-count=1 HEAD --not --remotes -- 2>/dev/null) || { + echo "error: could not inspect prior copy '$worktree' for unpushed commits; refusing to refresh it" >&2 + return 1 + } + if [ -n "$unpushed" ]; then + echo "error: prior copy '$worktree' for $ID holds commits not on any remote; refusing to refresh over that work - land or discard it first" >&2 + return 1 + fi +} + +# The first backend call that opens anything with the leased path as its cwd +# ends the return-on-abort window (header): from here a refusal retains the +# lease rather than terminating whatever that call left inside the copy. +spawn_lease_endpoint_begins() { + SPAWN_LEASE_RETURN_PENDING=0 +} + herdr_projection_meta_field_exact() { # local meta=$1 key=$2 count [ -f "$meta" ] && [ ! -L "$meta" ] || return 1 @@ -2067,17 +2192,37 @@ if [ "$RELAUNCH" -eq 1 ]; then WT_TARGET=$T SES=${T%%:*} else +# Lease and refresh the task copy BEFORE any endpoint exists (header: the pane +# is created already inside it; nothing else may enter the copy first). Orca +# owns its own worktree and a secondmate launches in its home, so both keep +# the project or home path as the pane's starting directory. +PANE_CWD=$PROJ_ABS +if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then + if WT=$(spawn_prior_record_worktree); then + # The record's own copy: never returned by this spawn, refreshed in place. + validate_spawn_worktree "prior record" "(no endpoint created yet)" + spawn_refuse_unpushed_head "$WT" || exit 1 + else + WT=$(spawn_lease_pool_worktree) || exit 1 + SPAWN_LEASE_WT=$WT + SPAWN_LEASE_RETURN_PENDING=1 + validate_spawn_worktree "treehouse get --lease" "(no endpoint created yet)" + fi + freshen_spawn_worktree_base "$WT" || exit 1 + PANE_CWD=$WT +fi case "$BACKEND" in tmux) SES=$(fm_backend_tmux_container_ensure) T="$SES:$W" # #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 + # non-default tmux config cannot rename the window away from fm- as soon + # as its shell starts in the 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 + spawn_lease_endpoint_begins + WID=$(fm_backend_tmux_create_task "$SES" "$W" "$PANE_CWD") || exit 1 WT_TARGET="$WID" ;; herdr) @@ -2134,6 +2279,10 @@ case "$BACKEND" in set -e case "$HERDR_RECLAIM_STATUS" in 0) + # The reclaimed pane already existed, so it was not created + # inside the copy; the settle step below sends it one plain + # `cd` there after the refresh (header). + HERDR_RECLAIMED_PANE=1 HERDR_PROJECTED=1 HERDR_WORKSPACE_ID=$HERDR_RECOVERY_WORKSPACE_ID HERDR_SEEDED_DEFAULT_TAB_ID="" @@ -2182,8 +2331,9 @@ case "$BACKEND" in else HERDR_PROJECTION_ID=$(fm_backend_herdr_projection_journal_create "$STATE" "$ID") || exit 1 HERDR_PROJECTION_LABEL=$(fm_backend_herdr_projection_workspace_label "$ID" "$HERDR_PROJECTION_ID") + spawn_lease_endpoint_begins if ! FM_HOME="$HERDR_LABEL_HOME" fm_backend_herdr_projection_create_task \ - "$PROJ_ABS" "$HERDR_PROJECTION_LABEL" "$W"; then + "$PANE_CWD" "$HERDR_PROJECTION_LABEL" "$W"; then if [ "${FM_BACKEND_HERDR_PROJECTION_CLEANUP_SAFE:-0}" = 1 ]; then HERDR_PROJECTION_ABORT_CLEANUP=1 HERDR_PROJECTION_ABORT_SESSION=$FM_BACKEND_HERDR_PROJECTION_SESSION @@ -2236,7 +2386,8 @@ case "$BACKEND" in HERDR_SEEDED_DEFAULT_TAB_ID=${HERDR_CONTAINER_RAW#*$'\t'} HERDR_SES=${CONTAINER%%:*} HERDR_WORKSPACE_ID=${CONTAINER#*:} - HERDR_TASK_IDS=$(FM_HOME="$HERDR_LABEL_HOME" fm_backend_herdr_create_task "$CONTAINER" "$W" "$PROJ_ABS" "$HERDR_SEEDED_DEFAULT_TAB_ID") || exit 1 + spawn_lease_endpoint_begins + HERDR_TASK_IDS=$(FM_HOME="$HERDR_LABEL_HOME" fm_backend_herdr_create_task "$CONTAINER" "$W" "$PANE_CWD" "$HERDR_SEEDED_DEFAULT_TAB_ID") || exit 1 read -r HERDR_TAB_ID HERDR_PANE_ID < 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. + # Compare physically resolved paths: a symlinked prefix would otherwise make + # the pane's OS-level cwd read differ from the leased path string forever. # - # 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. - candidate="" - for _ in $(seq 1 60); do + # A brand-new pane can transiently report an unrelated stale path (seen live + # on some tmux/WSL setups as another real git checkout entirely) before its + # shell settles. Only a read equal to the leased path is accepted, so a stale + # read costs one more poll rather than ever being recorded as the worktree. + # A reclaimed Herdr pane predates the lease, so it is moved into the copy + # with one plain `cd` now that the refresh is already complete; the copy is + # still never entered before its refresh. + if [ "$HERDR_RECLAIMED_PANE" = 1 ]; then + spawn_send_text_line "$WT_TARGET" "cd $(shell_quote "$WT")" + fi + WT_REAL=$(real_path_or_raw "$WT") + settled=0 + p= + for _ in $(seq 1 "${FM_SPAWN_SETTLE_POLLS:-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" - break - fi - candidate="$p_real" - else - candidate="" - fi - else - candidate="" + if [ -n "$p" ] && [ "$(real_path_or_raw "$p")" = "$WT_REAL" ]; then + settled=1 + break fi sleep 1 done - if [ -z "$WT" ]; then - echo "error: treehouse get did not enter a worktree within 60s; inspect window $T" >&2 + if [ "$settled" -ne 1 ]; then + echo "error: endpoint $T did not settle in the leased worktree '$WT' (last read '${p:-none}'); refusing to launch outside the refreshed copy. Inspect window $T; the lease is retained - release it with: (cd '$PROJ_ABS' && treehouse return --force '$WT')" >&2 exit 1 fi - validate_spawn_worktree "treehouse get" "$T" + validate_spawn_worktree "leased worktree" "$T" fi -if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ]; then +if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ] && [ "$BACKEND" = orca ]; then + # Orca creates its own worktree and terminal; its refresh keeps its existing + # place after terminal creation. freshen_spawn_worktree_base "$WT" || exit 1 fi diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ad9e042ba11..cbb0fe98375 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2714,65 +2714,12 @@ else fi fi -# Every landed/discard-work refusal above has now passed (or --force skipped -# them). Fix 1 and Fix 2 (see script header) run here, unconditionally on -# --force, and before ANY destructive step below - a still-parked run or a -# leaked process can own live work in this exact worktree. Not for -# kind=secondmate: a secondmate home's own runtime lifecycle is owned by the -# dedicated process-event and firstmate-home removal machinery further below, -# not by task-worktree cleanup. -if [ "$KIND" != secondmate ]; then - conclude_task_no_mistakes_run "$WT" - reap_task_worktree_processes worktree "$WT" "$TASK_TMP" -fi - -# Fix 3 (see script header): sweep remote job workers abandoned by an already -# pruned code root. Best effort - a sweep failure never blocks this teardown. -"$SCRIPT_DIR/fm-remote-job-reap-orphans.sh" >&2 || true - -# Best-effort: drop the local task branch so the shared repo does not accumulate refs. -if [ "$BACKEND" = orca ] && [ "$KIND" != secondmate ]; then - if [ "$ORCA_PATH_MATCH_VERIFIED" != 1 ]; then - require_orca_worktree_path_match_if_present "$ORCA_WORKTREE_ID" "$WT" || exit 1 - ORCA_PATH_MATCH_VERIFIED=1 - fi - if [ -d "$WT" ]; then - branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) - if [ "$branch" != "HEAD" ]; then - if git -C "$WT" checkout --detach -q 2>/dev/null; then - git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true - fi - fi - rm -f "$WT/.claude/settings.local.json" "$WT/.opencode/plugins/fm-turn-end.js" \ - "$WT/.opencode/plugins/fm-busy-state.js" \ - "$WT/.fm-grok-turnend" "$WT/.fm-kimi-turnend" - fi - [ -z "$T_ORCA" ] || fm_backend_kill "$BACKEND" "$T" "$(meta_value "$META" zellij_tab_id)" "fm-$ID" 2>/dev/null || true - fm_backend_remove_worktree "$BACKEND" "$ORCA_WORKTREE_ID" -elif [ -d "$WT" ] && [ "$KIND" != secondmate ]; then - branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) - if [ "$branch" != "HEAD" ]; then - if git -C "$WT" checkout --detach -q 2>/dev/null; then - git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true - fi - fi - # Remove our hook file so a reused pool worktree cannot fire signals for a dead task. - rm -f "$WT/.claude/settings.local.json" "$WT/.opencode/plugins/fm-turn-end.js" \ - "$WT/.fm-grok-turnend" "$WT/.fm-kimi-turnend" - # Kills remaining processes in the worktree (including the agent), resets, returns - # to pool. treehouse resolves the pool from the working directory, so run it from - # the project. teardown_treehouse_return tolerates transient and stale git locks - # left by a killed crew process; see the script header for retry and stale-lock proof. - post_lock_cleanup_check= - if [ "$FORCE" != "--force" ] && [ "$KIND" != scout ] && [ "$KIND" != secondmate ]; then - post_lock_cleanup_check=validate_worktree_teardown_safety - fi - teardown_treehouse_return "$WT" "$PROJ" "worktree" "$post_lock_cleanup_check" || { - echo "error: treehouse return failed for worktree $WT; teardown aborted" >&2 - exit 1 - } -fi - +# Close the task endpoint BEFORE the worktree process reap and the copy's +# return below: the pane's shell now starts inside the leased copy +# (bin/fm-spawn.sh header), so both the cwd-rooted reap and `treehouse return` +# would otherwise kill that shell under the endpoint and skip the planned +# focus-preserving Herdr close. Every landed/discard-work refusal already ran +# above, so a refused close here retains the copy together with every record. HERDR_PRESENTATION_JOURNAL="$STATE/$ID.herdr-presentation" HERDR_PRESENTATION_RETIRE_CANDIDATE=0 HERDR_PRESENTATION_SESSION= @@ -2795,8 +2742,8 @@ if [ "$BACKEND" = herdr ] \ fi if [ "$HERDR_PRESENTATION_RETIRE_CANDIDATE" = 1 ]; then - # The presentation lock was acquired before the worktree return above; a - # contended lock already refused this teardown while everything was intact. + # The presentation lock was acquired at the start of teardown; a contended + # lock already refused this teardown while everything was intact. if teardown_herdr_session_lock_held "$HERDR_PRESENTATION_SESSION"; then # stderr is deliberately NOT discarded here. This is the highest-frequency # projected-close call site, and the helper's only stderr output is a real @@ -2847,6 +2794,66 @@ if [ "$BACKEND" = herdr ]; then exit 1 fi fi + +# Every landed/discard-work refusal above has now passed (or --force skipped +# them). Fix 1 and Fix 2 (see script header) run here, unconditionally on +# --force, and before ANY destructive step below - a still-parked run or a +# leaked process can own live work in this exact worktree. Not for +# kind=secondmate: a secondmate home's own runtime lifecycle is owned by the +# dedicated process-event and firstmate-home removal machinery further below, +# not by task-worktree cleanup. +if [ "$KIND" != secondmate ]; then + conclude_task_no_mistakes_run "$WT" + reap_task_worktree_processes worktree "$WT" "$TASK_TMP" +fi + +# Fix 3 (see script header): sweep remote job workers abandoned by an already +# pruned code root. Best effort - a sweep failure never blocks this teardown. +"$SCRIPT_DIR/fm-remote-job-reap-orphans.sh" >&2 || true + +# Best-effort: drop the local task branch so the shared repo does not accumulate refs. +if [ "$BACKEND" = orca ] && [ "$KIND" != secondmate ]; then + if [ "$ORCA_PATH_MATCH_VERIFIED" != 1 ]; then + require_orca_worktree_path_match_if_present "$ORCA_WORKTREE_ID" "$WT" || exit 1 + ORCA_PATH_MATCH_VERIFIED=1 + fi + if [ -d "$WT" ]; then + branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) + if [ "$branch" != "HEAD" ]; then + if git -C "$WT" checkout --detach -q 2>/dev/null; then + git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true + fi + fi + rm -f "$WT/.claude/settings.local.json" "$WT/.opencode/plugins/fm-turn-end.js" \ + "$WT/.opencode/plugins/fm-busy-state.js" \ + "$WT/.fm-grok-turnend" "$WT/.fm-kimi-turnend" + fi + [ -z "$T_ORCA" ] || fm_backend_kill "$BACKEND" "$T" "$(meta_value "$META" zellij_tab_id)" "fm-$ID" 2>/dev/null || true + fm_backend_remove_worktree "$BACKEND" "$ORCA_WORKTREE_ID" +elif [ -d "$WT" ] && [ "$KIND" != secondmate ]; then + branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) + if [ "$branch" != "HEAD" ]; then + if git -C "$WT" checkout --detach -q 2>/dev/null; then + git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true + fi + fi + # Remove our hook file so a reused pool worktree cannot fire signals for a dead task. + rm -f "$WT/.claude/settings.local.json" "$WT/.opencode/plugins/fm-turn-end.js" \ + "$WT/.fm-grok-turnend" "$WT/.fm-kimi-turnend" + # Kills any process still in the worktree (the endpoint was closed above), + # resets, and returns to pool. treehouse resolves the pool from the working + # directory, so run it from the project. teardown_treehouse_return tolerates transient and stale git locks + # left by a killed crew process; see the script header for retry and stale-lock proof. + post_lock_cleanup_check= + if [ "$FORCE" != "--force" ] && [ "$KIND" != scout ] && [ "$KIND" != secondmate ]; then + post_lock_cleanup_check=validate_worktree_teardown_safety + fi + teardown_treehouse_return "$WT" "$PROJ" "worktree" "$post_lock_cleanup_check" || { + echo "error: treehouse return failed for worktree $WT; teardown aborted" >&2 + exit 1 + } +fi + if [ "$KIND" = secondmate ]; then [ -n "$HOME_PATH" ] || HOME_PATH=$WT handoff_wake_retire_stage \ diff --git a/docs/architecture.md b/docs/architecture.md index 0e3e0042635..6762e948f81 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -167,7 +167,7 @@ All are harness-scoped rather than a global pattern union, and none is a recorde ## Runtime session backends The runtime backend is the session-provider layer below firstmate's scripts. -It owns task endpoint creation, bounded capture, text/key sends, current-path reads for spawn-time worktree discovery when the backend does not create the worktree itself, live-window fallback lookup, agent-process liveness probes where verified, and endpoint teardown. +It owns task endpoint creation, bounded capture, text/key sends, current-path reads that confirm a new pane sits in its leased worktree when the backend does not create the worktree itself, live-window fallback lookup, agent-process liveness probes where verified, and endpoint teardown. `bin/fm-backend.sh` centralizes backend selection, `state/.meta` helpers, metadata-only cleanup identity validation, selector resolution, and operation dispatch; `bin/backends/tmux.sh` is the verified reference adapter ([`docs/tmux-backend.md`](tmux-backend.md)), and `bin/backends/herdr.sh` (P2), `bin/backends/zellij.sh` (P3), `bin/backends/orca.sh` (P4), and `bin/backends/cmux.sh` (P5) are experimental task-spawn adapters. [`configuration.md`](configuration.md#runtime-backend-configbackend--fm_backend) owns new-spawn backend selection precedence and authorization. Runtime auto-detection is innermost-first: `$TMUX` wins over `HERDR_ENV=1`, which wins over cmux's primary `CMUX_WORKSPACE_ID` marker and documented fallback signals; auto-detected herdr or cmux prints a one-time opt-out notice, auto-detected tmux stays silent, and zellij and orca are never auto-detected (only explicit selection). @@ -195,7 +195,8 @@ Codex App support is recorded in `docs/codex-app-backend.md`; it is not selectab Crewmates never intentionally touch your project clone; [treehouse](https://github.com/kunchenguid/treehouse) pools clean worktrees for tmux, herdr, zellij, and cmux tasks, while Orca creates its own worktrees for `backend=orca`. For ship and scout work, `fm-spawn.sh` refuses to launch unless the resolved task path is a real git worktree root that is distinct from the project primary checkout. `fm-spawn.sh` also owns the base-freshness boundary for every fresh ship and scout: no worker starts until its clean task worktree matches the fetched tip of origin's resolved default branch, and any unsafe or unverifiable base stops the spawn. -Its header owns the exact refusal mechanics, while `tests/fm-spawn-pool-base-freshen.test.sh` owns the portable regression coverage. +On the session-provider backends the copy is leased and refreshed from the spawn process before any pane exists, and the pane is created already inside it, so no worker shell can hold the worktree's git lock while the refresh runs. +Its header owns the exact order and refusal mechanics, while `tests/fm-spawn-pool-base-freshen.test.sh` and `tests/fm-spawn-lease-before-enter.test.sh` own the portable regression coverage. The firstmate repo has one extra exposure because it can dispatch crewmates to work on itself. Its operating checkout (`FM_ROOT`) and the disposable crewmate worktrees are all linked git worktrees of the same repository, so the valid discriminator is branch state, not whether the checkout is linked. diff --git a/docs/cmux-backend.md b/docs/cmux-backend.md index 51618de5b5f..e90442ddc76 100644 --- a/docs/cmux-backend.md +++ b/docs/cmux-backend.md @@ -87,8 +87,8 @@ 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`. -Spawn-time worktree discovery sends begin and end markers around `pwd`, captures the marked block, and joins wrapped path lines. +`current_directory` follows a top-level shell `cd` but not a foreground subshell such as the one `treehouse get` opens. +Spawn creates the surface already inside the leased worktree and never types `treehouse get` into it (`bin/fm-spawn.sh` header); its confirmation that the live shell sits there sends begin and end markers around `pwd`, captures the marked block, and joins wrapped path lines. An ordinary metadata-routed `fm-send.sh` text steer becomes a durable steering-inbox record, and only its best-effort constant doorbell passes through cmux's submit machinery. On the typed plane, literal send and Enter are separate calls. diff --git a/docs/zellij-backend.md b/docs/zellij-backend.md index fb58e52a621..7c03f575a79 100644 --- a/docs/zellij-backend.md +++ b/docs/zellij-backend.md @@ -66,9 +66,10 @@ 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`. -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. +`pane_cwd` follows a top-level shell `cd` but not a foreground subshell such as the one `treehouse get` opens. +Spawn-time worktree confirmation therefore sends begin and end markers around `pwd`, captures the marked block, and joins wrapped path lines. +Spawn creates the pane already inside the leased worktree and never types `treehouse get` into it (`bin/fm-spawn.sh` header), so this probe only confirms where the live shell sits. +This active probe is scoped to that confirmation and is not advertised as a general live-cwd API. `new-tab` has no no-focus flag and temporarily focuses the created tab in attached clients. The adapter records the previously active tab and immediately restores it with `go-to-tab-by-id`. diff --git a/tests/fixtures.sh b/tests/fixtures.sh index 88f10501fcd..118f5eaae2a 100755 --- a/tests/fixtures.sh +++ b/tests/fixtures.sh @@ -244,14 +244,15 @@ fm_test_spawn_brief() { } # fm_test_make_spawn_fakebin [extra-exit0-tool...] -# Creates /fakebin with the spawn tmux stub, a no-op treehouse, and any -# extra exit-0 tools. Echoes the fakebin path. +# Creates /fakebin with the spawn tmux stub, the leasing treehouse stub +# (fm_test_fake_treehouse), and any extra exit-0 tools. Echoes the fakebin path. fm_test_make_spawn_fakebin() { local dir=$1 fakebin shift fakebin=$(fm_fakebin "$dir") fm_test_fake_tmux_spawn "$fakebin" - fm_fake_exit0 "$fakebin" treehouse "$@" + fm_test_fake_treehouse "$fakebin" + [ $# -eq 0 ] || fm_fake_exit0 "$fakebin" "$@" printf '%s\n' "$fakebin" } diff --git a/tests/fm-backend-herdr-presentation-e2e.test.sh b/tests/fm-backend-herdr-presentation-e2e.test.sh index ac62abb4275..53ccdba7d39 100755 --- a/tests/fm-backend-herdr-presentation-e2e.test.sh +++ b/tests/fm-backend-herdr-presentation-e2e.test.sh @@ -24,6 +24,7 @@ TMP_ROOT=$(mktemp -d "$(cd "${TMPDIR:-/tmp}" && pwd -P)/fm-herdr-presentation.XX FAKEBIN="$TMP_ROOT/fakebin" HERDR_CALL_LOG="$TMP_ROOT/herdr-calls.log" TREEHOUSE_CALL_LOG="$TMP_ROOT/treehouse-calls.log" +LEASED_WORKTREES_LOG="$TMP_ROOT/leased-worktrees.log" MOVE_CALL_LOG="$TMP_ROOT/workspace-move-calls.log" FOCUS_AUDIT_LOG="$TMP_ROOT/focus-audit.log" ACTIVE_SEEDED_CONTROL="$TMP_ROOT/active-seeded-control" @@ -31,10 +32,11 @@ POST_CREATE_ABORT_CONTROL="$TMP_ROOT/post-create-abort-control" mkdir -p "$FAKEBIN" : > "$HERDR_CALL_LOG" : > "$TREEHOUSE_CALL_LOG" +: > "$LEASED_WORKTREES_LOG" : > "$MOVE_CALL_LOG" : > "$FOCUS_AUDIT_LOG" REAL_MOVER="$ROOT/bin/backends/herdr-workspace-move.py" -export REAL_HERDR REAL_TREEHOUSE REAL_MOVER HERDR_CALL_LOG TREEHOUSE_CALL_LOG MOVE_CALL_LOG FOCUS_AUDIT_LOG HERDR_ORIGINAL_PATH HERDR_LAB_HELPER +export REAL_HERDR REAL_TREEHOUSE REAL_MOVER HERDR_CALL_LOG TREEHOUSE_CALL_LOG LEASED_WORKTREES_LOG MOVE_CALL_LOG FOCUS_AUDIT_LOG HERDR_ORIGINAL_PATH HERDR_LAB_HELPER export ACTIVE_SEEDED_CONTROL POST_CREATE_ABORT_CONTROL TMP_ROOT # Log every production-adapter call, remove its already-validated trailing @@ -131,6 +133,10 @@ case "${1:-} ${2:-}" in "workspace create") mutation=workspace-create; mutation_target=$label ;; "tab create") mutation=tab-create; mutation_target=$label ;; "pane close") mutation=pane-close ;; + # The emptying-close plan removes a lone idle shell through Herdr's + # pane-death path instead of an explicit close; its process-info read is the + # one CLI call that marks that removal, so audit it as the pane's removal. + "pane process-info") mutation=pane-death; mutation_target=$(arg_value --pane "$@" || true) ;; "tab focus") mutation=tab-focus ;; esac refusal_probe=0 @@ -207,7 +213,13 @@ set -u done printf '\n' } >> "$TREEHOUSE_CALL_LOG" -if [ -d "$POST_CREATE_ABORT_CONTROL" ] && [ "${1:-}" = get ]; then +# fm-spawn leases the copy from its own process before any pane exists; record +# every leased path so cleanup can return the ones a deliberately aborted spawn +# retains for inspection. +if [ "${1:-} ${2:-}" = "get --lease" ]; then + out=$("$REAL_TREEHOUSE" "$@") || exit $? + printf '%s\n' "$out" >> "$LEASED_WORKTREES_LOG" + printf '%s\n' "$out" exit 0 fi exec "$REAL_TREEHOUSE" "$@" @@ -280,6 +292,7 @@ cleanup_all() { "$REAL_TREEHOUSE" return --force "$wt" >/dev/null 2>&1 || true done </dev/null || true) EOF if [ "$LAB_READY" -eq 1 ]; then PATH="$HERDR_ORIGINAL_PATH" \ @@ -346,7 +359,7 @@ assert_raw_presentation_mutations_preserved_since() { # local start=$1 pane_id=$2 expected=$3 sed -n "$((start + 1)),\$p" "$FOCUS_AUDIT_LOG" | awk -F '\t' -v pane="$pane_id" -v expected="$expected" ' - $1 == "pane-close" && $4 == pane { + ($1 == "pane-close" || $1 == "pane-death") && $4 == pane { saw_close = 1 if ($2 != expected) { bad = 1 } else if ($3 == expected) { preserved = 1 } @@ -830,7 +843,11 @@ FAIL_CLOSED_PANES=$(sed -n "$((FAIL_START + 1)),\$p" "$HERDR_CALL_LOG" | awk -F assert_no_ordering_lifecycle_calls_since "$FAIL_START" "failed presentation ordering" pass "real Herdr lab: forced workspace.move failure leaves a successful worker in default order with a warning and no cleanup" +# The copy is leased and refreshed before the pane exists, so the post-create +# abort is armed on the pane's cwd read instead: the wrapper reports a path +# that is not the leased copy, and a short settle window keeps the wait bounded. mkdir -p "$POST_CREATE_ABORT_CONTROL" +export FM_SPAWN_SETTLE_POLLS=3 ABORT_START=$(log_line_count) ABORT_FOCUS_START=$(focus_audit_line_count) spawn_task abort-a "$HOME_DIR" "$PROJECT_DIR" > "$TMP_ROOT/abort-a.out" 2> "$TMP_ROOT/abort-a.err" & @@ -841,17 +858,18 @@ if wait "$ABORT_A_PID"; then ABORT_A_STATUS=0; else ABORT_A_STATUS=$?; fi if wait "$ABORT_B_PID"; then ABORT_B_STATUS=0; else ABORT_B_STATUS=$?; fi finish_concurrent_expected_abort abort-a "$ABORT_A_STATUS" "$TMP_ROOT/abort-a.out" "$TMP_ROOT/abort-a.err" finish_concurrent_expected_abort abort-b "$ABORT_B_STATUS" "$TMP_ROOT/abort-b.out" "$TMP_ROOT/abort-b.err" -grep -F "did not yield an isolated worktree" "$TMP_ROOT/abort-a.err" >/dev/null 2>&1 \ - || fail "post-create abort fixture A did not reach the armed validation failure" -grep -F "did not yield an isolated worktree" "$TMP_ROOT/abort-b.err" >/dev/null 2>&1 \ - || fail "post-create abort fixture B did not reach the armed validation failure" +unset FM_SPAWN_SETTLE_POLLS +grep -F "did not settle in the leased worktree" "$TMP_ROOT/abort-a.err" >/dev/null 2>&1 \ + || fail "post-create abort fixture A did not reach the armed settle failure" +grep -F "did not settle in the leased worktree" "$TMP_ROOT/abort-b.err" >/dev/null 2>&1 \ + || fail "post-create abort fixture B did not reach the armed settle failure" ABORT_A_PANE=$(cat "$POST_CREATE_ABORT_CONTROL/abort-a/task-pane") ABORT_B_PANE=$(cat "$POST_CREATE_ABORT_CONTROL/abort-b/task-pane") ABORT_SEQUENCE=$(sed -n "$((ABORT_FOCUS_START + 1)),\$p" "$FOCUS_AUDIT_LOG" | awk -F '\t' -v a="$ABORT_A_PANE" -v b="$ABORT_B_PANE" ' $1 == "workspace-create" && $4 ~ /^└ abort-a · p:/ { print "create-a" } $1 == "workspace-create" && $4 ~ /^└ abort-b · p:/ { print "create-b" } - $1 == "pane-close" && $4 == a { print "close-a" } - $1 == "pane-close" && $4 == b { print "close-b" } + ($1 == "pane-close" || $1 == "pane-death") && $4 == a { print "close-a" } + ($1 == "pane-close" || $1 == "pane-death") && $4 == b { print "close-b" } ') case "$ABORT_SEQUENCE" in $'create-a\nclose-a\ncreate-b\nclose-b'|$'create-b\nclose-b\ncreate-a\nclose-a') ;; @@ -1195,6 +1213,10 @@ for RESTART_ID in fm-hibit-resume-r1 wheelhouse-healing-r1; do NEW_RESTART_WT=$(remember_meta_worktree "$RESTART_META") NEW_RESTART_WSID=$(grep '^herdr_workspace_id=' "$RESTART_META" | cut -d= -f2-) NEW_RESTART_PANE=$(grep '^herdr_pane_id=' "$RESTART_META" | cut -d= -f2-) + # A reclaim refreshes the record's own copy rather than leasing a second one + # (bin/fm-spawn.sh header), so the pool never grows per recovery attempt. + [ "$NEW_RESTART_WT" = "$OLD_RESTART_WT" ] \ + || fail "$RESTART_ID reclaim leased a second copy ($NEW_RESTART_WT) instead of reusing the record's ($OLD_RESTART_WT)" [ "$NEW_RESTART_WSID" = "$OLD_RESTART_WSID" ] \ || fail "$RESTART_ID reclaim flattened into a different workspace" [ "$NEW_RESTART_PANE" != "$OLD_RESTART_PANE" ] \ @@ -1224,14 +1246,14 @@ for RESTART_ID in fm-hibit-resume-r1 wheelhouse-healing-r1; do || fail "$RESTART_ID repeated reclaim changed workspace identity" [ "$NEW_RESTART_PANE" != "$PRIOR_RESTART_PANE" ] \ || fail "$RESTART_ID repeated reclaim reused the prior husk pane" - "$REAL_TREEHOUSE" return --force "$PRIOR_RESTART_WT" >/dev/null 2>&1 || true + [ "$NEW_RESTART_WT" = "$PRIOR_RESTART_WT" ] \ + || fail "$RESTART_ID repeated reclaim leased a second copy instead of reusing the record's" fi teardown_task "$RESTART_ID" "$HOME_DIR" > "$TMP_ROOT/$RESTART_ID-teardown.out" 2> "$TMP_ROOT/$RESTART_ID-teardown.err" \ || fail "$RESTART_ID teardown after reclaim failed: $(cat "$TMP_ROOT/$RESTART_ID-teardown.err")" [ ! -e "$HOME_DIR/state/$RESTART_ID.herdr-presentation" ] \ || fail "$RESTART_ID exact reclaimed teardown did not retire its journal" - "$REAL_TREEHOUSE" return --force "$OLD_RESTART_WT" >/dev/null 2>&1 || true "$REAL_TREEHOUSE" return --force "$NEW_RESTART_WT" >/dev/null 2>&1 || true done pass "real Herdr lab: Hi Bit and Wheelhouse-style same-identity restarts reclaim one nested space with exact focus and idempotence" diff --git a/tests/fm-backend.test.sh b/tests/fm-backend.test.sh index ece981b1222..5fef478aba8 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -785,7 +785,12 @@ esac exit 0 SH chmod +x "$fb/tmux" - fm_fake_exit0 "$fb" treehouse + cat > "$fb/treehouse" < # (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 +# from that physical read even before the pane settles in the leased copy, 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. @@ -855,7 +860,12 @@ esac exit 0 SH chmod +x "$fb/tmux" - fm_fake_exit0 "$fb" treehouse + cat > "$fb/treehouse" < git -C "$primary" add AGENTS.md bin git -C "$primary" commit -qm 'seed primary default branch' fakebin=$(fm_fakebin "$dir") - fm_fake_exit0 "$fakebin" gh treehouse tmux node + fm_test_fake_treehouse "$fakebin" + fm_fake_exit0 "$fakebin" gh tmux node log="$dir/probe.log" : > "$log" install_fake_ssh "$fakebin" diff --git a/tests/fm-captain-hold-lifecycle.test.sh b/tests/fm-captain-hold-lifecycle.test.sh index 5136d2f9b3e..d27daaede9e 100755 --- a/tests/fm-captain-hold-lifecycle.test.sh +++ b/tests/fm-captain-hold-lifecycle.test.sh @@ -29,7 +29,8 @@ make_home() { # ## Done EOF fakebin=$(fm_fakebin "$home") - fm_fake_exit0 "$fakebin" tmux treehouse no-mistakes gh gh-axi + fm_test_fake_treehouse "$fakebin" + fm_fake_exit0 "$fakebin" tmux no-mistakes gh gh-axi 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_test_fake_treehouse "$fakebin" + fm_fake_exit0 "$fakebin" tmux no-mistakes gh gh-axi 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-kimi-harness.test.sh b/tests/fm-kimi-harness.test.sh index d04b2d8e7dc..03539a2b77e 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_test_fake_treehouse "$fakebin" + fm_fake_exit0 "$fakebin" gh-axi gh 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 83a0747458b..f261d9f54f0 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_test_fake_treehouse "$fakebin" + fm_fake_exit0 "$fakebin" gh-axi gh printf '%s\n' "$fakebin" } diff --git a/tests/fm-public-followup.test.sh b/tests/fm-public-followup.test.sh index c0d5a7779da..95c18d1ca20 100755 --- a/tests/fm-public-followup.test.sh +++ b/tests/fm-public-followup.test.sh @@ -112,7 +112,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_test_fake_treehouse "$home/fakebin" + fm_fake_exit0 "$home/fakebin" tmux no-mistakes gh gh-axi printf '%s\n' "$home" } @@ -784,7 +785,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_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi assert_local_secondmate_parent_record "$child" "$parent_resolved" @@ -821,7 +823,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_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi 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" \ @@ -852,7 +855,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_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi 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" @@ -887,7 +891,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_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi 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" @@ -919,7 +924,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_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi 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" "spawn_gen=public-followup-fixture" @@ -981,7 +987,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_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi 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" @@ -1412,7 +1419,8 @@ test_dropped_baton_now_surfaces_open_loop() { "$ROOT/bin/fm-home-seed.sh" mate "$child" --no-projects >/dev/null || fail "seed failed" child=$(cd "$child" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi log="$TMP_ROOT/curl.log"; : > "$log" seed_repro_commitment "$parent" public-final-pi-rearm-repro req-pirearm \ @@ -1462,7 +1470,8 @@ test_control_registered_followon_is_guarded() { "$ROOT/bin/fm-home-seed.sh" mate "$child" --no-projects >/dev/null || fail "seed failed" child=$(cd "$child" && pwd -P) make_fake_curl "$child" >/dev/null - fm_fake_exit0 "$child/fakebin" tmux treehouse no-mistakes gh gh-axi + fm_test_fake_treehouse "$child/fakebin" + fm_fake_exit0 "$child/fakebin" tmux no-mistakes gh gh-axi seed_repro_commitment "$parent" public-final-pi-rearm-ship req-pirearm2 \ secondmate:mate pi-rearm-loop-fix-r1 fm_write_meta "$parent/state/mate.meta" "kind=secondmate" "home=$child" diff --git a/tests/fm-secondmate-harness.test.sh b/tests/fm-secondmate-harness.test.sh index ee8039a9c88..597f77f4c3b 100755 --- a/tests/fm-secondmate-harness.test.sh +++ b/tests/fm-secondmate-harness.test.sh @@ -629,8 +629,9 @@ meta_field() { grep "^$2=" "$1" 2>/dev/null | tail -1 | cut -d= -f2-; } # `send-keys -l ` launch command into FM_FAKE_LAUNCH_LOG, mirroring the # 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. +# `#{pane_current_path}` probe from FM_FAKE_PANE_PATH, and the leasing +# treehouse stub reports that same path, so this same fakebin works for a +# crew/scout (non-secondmate) spawn's lease-then-settle sequence. make_launch_capturing_tmux() { local dir=$1 fakebin="$1/fakebin" mkdir -p "$fakebin" @@ -660,6 +661,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" + fm_test_fake_treehouse "$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 efd61dd804f..48a2ef24c60 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_test_fake_treehouse "$fakebin" + fm_fake_exit0 "$fakebin" node chrome-devtools-axi gh 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-lease-before-enter.test.sh b/tests/fm-spawn-lease-before-enter.test.sh new file mode 100755 index 00000000000..950041cb8a6 --- /dev/null +++ b/tests/fm-spawn-lease-before-enter.test.sh @@ -0,0 +1,252 @@ +#!/usr/bin/env bash +# Regression tests for fm-spawn's lease-then-refresh-then-enter order. +# +# The pooled copy must be leased from the spawn process and refreshed BEFORE +# any pane exists, because the first shell to enter a worktree runs git (its +# prompt, gitstatusd) and takes the same index.lock the refresh needs; on a +# large repository that race refused every spawn. These tests drive the real +# spawn path with a fake terminal whose window creation observes the copy the +# moment the pane appears, so they prove the refresh already happened with no +# shell in the copy, that a git lock taken by the new pane cannot make the +# refresh refuse a clean copy, and that a refused launch hands a clean lease +# back while a copy holding uncommitted work stays leased. +set -u + +# shellcheck source=tests/fixtures.sh +. "$(dirname "${BASH_SOURCE[0]}")/fixtures.sh" + +TMP_ROOT=$(fm_test_tmproot fm-spawn-lease-before-enter) + +# Spawn-world tmux that additionally records every argv to FM_TMUX_REC and, on +# new-window, snapshots the pooled copy's HEAD into FM_FAKE_NEWWINDOW_HEAD at +# the instant the pane appears. With FM_FAKE_PANE_LOCK=1 it also takes the +# copy's index.lock then, exactly as a fresh shell's git prompt would. +make_lease_fakebin() { # + local dir=$1 fakebin + fakebin=$(fm_fakebin "$dir") + cat > "$fakebin/tmux" <<'SH' +#!/usr/bin/env bash +set -u +[ -z "${FM_TMUX_REC:-}" ] || printf 'tmux %s\n' "$*" >> "$FM_TMUX_REC" +case "$*" in + *"#{pane_current_path}"*) printf '%s\n' "${FM_FAKE_PANE_PATH:-}"; exit 0 ;; +esac +case "${1:-}" in + display-message) printf 'firstmate\n'; exit 0 ;; + new-window) + if [ "${FM_FAKE_NEWWINDOW_FAIL:-0}" = 1 ]; then + printf 'fake tmux: new-window refused\n' >&2 + exit 1 + fi + if [ -n "${FM_FAKE_NEWWINDOW_HEAD:-}" ]; then + git -C "${FM_FAKE_PANE_PATH:?}" rev-parse HEAD > "$FM_FAKE_NEWWINDOW_HEAD" 2>/dev/null || printf 'unreadable\n' > "$FM_FAKE_NEWWINDOW_HEAD" + fi + if [ "${FM_FAKE_PANE_LOCK:-0}" = 1 ]; then + : > "$(git -C "${FM_FAKE_PANE_PATH:?}" rev-parse --git-path index.lock)" + fi + printf '%s\n' "@leasewid" + exit 0 + ;; + list-windows) exit 0 ;; + has-session|new-session|kill-window|send-keys|set-window-option) exit 0 ;; +esac +exit 0 +SH + chmod +x "$fakebin/tmux" + fm_test_fake_treehouse "$fakebin" + printf '%s\n' "$fakebin" +} + +# A project with a bare origin, a detached pooled copy allocated at the initial +# commit, and an origin/main advanced after that allocation - the exact shape +# whose refresh the pane could race. +make_case() { # + local name=$1 id=$2 case_dir home project origin pool publisher fakebin initial + case_dir="$TMP_ROOT/$name" + home="$case_dir/home" + project="$case_dir/project" + origin="$case_dir/origin.git" + pool="$case_dir/pool" + publisher="$case_dir/publisher" + fakebin=$(make_lease_fakebin "$case_dir/fake") + + fm_test_spawn_home "$home" codex + fm_test_spawn_brief "$home" "$id" + + git init --quiet -b main "$project" + printf 'base\n' > "$project/README.md" + git -C "$project" add README.md + git -C "$project" -c user.name='Firstmate Tests' -c user.email='tests@example.invalid' commit -qm initial + git clone --quiet --bare "$project" "$origin" + git -C "$project" remote add origin "file://$origin" + initial=$(git -C "$project" rev-parse HEAD) + git -C "$project" worktree add --quiet --detach "$pool" "$initial" + + git clone --quiet "file://$origin" "$publisher" + printf 'advanced\n' > "$publisher/advanced-main.txt" + git -C "$publisher" add advanced-main.txt + git -C "$publisher" -c user.name='Firstmate Tests' -c user.email='tests@example.invalid' commit -qm advance-main + git -C "$publisher" push --quiet origin main + + printf '%s\n' "$case_dir|$home|$project|$pool|$fakebin|$initial" +} + +read_case_record() { + IFS='|' read -r CASE_DIR HOME_DIR PROJECT_DIR POOL_DIR FAKEBIN_DIR INITIAL_SHA < "$TMUX_REC" + : > "$TREEHOUSE_LOG" +} + +run_spawn() { # [spawn args...] + local id=$1 + shift + FM_TMUX_REC="$TMUX_REC" FM_TREEHOUSE_LOG="$TREEHOUSE_LOG" \ + FM_FAKE_NEWWINDOW_HEAD="$NEWWINDOW_HEAD" \ + fm_test_run_spawn "$HOME_DIR" "$POOL_DIR" "$FAKEBIN_DIR" \ + "$id" "$PROJECT_DIR" "$@" +} + +test_refresh_completes_before_any_pane_exists() { + local rec id out status current seen + id='lease-order-l1' + rec=$(make_case order "$id") + read_case_record "$rec" + + out=$(run_spawn "$id" --mode no-mistakes --yolo off) + status=$? + expect_code 0 "$status" "spawn should lease, refresh, then create the pane"$'\n'"$out" + assert_contains "$out" "spawned $id" "spawn did not report success" + current=$(git -C "$POOL_DIR" rev-parse origin/main) + [ "$current" != "$INITIAL_SHA" ] || fail "fixture did not prove origin/main advanced past the pool base" + seen=$(cat "$NEWWINDOW_HEAD" 2>/dev/null || true) + [ -n "$seen" ] || fail "the fake terminal never observed the copy at window creation" + [ "$seen" = "$current" ] \ + || fail "the pane was created before the copy was refreshed (HEAD at creation $seen, origin/main $current)" + assert_grep "get --lease --lease-holder fm-$id" "$TREEHOUSE_LOG" \ + "spawn did not lease the copy from its own process" + assert_grep "new-window -dP -F #{window_id} -t firstmate: -n fm-$id -c $POOL_DIR" "$TMUX_REC" \ + "the pane was not created with the leased copy as its starting directory" + assert_no_grep "treehouse get" "$TMUX_REC" "a treehouse get was still typed into the pane" + assert_grep "worktree=$POOL_DIR" "$HOME_DIR/state/$id.meta" "meta did not record the leased copy" + assert_no_grep "return" "$TREEHOUSE_LOG" "a successful spawn returned its own lease" + if [ "${FM_TEST_EVIDENCE:-0}" = 1 ]; then + printf '# observed HEAD at window creation=%s origin/main=%s\n' "$seen" "$current" + fi + pass "the leased copy is refreshed before the pane exists and the pane starts inside it" +} + +test_pane_git_lock_cannot_refuse_a_clean_copy() { + local rec id out status current lock + id='lease-pane-lock-l2' + rec=$(make_case pane-lock "$id") + read_case_record "$rec" + + out=$(FM_FAKE_PANE_LOCK=1 run_spawn "$id" --mode no-mistakes --yolo off) + status=$? + expect_code 0 "$status" "a git lock taken by the new pane must not refuse the spawn"$'\n'"$out" + assert_contains "$out" "spawned $id" "spawn did not report success" + current=$(git -C "$POOL_DIR" rev-parse origin/main) + [ "$(git -C "$POOL_DIR" rev-parse HEAD)" = "$current" ] \ + || fail "the copy was not refreshed to current origin/main" + lock=$(git -C "$POOL_DIR" rev-parse --git-path index.lock) + [ -f "$lock" ] || fail "fixture did not leave the pane's simulated index.lock in place" + # The divergence must be real: with that lock held, the refresh itself is + # exactly what git refuses, so the old create-then-refresh order would have + # stopped here. + git -C "$POOL_DIR" reset --hard "$current" >/dev/null 2>&1 \ + && fail "fixture lock did not block a reset; the simulated pane race is vacuous" + rm -f "$lock" + pass "a git lock taken by the new pane cannot make the refresh refuse a clean copy" +} + +test_refused_launch_returns_a_clean_lease() { + local rec id out status before + id='lease-return-l3' + rec=$(make_case return-clean "$id") + read_case_record "$rec" + git -C "$POOL_DIR" remote set-url origin "file://$CASE_DIR/missing-origin.git" + before=$(git -C "$POOL_DIR" rev-parse HEAD) + + out=$(run_spawn "$id" --mode no-mistakes --yolo off) + status=$? + [ "$status" -ne 0 ] || fail "spawn succeeded despite an unreachable origin" + assert_contains "$out" "could not fetch origin" "spawn did not clearly refuse an unreachable origin" + assert_grep "return --force $POOL_DIR" "$TREEHOUSE_LOG" \ + "a refused launch left a clean, never-entered lease held" + assert_no_grep "new-window" "$TMUX_REC" "a refused launch still created a pane" + [ "$(git -C "$POOL_DIR" rev-parse HEAD)" = "$before" ] || fail "the refusal moved HEAD" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "a refused launch published a task record" + pass "a launch refused before any endpoint exists returns its clean lease to the pool" +} + +test_refusal_after_create_began_retains_the_lease() { + local rec id out status + id='lease-retain-l6' + rec=$(make_case retain-after-create "$id") + read_case_record "$rec" + + out=$(FM_FAKE_NEWWINDOW_FAIL=1 run_spawn "$id" --mode no-mistakes --yolo off) + status=$? + [ "$status" -ne 0 ] || fail "spawn succeeded although window creation was refused" + assert_grep "new-window" "$TMUX_REC" "the create call that ends the return window never ran" + # `treehouse return` terminates every process inside the copy, so once a + # create call may have opened a shell there the lease must be kept, not + # returned under that shell. + assert_no_grep "^return" "$TREEHOUSE_LOG" \ + "a refusal after window creation began returned the lease under a possible pane" + assert_contains "$out" "leased copy '$POOL_DIR' is retained" \ + "the retained lease was not reported" + assert_contains "$out" "treehouse return --force '$POOL_DIR'" \ + "the retained lease's release command was not reported" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "a refused launch published a task record" + pass "a refusal after endpoint creation began retains the lease instead of terminating what may sit in it" +} + +test_dirty_copy_stays_leased_and_untouched() { + local rec id out status + id='lease-dirty-l4' + rec=$(make_case dirty "$id") + read_case_record "$rec" + printf 'keep this local work\n' > "$POOL_DIR/uncommitted.txt" + + out=$(run_spawn "$id" --mode no-mistakes --yolo off) + status=$? + [ "$status" -ne 0 ] || fail "spawn succeeded despite a dirty pooled copy" + assert_contains "$out" "is not clean" "spawn did not clearly refuse a dirty copy" + assert_contains "$out" "holds uncommitted work; leaving it leased" \ + "the refusal did not say the dirty copy stays leased" + assert_no_grep "return" "$TREEHOUSE_LOG" "a dirty copy was returned, which would reset it" + assert_no_grep "new-window" "$TMUX_REC" "a refused launch still created a pane" + assert_grep 'keep this local work' "$POOL_DIR/uncommitted.txt" "the dirty copy's work was discarded" + pass "a copy holding uncommitted work is refused, stays leased, and is never reset" +} + +test_lease_without_a_path_refuses_before_any_endpoint() { + local rec id out status + id='lease-nopath-l5' + rec=$(make_case no-path "$id") + read_case_record "$rec" + + out=$(FM_FAKE_LEASE_PATH="$CASE_DIR/not-a-directory" run_spawn "$id" --mode no-mistakes --yolo off) + status=$? + [ "$status" -ne 0 ] || fail "spawn succeeded without a usable leased path" + assert_contains "$out" "did not report exactly one existing absolute worktree path" \ + "spawn did not clearly refuse an unusable lease result" + assert_no_grep "new-window" "$TMUX_REC" "spawn created a pane without a leased copy" + assert_no_grep "return" "$TREEHOUSE_LOG" "spawn tried to return a lease it never held" + pass "a lease that yields no usable path refuses before any endpoint exists" +} + +test_refresh_completes_before_any_pane_exists +test_pane_git_lock_cannot_refuse_a_clean_copy +test_refused_launch_returns_a_clean_lease +test_refusal_after_create_began_retains_the_lease +test_dirty_copy_stays_leased_and_untouched +test_lease_without_a_path_refuses_before_any_endpoint + +echo "# all fm-spawn-lease-before-enter tests passed" diff --git a/tests/fm-spawn-worktree-settle.test.sh b/tests/fm-spawn-worktree-settle.test.sh index 66f3c837aff..fafd43cf967 100755 --- a/tests/fm-spawn-worktree-settle.test.sh +++ b/tests/fm-spawn-worktree-settle.test.sh @@ -1,17 +1,15 @@ #!/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 worktree settle loop (bin/fm-spawn.sh, +# the poll after the pane is created inside the leased worktree). # # 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 worktree it was created in. That stale path +# is a real git checkout, just the wrong one, so a loop that accepted any +# plausible worktree would silently record 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 +# is the leased, settled worktree, never the stale first read. set -u # shellcheck source=tests/lib.sh @@ -54,7 +52,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_test_fake_treehouse "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/fm-tangle-guard.test.sh b/tests/fm-tangle-guard.test.sh index 8bcbb392e60..f5476be1b5c 100755 --- a/tests/fm-tangle-guard.test.sh +++ b/tests/fm-tangle-guard.test.sh @@ -197,8 +197,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 any later rename attempt; +# - the window is created already inside the leased worktree (no treehouse get +# is ever typed into it) and the worktree wait loop targets 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. @@ -221,7 +222,7 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + fm_test_fake_treehouse "$fakebin" printf '%s\n' "$fakebin" } @@ -260,9 +261,12 @@ 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 window starts in the leased worktree and the worktree + # wait loop targets the stable id; no treehouse get is typed into the pane. + assert_grep "-n fm-rec-win-gg7 -c $wt" "$rec" \ + "new-window must start in the leased worktree" + assert_no_grep "treehouse get" "$rec" \ + "treehouse get must never be typed into the pane" 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 9eed7c5005c..d55112589f3 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_test_fake_treehouse "$fakebin" printf '%s\n' "$fakebin" } diff --git a/tests/lib.sh b/tests/lib.sh index 12164936914..4ab1bbce482 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -182,6 +182,27 @@ SH done } +# fm_test_fake_treehouse +# Spawn-world treehouse: `get --lease ...` prints the path fm-spawn.sh will +# treat as the leased copy - FM_FAKE_LEASE_PATH, falling back to the pane path +# FM_FAKE_PANE_PATH so a suite whose fake pane already "sits" in the worktree +# needs no second variable. Every invocation is appended to FM_TREEHOUSE_LOG +# (one argv per line, space-joined) when that variable is set, and every other +# subcommand (return, prune, ...) exits 0 silently. +fm_test_fake_treehouse() { + local fakebin=$1 + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +set -u +[ -z "${FM_TREEHOUSE_LOG:-}" ] || printf '%s\n' "$*" >> "$FM_TREEHOUSE_LOG" +case "${1:-} ${2:-}" in + "get --lease") printf '%s\n' "${FM_FAKE_LEASE_PATH:-${FM_FAKE_PANE_PATH:-}}" ;; +esac +exit 0 +SH + chmod +x "$fakebin/treehouse" +} + # fm_fake_version_tool # The stub answers `--version` with when that variable is set # and non-empty, and with otherwise; every other invocation From f8c5b2d8f411784c5ac43cf7ee7e4592a4a20180 Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Wed, 2 Sep 2026 18:34:27 +0530 Subject: [PATCH 2/6] no-mistakes(review): Fix recovery ordering and cleanup safeguards --- bin/fm-spawn.sh | 155 ++++++++++++++---- bin/fm-teardown.sh | 52 ++++-- .../fm-backend-herdr-presentation-e2e.test.sh | 43 ++++- tests/fm-spawn-lease-before-enter.test.sh | 26 +++ tests/fm-teardown.test.sh | 22 ++- 5 files changed, 252 insertions(+), 46 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 5940b621285..211304b5873 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -149,6 +149,10 @@ # (a quarantined Herdr recovery) refreshes that same copy instead of leasing # another, and refuses when its HEAD holds commits no remote has, so a # retry never grows the pool or resets a prior worker's unlanded commits. +# Recovery first classifies the recorded endpoint and moves an agent-free +# shell back to the primary project before inspecting or refreshing its +# recorded copy. The reclaimed replacement likewise starts outside the copy +# and enters it only after refresh. # The old order (create the pane, enter the copy # from its shell, then refresh) raced a fresh shell's git prompt against the # refresh for the worktree's index.lock under the shared .git/worktrees/, and @@ -758,7 +762,11 @@ CONFIG_INHERIT_LOCK= CONFIG_INHERIT_LOCK_HELD=0 SPAWN_LEASE_RETURN_PENDING=0 SPAWN_LEASE_WT= +SPAWN_PRIOR_RECORD_WT= HERDR_RECLAIMED_PANE=0 +HERDR_RECOVERY_PREFLIGHTED=0 +HERDR_RECOVERY_ENDPOINT_STATE= +HERDR_RECOVERY_TARGET= spawn_fresh_commit_rollback() { if fm_backlog_atomic_transition rollback "$STATE/$ID.meta" \ @@ -2027,17 +2035,56 @@ spawn_lease_return_on_abort() { # fi } -# A fresh spawn for a task id whose durable record still names a pooled copy -# (a quarantined Herdr recovery, a relaunch-shaped resume) reuses that copy -# rather than leasing a second one per attempt: the record's copy is already -# this task's lease. Prints the path only when it is a real directory distinct -# from the primary checkout. -spawn_prior_record_worktree() { # -> prints the record's worktree path - local prior - prior=$(herdr_projection_meta_field_exact "$STATE/$ID.meta" worktree 2>/dev/null) || return 1 - [ -n "$prior" ] && [ -d "$prior" ] || return 1 - [ "$(real_path_or_raw "$prior")" != "$PROJ_ABS_REAL" ] || return 1 - printf '%s\n' "$prior" +# A fresh Herdr recovery reuses a durable record's copy only when the record +# proves the same task, project, kind, backend, endpoint, and projection +# journal. An absent record means ordinary leasing; any existing record that +# cannot establish that identity refuses rather than leasing a second copy or +# overwriting foreign metadata. +spawn_prior_record_worktree() { + local meta="$STATE/$ID.meta" journal="$STATE/$ID.herdr-presentation" + local prior prior_project prior_project_real prior_kind kind_count + SPAWN_PRIOR_RECORD_WT= + if [ ! -e "$meta" ] && [ ! -L "$meta" ]; then + return 1 + fi + fm_backlog_record_present "$meta" "task record" "$STATE" || { + echo "error: existing task record for $ID is unsafe: $FM_BACKLOG_TRANSITION_ERROR" >&2 + return 2 + } + fm_backend_validate_task_endpoint "$meta" "$ID" || return 2 + prior=$(herdr_projection_meta_field_exact "$meta" worktree 2>/dev/null) || { + echo "error: existing task record for $ID has no exact worktree; refusing recovery" >&2 + return 2 + } + [ -d "$prior" ] && [ "$(real_path_or_raw "$prior")" != "$PROJ_ABS_REAL" ] || { + echo "error: existing task record for $ID names an unusable isolated copy '$prior'; refusing recovery" >&2 + return 2 + } + prior_project=$(herdr_projection_meta_field_exact "$meta" project 2>/dev/null) || { + echo "error: existing task record for $ID has no exact project; refusing recovery" >&2 + return 2 + } + prior_project_real=$(real_path_or_raw "$prior_project") + if [ "$prior_project_real" != "$PROJ_ABS_REAL" ]; then + echo "error: existing task record for $ID belongs to project '$prior_project', not '$PROJ_ABS'; refusing to reuse or replace its copy" >&2 + return 2 + fi + kind_count=$(grep -c '^kind=' "$meta" 2>/dev/null || true) + case "$kind_count" in + 0) prior_kind=ship ;; + 1) prior_kind=$(herdr_projection_meta_field_exact "$meta" kind 2>/dev/null) || prior_kind= ;; + *) prior_kind= ;; + esac + if [ -z "$prior_kind" ] || [ "$prior_kind" != "$KIND" ]; then + echo "error: existing task record for $ID has kind '${prior_kind:-ambiguous}', not '$KIND'; refusing recovery" >&2 + return 2 + fi + if [ "$BACKEND" != herdr ] || [ "$FM_BACKEND_VALIDATED_BACKEND" != herdr ] \ + || [ ! -f "$journal" ] || [ -L "$journal" ]; then + echo "error: existing task record for $ID is not an exact Herdr presentation recovery; refusing to reuse or replace its copy" >&2 + return 2 + fi + SPAWN_PRIOR_RECORD_WT=$prior } # A reused copy is refreshed under the same clean check as a leased one, and @@ -2081,6 +2128,8 @@ herdr_projection_existing_meta_allows_flat() { # HERDR_RECOVERY_WORKSPACE_ID="" HERDR_RECOVERY_TAB_ID="" HERDR_RECOVERY_PANE_ID="" + HERDR_RECOVERY_ENDPOINT_STATE="" + HERDR_RECOVERY_TARGET="" old_backend=$(fm_backend_of_meta "$meta") old_target=$(fm_backend_target_of_meta "$meta") [ -n "$old_target" ] || { @@ -2121,6 +2170,8 @@ herdr_projection_existing_meta_allows_flat() { # return 1 } old_state=$(fm_backend_herdr_pane_agent_state "$old_session" "$old_pane") + HERDR_RECOVERY_ENDPOINT_STATE=$old_state + HERDR_RECOVERY_TARGET=$old_target case "$old_state" in dead|no-agent) return 0 ;; live|unknown) @@ -2139,6 +2190,45 @@ herdr_projection_existing_meta_allows_flat() { # esac } +herdr_projection_recovery_preflight_before_refresh() { + local seen settled=0 + HERDR_LABEL_HOME=$FM_HOME + HERDR_LAUNCHER_RELATIONSHIP=launcher-home + HERDR_PRESENTATION_JOURNAL=$(fm_backend_herdr_projection_journal_path "$STATE" "$ID") + HERDR_SES=$(fm_backend_herdr_session) + HERDR_PARENT_LABEL=$(FM_HOME="$HERDR_LABEL_HOME" fm_backend_herdr_workspace_label) + fm_backend_herdr_server_ensure "$HERDR_SES" || { + echo "error: herdr presentation recovery could not ensure its exact named session" >&2 + return 1 + } + spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" || { + echo "error: herdr presentation recovery could not acquire its session lock; refusing a concurrent resume" >&2 + return 1 + } + herdr_projection_existing_meta_allows_flat "$STATE/$ID.meta" || return 1 + fm_backend_herdr_projection_recovery_allows_flat \ + "$HERDR_SES" "$HERDR_PRESENTATION_JOURNAL" "$ID" || return 1 + HERDR_RECOVERY_PREFLIGHTED=1 + [ "$HERDR_RECOVERY_ENDPOINT_STATE" = no-agent ] || return 0 + fm_backend_herdr_send_text_line "$HERDR_RECOVERY_TARGET" \ + "cd $(shell_quote "$PROJ_ABS")" || { + echo "error: existing Herdr shell for $ID could not be moved out of its recorded copy before refresh" >&2 + return 1 + } + for _ in $(seq 1 "${FM_SPAWN_SETTLE_POLLS:-60}"); do + seen=$(fm_backend_herdr_current_path "$HERDR_RECOVERY_TARGET" || true) + if [ -n "$seen" ] && [ "$(real_path_or_raw "$seen")" = "$PROJ_ABS_REAL" ]; then + settled=1 + break + fi + sleep 1 + done + if [ "$settled" -ne 1 ]; then + echo "error: existing Herdr shell for $ID did not leave its recorded copy before refresh (last read '${seen:-none}')" >&2 + return 1 + fi +} + # Backlog preflight (bin/fm-backlog-transition-lib.sh). This spawn is about to # become the sole owner of the row's In-flight transition, so prove the row is # transitionable BEFORE any endpoint, worktree, or record exists: a refusal here @@ -2198,15 +2288,22 @@ else # the project or home path as the pane's starting directory. PANE_CWD=$PROJ_ABS if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then - if WT=$(spawn_prior_record_worktree); then + if spawn_prior_record_worktree; then + WT=$SPAWN_PRIOR_RECORD_WT # The record's own copy: never returned by this spawn, refreshed in place. + herdr_projection_recovery_preflight_before_refresh || exit 1 validate_spawn_worktree "prior record" "(no endpoint created yet)" spawn_refuse_unpushed_head "$WT" || exit 1 else - WT=$(spawn_lease_pool_worktree) || exit 1 - SPAWN_LEASE_WT=$WT - SPAWN_LEASE_RETURN_PENDING=1 - validate_spawn_worktree "treehouse get --lease" "(no endpoint created yet)" + prior_status=$? + if [ "$prior_status" -eq 1 ]; then + WT=$(spawn_lease_pool_worktree) || exit 1 + SPAWN_LEASE_WT=$WT + SPAWN_LEASE_RETURN_PENDING=1 + validate_spawn_worktree "treehouse get --lease" "(no endpoint created yet)" + else + exit 1 + fi fi freshen_spawn_worktree_base "$WT" || exit 1 PANE_CWD=$WT @@ -2256,19 +2353,21 @@ case "$BACKEND" in HERDR_SES=$(fm_backend_herdr_session) HERDR_PARENT_LABEL=$(FM_HOME="$HERDR_LABEL_HOME" fm_backend_herdr_workspace_label) if [ -e "$HERDR_PRESENTATION_JOURNAL" ] || [ -L "$HERDR_PRESENTATION_JOURNAL" ]; then - fm_backend_herdr_server_ensure "$HERDR_SES" || { - echo "error: herdr presentation recovery could not ensure its exact named session" >&2 - exit 1 - } - spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" || { - echo "error: herdr presentation recovery could not acquire its session lock; refusing a concurrent resume" >&2 - exit 1 - } - if [ -e "$STATE/$ID.meta" ] || [ -L "$STATE/$ID.meta" ]; then - herdr_projection_existing_meta_allows_flat "$STATE/$ID.meta" || exit 1 + if [ "$HERDR_RECOVERY_PREFLIGHTED" != 1 ]; then + fm_backend_herdr_server_ensure "$HERDR_SES" || { + echo "error: herdr presentation recovery could not ensure its exact named session" >&2 + exit 1 + } + spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" || { + echo "error: herdr presentation recovery could not acquire its session lock; refusing a concurrent resume" >&2 + exit 1 + } + if [ -e "$STATE/$ID.meta" ] || [ -L "$STATE/$ID.meta" ]; then + herdr_projection_existing_meta_allows_flat "$STATE/$ID.meta" || exit 1 + fi + fm_backend_herdr_projection_recovery_allows_flat \ + "$HERDR_SES" "$HERDR_PRESENTATION_JOURNAL" "$ID" || exit 1 fi - fm_backend_herdr_projection_recovery_allows_flat \ - "$HERDR_SES" "$HERDR_PRESENTATION_JOURNAL" "$ID" || exit 1 if [ "${HERDR_RECOVERY_BACKEND:-}" = herdr ]; then set +e FM_HOME="$HERDR_LABEL_HOME" fm_backend_herdr_projection_reclaim_task \ diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index cbb0fe98375..4734962c630 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -108,10 +108,10 @@ # checks before any destructive return. Teardown output notes every wait, retry, and # removal so the operator can see what happened. # -# Pre-teardown cleanup sequence (runs once every landed/discard-work safety -# refusal above has already passed, and BEFORE any worktree return, branch -# delete, or backend kill below - a still-active run or a leaked process may -# own live work in that worktree): +# Pre-teardown cleanup sequence runs once every landed/discard-work safety +# refusal above has already passed. The task's parked run is concluded before +# its endpoint closes; leaked processes are reaped after that close but before +# any worktree return or branch delete. # Fix 1 - conclude the task's own no-mistakes run. A ship task's worktree can # be torn down while its no-mistakes pipeline run is still PARKED at a gate # (awaiting_approval/fix_review/any awaiting_agent field), with no worker @@ -142,6 +142,9 @@ # roots are unique per task and never # shared, so this can never reach another task's or the primary's # processes. Idempotent: nothing left to find is a silent no-op. +# When lsof is unavailable, teardown captures a verified tmux pane process +# group before closing the endpoint, then reaps that captured group after +# the close removes the pane lookup target. # Fix 3 - sweep abandoned remote job workers. A remote job worker started # from a worktree's own bin/ outlives that worktree's removal without # being reachable by Fix 2, because its working directory is wherever it @@ -1644,8 +1647,10 @@ $dir_pids" TASK_PIDS=$(printf '%s\n' "$pids" | grep -E '^[0-9]+$' | sort -un || true) } -reap_task_backend_process_group() { #