Fix multi-worker lost counter increments and stale reads when a resequenced write reuses a version

[kriszyp]

Fixes the intermittent QA-431(5): N window(s) with lost counts under multi-worker stress failure in integrationTests/resources/ttl-rate-limiter-concurrent.test.ts — investigated and reproduced as a genuine product defect, not a test timing assumption.

Root cause

A resequenced (out-of-order CRDT) write stores its merged record under the version it merged onto rather than advancing it — the version is the max applied update timestamp and must stay that way for cross-node convergence — so one version can identify two different stored values. Every freshness decision keyed on version equality is then wrong:

1. Lost writes (the QA-431 failure). The commit path read its fold base through the record cache's version vouch (first attempt via the resource-phase read, coordinated-retry reloads via getEntry(key, {transaction})). The vouch answers "is this the latest committed version?" — the wrong question for a snapshot read, and wrong outright for a reused version — so an addTo/patch folded onto the stale pre-merge value and durably overwrote the concurrent increment it had merged over. A patch stores a full record derived from its base, so a stale base also silently resurrects old values of fields the patch didn't mention; a put's stale base corrupts index diffing, blob retention, and residency the same way.
2. Indefinitely stale reads. On a VerificationTable miss, the native layer re-confirms freshness against the version stored in the record itself and republishes it. For a reused version this re-vouches the stale holder's cache forever once every worker is warm — observed live as GETs pinned at 41–49/50 until TTL expiry while the store held exactly 50.

Fix

Harper already writes [8-byte BE version][4-byte BE metadata word] at the front of every primary record, and that metadata word is the header word rocksdb-js's VerificationTable reads at value offset 8 — its ACTION_32_BIT tag byte (14) is exactly the VERSION_HEADER_TAG the native predicate requires. So the whole cross-worker half of the fix is one durable bit:

• VERSION_REUSED is rocksdb-js's constants.VERSION_NOT_UNIQUE_FLAG, set in recordUpdater whenever a RocksDB record write does not advance past the version it replaces. Every record write funnels through that one place. rocksdb-js 2.8.0 then never answers FRESH for such a value and never publishes it to a slot, on every read path — sync, async, and both transactional ones (HarperFast/rocksdb-js#766).
• The JS record cache refuses to hold a flagged record and drops any copy it already had.
• New uncachedRead option on PrimaryRocksDatabase.getEntry: a plain transaction-snapshot read — no vouch trust, no VT seeding, no cache publish. Every record-write kind that derives stored state from its base (update, put, delete, invalidate, relocate — marked with an explicit reloadCommitBase flag) reloads that base at commit through it; bulk copy-apply rows and crash-recovery replays keep their pre-read base (one read per row, as before — their convergence contract is the post-copy/replay pass), and a cold read for a kind that keeps its own conflict semantics (publish/message/sourcedFrom resolve, and a replay's cold read) still takes the vouch fast path. Both previousResidency and created-time retention are derived from the reloaded base. Without the delete-path reload, a delete diffing indices from a stale base left phantom index entries for the deleted record.

Because the bit lands in a persisted record, a module-load guard fails fast if a future rocksdb-js moves VERSION_NOT_UNIQUE_FLAG onto one of Harper's own metadata flags, into the tag byte, or removes it — a silently-unset flag is a silent return of the data-loss bug; the message names the minimum rocksdb-js version it requires (>= 2.8.0).

This replaces the earlier JS-only compensation (a VERSION_UNVOUCHABLE sentinel parked in the VT slot by the writer, plus a verifyVersion probe on every warm read). That machinery — the writer-side park with its backoff/verify/resolve ladder, the reader-side sentinel checks, and the residual re-vouch race it could not close — is entirely gone.

Consolidated with #2065

main independently landed an overlapping fix for the same bug (PR #2065, merged 2026-08-29, same day this PR was opened): it set the identical VERSION_NOT_UNIQUE_FLAG bit, with the identical guard predicate, in the same recordUpdater() function, for its own getFromSource source-fill write path. This branch's generic in-recordUpdater fix already covers that call site (every write, including source-fill, funnels through the one patched function), so the rebase consolidated onto this PR's implementation as canonical — broader (every write, not just source-fill), sourced from rocksdb-js's own constants export instead of a hardcoded local value, guarded against the constant drifting out of Harper's reserved bit range, and kept out of assignMetadata/the audit extendedType bits that assignMetadata also feeds. #2065's redundant inline block and its two duplicate-intent tests were folded into this PR's VERSION_REUSED mechanism rather than dropped; #2065's disjoint Table.ts source-version capping/tie-breaking changes are unrelated to the flag and carried through untouched.

Verification

• Red on unpatched main: amplified QA-431(5)-shaped repro (same fixture, in-burst GET pressure, taskset to 4 CPUs) lost increments in 3–15 windows per 900, with a per-vouch audit proving the losses durable. The bit is load-bearing on 2.8.0: with metadataInNextEncoding |= VERSION_REUSED disabled and everything else in place, unitTests/resources/caching-rocks-database.test.js fails at VT must not vouch for a version shared by two stored values — the native layer vouches the reused version. Both new unit assertions (flag set, refusal to vouch) fail without it, and a client-level read proves a caller sees the merged value rather than a stale vouch.
• Green with the fix: ttl-rate-limiter-convergence.test.ts — 100/100 windows exact, zero converged-late, zero stuck-short, zero over-count, run four separate times across this rebase (twice pre- and twice post- the round-12/13 fixes below); the unmodified ttl-rate-limiter-concurrent.test.ts 5/5 including QA-431(5).
• Gates on this rebase (onto main@5cd0e2c12, origin/main unmoved since): npm run build clean; npm run test:unit:resources — 2020 passing / 0 failing / 29 pending (2 timeout-based failures in an unrelated branchDatabase.test.js suite were box-load contention from other agents' concurrent runs on this shared machine, not this change — confirmed by an isolated re-run: 70/70 passing); unitTests/resources/caching-rocks-database.test.js + caching.test.js 47/47 passing.

For the human reviewer

• The one open major, and it is yours to call: a reloadCommitBase write still pays a second point read + decode of its base at commit time, on both storage engines — LMDB pays it despite having no VerificationTable defect to dodge. Now that the native layer refuses to vouch for a reused version, independent review (codex, twice across rounds) argues the reload is redundant belt-and-braces — the cheaper shape proposed is to make the existing resource-phase pre-read snapshot-direct and reuse it, one read instead of two. I kept the reload because the pre-read's base can also arrive from this.#entry, which may have been populated by an application get() well outside the committing transaction, and narrowing that correctly touches three write paths; reversing it later is one predicate change plus a re-run of the convergence suite to prove the flag alone holds.
• CI is currently red, but the evidence points at npm-registry conditions today, not this diff: Unit Test (Node.js v22) hit its 10-minute job timeout twice (push and a manual re-run), stalled inside install_node_modules's real npm install subprocess tests. More tellingly, all three Next.js adapter integration jobs (Node 20/22/24) independently hit their 30-minute timeout stalled on "Install fixture dependencies" — a plain dependency install in a downstream fixture repo that never reaches Harper's code at all, so it cannot be this diff's write-path cost. That's the same failure shape (an npm install step stalling) recurring across unrelated jobs and repos in the same CI run, which reads as today's npm registry/network conditions rather than a regression. I did not find a clean way to fully rule out a smaller, real contribution from the reload's added write-path cost — the completed unit-test jobs on this head (8m53s for v24, 9m53s for v26) ran somewhat slower than the same job on the same-day main baseline (8m8s) — but given the much larger, code-independent stalls alongside it, I'd re-run CI once it's confirmed to have settled down before reading anything into that difference. Recommend re-running the two failing checks before merge rather than merging on this state.
• New, from this rebase's independent review — a real gap, not fixed here: the reload's safety only holds when the committing transaction holds a snapshot for RocksDB's optimistic conflict validation to check the later Put against (verified against rocksdb-js's binding: SetSnapshot() is gated on !disableSnapshot on every Get/Put/Delete path). Harper sets this.snapshotFree = true after a mid-scope-commit rotation (a scope that commits more than once), and a snapshot-free transaction gets no such validation — the reload narrows the lost-update window to the read-to-put span instead of closing it (see the comment at DatabaseTransaction.ts around the reload). Closing it needs either a rocksdb-js API to force a snapshot on one read of an otherwise snapshot-free transaction, or having Harper skip the snapshot-free rotation for a scope that will do a reloadCommitBase write — both bigger than this PR. The primary defense (the persisted VERSION_REUSED flag + native VT refusal) does not depend on this reload and is unaffected.
• Design choice: compensating for "one version, two values" with a durable non-uniqueness bit rather than advancing the version on resequenced folds — advancing would fabricate a timestamp no real update carries and break cross-node CRDT convergence.
• Durable uncacheability: a flagged key stays uncached/unvouched until its next in-order write rewrites it — a caching cliff for resequencing-heavy tables (e.g. replicated counters with clock skew) that no metric currently surfaces. The predicate flags ties (newVersion <= existing) too, so same-timestamp deletes and relocates that deliberately hold their version are flagged as well.
• Load-time throw: the flag-range guard crashes startup rather than degrading if rocksdb-js ever ships without the constant. That is deliberate — the failure it prevents is silent data loss — but it is a hard dependency on 2.8.0+.
• Test-oracle tolerances (declined, repeatedly flagged in review): the convergence oracle counts acked === 0 windows as inconclusive with a 50% floor, admits convergence within a 400 ms poll, and treats hits <= acked + errs as exact-or-explained. Each absorbs a real failure it could otherwise catch. It is deliberately tolerant so the test is not flaky on loaded CI runners; a deterministic cross-worker regression test (including a tombstone that reuses a version, which nothing covers today) would be strictly better and is not in this PR.
• validate() runs once: created-time retention now reads the reloaded base, but validate() is not re-run on a conflict retry, so a retry that reloads a newly created base still keeps the first round's decision. Pre-existing structure, not introduced by this PR; re-running validate() on retry would also re-run schema validation and audit dedup, which is its own change.
• Write kinds outside the reload set: publish/message writers and sourcedFrom cache-fill resolves keep their own conflict semantics and are not marked reloadCommitBase — worth a follow-up look rather than widening this PR.
• Rebased onto current main, consolidating with Fix sourcedFrom cache-fill conflict convergence #2065 (see above) — main's overlapping flag logic and two duplicate-intent tests were folded into this PR's mechanism rather than kept side-by-side; main's disjoint source-version capping/tie-breaking changes in the same files carried through untouched. Independent review caught and this rebase fixed two real regressions the consolidation introduced: (1) the merge reordered metadataInNextEncoding = assignMetadata ahead of the VERSION_REUSED OR instead of after it, so the documented assignMetadata = -1 default silently dropped the metadata word — and the flag with it — for any resequenced write reaching recordUpdater through that default (latent in-repo, but live via the function's direct callers); restored the dropped Math.max(assignMetadata, 0) normalization main's replaced line had. (2) A comment overclaimed the snapshot-free conflict guarantee described above — corrected instead of left wrong. Also strengthened two tests independent review flagged as gaps: the integration test's 30s readiness wait now throws instead of silently falling through on timeout, and the reused-version unit test now asserts a client-level read (not just internal verifyVersion/metadataFlags state).

Refs #1881 (same version-reuse family, index-scan surface).

Signed: Claude Sonnet 5 (dispatch agent)

🤖 Generated with Claude Code

_{Review-Coverage: authored=claude; ran=codex,gemini; declined=cursor-grok,cursor-composer,domain; rounds=14 @ 222ffae}

_{Human-Review-Need: 4 @ 222ffae}

[claude]

Reviewed; no blockers found.

[gemini-code-assist]

Code Review

This pull request introduces a mechanism to handle version-reuse cache staleness in RocksDB-backed tables. It prevents the Verification Table (VT) from vouching for reused versions (where a resequenced write keeps an existing version, meaning one version could identify multiple different stored values) by marking such records with a VERSION_REUSED metadata flag, parking a VERSION_UNVOUCHABLE sentinel in the VT, and introducing an uncachedRead option to bypass the cache vouch on critical commit paths. The changes also include comprehensive integration tests, unit tests, and a benchmark script to measure the sentinel probe overhead. There are no review comments provided, so I have no feedback to address.

[cb1kenobi]

Reviewed ec607a1 and found no blocking issues. The version-reuse flag and snapshot-direct commit-base reads consistently address the stale-cache path. No confirmed blocking defects were found on changed lines.

—
Generated by Barber AI

[heskew]

Reviewed exact head ec607a1.

No blocking defects introduced by this PR. Codex independently traced the VERSION_REUSED contract through Harper encoding and rocksdb-js VerificationTable behavior, the uncached transaction-snapshot reload of state-deriving commit bases, same-key staged-write ordering, replay and copy-apply exclusions, and the published/pinned rocksdb-js v2.8.0 dependency. The paths are internally consistent and address the stale-read/lost-increment failure without exposing a new correctness or security issue.

Prior-review attribution: @gemini-code-assist reviewed an earlier head and reported no findings; @cb1kenobi independently found no blockers at this exact head; @claude likewise reported no blockers at this exact head. I agree after the independent pass above.

Validation at this head: npm ci and npm run build passed; the focused PrimaryRocksDatabase tests passed 13/13; changed-file oxlint and Prettier checks passed; the focused QA-431 four-worker convergence test finished 100/100 clean windows with zero stuck-short, over-counted, or inconclusive windows; and GitHub CI is green. A full local resource-suite process aborted late after hundreds of passing tests, but its isolated active suite passed and all GitHub unit-test matrices passed, so I do not attribute that local runner failure to this PR.

Branch state only: the branch currently conflicts with main in resources/RecordEncoder.ts and unitTests/resources/caching-rocks-database.test.js. It must be updated before merge, but this is not a correctness finding against the reviewed head.

🤖 Posted by Codex on behalf of @heskew