diff --git a/bin/backends/orca.sh b/bin/backends/orca.sh index 8b16f99d14..7f6bc1b1ee 100644 --- a/bin/backends/orca.sh +++ b/bin/backends/orca.sh @@ -411,7 +411,7 @@ EOF } fm_backend_orca_quiesce_terminal() { # [expected-worktree-id] [expected-label] - local terminal=$1 expected_worktree_id=${2:-} expected_label=${3:-} attempt state + local terminal=$1 expected_worktree_id=${2:-} expected_label=${3:-} state [ -n "$terminal" ] || return 1 if [ -n "$expected_worktree_id" ] || [ -n "$expected_label" ]; then state=$(fm_backend_orca_terminal_state "$terminal" "$expected_worktree_id" "$expected_label") @@ -424,7 +424,7 @@ fm_backend_orca_quiesce_terminal() { # [expected-worktree-id] [ex echo "error: failed to close Orca terminal $terminal" >&2 return 1 } - for attempt in 1 2 3 4 5; do + for _ in 1 2 3 4 5; do state=$(fm_backend_orca_terminal_state "$terminal" "$expected_worktree_id" "$expected_label") [ "$state" != absent ] || return 0 sleep 0.1 diff --git a/bin/fm-account-routing-lib.sh b/bin/fm-account-routing-lib.sh index 9d1723dced..d96617ecc8 100644 --- a/bin/fm-account-routing-lib.sh +++ b/bin/fm-account-routing-lib.sh @@ -986,6 +986,7 @@ fm_secondmate_registry_lock_acquire() { fm_secondmate_registry_query() { local registry=$1 mode=${2:-validate} expected_id=${3:-} key=${4:-} + # shellcheck disable=SC2016 # Perl source is intentionally a literal shell argument. fm_account_system_perl -MErrno=ENOENT -e ' my ($registry, $mode, $expected_id, $key) = @ARGV; if (!lstat($registry)) { diff --git a/bin/fm-fleet-sync.sh b/bin/fm-fleet-sync.sh index a231fad35f..80b97c2d58 100755 --- a/bin/fm-fleet-sync.sh +++ b/bin/fm-fleet-sync.sh @@ -682,6 +682,7 @@ run_sync_project_bounded() ( echo "$project: skipped: refresh lock ownership cannot be proved" return 0 } + # shellcheck disable=SC2031 # This whole function is a subshell; the value is intentionally local to it. export FM_PROCESS_TREE_GUARD_FILE="$lock_owner_dir/process-group" trap 'fm_lock_release "$checkout_lock"' EXIT if fm_run_bounded "$FLEET_SYNC_TIMEOUT" \ diff --git a/bin/fm-home-seed.sh b/bin/fm-home-seed.sh index 58b881ea8e..eac1c90072 100755 --- a/bin/fm-home-seed.sh +++ b/bin/fm-home-seed.sh @@ -539,7 +539,6 @@ SEED_HOME_CREATED=0 SEED_HOME_BACKED_UP=0 SEED_BACKUP_DIR= SEED_CREATED_PROJECTS_FILE= -SEED_PARENT_REG_EXISTED=0 SEED_PARENT_BRIEF= SEED_PARENT_BRIEF_CREATED=0 SEED_PARENT_BRIEF_DIR_CREATED=0 @@ -923,7 +922,6 @@ seed_home() { SEED_BACKUP_DIR=$(mktemp -d "${TMPDIR:-/tmp}/fm-home-seed.XXXXXX") SEED_CREATED_PROJECTS_FILE="$SEED_BACKUP_DIR/created-projects" : > "$SEED_CREATED_PROJECTS_FILE" - SEED_PARENT_REG_EXISTED=0 SEED_PARENT_BRIEF="$DATA/$id/brief.md" SEED_PARENT_BRIEF_CREATED=0 SEED_PARENT_BRIEF_DIR_CREATED=0 diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 6444e0f591..63bfecd7e9 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -86,9 +86,10 @@ # Before a secondmate launch, the home must fast-forward safely to the primary # default-branch commit and independently match the live default tip. # Any unproven freshness state refuses launch. -# Ship/scout spawns refresh the primary checkout before Treehouse acquisition, -# surface dirty pool entries, and durably lease one available worktree before -# creating the endpoint. They refuse to create that endpoint unless the leased +# Ship/scout spawns canonicalize a symlinked project path before exact-root +# refresh and pool preflight, surface dirty pool entries, and durably lease +# one available worktree before creating the endpoint. They refuse to create +# that endpoint unless the leased # path is a clean isolated worktree from the requested repository whose HEAD # matches its live upstream or local default-branch tip. Dirty acquisitions # remain under their durable lease for manual recovery. Other pre-commit @@ -2728,9 +2729,11 @@ fi # still-symlinked PROJ_ABS can misfire both ways: false-negative (the poll # below never notices the pane left the project) or false-positive (the # isolation guard refuses a spawn that never actually tangled). Canonicalize -# once here so every downstream comparison uses the same physical form +# once here so exact-root preflight and every downstream comparison use the +# same physical form # (docs/herdr-backend.md "Known gaps"). PROJ_ABS_REAL=$(cd "$PROJ_ABS" 2>/dev/null && pwd -P) || PROJ_ABS_REAL="$PROJ_ABS" +PROJ_ABS=$PROJ_ABS_REAL real_path_or_raw() { # local path=$1 real @@ -3393,11 +3396,11 @@ EOF fi WORKTREE_CREATED=1 fi - [ "$(fm_backend_orca_terminal_state "$ORCA_TERMINAL" "$ORCA_WORKTREE_ID" "$W")" = present ] \ - && fm_backend_orca_worktree_terminal_contains "$ORCA_WORKTREE_ID" "$W" "$ORCA_TERMINAL" || { - echo "error: Orca terminal is not authoritatively bound to worktree $ORCA_WORKTREE_ID and task $W" >&2 - exit 1 - } + if [ "$(fm_backend_orca_terminal_state "$ORCA_TERMINAL" "$ORCA_WORKTREE_ID" "$W")" != present ] \ + || ! fm_backend_orca_worktree_terminal_contains "$ORCA_WORKTREE_ID" "$W" "$ORCA_TERMINAL"; then + echo "error: Orca terminal is not authoritatively bound to worktree $ORCA_WORKTREE_ID and task $W" >&2 + exit 1 + fi T="$ORCA_TERMINAL" ENDPOINT_CREATED=1 ;; @@ -3618,11 +3621,11 @@ fi # process (go build, go test, ...) inherit it. Sent before the launch command so # the env is set when the agent starts; the brief sleep lets the export land. if [ "$BACKEND" = orca ]; then - [ "$(fm_backend_orca_terminal_state "$T" "$ORCA_WORKTREE_ID" "$W")" = present ] \ - && fm_backend_orca_worktree_terminal_contains "$ORCA_WORKTREE_ID" "$W" "$T" || { - echo "error: Orca terminal authority changed before launch for $ID" >&2 - exit 1 - } + if [ "$(fm_backend_orca_terminal_state "$T" "$ORCA_WORKTREE_ID" "$W")" != present ] \ + || ! fm_backend_orca_worktree_terminal_contains "$ORCA_WORKTREE_ID" "$W" "$T"; then + echo "error: Orca terminal authority changed before launch for $ID" >&2 + exit 1 + fi validate_orca_abort_worktree_identity || { echo "error: Orca worktree authority changed before launch for $ID" >&2 exit 1 diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ec3c764982..d5b4f7b6e8 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -26,6 +26,9 @@ # Every authorized Treehouse return is process-tree bounded by # FM_TREEHOUSE_RETURN_TIMEOUT while holding the same common checkout mutation # lock across its retry and stale-index-lock recovery sequence. +# Per-task temp cleanup accepts only the exact /tmp/fm- root, treats that +# exact root as already clean when absent, and still refuses wrong paths, +# symlinks, or traversal. # Uncommitted changes are never landed. # Ordinary teardown first proves that metadata names the exact registered project, # worktree, and task lease, then quiesces the endpoint before its final safety checks. @@ -1820,6 +1823,7 @@ if not stat.S_ISDIR(os.lstat(base).st_mode): print(base) PY ) || return 1 + [ -e "$base/fm-$ID" ] || [ -L "$base/fm-$ID" ] || return 0 removal_tree_operation "$base/fm-$ID" "task temp root" remove } @@ -4152,7 +4156,8 @@ fi remove_grok_turnend_auth "$STATE" "$ID" fm_backend_clear_transition "$BACKEND" "$STATE" "$T" || true # Remove the per-task temp root (/tmp/fm-/, incl. its gotmp/) recorded by spawn. -# Read before the state-file rm below; empty (pre-fix tasks without tasktmp=) is a no-op. +# Read before the state-file rm below; empty metadata and an already-absent +# exact root are no-ops. [ -z "$TASK_TMP" ] || safe_remove_task_tmp "$TASK_TMP" || exit 1 rm -f "$STATE/$ID.status" "$STATE/$ID.turn-ended" "$STATE/$ID.check.sh" "$STATE/$ID.meta" "$STATE/$ID.pi-ext.ts" "$STATE/$ID.grok-turnend-token" [ -z "$ACCOUNT_DELETE_LOCK" ] || fm_account_lifecycle_lock_release "$ACCOUNT_DELETE_LOCK" >/dev/null 2>&1 || true diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index b0324a9ed2..e7583bd2d6 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -855,7 +855,7 @@ Covered by the unit cases in `tests/fm-afk-launch.test.sh` (clear-on-fresh-entry - **RESOLVED: worktree-discovery isolation guard's symlinked-project-prefix false refusal.** Originally discovered while building the runtime-backend-auto-detection real smoke test (`tests/fm-backend-autodetect-smoke.test.sh`), which needed a scratch project. `fm-spawn.sh`'s `PROJ_ABS` was a LOGICAL `cd && pwd` (symlink components kept), while herdr's `foreground_cwd` (and real tmux's `pane_current_path`, on the same OS-level cwd primitive) report the PHYSICALLY resolved path. When the project itself lived under a symlinked directory (e.g. macOS's `/tmp` -> `/private/tmp`), the very first worktree-discovery poll saw two different strings for the identical starting directory and the isolation guard false-refused the spawn as "not isolated" before `treehouse get` ever moved the pane - backend-agnostic, not specific to herdr. - Fixed 2026-07-06 (backlog `fm-spawn-symlink-guard-s8`): `bin/fm-spawn.sh` now canonicalizes once into `PROJ_ABS_REAL` (`cd "$PROJ_ABS" && pwd -P`) right after `PROJ_ABS` is resolved, canonicalizes each observed pane cwd for the worktree-discovery comparison, and uses `PROJ_ABS_REAL` in `validate_spawn_worktree`'s own primary-vs-worktree comparison instead of recomputing from the still-symlinked `PROJ_ABS`. + Fixed 2026-07-06 (backlog `fm-spawn-symlink-guard-s8`) and completed for exact-root preflight on 2026-07-25: `bin/fm-spawn.sh` canonicalizes `PROJ_ABS` once with `cd "$PROJ_ABS" && pwd -P` before checkout refresh and Treehouse pool preflight, canonicalizes each observed pane cwd for the worktree-discovery comparison, and reuses that physical project root in `validate_spawn_worktree`. This removes both failure directions: a symlinked prefix can no longer false-refuse an isolated spawn, and, since both sides are physically resolved for comparison, a genuinely tangled spawn (worktree resolves to the same physical directory as the project) still correctly refuses. Verified with GNU bash 5.3.9(1)-release (aarch64-apple-darwin25.3.0) and git 2.53.0 on macOS (Darwin 25.5.0): added `tests/fm-backend.test.sh:test_spawn_symlinked_project_prefix_avoids_false_refusal`, which drives the real `bin/fm-spawn.sh` against fake-tmux panes whose first `pane_current_path` poll returns both the project's `pwd -P`-resolved physical path and its logical symlink-preserving path while `PROJ_ABS` is reached through a synthetic symlinked prefix (`ln -s `, project passed as `/proj`). Confirmed the test reproduces the original bug against the pre-fix script (`git stash` the `bin/fm-spawn.sh` change and rerun: `not ok - fm-spawn.sh should succeed for a project reached through a symlinked prefix` / `error: treehouse get did not yield an isolated worktree ...`), and passes against the fix (`bash tests/fm-backend.test.sh` reports `ok - fm-spawn.sh: a project reached through a symlinked prefix (e.g. macOS /tmp -> /private/tmp) does not trip the isolation guard's false refusal`, with the rest of that suite's assertions unaffected). diff --git a/tests/fm-account-directory.test.sh b/tests/fm-account-directory.test.sh index 8ad0c12d48..9d7496cd39 100755 --- a/tests/fm-account-directory.test.sh +++ b/tests/fm-account-directory.test.sh @@ -12,8 +12,10 @@ FAKEBIN=$(fm_fakebin "$TMP_ROOT") QUOTA_LOG="$TMP_ROOT/quota.log" HERDR_LOG="$TMP_ROOT/herdr.log" TREEHOUSE_LOG="$TMP_ROOT/treehouse.log" +export FM_TREEHOUSE_ROOT="$TMP_ROOT/treehouse" +export FM_CHECKOUT_REFRESH_STATE_BASE="$TMP_ROOT/checkout-refresh" -mkdir -p "$ACCOUNT_ROOT/codex" "$ACCOUNT_ROOT/claude" +mkdir -p "$ACCOUNT_ROOT/codex" "$ACCOUNT_ROOT/claude" "$FM_TREEHOUSE_ROOT" cat > "$FAKEBIN/quota-axi" <<'SH' #!/usr/bin/env bash @@ -70,7 +72,10 @@ case "${3:-}" in *) exit 67 ;; esac if [ -n "${FM_FAKE_HERDR_DRIFT_WORKTREE:-}" ]; then - git -C "$FM_FAKE_HERDR_DRIFT_WORKTREE" switch --quiet --detach || exit 68 + git -C "$FM_FAKE_HERDR_DRIFT_WORKTREE" \ + -c user.name='Firstmate Test' -c user.email='firstmate-test@example.invalid' \ + commit --allow-empty --quiet \ + -m 'fixture identity drift' || exit 68 fi SH chmod +x "$FAKEBIN/herdr" @@ -242,6 +247,7 @@ case "${1:-}" in fi prev=$argument done + [ "${FM_FAKE_TMUX_SEND_FAIL:-0}" != 1 ] || exit 73 exit 0 ;; esac @@ -251,11 +257,28 @@ SH cat > "$fakebin/treehouse" <<'SH' #!/usr/bin/env bash set -u -printf '%s\n' "$*" >> "${FM_FAKE_TREEHOUSE_LOG:?}" -[ "${1:-}" = return ] || exit 0 -[ "${FM_FAKE_TREEHOUSE_RETURN_FAIL:-0}" != 1 ] || exit 71 -target=${@: -1} -git worktree remove --force "$target" +case "${1:-}" in + get) + [ "${2:-}" = --lease ] || exit 2 + worktree=${FM_FAKE_TREEHOUSE_WORKTREE:?} + printf '%s\n' "$*" >> "${FM_FAKE_TREEHOUSE_LOG:?}" + git -C "$worktree" checkout --detach --quiet || exit 1 + printf '%s\n' "$worktree" + ;; + return) + [ "${FM_FAKE_TREEHOUSE_RETURN_FAIL:-0}" != 1 ] || exit 71 + target=${@: -1} + [ "$target" != . ] || target=$(pwd -P) + [ "$target" = "${FM_FAKE_TREEHOUSE_WORKTREE:?}" ] || exit 72 + cd "${FM_TREEHOUSE_RETURN_PROJECT:?}" || exit 1 + git worktree remove --force "$target" || exit 1 + git worktree prune || exit 1 + [ ! -e "$target" ] || rm -rf -- "$target" + [ ! -e "$target" ] || exit 1 + printf 'return --force %s\n' "$target" >> "${FM_FAKE_TREEHOUSE_LOG:?}" + ;; + *) exit 2 ;; +esac SH chmod +x "$fakebin/treehouse" cat > "$fakebin/forbidden-agent-fleet" <<'SH' @@ -276,8 +299,10 @@ run_direct_spawn() { FM_SPAWN_NO_GUARD=1 FM_FAKE_PANE_PATH="$worktree" TMUX="fake,1,0" \ FM_FAKE_LAUNCH_LOG="$launch_log" FM_FAKE_ENDPOINT_FILE="$home/state/.fake-endpoint" \ FM_FAKE_ENDPOINT_LABEL="fm-${1:-unknown}" FM_FAKE_KILL_RETAIN="${FM_FAKE_KILL_RETAIN:-0}" \ + FM_FAKE_TMUX_SEND_FAIL="${FM_FAKE_TMUX_SEND_FAIL:-0}" \ FM_FAKE_HERDR_DRIFT_WORKTREE="${FM_FAKE_HERDR_DRIFT_WORKTREE:-}" \ FM_FAKE_TREEHOUSE_LOG="$TREEHOUSE_LOG" \ + FM_FAKE_TREEHOUSE_WORKTREE="$worktree" \ FM_FAKE_TREEHOUSE_RETURN_FAIL="${FM_FAKE_TREEHOUSE_RETURN_FAIL:-0}" \ PATH="$FAKEBIN:$PATH" \ FM_ACCOUNT_DIRECTORY_TEST_LAB=firstmate-account-directory-test-lab-v1 \ @@ -541,8 +566,10 @@ test_direct_recovery_rejects_worktree_from_another_project() { status=$? fi [ "$status" -ne 0 ] || fail "direct recovery launched in a worktree from another project" - assert_contains "$out" "does not belong to recorded project" \ - "direct recovery project-identity refusal was not actionable" + case "$out" in + *"does not belong to recorded project"*|*"returned redirected or unprovable Git metadata"*) ;; + *) fail "direct recovery project-identity refusal was not actionable"$'\n'"$out" ;; + esac [ ! -e "$SPAWN_HOME/state/.fake-endpoint" ] || fail "project-identity mismatch created a replacement endpoint" [ ! -s "$QUOTA_LOG" ] || fail "project-identity mismatch read account quota before refusing recovery" [ ! -s "$HERDR_LOG" ] || fail "project-identity mismatch installed a profile hook before refusing recovery" @@ -686,7 +713,8 @@ test_direct_recovery_rechecks_identity_after_account_prepare() { else status=$? fi - [ "$status" -ne 0 ] || fail "direct recovery ignored worktree identity drift during account preparation" + [ "$status" -ne 0 ] \ + || fail "direct recovery ignored worktree identity drift during account preparation"$'\n'"$out" assert_contains "$out" "changed branch identity" \ "post-prepare identity drift refusal was not actionable" [ ! -e "$SPAWN_HOME/state/.fake-endpoint" ] || fail "post-prepare identity drift created a replacement endpoint" @@ -704,6 +732,7 @@ test_direct_recovery_tracks_retained_replacement_endpoint() { id=direct-retained-endpoint-z7 record=$(make_spawn_case direct-retained-endpoint codex "$id") read_spawn_case "$record" + rm -rf "/tmp/fm-$id" run_direct_spawn "$SPAWN_HOME" "$SPAWN_WORKTREE" "$SPAWN_LAUNCH_LOG" \ "$id" "$SPAWN_PROJECT" --account-pool legacy-codex-pool >/dev/null 2>&1 @@ -807,10 +836,10 @@ test_failed_new_direct_spawn_returns_worktree_after_endpoint_cleanup() { read_spawn_case "$record" recorded_worktree=$(cd "$SPAWN_WORKTREE" && pwd -P) : > "$TREEHOUSE_LOG" - : > "/tmp/fm-$id" - if out=$(run_direct_spawn "$SPAWN_HOME" "$SPAWN_WORKTREE" "$SPAWN_LAUNCH_LOG" \ - "$id" "$SPAWN_PROJECT" --account-pool legacy-codex-pool 2>&1); then + if out=$(FM_FAKE_TMUX_SEND_FAIL=1 \ + run_direct_spawn "$SPAWN_HOME" "$SPAWN_WORKTREE" "$SPAWN_LAUNCH_LOG" \ + "$id" "$SPAWN_PROJECT" --account-pool legacy-codex-pool 2>&1); then status=0 else status=$? @@ -822,7 +851,7 @@ test_failed_new_direct_spawn_returns_worktree_after_endpoint_cleanup() { [ ! -e "$SPAWN_WORKTREE" ] || fail "failed new direct spawn left its worktree registered" [ ! -e "$SPAWN_HOME/state/$id.meta" ] || fail "successful direct rollback left task metadata" [ ! -e "$SPAWN_HOME/state/.fake-endpoint" ] || fail "successful direct rollback left its endpoint" - rm -f "/tmp/fm-$id" + [ ! -e "/tmp/fm-$id" ] || fail "successful direct rollback left its task temp root" pass "failed new direct spawn removes its endpoint and returns its worktree" } @@ -834,9 +863,8 @@ test_failed_new_direct_spawn_retains_cleanup_when_worktree_return_fails() { record=$(make_spawn_case direct-new-return-fail codex "$id") read_spawn_case "$record" : > "$TREEHOUSE_LOG" - : > "/tmp/fm-$id" - if out=$(FM_FAKE_TREEHOUSE_RETURN_FAIL=1 \ + if out=$(FM_FAKE_TMUX_SEND_FAIL=1 FM_FAKE_TREEHOUSE_RETURN_FAIL=1 \ run_direct_spawn "$SPAWN_HOME" "$SPAWN_WORKTREE" "$SPAWN_LAUNCH_LOG" \ "$id" "$SPAWN_PROJECT" --account-pool legacy-codex-pool 2>&1); then status=0 @@ -851,7 +879,7 @@ test_failed_new_direct_spawn_retains_cleanup_when_worktree_return_fails() { assert_grep "rollback_pending=1" "$meta" \ "direct return failure did not fail closed" [ ! -e "$SPAWN_HOME/state/.fake-endpoint" ] || fail "direct return failure retained an already-removed endpoint" - rm -f "/tmp/fm-$id" + rm -rf "/tmp/fm-$id" pass "direct spawn persists cleanup state when worktree return cannot be confirmed" } diff --git a/tests/fm-account-routing.test.sh b/tests/fm-account-routing.test.sh index 8fbd9fb68c..134361b2f6 100755 --- a/tests/fm-account-routing.test.sh +++ b/tests/fm-account-routing.test.sh @@ -502,7 +502,7 @@ test_off_is_byte_compatible_and_never_calls_agent_fleet() { test_failed_freshness_proof_rolls_back_unmanaged_resources() { local id rec out status default_branch - id=checkout-freshness-rollback-z1a + id='checkout-freshness-rollback-z1a' rec=$(make_case checkout-freshness-rollback claude "$id") read_case "$rec" default_branch=$(git -C "$PROJ_DIR" branch --show-current) @@ -536,7 +536,7 @@ test_failed_freshness_proof_rolls_back_unmanaged_resources() { test_local_only_spawn_uses_local_default_tip() { local id rec out status - id=checkout-local-only-z1b + id='checkout-local-only-z1b' rec=$(make_case checkout-local-only claude "$id") read_case "$rec" git -C "$PROJ_DIR" remote remove origin @@ -555,7 +555,7 @@ test_local_only_spawn_uses_local_default_tip() { test_dirty_acquisition_is_retained_without_force_return() { local id rec draft out status - id=checkout-dirty-retain-z1c + id='checkout-dirty-retain-z1c' rec=$(make_case checkout-dirty-retain claude "$id") read_case "$rec" draft="$WT_DIR/.agents/skills/unlanded/SKILL.md" @@ -587,7 +587,7 @@ test_dirty_acquisition_is_retained_without_force_return() { test_treehouse_acquisition_timeout_is_bounded_before_endpoint_creation() { local id rec out status - id=checkout-acquire-timeout-z1f + id='checkout-acquire-timeout-z1f' rec=$(make_case checkout-acquire-timeout claude "$id") read_case "$rec" @@ -609,7 +609,7 @@ test_treehouse_acquisition_timeout_is_bounded_before_endpoint_creation() { test_changed_acquisition_is_retained_during_unmanaged_rollback() { local id rec marker release out_file spawn_pid - id=checkout-changed-rollback-z1g + id='checkout-changed-rollback-z1g' rec=$(make_case checkout-changed-rollback pi "$id") read_case "$rec" marker="$CASE_DIR/gotmp-send-started" @@ -645,7 +645,7 @@ test_changed_acquisition_is_retained_during_unmanaged_rollback() { test_unmanaged_postinstall_failure_restores_prior_state() { local id rec expected out status artifact common key expected_lock lock_marker - id=checkout-unmanaged-restore-z1d + id='checkout-unmanaged-restore-z1d' rec=$(make_case checkout-unmanaged-restore pi "$id") read_case "$rec" fm_write_meta "$HOME_DIR/state/$id.meta" \ @@ -695,7 +695,7 @@ test_unmanaged_postinstall_failure_restores_prior_state() { test_spawn_rollback_relays_unverified_treehouse_cleanup() { local id rec out status real_ps common key lock group owner child_pid - id=checkout-unverified-return-z1h + id='checkout-unverified-return-z1h' rec=$(make_case checkout-unverified-return pi "$id") read_case "$rec" real_ps=$(command -v ps) @@ -752,7 +752,7 @@ SH test_unmanaged_rollback_waits_for_metadata_lock() { local id rec marker release out_file spawn_pid held - id=checkout-unmanaged-rollback-lock-z1e + id='checkout-unmanaged-rollback-lock-z1e' rec=$(make_case checkout-unmanaged-rollback-lock pi "$id") read_case "$rec" fm_write_meta "$HOME_DIR/state/$id.meta" \ diff --git a/tests/fm-backend.test.sh b/tests/fm-backend.test.sh index a09459b60c..46a2013dda 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -36,6 +36,9 @@ fm_git_identity fmtest fmtest@example.invalid . "$ROOT/bin/fm-backend.sh" TMP_ROOT=$(fm_test_tmproot fm-backend-tests) +export FM_TREEHOUSE_ROOT="$TMP_ROOT/treehouse" +export FM_CHECKOUT_REFRESH_STATE_BASE="$TMP_ROOT/checkout-refresh" +mkdir -p "$FM_TREEHOUSE_ROOT" # fm_backend_detect's cmux fallback (bundle id + process ancestry, # docs/cmux-backend.md "Runtime auto-detection") consults uname, lsappinfo, @@ -967,6 +970,31 @@ test_peek_conformance_old_vs_new() { # --- old vs new: fm-spawn.sh -------------------------------------------------- +install_spawn_treehouse_fake() { # + local fakebin=$1 worktree=$2 + printf '%s\n' "$worktree" > "$fakebin/treehouse-worktree" + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + get) + [ "${2:-}" = --lease ] || exit 2 + IFS= read -r worktree < "$(dirname "$0")/treehouse-worktree" + git -C "$worktree" checkout --detach --quiet || exit 1 + printf '%s\n' "$worktree" + ;; + return) + worktree=$(pwd -P) || exit 1 + project=${FM_TREEHOUSE_RETURN_PROJECT:?} + cd "$project" || exit 1 + git worktree remove --force "$worktree" + ;; + *) exit 2 ;; +esac +SH + chmod +x "$fakebin/treehouse" +} + make_spawn_fakebin() { # -> echoes fakebin dir local dir=$1 wt=$2 fb="$1/fakebin" mkdir -p "$fb" @@ -983,7 +1011,7 @@ esac exit 0 SH chmod +x "$fb/tmux" - fm_fake_exit0 "$fb" treehouse + install_spawn_treehouse_fake "$fb" "$wt" printf '%s\n' "$fb" } @@ -1053,7 +1081,7 @@ esac exit 0 SH chmod +x "$fb/tmux" - fm_fake_exit0 "$fb" treehouse + install_spawn_treehouse_fake "$fb" "$wt" printf '%s\n' "$fb" } diff --git a/tests/fm-gotmp.test.sh b/tests/fm-gotmp.test.sh index 8ce412d29f..963fc71271 100755 --- a/tests/fm-gotmp.test.sh +++ b/tests/fm-gotmp.test.sh @@ -40,15 +40,33 @@ cleanup() { trap cleanup EXIT TMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/fm-gotmp-tests.XXXXXX") +TMP_ROOT=$(cd "$TMP_ROOT" && pwd -P) + +make_fixture_project() { + local project=$1 worktree=$2 id=$3 + mkdir -p "$project" "$(dirname "$worktree")" + git -C "$project" init -q + printf 'fixture\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 -C "$project" worktree add --quiet -b "fm/$id" "$worktree" +} # Build a fake FM_HOME/FM_ROOT so the real fm-teardown.sh (symlinked in) resolves # state and helper scripts inside it. Stub the helper scripts fm-teardown calls so no -# live tmux/treehouse/fleet state is touched. A nonexistent worktree path makes both -# `if [ -d "$WT" ]` guards skip, so teardown runs straight to the cleanup + state rm. +# live tmux/treehouse/fleet state is touched. The leased worktree and Treehouse state +# are real local fixtures because teardown validates both before allowing cleanup. make_fake_root() { - local id=$1 tasktmp=$2 - local fake="$TMP_ROOT/$id" - mkdir -p "$fake/bin/backends" "$fake/state" + local id=$1 tasktmp=$2 fixture=${3:-$1} + local fake="$TMP_ROOT/$fixture" project="$TMP_ROOT/$fixture/project" + local worktree="$TMP_ROOT/$fixture/treehouse-pool/1/worktree" + mkdir -p "$fake/bin/backends" "$fake/state" "$fake/data/$id" "$fake/fakebin" "$fake/user" + make_fixture_project "$project" "$worktree" "$id" + printf 'fixture scout report\n' > "$fake/data/$id/report.md" + cat > "$fake/treehouse-pool/treehouse-state.json" < "$fake/bin/fm-tasks-axi-lib.sh" <<'SH' fm_tasks_axi_backend_available() { return 1; } SH - # Meta with a nonexistent worktree so the dirty/treehouse blocks skip. - cat > "$fake/state/$id.meta" < "$fake/fakebin/tmux" <<'SH' +#!/usr/bin/env bash +case "${1:-}" in + list-windows) exit 0 ;; + *) exit 1 ;; +esac +SH + cat > "$fake/fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + return) + target=$(pwd -P) || exit 1 + project=${FM_TREEHOUSE_RETURN_PROJECT:?} + cd "$project" || exit 1 + git worktree remove --force "$target" + ;; + *) exit 2 ;; +esac +SH + chmod +x "$fake/fakebin/tmux" "$fake/fakebin/treehouse" + { + echo "window=fakeses:fm-$id" + echo "worktree=$worktree" + echo "project=$project" + echo "harness=claude" + echo "kind=scout" + echo "mode=no-mistakes" + echo "yolo=off" + [ -z "$tasktmp" ] || echo "tasktmp=$tasktmp" + } > "$fake/state/$id.meta" printf '%s' "$fake" } +run_teardown() { + local fake=$1 id=$2 + HOME="$fake/user" PATH="$fake/fakebin:$PATH" FM_HOME="$fake" \ + FM_TREEHOUSE_ROOT="$fake/treehouse-pool" FM_CHECKOUT_REFRESH_STATE_BASE="$fake/checkout-refresh" \ + bash "$fake/bin/fm-teardown.sh" "$id" +} + # --- fm-spawn side --- test_spawn_contract_and_mkdir_pattern() { @@ -134,6 +182,7 @@ test_spawn_contract_and_mkdir_pattern() { test_teardown_removes_tasktmp_dir() { local id=td-rm-z2 local task_tmp="/tmp/fm-$id" + local out mkdir -p "$task_tmp/gotmp" printf 'leftover\n' > "$task_tmp/gotmp/build-artifact" local fake @@ -141,10 +190,10 @@ test_teardown_removes_tasktmp_dir() { # Sanity: dir + contents exist before teardown. [ -d "$task_tmp/gotmp" ] || fail "precondition: gotmp missing before teardown" # Run the REAL teardown against the fake root. - FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>&1 \ - || fail "teardown exited non-zero with a valid tasktmp" + out=$(run_teardown "$fake" "$id" 2>&1) \ + || fail "teardown exited non-zero with a valid tasktmp"$'\n'"$out" [ ! -e "$task_tmp" ] \ - || fail "teardown did not remove the tasktmp dir ($task_tmp still exists)" + || fail "teardown did not remove the tasktmp dir ($task_tmp still exists)"$'\n'"$out" pass "fm-teardown removes the dir pointed to by tasktmp= in meta" } @@ -152,60 +201,50 @@ test_teardown_skips_gracefully_without_tasktmp() { # Backward compat: a meta from a pre-fix task has no tasktmp= line. Teardown must # not error and must not remove anything. local id=td-absent-z3 - local fake="$TMP_ROOT/$id-root" - mkdir -p "$fake/bin/backends" "$fake/state" - ln -s "$TEARDOWN" "$fake/bin/fm-teardown.sh" - ln -s "$ROOT/bin/fm-backend.sh" "$fake/bin/fm-backend.sh" - ln -s "$ROOT/bin/backends/tmux.sh" "$fake/bin/backends/tmux.sh" - ln -s "$ROOT/bin/fm-tmux-lib.sh" "$fake/bin/fm-tmux-lib.sh" - ln -s "$ROOT/bin/fm-composer-lib.sh" "$fake/bin/fm-composer-lib.sh" - ln -s "$ROOT/bin/fm-lock-lib.sh" "$fake/bin/fm-lock-lib.sh" - # fm-gate-refuse-lib.sh: teardown sources it before any fleet mutation. - ln -s "$ROOT/bin/fm-gate-refuse-lib.sh" "$fake/bin/fm-gate-refuse-lib.sh" - ln -s "$ROOT/bin/fm-account-routing-lib.sh" "$fake/bin/fm-account-routing-lib.sh" - cat > "$fake/bin/fm-guard.sh" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$fake/bin/fm-guard.sh" - cat > "$fake/bin/fm-fleet-sync.sh" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$fake/bin/fm-fleet-sync.sh" - cat > "$fake/bin/fm-tasks-axi-lib.sh" <<'SH' -fm_tasks_axi_backend_available() { return 1; } -SH - # No tasktmp= line at all. - cat > "$fake/state/$id.meta" </dev/null 2>&1 \ + local fake + fake=$(make_fake_root "$id" '') + run_teardown "$fake" "$id" >/dev/null 2>&1 \ || fail "teardown exited non-zero when tasktmp= was absent" pass "fm-teardown skips gracefully when tasktmp= is absent (backward compat)" } -test_teardown_skips_gracefully_when_dir_missing() { - # tasktmp= points to a path that does not exist. Teardown must not error. - local id=td-missing-z4 +test_second_teardown_with_already_removed_tasktmp_succeeds() { + # Two teardown generations for the same task id share the deterministic tasktmp. + # The first removes it; the second must treat the already-absent exact path as success. + local id=td-repeat-z4 local task_tmp="/tmp/fm-$id" - # Intentionally do NOT create $task_tmp. - [ ! -e "$task_tmp" ] || fail "precondition: task_tmp should not exist yet" - local fake - fake=$(make_fake_root "$id" "$task_tmp") - FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>&1 \ - || fail "teardown exited non-zero when tasktmp dir was missing" + local first second out + mkdir -p "$task_tmp/gotmp" + first=$(make_fake_root "$id" "$task_tmp" "$id-first") + out=$(run_teardown "$first" "$id" 2>&1) \ + || fail "first teardown exited non-zero with a valid tasktmp"$'\n'"$out" + [ ! -e "$task_tmp" ] || fail "first teardown did not remove $task_tmp" + second=$(make_fake_root "$id" "$task_tmp" "$id-second") + out=$(run_teardown "$second" "$id" 2>&1) \ + || fail "second teardown exited non-zero with an already-absent exact tasktmp"$'\n'"$out" [ ! -e "$task_tmp" ] || fail "teardown created/left the tasktmp dir unexpectedly" - pass "fm-teardown skips gracefully when tasktmp= points to a nonexistent dir" + pass "a second teardown for the same task succeeds after its exact tasktmp is already gone" +} + +test_teardown_refuses_wrong_tasktmp_path() { + local id=td-wrong-z5 + local wrong="$TMP_ROOT/wrong-tasktmp-$id" fake out + mkdir -p "$wrong" + printf 'retain\n' > "$wrong/sentinel" + fake=$(make_fake_root "$id" "$wrong") + if out=$(run_teardown "$fake" "$id" 2>&1); then + fail "teardown accepted a tasktmp path other than the exact /tmp/fm- path" + fi + case "$out" in + *"REFUSED: unsafe task temp path in metadata for $id: $wrong"*) ;; + *) fail "wrong tasktmp refusal did not name the unsafe metadata path"$'\n'"$out" ;; + esac + [ -f "$wrong/sentinel" ] || fail "wrong tasktmp path was modified despite refusal" + pass "fm-teardown still refuses and preserves a wrong tasktmp path" } test_spawn_contract_and_mkdir_pattern test_teardown_removes_tasktmp_dir test_teardown_skips_gracefully_without_tasktmp -test_teardown_skips_gracefully_when_dir_missing +test_second_teardown_with_already_removed_tasktmp_succeeds +test_teardown_refuses_wrong_tasktmp_path diff --git a/tests/fm-spawn-dispatch-profile.test.sh b/tests/fm-spawn-dispatch-profile.test.sh index 477e8b9541..18fae7d71e 100755 --- a/tests/fm-spawn-dispatch-profile.test.sh +++ b/tests/fm-spawn-dispatch-profile.test.sh @@ -12,9 +12,39 @@ set -u SPAWN="$ROOT/bin/fm-spawn.sh" TMP_ROOT=$(fm_test_tmproot fm-spawn-dispatch-profile) +FM_TEST_REAL_GIT=$(command -v git) +export FM_TEST_REAL_GIT +export FM_TREEHOUSE_ROOT="$TMP_ROOT/treehouse" +export FM_CHECKOUT_REFRESH_STATE_BASE="$TMP_ROOT/checkout-refresh" +mkdir -p "$FM_TREEHOUSE_ROOT" + +install_spawn_treehouse_fake() { + local fakebin=$1 worktree=$2 + printf '%s\n' "$worktree" > "$fakebin/treehouse-worktree" + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + get) + [ "${2:-}" = --lease ] || exit 2 + IFS= read -r worktree < "$(dirname "$0")/treehouse-worktree" + git -C "$worktree" checkout --detach --quiet || exit 1 + printf '%s\n' "$worktree" + ;; + return) + worktree=$(pwd -P) || exit 1 + project=${FM_TREEHOUSE_RETURN_PROJECT:?} + cd "$project" || exit 1 + git worktree remove --force "$worktree" + ;; + *) exit 2 ;; +esac +SH + chmod +x "$fakebin/treehouse" +} make_spawn_fakebin() { - local dir=$1 fakebin + local dir=$1 worktree=$2 fakebin fakebin=$(fm_fakebin "$dir") cat > "$fakebin/tmux" <<'SH' #!/usr/bin/env bash @@ -42,7 +72,29 @@ esac exit 0 SH chmod +x "$fakebin/tmux" - fm_fake_exit0 "$fakebin" treehouse + install_spawn_treehouse_fake "$fakebin" "$worktree" + cat > "$fakebin/git" <<'SH' +#!/usr/bin/env bash +set -u +case " $* " in + *" ls-remote --symref origin HEAD "*) + repository=. + while [ $# -gt 0 ]; do + if [ "$1" = -C ]; then + shift + repository=${1:?} + fi + shift + done + remote_head=$("$FM_TEST_REAL_GIT" -C "$repository" \ + symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null) || exit 1 + remote_tip=$("$FM_TEST_REAL_GIT" -C "$repository" rev-parse "$remote_head") || exit 1 + printf 'ref: refs/heads/%s\tHEAD\n%s\tHEAD\n' "${remote_head#origin/}" "$remote_tip" + ;; + *) exec "$FM_TEST_REAL_GIT" "$@" ;; +esac +SH + chmod +x "$fakebin/git" printf '%s\n' "$fakebin" } @@ -54,7 +106,7 @@ make_spawn_case() { proj="$case_dir/project" wt="$case_dir/wt" launchlog="$case_dir/launch.log" - fakebin=$(make_spawn_fakebin "$case_dir/fake") + fakebin=$(make_spawn_fakebin "$case_dir/fake" "$wt") mkdir -p "$home/data" "$home/projects" "$home/state" "$home/config" printf '%s\n' "$harness" > "$home/config/crew-harness" fm_git_worktree "$proj" "$wt" "wt-$name" @@ -73,18 +125,34 @@ enable_dispatch_profile() { } make_seeded_secondmate_home() { - local home=$1 id=$2 - mkdir -p "$home/bin" "$home/data" - printf '# Firstmate\n' > "$home/AGENTS.md" + local home=$1 id=$2 registry=$3 home_real + git clone --quiet --no-checkout "$ROOT" "$home" + git -C "$home" checkout --quiet -b main "$("$FM_TEST_REAL_GIT" -C "$ROOT" rev-parse HEAD)" + git -C "$home" update-ref refs/remotes/origin/main HEAD + git -C "$home" symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/main + git -C "$home" branch --set-upstream-to=origin/main main >/dev/null + mkdir -p "$home/data" printf '%s\n' "$id" > "$home/.fm-secondmate-home" printf 'charter for %s\n' "$id" > "$home/data/charter.md" + home_real=$(cd "$home" && pwd -P) + printf -- '- %s - test secondmate (home: %s; scope: dispatch profile test; projects: ; added 2026-07-25)\n' \ + "$id" "$home_real" > "$registry" +} + +make_primary_home() { + local home=$1 + git clone --quiet --no-checkout "$ROOT" "$home" + git -C "$home" checkout --quiet -b main "$("$FM_TEST_REAL_GIT" -C "$ROOT" rev-parse HEAD)" + git -C "$home" update-ref refs/remotes/origin/main HEAD + git -C "$home" symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/main + git -C "$home" branch --set-upstream-to=origin/main main >/dev/null } run_spawn() { local home=$1 wt=$2 fakebin=$3 launchlog=$4 shift 4 : > "$launchlog" - FM_ROOT_OVERRIDE='' FM_HOME="$home" \ + FM_ROOT_OVERRIDE="${FM_TEST_ROOT_OVERRIDE:-}" FM_HOME="$home" \ FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \ FM_SPAWN_NO_GUARD=1 FM_FAKE_PANE_PATH="$wt" TMUX="fake,1,0" \ @@ -366,17 +434,21 @@ test_batch_forwards_shared_profile_flags() { } test_active_dispatch_profile_does_not_block_secondmate_launch() { - local rec id sm out status + local rec id primary sm out status id=profile-secondmate-z16 rec=$(make_spawn_case profile-secondmate codex "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" + primary="$CASE_DIR/primary-home" sm="$CASE_DIR/secondmate-home" - make_seeded_secondmate_home "$sm" "$id" + make_primary_home "$primary" + make_seeded_secondmate_home "$sm" "$id" "$HOME_DIR/data/secondmates.md" - out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$sm" --secondmate) + out=$(FM_TEST_ROOT_OVERRIDE="$primary" \ + run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$sm" --secondmate) status=$? - expect_code 0 "$status" "secondmate spawn should be exempt from the dispatch-profile explicit harness requirement" + expect_code 0 "$status" \ + "secondmate spawn should be exempt from the dispatch-profile explicit harness requirement"$'\n'"$out" assert_contains "$out" "spawned $id harness=codex kind=secondmate" "secondmate launch did not use secondmate harness resolution" assert_grep "kind=secondmate" "$HOME_DIR/state/$id.meta" "secondmate meta missing kind=secondmate" assert_meta_profile "$HOME_DIR/state/$id.meta" codex default default