fix(tools): run async tool results safely under a running event loop - #6979
fix(tools): run async tool results safely under a running event loop#6979Shailendra005 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSynchronous tool execution now resolves coroutines safely when an event loop is already running. A shared utility handles direct execution and worker-thread execution. Tool invocation tests cover async functions and coroutine-returning functions. ChangesAsync tool execution
Sequence Diagram(s)sequenceDiagram
participant Caller
participant BaseTool
participant run_coroutine_sync
participant WorkerThread
Caller->>BaseTool: call run or invoke
BaseTool->>run_coroutine_sync: execute coroutine synchronously
run_coroutine_sync->>WorkerThread: run coroutine when loop is active
WorkerThread-->>run_coroutine_sync: return result
run_coroutine_sync-->>BaseTool: return result
BaseTool-->>Caller: return tool result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/utilities/async_utils.py`:
- Around line 20-21: Update the documentation around the synchronous execution
helper and its fallback description to state that Future.result() blocks the
calling thread, including an active event-loop thread; the worker thread only
avoids re-entering the loop. Direct asynchronous callers to use arun() or
ainvoke() where applicable.
In `@lib/crewai/tests/tools/test_async_tools.py`:
- Around line 187-203: Add a test alongside
test_structured_tool_invoke_inside_running_loop using a regular def tool
function that returns an inner coroutine, then invoke it from the active loop
through CrewStructuredTool.invoke and assert the resolved result. Keep the test
focused on observable behavior and cover the returned-coroutine path separately
from the async def case.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e41ba2ee-f6c5-4a13-a3f6-cfeb50eee851
📒 Files selected for processing (4)
lib/crewai/src/crewai/tools/base_tool.pylib/crewai/src/crewai/tools/structured_tool.pylib/crewai/src/crewai/utilities/async_utils.pylib/crewai/tests/tools/test_async_tools.py
…coroutine_sync docstring
|
Thanks for the review — both points addressed in 81cab34:
|
What
Tool execution paths that resolve a coroutine result now use the project's established running-loop guard instead of a bare
asyncio.run(), which raisedRuntimeError: asyncio.run() cannot be called from a running event loopwhen a tool was invoked inside a running loop (FastAPI, Jupyter, anyasync def).Why
BaseTool.run, the@tool-decoratorTool.run, andCrewStructuredTool.invokeall didasyncio.run(result)unconditionally. The codebase already has the correct guard (tasks/llm_guardrail.py::_run_coroutine_sync, also inmcp_native_tool.py,a2a/utils/*,mcp/tool_resolver.py,project/*); the tool paths just didn't use it.How
crewai/utilities/async_utils.py::run_coroutine_sync— a generic (TypeVar) version of the existing guard. It lives inutilitiesbecausebase_toolalready importsstructured_tool, so a helper in either module would create a circular import; both now import from the shared util.asyncio.run; when a loop is running it runs the coroutine to completion in a one-workerThreadPoolExecutorwith a copiedcontextvarscontext, so the caller's loop is neither blocked nor re-entered.Tests
New
TestToolRunUnderRunningLoopintests/tools/test_async_tools.pyexercises all three call sites from insideasyncio.run(...). Verified failing before the change (RuntimeError) and passing after.Notes
The other duplicate guard call sites are intentionally left untouched to keep this to one logical change; consolidating them onto
run_coroutine_synccould be a follow-up.Fixes #6978