fix(rate-limiter): check-then-set race in the load test, plus three more bugs

[frankstupak]

The "CI flakiness" commit was hiding three real bugs

HEAD of this repo is fix(rate-limiter): relax remaining assertion in load test for CI flakiness. The assertion wasn't flaky — the code and the test were broken, three different ways. This PR fixes all of them, makes the test suite actually execute the Lua scripts (it never did), and picks up an 800x+ hot-path win along the way.

Bug 1 — Sliding window undercounts same-millisecond bursts (Lua)

ZADD key now tostring(now) uses the timestamp as the sorted-set member. Members are unique, so two hits in the same millisecond silently dedupe into one entry: the limiter undercounts and over-admits under exactly the bursty traffic it exists to stop. Fix: the caller passes a unique member per hit (ARGV[4], now-seq-rand) — the standard pattern in every production Redis sliding-window implementation.

Regression test: 3 hits with identical nowMs, limit 2 → third must deny. On the old Lua it was allowed.

Bug 2 — Denied hits were recorded: unbounded memory + self-lockout

Every rejected request was still ZADDed (and pushed, in the memory path). Two consequences: (a) a flooding client grows the sorted set without bound for as long as the flood lasts; (b) a client that backs off stays locked out by its own rejected traffic instead of recovering when its allowed hits age out. Fixed with reject-before-add: storage is now bounded at limit entries per key.

Measured (bench included, bench/bench-sliding-window.js): limit=100, 50,000 hits in one window → old stores 50,000 timestamps, new stores 100.

Bug 3 — The load test itself: baseTime = Date.now()

The fixed-window load test walks 2,000 ms of simulated time from an unpinned wall-clock start. Land within 2s of a window boundary (~3% of runs) and the window rolls over mid-loop, the counter resets, and assertions fail. The relaxation commit loosened the remaining assertion but left allowedCount strict — the test was still flaky after the "fix". Pinned baseTime to a window boundary and restored the strict assertion.

Bonus: the Lua scripts were never tested

The suite's MockRedisClient re-implemented all three algorithms in JS and never evaluated the Lua. The "Redis vs memory equivalence" tests were comparing two JS re-implementations with each other. The mock even pushed duplicate members where real ZADD dedupes — which is precisely how Bug 1 stayed invisible. All Redis-path tests (61 pre-existing + 9 new) now execute the real scripts through ioredis-mock's fengari Lua VM. That immediately surfaced a fourth issue: math.floor(now/win)*win overflows 32-bit Lua integers on millisecond epochs (fine on real Redis Lua 5.1 doubles, broken under fengari) — rewritten float-safe so the same script is correct on both.

Also fixed

• Token bucket resetMs: returned the full refill interval whenever the bucket hit zero, ignoring time already elapsed toward the next token — Retry-After overstated by up to one whole period. Now refillMs - (now - lastRefill). A long-idle full bucket also restarts its refill clock at now.
• Fixed window Lua: INCR instead of GET/SET; TTL set once at window creation and expiring at the window end — the old per-hit PEXPIRE(win) kept dead window keys alive up to a full extra window after the last hit.

Performance (in-memory sliding window, hot key)

The old memory path ran filter() + reallocated the timestamp array on every request. Replaced with an advancing head index (amortized O(1) prune, compaction when the dead prefix dominates):

Scenario	old	new	speedup
Sustained flood (~100x over limit)	5.5k ops/s	4.4M ops/s	808x
High-limit steady state (10k/min, at limit)	5.3k ops/s	7.7M ops/s	1470x
Entries stored under flood (limit=100, 50k hits)	50,000	100	500x less

Reproduce: node bench/bench-sliding-window.js (old algorithm embedded verbatim as baseline).

Verification

• 70/70 tests green (61 existing + 9 regression), now against the real Lua
• tsc --noEmit clean, eslint clean
• Root jest: both rate-limiter suites pass. 6 unrelated suites (nextjs-backend, autocomplete, api-scenarios) fail identically on pristine main — pre-existing, untouched here.

Public API unchanged — same functions, same LimitOpts/LimitResult, same result semantics on the documented paths.

— Lumen Industries