Repository navigation
fix(audio): preserve recording outcomes during cancellation and cleanup - #2482
HAYDEN-OAI merged 2 commits into
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: 71041d1950
ℹ️ 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".
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 33011076812 --repo openai/openai-node \
--name castiron-custom-code-33011076812-1 --dir /tmp/castiron-custom-code-33011076812-1
git apply --stat /tmp/castiron-custom-code-33011076812-1/custom-code.patch
cat /tmp/castiron-custom-code-33011076812-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 520d6044717790d922e9659427518850a80a0345 ac64d0911700060062b3ecb5f984b78b9df6b7f3
python3 scripts/castiron/custom_code_report.py report \
--base 520d6044717790d922e9659427518850a80a0345 \
--head ac64d0911700060062b3ecb5f984b78b9df6b7f3 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-ac64d0911700
cat /tmp/castiron-custom-code-ac64d0911700/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Please address the two already-open inline P2 findings before merging: (1) reduce the new 702-line/37-case regression suite to focused coverage that does not substantially duplicate audio-recording.test.ts or conflict with the repository's test-maintainability guidance; and (2) update recordAudio's public cancellation/timeout contract to document rejection when process termination fails. I independently validated both existing threads and found no additional actionable runtime/security issue, so I am intentionally not duplicating their inline comments.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the updated commit and confirmed both requested changes are fixed: the redundant 702-line suite is replaced with focused regressions in the existing audio tests, and public recording documentation now covers cancellation/termination failures. Cancellation outcomes, cleanup, listener races, and privacy remain correct.
4f74d16 to
2e17fd3
Compare
2e17fd3 to
ac64d09
Compare
Summary
openai/helpers/audiopublic entrypoint, including independent cleanup failures, late private output, cross-signal races, and native subprocess failure modes.Validation