chore: disallow overriding of internal tool impl

Author: himanshusinghs

src/server.ts

@@ -31,11 +31,11 @@ export interface ServerOptions {
     elicitation: Elicitation;
     connectionErrorHandler: ConnectionErrorHandler;
     /**
-     * Custom tool constructors to register with the server.
-     * This will override any default tools. You can use both existing and custom tools by using the `mongodb-mcp-server/tools` export.
+     * Custom tool constructors to register with the server. The tools provided
+     * here will be registered in addition to the default tools.
      *
      * ```ts
-     * import { AllTools, ToolBase } from "mongodb-mcp-server/tools";
+     * import { ToolBase } from "mongodb-mcp-server/tools";
      * class CustomTool extends ToolBase {
      *     name = "custom_tool";
      *     // ...
@@ -47,11 +47,11 @@ export interface ServerOptions {
      *     telemetry: myTelemetry,
      *     elicitation: myElicitation,
      *     connectionErrorHandler: myConnectionErrorHandler,
-     *     tools: [...AllTools, CustomTool],
+     *     additionalTools: [CustomTool],
      * });
      * ```
      */
-    tools?: (new (params: ToolConstructorParams) => ToolBase)[];
+    additionalTools?: (new (params: ToolConstructorParams) => ToolBase)[];
 }
 
 export class Server {
@@ -60,7 +60,8 @@ export class Server {
     private readonly telemetry: Telemetry;
     public readonly userConfig: UserConfig;
     public readonly elicitation: Elicitation;
-    private readonly toolConstructors: (new (params: ToolConstructorParams) => ToolBase)[];
+    private readonly internalToolImplementations: (new (params: ToolConstructorParams) => ToolBase)[] = AllTools;
+    private readonly additionalToolImplementations: (new (params: ToolConstructorParams) => ToolBase)[];
     public readonly tools: ToolBase[] = [];
     public readonly connectionErrorHandler: ConnectionErrorHandler;
 
@@ -80,7 +81,7 @@ export class Server {
         telemetry,
         connectionErrorHandler,
         elicitation,
-        tools,
+        additionalTools,
     }: ServerOptions) {
         this.startTime = Date.now();
         this.session = session;
@@ -89,7 +90,7 @@ export class Server {
         this.userConfig = userConfig;
         this.elicitation = elicitation;
         this.connectionErrorHandler = connectionErrorHandler;
-        this.toolConstructors = tools ?? AllTools;
+        this.additionalToolImplementations = additionalTools ?? [];
     }
 
     async connect(transport: Transport): Promise<void> {
@@ -240,15 +241,37 @@ export class Server {
     }
 
     private registerTools(): void {
-        for (const toolConstructor of this.toolConstructors) {
+        const toolImplementations = [
+            ...this.internalToolImplementations.map((toolConstructor) => ({
+                toolConstructor,
+                source: "internal",
+            })),
+            ...this.additionalToolImplementations.map((toolConstructor) => ({
+                toolConstructor,
+                source: "additional",
+            })),
+        ] as const;
+        const registeredTools: Set<string> = new Set();
+
+        for (const { source, toolConstructor } of toolImplementations) {
             const tool = new toolConstructor({
                 session: this.session,
                 config: this.userConfig,
                 telemetry: this.telemetry,
                 elicitation: this.elicitation,
             });
+
+            if (registeredTools.has(tool.name)) {
+                throw new Error(
+                    source === "internal"
+                        ? `Tool name collision detected for internal tool - '${tool.name}'`
+                        : `Tool name collision detected for additional tool - '${tool.name}'. Cannot register an additional tool with the same name as that of an internal tool.`
+                );
+            }
+
             if (tool.register(this)) {
                 this.tools.push(tool);
+                registeredTools.add(tool.name);
             }
         }
     }

src/transports/base.ts

@@ -40,7 +40,7 @@ export type TransportRunnerConfig = {
     createAtlasLocalClient?: AtlasLocalClientFactoryFn;
     additionalLoggers?: LoggerBase[];
     telemetryProperties?: Partial<CommonProperties>;
-    tools?: (new (params: ToolConstructorParams) => ToolBase)[];
+    additionalTools?: (new (params: ToolConstructorParams) => ToolBase)[];
     /**
      * Hook which allows library consumers to fetch configuration from external sources (e.g., secrets managers, APIs)
      * or modify the existing configuration before the session is created.
@@ -56,7 +56,7 @@ export abstract class TransportRunnerBase {
     private readonly connectionErrorHandler: ConnectionErrorHandler;
     private readonly atlasLocalClient: Promise<Client | undefined>;
     private readonly telemetryProperties: Partial<CommonProperties>;
-    private readonly tools?: (new (params: ToolConstructorParams) => ToolBase)[];
+    private readonly additionalTools?: (new (params: ToolConstructorParams) => ToolBase)[];
     private readonly createSessionConfig?: CreateSessionConfigFn;
 
     protected constructor({
@@ -66,15 +66,15 @@ export abstract class TransportRunnerBase {
         createAtlasLocalClient = defaultCreateAtlasLocalClient,
         additionalLoggers = [],
         telemetryProperties = {},
-        tools,
+        additionalTools,
         createSessionConfig,
     }: TransportRunnerConfig) {
         this.userConfig = userConfig;
         this.createConnectionManager = createConnectionManager;
         this.connectionErrorHandler = connectionErrorHandler;
         this.atlasLocalClient = createAtlasLocalClient();
         this.telemetryProperties = telemetryProperties;
-        this.tools = tools;
+        this.additionalTools = additionalTools;
         this.createSessionConfig = createSessionConfig;
         const loggers: LoggerBase[] = [...additionalLoggers];
         if (this.userConfig.loggers.includes("stderr")) {
@@ -149,7 +149,7 @@ export abstract class TransportRunnerBase {
             userConfig,
             connectionErrorHandler: this.connectionErrorHandler,
             elicitation,
-            tools: this.tools,
+            additionalTools: this.additionalTools,
         });
 
         // We need to create the MCP logger after the server is constructed

tests/integration/customTools.test.ts

@@ -8,11 +8,11 @@ import { defaultTestConfig, setupIntegrationTest } from "./helpers.js";
 describe("Custom Tools", () => {
     const { mcpClient, mcpServer } = setupIntegrationTest(() => ({ ...defaultTestConfig }), {
         serverOptions: {
-            tools: [CustomGreetingTool, CustomCalculatorTool],
+            additionalTools: [CustomGreetingTool, CustomCalculatorTool],
         },
     });
 
-    it("should register custom tools instead of default tools", async () => {
+    it("should register custom tools in addition to default tools", async () => {
         // Check that custom tools are registered
         const tools = await mcpClient().listTools();
         const customGreetingTool = tools.tools.find((t) => t.name === "custom_greeting");
@@ -21,9 +21,9 @@ describe("Custom Tools", () => {
         expect(customGreetingTool).toBeDefined();
         expect(customCalculatorTool).toBeDefined();
 
-        // Check that default tools are NOT registered since we only provided custom tools
+        // Check that default tools are also registered
         const defaultTool = tools.tools.find((t) => t.name === "list-databases");
-        expect(defaultTool).toBeUndefined();
+        expect(defaultTool).toBeDefined();
     });
 
     it("should execute custom tools", async () => {

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: [

tests/integration/tools/mongodb/mongodbTool.test.ts

@@ -19,7 +19,6 @@ import { setupMongoDBIntegrationTest } from "./mongodbHelpers.js";
 import { ErrorCodes } from "../../../../src/common/errors.js";
 import { Keychain } from "../../../../src/common/keychain.js";
 import { Elicitation } from "../../../../src/elicitation.js";
-import { MongoDbTools } from "../../../../src/tools/mongodb/tools.js";
 import { VectorSearchEmbeddingsManager } from "../../../../src/common/search/vectorSearchEmbeddingsManager.js";
 
 const injectedErrorHandler: ConnectionErrorHandler = (error) => {
@@ -89,7 +88,7 @@ describe("MongoDBTool implementations", () => {
 
     async function cleanupAndStartServer(
         config: Partial<UserConfig> | undefined = {},
-        toolConstructors: (new (params: ToolConstructorParams) => ToolBase)[] = [...MongoDbTools, RandomTool],
+        additionalTools: (new (params: ToolConstructorParams) => ToolBase)[] = [RandomTool],
         errorHandler: ConnectionErrorHandler | undefined = connectionErrorHandler
     ): Promise<void> {
         await cleanup();
@@ -140,7 +139,7 @@ describe("MongoDBTool implementations", () => {
             mcpServer: internalMcpServer,
             connectionErrorHandler: errorHandler,
             elicitation,
-            tools: toolConstructors,
+            additionalTools,
         });
 
         await mcpServer.connect(serverTransport);
@@ -237,7 +236,7 @@ describe("MongoDBTool implementations", () => {
 
     describe("when MCP is using injected connection error handler", () => {
         beforeEach(async () => {
-            await cleanupAndStartServer(defaultTestConfig, [...MongoDbTools, RandomTool], injectedErrorHandler);
+            await cleanupAndStartServer(defaultTestConfig, [RandomTool], injectedErrorHandler);
         });
 
         describe("and comes across a MongoDB Error - NotConnectedToMongoDB", () => {
@@ -263,7 +262,7 @@ describe("MongoDBTool implementations", () => {
                 // This is a misconfigured connection string
                 await cleanupAndStartServer(
                     { connectionString: "mongodb://localhost:1234" },
-                    [...MongoDbTools, RandomTool],
+                    [RandomTool],
                     injectedErrorHandler
                 );
                 const toolResponse = await mcpClient?.callTool({
@@ -287,7 +286,7 @@ describe("MongoDBTool implementations", () => {
                 // This is a misconfigured connection string
                 await cleanupAndStartServer(
                     { connectionString: mdbIntegration.connectionString(), indexCheck: true },
-                    [...MongoDbTools, RandomTool],
+                    [RandomTool],
                     injectedErrorHandler
                 );
                 const toolResponse = await mcpClient?.callTool({
@@ -318,11 +317,28 @@ describe("MongoDBTool implementations", () => {
                 injectedErrorHandler
             );
             const tools = await mcpClient?.listTools({});
-            expect(tools?.tools).toHaveLength(1);
+            expect(tools?.tools.find((tool) => tool.name === "Random")).toBeDefined();
             expect(tools?.tools.find((tool) => tool.name === "UnusableVoyageTool")).toBeUndefined();
         });
     });
 
+    describe("when an external tool with same name as that of internal tool is attempted to be registered", () => {
+        it("server should throw during initialization itself", async () => {
+            const serverStartPromise = cleanupAndStartServer(
+                { connectionString: mdbIntegration.connectionString(), indexCheck: true },
+                [
+                    class SimilarRandomTool extends RandomTool {
+                        override name = "find";
+                    },
+                ],
+                injectedErrorHandler
+            );
+            await expect(serverStartPromise).rejects.toThrow(
+                "Tool name collision detected for additional tool - 'find'. Cannot register an additional tool with the same name as that of an internal tool."
+            );
+        });
+    });
+
     describe("resolveTelemetryMetadata", () => {
         it("should return empty metadata when not connected", async () => {
             await cleanupAndStartServer();