Repository navigation
🐛 fix(nps-relay): release the per-IP slot and day count when the create-only registration write loses the race - #560
Conversation
…te-only registration write loses the race Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
…ier config Co-authored-by: Copilot <[email protected]> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for hivecommons-docs canceled.
|
|
Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor. hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 Evidence bundle `#560e028429` sha256:cce889d9a0280b6269c621b3a97d7cb0f4ed2f113f51588e4b884676cfd6bdd4 — download from the dashboard or `hivectl review evidence #560`. |
There was a problem hiding this comment.
Logic looks correct to me; the only thing blocking this is a Prettier failure in the new test.
Read netlify/nps-relay/relay.ts at b396799. The losing-write branch (relay.ts:784-792) now calls releaseSlot(store, ipKey, now) and a compareAndSwap decrement on dayKey, which is exactly the pair reserved at relay.ts:750-765 and mirrors the existing dayDenied release at relay.ts:766-769. releaseSlot (relay.ts:575-586) removes the now timestamp by value, so releasing the slot reserved in this same request is safe even when other live timestamps exist, and Math.max(0, count - 1) keeps the counter from going negative if the record was reset in between. The new test drives the lost race through a RelayStore wrapper that fails only onlyIfNew writes on INSTALL_PREFIX keys, so the IP and day CAS writes still land and the assertions check the released state — it fails without the fix.
Blocker (CI, not logic): the TypeScript & ESLint job fails in its Prettier step on netlify/nps-relay/relay.test.ts. The offending span is relay.test.ts:506-509; Prettier wants
const res = await handleRelayRequest(register(), deps({ store: losing }));on one line. npx prettier --write netlify/nps-relay/relay.test.ts fixes it. Once that is pushed, nothing else stands out — Vitest, Build and npm audit are already green on this head.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read handleRegister and the CAS helpers at the PR head; this looks correct to me.
- The new release in the
!created.modifiedbranch (netlify/nps-relay/relay.ts:784-792) is the samereleaseSlot(store, ipKey, now)+ day-counter CAS pattern thedayDeniedpath already uses atrelay.ts:766-769, so the three reservation/release paths are now symmetric.Math.max(0, count - 1)keeps the counter from going negative if the key was reset in between. - Both releases run before
getInstall, so the 200 (same key) and 409 (different key) outcomes both give the quota back — matching #559's evidence line. - Verified the test is a real regression guard: with
relay.tsreverted to the merge-base,releases the per-IP slot and day count when the create-only write loses the racefails withexpected { count: 1 } to deeply equal { count: +0 }; at the PR head all 50 relay tests pass.
Changelog fragment name follows changelog.d/README.md (fixed-…). No findings.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
…ubmit request (#564) Three fixes in a row (#555, #558, #560) each closed one path where a reserved slot was not released after a later refusal. Each shipped a site-local regression test; nothing asserted the class. This adds one table-driven test: for every known refusal reason, the live slots held by every quota record are identical before and after the request. Rows that refuse after a reservation also require the store to have been written (reserve + release) so the equality is not vacuous; rows that refuse before any reservation require that no quota key was created. Reverting any one of the three release hunks makes exactly one row fail. Closes #563 Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <[email protected]>
handleRegister reserved the per-IP slot and the day count before the create-only write, and leaked both when that write lost the race, so a rare double-registration burned one of the IP's 3 slots for 24h. The losing branch now releases both reservations, mirroring #555/#558.
Closes #559
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88