Repository navigation
fix: fail fast when STT websocket closes during setup - #4585
sylvesterkaczmarek wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e459480025
ℹ️ 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".
|
Thanks for investigating this path and for fixing the completion race identified in the automated review. The current head appears to propagate the setup-close error correctly, but the motivating case is still demonstrated only with an empty mocked WebSocket. We do not yet have evidence that a supported OpenAI STT endpoint or compatible proxy normally closes with a clean close code before either setup event, or that the existing 10-second timeout causes a meaningful operational problem. I am going to close this PR for now. We can reconsider it with a real endpoint or proxy reproduction that includes the close code, the setup phase where it occurs, and the observed delay. |
@seratch I reproduced this against a real loopback WebSocket-compatible proxy. The server completed an actual WebSocket handshake and sent a normal close frame with code 1000. Measured through the public transcribe_turns() API:
This is controlled compatible-proxy evidence. It does not claim that the hosted OpenAI endpoint normally emits this close. Is this sufficient evidence for reconsidering the PR? |
Summary
A streamed STT WebSocket that closes cleanly before
session.createdorsession.updatedcurrently leaves_setup_connection()waiting on_state_queueuntil the 10-second setup timeout, because the listener completion sentinel is only sent to_event_queue.This change sends the listener-completion sentinel to the setup state queue as well and makes
_wait_for_event()treat it as an immediate listener failure. Clean connection closure during setup now fails promptly instead of waiting for a timeout.Regression coverage
Added a focused test with a WebSocket that closes without emitting setup events. The test requires the session to raise
STTWebsocketConnectionErrorwithin one second, so the previous timeout-based behavior fails the regression.Full repository tests were not run locally because this environment does not contain the Agents SDK development checkout/dependencies; CI should run the voice test suite.