fix(replication): use getSync for hdb_nodes point reads (RocksDB MaybePromise)

[kriszyp]

Fixes a class of RocksDB store.get() misuse on hdb_nodes. On RocksDB store.get() returns a MaybePromise — the record synchronously when it's cached, but a Promise on a block-cache miss. An un-awaited primaryStore.get(...)?.field then silently misfires once hdb_nodes grows past the block cache (or right after a cold-cache restart): a Promise is truthy and has none of the record's fields. Same family as #474; this PR covers the other call sites.

Summary

Switch these hdb_nodes point reads to the synchronous getSync():

• subscriptionManager.ts — the self-node replication gate (!selfNodeRow?.replicates). This is the worst one: a Promise self-row silently sets isFullyReplicating = false and logs "Disabling replication". Also ensureThisNode, ensureNode (so the revoked_certificates merge and the sticky LOCAL_ONLY/investigate: v5-leaf → v4 audit forwarding rejected by v4 cert validation #246 bit survive), and the identity-mismatch enumeration. Extracted a small readNodeRowSync helper for the self-gate.
• knownNodes.ts — the full get()→getSync() sweep, incorporated from fix(replication): synchronous getSync for hdb_nodes point lookups (#470) #474 (which this PR supersedes): readNodeForAuth, storeRecordRangeVisible, probeNodeRow, the two isLeader-patch reads (server.nodes push / watcher-listener forward), and shouldReplicateFromNode's self-record term via the new selfNodeReplicates helper. shouldReplicateFromNode is the load-bearing predicate for shouldSubscribe and the wedge-reconcile isDesired check, so a Promise there silently unsubscribed a still-desired peer (Codex flagged this as a P1).
• replicator.ts — getRetrievalConnectionByName (a Promise node?.url is undefined, so the retrieval connection silently never opens).
• clusterStatus.ts — the self-record shard/url read (a Promise omits them from cluster_status).

Supersedes #474. #474 had independently converged on the same root cause (its comments now note the #352 "empty object" was "actually a pending Promise") and was itself a comprehensive knownNodes.ts getSync sweep. Its entire knownNodes.ts change + selfNodeReplicates helper + tests are folded in here verbatim, so #474 can be closed. Note: three of its sites (readNodeForAuth / storeRecordRangeVisible / probeNodeRow) were ones this PR's original scan under-classified as safe "defensive readers" — a Promise doesn't throw and is truthy, so it slips through their try/catch + existence checks. That miss is the argument for the typed-Store<V> follow-up, which turns this whole class into compile errors. Since main is the 5.1.x line, merging here serves the patch directly (no separate patch branch needed).

Tests

• unitTests/replication/readNodeRowSync.test.mjs — deterministic; models the RocksDB MaybePromise contract with a rocksLikeStore stub (same pattern as fix(replication): synchronous getSync for hdb_nodes point lookups (#470) #474's selfNodeReplicates test). Red-before/green-after.
• integrationTests/cluster/blockCacheEviction.test.mjs — 3-node RocksDB cluster with a 1 MB block cache, a cold rolling restart, and a remove_node/add_node cycle; asserts the post-restart write converges, cluster_status reports the self record, and no node logs "Disabling replication".

Where to look

• Uniform getSync (not await) is a deliberate minimal-diff choice — it restores the prior LMDB-always-sync semantics; these are tiny system rows so a blocking point read is negligible. Flagging it as the main thing to sanity-check.
• The integration test is a strong behavioral guard but, being cache-timing dependent, is not perfectly deterministic; readNodeRowSync.test.mjs is the deterministic guard.
• Depends on the paired core PR fix: use getSync for system-table point reads (RocksDB MaybePromise) harper#1473; the core submodule pointer is bumped to it.

Generated by Claude Code (Claude Opus 4.8).

[gemini-code-assist]

Code Review

This pull request addresses a critical replication bug where RocksDB's store.get() returns a Promise on a cache miss, causing replication to be silently disabled or nodes to be dropped from cluster status. The fix replaces these un-awaited get() calls with synchronous getSync() calls across several replication paths (clusterStatus.ts, knownNodes.ts, replicator.ts, and subscriptionManager.ts), and introduces integration and unit tests to prevent regressions. The review feedback focuses on performance optimizations, suggesting the avoidance of redundant synchronous database reads by passing pre-read records, utilizing getRange instead of looping over keys with synchronous point reads, and parallelizing asynchronous operations in the test setup using Promise.all to speed up execution.

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.

[kriszyp]

Thanks for the review, Gemini — all six suggestions are performance / test-speed micro-optimizations, and we're intentionally not optimizing the functions this PR touches (they're cold paths handling tiny system records). Adjudication, resolving the threads:

Sync-read suggestions (declining):

• ensureThisNode pre-read param / pass existingNode — keeping ensureThisNode parameterless for a minimal, reviewable diff. The duplicate getSync(thisName) is a single tiny hdb_nodes point read on the startup/config-change path; the cost isn't meaningful here.
• getRange({}) instead of N getSync in the identity-mismatch enumeration — that branch only runs when this node's self row is absent (a rare diagnostic), so the per-node point reads aren't a concern. getRange is a fair future cleanup, but out of scope for this fix.

Test-parallelization suggestions (declining):

• Sequential setup (seeding, add_node, waitForCount) is intentional for readability and determinism. In particular, adding nodes B and C serially avoids racing two subscribers into the same leader in a test that asserts convergence — test wall-clock isn't a concern here.

None of these affect correctness, so I'm resolving the threads. Appreciate the thoroughness!

— KrAIs (Claude Opus 4.8)