fix(table): make TTL eviction path errors non-fatal on both storage engines (#1287)

[kriszyp]

Fixes #1287.

Summary

TableResource.evict() runs fire-and-forget from the record-expiration sweep, the read path, and the concurrency-limited cleanup scan. Two engine-specific defects broke the eviction path under multiple workers (neither reproduced single-worker):

• LMDB: evict() returned DatabaseTransaction.commit()'s plain {txnTime} object. The optimistic writes go straight to the store, so the wrapper transaction has no tracked writes and commit() is not a promise. The cleanup scan then did resolution.catch(...) and threw TypeError: resolution.catch is not a function every cycle, aborting the scan — physical eviction never kept up and the store grew unbounded (lazy-expiry-on-read kept queries correct throughout). evict() now returns the store's write promises (Promise.all([index-cleanup, removal])), so callers get a real thenable that resolves once the removal is durable.

• RocksDB: evict() returned the raw RocksTransaction.commit() promise, which bypasses DatabaseTransaction's ERR_BUSY retry and rejects on a concurrent-write conflict. Two fire-and-forget callers let it escape as an unhandledRejection on [main/0], exiting the process. evict() now swallows the conflict (eviction is best-effort) and aborts the open transaction handle — matching the batched-eviction path's commit-failure handling.

evict() now always returns a non-rejecting thenable on the commit path. Skip/no-op paths keep returning undefined (all callers guard with if (resolution)).

Where to look

• resources/Table.ts — the static evict() method. The RocksDB .catch both swallows ERR_BUSY and calls abort() on the raw transaction; confirm that abort-after-failed-commit is the right lifecycle (it mirrors commitItems in the same file).

Tests

• unitTests/resources/evictionLmdb.test.js — asserts evict() returns a thenable and physically removes the record + index entry (LMDB; runs under test:unit:lmdb). Fails on the exact resolution.catch is not a function signature without the fix.
• unitTests/resources/evictionBusy.test.js — injects ERR_BUSY into the eviction commit and asserts evict() resolves rather than rejecting (RocksDB).

Review notes

A cross-model review (Codex + Gemini + Harper-domain pass) ran on this change. All actionable findings were addressed in the diff (the RocksDB transaction-abort and the captured LMDB index-cleanup promise both came from that review). No open concerns remain; the one item left for the human reviewer is the abort-after-failed-commit lifecycle call-out above.

🤖 Generated by Claude (Opus 4.8) via the cross-model-review + harper-engineering-guidelines workflow.

[gemini-code-assist]

Code Review

This pull request improves the evict() method in resources/Table.ts to ensure robust promise handling and error swallowing for both LMDB and RocksDB storage engines, preventing unhandled rejections and TypeError crashes during eviction sweeps. Regression tests have been added to verify these behaviors. The review feedback suggests removing a redundant optional chaining operator (?.) on transaction.abort() since the method is guaranteed to exist.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[claude]

Reviewed; no blockers found.