fix(tests): root-cause wake/watcher suite flakiness

.github/workflows/ci.yml

@@ -390,6 +390,18 @@ jobs:
             exit 1
           }
 
+          # The wake lock's reclaim rests on naming the running frame, and this
+          # is the only shell here with no BASHPID to name it with, so the
+          # portable Linux lanes cannot exercise that half of the contract.
+          lock_output=$(FM_TEST_ONLY=test_self_held_lock_reclaims_instead_of_deadlocking \
+            /bin/bash tests/fm-wake-queue.test.sh)
+          printf '%s\n' "$lock_output"
+          lock_count=$(printf '%s\n' "$lock_output" | grep -c '^ok - ')
+          [ "$lock_count" -eq 1 ] || {
+            echo "::error::expected 1 wake-lock frame-identity regression, got $lock_count"
+            exit 1
+          }
+
           command -v npm >/dev/null || { echo "::error::npm is required to install tasks-axi"; exit 1; }
           npm install -g tasks-axi@0.2.5 >/dev/null
           PATH="$(npm prefix -g)/bin:$PATH"

bin/fm-wake-lib.sh

@@ -36,6 +36,29 @@ fm_pid_alive() {
   kill -0 "$pid" 2>/dev/null
 }
 
+# fm_pid_is_this_frame <recorded-pid>
+# True when a pid recorded in a lock names the exact frame asking. Lock reclaim
+# turns on this and nothing else, and the case it has to get right is a subshell
+# looking at a lock its parent still holds: the parent is alive, so reading that
+# lock as "mine, abandoned" would hand the subshell a live hold.
+#
+# BASHPID names the frame directly and must be read inline - a command
+# substitution would answer for its own subshell, and $(this function) would
+# too, so CALL it. Stock macOS bash 3.2 has no BASHPID at all, and its $$ is the
+# same value in every subshell, so there $$ names a frame only where there is no
+# subshell to confuse it with: the main shell, the one place BASH_SUBSHELL is 0.
+# Deeper frames on that shell get no reclaim and keep waiting, which is what the
+# lock did before reclaim existed.
+fm_pid_is_this_frame() {
+  local recorded=$1
+  [ -n "$recorded" ] || return 1
+  if [ -n "${BASHPID:-}" ]; then
+    [ "$recorded" = "$BASHPID" ]
+    return
+  fi
+  [ "${BASH_SUBSHELL:-0}" = 0 ] && [ "$recorded" = "$$" ]
+}
+
 fm_pid_identity() {
   local pid=$1 out proc_root stat_line starttime cmdline_hex identity_key
   local -a stat_fields
@@ -801,11 +824,10 @@ fm_lock_try_acquire() {
     return 0
   fi
 
-  # Compare against ${BASHPID:-$$} inline, never via a command substitution:
-  # $() forks a subshell whose BASHPID is not this frame's pid.
+  # fm_pid_is_this_frame owns what counts as "this frame" on each shell.
   pid=$(cat "$lockdir/pid" 2>/dev/null || true)
-  if [ -n "$pid" ] && [ "$pid" = "${BASHPID:-$$}" ]; then
-    # The recorded holder is THIS very process. Single-threaded bash can only
+  if fm_pid_is_this_frame "$pid"; then
+    # The recorded holder is THIS very frame. Single-threaded bash can only
     # observe that when an interrupting trap abandoned the frame that held the
     # lock mid-critical-section (e.g. TERM inside a recovery-marker section,
     # with the EXIT path then re-acquiring the same lock), and every

tests/fm-inactive-reconcile.test.sh

@@ -119,7 +119,8 @@ prime_seen() { # <state> <status>
   ' _ "$ROOT/bin/fm-wake-lib.sh" "$1" "$2"
 }
 
-reap() { kill "$1" 2>/dev/null || true; wait "$1" 2>/dev/null || true; }
+# reap comes from tests/lib.sh, which owns the bounded stop-and-collect these
+# watcher fixtures need; a local `kill; wait` here could hang the whole suite.
 
 # The main retains a terminal presentation receipt until the corresponding wake
 # is handled and acknowledged.

tests/fm-wake-queue.test.sh

@@ -68,7 +68,7 @@ test_signal_catchup_without_running_watcher() {
   # tested.
   printf 'blocked: first\n' > "$status_file"
   PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" &
-  wait_for_exit "$!" 40 || fail "watcher did not exit for first signal"
+  wait_for_exit "$!" || fail "watcher did not exit for first signal"
   grep -F "signal: $status_file" "$out" >/dev/null || fail "watcher did not print first signal"
   FM_STATE_OVERRIDE="$state" "$DRAIN" > "$drain_out" 2> "$drain_err" || fail "drain after first signal failed"
   grep "$(printf '\tsignal\t')" "$drain_out" | grep -F "$status_file" >/dev/null || fail "first signal was not queued"
@@ -80,7 +80,7 @@ test_signal_catchup_without_running_watcher() {
   printf 'done: second\n' >> "$status_file"
   : > "$out"
   PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" &
-  wait_for_exit "$!" 40 || fail "watcher did not exit for second signal"
+  wait_for_exit "$!" || fail "watcher did not exit for second signal"
   grep -F "signal: $status_file" "$out" >/dev/null || fail "signal written with no watcher was not caught"
   pass "signal written while no watcher runs is caught on next run"
 }
@@ -107,7 +107,7 @@ test_stale_enqueue_before_suppressor() {
   printf '%s' "$pane_hash" > "$state/.hash-$key"
   printf '1\n' > "$state/.count-$key"
   PATH="$fakebin:$PATH" FM_FAKE_TMUX_WINDOW="$window" FM_FAKE_TMUX_CAPTURE="$capture_file" FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" &
-  wait_for_exit "$!" 40 || fail "watcher did not exit for stale pane"
+  wait_for_exit "$!" || fail "watcher did not exit for stale pane"
   grep -Fx "stale: $window" "$out" >/dev/null || fail "watcher did not print stale wake"
   FM_STATE_OVERRIDE="$state" "$DRAIN" > "$drain_out" || fail "drain after stale wake failed"
   grep "$(printf '\tstale\t')" "$drain_out" | grep -F "$window" >/dev/null || fail "stale wake was not queued"
@@ -144,7 +144,7 @@ test_not_working_stale_enqueue_before_suppressor() {
   PATH="$fakebin:$PATH" FM_FAKE_TMUX_WINDOW="$window" FM_FAKE_TMUX_CAPTURE="$capture_file" \
     FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" \
     FM_STALE_ESCALATE_SECS=999 FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" &
-  wait_for_exit "$!" 40 || fail "watcher did not surface a not-provably-working stale"
+  wait_for_exit "$!" || fail "watcher did not surface a not-provably-working stale"
   grep -Fx "stale: $window" "$out" >/dev/null || fail "watcher did not print the immediate stale wake"
   FM_STATE_OVERRIDE="$state" "$DRAIN" > "$drain_out" || fail "drain after the immediate stale wake failed"
   grep "$(printf '\tstale\t')" "$drain_out" | grep -F "$window" >/dev/null || fail "immediate stale wake was not queued"
@@ -169,7 +169,7 @@ SH
   FM_STATE_OVERRIDE="$state" "$ROOT/bin/fm-check-register.sh" task >/dev/null \
     || fail "could not register queue custom check"
   PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=0 FM_HEARTBEAT=999999 "$WATCH" > "$out" &
-  wait_for_exit "$!" 40 || fail "watcher did not exit for check output"
+  wait_for_exit "$!" || fail "watcher did not exit for check output"
   grep -F "check: $check_file: merged: https://example.test/pr/1" "$out" >/dev/null || fail "watcher did not print check wake"
   FM_STATE_OVERRIDE="$state" "$DRAIN" > "$drain_out" || fail "drain after check wake failed"
   grep "$(printf '\tcheck\t')" "$drain_out" | grep -F "$check_file" | grep -F 'merged: https://example.test/pr/1' >/dev/null || fail "check wake was not queued"
@@ -1115,9 +1115,16 @@ test_self_announced_append_guards() {
 # A trap that fires inside a lock's critical section abandons the holding
 # frame, and the exit path then re-acquires the same lock (a TERM inside a
 # recovery-marker section is the reproduced case: the watcher's reap wedged
-# forever spinning against its own pid). The same-process re-acquire must
-# reclaim the abandoned hold, while a SUBSHELL still waits on its parent's
-# live hold exactly as before.
+# forever spinning against its own pid). The same-frame re-acquire must reclaim
+# the abandoned hold, while a SUBSHELL still waits on its parent's live hold
+# exactly as before.
+#
+# Both halves turn on naming the running frame, and a shell whose $$ is shared
+# with every subshell can only do that where there is no subshell to confuse it
+# with (fm_pid_is_this_frame owns why). Stock macOS bash 3.2 is such a shell and
+# read the subshell below as its own parent before that distinction existed, so
+# both halves are asserted on whatever shell runs this file rather than on an
+# assumed one.
 test_self_held_lock_reclaims_instead_of_deadlocking() {
   local dir state rc
   dir=$(make_case self-held-lock)
@@ -1131,7 +1138,8 @@ test_self_held_lock_reclaims_instead_of_deadlocking() {
     fm_lock_release "$lock"
     [ ! -e "$lock" ] && [ ! -L "$lock" ] || exit 12
   ' _ "$ROOT/bin/fm-wake-lib.sh" "$state" || rc=$?
-  [ "$rc" -eq 0 ] || fail "self-held lock was not reclaimed cleanly (rc=$rc)"
+  [ "$rc" -eq 0 ] \
+    || fail "self-held lock was not reclaimed cleanly (rc=$rc, bash $BASH_VERSION)"
   rc=0
   FM_STATE_OVERRIDE="$state" bash -c '
     . "$1"
@@ -1140,8 +1148,8 @@ test_self_held_lock_reclaims_instead_of_deadlocking() {
     ( fm_lock_try_acquire "$lock" && exit 13; exit 0 ) || exit 13
     fm_lock_release "$lock"
   ' _ "$ROOT/bin/fm-wake-lib.sh" "$state" || rc=$?
-  [ "$rc" -eq 0 ] || fail "a subshell reclaimed its parent's live hold (rc=$rc)"
-  pass "an abandoned same-process lock hold is reclaimed; a parent's live hold is not"
+  [ "$rc" -eq 0 ] || fail "a subshell reclaimed its parent's live hold (rc=$rc, bash $BASH_VERSION)"
+  pass "an abandoned same-frame lock hold is reclaimed; a parent's live hold never is"
 }
 
 # Drain-time historical annotation staleness: a turn-ended-only wake row must
@@ -1193,6 +1201,14 @@ test_historical_annotation_skips_announced_status() {
   pass "historical annotations replay nothing already announced and keep everything new"
 }
 
+# CI's stock macOS Bash lane sets FM_TEST_ONLY to run just the frame-identity
+# lock regression, whose contract differs on a shell with no BASHPID. Every
+# other case here is shell-agnostic and is covered by the portable lanes.
+if [ -n "${FM_TEST_ONLY:-}" ]; then
+  "$FM_TEST_ONLY"
+  exit 0
+fi
+
 test_self_held_lock_reclaims_instead_of_deadlocking
 test_secondmate_foreign_queue_stall_is_one_shot_and_read_only
 test_secondmate_stall_marker_rejects_symlink