Add Table.oldestRetainedAuditTime(), so a consumer resuming an audit cursor can tell a complete catch-up from a silently truncated one by dawsontoth · Pull Request #2458 · HarperFast/harper

dawsontoth · 2026-09-01T23:59:05Z

Adds Table.oldestRetainedAuditTime(), the floor of a database's retained audit history, so a consumer resuming from a saved audit cursor can tell a complete catch-up from a silently truncated one. A cursor below the returned time must re-read — that is the action, not a diagnosis: such a cursor may have lost history and the floor cannot certify otherwise, which is not the same as history having been lost. Infinity means the floor is unknown and fails closed, so no cursor reads as safe. The comparison holds only for a cursor in the audit-log time domain (subscribe's localTime, not getHistory's). It is diagnostic about pruning specifically: cursor >= floor is not on its own a certificate that resuming is safe, because the floor is not a database-generation check (see the limitations below).

const floor = tables.Product.oldestRetainedAuditTime();
if (cursor >= floor) subscription = await tables.Product.subscribe({ startTime: cursor });
else await fullResync();

The gap this closes is live today: Table.subscribe's startTime replay begins wherever the audit log now begins, and MQTT durable sessions hand it a persisted per-topic startTime on every resume, so a client offline longer than logging.auditRetention loses messages — QoS 1/2 included — with no signal at all.

Making that answer trustworthy was most of the work. All five paths that prune audit history now raise the floor before removing anything, monotonically, in a store transaction whose commit is actually verified; only one path did before, and it recorded afterwards, so a crash in between left a floor certifying history that was already gone. The floor lives under a new key whose presence is the trust marker: a store without one has retention history that cannot be accounted for, so it gets a one-time resync epoch rather than a permissive baseline. Untrustworthy metadata resolves to Infinity rather than to a number, so a consumer spelling the check as cursor < floor cannot read corrupt bytes as safe.

Two fail-open paths this uncovered along the way are worth naming, because neither is specific to the new accessor. A prune bound of NaN, a negative, or -0 was accepted by the range even though the floor declined it — audit keys are raw float64, so those values set the sign bit or the quiet-NaN pattern and sort above every real timestamp, making getRange({ start: 1, end: NaN }) span the whole log. delete_transaction_logs_before reaches exactly that through Number.parseInt on a non-numeric timestamp, so it now validates at its own boundary and reports a 400, and the bound guard itself throws rather than declining silently. And RocksDB's transactionSync returns undefined on a swallowed abort rather than throwing, which RecordEncoder.saveStructures already documents; the floor write now requires an explicit true.

The branch originally retired getLastRemoved/updateLastRemoved, which had no consumers and did not work: both read through the audit store's value decoder, so on LMDB the raw eight float bytes decoded as an audit entry and the function returned a stale module global — measured, it stored 1234567.5 and returned 1 — while on RocksDB the same call threw in msgpack decode. That retirement was reverted when this branch merged #2338, which hardened the marker's write path and added five tests around it; deleting it in a merge would have silently undone that. Both markers are live now and the code says why they are separate keys: last-removed records where the LMDB retention loop got to, after the fact, while the floor is written ahead of all five prune paths with its commit verified. Fixing getLastRemoved's decoder is left alone deliberately — it has no consumers, and the floor is what the accessor reads.

For the human reviewer

Six judgment calls from the final review's decision ledger — five open, one (4) already ruled on. All are scope or contract choices; the mechanics went through four review rounds and converged from a blocker through majors to an adjudicated minor.

Ship the primitive before its consumer (Consume the audit staleness floor in Table.subscribe: an opt-in stale-start check so MQTT durable resume signals truncation instead of replaying short #2448). Chosen: land the floor, leave Table.subscribe untouched. Alternative: land both together, which is what makes the guarantee real and what one reviewer argued for. Why not: subscribe also serves WebSocket and SSE reconnects and sourcedFrom caching-table subscriptions, so turning a fallen-off cursor into an error by default changes behavior for every subscription consumer at once. Consume the audit staleness floor in Table.subscribe: an opt-in stale-start check so MQTT durable resume signals truncation instead of replaying short #2448 scopes it as an opt-in SubscriptionRequest flag. Fully reversible; nothing here presumes how the consumer arrives. Cost of a "no": this API has no in-tree caller until Consume the audit staleness floor in Table.subscribe: an opt-in stale-start check so MQTT durable resume signals truncation instead of replaying short #2448 lands, so nothing proves the floor is the right shape for the resume check it exists to serve.
One database-scoped floor, not per-table. The audit store is per-database and entries carry a tableId, so an exact per-table floor needs a scan for that table's oldest entry. The cost of the choice: deleteHistory on one table raises the floor for every sibling's consumers, forcing resyncs of history that still exists. This is the expensive one to change later — it changes the meaning of a value consumers will have persisted, so it is cheap now and costly after Consume the audit staleness floor in Table.subscribe: an opt-in stale-start check so MQTT durable resume signals truncation instead of replaying short #2448.
Infinity is the unknown sentinel. It makes both cursor >= floor and cursor < floor fail closed, which is why it beat undefined/null/throw — a null floor would make a naive cursor >= floor read as safe. Against it: Infinity type-checks silently into arithmetic, and the return type is what every consumer codes against and is hard to change once public.
Upgrade stamps a resync epoch — decided, not open. Every existing store lacking the record gets max(Date.now(), newest retained audit key) on first open. The alternative baseline is unsound: a table-scoped backup taken without include_audit leaves records and cursors with no audit history, indistinguishable from a fresh database. The final review routed the residual question to a human — stamp, or preserve an unknown floor for any store with no record — and Dawson ruled: stamp. Recorded here because the objection is real and now accepted rather than unanswered: the bootstrap is only as good as what survives, and RocksTransactionLogStore.getKeys() is unimplemented, so on RocksDB the clock-rollback guard reduces to Date.now(). Never stamping would make every upgraded deployment fail closed forever, which is worse than a bound that holds on LMDB and degrades to the wall clock on RocksDB. Consequence to expect: on upgrade, every existing deployment's older cursors read stale at once — though only once Consume the audit staleness floor in Table.subscribe: an opt-in stale-start check so MQTT durable resume signals truncation instead of replaying short #2448 turns the floor into behavior. Chris Barber then sharpened the objection to its strongest form: a legacy deleteHistory can remove one table's entries from above every survivor, so the clock-rollback guard is bounded by state that cannot see the gap. Deferring that looked free and was not — the bootstrap runs at first open by this version, so an affected store persists the bad epoch immediately, and nothing could later tell it from a floor a real prune earned. So the guess is now recorded as a guess under its own key, written ahead of the floor. The cross-model round at 1f5ab5b then caught the reading I first attached to it: floor > bootstrap does not mean a prune earned the floor, because a prune certifies only what it removed and says nothing about history removed before tracking began — which may sit above the epoch, since that is exactly what the guess cannot see. So the record's presence is the signal and no comparison retires it; only a database generation does. The recorded value's job is telling that repair how far the guess reached. The epoch can still be wrong; it can now be found and fixed, which is what restore_backup rolls a database back in time with no epoch, so audit cursors, record versions, and replication sequence state all read as valid afterwards #2451 consumes — and it is the only route by which a store reading unknown can ever earn a real floor back.
A floor that cannot be written blocks the prune — routed to A failed audit-floor write blocks the boot purge exactly when the disk is full, making #1115's reclamation unreachable #2486, not settled here. raiseAuditFloor throws, and it is called first precisely so the throw stops the prune. Raised independently by Chris Barber and by the cross-model round's graded leg. The sharp case: on a full volume the thing that fails is the 8-byte floor write, so Resync re-delivery (~6.7×) + cleanup starvation balloon a far-behind node's transaction logs (compounds #1114 OOM) #1115's boot purge — the one built to let a crash-looping node shed its backlog — becomes unreachable in exactly the condition it exists for. Not a boot crash (resources/replayLogs.ts:79-86 catches and warns), but the purge is skipped, which an earlier draft of this line described too kindly as availability winning. The retention loop swallows and warns; deleteHistory and the bridge operation propagate (correctness wins there, and the caller can see it). What rules out the obvious fix: you cannot record that you pruned without recording, since the unknown sentinel is a write to the store that just refused one — so the escapes are reserved headroom for the fixed-size marker, an in-memory poison, or accepting an inaccurate floor. A failed audit-floor write blocks the boot purge exactly when the disk is full, making #1115's reclamation unreachable #2486 has the analysis. Nothing reads the floor yet, so this is reversible per call site with no migration.
deleteHistory raises unprobed. The retention loop probes for an eligible entry before raising; deleteHistory raises to endTime before knowing whether this table has any entry below it, so a routine per-table trim permanently advances the database floor even when it deletes nothing. Symmetry would mean a tableId-filtered scan, which is the per-table cost decision 2 avoids.

Two deliberately-deferred limitations, both stated in the public contract — the accessor's JSDoc, the DESIGN.md row, and the docs page — rather than only in getAuditFloor's internal caveat, which is what Kris's review correctly objected to: copying a database's state without its history — restore_backup reinstalls the backup's floor, and a RocksDB checkpoint copies the floor but no transaction logs — needs a database-level generation because the same copy also rolls back record versions and per-node replication sequence state (#2451); and the check/use race, where a prune can land between reading the floor and subscribing (milliseconds against retention horizons of days, and losing it degrades to today's silent truncation, never worse — closed only by validating inside the resume, #2448).

Three reviewer findings were dropped as factually wrong, each measured rather than argued: that asBinary() through RocksDB's txn.putSync stringifies and corrupts the floor (the round-trip is asserted byte-exact on a real RocksDB store), that writing the floor key makes an empty LMDB store feed a Symbol to openAuditStore's time-reversal comparison and abort opening (getKeys({ reverse: true, limit: 1 }) yields zero keys on a floor-only store under the audit key encoder — now pinned as a test), and that the LMDB eligibility probe can race a commit into the range it gates (probe, raise and scan construction are synchronous in one thread).

Verification

Route (a) — extended existing tests, plus a new unit suite. No new integration entry: the behavior is entirely within the storage layer and reachable from unit tests on both engines.

unitTests/resources/auditFloor.test.js (new): 25 tests on RocksDB, 30 on LMDB (7 and 2 engine-skipped respectively) — floor establishment, the exact resume predicate at F-1/F/F+1, six shapes of untrustworthy metadata all resolving to Infinity, monotonicity, database scoping and non-leakage, NaN/-0/non-number bounds refused before anything is deleted, deleteHistory(Infinity), a real legacy standalone audit root including a close-and-reopen durability check, floor-before-purge ordering observed from inside purgeLogs on a real store, and the empty-pass no-write case.
unitTests/resources/auditPurge.test.js (extended): purgeAgedLogs records the floor at the same cutoff, before purgeLogs.
unitTests/resources/deleteTransactionLogsBeforeRocks.test.js (extended, 8 tests): the whole-database purge route advances the floor database-scoped, and the new bound guard refuses five bad timestamp shapes with a 400 while accepting Date, a numeric string, a number, 0 and "0".
Fails-on-base: with the pre-fix guards restored, 7 of the new tests fail (2 for the unbounded-deleteHistory blocker, 5 for the round-2 majors) and pass with the fixes.
CI at this SHA: Unit Test (Node 22/24/26 + Windows), Integration Tests, Format Check and Lint all green. Two flakes were hit and attributed rather than assumed: mqtt-test.mjs "subscribe to retained record with patch operations" (its own comment notes the two messages "can arrive out of order on a loaded CI runner" behind a fixed delay(20) — harper#1138 shape), and the #1854 RocksDB oracle whose readOnly handle races compaction unlinking an SST, which is exactly what Give the #1854 RocksDB oracle a checkpoint it cannot race #2452 is open to fix. A third red was the Node 22 leg hitting the workflow's 10-minute cap at 10m18s; the same production code ran 7m32s one commit earlier against 7m34s–8m2s on recent main, so it is runner variance, not added cost.
npm run test:unit:resources: 1965 passing / 35 pending on RocksDB, 1622 / 205 on LMDB (post-merge; the merge brought in Apply audit retention continuously to RocksDB transaction logs #2338's own suites). unitTests/bin/**: 269 passing on LMDB, including copyDbIntegrity — its byte-for-byte audit-log comparison walks symbol keys, so it covers the new key surviving an LMDB→RocksDB copy unchanged.
npm run test:unit:main: 21 failures, 20 of which reproduce identically on clean origin/main (compared by test name on a baseline worktree); the 21st, resolveComponentName, passes in isolation on both refs — an order-dependent git fixture. None attributable to this change.
Merged origin/main (17 commits) rather than rebased, since the PR is Ready with reviewers assigned. Apply audit retention continuously to RocksDB transaction logs #2338 reworked the same function, so the conflict was semantic: resolved onto main's version, keeping its cleanupStopped/storeClosing guards, explicit iterator, stopAuditCleanup(), per-pass priority, retention-derived RocksDB cadence and pendingLastRemoved retry-carry. Verified main's 37 audit-log tests pass exactly as on a clean origin/main worktree built for the comparison.
Two defects the merge surfaced, both fixed: the eligibility probe relied on the range honoring limit: 1 with no break (it hung against a getRange that ignores it), and the floor shared the last-removed marker's scratch buffer, so a floor read could rewrite a marker still in flight.
npx prettier --check . clean; npm run lint:required 0 errors.

Docs companion: HarperFast/documentation#660, plus HarperFast/documentation#666 correcting the same over-promise Kris flagged here.

Complexity: complicated

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

_{Human-Review-Need: 4 @ a184017}

…detectable `Table.subscribe`'s `startTime` replay begins wherever the audit log now begins, so a consumer resuming below the retention horizon is silently handed a short replay. MQTT durable sessions ride that path with a persisted per-topic `startTime`, so a client offline longer than `logging.auditRetention` loses messages — QoS 1/2 included — with no signal. Adds `Table.oldestRetainedAuditTime()`: the database-scoped floor of retained audit history. A consumer whose last-processed cursor is `>=` it can resume incrementally; below it, history it needs is gone. `Infinity` means the floor is unknown and fails closed, so no cursor reads as safe. Making that answer true required the floor to actually be maintained: - Every one of the five prune paths now raises the floor BEFORE removing anything (retention loop, RocksDB steady-state purge, boot `purgeAgedLogs`, `Table.deleteHistory`, and the whole-database `delete_transaction_logs_before`). Only the first did so before, and it recorded afterwards — a crash in between left a floor certifying history that was already gone. - The read-modify-write runs in a store transaction, since pruning is not confined to one worker and two unsynchronized advances could leave the lower cutoff in place. - The floor lives under a new key whose presence is the trust marker. A store without one has retention history we cannot account for — including the empty audit store an LMDB→RocksDB migration leaves behind, since `bin/copyDb.ts` deliberately does not migrate it — so `openAuditStore` stamps a one-time resync epoch rather than a permissive baseline. - Untrustworthy metadata (wrong length, NaN, negative) resolves to `Infinity` rather than to a number, so a consumer spelling the check as `cursor < floor` cannot read corrupt bytes as safe. This retires `getLastRemoved`/`updateLastRemoved`, which had no consumers and did not work: both read through the audit store's value decoder, so on LMDB the raw float bytes decoded as an audit entry and the function returned a stale module global (measured: stored 1234567.5, got back 1), while on RocksDB the same call threw in msgpack decode. Also fixes `deleteHistory` scanning from `start: 0`, which included the metadata symbol keys and logged a decode error for each on every call; `getHistory` already used `start: 1` for exactly this reason. Consuming the floor inside `Table.subscribe` is deliberately not here: that path also serves WebSocket/SSE reconnects and `sourcedFrom` caching tables, so erroring a fallen-off cursor by default is a behavior change that needs its own design pass. Tracked as an opt-in check in #2448. Closes #2447 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… legacy layout Three real holes the reviewers found, all in the same direction — a floor that certifies history already gone: - `deleteHistory(Infinity)` removed every audit entry while `raiseAuditFloor` ignored the cutoff as non-finite, leaving the old floor in place. `cutoff > current` is now the whole guard: it still rejects NaN and negatives, and Infinity is stored, where it decodes back to "unknown" so no cursor reads as safe. Both new tests fail with the previous guard. - The "this store is brand new, use a permissive baseline" case is gone. Creating the audit DBI proved only that the DBI was absent, which is also what a table-scoped backup taken without `include_audit` leaves behind — records and their cursors, no audit history, stamped as if nothing had been pruned. Every store without a floor now gets the resync epoch, which also makes the two engines behave identically. - A legacy `auditPath` audit store is opened as its own standalone LMDB root and has no `.rootStore`, so every prune through it hit `undefined.transactionSync`. It owns the transaction itself now, and gets a floor at open. Two smaller ones: - The RocksDB purge branch had no try/catch, unlike the LMDB pass. Reclamation calls it without awaiting, so a `purgeLogs` throw was an unhandled rejection with no log line. - An idle LMDB database wrote a floor transaction on every retention pass forever. It now probes for an eligible entry first — sound because `end` is fixed and audit keys only move forward, so nothing can drop below it between the probe and the loop. Writes now own their eight bytes rather than handing the store a live view of the reused module buffer, and the added comments are trimmed to the invariants the code cannot state. Also documents the one gap left open: `restore_backup` reinstalls the backup's floor, so a cursor from after the backup point reads as safe. Not fixed here because a restore also rolls back record versions and per-node replication sequence state — making this one field honest while those stay stale gives a consumer a less coherent answer, not a safer one. Filed as #2451. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…unds the range honors The adjudicated round-2 review found three more fail-open paths, each confirmed against evidence already in the repo: - RocksDB's `transactionSync` returns undefined on a swallowed abort rather than throwing — `RecordEncoder.saveStructures` documents exactly this — so a floor write that never committed read as success and the caller went on to prune. The callback now returns `true` and a result that is not `true` throws. - A legacy `auditPath` audit root is opened with an encoder that has no Uint8Array passthrough, so writing raw floor bytes reached `createAuditEntry` and threw `Invalid audit entry type` — failing startup for any install still on that layout. The write is wrapped in `asBinary()`, which bypasses both engines' encoders. The previous "legacy" test bound its stand-in off a non-legacy store and proved nothing; it now opens a real legacy-shaped root. - A NaN or negative prune bound was declined by the floor but honored by the range: audit keys are raw float64, so NaN and negatives sort ABOVE every real timestamp and `getRange({ start: 1, end: NaN })` spans the whole log. `delete_transaction_logs_before` reaches that through `Number.parseInt` on a non-numeric timestamp, so declining only the floor update left the prune deleting everything. `raiseAuditFloor` now throws on such a bound, which stops the prune — the same reason it is called first. And three smaller ones: - `establishAuditFloor` keys off the record's absence rather than its decoded value, so reopening no longer replaces a deliberately-stored `Infinity` (or corrupt bytes) with `Date.now()`, lowering a floor that never lowers. Reading it outside the transaction also keeps the common case — a floor already established, every worker, every database, every boot — off the env write lock. - Read-only mode makes `raiseAuditFloor` throw instead of returning quietly. Only `scheduleAuditCleanup` and `purgeAgedLogs` check read-only themselves, so for `deleteHistory` and the whole-database purge this throw is the guard. - Stale and reviewer-addressed comments trimmed, including two that described behavior removed in round 1. One round-2 finding was dropped as factually wrong by adjudication: the LMDB eligibility probe and the scan it gates are synchronous in one thread, so an empty probe cannot race a commit into the range. All five new tests fail without these fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… startup fragility - `-0` and a non-number slipped past the `< 0` bound check but are still ordered keys the prune range honors: `-0` sets the float64 sign bit and a non-number takes the ordered-binary branch of the key encoder, so both sort outside the timestamp space the bound means to describe. The guard now requires a non-negative number and rejects `-0` explicitly. - `establishAuditFloor`'s absence check is repeated inside the transaction. The outer read is only there to keep the common case off the env write lock; on its own it could race another worker's prune storing `Infinity` and then lower it to `Date.now()`, since Infinity decodes to the same unknown sentinel. - A failed first-time floor write no longer aborts a database open. An unrecorded floor already reads as unknown, which is the fail-closed answer; failing startup turned a metadata write failure into an outage. - `raiseAuditFloor` pre-checks the floor lock-free and returns before taking the write lock when the cutoff cannot move it — the common case for a RocksDB reclamation pass on an idle database. The in-transaction guard stays authoritative, and the pre-check is guarded so it never decides the error a store with no audit store at all reports. - Documents that `getHistory` is NOT in the floor's time domain: it reports each entry's origin `version` under the name `localTime`, which a backdated or replicated write makes differ from the audit-log key the floor is. A cursor saved from `getHistory` is not comparable to the floor. The round-3 blocker — that writing the floor key makes an empty LMDB audit store feed a Symbol to openAuditStore's time-reversal comparison and abort opening — is not real. Measured with the audit key encoder, `getKeys({ reverse: true, limit: 1 })` yields zero keys on a store whose only key is the floor, so the comparison never runs. Pinned as a test rather than left to argument. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ygiene Round 4 adjudicated to minor. Remaining items: - `delete_transaction_logs_before` validates `timestamp` at its own boundary and reports a 400, as the method already does for an unknown table two branches above. Without it a non-numeric timestamp reached `raiseAuditFloor` and came back as a bare Error, reporting operator input error as a server fault. - The documented limitation now names RocksDB checkpoints alongside restore. A branch database copies the floor record but no transaction logs (`branchDatabase.ts` says so itself), so it inherits a present — therefore trusted — floor for history it does not have. Same lineage-copy family, added to #2451's scope. - The test suite restored only the retention and left the module-global default cleanup delay at 1 ms for every audit store opened later in the process. - The legacy-root test now closes and reopens the store, so something actually proves the floor bytes are durable — which is what the whole floor-before-prune ordering rests on, and every other assertion read back through the handle that wrote them. - Dropped the three `// floor first` call-site markers and the reviewer-addressed phrasing; the ordering requirement is stated once, in raiseAuditFloor's JSDoc. Two round-4 findings were dropped as factually wrong by adjudication: that negative and `-0` horizons are legitimate no-ops (they set the float64 sign bit and sort above every timestamp, so they are delete-everything bounds), and that idle RocksDB databases now take a write transaction per wall-clock tick (that branch has no timer loop — it runs at open and then only from the reclamation callback, which fires under disk pressure). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- `establishAuditFloor` bootstraps to `max(Date.now(), newest retained entry)` rather than bare `Date.now()`. A clock that has rolled back would otherwise stamp a floor BELOW history the database may already have pruned, certifying a stale cursor. The newest retained entry is a lower bound the clock cannot argue with: everything at or above it is demonstrably still there. (`openAuditStore` separately error-logs the reversal itself.) - `-0` reached the new `delete_transaction_logs_before` guard as `-0 >= 0` and passed, then came back as the bare Error the guard exists to prevent. Rejected explicitly, and added to the boundary test's cases. - The accessor's JSDoc no longer claims a database opened by a version that recorded no floor reports `Infinity` — a successful open now establishes a finite epoch, so `Infinity` is reachable only when the floor could not be written at all. Also adds the boundary tests for the round-4 guard: five rejected timestamp shapes with a 400, and five legitimate ones (Date, numeric string, number, 0, "0") accepted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gemini-code-assist

Code Review

This pull request implements an audit retention floor mechanism to track the oldest retained audit history time, ensuring consumers resuming incremental audit-log consumption can reliably detect if their history has been pruned. The floor is raised before any pruning occurs across all prune paths, with updates made to ResourceBridge.ts, Table.ts, auditStore.ts, and databases.ts, alongside new comprehensive unit tests and design documentation. The review feedback suggests returning 0 immediately in Table.ts's deleteHistory when using RocksDB to avoid unnecessary iteration, and using strict assertions (assert.strictEqual and assert.deepStrictEqual) in the unit tests to prevent type-coercion bugs.

dawsontoth · 2026-09-02T00:02:51Z

Docs companion: HarperFast/documentation#660.

Dawson ruled on the open question the final review routed to a human: stamp the resync epoch on every floorless store, rather than leaving such a store permanently unknown. That is what the code already did; this records why the residual objection is accepted rather than open. The bootstrap is bounded by what survives, and RocksTransactionLogStore.getKeys() is unimplemented, so on RocksDB the clock-rollback guard reduces to Date.now(). The alternative — never stamping — makes every upgraded deployment fail closed forever, which is worse than a bound that holds on one engine and degrades to the wall clock on the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Per gemini's review on #2458. `assert.deepEqual` on a scalar was the wrong tool regardless, and the other two compare numbers where a string-vs-number would have been masked. AGENTS.md's house style is plain `assert` with no `node:assert/strict` import, which this keeps — it calls the strict methods directly, as that guidance allows for checks that need them. The other review comment (early-return from deleteHistory on RocksDB) is declined and filed as #2469: the suggested fix would disable the cleanupDeletedRecords branch, which does real work on RocksDB and has a RocksDB-only regression test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

#2338 ("Apply audit retention continuously to RocksDB transaction logs") landed on main and reworked the same function this branch changes, so the conflicts in resources/auditStore.ts were semantic rather than textual. Resolved by taking main's version as the base and re-applying the floor work onto it, so none of #2338's structure is lost: cleanupStopped/storeClosing guards, the explicit iterator with its cursor release, stopAuditCleanup(), the per-pass cleanup priority, the retention-derived RocksDB cadence, and the pendingLastRemoved retry-carry. Three things that changed as a result: - **The `last-removed` marker stays.** This branch had retired it as broken and unconsumed. #2338 hardened its write path and added five tests around it (retry-on-failure, no write from a retired pass, contained initialization failure), so deleting it in a merge would silently revert that work. The two markers now coexist: `last-removed` as #2338 left it, `audit-floor` as the write-ahead, verified, monotonic primitive the accessor reads. The key doc says why they are separate. - **The retention loop raises the floor off its first eligible entry** instead of a separate probe range. One cursor, so an idle database still writes no floor, and nothing else observing that range sees an extra advance — which is what #2338's gated-pass fixture asserts on. Strictly before any removal, unchanged. - **Floor writes go through `put` inside the write transaction, not `putSync`.** lmdb's `putSync` is `put(...) === SYNC_PROMISE_SUCCESS`, so it discards whatever put returns; a put that hands back a real rejection — which #2338's marker-failure fixtures install — then leaks with no owner. Inside a write transaction put is synchronous and returns an already-resolved sentinel, so the floor is still visible immediately and the failure case now has an owner. Also fixes a latent bug the merge surfaced: the eligibility probe relied on the range honoring `limit: 1` and had no `break`. Against a getRange that ignores it — the RocksDB log store does, and #2338's fixture installs a synthetic infinite range — it would have walked the whole log or never returned. Moot now that the scan's own iterator answers the question. Verified: main's 37 audit-log tests pass exactly as on pristine main (confirmed against a clean origin/main worktree), test:unit:resources is 1965/0 on RocksDB and 1622/0 on LMDB, and copyDbIntegrity still round-trips the audit log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude · 2026-09-02T15:20:08Z

Reviewed the delta since my last pass (717c0ee89→754af5378): the only change is main's unrelated Exclusive-record-locks merge (#2462) — verified no audit-floor files or identifiers were touched (raiseAuditFloor/establishAuditFloor/updateAuditFloor/oldestRetainedAuditTime all absent from the diff), so the prior merge-revert pattern (#2434's cheat-sheet row) didn't recur here. No new blockers in this delta. One pre-existing blocker remains open and unchanged: cb1kenobi's "Legacy bootstrap can certify a selectively pruned gap" (auditStore.ts) is still awaiting his sign-off after the author's bootstrap-provenance mitigation (fbb96f673/0f479e1f6) made the deferral repairable via #2451 without resolving it. Not re-flagging inline — unchanged, already tracked on three open threads — noting it here so it isn't lost under a clean-pass reading.

Caught in review on #2458. The merge with #2338 kept the `last-removed` marker and said so in its own message and in the `AUDIT_FLOOR_KEY` comment, but this prose was not updated with them. DESIGN.md is the section index AGENTS.md points contributors at, so calling an actively written and actively tested marker "retired" is exactly the sentence that would get it deleted — silently reverting #2338's hardened write path and its five tests. Says what is true instead: both markers are live, why they coexist, and why the floor needed its own key rather than reusing that one. Also splits the trust-marker bullet, which had grown two unrelated claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Caught in review. `decodeAuditFloor` wrote into the shared FLOAT_BUFFER on every floor READ — including `raiseAuditFloor`'s pre-check, which runs on each prune — while `updateLastRemoved` sets FLOAT_TARGET and hands that same buffer to an async `put` it has not yet consumed. A floor read landing in that window would rewrite a marker still in flight. The floor now has its own FLOOR_TARGET/FLOAT pair, which removes the interaction rather than reasoning about whether the encoder's needsStableBuffer copies in time. The two mechanisms coexist; sharing a scratch buffer between them was an artifact of the merge, not a decision. Also drops two test comments that restated the test title. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

All three describe behavior the merge changed, which is the same staleness that put "retired" in DESIGN.md: - the RocksDB skip said the Rocks branch "runs on demand, not on a loop", which #2338 made untrue — it now re-arms on a retention-derived cadence. It says why the probe assertion does not apply there instead. - a test named the floor "the audit store's only key", which it stopped being once the merge kept last-removed's initialization alongside it. - one comment restated the fixture value on the line above it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… record Both from review, both fail-open: - `decodeAuditFloor` accepted `-0`: it passes `Number.isFinite` and `>= 0`, so it came back as a permissive zero and every cursor compared as safe — from bytes with the sign bit set that nothing here writes. `raiseAuditFloor` already rejected the same value as a cutoff; the read side now agrees, because a one-sided guard is how corrupt metadata certifies a stale cursor. - `raiseAuditFloor`'s lock-free pre-check returned early when the record was absent, since an absent floor decodes to `Infinity` and no cutoff beats it. The prune then proceeded leaving no marker at all, and the next open would stamp a FINITE epoch — so a bound above that epoch (a future `endTime`, or a rolled-back clock) certified cursors whose history the prune had deleted. An absent record now persists the unknown sentinel before the prune is allowed to continue, which is the honest value: a store with no record may have been pruned before this run too, and unknown is the only claim that covers it. The pre-check now keys on the record's presence rather than on the comparison, and reuses the bytes it already read instead of decoding twice. Both tests fail with either fix reverted. `-0` joins the untrustworthy-metadata table; the sentinel test clears the record through the root store on RocksDB, where the log store's own remove() is a no-op. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…re it A bug in the previous commit, caught in review. The absent-record branch ran its own transaction with `(_current, recorded) => recorded ? undefined : UNKNOWN` and then returned unconditionally. If another worker's establishAuditFloor landed between the lock-free read and that transaction, `recorded` was true, the resolver wrote nothing, and the caller pruned to `cutoff` against a floor still sitting below it — certifying every cursor in between. That is the silent gap this function exists to prevent, reintroduced by the fix for a different one. Collapsed to a single transaction whose resolver decides both cases where they are race-free: no record means persist the unknown sentinel, a record means the ordinary monotonic raise. The lock-free pre-check now skips only what it can prove is a no-op — a record that already sits at or above the cutoff — and never rules on absence. The new test hides the record from the pre-check read only, so the transaction sees the real store; it fails against the previous shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cb1kenobi's finding stands: `establishAuditFloor`'s `max(Date.now(), newest surviving key)` is bounded by what survives, and a legacy `deleteHistory` can remove one table's entries from above every survivor, so a clock rolled back into that window stamps a floor below history that is gone. Refusing to stamp is worse — `AUDIT_FLOOR_UNKNOWN` is absorbing, so it would make every upgraded deployment fail closed with no route back. But the reason deferring the real fix looked cheap was wrong: the bootstrap runs at first open by this version, not when anything reads the floor, so an affected store would persist the bad epoch immediately and nothing could later tell it apart from a floor a real prune earned. So the guess is now recorded as a guess, under its own key, written before the floor and never touched again. `floor === bootstrap` means still a guess; `floor > bootstrap` means a prune earned it; absent reads as suspect, because repair means RAISING a floor and that is always safe. #2451 consumes this — and it is also the only route by which a store that reads unknown can ever earn a real floor back. Adoption is deliberately not bounded by the newest surviving key: `floor === bootstrap` is the whole signal, and re-deriving the epoch would destroy it. There is also no read-it-first fast path, though one is tempting — the ablation below showed that two routes to the same value left the second one untested, which is precisely how this function's neighbours grew four fail-opens earlier in this PR. `updateAuditFloor` takes the key rather than hardcoding it, so the provenance record gets the same verified commit as the floor instead of a second write path. Coverage verified by ablation on BOTH engines, not by assuming the branch is shared: with the record write and adoption removed, the same four tests fail on RocksDB and on LMDB (the fifth is a negative test and correctly still passes). Suites: 1975 RocksDB / 1634 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude · 2026-09-03T14:09:53Z

+		// shared log, so a table whose entries were the newest and all fell below that bound leaves the
+		// log's newest survivor being a sibling's OLDER entry — removed history above every surviving key.
+		// A clock rolled back to between the two then stamps an epoch below entries that are gone, and a
+		// cursor in that window resumes over the gap (cb1kenobi on #2458). Also unclosed:


Still open — not resolved by this push. fbb96f673 adds a bootstrap-provenance record (AUDIT_FLOOR_BOOTSTRAP_KEY) so a later release can identify and repair a suspect epoch (#2451), but the epoch stamped here can still land below history a legacy deleteHistory already removed — the exact scenario cb1kenobi confirmed reachable in the thread above. That's a defensible deferral (the accessor is diagnostic-only per 2a0d9ba00, and nothing reads the floor yet), but it's the author's own call on-thread, not a maintainer sign-off. Flagging so it isn't lost under a clean pass on this run — cb1kenobi still needs to close or hold this before merge.

Correct, and the framing is right: fbb96f673 makes the deferral repairable, it does not make the stamped epoch correct. cb1kenobi's thread stays open for him to close or hold — I am not resolving it on my own reply, and Dawson has the three options on the record there.

One update since you flagged it: he chose the provenance-record option, so what is deferred to #2451 is now the epoch fix itself rather than the ability to find affected stores. #2448 is also formally marked blocked by #2451 now, with both this finding and Kris's restore/checkpoint hole written in as gating criteria — so neither can be lost under a clean pass on a later run.

🤖 Addressed by Claude Code

Kris Zyp on HarperFast/harper#2458: the "does not report a cursor as safe when history it needed is gone" bullet is stronger than what the floor can see. Restoring a backup, or opening a RocksDB checkpoint, replaces a database's state with a copy of an earlier state and reinstalls that copy's floor, so a cursor saved after the copy point compares as safe against a floor that predates it. Scope the one-direction guarantee to retention pruning and add the rollback limit as its own bullet, so nobody reads this method as the only gate on resuming across a restore.

cb1kenobi is right about the guard. `Number.parseInt` takes a numeric PREFIX, so '9999999999999oops' parsed to a year-2286 bound that satisfied every check and reached purgeLogs, and '1e3' silently became 1. `Infinity` passed too — `typeof` and `>= 0` both accept it — and that one is worse than an over-wide purge: it records the unknown sentinel, which `raiseAuditFloor` can never lift, so the database's floor would read unknown permanently. So parse with `Number` (rejecting empty/whitespace explicitly, since Number('') is 0 where parseInt correctly refused) and require `Number.isFinite`, which also does the type check because it never coerces. '1e3' and '1234.5' now land on their exact values rather than parseInt's 1 and a truncation. On severity, for the record rather than to argue the fix away: every caller of this bridge method goes through `utility/logging/transactionLog.ts`, whose Joi schema is `Joi.date().timestamp().required()`. Measured against the real schema, it rejects all of '9999999999999oops', ' 12abc', Infinity, -Infinity, '' and ' ', and coerces whatever passes to a Date — so the string and number branches never see raw operator input today and the unbounded purge was not reachable through the operations API, the job runner, or serverUtilities. That makes this defense-in-depth rather than a live major. Fixed anyway, because a guard that looks like protection and isn't is worse than no guard, and this one is the only thing standing between a direct bridge caller and a whole-log purge. The ablation is the useful part: restoring the old guard fails two tests, not one. The malformed bound really does purge and really does raise the floor, which then breaks the floor assertion of the next test against the same database. Coverage note, stated rather than left implicit: the guard tests live in a RocksDB-gated file, but the guard runs before any engine branch, so one engine's coverage is the whole of it. Suites: 1975 RocksDB / 1634 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Two findings from the cross-model round at 1f5ab5b, both against yesterday's provenance work and both correct. **The matrix over-claimed.** "finite, > bootstrap ⇒ a real prune raised it; earned, not guessed" does not follow. A prune raising the floor above the epoch certifies only what that prune removed, and says nothing about history removed before tracking began — which may sit ABOVE the epoch, since that is exactly the case the guess cannot see. The worked counterexample: a v4-era deleteHistory removes tableA up to t=1000 while sibling tableB's newest survivor is 900; a rolled-back clock stamps bootstrap=900 and floor=900; a later retention pass raises the floor to 950; a repair keyed on `floor > bootstrap` reads 950 > 900, calls it earned, and leaves a consumer at cursor 970 certified over tableA's missing 950-1000 — skipping precisely the stores the record exists to find. So the mark's PRESENCE is the signal, not any comparison against the floor. A store carrying the record has an unverified pre-tracking window for as long as the record exists, and only a database generation (#2451) retires it. The recorded value's job is telling that repair how far the guess reached. No runtime change — nothing reads the record yet — so this is the shipped contract being wrong, in DESIGN.md and the docstring. **An undecodable record pinned the floor to unknown forever**, and the comment claiming "the floor stays absent, so a later open retries" could not come true: the resolver skipped the write whenever a record existed, so the read back failed identically on every later open. That is the fail-closed-forever state this bootstrap exists to avoid, reached by a torn 4-byte write. Undecodable bytes are now overwritten — safe here and not for the floor, where a present record may be a deliberate AUDIT_FLOOR_UNKNOWN and rewriting it would lower a floor. I had considered this exact asymmetry and chose wrong. Also trims the aggregate comment nit two legs raised independently: reviewer handles out of source comments (issue refs stay), the provenance matrix no longer duplicated verbatim between the docstring and DESIGN.md, and the self-defensive design-argument prose cut. Ablation on both engines: reverting the overwrite fails the same one test on RocksDB and LMDB. Suites 1975 RocksDB / 1634 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The delta round at 0f479e1 caught the fix landing everywhere except the place that teaches it. Three test comments still asserted the contract the previous commit retired — "floor === record means still a guess, floor > record means a prune earned it" — while the docstring and DESIGN.md now say the opposite: the record's presence is the whole signal, and only a database generation retires the mark, because a prune certifies nothing about history removed before tracking began. Same class of miss as the DESIGN.md/PR-body staleness earlier in this branch: I corrected the contract in its two homes and left it standing in the tests that document it. The surviving property those tests do pin is narrower and still worth pinning: the record must not drift with the floor, because its value is what tells a later repair how far the guess reached. Rewritten to say that, and to say explicitly what it does not license. Also trims the comment prose three rounds have now flagged, keeping the mechanism and dropping the self-justification: the "Neither half of that is belt-and-braces" / "(measured)" block in updateAuditFloor, and the "Belt to that braces" note that explained why a guard is unreachable rather than what it protects. Suites 1975 RocksDB / 1634 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…laces Chris Barber found this reviewing the docs companion (documentation#666), and it was not only a docs slip — the same inversion was in the source it was written from. `cursor >= floor` was described as meaning no entry was pruned "below the cursor", in `getAuditFloor`'s contract, the DESIGN.md cheat sheet, and the public JSDoc on `oldestRetainedAuditTime`. Backwards, and specifically it claims the one thing the floor never promises. Entries below the cursor sit below the floor and are exactly what a prune takes; what the comparison guarantees is that nothing was pruned *after* the cursor — the history the consumer still needs. A reader who trusted the old wording could have accepted an incomplete audit range as complete, which is the failure this primitive exists to prevent. No behavior change; the code has always compared the right way round. Suites 1975 RocksDB / 1634 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

One conflict, in `Table.deleteHistory`'s audit-range scan, and it is a comment conflict rather than a code one: both sides independently changed `start: 0` to `start: 1`, because scanning from 0 picks up the symbol keys (0 encodes to all zero bytes). Someone hit the same bug upstream while this branch was open. Resolved by keeping both intents — the mechanism from this branch (why zero is wrong) and the cross-reference from main (getHistory does the same thing). `resources/databases.ts` auto-merged; verified this branch's `establishAuditFloor` call survived it. Suites after the merge: 2043 passing RocksDB, 1666 LMDB, zero failures (up from 1975/1634 — main brought ~100 tests). Lint clean of new problems (13 pre-existing warnings, none in files this branch touches), prettier clean.

cb1kenobi · 2026-09-03T15:56:31Z

 			let entriesDeleted = 0;
+			// LMDB only: RocksTransactionLogStore.remove() is a no-op, so a RocksDB deleteHistory removes
+			// nothing and must not claim it did.
+			if (!isRocksDB) raiseAuditFloor(auditStore, endTime);


deleteHistory(Infinity) permanently pins the whole database's floor to unknown — Severity: major

raiseAuditFloor deliberately stores Infinity for an infinite cutoff, and Infinity is absorbing. The lock-free pre-check returns early for any present record because no cutoff > Infinity, and establishAuditFloor skips any store that already has a record. So once Table.deleteHistory(Infinity) runs against one table, oldestRetainedAuditTime() returns Infinity for that database forever — for every sibling table too — and nothing short of the database generation in harper#2451 can restore a finite floor.

Failure mode: an operator clears one table's audit history (Product.deleteHistory(Infinity), or deleteHistory() if endTime defaults to Infinity the way getHistory's does two methods below). Sibling table Order's audit log is untouched and fully retained, but every Order consumer — including a cursor saved a second after the call — now evaluates cursor >= Infinity as false and full-resyncs on every resume, permanently. The accessor this PR adds is dead for that database from then on.

This is the same hazard rejected at the bridge boundary in 1f5ab5b: "Infinity ... records the unknown sentinel, which raiseAuditFloor can never lift, so the database's floor would read unknown permanently." delete_transaction_logs_before now returns a 400 for it while the direct API writes it by design, so the two entry points disagree about whether that outcome is acceptable.

Simplest fix: keep the write-ahead ordering but record a finite bound — raise to Date.now() before the scan, then raise again to Date.now() once the scan returns. Both raises are monotonic and finite, and the second covers any entry written and removed during the scan, which is the only reason an infinite bound looked necessary. Equivalently, clamp a non-finite cutoff to Date.now() inside raiseAuditFloor rather than persisting the absorbing sentinel.

—
Reviewed 1eafd13

Confirmed and fixed in 6d413ec44. The absorbing-sentinel chain is exactly as you traced it, and the database-scoped consequence is the part that makes it a major rather than a wart: clearing one table's history would condemn every consumer of every other table in that database to a permanent full resync, cursors saved after the call included.

You are also right that the two entry points disagreed. The bridge started returning 400 for Infinity in 1f5ab5b for this precise reason while the direct API persisted it by design — I wrote both, a day apart, without reconciling them.

boundedAuditPruneEnd now substitutes a finite bound for an unbounded request, and deleteHistory uses that same value as the scan's range end as well as the floor. That is what lets the write-ahead ordering hold without an infinite bound: the prune cannot reach an entry the floor does not already cover, so the entries-written-during-the-scan race your double-raise was covering cannot arise. An entry written after the bound is computed is not history the call asked to remove.

Two places I narrowed your suggestion, both worth flagging in case you disagree:

The clamp is at the call site, not inside raiseAuditFloor. Clamping there would also catch NaN, negatives and -0, which that function throws on deliberately — they are ordered keys the prune range would honor, so silently substituting a bound for them would re-open the whole-log-delete hazard rather than close one.

Date.now() alone is not a sound bound. Audit keys above the wall clock are reachable — a rolled-back clock, or a backdated/replicated write — and a removed entry at Date.now() + 1h would sit above a Date.now() floor and read as resumable. The bound is therefore taken strictly above the newest key in the log as well as at or above the clock.

One correction to the report: deleteHistory's endTime defaults to 0, not Infinity — only getHistory's does — so a bare deleteHistory() never reached this. It took an explicit Infinity, which narrows the blast radius but not the severity.

Left open deliberately, as your call rather than mine: raiseAuditFloor still accepts Infinity and stores the sentinel, now with no production caller that passes it. I kept it reachable rather than making it throw like NaN, because the sentinel is still the honest answer for a caller that genuinely cannot bound its prune — but there is no such caller today, and a dead absorbing path is an invitation. Say the word and I will make it throw, which would also make it agree with the bridge's 400 by construction.

Ablation: with the clamp removed, the new test fails on LMDB, the only engine where deleteHistory raises. Suites 2043 RocksDB / 1667 LMDB.

🤖 Addressed by Claude Code

Chris Barber on documentation#666, and this one is mine from the last round: I fixed the time-direction overclaim and introduced its mirror image. "A cursor below the floor has definitely lost history" does not hold, and it contradicts two bullets on the same page. An `Infinity` floor — first open by a floor-recording version, or a migration between storage engines — puts every cursor below it with nothing necessarily pruned; the floor is unknown, not breached. And the "errs in one direction only" bullet says outright that the floor can ask for a resync that was not strictly necessary, which is exactly a `cursor < floor` that lost nothing. A consumer written from that sentence would raise an "audit history lost" alert on every cold start of a freshly upgraded node. So: `cursor < floor` means the consumer MUST resync, and that action is certain. Whether history was actually lost is not, and the floor cannot certify it either way. `Table.ts` and `getAuditFloor`'s contract both said "has lost history it needs" — the docs inherited the overclaim from here rather than the reverse, so both are corrected too. Suites 2043 RocksDB / 1666 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…itely lost history" Chris Barber on #666. The wording I added last round overclaimed in the mirror image of the direction bug it was fixing: an `Infinity` floor puts every cursor below it with nothing necessarily pruned, and the "errs in one direction only" bullet on the same page says the floor can ask for a resync that was not strictly necessary — which is exactly a `cursor < floor` that lost nothing. The certainty belongs to the action. Summary line and the sample's resync branch now say the history *may* have been pruned and the floor cannot certify otherwise. Same correction applied to the engine-side contract it came from, HarperFast/harper#2458.

cb1kenobi, major, and correct. `Infinity` is both the unknown sentinel and absorbing: `raiseAuditFloor`'s lock-free pre-check skips any present record no cutoff exceeds, and `establishAuditFloor` skips any store that has one. So a single `Table.deleteHistory(Infinity)` left `oldestRetainedAuditTime()` returning `Infinity` for that database forever — and because the floor is database-scoped, for every sibling table too. An operator clearing one table's history would permanently condemn every consumer of every other table in the database to a full resync on each resume, including cursors saved after the call. The accessor this PR adds would be dead for that database from then on. He also noted the two entry points disagreed: `delete_transaction_logs_before` started returning 400 for `Infinity` in 1f5ab5b for this exact reason, while the direct API persisted it by design. `boundedAuditPruneEnd` now substitutes a finite bound for an unbounded request — strictly above the newest key in the log and never below the clock — and `deleteHistory` uses that same value as the scan's range end, not just as the floor. That is what makes write-ahead ordering hold without an infinite bound: the prune cannot reach an entry the floor does not already cover, so the race his suggested double-raise was covering cannot arise. An entry written after the bound is computed is not history the call asked to remove. Two deliberate narrowings of his suggestion. The clamp lives at the call site, not inside `raiseAuditFloor`: clamping there would catch NaN and negatives too, which that function throws on precisely because they are ordered keys the prune range would honor. And `Date.now()` alone is not a sound bound — audit keys above the clock are reachable (a rolled-back clock, a backdated write), which is why the bound is taken above the newest key as well. One correction to the report: `deleteHistory`'s `endTime` defaults to 0, not `Infinity` — only `getHistory`'s does — so a bare `deleteHistory()` never hit this; it took an explicit `Infinity`. Ablation: with the clamp removed the new test fails on LMDB, which is the only engine where deleteHistory raises. Suites 2043 RocksDB / 1667 LMDB, zero failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The docstring opens by ruling out `getHistory`'s `localTime` as a cursor, then 13 lines later claims the floor will "never certify a cursor a prune truncated" — stated absolutely, so the sentence quoted on its own drops the condition that makes it true. That is how the claim reached the public docs unconditioned (HarperFast/documentation#666), where a reviewer caught the same conflict against the same counterexample. A `getHistory` cursor is an origin version, so a record written with origin 900 can sit at audit key 400: persist 900 against a floor of 500 and the check passes while the consumer's real position is below the floor. Names that as outside the guarantee rather than leaving it to collide with the validity note above. Comment-only; the contract does not change.

The summary says a consumer whose cursor is `>=` the floor can resume incrementally; the `getHistory` exclusion that makes it true is two paragraphs down. Same shape as the public docs summary a reviewer just caught (HarperFast/documentation#666), and the summary is the sentence that gets lifted to restate the contract. One word: "audit-log cursor", so the domain travels with the claim. The paragraph below still defines what that domain is. Left the DESIGN.md cheat-sheet row alone — it already hedges with "only that", says "audit cursor" in its own question, and points at the full contract rather than standing in for it.

Last of the unconditioned `cursor >= floor` restatements found by a sweep of every guarantee-shaped sentence in this contract, prompted by the third instance turning up in review (HarperFast/documentation#666). The opening summary needed no change here: it claims only that every entry at or after the floor is retained, and never the converse — which is the direction the public docs had backwards.

`boundedAuditPruneEnd` returned `cutoff` unchanged for anything that was not `Infinity`, so a finite bound above everything reachable was recorded verbatim — and a recorded floor never comes down (`raiseAuditFloor` only raises, `establishAuditFloor` skips a store that has a record). There is no route back short of the generation in #2451. Reachable two ways, both operator-supplied and neither guarded: `delete_transaction_logs_before({ timestamp: Date.now() * 1000 })` — an ms/µs slip — and a bare '9999999999999', which is finite and so cleared the bridge's finiteness check. `Table.deleteHistory(farFuture)` on LMDB reaches it identically. From then on `oldestRetainedAuditTime()` reports a year-2286+ floor for EVERY table in the database, including cursors saved after the call: entries written later land below the floor, so the floor's promise is false about history that is still there. Same permanent, database-wide outcome as `deleteHistory(Infinity)` in 6d413ec, differing only in degree. Clamps in `boundedAuditPruneEnd` rather than rejecting future timestamps at the bridge, which would break the legitimate `Date.now() + 60_000` "purge everything" idiom. The bridge's whole-database purge now takes the clamped bound for BOTH the floor and `purgeLogs`, so it cannot remove an entry the floor does not cover. `cutoff > bound` not `Math.min`, so NaN still falls through to raiseAuditFloor's rejection. Also corrects raiseAuditFloor's JSDoc, which claimed the over-report is bounded because "the retention paths pass `Date.now() - auditRetention`". True of those three; the two operator paths have no ceiling of their own, which is exactly where it mattered. Trade recorded: on RocksDB `getKeys` returns [], so the bound reduces to Date.now() and an above-clock key survives a purge that asked for it. That is the safe direction — entry kept, floor honest — versus removing history the floor does not cover. Tests on both engine branches, both verified by ablation (restoring the `cutoff !== Infinity` early return fails 1 on LMDB, 2 on RocksDB). unitTests/resources: 1495 passing RocksDB, 1120 LMDB; lint clean.

All three statements of the contract said the floor promises nothing below the CURSOR, and gave as the reason that such entries "sit below the floor". That reason only holds when cursor === floor. For any cursor strictly above it, `[floor, cursor)` is below the cursor and at or above the floor — and the floor's own definition guarantees nothing after it was pruned, so that range is covered. Consequence of getting it backwards: a consumer holding cursor 900 against a floor of 500 that wants `[500, 900)` — replaying events it processed but never committed, backfilling a derived store, an operator audit — was told the range may well be gone, so it falls back to a full re-read and discards per-event history the floor guarantees is intact. An operator reads the same sentence as a prune that removed data still on disk. Found in the public docs by a reviewer (HarperFast/documentation#666); all three engine-side copies had it identically. Comment-only. Build clean, audit suites pass.

`boundedAuditPruneEnd` and its docblock were inserted between `raiseAuditFloor`'s JSDoc and the function, so TypeScript attached the second block to `boundedAuditPruneEnd` (correct) and nothing to `raiseAuditFloor`. Moves the orphan down; `boundedAuditPruneEnd`'s block stays where it is. The contract that went missing is the write-ahead ordering — "call this before removing anything", "the throw is what stops the prune", "never lowers" — and c70bda6 had already removed the three `// floor first` call-site markers on the grounds that the requirement is stated once in this JSDoc. It was stated zero times anywhere a caller looks, so anyone adding a sixth prune path had nothing telling them the order is load-bearing. That is the invariant the whole floor depends on: a floor written after the removal is lost on a crash in between, and the surviving lower floor then certifies a cursor whose history is gone. Scanned every .ts this branch touches for the same pattern (a `*/` immediately followed by another `/**`); this was the only one. Comment move only. Build clean, 47 audit tests pass, lint clean.

…st-retained-audit-time # Conflicts: # resources/DESIGN.md

My conflict resolution for the "Where is X" table took our side wholesale, which silently reverted main's update to the chained-conditions row. Main documents #2434 as landed — an indexed scan spanning more than one indexed value collapses to one result per record before paging (`search.ts → distinctRecords`), so `limit`/`offset` count records not index entries, element equality stays uncollapsed because `[indexedValue, primaryKey]` is already unique — and names the gap that IS still open: an undeclared (untyped) indexed attribute holding an array. Our older text re-listed #2434 itself as the open gap and lost the untyped-attribute one entirely. DESIGN.md is what AGENTS.md points contributors at, so that row was telling the next reader a shipped fix is still broken while hiding the real gap. Why the resolution looked safe: I compared the two sides row by row with a script that split each line on '|'. That cell contains an ESCAPED pipe (`\|=`), so the split truncated the content column at it, and both sides' prefixes up to that point were identical — the script reported "content differs on shared rows: none" while comparing only the matching halves. Re-ran with a parser that splits on unescaped pipes only: two rows differ, this one (main's, restored here verbatim) and the `computed-history` row, whose difference is this branch's own `oldestRetainedAuditTime` addition. No row from main is dropped now, and `distinctRecords` and the untyped gap are both present.

…st-retained-audit-time # Conflicts: # resources/DESIGN.md

dawsontoth and others added 6 commits September 1, 2026 17:21

dawsontoth added this to the v5.3 milestone Sep 1, 2026

dawsontoth mentioned this pull request Sep 1, 2026

Document Table.oldestRetainedAuditTime() and the subscription catch-up horizon HarperFast/documentation#660

Merged

dawsontoth requested review from cb1kenobi and kriszyp September 2, 2026 00:01

gemini-code-assist Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread resources/Table.ts

Comment thread unitTests/resources/auditPurge.test.js Outdated

dawsontoth mentioned this pull request Sep 2, 2026

Add a supported audit staleness-floor accessor: nothing distinguishes a pruned cursor from a complete one, so MQTT durable resume truncates silently #2447

Open

dawsontoth closed this Sep 2, 2026

dawsontoth reopened this Sep 2, 2026

dawsontoth mentioned this pull request Sep 2, 2026

Table.deleteHistory() returns a non-zero count on RocksDB, where RocksTransactionLogStore.remove() is a no-op and nothing is deleted #2469

Open

dawsontoth marked this pull request as ready for review September 2, 2026 14:23

claude Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread resources/DESIGN.md Outdated

dawsontoth and others added 2 commits September 2, 2026 11:23

cb1kenobi reviewed Sep 2, 2026

View reviewed changes

Comment thread resources/auditStore.ts Outdated

Comment thread resources/auditStore.ts Outdated

dawsontoth and others added 2 commits September 2, 2026 12:00

cb1kenobi reviewed Sep 2, 2026

View reviewed changes

Comment thread resources/auditStore.ts Outdated

cb1kenobi reviewed Sep 2, 2026

View reviewed changes

Comment thread resources/auditStore.ts Outdated

claude Bot reviewed Sep 3, 2026

View reviewed changes