fix: enforce realtime text guardrails and synchronize streaming cancellation#3933
fix: enforce realtime text guardrails and synchronize streaming cancellation#3933seratch wants to merge 1 commit into
Conversation
![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: 4801764e70
ℹ️ 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".
gnanirahulnutakki
left a comment
gnanirahulnutakki
left a comment
There was a problem hiding this comment.
Thanks—this is a strong direction, and the text-only Realtime guardrail path is well scoped. The focused Realtime and soft-cancel suites pass locally (157 passed), and the new event names match the Realtime contract.
One blocking cancellation gap remains: event-consumption synchronization is only applied to NextStepRunAgain. A handoff can still start the next agent's model request while the consumer is processing the first turn's handoff_requested event. On the current head I deterministically observed two model requests and Agent2's final output before cancel(mode="after_turn") took effect.
Please gate the handoff (and any equivalent resumed continuation path) before another model turn begins, and add a controlled-interleaving regression test. Once that is covered, the cancellation approach looks sound.
4801764 to
81aa66b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81aa66bc8b
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: ℹ️ 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. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: ℹ️ 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. |
gnanirahulnutakki
left a comment
gnanirahulnutakki
left a comment
There was a problem hiding this comment.
Re-reviewed at 81aa66bc. The original cancellation gap is addressed: the event-consumption barrier now covers ordinary run-again, handoff, resumed run-again, and resumed handoff continuations before another model request can start. The new controlled-interleaving tests exercise both handoff and resume paths.
Local verification: 169 focused Realtime/cancellation/resume tests passed, Ruff and formatting checks passed, and an independent follow-up-commit review found no issues. GitHub’s current test, lint, typecheck, docs, and Windows matrix is green. I found no remaining issues in the follow-up.
81aa66b to
558ddc1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87ccdd53f6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ad58eebcd
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e849790f3b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5275431a7
ℹ️ 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".
a527543 to
085f939
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 085f939149
ℹ️ 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".
gnanirahulnutakki
left a comment
gnanirahulnutakki
left a comment
There was a problem hiding this comment.
Re-reviewed the current head 085f9391 after the force-push. The follow-up closes the remaining public-result state gap: _publish_streamed_result_agent() now updates current_agent and the output schema before the handoff event-consumption barrier can honor after_turn cancellation, and the ordinary plus resumed handoff tests assert current_agent, last_agent, to_state(), and the schema.
Local verification on this exact head: uv run pytest -q tests/test_soft_cancel.py tests/test_run_impl_resume_paths.py → 34 passed. GitHub's current test matrix, typecheck, lint, docs, and Windows checks are all green. I found no remaining issue in the cancellation follow-up.
|
@gnanirahulnutakki Thanks for sharing your review results, but could you refrain from posting general comments? Pointing specific points out would be appreciated. |
|
Understood—thanks for the guidance. I’ll keep future feedback to specific, actionable inline findings and avoid posting general review-summary comments. |
085f939 to
e6b9328
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6b93288b9
ℹ️ 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".
| ): | ||
| await self._clear_pending_response_create(parsed.error.event_id) | ||
| await self._emit_event(RealtimeModelErrorEvent(error=parsed.error)) | ||
| pending = await self._clear_pending_response_create(parsed.error.event_id) |
There was a problem hiding this comment.
When server-side VAD wins the race after a correlated guardrail-recovery response.create has been sent, the API can reject that create while the VAD response is active. This cleanup is gated by not self._ongoing_response, so the matching error is emitted without clearing _pending_response_create; mark_response_done() then deliberately preserves the pending entry, leaving _response_control as create_requested and blocking every subsequent local response indefinitely. Handle matching create errors even while another response is active, retrying or failing the recovery deterministically. The fresh evidence in this revision is that mismatched VAD response.created events now intentionally retain the pending recovery, making this failure path reachable.
AGENTS.md reference: AGENTS.md:L109-L109
Useful? React with 👍 / 👎.
| manual = event.response_create_id is not None | ||
| request_version = await self._reserve_response_create_request(manual=manual) | ||
| self._start_response_create( | ||
| request_version, | ||
| manual=manual, | ||
| response_create_id=event.response_create_id, |
There was a problem hiding this comment.
When delayed guardrails from two different responses trip while another response is active, each recovery call sends its conversation.item.create immediately, while only the corresponding response.create is deferred and kept separate by manual=True. Both recovery items therefore enter the conversation before the first deferred create runs, so its response consumes both items and the second create later runs without its intended new input, producing combined or duplicate recovery output despite having distinct correlation IDs. Queue each recovery input together with its create, rather than sequencing only the creates.
AGENTS.md reference: AGENTS.md:L109-L109
Useful? React with 👍 / 👎.
| elif event.type == "output_text_delta": | ||
| assert isinstance(event, RealtimeModelOutputTextDeltaEvent) | ||
| self._record_output_guardrail_delta( | ||
| event.item_id, | ||
| event.delta, | ||
| event.response_id, | ||
| is_audio_output=False, |
There was a problem hiding this comment.
This branch expands debounce_text_length to text-only output, but the published RealtimeGuardrailsSettings API documentation in src/agents/realtime/config.py:231-233 still says the threshold applies only to transcript deltas. Users configuring text-only sessions can therefore miss that their output is accumulated and checked under this setting; update the public setting description alongside the new behavior.
AGENTS.md reference: AGENTS.md:L69-L69
Useful? React with 👍 / 👎.
2 participants
This pull request fixes two streaming runtime issues affecting output safety and cancellation behavior.
cancel(mode="after_turn")from starting another model request while stream consumers are still processing events.