feat(proxy): fall back to raw HTTP on replay-safe bridge cooldown suppressions - #1890
Closed
dpearson2699 wants to merge 12 commits into
Closed
feat(proxy): fall back to raw HTTP on replay-safe bridge cooldown suppressions#1890dpearson2699 wants to merge 12 commits into
dpearson2699 wants to merge 12 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
dpearson2699
force-pushed
the
feature/1886-bridge-cooldown-http-fallback
branch
from
August 23, 2026 21:29
3fb7d97 to
8a6bc15
Compare
This was referenced Aug 24, 2026
…is unavailable A websocket-only upstream outage currently cascades into account unavailability: every connect timeout records a transient account error, retrying clients drive all accounts into error backoff, and hard session affinity then fails follow-up turns closed with previous_response_owner_unavailable on every transport. codex-rs activates its session-scoped HTTP transport fallback only when the websocket handshake is rejected with HTTP 426 (rust-v0.149.0, core/src/client.rs); in-band 5xx error events retry on the websocket transport, so the existing in-band service-level failures never steer clients to HTTP. - Surface server-level transient websocket connect failures without recording account health or rotating accounts. - Arm a bounded (60s) transport-failure marker on that path, cleared on the next successful upstream websocket connect, and deny responses websocket handshakes with HTTP 426 while it is armed or while upstream_stream_transport is pinned to http. - Bypass the HTTP responses bridge under a pinned http transport and fall back from transient pre-stream bridge session-creation failures to raw HTTP streaming (skipped while an API-key usage reservation is unsettled). Closes Soju06#1885
Address Codex review on Soju06#1886: - Gate the connect-stage surface on failure_phase="connect" so OAuth refresh transport errors (and other account-scoped failures sharing the upstream_unavailable envelope) keep the classify-penalize-failover path toward healthy accounts instead of arming the instance-wide 426 marker. The websocket open timeout and invalid-handshake raises now carry that provenance alongside the existing InvalidStatus/OSError sites. - Arm the handshake-denial marker when a websocket open exhausts the request budget, covering deployments whose budget is shorter than the open timeout where the budget-exhausted emit bypasses the failover decision. - Force the HTTP upstream transport on bridged and raw responses paths while the marker is armed, so a sticky follow-up that the 426 denial moved to the HTTP route cannot resolve back onto the unavailable websocket upstream through the smart policy. - Restrict the bridge raw-HTTP replay to failures carrying pre-submit session-creation provenance, so a post-dispatch upstream_unavailable can never dispatch the same turn twice. - Move the marker into _service/streaming/transport_health.py shared by the websocket mixin, the responses websocket routes, and the bridge.
The budgeted websocket opener's process-network recovery branch reaches the same budget-exhausted emit as the stalled-open branch and bypasses the failover decision, so it must arm the handshake-denial marker too. Only the websocket open runs inside that loop, so the provenance is unambiguous; the outer connect-attempt wrapper also covers token refresh and deliberately stays unarmed.
The model-replacement connect branch surfaces failures without entering the failover decision, so a qualifying connect-phase 5xx transport failure on the replacement open never armed the handshake-denial marker. Extract the provenance classifier shared by both paths and arm the marker in the forced-surface branch too.
The budgeted opener's loop also runs route resolution, whose upstream_proxy_unavailable failures are pre-dispatch route evidence; an exhausted recovery wait on one must not deny subsequent handshakes with 426. Reuse the shared connect-provenance classifier before arming.
The proxy architecture matrix allows http_bridge and websocket to import only their listed domains (support among them, streaming not), so the shared transport-failure marker belongs in _service/support.py rather than a new streaming module. Also align the test harness override of _handle_websocket_connect_error with the base signature for ty.
An exhausted token-refresh loop surfaces the same pre-submit 502 upstream_unavailable envelope as a websocket-open failure, but it is account evidence: replaying it over raw HTTP re-runs the same failing refresh and buries the actionable error under no_accounts (caught by test_v1_responses_http_bridge_transient_refresh_failure_returns_ upstream_error in integration-bridge CI). Gate the wrapper fallback on failure_phase == "connect", mirroring the websocket-side provenance rule, and record the refresh case as a propagation scenario in the OpenSpec delta.
dpearson2699
force-pushed
the
feature/1886-bridge-cooldown-http-fallback
branch
from
August 24, 2026 21:21
8a6bc15 to
80021f4
Compare
Collaborator
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80021f4f43
ℹ️ 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".
The sanitized error code cannot carry transport provenance in either direction, so classifying on it both missed real outages and captured scoped failures: - The responses policy preserves the upstream handshake body, so a direct 5xx upgrade rejection surfaced as `upstream_error` and never armed the 426 marker — the common direct outage left Codex clients retrying websocket indefinitely. - A routed 5xx handshake, a TLS verification failure and host-wide network loss all share the `upstream_unavailable` envelope, so each was misclassified as a global outage and pushed unrelated clients onto HTTP. The direct upstream open now stamps host-scoped transport provenance and withholds it from credential-scoped rejections, TLS failures, host-wide network loss and every routed open; the shared classifier moves to the support domain and keys on that provenance. Also: - A request budget shorter than the local websocket-connect admission wait expired before the connector ever ran and still armed the marker, answering local contention by forcing every client onto HTTP. The budgeted opener now tracks whether the connector began. - Bridge session creation runs its own pre-dispatch failover and never reaches the websocket failover decision, so bridge-only traffic left the marker clear. The bridge fallback now arms it for failures the classifier recognizes. - The bridge injects the durable anchor into its own prepared payload, but the fallback replays the incoming payload and the raw path never injects a response anchor. A hard-continuity follow-up therefore lost prior context; such turns are no longer replayed over raw HTTP. Regression coverage exercises the real client conversion for each connect site and the real bridge reconstruction for the anchor provenance.
dpearson2699
force-pushed
the
feature/1886-bridge-cooldown-http-fallback
branch
from
August 25, 2026 22:32
80021f4 to
215ab87
Compare
…paths Two places still decided a websocket outage by something other than the connect-site provenance the classifier now carries: - The bridge fallback wrapper still compared the sanitized code against `upstream_unavailable`. A direct 5xx bridge connect preserves the upstream envelope, so it surfaces as `upstream_error` and was rethrown before ever reaching the classifier — the exact outage this change targets stayed on the dead websocket bridge and left the marker clear. The wrapper now gates on the classifier, which subsumes the code, phase and status checks. - The budget-timeout marker keyed on any connector having begun. A stalled routed open therefore armed a process-wide denial from one account's unhealthy proxy endpoint, and because the budget cancels the open rather than failing it, no error exists for the routed exclusion to act on. The progress flag is now confined to the direct connector.
dpearson2699
force-pushed
the
feature/1886-bridge-cooldown-http-fallback
branch
from
August 26, 2026 04:35
215ab87 to
ce7d7f7
Compare
Arming became direct-scoped, but clearing did not: any successful open, routed included, cleared the denial state. By this change's own provenance argument a routed success proves only that one account's proxy endpoint is healthy, so a routed success during a direct-upstream outage readmitted handshakes early and cost an extra failed direct attempt before the marker re-armed. Clearing is now direct-scoped too. Because a routed open can neither arm nor clear, an all-routed deployment never uses the marker at all, and a mixed one still expires it on the bounded window.
…pressions The bridge retry circuit's pre-dispatch submission gate suppresses new turns on a cooling hard-affinity session with a bounded 503. For a request whose state is provably undispatched — no client or proxy-injected continuation identity, no file account pin, and none of the unambiguous-boundary markers (response_id, response events, downstream visibility, a pending response.create send, a prior replay) — that 503 now carries the shared pre-submit provenance and the streaming wrapper degrades the turn to raw HTTP instead. Ambiguous continuations keep the bounded 503 with its retry hint, because a replay could execute the turn twice. The pre-submit provenance attribute moves to http_bridge/helpers.py so request_submit.py and streaming.py share one definition.
dpearson2699
force-pushed
the
feature/1886-bridge-cooldown-http-fallback
branch
from
August 26, 2026 10:01
ce7d7f7 to
ade890a
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
2 participants
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.
Important
Stacked on #1886. This branch is based on
feature/1885-websocket-connect-http-fallbackand the diff shown here includes #1886's commits until it merges. Only the top commit (3fb7d97) is this PR's change — please review #1886 first. Once #1886 lands onmainI will rebase this branch so the diff collapses to the single commit. (Native stacked PRs can't be used here: cross-fork stacks aren't supported in the public preview.)Summary
When the HTTP responses bridge's retry circuit is cooling down, a suppressed submission whose state is provably undispatched now degrades to raw HTTP streaming instead of returning the bounded 503 ("HTTP responses session bridge is cooling down after repeated upstream timeouts; retry shortly."). Ambiguous continuations keep the bounded 503, because replaying them could execute the turn twice.
Type of change
feat:— new user-facing feature or capabilityLinked issue: extends #1885 / stacked on #1886
OpenSpec
Change directory:
openspec/changes/fall-back-to-http-on-websocket-connect-failure/(extends #1886's change folder with one requirement clause and two scenarios; the folder is unarchived while the stack is in flight)Changes
request_submit.py):_http_bridge_cooldown_suppression_is_replay_safemirrors the codebase's own unambiguous-boundary markers (from_http_bridge_can_replace_retired_gate_session): no client or proxy-injected continuation identity, no file account pin,response_id is None, zero response events, zero replays, no downstream sequence/visibility, and no pendingresponse.createsend. Only when every marker holds is the cooldown 503 tagged with the shared pre-submit provenance attribute.streaming.py): the raw-HTTP fallback from feat(proxy): fall back to HTTP transport when the upstream websocket is unavailable #1886 now also accepts taggedupstream_request_timeoutfailures (all other gates unchanged: nothing streamed, no unsettled API-key reservation, 5xx, provenance tag required).helpers.py):_HTTP_BRIDGE_PRE_SUBMIT_FAILURE_ATTRmoves tohelpers.pysorequest_submit.pyandstreaming.pyshare one definition (samehttp_bridgedomain, architecture check passes).Real-world motivation
Observed live on 2026-08-22 ~22:31 UTC (single-account view, #1886 already deployed): a bridge turn died mid-stream (
stream_incomplete), the circuit opened, and the Codex client's rapid resends produced asubmit_retry_circuit_suppressed detail=hard_key_cooldown→ 503 loop every ~2s until the cooldown expired. Those specific turns were ambiguous continuations — this PR deliberately keeps them on the bounded 503 (their replay safety is not provable, and the loop is bounded and self-clearing). What this PR fixes is the collateral damage: fresh turns (new conversations, other sessions landing on the same cooling hard key) were also 503-blocked despite being provably undispatched; they now complete over raw HTTP.Simplicity
Test plan
New coverage: the predicate is tested against every unambiguous-boundary marker individually (10 negative cases + the positive case), and the wrapper is tested for both the tagged-fallback and untagged-propagation paths.
Checklist