test(adapters): lock in OpenAI Agents hook enforcement - #57
Merged
Conversation
The OpenAI Agents adapter raises budget halts (BudgetExceeded) and approval denials (ApprovalDenied) from its async RunHooks — the same raise-in-a-hook pattern that silently no-opped in the LangChain adapter until it set raise_error. Checked whether the OpenAI Agents SDK has the same flaw: it does not. Runner.run awaits hooks via asyncio.gather without return_exceptions, so a halt raised in on_agent_start propagates out and stops the run — verified directly. No adapter change needed. But the adapter was untested. Added a unit test (one agent turn == one governed step; the loop budget halts at the cap), a tool-gating test (a denied tool raises ApprovalDenied), and a skip-unless-openai-agents integration test that runs a real agent with a fake model and asserts the halt propagates out of Runner.run — guarding the SDK's propagation behavior against a future regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The LangChain fix (#55) found that an SDK can silently swallow a budget halt raised inside a callback. The OpenAI Agents adapter raises halts from its async
RunHooks(on_agent_start→run.step(),on_tool_start→ approval gate) — the same raise-in-a-hook pattern — so it deserved the same scrutiny.Finding: no bug here
The OpenAI Agents SDK awaits hooks via
await asyncio.gather(hooks.on_agent_start(...), ...)withoutreturn_exceptions=True, so an exception raised in a hook propagates out ofRunner.run. I verified this directly (a hook raising a sentinel propagates out of a realRunner.run). No adapter change needed — unlike LangChain, nothing is swallowed.What this PR adds
The adapter was correct but untested (the same gap LangChain had before #55). Three tests in
test_sdk.py:test_openai_agents_hooks_enforce_loop_budget— one agent turn == one governed step; the loop budget halts at the cap (BudgetExceeded). CI-safe (the adapter falls back to anobjectbase whenagentsisn’t installed).test_openai_agents_hooks_gate_denied_tool— a denied tool raisesApprovalDenied.test_openai_agents_integration_halts_run—@skipUnless(openai-agents installed): runs a realRunner.runwith a fake model (no API key) and asserts the budget halt propagates out and stops the agent. This is the regression guard for the SDK’s propagation behavior.Verification
openai-agents 0.17.4(py3.12): all 15 SDK tests pass, including the integration test.