fix(supervision): bound lock steal recursion on progress, not depth

[zeeshaanahmad]

Lock steal recursion is bounded on progress, not on depth

Starting from the disproof, not from the depth bound

A depth bound on this same recursion was written during the signal-deferral work (#7) and reverted before it landed, because its own red/green proof showed it would deadlock legitimate recovery: an unbounded walk of a fully abandoned steal chain genuinely reclaims the lock, and capping the walk turns slow, noisy recovery into a permanent refusal that fm_lock_acquire_wait then spins on forever. That conclusion holds and is not re-litigated here. This change does not re-land the depth bound in any form.

1. The legitimate recursion, characterised first

fm_lock_try_acquire calls itself in exactly one place: to acquire "<lockdir>.steal", the mutex that serialises displacing an abandoned holder.

A level is entered only because the level above it exists on disk with a dead holder. So the recursion depth equals the length of the abandoned chain, and a chain grows one level per crash that happens while the steal mutex is held. Measured against the real code path (a process that acquires L, L.steal, L.steal.steal, L.steal.steal.steal and is then kill -9ed):

• the walk descends to the first level it can create, then unwinds reclaiming every level under it;
• it is self-cleaning - after reclaiming L, every .steal level is gone, verified by listing the directory;
• the maximum legitimate depth is therefore structurally bounded by the filesystem itself, since every existing level had to be created with a name under NAME_MAX and each level adds six bytes. Nothing else bounds it, and nothing else should.

That is why no depth number is safe to calibrate: the depth that must be tolerated is "however many crashes happened since the last successful reclaim", and the cost of guessing low is a lock nobody can ever take again.

2. What actually runs away, and the guard chosen

The runaway is the case with no abandoned holder at all.

fm_lock_try_create fails for two very different reasons: the lock exists (contention - the case the steal path is for), or it could not be created at all. In the second case the lock path is absent, the pid read is empty, fm_pid_alive is false and fm_lock_mid_acquire_is_fresh is false, so control falls into the steal path anyway - where the deeper level fails for the identical reason, while the name grows by .steal each time. Past NAME_MAX every level keeps failing and the recursion never terminates.

Reproduced deterministically, with a cause that is not exotic: a state directory removed or moved out from under a running watcher. fm_lock_try_acquire "$state/gone/.contend.lock" was still recursing after 15s and had to be killed; its stderr showed names grown past the OS limit.

The guard is progress, not depth, and it is applied by attempting progress rather than inferring it. Before recursing, if the lock path is absent the create is retried once:

• it succeeds -> the lock is taken and returned normally;
• the path reappeared -> report contention and let the caller come back, rather than steal on evidence this frame gathered before the new holder existed;
• still absent, still uncreatable -> refuse, loudly, once per path per process.

Recursion is therefore untouched whenever there is something to displace, at any depth, which is precisely the property the disproof requires.

Why this over the alternatives the task listed

• Per-entry-point depth accounting and a generous calibrated ceiling both still terminate in a refusal on a legitimate walk. Any ceiling is a number against "crashes since the last reclaim", and being wrong strands the lock permanently. Both inherit the reverted bound's failure mode, only later.
• Cycle detection on lock identity does not fire here: the path is strictly different at every level (the name grows), so there is no cycle to detect. It would have to be re-expressed as "the same failure repeating", which is what the progress test already is, more directly.
• Structured re-entry tokens would add state threaded through a recursion whose legitimate depth is already bounded by the filesystem. Machinery for a bound that already exists.

The progress test also needs no calibration and no new tunable, and its failure mode points the safe way: if it is ever wrong it refuses a lock nobody holds, which the caller retries, instead of refusing a lock it could have recovered.

A benign race the first version got wrong

The first version tested progress by inspection alone - "path absent -> refuse". Running the existing suite surfaced the counterexample immediately: test_cycle_exit_ledger_links_successor_and_stays_bounded printed the new refusal for .watcher-down.lock in a state directory that plainly existed. The holder had released between the create attempt and the check, so the lock was simply free, and the old code reached it (wastefully) through the steal path. Inferring the fault from absence turned a benign, non-rare race into a spurious failure and a scary operator warning.

Retrying the create fixed it, and the warning no longer appears anywhere in the suite. Recorded because the lesson generalises: absence of a lock is ambiguous evidence, and the cheap way to disambiguate is to attempt the thing rather than to reason about it.

3. Red-then-green

Both cases are pinned in tests/fm-watcher-lock.test.sh, alongside the existing steal-primitive cases. The runaway case is bounded in the test (150 x 0.1s, then kill -9 and a named failure) because the regression is an unbounded loop and a hanging suite reports nothing.

case	base 85643b8	this branch
a fully abandoned steal chain (4 levels) is walked and cleaned up	ok (control)	ok
an uncreatable lock is refused instead of recursed on	not ok - acquiring an uncreatable lock never returned (unbounded steal recursion)	ok
one refusal warning per path per process	(n/a - no refusal exists at base)	ok

The first row is the row that judges the design: it passes on both sides. The guard did not buy termination by breaking the recovery the reverted depth bound broke - that is the case a depth cap of 4 or less would have turned into a permanent refusal.

Upstream check

Fetched upstream (kunchenguid/firstmate) and searched its open issues for lock, recursion, deadlock and steal before implementing. No open upstream issue covers this defect. recursion returns zero. The nearest neighbours are different faults in different code: kunchenguid#1972 (remote job worker wedged on an orphaned lock temp file), kunchenguid#1508 (Windows/MSYS kill -0 reporting every pid alive, so fm_lock_acquire_wait spins), kunchenguid#2251 and kunchenguid#2270 (the watcher/daemon symptoms already addressed by #7). Nothing was adopted from them.

Not harness-dependent

The verdict comes from filesystem state and lock ownership, not from anything a vendor emits, so per the coding guidelines this is pinned by a portable regression with real processes and no live-harness guard is owed. No per-harness verification record changes.

Local verification

macOS 26.4.1 (arm64), /bin/bash 3.2.57. Measured against the pushed head.

Red evidence was produced in an isolated copy running bin/ restored to 85643b8 with tests/ from this branch, so the only difference is the fix itself.

bash bin/fm-lint.sh                          # clean, ShellCheck 0.11.0 (pinned)
bash bin/fm-doc-audience-check.sh            # ok surfaces=67 local_links=243
bash tests/fm-watcher-lock.test.sh           # 33 ok, 0 not ok  (31 before, +2 new)
bash tests/fm-watcher-signal-safety.test.sh  #  5 ok, 0 not ok
bash tests/fm-wake-queue.test.sh             # 19 ok, 0 not ok
bash tests/fm-watch-arm.test.sh              # 14 ok, 0 not ok
bash tests/fm-liveness-source.test.sh        # 15 ok, 0 not ok
bash tests/fm-daemon.test.sh                 # 99 ok, 0 not ok

tests/fm-watch-triage.test.sh: pre-existing load flakiness, confirmed on both sides

This suite could not be brought to a clean run on this machine because it was saturated by unrelated work throughout (load averages 9.7 to 55.6). It failed on both sides, at a different case every run - the exact signature #7 recorded for it, from absorb gates built on fixed wall-clock slices rather than artifacts:

run	side	load	failing case
1	this branch	14.5	changing-hash busy pane past the turn-age bound
2	base 85643b8	9.7	not-provably-working non-terminal stale
3	this branch	12.0	provably-working .seen-* suppressor
4	base 85643b8	18.1	stale suppressor on paused absorb
5	this branch	44.2	working: note with an idle pane
6	this branch	55.6	stale suppressor on absorb

Base failed 2/2 and this branch 4/4, never twice at the same case. Converting that suite's gates to artifact-based waits remains its own task, as #7 concluded.

Not run, and why

• The full bin/fm-test-run.sh suite: left to the pipeline, which ran against the pushed head cddb7b7 and completed its review, test, document and lint steps with zero findings.
• The live-harness-optin guards: nothing here is harness-dependent (see above), so none is owed.
• CI: bin/fm-ci-probe.sh reports none for this fork, so no check run can ever register and the ci step was skipped deliberately rather than left to time out. The PR shows no checks for that reason, not because any check failed.

Merge reasoning (firstmate)

• Honors the disproof it was ordered to start from: no depth bound in any form. The chosen guard bounds on PROGRESS (retry the create once when the lock path is absent; recurse only when a dead holder exists to displace), which preserves arbitrarily deep legitimate recovery - the exact property the reverted design violated - while cutting the actual runaway (recursing on stale evidence).
• The legitimate-recursion characterization is measured against the real code path (crash-grown steal chains via kill -9), and the alternatives the task listed are each rejected with the specific failure mode they inherit, not hand-waved.
• Final diff verified via the raw files API: 2 files (the lock library and a new dedicated test suite); red-then-green per the PR body; pipeline passed with 0 findings at every step; ci step skipped by the freshly-fixed probe reporting none for this fork - the probe fix's first live save.