Skip to content

fix(bin): let stale task cleanup skip reassigned pool slots #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
markpol merged 2 commits into from
Sep 28, 2026
Merged

fix(bin): let stale task cleanup skip reassigned pool slots #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7135513
Allow stale teardown records to leave reassigned pool slots intact
markpol Sep 27, 2026
2d30444
no-mistakes(document): Clarify reassigned slot teardown documentation
markpol Sep 27, 2026
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
23 changes: 14 additions & 9 deletions bin/fm-teardown.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,11 @@
# name a slot a DIFFERENT live task now holds. Cleanup kills every process under
# that path and hard-resets it before returning it, so releasing a slot that is
# not genuinely this task's destroys another worker's live work. Before the first
# 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.
# cleanup step, teardown reads the slot claim. If this task still owns the slot
# (or the claim is absent), it 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=. Two records naming this task's slot refuse its
# return; a positively reassigned slot is never returned.
# 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 @@ -2376,8 +2377,9 @@ 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 record scan, when this task still owns the slot, 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
Expand All @@ -2387,8 +2389,9 @@ require_exclusive_task_worktree_slot() {
#
# A claim naming another task does not refuse: it means the slot is no longer
# this task's, so the record's own cleanup proceeds and every slot step is
# skipped (see the script header for why refusing would strand the record and
# why skipping discards nothing). Returns TEARDOWN_SLOT_REASSIGNED_RC for that
# skipped, including the exclusivity scan that would reject the other task's
# legitimate record (see the script header for why refusing would strand the
# record and why skipping discards nothing). Returns TEARDOWN_SLOT_REASSIGNED_RC for that
# state so each caller gates its slot steps on one determination; the claimant
# stays in FM_TREEHOUSE_SLOT_OWNER_ID and FM_TREEHOUSE_SLOT_OWNER_HOME.
#
Expand Down Expand Up @@ -3308,8 +3311,10 @@ remove_secondmate_registry_entry() {
return "$rc"
}

require_exclusive_task_worktree_slot || exit 1
require_owned_task_worktree_slot || exit 1
if teardown_owns_worktree; then
require_exclusive_task_worktree_slot || exit 1
fi

validate_pr_poll_cleanup "$STATE" "$ID" || exit 1

Expand Down
3 changes: 2 additions & 1 deletion docs/architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,8 @@ After the forge accepts firstmate's merge request, the merge path persists the r
A later merged poll consumes only that matching persisted value; with no match it records the landing as external rather than consulting a live away-posture record that may have been archived or replaced.
[`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 pool worktree is only returned after teardown passes the slot-ownership proof: when the slot claim names this task or is absent, a contradictory task record or a supported live endpoint refuses without touching either task, and no discard authority relaxes that.
A claim naming another task proves reassignment, so teardown skips every slot operation and cleans up only the stale task record.
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.
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.
Before the worktree is returned, teardown concludes the task's own no-mistakes run when it is parked at a gate, including a run whose head the task copy cannot resolve - the shared runs-ledger continuation proof is the only recognition for that case, so cleanup never orphans a parked run the pipeline advanced past the submitted head.
Expand Down
75 changes: 75 additions & 0 deletions tests/fm-teardown-endpoint-safety.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,80 @@ test_reused_pool_slot_refuses_before_touching_the_other_task() {
pass "fm-teardown: a pool slot named by a second task record is never returned, killed, or reset"
}

# Two records can name one slot after reuse. Only the slot's positive claim
# distinguishes the stale record from its current owner; an absent or unsafe
# claim never licenses either record to release the contested slot.
test_shared_slot_claim_distinguishes_stale_from_current_owner() {
local dir id=stale-task other=current-task scenario target rc
for scenario in stale current absent unsafe; do
dir=$(make_case "shared-slot-$scenario")
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"
target=$id
case "$scenario" in
stale) claim_pool_slot "$dir" "$other" ;;
current) claim_pool_slot "$dir" "$other"; target=$other ;;
absent) ;;
unsafe) printf 'not-a-claim\n' > "$dir/pool/1/.fm-slot-owner" ;;
esac

set +e
run_case "$dir" "$target" > "$dir/stdout" 2> "$dir/stderr"
rc=$?
set -e
if [ "$scenario" = stale ]; then
[ "$rc" -eq 0 ] || fail "stale record could not finish its own cleanup: $(cat "$dir/stderr")"
assert_reassigned_slot_left_alone "$dir" "$id" "$other" "shared-slot stale record"
assert_present "$dir/home/state/$other.meta" "stale cleanup removed the current owner's record"
assert_present "$dir/worktree/sentinel" "stale cleanup changed the current owner's copy"
assert_contains "$(cat "$dir/runtime.log")" "fm-$id" \
"stale cleanup did not close its own endpoint"
! grep -F "kill-window" "$dir/runtime.log" | grep -Fq "fm-$other" \
|| fail "stale cleanup closed the current owner's endpoint: $(cat "$dir/runtime.log")"
else
[ "$rc" -ne 0 ] || fail "contested slot with $scenario claim was returned"
assert_present "$dir/home/state/$id.meta" "refusal erased the stale task record"
assert_present "$dir/home/state/$other.meta" "refusal erased the current task record"
assert_present "$dir/worktree/sentinel" "refusal changed the contested copy"
[ ! -s "$dir/runtime.log" ] \
|| fail "contested slot with $scenario claim reached the runtime: $(cat "$dir/runtime.log")"
if [ "$scenario" = unsafe ]; then
assert_contains "$(cat "$dir/stderr")" "$dir/pool/1/.fm-slot-owner" \
"unsafe claim refusal did not identify its file"
else
assert_contains "$(cat "$dir/stderr")" "$other" \
"contested slot refusal did not name the other record"
fi
fi
done

# Ordinary cleanup of a completed ship must skip the current owner's dirty
# slot even without --force; the two task records still name the same copy.
dir=$(make_case shared-slot-completed-ships)
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=ship"
fm_write_meta "$dir/home/state/$other.meta" \
"window=firstmate:fm-$other" "endpoint_task_id=$other" \
"worktree=$dir/worktree" "project=$dir/project" "kind=ship"
claim_pool_slot "$dir" "$other"
FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" \
FM_RUNTIME_LOG="$dir/runtime.log" PATH="$dir/fakebin:$PATH" \
"$TEARDOWN" "$id" > "$dir/stdout" 2> "$dir/stderr" \
|| fail "completed ship's stale record could not clean up without --force: $(cat "$dir/stderr")"
assert_reassigned_slot_left_alone "$dir" "$id" "$other" "completed ship without --force"
assert_present "$dir/home/state/$other.meta" "completed ship cleanup removed the current task record"
assert_present "$dir/worktree/sentinel" "completed ship cleanup changed the current owner's dirty copy"

pass "fm-teardown: a positive other-task claim cleans only the stale record; current, absent and unsafe claims cannot release a contested slot"
}

test_cross_home_pool_slot_collision_refuses() {
local dir id=stale-task other=secondmate-task second_home second_project rc
dir=$(make_case slot-reuse-cross-home)
Expand Down Expand Up @@ -1400,6 +1474,7 @@ test_orca_close_failure_refuses_even_under_force
test_already_gone_endpoint_still_completes_without_a_refusal
test_bare_relative_origin_shares_project_lock_with_clone
test_reused_pool_slot_refuses_before_touching_the_other_task
test_shared_slot_claim_distinguishes_stale_from_current_owner
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
Expand Down