feat(core): subagent local execution and tool isolation#22718
Conversation
|
Hi @akh64bit, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
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 completes the subagent tool isolation feature by integrating localized configuration and MCP manager updates directly into the agent scheduling process. It ensures that each subagent operates within its own isolated environment for tools, prompts, and resources, enhancing modularity and preventing conflicts between different agent contexts. The changes involve creating dedicated registries for subagents, cloning tools to maintain message bus isolation, and properly initializing MCP servers within these isolated contexts. 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: +2.13 kB (+0.01%) Total Size: 26.2 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request finalizes the subagent tool isolation feature by wiring up localized configurations and registry management into the agent execution loop. The changes introduce private registries for LocalAgentExecutor, clone tools to ensure message bus isolation, and update the McpClientManager to handle multiple registry sets for different subagents. The implementation appears robust and correctly addresses the complex requirements of tool isolation. I did not find any issues of high or critical severity in this pull request.
Note: Security Review did not run due to the size of the PR.
| private get config(): Config { | ||
| return this.context.config; | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, no-restricted-syntax | ||
| const agentConfig: Config = Object.create(this.context.config); |
There was a problem hiding this comment.
I just fixed a bug on Friday related to this pattern of unsafe partial cloning: #22397.
Can we either follow one of the safe proxying patterns I used in #22408 or make targeted refactors to avoid the need to pass around this incorrectly typed object?
@joshualitt just did some related refactors to break a few properties off of config into the AgentLoopContext type. Maybe you can do similar: #21198
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| /* eslint-disable @typescript-eslint/no-explicit-any */ |
There was a problem hiding this comment.
Why are we adding a blanket level disable here?
There was a problem hiding this comment.
Why was package-lock changed?
| this.config | ||
| .getPolicyEngine() | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion | ||
There was a problem hiding this comment.
if you test this with /mcp list in the interactive mode, it seems that the server shows up but the active tools don't. I assume this is b/c the mcp tool is limited to the subagent but can we add a fast follow to fix the UX for this?
There was a problem hiding this comment.
I agree. Lets handle this in a follow up PR.
abhipatel12
left a comment
There was a problem hiding this comment.
LGTM, overall, we need to revisit whether or not the disabled lints is something we want to keep doing.
You may need to run npm run format.
Addresses PR feedback by adding promptRegistry and resourceRegistry directly to AgentLoopContext and updating Config to implement these properly with backing fields. This avoids the need to use Object.create() to proxy the global configuration object in local-executor and agent-scheduler.
1122536 to
2bc030c
Compare
Summary
This is the 3rd and final PR of the subagent tool isolation rollout. It wires up the localized configuration and MCP manager updates directly into the agent scheduling loop, finalizing the tool isolation feature.
Details
LocalAgentExecutorto instantiate its own privateToolRegistry,PromptRegistry, andResourceRegistry..clone()method (from PR 1) to ensure core tools respect the subagent's isolatedmessageBus.maybeDiscoverMcpServer(..., registries)method (from PR 2).agentConfig.toolRegistryinagent-scheduler.tsto ensure the core execution loop leverages the subagent-specific registries.Child of: #22712
Related Issues
Fixes #21901
How to Validate
npm run typecheckandnpm run testto verify everything compiles and all existing tests pass.Pre-Merge Checklist