[5660] chore(sdk): validate reserved tool names before secret lookup and adapter calls - #5672
Conversation
|
Someone is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
✅ Thanks @MarceloAdan73! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon. |
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesTool name validation
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant ToolResolver
participant SecretProvider
participant GatewayAdapter
ToolResolver->>ToolResolver: Validate declared tool names
ToolResolver->>SecretProvider: Look up secrets for valid configurations
ToolResolver->>GatewayAdapter: Resolve valid gateway configurations
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
0ebf448 to
906dab0
Compare
906dab0 to
7ff9c9b
Compare
|
@all-contributors please add @MarceloAdan73 for code |
|
I've put up a pull request to add @MarceloAdan73! 🎉 |
mmabrouk
left a comment
There was a problem hiding this comment.
Thanks @MarceloAdan73 ! LGTM
Summary
When a tool config declares a custom tool whose name collides with a Pi built-in, the SDK's
ToolResolverrejects it withReservedToolNameError— but only after resolving secrets and calling the workflow, platform, and gateway adapters. A reserved-namedcodetool with a missing secret today surfacesMissingToolSecretError(wrong problem), and reserved-named gateway/reference/platform tools still do secret lookups and network calls before being refused (wasted work for a payload that will always be rejected).This PR moves the reserved/duplicate declared-name validation to the top of
resolve(), before any secret lookup or adapter call, while keeping the final_validate_unique_names(tool_specs)pass to cover adapter-produced spec names. Behavior for valid payloads is identical — only error precedence changes.Demo
Before (adapter invoked, wrong error) vs after (fails fast, zero adapter calls):
Symptom
A reserved-named
codetool with a missing secret surfacesMissingToolSecretErrorinstead ofReservedToolNameError, pointing the author at the wrong problem. A reserved-named gateway, reference, or platform tool still reaches its adapter (secret lookups, network calls) before rejection, so the resolver does work for a payload that will always be refused.The rejection always happened before any spec left
resolve(), so no reserved name reached the runner; this is error-precedence and wasted-work hygiene, not a bypass.Change
Declared custom tool names are now validated up front, before any secret lookup or adapter resolution:
_validate_declared_config_names()at the top ofresolve(), raisingReservedToolNameError(reserved built-in names) orDuplicateToolNameError(duplicates among declared names) before any secret or adapter work._check_tool_name()so both the new up-front pass and the final_validate_unique_names(tool_specs)use the same logic (no duplication)._validate_unique_names()call stays unchanged, still covering adapter-produced spec names (e.g. a gatewayintegration__actionfallback) that the up-front pass cannot see.Before / After
codetool namedreadwith missing secretMissingToolSecretError(wrong problem)ReservedToolNameError(fail fast)nameReservedToolNameErrorbefore any adapter callTests
test_reserved_name_fails_fast_before_secret_lookup_and_adapter_calls: reserved-namedcodetool with a missing secret raisesReservedToolNameError(notMissingToolSecretError), and the secret provider and gateway adapter are never invoked.test_reserved_gateway_name_fails_before_adapter_call: reserved gateway name is rejected before the adapter runs.agentadistribution not installed in the local venv;test_resolver.py25/25 green). Ruff 0.15.12 (CI version) and ruff format pass on both files.AI model used
This change was produced with an AI coding agent (DeepSeek v4 flash, opencode agent) following the step-by-step instructions in this issue. The session: read
resolver.pyandtest_resolver.py, implemented the up-front validation pass factoring the shared name checks, added the two fast-fail tests, ran the targeted tests (25/25), sanity-checked test validity by reverting the fix (both new tests failed, confirming they are not vacuous), restored the fix, ran the full unit suite (only pre-existing environment gaps failed), and verified ruff 0.15.12 + format pass.Closes #5660