Skip to content

Commit 4085b4f

Browse files
fix(patch): cherry-pick 8432bce to release/v0.34.0-preview.1-pr-22069 to patch version v0.34.0-preview.1 and create version 0.34.0-preview.2 (#22205)
Co-authored-by: Abhi <43648792+abhipatel12@users.noreply.github.com> Co-authored-by: Abhi <abhipatel@google.com>
1 parent 881822f commit 4085b4f

8 files changed

Lines changed: 165 additions & 104 deletions

File tree

packages/core/src/agents/agentLoader.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,11 @@ const localAgentSchema = z
107107
display_name: z.string().optional(),
108108
tools: z
109109
.array(
110-
z.string().refine((val) => isValidToolName(val), {
111-
message: 'Invalid tool name',
112-
}),
110+
z
111+
.string()
112+
.refine((val) => isValidToolName(val, { allowWildcards: true }), {
113+
message: 'Invalid tool name',
114+
}),
113115
)
114116
.optional(),
115117
model: z.string().optional(),

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
type PartListUnion,
3434
type Tool,
3535
type CallableTool,
36+
type FunctionDeclaration,
3637
} from '@google/genai';
3738
import type { Config } from '../config/config.js';
3839
import { MockTool } from '../test-utils/mock-tool.js';
@@ -550,6 +551,34 @@ describe('LocalAgentExecutor', () => {
550551

551552
getToolSpy.mockRestore();
552553
});
554+
555+
it('should not duplicate schemas when instantiated tools are provided in toolConfig', async () => {
556+
// Create an instantiated mock tool
557+
const instantiatedTool = new MockTool({ name: 'instantiated_tool' });
558+
559+
// Create an agent definition containing the instantiated tool
560+
const definition = createTestDefinition([instantiatedTool]);
561+
562+
// Create the executor
563+
const executor = await LocalAgentExecutor.create(
564+
definition,
565+
mockConfig,
566+
onActivity,
567+
);
568+
569+
// Extract the prepared tools list using the private method
570+
const toolsList = (
571+
executor as unknown as { prepareToolsList: () => FunctionDeclaration[] }
572+
).prepareToolsList();
573+
574+
// Filter for the specific tool schema
575+
const foundSchemas = (
576+
toolsList as unknown as FunctionDeclaration[]
577+
).filter((t: FunctionDeclaration) => t.name === 'instantiated_tool');
578+
579+
// Assert that there is exactly ONE schema for this tool
580+
expect(foundSchemas).toHaveLength(1);
581+
});
553582
});
554583

555584
describe('run (Execution Loop and Logic)', () => {

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

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,13 @@ import {
1616
type Schema,
1717
} from '@google/genai';
1818
import { ToolRegistry } from '../tools/tool-registry.js';
19-
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
19+
import { type AnyDeclarativeTool } from '../tools/tools.js';
20+
import {
21+
DiscoveredMCPTool,
22+
isMcpToolName,
23+
parseMcpToolName,
24+
MCP_TOOL_PREFIX,
25+
} from '../tools/mcp-tool.js';
2026
import { CompressionStatus } from '../core/turn.js';
2127
import { type ToolCallRequestInfo } from '../scheduler/types.js';
2228
import { type Message } from '../confirmation-bus/types.js';
@@ -141,28 +147,55 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
141147
runtimeContext.getAgentRegistry().getAllAgentNames(),
142148
);
143149

144-
const registerToolByName = (toolName: string) => {
150+
const registerToolInstance = (tool: AnyDeclarativeTool) => {
145151
// Check if the tool is a subagent to prevent recursion.
146152
// We do not allow agents to call other agents.
147-
if (allAgentNames.has(toolName)) {
153+
if (allAgentNames.has(tool.name)) {
148154
debugLogger.warn(
149-
`[LocalAgentExecutor] Skipping subagent tool '${toolName}' for agent '${definition.name}' to prevent recursion.`,
155+
`[LocalAgentExecutor] Skipping subagent tool '${tool.name}' for agent '${definition.name}' to prevent recursion.`,
150156
);
151157
return;
152158
}
153159

160+
agentToolRegistry.registerTool(tool);
161+
};
162+
163+
const registerToolByName = (toolName: string) => {
164+
// Handle global wildcard
165+
if (toolName === '*') {
166+
for (const tool of parentToolRegistry.getAllTools()) {
167+
registerToolInstance(tool);
168+
}
169+
return;
170+
}
171+
172+
// Handle MCP wildcards
173+
if (isMcpToolName(toolName)) {
174+
if (toolName === `${MCP_TOOL_PREFIX}*`) {
175+
for (const tool of parentToolRegistry.getAllTools()) {
176+
if (tool instanceof DiscoveredMCPTool) {
177+
registerToolInstance(tool);
178+
}
179+
}
180+
return;
181+
}
182+
183+
const parsed = parseMcpToolName(toolName);
184+
if (parsed.serverName && parsed.toolName === '*') {
185+
for (const tool of parentToolRegistry.getToolsByServer(
186+
parsed.serverName,
187+
)) {
188+
registerToolInstance(tool);
189+
}
190+
return;
191+
}
192+
}
193+
154194
// If the tool is referenced by name, retrieve it from the parent
155195
// registry and register it with the agent's isolated registry.
156196
const tool = parentToolRegistry.getTool(toolName);
157197
if (tool) {
158-
if (tool instanceof DiscoveredMCPTool) {
159-
// Subagents MUST use fully qualified names for MCP tools to ensure
160-
// unambiguous tool calls and to comply with policy requirements.
161-
// We automatically "upgrade" any MCP tool to its qualified version.
162-
agentToolRegistry.registerTool(tool.asFullyQualifiedTool());
163-
} else {
164-
agentToolRegistry.registerTool(tool);
165-
}
198+
registerToolInstance(tool);
166199
}
167200
};
168201

@@ -1171,22 +1204,14 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
11711204
const { toolConfig, outputConfig } = this.definition;
11721205

11731206
if (toolConfig) {
1174-
const toolNamesToLoad: string[] = [];
11751207
for (const toolRef of toolConfig.tools) {
1176-
if (typeof toolRef === 'string') {
1177-
toolNamesToLoad.push(toolRef);
1178-
} else if (typeof toolRef === 'object' && 'schema' in toolRef) {
1179-
// Tool instance with an explicit schema property.
1180-
toolsList.push(toolRef.schema);
1181-
} else {
1208+
if (typeof toolRef === 'object' && !('schema' in toolRef)) {
11821209
// Raw `FunctionDeclaration` object.
11831210
toolsList.push(toolRef);
11841211
}
11851212
}
1186-
// Add schemas from tools that were registered by name.
1187-
toolsList.push(
1188-
...this.toolRegistry.getFunctionDeclarationsFiltered(toolNamesToLoad),
1189-
);
1213+
// Add schemas from tools that were explicitly registered by name, wildcard, or instance.
1214+
toolsList.push(...this.toolRegistry.getFunctionDeclarations());
11901215
}
11911216

11921217
// Always inject complete_task.

packages/core/src/tools/mcp-tool.ts

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export function parseMcpToolName(name: string): {
5858
// Remove the prefix
5959
const withoutPrefix = name.slice(MCP_TOOL_PREFIX.length);
6060
// The first segment is the server name, the rest is the tool name
61+
// Must be strictly `server_tool` where neither are empty
6162
const match = withoutPrefix.match(/^([^_]+)_(.+)$/);
6263
if (match) {
6364
return {
@@ -390,25 +391,6 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool<
390391
`${this.serverName}${MCP_QUALIFIED_NAME_SEPARATOR}${this.serverToolName}`,
391392
);
392393
}
393-
394-
asFullyQualifiedTool(): DiscoveredMCPTool {
395-
return new DiscoveredMCPTool(
396-
this.mcpTool,
397-
this.serverName,
398-
this.serverToolName,
399-
this.description,
400-
this.parameterSchema,
401-
this.messageBus,
402-
this.trust,
403-
this.isReadOnly,
404-
this.getFullyQualifiedName(),
405-
this.cliConfig,
406-
this.extensionName,
407-
this.extensionId,
408-
this._toolAnnotations,
409-
);
410-
}
411-
412394
protected createInvocation(
413395
params: ToolParams,
414396
messageBus: MessageBus,

packages/core/src/tools/tool-names.test.ts

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ vi.mock('./tool-names.js', async (importOriginal) => {
2525
...actual,
2626
TOOL_LEGACY_ALIASES: mockedAliases,
2727
isValidToolName: vi.fn().mockImplementation((name: string, options) => {
28-
if (mockedAliases[name]) return true;
28+
if (Object.prototype.hasOwnProperty.call(mockedAliases, name))
29+
return true;
2930
return actual.isValidToolName(name, options);
3031
}),
3132
getToolAliases: vi.fn().mockImplementation((name: string) => {
@@ -55,11 +56,9 @@ describe('tool-names', () => {
5556
expect(isValidToolName(`${DISCOVERED_TOOL_PREFIX}my_tool`)).toBe(true);
5657
});
5758

58-
it('should validate MCP tool names (server__tool)', () => {
59-
expect(isValidToolName('server__tool')).toBe(true);
60-
expect(isValidToolName('my-server__my-tool')).toBe(true);
61-
expect(isValidToolName('my.server__my:tool')).toBe(true);
62-
expect(isValidToolName('my-server...truncated__tool')).toBe(true);
59+
it('should validate modern MCP FQNs (mcp_server_tool)', () => {
60+
expect(isValidToolName('mcp_server_tool')).toBe(true);
61+
expect(isValidToolName('mcp_my-server_my-tool')).toBe(true);
6362
});
6463

6564
it('should validate legacy tool aliases', async () => {
@@ -69,28 +68,33 @@ describe('tool-names', () => {
6968
}
7069
});
7170

72-
it('should reject invalid tool names', () => {
73-
expect(isValidToolName('')).toBe(false);
74-
expect(isValidToolName('invalid-name')).toBe(false);
75-
expect(isValidToolName('server__')).toBe(false);
76-
expect(isValidToolName('__tool')).toBe(false);
77-
expect(isValidToolName('server__tool__extra')).toBe(false);
71+
it('should return false for invalid tool names', () => {
72+
expect(isValidToolName('invalid-tool-name')).toBe(false);
73+
expect(isValidToolName('mcp_server')).toBe(false);
74+
expect(isValidToolName('mcp__tool')).toBe(false);
75+
expect(isValidToolName('mcp_invalid server_tool')).toBe(false);
76+
expect(isValidToolName('mcp_server_invalid tool')).toBe(false);
77+
expect(isValidToolName('mcp_server_')).toBe(false);
7878
});
7979

8080
it('should handle wildcards when allowed', () => {
8181
// Default: not allowed
8282
expect(isValidToolName('*')).toBe(false);
83-
expect(isValidToolName('server__*')).toBe(false);
83+
expect(isValidToolName('mcp_*')).toBe(false);
84+
expect(isValidToolName('mcp_server_*')).toBe(false);
8485

8586
// Explicitly allowed
8687
expect(isValidToolName('*', { allowWildcards: true })).toBe(true);
87-
expect(isValidToolName('server__*', { allowWildcards: true })).toBe(true);
88+
expect(isValidToolName('mcp_*', { allowWildcards: true })).toBe(true);
89+
expect(isValidToolName('mcp_server_*', { allowWildcards: true })).toBe(
90+
true,
91+
);
8892

8993
// Invalid wildcards
90-
expect(isValidToolName('__*', { allowWildcards: true })).toBe(false);
91-
expect(isValidToolName('server__tool*', { allowWildcards: true })).toBe(
92-
false,
93-
);
94+
expect(isValidToolName('mcp__*', { allowWildcards: true })).toBe(false);
95+
expect(
96+
isValidToolName('mcp_server_tool*', { allowWildcards: true }),
97+
).toBe(false);
9498
});
9599
});
96100

packages/core/src/tools/tool-names.ts

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,12 @@ export const DISCOVERED_TOOL_PREFIX = 'discovered_tool_';
221221
/**
222222
* List of all built-in tool names.
223223
*/
224+
import {
225+
isMcpToolName,
226+
parseMcpToolName,
227+
MCP_TOOL_PREFIX,
228+
} from './mcp-tool.js';
229+
224230
export const ALL_BUILTIN_TOOL_NAMES = [
225231
GLOB_TOOL_NAME,
226232
WRITE_TODOS_TOOL_NAME,
@@ -290,25 +296,44 @@ export function isValidToolName(
290296
return true;
291297
}
292298

293-
// MCP tools (format: server__tool)
294-
if (name.includes('__')) {
295-
const parts = name.split('__');
296-
if (parts.length !== 2 || parts[0].length === 0 || parts[1].length === 0) {
299+
// Handle standard MCP FQNs (mcp_server_tool or wildcards mcp_*, mcp_server_*)
300+
if (isMcpToolName(name)) {
301+
// Global wildcard: mcp_*
302+
if (name === `${MCP_TOOL_PREFIX}*` && options.allowWildcards) {
303+
return true;
304+
}
305+
306+
// Explicitly reject names with empty server component (e.g. mcp__tool)
307+
if (name.startsWith(`${MCP_TOOL_PREFIX}_`)) {
297308
return false;
298309
}
299310

300-
const server = parts[0];
301-
const tool = parts[1];
311+
const parsed = parseMcpToolName(name);
312+
// Ensure that both components are populated. parseMcpToolName splits at the second _,
313+
// so `mcp__tool` has serverName="", toolName="tool"
314+
if (parsed.serverName && parsed.toolName) {
315+
// Basic slug validation for server and tool names.
316+
// We allow dots (.) and colons (:) as they are valid in function names and
317+
// used for truncation markers.
318+
const slugRegex = /^[a-z0-9_.:-]+$/i;
319+
320+
if (!slugRegex.test(parsed.serverName)) {
321+
return false;
322+
}
323+
324+
if (parsed.toolName === '*') {
325+
return options.allowWildcards === true;
326+
}
327+
328+
// A tool name consisting only of underscores is invalid.
329+
if (/^_*$/.test(parsed.toolName)) {
330+
return false;
331+
}
302332

303-
if (tool === '*') {
304-
return !!options.allowWildcards;
333+
return slugRegex.test(parsed.toolName);
305334
}
306335

307-
// Basic slug validation for server and tool names.
308-
// We allow dots (.) and colons (:) as they are valid in function names and
309-
// used for truncation markers.
310-
const slugRegex = /^[a-z0-9_.:-]+$/i;
311-
return slugRegex.test(server) && slugRegex.test(tool);
336+
return false;
312337
}
313338

314339
return false;

packages/core/src/tools/tool-registry.test.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,13 +310,13 @@ describe('ToolRegistry', () => {
310310
excludedTools: ['tool-a'],
311311
},
312312
{
313-
name: 'should match simple MCP tool names, when qualified or unqualified',
314-
tools: [mcpTool, mcpTool.asFullyQualifiedTool()],
313+
name: 'should match simple MCP tool names',
314+
tools: [mcpTool],
315315
excludedTools: [mcpTool.name],
316316
},
317317
{
318-
name: 'should match qualified MCP tool names when qualified or unqualified',
319-
tools: [mcpTool, mcpTool.asFullyQualifiedTool()],
318+
name: 'should match qualified MCP tool names',
319+
tools: [mcpTool],
320320
excludedTools: [mcpTool.name],
321321
},
322322
{
@@ -414,9 +414,9 @@ describe('ToolRegistry', () => {
414414
const toolName = 'my-tool';
415415
const mcpTool = createMCPTool(serverName, toolName, 'desc');
416416

417-
// Register same MCP tool twice (one as alias, one as qualified)
417+
// Register same MCP tool twice
418+
toolRegistry.registerTool(mcpTool);
418419
toolRegistry.registerTool(mcpTool);
419-
toolRegistry.registerTool(mcpTool.asFullyQualifiedTool());
420420

421421
const toolNames = toolRegistry.getAllToolNames();
422422
expect(toolNames).toEqual([`mcp_${serverName}_${toolName}`]);
@@ -698,9 +698,8 @@ describe('ToolRegistry', () => {
698698
const toolName = 'my-tool';
699699
const mcpTool = createMCPTool(serverName, toolName, 'description');
700700

701-
// Register both alias and qualified
702701
toolRegistry.registerTool(mcpTool);
703-
toolRegistry.registerTool(mcpTool.asFullyQualifiedTool());
702+
toolRegistry.registerTool(mcpTool);
704703

705704
const declarations = toolRegistry.getFunctionDeclarations();
706705
expect(declarations).toHaveLength(1);

0 commit comments

Comments
 (0)