-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: validate tool types in Agent.tools list to prevent runtime AttributeError (fixes #1443) #1974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issue #1443 by adding validation to detect invalid tool types during Agent initialization, preventing confusing runtime AttributeErrors. The fix validates that each item in the tools list is an instance of a valid Tool type before the agent is used.
Key changes:
- Added early validation in
Agent.__post_init__to check tool types and provide clear error messages - Enhanced imports to include all valid tool types for validation
- Added comprehensive test coverage for invalid tool scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/agents/agent.py | Added tool type validation logic in __post_init__ and expanded tool imports for validation |
| tests/test_agent_config.py | Added test cases validating error handling for invalid tool types (raw functions, strings, mixed valid/invalid) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/agents/agent.py
Outdated
| # Validate each tool is a valid Tool type | ||
| for i, tool in enumerate(self.tools): | ||
| if not isinstance( | ||
| tool, | ||
| ( | ||
| FunctionTool, | ||
| FileSearchTool, | ||
| WebSearchTool, | ||
| ComputerTool, | ||
| LocalShellTool, | ||
| ImageGenerationTool, | ||
| CodeInterpreterTool, | ||
| Handoff, | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow HostedMCPTool in tools validation
The new __post_init__ check enumerates acceptable tool classes but omits HostedMCPTool, even though Agent.tools is typed as list[Tool] and Tool already includes HostedMCPTool. Any existing agent that supplies a hosted MCP tool (previously valid) will now fail initialization with UserError, breaking integrations that rely on hosted MCP servers. Consider either checking isinstance(tool, Tool) or adding HostedMCPTool to the tuple so valid tools continue to pass.
Useful? React with 👍 / 👎.
…buteError (fixes openai#1443) This fix addresses issue openai#1443 where passing invalid tool types (e.g., raw functions instead of FunctionTool objects) in the tools list would pass initialization but fail at runtime with: AttributeError: 'function' object has no attribute 'name' (in src/agents/run.py line 584) Changes: - Added validation in Agent.__post_init__ to check each tool in the list - Invalid tools now raise UserError at initialization with helpful message - Suggests using @function_tool decorator when raw functions are detected This provides better developer experience by: 1. Catching errors early (at init vs runtime) 2. Providing clear error messages 3. Suggesting the correct fix Test Coverage: - Added test_tools_content_validation_issue_1443 with 3 test cases - Tests raw functions, strings, and mixed valid/invalid tools - Verifies correct error index is reported
969a695 to
ad0d90b
Compare
|
Thanks for the review! I've updated the code to use Changes:
This approach is cleaner and future-proof! ✨ |
|
Thanks for catching that! However, I've already updated the code to use from typing import get_args
valid_tool_types = get_args(Tool) + (Handoff,)When I run this, it returns: So existing agents using The benefit of this approach is that any future tool types added to the |
| # Tool is a Union type, so we need to get the valid types from it | ||
| from typing import get_args | ||
|
|
||
| valid_tool_types = get_args(Tool) + (Handoff,) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe Handoff should not be included in valid_tool_types.
See the related discussion here:
#1897 (comment)
|
|
||
| # Validate each tool is a valid Tool type | ||
| # Tool is a Union type, so we need to get the valid types from it | ||
| from typing import get_args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this import should be placed at the top of the file with the other imports.
|
Thanks for sending this. The latest code already validates if the "tools" is a list object. So, the original issue could be closed now. |
Summary
This PR fixes issue #1443 where passing invalid tool types (e.g., raw functions instead of
FunctionToolobjects) in thetoolslist would pass initialization but fail at runtime.Problem
Currently, when users mistakenly pass raw functions or other invalid types in the
toolsparameter, the error only appears during runtime:This is confusing because:
@function_tooldecoratorWhy AttributeError Occurs at Runtime
The root cause is in
src/agents/run.py:584:This code assumes all items in
all_toolshave a.nameattribute. However:function) do NOT have a.nameattribute (they only have__name__).nameattributeWhen a raw function reaches this line, Python raises
AttributeError: 'function' object has no attribute 'name'.The existing validation in
Agent.__post_init__only checks iftoolsis a list, not the type of each element:Solution
Add validation in
Agent.__post_init__to check each tool in the list:This provides better developer experience by:
@function_tooldecorator when raw functions detectedChanges
src/agents/agent.py:
UserErrorimport at module level__post_init__to check each tool type usingget_args(Tool)tests/test_agent_config.py:
test_tools_content_validation_issue_1443with 3 test cases:Testing
All tests pass:
pytest tests/test_agent_config.py::TestAgentValidation -v # 6 passedNote on PR #1927
This PR builds on the learnings from PR #1927. While #1927 attempted to change
TypeErrortoUserErrorfor existing validations, this PR adds new validation that was previously missing, preventing runtimeAttributeErrorby catching invalid tool types early.Fixes #1443