chore: prevent against tool name collision

This commit ensures that when toolMetadataOverrides accidentally lead to
tool name collision then we error out before even starting the
server.

Author: himanshusinghs

src/common/config/argsParserOptions.ts

@@ -55,6 +55,11 @@ export const OPTIONS = {
         "exportTimeoutMs",
         "exportCleanupIntervalMs",
         "voyageApiKey",
+        // Note: toolMetadataOverrides expects a JSON object but we mention it
+        // as a string only to let yargs-parser know that this key needs to be
+        // parsed. The internal handling of the key as an object is done within
+        // yargs-parser itself.
+        "toolMetadataOverrides",
     ],
     boolean: [
         "apiDeprecationErrors",

src/server.ts

@@ -241,13 +241,27 @@ export class Server {
     }
 
     private registerTools(): void {
+        const toolsToRegister: ToolBase[] = [];
+        const usedNames = new Set<string>();
+
         for (const toolConstructor of this.toolConstructors) {
             const tool = new toolConstructor({
                 session: this.session,
                 config: this.userConfig,
                 telemetry: this.telemetry,
                 elicitation: this.elicitation,
             });
+
+            if (usedNames.has(tool.name)) {
+                throw new Error(
+                    `Tool name collision detected: '${tool.name}'. This might be due to toolMetadataOverrides configuration.`
+                );
+            }
+            usedNames.add(tool.name);
+            toolsToRegister.push(tool);
+        }
+
+        for (const tool of toolsToRegister) {
             if (tool.register(this)) {
                 this.tools.push(tool);
             }

tests/integration/server.test.ts

@@ -1,6 +1,69 @@
-import { defaultTestConfig, expectDefined } from "./helpers.js";
+import { MCPConnectionManager } from "../../src/common/connectionManager.js";
+import { ExportsManager } from "../../src/common/exportsManager.js";
+import { CompositeLogger } from "../../src/common/logger.js";
+import { DeviceId } from "../../src/helpers/deviceId.js";
+import { Session } from "../../src/common/session.js";
+import {
+    defaultTestConfig,
+    expectDefined,
+    validateThrowsForInvalidArguments,
+    validateToolMetadata,
+} from "./helpers.js";
 import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";
 import { describe, expect, it } from "vitest";
+import { Elicitation, Keychain, Telemetry } from "../../src/lib.js";
+import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js";
+import { defaultCreateAtlasLocalClient } from "../../src/common/atlasLocal.js";
+import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
+import { Server } from "../../src/server.js";
+import { connectionErrorHandler } from "../../src/common/connectionErrorHandler.js";
+import { InMemoryTransport } from "./inMemoryTransport.js";
+import type { UserConfig } from "../../src/common/config/userConfig.js";
+import { type OperationType, ToolBase, type ToolCategory } from "../../src/tools/tool.js";
+import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
+import type { TelemetryToolMetadata } from "../../src/telemetry/types.js";
+import { AllTools } from "../../src/tools/index.js";
+class TestToolOne extends ToolBase {
+    public internalName = "test-tool-one";
+    public category: ToolCategory = "mongodb";
+    public operationType: OperationType = "delete";
+    protected internalDescription = "A test tool one for verification tests";
+    protected argsShape = {};
+    protected async execute(): Promise<CallToolResult> {
+        return Promise.resolve({
+            content: [
+                {
+                    type: "text",
+                    text: "Test tool executed successfully",
+                },
+            ],
+        });
+    }
+    protected resolveTelemetryMetadata(): TelemetryToolMetadata {
+        return {};
+    }
+}
+
+class TestToolTwo extends ToolBase {
+    public internalName = "test-tool-two";
+    public category: ToolCategory = "mongodb";
+    public operationType: OperationType = "delete";
+    protected internalDescription = "A test tool two for verification tests";
+    protected argsShape = {};
+    protected async execute(): Promise<CallToolResult> {
+        return Promise.resolve({
+            content: [
+                {
+                    type: "text",
+                    text: "Test tool executed successfully",
+                },
+            ],
+        });
+    }
+    protected resolveTelemetryMetadata(): TelemetryToolMetadata {
+        return {};
+    }
+}
 
 describe("Server integration test", () => {
     describeWithMongoDB(
@@ -97,4 +160,122 @@ describe("Server integration test", () => {
             }),
         }
     );
+
+    describeWithMongoDB(
+        "Tool with overridden metadata",
+        (integration) => {
+            validateToolMetadata(integration, "new-connect", "new connect tool description", [
+                {
+                    name: "connectionString",
+                    description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
+                    type: "string",
+                    required: true,
+                },
+            ]);
+
+            validateThrowsForInvalidArguments(integration, "new-connect", [{}, { connectionString: 123 }]);
+
+            it("should not have overridden connect tool registered", async () => {
+                const { tools } = await integration.mcpClient().listTools();
+                const tool = tools.find((tool) => tool.name === "connect");
+                expect(tool).toBeUndefined();
+            });
+        },
+        {
+            getUserConfig() {
+                return {
+                    ...defaultTestConfig,
+                    toolMetadataOverrides: {
+                        connect: {
+                            name: "new-connect",
+                            description: "new connect tool description",
+                        },
+                    },
+                };
+            },
+        }
+    );
+
+    describe("when toolMetadataOverrides leads to tool name collision", () => {
+        it.each([
+            {
+                config: {
+                    toolMetadataOverrides: {
+                        "list-databases": { name: "my-tool" },
+                        "list-collections": { name: "my-tool" },
+                    },
+                },
+                additionalTools: [],
+            },
+            {
+                config: {
+                    toolMetadataOverrides: {
+                        "list-databases": { name: "list-collections" },
+                    },
+                },
+                additionalTools: [],
+            },
+            {
+                config: {
+                    toolMetadataOverrides: {
+                        "test-tool-one": {
+                            name: "connect",
+                        },
+                    },
+                },
+                additionalTools: [TestToolOne, TestToolTwo],
+            },
+            {
+                config: {
+                    toolMetadataOverrides: {
+                        "test-tool-one": {
+                            name: "test-tool-two",
+                        },
+                    },
+                },
+                additionalTools: [TestToolOne, TestToolTwo],
+            },
+        ] as { config: Partial<UserConfig>; additionalTools: (new () => ToolBase)[] }[])(
+            "should throw an error when tool names collide due to overrides - %",
+            async ({ config, additionalTools }) => {
+                const userConfig: UserConfig = {
+                    ...defaultTestConfig,
+                    ...config,
+                };
+                const logger = new CompositeLogger();
+                const deviceId = DeviceId.create(logger);
+                const connectionManager = new MCPConnectionManager(userConfig, logger, deviceId);
+                const exportsManager = ExportsManager.init(userConfig, logger);
+
+                const session = new Session({
+                    userConfig: userConfig,
+                    logger,
+                    exportsManager,
+                    connectionManager,
+                    keychain: Keychain.root,
+                    vectorSearchEmbeddingsManager: new VectorSearchEmbeddingsManager(userConfig, connectionManager),
+                    atlasLocalClient: await defaultCreateAtlasLocalClient(),
+                });
+
+                const telemetry = Telemetry.create(session, userConfig, deviceId);
+                const mcpServerInstance = new McpServer({ name: "test", version: "1.0" });
+                const elicitation = new Elicitation({ server: mcpServerInstance.server });
+
+                const server = new Server({
+                    session,
+                    userConfig: userConfig,
+                    telemetry,
+                    mcpServer: mcpServerInstance,
+                    elicitation,
+                    connectionErrorHandler,
+                    tools: [...AllTools, ...additionalTools],
+                });
+
+                const transport = new InMemoryTransport();
+
+                // We expect this to fail with our new guardrail
+                await expect(server.connect(transport)).rejects.toThrow(/Tool name collision detected/);
+            }
+        );
+    });
 });

tests/integration/tools/mongodb/create/createIndex.test.ts

@@ -510,7 +510,7 @@ describeWithMongoDB(
                 expect(indexes).toHaveLength(1);
                 expect(indexes[0]?.name).toEqual("vector_1_vector");
                 expect(indexes[0]?.type).toEqual("vectorSearch");
-                expect(indexes[0]?.status).toEqual("PENDING");
+                expect(indexes[0]?.status).toEqual(expect.stringMatching(/PENDING|BUILDING/));
                 expect(indexes[0]?.queryable).toEqual(false);
                 expect(indexes[0]?.latestDefinition).toEqual({
                     fields: [