Skip to content

fix(openai): reuse connections that free up while parallel agents wait, so prompts stay cached - #185

Merged
bman654 merged 1 commit into
mainfrom
fix/pacing-head-rematch
Sep 5, 2026
Merged

bman654 merged 1 commit into
mainfrom
fix/pacing-head-rematch

Conversation

@bman654

@bman654 bman654 commented Sep 5, 2026

Copy link
Copy Markdown
Owner

When you run a lot of agents at once on a ChatGPT/Codex plan, clodex spaces out how fast it opens
new connections to OpenAI. A turn that gets held in that queue used to decide what it needed the
moment it arrived, and then stick to that decision. So if another turn finished while it was
waiting — freeing up a connection that was already holding exactly the conversation it was about to
continue — it opened a second connection anyway. Now it takes over the freed one instead. That
saves a connection, keeps the prompt cached rather than resending it, and stops one conversation's
duplicates from pushing another conversation's connection out. This is an edge case, not
something you hit routinely
(what is and is not known about that is below), and nothing else
about pacing changes: a turn that
already has a connection to reuse when it arrives is still never delayed.

Closes #173.

Stacked. This branch is based on main but is intended to land after the #174 retry-after
fix and will be rebased onto it. Both touch src/oauth/responses-websocket.ts; the diffs are
disjoint (#174 at failContext ~1313 and the 403 site ~1919, this one at ~2050-2320), so the
rebase should carry cleanly.

The user-visible failure

Under parallel fan-out on one Claude session, clodex opened a new WebSocket connection for a turn
that could have continued a chain head sitting idle in the same partition. Each duplicate head
consumes one of the 8 nursery slots, so it evicts another conversation's reusable head; that
conversation then resends its full context on its next turn — a cache miss, a larger prompt — and
opens another connection, which is more pacing, which widens the window in which this happens
again.

Root cause

#172 introduced an await (pacing admission) between the head scan and connection creation. After
that wait it re-read the partition and demoted itself to parallel_isolated if a sibling had gone
in flight meanwhile. It did not handle the other direction: a sibling that completed during
the wait leaves an idle, reusable head, and nothing re-ran continuationMatch after the wait.

Why a match is possible at all — the non-obvious part

A continuation needs the head's stored requestInput ++ expectedAssistant to be a strict prefix of
the waiting request's input. That reads as though the waiter must already contain the head's
output, which two independent agents never do — and on that reading the whole scenario is
unreachable. It is wrong because expectedAssistant can be empty: continuationMatch's guard
is !entry.expectedAssistant, and [] passes it. A response that completes with no output
items
stores a prefix equal to its own input alone, so any same-partition request that merely
extends that input matches it having copied nothing from it. Two subagents fanned out from one
Claude session open with byte-identical inputs, which is exactly that condition. This shape is
staged directly by a test (below), and the test's premise is checked: give the predecessor output
and the same test opens a third socket instead.

Production reachability — what is and is not known

The necessary conditions are enumerable; the frequency is not known, and the available
diagnostics cannot establish it.
All four must hold at once:

  1. a predecessor leaves a reusable head — a nursery creation or a continuation; a
    parallel_isolated socket is discarded and leaves nothing;
  2. that head's stored input is a strict prefix of the waiting request's input;
  3. it completes inside the waiter's queue wait, bounded at 4,833ms at the shipped defaults; and
  4. the waiter found no match of its own on arrival (matchingCandidateCount == 0).

Streaming all 17 local ledger files (178,298 head decisions, 63,508 primary creations) found no
qualifying opportunity
. That is the honest limit of what the corpus supports, because the same
corpus contains zero ws_new_connection_paced events and zero decisions carrying
pacingWaitedMs
— it records no queued requests at all. With no paced requests in it there is no
denominator, so it cannot be turned into a rate; an earlier draft of this description quoted "about
1 in 20,500 paced requests" and that figure is withdrawn as unsupported.

Nor is that zero evidence that pacing does not engage. ws_new_connection_paced is emitted
only on a nonzero wait, a refusal or an abort, so its absence cannot distinguish "the pacer never
delayed anything" from "the pacer was not there". Replaying the real connection-creation timestamps
from those files through the shipped 60/min, burst-10 bucket predicts 0 waits in the post-release
files but 1,686 waits and 1,590 refusals in the 2026-08-27 file.

What can be said plainly: this is an edge taken when it appears, not something users hit routinely,
and the change is judged on being safe and free rather than on frequency. The log analysis above
was run by the review panel against local ledgers, not re-derived by the author of this change.

Unreachable entirely with CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=0.

The change

src/oauth/responses-websocket.ts (surgical; the file is under the repo's "do not restructure"
directive):

  • The candidate scan is hoisted verbatim into scanForHeads() — same expressions, same
    ordering, same tie-breaks — and the continuation-application block into continueOnHead(), which
    returns its decision so the arrival if/else chain still assigns decision on every path. Both
    hoists are pure moves; the pre-existing 121 tests pass against either version.
  • A request that was queued runs that same scan a second time after admission. On a match it
    adopts the head and leaves exactly the state an arrival-time match would have left: the promotion
    and eviction side effects, persistent restored (so a transport-retry replacement is still
    reusable), the scan results replacing the arrival ones so the ledger is coherent, and
    promptChanges re-derived against the head actually being continued rather than whichever idle
    branch the arrival scan picked as a diagnostic stand-in.
  • The gate reads the pacer's new queued flag, not waitedMs — the latter is a difference of
    two clock reads, so a clock stepped backwards mid-wait reports 0 for a request that really did
    queue, and gating on the number would skip the re-scan precisely there.
  • A rematch returns its pacing token (admission.release?.()). It opened no connection, so
    holding the token would delay the next request for an upgrade that never happened — the same rule
    a cancellation already followed. Measured before this: a real-pacer probe showed the next
    connection waiting an unnecessary ~1s.
  • feat(openai): reduce the risk of rate-limit errors when many agents run at once #172's in-flight demotion is preserved exactly and is still unconditional; it is now guarded by
    !selected, which is a no-op on the path feat(openai): reduce the risk of rate-limit errors when many agents run at once #172 shipped.
  • A gap warning raised while classifying against the arrival partition — "Prompt caching is
    degraded for this turn" — is deferred rather than raised, and dropped if the request then
    continues a freed head. Nothing was degraded, and a scary notice on a perfectly cached turn is how
    the one warning whose value depends on being believed gets trained away. Deferring the whole
    warning rather than emitting and retracting it also means a dropped warning costs none of the
    per-signature budget the next genuine occurrence needs. It is dropped only on that path: a
    refusal, an abort or any non-rematch outcome still raises it exactly as before.
  • New ledger fields pacingRescanOutcome (continuation | parallel_isolated | no_change),
    recorded only when the re-scan ran, and suppressedMismatchWarnings when a warning was dropped;
    the drop is also traced, so it is never invisible.

src/oauth/ws-upgrade-pacer.ts: UpgradeAdmission gains optional queued and release. release
is one-shot — a second call would mint a token the bucket never charged, which is the one way a
refund can raise the sustained rate instead of correcting it.

Atomicity

Nothing between the re-scan and dispatchContext awaits — the nursery eviction, the diagnostic
emit and new ReadableStream({ start }) are all synchronous, and start runs synchronously in the
constructor — so selection and the inFlight claim happen in one synchronous run. Two queued
requests released together cannot both adopt one head. (Verified by the review panel with an
inserted-yield mutation, not only by argument.)

What I deliberately left out

  • The post-wait block cannot undo an arrival-time parallel_isolated demotion when the
    partition simply goes quiet during the wait.
    Such a request already has persistent === false
    and, absent a re-match, keeps it, so it opens an isolated socket that retains no head at all.
    Isolated sockets are 35-38% of decisions in busy diagnostics files — a larger share of the
    same harm than the slice this PR closes. Leaving it is a deliberate scope choice, not an
    oversight; re-persisting such a request is a behaviour change to feat(openai): reduce the risk of rate-limit errors when many agents run at once #172's demotion and deserves its
    own issue, tests and reachability analysis.
  • No re-scan for a request admitted on arrival. It resumes in the same microtask turn, and a head
    can only be freed by an upstream completion, which arrives on a socket event — a macrotask.
  • No change to ws_new_connection_paced (it still records nothing on a zero wait), and no widening
    of continuationMatch. The safety property is entirely delegated to the unchanged matcher.

Discriminating tests and the mutations that prove them

Full-file runs throughout, never -t. Snapshots restored with cp, never git checkout. All
head/chain state is staged through production producers (response.output_item.done /
response.completed), never planted in the request input; every clock is injected, so no test
depends on Date.now() ordering.

Test Mutation Result
does not continue a freed head that diverges deep inside a long history — 24 production-produced turns plus the head's own answer, agreeing on the first 19 items and differing at item 20 make the prefix check compare only the first item RED: length 2 → 1, i.e. a shallow matcher continued a chain that was not ours. The shallow negative below stays GREEN under that same mutation, which is why this test exists
continues a head opened by a shorter sibling that overtook it from an empty partition#173's own ordering: the queued request is first into an empty partition (new_partition_head, still persistent), then overtaken rematch only requests that arrived non-persistent RED: length 1 → 2
no "caching is degraded" warning for a turn that then continued a freed head always flush the deferred warnings RED on the user-visible assertion: the notice contains Prompt caching is degraded
same, deferral itself make the warning raise immediately instead of deferring RED
continues a head a sibling freed mid-wait delete the re-scan RED: length 1 → 2 (the duplicate)
continues a head that completed with no output, having copied nothing from it delete the re-scan RED: opens a third socket
same, premise check give the predecessor output RED: no match, third socket — so the empty-expectedAssistant shape is what carries it
opens its own head when the freed head does not match (safety) widen continuationMatch to accept anything RED: length 2 → 1, i.e. it continued a chain whose lineage did not match
re-matches on the queued flag, not elapsed time gate on waitedMs > 0 RED
adopted head stays reusable when its transport then fails drop the persistent restore RED: length 2 → 3
prompt drift is reported against the adopted head drop the re-derivation RED
…and its trace line drop the debug emission RED
re-scanned candidate counts in the ledger drop the scan-result reassignment RED
#172 regression: in-flight demotion still yields parallel_isolated (existing test, extended with the new ledger field) delete the re-scan RED
no re-scan for a request admitted on arrival (CLODEX_WS_MAX_NEW_CONNECTIONS_PER_MIN=0, production shared pacer) remove the queued gate RED
pacer: a released token returns to the bucket, and only once drop the refund / drop the idempotence guard RED in both directions
pacer: returns a queued admission's token, and only the fraction it debited — driven to the debt floor, where a reservation can only take 0.833 of a token make the queued path's release a no-op RED
same refund a whole token instead of the consumed fraction RED: the next arrival is admitted immediately on capacity the bucket never charged
pacer: reports queued even when the clock steps backwards derive queued from elapsed time RED

The token-release chain is proven in two halves: the pacer unit tests prove the bucket honours a
release on both the immediate and the queued path and refunds only the fraction it debited; the
transport tests prove the call site releases exactly once on a rematch and never when it actually
opens a connection.

Runtime evidence and environment

export CLODEX_HOME=$(mktemp -d) CLAUDE_CODE_ENTRYPOINT=cli && pnpm typecheck && pnpm test && pnpm build — typecheck clean, 2331 tests / 107 files passed, build success. Node v24.14.1
(matches .nvmrc and CI), macOS 15 (darwin 25.6.0), pnpm 10.34.5.

The suite was run both with and without the ambient CLODEX_WS_MAX_CONNECTIONS=64 /
CLODEX_WS_MAX_NURSERY_CONNECTIONS=24 that this machine exports, green in both, so no assertion
here depends on the local caps. No outbound HTTP(S) proxy variables were set; ws is mocked and the
dynamic import is warmed before any concurrent phase, so no test opens a real socket.

What I could NOT verify

  • No live-provider run. Nothing here was exercised against real ChatGPT OAuth traffic; all
    behavioural evidence is the fake-socket suite. No clodex claude -p smoke test was run on this
    branch — the change is confined to the OAuth WebSocket transport, but the Anthropic passthrough
    and translated legs are untested here and that is a known gap, not a silent one.
  • The connection/cache saving is inferred, not measured end to end: one fewer socket and a
    retained prefix follow from the mechanism, but no before/after cache-hit measurement was taken.
  • The ~1s unnecessary delay the token release removes was observed by the review panel with a
    real-pacer probe; this branch pins the call and the bucket behaviour separately rather than
    re-measuring that latency.
  • The log analysis behind the reachability statement was run by the review panel, not re-derived
    here — and, as stated above, it establishes the absence of an observed opportunity, not a rate.
  • The "Prompt caching is degraded" suppression is verified against the tool-argument canary; the
    reasoning-gap canary shares the same deferral seam but is not separately staged in a rematch.
  • The microtask/macrotask argument for skipping the re-scan on arrival, and the synchronous-start
    basis of the atomicity argument, are reasoning from the specification and the event-loop model —
    the panel's inserted-yield mutation corroborates the atomicity conclusion, but neither is a
    direct instrumented measurement of production.

Failure and rollback

Every new path is additive and fails closed onto existing behaviour. If the re-scan finds nothing,
the request proceeds exactly as #172 left it. The one timing change on an unaffected path is that a
gap warning is now raised just after the pacing admission rather than just before it — the same
synchronous run of the same request, and it is still raised on the refusal and abort exits. If a pacer implementation omits queued or release
(both optional), the code falls back to the elapsed time and simply keeps the token — i.e. today's
behaviour. Reverting the commit restores #172 exactly; no persisted state, config or on-disk format
is touched, so there is no upgrade or migration concern.

…t, so prompts stay cached

When many agents share one session, clodex spaces out the new connections it opens to OpenAI. A
turn held in that queue was sized up against the connections that existed when it arrived, so when
another turn finished and freed one up while it waited, it opened a second connection anyway
instead of picking that one up. Duplicates crowd out other conversations' connections, and every
one evicted costs that conversation a full resend of its context on its next turn: a cache miss, a
bigger prompt, and one more connection.

A turn that was queued now looks again before it opens anything, and takes over a connection whose
conversation it continues exactly. That is the same exact-history check used on arrival, so a
freed connection whose conversation does not line up is still never continued, and a turn that was
let through on arrival behaves exactly as before. Having opened nothing, it also hands back the
new-connection allowance it was charged, so the next turn is not delayed for a connection that was
never opened.

This is an uncommon case, not a common one: it needs a same-conversation predecessor to finish
inside the few seconds this turn spends queued. The reachability measurement is in the pull
request.

Refs #173
@bman654
bman654 changed the base branch from fix/throttle-retry-after-header to main September 5, 2026 14:35
@bman654
bman654 merged commit 6cc786f into main Sep 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pacing: a queued request does not re-match a head that freed up while it waited

1 participant