feat(tool): sort tool list alphabetically for deterministic output (#3095)

Co-authored-by: Pascal Birchler <pascalb@google.com>
This commit is contained in:
Sambhav Khanna 2025-07-15 14:35:35 -04:00 committed by GitHub
parent d3ee9de3c3
commit 40c4070846
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 69 additions and 12 deletions

View File

@ -104,8 +104,12 @@ const createMockCallableTool = (
}); });
class MockTool extends BaseTool<{ param: string }, ToolResult> { class MockTool extends BaseTool<{ param: string }, ToolResult> {
constructor(name = 'mock-tool', description = 'A mock tool') { constructor(
super(name, name, description, { name = 'mock-tool',
displayName = 'A mock tool',
description = 'A mock tool description',
) {
super(name, displayName, description, {
type: Type.OBJECT, type: Type.OBJECT,
properties: { properties: {
param: { type: Type.STRING }, param: { type: Type.STRING },
@ -174,42 +178,93 @@ describe('ToolRegistry', () => {
}); });
}); });
describe('getAllTools', () => {
it('should return all registered tools sorted alphabetically by displayName', () => {
// Register tools with displayNames in non-alphabetical order
const toolC = new MockTool('c-tool', 'Tool C');
const toolA = new MockTool('a-tool', 'Tool A');
const toolB = new MockTool('b-tool', 'Tool B');
toolRegistry.registerTool(toolC);
toolRegistry.registerTool(toolA);
toolRegistry.registerTool(toolB);
const allTools = toolRegistry.getAllTools();
const displayNames = allTools.map((t) => t.displayName);
// Assert that the returned array is sorted by displayName
expect(displayNames).toEqual(['Tool A', 'Tool B', 'Tool C']);
});
});
describe('getToolsByServer', () => { describe('getToolsByServer', () => {
it('should return an empty array if no tools match the server name', () => { it('should return an empty array if no tools match the server name', () => {
toolRegistry.registerTool(new MockTool()); toolRegistry.registerTool(new MockTool());
expect(toolRegistry.getToolsByServer('any-mcp-server')).toEqual([]); expect(toolRegistry.getToolsByServer('any-mcp-server')).toEqual([]);
}); });
it('should return only tools matching the server name', async () => { it('should return only tools matching the server name, sorted by name', async () => {
const server1Name = 'mcp-server-uno'; const server1Name = 'mcp-server-uno';
const server2Name = 'mcp-server-dos'; const server2Name = 'mcp-server-dos';
const mockCallable = {} as CallableTool; const mockCallable = {} as CallableTool;
const mcpTool1 = new DiscoveredMCPTool( const mcpTool1_c = new DiscoveredMCPTool(
mockCallable, mockCallable,
server1Name, server1Name,
'server1Name__tool-on-server1', `${server1Name}__zebra-tool`,
'd1', 'd1',
{}, {},
'tool-on-server1', 'zebra-tool',
); );
const mcpTool1_a = new DiscoveredMCPTool(
mockCallable,
server1Name,
`${server1Name}__apple-tool`,
'd2',
{},
'apple-tool',
);
const mcpTool1_b = new DiscoveredMCPTool(
mockCallable,
server1Name,
`${server1Name}__banana-tool`,
'd3',
{},
'banana-tool',
);
const mcpTool2 = new DiscoveredMCPTool( const mcpTool2 = new DiscoveredMCPTool(
mockCallable, mockCallable,
server2Name, server2Name,
'server2Name__tool-on-server2', 'server2Name__tool-on-server2',
'd2', 'd4',
{}, {},
'tool-on-server2', 'tool-on-server2',
); );
const nonMcpTool = new MockTool('regular-tool'); const nonMcpTool = new MockTool('regular-tool');
toolRegistry.registerTool(mcpTool1); toolRegistry.registerTool(mcpTool1_c);
toolRegistry.registerTool(mcpTool1_a);
toolRegistry.registerTool(mcpTool1_b);
toolRegistry.registerTool(mcpTool2); toolRegistry.registerTool(mcpTool2);
toolRegistry.registerTool(nonMcpTool); toolRegistry.registerTool(nonMcpTool);
const toolsFromServer1 = toolRegistry.getToolsByServer(server1Name); const toolsFromServer1 = toolRegistry.getToolsByServer(server1Name);
expect(toolsFromServer1).toHaveLength(1); const toolNames = toolsFromServer1.map((t) => t.name);
expect(toolsFromServer1[0].name).toBe(mcpTool1.name);
// Assert that the array has the correct tools and is sorted by name
expect(toolsFromServer1).toHaveLength(3);
expect(toolNames).toEqual([
`${server1Name}__apple-tool`,
`${server1Name}__banana-tool`,
`${server1Name}__zebra-tool`,
]);
// Assert that all returned tools are indeed from the correct server
for (const tool of toolsFromServer1) {
expect((tool as DiscoveredMCPTool).serverName).toBe(server1Name);
}
// Assert that the other server's tools are returned correctly
const toolsFromServer2 = toolRegistry.getToolsByServer(server2Name); const toolsFromServer2 = toolRegistry.getToolsByServer(server2Name);
expect(toolsFromServer2).toHaveLength(1); expect(toolsFromServer2).toHaveLength(1);
expect(toolsFromServer2[0].name).toBe(mcpTool2.name); expect(toolsFromServer2[0].name).toBe(mcpTool2.name);

View File

@ -308,7 +308,9 @@ export class ToolRegistry {
* Returns an array of all registered and discovered tool instances. * Returns an array of all registered and discovered tool instances.
*/ */
getAllTools(): Tool[] { getAllTools(): Tool[] {
return Array.from(this.tools.values()); return Array.from(this.tools.values()).sort((a, b) =>
a.displayName.localeCompare(b.displayName),
);
} }
/** /**
@ -321,7 +323,7 @@ export class ToolRegistry {
serverTools.push(tool); serverTools.push(tool);
} }
} }
return serverTools; return serverTools.sort((a, b) => a.name.localeCompare(b.name));
} }
/** /**