fix(openai-agents): Patch tool functions following library refactor #5445
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 6 passed | Total: 6 | Pass Rate: 100% | Execution Time: 1.20s All tests are passing successfully. ❌ Patch coverage is 5.00%. Project has 15306 uncovered lines. Files with missing lines (181)
Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| def _patch_run_get_all_tools() -> None: | ||
| agents.run.get_all_tools = _create_run_loop_get_all_tools_wrapper( | ||
| run_loop.get_all_tools |
There was a problem hiding this comment.
Monkey patch targets wrong module for v0.8+ tools
High Severity
_patch_run_get_all_tools wraps run_loop.get_all_tools but assigns the wrapper to agents.run.get_all_tools instead of run_loop.get_all_tools. Since the function was moved from a classmethod on AgentRunner to a standalone function in agents.run_internal.run_loop, the library's internal code likely calls it via the run_loop module — not via agents.run. Unlike all the other patches in this integration (which target classmethods on classes and rely on Python's descriptor protocol), this patches a module-level attribute that callers inside the library probably never look up, meaning the Sentry tool instrumentation would silently not work for v0.8+ users.
There was a problem hiding this comment.
need to patch the reference used in the library.
|
|
||
|
|
||
| def _patch_run_get_all_tools() -> None: | ||
| agents.run.get_all_tools = _create_run_loop_get_all_tools_wrapper( |
There was a problem hiding this comment.
we're creating a lot of random indirection and cognitive overhead with these wrappers. I would just move the patching logic here in this function and remove those wrappers and just keep the _get_all_tools in the tools.py file.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| @staticmethod | ||
| def setup_once() -> None: | ||
| _patch_error_tracing() | ||
| _patch_tools() |
There was a problem hiding this comment.
let's keep _patch_tools for the isolation and add the tools stuff in there
There was a problem hiding this comment.
I want to avoid having the same if condition in all the patch_* functions that boils down to checking the version. Could we consider breaking into _patch_run_get_all_tools() and _patch_agent_runner_get_all_tools(), or do we want different names for these functions that are not based on the location+name of what we're patching?
There was a problem hiding this comment.
ok then keep this for now, we'll look at the shape of the __init__ file at the end, and we remove all the wrappers first
2 participants
Description
Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)