fix(replication): synchronous getSync for hdb_nodes point lookups (#470)

[kriszyp]

Fixes #470

Root cause (corrected — supersedes this PR's first revision)

The system database is RocksDB, whose get() returns a MaybePromise: synchronous when the
row is in the block cache / memtable, a Promise on a cache-miss disk read. The replication
point-lookups in knownNodes.ts assumed a synchronous (LMDB-style) get() and read .replicates /
.name off the result directly. Once system grows past what stays in block cache, the hdb_nodes rows
are evicted, get() returns a Promise, and (promise)?.replicates is undefined — silently disabling
shouldReplicateFromNode's self-gate (the isDesired predicate for findWedgedNodeUrls/reconcileWorkers
and onDatabase re-subscribe), resolveNodeForAuth, and the change-stream fullRecord reads. That
breaks replication recovery: a still-desired peer is excluded from all re-drive/re-subscribe with no
signal (the observed preprod wedge — connected:false, retries:0). Confirmed live via CDP:
ps.get(name) → Promise, await ps.get(name) / ps.getSync(name) → the full pristine record.

Change

Use getSync() — the synchronous point read already used throughout core/resources/Table.ts and
search.ts — at the four affected sites:

• selfNodeReplicates → store.getSync(name)?.replicates (a one-line testable seam; the scan-fallback
  helper from the first revision is removed).
• resolveNodeForAuth → synchronous getSync point read (the range-visible { name } reconstruction
  remains only as defense-in-depth and should now be rare).
• the two change-stream fullRecord reads (get() → getSync()).

A deleted/absent row yields null/undefined (?. → falsy, correct); a genuine replicates: false
is preserved.

Tests

unitTests/replication/selfNodeReplicates.test.mjs rewritten to model the real RocksDB MaybePromise
contract: a store stub whose get() returns a Promise on a block-cache miss but getSync() returns the
value. The bug-closing case asserts that the un-awaited get(...)?.replicates is undefined (the old
hazard) while selfNodeReplicates returns the correct value via getSync; plus cache-hit, replicates
object, genuine false preserved, absent → undefined, and clean-null tombstone → undefined (no revive).
readNodeForAuth.test.mjs stub updated to expose getSync. Full replication unit suite: 185 passing,
0 failing. Built via tsc (only pre-existing _socket/etc. errors in replicationConnection.ts,
none in changed files).

Notes

• Broader than one cluster — any RocksDB-backed system has these unawaited sync point reads; it
  surfaces once system exceeds block-cache size. Worth a guard test asserting the self-gate works on a
  RocksDB-backed system.
• harper#1463 reframed (no decode bug; sync-vs-async). The #352 narrative in fix(replication): decode-resilient outbound subscriptions + idempotent node-update watcher on deploy reload (#460) #461's surrounding
  comments is superseded — a comment-only cleanup can follow.

🤖 Generated with Claude Code

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[claude]

Reviewed; no blockers found.

[kriszyp]

Cross-model review (Codex) — dispositions

Codex raised three findings against the first revision; two were real and are now fixed, one is declined with reasoning.

• ✅ Fixed (High): probeNodeRow still used store.get(key). On a RocksDB cache miss a tombstone is Promise<null>, so record == null was false → misclassified as decode-failure → a removed node could be reconstructed/revived. Now getSync.
• ✅ Fixed (Low): storeRecordRangeVisible's fallback store.get(name) != null would be truthy for a Promise<...> on a cache miss, making an absent key look range-visible. Now getSync.
• ⏸️ Declined (the self-record fallback removal): Codex flagged that dropping the scan/default fallback in selfNodeReplicates is unsafe "unless core guarantees point getSync decodes every row the scan path can." It does, by construction: getSync and getRange use the same decoder instance (decoder === encoder, CDP-verified) and the same shared-structures array — so getSync decodes any row the scan path decodes. The "range decodes / point doesn't" symptom was never a decode failure; it was the async Promise (await get() and getSync() both return the full, valid record; structures present and correct). Removing the fallback returns selfNodeReplicates to its original pre-fix(replication): synchronous getSync for hdb_nodes point lookups (#470) #474 semantics (get(self)?.replicates), which is not a regression — the scan-fallback was added in this PR's misdiagnosed first revision. If a row were ever genuinely undecodable (real corruption), getRange would fail identically, so a getRange-based fallback couldn't help anyway.

Full replication unit suite: 185 passing, 0 failing after both fixes.

[kriszyp]

Superseded by #476, which folds this PR's knownNodes.ts getSync sweep + selfNodeReplicates helper + tests in verbatim and extends the same fix across the rest of replication (subscriptionManager, replicator, clusterStatus) plus the core submodule (HarperFast/harper#1473).

Nice call converging on the MaybePromise root cause — that reframing (the #352 "empty object" being a pending Promise) turned out to be the crux, and #476 carries it everywhere. Closing in favor of #476.

— KrAIs (Claude Opus 4.8)