Skip to content

Commit 9cf4e2a

Browse files
committed
addressing christian's comments
1 parent 5bd66a0 commit 9cf4e2a

4 files changed

Lines changed: 12 additions & 43 deletions

File tree

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
162162
return;
163163
}
164164

165-
if (tool instanceof DiscoveredMCPTool) {
166-
// Subagents MUST use fully qualified names for MCP tools to ensure
167-
// unambiguous tool calls and to comply with policy requirements.
168-
// We automatically "upgrade" any MCP tool to its qualified version.
169-
agentToolRegistry.registerTool(tool.asFullyQualifiedTool());
170-
} else {
171-
agentToolRegistry.registerTool(tool);
172-
}
165+
agentToolRegistry.registerTool(tool);
173166
};
174167

175168
const registerToolByName = (toolName: string) => {

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -391,25 +391,6 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool<
391391
`${this.serverName}${MCP_QUALIFIED_NAME_SEPARATOR}${this.serverToolName}`,
392392
);
393393
}
394-
395-
asFullyQualifiedTool(): DiscoveredMCPTool {
396-
return new DiscoveredMCPTool(
397-
this.mcpTool,
398-
this.serverName,
399-
this.serverToolName,
400-
this.description,
401-
this.parameterSchema,
402-
this.messageBus,
403-
this.trust,
404-
this.isReadOnly,
405-
this.getFullyQualifiedName(),
406-
this.cliConfig,
407-
this.extensionName,
408-
this.extensionId,
409-
this._toolAnnotations,
410-
);
411-
}
412-
413394
protected createInvocation(
414395
params: ToolParams,
415396
messageBus: MessageBus,

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);

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -222,14 +222,10 @@ export class ToolRegistry {
222222
*/
223223
registerTool(tool: AnyDeclarativeTool): void {
224224
if (this.allKnownTools.has(tool.name)) {
225-
if (tool instanceof DiscoveredMCPTool) {
226-
tool = tool.asFullyQualifiedTool();
227-
} else {
228-
// Decide on behavior: throw error, log warning, or allow overwrite
229-
debugLogger.warn(
230-
`Tool with name "${tool.name}" is already registered. Overwriting.`,
231-
);
232-
}
225+
// Decide on behavior: throw error, log warning, or allow overwrite
226+
debugLogger.warn(
227+
`Tool with name "${tool.name}" is already registered. Overwriting.`,
228+
);
233229
}
234230
this.allKnownTools.set(tool.name, tool);
235231
}

0 commit comments

Comments
 (0)