fix: validate tool kwargs even when empty to prevent cryptic TypeError (#4611)#4612
Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Open
fix: validate tool kwargs even when empty to prevent cryptic TypeError (#4611)#4612devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Conversation
#4611) When BaseTool.run() was called with no keyword arguments (empty dict), _validate_kwargs skipped validation because 'if kwargs' is False for {}. This meant required schema fields like 'queries' in RecallMemoryTool were never checked, causing a confusing TypeError: '_run() missing 1 required positional argument: queries' Fix: Remove the 'kwargs' truthiness check so validation always runs when an args_schema with fields is present. Skip validation only when positional args are provided (backwards compat for tool.run('value') style calls). Also adds dedicated test coverage for RecallMemoryTool and RememberTool. Co-Authored-By: João <joao@crewai.com>
Contributor
Author
|
Prompt hidden (unlisted session) |
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import Any |
Comment on lines
+13
to
+18
| from crewai.tools.memory_tools import ( | ||
| RecallMemorySchema, | ||
| RecallMemoryTool, | ||
| RememberTool, | ||
| create_memory_tools, | ||
| ) |
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.
fix: validate tool kwargs even when empty to prevent cryptic TypeError (#4611)
Summary
Fixes #4611 —
RecallMemoryTool._run() missing 1 required positional argument: 'queries'.Root cause:
BaseTool._validate_kwargshad aif kwargs and ...guard that skipped schema validation whenkwargswas an empty dict{}. In the native tool-calling path, if the LLM sends back empty or malformed arguments,tool.run(**{})bypasses validation entirely, and_run()is called without required arguments — producing a confusingTypeErrorinstead of a clearValueErrorwith field-level details.Fix: Remove the
kwargstruthiness check soargs_schema.model_validate()always runs when a schema with fields is present. Addif not args:guards inrun()/arun()to preserve backwards compatibility for positional-arg calls liketool.run("value").Applied consistently to all 4 entry points:
BaseTool.run,BaseTool.arun,Tool.run,Tool.arun.Review & Testing Checklist for Human
if not args:guard is safe: When positional args are provided (tool.run("val")), kwargs validation is skipped. Confirm this doesn't allow invalid kwargs to slip through silently in any important code path (the native tool-calling path always uses kwargs, never positional args, so it should be fine).tool.run()—model_validate({})should return defaults. Worth a quick manual check.test_max_usage_count_is_respectedVCR test was skipped locally due to missing API key).Suggested test plan: Create a simple crew with
memory=True, give the agent a task that would trigger memory search, and confirm it no longer crashes with theTypeError. The new unit tests cover the direct tool invocation path.Notes