fix(core): fence lock renewal and release by generation

[iceteaSA]

Fixes #90.

Eviction was already fenced. Renewal and release were its unswept twins: both read the owner record, awaited, then acted on the lock by pathname. Branched from 9bf8f4c, independent of #87.

The three interleavings

1. Renewal — P1 confirms ownership with its lease still valid, stalls, the lease expires, P2 evicts and legitimately acquires, P1 resumes and writeOwner() lands unconditionally over P2's record. Two processes now believe they hold the lock.
2. Release — same shape, destructive. P1 confirms itself owner, stalls past expiry, P2 acquires, P1's queued rm removes P2's live lock and admits a third holder.
3. Renewal after release — a renewal that passes its check before release() can land its write afterwards, recreating an orphan lock nobody owns that blocks contenders until TTL.

All three violate the invariant the eviction fencing exists to hold: the filesystem offers atomic claim but no atomic conditional-delete, so the acceptable worst case is zero winners, never two.

Approach

Both paths now go through the existing eviction-marker primitive as a generation fence, and release awaits any in-flight renewal. ownerId remains the generation and the on-disk format is unchanged, so older builds and other processes reading the same file are unaffected.

The diff is large relative to the fix because the marker helpers were hoisted for reuse. Eviction semantics are untouched — the reviewer verified the four eviction fences survive 1-for-1 (old 236/240/243/249 → new 274/278/281/287, same checks, same order) and that src/tests/review-fixes.test.ts is unmodified.

Honest severity

Each interleaving needs the holder to stall past the 120s TTL between the check and the act — realistically severe event-loop starvation. Worth fixing because it breaks a stated invariant, not because it is likely to fire.

A testability seam, called out deliberately

Forcing the post-check/pre-write race required a new onStep('renewal-write-fenced') seam in production code. It is guarded (if (options.onStep)), so nothing is awaited when unhooked.

Review flagged that as a footgun rather than a bug: anyone later installing a hook there for telemetry would reopen the two-winner race. So the fence is now re-checked after the seam, immediately before the write — the invariant is structural rather than dependent on nobody hooking that point. A regression test pins that re-check specifically.

The extra ownsEvictionMarker() call is one more filesystem read per renewal (interval ≥1s). It is fail-closed: a transient read error aborts the renewal, the lock expires naturally, and the result is zero-winner-then-one-winner — the degradation this design already accepts.

Tests

1069 pass / 0 fail (1064 on 9bf8f4c), tsc clean.

An earlier revision's proof was invalid and was redone. It red-first'd against 9bf8f4c, where the tests hung at 1s because that commit lacks the seams — a missing-API red, not a behavioural one. The proof is now fault injection on the current fencing code, with each RED a concrete assertion about observed state:

fault injected	observed RED
renewal fence disabled	stalled renewal overwrote successor's ownerId
release fence disabled	successor's lock path absent after stale release
release/renewal serialization disabled	orphan lock present after release
post-seam re-check deleted	successor's ownerId overwritten

Plus 3,000 plain contended rounds electing exactly one owner. That stress seeds a stale lock only, so it exercises happy-path eviction under contention rather than the crashed-evictor branch — that branch stays covered by the existing unchanged tests.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fences refresh-lock renewal and release by generation to prevent a stalled holder from overwriting a successor or deleting a live lock. Previously both validated ownership then acted by pathname; now both run under the eviction-marker fence, and release waits for any in-flight renewal, so the worst case is zero winners, never two.

• Reuses the eviction marker via hoisted helpers (withEvictionMarker, ownsEvictionMarker, recoverStaleEvictionMarker); eviction semantics unchanged.
• Renewal: confirm current ownerId and that the lease is unexpired, emit renewal-owner-confirmed, re-check the marker before and after onStep('renewal-write-fenced'), then write; emit renewal-finished regardless. If the marker is busy, emit renewal-marker-unavailable and reschedule; transient errors also reschedule.
• Release: cancel the timer, await any in-flight renewal, confirm current ownerId, re-check the marker, then remove the lock; bounded retries recover a stale marker; never delete by pathname without the marker. Emits release-owner-confirmed.
• On-disk format and ownerId generation are unchanged; no migration. One extra fs read per renewal (fail-closed on transient errors).
• Tests cover renewal and release races, release-vs-renewal serialization, the post-seam re-check, marker contention and injected failures, stale-marker recovery, and a 3,000-round single-winner contention stress.

^{Written for commit f425bd4. Summary will update on new commits.}

[cubic-dev-ai]

All reported issues were addressed across 2 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[iceteaSA]

Updated to f425bd4 (force-push; the previous head 2754324 is preserved on our side).

Cubic found two real bugs after this PR had already passed independent cross-family review, and one of them was a liveness regression this branch introduced — worth stating plainly rather than burying in a thread.

scheduleRenewal() was called inside the withEvictionMarker action body, and that body does not run when the marker cannot be acquired. So transient marker contention from any other process permanently ended the renewal chain: the lock stopped being renewed, expired at TTL, and was stolen — while the owner still believed it held it. Two owners, from a transient condition, in the code meant to prevent exactly that. The pre-fencing renewal path took no marker, so it did not have this failure mode.

The fix inverts the default rather than patching each path: continuation now happens unless we deliberately decided to stop. All ten exits of the renewal attempt were enumerated and classified — four stop, six continue — because the bug existed precisely because one exit silently skipped the continuation.

Also fixed: release() could delete nothing and return when the marker was held or stale, leaving the lock until TTL. It now recovers stale markers and retries, bounded at 8 attempts, with a deliberate documented give-up that prefers letting the lease expire over risking removal of a successor's lock.

Declined, with reasoning in-thread: an "atomic generation-conditional mutation" for the residual pre-write window. That primitive does not exist on a plain filesystem — it is the constraint this module is built around. The window is now bounded and documented in-source instead: two winners require both a writeOwner stall past EVICT_TTL (5s) and the lease expiring during that same stall.

Gate: 1072 pass / 0 fail (1064 on 9bf8f4c), tsc clean. Re-reviewed after the fix: APPROVE, 0 must / 0 should, with the exit enumeration independently reproduced and all three new tests fault-injected to concrete assertion failures.