From 40c40708464f641b2cd5bee119f951453e92f7bd Mon Sep 17 00:00:00 2001
From: Sambhav Khanna <125531539+sambhavKhanna@users.noreply.github.com>
Date: Tue, 15 Jul 2025 14:35:35 -0400
Subject: [PATCH] feat(tool): sort tool list alphabetically for deterministic
 output (#3095)

Co-authored-by: Pascal Birchler <pascalb@google.com>
---
 packages/core/src/tools/tool-registry.test.ts | 75 ++++++++++++++++---
 packages/core/src/tools/tool-registry.ts      |  6 +-
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts
index 853f6458..38b058ea 100644
--- a/packages/core/src/tools/tool-registry.test.ts
+++ b/packages/core/src/tools/tool-registry.test.ts
@@ -104,8 +104,12 @@ const createMockCallableTool = (
 });
 
 class MockTool extends BaseTool<{ param: string }, ToolResult> {
-  constructor(name = 'mock-tool', description = 'A mock tool') {
-    super(name, name, description, {
+  constructor(
+    name = 'mock-tool',
+    displayName = 'A mock tool',
+    description = 'A mock tool description',
+  ) {
+    super(name, displayName, description, {
       type: Type.OBJECT,
       properties: {
         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', () => {
     it('should return an empty array if no tools match the server name', () => {
       toolRegistry.registerTool(new MockTool());
       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 server2Name = 'mcp-server-dos';
       const mockCallable = {} as CallableTool;
-      const mcpTool1 = new DiscoveredMCPTool(
+      const mcpTool1_c = new DiscoveredMCPTool(
         mockCallable,
         server1Name,
-        'server1Name__tool-on-server1',
+        `${server1Name}__zebra-tool`,
         '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(
         mockCallable,
         server2Name,
         'server2Name__tool-on-server2',
-        'd2',
+        'd4',
         {},
         'tool-on-server2',
       );
       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(nonMcpTool);
 
       const toolsFromServer1 = toolRegistry.getToolsByServer(server1Name);
-      expect(toolsFromServer1).toHaveLength(1);
-      expect(toolsFromServer1[0].name).toBe(mcpTool1.name);
+      const toolNames = toolsFromServer1.map((t) => t.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);
       expect(toolsFromServer2).toHaveLength(1);
       expect(toolsFromServer2[0].name).toBe(mcpTool2.name);
diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts
index 6f9d5ad5..d2303fc9 100644
--- a/packages/core/src/tools/tool-registry.ts
+++ b/packages/core/src/tools/tool-registry.ts
@@ -308,7 +308,9 @@ export class ToolRegistry {
    * Returns an array of all registered and discovered tool instances.
    */
   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);
       }
     }
-    return serverTools;
+    return serverTools.sort((a, b) => a.name.localeCompare(b.name));
   }
 
   /**