Repository navigation
Fix Realtime example keepalive while awaiting input - #4207
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: ead03d3104
ℹ️ 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".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Request changes: confirmed the existing Ctrl-C shutdown regression in #4207 (comment). Moving input() to the default executor lets pings run, but cancelling the await does not interrupt the blocked input worker; asyncio.run waits for executor shutdown. Preserve timely SIGINT exit without another input line and add that subprocess regression. Referencing the existing finding rather than duplicating its inline comment.
Hosted lint also fails at tests/lib/test_realtime_example_keepalive.py:21 because server.sockets is typed Iterable[socket], which cannot be indexed. Use iterator access or materialize the iterable. Other hosted checks are still running. Static review with an independent reviewer; no candidate code or tests executed.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the complete updated diff and author replies, with an independent static pass. The synchronous Realtime client keeps input() on the main thread while the WebSocket reader handles pings, addressing the Ctrl-C shutdown finding. Context-manager cleanup remains in place for quit, EOF and interruption. The regression covers conversation output, pongs at both prompts, and prompt exit without another input line; iterator access also fixes the server.sockets typing error. No remaining actionable findings.
Hosted checks pass or skip, including lint and Python 3.10/3.14 tests. No candidate code or local tests executed during review.
The interactive Realtime example previously blocked WebSocket processing while
input()waited for a message, so it could miss server pings. Use the SDK's synchronous Realtime connection, which handles socket reads while the main thread waits for console input. This also preserves prompt Ctrl+C exit without a blocked input worker.The subprocess regression tests run the actual example against a local WebSocket peer. They check idle pongs, a streamed response followed by another prompt, normal quit, EOF, and SIGINT without another input line. The test reads the server socket through its iterable interface.
Validation: the exact sample passed on OpenAI SDKs 3.40.0 and 3.41.0 with websockets 13.0 and 15.0.1. Additional Linux terminal checks covered Ctrl+C sent through a controlling PTY. The three committed regression cases and focused Ruff formatting, Ruff lint, and ty checks pass. Windows console-signal behavior was not exercised locally.