Repository navigation
fix(tools): warn when a tool parameter named agent has a non-Agent annotation - #5622
Open
surlymochan wants to merge 1 commit into
Open
surlymochan wants to merge 1 commit into
surlymochan wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Human Overview
Description
A
@toolparameter namedagentis silently excluded from the generatedinputSchema, and at call time it receives the invokingAgentinstance frominvocation_state, bypassing validation entirely. When the parameter carries an ordinary annotation such asLiteral["reviewer", "explorer"], both halves fail invisibly: the model never sees the parameter, and the tool body receives anAgentobject where its annotation promises something else.This makes the collision loud at definition time, following option (1) + (3) from #3499.
FunctionToolMetadatanow checks theagentparameter when a tool is defined: if it is annotated with a type that cannot hold anAgent(unwrappingAnnotatedand unions;Agentsubclasses pass), it emits aUserWarningnaming the parameter and the remedy — rename it, or annotate it asAgent. Unannotated andAny-annotated parameters keep the current behavior, as does a context parameter that happens to be namedagent. The@tooldocstring now documents the reserved parameter names (self,cls,agent).A warning rather than an error: injection into an annotated
agentparameter is existing, tested behavior (test_stream_with_agentinjects intoagent: dict), so rejecting it outright would break working tools. The warning follows the codebase convention for developer-facing usage notices (warnings.warnwithstacklevel, as inmodels/_validation.py).Related Issues
Fixes #3499
Documentation PR
None needed. The reserved parameter names are documented in the
@tooldocstring itself.Type of Change
Bug fix
Testing
Issue reproduction script (the exact
delegatetool from Tool parameter namedagentis silently dropped from inputSchema (reserved special parameter, no warning) #3499): now emits theUserWarningat definition time;agent: Agentand untypedagenttools are still created withagentexcluded from the schema.tests/strands/tools/test_decorator.py— 79 passed, including 4 new tests (warning on aLiteral-annotatedagent, schema exclusion forAgentandAgent | Noneannotations, exemption for a context parameter namedagent). The 10 remaining failures in that file are pre-existing sandbox issues (botocore reaching the EC2 metadata endpoint while constructing a default Bedrock-backedAgent); the same tests fail identically on an unmodified checkout in this environment.ruff format --checkandruff checkon both changed files — clean (standalone ruff).mypyonsrc/strands/tools/decorator.py— no new errors (standalone run with imports skipped reports the same 5 pre-existing errors as an unmodified checkout; the full static-analysis env could not be built in this sandbox).I ran
hatch run prepare(could not build thehatch-static-analysisenv in this sandbox; the test, ruff, and mypy checks listed above were run instead)Additional Details
Extended context (optional)
Design discussion on #3499 converged on flagging the collision when the tool is defined rather than at call time. Two of the proposals suggested raising
ValueError; this PR warns instead, becausetests/strands/tools/test_decorator.py::test_stream_with_agentcodifies injection into anagent: dictparameter as supported behavior, and a hard error would break that contract for every existing tool that relies on it. If maintainers prefer the stricter behavior, the check inFunctionToolMetadata._validate_agent_parameteris the single place to change.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.