Skip to content

feat(oauth): log the usage-limit state the Responses socket already reports - #237

Open
TweedBeetle wants to merge 1 commit into
bman654:mainfrom
TweedBeetle:feat/ws-rate-limits-telemetry
Open

TweedBeetle wants to merge 1 commit into
bman654:mainfrom
TweedBeetle:feat/ws-rate-limits-telemetry

Conversation

@TweedBeetle

Copy link
Copy Markdown
Contributor

What this changes for users

When you use a ChatGPT-plan model through clodex, OpenAI's server reports how much of your usage limit is left on the same connection clodex already has open, and clodex was ignoring it. With --ws-diagnostics on, clodex now writes that report into the diagnostics log, including the separately metered allowances (such as a reserve bucket) that arrive with it. How requests are sent and answered does not change.

Problem and root cause

This affects anyone on the OpenAI OAuth route who runs with --ws-diagnostics. The Responses WebSocket sends a codex.rate_limits event; native Codex parses it (codex-rs/codex-api/src/endpoint/responses_websocket.rs, parse_rate_limit_event, rust-v0.154.0). clodex never recorded it:

  • handleSocketMessage returns before parsing when no request is in flight, so a frame that arrives between or after responses was never read.
  • A frame that arrived during a response was parsed as response data and then ignored.

I found this while trying to tell which allowance a request had drawn from. rate_limits answers that for the main window, but the other allowances report in additional_rate_limits on the same frame, so the change keeps both.

The change

  • codex.rate_limits is recorded as a ws_rate_limits event with a phase of during_response or idle. rate_limits, additional_rate_limits, code_review_rate_limits, credits and promo pass through without coercion, so a fractional percent survives and a null or missing field is not reported as zero. Each gets a …Bytes size in UTF-8 bytes, and a value whose JSON is over 8,000 bytes is dropped while its size is kept. fieldCount gives the number of top-level keys and fieldsPresent lists at most 24 of their names, each through boundedDiagnosticIdentifier.
  • Correlation. A frame that arrives during a response goes out through that request's ctx.emitDiagnostic, so it carries the request's requestId and claudeSessionId. Idle frames go through a connection-scoped sink that passes an explicit empty correlation, because socket callbacks run in the async context of the request that created the socket and reading diagnosticContext.getStore() there returns that first request's ids. An earlier version of this branch did that; running on our proxy, most frames on reused connections carried the id of an older request, and a single id appeared on 166 frames.
  • The connection sink is wired at both createConnection call sites, including the transport-retry replacement.
  • With diagnostics off, nothing extra is parsed or recorded: an idle frame is not even decoded, and a meter frame during a response is skipped.
  • README and --help text for --ws-diagnostics mention the new events, and .claude/docs/oauth-continuation.md documents them.

One departure from the rest of the WS diagnostics log: elsewhere it records upstream strings only as bounded identifiers or hashes, which is why the README calls it sanitized. These events record the server's usage objects verbatim, and those include account state such as credit balance and promotions. The log is a local file the user opts into, and the README, --help text and doc now say so.

Left out on purpose: nothing consumes these events, nothing reaches the terminal, and no quota logic acts on them. An earlier draft also logged meter-named headers from a successful upgrade; 1,227 real upgrades carried none, so I removed it rather than ship a listener that has only ever reported zero.

Evidence

  • pnpm typecheck && pnpm test && pnpm build on this branch: 118 files, 2,664 tests passed, CLODEX_HOME="$(mktemp -d)", proxy variables unset, node v22.23.2.

  • Mutations, each run against the full tests/responses-websocket.test.ts with the source restored between runs. Every one turns at least one test red:

    • connection sink reads the ambient async context instead of an empty correlation
    • in-response frames sent through the connection sink instead of the request's sink
    • both of the above together, which is the earlier behaviour
    • replacement connection created without the sink
    • idle observer returns immediately (4 tests fail)
    • fieldsPresent cap removed; fieldsPresent name cleaning removed; planType cleaning removed
    • ledger size cap removed; additionalRateLimitsBytes removed; code_review_rate_limits or promo not recorded
    • codex.response.metadata accepted as a meter event again
    • the if (!emit) guard removed, so a meter frame with diagnostics off would throw inside the message handler
    • ledger size, or the 8,000 limit, counted in characters instead of UTF-8 bytes; fieldCount removed

    One mutation stays green on purpose: dropping the connection-sink check in handleSocketMessage changes nothing observable, because observeQuotaEvent returns early without a sink. The check is there so an idle frame is not decoded when diagnostics are off.

  • The correlation test reproduces the production condition directly: it opens the socket inside request A's diagnostic context, runs request B on the reused socket, and emits every socket event from inside A's context. It checks that the in-response frame carries B's ids and that the idle frame carries none.

  • The idle "stays silent" test now also sends a real meter frame afterwards and expects exactly one event, so it cannot pass with the observer removed.

  • Reachability: stated above.

  • Runtime: an earlier version of this change (fork commit 9453a67, before the correlation fix) ran on one machine's proxy on the OpenAI OAuth route from 2026-09-12 to 2026-09-13, logging 3,851 ws_rate_limits frames, all during a response. Live frames have type: codex.rate_limits and carry additional_rate_limits as an object keyed by allowance name (gpt-reserve, GPT-5.3-Codex-Spark), each with primary/secondary windows; the sibling-ledger test uses that shape. On a request to gpt-reserve, rate_limits.primary was the reserve's own 7-day window.

  • Concurrency: main has not moved since the branch cut. Open feat(oauth): keep parallel conversations cached instead of dropping them at a fixed limit #236 touches the same files plus .claude/docs/oauth-continuation.md; the only conflict is that both append a describe block to the end of tests/responses-websocket.test.ts. With both blocks kept, typecheck passes and the file passes (183 tests), so either land order works.

  • Commit summary line reads as a release note for a non-technical user.

  • Manual smoke of the passthrough and translated legs: no launch path is touched. The corrected correlation has not yet run against the live server; only the earlier version has.

What I could not verify:

  • The idle path. Every frame captured so far arrived during a response. The idle branch is covered by tests only.
  • What the sibling ledgers mean. I am reading additional_rate_limits as the per-allowance report from what the server sends on reserve and Spark requests. I have not found it documented.

Failure and rollback behavior

Observation only. Nothing changes a request, a head decision or an upstream write. The handler returns early when diagnostics are off, when the frame is not JSON, or when it is not a meter event. Nothing is written anywhere except the diagnostics log, and reverting the commits restores the previous behaviour.

…eports

The OpenAI Responses WebSocket sends a codex.rate_limits event on the connection clodex already
holds; native Codex parses it (codex-rs/codex-api/src/endpoint/responses_websocket.rs,
parse_rate_limit_event, rust-v0.154.0). clodex dropped it: handleSocketMessage returns before
parsing when no request is in flight, and during a response the frame was never recorded.

With --ws-diagnostics this records a ws_rate_limits event per frame: rate_limits,
additional_rate_limits, code_review_rate_limits, credits and promo, uncoerced, each with its
serialized size and dropped above 8,000 characters, plus bounded field names and plan type. A
frame inside a response is emitted through the request's own diagnostic sink, so it carries that
request's ids. Idle frames use a connection sink with an explicit empty correlation: socket
callbacks run in the async context of the request that created the socket, so the ambient
context would stamp an older request's ids on a reused connection. The sink is wired at both
createConnection call sites, including the transport-retry replacement.

Observation only. No request, head decision or upstream write changes. Documented in
.claude/docs/oauth-continuation.md; README and --help mention the events.

Session: a5ae056f-79a5-4beb-831d-c16d7b2d37dc

@bman654 bman654 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and for writing down the mutations you ran. We reproduced the change and found no
blocking defect. It only observes: it never changes a request, a head decision, or what goes
upstream, and it does nothing unless the user starts clodex server --ws-diagnostics. Everything
below is MINOR or smaller. Fix it here if you are touching the branch anyway, or in a follow-up.

Timing: we'll hold the merge until tomorrow in case you want to adjust any of the non-blocking
items below. If we don't hear back by then, we'll merge as is.

main has moved, so the PR now shows a conflict. We will resolve it on our side (details at the
end), so you don't need to rebase.

Non-blocking notes

1. Record the limit name as a value, not only as a key name (MINOR, conditional).
observeQuotaEvent (src/oauth/responses-websocket.ts:2195-2233) records rate_limits, the four
sibling fields and plan_type, but drops the values of the top-level metered_limit_name and
limit_name. At the tag you cite (rust-v0.154.0), native Codex's RateLimitEvent declares both
and uses metered_limit_name.or(limit_name) as the snapshot's limit_id
(codex-rs/codex-api/src/rate_limits.rs:124-160). That id says which allowance a report belongs
to, which is the question this PR set out to answer. A probe frame carrying both fields produces an
event that lists them in fieldsPresent (when they fall within the first 24 keys) with no value
for either.

This stays MINOR and conditional because we have no live frame carrying either field, and the
direct codex.rate_limits fixture in Codex's client_websockets.rs sends neither. Did any of
your 3,851 captured frames contain either key?
If none did, we'll drop this note. If they did,
record them in a bounded form that keeps the values you actually saw. Check the real values before
reaching for boundedDiagnosticIdentifier: it allows only [a-zA-Z0-9_.:/-] (:1135-1140), so a
human-readable name with a space would vanish. Add a positive test and a hostile-string test.

2. The startup warning still describes the old scope of the log (MINOR).
You updated README line 149 and --help (src/cli.ts:517-519) to say these reports are recorded
verbatim and include credits. The line a user actually sees when the server starts still reads
"Diagnostic mode records request headers and metadata; credential headers are redacted."
(src/server/index.ts:514, src/http-proxy/index.ts:245). Someone with --ws-diagnostics already
in a script won't reread --help after upgrading. This is not a security hole: the file is 0600,
the flag is opt-in, nothing leaves the machine, and the new data is account state rather than
conversation or credentials. It is a gap in what the user is told they consented to. Narrowest fix:
add something like "and OpenAI usage-limit reports verbatim (including credits and promotions)" to
both lines.

3. The main-ledger byte count is unpinned (MINOR, test coverage).
Filling rateLimitsBytes from the promo ledger (:2223) still passes the full
tests/responses-websocket.test.ts run (172/172), although the doc promises that size
(.claude/docs/oauth-continuation.md:610-611). Replacing connectionId/generation
(:2213-2215) also stays green. Those two aren't documented, so pinning them is optional hardening.
Narrowest fix: one whole-event assertion in an existing test, with expected sizes derived from the
fixture via Buffer.byteLength(JSON.stringify(fixture)). Optionally, check
connectionId/generation against the same request's ws_head_decision.

4. idle means "can't be attributed", not "not caused by a response" (MINOR, wording).
The doc (oauth-continuation.md:599-601) says an idle frame's change "belongs to no particular
response", and the source comment (responses-websocket.ts:2183-2189) says attributing it would
"invent a number". The code only knows whether entry.current was set when the frame arrived, and
on a persistent head response.completed clears current (:2078-2096). A meter frame sent
right after completion, and caused by that response, is labeled idle. Suggest wording like
"clodex cannot safely attribute an idle frame to a request". The code itself is fine.

5. Commit summary / release note (NIT, we'll handle it at merge).
feat(oauth): log the usage-limit state the Responses socket already reports uses internal wording
("Responses socket") and doesn't say why a user would care. Unless you'd rather pick your own, we'll
squash with something like
feat(oauth): log OpenAI usage limits to help diagnose depleted allowances.

Checks that came back clean

Every run used unset proxy variables, a throwaway CLODEX_HOME and CLAUDE_CODE_ENTRYPOINT=cli. We
ran the whole test file each time and restored the source from a snapshot between mutations.

  • Socket callback context (the premise of your correlation test). We ran the real ws@8.21.0
    client against a loopback WebSocketServer under AsyncLocalStorage on Node 24.14.1, both
    directly and through an HTTP CONNECT proxy agent. open, message (a frame sent while request B
    was active, and an unsolicited one) and close all ran in the store of the request that created
    the socket. Emitting inside context A by hand therefore matches real behavior, and the explicit
    empty correlation for idle frames is correct.
  • Correlation mutations. Having the connection sink read the ambient store fails the correlation
    test (stale req-A). Routing in-response frames through the connection sink fails the same test.
  • Feature deletion. Removing both observer calls turns 10 tests red. The only usage-limit test
    that stays green is the "diagnostics off" negative, as expected.
  • Representative listed mutations. Idle observer returning immediately: 4 red. Replacement
    socket without the sink: 1 red. if (!emit) guard removed: 1 red. Beyond your list, a mislabeled
    phase (3 red), fieldCount off by one (1 red) and a dropped null credits (1 red) are also
    pinned.
  • No change to responses. With diagnostics off, the idle path returns before decoding
    (:1810-1813), and during a response observeQuotaEvent exits before serializing. The observer
    neither returns early nor mutates ctx. trackReasoningProtocol, captureOutput and terminal
    handling run as before.
  • Hostile input. Malformed JSON, null, arrays, primitives, and an onDiagnostic callback that
    throws (on both active and idle frames) all leave the response intact. boundedLedger catches
    serialization errors, and emitDiagnostic already swallows callback errors (:283-297).
  • Scope of the opt-in. Only clodex server --ws-diagnostics enables this. No env var, no
    --trace, no saved mode and neither clodex claude nor clodex-claude turns it on. Log
    directories are 0700, files are 0600, and every line still goes through credential redaction.
  • Vendor facts. At rust-v0.154.0, responses_websocket.rs matches exactly codex.rate_limits
    and calls parse_rate_limit_event. rate_limits.rs allows a fractional used_percent and
    optional windows and credits, as you describe.
  • Ledger census. Our local diagnostics corpus (49 files, about 1.02M rows) has no
    codex.rate_limits rows. That is expected, because those processes ran without this observer. It
    neither confirms nor contradicts your capture. The idle branch is reachable in the state machine,
    but like you we cannot say whether OpenAI ever sends an idle frame.
  • CI (test, commitlint) is green at ba7d8e1.

Main moved: #236 merged after this branch was cut

The body's concurrency note ("main has not moved") is now out of date: #236 and the 2.13.0 release
commit have landed. The only conflict is the one you predicted, with both PRs appending a describe
block to tests/responses-websocket.test.ts. A trial merge of main into ba7d8e1 that keeps both
blocks (f2658d1) leaves src/ unaffected by the resolution. Every ConnectionEntry is still
built in createConnection, and both of its callers pass the sink. #236 added no other construction
path. On Node 24.14.1: pnpm typecheck OK, pnpm test 118 files / 2,675 tests passed, pnpm build
OK. We'll merge main into your branch with that resolution before landing.

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.

2 participants