feat(openai): reduce the risk of rate-limit errors when many agents run at once - #172
Merged
Merged
Conversation
This was referenced Sep 4, 2026
Owner
Author
Follow-ups filed from this reviewBoth were found while reviewing this change and are deliberately not fixed here.
|
bman654
force-pushed
the
feat/pace-new-openai-connections
branch
from
September 4, 2026 02:27
7ccaf9a to
b79d569
Compare
…un at once OpenAI's edge rejects a new ChatGPT/Codex connection with HTTP 403. In the traffic sampled while investigating, those rejections clustered in the minutes that opened the most new connections; that the rate is what the edge reacts to is the working assumption here, not a demonstrated cause. clodex already turned the rejection into a retryable rate limit, but nothing limited how fast it asked for new connections. A process-wide token bucket now paces the creation of primary new connections, at 60 a minute with an allowance of 10 opened back to back. A turn that reuses a connection it already has never consults the limiter, so continuations are never delayed. The two replacement paths a retry uses are exempt by design, so the bound is on admissions rather than on sockets. Work over the rate is queued for a bounded time; anything still over is answered with the same retryable 429 shape the 403 path already produces, which the AI SDK backs off and retries. A refusal deliberately debits no token: a refused request opens no connection and, while retries remain, will be retried — so charging it would let each retry deepen the deficit that caused it. Because only an admitted request debits, and only within the bound, the queue is bounded by construction. The wait bound is derived rather than chosen. Every attempt of one request shares one no-data deadline, so the whole retry ladder has to fit inside it. The retry count is read from CLODEX_UPSTREAM_MAX_RETRIES rather than assumed, so the bound stays correct whatever the retry default is. With retries turned off the limiter cannot refuse, so it shapes the opening burst and then stops limiting, which the docs state plainly rather than implying a guarantee. Configurable via CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN (1-600, 0 disables); out-of-range values clamp and malformed values are ignored, each with a one-time notice. A rate that cannot be paced safely also reports itself rather than switching off silently. One new connection per second is an aggregate ceiling, not a brief pause: about 20 agents settle at roughly 20 seconds per turn against ~3s unpaced. That trade is documented in both the README and the OAuth continuation doc.
…user configured Reconciles this branch with #171, which replaced `upstreamMaxRetries()` with `upstreamRequestBudget()` and made both terms of the pacer's wait-bound inequality user-configurable. The pacer now resolves the idle deadline and the retry count together from the same budget call every SDK generation entry point makes, instead of assuming a fixed 120s deadline. At the shipped defaults the bound tightens from 15s to 4833ms, because #171 raised the default retry count to five and its 62s backoff ladder has to fit the same 120s window. Adds the composition tests neither PR's CI could run: the ladder inequality across the whole resolvable timeout and retry space, the pair rule where a short total timeout drags the idle timeout down, and the degradation case where a deadline barely wider than its own ladder leaves no room to queue and pacing turns itself off rather than overrunning it.
…nning it A refusal sends its own retry-after hint, and the AI SDK substitutes that hint for its own backoff rung whenever the hint is under 60 seconds rather than taking the larger of the two. The hint was derived from the token deficit and could be far longer than the wait bound, so it replaced the exact term the bound was derived against. Measured: at 2 connections/minute inside a 10s deadline the refusal asked for 30s. The request died at the deadline having made one attempt, and both retries its budget had paid for never ran. The hint is now capped at the derived bound, with a one-second floor because retry-after is whole seconds and zero would mean retry immediately. One second is below the SDK's smallest rung, so it stays inside the ladder term the bound already budgets. The deficit is still reported honestly to diagnostics. Four smaller corrections ride along. Notice dedupe keys are namespaced, so a pacing rate spelled like the pacing-disabled key can no longer suppress the notice that pacing turned itself off. The docs no longer promise that rate-limit errors stop, since pacing reduces the risk and can itself refuse a turn. The claim that the throttle is scoped to the account is dropped, because one account on one machine cannot determine that. A development harness is updated to stop calling the retry helper that the timeout change removed.
…ection rates Capping the refusal hint at the wait bound stopped requests overrunning their deadline, but it created the opposite failure at low rates. At one new connection a minute the first free slot is 60s away, while six attempts about four seconds apart are all spent inside 20s. Every turned-away request used up its retries before a slot could exist and failed as a rate-limit error, which is the failure this feature exists to reduce. Measured at 1/minute and 2/minute: 10 of 10 terminal. No hint strategy fixes that. Serving a 60s wait needs 60s, and a 120s deadline covering six attempts cannot fund one. So overflow is turned away only when the retry schedule can outlast the wait for a slot; below that the excess is admitted late instead, with a one-time notice. That is the rule already applied when the deadline leaves no room to queue at all. Two test defects found alongside it. The wait-bound matrix derived its expected hint cap by calling the function under test with a non-finite value that the function maps to one second, so it asserted one degenerate value at every bound and a cap applied only below two seconds passed all 131 tests. The cap is now derived independently, with direct cases at the shipped bound and the ceiling. The claim that exponential backoff is an upper bound on the paced case was also false wherever the capped hint exceeds an early rung, so the source and the deep doc now state the per-gap maximum the tests actually assert.
* origin/main: fix(openai): stop long sessions on OpenAI models dying with "Prompt is too long" (#167)
…not just the default one Every committed serviceability case used the shipped 4833ms bound and five retries, so a rule that ignored the budget entirely passed all 263 relevant tests while recreating terminal failures under a supported configuration: a 10-second idle timeout with three connections a minute leaves a two-second retry schedule against a twenty-second wait for a slot. Adds a budget matrix over both the shipped and a shortened deadline, including the exact threshold either side of thirty connections a minute, and a behavioural pair that proves the constructor consults the rule rather than a helper truth table proving only that the rule exists. The low-rate fallback's shaping was also unasserted. The tests checked that nothing was refused but never that anything waited, so zeroing the refill rate made low-rate pacing indistinguishable from pacing switched off and stayed green. Both the unit and the end-to-end test now assert the retained delay. Corrects a claim that outran its evidence: capping the hint at the wait bound was described as the only possible strategy, and it is not. A separately budgeted twelve-second hint reaches a one-a-minute refill and still fits the conservative bound. What is actually true is narrower, and is now what the source and the deep doc say: no strategy admits all the overflow inside the deadline while preserving the configured ceiling. Shaping is a trade-off chosen for that reason, not a forced move. The one-time notice no longer implies every excess connection is delayed, and the harness index now records that one harness cannot run at all.
… ceiling The explanation of the low-rate fallback said it spends the shortfall on latency the user configured rather than on errors. That is not what happens. The user configures a connection rate, not a latency, and the fallback mostly adds no latency at all: at one connection a minute with twenty simultaneous requests, nineteen are admitted immediately, one waits out the bound and none is turned away. What actually gives way is the configured ceiling. Both the module and the deep doc now say that: the fallback avoids guaranteed local failures and keeps bounded shaping of the opening burst, at the cost of relaxing the ceiling the user asked for. Comments and documentation only; no behaviour changes.
bman654
force-pushed
the
feat/pace-new-openai-connections
branch
from
September 4, 2026 07:59
b79d569 to
72c12eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When you run a lot of agents at once on a ChatGPT/Codex plan, OpenAI sometimes starts rejecting
clodex's attempts to open new connections, and those turns come back as rate-limit errors. clodex
already handled the rejection gracefully — it retries — but nothing limited how fast it asked for
new connections. This change makes clodex pace itself, which should make a burst of parallel work
less likely to trip the limit. (In the traffic sampled, rejections clustered in the busiest minutes;
that the rate is what triggers them is a reasonable reading of that, not something proven.) A
follow-up turn in a conversation that already has a connection is never slowed down; only work that
needs a brand new connection goes through the limiter.
There is a real cost, and it is stated up front in the README rather than buried: this is a
throughput ceiling, not a brief pause. One new connection per second means roughly 20 agents settle
at about 20 seconds per turn instead of a few seconds. You wait longer, in exchange for a lower
chance of losing turns to rate-limit errors — a reduction in risk, not a guarantee. Under a heavy
enough fan-out the limiter itself answers a turn with a rate-limit response, and the README says so.
The failure this addresses
OpenAI's edge returns HTTP 403 on a Responses WebSocket upgrade when an account opens new
connections too fast.
responses-websocket.tsmaps every upgrade 403 to a retryable Anthropic 429with a backoff hint, which the AI SDK honours — so the user sees latency and retries rather than a
hard failure, but under a big fan-out they see a lot of both.
Evidence
Measured by re-reading one machine's own
ws_head_decisiondiagnostics (103,698 records spanningabout a day and a half), bucketing records that carry a
createdConnectionIdby wall-clock minute:On a 200k-line slice of the same log, 11,417 of 26,430 head decisions opened a connection and 10,229
of those were the non-reusable parallel fan-out kind — new connections move with the number of
concurrent agents, not with conversation length.
This is a correlation in one account's traffic over one window, not a published limit and not a
demonstrated cause. The instantaneous rate was 3-5/second in the bad minutes and the clean ones
alike, which is why the limiter shapes a sustained rate rather than a burst. The single rejection in
a 41/minute minute does not fit the pattern and is unexplained; it is left visible rather than
dropped, because it is the main reason to treat the sustained-rate inference as weak.
What changed
A process-wide token bucket (
src/oauth/ws-upgrade-pacer.ts) gating connection creation only,called from one site in
responses-websocket.ts. Reuse of an established or nursery head neverconsults it. Defaults: 60/minute sustained, burst 10.
CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MINoverrides the rate (1-600;
0disables; out-of-range clamps and malformed values are ignored, eachwith a one-time notice).
Overflow is refused, not queued indefinitely, using the same retryable 429 frame shape the 403
path already produces. A refusal debits no token — that is the load-bearing detail. A refused
request opens no connection and will be retried, so charging it would let every retry deepen the
deficit that caused the refusal. Because only an admitted request debits, and only within the wait
bound,
tokenscannot fall below-bound × refill: the queue is bounded by construction.The wait bound is derived, not chosen. Every attempt of one request shares one no-data deadline —
the timer starts before the SDK call and only a stream part resets it — so the whole ladder must fit:
(maxRetries + 1) × bound + totalBackoff < idleTimeout. Both terms are read, through the sameupstreamRequestBudget()call every SDK generation entry point makes, so the pacer sizes its waitagainst the deadline the paced request will actually spend. At the shipped defaults that is 4833ms.
Where a configuration leaves no room to queue at all, the bound floors to zero and pacing turns
itself off with a notice; it degrades to less pacing, never to a request pushed past its deadline.
The design that was rejected, and why it matters
The first implementation admitted anyway once the bound expired. A review measured it against the
real class at shipping defaults and found sustained output equal to sustained input:
82/min is the rate that produced 39 of the 40 observed rejections. The debt floor and the wait bound
were the same number by construction, so the shaping reservoir was exhausted exactly where the bound
took over: every connection past the first ~24 waited the full 15s and was then opened anyway. It
would have shipped a limiter that did not limit, plus 15s of latency. There is now a test that
measures the final minute of a four-minute overload; against the old behaviour it reads 82.
The defect an adversarial review found after the rebase, and the fix
Worth reading even if you skip the rest, because it invalidated the bound derivation this feature is
built on and it survived every earlier review.
The bound budgets the SDK's 2s/4s/8s/16s/32s ladder as the delay between attempts. A refusal sends
its own
Retry-After, and the SDK spends that hint INSTEAD of the rung —getRetryDelayInMsinai@7.0.22(util/retry-with-exponential-backoff.ts:46-56) returns the supplied delay whenever itis under 60s; it does not take the larger of the two. The hint was derived from the token deficit,
which is by definition larger than the bound (that is why the request was refused), so the pacer was
replacing the exact term its own inequality was derived against.
Executed repro: at 2 connections/minute inside a 10s deadline, the inequality passes
(3 × 666ms + 6000ms = 7998ms < 10000ms) and the refusal still asked for 30 seconds. The request
died at the deadline with
attemptCount: 1— both retries its budget had paid for never ran. Thisis a default-on hard abort of the same shape as the 152s-vs-120s error, reached by a different
route.
Fix. The hint is capped at the derived bound, floored at one second, in a new
pacedRetryAfterSeconds.requiredWaitMsstill reports the true deficit to diagnostics; only thenumber the client is asked to honour is capped. The one-second floor exists because
Retry-Afteriswhole seconds and zero would mean "retry immediately" — a hot loop that burns the retry budget in
milliseconds — and it is safe because 1s is strictly below the SDK's smallest rung, so it stays
inside the ladder term the bound already budgets. That floor matters wherever the bound is
sub-second, e.g.
IDLE=10000gives a 666ms bound.The property now asserted across all 88 environment combinations is the one that accounts for the
substitution, not just the ladder:
Cost, stated rather than buried. A refused request now comes back sooner than its deficit needs
and may be refused again. That is the right trade — it spends its retries inside its deadline
instead of spending its whole deadline on one oversized sleep — but it does not make refusals
succeed, and at rates far below the deficit a fan-out can still end in a rate-limit error.
A second blocker, created by that fix, and a false oracle that hid it
The re-review of the fix above found both. Recording them because the pattern matters more than
either defect.
Capping the hint fixed one failure and created its mirror image. With the hint capped at the
bound, a refused request comes back every ~4s. At
CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=1thefirst free slot is 60 seconds away, so all six attempts are spent inside 20 seconds and the request
fails as a rate-limit error — manufactured by the feature that exists to reduce them. Executed
against the real pacer and real SDK, 20 concurrent, default 120s/5 retries:
Both rates are documented as supported. The fix had traded 7 deadline failures for 10 terminal ones.
The pacer now refuses only when its retry schedule can outlast the wait for a slot
(
canRefuseAtRate); below that it shapes the opening burst and then admits the remaining overflowinstead of failing it, with a notice. That is exactly the rule the zero-bound case already applied,
and the review correctly pointed out I had been applying it to a zero bound but not to a positive
bound that equally cannot make progress.
I first justified that with a claim that was false, and a later round disproved it. I wrote that
no hint strategy could fix 1/minute. A separately budgeted 12-second hint does reach the refill —
attempts at 0, 12, 24, 36, 48, 60s — and still fits the conservative mixed-gap bound at the shipped
budget:
6 × 4833 + max(12,2) + max(12,4) + max(12,8) + max(12,16) + max(12,32) = 112,998ms < 120,000ms. The true statement is narrower: no strategy admits all the overflow inside thedeadline while preserving the configured ceiling — at 1/minute, ten simultaneous overflow requests
need ten minutes of capacity, so a longer hint rescues one and still fails nine. A separately
budgeted hint is a reasonable follow-up, and is named as one in the source and the deep doc. Both
now carry the correction rather than the original claim.
A later round caught a second overstatement in the very sentence that replaced the first, which
is worth recording because it is the identical failure mode one level down. I had written that
shaping "spends the shortfall on latency the user configured rather than on errors." It does not:
the user configures a connection rate, not a latency, and the fallback mostly adds no latency at
all — at 1/minute with 20 simultaneous requests, 19 are admitted immediately, one waits out the
bound, and none is refused. What actually gives way is the configured ceiling. The accurate
statement, now in both places: it avoids guaranteed local failures while retaining bounded
opening-burst shaping, at the cost of relaxing the configured ceiling.
Verified in a real run — the notice reaches the terminal:
The false oracle is the more instructive finding. The matrix that was supposed to prove the cap
computed its expected value by calling the function under test — with
Number.POSITIVE_INFINITY,which that function deliberately maps to one second. So
hintCapMsread 1000ms for every bound,including the shipped 4833ms and the 15000ms ceiling, and the whole 88-combination matrix was
asserting one degenerate value. The reviewer proved the consequence: this mutation survived all 131
pacer tests —
— while keeping the two explicitly tested bounds (666ms, 2000ms) green, and under it the original
deadline-overrun blocker returns in full at the shipped bound. The expected cap is now derived
independently of the implementation, with direct cases at 4833ms and 15000ms; that mutation now
fails 6 tests.
The lesson is the one this PR keeps relearning: an oracle that calls the implementation cannot
falsify the implementation. Three of the four defects found after the first "green" were of that
shape — verifying a model of the behaviour rather than the behaviour.
A third round: two more mutations that survived a green suite
Neither is an implementation defect — both are tests that could not tell the shipped code apart
from a broken one.
The serviceability rule was only ever tested at the default budget. Every committed case used
the shipped 4833ms bound and five retries, so this passed all 142 pacer tests, all 121 WebSocket
tests and typecheck:
It recreates the terminal-failure defect under a fully supported configuration —
CLODEX_UPSTREAM_IDLE_TIMEOUT_MS=10000with rate 3, where a 2s retry schedule faces a 20s wait.There is now a budget matrix across both the shipped and a shortened deadline, including the
threshold either side of 30/minute, plus a behavioural pair that proves the constructor consults the
rule rather than a truth table proving only that the rule exists. That mutation now fails 4 tests.
The low-rate fallback's shaping was unasserted. This also passed everything:
It makes low-rate pacing identical to pacing switched off. The tests asserted that nothing was
refused, never that anything waited — including the end-to-end test whose own comment said a request
"really does wait out the ~4.8s bound". Both now assert the retained delay. The mutation fails 3
tests.
Writing that assertion corrected my own model of the code: I predicted one delayed request and
measured two. Simultaneous arrivals hit the debt floor and pass straight through, but sequential
arrivals refill roughly what they consume, so each one past the burst keeps paying the bound. At
1/minute the effective ceiling is therefore about one per bound — weaker than configured, but not
absent, which is the distinction the mutation erased.
Four smaller corrections from the same review
string and shared one set with the pacing-disabled notice, so
CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=disabled:3:14001suppressed the one notice that must nevergo missing — that pacing turned itself off. Reproduced, fixed, and pinned with that exact string.
all said the trade buys "not tripping the throttle", while the module header correctly calls the
causal link an assumption. They now say it lowers the chance and can itself refuse a turn.
refusal at 15 simultaneous connections — inside the many-agent workload this feature targets. It
now says that, and notes the rebase moved the threshold down from 26 to 15.
an account-, IP-, model- or edge-level limit apart. One shared bucket is now presented as the
conservative choice under that uncertainty rather than as a modelled fact.
Deliberately not in this PR
out of scope by design.
previous_response_not_foundretry each build a connection through
createReplacement. Both recover a request that was alreadyadmitted, both are capped at one per request, and both run inside socket callbacks where an await
would restructure the retry path. Each now has a negative test. This means the bound is on
admissions, not on sockets, which the docs say explicitly.
Verification
Re-run in full after the rebase onto #171, on the merged tree rather than on either branch
alone.
pnpm typecheck && pnpm test && pnpm buildgreen: 2260 tests, 106 files. Repeated under athrowaway
CLODEX_HOME. Node 24.14.1, macOS arm64, pnpm 10.34.5.Smoke tested through real Claude Code on the default proxy mode, not just the changed path. The
pre-rebase branch ran Anthropic passthrough (
HAIKU-OK), the translated OAuth WebSocket leg(
MODEL-OK),CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=0(DISABLED-OK), a malformed pacing valuewhich printed its one-time notice and still answered (
MALFORMED-OK), andCLODEX_UPSTREAM_MAX_RETRIES=0(NORETRY-OK). All were re-run against the rebased build, plus twolegs that only exist now that #171 made the deadline configurable:
HAIKU-OKMODEL-OKCLODEX_UPSTREAM_IDLE_TIMEOUT_MS=30000— bound derives to 2000msSHORTIDLE-OKCLODEX_UPSTREAM_IDLE_TIMEOUT_MS=14001— bound floors to 0, pacing disables itselfDEGRADE-OKCLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=0DISABLED-OKCLODEX_UPSTREAM_MAX_RETRIES=0NORETRY-OKA
CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=1leg also answered (PACED-OK), but that leg provesonly that the tightest configuration still serves a request — a single-turn prompt needs one
connection and the burst allowance covers it, so no refusal was produced on the wire. The refusal
path's evidence is the unit tests and the reviewer's executed repro, not this leg.
The
DEGRADE-OKleg matters because it is the safe-degradation path, and #171 is what made itreachable at all — on the pre-rebase branch the deadline was a constant, so no user configuration
could floor the bound. The run also printed the notice to the real terminal after Claude Code
exited, so the "it never turns itself off silently" claim is now an observation rather than a
reading of the code:
Mutations run, and what caught them
Every behaviour below was mutated individually and the suite re-run in full.
never asks the pacer for a request that reuses an existing connectiondemotes a concurrent sibling…([nursery,nursery]vs[isolated,nursery])ages the heads it reports from after the waitshapes the opening burst but stops taxing…>→>=; break warn-once dedupecode,retry_after_seconds,retry after Nsproseretry-afterheader?? sharedWsUpgradePacer()fallbackpaces through the real process-wide bucket…sharedWsUpgradePacer()itselfTwo holes were found this way and closed: the
retry-afterheader shipped with no test, and theframe's
retry_after_secondsis invisible to the SDK (its chunk schema strips unknown fields), sothe prose is the only channel that reaches the client — now asserted directly on the frame.
The last two rows matter because every other integration test injects a fake pacer through a
test-only option. One test uses no injected pacer, drives 11 real connections through the shared
bucket with the env var set, and is the sole evidence for the production singleton and env wiring.
Mutations re-run after the rebase
The table above was measured on the pre-rebase branch. The guards that the rebase could plausibly
have broken were re-mutated against the merged tree, running
tests/ws-upgrade-pacer.test.ts+tests/responses-websocket.test.tseach time:maxRetriesat 5 instead of reading itmaxRetriesat 2 — the pre-rebase?? SDK_DEFAULT_MAX_RETRIESbehaviourretry-afterhint — the blocker aboveTwo mutations survived at some point and each produced a test. Dropping the one-second hint floor
was invisible until an explicit lower-bound assertion pinned it. Capping the hint only below a 2s
bound was invisible because of the false oracle described above, and is now pinned by direct cases
at the two bounds production actually uses.
What review attacked and could not break
Stated because it is the stronger half of the evidence:
simulation put the threshold at 26 simultaneous new connections. feat(timeouts): let you raise the 10-minute limit so long agent runs can finish #171's five-retry default
tightens the bound and moves it to 15 simultaneous new connections, now measured directly
rather than derived (
refuses only past a large simultaneous fan-out at the shipped defaults).Ten concurrent agents produced zero hard failures over a 300-second closed-loop run on the
pre-rebase branch.
retry-afterheader is genuinely read.tokensneverexceeds capacity, and admission order is FIFO with deadlines provably
1/rateapart.turn a spread-out arrival pattern into a herd.
createdConnectionIdremains an exact prediction — there is no await between the diagnostic andthe connection.
Boundaries I could not verify
reading of it, not a known threshold.
— closed by the rebase. Both terms are now read fromidleTimeoutMsis assumed at 120sthe resolved budget. What is still assumed is that the pacer's snapshot matches the request's: the
pacer resolves the budget once when the process-wide bucket is built, and no production caller
overrides
idleTimeoutMson that call (verified by grep — the override seam exists for directadapter callers and is unused), so the two agree for any environment a user can set. A future
caller that starts passing an override would break that agreement silently.
2000 × (2^n − 1)backoff ladder rather thanintegrating with the SDK, so a change to the SDK's backoff policy would not turn it red. This is
the exact class of gap that produced the
Retry-Afterblocker above, and it is not closed.max(hintCap, rung), which does not model a provider-suppliedretry-afteron a non-pacing failure. If an upstream 429 unrelated to pacing carries its ownhint of up to 60s, the SDK spends that instead, and no bound clodex derives can prevent it. That
is pre-existing and outside this feature's control, but it means the inequality is a guarantee
about clodex's own contribution to the delay, not about total elapsed time.
each one permits one extra admission at that instant. The bound is on reservations, not wall-clock
departures: a stalled event loop releases overdue timers together and the pacer neither observes
nor corrects for that.
One test-harness defect found and fixed along the way
A concurrency test showed two connections constructed but only one in the fake socket registry. That
looked like it could be a socket leak in a connection-management change, so it was chased rather than
waved off. Instrumenting the product side settled it:
createConnectionran twice and bothconstructions returned, but the fake constructor ran once — the second call got the real
wsmodule. Two concurrent dynamic imports of a mocked module race in vitest, and one resolved unmocked,
which also meant the suite was quietly constructing a real socket to
wss://chatgpt.com. Resolvingthe mock before the concurrent phase makes the counts agree exactly. Harness, not product — the
product created exactly the two connections it should. Any future test that fans out two
wsFetchcalls needs the same warm-up.
Rebased onto #171 (done — this is no longer a merge-order hazard)
#171 has landed. This branch was cut before it and has been rebased onto current
main, whichincludes #171, #175, #167 and the 2.9.0 release. Neither PR's CI had ever run against the other, so the
combination was treated as untested and re-verified from scratch rather than trusted. (2.9.0 went
out without this change so the 2.1.260 patch fix could ship; this lands in the next release.)
What #171 changed underneath this branch. It deleted
upstreamMaxRetries()(which returnednumber | undefined) in favour ofupstreamRequestBudget(options?): { idleTimeoutMs, totalTimeoutMs, maxRetries }, which alwaysreturns numbers — so the old
upstreamMaxRetries() ?? SDK_DEFAULT_MAX_RETRIESfallback would notmerely have failed to compile, it had no meaning left. It also raised the default retry count to 5
and made the idle timeout, total timeout and retry count user-configurable. That changes both
inputs to this feature's bound inequality, so the bound could no longer be derived from constants
the way this branch derived it.
What the reconciliation does.
src/oauth/ws-upgrade-pacer.tsnow resolves the deadline and theretry count together from a single
upstreamRequestBudget()call — the same call every SDKgeneration entry point makes — instead of assuming a fixed 120s deadline. The constructor reads the
environment only when a term is not injected, so a fully injected test pacer never depends on
ambient configuration.
ASSUMED_UPSTREAM_IDLE_TIMEOUT_MSandSDK_DEFAULT_MAX_RETRIESare gone.The bound tightens on its own, which is the point of reading rather than assuming. At the
shipped defaults it goes from 15s to 4833ms: five retries cost a 62s backoff ladder, leaving 58s for
six attempts, half of which is reserved for the provider's own first byte.
Why the inequality now holds for every configuration rather than by coincidence at one of them:
pacing takes at most half of what the ladder leaves, so
attempts × bound + backoff ≤ (idle + backoff) / 2 < idlewheneverbackoff < idle, andupstreamRequestBudgetguarantees that side condition by cappingmaxRetriesat the largest ladderfitting the resolved deadline. Worst cases, all executed:
IDLE=30000IDLE=10000(floor)IDLE=3600000(ceiling)IDLE=600000 TOTAL=60000IDLE=14001MAX_RETRIES=0MAX_RETRIES=99 IDLE=3600000Every row above is machine-produced from the real
upstreamRequestBudgetandwsNewConnectionMaxWaitMs, not computed by hand — one row was wrong when I did compute it by hand.The
IDLE=14001row is the degradation case, and it is the one #171 made reachable: a deadlinebarely wider than its own 14000ms backoff ladder leaves nothing to queue in, so the bound floors to
zero and the pacer turns itself off with a notice rather than refusing everything past the burst.
Confirmed in a real run, not just in a test — see the smoke matrix below.
Doc conflicts.
README.mdand.claude/docs/oauth-continuation.mdconflicted. #171's rewritten"Provider timeouts" and "Upstream retries" bullets are kept verbatim and this branch's now-stale
pre-#171 "Upstream retries" bullet was dropped; the pacing bullet and pacing section are kept and
their bound arithmetic updated.
One harness is partially repaired, and I am not claiming more than that.
.claude/harnesses/fix-parent-notice-tui-and-epipe-round2.harness.tscalledupstreamMaxRetries,the last tracked reference to the symbol #171 deleted; that call is updated. It still does not
execute, for an unrelated and older reason: it imports
tests/helpers/register-ts-resolve-hook.mjs, which was never committed —git log --allforthat path is empty and no equivalent hook exists in the tree, so both probes have exited 1 at
module resolution since #117. I declined to fabricate an untested resolve hook to revive a harness
neither PR touches; the reference site now records why it cannot run. It is outside
tsconfig'sinclude, so nothing typechecks or runs it either way.Review coverage, stated honestly
Five adversarial reviews ran against this branch: a deadlock/starvation lens, a
test-discrimination lens, a counterexample lens against the refusal semantics, and a cold review of
the final state. Their blocking findings are what produced the current design — the rejected
admit-anyway table above, the no-debit refusal, the derived bound, and the classification demotion
all came out of them.
The fifth, a claims-and-documentation audit, reported after the first push and returned
FIX-BEFORE-MERGE on prose alone — no code defects. Its findings have been applied: the commit
summary no longer promises that rate-limit errors stop; correlation is no longer stated as cause in
the module header, the deep doc, the README or the user-facing error string; "every new connection"
became "primary" to match the documented exemptions; the admission bound is now scoped to
retries-enabled; "will be retried" is qualified because a refusal on the final attempt is terminal;
the burst rationale no longer cites a statistic the header says distinguishes nothing; and four test
names that claimed more than they proved were renamed.
A sixth ran against the rebased tree, cross-family (OpenAI) against an Anthropic author,
pointed specifically at the derived-bound arithmetic under user-configured timeouts and at whether
any claim here outruns its evidence. It ran three rounds, each against the previous round's fix,
and each found something the previous round's green suite did not:
Retry-Afterblocker — the SDK substitutes the hint for its own rung, invalidating thebound derivation. Five earlier reviews and a green suite had all missed it, because every
one of them, mine included, checked the ladder the bound budgeted rather than the delay the SDK
actually takes.
return undetected, plus the safety proof that was still false as written.
implementation defects, plus the disproof of my "no hint strategy can fix this" claim.
Independent work by that reviewer that this PR did not do and should be credited: a behavioural
matrix over 52,800 resolved budget/rate combinations confirming that whenever the pacer refuses an
overflow request, retrying on its emitted hint always eventually reaches an admission; and
real-pacer/real-SDK runs confirming both blockers closed with elapsed times matching the derived
queue bounds.
Rounds 2 to 4 are the argument for not pushing after round 1. The fix for a blocker is exactly
where the next blocker comes from, and the suite was green after every single round — 2080,
2224, 2236 and 2260 tests, all passing, with a live defect present at three of those four points.
The lesson generalises past this PR: the inequality was verified against a model of the retry
policy, not against the retry policy — and then the test for the fix was verified against a model
of itself. The fix is pinned by tests that encode the SDK's real substitution rule, but they still
re-encode the 2s/4s/8s ladder rather than integrating with the SDK, so a change to the SDK's
backoff policy would not turn them red. That limitation is listed below and is unchanged.
One finding is worth calling out as a process failure on my part. It found the sentence claiming
the SDK's backoff "waits OUTSIDE the request's no-data deadline" still present in
ws-upgrade-pacer.tsafter I had reported verifying it was gone. It was: a reviewer'smutate-and-restore reinstated it from a snapshot taken before the correction, and my verification
predated the restore. Every claim in this PR was therefore re-audited from scratch afterwards rather
than trusted from an earlier check.
Failure and rollback
CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=0disables pacing completely and restores today's behaviour.If the derived bound leaves no room to queue, the pacer disables itself and says so rather than
turning off silently. With
CLODEX_UPSTREAM_MAX_RETRIES=0it cannot refuse — nothing would retry arefusal — so it shapes the opening burst and then stops limiting; that is documented as a limitation,
not sold as a guarantee.