From 1ccfa333c110b0cd17c52c781d9fe4af621ede2b Mon Sep 17 00:00:00 2001 From: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 01:56:11 -0400 Subject: [PATCH 1/3] fix(nps-relay): release the per-IP rate slot when the install limit refuses a submission Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- changelog.d/fixed-554-nps-relay-ip-slot.md | 2 ++ netlify/nps-relay/relay.test.ts | 35 ++++++++++++++++++++++ netlify/nps-relay/relay.ts | 26 ++++++++++++++-- 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 changelog.d/fixed-554-nps-relay-ip-slot.md diff --git a/changelog.d/fixed-554-nps-relay-ip-slot.md b/changelog.d/fixed-554-nps-relay-ip-slot.md new file mode 100644 index 00000000..d99de57f --- /dev/null +++ b/changelog.d/fixed-554-nps-relay-ip-slot.md @@ -0,0 +1,2 @@ +- The NPS relay no longer consumes the per-IP rate slot when the per-install limit + refuses a submission (hivecommons/docs#554). diff --git a/netlify/nps-relay/relay.test.ts b/netlify/nps-relay/relay.test.ts index 4df9f767..c53ee7fe 100644 --- a/netlify/nps-relay/relay.test.ts +++ b/netlify/nps-relay/relay.test.ts @@ -19,6 +19,7 @@ import { HEADER_TIMESTAMP, INSTALL_PREFIX, INSTALL_RATE_PREFIX, + IP_RATE_PREFIX, MAX_ACK_BODY_BYTES, MAX_ENTRIES, MAX_FEEDBACK_CHARS, @@ -673,6 +674,40 @@ describe("POST /api/nps (submit)", () => { expect(store.entries()).toHaveLength(MAX_PER_INSTALL_PER_WINDOW); }); + it("releases the per-IP slot when the install limit refuses the submission", async () => { + // The pre-check read lags one slot behind, so it passes and only the + // install reservation refuses; the IP slot taken just before must go back. + const stale = new (class extends MemoryStore { + async get(key: string): Promise { + const rec = await super.get(key); + if (!key.startsWith(INSTALL_RATE_PREFIX) || !rec) return rec; + const { times } = rec as { times: number[] }; + return { times: times.slice(1) }; + } + })(); + store = stale; + await registerOk(); + store.data.set(`${INSTALL_RATE_PREFIX}${hive.installId}`, { + times: Array.from({ length: MAX_PER_INSTALL_PER_WINDOW }, () => now - 1), + }); + const res = await handleRelayRequest(submit(GOOD), deps()); + expect(res.status).toBe(429); + const ipRecords = [...store.data.entries()].filter(([k]) => + k.startsWith(IP_RATE_PREFIX) + ); + for (const [, rec] of ipRecords) { + expect((rec as { times: number[] }).times).toEqual([]); + } + + const other = newHive(); + await registerOk(other, "198.18.0.2"); + const next = await handleRelayRequest( + submit(GOOD, { signer: other }), + deps() + ); + expect(next.status).toBe(201); + }); + it("omits feedback and dashboard_version from the entry when not supplied", async () => { const res = await handleRelayRequest( submit({ hive_id: "solo-hive", score: 2 }), diff --git a/netlify/nps-relay/relay.ts b/netlify/nps-relay/relay.ts index fa9b1103..3b503015 100644 --- a/netlify/nps-relay/relay.ts +++ b/netlify/nps-relay/relay.ts @@ -571,6 +571,20 @@ function reserveSlot( }); } +/** Gives back one slot taken by `reserveSlot` at `now`. */ +function releaseSlot( + store: RelayStore, + key: string, + now: number +): Promise { + return compareAndSwap(store, key, current => { + const times = liveTimes(current, now); + const i = times.indexOf(now); + if (i !== -1) times.splice(i, 1); + return { next: { times } satisfies RateRecord }; + }); +} + /** Whether a windowed rate record is already at `max`, without reserving. */ async function atLimit( store: RelayStore, @@ -835,8 +849,11 @@ async function handleSubmit( if (await atLimit(store, installKey, MAX_PER_INSTALL_PER_WINDOW, now)) return RATE_LIMITED(); - // Reserve both slots atomically before storing the entry, so concurrent - // submissions cannot all squeeze into the same remaining slot. + // Reserve each slot with its own conditional write before storing the + // entry, so concurrent submissions cannot all squeeze into the same + // remaining slot. The two writes are not atomic together: if the install + // reservation is refused, the IP slot is released so a rejected submission + // does not consume the independent IP quota. const ipDenied = await reserveSlot(store, ipKey, MAX_PER_IP_PER_WINDOW, now); if (ipDenied) return ipDenied; const installDenied = await reserveSlot( @@ -845,7 +862,10 @@ async function handleSubmit( MAX_PER_INSTALL_PER_WINDOW, now ); - if (installDenied) return installDenied; + if (installDenied) { + await releaseSlot(store, ipKey, now); + return installDenied; + } const entry: RelayEntry = { id: newEntryId(now), From d0007c3b33d83a9b97b405964b572552848c9daa Mon Sep 17 00:00:00 2001 From: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 01:03:44 -0500 Subject: [PATCH 2/3] test(nps-relay): keep the stale read confined to the pre-check The stale-store double in the release test overrode get(), which MemoryStore.getWithMetadata also routes through, so the install reservation read the same lagging record and accepted the submission. Only the atLimit pre-check should lag; the conditional read must see the full record so the reservation refuses and the IP slot is released. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- netlify/nps-relay/relay.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/netlify/nps-relay/relay.test.ts b/netlify/nps-relay/relay.test.ts index c53ee7fe..6d3a2540 100644 --- a/netlify/nps-relay/relay.test.ts +++ b/netlify/nps-relay/relay.test.ts @@ -675,8 +675,9 @@ describe("POST /api/nps (submit)", () => { }); it("releases the per-IP slot when the install limit refuses the submission", async () => { - // The pre-check read lags one slot behind, so it passes and only the - // install reservation refuses; the IP slot taken just before must go back. + // The pre-check read (plain get) lags one slot behind, so it passes and + // only the install reservation (conditional read via getWithMetadata) + // refuses; the IP slot taken just before must go back. const stale = new (class extends MemoryStore { async get(key: string): Promise { const rec = await super.get(key); @@ -684,6 +685,14 @@ describe("POST /api/nps (submit)", () => { const { times } = rec as { times: number[] }; return { times: times.slice(1) }; } + + async getWithMetadata( + key: string + ): Promise<{ data: unknown; etag?: string } | null> { + const data = await super.get(key); + if (data === null) return null; + return { data, etag: this.etags.get(key) }; + } })(); store = stale; await registerOk(); From 33495ba5a3f3a89f683af9d4f4e04708f8307879 Mon Sep 17 00:00:00 2001 From: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 01:07:17 -0500 Subject: [PATCH 3/3] test(nps-relay): assert the install record is untouched after the refusal The release test now also checks that the install rate record still holds exactly MAX_PER_INSTALL_PER_WINDOW stamps after the 429, proving the conditional reservation refused rather than writing an extra slot. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- netlify/nps-relay/relay.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/netlify/nps-relay/relay.test.ts b/netlify/nps-relay/relay.test.ts index 6d3a2540..5eacb181 100644 --- a/netlify/nps-relay/relay.test.ts +++ b/netlify/nps-relay/relay.test.ts @@ -701,6 +701,10 @@ describe("POST /api/nps (submit)", () => { }); const res = await handleRelayRequest(submit(GOOD), deps()); expect(res.status).toBe(429); + const installRec = store.data.get( + `${INSTALL_RATE_PREFIX}${hive.installId}` + ) as { times: number[] }; + expect(installRec.times).toHaveLength(MAX_PER_INSTALL_PER_WINDOW); const ipRecords = [...store.data.entries()].filter(([k]) => k.startsWith(IP_RATE_PREFIX) );