From ea1a6384e177667a30bd5970a2e92fc14fc2c224 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 26 Nov 2025 18:04:48 +0100 Subject: [PATCH 1/2] chore: disallow overriding of internal tool impl --- src/server.ts | 41 +++++++++++++++---- src/transports/base.ts | 10 ++--- tests/integration/customTools.test.ts | 8 ++-- .../tools/mongodb/mongodbTool.test.ts | 30 ++++++++++---- 4 files changed, 64 insertions(+), 25 deletions(-) diff --git a/src/server.ts b/src/server.ts index 3c99b376a..3579c347e 100644 --- a/src/server.ts +++ b/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 { @@ -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 = 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); } } } diff --git a/src/transports/base.ts b/src/transports/base.ts index 787df80a8..70e0f02b1 100644 --- a/src/transports/base.ts +++ b/src/transports/base.ts @@ -40,7 +40,7 @@ export type TransportRunnerConfig = { createAtlasLocalClient?: AtlasLocalClientFactoryFn; additionalLoggers?: LoggerBase[]; telemetryProperties?: Partial; - 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; private readonly telemetryProperties: Partial; - private readonly tools?: (new (params: ToolConstructorParams) => ToolBase)[]; + private readonly additionalTools?: (new (params: ToolConstructorParams) => ToolBase)[]; private readonly createSessionConfig?: CreateSessionConfigFn; protected constructor({ @@ -66,7 +66,7 @@ export abstract class TransportRunnerBase { createAtlasLocalClient = defaultCreateAtlasLocalClient, additionalLoggers = [], telemetryProperties = {}, - tools, + additionalTools, createSessionConfig, }: TransportRunnerConfig) { this.userConfig = userConfig; @@ -74,7 +74,7 @@ export abstract class TransportRunnerBase { 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 diff --git a/tests/integration/customTools.test.ts b/tests/integration/customTools.test.ts index 955999d35..09835c9f2 100644 --- a/tests/integration/customTools.test.ts +++ b/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 () => { diff --git a/tests/integration/tools/mongodb/mongodbTool.test.ts b/tests/integration/tools/mongodb/mongodbTool.test.ts index 12d374699..1fa129a10 100644 --- a/tests/integration/tools/mongodb/mongodbTool.test.ts +++ b/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 | undefined = {}, - toolConstructors: (new (params: ToolConstructorParams) => ToolBase)[] = [...MongoDbTools, RandomTool], + additionalTools: (new (params: ToolConstructorParams) => ToolBase)[] = [RandomTool], errorHandler: ConnectionErrorHandler | undefined = connectionErrorHandler ): Promise { 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(); From 776c8e3dd8f34afcae0e05a7ddb761cfb75c1217 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 27 Nov 2025 13:41:18 +0100 Subject: [PATCH 2/2] chore: updated exports for library --- src/lib.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/lib.ts b/src/lib.ts index babdbde77..3b05eb54f 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -1,6 +1,7 @@ export { Server, type ServerOptions } from "./server.js"; export { Session, type SessionOptions } from "./common/session.js"; -export { type UserConfig } from "./common/config/userConfig.js"; +export { type UserConfig, UserConfigSchema } from "./common/config/userConfig.js"; +export { createUserConfig as parseCliArgumentsAsUserConfig } from "./common/config/createUserConfig.js"; export { LoggerBase, type LogPayload, type LoggerType, type LogLevel } from "./common/logger.js"; export { StreamableHttpRunner } from "./transports/streamableHttp.js"; export { StdioRunner } from "./transports/stdio.js"; @@ -8,19 +9,21 @@ export { TransportRunnerBase, type TransportRunnerConfig } from "./transports/ba export { ConnectionManager, ConnectionStateConnected, + createMCPConnectionManager, type AnyConnectionState, type ConnectionState, type ConnectionStateDisconnected, type ConnectionStateErrored, type ConnectionManagerFactoryFn, } from "./common/connectionManager.js"; -export type { - ConnectionErrorHandler, - ConnectionErrorHandled, - ConnectionErrorUnhandled, - ConnectionErrorHandlerContext, +export { + connectionErrorHandler, + type ConnectionErrorHandler, + type ConnectionErrorHandled, + type ConnectionErrorUnhandled, + type ConnectionErrorHandlerContext, } from "./common/connectionErrorHandler.js"; -export { ErrorCodes } from "./common/errors.js"; +export { ErrorCodes, MongoDBError } from "./common/errors.js"; export { Telemetry } from "./telemetry/telemetry.js"; export { Keychain, registerGlobalSecretToRedact } from "./common/keychain.js"; export type { Secret } from "./common/keychain.js";