From ff0bf25d06c41cf03cc6d383edffe2455a56c86f Mon Sep 17 00:00:00 2001 From: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:19:52 -0400 Subject: [PATCH 1/2] fix(bin): let a stale record on a reassigned slot retire records-only (#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes #6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots (cherry picked from commit 65c75b0dab02f2bd6c293cf21a20f712dfac16bd) --- bin/fm-teardown.sh | 21 +++++++++---- docs/architecture.md | 2 +- tests/fm-teardown-endpoint-safety.test.sh | 38 +++++++++++++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 6bed0603..4b1883ba 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -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 @@ -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 @@ -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. diff --git a/docs/architecture.md b/docs/architecture.md index 554963fc..1efc9605 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -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. diff --git a/tests/fm-teardown-endpoint-safety.test.sh b/tests/fm-teardown-endpoint-safety.test.sh index b0e5d6f1..fb51ca1d 100755 --- a/tests/fm-teardown-endpoint-safety.test.sh +++ b/tests/fm-teardown-endpoint-safety.test.sh @@ -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 " "$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() { @@ -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 From 2ad8ae706b7b9deea5af1729f1af114a58a17ba9 Mon Sep 17 00:00:00 2001 From: Arjun Madhavan Date: Thu, 1 Oct 2026 17:43:24 -0400 Subject: [PATCH 2/2] test(teardown): cover a reassigned descendant slot in forced secondmate teardown The descendant preflight of a forced secondmate teardown runs the same slot guards as a task's own teardown. Pin that a child's stale record on a slot claimed by another recorded task is retired records-only, leaving the slot, its claim, and the other task's record untouched. --- tests/fm-secondmate-safety.test.sh | 71 ++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/fm-secondmate-safety.test.sh b/tests/fm-secondmate-safety.test.sh index 38fabff2..a2b3d3d6 100755 --- a/tests/fm-secondmate-safety.test.sh +++ b/tests/fm-secondmate-safety.test.sh @@ -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" < "$home/data/secondmates.md" + cat > "$subhome/state/stale-child.meta" < "$home/state/live-task.meta" </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" @@ -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