Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions bin/fm-teardown.sh
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@
# cleanup step, teardown verifies record exclusivity: no OTHER task record in
# this home or any locally registered Firstmate home may name the same live path
# in its worktree= or home=. One live path with two task records is the reuse
# collision itself, whichever record is stale.
# collision itself, whichever record is stale. The one exception is a slot whose
# owner claim (below) names another task: this teardown is then records-only and
# touches nothing under the slot, so the scan is skipped rather than stranding
# the stale record and, with it, the claimant's own teardown.
# That scan alone cannot prove THIS record is the current owner, because the task
# that took the slot next may leave no record it can reach - its own worker may
# have exited and its record been cleaned up, or it may live in a home this
Expand Down Expand Up @@ -2378,6 +2381,12 @@ require_exclusive_worktree_slot_record() {
local record_meta=$1 record_id=$2 record_state=$3 worktree=$4
local slot state_dir other other_id field other_path other_slot
slot=$(canonical_existing_dir "$worktree") || return 0
# A slot whose owner claim names another task was reassigned, so this record's
# teardown is records-only and touches nothing under it; another record naming
# the slot is then no hazard, and refusing would strand this stale record and
# block the claimant's own teardown behind it.
fm_treehouse_slot_owner_state "$slot" "$record_id"
[ "$FM_TREEHOUSE_SLOT_OWNER" != other ] || return 0
collect_local_firstmate_states "$record_state" || return 1
for state_dir in "${TREEHOUSE_OWNER_STATES[@]}"; do
for other in "$state_dir"/*.meta; do
Expand Down Expand Up @@ -2411,11 +2420,11 @@ require_exclusive_task_worktree_slot() {
# Positive slot ownership, read from the claim the task that took the slot wrote
# into the slot itself (bin/fm-wake-lib.sh owns the claim and its states).
#
# The record scan above proves that no OTHER task record names this slot. It
# cannot prove that THIS record is not the stale one, because the task that took
# the slot next may leave no record this scan can reach: its own worker may have
# exited and its record been cleaned up, or it may belong to a home this machine
# does not register. The claim closes that gap from the other side - it names the
# For a slot this task still claims, or one with no claim, the record scan above
# proves that no OTHER task record names it. It cannot prove that THIS record is
# not the stale one, because the task that took the slot next may leave no record
# this scan can reach: its own worker may have exited and its record been cleaned
# up, or it may belong to a home this machine does not register. The claim closes that gap from the other side - it names the
# task that actually took the slot, and it is written under the same project lock
# that allocates it - so a claim naming another task is proof the slot was
# reassigned after this record was written.
Expand Down
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ A later merged poll consumes only that matching persisted value; with no match i
[`bin/fm-merge-authority-lib.sh`](../bin/fm-merge-authority-lib.sh)'s header owns resolution, private atomic persistence, identity-checked consumption, and retirement, while only the merge path gates on the answer.
Teardown is fail-closed for ship worktrees: dirty worktrees refuse, and committed work must be landed before the worktree is returned.
A pool worktree is only returned after teardown passes the slot-ownership proof: a contradictory task record or a supported live endpoint refuses without touching either task, and no discard authority relaxes that.
A slot's own owner claim, written by the spawn that takes it under the allocation lock and owned by [`bin/fm-wake-lib.sh`](../bin/fm-wake-lib.sh), covers a slot reassigned to a task that left no record the scan could reach: a claim naming a different task releases nothing - teardown warns, names the claimant, and finishes only the task's own cleanup - because Treehouse's own live process lease cannot answer ownership once the worker's exit releases it.
A slot's own owner claim, written by the spawn that takes it under the allocation lock and owned by [`bin/fm-wake-lib.sh`](../bin/fm-wake-lib.sh), covers a slot reassigned to another task, including one that left no record the scan could reach: a claim naming a different task releases nothing, even alongside that task's contradictory record - teardown warns, names the claimant, and finishes only the task's own cleanup - because Treehouse's own live process lease cannot answer ownership once the worker's exit releases it.
Allocation and return serialize on one project lock per machine-local Firstmate tree: every home reachable through local parent links shares that lock, and a home seeded from another machine anchors its own, because a lock taken on this filesystem is neither held nor observable across that boundary.
Pooled worktree and secondmate-home paths are reused, so before returning a slot or destroying a worktree, teardown decides whether this task is the only record claiming that allocation; when it is not, cleanup drops to records-only, leaving the worktree, its branch, and the pooled slot untouched while still retiring the records and endpoint the task exclusively owns, so neither claimant is ever stranded and neither one's work can be returned or deleted by the other's cleanup.
[`bin/fm-allocation-lib.sh`](../bin/fm-allocation-lib.sh) owns that decision, why a matching path, a pane label, a dead endpoint, or a task-ID lease is not ownership, and why `--force` does not lift the allocation half.
Expand Down
71 changes: 71 additions & 0 deletions tests/fm-secondmate-safety.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,76 @@ EOF
pass "forced secondmate teardown refuses duplicated descendant pool slots"
}

# The descendant form of a reassigned slot: the child's stale record still names
# the slot, and so does the record of the task the slot went to - here a task of
# the parent home - whose claim is in the slot. The claim proves the child's
# record is the stale one, so the forced teardown finishes and leaves the slot,
# its claim, and the other task's record exactly as they were.
test_secondmate_force_teardown_skips_reassigned_child_slot() {
local home subhome childproj childwt fakebin log err rc
home="$TMP_ROOT/force-reassigned-slot-home"
subhome="$TMP_ROOT/force-reassigned-slot-subhome"
childproj="$subhome/projects/alpha"
childwt="$TMP_ROOT/force-reassigned-slot-pool/1/alpha"
err="$TMP_ROOT/force-reassigned-slot.err"
mkdir -p "$home/state" "$home/data" "$subhome/state" "$(dirname "$childwt")"
fm_git_worktree "$childproj" "$childwt" reassigned-child
printf '{"worktrees":[{"name":"1","path":"%s"}]}\n' "$childwt" \
> "$TMP_ROOT/force-reassigned-slot-pool/treehouse-state.json"
: > "$childwt/sentinel"
printf 'task=live-task\nhome=%s\n' "$home" > "$(dirname "$childwt")/.fm-slot-owner"
printf 'domain\n' > "$subhome/.fm-secondmate-home"
cat > "$home/state/domain.meta" <<EOF
window=firstmate:fm-domain
worktree=$subhome
project=$subhome
harness=echo
kind=secondmate
mode=secondmate
yolo=off
home=$subhome
projects=alpha
EOF
printf '%s\n' '- domain - design domain (home: '"$subhome"'; scope: design domain; projects: alpha; added 2026-06-22)' > "$home/data/secondmates.md"
cat > "$subhome/state/stale-child.meta" <<EOF
window=firstmate:fm-stale-child
worktree=$childwt
project=$childproj
harness=echo
kind=ship
mode=no-mistakes
yolo=off
EOF
cat > "$home/state/live-task.meta" <<EOF
window=firstmate:fm-live-task
worktree=$childwt
project=$childproj
harness=echo
kind=ship
mode=no-mistakes
yolo=off
EOF
fakebin=$(make_fake_tmux "$TMP_ROOT/force-reassigned-slot-fake")
log="$TMP_ROOT/force-reassigned-slot-fake/tmux.log"

set +e
PATH="$fakebin:$PATH" FM_HOME="$home" FM_FAKE_TMUX_LOG="$log" \
FM_FAKE_TMUX_CAPTURE="$TMP_ROOT/force-reassigned-slot-fake/pane.txt" \
"$ROOT/bin/fm-teardown.sh" domain --force >/dev/null 2>"$err"
rc=$?
set -e
[ "$rc" -eq 0 ] || fail "forced secondmate teardown refused a child slot its claim proves was reassigned: $(cat "$err")"
[ ! -e "$home/state/domain.meta" ] || fail "forced secondmate teardown left the secondmate record"
[ -e "$childwt/sentinel" ] || fail "forced secondmate teardown reset the slot the child's record no longer owns"
[ -e "$home/state/live-task.meta" ] || fail "forced secondmate teardown removed the record of the task the slot went to"
grep -Fx 'task=live-task' "$(dirname "$childwt")/.fm-slot-owner" >/dev/null \
|| fail "forced secondmate teardown removed or rewrote the other task's slot claim"
grep -F "treehouse return" "$log" >/dev/null \
&& fail "forced secondmate teardown returned a slot reassigned to another task: $(cat "$log")"
grep -F 'live-task' "$err" >/dev/null || fail "forced secondmate teardown did not name the task the slot went to"
pass "forced secondmate teardown skips a descendant slot its claim proves was reassigned"
}

test_secondmate_force_teardown_preserves_child_on_unproven_lock() {
local home subhome childproj childwt fakebin log err rc lock
home="$TMP_ROOT/force-lock-home"
Expand Down Expand Up @@ -3081,6 +3151,7 @@ test_secondmate_teardown_refuses_failed_leased_home_return
test_secondmate_teardown_removes_plain_clone_home_without_treehouse_return
test_secondmate_force_teardown_discards_child_work
test_secondmate_force_teardown_refuses_duplicated_child_slot
test_secondmate_force_teardown_skips_reassigned_child_slot
test_secondmate_force_teardown_preserves_child_on_unproven_lock
test_secondmate_force_teardown_allows_non_state_operational_dir_symlinks_inside_home
test_secondmate_force_teardown_refuses_operational_dir_symlink_outside_home
Expand Down
38 changes: 38 additions & 0 deletions tests/fm-teardown-endpoint-safety.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,43 @@ test_reassigned_pool_slot_finishes_own_cleanup_without_touching_the_slot() {
pass "fm-teardown: a pool slot claimed by another task is left alone while the task's own cleanup finishes"
}

# The reuse collision where BOTH records survive: the stale task's record still
# names the slot the pool handed on, and the claimant's own record names it too.
# The claim proves the stale record's teardown is records-only, so the record
# scan must not refuse it; once it is gone, the claimant tears down normally.
test_stale_record_on_claimed_slot_retires_then_claimant_tears_down() {
local dir id=stale-task other=live-task rc

dir=$(make_case slot-reassigned-both-records)
mark_case_as_treehouse_pool "$dir"
fm_write_meta "$dir/home/state/$id.meta" \
"window=firstmate:fm-$id" "endpoint_task_id=$id" \
"worktree=$dir/worktree" "project=$dir/project" "kind=scout"
fm_write_meta "$dir/home/state/$other.meta" \
"window=firstmate:fm-$other" "endpoint_task_id=$other" \
"worktree=$dir/worktree" "project=$dir/project" "kind=scout"
claim_pool_slot "$dir" "$other"

set +e
run_case "$dir" "$id" > "$dir/stdout" 2> "$dir/stderr"
rc=$?
set -e
[ "$rc" -eq 0 ] || fail "records-only teardown of a stale record on a claimed slot failed: $(cat "$dir/stderr")"
assert_reassigned_slot_left_alone "$dir" "$id" "$other" "stale record beside the claimant's record"
assert_present "$dir/worktree/sentinel" "records-only teardown reset the claimant's slot"
assert_present "$dir/home/state/$other.meta" "records-only teardown removed the claimant's record"

: > "$dir/runtime.log"
run_case "$dir" "$other" > "$dir/stdout" 2> "$dir/stderr" \
|| fail "claimant teardown failed after the stale record retired: $(cat "$dir/stderr")"
assert_absent "$dir/home/state/$other.meta" "claimant teardown left its record"
assert_absent "$dir/pool/1/.fm-slot-owner" "claimant teardown left its spent slot claim behind"
grep -Fq "treehouse <return>" "$dir/runtime.log" \
|| fail "claimant teardown did not return its pool slot: $(cat "$dir/runtime.log")"

pass "fm-teardown: a stale record on a claimed slot retires, then the claimant tears down"
}

# The two states that must never become a false refusal: the task's own claim,
# and no claim at all (a slot taken before claims existed, or already returned).
test_own_and_absent_slot_claims_still_tear_down() {
Expand Down Expand Up @@ -1540,6 +1577,7 @@ test_reused_pool_slot_refuses_before_touching_the_other_task
test_cross_home_pool_slot_collision_refuses
test_sole_slot_record_still_tears_down
test_reassigned_pool_slot_finishes_own_cleanup_without_touching_the_slot
test_stale_record_on_claimed_slot_retires_then_claimant_tears_down
test_own_and_absent_slot_claims_still_tear_down
test_recorded_endpoint_that_changed_directory_still_tears_down
test_project_lock_anchors_at_the_local_root_across_home_layouts
Expand Down
Loading