Skip to content

fix(openai realtime): do not commit a turn the server closed itself - #6965

Closed
longcw wants to merge 1 commit into
mainfrom
longc/openai-rt-double-commit
Closed

longcw wants to merge 1 commit into
mainfrom
longc/openai-rt-double-commit

Conversation

@longcw

@longcw longcw commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Problem: With server-side turn detection the server closes each audio segment itself, and the framework still calls commit_audio() on every client turn. The server answers that second commit with input_audio_buffer_commit_empty, and #6642 only suppressed the error, so every turn still pays for a commit the server rejects.

Fix: The session now handles input_audio_buffer.committed and clears the count of pushed audio, so commit_audio() has nothing left to send. The meaning of commit_audio() does not change, and the error suppression stays for a client commit and a server commit that cross on the wire.

Follows up #6642.

Context for reviewing and coding agents

How to see it

Three unit tests in tests/test_realtime/test_openai_realtime_model.py push a input_audio_buffer.committed event into the session and then call commit_audio(). The first test fails on main, where the handler does not exist. The other two hold the two cases that must keep committing: audio pushed after a server commit, and the echo of the client's own commit when the server does no segmentation.

uv run pytest tests/test_realtime/test_openai_realtime_model.py --unit

Blast radius

commit_audio() has two callers, agent_activity.py:1777 and agent_activity.py:2554, and both run only when _rt_turn_detection_enabled is False, which is the client-side turn taking path. A session that leaves turn taking on the server never reaches them and sees no change. The fix stays inside the openai realtime plugin; other realtime plugins keep their own commit path.

Why the handler is gated on turn_detection

When turn_detection is None the server never commits on its own, so every input_audio_buffer.committed is the echo of a client commit. Audio that arrived while that commit was in flight is still the client's to commit, and the count must survive the echo. With server VAD on, the server segments that audio itself, so clearing the count loses nothing.

Alternatives rejected

The caller in agent_activity cannot make this decision. It knows that server-side turn taking is off, but create_response=False leaves the server detecting and committing turns, and only the plugin sees that setting.

The error suppression alone is not enough. It hides every input_audio_buffer_commit_empty while turn_detection is set, including one that comes from a genuine client mistake, and the needless commit still goes out on each turn.

With server-side turn detection the server commits each segment it
detects, but the framework still called commit_audio() on every client
turn. Each turn asked the server to close a buffer it had already
emptied and logged input_audio_buffer_commit_empty, which #6642
suppressed instead of removing the commit. create_response=False makes
this fire on every turn, because the client owns the reply while the
server keeps segmenting.

The session now tracks the server's own commits and clears the count of
pushed audio, so commit_audio() has nothing left to send and keeps its
meaning. Only a segment the server closed itself counts: a client
commit is acknowledged with the same event, and the audio that arrived
while it was in flight is still the client's to commit. The
suppression stays for the race where the two commits cross on the wire.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@longcw

longcw commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Closing. #6642 is sufficient.

_handle_error returns on input_audio_buffer_commit_empty before the log, before _emit_error, and before the fatal check, so the second commit changes no state. Either the server buffer holds less than 100ms of unsegmented audio and the commit is rejected, or it holds more and the commit is correct. Neither case harms the session.

The change also does not let us remove the suppression, because a client commit and a server commit can still cross on the wire.

@longcw longcw closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants