Fix flaky de-auth positive control: remove_node revokes locally before notifying the peer

[kriszyp]

Fixes the flaky positive control in hdbNodesReloadSendAuth.test.mjs ("remove_node still genuinely de-authorizes a peer", failed on the 2026-08-22 07:10 UTC main run, shard 4/6, Node 26.5.0) by fixing the ordering defect in remove_node that the flake exposed, plus a test-side hardening.

Root cause

remove_node sent remove_node_back to the peer being removed and awaited the peer's response (sendOperationToNode resolves on OPERATION_RESPONSE) before deleting the peer's hdb_nodes row locally. That hands the removed peer a full round trip's head start: it deletes its self row and its cooperative teardown closes its dialed sockets (1008 No longer subscribed) — and those sockets are exactly the server-side sockets on the removing node where the dynamic send-authorization watch lives. The watch can then only produce its 1008 Unauthorized database subscription close if one of two knife-edge races is won:

• the peer's self-row tombstone replicates back and fires the watch before the peer's own close frames land, or
• the local hdb_nodes delete event reaches the watch before those close frames land.

In the failing CI job both were lost (margins observed locally are ≈1–3 ms): every socket closed 1008 No longer subscribed within ~30 ms and the Unauthorized database subscription signature never appeared — not even 48 s later, since a closed socket's watch is retired. The test's single post-disconnect log read then failed the positive control.

Timeline from the failing job (A = 127.0.0.2 removed by B = 127.0.0.3):

07:15:49.981 B main/0   remove_node { hostname: 127.0.0.2 }
07:15:49.984 B main/0   remove_node_back sent to A (awaited)
07:15:49.985 A http/1   remove_node_back processed → A deletes self row, tears down
07:15:49.994 B http/1   755-1s (A-dialed, watch-bearing) sockets closed 1008 No longer subscribed  ← A's FINs
07:15:50.00x B main/0   local hdb_nodes delete + "Node was deleted, unsubscribing"                 ← too late
             (no "Unauthorized database subscription" ever logged on any node)

Product fix (replication/setNode.ts)

Delete the local hdb_nodes row first, then send remove_node_back. Beyond stabilizing the close signature, this is the right revocation order on its own merits:

• Local revocation no longer waits on a round trip to a peer that may be slow, wedged, or offline — previously an unreachable peer delayed local de-authorization by the full connect/response timeout while it was being warned about ("it may not even be online anymore" — the code contradicted its own comment).
• The send-auth watch now consumes the local delete event before the peer can possibly have learned of the removal, so the Unauthorized database subscription close is produced deterministically instead of racing the peer's cooperative teardown.

remove_node_back travels on its own freshly-dialed operation connection and the peer authorizes it against the removing node's row (still present on the peer), so the reorder does not affect its delivery; the reciprocal self-row deletion on the peer is unchanged and still awaited before remove_node returns.

Test fix (integrationTests/cluster/hdbNodesReloadSendAuth.test.mjs)

The positive control now polls (20 s deadline, 250 ms interval) a post-remove_node log slice for either of the gate's two signatures — the Unauthorized database subscription close reason, or the same watch's warn-level hdb_nodes no longer authorizes line on the removing node — instead of sampling the full logs once. The slice prevents a vacuous pass from an earlier marker in isolated runs; the warn-line alternative removes a silent dependency on debug-level logging (the close reason only ever appears in debug lines). Both signatures come only from the send-auth watch, so a disarmed gate still turns the control red (re-demonstrated below).

Verification

• Fails on base / mechanism reproduction: the failing CI run's own logs show every close as 1008 No longer subscribed and no Unauthorized database subscription line even 48 s after the assertion. Locally, amplifying the peer's head start by 30 ms (a dist-only patch of the old ordering, simulating a contended main thread) under CPU load (12 spinner processes on 20 cores) reproduced the exact assertion failure in 2/6 iterations, each with the CI signature: zero marker lines anywhere, and the new 20 s marker poll exhausted — the marker never appears once the watch-bearing sockets die; it is not merely late.
• Fix validation: with the fix, 12/12 iterations of the full test file pass under the same CPU load (plus an unamplified baseline run); the marker is logged within ~10 ms of remove_node, before the peer can have learned of the removal.
• Control is still a genuine control: with the gate deliberately disarmed (shouldCloseSendAuthWatch patched in dist to always return false), the positive control fails: the main reload-marker test stays green and the positive control fails after the full 20 s poll — so the control still turns red if the gate stops closing.
• Suites: unit suite (npm run test:unit): 826 passing. Targeted cluster integration suites covering remove_node/send-gate behavior — removeNodeBlastRadius, replicationTopology, reverseBaseCopyDeclined, replicationReconnect, blockCacheEviction, systemDbDynamicSendGate, systemDbExcludedPeerChurn — 22/22 passing. Full test:integration:all left to PR CI.

Refs the 2026-08-22 Integration Tests failure on main (run 32558915650, sha d2a193f).

For the human reviewer

These are the decision-ledger items from the independent review (codex + cursor-grok + harper-domain; verdict COMMENTS, adjudicated severity minor; all 3 findings fixed pre-push):

• Reorder vs test-tolerance: local revocation now precedes peer notification. Independently defensible as a security-window fix (a removed peer's send authorization no longer outlives a round trip to that peer, including the connect timeout of an offline one), but it does change cluster-membership ordering in service of stabilizing a control; reversing is a one-line move. No repo dependency on the old order found (QA-758 removeNodeBlastRadius asserts end-state only, and passes).
• Await vs fire-and-forget for remove_node_back: kept awaited, so remove_node's success response still means "both sides attempted cleanup" and the offline-peer warning lands before returning. The deleted stale comment claimed fire-and-forget was the intent; switching is cheap but is an API-semantics call I did not make unilaterally.
• Crash window direction: if the removing node crashes after the local delete but before the notify, the peer keeps its row and reconnect-loops into 1008 rejections with no scheduled reconciliation (only warn-line evidence). That replaces the previous inverse window (peer told to stop while we still authorized it). Fail-closed is the right default, but the orphaned-peer-row cleanup gap predates and survives this change.
• Residual control fragility: the gate signature now requires only that the removing node's replication worker drain one in-process change event faster than a full network round trip + peer teardown — structurally favored (12/12 under load), not the previous ~1–3 ms coin flip — but it is still a race, and a worker wedged for an entire RTT would fail the control.

Complexity: small — one ordering change in a cold administrative operation plus test hardening.

— Claude Fable 5

🤖 Generated with Claude Code

https://claude.ai/code/session_01GbV4RZzUbNLa97qTLpkcCb

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

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

[gemini-code-assist]

Code Review

This pull request updates the node removal process in 'replication/setNode.ts' to delete the node record locally before notifying the peer, ensuring that authorization is revoked locally first. It also updates the integration test 'hdbNodesReloadSendAuth.test.mjs' to poll and slice logs post-removal to handle a race condition between socket teardown paths. There are no review comments, so I have no feedback to provide.

[claude]

Reviewed; no blockers found.