diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 9158fce64df..33f239e4fa4 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1825,6 +1825,37 @@ real_path_or_raw() { # fi } +# A fresh task may only record a worktree that no other live task record names. +# A holder's task branch is durable ownership evidence even after its endpoint +# stops; an unreadable or live endpoint is also retained as a live claim. A +# detached pool slot with a missing or dead endpoint has no live holder branch. +spawn_refuse_worktree_collision() { # + local task_id=$1 worktree=$2 target_real current_branch other_meta holder_id holder_wt holder_real + local holder_backend holder_target holder_state + target_real=$(real_path_or_raw "$worktree") + current_branch=$(git -C "$worktree" symbolic-ref --quiet --short HEAD 2>/dev/null || true) + for other_meta in "$STATE"/*.meta; do + [ -f "$other_meta" ] && [ ! -L "$other_meta" ] || continue + holder_id=$(basename "$other_meta" .meta) + [ "$holder_id" != "$task_id" ] || continue + holder_wt=$(fm_meta_get "$other_meta" worktree) + [ -n "$holder_wt" ] || continue + holder_real=$(real_path_or_raw "$holder_wt") + [ "$holder_real" = "$target_real" ] || continue + holder_backend=$(fm_backend_of_meta "$other_meta") + holder_target=$(fm_backend_target_of_meta "$other_meta") + holder_state=$(fm_backend_agent_state "$holder_backend" "$holder_target") + case "$holder_state" in + dead|missing) + [ "$current_branch" = "fm/$holder_id" ] || continue + ;; + esac + echo "error: spawn refused: worktree '$worktree' is already held by task '$holder_id'" >&2 + echo "Resolve task '$holder_id' and retry after its work is safely landed and its record is cleaned up; do not bypass this refusal." >&2 + return 1 + done +} + # Session-provider container-ensure + task creation. tmux stays exactly as P1 # left it (same session-name / new-window sequence, see bin/backends/tmux.sh); # a herdr spawn goes through the version-gated, workspace-per-HOME, @@ -2469,6 +2500,9 @@ elif [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then validate_spawn_worktree "treehouse get" "$T" fi +if [ "$RELAUNCH" -eq 0 ]; then + spawn_refuse_worktree_collision "$ID" "$WT" || exit 1 +fi if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ]; then freshen_spawn_worktree_base "$WT" || exit 1 fi diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ad9e042ba11..5050c81b5fd 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1254,6 +1254,26 @@ canonical_existing_dir() { ( cd "$target" && pwd -P ) } +# A task may only tear down its own task branch. A pooled copy checked out on +# another task's branch belongs to that task, so this guard refuses before any +# branch, endpoint, worktree, or record mutation. +teardown_refuse_if_other_task_branch() { + local branch holder_id + [ "$KIND" != secondmate ] || return 0 + [ -d "$WT" ] || return 0 + branch=$(git -C "$WT" symbolic-ref --quiet --short HEAD 2>/dev/null || true) + [ -n "$branch" ] || return 0 + [ "$branch" != "fm/$ID" ] || return 0 + case "$branch" in + fm/*) holder_id=${branch#fm/} ;; + *) return 0 ;; + esac + [ -n "$holder_id" ] && [ "$holder_id" != "$ID" ] || return 0 + echo "REFUSED: task $ID's recorded worktree $WT is checked out on branch $branch for task $holder_id; refusing teardown to avoid changing another task's copy." >&2 + echo "Resolve task $holder_id and retry teardown after its work is safely landed; do not use --force to bypass this refusal." >&2 + return 1 +} + retry_wait_secs_is_valid() { [[ "$1" =~ ^([0-9]+([.][0-9]*)?|[.][0-9]+)$ ]] } @@ -2664,6 +2684,8 @@ if [ "$BACKEND" = orca ] && [ "$KIND" != scout ] && [ "$KIND" != secondmate ] && ORCA_PATH_MATCH_VERIFIED=1 fi +teardown_refuse_if_other_task_branch || exit 1 + if [ -d "$WT" ] && [ "$FORCE" != "--force" ]; then if validate_worktree_teardown_safety; then : diff --git a/tests/fm-worktree-ownership.test.sh b/tests/fm-worktree-ownership.test.sh new file mode 100644 index 00000000000..6d1a0944b9c --- /dev/null +++ b/tests/fm-worktree-ownership.test.sh @@ -0,0 +1,195 @@ +#!/usr/bin/env bash +# Tests for task ownership guards around pooled worktrees. +set -u + +# shellcheck source=tests/fixtures.sh +. "$(dirname "${BASH_SOURCE[0]}")/fixtures.sh" + +SPAWN="$ROOT/bin/fm-spawn.sh" +TEARDOWN="$ROOT/bin/fm-teardown.sh" +TMP_ROOT=$(fm_test_tmproot fm-worktree-ownership) + +make_spawn_case() { + local name=$1 dir="$TMP_ROOT/spawn-$1" home project worktree fakebin + home="$dir/home" + project="$dir/project" + worktree="$dir/worktree" + mkdir -p "$dir" + fm_test_spawn_home "$home" codex + fm_test_spawn_brief "$home" spawn-task + fm_git_worktree "$project" "$worktree" "fm/spawn-owner" + fakebin=$(fm_test_make_spawn_fakebin "$dir/fakebin") + printf '%s\n' "$home|$project|$worktree|$fakebin" +} + +read_spawn_case() { + IFS='|' read -r HOME_DIR PROJECT_DIR WORKTREE_DIR FAKEBIN_DIR < "$dir/fakebin/tmux" <<'SH' +#!/usr/bin/env bash +printf 'tmux %s\n' "$*" >> "${FM_RUNTIME_LOG:?}" +exit 0 +SH + cat > "$dir/fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +printf 'treehouse %s\n' "$*" >> "${FM_RUNTIME_LOG:?}" +exit 0 +SH + cat > "$dir/fakebin/no-mistakes" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = axi ] && [ "${2:-}" = status ]; then + exit 0 +fi +exit 0 +SH + chmod +x "$dir/fakebin/tmux" "$dir/fakebin/treehouse" "$dir/fakebin/no-mistakes" + touch "$home/state/.last-watcher-beat" + printf '%s\n' "$dir|$home|$project|$worktree|$fakebin" +} + +read_teardown_case() { + IFS='|' read -r CASE_DIR HOME_DIR PROJECT_DIR WORKTREE_DIR FAKEBIN_DIR <&1) + rc=$? + set -e + + expect_code 1 "$rc" "spawn collision must refuse" + assert_contains "$out" "worktree '$WORKTREE_DIR' is already held by task 'spawn-owner'" \ + "spawn collision did not name the holding task and worktree" + assert_contains "$out" "Resolve task 'spawn-owner'" \ + "spawn collision did not provide an operator next step" + assert_absent "$HOME_DIR/state/spawn-task.meta" \ + "refused spawn published task metadata" + after=$(git -C "$WORKTREE_DIR" rev-parse HEAD) + [ "$after" = "$before" ] || fail "refused spawn changed the held worktree" + pass "fm-spawn refuses a worktree already named by another task" +} + +test_spawn_free_worktree_succeeds() { + local rec out rc + rec=$(make_spawn_case free) + read_spawn_case "$rec" + + set +e + out=$(run_spawn spawn-task 2>&1) + rc=$? + set -e + + expect_code 0 "$rc" "free spawn must succeed" + assert_contains "$out" "spawned spawn-task" "free spawn did not report success" + assert_grep "worktree=$WORKTREE_DIR" "$HOME_DIR/state/spawn-task.meta" \ + "free spawn did not record its worktree" + pass "fm-spawn still succeeds on a genuinely free worktree" +} + +test_teardown_other_task_branch_refuses() { + local rec out rc branch + rec=$(make_teardown_case collision) + read_teardown_case "$rec" + write_teardown_meta teardown-task + git -C "$WORKTREE_DIR" branch -m fm/other-task + fm_write_meta "$HOME_DIR/state/other-task.meta" \ + "window=firstmate:fm-other-task" \ + "endpoint_task_id=other-task" \ + "worktree=$WORKTREE_DIR" \ + "project=$PROJECT_DIR" \ + "kind=ship" + : > "$CASE_DIR/runtime.log" + + set +e + out=$(run_teardown teardown-task 2>&1) + rc=$? + set -e + + expect_code 1 "$rc" "teardown of another task branch must refuse" + assert_contains "$out" "branch fm/other-task for task other-task" \ + "teardown refusal did not name the branch and holding task" + assert_contains "$out" "retry teardown after its work is safely landed" \ + "teardown refusal did not provide an operator next step" + assert_present "$HOME_DIR/state/teardown-task.meta" \ + "refused teardown removed task metadata" + assert_present "$HOME_DIR/state/other-task.meta" \ + "refused teardown removed holding metadata" + assert_present "$WORKTREE_DIR/.git" "refused teardown changed worktree git state" + branch=$(git -C "$WORKTREE_DIR" symbolic-ref --short HEAD) + [ "$branch" = fm/other-task ] || fail "refused teardown changed branch to $branch" + [ ! -s "$CASE_DIR/runtime.log" ] || fail "refused teardown ran cleanup: $(cat "$CASE_DIR/runtime.log")" + pass "fm-teardown refuses a copy checked out on another task's branch" +} + +test_teardown_own_task_branch_succeeds() { + local rec out rc + rec=$(make_teardown_case own) + read_teardown_case "$rec" + write_teardown_meta teardown-task + : > "$CASE_DIR/runtime.log" + + set +e + out=$(run_teardown teardown-task 2>&1) + rc=$? + set -e + + expect_code 0 "$rc" "teardown of own branch must succeed" + assert_contains "$out" "teardown teardown-task complete" \ + "own-branch teardown did not report success" + assert_absent "$HOME_DIR/state/teardown-task.meta" \ + "own-branch teardown left task metadata" + pass "fm-teardown still succeeds for a copy on its own task branch" +} + +test_spawn_collision_refuses +test_spawn_free_worktree_succeeds +test_teardown_other_task_branch_refuses +test_teardown_own_task_branch_succeeds + +echo "# all fm-worktree-ownership tests passed"