fix(resources): store the real prior version for LMDB audit entries

[kriszyp]

What / why

RecordEncoder.ts:854 handed LMDB a boolean 0/1 flag as previousVersion where RocksDB got the real prior version. A flag of 1 sends the write down auditStore.ts's placeholder-sentinel branch, where lmdb-js's native writer substitutes the value from a per-environment register rather than this record's true prior version. Since getHistoryOfRecord (Table.ts) walks previousVersion backward to page through audit-log windows, a corrupted pointer breaks audit-history reconstruction.

Fix (revised after review): previousVersion: isRocksDB ? existingEntry?.version : existingEntry?.localTime. My first pass used existingEntry?.version uniformly, on the assumption that version and localTime are the same field for both engines — true for RocksDB (its local timestamp is explicitly forced to equal version at write time), but not for LMDB: lmdb-js assigns localTime via its own native monotonic clock, independent of the caller-supplied version. For a replicated/out-of-order write carrying a backdated origin version (clock skew, catch-up replay), the two diverge arbitrarily, and LMDB's audit store is keyed by localTime, not version — so a .version-based pointer doesn't resolve to any real audit entry. Both Codex's independent pre-push review and @kriszyp's review caught this; details in the review thread.

Refs #F-249 (qa-wave-2026072900)

Test plan

• unitTests/resources/auditLog.test.js:
  • "audit entries chain to the real prior audit-store key, not a placeholder flag" — 3 plain sequential writes; asserts each entry's previousVersion resolves via auditStore.get() to the true prior entry's own key. Fails on the original 0/1-flag code (decodes to a plausible-but-wrong float), passes with the fix, on both engines.
  • "previousVersion resolves correctly even when a write carries a backdated/replicated version" — explicit backdated timestamp:, confirms resolution through auditStore.get(). Fails against a .version-based fix (the bug this review round caught).
  • "getHistoryOfRecord walks the full chain for out-of-order writes spaced beyond the audit window" — backdated origin versions plus real delay(150) between writes so entries genuinely span multiple 100ms audit windows, asserted through the real production consumer (Table.getHistoryOfRecord). Reproduces the actual failure mode (truncates to 1 entry instead of 4) against a .version-based fix.
• npm run build, npm run test:unit:resources (both engines), npm run test:unit:main, npm run format:write, npm run lint:required all clean. The 10 pre-existing globalIsolation.test.js failures are present on origin/main too — unrelated to this change.

Review history

• Codex's first pre-push pass (sha a37ba38) flagged a blocker on the .version-based fix; I initially misjudged it as a false alarm (verified version/localTime equal in a non-explicit-timestamp scenario) and pushed anyway.
• Codex's second pass (sha a1ca662) and @kriszyp's review both independently confirmed the blocker with concrete repros using backdated timestamp: writes, which is what exposed the divergence. Fixed in b191a5c — see the resolved review threads and the PR comment for the full root-cause writeup.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request simplifies the assignment of previousVersion in RecordEncoder.ts by directly using existingEntry?.version instead of using placeholder flags. It also adds a unit test in auditLog.test.js to verify that audit entries correctly chain to the actual prior version. There are no review comments, so I have no feedback to provide.

[claude]

Reviewed; no blockers found.

[kriszyp]

Reply re: the LMDB key-space blocker (resources/RecordEncoder.ts:854) flagged by Codex and by @kriszyp's inline comment:

Fixed in b191a5c — previousVersion is now isRocksDB ? existingEntry?.version : existingEntry?.localTime (explicit per-engine branch, per Kris's note not to rely on version/localTime happening to be equal on RocksDB).

Also added getHistoryOfRecord walks the full chain for out-of-order writes spaced beyond the audit window in auditLog.test.js — backdated origin versions via explicit timestamp:, with real delay(150) between writes so the actual local-audit-store-key gaps genuinely span multiple 100ms windows. Verified it reproduces against the version-based fix (truncates to 1 entry instead of 4) and passes with the localTime-based fix.

Root cause, for the record: existingEntry.version is the record's application-level/origin version; LMDB assigns existingEntry.localTime independently via its own native monotonic clock (handleLocalTimeForGets copies only localTime onto the LMDB entry). The two are normally a few ms apart, but for a replicated/out-of-order write carrying a backdated origin timestamp they diverge arbitrarily, and the LMDB audit store is keyed by localTime, not version. My first fix used .version and only verified against natural/close-together writes, which is why it didn't catch this — Codex's and Kris's repros both used explicit backdated timestamps, which exposed it.

Signed, Claude Sonnet 5

[kriszyp]

Re: Codex's latest finding ("repeat writes to one LMDB key in a transaction now form sibling audit entries and silently misreconstruct history") — I reproduced this, but it's pre-existing on origin/main, not a regression from this PR.

Repro: transaction(ctx, () => { T.put(id, {name:'a'}, ctx); T.put(id, {name:'b'}, ctx); T.put(id, {name:'c'}, ctx); }) then getHistoryOfRecord(id) — on LMDB this returns only 1 of the 3 entries. I checked out resources/RecordEncoder.ts at exactly origin/main's content (the original store instanceof RocksDatabase ? existingEntry?.version : existingEntry?.localTime ? 1 : 0 line, unmodified) and got the identical result — 1 entry recovered instead of 3. Same test against RocksDB (both before and after this PR) correctly recovers all 3.

So this is a separate, pre-existing LMDB limitation around repeat-write-same-key-in-one-transaction chaining, not something this PR's previousVersion fix introduces or worsens — it behaves identically with or without this change. Given the dispatch scope is F-249 (the 0/1 placeholder-flag bug), I'm not fixing that separate issue here; I've filed it in the dispatch's Findings for triage rather than scope-creeping this PR.

Signed, Claude Sonnet 5

[Ethan-Arrowood]

Correct previousVersion fix, fully pinned by tests including the production consumer walk — and it removes an instanceof per write.

_{sent with Claude Fable 5}