Caches have no cross-process lock: concurrent CLI/menubar/MCP runs clobber each other's work

[avs-io]

Problem

Cache writes are correctly atomic (temp file, fsync, rename), but nothing coordinates across processes. daily-cache serializes only in-process via a promise chain (src/daily-cache.ts:184-190); session-cache, cursor-cache, and codex-cache do read-mutate-rename with no lockfile. CodeBurn deliberately runs as several concurrent processes: CLI, menubar refresh (every 30s), the guard statusline daemon, and the MCP server. Last-writer-wins means a full parse can overwrite a concurrent incremental append; nothing corrupts, but both processes re-parse next run. It is an invisible throughput tax that peaks exactly when the tool is used most, and it can make first-paint feel slow for no visible reason.

Evidence

• src/daily-cache.ts:184-190 (in-process lock chain only)
• No O_EXCL/flock usage in src/session-cache.ts or src/daily-cache.ts; the only lockfile in the repo is src/act/journal.ts:56.
• Verified on main at 1a75484.

Proposed fix

Reuse the act journal's lockfile pattern: acquire an O_EXCL lockfile with a stale-lock timeout around saveCache/saveDailyCache (and optionally around the expensive parse itself so concurrent processes share one result instead of racing). The orphaned-temp GC already in session-cache.ts is the model for stale-lock cleanup.

[avs-io]

Design proposal before any code, since this touches every cache consumer. Feedback welcome — particularly on lock scope (§2 recommends locking the full parse+write transaction, not just writes) and the first-PR slice (§7, session-cache only).

1. Problem restatement

Cache publication is atomic, but cache computation is not coordinated across processes. daily-cache serializes only within one process, while the session, Cursor, and Codex caches can each load an old snapshot, compute independently, and replace a newer result; the file remains valid, but the losing work must be repeated on the next refresh. The macOS menubar and dashboard can refresh every 30 seconds, MCP calls aggregate through the same parser, and guard install/guard refresh invoke parseAllSessions; the guard statusline uses a separate per-session cache and is not part of this race.

2. Lock scope decision

Cache	Write-only lock	Parse + write lock
session-cache.json	Holds for one fsync/rename. It prevents simultaneous renames but still permits a full scan based on snapshot A to overwrite an incremental update based on snapshot B, so it neither fixes lost cache updates nor saves parsing.	Lock loadCache through discovery, reconciliation, changed-source parsing, and saveCache. A waiter blocks for the holder's changed-source scan; after acquiring it reloads the saved fingerprints and normally parses nothing. A cold scan can hold the lock for substantially longer than an incremental refresh.
daily-cache.json	Holds for one fsync/rename, but two processes may still backfill from the same lastComputedDate and the later stale snapshot wins.	Replace the process-local withDailyCacheLock transaction with a cross-process transaction covering load, gap parse, merge, and save. Wait is normally near zero when there is no gap; first run or version/config invalidation can hold it for the 365-day backfill and saves every duplicate backfill.
cursor-results.json	Serializes only final replacement; every miss still runs the same SQLite query, and an older DB fingerprint may replace a newer result.	Lock the cache check, Cursor DB parse, fingerprint recheck, and write. A waiter can wait for the first full Cursor query, documented as potentially taking about a minute, then consume that result instead of querying again.
codex-results.json	Does not protect the per-process memCache: two processes can load disjoint file sets and the later flush drops the other set.	Cover Codex cache load, per-file parsing/mutation, eviction, and flush. Current callers perform this inside parseAllSessions, so the outer session-cache transaction provides this coordination without a nested Codex lock.

Recommendation: lock the complete read/reconcile/parse/write transaction. A write-only lock does not solve the stated race. Keep locks per cache rather than using one global cache lock; in the current call graph, the session transaction also covers Cursor and Codex provider-cache writes. If daily hydration calls parseAllSessions, lock order is always daily then session; no path may acquire them in reverse order.

3. Mechanism

Add one small cache-lock helper based on the journal's writeFile(..., { flag: 'wx' }) pattern. Put each lock beside its payload so it follows the payload's configured location and stays on the same filesystem:

• session-cache.json.lock
• daily-cache.json.lock
• cursor-results.json.lock
• codex-results.json.lock

The lock body is { "pid": number, "createdAt": ISO-8601 string, "token": random string }. pid and createdAt are diagnostic; the random token is ownership. The exclusive create is the ownership decision. Close the file immediately after creation, update its mtime every 10 seconds while the transaction runs, and in finally remove it only if its token still matches.

Use 90 seconds as the stale threshold: three normal 30-second menubar refresh periods, while still recovering an abandoned cache lock without manual cleanup. A live long parse is not stale because of the heartbeat. On EEXIST, wait with short jittered polling. A stale contender first exclusively creates a *.lock.takeover guard, then rereads the owner token and mtime; it removes the primary lock only if both still match the stale observation, otherwise it restarts. The takeover guard is subject to the same stale cleanup. This serializes two stale contenders instead of allowing both to unlink the canonical lock. PID liveness is not an ownership test because PIDs are reused and portable process probing is inconsistent.

Cleanup is lazy, like cleanupOrphanedTempFiles: normal release happens in finally; after a crash, the next contender recognizes the old mtime and removes the stale lock/takeover file. Extend the existing opportunistic cache-directory cleanup to remove stale lock sidecars and old randomized temp files, but never remove a fresh lock.

Use an advisory lockfile rather than flock. Node core exposes portable exclusive creation but no portable flock, and Windows locking/share modes do not match POSIX advisory-lock lifetime semantics. O_CREAT|O_EXCL is available through Node on macOS, Linux, and Windows. Because the handle is closed immediately, takeover does not depend on unlinking an open file, which Windows may reject. Payload temp files remain on the same filesystem and retain the existing rename replacement behavior.

This guarantee is for a normal local cache directory. O_EXCL and rename semantics are not reliable on every NFS/SMB implementation, so a network-mounted CODEBURN_CACHE_DIR remains best-effort rather than a supported distributed lock. References: Node filesystem flags, Linux O_EXCL and NFS caveat, and Windows close/delete behavior.

4. Failure modes

Failure	Rule / result
Lock holder crashes	Its closed lockfile remains. After 90 seconds without a heartbeat, the next contender takes it over; orphaned temp cleanup remains independent.
Two processes race stale takeover	Only one can create *.lock.takeover with wx. It revalidates token and mtime before removal; the other polls and must not unlink the new owner's lock.
Wall clock moves	Staleness uses Date.now() - mtimeMs; a negative age is fresh. A forward jump can nominate a lock as stale, but the takeover guard, token/mtime recheck, and 10-second heartbeat limit false takeover. Tests inject time rather than changing the system clock.
Parse exceeds 90 seconds	Heartbeats keep the lease live indefinitely. Waiters continue polling; the macOS menubar's existing 45-second subprocess timeout may abandon its waiter, but it does not invalidate the holder's lease.
Reader arrives during a write	Pure reads never take or wait on the lock and see either the previous or next complete file through existing temp + rename publication. A read that begins a read-modify-write refresh is inside the locked transaction and reloads after acquisition.
Lock cleanup/release fails	Do not fail a completed cache read result solely because cleanup failed. Leave the sidecar for stale cleanup and emit a bounded diagnostic.

5. What does not change

• No cache schema, version, fingerprint, or on-disk payload format changes.
• Payload writes remain randomized temp file + fsync where currently present + same-directory rename; the lock does not replace atomic publication.
• Pure cache reads remain non-blocking and can serve the previous complete generation.
• With one process, reconciliation and returned data are unchanged; the added work is lock create/heartbeat/remove around a refresh transaction.

6. Test plan

Unit-test the helper with injected clock and poll functions:

• one wx acquisition succeeds and a second cannot enter until release;
• release removes only the matching token and cannot delete a successor's lock;
• a fresh lock is not stolen, a lock older than 90 seconds is taken over, and a heartbeat prevents takeover past 90 seconds;
• two concurrent stale contenders produce exactly one owner and leave no takeover sidecar;
• negative clock age is treated as fresh and a long transaction remains owned through repeated heartbeats;
• callback rejection still releases its owned lock, while simulated unlink failure is recovered by later stale cleanup;
• pure loadCache and loadDailyCache reads complete while the corresponding writer lock is held.

Add a child_process stress test using an isolated CODEBURN_CACHE_DIR and a fixture provider whose parse callback appends one line to a counter file. Seed K changed source files, release N child processes from a barrier into the same parseAllSessions call, and assert:

1. All N children exit successfully and each returns the same K-session result.
2. Final session-cache.json parses and contains the union of all K source paths, fingerprints, and deduplication keys; no update is lost.
3. The provider parse counter is exactly K, not N x K, because waiters reload the holder's cache.
4. No session-cache.json.lock, takeover sidecar, or old temp file remains.
5. Repeat with one child killed while holding the lock: after injected stale age, exactly one successor takes over and the final assertions still pass.

For the daily-cache follow-up, use the same barrier and count the injected historical parseSessions callback: N hydrators produce the same days, the callback runs once, lastComputedDate is the newest expected date, and the final days are the complete date-keyed union.

7. Rollout

First PR: add the reusable lock helper and put only the session-cache.json load/reconcile/parse/save transaction under it, including unit and multi-process stress coverage. This is the smallest honest slice: it fixes the reported full-parse versus incremental-write race and, through the current parseAllSessions call graph, coordinates Cursor and Codex provider-cache writes too.

Follow-up: replace withDailyCacheLock with the cross-process daily transaction and add its multi-process test. Add dedicated Cursor or Codex transaction locks only if those caches gain callers outside parseAllSessions; otherwise nested locks add no protection. A later cleanup can consolidate orphaned temp/lock GC across all cache files and document the local-filesystem requirement for CODEBURN_CACHE_DIR.

[avs-io]

Addendum, since #699 (300e949) landed after the design above was posted: main now has hydrating.lock in session-cache.ts — an advisory cold-hydration lock (exclusive-create, pid-liveness + 15-min freshness, wait-then-reload-warm-cache), explicitly an optimization rather than a correctness gate, and a no-op when the cache is warm (beginColdHydration(isCold=false)).

Mapping that onto the proposal:

• Now covered: the concurrent cold-start case — two processes both facing a full parse no longer duplicate the work or race their initial writes. That was the most expensive instance of the §1 race.
• Still open: the warm-path lost-update race (a full rescan clobbering a newer incremental refresh, §2 row 1), cross-process daily-cache backfill (withDailyCacheLock is still a per-process promise chain), and the cursor/codex provider-cache races (§2 rows 3–4).
• Design adjustment if this goes ahead: rather than adding a parallel session-cache lock, the natural first slice is now extending hydrating.lock's lifecycle to cover the warm refresh transaction too — same sidecar, same takeover rules, one lock per cache as proposed. The token-ownership + heartbeat details in §3 would harden the existing pid-liveness check (pid reuse is the gap), but that can be a separate decision.

No code from my side until there's a nod on whether the remaining scope is worth doing.

[avs-io]

Design revision after a further adversarial pass on my side — two parts of the addendum above don't survive scrutiny, so replacing them before any code:

Withdrawn: extending hydrating.lock. It shipped as a cold-start optimization whose failure mode is "proceed unlocked", and its tests commit to "never a correctness gate". Extending its lifecycle into a warm-path correctness role would silently invert that contract behind the same name. It stays exactly as it is.

Revised mechanism:

• A separate strict gate (session-refresh.lock) around the warm read→reconcile→parse→write transaction, with four explicit outcomes: acquired, completed-by-other, timed-out, unavailable. Only the first two may mutate; timeout/unavailable serves the previous complete snapshot (or a retriable error) — a correctness lock must never fail open.
• An in-process single-flight mutex ahead of the filesystem lock: pid-based ownership can't serialize two concurrent calls inside one MCP process.
• A publication fence: before renaming the payload, the holder revalidates its ownership token under the takeover guard and aborts if displaced. Without this, lease takeover of a suspended/clock-jumped owner produces two live writers — token-scoped release alone doesn't fence the displaced owner's rename.
• UI-bounded waits: menubar-style callers wait well under their 45s subprocess timeout and then get the prior complete snapshot; a waiter's timeout never steals or invalidates the holder's lease.
• Lock order daily → session enforced through one shared acquisition API (today it's safe only by call-graph convention).
• Monotonic clocks for wait deadlines; filesystem mtimes only as crash-recovery hints. Durability claim narrowed to concurrent-process safety; network-mounted cache dirs explicitly unsupported.

First-PR slice, sharpened: lead with a red multi-process lost-update regression (two warm writers, one loses an update — proving the race before fixing it), then the warm session-cache transaction only. No claimed Cursor/Codex protection — their coverage via parseAllSessions is a call-graph fact, not a cache-level guarantee.

Still nothing lands without your go-ahead; this supersedes §2–§4 of the original design.