test(cluster): pin remove_node blast-radius durability across restart (F-225)

[kriszyp]

One exploratory-QA spec promoted into the tracked cluster suite. Test-only — no product code touched.

What it pins

integrationTests/cluster/removeNodeBlastRadius.test.mjs (from QA-758, candidate P-532 — regression anchor for finding F-225)

F-225 established that the reciprocal remove-node operation is registered under the literal string 'remove_node_back;' (stray semicolon, core replication/setNode.ts), so it can never match an incoming operation: 'remove_node_back'. The consequence is asymmetric removal: when A calls remove_node(B), A drops its own hdb_nodes row for B and best-effort-sends remove_node_back to B — which B can never handle — so B's row for A, and B's live replication connection to A, survive.

This spec pins the durability half of that blast radius, in both directions, as hard assertions:

• B's stale peer state survives a restart of B (re-passing B's original config, so the replication.databases filter isn't silently wiped).
• The removal itself survives a restart of A, the remover.

hdb_nodes is a persisted system-database table, so both answers should be restart-invariant — this is a row-persistence property, not a process-lifetime one. Pinning it means a future fix to the operation name can't quietly regress into "removal works until you restart."

One leg is deliberately observational, not asserted

The (b) leg simulates the fixed end-state (delete B's hdb_nodes row for A directly) and then has B re-subscribe to A. It still succeeds today, because SUBSCRIPTION_REQUEST handling gates on the connection-level authorization captured at connect time and then falls through to a reactive watch on future hdb_nodes change events — there is no synchronous point-read of hdb_nodes for the requesting peer at that moment.

That is a separate, currently-live bug (tracked as F-228 / D-226). So this leg computes the observation and console.logs it, with no assertion: asserting the observed accept would freeze a known defect into CI, and asserting the refusal it should eventually produce would fail today. Its own asserted regression test lands with the fix. The leg's setup preconditions (row exists → row deleted) remain hard-asserted, since those aren't the disputed behavior. This follows the same precedent as hdbNodesReloadSendAuth's positive-control leg in #615.

Verification

• 4/4 green on three cold runs in the promoted location on current main — 14.3 s / 18.1 s / 16.6 s. Two nodes, no bulk data.
• Re-run again after the header/idiom edits rather than assuming comment-only edits are behaviour-neutral.
• prettier --check clean and oxlint --deny-warnings exit 0, both run from inside the repo tree (not at the snapshot path, where prettier would silently fall back to defaults).
• No hard-coded developer-machine paths; teardownHarper runs unconditionally for both nodes.
• requires-isolation: yes — it kills and restarts nodes and mutates topology, so it stays its own file rather than joining a shared instance.

Not duplicative: deleteResurrectionOnRejoin covers record resurrection on rejoin, and addNodeLeaderNoMeshLeak / systemDbExcludedPeerChurn assert different properties of node membership. Nothing in integrationTests/cluster/ asserted restart-durability of the removal on either side.

Opened as a single-spec PR: the sibling qa-promote PRs (#612 replication/failover/blob-copy, #615 node-lifecycle auth) are both at their batch cap and this is the only currently gate-passing candidate in the cluster bank.

—
Reviewed and opened by KrAIs (Claude Opus 5) as part of the qa-explorer promotion pipeline.

🤖 Generated with Claude Code

[kriszyp]

Closing as a duplicate — integrationTests/cluster/removeNodeBlastRadius.test.mjs is already promoted in #615 ("test(cluster): pin three node-lifecycle invariants"), which I missed before opening this.

Beyond the duplication, #615's version is the better one and should be the one that lands. This PR's (a) leg asserts that B keeps replicating after being removed — i.e. it pins the F-225 defect itself, and would turn red the moment the remove_node_back; name is fixed. #615 deliberately only logs B's post-removal behavior for exactly that reason, and asserts just the remover-side durability, which is fix-agnostic. No action needed here; review effort belongs on #615.

— KrAIs (Claude Opus 5), qa-explorer promotion pipeline

[gemini-code-assist]

Code Review

This pull request adds a new integration test suite (removeNodeBlastRadius.test.mjs) to verify the durability and blast radius of the remove_node_back bug (F-225). The feedback suggests avoiding CommonJS globals like module.path in ES modules by using an ESM-safe fallback, and assigning asynchronous process instances within their respective promise chains inside Promise.all to ensure reliable cleanup during teardown if a startup fails.

[gemini-code-assist]

In ES modules (ESM), avoid using CommonJS-specific globals like module (e.g., module.path) as a fallback when import.meta.dirname is undefined. Use an ESM-safe fallback such as new URL('.', import.meta.url).pathname instead.

Suggested change

	import.meta.dirname ?? module.path,
	import.meta.dirname ?? new URL('.', import.meta.url).pathname,

References

1. In ES modules (ESM), avoid using CommonJS-specific globals like module (e.g., module.path) as a fallback when import.meta.dirname is undefined. Use an ESM-safe fallback such as new URL('.', import.meta.url).pathname instead.

[gemini-code-assist]

When starting multiple asynchronous processes in parallel using Promise.all, assign the successfully started instances within their respective promise chains (e.g., using .then()) rather than waiting for Promise.all to resolve. This ensures that if one process fails, the already started processes are still recorded and can be properly cleaned up during teardown.

Suggested change

	await Promise.all([
	startHarper(ctxA, nodeStartOptions(ctxA.harper)),
	startHarper(ctxB, nodeStartOptions(ctxB.harper)),
	]);
	ctx.nodeA = ctxA.harper;
	ctx.nodeB = ctxB.harper;
	await Promise.all([
	startHarper(ctxA, nodeStartOptions(ctxA.harper)).then(() => { ctx.nodeA = ctxA.harper; }),
	startHarper(ctxB, nodeStartOptions(ctxB.harper)).then(() => { ctx.nodeB = ctxB.harper; }),
	]);

References

1. When starting multiple asynchronous processes in parallel using Promise.all, assign the successfully started instances within their respective promise chains (e.g., using .then()) rather than waiting for Promise.all to resolve. This ensures that if one process fails, the already started processes are still recorded and can be properly cleaned up during teardown.

[claude]

PR was self-closed by the author as a duplicate of #615, which carries the same spec with a fix-agnostic assertion set. No action needed.