diff --git a/tests/fm-reconcile-worktree-pointers.test.sh b/tests/fm-reconcile-worktree-pointers.test.sh new file mode 100755 index 00000000000..f1b8fa80dbf --- /dev/null +++ b/tests/fm-reconcile-worktree-pointers.test.sh @@ -0,0 +1,312 @@ +#!/usr/bin/env bash +# Tests for bin/fm-reconcile-worktree-pointers.sh and the binding-independent +# ownership proof in bin/fm-worktree-owner-lib.sh. +# +# THE GAP THESE COVER. The current-owner check in bin/fm-teardown.sh reads an +# ownership binding out of the copy, so it protects every copy assigned after +# bindings existed and deliberately does nothing for the collisions that already +# exist - a record with no declared binding, over a copy carrying none. Those are +# exactly the collisions a home accumulates, because the lanes holding stale +# pointers are preserved lanes that are never torn down and therefore never +# repaired. The second proof reads the branch the copy actually has checked out +# and cross-confirms it against that claimant's own record. +# +# Matrix: +# (r1) unbound copy on another recorded task's branch -> STALE, pointer retired +# (r2) unbound copy still on this lane's own branch -> quiet (nothing to repair) +# (r3) branch names a task this home does not record -> UNRESOLVED, untouched +# (r11) branch's claimant records a different copy -> UNRESOLVED, untouched +# (r4) readable binding disagrees with the branch -> binding wins, quiet +# (r5) default run reports without changing anything -> dry-run by default +# (r6) re-run over an already-retired pointer -> counted, not rewritten +# (r7) copy on a detached HEAD -> UNRESOLVED, untouched +# (r8) kind=secondmate home -> skipped entirely +# (r9) any repair -> never appends to a status log +# (r10) --help -> pure read, real usage +set -u + +# shellcheck source=tests/lib.sh disable=SC1091 +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# shellcheck source=/dev/null +. "$ROOT/bin/fm-worktree-binding-lib.sh" +fm_git_identity fmtest fmtest@example.invalid + +RECONCILE="$ROOT/bin/fm-reconcile-worktree-pointers.sh" +TMP_ROOT=$(fm_test_tmproot fm-reconcile-pointer-tests) + +# Build a home with a project repo and one pooled copy. The copy starts on +# fm/lane-a, which is lane-a's own branch. Echoes the case dir. +make_home() { + local name=$1 case_dir + case_dir="$TMP_ROOT/$name" + mkdir -p "$case_dir/state" + fm_git_worktree "$case_dir/project" "$case_dir/wt" fm/lane-a + printf '%s\n' "$case_dir" +} + +# Record a task as holding the shared copy. Args: case_dir task_id [extra kv...] +record_lane() { + local case_dir=$1 id=$2 + shift 2 + fm_write_meta "$case_dir/state/$id.meta" \ + "window=firstmate:fm-$id" \ + "endpoint_task_id=$id" \ + "worktree=$case_dir/wt" \ + "project=$case_dir/project" \ + "kind=ship" \ + "mode=no-mistakes" \ + "$@" +} + +# Hand the copy to another task the way a fresh spawn into a recycled pool slot +# does, but WITHOUT writing an ownership binding - the pre-binding shape that the +# existing check cannot see. Args: case_dir branch +hand_copy_to_branch() { + local case_dir=$1 branch=$2 + git -C "$case_dir/wt" checkout -q -b "$branch" +} + +run_reconcile() { + local case_dir=$1; shift + FM_ROOT_OVERRIDE="$ROOT" FM_STATE_OVERRIDE="$case_dir/state" "$RECONCILE" "$@" +} + +# (r1) The reproduction: lane-a's record still names a slot the pool has since +# handed to lane-b. Neither side carries a binding, so only the branch the copy +# actually has checked out can settle it. +test_unbound_recycled_slot_is_retired() { + local case_dir out + case_dir=$(make_home unbound-recycled) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + + out=$(run_reconcile "$case_dir" --apply) + + assert_contains "$out" "RETIRED: lane-a" "unbound: the stale pointer is retired" + assert_contains "$out" "owned by lane-b" "unbound: the report names the owning task" + assert_contains "$out" "via branch" "unbound: the report names the proof it used" + assert_grep "worktree_retired=lane-b" "$case_dir/state/lane-a.meta" \ + "unbound: the durable retirement names the proven owner" + assert_grep "worktree=$case_dir/wt" "$case_dir/state/lane-a.meta" \ + "unbound: the stale pointer is kept as history, not deleted" + assert_no_grep "worktree_retired" "$case_dir/state/lane-b.meta" \ + "unbound: the live owner's own record is left alone" + [ "$(git -C "$case_dir/wt" rev-parse --abbrev-ref HEAD)" = fm/lane-b ] \ + || fail "unbound: the copy itself must be untouched" + pass "reconcile retires a stale pointer over a copy that carries no binding" +} + +# (r2) The quiet case that matters most: an old unbound record whose copy is +# still its own. Repairing this would retire a live pointer. +test_own_copy_is_not_reassigned() { + local case_dir out + case_dir=$(make_home own-copy) + record_lane "$case_dir" lane-a + + out=$(run_reconcile "$case_dir" --apply) + + assert_not_contains "$out" "lane-a" "own-copy: a lane holding its own copy is not reported" + assert_contains "$out" "0 stale pointer(s)" "own-copy: nothing is stale" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "own-copy: a live pointer must never be retired" + pass "reconcile leaves a lane that still holds its own copy alone" +} + +# (r3) One-sided evidence. The copy carries some fm/* branch, but this home has +# no record of that task holding it, so nothing is proven either way. +test_branch_without_a_matching_record_is_unresolved() { + local case_dir out + case_dir=$(make_home ghost-branch) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/ghost + + out=$(run_reconcile "$case_dir" --apply) + + assert_contains "$out" "UNRESOLVED: lane-a" "ghost: an unconfirmed branch is not a verdict" + assert_contains "$out" "no record of task ghost" "ghost: the report says what was missing" + assert_not_contains "$out" "RETIRED" "ghost: nothing is retired on one-sided evidence" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "ghost: the record is left untouched" + pass "reconcile refuses to call a reassignment from the branch name alone" +} + +# (r4) A readable binding is authoritative. A worker that checks out some other +# task's branch inside its OWN copy must not look reassigned. +# (r11) The other half of the cross-confirmation: the branch names a task this +# home DOES record, but that task's record points somewhere else. A branch left +# behind by an earlier occupant looks exactly like this, and calling it the +# current owner would retire a live pointer on stale evidence. +test_branch_whose_claimant_records_another_copy_is_unresolved() { + local case_dir out + case_dir=$(make_home elsewhere) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + mkdir -p "$case_dir/other-wt" + fm_write_meta "$case_dir/state/lane-b.meta" \ + "window=firstmate:fm-lane-b" \ + "endpoint_task_id=lane-b" \ + "worktree=$case_dir/other-wt" \ + "project=$case_dir/project" \ + "kind=ship" \ + "mode=no-mistakes" + + out=$(run_reconcile "$case_dir" --apply) + + assert_contains "$out" "UNRESOLVED: lane-a" \ + "elsewhere: a claimant that records another copy proves nothing" + assert_not_contains "$out" "RETIRED: lane-a" \ + "elsewhere: lane-a's live pointer must survive" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "elsewhere: the record is left untouched" + pass "reconcile requires the claimant's own record to name the same copy" +} + +test_binding_outranks_the_checked_out_branch() { + local case_dir out + case_dir=$(make_home binding-wins) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + fm_worktree_binding_write "$case_dir/wt" lane-a \ + || fail "binding-wins: could not bind the copy to lane-a" + + out=$(run_reconcile "$case_dir" --apply) + + assert_not_contains "$out" "RETIRED: lane-a" \ + "binding-wins: the copy's own binding settles ownership" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "binding-wins: lane-a keeps its live pointer" + assert_contains "$out" "RETIRED: lane-b" \ + "binding-wins: the other claimant is the stale one here" + pass "reconcile trusts the copy's ownership binding over its checked-out branch" +} + +# (r5) Reporting must be the default; a repair tool that mutates on a bare +# invocation cannot be run to find out what it would do. +test_default_run_changes_nothing() { + local case_dir out + case_dir=$(make_home dry-run) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + + out=$(run_reconcile "$case_dir") + + assert_contains "$out" "STALE: lane-a" "dry-run: the stale pointer is reported" + assert_contains "$out" "re-run with --apply" "dry-run: the report names the repair" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "dry-run: a default run must not write" + pass "reconcile reports without changing anything unless --apply is given" +} + +# (r6) Re-runnable, because this accumulation is structural and will need +# draining again rather than being caught in one pass. +test_rerun_is_idempotent() { + local case_dir out before + case_dir=$(make_home rerun) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + run_reconcile "$case_dir" --apply >/dev/null + before=$(cat "$case_dir/state/lane-a.meta") + + out=$(run_reconcile "$case_dir" --apply) + + assert_contains "$out" "1 already retired" "rerun: an already-retired pointer is counted" + assert_not_contains "$out" "RETIRED: lane-a" "rerun: it is not retired twice" + [ "$(cat "$case_dir/state/lane-a.meta")" = "$before" ] \ + || fail "rerun: the record must not be rewritten on a second pass" + pass "reconcile is idempotent across repeated runs" +} + +# (r7) A pooled slot sitting on a detached HEAD says nothing about ownership. +test_detached_head_is_unresolved() { + local case_dir out + case_dir=$(make_home detached) + record_lane "$case_dir" lane-a + git -C "$case_dir/wt" checkout -q --detach + + out=$(run_reconcile "$case_dir" --apply) + + assert_contains "$out" "UNRESOLVED: lane-a" "detached: ownership is unprovable" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "detached: nothing is retired" + pass "reconcile leaves a copy on a detached HEAD alone" +} + +# (r8) A secondmate home is a persistent home, not a pooled slot; it is never +# recycled underneath its record and its own removal validation owns it. +test_secondmate_home_is_skipped() { + local case_dir out + case_dir=$(make_home secondmate) + fm_write_meta "$case_dir/state/lane-a.meta" \ + "window=firstmate:fm-lane-a" \ + "endpoint_task_id=lane-a" \ + "worktree=$case_dir/wt" \ + "project=$case_dir/project" \ + "kind=secondmate" \ + "mode=no-mistakes" + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + + out=$(run_reconcile "$case_dir" --apply) + + assert_not_contains "$out" "lane-a" "secondmate: a persistent home is out of scope" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "secondmate: its record is untouched" + pass "reconcile skips secondmate homes" +} + +# (r9) Every stale pointer belongs to a deliberately paused lane, and ANY status +# append re-declares that lane's current state and un-throttles its pause. A +# repair that announced itself in the status log would wake every lane it fixed. +test_repair_never_touches_the_status_log() { + local case_dir + case_dir=$(make_home quiet-status) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + printf 'paused [key=preserved]: do not clean up\n' > "$case_dir/state/lane-a.status" + + run_reconcile "$case_dir" --apply >/dev/null + + [ "$(cat "$case_dir/state/lane-a.status")" = 'paused [key=preserved]: do not clean up' ] \ + || fail "quiet-status: the repair must not append to a paused lane's status log" + assert_absent "$case_dir/state/lane-b.status" \ + "quiet-status: the repair must not create a status log for the live lane" + pass "reconcile repairs records without waking any paused lane" +} + +# (r10) --help must stay a pure read, and must keep describing the real script. +test_help_is_a_pure_read() { + local case_dir out rc + case_dir=$(make_home help) + record_lane "$case_dir" lane-a + hand_copy_to_branch "$case_dir" fm/lane-b + record_lane "$case_dir" lane-b + + set +e + out=$(run_reconcile "$case_dir" --help) + rc=$? + set -e + + expect_code 0 "$rc" "help: --help exits cleanly" + assert_contains "$out" "Usage: fm-reconcile-worktree-pointers.sh [--apply]" \ + "help: --help states how to run the repair" + assert_no_grep "worktree_retired" "$case_dir/state/lane-a.meta" \ + "help: --help must never change a record" + pass "--help prints the script's own documentation and changes nothing" +} + +test_unbound_recycled_slot_is_retired +test_own_copy_is_not_reassigned +test_branch_without_a_matching_record_is_unresolved +test_branch_whose_claimant_records_another_copy_is_unresolved +test_binding_outranks_the_checked_out_branch +test_default_run_changes_nothing +test_rerun_is_idempotent +test_detached_head_is_unresolved +test_secondmate_home_is_skipped +test_repair_never_touches_the_status_log +test_help_is_a_pure_read diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index cfa4f81bea1..cfb0c1f5857 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -52,6 +52,9 @@ # (z6) binding declared in meta but missing from the copy -> REFUSE, --force overrides # (z7) plain re-run over an already-retired pointer -> ALLOW (durably idempotent) # (z8) --force with --forget-worktree on a reassigned copy -> ALLOW (other copy untouched) +# (z9) reassigned copy with NO binding anywhere -> REFUSE, names the owner +# (z10) same collision with --forget-worktree -> pointer retired, copy untouched +# (z11) unbound copy on a branch this home does not record -> no verdict (pre-existing behaviour) # # Also covers backlog teardown-lock-race: a git index.lock left in the worktree by a # killed crew process (bin/fm-teardown.sh's teardown_treehouse_return). @@ -861,6 +864,111 @@ test_missing_binding_refuses_until_forced() { pass "a declared-but-missing ownership record refuses, and --force overrides it" } +# --- pre-binding collisions ------------------------------------------------- +# +# The collisions a home has ALREADY accumulated look different from the ones +# above: neither the stale record nor the recycled copy carries an ownership +# binding, because both predate it. Ownership is then read from the branch the +# copy actually has checked out, cross-confirmed against that claimant's own +# record (bin/fm-worktree-owner-lib.sh). + +# Hand the copy to another task WITHOUT writing an ownership binding, the way a +# slot recycled before bindings existed looks. Args: case_dir owner_task_id +hand_unbound_worktree_to_other_task() { + local case_dir=$1 owner=$2 + git -C "$case_dir/wt" checkout -q -b "fm/$owner" + fm_write_meta "$case_dir/state/$owner.meta" \ + "window=firstmate:fm-$owner" \ + "endpoint_task_id=$owner" \ + "worktree=$case_dir/wt" \ + "project=$case_dir/project" \ + "kind=ship" \ + "mode=no-mistakes" +} + +# (z9) The uncovered case: no binding anywhere, so the pre-fix code let teardown +# walk straight into a live task's copy. +test_unbound_recycled_slot_refuses_and_names_the_live_owner() { + local case_dir rc + case_dir=$(make_case unbound-recycled-refuses) + write_meta "$case_dir" no-mistakes ship + log_treehouse_calls "$case_dir" + hand_unbound_worktree_to_other_task "$case_dir" live-lane + wt_commit "$case_dir" "live lane work" + push_current_branch_to_fork "$case_dir" fm/live-lane + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unbound-recycled: teardown must refuse a copy another task owns" + assert_contains "$(cat "$case_dir/stderr")" "live-lane" \ + "unbound-recycled: the refusal names the task that owns the copy" + assert_contains "$(cat "$case_dir/stderr")" "fm/live-lane" \ + "unbound-recycled: the refusal names the branch actually checked out" + assert_contains "$(cat "$case_dir/stderr")" "fm-reconcile-worktree-pointers.sh --apply" \ + "unbound-recycled: the refusal names the repair that leaves the lane intact" + git -C "$case_dir/project" rev-parse --verify -q fm/live-lane >/dev/null \ + || fail "unbound-recycled: the live lane's branch was deleted" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "unbound-recycled: the live lane's copy was returned to the pool" + pass "a recycled slot with no ownership binding still refuses teardown" +} + +# (z10) --forget-worktree becomes available on that same proof, so an unbound +# collision is retirable rather than permanently stuck. +test_unbound_recycled_slot_is_retirable() { + local case_dir rc + case_dir=$(make_case unbound-recycled-forget) + write_meta "$case_dir" no-mistakes ship + log_treehouse_calls "$case_dir" + hand_unbound_worktree_to_other_task "$case_dir" live-lane + wt_commit "$case_dir" "live lane work" + + set +e + run_teardown "$case_dir" --forget-worktree > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "unbound-forget: retiring an unbound stale pointer should complete"$'\n'"$(cat "$case_dir/stderr")" + [ ! -f "$case_dir/state/task-x1.meta" ] \ + || fail "unbound-forget: the stale task's own records were not cleaned up" + [ -d "$case_dir/wt" ] || fail "unbound-forget: the live lane's copy was removed" + [ "$(git -C "$case_dir/wt" rev-parse --abbrev-ref HEAD)" = fm/live-lane ] \ + || fail "unbound-forget: the live lane's copy was reset off its branch" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "unbound-forget: the live lane's copy was returned to the pool" + pass "an unbound recycled slot can be retired with --forget-worktree" +} + +# (z11) Quiet: a copy carrying some fm/* branch this home has no record of proves +# nothing. Calling that a reassignment would refuse legitimate cleanups and hand +# --forget-worktree a bypass, so the pre-existing behaviour must stand. +test_unconfirmed_branch_is_not_a_reassignment() { + local case_dir rc + case_dir=$(make_case unbound-unconfirmed) + write_meta "$case_dir" local-only ship + log_treehouse_calls "$case_dir" + git -C "$case_dir/wt" checkout -q -b fm/ghost-lane + wt_commit "$case_dir" "this task's own work" + add_fork_with_pushed_branch "$case_dir" + + set +e + run_teardown "$case_dir" --forget-worktree > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unbound-unconfirmed: --forget-worktree must refuse without a proven reassignment" + assert_contains "$(cat "$case_dir/stderr")" "proven reassignment" \ + "unbound-unconfirmed: the refusal explains why the flag does not apply" + assert_not_contains "$(cat "$case_dir/stderr")" "ghost-lane owns it now" \ + "unbound-unconfirmed: an unrecorded branch is never called the owner" + [ -f "$case_dir/state/task-x1.meta" ] \ + || fail "unbound-unconfirmed: records were cleaned up anyway" + pass "an unrecorded branch in an unbound copy is not treated as a reassignment" +} + test_local_only_fork_remote_allows() { local case_dir rc case_dir=$(make_case fork-allow) @@ -2993,6 +3101,9 @@ test_recycled_slot_refuses_even_under_force test_forget_worktree_retires_the_stale_pointer_and_spares_the_copy test_force_and_forget_worktree_together_complete test_forget_worktree_refuses_without_a_proven_reassignment +test_unbound_recycled_slot_refuses_and_names_the_live_owner +test_unbound_recycled_slot_is_retirable +test_unconfirmed_branch_is_not_a_reassignment test_retired_pointer_is_honoured_on_a_plain_rerun test_own_binding_tears_down_normally test_missing_binding_refuses_until_forced