fix(core): resolve MCP tool FQN validation, schema export, and wildcards in subagents#22069
fix(core): resolve MCP tool FQN validation, schema export, and wildcards in subagents#22069abhipatel12 merged 3 commits intomainfrom
Conversation
68ddf38 to
bbe0871
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the system's ability to handle tool names within subagents by modernizing the validation and resolution mechanisms. It primarily addresses issues with Fully Qualified Names (FQNs) for MCP tools, ensuring that subagents can correctly interpret and utilize the standard Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Size Change: +286 B (0%) Total Size: 26.5 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses several issues related to MCP tool name validation and subagent tool loading. The transition to validating fully qualified mcp_ names in tool-names.ts and removing legacy __ parsing logic is a great improvement for consistency and correctness. The added support for wildcards in subagent definitions within local-executor.ts is also a valuable feature enhancement.
I've found one minor bug in the new validation logic where tool names consisting only of underscores could pass validation. I've left a specific comment with a suggested fix. Overall, this is a solid set of changes that improves the robustness of tool handling for subagents.
bbe0871 to
6eca4bb
Compare
| return; | ||
| } | ||
|
|
||
| if (tool instanceof DiscoveredMCPTool) { |
There was a problem hiding this comment.
Are there ever cases where this instanceof check wouldn't work right at runtime due to the object being deserialized instead of constructed the normal way?
There was a problem hiding this comment.
Taking a look through the code and double checking with Gemini, it appears this is safe. Also seems we do this same thing more than 20 places around the codebase, so should be safe.
gundermanc
left a comment
There was a problem hiding this comment.
Approved with a suggestion.
6eca4bb to
4ae29de
Compare
4ae29de to
9cf4e2a
Compare
|
/patch both |
|
🚀 [Step 1/4] Patch workflow(s) waiting for approval! 📋 Details:
⏳ Status: The patch creation workflow has been triggered and is waiting for deployment approval. Please visit the specific workflow links below and approve the runs. 🔗 Track Progress: |
…rds in subagents (#22069)
|
🚀 [Step 2/4] Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
…rds in subagents (#22069)
|
🚀 [Step 2/4] Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 [Step 3/4] Patch Release Waiting for Approval! 📋 Release Details:
⏳ Status: The patch release has been triggered and is waiting for deployment approval. Please visit the specific workflow run link below and approve the deployment. You'll receive another update when it completes. 🔗 Track Progress: |
|
🚀 [Step 3/4] Patch Release Waiting for Approval! 📋 Release Details:
⏳ Status: The patch release has been triggered and is waiting for deployment approval. Please visit the specific workflow run link below and approve the deployment. You'll receive another update when it completes. 🔗 Track Progress: |
|
✅ [Step 4/4] Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
|
✅ [Step 4/4] Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
|
Done bro |
Summary
Fixes an issue where subagents rejected standard
mcp_{serverName}_{toolName}Fully Qualified Names (FQNs) due to outdated validation logic that incorrectly permitted only the deprecatedserver__toollegacy format. It additionally adds internal support for resolving generic and MCP wildcards during subagent tool loads.Details
isValidToolNamelogic intool-names.tsnow securely validatesmcp_names by strictly adhering to the parsing constraints given bymcp-tool.ts.__insidetool-registry.ts, as the Policy Engine normalizes structure tomcp_beforegetTool(name)executes.tool-names.test.ts.agentLoader.tsZod schema to allow wildcards when parsing thetoolsarray of a subagent configuration file (by supplying{ allowWildcards: true }toisValidToolName).*) and MCP wildcard (mcp_*,mcp_server_*) resolution natively insidelocal-executor.tssince subagents lacked the ability to expand tools organically.local-executor.tsprepareToolsList, and updatedToolRegistry.getFunctionDeclarationsFiltered(used by subagents) to properly rewrite the tool schemanameto the Fully Qualified Name (e.g.mcp_server_tool) just likegetFunctionDeclarationsdoes. This ensures the model executes authorized tools instead of hallucinating stripped bare names.Related Issues
N/A
How to Validate
settings.json.mcp_serverName_*).npm run testacross@google/gemini-cli-core.npm run preflight.Pre-Merge Checklist