fix(streaming): recover transient Codex streams before commitment

[jroth1111]

Summary

• add opt-in, bounded full-stream recovery for transient Codex/OpenAI Responses failures;
• buffer translated output until terminal success and release only the winning attempt;
• retry the same selected auth/model route before normal cooldown and fallback accounting;
• preserve one-time unauthorized refresh, request cancellation, pinned auth, Home dispatch behavior, and legacy bootstrap compatibility;
• add a shared bounded SSE framer that handles split, CRLF, glued, and in-string marker edge cases;
• move streaming retry ownership into the conductor so handlers and interceptors never observe losing attempts.

Fixes #4642.

Why this matters

When Codex is reachable but transiently returns 502/503/504 or ends a stream before completion, normal cooldown accounting can make immediate client retries fail locally with 503 auth_unavailable. Duration-bounded recovery keeps the original request inside CPA, retries the same route, and exposes only the winning response until the configured deadline or client cancellation. Rate limits, authentication failures, and invalid requests remain excluded.

Root cause

Codex response.created is translated into Claude message_start, and the existing bootstrap layers treated the first non-empty translated chunk as irreversible commitment. A later HTTP-200 terminal SSE server_error was therefore surfaced as 502 instead of being recoverable inside the proxy. Normal transient failure accounting could then place the only credential/model into a 60-second cooldown, causing immediate Claude Code retries to return 503 auth_unavailable when the configured retry wait ceiling was shorter.

Behavior

When recovery is enabled explicitly or with a positive attempt cap:

1. the conductor acquires a nonblocking bounded recovery slot;
2. translated chunks are retained until a valid terminal event;
3. eligible 408/500/502/503/504/529, generic terminal server_error, incomplete EOF, and temporary transport failures may retry within one request-scoped budget;
4. retries remain on the selected route before normal failure accounting;
5. a successful attempt replaces all losing chunks and headers;
6. cancellation stops buffering/backoff without cooling the credential;
7. generic 502 exhaustion receives normal cooldown/fallback handling, while request-scoped incomplete-stream 408 remains availability-neutral;
8. byte overflow or concurrency saturation fails open to ordinary streaming and permanently disables replay for that request.

No timeout is added to an established upstream connection. max-retry-window-seconds only limits whether another attempt may begin.

Configuration

Recovery is disabled by default:

streaming:
  recovery:
    enabled: false
    attempts: 0 # no attempt cap when enabled; positive values retain compatibility mode
    max-buffer-bytes: 8388608
    max-retry-window-seconds: 20 # retry-start deadline; maximum 3600
    max-concurrent: 16
    initial-backoff-milliseconds: 250
    max-backoff-milliseconds: 2000

All numeric settings are normalized to explicit safe upper bounds before time.Duration or atomic-counter conversion.

Safety properties

• no replay after downstream bytes;
• no losing attempt reaches response interceptors or client history;
• exact shared attempt/deadline budget across model/auth selection;
• bounded retained bytes and concurrent buffered responses;
• overflow chunk is forwarded exactly once without cloning it before the cap check;
• recovery slot remains held until the buffered response is drained or canceled;
• one-time 401 refresh does not consume the transient recovery budget;
• Home dispatch does not enable recovery or redispatch;
• existing executors remain conservative through zero-value StreamCommitmentUnknown.

Tests

Added coverage for:

• config defaults, bounds, duration-only mode, cloning, diff output, and extreme-value normalization;
• generic Codex error / response.failed server errors and stream commitments;
• queued/created/in-progress lifecycle events as provisional;
• three-or-more glued SSE events, CRLF, split input, partial final frames, in-string }data:/}event: markers followed by real boundaries, and scanner limits;
• losing-attempt discard and winning headers/chunks only;
• cancellation during buffering and backoff with no cooldown or second request;
• recovery-enabled unauthorized refresh;
• exact attempt/window budget, pinned auth, Home disablement, concurrency gate lifetime, overflow ordering, and overflow followed by a later visible error;
• request-scoped incomplete 408 exhaustion without cooldown/fallback;
• generic 502 exhaustion with normal cooldown;
• real BaseAPIHandler + CodexExecutor + httptest.Server two-attempt Claude recovery, proving one downstream message_start, no losing response ID, no error event, and a complete winning stream.

Validation performed on current upstream/dev (c9417c8a):

go test ./... -count=1 -timeout=15m
PASS

go test -race ./internal/sse ./sdk/cliproxy/auth ./sdk/api/handlers \
  -run 'Test.*(Recovery|SSE|Cancellation|Overflow|Unauthorized|Slot|Home|Pinned)' -count=1
PASS

go vet ./internal/sse ./internal/config ./internal/runtime/executor \
  ./sdk/cliproxy/auth ./sdk/api/handlers/...
PASS

go build -o test-output ./cmd/server && rm test-output
PASS

git diff --check
PASS

Related work

• Add provider/model transient-error circuit breaker to prevent retry amplification #4634 was closed as not planned. This PR does not add provider/model circuit state or fail-fast admission; it continues CPA's request-completion priority with bounded retries only inside the active request.
• fix(streaming): retry Responses before semantic output #4580 provides narrower syntactic-prefix bootstrap recovery and overlaps the same bootstrap ownership. If fix(streaming): retry Responses before semantic output #4580 lands first, this branch should rebase onto it and retain that default-path fix; the two should not merge independently without reconciliation.
• fix(streaming): keep the connection alive during the pre-first-chunk wait #4459 provides the shared pre-first-chunk keep-alive helper needed when full recovery intentionally delays the first semantic response. To avoid duplicating that active contribution, this PR is draft pending maintainer direction on stacking/rebasing; it should not merge with streaming.keepalive-seconds enabled until the two are reconciled.
• fix(auth): retry transient upstream overload (503/529) #4584 covers HTTP 503/529 retry policy and has separate conductor integration concerns.
• fix(codex): retry transient capacity errors #3229 covers capacity-specific Codex SSE classification.
• fix(codex): preserve upstream error headers #4635 preserves HTTP non-2xx Codex response headers and Retry-After; this PR does not depend on it.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce85c9cc3f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0405648949

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 663adca221

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Start keep-alives while recovery buffers the stream

When recovery.attempts > 0 and the response fits within max-buffer-bytes, this synchronous collection does not return until a terminal event. The fresh evidence beyond the earlier default-mode report is that full recovery still blocks the OpenAI, Claude, and Gemini handlers before they receive their first chunk and start the configured keep-alive forwarder, so a long generation sends no downstream bytes despite streaming.keepalive-seconds and can be terminated by an ingress idle timeout. Provide a downstream heartbeat path while buffering without timing out the established upstream connection.

AGENTS.md reference: AGENTS.md:L58-L58

Useful? React with 👍 / 👎.

[jroth1111]

Agreed. The existing #4459 implements the shared pre-first-chunk heartbeat path across Claude, Gemini, OpenAI chat/completions, and Responses, including the status-code trade-off after the first heartbeat. I do not want to duplicate or silently supersede that active contribution in this PR. I have marked #4643 draft and documented the dependency while requesting maintainer direction on whether to stack/rebase #4643 onto #4459 or wait for #4459 to land. Until reconciled, #4643 should not merge for deployments relying on streaming.keepalive-seconds during the full-buffer window.

[zzl360]

We reproduced the incomplete-EOF case that this PR is designed to recover.

In the affected client session, the original requests were large (about 200k-232k input tokens), but the failure continued after a successful compact reduced the request to 24,987 input tokens (about 96 KiB). Four consecutive attempts returned HTTP 200 with zero usage and then closed before response.completed. Other sessions continued to succeed, so context pressure and a global proxy/account outage are not sufficient explanations.

The important recovery boundary for us is:

response.queued / response.created / response.in_progress
[optionally semantic output]
EOF or transient terminal error before response.completed

The narrower #4580 bootstrap fix cannot recover once a complete non-empty Responses data frame has committed. #4643 full-stream buffering, same-route retry, losing-attempt discard, and request-scoped incomplete-stream handling match this production case much more closely.

We are carrying a temporary local extension to #4580 for provisional lifecycle-only failures, with the intent to drop it once #4643 is reconciled and merged. Please keep regression coverage for both provisional-only EOF and EOF after semantic output; the latter is the key reason the full-stream design is needed.

[zzl360]

Additional production case from the same session family: a streaming Responses request can fail synchronously in AuthManager.ExecuteStream, before a StreamResult/SSE prefix exists. We observed repeated HTTP 502 responses with an upstream OpenAI-compatible body whose code was server_is_overloaded (four client-level attempts; each CPA request had one upstream attempt).

This path returns before #4580's bootstrap retry loop in sdk/api/handlers/handlers_stream.go, so streaming.bootstrap-retries: 1 has no effect. It is distinct from the provisional-lifecycle/incomplete-EOF case in my previous comment.

Please include regression coverage in #4643 for:

1. synchronous eligible 502/503/504 from the initial ExecuteStream call is retried;
2. a retry can succeed before any downstream bytes are committed;
3. when retries exhaust, the final upstream HTTP status and structured error body/code/message are preserved rather than replaced by a generic missing-response.completed error.

We are adding this as a temporary local extension alongside our #4580-based patch and intend to remove it once #4643 covers the case.

[jroth1111]

Added 639999d8 to cover the synchronous failure path reported above.

The new manager-level regression test exercises direct ExecuteStream errors before a StreamResult exists for HTTP 502, 503, and 504. For each status it verifies that:

• recovery retries the same selected Codex auth;
• only the winning attempt's stream and headers reach the caller;
• the recovered failure does not create credential/model cooldown or failure accounting;
• exactly one configured recovery retry is consumed.

A separate exhaustion test verifies that the exact final structured error and HTTP status are returned when the recovery budget is spent, rather than synthesizing an incomplete-stream error.

Validation passed:

• go test ./...
• go vet ./sdk/cliproxy/auth
• go build ./...
• git diff --check

[jroth1111]

Added duration-bounded recovery in a663d849.

When Codex is reachable but transiently returns 502/503/504 or terminates a stream before completion, CPA can otherwise cool the credential and make immediate client retries fail locally with 503 auth_unavailable. This mode keeps the original request inside CPA, retries the same route, and exposes only the winning response until the retry-start deadline or client cancellation.

It is explicitly opt-in: enabled: true with attempts: 0 uses only max-retry-window-seconds as the bound. Positive attempts retains the existing attempt cap. HTTP 429, authentication failures, invalid requests, and cancellation are not eligible for transparent recovery.

Validation: full go test ./..., focused race tests, vet, required server build, and git diff --check all pass.