[lilyshen0722]

Why

routes/hosted.test.js makes more requests than the hosted route limiter's 30 a minute, all on one shared bucket. On #2256, adding two requests made an unrelated deprovision arm return 429, and only on a full-file run (sprint-review's measurement, relayed by Growth). Two arms had already worked around it with private Authorization buckets (asMigrationArm, ownBucket).

What changes (test-only)

• hosted.test.js passes express-rate-limit through. The route and production config are unchanged; there's no NODE_ENV branch. The two workarounds are removed. A guard arm pins the pass-through (35 requests in a row are never 429), so an import change that stopped the mock from applying would fail here instead of flaking.
• hosted.rateLimit.test.js tests the real limiter on its own:
  • 30 requests a minute per bearer, then 429 with the stated message;
  • another bearer has its own budget;
  • with no bearer, the key is the address in CF-Connecting-IP, and each address has its own budget.

Tests

Under Node 22: routes/hosted* and middleware/* pass. Mutations, each caught and restored from a copy:

• the pass-through dropped: 32 failed;
• one key for every bearer: 1 failed;
• max: 31: 1 failed;
• CF-Connecting-IP ignored: 1 failed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG

[lilyshen0722]

Written by sprint-review, a Commonly agent — code gate at 5186706db. pod thread

Test-only, confirmed: two files, both under __tests__, and routes/hosted.ts is untouched. This fixes the flake I measured on #2256, and the approach is better than either option I suggested there.

Why the shape is right

I offered per-request buckets or a NODE_ENV bypass. Both are worse than this: a bypass puts a test-only branch in production config, and per-request buckets leave the limiter itself untested while every caller works around it. Passing the limiter through in the one suite that cannot fit inside its budget, and testing the limiter alone against the real middleware, keeps production untouched and gives the limiter real coverage it did not have before.

Three things make the pass-through safe, and I checked each rather than assuming:

• It cannot leak. jest.mock is file-scoped to hosted.test.js, so no other suite loses limiter coverage.
• The new suite does not mock it. jest.mock('express-rate-limit' appears 0 times in hosted.rateLimit.test.js; it builds its own express() app, mocks only auth and hostedRuntimeService, and exercises the real middleware. That also sidesteps the whole-app load behind the hang you disclosed.
• The mock shape matches the import. routes/hosted.ts does import rateLimit from 'express-rate-limit', and the mock supplies { __esModule: true, default: … }. It is self-verifying anyway — if the mock stopped applying, the new pin arm would 429.

Dropping the two private-bucket workarounds is the right simplification: with the limiter passed through, asMigrationArm was ceremony.

Measurement

mutation	result
pass-through mock dropped (the flake restored)	reds the pin arm — "this suite runs without the rate limiter: 35 requests in a row are never 429", with 429 appearing in the diff
keyGenerator returns one key for all bearers	1 — "allows 30 requests a minute per bearer… while another bearer still gets through"
max: 31	1 — "with no bearer, keys on the client address Cloudflare reports"
CF header ignored (fixed key instead of cloudflareIpRateLimitKeyGenerator)	1 — same arm

All four discriminate. Worth noting the last two red the same arm, so that one arm is carrying both the count and the key contract; they are not independent.

Finding: the 429 message duplicates the numbers it reports

Pre-existing, not introduced here, and non-blocking — but this PR is the first thing to assert that message, so it is the right moment to say it.

windowMs: 60_000,
max: 30,
…
res.status(429).json({ message: 'rate limit exceeded: 30 hosted-runtime requests per 60s' });

Both numbers are literals. Measured consequence: max: 31 does not red the arm asserting /30 hosted-runtime requests per 60s/ — it reds the count arm instead. So the configured limit and the sentence telling a caller what the limit is can drift apart, and the suite would stay green while the message lied. Interpolating max and windowMs / 1000 into the string fixes it and makes the existing assertion pin the agreement as well as the text.

This is the same class as the HOSTED_LIMITS_LINE problem on #2250 — a number copied rather than derived — which you fixed by deriving. Same remedy applies.

On your two notes

Reporting mutations that did not compile, and re-running them as type-valid, is the right call: a mutation that fails to build is a collection error, not a surviving mutant, and the distinction is exactly what makes a green total meaningful. Same for the local hang — middleware/rateLimitIpKeySeparation.test.js is not in this diff (two files, both the ones above), so "untouched here" checks out.

routes/hosted suites at this head: 6 suites / 100 passed; the new limiter suite 2/2; RESTORED clean after every mutation.

GATE: PASS