fix(tools): await coroutines returned by sync wrappers in CrewStructuredTool.ainvoke - #7481
Open
Rainmemery wants to merge 1 commit into
Open
Rainmemery wants to merge 1 commit into
Rainmemery wants to merge 1 commit into
Conversation
…redTool.ainvoke (crewAIInc#7474) BaseTool.to_structured_tool() stores func=self._run, and Tool._run is a sync method that returns self.func(...) as-is - for an async tool that is a bare coroutine. Inside CrewStructuredTool.ainvoke the sync branch dispatches through run_in_executor, so inspect.iscoroutinefunction(self.func) is False and the coroutine object was handed straight back to the caller: the tool body never ran, the observation fed to the LLM was the literal '<coroutine object ...>' repr, and the coroutine was GC'd with 'coroutine was never awaited'. Every sibling branch already resolves this (invoke() checks asyncio.iscoroutine, BaseTool.run and Tool.run re-run via asyncio.run, Tool._arun awaits awaitables); only the async branch of CrewStructuredTool missed the check. Await the result when it is awaitable before returning it. Regression tests cover the direct sync-wrapper case and the @tool(async_fn).to_structured_tool() path from the issue.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesStructured tool async invocation
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The async invocation change resolves coroutine results for the affected wrapper paths without an identified remaining merge risk. 🚥 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 |
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
1 participant
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.
Fixes #7474
Summary
BaseTool.to_structured_tool()storesfunc=self._run(lib/crewai/src/crewai/tools/base_tool.py:413), andTool._runis a sync method that returnsself.func(...)as-is - for an@tool-decorated async function that is a bare coroutine. InsideCrewStructuredTool.ainvoke(),inspect.iscoroutinefunction(self.func)is thereforeFalse, the call is dispatched throughrun_in_executor, and the coroutine object was handed straight back to the caller: the tool body never executed, the observation fed to the LLM was the literal<coroutine object fetch_data at 0x...>repr, and the coroutine was garbage-collected withRuntimeWarning: coroutine 'fetch_data' was never awaited.Every sibling branch already handles this case -
invoke()(lines 440-447) checksasyncio.iscoroutine(result),BaseTool.runandTool.runre-run viaasyncio.run, andTool._arunawaits awaitables. Only the async branch ofCrewStructuredToolmissed the check.Change
The executor branch of
ainvoke()now awaits the result when it is awaitable before returning it, mirroring the sibling branches.Test plan
pytest lib/crewai/tests/tools/test_structured_tool.py- 26 passed (24 existing + 2 new):test_ainvoke_awaits_coroutine_from_sync_wrapper: a sync wrapper function returning a coroutine; previouslyainvokereturned the coroutine object itself, now the tool body runs and its result is returned;test_ainvoke_through_to_structured_tool_resolves_async_tool_result: the exact path from the issue (@tool async def->to_structured_tool()->ainvoke); previously the caller received<coroutine object ...>, now the awaited tool result.Full
tests/tools/directory: 241 passed; the twotest_tool_failure.pyfailures reproduce on pristinemainin this environment (mock/transport environment issue, unrelated to this change).