fix: re-raise output guardrail exceptions in streaming path#2752
fix: re-raise output guardrail exceptions in streaming path#2752gn00295120 wants to merge 1 commit intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes error-handling and concurrency issues in the Agents SDK, primarily ensuring streaming runs correctly surface output-guardrail tripwires and improving session/tool configuration robustness.
Changes:
- Re-raise
OutputGuardrailTripwireTriggeredin the streaming final-output path instead of swallowing it, and log other unexpected guardrail failures. - Fix ineffective locking in file-based
SQLiteSessionoperations by reusing a shared lock instead of allocating a new lock per call. - Add
Agent.toolselement validation (and new tests) to fail fast on invalid tool entries.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/agents/run.py |
Adjusts streaming final-output guardrail exception handling to re-raise tripwire exceptions and log unexpected errors. |
src/agents/memory/sqlite_session.py |
Introduces a persistent file-lock to provide real mutual exclusion for file-based SQLite sessions. |
src/agents/agent.py |
Adds per-element validation for Agent.tools and raises UserError with guidance on invalid entries. |
tests/test_streaming_output_guardrail_fix.py |
Adds regression tests ensuring streaming output guardrail tripwires propagate and non-tripping guardrails don’t false-positive. |
tests/test_agent_config.py |
Adds tests to ensure invalid tool-list contents fail during Agent initialization (Issue #1443). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix OutputGuardrailTripwireTriggered silently swallowed by bare except Exception in run_loop.py streaming path - Re-raise asyncio.CancelledError for Python 3.9/3.10 compatibility - Fix SQLiteSession race condition: replace per-call threading.Lock() with shared self._file_lock for file-based databases
63d7513 to
8650178
Compare
|
Thanks for pointing this issue out, and this should be fixed. Regarding the solution, I think #2758 is more robust, so let me go with the PR. Thanks again for reporting this issue. |
You're welcome. Long time no see👍 |
Summary
Two critical bug fixes:
1. Streaming Output Guardrail Bypass (HIGH severity)
File:
src/agents/run.pyBefore: In the streaming path (
AgentRunner._run_streamed_impl), a bareexcept Exceptionclause caught and silently discardedOutputGuardrailTripwireTriggered, allowing the run to continue withfinal_outputset despite the guardrail having tripped:After:
OutputGuardrailTripwireTriggeredis re-raised immediately, stopping the run loop. Only truly unexpected errors are caught and logged:2. SQLiteSession Race Condition
File:
src/agents/memory/sqlite_session.pyBefore: File-based SQLite sessions created a new
threading.Lock()on every method call, providing zero mutual exclusion:After: A shared
self._file_lockis created once in__init__and reused across all 4 call sites:Test plan
tests/test_streaming_output_guardrail_fix.pywith 3 cases:OutputGuardrailTripwireTriggeredOutputGuardrailTripwireTriggeredtest_agent_runner_streamed,test_agent_runner,test_guardrails, andtest_sessionpass