diff --git a/bin/fm-backend.sh b/bin/fm-backend.sh index 3233921e071..6e06c590145 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -600,6 +600,10 @@ fm_backend_expected_label_of_selector() { # fm_backend_source() { # local name=$1 fm_backend_validate "$name" || return 1 + # A missing or unreadable adapter must come back as an ordinary failed + # return: on Bash 3.2 a `.` of a missing file can be a fatal shell error that + # skips the caller's own refusal; teardown's EXIT guard handles that abort. + [ -r "$FM_BACKEND_LIB_DIR/backends/$name.sh" ] || return 1 case "$name" in tmux) if [ -z "${_FM_BACKEND_TMUX_SOURCED:-}" ]; then diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index cbbad96b468..4dc73ed82f3 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -19,6 +19,13 @@ # home with a backlog but no compatible tasks-axi refuses before cleanup. # None of this loosens the landed-work gates below: the transition runs only on # the paths that already proceed to remove the record. +# An exit with status zero before this invocation confirms task-record removal +# is an aborted teardown: +# the EXIT guard returns failure and reports it on the original stderr, even if +# the triggering call discarded stderr, without continuing cleanup. +# Local and remote completion record that removal while holding the meta lock, +# so replacement metadata published afterward does not cause a false abort. +# tests/fm-gotmp.test.sh covers this guard and adapter-load failure retention. # The close - and only the close - is replaced by `tasks-axi reopen` with the # deliverable recorded while the backlog item is still an open captain call # (bin/fm-captain-hold.sh `open` owns that predicate), because the policy holds @@ -75,7 +82,11 @@ # is the approved discard path that prevalidates child removal targets, locks each # descendant home's task set before enumeration, and holds those locks through # child cleanup. Contention refuses the complete forced teardown before child -# mutation. Local and remote retirement serialize their destructive phase with +# mutation. Required child adapters are loaded recursively before child cleanup; +# unavailable adapters or fatal source exits preserve parent and child metadata, +# work, and scratch. After successful loading, endpoint kills keep their existing +# best-effort behavior and Herdr's exact-pane disappearance requirement. +# Local and remote retirement serialize their destructive phase with # that mate's backlog-handoff lock under the registry lock. Pending handoff wake # state is retired with the home, and local removal failure restores that state # before preserving the route for retry. Teardown then discards child work, kills @@ -262,6 +273,7 @@ CONTROL_LOCK="$STATE/.control-$ID.lock" CONTROL_LOCK_HELD=0 META_LOCK= META_LOCK_HELD=0 +TEARDOWN_RECORD_REMOVED=0 DESCENDANT_LOCK_PATHS=() DESCENDANT_TASK_STATES=() DESCENDANT_TASK_IDS=() @@ -297,8 +309,15 @@ teardown_release_locks() { CONTROL_LOCK_HELD=0 fi fm_lease_guard_release || true + # A fatal source error can leave $? at 0 on Bash 3.2, just like an explicit + # exit 0. Descriptor 3 survives best-effort callers redirecting their stderr. + if [ "$status" -eq 0 ] && [ "$TEARDOWN_RECORD_REMOVED" != 1 ]; then + echo "error: teardown of $ID aborted before its task record was removed; every durable record is retained" >&3 + exit 1 + fi return "$status" } +exec 3>&2 trap teardown_release_locks EXIT fm_lock_try_acquire "$CONTROL_LOCK" || { echo "error: another lifecycle action is already running for task $ID; nothing was changed" >&2 @@ -778,6 +797,7 @@ remote_secondmate_teardown() { mv -f -- "$tmp" "$SECONDMATE_REG" status_retire_presentation_task "$STATE" "$ID" || return 1 fm_backlog_atomic_transition remove "$STATE/$ID.meta" "task record" "$STATE" || return 1 + TEARDOWN_RECORD_REMOVED=1 rm -f -- "$STATE/$ID.turn-ended" printf 'teardown %s complete (remote %s:%s)\n' "$ID" "$remote_host" "$remote_home" return 0 @@ -2580,7 +2600,7 @@ $session $lock_path" return 1 } -preflight_firstmate_home_herdr_children() { # +preflight_firstmate_home_backend_children() { # local home=$1 sub_state child_meta child_id child_backend child_target child_kind child_home child_wt sub_state="$home/state" [ -d "$sub_state" ] || return 0 @@ -2592,6 +2612,9 @@ preflight_firstmate_home_herdr_children() { # child_target=$FM_BACKEND_VALIDATED_TARGET if [ "$child_backend" = herdr ]; then teardown_herdr_preflight_target "$child_target" "$child_id" || return 1 + elif ! fm_backend_source "$child_backend"; then + echo "REFUSED: $child_backend adapter is unavailable for child $child_id; forced teardown changed nothing" >&2 + return 1 fi child_kind=$(meta_value "$child_meta" kind) [ -n "$child_kind" ] || child_kind=ship @@ -2599,7 +2622,7 @@ preflight_firstmate_home_herdr_children() { # child_wt=$(meta_value "$child_meta" worktree) child_home=$(meta_value "$child_meta" home) [ -n "$child_home" ] || child_home=$child_wt - preflight_firstmate_home_herdr_children "$child_home" || return 1 + preflight_firstmate_home_backend_children "$child_home" || return 1 fi done } @@ -2732,7 +2755,7 @@ if [ "$KIND" = secondmate ]; then if [ "$BACKEND" = herdr ]; then teardown_herdr_preflight_target "$T" "$ID" || exit 1 fi - preflight_firstmate_home_herdr_children "$HOME_PATH" || exit 1 + preflight_firstmate_home_backend_children "$HOME_PATH" || exit 1 fi fi @@ -3135,6 +3158,7 @@ else exit 1 fi fi +TEARDOWN_RECORD_REMOVED=1 fm_lock_release "$META_LOCK" META_LOCK_HELD=0 if [ "$KIND" != scout ] && [ "$KIND" != secondmate ] && [ "$MODE" != local-only ]; then diff --git a/tests/fm-gotmp.test.sh b/tests/fm-gotmp.test.sh index d29c540e648..48f7a12fa1b 100755 --- a/tests/fm-gotmp.test.sh +++ b/tests/fm-gotmp.test.sh @@ -44,8 +44,11 @@ TMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/fm-gotmp-tests.XXXXXX") # state and helper scripts inside it. Stub the helper scripts fm-teardown calls so no # live tmux/treehouse/fleet state is touched. A nonexistent worktree path makes both # `if [ -d "$WT" ]` guards skip, so teardown runs straight to the cleanup + state rm. +# make_fake_root []: with one argument the meta carries no +# tasktmp= line at all (a pre-fix task); with two it records the given path. make_fake_root() { - local id=$1 tasktmp=$2 + local id=$1 tasktmp_line= + [ $# -lt 2 ] || tasktmp_line="tasktmp=$2" local fake="$TMP_ROOT/$id" mkdir -p "$fake/bin/backends" "$fake/state" "$fake/data" # Symlink the REAL teardown so the test exercises actual code, not a copy. @@ -104,6 +107,14 @@ SH exit 0 SH chmod +x "$fake/bin/fm-fleet-sync.sh" + # fm-remote-job-reap-orphans.sh: stub (teardown calls it with `|| true`). The + # real sweep signals orphaned remote job workers on this machine, so it must + # never be symlinked into a fixture. + cat > "$fake/bin/fm-remote-job-reap-orphans.sh" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fake/bin/fm-remote-job-reap-orphans.sh" # fm-tasks-axi-lib.sh: stub (teardown sources it). Report no backend so the # fused backlog close is skipped and the follow-up echo takes the plain-message # path; there is no tasks-axi and no backlog in this fixture. @@ -124,8 +135,8 @@ harness=claude kind=ship mode=no-mistakes yolo=off -tasktmp=$tasktmp META + [ -z "$tasktmp_line" ] || printf '%s\n' "$tasktmp_line" >> "$fake/state/$id.meta" printf '%s' "$fake" } @@ -148,75 +159,50 @@ test_teardown_removes_tasktmp_dir() { pass "fm-teardown removes the dir pointed to by tasktmp= in meta" } +test_teardown_preserves_replacement_record() { + local id=td-replacement-z8 fake task_tmp rc=0 + task_tmp="$TMP_ROOT/fm-$id" + mkdir -p "$task_tmp/gotmp" + fake=$(make_fake_root "$id" "$task_tmp") + cp "$fake/state/$id.meta" "$fake/replacement.meta" + printf 'spawn_gen=replacement\n' >> "$fake/replacement.meta" + cat > "$fake/bin/fm-fleet-sync.sh" <<'SH' +#!/usr/bin/env bash +set -eu +. "$FM_HOME/bin/fm-wake-lib.sh" +meta="$FM_HOME/state/$TEST_REPLACEMENT_ID.meta" +lock=$(fm_meta_lock_path "$meta") +fm_lock_try_acquire "$lock" +trap 'fm_lock_release "$lock"' EXIT +[ ! -e "$meta" ] && [ ! -L "$meta" ] +[ ! -e "$TEST_ORIGINAL_TASKTMP" ] +cp "$FM_HOME/replacement.meta" "$meta" +SH + FM_HOME="$fake" TEST_REPLACEMENT_ID="$id" TEST_ORIGINAL_TASKTMP="$task_tmp" \ + bash "$fake/bin/fm-teardown.sh" "$id" >"$fake/teardown.stdout" 2>"$fake/teardown.stderr" || rc=$? + cmp -s "$fake/replacement.meta" "$fake/state/$id.meta" \ + || fail "post-cleanup replacement metadata was not published or preserved" + [ ! -e "$task_tmp" ] || fail "original task scratch survived cleanup" + [ "$rc" -eq 0 ] || { + cat "$fake/teardown.stderr" >&2 + fail "teardown rejected replacement metadata after successful cleanup" + } + grep -q "teardown $id complete" "$fake/teardown.stdout" \ + || fail "original teardown did not report completion" + if grep -q 'aborted before its task record was removed' "$fake/teardown.stderr"; then + fail "completed teardown reported a false abort" + fi + pass "fm-teardown succeeds and preserves replacement metadata published after cleanup" +} + test_teardown_skips_gracefully_without_tasktmp() { # Backward compat: a meta from a pre-fix task has no tasktmp= line. Teardown must # not error and must not remove anything. local id=td-absent-z3 - local fake="$TMP_ROOT/$id-root" - mkdir -p "$fake/bin/backends" "$fake/state" "$fake/data" - ln -s "$TEARDOWN" "$fake/bin/fm-teardown.sh" - ln -s "$ROOT/bin/fm-backend.sh" "$fake/bin/fm-backend.sh" - cat > "$fake/bin/backends/tmux.sh" <<'SH' -fm_backend_tmux_kill() { return 0; } -SH - ln -s "$ROOT/bin/fm-tmux-lib.sh" "$fake/bin/fm-tmux-lib.sh" - ln -s "$ROOT/bin/fm-cursor-lib.sh" "$fake/bin/fm-cursor-lib.sh" - ln -s "$ROOT/bin/fm-composer-lib.sh" "$fake/bin/fm-composer-lib.sh" - ln -s "$ROOT/bin/fm-nm-run-lib.sh" "$fake/bin/fm-nm-run-lib.sh" - ln -s "$ROOT/bin/fm-lock-lib.sh" "$fake/bin/fm-lock-lib.sh" - # fm-lease-lib.sh: teardown sources it for the supervision lease guard. - ln -s "$ROOT/bin/fm-lease-lib.sh" "$fake/bin/fm-lease-lib.sh" - ln -s "$ROOT/bin/fm-control-lib.sh" "$fake/bin/fm-control-lib.sh" - ln -s "$ROOT/bin/fm-classify-lib.sh" "$fake/bin/fm-classify-lib.sh" - # fm-timeout-lib.sh: the shared hard bound fm-classify-lib.sh sources for the - # wedge detector's bounded worktree write probe. - ln -s "$ROOT/bin/fm-timeout-lib.sh" "$fake/bin/fm-timeout-lib.sh" - ln -s "$ROOT/bin/fm-wake-lib.sh" "$fake/bin/fm-wake-lib.sh" - # fm-gate-refuse-lib.sh: teardown sources it before any fleet mutation. - ln -s "$ROOT/bin/fm-gate-refuse-lib.sh" "$fake/bin/fm-gate-refuse-lib.sh" - # fm-pr-lib.sh: teardown uses its canonical task-ID validator for poll cleanup. - ln -s "$ROOT/bin/fm-pr-lib.sh" "$fake/bin/fm-pr-lib.sh" - # fm-public-followup-lib.sh (and the fm-x-lib.sh it sources): teardown sources - # it for the relay-activation gate on the promised-public-reply check. Neither - # does anything in this fixture, which has no .env, but both are real siblings - # teardown now requires. - ln -s "$ROOT/bin/fm-public-followup-lib.sh" "$fake/bin/fm-public-followup-lib.sh" - ln -s "$ROOT/bin/fm-x-lib.sh" "$fake/bin/fm-x-lib.sh" - ln -s "$ROOT/bin/fm-secondmate-registry-lib.sh" "$fake/bin/fm-secondmate-registry-lib.sh" - ln -s "$ROOT/bin/fm-secondmate-parent-lib.sh" "$fake/bin/fm-secondmate-parent-lib.sh" - ln -s "$ROOT/bin/fm-pending-reply-lib.sh" "$fake/bin/fm-pending-reply-lib.sh" - ln -s "$ROOT/bin/fm-marker-lib.sh" "$fake/bin/fm-marker-lib.sh" - ln -s "$ROOT/bin/fm-operational-input.sh" "$fake/bin/fm-operational-input.sh" - ln -s "$ROOT/bin/fm-inactive-reconcile.sh" "$fake/bin/fm-inactive-reconcile.sh" - ln -s "$ROOT/bin/fm-parent-channel-lib.sh" "$fake/bin/fm-parent-channel-lib.sh" - cat > "$fake/bin/fm-guard.sh" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$fake/bin/fm-guard.sh" - cat > "$fake/bin/fm-fleet-sync.sh" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$fake/bin/fm-fleet-sync.sh" - cat > "$fake/bin/fm-tasks-axi-lib.sh" <<'SH' -FM_TASKS_AXI_MIN=0.2.4 -fm_tasks_axi_backend() { printf 'markdown\n'; } -fm_tasks_axi_backend_available() { return 1; } -fm_tasks_axi_compatible() { return 1; } -fm_backlog_backend_manual() { return 1; } -SH - ln -s "$ROOT/bin/fm-backlog-transition-lib.sh" "$fake/bin/fm-backlog-transition-lib.sh" - # No tasktmp= line at all. - cat > "$fake/state/$id.meta" < "$FM_HOME/sibling-source-reached" +set -o posix +# shellcheck source=/dev/null +. "$FM_BACKEND_LIB_DIR/missing-sibling.sh" +fm_backend_tmux_kill() { return 0; } +SH + err="$TMP_ROOT/$id.stderr" + if FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>"$err"; then + fail "teardown exited 0 with a sourced sibling missing" + fi + [ -e "$fake/state/$id.meta" ] \ + || fail "teardown removed the task record after a fatal source failure" + [ -e "$task_tmp" ] \ + || fail "teardown removed the tasktmp dir after a fatal source failure" + [ -e "$fake/sibling-source-reached" ] || fail "teardown did not reach the missing sibling" + pass "fm-teardown exits non-zero and retains every record when a sourced sibling is missing" +} + +test_teardown_rejects_zero_status_abort() { + # Pin the EXIT contract independently of the shell version's source-error + # status: an adapter that exits 0 before cleanup is still an aborted teardown. + local id=td-zero-z6 fake err task_tmp + task_tmp="$TMP_ROOT/fm-$id" + mkdir -p "$task_tmp/gotmp" + fake=$(make_fake_root "$id" "$task_tmp") + printf 'exit 0\n' > "$fake/bin/backends/tmux.sh" + err="$TMP_ROOT/$id.stderr" + if FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>"$err"; then + fail "teardown accepted an exit 0 before cleanup" + fi + [ -e "$fake/state/$id.meta" ] || fail "zero-status abort removed the task record" + [ -d "$task_tmp/gotmp" ] || fail "zero-status abort removed task scratch" + grep -q "aborted before its task record was removed" "$err" \ + || fail "zero-status abort did not reach the original stderr" + pass "fm-teardown rejects a zero-status abort and reports it on original stderr" +} + +test_missing_adapter_returns_to_caller() { + local id=td-noadapter-z7 fake out + fake=$(make_fake_root "$id") + rm "$fake/bin/backends/tmux.sh" + out=$(FM_HOME="$fake" bash --posix -c ' + . "$1/bin/fm-backend.sh" + if fm_backend_source tmux; then exit 1; fi + printf "adapter refused\n" + ' _ "$fake" 2>&1) || fail "missing adapter aborted instead of returning failure" + [ "$out" = 'adapter refused' ] || fail "missing adapter did not reach caller refusal" + [ -e "$fake/state/$id.meta" ] || fail "adapter probe removed task metadata" + pass "a missing adapter returns failure so the caller can refuse safely" +} + +test_forced_parent_preflights_child_adapters() { + local mode id fake home task_tmp child_wt child_tmp err log path + for mode in missing-sibling missing-adapter zero-exit kill-failure success; do + id="td-child-$mode" + task_tmp="$TMP_ROOT/fm-$id" + home="$TMP_ROOT/home-$id" + child_wt="$TMP_ROOT/work-$id" + child_tmp="$home/tasktmp" + mkdir -p "$task_tmp/gotmp" "$home/state" "$child_tmp/gotmp" "$child_wt" + fake=$(make_fake_root "$id" "$task_tmp") + printf '%s\n' "$id" > "$home/.fm-secondmate-home" + printf 'parent work\n' > "$home/work-note" + printf 'parent scratch\n' > "$task_tmp/gotmp/artifact" + printf 'child scratch\n' > "$child_tmp/gotmp/artifact" + git -C "$child_wt" init -q || fail "child fixture git init failed" + git -C "$child_wt" -c user.name=Test -c user.email=test@example.invalid \ + -c commit.gpgsign=false commit -q --allow-empty -m fixture \ + || fail "child fixture commit failed" + printf 'child work\n' > "$child_wt/work-note" + cat > "$fake/state/$id.meta" < "$home/state/child-z.meta" < "$fake/bin/backends/zellij.sh" + if [ "$mode" = missing-sibling ]; then + printf 'set -o posix\n' >> "$fake/bin/backends/zellij.sh" + fi + cat >> "$fake/bin/backends/zellij.sh" <<'SH' +. "$FM_BACKEND_LIB_DIR/fm-backend-hometag-lib.sh" +fm_backend_zellij_kill() { + printf '%s\n' "$FM_HOME" "$FM_ROOT" "$@" >> "$TEST_CHILD_KILL_LOG" + return "$TEST_CHILD_KILL_RC" +} +SH + case "$mode" in + missing-sibling) ;; + missing-adapter) rm "$fake/bin/backends/zellij.sh" ;; + zero-exit) printf 'exit 0\n' > "$fake/bin/fm-backend-hometag-lib.sh" ;; + *) : > "$fake/bin/fm-backend-hometag-lib.sh" ;; + esac + printf '#!/usr/bin/env bash\nexit 1\n' > "$fake/bin/treehouse" + chmod +x "$fake/bin/treehouse" + err="$fake/teardown.stderr" + log="$fake/child-kill.log" + local kill_rc=0 rc=0 + [ "$mode" != kill-failure ] || kill_rc=1 + FM_HOME="$fake" PATH="$fake/bin:$PATH" TEST_CHILD_KILL_LOG="$log" TEST_CHILD_KILL_RC="$kill_rc" \ + bash "$fake/bin/fm-teardown.sh" "$id" --force > "$fake/teardown.stdout" 2>"$err" || rc=$? + case "$mode" in + missing-sibling|missing-adapter|zero-exit) + [ "$rc" -ne 0 ] || fail "$mode: forced parent teardown accepted child adapter failure" + cmp -s "$fake/state/$id.meta" "$fake/parent.meta.before" || fail "$mode: parent record changed" + cmp -s "$home/state/child-z.meta" "$fake/child.meta.before" || fail "$mode: child record changed" + [ "$(cat "$home/work-note")" = 'parent work' ] || fail "$mode: parent work changed" + [ "$(cat "$child_wt/work-note")" = 'child work' ] || fail "$mode: child work changed" + [ "$(cat "$task_tmp/gotmp/artifact")" = 'parent scratch' ] || fail "$mode: parent scratch changed" + [ "$(cat "$child_tmp/gotmp/artifact")" = 'child scratch' ] || fail "$mode: child scratch changed" + [ ! -e "$log" ] || fail "$mode: child kill ran before preflight completed" + case "$mode" in + missing-sibling) grep -q 'fm-backend-hometag-lib.sh' "$err" || fail "missing sibling diagnostic absent: $(cat "$err")" ;; + missing-adapter) grep -q 'REFUSED: zellij adapter is unavailable for child child-z' "$err" || fail "missing adapter refusal absent: $(cat "$err")" ;; + zero-exit) grep -q 'aborted before its task record was removed' "$err" || fail "zero-status abort diagnostic absent: $(cat "$err")" ;; + esac + ;; + *) + [ "$rc" -eq 0 ] || { cat "$err" >&2; fail "$mode: forced parent teardown failed"; } + for path in "$fake/state/$id.meta" "$home" "$child_wt" "$task_tmp"; do + [ ! -e "$path" ] || fail "$mode: cleanup retained $path" + done + printf '%s\n' "$home" "$home" fakeses:1 1 fm-child-z > "$fake/expected-kill.log" + cmp -s "$log" "$fake/expected-kill.log" || fail "$mode: child kill lost its owning home or endpoint" + ;; + esac + pass "forced parent child-adapter preflight: $mode" + done +} + test_teardown_removes_tasktmp_dir +test_teardown_preserves_replacement_record test_teardown_skips_gracefully_without_tasktmp test_teardown_skips_gracefully_when_dir_missing +test_teardown_fails_loudly_when_a_sourced_sibling_is_missing +test_teardown_rejects_zero_status_abort +test_missing_adapter_returns_to_caller +test_forced_parent_preflights_child_adapters