Deflake deferred cache and await LMDB reload markers

[kriszyp]

The reload-marker assertion was not flaky. Under LMDB, writeReloadMarker() could resolve before its asynchronous audit-store write completed, so the immediate audit scan correctly exposed a transaction completion-ordering bug. The cache assertion was a separate eventual-propagation race.

This keeps the cache test's bounded poll, restores the reload marker's immediate-visibility contract, and applies the asynchronous commit-callback contract consistently across both storage engines.

Refs #2178

What changed

• cache-test.mjs polls until the deferred source update is visible. It retries empty and non-success responses but still fails fast on server errors.
• Promise-returning transaction commit callbacks are awaited by LMDB's optimistic and exclusive paths and by RocksDB before the native transaction commits.
• Publish and reload-marker callbacks return their updateRecord() completion so transaction resolution includes audit persistence.
• Reload markers explicitly opt out of replication confirmation. Stored null-key root-topic publishes still receive confirmation, including when a prior null-key record exists.
• Replication confirmation checks the stored entry in both engines rather than dereferencing a missing entry.
• The disconnected LMDB outstandingCommit variable was removed; it had no effect on overload tracking or transaction behavior.
• Reload-marker tests cover LMDB's optimistic and contention paths, plus RocksDB parity. A separate RocksDB test verifies the promise-returning callback contract.

Why this layer

The invariant belongs to the transaction wrapper: when a transaction callback starts asynchronous persistence, the returned commit promise must not resolve until that persistence settles. Polling in the test would hide the broken contract, and awaiting only in writeReloadMarker() would leave transaction chaining and error propagation inconsistent.

Null is not enough to identify a reload marker because Harper also stores root-topic publishes at the null key. The write therefore carries explicit metadata when it stages only a local audit marker.

The default path remains synchronous. Promise aggregation occurs only when a commit callback returns a promise.

Verification

• Focused RocksDB reload-marker tests: 4 passing.
• Focused RocksDB promise-returning callback test: 1 passing.
• Focused LMDB reload-marker tests: 4 passing, covering optimistic and forced-contention paths.
• Negative controls at the merge base failed behaviorally: RocksDB incorrectly confirmed the local-only marker and did not observe callback completion; LMDB missed immediate marker persistence and root-topic confirmation.
• HARPER_STORAGE_ENGINE=lmdb npm run test:unit:resources: changed tests passed within 1,353 passing and 93 pending; only the two existing Node.js 26 randomAccessFields assertion failures remained.
• Default npm run test:unit:resources: changed tests passed within 1,555 passing and 15 pending; the three known Node.js 26 failures remained in databases.test.js, randomAccessFieldsDirective.test.js, and replayStructures.test.js.
• npm run test:integration:all: changed areas passed; the run ended in the unrelated Ollama backend group because Node rejected json/systemSchema.json without a JSON import attribute, cancelling the remaining Ollama cases.
• npm run build, npm run lint:required, npm run format:write, and git diff --check: passed.

The full local test:unit:main gate remains unavailable because this checkout's unit bootstrap opens an ambient install database before test setup; it fails during module load in that external database and does not reach the changed tests.

Review coverage

The required full no-Claude review ran Gemini. Claude and its domain adjudication leg were pruned by policy, while both Cursor legs were pruned because this branch had reached its two-round cap. Gemini returned eight ungraded finding bullets; source tracing and the executed engine tests did not substantiate them, so no additional code changes were made. In particular, LMDB builds the conditional batch synchronously and preserves the eventual false conflict result, both transaction modules export the tested confirmation seam, and both engines accept the stored null-key publish exercised by the tests.

— GPT-5 Codex

_{Review-Coverage: authored=unknown; ran=none; rounds=1 @ d92bacc}

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

[gemini-code-assist]

Code Review

This pull request refactors test files to use a shared waitFor utility instead of hardcoded delays or local helper implementations, improving test reliability. In cache-test.mjs, a hardcoded delay is replaced with waitFor to poll the cache endpoint. In reloadMarker.test.js, the local waitFor implementation is removed in favor of the shared utility. The feedback suggests using optional chaining when checking cacheResponse.data.name within the polling loop to prevent potential TypeErrors from prematurely failing the test.

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Reviewed 248e9fdf — no issues found. All three prior findings are addressed; this PR looks good, nice job!

I re-ran the decisive mutation to confirm the reverted assertion is genuinely falsifiable again, not just the right shape:

• Reload-marker one-shot scan (the Medium). I deferred the writeReloadMarker() audit write by 200 ms in production code (resources/Table.ts), rebuilt with tsc -p tsconfig.build.json, and confirmed the injected marker landed exactly once in dist/resources/Table.js before trusting the run. Test 3 now fails (AssertionError: a reload audit entry was written), where the polled version had absorbed the same delay and passed. Tests 1 and 2 kept passing but slowed from 41 ms/107 ms to 233 ms/323 ms, which independently confirms the defer was live. The commit-visibility contract is pinned again.
• tableId filter. This one is doing real work. Probing the audit store during test 3 shows ReloadTable.tableId = 2 while the scan range contains reload entries for [1, 2] — entry 1 is the marker from test 2, since auditStore is the database-level txn-log store shared across tables in the database. Without the filter the scan matched test 2's marker first and the assertions passed against the wrong table's entry. Worth noting the filter is not vacuous (both sides are real numbers, so no undefined === undefined no-op).
• No flake reintroduced. 12/12 clean runs of unitTests/resources/reloadMarker.test.js, and test 3 passes both standalone and as part of the full file.

One thing I could not verify: unitTests/apiTests/cache-test.mjs would not run here — an unrelated long-running mocha process on this machine holds the RocksDB lock on /Users/chris/harper/database/data, and the suite fails at module load before the test DB path is set. The validateStatus: (status) => status < 500 change is reviewed statically only. It reads correctly: waitFor does not catch, so a 5xx now rejects out of the poll and fails fast with the axios status attached, while a 4xx still retries — which is the intended split.

—
Generated by Barber AI

[heskew]

Reviewed; no blockers found. Thorough handling of async transaction commit callbacks and reload marker confirmation across both RocksDB and LMDB engines.

---

🤖 Posted by Antigravity on behalf of @heskew