Repository navigation
feat: Add native Session voice workflows - #5596
Conversation
Codex Review Summary: ✅ CompletedThis 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: f38b326fcc
ℹ️ 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".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed f38b326 with an independent static reviewer. Request changes for the existing finding #5596 (comment), independently confirmed: closing stream() immediately after session_ended bypasses _check_errors; the new synthesis_failed path also skips collecting the producer exception, and cleanup consumes task exceptions. A synthesis failure can therefore disappear at the terminal-event boundary. Preserve the synthesis exception through finalization while retaining caller cancellation precedence and joining owned tasks. The author has acknowledged a fix in progress; this review records the current commit's decision without duplicating the inline finding.
The remaining review found no additional actionable issues in exact-turn ownership, shielded admission, overlap rejection, failed-cleanup reservation, shared Session history, application-controlled approvals or example lifetime management. Hosted CI is pending. No candidate code or tests executed locally.
f38b326 to
1106e85
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed 1106e85 with an independent static reviewer. The synthesis-error finding is fixed: finalization captures speech failures, joins owned tasks, preserves already-settled producer errors and caller cancellation precedence, and suppresses only the cancellation issued to stop a pending producer after synthesis failure. Native speech now inherits the Session output contract (no str override), waits for accepted final_output and rejects structured output before synthesis. Rejection, repair, completion acceptance, terminal close and ownership regressions cover these paths.
No new actionable findings. Approval supersedes my previous request changes. Hosted CI on this new head remains pending; this approval does not claim the earlier CI failure has been resolved. No candidate code or tests executed locally.
1106e85 to
7804ff6
Compare
Summary
This pull request adds speech workflows backed by a caller-owned native Session. The optional
agents.voiceentrypoint andvoiceinstall extra reuse the existing speech pipeline and providers.NativeSessionVoiceWorkflowsends each transcript to the same Session and waits for its accepted final string before speech synthesis. It preserves configured output validation and repair. Structured results require a custom voice workflow that chooses what to speak.The Session owns history and tools; the application answers approval requests. Early exit or cancellation stops and joins only its own Turn, including cancellation during submission. The speech pipeline closes suspended workflows when playback stops or synthesis fails. Cleanup no longer silently drops terminal synthesis errors. Examples cover WAV input and output, plus Realtime delegation through an ordinary function tool with explicit lifetime cleanup.
This PR targets
feat/codex-harness.Test plan
.agents/skills/code-change-verification/scripts/run.sh: passed on the current commit. Format, lint, Mypy, and Pyright passed. The parallel test phase had 5,627 passed and 250 skipped; the serial phase had 53 passed.See the PR checks for current hosted CI results.
Issue number
None linked.
Checks
AGENTS.md, including.agents/skills/code-change-verification/scripts/run.shwhen requiredAGENTS.md