The probe round on the mail queue was supposed to be a quick check. The queue is shared state between two long-running processes — the foreground attorney REPL and the background prefetcher worker — and the surface is small. A few functions, all doing JSON-on-disk read-modify-write through a single lock helper.
The lock helper looked like this:
async function withLock(fn, { timeoutMs = 3000 } = {}) {
const start = Date.now();
while (existsSync(LOCK_FILE)) {
if (Date.now() - start > timeoutMs) break; // stale lock — proceed anyway
await new Promise(r => setTimeout(r, 50));
}
try {
writeFileSync(LOCK_FILE, String(process.pid));
return await fn();
} finally {
try { writeFileSync(LOCK_FILE, ''); } catch {}
}
}
Read this carefully. There are two structural problems and they compound.
First, the wait loop uses existsSync. The lock is "acquired" by checking that the file doesn't exist, then writing your PID to it. This pattern only works as a lock if the check and the write are atomic — if some other process could be doing the same check in between. They cannot be atomic when implemented as separate existsSync and writeFileSync calls. Both processes can do the check, both find no file, both do the write, both think they have the lock.
Second, the release writes an empty string to the lock file instead of deleting it. This means after release, the file still exists. The next acquirer sees the file in the existsSync check and waits — but only until the 3-second timeout, after which the comment says "proceed anyway." The lock is bypassed under almost any contention.
I wrote a stress probe: 10 processes, each calling upsertItem 30 times, for 300 expected entries in the final queue. The probe measured how many of those 300 actually survived.
Six.
Two hundred and ninety-four out of 300 writes were silently overwritten. 98 percent data loss. Every queue mutation in production was running through this lock — every new mail download, every status update, every analyze completion. Most mutations win their races just by being faster than the racing peer, but a non-trivial fraction lose silently.
The textbook fix and the deviation
The proven lock pattern in the rest of this codebase uses openSync(LOCK_FILE, 'wx') — which maps to the POSIX O_CREAT | O_EXCL open. That open is atomic at the kernel level: exactly one of N concurrent calls succeeds. Combined with PID-liveness detection (you can check whether the lock-holder's process is still alive) and an unlinkSync on release, the pattern provides actual mutual exclusion. Two other modules in this codebase had already converted to this pattern in earlier fixes.
Copying that pattern into the queue's withLock got me from "98% data loss" to "varies between 14 and 122 losses per 300 expected." Better than 294, but still nowhere near right. The lock was real now, but something else was wrong.
I traced. Every individual upsertItem call returned successfully. The per-process logs showed no exceptions. But the resulting queue was missing entries. The only way for a successful upsertItem to lose its write is if another process's lock-protected write came in between the acquire and the release of the first process — which would mean the lock was being broken.
The audit-log lock and the binding lock — both of which use the same O_EXCL pattern — had an extra branch: if the lock file's PID content was unreadable, fall back to an mtime check. If the file is older than 10 seconds, treat it as stale. This handles the case where a crashed process leaves a lock file behind with a PID that's no longer alive.
Under the queue's specific access pattern — where the work inside the lock is very short, tens of milliseconds — there's a tiny window between two operations:
fd = openSync(LOCK_FILE, 'wx'); // creates the file, empty body
writeFileSync(LOCK_FILE, String(process.pid)); // writes the PID
Process A does step 1: the file exists, empty. Process B tries openSync — fails EEXIST. Process B falls into the catch, reads LOCK_FILE — content is empty string. parseInt("") is NaN, not a valid PID. Process B falls into the "unreadable PID" branch, checks mtime.
On most kernels and most filesystems, that mtime is right now (the file was just created milliseconds ago), so the stale check correctly says "not stale, keep waiting." But on macOS APFS under tight contention, I observed the mtime returning values that occasionally tripped the stale check. The lock would get unlinkSync'd by B while A still believed it held the lock. Both then proceeded.
The fix for the queue specifically is to drop the mtime fallback. A held lock with an unreadable PID just means "keep waiting." A crashed lock holder whose PID was never written stays locked until the 30-second timeout, then throws — which is the correct fail-loud behavior. The cost is one specific failure mode (a process crashes between openSync and the PID write) ends up waiting 30 seconds instead of immediately failing over. That's an acceptable trade for the queue's tight inner loop, where the alternative is silent data loss.
After dropping the mtime fallback: 10 consecutive runs of the 300-write stress test, all 10 runs at 300 of 300 entries. Three thousand cumulative writes, zero losses.
What this means for the audit log and binding locks
The audit log and binding locks still have the mtime fallback. I tested them under the same stress and they remained at 300 of 300 — neither showed the same false-stale-detection. The difference is timing: the audit log's work inside the lock is dominated by an fsync (milliseconds) and the rotation work (tens of milliseconds when it fires), while the queue's work is just JSON parse + manipulate + write (single-digit milliseconds usually). The tighter the work-inside-lock, the more often the race window between openSync and the PID write gets hit by another process. The queue exposes that window enough to break things; the audit log doesn't.
I left the audit and binding locks alone in this release. They're known-working at 300 of 300; changing them carries regression risk for the more critical paths. If a future probe shows them losing entries under a different access pattern, the fix is the same: drop the mtime fallback.
The other queue improvements
Three smaller fixes shipped in the same release:
Atomic queue file writes. The pre-fix wrote JSON directly to the queue file with writeFileSync(QUEUE_FILE, ...). A SIGKILL between the start of the write and its completion would leave the queue truncated or partial. Replaced with tmp + fsync + rename, so the file at the canonical path is always either the previous valid state or the new valid state, never a mix.
Corrupt-file preservation. The pre-fix readQueue silently returned an empty queue when JSON.parse failed. The next writeQueue then overwrote the corrupted file with the empty queue, destroying the original. Now parse failure preserves the bad file at mail-queue.corrupt.<ISO-timestamp>.json before returning empty. A compliance review can recover entries by inspecting the preserved file.
Null-body guard. Same null-body shape that's been fixed in five other places in this codebase. JSON.parse(body) returning null would crash on the next property access. Now an explicit non-object guard with the corrupt-file preservation path.
The general lesson
The thing that hurt the most about this bug is how plausible the broken code looked. The wait loop has a comment explaining the timeout. The release has a try/finally. The variable is named LOCK_FILE. If you were skimming the file you'd say "yeah, this is a lock helper." The pattern doesn't have any obvious red flags — there's no // TODO: fix this, no commented-out code, no obvious test gap. It's just structurally wrong in a way that requires running concurrent processes to see.
The lesson I keep coming back to: file-based mutual exclusion has exactly one safe primitive on POSIX, and it is open(O_CREAT | O_EXCL). Anything that uses exists? followed by write is racy by construction. Anything that doesn't unlink on release is racy by construction. If you see those patterns in a "lock," you have a lock-shaped object that does not actually lock.
And the meta-lesson on probing: the only way to find concurrency bugs is with concurrent probes. Single-process tests of this queue passed cleanly — sequential 100-write tests, single-process burst tests, single-process error-injection tests, all clean. The 98%-loss bug only appeared with spawned child processes hitting the queue from outside the JavaScript event loop's serialization. Audit logs and queue files are exactly the surfaces where multi-process probes are mandatory.
The version on the install URL is 4.5.99.