From 23c10992e5df84325ddb5616b6cd472afe4cfc05 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 3 Dec 2025 23:12:21 +0100 Subject: [PATCH 1/6] chore: bump mcp SDK, refactor tool arguments This bumps the SDK to the latest versions which patches some security issues. This also simplifies our typing and structure of our arguments so we can have a more consistant and simpler to reason about API across tools. --- package.json | 2 +- pnpm-lock.yaml | 25 +++--- src/elicitation.ts | 4 +- src/tools/atlas/atlasTool.ts | 11 +-- src/tools/atlas/connect/connectCluster.ts | 11 +-- src/tools/atlas/read/getPerformanceAdvisor.ts | 8 +- src/tools/atlasLocal/atlasLocalTool.ts | 20 +++-- .../atlasLocal/connect/connectDeployment.ts | 11 ++- .../atlasLocal/create/createDeployment.ts | 4 +- .../atlasLocal/delete/deleteDeployment.ts | 4 +- src/tools/mongodb/create/insertMany.ts | 9 ++- src/tools/mongodb/mongodbTool.ts | 5 +- src/tools/tool.ts | 81 ++++++++++++------- tests/tsconfig.json | 0 tests/unit/toolBase.test.ts | 38 +++++---- 15 files changed, 129 insertions(+), 104 deletions(-) delete mode 100644 tests/tsconfig.json diff --git a/package.json b/package.json index 0da0abb8a..d8dd28fa1 100644 --- a/package.json +++ b/package.json @@ -115,7 +115,7 @@ "vitest": "^3.2.4" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.22.0", + "@modelcontextprotocol/sdk": "^1.24.2", "@mongodb-js/device-id": "^0.3.1", "@mongodb-js/devtools-proxy-support": "^0.5.3", "@mongosh/arg-parser": "^3.19.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9878f64f1..c614b9ea2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -13,8 +13,8 @@ importers: .: dependencies: '@modelcontextprotocol/sdk': - specifier: ^1.22.0 - version: 1.22.0 + specifier: ^1.24.2 + version: 1.24.2(zod@3.25.76) '@mongodb-js/device-id': specifier: ^0.3.1 version: 0.3.3 @@ -606,11 +606,12 @@ packages: engines: {node: '>=22.7.5'} hasBin: true - '@modelcontextprotocol/sdk@1.22.0': - resolution: {integrity: sha512-VUpl106XVTCpDmTBil2ehgJZjhyLY2QZikzF8NvTXtLRF1CvO5iEE2UNZdVIUer35vFOwMKYeUGbjJtvPWan3g==} + '@modelcontextprotocol/sdk@1.24.2': + resolution: {integrity: sha512-hS/kzSfchqzvUeJUsdiDHi84/kNhLIZaZ6coGQVwbYIelOBbcAwUohUfaQTLa1MvFOK/jbTnGFzraHSFwB7pjQ==} engines: {node: '>=18'} peerDependencies: '@cfworker/json-schema': ^4.1.1 + zod: ^3.25 || ^4.0 peerDependenciesMeta: '@cfworker/json-schema': optional: true @@ -5003,18 +5004,19 @@ snapshots: '@kwsites/promise-deferred@1.1.1': {} - '@modelcontextprotocol/inspector-cli@0.17.2': + '@modelcontextprotocol/inspector-cli@0.17.2(zod@3.25.76)': dependencies: - '@modelcontextprotocol/sdk': 1.22.0 + '@modelcontextprotocol/sdk': 1.24.2(zod@3.25.76) commander: 13.1.0 spawn-rx: 5.1.2 transitivePeerDependencies: - '@cfworker/json-schema' - supports-color + - zod '@modelcontextprotocol/inspector-client@0.17.2': dependencies: - '@modelcontextprotocol/sdk': 1.22.0 + '@modelcontextprotocol/sdk': 1.24.2(zod@3.25.76) '@radix-ui/react-checkbox': 1.3.3(react-dom@18.3.1(react@18.3.1))(react@18.3.1) '@radix-ui/react-dialog': 1.1.15(react-dom@18.3.1(react@18.3.1))(react@18.3.1) '@radix-ui/react-icons': 1.3.2(react@18.3.1) @@ -5047,7 +5049,7 @@ snapshots: '@modelcontextprotocol/inspector-server@0.17.2': dependencies: - '@modelcontextprotocol/sdk': 1.22.0 + '@modelcontextprotocol/sdk': 1.24.2(zod@3.25.76) cors: 2.8.5 express: 5.1.0 shell-quote: 1.8.3 @@ -5062,10 +5064,10 @@ snapshots: '@modelcontextprotocol/inspector@0.17.2(@types/node@24.10.1)(typescript@5.9.3)': dependencies: - '@modelcontextprotocol/inspector-cli': 0.17.2 + '@modelcontextprotocol/inspector-cli': 0.17.2(zod@3.25.76) '@modelcontextprotocol/inspector-client': 0.17.2 '@modelcontextprotocol/inspector-server': 0.17.2 - '@modelcontextprotocol/sdk': 1.22.0 + '@modelcontextprotocol/sdk': 1.24.2(zod@3.25.76) concurrently: 9.2.1 node-fetch: 3.3.2 open: 10.2.0 @@ -5085,7 +5087,7 @@ snapshots: - typescript - utf-8-validate - '@modelcontextprotocol/sdk@1.22.0': + '@modelcontextprotocol/sdk@1.24.2(zod@3.25.76)': dependencies: ajv: 8.17.1 ajv-formats: 3.0.1(ajv@8.17.1) @@ -5096,6 +5098,7 @@ snapshots: eventsource-parser: 3.0.6 express: 5.1.0 express-rate-limit: 7.5.1(express@5.1.0) + jose: 6.1.2 pkce-challenge: 5.0.0 raw-body: 3.0.1 zod: 3.25.76 diff --git a/src/elicitation.ts b/src/elicitation.ts index c3d30d5b9..32707c6b4 100644 --- a/src/elicitation.ts +++ b/src/elicitation.ts @@ -1,4 +1,4 @@ -import type { ElicitRequest } from "@modelcontextprotocol/sdk/types.js"; +import type { ElicitRequestFormParams } from "@modelcontextprotocol/sdk/types.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; export class Elicitation { @@ -37,7 +37,7 @@ export class Elicitation { * The schema for the confirmation question. * TODO: In the future would be good to use Zod 4's toJSONSchema() to generate the schema. */ - public static CONFIRMATION_SCHEMA: ElicitRequest["params"]["requestedSchema"] = { + public static CONFIRMATION_SCHEMA: ElicitRequestFormParams["requestedSchema"] = { type: "object", properties: { confirmation: { diff --git a/src/tools/atlas/atlasTool.ts b/src/tools/atlas/atlasTool.ts index da7488d57..368368882 100644 --- a/src/tools/atlas/atlasTool.ts +++ b/src/tools/atlas/atlasTool.ts @@ -1,4 +1,3 @@ -import type { ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { AtlasMetadata } from "../../telemetry/types.js"; import { ToolBase, type ToolArgs, type ToolCategory } from "../tool.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -83,17 +82,15 @@ For more information on Atlas API access roles, visit: https://www.mongodb.com/d * @returns The tool metadata */ protected resolveTelemetryMetadata( - result: CallToolResult, - ...args: Parameters> + args: ToolArgs, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + { result }: { result: CallToolResult } ): AtlasMetadata { const toolMetadata: AtlasMetadata = {}; - if (!args.length) { - return toolMetadata; - } // Create a typed parser for the exact shape we expect const argsShape = z.object(this.argsShape); - const parsedResult = argsShape.safeParse(args[0]); + const parsedResult = argsShape.safeParse(args); if (!parsedResult.success) { this.session.logger.debug({ diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index bbdecd3a0..ffb3c3115 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -9,7 +9,6 @@ import type { AtlasClusterConnectionInfo } from "../../../common/connectionManag import { getDefaultRoleFromConfig } from "../../../common/atlas/roles.js"; import { AtlasArgs } from "../../args.js"; import type { ConnectionMetadata } from "../../../telemetry/types.js"; -import type { ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js"; const addedIpAccessListMessage = "Note: Your current IP address has been added to the Atlas project's IP access list to enable secure connection."; @@ -33,9 +32,7 @@ export class ConnectClusterTool extends AtlasToolBase { public name = "atlas-connect-cluster"; protected description = "Connect to MongoDB Atlas cluster"; static operationType: OperationType = "connect"; - protected argsShape = { - ...ConnectClusterArgs, - }; + protected argsShape = ConnectClusterArgs; private queryConnection( projectId: string, @@ -321,10 +318,10 @@ export class ConnectClusterTool extends AtlasToolBase { } protected override resolveTelemetryMetadata( - result: CallToolResult, - ...args: Parameters> + args: ToolArgs, + { result }: { result: CallToolResult } ): ConnectionMetadata { - const parentMetadata = super.resolveTelemetryMetadata(result, ...args); + const parentMetadata = super.resolveTelemetryMetadata(args, { result }); const connectionMetadata = this.getConnectionInfoMetadata(); if (connectionMetadata && connectionMetadata.project_id !== undefined) { // delete the project_id from the parent metadata to avoid duplication diff --git a/src/tools/atlas/read/getPerformanceAdvisor.ts b/src/tools/atlas/read/getPerformanceAdvisor.ts index 1f18470b5..71cb525e5 100644 --- a/src/tools/atlas/read/getPerformanceAdvisor.ts +++ b/src/tools/atlas/read/getPerformanceAdvisor.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import { AtlasToolBase } from "../atlasTool.js"; -import type { CallToolResult, ServerNotification, ServerRequest } from "@modelcontextprotocol/sdk/types.js"; +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import type { OperationType, ToolArgs } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; import { @@ -13,7 +13,6 @@ import { SLOW_QUERY_LOGS_COPY, } from "../../../common/atlas/performanceAdvisorUtils.js"; import { AtlasArgs } from "../../args.js"; -import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/protocol.js"; import type { PerfAdvisorToolMetadata } from "../../../telemetry/types.js"; const PerformanceAdvisorOperationType = z.enum([ @@ -134,12 +133,11 @@ export class GetPerformanceAdvisorTool extends AtlasToolBase { } protected override resolveTelemetryMetadata( - result: CallToolResult, args: ToolArgs, - extra: RequestHandlerExtra + { result }: { result: CallToolResult } ): PerfAdvisorToolMetadata { return { - ...super.resolveTelemetryMetadata(result, args, extra), + ...super.resolveTelemetryMetadata(args, { result }), operations: args.operations, }; } diff --git a/src/tools/atlasLocal/atlasLocalTool.ts b/src/tools/atlasLocal/atlasLocalTool.ts index c7ef7b12c..1e87d947a 100644 --- a/src/tools/atlasLocal/atlasLocalTool.ts +++ b/src/tools/atlasLocal/atlasLocalTool.ts @@ -1,7 +1,6 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import type { ToolArgs, ToolCategory } from "../tool.js"; +import type { ToolArgs, ToolCategory, ToolExecutionContext } from "../tool.js"; import { ToolBase } from "../tool.js"; -import type { ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { Client } from "@mongodb-js/atlas-local"; import { LogId } from "../../common/logger.js"; import type { ConnectionMetadata } from "../../telemetry/types.js"; @@ -15,7 +14,11 @@ export abstract class AtlasLocalToolBase extends ToolBase { return this.session.atlasLocalClient !== undefined && super.verifyAllowed(); } - protected async execute(...args: Parameters>): Promise { + protected async execute( + args: ToolArgs, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _context: ToolExecutionContext + ): Promise { const client = this.session.atlasLocalClient; // If the client is not found, throw an error @@ -38,7 +41,7 @@ please log a ticket here: https://github.com/mongodb-js/mongodb-mcp-server/issue }; } - return this.executeWithAtlasLocalClient(client, ...args); + return this.executeWithAtlasLocalClient(args, { client }); } private async lookupDeploymentId(client: Client, containerId: string): Promise { @@ -72,8 +75,8 @@ please log a ticket here: https://github.com/mongodb-js/mongodb-mcp-server/issue } protected abstract executeWithAtlasLocalClient( - client: Client, - ...args: Parameters> + args: ToolArgs, + context: { client: Client } ): Promise; protected handleError( @@ -119,7 +122,10 @@ please log a ticket here: https://github.com/mongodb-js/mongodb-mcp-server/issue return super.handleError(error, args); } - protected resolveTelemetryMetadata(result: CallToolResult): ConnectionMetadata { + protected resolveTelemetryMetadata( + _args: ToolArgs, + { result }: { result: CallToolResult } + ): ConnectionMetadata { const toolMetadata: ConnectionMetadata = {}; // Atlas Local tools set the deployment ID in the result metadata for telemetry diff --git a/src/tools/atlasLocal/connect/connectDeployment.ts b/src/tools/atlasLocal/connect/connectDeployment.ts index e7b75de2a..8fdeed7c9 100644 --- a/src/tools/atlasLocal/connect/connectDeployment.ts +++ b/src/tools/atlasLocal/connect/connectDeployment.ts @@ -14,8 +14,8 @@ export class ConnectDeploymentTool extends AtlasLocalToolBase { }; protected async executeWithAtlasLocalClient( - client: Client, - { deploymentName }: ToolArgs + { deploymentName }: ToolArgs, + { client }: { client: Client } ): Promise { // Get the connection string for the deployment const connectionString = await client.getConnectionString(deploymentName); @@ -36,7 +36,10 @@ export class ConnectDeploymentTool extends AtlasLocalToolBase { }; } - protected override resolveTelemetryMetadata(result: CallToolResult): ConnectionMetadata { - return { ...super.resolveTelemetryMetadata(result), ...this.getConnectionInfoMetadata() }; + protected override resolveTelemetryMetadata( + args: ToolArgs, + { result }: { result: CallToolResult } + ): ConnectionMetadata { + return { ...super.resolveTelemetryMetadata(args, { result }), ...this.getConnectionInfoMetadata() }; } } diff --git a/src/tools/atlasLocal/create/createDeployment.ts b/src/tools/atlasLocal/create/createDeployment.ts index 08e765fa6..d7f4f50e4 100644 --- a/src/tools/atlasLocal/create/createDeployment.ts +++ b/src/tools/atlasLocal/create/createDeployment.ts @@ -13,8 +13,8 @@ export class CreateDeploymentTool extends AtlasLocalToolBase { }; protected async executeWithAtlasLocalClient( - client: Client, - { deploymentName }: ToolArgs + { deploymentName }: ToolArgs, + { client }: { client: Client } ): Promise { const deploymentOptions: CreateDeploymentOptions = { name: deploymentName, diff --git a/src/tools/atlasLocal/delete/deleteDeployment.ts b/src/tools/atlasLocal/delete/deleteDeployment.ts index b29a323b6..ac55520d0 100644 --- a/src/tools/atlasLocal/delete/deleteDeployment.ts +++ b/src/tools/atlasLocal/delete/deleteDeployment.ts @@ -13,8 +13,8 @@ export class DeleteDeploymentTool extends AtlasLocalToolBase { }; protected async executeWithAtlasLocalClient( - client: Client, - { deploymentName }: ToolArgs + { deploymentName }: ToolArgs, + { client }: { client: Client } ): Promise { // Lookup telemetry metadata // We need to lookup the telemetry metadata before deleting the deployment diff --git a/src/tools/mongodb/create/insertMany.ts b/src/tools/mongodb/create/insertMany.ts index e68e97a1f..6e1041e8c 100644 --- a/src/tools/mongodb/create/insertMany.ts +++ b/src/tools/mongodb/create/insertMany.ts @@ -43,13 +43,14 @@ export class InsertManyTool extends MongoDBToolBase { database, collection, documents, - embeddingParameters: providedEmbeddingParameters, + ...conditionalArgs }: ToolArgs): Promise { const provider = await this.ensureConnected(); - const embeddingParameters = this.isFeatureEnabled("search") - ? (providedEmbeddingParameters as z.infer) - : undefined; + let embeddingParameters: z.infer | undefined; + if ("embeddingParameters" in conditionalArgs) { + embeddingParameters = conditionalArgs.embeddingParameters; + } // Process documents to replace raw string values with generated embeddings documents = await this.replaceRawValuesWithEmbeddingsIfNecessary({ diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index e86de9192..888dffdc9 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -7,7 +7,6 @@ import { ErrorCodes, MongoDBError } from "../../common/errors.js"; import { LogId } from "../../common/logger.js"; import type { Server } from "../../server.js"; import type { ConnectionMetadata } from "../../telemetry/types.js"; -import type { ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js"; export const DbOperationArgs = { database: z.string().describe("Database name"), @@ -119,9 +118,9 @@ export abstract class MongoDBToolBase extends ToolBase { */ protected resolveTelemetryMetadata( // eslint-disable-next-line @typescript-eslint/no-unused-vars - _result: CallToolResult, + _args: ToolArgs, // eslint-disable-next-line @typescript-eslint/no-unused-vars - _args: Parameters> + { result }: { result: CallToolResult } ): ConnectionMetadata { return this.getConnectionInfoMetadata(); } diff --git a/src/tools/tool.ts b/src/tools/tool.ts index 5c00cfab8..1efe3192c 100644 --- a/src/tools/tool.ts +++ b/src/tools/tool.ts @@ -1,6 +1,5 @@ -import type { z } from "zod"; -import { type ZodRawShape, type ZodNever } from "zod"; -import type { RegisteredTool, ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js"; +import type { z, ZodRawShape } from "zod"; +import type { RegisteredTool } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { CallToolResult, ToolAnnotations } from "@modelcontextprotocol/sdk/types.js"; import type { Session } from "../common/session.js"; import { LogId } from "../common/logger.js"; @@ -11,10 +10,13 @@ import type { Server } from "../server.js"; import type { Elicitation } from "../elicitation.js"; import type { PreviewFeature } from "../common/schemas.js"; -export type ToolArgs = z.objectOutputType; -export type ToolCallbackArgs = Parameters>; +export type ToolArgs = { + [K in keyof T]: z.infer; +}; -export type ToolExecutionContext = Parameters>[1]; +export type ToolExecutionContext = { + signal: AbortSignal; +}; /** * The type of operation the tool performs. This is used when evaluating if a tool is allowed to run based on @@ -331,7 +333,10 @@ export abstract class ToolBase { * } * ``` */ - protected abstract execute(...args: ToolCallbackArgs): Promise; + protected abstract execute( + args: ToolArgs, + { signal }: ToolExecutionContext + ): Promise; /** * Get the confirmation message shown to users when this tool requires @@ -351,7 +356,7 @@ export abstract class ToolBase { * ``` */ // eslint-disable-next-line @typescript-eslint/no-unused-vars - protected getConfirmationMessage(...args: ToolCallbackArgs): string { + protected getConfirmationMessage(args: ToolArgs): string { return `You are about to execute the \`${this.name}\` tool which requires additional confirmation. Would you like to proceed?`; } @@ -367,12 +372,12 @@ export abstract class ToolBase { * @returns A promise resolving to `true` if confirmed or confirmation not * required, `false` otherwise */ - public async verifyConfirmed(args: ToolCallbackArgs): Promise { + public async verifyConfirmed(args: ToolArgs): Promise { if (!this.config.confirmationRequiredTools.includes(this.name)) { return true; } - return this.elicitation.requestConfirmation(this.getConfirmationMessage(...args)); + return this.elicitation.requestConfirmation(this.getConfirmationMessage(args)); } /** @@ -414,7 +419,10 @@ export abstract class ToolBase { return false; } - const callback: ToolCallback = async (...args) => { + const callback = async ( + args: ToolArgs, + { signal }: ToolExecutionContext + ): Promise => { const startTime = Date.now(); try { if (!(await this.verifyConfirmed(args))) { @@ -440,8 +448,8 @@ export abstract class ToolBase { noRedaction: true, }); - const result = await this.execute(...args); - this.emitToolEvent(startTime, result, ...args); + const result = await this.execute(args, { signal }); + this.emitToolEvent(args, { startTime, result }); this.session.logger.debug({ id: LogId.toolExecute, @@ -456,19 +464,35 @@ export abstract class ToolBase { context: "tool", message: `Error executing ${this.name}: ${error as string}`, }); - const toolResult = await this.handleError(error, args[0] as ToolArgs); - this.emitToolEvent(startTime, toolResult, ...args); + const toolResult = await this.handleError(error, args); + this.emitToolEvent(args, { startTime, result: toolResult }); return toolResult; } }; - this.registeredTool = server.mcpServer.tool( - this.name, - this.description, - this.argsShape, - this.annotations, - callback - ); + this.registeredTool = + // Note: We use explicit type casting here to avoid "excessively deep and possibly infinite" errors + // that occur when TypeScript tries to infer the complex generic types from `typeof this.argsShape` + // in the abstract class context. + ( + server.mcpServer.registerTool as ( + name: string, + config: { + description?: string; + inputSchema?: ZodRawShape; + annotations?: ToolAnnotations; + }, + cb: (args: ToolArgs, extra: ToolExecutionContext) => Promise + ) => RegisteredTool + )( + this.name, + { + description: this.description, + inputSchema: this.argsShape, + annotations: this.annotations, + }, + callback + ); return true; } @@ -561,7 +585,7 @@ export abstract class ToolBase { protected handleError( error: unknown, // eslint-disable-next-line @typescript-eslint/no-unused-vars - args: ToolArgs + args: z.infer> ): Promise | CallToolResult { return { content: [ @@ -599,8 +623,8 @@ export abstract class ToolBase { * ``` */ protected abstract resolveTelemetryMetadata( - result: CallToolResult, - ...args: Parameters> + args: ToolArgs, + { result }: { result: CallToolResult } ): TelemetryToolMetadata; /** @@ -610,15 +634,14 @@ export abstract class ToolBase { * @param args - The arguments passed to the tool */ private emitToolEvent( - startTime: number, - result: CallToolResult, - ...args: Parameters> + args: ToolArgs, + { startTime, result }: { startTime: number; result: CallToolResult } ): void { if (!this.telemetry.isTelemetryEnabled()) { return; } const duration = Date.now() - startTime; - const metadata = this.resolveTelemetryMetadata(result, ...args); + const metadata = this.resolveTelemetryMetadata(args, { result }); const event: ToolEvent = { timestamp: new Date().toISOString(), source: "mdbmcp", diff --git a/tests/tsconfig.json b/tests/tsconfig.json deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/unit/toolBase.test.ts b/tests/unit/toolBase.test.ts index b5dc928c6..169ad3bc8 100644 --- a/tests/unit/toolBase.test.ts +++ b/tests/unit/toolBase.test.ts @@ -2,13 +2,7 @@ import type { Mock } from "vitest"; import { describe, it, expect, vi, beforeEach, type MockedFunction } from "vitest"; import type { ZodRawShape } from "zod"; import { z } from "zod"; -import type { - ToolCallbackArgs, - OperationType, - ToolCategory, - ToolConstructorParams, - ToolArgs, -} from "../../src/tools/tool.js"; +import type { OperationType, ToolCategory, ToolConstructorParams, ToolArgs } from "../../src/tools/tool.js"; import { ToolBase } from "../../src/tools/tool.js"; import type { CallToolResult, ToolAnnotations } from "@modelcontextprotocol/sdk/types.js"; import type { Session } from "../../src/common/session.js"; @@ -75,10 +69,7 @@ describe("ToolBase", () => { it("should return true when tool is not in confirmationRequiredTools list", async () => { mockConfig.confirmationRequiredTools = ["other-tool", "another-tool"]; - const args = [ - { param1: "test" }, - {} as ToolCallbackArgs<(typeof testTool)["argsShape"]>[1], - ] as ToolCallbackArgs<(typeof testTool)["argsShape"]>; + const args = { param1: "test" }; const result = await testTool.verifyConfirmed(args); expect(result).toBe(true); @@ -88,8 +79,8 @@ describe("ToolBase", () => { it("should return true when confirmationRequiredTools list is empty", async () => { mockConfig.confirmationRequiredTools = []; - const args = [{ param1: "test" }, {} as ToolCallbackArgs<(typeof testTool)["argsShape"]>[1]]; - const result = await testTool.verifyConfirmed(args as ToolCallbackArgs<(typeof testTool)["argsShape"]>); + const args = { param1: "test" }; + const result = await testTool.verifyConfirmed(args); expect(result).toBe(true); expect(mockRequestConfirmation).not.toHaveBeenCalled(); @@ -99,8 +90,8 @@ describe("ToolBase", () => { mockConfig.confirmationRequiredTools = ["test-tool"]; mockRequestConfirmation.mockResolvedValue(true); - const args = [{ param1: "test", param2: 42 }, {} as ToolCallbackArgs<(typeof testTool)["argsShape"]>[1]]; - const result = await testTool.verifyConfirmed(args as ToolCallbackArgs<(typeof testTool)["argsShape"]>); + const args = { param1: "test", param2: 42 }; + const result = await testTool.verifyConfirmed(args); expect(result).toBe(true); expect(mockRequestConfirmation).toHaveBeenCalledTimes(1); @@ -113,8 +104,8 @@ describe("ToolBase", () => { mockConfig.confirmationRequiredTools = ["test-tool"]; mockRequestConfirmation.mockResolvedValue(false); - const args = [{ param1: "test" }, {} as ToolCallbackArgs<(typeof testTool)["argsShape"]>[1]]; - const result = await testTool.verifyConfirmed(args as ToolCallbackArgs<(typeof testTool)["argsShape"]>); + const args = { param1: "test" }; + const result = await testTool.verifyConfirmed(args); expect(result).toBe(false); expect(mockRequestConfirmation).toHaveBeenCalledTimes(1); @@ -158,7 +149,13 @@ describe("ToolBase", () => { }); it("should return empty metadata by default", async () => { - await mockCallback({ param1: "value1" }, {} as never); + await mockCallback( + { + param1: "value1", + param2: 3, + }, + {} as never + ); const event = ((mockTelemetry.emitEvents as Mock).mock.lastCall?.[0] as ToolEvent[])[0]; expectDefined(event); expect(event.properties.result).to.equal("success"); @@ -285,8 +282,9 @@ class TestTool extends ToolBase { } protected resolveTelemetryMetadata( - result: CallToolResult, - args: ToolArgs + args: ToolArgs, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + { result }: { result: CallToolResult } ): TelemetryToolMetadata { if (args.param2 === 3) { return { From f881902d800103d4a7eb99ce7079cf3a46dece95 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 3 Dec 2025 23:24:25 +0100 Subject: [PATCH 2/6] chore: use .tool instead of .registerTool doesn't seem like registerTool even exists... --- src/tools/tool.ts | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/tools/tool.ts b/src/tools/tool.ts index 1efe3192c..53fb97c14 100644 --- a/src/tools/tool.ts +++ b/src/tools/tool.ts @@ -470,29 +470,18 @@ export abstract class ToolBase { } }; - this.registeredTool = - // Note: We use explicit type casting here to avoid "excessively deep and possibly infinite" errors - // that occur when TypeScript tries to infer the complex generic types from `typeof this.argsShape` - // in the abstract class context. - ( - server.mcpServer.registerTool as ( - name: string, - config: { - description?: string; - inputSchema?: ZodRawShape; - annotations?: ToolAnnotations; - }, - cb: (args: ToolArgs, extra: ToolExecutionContext) => Promise - ) => RegisteredTool - )( - this.name, - { - description: this.description, - inputSchema: this.argsShape, - annotations: this.annotations, - }, - callback - ); + this.registeredTool = ( + server.mcpServer.tool as ( + // Note: We use explicit type casting here to avoid "excessively deep and possibly infinite" errors + // that occur when TypeScript tries to infer the complex generic types from `typeof this.argsShape` + // in the abstract class context. + name: string, + description: string, + argsShape: ZodRawShape, + annotations: ToolAnnotations, + cb: (args: ToolArgs, { signal }: ToolExecutionContext) => Promise + ) => RegisteredTool + )(this.name, this.description, this.argsShape, this.annotations, callback); return true; } From 92f160532bf636bb46c3fedf43f7fc59386e48c1 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 3 Dec 2025 23:32:20 +0100 Subject: [PATCH 3/6] chore: remove (HTTP 400) to match new error message --- .../integration/transports/configOverrides.test.ts | 14 +++++++------- .../integration/transports/streamableHttp.test.ts | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/transports/configOverrides.test.ts b/tests/integration/transports/configOverrides.test.ts index 7157339f2..1b09dd191 100644 --- a/tests/integration/transports/configOverrides.test.ts +++ b/tests/integration/transports/configOverrides.test.ts @@ -112,7 +112,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); expect(error.message).toContain(`Config key connectionString is not allowed to be overridden`); } }); @@ -199,7 +199,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); expect(error.message).toContain(`Config key ${configKey} is not allowed to be overridden`); } }); @@ -222,7 +222,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); // Should contain at least one of the not-allowed field errors const hasNotAllowedError = error.message.includes("Config key apiBaseUrl is not allowed to be overridden") || @@ -379,7 +379,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); expect(error.message).toContain(`Cannot apply override for readOnly: Can only set to true`); } }); @@ -455,7 +455,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); expect(error.message).toContain( "Cannot apply override for idleTimeoutMs: Can only set to a value lower than the base value" ); @@ -479,7 +479,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); expect(error.message).toContain( "Cannot apply override for idleTimeoutMs: Can only set to a value lower than the base value" ); @@ -539,7 +539,7 @@ describe("Config Overrides via HTTP", () => { if (!(error instanceof Error)) { throw new Error("Expected an error to be thrown"); } - expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)"); + expect(error.message).toContain("Error POSTing to endpoint"); expect(error.message).toContain( "Cannot apply override for previewFeatures: Can only override to a subset of the base value" ); diff --git a/tests/integration/transports/streamableHttp.test.ts b/tests/integration/transports/streamableHttp.test.ts index a901a00b1..a85219f4d 100644 --- a/tests/integration/transports/streamableHttp.test.ts +++ b/tests/integration/transports/streamableHttp.test.ts @@ -93,7 +93,7 @@ describe("StreamableHttpRunner", () => { throw err; } else { expect(err).toBeDefined(); - expect(err?.toString()).toContain("HTTP 403"); + expect(err?.toString()).toContain("Error POSTing to endpoint"); } } }); From b245ba17144d580b7fed7ecf725a241de15c5e1b Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 3 Dec 2025 23:33:17 +0100 Subject: [PATCH 4/6] chore: args type --- src/tools/atlasLocal/read/listDeployments.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/tools/atlasLocal/read/listDeployments.ts b/src/tools/atlasLocal/read/listDeployments.ts index 393a65fec..eb71903b5 100644 --- a/src/tools/atlasLocal/read/listDeployments.ts +++ b/src/tools/atlasLocal/read/listDeployments.ts @@ -1,6 +1,6 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasLocalToolBase } from "../atlasLocalTool.js"; -import type { OperationType } from "../../tool.js"; +import type { OperationType, ToolArgs } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; import type { Deployment } from "@mongodb-js/atlas-local"; import type { Client } from "@mongodb-js/atlas-local"; @@ -11,7 +11,10 @@ export class ListDeploymentsTool extends AtlasLocalToolBase { static operationType: OperationType = "read"; protected argsShape = {}; - protected async executeWithAtlasLocalClient(client: Client): Promise { + protected async executeWithAtlasLocalClient( + _args: ToolArgs, + { client }: { client: Client } + ): Promise { // List the deployments const deployments = await client.listDeployments(); From 070a8d240ed5361163e9877b456d70a4a1d2943f Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 3 Dec 2025 23:38:24 +0100 Subject: [PATCH 5/6] chore: update test to match --- tests/unit/toolBase.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/toolBase.test.ts b/tests/unit/toolBase.test.ts index 169ad3bc8..8ab856a57 100644 --- a/tests/unit/toolBase.test.ts +++ b/tests/unit/toolBase.test.ts @@ -159,10 +159,10 @@ describe("ToolBase", () => { const event = ((mockTelemetry.emitEvents as Mock).mock.lastCall?.[0] as ToolEvent[])[0]; expectDefined(event); expect(event.properties.result).to.equal("success"); + expect(event.properties).toHaveProperty("test_param2"); expect(event.properties).not.toHaveProperty("project_id"); expect(event.properties).not.toHaveProperty("org_id"); expect(event.properties).not.toHaveProperty("atlas_local_deployment_id"); - expect(event.properties).not.toHaveProperty("test_param2"); }); it("should include custom telemetry metadata", async () => { From 1e90969c713c3edc1e0ccf2e89d3a6c0947c5bd8 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 4 Dec 2025 11:23:30 +0100 Subject: [PATCH 6/6] Revert "chore: use .tool instead of .registerTool" Now with actual fix for our mock... I thought there was something wrong This reverts commit f881902d800103d4a7eb99ce7079cf3a46dece95. --- src/tools/tool.ts | 35 +++++++++++++++++++++++------------ tests/unit/toolBase.test.ts | 8 ++++---- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/tools/tool.ts b/src/tools/tool.ts index 53fb97c14..1efe3192c 100644 --- a/src/tools/tool.ts +++ b/src/tools/tool.ts @@ -470,18 +470,29 @@ export abstract class ToolBase { } }; - this.registeredTool = ( - server.mcpServer.tool as ( - // Note: We use explicit type casting here to avoid "excessively deep and possibly infinite" errors - // that occur when TypeScript tries to infer the complex generic types from `typeof this.argsShape` - // in the abstract class context. - name: string, - description: string, - argsShape: ZodRawShape, - annotations: ToolAnnotations, - cb: (args: ToolArgs, { signal }: ToolExecutionContext) => Promise - ) => RegisteredTool - )(this.name, this.description, this.argsShape, this.annotations, callback); + this.registeredTool = + // Note: We use explicit type casting here to avoid "excessively deep and possibly infinite" errors + // that occur when TypeScript tries to infer the complex generic types from `typeof this.argsShape` + // in the abstract class context. + ( + server.mcpServer.registerTool as ( + name: string, + config: { + description?: string; + inputSchema?: ZodRawShape; + annotations?: ToolAnnotations; + }, + cb: (args: ToolArgs, extra: ToolExecutionContext) => Promise + ) => RegisteredTool + )( + this.name, + { + description: this.description, + inputSchema: this.argsShape, + annotations: this.annotations, + }, + callback + ); return true; } diff --git a/tests/unit/toolBase.test.ts b/tests/unit/toolBase.test.ts index 8ab856a57..28fa2ddfb 100644 --- a/tests/unit/toolBase.test.ts +++ b/tests/unit/toolBase.test.ts @@ -132,11 +132,11 @@ describe("ToolBase", () => { beforeEach(() => { const mockServer = { mcpServer: { - tool: ( + registerTool: ( name: string, - description: string, - paramsSchema: unknown, - annotations: ToolAnnotations, + { + description, + }: { description: string; inputSchema: ZodRawShape; annotations: ToolAnnotations }, cb: ToolCallback ): void => { expect(name).toBe(testTool.name);