Repository navigation
feat(websocket): recover queued binary frames after reconnect - #4191
Conversation
Castiron custom codeEvaluated main: Trusted main policy: ✅ No new custom-code files detected. 0 mixed files remain; 0 existing customizations changed. Compared A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 40462487697 --repo openai/openai-python \
--name castiron-custom-code-40462487697-1 --dir /tmp/castiron-custom-code-40462487697-1
git apply --stat /tmp/castiron-custom-code-40462487697-1/custom-code.patch
cat /tmp/castiron-custom-code-40462487697-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 3596d18f05b63081fd64c6da82b515293afb98c6 194bc68cbb390798bb1311f0368375a2fe65ac4a
python3 scripts/castiron/custom_code_report.py report \
--base 3596d18f05b63081fd64c6da82b515293afb98c6 \
--head 194bc68cbb390798bb1311f0368375a2fe65ac4a --target-branch main --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-194bc68cbb39
cat /tmp/castiron-custom-code-194bc68cbb39/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
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. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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.
Reviewed binary frame type/order preservation, queue byte accounting, sync/async reconnect behavior, and backward compatibility of unsent_messages alongside unsent_frames. No actionable findings.
Static review of commit 194bc68. Hosted checks show no failures (applicable checks passed; other jobs skipped). No local tests or candidate code were run.
![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: f073d8aed2
ℹ️ 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.
Correcting my earlier approval after validating the existing payload-retention finding: #4191 (comment) . I missed the repository's explicit requirement to keep sensitive request/customer data out of exceptions.
unsent_frames stores complete opaque binary request payloads directly on the exception. Normal exception text is safe, but reporters that collect exception attributes can retain those bytes. Keep recovery payloads outside the exception and expose safe metadata or a recovery handle; preserve the necessary recovery behavior. This is a P2 redaction/policy issue; no actual data disclosure was established. Referencing the existing thread instead of duplicating an inline comment.
The merge from main leaves the six feature files unchanged. Static re-review with an independent reviewer; no candidate code or local tests run. Applicable completed checks pass; Python 3.10/3.14 tests are still running.
markstuart-oai
changed the title
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the diagnostic-redaction fix with an independent static second pass. The slotted recovery holder now redacts ordinary exception-attribute snapshots while preserving explicit recovery, list identity, mutation, assignment, and legacy subclass initialization. This addresses my previous finding; no new actionable findings. Deliberate inspection of recovery properties is still explicit access to the caller's data.
Hosted checks show no failures; Python 3.14 remains running. No candidate code or tests run locally.
Automated Release PR --- ## [3.41.0](openai/openai-python@v3.40.0...v3.41.0) (2026-10-28) ### Features * **api:** accept typed references in Realtime response inputs ([openai#4189](openai#4189)) ([8ecb487](openai@8ecb487)) * **api:** add response tool usage models ([openai#4201](openai#4201)) ([55fc500](openai@55fc500)) * **api:** filter agent sessions by metadata ([openai#4200](openai#4200)) ([77fd116](openai@77fd116)) * **websocket:** recover queued binary frames after reconnect ([openai#4191](openai#4191)) ([f079868](openai@f079868)) ### Bug Fixes * **examples:** detect incomplete WebSocket responses ([openai#4187](openai#4187)) ([8559299](openai@8559299)) * **examples:** handle interruptions and disconnects in push-to-talk ([openai#4192](openai#4192)) ([35fc9f3](openai@35fc9f3)) * keep consumer fixtures clean under strict Pyright ([openai#4204](openai#4204)) ([5012565](openai@5012565)) ### Chores * **api:** document upcoming model shutdowns ([openai#4199](openai#4199)) ([5b43cce](openai@5b43cce)) * **api:** update image-model documentation ([openai#4198](openai#4198)) ([fa67521](openai@fa67521)) * enforce generated-file ownership ([openai#4193](openai#4193)) ([1b37b9a](openai@1b37b9a)) * migrate development type checking to ty ([openai#4202](openai#4202)) ([45b9531](openai@45b9531)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: openai-sdks[bot] <284451331+openai-sdks[bot]@users.noreply.github.com>
Summary
During reconnect,
send_raw(bytes)used to decode queued bytes as UTF-8. A valid UTF-8 payload changed from a binary frame to a text frame. Other bytes raisedUnicodeDecodeErrorbefore the SDK could send them.Preserve each frame and its place in the queue. If recovery stops,
unsent_framesreturns the original text and bytes in order.unsent_messagesretains its text-only compatibility list, including valid UTF-8 binary as text. Ordinary exception-attribute snapshots render a fixed redacted holder; explicit recovery access still returns the data.Verification
The previous transport failed 28 wire cases against local WebSocket peers; the initial fix passed 357 queue, recovery and session tests and 328 URL and security tests. Sync and async tests cover all seven bindings.
On the current revision, 109 focused queue/reconnect/wire tests pass in both SDK copies, including attribute snapshots, recovery-list identity and writes, and assignment before parent initialization. Pinned Pyright and mypy pass on the changed source and tests; scoped Ruff and git diff checks pass. Hosted CI passes all eight required checks, including OkTest, and all 26 reported checks are clear.
Release
Release as a minor version. Pyright users whose subclasses redeclare
unsent_messages: list[str]should remove that redundant declaration and use the inherited writable property. Runtime reads, mutation, assignment, and assignment beforesuper().__init__remain supported. This follows README's static-typing minor-release policy. A normal squash merge uses the currentfeat(websocket):PR title and selects the required minor release. If using rebase, preserve that feature classification in the merged commit history.