fix(audio): prefer readable inputs over body metadata - #2548
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. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 existing customizations unchanged
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 33451672913 --repo openai/openai-node \
--name castiron-custom-code-33451672913-1 --dir /tmp/castiron-custom-code-33451672913-1
git apply --stat /tmp/castiron-custom-code-33451672913-1/custom-code.patch
cat /tmp/castiron-custom-code-33451672913-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin eea2292a4a523da9405161dde0a79ac5dc2ecb2a d06e3bdcc5a441213bb83f4e111aa236272d7887
python3 scripts/castiron/custom_code_report.py report \
--base eea2292a4a523da9405161dde0a79ac5dc2ecb2a \
--head d06e3bdcc5a441213bb83f4e111aa236272d7887 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-d06e3bdcc5a4
cat /tmp/castiron-custom-code-d06e3bdcc5a4/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![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: 6445c0787c
ℹ️ 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".
|
@codex review Addressed the Response compatibility finding in d06e3bd. The discriminator now checks the readable event/flow protocol ( The new decorated-Response regression fails on the prior head and passes now; an already-ended readable with body metadata remains accepted. Validation: 46 focused audio tests, 7,014 handwritten plus 556 generated tests, lint, strict type checking, CJS/ESM builds, unchanged public declarations, and 72 built-SDK CJS/ESM input cases on Node 22.0.0, 24.20.0, and 26.7.0. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security 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. |
2 participants
Summary
Preserve the documented Node-readable input path in
playAudio()when the readable carries an unrelatedbodyfield.The response discriminator previously treated any non-
undefinedbodyas a fetch response. A valid Node readable decorated withbody: nullor Buffer metadata was rejected beforeffplaystarted. If that metadata was itself readable, the helper played the metadata bytes instead of the input stream.The discriminator now recognizes the outer readable event/flow protocol (
pipe,on,pause, andresume) before treating abodyfield as a response. A Response with an unrelatedpipemethod still follows its response body, and ended Node readables retain their stream path. Native Response inputs, the existing response-like Node-body path, File inputs, process arguments, and error handling are otherwise unchanged. No generated files, dependencies, exported types, or custom-code budget settings change.Regression coverage
The existing public-subpath audio suite now covers Node readables with null, Buffer, and readable-stream body metadata. It verifies that the outer stream's audio reaches
ffplay, metadata identity is preserved, and readable metadata remains unconsumed.All three metadata regressions fail before the fix: two reject and one captures the wrong bytes. The review follow-up also reproduces and fixes a Response carrying a callable
pipeproperty, with an already-ended readable as a control. All 46 focused playback/recording tests pass, including the existing Response, File, process-error, source-error, and cancellation controls.Verification
./scripts/test tests/helpers/audio.test.ts tests/helpers/audio-recording.test.ts— 46 tests passed.pnpm lint— passed.pnpm exec tsc --noEmit— passed.pnpm build— CommonJS and ESM passed.pipemethods, bodyless Responses with that metadata, and ended Node readables. A localffplayfixture captured stdin and process invocation counts; no live API, audio hardware, or real credentials were used.helpers/audio.d.tsandhelpers/audio.d.mtsare byte-for-byte identical to the baseline packed declarations.CI=true ./scripts/test— 7,014 handwritten tests across 163 files and 556 generated tests across 82 suites passed, plus all 12 snapshots; the existing two generated test skips and one skipped suite are unchanged.Two adversarial review passes checked protocol precedence, inherited Node stream methods, unchanged Response/File handling and process lifecycle, metadata preservation, unchanged public declarations, and supported runtime behavior. The fix adds no new parser, retained state, or fallback abstraction.