Skip to content

Commit 7bfe6ac

Browse files
authored
feat(core): subagent local execution and tool isolation (google-gemini#22718)
1 parent bd34a42 commit 7bfe6ac

7 files changed

Lines changed: 222 additions & 56 deletions

File tree

packages/cli/src/test-utils/AppRig.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,14 @@ export class AppRig {
280280
}
281281

282282
private stubRefreshAuth() {
283-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion, @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment
283+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
284284
const gcConfig = this.config as any;
285285
gcConfig.refreshAuth = async (authMethod: AuthType) => {
286286
gcConfig.modelAvailabilityService.reset();
287287

288288
const newContentGeneratorConfig = {
289289
authType: authMethod,
290-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
290+
291291
proxy: gcConfig.getProxy(),
292292
apiKey: process.env['GEMINI_API_KEY'] || 'test-api-key',
293293
};
@@ -456,7 +456,7 @@ export class AppRig {
456456
const actualToolName = toolName === '*' ? undefined : toolName;
457457
this.config
458458
.getPolicyEngine()
459-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
459+
460460
.removeRulesForTool(actualToolName as string, source);
461461
this.breakpointTools.delete(toolName);
462462
}
@@ -729,7 +729,7 @@ export class AppRig {
729729
.getGeminiClient()
730730
?.getChatRecordingService();
731731
if (recordingService) {
732-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-type-assertion
732+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
733733
(recordingService as any).conversationFile = null;
734734
}
735735
}
@@ -749,7 +749,7 @@ export class AppRig {
749749
MockShellExecutionService.reset();
750750
ideContextStore.clear();
751751
// Forcefully clear IdeClient singleton promise
752-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-type-assertion
752+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
753753
(IdeClient as any).instancePromise = null;
754754
vi.clearAllMocks();
755755

packages/core/src/agents/agent-scheduler.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ describe('agent-scheduler', () => {
4242

4343
it('should create a scheduler with agent-specific config', async () => {
4444
const mockConfig = {
45+
getPromptRegistry: vi.fn(),
46+
getResourceRegistry: vi.fn(),
4547
messageBus: mockMessageBus,
4648
toolRegistry: mockToolRegistry,
4749
} as unknown as Mocked<Config>;
@@ -91,6 +93,8 @@ describe('agent-scheduler', () => {
9193
} as unknown as Mocked<ToolRegistry>;
9294

9395
const config = {
96+
getPromptRegistry: vi.fn(),
97+
getResourceRegistry: vi.fn(),
9498
messageBus: mockMessageBus,
9599
} as unknown as Mocked<Config>;
96100
Object.defineProperty(config, 'toolRegistry', {
@@ -123,6 +127,8 @@ describe('agent-scheduler', () => {
123127

124128
it('should create an AgentLoopContext that has a defined .config property', async () => {
125129
const mockConfig = {
130+
getPromptRegistry: vi.fn(),
131+
getResourceRegistry: vi.fn(),
126132
messageBus: mockMessageBus,
127133
toolRegistry: mockToolRegistry,
128134
promptId: 'test-prompt',

packages/core/src/agents/agent-scheduler.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import type {
1111
CompletedToolCall,
1212
} from '../scheduler/types.js';
1313
import type { ToolRegistry } from '../tools/tool-registry.js';
14+
import type { PromptRegistry } from '../prompts/prompt-registry.js';
15+
import type { ResourceRegistry } from '../resources/resource-registry.js';
1416
import type { EditorType } from '../utils/editor.js';
1517

1618
/**
@@ -25,6 +27,10 @@ export interface AgentSchedulingOptions {
2527
parentCallId?: string;
2628
/** The tool registry specific to this agent. */
2729
toolRegistry: ToolRegistry;
30+
/** The prompt registry specific to this agent. */
31+
promptRegistry?: PromptRegistry;
32+
/** The resource registry specific to this agent. */
33+
resourceRegistry?: ResourceRegistry;
2834
/** AbortSignal for cancellation. */
2935
signal: AbortSignal;
3036
/** Optional function to get the preferred editor for tool modifications. */
@@ -51,16 +57,19 @@ export async function scheduleAgentTools(
5157
subagent,
5258
parentCallId,
5359
toolRegistry,
60+
promptRegistry,
61+
resourceRegistry,
5462
signal,
5563
getPreferredEditor,
5664
onWaitingForConfirmation,
5765
} = options;
5866

59-
// Create a proxy/override of the config to provide the agent-specific tool registry.
6067
const schedulerContext = {
6168
config,
6269
promptId: config.promptId,
6370
toolRegistry,
71+
promptRegistry: promptRegistry ?? config.getPromptRegistry(),
72+
resourceRegistry: resourceRegistry ?? config.getResourceRegistry(),
6473
messageBus: toolRegistry.messageBus,
6574
geminiClient: config.geminiClient,
6675
sandboxManager: config.sandboxManager,

packages/core/src/agents/local-executor.test.ts

Lines changed: 94 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,43 @@ import {
1313
afterEach,
1414
type Mock,
1515
} from 'vitest';
16+
17+
const {
18+
mockSendMessageStream,
19+
mockScheduleAgentTools,
20+
mockSetSystemInstruction,
21+
mockCompress,
22+
mockMaybeDiscoverMcpServer,
23+
mockStopMcp,
24+
} = vi.hoisted(() => ({
25+
mockSendMessageStream: vi.fn().mockResolvedValue({
26+
async *[Symbol.asyncIterator]() {
27+
yield {
28+
type: 'chunk',
29+
value: { candidates: [] },
30+
};
31+
},
32+
}),
33+
mockScheduleAgentTools: vi.fn(),
34+
mockSetSystemInstruction: vi.fn(),
35+
mockCompress: vi.fn(),
36+
mockMaybeDiscoverMcpServer: vi.fn().mockResolvedValue(undefined),
37+
mockStopMcp: vi.fn().mockResolvedValue(undefined),
38+
}));
39+
40+
vi.mock('../tools/mcp-client-manager.js', () => ({
41+
McpClientManager: class {
42+
maybeDiscoverMcpServer = mockMaybeDiscoverMcpServer;
43+
stop = mockStopMcp;
44+
},
45+
}));
46+
1647
import { debugLogger } from '../utils/debugLogger.js';
1748
import { LocalAgentExecutor, type ActivityCallback } from './local-executor.js';
1849
import { makeFakeConfig } from '../test-utils/config.js';
1950
import { ToolRegistry } from '../tools/tool-registry.js';
51+
import { PromptRegistry } from '../prompts/prompt-registry.js';
52+
import { ResourceRegistry } from '../resources/resource-registry.js';
2053
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
2154
import { LSTool } from '../tools/ls.js';
2255
import { LS_TOOL_NAME, READ_FILE_TOOL_NAME } from '../tools/tool-names.js';
@@ -70,18 +103,6 @@ import type {
70103
import { getModelConfigAlias, type AgentRegistry } from './registry.js';
71104
import type { ModelRouterService } from '../routing/modelRouterService.js';
72105

73-
const {
74-
mockSendMessageStream,
75-
mockScheduleAgentTools,
76-
mockSetSystemInstruction,
77-
mockCompress,
78-
} = vi.hoisted(() => ({
79-
mockSendMessageStream: vi.fn(),
80-
mockScheduleAgentTools: vi.fn(),
81-
mockSetSystemInstruction: vi.fn(),
82-
mockCompress: vi.fn(),
83-
}));
84-
85106
let mockChatHistory: Content[] = [];
86107
const mockSetHistory = vi.fn((newHistory: Content[]) => {
87108
mockChatHistory = newHistory;
@@ -2722,6 +2743,67 @@ describe('LocalAgentExecutor', () => {
27222743
});
27232744
});
27242745

2746+
describe('MCP Isolation', () => {
2747+
it('should initialize McpClientManager when mcpServers are defined', async () => {
2748+
const { MCPServerConfig } = await import('../config/config.js');
2749+
const mcpServers = {
2750+
'test-server': new MCPServerConfig('node', ['server.js']),
2751+
};
2752+
2753+
const definition = {
2754+
...createTestDefinition(),
2755+
mcpServers,
2756+
};
2757+
2758+
vi.spyOn(mockConfig, 'getMcpClientManager').mockReturnValue({
2759+
maybeDiscoverMcpServer: mockMaybeDiscoverMcpServer,
2760+
} as unknown as ReturnType<typeof mockConfig.getMcpClientManager>);
2761+
2762+
await LocalAgentExecutor.create(definition, mockConfig);
2763+
2764+
const mcpManager = mockConfig.getMcpClientManager();
2765+
expect(mcpManager?.maybeDiscoverMcpServer).toHaveBeenCalledWith(
2766+
'test-server',
2767+
mcpServers['test-server'],
2768+
expect.objectContaining({
2769+
toolRegistry: expect.any(ToolRegistry),
2770+
promptRegistry: expect.any(PromptRegistry),
2771+
resourceRegistry: expect.any(ResourceRegistry),
2772+
}),
2773+
);
2774+
});
2775+
2776+
it('should inherit main registry tools', async () => {
2777+
const parentMcpTool = new DiscoveredMCPTool(
2778+
{} as unknown as CallableTool,
2779+
'main-server',
2780+
'tool1',
2781+
'desc1',
2782+
{},
2783+
mockConfig.getMessageBus(),
2784+
);
2785+
2786+
parentToolRegistry.registerTool(parentMcpTool);
2787+
2788+
const definition = createTestDefinition();
2789+
definition.toolConfig = undefined; // trigger inheritance
2790+
2791+
vi.spyOn(mockConfig, 'getMcpClientManager').mockReturnValue({
2792+
maybeDiscoverMcpServer: vi.fn(),
2793+
} as unknown as ReturnType<typeof mockConfig.getMcpClientManager>);
2794+
const executor = await LocalAgentExecutor.create(
2795+
definition,
2796+
mockConfig,
2797+
onActivity,
2798+
);
2799+
const agentTools = (
2800+
executor as unknown as { toolRegistry: ToolRegistry }
2801+
).toolRegistry.getAllToolNames();
2802+
2803+
expect(agentTools).toContain(parentMcpTool.name);
2804+
});
2805+
});
2806+
27252807
describe('DeclarativeTool instance tools (browser agent pattern)', () => {
27262808
/**
27272809
* The browser agent passes DeclarativeTool instances (not string names) in
@@ -2827,13 +2909,11 @@ describe('LocalAgentExecutor', () => {
28272909
const navTool = new MockTool({ name: 'navigate_page' });
28282910

28292911
const definition = createInstanceToolDefinition([clickTool, navTool]);
2830-
28312912
const executor = await LocalAgentExecutor.create(
28322913
definition,
28332914
mockConfig,
28342915
onActivity,
28352916
);
2836-
28372917
const registry = executor['toolRegistry'];
28382918
expect(registry.getTool('click')).toBeDefined();
28392919
expect(registry.getTool('navigate_page')).toBeDefined();

0 commit comments

Comments
 (0)