fix(plugin): never read an empty peer list as "this node owns every URL"

packages/plugin/README.md

@@ -830,6 +830,21 @@ land where no claim will look. Such a row is recovered only on the next floor re
 (`queue.claimFloor.resetInterval`), and until then the URL silently does not render. Use
 `POST /prerender_admin/revalidate` — or the button — which writes through the funnel.
 
+### Where residency comes from
+
+Ownership is rendezvous-hashed over the cluster's node names, read from `server.nodes` on every
+call. It is read per call, and never snapshotted, because `server.nodes` is not a stable list:
+Harper initialises it empty and fills it asynchronously from `hdb_nodes`, replaces it while
+rebuilding after a subscription restart, and briefly drops a node while applying an update to it.
+
+**An empty peer list is treated as "not known yet", never as "this node is alone."** With no
+peers, rendezvous hashing makes the local node the owner of every URL — so every schedule row
+would be stored locally rather than routed, where nothing claims it correctly and nothing removes
+it. The last non-empty list therefore wins over an empty one, while any non-empty list is adopted
+at once so a real membership change still takes effect. A node that has never seen a peer logs a
+warning and owns everything, which is correct for a single-node deployment and a misconfiguration
+on a cluster.
+
 ### Schedule repair: the half-written target
 
 A `RenderTarget` and its `RenderSchedule` row live in **separate databases**, so creating a

packages/plugin/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@harperfast/prerender",
-	"version": "0.50.0",
+	"version": "0.50.1",
 	"type": "module",
 	"description": "Configurable Harper plugin for prerendering pages for bots and crawlers",
 	"license": "Apache-2.0",

packages/plugin/src/util/peer.js

@@ -23,7 +23,7 @@
  */
 
 import { config } from '../config.js';
-import { nodes } from './residency.js';
+import { getNodes } from './residency.js';
 
 // Peers are reached over TLS: the Harper HTTP port serves TLS in every real deployment, and
 // the node certificates chain to a publicly-trusted CA, so standard validation applies (no
@@ -57,7 +57,7 @@ export const peerOrigin = (hostname) => {
  * not weaken the guard — membership in the known set is still required.
  */
 export const isKnownNode = (hostname) =>
-	typeof hostname === 'string' && nodes.some((node) => node.toLowerCase() === hostname.toLowerCase());
+	typeof hostname === 'string' && getNodes().some((node) => node.toLowerCase() === hostname.toLowerCase());
 
 /**
  * The subset of the caller's headers to forward. Only credentials, nothing else — the peer

packages/plugin/src/util/residency.js

@@ -1,12 +1,89 @@
 import { fnv1a32 } from './hash.js';
 
-export const nodes = [server.hostname, ...(server.nodes?.map(({ name }) => name) ?? [])].sort();
-
 /**
  * Rendezvous (HRW) hashing: deterministically picks the node responsible for a
  * given URL, so every node agrees on the owner without coordination.
+ *
+ * THE NODE LIST MUST BE READ LAZILY, AND AN EMPTY PEER LIST IS NEVER AN ANSWER.
+ *
+ * `server.nodes` is derived state, not configuration. harper-pro initialises it to `[]` at
+ * module scope and only fills it when `subscribeToNodeUpdates` runs a full `hdb_nodes` scan
+ * (`replication/knownNodes.ts`), so anything evaluated before that scan sees no peers on a
+ * healthy multi-node cluster — and that ordering repeats for every worker start, not only the
+ * first. Later rebuilds do empty and refill the array, but synchronously (the scan is a plain
+ * `for...of`, and the per-update path filters and pushes before its first await), so they are
+ * atomic to any observer and are NOT a window of their own. What they establish is that this
+ * list is rebuilt from a table rather than fixed at boot, which is why capturing it once is
+ * wrong in principle and not merely unlucky.
+ *
+ * With no peers, HRW makes this node the owner of every URL. `RenderSchedule.setResidencyById`
+ * then reports self for every key, so Harper stores every schedule row LOCALLY instead of
+ * routing it to its owner (`core/resources/Table.ts`, which omits the local record only when
+ * the computed residency excludes `server.hostname`). That is silent and permanent: nothing
+ * deletes a schedule row from a node that does not own it, the owner never gets the row, and
+ * the non-owner re-claims and re-renders it forever because its own reschedule routes away.
+ * Capturing the list at module load made one unlucky evaluation poison a worker for its whole
+ * lifetime, which has been observed in production as a multi-percent share of a corpus.
+ *
+ * So: the last non-empty list wins over an empty one, and any non-empty list is adopted
+ * immediately (a genuine membership change must still take effect). Before any peer has ever
+ * been seen, self-only is the only answer available — and the correct one for a single-node
+ * deployment, which is why this warns rather than throws.
  */
+
+// Cache identity AND length: `knownNodes.ts` both reassigns `server.nodes` (filter-then-push)
+// and pushes onto the existing array, so neither check alone sees every change. Steady state is
+// two comparisons, which matters because the reconcile and orphan sweeps call this once per row
+// over the whole corpus. It would miss a same-length rename mutated into the array in place;
+// `knownNodes.ts` has no such path, and every mutation it does perform changes one or the other.
+let cachedFrom;
+let cachedLength = -1;
+let resolvedNodes = [];
+let selfOnly;
+let warnedPeerless = false;
+
+function currentNodes() {
+	const peers = server.nodes;
+	const length = peers?.length ?? 0;
+
+	if (length !== 0 && (peers !== cachedFrom || length !== cachedLength)) {
+		cachedFrom = peers;
+		cachedLength = length;
+		// A decode-miss descriptor can reach `server.nodes` without a name. Left in, it sorts
+		// into the ring as `undefined` and — when it wins the hash — makes this function return
+		// undefined, which Harper reads as a residency that excludes every node and stores the
+		// record nowhere at all.
+		const names = peers.map((node) => node?.name).filter(Boolean);
+		// Latch only a list that actually contains a peer: a populated `server.nodes` that
+		// yields no usable name is "not known yet" for the same reason an empty one is, and
+		// must not be allowed to overwrite a good list with a self-only one.
+		if (names.length) resolvedNodes = [...new Set([server.hostname, ...names])].sort();
+	}
+
+	if (resolvedNodes.length) return resolvedNodes;
+
+	if (!warnedPeerless) {
+		warnedPeerless = true;
+		// `globalThis.logger`, not a bare `logger`: optional chaining does not guard an
+		// UNDECLARED identifier, and a residency throw makes Harper drop the record — the
+		// diagnostic must not be able to break the decision it reports on.
+		globalThis.logger?.warn?.(
+			`[prerender] residency has never seen a peer, so ${server.hostname} maps to itself for every URL. ` +
+				`Schedule rows written now are stored locally instead of on their owner, and nothing removes ` +
+				`them later. Expected only on a single-node deployment; on a cluster it means this process ` +
+				`started before hdb_nodes was populated.`
+		);
+	}
+
+	// Cached like the peered list: a single-node deployment takes this branch on every call.
+	return (selfOnly ??= [server.hostname]);
+}
+
+/** The cluster's known node names, self included. Sorted, deduplicated, never empty. */
+export const getNodes = () => currentNodes();
+
 export function getResidencyByUrl(url) {
+	const nodes = currentNodes();
 	let bestIdx = 0;
 	let bestScore = -1;
 

packages/plugin/test/residency.test.js

@@ -0,0 +1,205 @@
+import { test, beforeEach, afterEach } from 'node:test';
+import assert from 'node:assert/strict';
+
+/**
+ * Residency is the shard map for the whole RenderSchedule keyspace, and a wrong answer is
+ * silent: the row is stored on a node that does not own it, the owner never gets it, and
+ * nothing deletes it. The properties pinned here are therefore about WHERE the node list comes
+ * from — an empty `server.nodes` must never read as "this node owns everything" — plus the
+ * mapping itself, so no refactor can re-shard a live cluster.
+ */
+
+const A = 'node-a.example.com';
+const B = 'node-b.example.com';
+const C = 'node-c.example.com';
+const SELF = 'node-d.example.com';
+const ALL = [A, B, C, SELF].sort();
+
+let warnings;
+let freshCounter = 0;
+
+const setServer = (hostname, peerNames) => {
+	globalThis.server = { hostname, nodes: peerNames === null ? undefined : peerNames.map((name) => ({ name })) };
+};
+
+// Each test gets its own module instance so "never seen a peer" and the warn-once latch are
+// testable; every OTHER test proves the module needs no reload to pick a change up.
+const freshResidency = () => import(`../src/util/residency.js?fresh=${freshCounter++}`);
+
+beforeEach(() => {
+	warnings = [];
+	globalThis.logger = { warn: (m) => warnings.push(String(m)), error: () => {}, trace: () => {}, debug: () => {} };
+});
+
+afterEach(() => {
+	delete globalThis.server;
+	delete globalThis.logger;
+});
+
+test('a peer list that appears AFTER module load is picked up without a reload', async () => {
+	setServer(SELF, []);
+	const { getResidencyByUrl, getNodes } = await freshResidency();
+
+	// The load-time snapshot bug: with no peers, self owns everything.
+	assert.equal(getResidencyByUrl('https://example.com/a'), SELF);
+	assert.deepEqual(getNodes(), [SELF]);
+
+	globalThis.server.nodes = [A, B, C].map((name) => ({ name }));
+
+	assert.deepEqual(getNodes(), ALL);
+});
+
+test('every node computes the SAME owner from the same membership', async () => {
+	const url = 'https://example.com/catalog?page=2';
+	const verdicts = [];
+	for (const self of ALL) {
+		setServer(
+			self,
+			ALL.filter((h) => h !== self)
+		);
+		const { getResidencyByUrl } = await freshResidency();
+		verdicts.push(getResidencyByUrl(url));
+	}
+	assert.equal(new Set(verdicts).size, 1, 'nodes disagreed on the owner');
+});
+
+test('the keyspace mapping is unchanged', async () => {
+	// A CHANGE-DETECTOR ON PURPOSE. These pairs are computed by the current FNV-1a + HRW pair,
+	// and one key is pinned per node so the assertion also proves the ring still spreads rather
+	// than collapsing onto self. Any edit that moves one of them re-shards every RenderSchedule
+	// row on every running cluster — the hazard util/hash.js already warns about for the mixing
+	// constant. If one of these fails, that is the finding; do not update the expectation.
+	setServer(SELF, [A, B, C]);
+	const { getResidencyByUrl } = await freshResidency();
+	assert.equal(getResidencyByUrl('https://example.com/catalog?page=0'), A);
+	assert.equal(getResidencyByUrl('https://example.com/catalog?page=2'), B);
+	assert.equal(getResidencyByUrl('https://example.com/catalog?page=8'), C);
+	assert.equal(getResidencyByUrl('https://example.com/catalog?page=3'), SELF);
+});
+
+test('self appearing in server.nodes does not re-shard the keyspace', async () => {
+	// server.nodes is meant to hold PEERS only, but self has leaked into it before
+	// (harper-pro#489). Deduplicating must not move a single key: HRW takes the max score over
+	// the NAMES present, so a repeated name cannot change which name wins.
+	setServer(SELF, [A, B, C]);
+	const clean = await freshResidency();
+	setServer(SELF, [A, B, C, SELF]);
+	const withSelf = await freshResidency();
+
+	assert.deepEqual(withSelf.getNodes(), clean.getNodes());
+	for (let i = 0; i < 500; i++) {
+		const url = `https://example.com/p/${i}`;
+		assert.equal(withSelf.getResidencyByUrl(url), clean.getResidencyByUrl(url), url);
+	}
+});
+
+test('a transient empty peer list does not move ownership', async () => {
+	setServer(SELF, [A, B, C]);
+	const { getResidencyByUrl, getNodes } = await freshResidency();
+	const url = 'https://example.com/catalog?page=2';
+	assert.equal(getResidencyByUrl(url), B);
+
+	// knownNodes.ts assigns a fresh empty array while rebuilding from hdb_nodes.
+	globalThis.server.nodes = [];
+	assert.equal(getResidencyByUrl(url), B, 'ownership moved to self during a rebuild window');
+	assert.deepEqual(getNodes(), ALL);
+
+	globalThis.server.nodes = undefined;
+	assert.equal(getResidencyByUrl(url), B, 'ownership moved to self when server.nodes went away');
+});
+
+test('a genuine membership change IS adopted', async () => {
+	setServer(SELF, [A, B, C]);
+	const { getNodes } = await freshResidency();
+	getNodes();
+
+	// One node retired: still a non-empty list, so it must take effect.
+	globalThis.server.nodes = [B, C].map((name) => ({ name }));
+	assert.deepEqual(getNodes(), [B, C, SELF].sort());
+	assert.ok(!getNodes().includes(A));
+});
+
+test('a push onto the existing array is seen (same identity, new length)', async () => {
+	setServer(SELF, [A]);
+	const { getNodes } = await freshResidency();
+	assert.deepEqual(getNodes(), [SELF, A].sort());
+
+	// knownNodes.ts pushes onto server.nodes rather than reassigning it.
+	globalThis.server.nodes.push({ name: C });
+	assert.deepEqual(getNodes(), [A, C, SELF].sort());
+});
+
+test('a nameless node descriptor is never returned as an owner', async () => {
+	// knownNodes.ts can push a reconstructed descriptor with no name. Left in the ring it wins
+	// the hash for some keys, and returning undefined makes Harper store the row nowhere.
+	setServer(SELF, [A, B, C]);
+	globalThis.server.nodes.push({ name: undefined }, {}, null);
+	const { getResidencyByUrl, getNodes } = await freshResidency();
+
+	assert.deepEqual(getNodes(), ALL);
+	for (let i = 0; i < 300; i++) {
+		const owner = getResidencyByUrl(`https://example.com/p/${i}`);
+		assert.ok(getNodes().includes(owner), `key ${i} resolved to ${owner}`);
+	}
+});
+
+test('a populated peer list with no usable names is treated as peerless', async () => {
+	// A decode miss can put descriptors in server.nodes that carry no name. A list of nothing
+	// but those is "not known yet", not "this node is alone" — it must warn, and it must not
+	// overwrite a good list with a self-only one.
+	setServer(SELF, []);
+	globalThis.server.nodes = [{ name: undefined }, {}, null];
+	const { getResidencyByUrl, getNodes } = await freshResidency();
+
+	assert.deepEqual(getNodes(), [SELF]);
+	assert.equal(warnings.filter((w) => w.includes('never seen a peer')).length, 1);
+
+	globalThis.server.nodes = [A, B, C].map((name) => ({ name }));
+	const url = 'https://example.com/catalog?page=2';
+	assert.equal(getResidencyByUrl(url), B);
+
+	// ...and a later all-nameless list must not clobber the good one.
+	globalThis.server.nodes = [{ name: undefined }, {}];
+	assert.equal(getResidencyByUrl(url), B);
+	assert.deepEqual(getNodes(), ALL);
+});
+
+test('a peerless residency decision warns exactly once', async () => {
+	setServer(SELF, []);
+	const { getResidencyByUrl } = await freshResidency();
+	for (let i = 0; i < 5; i++) getResidencyByUrl(`https://example.com/${i}`);
+
+	const peerless = warnings.filter((w) => w.includes('never seen a peer'));
+	assert.equal(peerless.length, 1);
+	assert.match(peerless[0], /stored locally instead of on their owner/);
+});
+
+test('a single-node deployment resolves to itself without repeated warnings', async () => {
+	setServer(SELF, null);
+	const { getResidencyByUrl, getNodes } = await freshResidency();
+	assert.equal(getResidencyByUrl('https://example.com/a'), SELF);
+	assert.deepEqual(getNodes(), [SELF]);
+	assert.equal(warnings.filter((w) => w.includes('never seen a peer')).length, 1);
+	// This branch is taken on EVERY call here, so it must not allocate either.
+	assert.equal(getNodes(), getNodes());
+});
+
+test('an unchanged peer list is not rebuilt per call', async () => {
+	// The reconcile and orphan sweeps call this once per row across the whole corpus, so the
+	// steady-state path must not allocate.
+	setServer(SELF, [A, B, C]);
+	const { getNodes } = await freshResidency();
+	assert.equal(getNodes(), getNodes(), 'the node list was rebuilt for an unchanged peer list');
+});
+
+test('isKnownNode accepts a peer that joined after module load', async () => {
+	// The same stale snapshot made peer.js reject legitimate peers, breaking the explainer's
+	// cross-node fetch. It reads through the same accessor, so it heals with it.
+	globalThis.server = { hostname: SELF, nodes: [], config: { http: { port: 9925, securePort: 9926 } } };
+	const peer = await import(`../src/util/peer.js?fresh=${freshCounter++}`);
+	assert.equal(peer.isKnownNode(C), false);
+
+	globalThis.server.nodes = [{ name: C }];
+	assert.equal(peer.isKnownNode(C), true);
+	assert.equal(peer.isKnownNode('evil.example.com'), false);
+});