[kriszyp]

⊙ Problem

On RocksDB, some writes keep a record's version: an out-of-order write, a revalidation whose source reports the same version, a replica store. They set VERSION_REUSED and usually renew the expiry. Table.evict() and the cleanup scan's eviction batch read "same version" as "unchanged", so an eviction that had read the expired copy deleted the renewed record, its index entries and its blob files. A batch that lost its commit could also unlink a kept record's blob.

🤔 Is this the right scope?

As specified, yes. It answers #3384's open question (the merged record does not keep the old expiry) and closes the #3413 blob deferral. An older out-of-order write overwriting the newer record's expiry is a separate defect and stays out.

💡 Solution

• On a reused version, eviction also requires the expiry the caller read. Renewed records survive; unchanged ones are still evicted.
• evict() takes that expiry as an optional last argument; internal callers pass it.
• The batch skips a record that gained a blob or became a replica row since the scan, leaving it to the next scan's per-record path.

Fixes #3384

⚖️ Alternatives

• Give same-version writes a new version — the RocksDB version is also the replicated ordering token.
• Keep the older expiry on same-version writes — a revalidation renews legitimately, and the blob case remains.
• On a reused version, evict only if expired — pressure eviction targets unexpired records. A cache revalidating under a stable source version could not shed load; certificate verdicts without expiry would never be evicted.

🤔 Should an omitted expiry block eviction of any reused version that has one?

No: omitting the expiry keeps version-only eviction. Callers that omit it evict what they just read (tests, one integration fixture). A stricter default would silently make them no-ops. Every internal caller with a gap between read and eviction passes the expiry. Changing this is one comparison.

🔧 Changes

• resources/Table.ts, evict(): new optional observedExpiresAt on the interface and the method; ⚠️ Look hardest: on a reused version a changed expiry skips the eviction. This check precedes TTL-triggered replica demotion. Replica-sweep calls skip it; their deadline validates them. The shared rule is reusedVersionChangedExpiry.
• resources/Table.ts, eviction batch: items carry the scanned expiresAt (stored on add); staging applies the same rule and re-checks HAS_BLOBS | REPLICA before removeEntry, which queues blob reclamation at staging.
• resources/Table.ts, callers: the read-path expiry check, the cleanup scan (batched and per-record) and the @expiresAt sweep pass the expiry they read.
• security/certificateVerification/verificationTables.ts: passes null, since those verdicts are selected for having no expiry.
• resources/DESIGN.md: the rule and where it is enforced.

🤔 Should the @expiresAt sweep compare the field it decides on?

It passes the metadata expiry used to hide expired records from reads. Checking the field belongs with enabling that sweep on RocksDB (#1481): it stops before evict() there, and only RocksDB sets VERSION_REUSED.

✅ Verification

• 8 new RocksDB tests in unitTests/resources/evictionBlobs.test.js fail on a fresh build of origin/main sources and pass with the fix (5 runs in a row). They drive the production callers and the scan → batch → commit path, injecting only the race.
• RocksDB resources 5025 passing on the current head; LMDB resources 3709 and integration 2760 before the rebase (failures below).
• End-to-end: not observable without injecting the race between a caller's read and the eviction.

🤔 Are the test helpers' comments worth keeping?

Yes. Each states a fact the body does not show: resequence must add a key the record lacks (older writes to existing keys are dropped); scanReadsBefore feeds only the next pass.

New tests and gate details

New tests (RocksDB; LMDB has no reused versions and skips them):

• evict() keeps a record a resequenced write renewed from the table expiration; evicting with the current expiry still removes it and reclaims its blob.
• evict() with null treats a gained expiry as a change; with no argument, only the version is checked.
• evict() keeps a cached record a revalidation renewed under an unchanged source lastModified.
• The read-path expiry check and the cleanup scan's per-record path pass the expiry they read (a same-version write is committed before evict()'s own read).
• The batch keeps a record renewed before staging and evicts a reused version whose expiry stayed unchanged.
• The batch keeps a record renewed between staging and commit (real ERR_BUSY, the retry re-applies the rule).
• An abandoned batch (two targeted ERR_BUSY) keeps the blob of a record that gained one at the same expiry; the re-reference signal shows no reclamation was queued.

Gate failures, none in code this PR touches:

• test:unit:resources before the rebase (RocksDB 1, LMDB 3): subscribeReadGateCallerImpact.test.js socket?.off is not a function, fixed on main by Give the read-gate WebSocket test's fake socket the off() REST calls on close #3562; LMDB replicationDirectiveAuthorization.test.js, LMDBTransaction has no replicationConfirmation export on main.
• test:integration:all (6): 4 Ollama tests (no local models); put-operation and ttl-rate-limiter-concurrent timed out under load and pass on rerun (25/25).
• test:unit:main did not finish in 55 minutes on a box at load ~22. Its 22 failures were all in ComponentV1.test.js, which passes alone (35/35). unitTests/security: 926 passing.
• @expiresAt sweep wiring is not behavior-tested: on RocksDB the sweep stops before evict() (@expiresAt per-record TTL never evicts on RocksDB (default engine) — index.getValues() is LMDB-only #1481), and LMDB never sets VERSION_REUSED.

PR QA: Do renewed cache records survive eviction while several workers read and revalidate them? — queued @ 381e3b3, qa-pr-harper-3575-381e3b3
Docs: none needed — evict()'s new argument is internal and undocumented.

_{🤖 Generated by Claude Code (Claude Opus 5.5); posted via @kriszyp.}

Release target: v5.4 — builds on the eviction fixes in #3413 and #3471, both milestoned v5.4 and not on v5.3
Related PRs: #2155 overlaps (also changes the evict() signature and the @expiresAt sweep call), 32 others independent
Complexity: complicated

Deferred findings

• 121445d3b552eb4f nit data-integrity — resources/Table.ts — @expiresAt sweep decides on value[expiresAtName] but passes metadata entry.expiresAt as observedExpiresAt — NOT FILED
• 12a199780137ad7b nit data-integrity — resources/Table.ts — omitted observedExpiresAt keeps version-only eviction for callers that await between read and evict — NOT FILED

Late Grok findings

3 surviving (highest minor) — Grok reviewed 8344eb9

• minor — resources/Table.ts:11011 — The @expiresAt index sweep decides to evict from the record's field (value[expiresAtName], :11011), but it passes metadata entry.expiresAt as observedExpiresAt (:11018). Failure case: a resequenced (out-of-order merged) patch keeps the same version, sets VERSION_REUSED, moves the field
• nit — unitTests/resources/evictionBlobs.test.js:240 — Added test comments at :213, :223, :240, :399, :421, :430. Most explain facts the code does not show (the retry count, how re-reference is cleared), but :213/:240 restate the helper's purpose. Trim to the ones that carry non-obvious facts
• nit — unitTests/resources/evictionBlobs.test.js:181 — Every new test produces VERSION_REUSED through a resequenced patch; none goes through the 304-revalidation or replica-store producer. The guard does not care which producer set the flag, so this is a suggestion, not a missing-branch gap

Not standing: 1 dropped-factually-wrong, 1 merged. Grok ran 21m (21m from launch to report); the fold adjudication (cursor-grok@8344eb95d326-c0761ddd, claude) took 1m.

Origin — the dispatch brief this PR was written from

Fixes #3384 (answers its 'Not proven' item) plus the #3413 blob deferral. Table.evict checks only entry.version !== existingVersion, and createEvictionBatcher.stageInto checks only version and value == null, then calls removeEntry at staging time without a HAS_BLOBS guard (the scan filters HAS_BLOBS|REPLICA only at scan time). An out-of-order write at the scanned version reuses the version (VERSION_REUSED) and _writeUpdate stamps a fresh future expiresAt, so eviction deletes a live record. In evict and stageInto re-read the entry in the eviction's own transaction and re-check the entry's own expiresAt against the threshold the scan used (skip a renewed record); in stageInto skip a record that now has HAS_BLOBS, or move blob reclamation to after commit as evict does. Leave the separate 'out-of-order write overwrites the head's expiresAt' defect to its own issue.

Acceptance

Tests on rocksdb: out-of-order write at the scanned version onto an expired record, then evict(id, R1, v1) and a batcher drain -> record, its index entry and its blob file survive; a blob appearing between scan and drain with a forced ERR_BUSY -> blob file survives; normal expiry eviction unchanged. Show each new test failing on origin/main sources (forced rebuild) before the fix.

_{Dispatch: task harper-evict-version-reused-guards · queued by finding-triage/wave-40 · ran by claude/opus/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex,cursor-grok; late=cursor-grok:adjudicated; adjudicated=domain; declined=muse; rounds=5; full=1 @ c41f97f}

_{Review-Attention: study ~20m (critical: Table.ts; sensitive: verificationTables.ts; decisions: expiry-as-reuse-discriminator, evict-tristate-observed-expiry, batch-skip-not-reroute, do-less-alternative) @ c41f97f}

[gemini-code-assist]

Code Review

This pull request updates the evict method and related eviction logic to correctly handle record renewals that occur between an eviction read and its commit. By introducing an observedExpiresAt parameter, the system can now detect if a record's expiry has changed due to a concurrent same-version write (e.g., revalidation or patch), preventing the accidental deletion of renewed records. The reviewer identified that the initial implementation incorrectly passed undefined for expiresAt in certain code paths, which would bypass the safety check; the suggested fixes ensure that expiresAt is correctly passed as null when undefined, maintaining consistency with the intended logic.