test: remove watcher lock timing flake

[karotkriss]

Intent

Fix the load-sensitive flake in tests/fm-watcher-lock.test.sh (GitHub issue #2844): the suite passes in isolation every time but fails intermittently under full-suite / ambient concurrent load, which taxes every full-suite run and blocks widening test parallelism.

Required approach (investigation first): reproduce under instrumented co-scheduled load and name the exact tripping assertion, then remove the timing sensitivity - event/condition waits over fixed sleeps, or tolerances derived from the component's own configured grace - WITHOUT loosening the watcher lock's fail-closed semantics (SIGSTOP handling and stale-heartbeat detection are safety code). The issue's fallback of quarantining the test from parallel lanes is acceptable only if the sensitivity is irreducible; the PR must state which was chosen and why.

Root cause established by repro (3/3 failures under 2x CPU oversubscription, identical assertion "arm did not start before self-eviction check"): two fixtures set FM_ARM_CONFIRM_TIMEOUT=1, and bin/fm-watch-arm.sh computes its confirmation deadline immediately AFTER forking the real child watcher, so a real fork + exec + lock acquisition + beacon publication had a 2-second wall-clock budget. Measured real startup under that load is 3.1-3.6s, so the arm correctly reported "FAILED - no live watcher with a fresh beacon" and killed the child, destroying the fixture's premise. The production code behaved honestly; the fixture was asking a real process startup to beat a one-second clock.

Re-running the fixed suite at 3x oversubscription surfaced a second case on the same mechanism: test_arm_propagates_immediate_wake_before_confirmation returned non-zero after printing the wake it already held. That child must also execute a registered check before exiting, and it runs on the production budget - measured at 1.9-2.3s idle but 9.1-13.1s under load, against 11s. It was a coin flip, not a fixed cost.

Fix (test-only, no production change, no default-behavior change):

• The two cases that must confirm a REAL child startup hold the arm to production's own budget instead of a shrunken fixture one. They stay at the production default rather than something roomier because the same budget also bounds the successor wait they deliberately spend, so the suite stays quick.
• The immediate-wake case gets an explicit budget with headroom over its measured loaded cost. That costs nothing on a passing run, because the arm returns as soon as the child settles.
• The two waits for the arm's typed failure are sized off the largest production default (30s on MSYS, 10s elsewhere) through a shared ceiling that is only spent when an arm genuinely fails to exit, instead of a fixed eight seconds.

The suite's own waits stay iteration-counted poll loops, which stretch under load where a wall-clock budget does not. CONTRIBUTING.md records the convention so the pattern does not come back.

Verification: 3/3 red before the change at 2x oversubscription, then 4/4 green at 3x (loadavg 75-80) with the whole suite completing, plus a clean unloaded run. The remaining fixed sleeps in the 40-way lock-contention cases were probed to 6x oversubscription without tripping, so they are left alone rather than rewritten speculatively.

Quarantine was rejected on evidence, not preference: bin/fm-test-isolation-proof.sh already excludes the watcher/wake/lock family, and bin/fm-test-run.sh already refuses --jobs > 1 for this script, yet the reporter's failure happened at --jobs 1 under ambient machine load. Quarantine was already in force and could not have prevented it.

VISION.md alignment: "This repository ships through its own discipline: firstmate work is validated like any other project's, and field incidents become regression coverage" - a flaky red trains contributors to ignore red. "Scripts stop safely and report when the world surprises them" - the arm's refusal was correct, so the test was corrected rather than the safety code weakened. Deliberately no default-behavior change anywhere in bin/.

What Changed

• Remove one-second confirmation budgets from two fixtures that launch real child watchers, so their startup windows are production-sized under load.
• Give the immediate-wake fixture an explicit confirmation budget with headroom over its measured loaded cost, since its child must also run a registered check inside that window.
• Bound typed-failure waits with a shared iteration-counted polling ceiling derived from the largest production confirmation default, without changing watcher safety behavior.
• Document the load-safe timeout and polling convention for test fixtures in CONTRIBUTING.md.

Fixes #2844

Risk Assessment

✅ Low: the test-only change directly addresses the reproduced timing race while preserving production watcher safety behavior and platform-specific confirmation defaults.

Testing

The real-process watcher-lock suite passed normally in 80 seconds and under 2x CPU oversubscription in 225 seconds, including the formerly failing self-eviction startup assertion, real-child peer handoff, immediate wake propagation, typed arm failure, stale-heartbeat detection, and SIGSTOP fail-closed behavior. Reviewer-visible CLI transcripts were captured, production scripts were unchanged, and no transient processes or worktree artifacts remained.

Evidence: Focused watcher-lock test transcript

Source: Focused watcher-lock test transcript

Command: bin/fm-test-run.sh tests/fm-watcher-lock.test.sh
Commit: 0d73e0e309fd845f55d16fc4ba9107ed0cf86039
Started: 2026-08-23T11:27:35-04:00
FM_TEST_BEGIN 2026-08-23T15:27:36Z tests/fm-watcher-lock.test.sh family=watcher-wake-lock expected_gate_skip=none
ok - simultaneous watcher starts leave exactly one live process
ok - fm_pid_identity real ps fallback is locale-invariant
ok - fm_pid_identity is locale-invariant across LC_ALL/LC_TIME
ok - /proc process identity ignores simulated btime changes
ok - /proc process identity detects pid reuse
ok - MSYS /proc process identity regression skipped on non-Windows host
ok - killed watcher stale lock is reclaimed
tests/fm-watcher-lock.test.sh: line 1061: 24589 Killed                     FM_STATE_OVERRIDE="$state" bash -c '
    . "$1"
    fm_lock_remove_path() {
      if [ "$1" = "$STATE/.watch.lock" ]; then
        kill -KILL "${BASHPID:-$$}"
      fi
      return 1
    }
    fm_lock_try_acquire "$2"
  ' _ "$LIB" "$lockdir" > /dev/null 2>&1
ok - stale watcher reclaim publishes durable recovery evidence before clear
ok - live watcher lock with stale heartbeat is actionable
ok - guard banner leads when down with pending wakes (repair-after-drain) and stays silent when live and fresh
ok - concurrent fm_lock_try_acquire yields exactly one winner
ok - dead-pid stale lock is reclaimed by a single acquirer
ok - concurrent stale-lock steal yields exactly one winner
ok - live steal mutex is not reclaimed
ok - live-held lock is not stolen
ok - empty mid-acquire lock keeps a minimum grace
ok - late original claimant cannot claim a recreated lock
ok - paused mid-acquire claimant backs off to active stealer
ok - watch restart preserves recovery without signaling a reused pid
ok - watch restart attaches to a verified healthy peer and later surfaces a successor gap
ok - watcher self-evicts when the lock pid no longer names it
ok - arm turns clean self-eviction without a successor into a typed failure
ok - arm attaches to a live fresh watcher and fails loudly when that cycle has no successor
ok - attached arm signals record a classified lifecycle entry
ok - arm starts cleanly and resurfaces recovery after a dead-pid lock
ok - arm cleans child watcher and temp output on HUP
WAKE_ACK_REQUIRED: after handling completes run bin/fm-wake-drain.sh --ack-through 1 --recovery-generation 63961.1787498901.bdQdaP
ok - arm propagates an immediate watcher wake before confirmation
ok - arm attaches to a peer watcher after child stands down and surfaces a missing successor
watcher: lock held by live pid 73276 but heartbeat is stale for 840799714s (>300s); inspect or stop that watcher before re-arming.
ok - arm reports FAILED and exits non-zero when no fresh watcher can be confirmed
ok - cycle-exit ledger links a verified successor and remains size-capped
ok - SIGSTOP distinguishes live PID from stale beacon and termination records the exit class
FM_TEST_END 2026-08-23T15:28:55Z tests/fm-watcher-lock.test.sh exit=0 duration_ms=79884 gate_skip=false
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=0 duration_ms=79978
FM_TEST_SUMMARY_FAMILY family=watcher-wake-lock count=1 duration_ms=79884 failed=0
FM_TEST_SLOWEST rank=1 script=tests/fm-watcher-lock.test.sh duration_ms=79884
Elapsed: 1:20.18
Max RSS: 43040 KB
Exit: 0
Finished: 2026-08-23T11:28:56-04:00

Evidence: Watcher-lock test under 2x CPU oversubscription

Source: Watcher-lock test under 2x CPU oversubscription

Command: 40 CPU burners (2x 20 logical CPUs) plus bin/fm-test-run.sh tests/fm-watcher-lock.test.sh
Commit: 0d73e0e309fd845f55d16fc4ba9107ed0cf86039
Logical CPUs: 20
Load workers: 40
Started: 2026-08-23T11:29:29-04:00
Pre-test loadavg: 4.07 22.14 61.33 49/1421 31215
FM_TEST_BEGIN 2026-08-23T15:29:29Z tests/fm-watcher-lock.test.sh family=watcher-wake-lock expected_gate_skip=none
ok - simultaneous watcher starts leave exactly one live process
ok - fm_pid_identity real ps fallback is locale-invariant
ok - fm_pid_identity is locale-invariant across LC_ALL/LC_TIME
ok - /proc process identity ignores simulated btime changes
ok - /proc process identity detects pid reuse
ok - MSYS /proc process identity regression skipped on non-Windows host
ok - killed watcher stale lock is reclaimed
tests/fm-watcher-lock.test.sh: line 1061: 33812 Killed                     FM_STATE_OVERRIDE="$state" bash -c '
    . "$1"
    fm_lock_remove_path() {
      if [ "$1" = "$STATE/.watch.lock" ]; then
        kill -KILL "${BASHPID:-$$}"
      fi
      return 1
    }
    fm_lock_try_acquire "$2"
  ' _ "$LIB" "$lockdir" > /dev/null 2>&1
ok - stale watcher reclaim publishes durable recovery evidence before clear
ok - live watcher lock with stale heartbeat is actionable
ok - guard banner leads when down with pending wakes (repair-after-drain) and stays silent when live and fresh
ok - concurrent fm_lock_try_acquire yields exactly one winner
ok - dead-pid stale lock is reclaimed by a single acquirer
ok - concurrent stale-lock steal yields exactly one winner
ok - live steal mutex is not reclaimed
ok - live-held lock is not stolen
ok - empty mid-acquire lock keeps a minimum grace
ok - late original claimant cannot claim a recreated lock
ok - paused mid-acquire claimant backs off to active stealer
ok - watch restart preserves recovery without signaling a reused pid
ok - watch restart attaches to a verified healthy peer and later surfaces a successor gap
ok - watcher self-evicts when the lock pid no longer names it
ok - arm turns clean self-eviction without a successor into a typed failure
ok - arm attaches to a live fresh watcher and fails loudly when that cycle has no successor
ok - attached arm signals record a classified lifecycle entry
ok - arm starts cleanly and resurfaces recovery after a dead-pid lock
ok - arm cleans child watcher and temp output on HUP
WAKE_ACK_REQUIRED: after handling completes run bin/fm-wake-drain.sh --ack-through 1 --recovery-generation 91600.1787499073.y7trbf
ok - arm propagates an immediate watcher wake before confirmation
ok - arm attaches to a peer watcher after child stands down and surfaces a missing successor
watcher: lock held by live pid 4052 but heartbeat is stale for 840799894s (>300s); inspect or stop that watcher before re-arming.
ok - arm reports FAILED and exits non-zero when no fresh watcher can be confirmed
ok - cycle-exit ledger links a verified successor and remains size-capped
ok - SIGSTOP distinguishes live PID from stale beacon and termination records the exit class
FM_TEST_END 2026-08-23T15:33:14Z tests/fm-watcher-lock.test.sh exit=0 duration_ms=225231 gate_skip=false
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=0 duration_ms=225697
FM_TEST_SUMMARY_FAMILY family=watcher-wake-lock count=1 duration_ms=225231 failed=0
FM_TEST_SLOWEST rank=1 script=tests/fm-watcher-lock.test.sh duration_ms=225231
Elapsed: 3:46.07
Max RSS: 42880 KB
Post-test loadavg: 48.82 36.88 58.88 54/1442 53947
Exit: 0
Finished: 2026-08-23T11:33:15-04:00

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

• bin/fm-test-run.sh tests/fm-watcher-lock.test.sh
• Started 40 yes CPU workers across 20 logical CPUs, then ran bin/fm-test-run.sh tests/fm-watcher-lock.test.sh
• Verified git diff --exit-code -- bin and confirmed no production script changes
• Verified all 40 load workers exited and the worktree remained clean

✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning

• ⚠️ linter found issues (exit code 1)

✅ **Push** - passed

✅ No issues found.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; no concrete changed-code defect or security issue was identified.

The test-only timing changes preserve watcher fail-closed behavior, return early on successful confirmation, and provide bounded waits that cover the platform-specific production deadlines.

_{Reviews (1): Last reviewed commit: "test(watcher): stop fixture confirmation..." | Re-trigger Greptile}