Commit Graph

9 Commits

Author SHA1 Message Date
sususu
bf44a7f892 style: gofmt codex bootstrap and cache-control tests 2026-09-22 00:18:05 +08:00
Luis Pater
563865e77a test: fix timing synchronization and channel races in streaming tests
- Wait for bootstrap initialization before advancing mock clock in Codex WebSocket tests
- Reorder data channel closures in OpenAI responses stream error tests
- Drain stream chunks in byte-cap release test to prevent dangling goroutines

Closes: #5981
2026-09-21 00:39:28 +08:00
sususu
6e307553f4 feat(codex): add optional time ceiling for stream bootstrap buffering
- Add `stream-bootstrap-timeout` configuration (defaulting to 0/unlimited, recommended 20s behind reverse proxies) to bound how long early handshake or trickled events may hold response headers.
- Release stream buffering into normal in-stream delivery once the time budget is exhausted on both SSE and WebSocket executors.
- Deliver post-timeout overload and status-bearing errors in-stream rather than triggering credential failover, preventing latency doubling on long reasoning turns.
- Provide thread-safe mock clock test harness and comprehensive unit tests covering timeout release, unlimited default, disabled ceilings, and post-timeout error delivery.
2026-09-14 14:25:45 +08:00
sususu98
bb20fa2d5e Merge pull request #5724 from Viggo95/fix/codex-bootstrap-buffer-noncontent-events
fix(codex): keep bootstrap buffer open for events that carry no output
2026-09-14 12:27:00 +08:00
sususu
0719520f2a feat(api-call): support $TOKEN$ replacement in request body data 2026-09-14 09:20:03 +08:00
Viggo95
cb73cd9936 fix(codex): buffer keepalive and empty item announcements during codex bootstrap
The bootstrap buffer released the stream on the first frame outside the
handshake allow-list. Upstream sends `keepalive` heartbeats and
`response.output_item.added` before the first token, so the downstream headers
were committed while nothing had been produced, and an overload rejection
arriving afterwards could no longer be retried on another credential.

Extend the allow-list with `keepalive`, and with `response.output_item.added`,
`response.content_part.added` and `response.reasoning_summary_part.added` when
they announce something that is still empty. `response.output_item.added` is
accepted only for `message`, `reasoning`, `function_call` and `custom_tool_call`
items, and only while their content, summary, arguments or input is empty: every
other item type stands for a server-side operation that may already be running -
a `web_search_call` is announced with status "in_progress" and its `searching`
event follows immediately - and failing the attempt over after one would run
that operation again on another credential. Part announcements are matched on a
closed list of textual part types for the same reason.

The list stays closed. "Nothing has happened yet" cannot be derived from the
absence of a TTFT token, because TTFT deliberately ignores server-side tool
traffic such as `response.shell_call_output_content.delta` and its `.done`
counterpart, so an unrecognised frame, an unrecognised item type and an
unrecognised part type all release the stream exactly as before.

Holding those frames also required fixing the bound on how much may be held.
codexBootstrapMaxBufferedEvents was enforced with len(bufferedChunks), and a
chunk count bounds only the downstream formats that render every upstream frame:
the OpenAI Chat Completions and Gemini translators return zero chunks for a frame
they do not recognise, which is true of every frame added here. Count frames read
from the upstream instead, and cap what a bootstrap accumulates over the upstream
frames and the chunks they translate into. The SSE scanner walks physical lines
and an event can arrive as one line (": keepalive"), two, or three
(event:/data:/blank), so the frame budget is sized for the widest framing rather
than derived from any separator. The websocket loop counts a message as soon as
it is read, before the branches that skip non-text and whitespace-only messages,
so a peer sending only frames the loop skips cannot hold the downstream headers
open; the message that exhausts the budget is still processed and delivered
rather than dropped. Both caps are checked before a frame is admitted, so one
oversized frame cannot be taken on the strength of an empty buffer. This widens
the window where one frame rendered one chunk - the websocket path effectively
moves from 16 frames to 48.

An empty `response.incomplete` seen while buffering is now delivered in-stream
instead of failing the attempt over, matching the websocket executor and the
documented contract that only overload and rate-limit rejections trigger
failover.

`isCodexHandshakeMetadataEvent` is renamed to `isCodexBootstrapBufferableEvent`
because the list is no longer only handshake metadata. Both config doc sites
describe what is held, that heartbeats are held too, that SSE also holds the
lines around a `data:` frame, and that the hold is bounded by frames and bytes
rather than by wall-clock time.
2026-09-12 22:27:44 +08:00
Luis Pater
3ae9093da8 fix(codex): treat model capacity errors as bootstrap overload failures
- Broaden pattern matching for Codex model capacity errors.
- Classify model capacity rejections as overload bootstrap failures to enable failover.

Closes: #5634
2026-09-10 12:06:01 +08:00
Luis Pater
ba7e55836d fix(codex): recognize retryable server errors for bootstrap failover
- Check `error.message` and `message` for retry advice on upstream `server_error` responses.
- Treat server errors indicating the request can be retried as eligible overload bootstrap failures.
2026-09-08 03:49:10 +08:00
sususu98
4b9d404fb0 feat(codex): add opt-in stream bootstrap buffering and overload failover (#5115)
* feat(codex): add opt-in stream bootstrap buffering

The upstream smuggles capacity rejections into an HTTP 200 stream. The
handshake events arrive normally and only a later event carries
{"error":{"type":"service_unavailable_error","code":
"server_is_overloaded"}}. By then the executor has already handed the
first chunk downstream, the response is committed, and the conductor can
no longer retry on another credential, so the request fails even though
other credentials were available.

When codex.stream-bootstrap-buffering is enabled the executor holds back
the handshake events until it can tell whether the stream carries real
output or a rejection. An overload rejection then fails the attempt
before any chunk is delivered, letting the conductor retry on another
credential; every other terminal failure is flushed in order and
delivered in-stream exactly as before.

Detection uses an event-type allow-list rather than a fixed count. On the
websocket transport codex.rate_limits and codex.response.metadata arrive
before response.created, making the first generated event the fifth
frame, so a small counter would release the stream before the rejection
is visible. Buffering is bounded and hitting the bound degrades to the
original unbuffered behaviour.

Two details are load-bearing. The error must be returned synchronously:
delivering it as the first stream chunk makes ExecuteStream downgrade it
into a committed 200 and the status is lost. And the websocket path must
not signal an upstream disconnect for a rejection it intends to retry,
because the downstream handler closes the client connection on that
signal and the retry would have nowhere to deliver.

The 503 status is produced only on this path rather than in the shared
codexTerminalFailureStatus mapping, so disabling the feature restores the
previous behaviour exactly, including cooldown classification and
retry-after parsing.

Defaults to false: response headers are withheld until generation
starts, which can trip client or reverse-proxy read timeouts.

* test(codex): pin bootstrap overload failover through the conductor

Executor-level tests cannot show what the client finally receives. These
exercise ExecuteStream end to end to pin three properties that are easy
to regress:

- consecutive overloaded credentials are skipped until one serves the
  request, and retries are capped by max-retry-credentials rather than
  multiplying with request-retry
- exhausting the pool surfaces the upstream status instead of a
  committed 200 stream
- with buffering disabled the rejection stays an in-stream error on a
  committed stream, which is the behaviour the feature must preserve

The third case also documents why the executor returns its error
synchronously: an error arriving as the first stream chunk is wrapped and
downgraded into a committed 200, silently losing the status.
2026-08-20 21:31:53 +08:00