Skip to content

Commit f9c1825

Browse files
Prithvi-Webclaude
andcommitted
test(rate-limit): a flood is judged with the limiter's clock held still — a busy runner that served 60 requests over 260 ms let 22 through by refill, which is right, and failed the test
'hammering an endpoint that walks the disk is still rate limited' fired 60 GET /api/fs/list at once and asserted at most 21 answered 200. The strict lane refills 10 tokens a second from Date.now(), so the count depended on how long the server took to answer: under load on 12 Oct 2026 (the local CI simulation's pt-BR leg) the 60 were served over ~260 ms, the bucket refilled 2.6 tokens, 22 got through, and the test failed on a product that was right. `limited > 0` read the clock the same way, in the other direction. The test now holds Date still for the flood (node:test mock timers, Date only, so sockets and server timers keep the real clock; Node 20.11+, CI runs 20.20.2) and asserts a count: exactly the strict lane's published burst answers 200 and every other request 429, both read from rateLimitLanes.describe(). It then advances that clock by one second and asserts exactly the sustained rate gets through, which proves the clock held still is the one the limiter reads. The old `<= 21` was also the only test that held the strict lane at the 20 burst / 10 per second AGENTS.md and the OpenAPI text promise; that promise is now an explicit assertion on describe(), since a count read from describe() alone would follow a widened capacity. Reproduced first: with the limiter's clock stepped 260 ms between two halves of the flood the old assertions saw 22 x 200 / 38 x 429 and failed with the CI message; with a real 400 ms pause, 24 x 200. The same real pause with the clock held still gives exactly 20 / 40. Hand mutants of src/middleware/rateLimiter.ts, each RED, each restored and checked by SHA-256: api capacity 20 -> 22 (the docs pin); the api bucket starting 2 tokens over its published capacity (21 x 200 — the old `<= 21` let this through); a refill credited during the burst on a still clock (25 x 200); the limiter reading performance.now() instead of Date (RED at the flood when the real clock refills during it, 21 x 200, or else at the one-second step, 0 x 200 / 20 x 429 — seen both ways). Runs: rateLimiterLanes 10 times under the normal env and 10 times under the CI pt-BR leg's env (LC_ALL/LANG pt_BR.UTF-8, TREEMAP_EXPECT_LOCALE=pt-BR), one at a time on a busy machine (1-minute load 9.5-11.9): 20 of 20 green, 8/8 each. noWallClockBounds, apiContract, frontendContract, goldenResponses, childEnvCarriesGuard and typecheck green. The other limiter tests were swept and left alone: every other assertion against the limiter is "no 429" or "some served" (boot burst, metadata lane, draining both ways, status polls, thumbnails, apiHardening's 50 subtrees), and elapsed time can only add tokens, so none of them reads the clock in the direction that fails. fleetSync's 429s are the pairing-guess count, checked before anything else, not the token bucket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 6ac10d8 commit f9c1825

1 file changed

Lines changed: 42 additions & 8 deletions

File tree

‎tests/rateLimiterLanes.test.ts‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,18 +163,52 @@ test('the metadata lane survives a boot burst several times over', async () => {
163163
}
164164
});
165165

166-
test('hammering an endpoint that walks the disk is still rate limited', async () => {
166+
/** How many responses came back with each status: `{ 200: n, 429: m }`. */
167+
function tally(results: Res[]): Record<number, number> {
168+
const out: Record<number, number> = {};
169+
for (const r of results) out[r.status] = (out[r.status] ?? 0) + 1;
170+
return out;
171+
}
172+
173+
/**
174+
* The flood is judged with the limiter's clock held still. The strict lane
175+
* refills 10 tokens a second from `Date.now()`, so how many of 60 "simultaneous"
176+
* requests get through depends on how long the server takes to answer them: on
177+
* 12 Oct 2026 a loaded machine served them over ~260 ms, the bucket rightly
178+
* refilled 2.6 tokens, 22 got through, and the old `<= 21` failed on a product
179+
* that was right. With the clock still, the answer is a count — exactly the
180+
* burst allowance, and every other request refused — whatever the machine.
181+
* The count is read from the lane table, so that it is what the limiter
182+
* publishes; the table itself is pinned to what the docs promise agents.
183+
*
184+
* Only `Date` is mocked (node:test's mock timers, Node 20.11 and later), so the
185+
* sockets and the server's own timers run on the real clock. The second half
186+
* checks that the clock held still is the one the limiter reads: one second
187+
* later on it, exactly the sustained rate gets through. A limiter moved to some
188+
* other clock lets none through there and fails here, instead of quietly
189+
* putting the wall clock back into the flood.
190+
*/
191+
test('hammering an endpoint that walks the disk is still rate limited', async (t) => {
167192
const s = await listen();
193+
const strict = rateLimitLanes.describe().find((l) => l.name === 'api')!;
194+
const flood = 60;
195+
const list = () => req(s.port, 'GET', `/api/fs/list?path=${encodeURIComponent(tmp)}`);
168196
try {
197+
assert.deepEqual({ burst: strict.burst, sustainedPerSecond: strict.sustainedPerSecond }, { burst: 20, sustainedPerSecond: 10 },
198+
'the strict burst allowance is unchanged — the split must not widen it (AGENTS.md and the OpenAPI text promise bursts of 20 at 10/s)');
199+
200+
t.mock.timers.enable({ apis: ['Date'], now: Date.now() });
169201
// 60 directory listings at once is not a boot, it is a runaway client.
170-
const results = await Promise.all(
171-
Array.from({ length: 60 }, () => req(s.port, 'GET', `/api/fs/list?path=${encodeURIComponent(tmp)}`)),
172-
);
173-
const limited = results.filter((r) => r.status === 429).length;
174-
assert.ok(limited > 0, 'the strict lane must still refuse a flood');
175-
assert.ok(results.filter((r) => r.status === 200).length <= 21,
176-
'the strict burst allowance is unchanged — the split must not widen it');
202+
const results = await Promise.all(Array.from({ length: flood }, list));
203+
assert.deepEqual(tally(results), { 200: strict.burst, 429: flood - strict.burst },
204+
'the strict lane lets exactly its burst allowance through a flood and refuses the rest — the split must not widen it');
205+
206+
t.mock.timers.tick(1000);
207+
const later = await Promise.all(Array.from({ length: strict.burst }, list));
208+
assert.deepEqual(tally(later), { 200: strict.sustainedPerSecond, 429: strict.burst - strict.sustainedPerSecond },
209+
'one second later on the same clock, exactly the sustained rate has refilled');
177210
} finally {
211+
t.mock.timers.reset();
178212
await s.close();
179213
}
180214
});

0 commit comments

Comments
 (0)