[kriszyp]

⊙ Problem

On RocksDB, Table.evict() could delete data it should have kept:

• Renewed records. On tables with no audit log and no source, it skipped the version check. A stale eviction deleted the renewed record, left its new index entries behind and unlinked its blob files.
• Blobs of surviving records. It queued blob files for deletion before committing. When the commit failed on a conflicting write, the record survived but its blob file was deleted.

🤔 Is the evict-only fix the right scope?

Yes, as the task specified. _writeDelete and updateRecord still delete blobs before commit (#2861, #3367). The shared fix belongs in removeEntry/DatabaseTransaction, not in each caller.

💡 Solution

• evict() reads the record in its own transaction and skips any version except the one its caller read, on every table.
• On RocksDB it deletes blob files only after the commit succeeds, keeping any file a newer record still uses. Index cleanup uses the record it read.
• LMDB keeps its conditional writes. Evictions without blobs skip the new step.

⚖️ Alternatives

• Make removeEntry wait for the commit on every RocksDB transactional delete. The root-cause fix for Aborted delete leaks blob bytes: removeEntry's unlink gate keys off the removal's own commit, not the outer transaction #2861, also covering _writeDelete and the batcher; it changes shared delete behavior, so it is a separate PR.
• Make rocksdb-js remove() resolve at commit. Out of scope, and Harper calls removeSync for these deletes anyway.

🤔 Accept the narrower race that post-commit deletion keeps?

Deleting after commit prevents blob loss on commit conflicts. A narrower race remains, shared by LMDB eviction and source-miss removal: a write that re-encodes the same blob between commit and cleanup can lose its re-reference flag. The fix belongs in blob.ts (#3382).

🤔 Leave reused versions to #3384?

A resequenced write can reuse the version the caller read, and the gate still evicts that record, as on main and in the batcher. Refusing reused versions changes when expired records leave, so it is separate.

🔧 Changes

• resources/Table.ts evict(): the version gate now runs on every table, and the entry ?? getEntry(id) fallbacks that read the live record are gone. Both engines index from the entry it read. ⚠️ Look hardest: the RocksDB branch stages removeSync and deletes blobs only from the commit's success handler; evictions without blobs keep the rejection-only path. The cleanup-scan comment now says why blob records stay on the per-record path.
• resources/DESIGN.md: the blob-deletion note now lists RocksDB evict() with the paths that delete after commit, and records the unconditional version gate.

✅ Verification

• New unitTests/resources/evictionBlobs.test.js runs on an audit: false table with no source, an index and file-backed blobs.
• The renewed-record and conflicted-eviction tests fail on base e187b83d2 (RocksDB: "the renewed record must survive"; "an abandoned eviction must not queue the competing write's blob for reclamation"). On LMDB the first passes on base and with the fix; the others are skipped on LMDB.
• Local test:unit:resources: LMDB 3413 passing; RocksDB 4512 passing, with 2 load flakes (replicationSeqCursor, subscriptionReplay) that pass alone. test:unit:main and test:integration:all could not finish locally next to 7 other suites, so CI ran them: all 28 unit and integration jobs (Linux, Bun, uWS, Windows) passed at ffbd495, which has the same production code. At 2a20c57, three unrelated flaky jobs were rerun.
• End to end: the tests drive the production Table.evict() against real stores, indexes and blob files. A running server cannot order these interleavings deterministically.
• PR QA: skipped @ 2a20c57 — dispatch unavailable (no worker token available to this agent).

Test cases

• Renewed record: the record, its index entry and its blob file survive a stale eviction. A later eviction of the live version still removes them.
• Conflicted eviction: the commit is ordered to fail with ERR_BUSY. The blob file and its re-reference flag survive.
• Refill before cleanup: a write committed between the eviction's commit and its cleanup keeps the blob file. Removing collectRetainedFileIds makes this test fail.
• Resequenced write: a write that reused the read version changed an indexed field. The eviction leaves no index entry behind. Indexing from the caller's copy makes this test fail.

Refs #2861

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

Release target: v5.4 — the milestone of #2861; v5.3 has the same code, but no known need for a backport
Related PRs: #2155 overlaps (rewrites the same evict() version check and removal), #3375 overlaps (adds a replica branch inside evict()), 31 others independent
Complexity: complicated

Deferred findings

• 8c7a0517e9e70382 minor data-integrity — resources/Table.ts — cleanup-scan HAS_BLOBS bypass untested; record gaining blobs before batch commit is reclaimed at staging — filed RocksDB eviction removes a resequenced record that reuses the version its caller read #3384
• 3e48438599eee7ae minor data-integrity — resources/Table.ts — post-commit eviction reclamation erases a concurrent writer's blob re-reference flag — filed Post-commit blob reclamation can clear an in-flight write's re-reference and unlink a live blob #3382

_{Dispatch: task harper-evict-rocksdb-version-gate-blob-commit · queued by unknown · ran by claude/opus/high · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; blocked=cursor-grok(failed); declined=cursor-composer,cursor-kimi,cursor-muse,muse; rounds=6; full=1 @ 2a20c57}

_{Review-Attention: study ~15m (critical: Table.ts; decisions: per-caller-post-commit-reclaim, reclaim-on-commit-not-durability, version-gate-reused-version) @ 2a20c57}

[gemini-code-assist]

Code Review

This pull request updates the record eviction logic (evict()) in resources/Table.ts to ensure that blob files are only reclaimed after the eviction transaction successfully commits, preventing premature deletion in case of conflicts or abandoned evictions. It also updates resources/DESIGN.md to document these changes and adds a comprehensive test suite in unitTests/resources/evictionBlobs.test.js. The review feedback points out a style guide violation in the new test file, where Node.js built-in modules (fs, crypto, and timers/promises) should be imported using the node: prefix.