Allow agents to call agents - #28738
Conversation
|
📊 PR Size: size/L
|
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 enables hierarchical agent delegation, allowing subagents to invoke other subagents. This functionality is opt-in, requiring explicit configuration in the agent's frontmatter. The implementation includes robust recursion protection, ensuring that agent nesting does not exceed a predefined depth, and maintains strict validation to prevent unauthorized tool or agent access. Highlights
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 the 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 counterproductive. 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. Footnotes
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
There was a problem hiding this comment.
Code Review
This pull request introduces subagent delegation and bounded recursion (capped at 3 levels) to prevent infinite loops. It updates the configuration schema, tool validation, and executor logic to allow agents to explicitly opt-in to calling other agents. The code reviewer identified a critical security vulnerability in local-executor.ts where nested subagents could bypass tool isolation by inheriting the grandparent's unrestricted tool registry instead of the parent's restricted registry. To address this, the reviewer suggested passing the current agent's isolated registries and message bus to the nested context, along with a corresponding test update.
| const nestedContext: AgentLoopContext = { | ||
| config: context.config, | ||
| promptId: context.promptId, | ||
| parentSessionId: context.parentSessionId, | ||
| toolRegistry: context.toolRegistry, | ||
| promptRegistry: context.promptRegistry, | ||
| resourceRegistry: context.resourceRegistry, | ||
| messageBus: context.messageBus, | ||
| geminiClient: context.geminiClient, | ||
| sandboxManager: context.sandboxManager, | ||
| agentDepth: nestedDepth, | ||
| }; |
There was a problem hiding this comment.
Using the grandparent's registries (context.toolRegistry, etc.) for the child agent's nestedContext creates a critical security vulnerability and tool isolation bypass. If a restricted subagent invokes another subagent, the child subagent will inherit the grandparent's unrestricted tool registry instead of the parent's restricted registry, allowing privilege escalation. The nestedContext must use the current agent's isolated registries (agentToolRegistry, agentPromptRegistry, agentResourceRegistry) and message bus (subagentMessageBus) to preserve tool isolation boundaries.
| const nestedContext: AgentLoopContext = { | |
| config: context.config, | |
| promptId: context.promptId, | |
| parentSessionId: context.parentSessionId, | |
| toolRegistry: context.toolRegistry, | |
| promptRegistry: context.promptRegistry, | |
| resourceRegistry: context.resourceRegistry, | |
| messageBus: context.messageBus, | |
| geminiClient: context.geminiClient, | |
| sandboxManager: context.sandboxManager, | |
| agentDepth: nestedDepth, | |
| }; | |
| const nestedContext: AgentLoopContext = { | |
| config: context.config, | |
| promptId: context.promptId, | |
| parentSessionId: context.parentSessionId, | |
| toolRegistry: agentToolRegistry, | |
| promptRegistry: agentPromptRegistry, | |
| resourceRegistry: agentResourceRegistry, | |
| messageBus: subagentMessageBus, | |
| geminiClient: context.geminiClient, | |
| sandboxManager: context.sandboxManager, | |
| agentDepth: nestedDepth, | |
| }; |
| it('should hand the nested agent tool an incremented depth', async () => { | ||
| stubAgentRegistry(['code-reviewer']); | ||
|
|
||
| const definition = createTestDefinition([ | ||
| LS_TOOL_NAME, | ||
| 'code-reviewer', | ||
| ]); | ||
| const executor = await LocalAgentExecutor.create( | ||
| definition, | ||
| mockConfig, | ||
| onActivity, | ||
| ); | ||
|
|
||
| const agentTool = executor['toolRegistry'].getTool( | ||
| AGENT_TOOL_NAME, | ||
| ) as AgentTool; | ||
| const nestedContext = agentTool['context']; | ||
| expect(nestedContext.agentDepth).toBe(1); | ||
| expect(nestedContext.config).toBe(mockConfig); | ||
| expect(nestedContext.toolRegistry).toBe(parentToolRegistry); | ||
| }); |
There was a problem hiding this comment.
Update the test to assert that the nested context inherits the current agent's isolated tool registry (executor['toolRegistry']) instead of the grandparent's registry (parentToolRegistry), aligning with the security fix.
it('should hand the nested agent tool an incremented depth', async () => {
stubAgentRegistry(['code-reviewer']);
const definition = createTestDefinition([
LS_TOOL_NAME,
'code-reviewer',
]);
const executor = await LocalAgentExecutor.create(
definition,
mockConfig,
onActivity,
);
const agentTool = executor['toolRegistry'].getTool(
AGENT_TOOL_NAME,
) as AgentTool;
const nestedContext = agentTool['context'];
expect(nestedContext.agentDepth).toBe(1);
expect(nestedContext.config).toBe(mockConfig);
expect(nestedContext.toolRegistry).toBe(executor['toolRegistry']);
});|
Hi, @rnett Can you please review this pr? Thanks! |
|
Hi @scidomino, can you please review the changes? |
Summary
Allow agents to call agents
Fixes #22092
Details
Fixes #22092 by letting subagents delegate to other subagents — or recurse into themselves — via their
tools:frontmatter. This was blocked twice over: the agent loader rejected agent names asInvalid tool name, and the executor stripped everyKind.Agenttool from subagent registries, so even a name that passed validation would have had no effect. isValidToolName()now takes anallowAgentNamesoption (checked after the MCP branch, so malformed names likemcp__toolstay rejected), and the executor resolves eachtools:entry through the tool registry, then the agent registry, then warns instead of silently dropping it. Resolved agents get aninvoke_agenttool whoseagent_nameis constrained to a JSON-schemaenumof exactly those agents. A bare agent name grants that one agent,agent_*orinvoke_agentgrants all of them, and a self-reference enables recursion;*and an omittedtools:deliberately grant no agent access, so no existing definition changes behaviour — notablygeneralist-agent, whose list is built fromgetAllToolNames(). Recursion is bounded three ways: an agent at max depth is never handed the tool,AgentTool.createInvocationrejects nesting pastMAX_AGENT_DEPTH = 3, and each level keeps its ownmax_turns/timeout_mins. Docs updated; 26 tests added acrosstool-names,agentLoader,agent-toolandlocal-executor, with the two pre-existing recursion tests still passing unmodified. Note that this introduces a deferred import cycle (local-executor→agent-tool→local-invocation), which ESM resolves correctly and the tests exercise in both load orders — happy to switch to a dynamic import if preferred.Related Issues
How to Validate
Pre-Merge Checklist