Skip to content

Gate Responses WebSocket transport on provider capability - #620

Open
voarsh2 wants to merge 2 commits into
just-every:mainfrom
voarsh2:pr/provider-transport-20260916
Open

voarsh2 wants to merge 2 commits into
just-every:mainfrom
voarsh2:pr/provider-transport-20260916

Conversation

@voarsh2

@voarsh2 voarsh2 commented Sep 16, 2026

Copy link
Copy Markdown

Summary

  • Add an explicit supports_websockets capability to model providers.
  • Default custom and OpenAI‑compatible providers to stream HTTP responses.
  • Mark the built‑in OpenAI provider as WebSocket‑capable.
  • Fall back to HTTP when WebSocket transport is requested but unsupported.
  • Use codex‑rs‑compatible session‑correlation header names.

Motivation

Model metadata can prefer responses over WebSocket, but an OpenAI‑compatible provider may only support the HTTP responses endpoint. Previously, the code-rs treated model preference as sufficient and could attempt an unsupported transport.

This change keeps existing custom providers on the safer HTTP path while preserving WebSocket behavior for the built‑in OpenAI provider.

No gateway‑specific behavior or configuration is hardcoded.

Keeps WebSocket support explicit so custom and proxy
providers default to the safer HTTP streaming path.

Updates built-in providers, configuration examples, test
fixtures, and provider deserialization coverage.
Falls back to HTTP streaming when a provider does not
advertise WebSocket capability.

Uses consistent correlation headers so compatible gateways
can preserve session and prompt-cache routing.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: afb73413f1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +679 to +683
if !self.provider.supports_websockets {
warn!(
"responses_websocket transport is disabled for this provider; using responses HTTP stream"
);
return self.stream_responses(prompt, log_tag).await;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve explicit Responses WebSocket configurations

Existing custom providers with wire_api = "responses_websocket" but no new supports_websockets setting deserialize that capability as false, so this branch silently switches them to HTTP. This breaks WebSocket-only providers that do not expose POST /responses; it also invalidates the configuration emitted by scripts/mock_responses_websocket_server.py:161-165. Treat the explicit WebSocket wire API as capability, or provide a compatibility-preserving migration.

Useful? React with 👍 / 👎.

Comment on lines +347 to 350
if self.websockets_disabled.load(Ordering::Relaxed)
|| !self.provider.supports_websockets
{
return None;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add integration coverage for the transport capability gate

The new tests only verify provider deserialization and defaults; none exercises ModelClient::stream. Add request-level integration cases proving that WebSocket-preferring WireApi::Responses and explicit WireApi::ResponsesWebsocket providers go directly to HTTP when unsupported, while supports_websockets = true still selects WebSocket. Without this, the central dispatch behavior—and the compatibility regression for existing explicit WebSocket configurations—can go unnoticed.

AGENTS.md reference: AGENTS.md:L23-L23

Useful? React with 👍 / 👎.

Comment on lines +411 to +413
.header("x-client-request-id", session_id)
.header("session-id", session_id)
.header("thread-id", session_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Test the emitted session-correlation headers

No request-level test verifies this header migration across HTTP streaming, WebSocket handshakes, or compaction. Add integration assertions that x-client-request-id, session-id, and thread-id contain the expected session override and that the legacy underscore headers are absent; otherwise a typo or a missed call site can silently break gateway routing and prompt-cache correlation.

Useful? React with 👍 / 👎.

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