cache: recover a corrupt session-refresh lock instead of freezing ingestion

[avs-io]

What

observe() in src/cache-refresh-lock.ts classified a stable unparseable session-refresh.lock body as the terminal 'unavailable'. stat and readFile both succeed, the before/after bracket agrees so the body is not 'changing', and JSON.parse throws without an ENOENT/EACCES/EPERM code, so all three attempts fall through.

parseAllSessions routes 'unavailable' to runParse(..., { readOnly: true }), which serves the prior complete snapshot and skips new and modified sources. Nothing repaired the lock, so a zero-byte or truncated lock froze warm-cache ingestion permanently across every later run while each command still exited successfully.

Reproduced against main: a 0-byte lock returns 'unavailable' in ~5 ms regardless of age — fresh or 300 s stale alike. Field report matched: a 0-byte session-refresh.lock whose takeover sidecar named a long-dead PID froze a session cache for days with codeburn doctor reporting every provider healthy.

Changes

• Observation.record becomes LockRecord | null. A null record owns nothing but is a real file with a real mtime, not an infrastructure failure.
• observe() catches the parse failure locally and returns that observation instead of 'unavailable'. Contention still outranks corruption: any attempt that sees the file change still yields 'changing'. EACCES/EPERM and unreadable directories still return 'unavailable'.
• tryTakeover and the age gate are byte-identical to main. A corrupt body is recovered only once its mtime is older than staleMs, exactly like an abandoned well-formed lock. Staleness is never waived.
• sameObservation compares an explicit null/non-null boundary, then token, mtime and a sha1 of the raw body. Two corrupt bodies have no tokens to compare, and a same-size rewrite moves neither size nor — on a coarse filesystem — mtime, so mtime alone is not a change signal.
• The heartbeat deliberately does not rewrite a body it cannot prove is its own.

The heartbeat decision, and why it went the other way

An earlier revision of this branch had the heartbeat repair its own corrupt body, on the reasoning that skipping it leaves an owner's mtime frozen so it ages out and gets taken over. Adversarial review broke that in three ways, each with a working repro and a passing counterfactual on main: a legitimately-replaced owner resurrecting itself and answering verifyStillOwner() true 34 consecutive times while the real owner was locked out of its own publication; a conservative foreign-version owner having its live lock overstamped and then deleted by the displaced process's removeIfOwned; and the disclosed external-rm case going 3/3 deterministic, with both processes' fences answering true in the same episode.

The guard does not prevent this, because createExclusive takes no guard — it publishes a directory entry before its body, so an unparseable body may be a successor's lock a millisecond from being written.

So an owner that cannot prove ownership ends its ownership. Its mtime stops advancing, the publication fence refuses, the parse is discarded, and a successor recovers the lock one staleMs later through the age gate. Losing a parse is the correct price for never having two owners, and it is the fail-safe direction. Re-verified after the change: 0/12 on the resurrection race (was 1/8), 3/3 clean on the rm window, and the foreign-version case now shows the legitimate owner's record intact.

Verification

Regression tests promoted from the adversarial repros into tests/cache-refresh-lock-corrupt-body.test.ts with two process fixtures. Both fail against main (expected 'unavailable' to be 'timed-out') and against the earlier revision of this branch, in both directions — they are not tautological.

tests/cache-refresh-lock.test.ts covers: stale zero-byte, stale malformed-JSON, stale wrong-shape, fresh corrupt held out for the wait, transient truncation still classified as contention, unreadable body still 'unavailable', a stale zero-byte takeover sidecar (a second corrupt state that froze recovery independently of the primary lock), and an end-to-end parseAllSessions case where a warm cache plus a 0-byte lock plus a dead-PID sidecar resumes ingesting a newly added session file.

Independently measured during review: every corrupt-state combination recovers, worst case exactly one staleMs window (90,008 ms at real defaults; the sidecar ages in parallel, never serially). Lock suite passes 26/26 on a real exFAT volume as well as APFS. ENOSPC mid-heartbeat on a filled volume leaves the body valid and the mtime advancing. The singleFlightTail cost of a contended corrupt lock is identical to a contended valid one, so corruption adds no queueing.

Full suite: 2474 passed. tsc --noEmit clean. One unrelated failure, tests/cli-durable-totals.test.ts, reproduces identically on clean main at 8578199 — pre-existing, and worth its own look.

Out of scope

• No change to the takeover protocol, ownership token, heartbeat interval, staleness threshold, wait budget, or publication fence.
• hydrating.lock (cold-start, advisory, fail-open) untouched. src/parser.ts untouched — read-only serving for timed-out/unavailable is the intended fail-closed behaviour from Caches have no cross-process lock: concurrent CLI/menubar/MCP runs clobber each other's work #645.
• createExclusive's bodyless window is left open with a comment. Recovery requires an mtime older than staleMs and the window is milliseconds wide on a file whose mtime is by definition now; closing it would mean link()ing a temp file into place, which is not portable to filesystems without hard links.
• observe()'s readFile is unbounded. Garbage at the lock path makes the poll loop expensive (measured: a 100 MB body burns ~1.4 s user CPU per 3 s of waiting). Pre-existing, bounded by waitMs, and a size check in the before stat would cap it — deliberately left for a follow-up so it gets its own review rather than riding in unexamined.
• An immutable (chflags uchg) corrupt lock still freezes forever. unlink is EPERM, so no code can recover it; main behaves identically.
• A foreign version's record shape reads as corrupt here, so a foreign lock is only protected while it heartbeats. Degradation is read-only and fail-safe.
• No diagnostic surfacing. That doctor reported healthy while ingestion was frozen is a real observability gap and is better addressed on its own.
• Windows and network filesystems unverified: no host available. On NFS/SMB open(path,'wx') exclusivity and mtime coherence are not guaranteed, which the Caches have no cross-process lock: concurrent CLI/menubar/MCP runs clobber each other's work #645 design already declares unsupported.

[iamtoruk]

Deep-reviewed with live multi-process probes — this holds up. Concurrent recovery is correctly serialized (3/3 single-owner across real child processes racing a zero-byte lock), a live holder is only displaceable while its body is corrupt — at which point its own fence is already dead, so the late-save rejection works in every interleaving probed — and crash-mid-recovery self-heals identically to main. The field scenario is genuinely fixed: on main, a stale zero-byte lock + zero-byte sidecar freezes ingestion permanently; here it recovers within 90s. Healthy-path perf measured identical (500-cycle A/B, fully overlapping). Port to the extraction branch is path-only (files byte-identical; patch applies clean).

Two cheap fixes to fold in:

1. The "why a live owner cannot be stolen" section overclaims: as implemented, the requireStale=false waiver's evidence is "byte-identical corrupt at the last poll AND at the guarded re-observe" (~50ms), not "unchanged across many heartbeat periods" — reproduced by truncating the body 50–200ms before a waiter's deadline (waiter acquires, holder's verifyStillOwner() correctly false). The safety argument that actually holds — and it does hold — is that a corrupt body means the incumbent's fence is already dead. Worth rewording so the docs defend the real invariant.
2. The new end-to-end test sets CLAUDE_CONFIG_DIR/CODEBURN_DESKTOP_SESSIONS_DIR and deletes them at the end of the test body; a mid-test failure leaks them into subsequent tests (and can clobber a developer's real config dir). Move the cleanup to afterEach.

Optional hardening, your call: an age floor on the waiver (e.g. require the corrupt mtime to be older than a heartbeat) closes a lab-only steal vector — the unguarded open('wx')→writeFile gap in createExclusive makes a mid-create owner stealable if a waiter's final poll lands in that ~7ms window (reproducible under threadpool saturation; returns unavailable on main). It would also require reworking the fresh-malformed-lock test's fake clock, hence not a drive-by. Also worth adding: a multi-process test for the waiver path itself (the new process test only covers the stale path).

One more datum: the only red test on base, main, and PR alike is the known cli-durable-totals.test.ts:182 time-of-day flake — unrelated to this change; a fix for it is coming to main.