Make an audit entry's field offsets independent of its previousVersion value (harper#2247)

[kriszyp]

The LMDB audit entry announces its optional leading 8-byte previousVersion field with that field's
own first byte being 0x42. The writer instead emitted the field for any truthy value, so a
previousVersion outside [2**33, 2**49) was written and then skipped by every reader, leaving the
entry 8 bytes longer than the reader believed. action, nodeId, tableId, recordId and
version then all parsed from the wrong offsets, with no corruption detector on the path.

That leading-byte test is a cross-version contract, not a local convention. It is how harperdb
4.7.36's reader decodes the field, and it is how both versions' replication senders decide to strip
the field before framing an entry for the wire (encoded[0] === 66 ? 8 : 0), after which the
receiver hands the raw frame back to this same readAuditEntry. Three of the five implementations
are outside this repo. That is why presence is enforced at the value here rather than restated as a
header flag: the only word that could carry a flag is the action word, and it sits after the
optional field, so a reader cannot consult it before deciding whether to skip. Moving the header in
front would make an unmodified sender stop stripping and forward the field to a v4 reader that
parses the entry 8 bytes off — the same defect, relocated onto a shipped version.

What changed

The writer derives presence from the byte it actually encoded and uses that one boolean for both the field and the action offset, so the two can no longer
disagree. A value that cannot announce itself is rejected rather than written. Absence is now stated explicitly (undefined, null, 0) instead
of inferred from truthiness, which is what stops NaN from being silently dropped.

The reader recovers entries already written the old way, including by a v4 peer: when the first byte can begin neither an action nor a previousVersion,
the field is physically present and the header follows it. The set of bytes that can begin an action is derived from what the encoding can express, so a reserved entry type minted by a newer peer
still decodes. Recovery is gated on the LMDB container, on a candidate the superseded writer could
actually have emitted (> 1, its own guard), and on the byte at offset 8 being a legal action start.
Recovery restores the field offsets; the recovered value itself is dropped. It cannot lead with
0x42 by construction, and audit keys carry the same constraint, so it could never have addressed a
retrievable entry. A recovered entry hands a sender the bytes without the stray prefix, so the same misparse is not passed to the next hop. Anything else that cannot begin an entry now returns the existing corrupt
sentinel instead of decoding at the wrong offset.

The previousVersion <= 1 placeholder branch is gone. lmdb-js substitutes previousTime ^ 0x40 at
commit, which is 2.0 whenever no previous time was recorded — that is precisely the poisoned entry
in the byte-level capture on HarperFast/harper-pro#737 — decoded correctly by this reader — so the branch could not satisfy the invariant even in principle.

RocksTransactionLogStore is untouched and keeps its unconditional write, because its uint32 prelude
flag already states presence there, making its value unconstrained.

For the human reviewer

Three judgment calls worth your attention, in order.

1. The writer throws. An unrepresentable previousVersion now aborts the audit-entry encode, and
that encode runs inside a user write. I chose this over silently omitting the field because
previousVersion is the record-history back-edge, so dropping it would trade this bug's silent
mis-decode for a silently truncated history — the same class of defect. Every producer that can
reach the LMDB container was enumerated and is already in range, so the throw should be unreachable:
RecordEncoder passes a decoded localTime, which always leads with 0x42 because getTimestamp
XORs 0x40 into a byte that had to be 0x02; the other site passes a value that came back out of
readAuditEntry; and harper-pro's two call sites pass null. If you would rather this degrade than
fail, it is a two-line change.

2. PENDING_LOCAL_TIME does not throw, it drops the link. This is the one place the change can
lose something real. previousVersion === 1 is the
"previous entry has no log position yet" sentinel, and RecordEncoder feeds existingEntry.localTime
straight through on the LMDB path with no pending guard, so throwing there could abort a real write.
The entry is recorded without a back-edge and warns with the producer stack. This is the one place I
accept losing a link, because a format whose presence signal is the value's own first byte cannot
express "to be filled in at commit". The pre-push review caught this; I had originally removed the branch outright.
The review asked twice what this costs, naming the concrete case: two writes to one key inside one
transaction. I measured it rather than arguing it, and my first attempt was wrong in a way worth
recording — two separately awaited puts commit independently, so they never reach the pending path.
With both writes in one transaction the second entry carries no back-edge, and origin/main
produces the identical result on the same scenario, because the first write of a transaction has
not published a localTime for the second to point at. So this change does not alter that path. The test in the diff pins what does matter there: neither entry misparses. I have not found a producer
that reaches the pending branch, which leaves it defensive; if one exists, it loses a link there.

3. Recovery discards the value it finds. An earlier revision preserved it, on the reasoning that
a real link should never be dropped. That was wrong in a way worth recording: RecordEncoder's
resolveRecord branch re-mints an audit entry from replacingEntry.previousVersion, so a preserved
out-of-band value flowed straight back into createAuditEntry, which now rejects it — and it does so
after the primary store.put has been issued, so the user's write fails and the record change can
commit with no audit entry. The value bought nothing anyway, since it could never address a
retrievable entry.

What this does not do. It does not fix the recurring red on harper-pro main
(replicationTopology.test.mjs, "Replicate larger v4 dataset"). In run 33709678114 every frame of
the RangeError: ... cannot be converted to a BigInt stack is under
/tmp/harperdb-legacy/node_modules/harperdb/, thrown from the legacy node's own recordId getter
inside its own audit-forwarding loop, which then closes its subscription. The mint, the misparse and
the throw all happen inside harperdb 4.7 before any v5 code runs. That trigger was addressed by
harper-pro#740; this PR closes the format hazard it left open.

Coverage gaps, stated rather than papered over. The RocksDB suite mirrors
RocksTransactionLogStore's prelude encode/decode rather than driving the real store, so it pins the
contract, not the store; the existing suites cover the real store on every audit write. No in-repo
test exercises a real replication sender or a v4 receiver, so the claim that a cleaned entry forwards
correctly is argued from the byte layout, not observed. The rejection test proves the audit value is
absent, not transactional atomicity with the primary record write.

Deliberately not fixed here. The audit key codec makes the same value-shaped inference. Its
writeKey runs on every audit key, so a warn-only guard would add per-key work without preventing an
unreadable key, and actually re-encoding keys would change key ordering and every key already
written. Worth its own issue.

Framing: Framing-Verdict: better-alternative-exists (round 1) → adopted → chosen-approach-sound
(round 2). Round 1's alternative was fail-closed enforcement at the codec boundary plus exact-action
legacy recovery; I rewrote the design around it rather than overruling.

Verification

Fails-on-base, with dist/resources/auditStore.js deleted and a full tsc before each run (the
built artifact was checked for previousVersion > 1 before the red run and for the new guard before
the green one):

origin/main source + new tests   16 passing, 19 failing
this branch      + new tests     35 passing,  0 failing

The 19 red failures are the writer-rejection cases, the legacy-recovery cases including the
harper-pro#737 hexdump, the encoded/size normalization case, and both real-LMDB cases.

The import-cycle regression test spawns a child node process that requires RecordEncoder first,
because the cycle only bites when it is the entry point and that order cannot be arranged in-process
once mocha has loaded both modules. Reproduced before the fix: under CommonJS the flag mask silently lost a bit and a valid recovery was
rejected as corrupt, decided only by load order. The mask is now built on first use, and the test spawns the child process.

Gates, run under an isolated HOME so another worktree's suite could not contend for the shared
system database (that contention, not this diff, produced the failures in my earlier runs):

leg	result
unitTests/resources/auditLog.test.js	77 passing (lmdb) / 76 (rocksdb), 0 failing
test:unit:resources (rocksdb)	2043 passing, 0 failing
test:unit:resources (lmdb)	1659 passing, 2 failing
test:unit:bin (lmdb)	274 passing, 0 failing
test:unit:main	5290 passing, 1 failing

Both non-green cells are pre-existing and were confirmed against origin/main sources rather than
assumed. The two lmdb failures are subscriptionReplay's "count: empty initial state" and "count:
only other-table records exist", each a 2000ms waitFor timeout; that file passes 36/36 when run
alone on this branch, and the same two fail on origin/main in the same full-leg run. The
test:unit:main failure is configValidator's domain-socket path-length test, which resolves a
relative rootPath against process.cwd() and so fails in any deeply nested worktree.

Reader cost was measured baseline vs patched over a 2000-entry decode scan, interleaved in one
process. The result is below the measurement floor on this machine: the same binary against itself
varies by about 23% run to run, and the measured delta swings between -12.8% and +4.9% across runs.
Structurally the added work is one 256-byte table load, and it runs only for entries that carry no
previousVersion, since a 0x42 first byte takes the earlier branch.

Refs #2247

🤖 Generated with Claude Code

https://claude.ai/code/session_01KLaWQDpmKH9qXfMbuMbWWB

_{Review-Coverage: authored=claude; ran=codex; adjudicated=domain; declined=gemini,cursor-grok,cursor-composer; rounds=6 @ 3cb5b71}

_{Human-Review-Need: 3 (decisions: parity-with-main-as-sufficient-evidence, pending-link-dropped-vs-deferred, throw-vs-warn-on-unrepresentable, reader-side-recovery-vs-migration, encoded-strips-prefix-silently, format-band-documented-as-permanent) @ 3cb5b71}

[gemini-code-assist]

Code Review

This pull request addresses issue #2247 by enforcing representability constraints on the previousVersion field in the LMDB audit format, ensuring only values within the representable range [2^33, 2^49) (which lead with the 0x42 byte) are written. It also introduces a recovery mechanism in readAuditEntry to safely parse legacy entries written before these constraints were enforced, updates the storage format documentation, and adds comprehensive unit tests. There are no review comments, and I have no feedback to provide.