[costajohnt]

Summary

• TokenRouter and similar upstreams reject bursts with 429 You have reached the request limit: Maximum 5 requests within 1 minutes and no rate-limit headers. Nothing parsed that number, so the Bottleneck limiter never learned the 5 rpm ceiling, and the eviction that runs on every 429 rebuilt the limiter with no pacing at all.
• updateFromResponseBody now parses that phrasing (plus N requests per minute, N requests per M seconds, N RPM) into a per-window cap, applies it to the limiter as an empty reservoir that refills N every window with calls spread window / N apart, and records capRequests / capWindowMs in learnedRateLimits.
• getLimiter reapplies a learned cap whenever the limiter is rebuilt (so it survives the 429 eviction) and logs that it did; loadPersistedLimits restores it after a restart, re-validating the persisted shape. An explicit per-connection RPM override still wins (at construction and on the live limiter; the cap stays recorded for when the override goes), a cap never spaces calls closer than the global or per-connection minTime floor, a later header-learned update keeps the cap, an operator refresh of the connection's limits forgets the learned cap in memory and in the persisted settings, and a cap that could not be honoured within the queue budget (say "1 request per 24 hours") is refused with a warning rather than turning every request into a queue timeout.
• The body path rebuilds the limiter itself before emptying the reservoir, so the hold lasts a full window even when the header hook did not run; the two eviction sites share one evictLimiter() helper.
• The capacity-503 side was already classified as model_capacity; this pins TokenRouter's exact system cpu overloaded / system disk overloaded wording in a test as the owner asked.

Related Issues

• Closes feat(resilience): Enforce learned per-upstream RPM caps and capacity backoff (TokenRouter 5 req/min, disk/cpu 503s) #13594

Validation

• Change type: provider / routing
• Focused tests: tests/unit/rate-limit-learned-cap-13594.test.ts (13 tests) plus the neighbouring rate-limit suites (rate-limit-manager, rateLimitManager-mintime-floor-9763, rate-limit-enhanced, chat-rate-limit-body-lock, ratelimitmanager-headers-split, limiter-lifecycle, rateLimitManager-idle-eviction, rateLimitManager-update-sequencing): 93 pass
• npm run lint on the touched files (eslint + prettier clean)
• Reconciled with release/v3.8.51 at e7214c7
• Production-code changes include new automated tests in this PR

Tests Added Or Updated

• tests/unit/rate-limit-learned-cap-13594.test.ts (new): parser positive and negative cases (including usage statements that must not be read as ceilings), cap applied and learned after the headers-then-body call order, cap survives a second 429 eviction, cap restored from persistence after initializeRateLimits(), cap clamped to the operator minTime floor, RPM override outranks the cap, header-learned update keeps the cap, body-only path rebuilds the limiter, operator refresh forgets the cap, unhonourable cap refused, persisted-cap bounds, capacity-503 classification.
• config/quality/eslint-suppressions.json: the no-unused-vars allowlist entry for rateLimitManager.ts is pruned; the previously unused status argument of updateFromResponseBody is now used.

Coverage Notes

• open-sse/services/rateLimitManager/requestCap.ts is fully covered by the parser tests. The three new branches in rateLimitManager.ts (body cap, creation-time cap, restore) each have a test.

Reviewer Notes

• /api/usage/quota reads learned entries by provider:connection key, so a cap learned on a model-scoped key (gemini, github, codex, antigravity) is not shown there; that is how header-learned values behave today too, so I left the route alone.
• A learned cap has no in-memory expiry; if a provider raises its ceiling the old cap paces until the next restart plus the existing 24h staleness rule, same as header-learned values today. Happy to add a TTL if you want one.
• The regex is intentionally narrow (N requests (within|per|in|every) [M] unit, or N RPM), and windows are bounded to 1s..24h. If other providers phrase their caps differently I can add cases; the issue asked for real examples before generalizing further.
• A learned cap ages out with the existing 24h staleness rule in loadPersistedLimits and is visible in getLearnedLimits() / /api/usage/quota like the header-learned values.

[abhisheksharma2411]

Really like this one — learning the ceiling from prose when the upstream sends no headers is the right call, and the details that are easy to get wrong are already right here: rebuilding the limiter so the reservoir clock starts now rather than updateSettings() waiting for a heartbeat, the note about disconnect() vs stop() poisoning the instance, and refusing a cap that can't fit the queue budget instead of turning every request into a timeout. I ran the parser against a set of real-ish 429 bodies rather than reading the regex.

The four motivating shapes all parse correctly, and the usage guard genuinely works on the verbose form:

"Maximum 5 requests within 1 minutes"            -> 5/60000ms   minTime=12000
"Rate limit exceeded: 60 requests per minute"    -> 60/60000ms  minTime=1000
"You have made 120 requests in 1 minute."        -> null   ✅ guard works
"Quota: you have used 500 requests per hour"     -> null   ✅

1. RPM_RE has no usage guard, so the same sentence parses two different ways. The verbose pattern carries (?!…\b(?:made|sent|used)\b); /(\d{1,7})\s*rpm\b/i carries nothing. Same meaning, opposite outcome:

"Rate limited. You have made 3 requests per minute."  -> null
"Rate limited. You have made 3 rpm."                  -> CAP 3/min, minTime=20000ms
"Too Many Requests. Current usage: 4 requests per minute" -> null
"Too Many Requests. Current usage: 4 rpm"                 -> CAP 4/min, minTime=15000ms

The harmful direction is a low figure: a body reporting current usage as 4 rpm pins the connection to one request every 15s, and because the cap is re-applied on every limiter rebuild it survives the eviction that would otherwise clear it. Worth giving RPM_RE the same cap-word requirement the verbose pattern has — rpm on its own is about as likely to appear in a usage readout as in a ceiling.

2. The queue-budget refusal guards the learn path only; both re-apply paths skip it. updateFromResponseBody refuses a cap whose minTime exceeds resolveRequestQueueMaxWaitMs(...) — the comment there explains exactly why. But:

• getLimiter (~L597) re-applies learned.capRequests/capWindowMs through capMinTimeWithFloor with no budget check.
• loadPersistedLimits (~L1198) does the same on a live limiter, and its validation is isValidRequestCap, which is a shape check only — it permits windows all the way to MAX_WINDOW_MS (24h).

So the "1 request per 24 hours" case the learn path is careful to refuse can still arrive via restore. The realistic route isn't a hand-edited settings file: a cap learned under a generous maxWaitMs, then an operator lowering that budget, is restored and applied on next boot regardless — and capMinTimeWithFloor can push minTime up further if the operator raises the minTime floor afterwards, again with no re-check. The fix looks like hoisting the same settings.minTime > queueBudgetMs test into a helper both paths call, since resolveRequestQueueMaxWaitMs is reachable from both.

Neither of these is a reason not to land it. Happy to re-run the probe on any follow-up push if that's useful.

[costajohnt]

Thanks for running real bodies through it, both of these were right. Pushed 50d0b9d.

1. RPM_RE now shares the cap-word prefix and the made/sent/used guard with the verbose pattern, so "current usage: 4 rpm" no longer pins the connection. One deliberate narrowing to flag: the cap word has to come before the figure, same as the verbose pattern, so "Throttled: 20 RPM exceeded" is no longer read as a cap. Added that as an explicit negative next to the usage cases.

2. The budget check moved into capSettingsWithinBudget(), which updateFromResponseBody, getLimiter and loadPersistedLimits all go through. A refused cap stays recorded (same as one held back by an rpm override) and is retried on the next limiter build. Added a test that learns a 1 per 5 min cap under a 10 min maxWaitMs override, restarts without the override, and checks neither the boot-time restore nor the 429 rebuild applies it, plus a positive for the boot-time restore path since that branch had no coverage before.

If the probe is handy, a re-run on 50d0b9d would be appreciated.

[abhisheksharma2411]

Re-ran the probe on 50d0b9d as asked. Both findings are closed.

-- CEILINGS (must still parse) --
  TokenRouter motivating case         CAP 5/60000ms   minTime=12000
  "60 requests per minute"            CAP 60/60000ms  minTime=1000
  "limit of 10 requests per 2 min"    CAP 10/120000ms minTime=12000
  "Limit: 20 RPM"                     CAP 20/60000ms  minTime=3000

-- USAGE (must NOT parse) --
  "You have made 3 requests per minute."   null
  "You have made 3 rpm."                   null   <- was CAP 3/min
  "Current usage: 4 requests per minute"   null
  "Current usage: 4 rpm"                   null   <- was CAP 4/min
  "Your observed throughput was 900 rpm"   null   <- was CAP 900/min
  "Quota: you have used 500 requests/hour" null

The three RPM cases that previously pinned a connection off a usage readout are all null now, and no ceiling regressed. capSettingsWithinBudget is reached from all three paths (getLimiter L630, loadPersistedLimits L1233, updateFromResponseBody L1294), which was the actual gap — the check existing but only on the learn path. rate-limit-learned-cap-13594.test.ts: 15 pass / 0 fail at that head.

Keeping a refused cap recorded and retrying it on the next limiter build is a better answer than what I suggested — a cap refused because of a temporarily tight maxWaitMs shouldn't be forgotten. And adding the positive boot-time-restore case is the right instinct; that branch having no coverage is what let the gap sit there.

One thing to weigh on the narrowing you flagged, since you raised it yourself:

"Throttled: 20 RPM exceeded"   -> null

That's a real ceiling, and requiring the cap word before the figure means "<N> RPM exceeded" — trailing-qualifier phrasing — is now missed. Silent under-reading, so the failure mode is benign (no pacing learned, same as today's behaviour before this PR) rather than the harmful over-restriction we just removed. So I'd land it as-is; it's the right side to err on. Worth keeping the negative test you added as the record of the trade, and if a real upstream turns up using that phrasing, adding exceed(ed|s) as a permitted suffix anchor would reclaim it without reopening the usage hole — a usage readout doesn't say "exceeded".

Nice turnaround.

[diegosouzapw]

Thanks @costajohnt — merging via the release merge-train. Validated in local merge-train (mt-train8e) on the devbox @ train tip b38c9340f092c8852ecfa7f6e0f9592fd6f7fc35 with the 26 sibling PRs of the owner-approved file-size rebaseline batch: typecheck:core, file-size (rebaselined entry _rebaseline_2026_09_18_merge_train_8_frozen_growth), complexity, cognitive-complexity, changelog-integrity green; changed-area node:test 722/722; vitest 479/482 where the 3 reds were 5s/20s timeouts under devbox load 18 and pass when re-run alone on the same train tip (flake class). Merged --admin per merge-gates §7.