From 6b6902e1c768512301630f57ce09e9b8dcf3008c Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 13 Nov 2025 11:34:38 +0100 Subject: [PATCH 01/16] chore: move config related code to common folder --- scripts/generateArguments.ts | 2 +- src/common/config.ts | 4 ++-- src/common/{ => config}/argsParserOptions.ts | 0 src/common/{ => config}/configUtils.ts | 0 tests/unit/common/config.test.ts | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename src/common/{ => config}/argsParserOptions.ts (100%) rename src/common/{ => config}/configUtils.ts (100%) diff --git a/scripts/generateArguments.ts b/scripts/generateArguments.ts index ee3695c4..69d10bc4 100644 --- a/scripts/generateArguments.ts +++ b/scripts/generateArguments.ts @@ -14,7 +14,7 @@ import { fileURLToPath } from "url"; import { UserConfigSchema, configRegistry } from "../src/common/config.js"; import assert from "assert"; import { execSync } from "child_process"; -import { OPTIONS } from "../src/common/argsParserOptions.js"; +import { OPTIONS } from "../src/common/config/argsParserOptions.js"; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); diff --git a/src/common/config.ts b/src/common/config.ts index 6e404b86..ff5e2fec 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -11,8 +11,8 @@ import { getLogPath, isConnectionSpecifier, validateConfigKey, -} from "./configUtils.js"; -import { OPTIONS } from "./argsParserOptions.js"; +} from "./config/configUtils.js"; +import { OPTIONS } from "./config/argsParserOptions.js"; import { similarityValues, previewFeatureValues } from "./schemas.js"; export const configRegistry = z4.registry(); diff --git a/src/common/argsParserOptions.ts b/src/common/config/argsParserOptions.ts similarity index 100% rename from src/common/argsParserOptions.ts rename to src/common/config/argsParserOptions.ts diff --git a/src/common/configUtils.ts b/src/common/config/configUtils.ts similarity index 100% rename from src/common/configUtils.ts rename to src/common/config/configUtils.ts diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index ebd4d0bb..827a5d6b 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -7,7 +7,7 @@ import { UserConfigSchema, warnIfVectorSearchNotEnabledCorrectly, } from "../../../src/common/config.js"; -import { getLogPath, getExportsPath } from "../../../src/common/configUtils.js"; +import { getLogPath, getExportsPath } from "../../../src/common/config/configUtils.js"; import type { CliOptions } from "@mongosh/arg-parser"; import { Keychain } from "../../../src/common/keychain.js"; import type { Secret } from "../../../src/common/keychain.js"; From 70d408c40cd70e1f68afc5a3306822a8bdc99483 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 13 Nov 2025 12:12:41 +0100 Subject: [PATCH 02/16] chore: untangle driverOptions from config --- src/common/config.ts | 30 +------------------ src/common/config/driverOptions.ts | 27 +++++++++++++++++ src/common/connectionManager.ts | 8 ++--- src/resources/common/config.ts | 4 +-- .../common/connectionManager.oidc.test.ts | 8 ++--- tests/integration/helpers.ts | 13 ++------ .../tools/mongodb/mongodbHelpers.ts | 3 +- .../tools/mongodb/mongodbTool.test.ts | 3 +- 8 files changed, 42 insertions(+), 54 deletions(-) create mode 100644 src/common/config/driverOptions.ts diff --git a/src/common/config.ts b/src/common/config.ts index ff5e2fec..35b4616c 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -1,5 +1,5 @@ import argv from "yargs-parser"; -import type { CliOptions, ConnectionInfo } from "@mongosh/arg-parser"; +import type { CliOptions } from "@mongosh/arg-parser"; import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; import { Keychain } from "./keychain.js"; import type { Secret } from "./keychain.js"; @@ -183,20 +183,6 @@ export const config = setupUserConfig({ env: process.env, }); -export type DriverOptions = ConnectionInfo["driverOptions"]; -export const defaultDriverOptions: DriverOptions = { - readConcern: { - level: "local", - }, - readPreference: "secondaryPreferred", - writeConcern: { - w: "majority", - }, - timeoutMS: 30_000, - proxy: { useEnvironmentVariableProxies: true }, - applyProxyToOIDC: true, -}; - // Gets the config supplied by the user as environment variables. The variable names // are prefixed with `MDB_MCP_` and the keys match the UserConfig keys, but are converted // to SNAKE_UPPER_CASE. @@ -414,17 +400,3 @@ export function setupUserConfig({ cli, env }: { cli: string[]; env: Record; -}): DriverOptions { - const { driverOptions } = generateConnectionInfoFromCliArgs(config); - return { - ...defaults, - ...driverOptions, - }; -} diff --git a/src/common/config/driverOptions.ts b/src/common/config/driverOptions.ts new file mode 100644 index 00000000..1a9a0a6d --- /dev/null +++ b/src/common/config/driverOptions.ts @@ -0,0 +1,27 @@ +import { generateConnectionInfoFromCliArgs, type ConnectionInfo } from "@mongosh/arg-parser"; +import type { UserConfig } from "../config.js"; + +export type DriverOptions = ConnectionInfo["driverOptions"]; +export const defaultDriverOptions: DriverOptions = { + readConcern: { + level: "local", + }, + readPreference: "secondaryPreferred", + writeConcern: { + w: "majority", + }, + timeoutMS: 30_000, + proxy: { useEnvironmentVariableProxies: true }, + applyProxyToOIDC: true, +}; + +export function createDriverOptions( + config: UserConfig, + defaults: Partial = defaultDriverOptions +): DriverOptions { + const { driverOptions } = generateConnectionInfoFromCliArgs(config); + return { + ...defaults, + ...driverOptions, + }; +} diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index bb8002d3..1f0b94e0 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -4,7 +4,8 @@ import { ConnectionString } from "mongodb-connection-string-url"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { type ConnectionInfo, generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; import type { DeviceId } from "../helpers/deviceId.js"; -import { defaultDriverOptions, setupDriverConfig, type DriverOptions, type UserConfig } from "./config.js"; +import { createDriverOptions, type DriverOptions } from "./config/driverOptions.js"; +import { type UserConfig } from "./config.js"; import { MongoDBError, ErrorCodes } from "./errors.js"; import { type LoggerBase, LogId } from "./logger.js"; import { packageInfo } from "./packageInfo.js"; @@ -394,10 +395,7 @@ export type ConnectionManagerFactoryFn = (createParams: { }) => Promise; export const createMCPConnectionManager: ConnectionManagerFactoryFn = ({ logger, deviceId, userConfig }) => { - const driverOptions = setupDriverConfig({ - config: userConfig, - defaults: defaultDriverOptions, - }); + const driverOptions = createDriverOptions(userConfig); return Promise.resolve(new MCPConnectionManager(userConfig, driverOptions, logger, deviceId)); }; diff --git a/src/resources/common/config.ts b/src/resources/common/config.ts index d67b0400..7949bd90 100644 --- a/src/resources/common/config.ts +++ b/src/resources/common/config.ts @@ -1,5 +1,5 @@ import { ReactiveResource } from "../resource.js"; -import { defaultDriverOptions } from "../../common/config.js"; +import { createDriverOptions } from "../../common/config/driverOptions.js"; import type { UserConfig } from "../../common/config.js"; import type { Telemetry } from "../../telemetry/telemetry.js"; import type { Session } from "../../lib.js"; @@ -38,7 +38,7 @@ export class ConfigResource extends ReactiveResource { connectionString: this.current.connectionString ? "set; access to MongoDB tools are currently available to use" : "not set; before using any MongoDB tool, you need to configure a connection string, alternatively you can setup MongoDB Atlas access, more info at 'https://github.com/mongodb-js/mongodb-mcp-server'.", - connectOptions: defaultDriverOptions, + connectOptions: createDriverOptions(this.config), atlas: this.current.apiClientId && this.current.apiClientSecret ? "set; MongoDB Atlas tools are currently available to use" diff --git a/tests/integration/common/connectionManager.oidc.test.ts b/tests/integration/common/connectionManager.oidc.test.ts index 595239ce..401f91fd 100644 --- a/tests/integration/common/connectionManager.oidc.test.ts +++ b/tests/integration/common/connectionManager.oidc.test.ts @@ -7,7 +7,7 @@ import { describeWithMongoDB, isCommunityServer, getServerVersion } from "../too import { defaultTestConfig, responseAsText, timeout, waitUntil } from "../helpers.js"; import type { ConnectionStateConnected, ConnectionStateConnecting } from "../../../src/common/connectionManager.js"; import type { UserConfig } from "../../../src/common/config.js"; -import { setupDriverConfig } from "../../../src/common/config.js"; +import { createDriverOptions } from "../../../src/common/config/driverOptions.js"; import path from "path"; import type { OIDCMockProviderConfig } from "@mongodb-js/oidc-mock-provider"; import { OIDCMockProvider } from "@mongodb-js/oidc-mock-provider"; @@ -142,11 +142,7 @@ describe.skipIf(process.platform !== "linux")("ConnectionManager OIDC Tests", as }, { getUserConfig: () => oidcConfig, - getDriverOptions: () => - setupDriverConfig({ - config: oidcConfig, - defaults: {}, - }), + getDriverOptions: () => createDriverOptions(oidcConfig), downloadOptions: { runner: true, downloadOptions: { enterprise: true, version: mongodbVersion }, diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 7796dda6..61a81f7c 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -6,13 +6,9 @@ import { Telemetry } from "../../src/telemetry/telemetry.js"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "./inMemoryTransport.js"; -import type { UserConfig, DriverOptions } from "../../src/common/config.js"; +import { type UserConfig, config } from "../../src/common/config.js"; +import { type DriverOptions, createDriverOptions } from "../../src/common/config/driverOptions.js"; import { McpError, ResourceUpdatedNotificationSchema } from "@modelcontextprotocol/sdk/types.js"; -import { - config, - setupDriverConfig, - defaultDriverOptions as defaultDriverOptionsFromConfig, -} from "../../src/common/config.js"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import type { ConnectionManager, ConnectionState } from "../../src/common/connectionManager.js"; import { MCPConnectionManager } from "../../src/common/connectionManager.js"; @@ -24,10 +20,7 @@ import type { MockClientCapabilities, createMockElicitInput } from "../utils/eli import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; import { defaultCreateAtlasLocalClient } from "../../src/common/atlasLocal.js"; -export const driverOptions = setupDriverConfig({ - config, - defaults: defaultDriverOptionsFromConfig, -}); +export const driverOptions = createDriverOptions(config); export const defaultDriverOptions: DriverOptions = { ...driverOptions }; diff --git a/tests/integration/tools/mongodb/mongodbHelpers.ts b/tests/integration/tools/mongodb/mongodbHelpers.ts index d53c97df..be654100 100644 --- a/tests/integration/tools/mongodb/mongodbHelpers.ts +++ b/tests/integration/tools/mongodb/mongodbHelpers.ts @@ -11,7 +11,8 @@ import { defaultDriverOptions, getDataFromUntrustedContent, } from "../../helpers.js"; -import type { UserConfig, DriverOptions } from "../../../../src/common/config.js"; +import type { UserConfig } from "../../../../src/common/config.js"; +import type { DriverOptions } from "../../../../src/common/config/driverOptions.js"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { EJSON } from "bson"; import { MongoDBClusterProcess } from "./mongodbClusterProcess.js"; diff --git a/tests/integration/tools/mongodb/mongodbTool.test.ts b/tests/integration/tools/mongodb/mongodbTool.test.ts index df28d483..57352844 100644 --- a/tests/integration/tools/mongodb/mongodbTool.test.ts +++ b/tests/integration/tools/mongodb/mongodbTool.test.ts @@ -4,7 +4,8 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { MongoDBToolBase } from "../../../../src/tools/mongodb/mongodbTool.js"; import { type ToolBase, type ToolConstructorParams, type OperationType } from "../../../../src/tools/tool.js"; -import { defaultDriverOptions, type UserConfig } from "../../../../src/common/config.js"; +import { type UserConfig } from "../../../../src/common/config.js"; +import { defaultDriverOptions } from "../../../../src/common/config/driverOptions.js"; import { MCPConnectionManager } from "../../../../src/common/connectionManager.js"; import { Session } from "../../../../src/common/session.js"; import { CompositeLogger } from "../../../../src/common/logger.js"; From d3fc7391fda868e6265b99d797ffc660b5b0985a Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 13 Nov 2025 12:20:08 +0100 Subject: [PATCH 03/16] chore: remove duplicate driverOptions test object --- tests/integration/helpers.ts | 6 ++---- tests/integration/telemetry.test.ts | 4 ++-- tests/unit/common/session.test.ts | 4 ++-- tests/unit/resources/common/debug.test.ts | 4 ++-- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 61a81f7c..929c8339 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -20,10 +20,6 @@ import type { MockClientCapabilities, createMockElicitInput } from "../utils/eli import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; import { defaultCreateAtlasLocalClient } from "../../src/common/atlasLocal.js"; -export const driverOptions = createDriverOptions(config); - -export const defaultDriverOptions: DriverOptions = { ...driverOptions }; - interface Parameter { name: string; description: string; @@ -52,6 +48,8 @@ export const defaultTestConfig: UserConfig = { loggers: ["stderr"], }; +export const defaultDriverOptions: DriverOptions = createDriverOptions(defaultTestConfig); + export const DEFAULT_LONG_RUNNING_TEST_WAIT_TIMEOUT_MS = 1_200_000; export function setupIntegrationTest( diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 28e4c3b4..ef741d37 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -1,7 +1,7 @@ import { Telemetry } from "../../src/telemetry/telemetry.js"; import { Session } from "../../src/common/session.js"; import { config } from "../../src/common/config.js"; -import { driverOptions } from "./helpers.js"; +import { defaultDriverOptions } from "./helpers.js"; import { DeviceId } from "../../src/helpers/deviceId.js"; import { describe, expect, it } from "vitest"; import { CompositeLogger } from "../../src/common/logger.js"; @@ -16,7 +16,7 @@ describe("Telemetry", () => { const deviceId = DeviceId.create(logger); const actualDeviceId = await deviceId.get(); - const connectionManager = new MCPConnectionManager(config, driverOptions, logger, deviceId); + const connectionManager = new MCPConnectionManager(config, defaultDriverOptions, logger, deviceId); const telemetry = Telemetry.create( new Session({ diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts index ed465f22..6e4d9331 100644 --- a/tests/unit/common/session.test.ts +++ b/tests/unit/common/session.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { Session } from "../../../src/common/session.js"; import { config } from "../../../src/common/config.js"; -import { driverOptions } from "../../integration/helpers.js"; +import { defaultDriverOptions } from "../../integration/helpers.js"; import { CompositeLogger } from "../../../src/common/logger.js"; import { MCPConnectionManager } from "../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../src/common/exportsManager.js"; @@ -25,7 +25,7 @@ describe("Session", () => { const logger = new CompositeLogger(); mockDeviceId = MockDeviceId; - const connectionManager = new MCPConnectionManager(config, driverOptions, logger, mockDeviceId); + const connectionManager = new MCPConnectionManager(config, defaultDriverOptions, logger, mockDeviceId); session = new Session({ apiClientId: "test-client-id", diff --git a/tests/unit/resources/common/debug.test.ts b/tests/unit/resources/common/debug.test.ts index 6758ebeb..e00ce4f2 100644 --- a/tests/unit/resources/common/debug.test.ts +++ b/tests/unit/resources/common/debug.test.ts @@ -3,7 +3,7 @@ import { DebugResource } from "../../../../src/resources/common/debug.js"; import { Session } from "../../../../src/common/session.js"; import { Telemetry } from "../../../../src/telemetry/telemetry.js"; import { config } from "../../../../src/common/config.js"; -import { driverOptions } from "../../../integration/helpers.js"; +import { defaultDriverOptions } from "../../../integration/helpers.js"; import { CompositeLogger } from "../../../../src/common/logger.js"; import { MCPConnectionManager } from "../../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../../src/common/exportsManager.js"; @@ -14,7 +14,7 @@ import { VectorSearchEmbeddingsManager } from "../../../../src/common/search/vec describe("debug resource", () => { const logger = new CompositeLogger(); const deviceId = DeviceId.create(logger); - const connectionManager = new MCPConnectionManager(config, driverOptions, logger, deviceId); + const connectionManager = new MCPConnectionManager(config, defaultDriverOptions, logger, deviceId); const session = vi.mocked( new Session({ From 2723a834d4436e312342a8497a75e4f35f1cf18f Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 12:07:41 +0100 Subject: [PATCH 04/16] fix: don't use config opts for further connections This commit adds a test to assert that we don't use the driver options derived from the CLI arguments -> userConfig for further connections (other than the first connection) such as switch-connection tool. The problem earlier was that MCPConnectionManager was accepting a driverOptions object which it was then using in conjunction with userConfig to construct a fresh set of driver options for all the connections established by the MCP server. Now this was particularly problematic because the driver options for the configured connection might not be compatible with the new connection attempts being made in the MCP server. Take for example - MCP server configured to connect with an OIDC enabled MongoDB server and later switch-connection tool using the same driver options to connect to a locally running mongodb server without any auth. To fix that we've removed DriverOptions object from the interface of MCPConnectionManager and instead put it on the connect method. It is the responsibility of the caller to provide a correct ConnectionInfo object. For pre-configured connections server constructs the ConnectionInfo object using user config and passes it down to the MCPConnectionManager.connect and for the rest usage, they simply pass the connection string that they want to connect to. --- src/common/connectionManager.ts | 22 +++--- src/resources/common/config.ts | 7 +- src/server.ts | 7 +- .../common/connectionManager.oidc.test.ts | 2 - tests/integration/helpers.ts | 7 +- tests/integration/telemetry.test.ts | 3 +- .../tools/atlas-local/atlasLocalHelpers.ts | 12 +--- tests/integration/tools/atlas/atlasHelpers.ts | 17 ++--- .../tools/atlas/performanceAdvisor.test.ts | 16 ++--- .../tools/mongodb/connect/connect.test.ts | 69 ++++++++++++++++++- .../tools/mongodb/mongodbHelpers.ts | 9 +-- .../tools/mongodb/mongodbTool.test.ts | 4 +- tests/unit/common/session.test.ts | 3 +- tests/unit/resources/common/debug.test.ts | 3 +- 14 files changed, 107 insertions(+), 74 deletions(-) diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index 1f0b94e0..5d583c6d 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -2,9 +2,8 @@ import { EventEmitter } from "events"; import type { MongoClientOptions } from "mongodb"; import { ConnectionString } from "mongodb-connection-string-url"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; -import { type ConnectionInfo, generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; +import { type ConnectionInfo } from "@mongosh/arg-parser"; import type { DeviceId } from "../helpers/deviceId.js"; -import { createDriverOptions, type DriverOptions } from "./config/driverOptions.js"; import { type UserConfig } from "./config.js"; import { MongoDBError, ErrorCodes } from "./errors.js"; import { type LoggerBase, LogId } from "./logger.js"; @@ -18,8 +17,8 @@ export interface AtlasClusterConnectionInfo { expiryDate: Date; } -export interface ConnectionSettings { - connectionString: string; +export interface ConnectionSettings extends Omit { + driverOptions?: ConnectionInfo["driverOptions"]; atlas?: AtlasClusterConnectionInfo; } @@ -137,7 +136,6 @@ export class MCPConnectionManager extends ConnectionManager { constructor( private userConfig: UserConfig, - private driverOptions: DriverOptions, private logger: LoggerBase, deviceId: DeviceId, bus?: EventEmitter @@ -158,7 +156,6 @@ export class MCPConnectionManager extends ConnectionManager { } let serviceProvider: Promise; - let connectionInfo: ConnectionInfo; let connectionStringAuthType: ConnectionStringAuthType = "scram"; try { @@ -174,11 +171,10 @@ export class MCPConnectionManager extends ConnectionManager { components: appNameComponents, }); - connectionInfo = generateConnectionInfoFromCliArgs({ - ...this.userConfig, - ...this.driverOptions, - connectionSpecifier: settings.connectionString, - }); + const connectionInfo = { + connectionString: settings.connectionString, + driverOptions: settings.driverOptions ?? {}, + }; if (connectionInfo.driverOptions.oidc) { connectionInfo.driverOptions.oidc.allowedFlows ??= ["auth-code"]; @@ -395,7 +391,5 @@ export type ConnectionManagerFactoryFn = (createParams: { }) => Promise; export const createMCPConnectionManager: ConnectionManagerFactoryFn = ({ logger, deviceId, userConfig }) => { - const driverOptions = createDriverOptions(userConfig); - - return Promise.resolve(new MCPConnectionManager(userConfig, driverOptions, logger, deviceId)); + return Promise.resolve(new MCPConnectionManager(userConfig, logger, deviceId)); }; diff --git a/src/resources/common/config.ts b/src/resources/common/config.ts index 7949bd90..684c03f3 100644 --- a/src/resources/common/config.ts +++ b/src/resources/common/config.ts @@ -1,8 +1,8 @@ import { ReactiveResource } from "../resource.js"; -import { createDriverOptions } from "../../common/config/driverOptions.js"; import type { UserConfig } from "../../common/config.js"; import type { Telemetry } from "../../telemetry/telemetry.js"; import type { Session } from "../../lib.js"; +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; export class ConfigResource extends ReactiveResource { constructor(session: Session, config: UserConfig, telemetry: Telemetry) { @@ -32,13 +32,14 @@ export class ConfigResource extends ReactiveResource { } toOutput(): string { + const connectionInfo = generateConnectionInfoFromCliArgs(this.current); const result = { telemetry: this.current.telemetry, logPath: this.current.logPath, - connectionString: this.current.connectionString + connectionString: connectionInfo.connectionString ? "set; access to MongoDB tools are currently available to use" : "not set; before using any MongoDB tool, you need to configure a connection string, alternatively you can setup MongoDB Atlas access, more info at 'https://github.com/mongodb-js/mongodb-mcp-server'.", - connectOptions: createDriverOptions(this.config), + connectOptions: connectionInfo.driverOptions, atlas: this.current.apiClientId && this.current.apiClientSecret ? "set; MongoDB Atlas tools are currently available to use" diff --git a/src/server.ts b/src/server.ts index 4e7e6d97..a276a4c7 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,3 +1,4 @@ +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { Session } from "./common/session.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; @@ -327,9 +328,11 @@ export class Server { context: "server", message: `Detected a MongoDB connection string in the configuration, trying to connect...`, }); - await this.session.connectToMongoDB({ - connectionString: this.userConfig.connectionString, + const connectionInfo = generateConnectionInfoFromCliArgs({ + ...this.userConfig, + connectionSpecifier: this.userConfig.connectionString, }); + await this.session.connectToMongoDB(connectionInfo); } catch (error) { // We don't throw an error here because we want to allow the server to start even if the connection string is invalid. this.session.logger.error({ diff --git a/tests/integration/common/connectionManager.oidc.test.ts b/tests/integration/common/connectionManager.oidc.test.ts index 401f91fd..729c5012 100644 --- a/tests/integration/common/connectionManager.oidc.test.ts +++ b/tests/integration/common/connectionManager.oidc.test.ts @@ -7,7 +7,6 @@ import { describeWithMongoDB, isCommunityServer, getServerVersion } from "../too import { defaultTestConfig, responseAsText, timeout, waitUntil } from "../helpers.js"; import type { ConnectionStateConnected, ConnectionStateConnecting } from "../../../src/common/connectionManager.js"; import type { UserConfig } from "../../../src/common/config.js"; -import { createDriverOptions } from "../../../src/common/config/driverOptions.js"; import path from "path"; import type { OIDCMockProviderConfig } from "@mongodb-js/oidc-mock-provider"; import { OIDCMockProvider } from "@mongodb-js/oidc-mock-provider"; @@ -142,7 +141,6 @@ describe.skipIf(process.platform !== "linux")("ConnectionManager OIDC Tests", as }, { getUserConfig: () => oidcConfig, - getDriverOptions: () => createDriverOptions(oidcConfig), downloadOptions: { runner: true, downloadOptions: { enterprise: true, version: mongodbVersion }, diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 929c8339..3e93a531 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -7,7 +7,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "./inMemoryTransport.js"; import { type UserConfig, config } from "../../src/common/config.js"; -import { type DriverOptions, createDriverOptions } from "../../src/common/config/driverOptions.js"; import { McpError, ResourceUpdatedNotificationSchema } from "@modelcontextprotocol/sdk/types.js"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import type { ConnectionManager, ConnectionState } from "../../src/common/connectionManager.js"; @@ -48,13 +47,10 @@ export const defaultTestConfig: UserConfig = { loggers: ["stderr"], }; -export const defaultDriverOptions: DriverOptions = createDriverOptions(defaultTestConfig); - export const DEFAULT_LONG_RUNNING_TEST_WAIT_TIMEOUT_MS = 1_200_000; export function setupIntegrationTest( getUserConfig: () => UserConfig, - getDriverOptions: () => DriverOptions, { elicitInput, getClientCapabilities, @@ -71,7 +67,6 @@ export function setupIntegrationTest( beforeAll(async () => { const userConfig = getUserConfig(); - const driverOptions = getDriverOptions(); const clientCapabilities = getClientCapabilities?.() ?? (elicitInput ? { elicitation: {} } : {}); const clientTransport = new InMemoryTransport(); @@ -97,7 +92,7 @@ export function setupIntegrationTest( const exportsManager = ExportsManager.init(userConfig, logger); deviceId = DeviceId.create(logger); - const connectionManager = new MCPConnectionManager(userConfig, driverOptions, logger, deviceId); + const connectionManager = new MCPConnectionManager(userConfig, logger, deviceId); const session = new Session({ apiBaseUrl: userConfig.apiBaseUrl, diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index ef741d37..2a3f18b4 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -1,7 +1,6 @@ import { Telemetry } from "../../src/telemetry/telemetry.js"; import { Session } from "../../src/common/session.js"; import { config } from "../../src/common/config.js"; -import { defaultDriverOptions } from "./helpers.js"; import { DeviceId } from "../../src/helpers/deviceId.js"; import { describe, expect, it } from "vitest"; import { CompositeLogger } from "../../src/common/logger.js"; @@ -16,7 +15,7 @@ describe("Telemetry", () => { const deviceId = DeviceId.create(logger); const actualDeviceId = await deviceId.get(); - const connectionManager = new MCPConnectionManager(config, defaultDriverOptions, logger, deviceId); + const connectionManager = new MCPConnectionManager(config, logger, deviceId); const telemetry = Telemetry.create( new Session({ diff --git a/tests/integration/tools/atlas-local/atlasLocalHelpers.ts b/tests/integration/tools/atlas-local/atlasLocalHelpers.ts index 3d3c09a6..4daf37ef 100644 --- a/tests/integration/tools/atlas-local/atlasLocalHelpers.ts +++ b/tests/integration/tools/atlas-local/atlasLocalHelpers.ts @@ -1,4 +1,4 @@ -import { defaultDriverOptions, defaultTestConfig, setupIntegrationTest, type IntegrationTest } from "../../helpers.js"; +import { defaultTestConfig, setupIntegrationTest, type IntegrationTest } from "../../helpers.js"; import { describe } from "vitest"; const isMacOSInGitHubActions = process.platform === "darwin" && process.env.GITHUB_ACTIONS === "true"; @@ -11,10 +11,7 @@ export type IntegrationTestFunction = (integration: IntegrationTest) => void; */ export function describeWithAtlasLocal(name: string, fn: IntegrationTestFunction): void { describe.skipIf(isMacOSInGitHubActions)(name, () => { - const integration = setupIntegrationTest( - () => defaultTestConfig, - () => defaultDriverOptions - ); + const integration = setupIntegrationTest(() => defaultTestConfig); fn(integration); }); } @@ -25,10 +22,7 @@ export function describeWithAtlasLocal(name: string, fn: IntegrationTestFunction */ export function describeWithAtlasLocalDisabled(name: string, fn: IntegrationTestFunction): void { describe.skipIf(!isMacOSInGitHubActions)(name, () => { - const integration = setupIntegrationTest( - () => defaultTestConfig, - () => defaultDriverOptions - ); + const integration = setupIntegrationTest(() => defaultTestConfig); fn(integration); }); } diff --git a/tests/integration/tools/atlas/atlasHelpers.ts b/tests/integration/tools/atlas/atlasHelpers.ts index 308fdc06..7871b47a 100644 --- a/tests/integration/tools/atlas/atlasHelpers.ts +++ b/tests/integration/tools/atlas/atlasHelpers.ts @@ -2,7 +2,7 @@ import { ObjectId } from "mongodb"; import type { ClusterDescription20240805, Group } from "../../../../src/common/atlas/openapi.js"; import type { ApiClient } from "../../../../src/common/atlas/apiClient.js"; import type { IntegrationTest } from "../../helpers.js"; -import { setupIntegrationTest, defaultTestConfig, defaultDriverOptions } from "../../helpers.js"; +import { setupIntegrationTest, defaultTestConfig } from "../../helpers.js"; import type { SuiteCollector } from "vitest"; import { afterAll, beforeAll, describe } from "vitest"; import type { Session } from "../../../../src/common/session.js"; @@ -15,15 +15,12 @@ export function describeWithAtlas(name: string, fn: IntegrationTestFunction): vo ? describe.skip : describe; describeFn(name, () => { - const integration = setupIntegrationTest( - () => ({ - ...defaultTestConfig, - apiClientId: process.env.MDB_MCP_API_CLIENT_ID || "test-client", - apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET || "test-secret", - apiBaseUrl: process.env.MDB_MCP_API_BASE_URL ?? "https://cloud-dev.mongodb.com", - }), - () => defaultDriverOptions - ); + const integration = setupIntegrationTest(() => ({ + ...defaultTestConfig, + apiClientId: process.env.MDB_MCP_API_CLIENT_ID || "test-client", + apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET || "test-secret", + apiBaseUrl: process.env.MDB_MCP_API_BASE_URL ?? "https://cloud-dev.mongodb.com", + })); fn(integration); }); } diff --git a/tests/integration/tools/atlas/performanceAdvisor.test.ts b/tests/integration/tools/atlas/performanceAdvisor.test.ts index f8b5ec24..70816b18 100644 --- a/tests/integration/tools/atlas/performanceAdvisor.test.ts +++ b/tests/integration/tools/atlas/performanceAdvisor.test.ts @@ -5,7 +5,6 @@ import { ObjectId } from "bson"; import type { Session } from "../../../../src/common/session.js"; import { DEFAULT_LONG_RUNNING_TEST_WAIT_TIMEOUT_MS, - defaultDriverOptions, defaultTestConfig, expectDefined, getResponseElements, @@ -132,15 +131,12 @@ describeWithAtlas("performanceAdvisor", (integration) => { }); describe("mocked atlas-get-performance-advisor", () => { - const integration = setupIntegrationTest( - () => ({ - ...defaultTestConfig, - apiClientId: process.env.MDB_MCP_API_CLIENT_ID || "test-client", - apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET || "test-secret", - apiBaseUrl: process.env.MDB_MCP_API_BASE_URL ?? "https://cloud-dev.mongodb.com", - }), - () => defaultDriverOptions - ); + const integration = setupIntegrationTest(() => ({ + ...defaultTestConfig, + apiClientId: process.env.MDB_MCP_API_CLIENT_ID || "test-client", + apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET || "test-secret", + apiBaseUrl: process.env.MDB_MCP_API_BASE_URL ?? "https://cloud-dev.mongodb.com", + })); let mockEmitEvents: MockInstance<(events: BaseEvent[]) => void>; let projectId: string; diff --git a/tests/integration/tools/mongodb/connect/connect.test.ts b/tests/integration/tools/mongodb/connect/connect.test.ts index 132e0fd9..a995d3d4 100644 --- a/tests/integration/tools/mongodb/connect/connect.test.ts +++ b/tests/integration/tools/mongodb/connect/connect.test.ts @@ -6,7 +6,8 @@ import { validateToolMetadata, } from "../../../helpers.js"; import { defaultTestConfig } from "../../../helpers.js"; -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; +import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; describeWithMongoDB( "SwitchConnection tool", @@ -88,6 +89,72 @@ describeWithMongoDB( } ); +describeWithMongoDB( + "SwitchConnection tool when server is configured to connect with complex connection", + (integration) => { + let connectFnSpy: MockInstance; + beforeEach(async () => { + connectFnSpy = vi.spyOn(NodeDriverServiceProvider, "connect"); + await integration.mcpServer().session.connectToMongoDB({ + connectionString: integration.connectionString(), + }); + }); + + it("should be able to connect to next connection and not use the connect options of the connection setup during server boot", async () => { + const newConnectionString = `${integration.connectionString()}`; + // Note: The connect function is called with OIDC options for the + // configured string + expect(connectFnSpy).toHaveBeenNthCalledWith( + 1, + expect.stringContaining(`${integration.connectionString()}/?directConnection=true`), + expect.objectContaining({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + oidc: expect.objectContaining({ openBrowser: { command: "not-a-browser" } }), + }), + undefined, + expect.anything() + ); + const response = await integration.mcpClient().callTool({ + name: "switch-connection", + arguments: { + connectionString: newConnectionString, + }, + }); + + const content = getResponseContent(response.content); + // The connection will still be connected because the --browser + // option only sets the command to be used when opening the browser + // for OIDC handling. + expect(content).toContain("Successfully connected"); + + // Now that we're connected lets verify the config + // Note: The connect function is called with OIDC options for the + // configured string + expect(connectFnSpy).toHaveBeenNthCalledWith( + 2, + expect.stringContaining(`${integration.connectionString()}`), + expect.not.objectContaining({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + oidc: expect.objectContaining({ openBrowser: { command: "not-a-browser" } }), + }), + undefined, + expect.anything() + ); + }); + }, + { + getUserConfig: (mdbIntegration) => ({ + ...defaultTestConfig, + // Setting browser in config is the same as passing `--browser` CLI + // argument to the MCP server CLI entry point. We expect that the + // further connection attempts stay detached from the connection + // options passed during server boot, in this case browser. + browser: "not-a-browser", + connectionString: `${mdbIntegration.connectionString()}/?directConnection=true`, + }), + } +); + describeWithMongoDB("Connect tool", (integration) => { validateToolMetadata( integration, diff --git a/tests/integration/tools/mongodb/mongodbHelpers.ts b/tests/integration/tools/mongodb/mongodbHelpers.ts index be654100..850dd7d5 100644 --- a/tests/integration/tools/mongodb/mongodbHelpers.ts +++ b/tests/integration/tools/mongodb/mongodbHelpers.ts @@ -8,11 +8,9 @@ import { getResponseContent, setupIntegrationTest, defaultTestConfig, - defaultDriverOptions, getDataFromUntrustedContent, } from "../../helpers.js"; import type { UserConfig } from "../../../../src/common/config.js"; -import type { DriverOptions } from "../../../../src/common/config/driverOptions.js"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { EJSON } from "bson"; import { MongoDBClusterProcess } from "./mongodbClusterProcess.js"; @@ -73,7 +71,6 @@ export type MongoSearchConfiguration = { search: true; image?: string }; export type TestSuiteConfig = { getUserConfig: (mdbIntegration: MongoDBIntegrationTest) => UserConfig; - getDriverOptions: (mdbIntegration: MongoDBIntegrationTest) => DriverOptions; downloadOptions: MongoClusterConfiguration; getMockElicitationInput?: () => ReturnType; getClientCapabilities?: () => MockClientCapabilities; @@ -81,7 +78,6 @@ export type TestSuiteConfig = { export const defaultTestSuiteConfig: TestSuiteConfig = { getUserConfig: () => defaultTestConfig, - getDriverOptions: () => defaultDriverOptions, downloadOptions: DEFAULT_MONGODB_PROCESS_OPTIONS, }; @@ -90,7 +86,7 @@ export function describeWithMongoDB( fn: (integration: MongoDBIntegrationTestCase) => void, partialTestSuiteConfig?: Partial ): void { - const { getUserConfig, getDriverOptions, downloadOptions, getMockElicitationInput, getClientCapabilities } = { + const { getUserConfig, downloadOptions, getMockElicitationInput, getClientCapabilities } = { ...defaultTestSuiteConfig, ...partialTestSuiteConfig, }; @@ -101,9 +97,6 @@ export function describeWithMongoDB( () => ({ ...getUserConfig(mdbIntegration), }), - () => ({ - ...getDriverOptions(mdbIntegration), - }), { elicitInput: mockElicitInput, getClientCapabilities } ); diff --git a/tests/integration/tools/mongodb/mongodbTool.test.ts b/tests/integration/tools/mongodb/mongodbTool.test.ts index 57352844..fb512c2f 100644 --- a/tests/integration/tools/mongodb/mongodbTool.test.ts +++ b/tests/integration/tools/mongodb/mongodbTool.test.ts @@ -5,7 +5,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { MongoDBToolBase } from "../../../../src/tools/mongodb/mongodbTool.js"; import { type ToolBase, type ToolConstructorParams, type OperationType } from "../../../../src/tools/tool.js"; import { type UserConfig } from "../../../../src/common/config.js"; -import { defaultDriverOptions } from "../../../../src/common/config/driverOptions.js"; import { MCPConnectionManager } from "../../../../src/common/connectionManager.js"; import { Session } from "../../../../src/common/session.js"; import { CompositeLogger } from "../../../../src/common/logger.js"; @@ -95,11 +94,10 @@ describe("MongoDBTool implementations", () => { ): Promise { await cleanup(); const userConfig: UserConfig = { ...defaultTestConfig, telemetry: "disabled", ...config }; - const driverOptions = defaultDriverOptions; const logger = new CompositeLogger(); const exportsManager = ExportsManager.init(userConfig, logger); deviceId = DeviceId.create(logger); - const connectionManager = new MCPConnectionManager(userConfig, driverOptions, logger, deviceId); + const connectionManager = new MCPConnectionManager(userConfig, logger, deviceId); const session = new Session({ apiBaseUrl: userConfig.apiBaseUrl, apiClientId: userConfig.apiClientId, diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts index 6e4d9331..c4a1c370 100644 --- a/tests/unit/common/session.test.ts +++ b/tests/unit/common/session.test.ts @@ -3,7 +3,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { Session } from "../../../src/common/session.js"; import { config } from "../../../src/common/config.js"; -import { defaultDriverOptions } from "../../integration/helpers.js"; import { CompositeLogger } from "../../../src/common/logger.js"; import { MCPConnectionManager } from "../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../src/common/exportsManager.js"; @@ -25,7 +24,7 @@ describe("Session", () => { const logger = new CompositeLogger(); mockDeviceId = MockDeviceId; - const connectionManager = new MCPConnectionManager(config, defaultDriverOptions, logger, mockDeviceId); + const connectionManager = new MCPConnectionManager(config, logger, mockDeviceId); session = new Session({ apiClientId: "test-client-id", diff --git a/tests/unit/resources/common/debug.test.ts b/tests/unit/resources/common/debug.test.ts index e00ce4f2..db4024fd 100644 --- a/tests/unit/resources/common/debug.test.ts +++ b/tests/unit/resources/common/debug.test.ts @@ -3,7 +3,6 @@ import { DebugResource } from "../../../../src/resources/common/debug.js"; import { Session } from "../../../../src/common/session.js"; import { Telemetry } from "../../../../src/telemetry/telemetry.js"; import { config } from "../../../../src/common/config.js"; -import { defaultDriverOptions } from "../../../integration/helpers.js"; import { CompositeLogger } from "../../../../src/common/logger.js"; import { MCPConnectionManager } from "../../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../../src/common/exportsManager.js"; @@ -14,7 +13,7 @@ import { VectorSearchEmbeddingsManager } from "../../../../src/common/search/vec describe("debug resource", () => { const logger = new CompositeLogger(); const deviceId = DeviceId.create(logger); - const connectionManager = new MCPConnectionManager(config, defaultDriverOptions, logger, deviceId); + const connectionManager = new MCPConnectionManager(config, logger, deviceId); const session = vi.mocked( new Session({ From 721885ead2823d62b3cd97d2fa6fbba8e2ce11f7 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 12:16:55 +0100 Subject: [PATCH 05/16] chore: remove driverOptions related code --- src/common/config/driverOptions.ts | 27 --------------------------- 1 file changed, 27 deletions(-) delete mode 100644 src/common/config/driverOptions.ts diff --git a/src/common/config/driverOptions.ts b/src/common/config/driverOptions.ts deleted file mode 100644 index 1a9a0a6d..00000000 --- a/src/common/config/driverOptions.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { generateConnectionInfoFromCliArgs, type ConnectionInfo } from "@mongosh/arg-parser"; -import type { UserConfig } from "../config.js"; - -export type DriverOptions = ConnectionInfo["driverOptions"]; -export const defaultDriverOptions: DriverOptions = { - readConcern: { - level: "local", - }, - readPreference: "secondaryPreferred", - writeConcern: { - w: "majority", - }, - timeoutMS: 30_000, - proxy: { useEnvironmentVariableProxies: true }, - applyProxyToOIDC: true, -}; - -export function createDriverOptions( - config: UserConfig, - defaults: Partial = defaultDriverOptions -): DriverOptions { - const { driverOptions } = generateConnectionInfoFromCliArgs(config); - return { - ...defaults, - ...driverOptions, - }; -} From 7587a797e689e74a8a00bbcfdc8b4bab9d50a0a0 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 14:22:54 +0100 Subject: [PATCH 06/16] chore: fix OIDC tests --- .../common/connectionManager.oidc.test.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/integration/common/connectionManager.oidc.test.ts b/tests/integration/common/connectionManager.oidc.test.ts index 729c5012..f6fd9fee 100644 --- a/tests/integration/common/connectionManager.oidc.test.ts +++ b/tests/integration/common/connectionManager.oidc.test.ts @@ -1,3 +1,4 @@ +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; import type { TestContext } from "vitest"; import { describe, beforeEach, afterAll, it, expect, vi } from "vitest"; import semver from "semver"; @@ -134,7 +135,19 @@ describe.skipIf(process.platform !== "linux")("ConnectionManager OIDC Tests", as // state of the connection manager connectionManager.changeState("connection-close", { tag: "disconnected" }); - await integration.connectMcpClient(); + // Note: Instead of using `integration.connectMcpClient`, + // we're connecting straight using Session because + // `integration.connectMcpClient` uses `connect` tool which + // does not work the same way as connect on server start up. + // So to mimic the same functionality as that of server + // startup we call the connectToMongoDB the same way as the + // `Server.connectToConfigConnectionString` does. + await integration.mcpServer().session.connectToMongoDB( + generateConnectionInfoFromCliArgs({ + ...oidcConfig, + connectionSpecifier: integration.connectionString(), + }) + ); }, DEFAULT_TIMEOUT); addCb?.(oidcIt); From c4cdb85f745542d002164ba9cc199751a5a86f46 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 22:56:01 +0100 Subject: [PATCH 07/16] chore: use yargs-parser to parse both CLI and ENV Earlier we had custom code to parse ENV variable and although we were using yargs-parser to parse CLI arguments, we were still using custom logic to validate the parsed keys. This commit removes the custom code entirely in favor of using yargs-parser to parse both CLI and ENV variables while also taking care of excluding unknown arguments. This helps remove a bunch of code that we did not needed in the first place. Additionally, it fixes one of the problems with positional argument parsing. Earlier we were considering a positional argument as connection specifier only if the deprecated flag `--connectionString` was also passed. Now we consider the positional argument on top priority, disregarding ENV variable, CLI arguments, etc. This commit also splits the UserConfig schema and creation of UserConfig for CLI entry point in two different files so that the UserConfig export on the library don't end up pulling yargs-parser along. --- scripts/generateArguments.ts | 2 +- src/common/atlas/roles.ts | 2 +- src/common/config.ts | 402 ---------- src/common/config/configUtils.ts | 64 +- src/common/config/createUserConfig.ts | 197 +++++ src/common/config/userConfig.ts | 173 +++++ src/common/connectionManager.ts | 2 +- src/common/exportsManager.ts | 2 +- src/common/search/embeddingsProvider.ts | 2 +- .../search/vectorSearchEmbeddingsManager.ts | 2 +- src/index.ts | 14 +- src/lib.ts | 2 +- src/resources/common/config.ts | 2 +- src/resources/resource.ts | 2 +- src/server.ts | 2 +- src/telemetry/telemetry.ts | 2 +- src/tools/tool.ts | 2 +- src/transports/base.ts | 2 +- .../common/connectionManager.oidc.test.ts | 2 +- .../common/connectionManager.test.ts | 2 +- tests/integration/elicitation.test.ts | 2 +- tests/integration/helpers.ts | 5 +- tests/integration/telemetry.test.ts | 4 +- .../tools/mongodb/mongodbHelpers.ts | 2 +- .../tools/mongodb/mongodbTool.test.ts | 2 +- .../transports/streamableHttp.test.ts | 22 +- tests/unit/common/config.test.ts | 703 ++++++++++-------- tests/unit/common/exportsManager.test.ts | 7 +- tests/unit/common/roles.test.ts | 2 +- tests/unit/common/session.test.ts | 8 +- tests/unit/resources/common/debug.test.ts | 14 +- tests/unit/telemetry.test.ts | 6 +- tests/unit/toolBase.test.ts | 2 +- 33 files changed, 838 insertions(+), 821 deletions(-) delete mode 100644 src/common/config.ts create mode 100644 src/common/config/createUserConfig.ts create mode 100644 src/common/config/userConfig.ts diff --git a/scripts/generateArguments.ts b/scripts/generateArguments.ts index 69d10bc4..04ff001e 100644 --- a/scripts/generateArguments.ts +++ b/scripts/generateArguments.ts @@ -11,7 +11,7 @@ import { readFileSync, writeFileSync } from "fs"; import { join, dirname } from "path"; import { fileURLToPath } from "url"; -import { UserConfigSchema, configRegistry } from "../src/common/config.js"; +import { UserConfigSchema, configRegistry } from "../src/common/config/userConfig.js"; import assert from "assert"; import { execSync } from "child_process"; import { OPTIONS } from "../src/common/config/argsParserOptions.js"; diff --git a/src/common/atlas/roles.ts b/src/common/atlas/roles.ts index 5d776c75..62f093a0 100644 --- a/src/common/atlas/roles.ts +++ b/src/common/atlas/roles.ts @@ -1,4 +1,4 @@ -import type { UserConfig } from "../config.js"; +import type { UserConfig } from "../config/userConfig.js"; import type { DatabaseUserRole } from "./openapi.js"; const readWriteRole: DatabaseUserRole = { diff --git a/src/common/config.ts b/src/common/config.ts deleted file mode 100644 index 35b4616c..00000000 --- a/src/common/config.ts +++ /dev/null @@ -1,402 +0,0 @@ -import argv from "yargs-parser"; -import type { CliOptions } from "@mongosh/arg-parser"; -import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; -import { Keychain } from "./keychain.js"; -import type { Secret } from "./keychain.js"; -import { z as z4 } from "zod/v4"; -import { - commaSeparatedToArray, - type ConfigFieldMeta, - getExportsPath, - getLogPath, - isConnectionSpecifier, - validateConfigKey, -} from "./config/configUtils.js"; -import { OPTIONS } from "./config/argsParserOptions.js"; -import { similarityValues, previewFeatureValues } from "./schemas.js"; - -export const configRegistry = z4.registry(); - -export const UserConfigSchema = z4.object({ - apiBaseUrl: z4.string().default("https://cloud.mongodb.com/"), - apiClientId: z4 - .string() - .optional() - .describe("Atlas API client ID for authentication. Required for running Atlas tools.") - .register(configRegistry, { isSecret: true }), - apiClientSecret: z4 - .string() - .optional() - .describe("Atlas API client secret for authentication. Required for running Atlas tools.") - .register(configRegistry, { isSecret: true }), - connectionString: z4 - .string() - .optional() - .describe( - "MongoDB connection string for direct database connections. Optional, if not set, you'll need to call the connect tool before interacting with MongoDB data." - ) - .register(configRegistry, { isSecret: true }), - loggers: z4 - .preprocess( - (val: string | string[] | undefined) => commaSeparatedToArray(val), - z4.array(z4.enum(["stderr", "disk", "mcp"])) - ) - .check( - z4.minLength(1, "Cannot be an empty array"), - z4.refine((val) => new Set(val).size === val.length, { - message: "Duplicate loggers found in config", - }) - ) - .default(["disk", "mcp"]) - .describe("An array of logger types.") - .register(configRegistry, { - defaultValueDescription: '`"disk,mcp"` see below*', - }), - logPath: z4 - .string() - .default(getLogPath()) - .describe("Folder to store logs.") - .register(configRegistry, { defaultValueDescription: "see below*" }), - disabledTools: z4 - .preprocess((val: string | string[] | undefined) => commaSeparatedToArray(val), z4.array(z4.string())) - .default([]) - .describe("An array of tool names, operation types, and/or categories of tools that will be disabled."), - confirmationRequiredTools: z4 - .preprocess((val: string | string[] | undefined) => commaSeparatedToArray(val), z4.array(z4.string())) - .default([ - "atlas-create-access-list", - "atlas-create-db-user", - "drop-database", - "drop-collection", - "delete-many", - "drop-index", - ]) - .describe( - "An array of tool names that require user confirmation before execution. Requires the client to support elicitation." - ), - readOnly: z4 - .boolean() - .default(false) - .describe( - "When set to true, only allows read, connect, and metadata operation types, disabling create/update/delete operations." - ), - indexCheck: z4 - .boolean() - .default(false) - .describe( - "When set to true, enforces that query operations must use an index, rejecting queries that perform a collection scan." - ), - telemetry: z4 - .enum(["enabled", "disabled"]) - .default("enabled") - .describe("When set to disabled, disables telemetry collection."), - transport: z4.enum(["stdio", "http"]).default("stdio").describe("Either 'stdio' or 'http'."), - httpPort: z4.coerce - .number() - .int() - .min(1, "Invalid httpPort: must be at least 1") - .max(65535, "Invalid httpPort: must be at most 65535") - .default(3000) - .describe("Port number for the HTTP server (only used when transport is 'http')."), - httpHost: z4 - .string() - .default("127.0.0.1") - .describe("Host address to bind the HTTP server to (only used when transport is 'http')."), - httpHeaders: z4 - .object({}) - .passthrough() - .default({}) - .describe( - "Header that the HTTP server will validate when making requests (only used when transport is 'http')." - ), - idleTimeoutMs: z4.coerce - .number() - .default(600_000) - .describe("Idle timeout for a client to disconnect (only applies to http transport)."), - notificationTimeoutMs: z4.coerce - .number() - .default(540_000) - .describe("Notification timeout for a client to be aware of disconnect (only applies to http transport)."), - maxBytesPerQuery: z4.coerce - .number() - .default(16_777_216) - .describe( - "The maximum size in bytes for results from a find or aggregate tool call. This serves as an upper bound for the responseBytesLimit parameter in those tools." - ), - maxDocumentsPerQuery: z4.coerce - .number() - .default(100) - .describe( - "The maximum number of documents that can be returned by a find or aggregate tool call. For the find tool, the effective limit will be the smaller of this value and the tool's limit parameter." - ), - exportsPath: z4 - .string() - .default(getExportsPath()) - .describe("Folder to store exported data files.") - .register(configRegistry, { defaultValueDescription: "see below*" }), - exportTimeoutMs: z4.coerce - .number() - .default(300_000) - .describe("Time in milliseconds after which an export is considered expired and eligible for cleanup."), - exportCleanupIntervalMs: z4.coerce - .number() - .default(120_000) - .describe("Time in milliseconds between export cleanup cycles that remove expired export files."), - atlasTemporaryDatabaseUserLifetimeMs: z4.coerce - .number() - .default(14_400_000) - .describe( - "Time in milliseconds that temporary database users created when connecting to MongoDB Atlas clusters will remain active before being automatically deleted." - ), - voyageApiKey: z4 - .string() - .default("") - .describe( - "API key for Voyage AI embeddings service (required for vector search operations with text-to-embedding conversion)." - ) - .register(configRegistry, { isSecret: true }), - disableEmbeddingsValidation: z4 - .boolean() - .default(false) - .describe("When set to true, disables validation of embeddings dimensions."), - vectorSearchDimensions: z4.coerce - .number() - .default(1024) - .describe("Default number of dimensions for vector search embeddings."), - vectorSearchSimilarityFunction: z4 - .enum(similarityValues) - .default("euclidean") - .describe("Default similarity function for vector search: 'euclidean', 'cosine', or 'dotProduct'."), - previewFeatures: z4 - .preprocess( - (val: string | string[] | undefined) => commaSeparatedToArray(val), - z4.array(z4.enum(previewFeatureValues)) - ) - .default([]) - .describe("An array of preview features that are enabled."), -}); - -export type UserConfig = z4.infer & CliOptions; - -export const config = setupUserConfig({ - cli: process.argv, - env: process.env, -}); - -// Gets the config supplied by the user as environment variables. The variable names -// are prefixed with `MDB_MCP_` and the keys match the UserConfig keys, but are converted -// to SNAKE_UPPER_CASE. -function parseEnvConfig(env: Record): Partial { - const CONFIG_WITH_URLS: Set = new Set<(typeof OPTIONS)["string"][number]>(["connectionString"]); - - function setValue( - obj: Record | undefined>, - path: string[], - value: string - ): void { - const currentField = path.shift(); - if (!currentField) { - return; - } - if (path.length === 0) { - if (CONFIG_WITH_URLS.has(currentField)) { - obj[currentField] = value; - return; - } - - const numberValue = Number(value); - if (!isNaN(numberValue)) { - obj[currentField] = numberValue; - return; - } - - const booleanValue = value.toLocaleLowerCase(); - if (booleanValue === "true" || booleanValue === "false") { - obj[currentField] = booleanValue === "true"; - return; - } - - // Try to parse an array of values - if (value.indexOf(",") !== -1) { - obj[currentField] = value.split(",").map((v) => v.trim()); - return; - } - - obj[currentField] = value; - return; - } - - if (!obj[currentField]) { - obj[currentField] = {}; - } - - setValue(obj[currentField] as Record, path, value); - } - - const result: Record = {}; - const mcpVariables = Object.entries(env).filter( - ([key, value]) => value !== undefined && key.startsWith("MDB_MCP_") - ) as [string, string][]; - for (const [key, value] of mcpVariables) { - const fieldPath = key - .replace("MDB_MCP_", "") - .split(".") - .map((part) => SNAKE_CASE_toCamelCase(part)); - - setValue(result, fieldPath, value); - } - - return result; -} - -function SNAKE_CASE_toCamelCase(str: string): string { - return str.toLowerCase().replace(/([-_][a-z])/g, (group) => group.toUpperCase().replace("_", "")); -} - -// Right now we have arguments that are not compatible with the format used in mongosh. -// An example is using --connectionString and positional arguments. -// We will consolidate them in a way where the mongosh format takes precedence. -// We will warn users that previous configuration is deprecated in favour of -// whatever is in mongosh. -function parseCliConfig(args: string[]): Partial> { - const programArgs = args.slice(2); - const parsed = argv(programArgs, OPTIONS as unknown as argv.Options) as unknown as Record< - keyof CliOptions, - string | number | undefined - > & { - _?: string[]; - }; - - const positionalArguments = parsed._ ?? []; - - // we use console.warn here because we still don't have our logging system configured - // so we don't have a logger. For stdio, the warning will be received as a string in - // the client and IDEs like VSCode do show the message in the log window. For HTTP, - // it will be in the stdout of the process. - warnAboutDeprecatedOrUnknownCliArgs( - { ...parsed, _: positionalArguments }, - { - warn: (msg) => console.warn(msg), - exit: (status) => process.exit(status), - } - ); - - // if we have a positional argument that matches a connection string - // store it as the connection specifier and remove it from the argument - // list, so it doesn't get misunderstood by the mongosh args-parser - if (!parsed.nodb && isConnectionSpecifier(positionalArguments[0])) { - parsed.connectionSpecifier = positionalArguments.shift(); - } - - delete parsed._; - return parsed; -} - -export function warnAboutDeprecatedOrUnknownCliArgs( - args: Record, - { warn, exit }: { warn: (msg: string) => void; exit: (status: number) => void | never } -): void { - let usedDeprecatedArgument = false; - let usedInvalidArgument = false; - - const knownArgs = args as unknown as UserConfig & CliOptions; - // the first position argument should be used - // instead of --connectionString, as it's how the mongosh works. - if (knownArgs.connectionString) { - usedDeprecatedArgument = true; - warn( - "Warning: The --connectionString argument is deprecated. Prefer using the MDB_MCP_CONNECTION_STRING environment variable or the first positional argument for the connection string." - ); - } - - for (const providedKey of Object.keys(args)) { - if (providedKey === "_") { - // positional argument - continue; - } - - const { valid, suggestion } = validateConfigKey(providedKey); - if (!valid) { - usedInvalidArgument = true; - if (suggestion) { - warn(`Warning: Invalid command line argument '${providedKey}'. Did you mean '${suggestion}'?`); - } else { - warn(`Warning: Invalid command line argument '${providedKey}'.`); - } - } - } - - if (usedInvalidArgument || usedDeprecatedArgument) { - warn("- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server."); - } - - if (usedInvalidArgument) { - exit(1); - } -} - -export function registerKnownSecretsInRootKeychain(userConfig: Partial): void { - const keychain = Keychain.root; - - const maybeRegister = (value: string | undefined, kind: Secret["kind"]): void => { - if (value) { - keychain.register(value, kind); - } - }; - - maybeRegister(userConfig.apiClientId, "user"); - maybeRegister(userConfig.apiClientSecret, "password"); - maybeRegister(userConfig.awsAccessKeyId, "password"); - maybeRegister(userConfig.awsIamSessionToken, "password"); - maybeRegister(userConfig.awsSecretAccessKey, "password"); - maybeRegister(userConfig.awsSessionToken, "password"); - maybeRegister(userConfig.password, "password"); - maybeRegister(userConfig.tlsCAFile, "url"); - maybeRegister(userConfig.tlsCRLFile, "url"); - maybeRegister(userConfig.tlsCertificateKeyFile, "url"); - maybeRegister(userConfig.tlsCertificateKeyFilePassword, "password"); - maybeRegister(userConfig.username, "user"); -} - -export function warnIfVectorSearchNotEnabledCorrectly(config: UserConfig, warn: (message: string) => void): void { - const vectorSearchEnabled = config.previewFeatures.includes("vectorSearch"); - const embeddingsProviderConfigured = !!config.voyageApiKey; - if (vectorSearchEnabled && !embeddingsProviderConfigured) { - warn(`\ -Warning: Vector search is enabled but no embeddings provider is configured. -- Set an embeddings provider configuration option to enable auto-embeddings during document insertion and text-based queries with $vectorSearch.\ -`); - } - - if (!vectorSearchEnabled && embeddingsProviderConfigured) { - warn(`\ -Warning: An embeddings provider is configured but the 'vectorSearch' preview feature is not enabled. -- Enable vector search by adding 'vectorSearch' to the 'previewFeatures' configuration option, or remove the embeddings provider configuration if not needed.\ -`); - } -} - -export function setupUserConfig({ cli, env }: { cli: string[]; env: Record }): UserConfig { - const rawConfig = { - ...parseEnvConfig(env), - ...parseCliConfig(cli), - }; - - if (rawConfig.connectionString && rawConfig.connectionSpecifier) { - const connectionInfo = generateConnectionInfoFromCliArgs(rawConfig as UserConfig); - rawConfig.connectionString = connectionInfo.connectionString; - } - - const parseResult = UserConfigSchema.safeParse(rawConfig); - if (parseResult.error) { - throw new Error( - `Invalid configuration for the following fields:\n${parseResult.error.issues.map((issue) => `${issue.path.join(".")} - ${issue.message}`).join("\n")}` - ); - } - // We don't have as schema defined for all args-parser arguments so we need to merge the raw config with the parsed config. - const userConfig = { ...rawConfig, ...parseResult.data } as UserConfig; - - warnIfVectorSearchNotEnabledCorrectly(userConfig, (message) => console.warn(message)); - registerKnownSecretsInRootKeychain(userConfig); - return userConfig; -} diff --git a/src/common/config/configUtils.ts b/src/common/config/configUtils.ts index 47285152..a13e4b66 100644 --- a/src/common/config/configUtils.ts +++ b/src/common/config/configUtils.ts @@ -4,35 +4,37 @@ import { ALL_CONFIG_KEYS } from "./argsParserOptions.js"; import * as levenshteinModule from "ts-levenshtein"; const levenshtein = levenshteinModule.default; -export function validateConfigKey(key: string): { valid: boolean; suggestion?: string } { - if (ALL_CONFIG_KEYS.has(key)) { - return { valid: true }; - } +/** + * Metadata for config schema fields. + */ +export type ConfigFieldMeta = { + /** + * Custom description for the default value, used when generating documentation. + */ + defaultValueDescription?: string; + /** + * Marks the field as containing sensitive/secret information, used for MCP Registry. + * Secret fields will be marked as secret in environment variable definitions. + */ + isSecret?: boolean; - let minLev = Number.MAX_VALUE; - let suggestion = ""; + [key: string]: unknown; +}; - // find the closest match for a suggestion +export function matchingConfigKey(key: string): string | undefined { + let minLev = Number.MAX_VALUE; + let suggestion = undefined; for (const validKey of ALL_CONFIG_KEYS) { - // check if there is an exact case-insensitive match - if (validKey.toLowerCase() === key.toLowerCase()) { - return { valid: false, suggestion: validKey }; - } - - // else, infer something using levenshtein so we suggest a valid key const lev = levenshtein.get(key, validKey); - if (lev < minLev) { + // Accepting upto 2 typos and should be better than whatever previous + // suggestion was. + if (lev <= 2 && lev < minLev) { minLev = lev; suggestion = validKey; } } - if (minLev <= 2) { - // accept up to 2 typos - return { valid: false, suggestion }; - } - - return { valid: false }; + return suggestion; } export function isConnectionSpecifier(arg: string | undefined): boolean { @@ -40,27 +42,13 @@ export function isConnectionSpecifier(arg: string | undefined): boolean { arg !== undefined && (arg.startsWith("mongodb://") || arg.startsWith("mongodb+srv://") || - !(arg.endsWith(".js") || arg.endsWith(".mongodb"))) + // Strings starting with a hyphen `-` are generally a sign of CLI + // flag so we exclude them from the possibility of being a + // connection specifier. + !(arg.endsWith(".js") || arg.endsWith(".mongodb") || arg.startsWith("-"))) ); } -/** - * Metadata for config schema fields. - */ -export type ConfigFieldMeta = { - /** - * Custom description for the default value, used when generating documentation. - */ - defaultValueDescription?: string; - /** - * Marks the field as containing sensitive/secret information, used for MCP Registry. - * Secret fields will be marked as secret in environment variable definitions. - */ - isSecret?: boolean; - - [key: string]: unknown; -}; - export function getLocalDataPath(): string { return process.platform === "win32" ? path.join(process.env.LOCALAPPDATA || process.env.APPDATA || os.homedir(), "mongodb") diff --git a/src/common/config/createUserConfig.ts b/src/common/config/createUserConfig.ts new file mode 100644 index 00000000..196d2f50 --- /dev/null +++ b/src/common/config/createUserConfig.ts @@ -0,0 +1,197 @@ +import argv from "yargs-parser"; +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; +import { Keychain } from "../keychain.js"; +import type { Secret } from "../keychain.js"; +import { isConnectionSpecifier, matchingConfigKey } from "./configUtils.js"; +import { OPTIONS } from "./argsParserOptions.js"; +import { UserConfigSchema, type UserConfig } from "./userConfig.js"; + +export type CreateUserConfigHelpers = { + onWarning: (message: string) => void; + onError: (message: string) => void; + closeProcess: (exitCode: number) => never; + cliArguments: string[]; +}; + +export const defaultUserConfigHelpers: CreateUserConfigHelpers = { + onWarning(message) { + console.warn(message); + }, + onError(message) { + console.error(message); + }, + closeProcess(exitCode) { + process.exit(exitCode); + }, + cliArguments: process.argv.slice(2), +}; + +export function createUserConfig({ + onWarning = defaultUserConfigHelpers.onWarning, + onError = defaultUserConfigHelpers.onError, + closeProcess = defaultUserConfigHelpers.closeProcess, + cliArguments = defaultUserConfigHelpers.cliArguments, +}: Partial = defaultUserConfigHelpers): UserConfig { + const { unknownCliArgumentErrors, deprecatedCliArgumentWarnings, userAndArgsParserConfig, connectionSpecifier } = + parseUserConfigSources(cliArguments); + + if (unknownCliArgumentErrors.length) { + const errorMessage = ` +${unknownCliArgumentErrors.join("\n")} +- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server. +`; + onError(errorMessage); + return closeProcess(1); + } + + if (deprecatedCliArgumentWarnings.length) { + const deprecatedMessages = ` +${deprecatedCliArgumentWarnings.join("\n")} +- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server. +`; + onWarning(deprecatedMessages); + } + + // If we have a connectionSpecifier, which can only appear as the positional + // argument, then that has to be used on priority to construct the + // connection string. In this case, if there is a connection string provided + // by the env variable or config file, that will be overridden. + if (connectionSpecifier) { + const connectionInfo = generateConnectionInfoFromCliArgs({ ...userAndArgsParserConfig, connectionSpecifier }); + userAndArgsParserConfig.connectionString = connectionInfo.connectionString; + } + + const configParseResult = UserConfigSchema.safeParse(userAndArgsParserConfig); + if (configParseResult.error) { + onError( + `Invalid configuration for the following fields:\n${configParseResult.error.issues.map((issue) => `${issue.path.join(".")} - ${issue.message}`).join("\n")}` + ); + return closeProcess(1); + } + + // TODO: Separate correctly parsed user config from all other valid + // arguments relevant to mongosh's args-parser. + const userConfig: UserConfig = { ...userAndArgsParserConfig, ...configParseResult.data }; + warnIfVectorSearchNotEnabledCorrectly(userConfig, onWarning); + registerKnownSecretsInRootKeychain(userConfig); + return userConfig; +} + +function parseUserConfigSources(cliArguments: string[]): { + unknownCliArgumentErrors: string[]; + deprecatedCliArgumentWarnings: string[]; + userAndArgsParserConfig: Record; + connectionSpecifier: string | undefined; +} { + const { + _: positionalAndUnknownArguments, + "--": endOfFlagArguments, + ...parsedUserAndArgsParserConfig + } = argv(cliArguments, { + ...OPTIONS, + // This helps parse the relevant environment variables. + envPrefix: "MDB_MCP_", + configuration: { + ...OPTIONS.configuration, + // Setting this to true will populate `_` variable which is + // originally used for positional arguments, now with the unknown + // arguments as well. The order of arguments are maintained. + "unknown-options-as-args": true, + // To avoid populating `_` with end-of-flag arguments we explicitly + // populate `--` variable and altogether ignore them later. + "populate--": true, + }, + }); + + // We don't make use of end of flag arguments but also don't want them to + // end up alongside unknown arguments so we are extracting them and having a + // no-op statement so ESLint does not complain. + void endOfFlagArguments; + + const [maybeConnectionSpecifier, ...unknownArguments] = positionalAndUnknownArguments; + // If the extracted connection specifier is not a connection specifier + // indeed, then we push it back to the unknown arguments list. This might + // happen for example when an unknown argument is provided without ever + // specifying a positional argument. + if (typeof maybeConnectionSpecifier !== "string" || !isConnectionSpecifier(maybeConnectionSpecifier)) { + if (maybeConnectionSpecifier) { + unknownArguments.unshift(maybeConnectionSpecifier); + } + } + + return { + unknownCliArgumentErrors: unknownArguments + .filter((argument): argument is string => typeof argument === "string" && argument.startsWith("--")) + .map((argument) => { + const argumentKey = argument.replace(/^(--)/, ""); + const matchingKey = matchingConfigKey(argumentKey); + if (matchingKey) { + return `Error: Invalid command line argument '${argument}'. Did you mean '--${matchingKey}'?`; + } + + return `Error: Invalid command line argument '${argument}'.`; + }), + deprecatedCliArgumentWarnings: cliArguments + .filter((argument) => argument.startsWith("--connectionString")) + .map((argument) => { + if (argument.startsWith("--connectionString")) { + return "Warning: The --connectionString argument is deprecated. Prefer using the MDB_MCP_CONNECTION_STRING environment variable or the first positional argument for the connection string."; + } + + return undefined; + }) + .filter((argument) => typeof argument === "string"), + userAndArgsParserConfig: parsedUserAndArgsParserConfig, + // A connectionSpecifier can be one of: + // - database name + // - host name + // - ip address + // - replica set specifier + // - complete connection string + connectionSpecifier: + typeof maybeConnectionSpecifier === "string" && isConnectionSpecifier(maybeConnectionSpecifier) + ? String(maybeConnectionSpecifier) + : undefined, + }; +} + +function registerKnownSecretsInRootKeychain(userConfig: Partial): void { + const keychain = Keychain.root; + + const maybeRegister = (value: string | undefined, kind: Secret["kind"]): void => { + if (value) { + keychain.register(value, kind); + } + }; + + maybeRegister(userConfig.apiClientId, "user"); + maybeRegister(userConfig.apiClientSecret, "password"); + maybeRegister(userConfig.awsAccessKeyId, "password"); + maybeRegister(userConfig.awsIamSessionToken, "password"); + maybeRegister(userConfig.awsSecretAccessKey, "password"); + maybeRegister(userConfig.awsSessionToken, "password"); + maybeRegister(userConfig.password, "password"); + maybeRegister(userConfig.tlsCAFile, "url"); + maybeRegister(userConfig.tlsCRLFile, "url"); + maybeRegister(userConfig.tlsCertificateKeyFile, "url"); + maybeRegister(userConfig.tlsCertificateKeyFilePassword, "password"); + maybeRegister(userConfig.username, "user"); +} + +function warnIfVectorSearchNotEnabledCorrectly(config: UserConfig, warn: (message: string) => void): void { + const vectorSearchEnabled = config.previewFeatures.includes("vectorSearch"); + const embeddingsProviderConfigured = !!config.voyageApiKey; + if (vectorSearchEnabled && !embeddingsProviderConfigured) { + warn(`\ +Warning: Vector search is enabled but no embeddings provider is configured. +- Set an embeddings provider configuration option to enable auto-embeddings during document insertion and text-based queries with $vectorSearch.\ +`); + } + + if (!vectorSearchEnabled && embeddingsProviderConfigured) { + warn(`\ +Warning: An embeddings provider is configured but the 'vectorSearch' preview feature is not enabled. +- Enable vector search by adding 'vectorSearch' to the 'previewFeatures' configuration option, or remove the embeddings provider configuration if not needed.\ +`); + } +} diff --git a/src/common/config/userConfig.ts b/src/common/config/userConfig.ts new file mode 100644 index 00000000..f820c834 --- /dev/null +++ b/src/common/config/userConfig.ts @@ -0,0 +1,173 @@ +// eslint-disable-next-line enforce-zod-v4/enforce-zod-v4 +import { z as z4 } from "zod/v4"; +import { type CliOptions } from "@mongosh/arg-parser"; +import { type ConfigFieldMeta, commaSeparatedToArray, getExportsPath, getLogPath } from "./configUtils.js"; +import { previewFeatureValues, similarityValues } from "../schemas.js"; + +// TODO: UserConfig should only be UserConfigSchema and not an intersection with +// CliOptions. When we pull apart these two interfaces, we should fix this type +// as well. +export type UserConfig = z4.infer & CliOptions; + +export const configRegistry = z4.registry(); + +export const UserConfigSchema = z4.object({ + apiBaseUrl: z4.string().default("https://cloud.mongodb.com/"), + apiClientId: z4 + .string() + .optional() + .describe("Atlas API client ID for authentication. Required for running Atlas tools.") + .register(configRegistry, { isSecret: true }), + apiClientSecret: z4 + .string() + .optional() + .describe("Atlas API client secret for authentication. Required for running Atlas tools.") + .register(configRegistry, { isSecret: true }), + connectionString: z4 + .string() + .optional() + .describe( + "MongoDB connection string for direct database connections. Optional, if not set, you'll need to call the connect tool before interacting with MongoDB data." + ) + .register(configRegistry, { isSecret: true }), + loggers: z4 + .preprocess( + (val: string | string[] | undefined) => commaSeparatedToArray(val), + z4.array(z4.enum(["stderr", "disk", "mcp"])) + ) + .check( + z4.minLength(1, "Cannot be an empty array"), + z4.refine((val) => new Set(val).size === val.length, { + message: "Duplicate loggers found in config", + }) + ) + .default(["disk", "mcp"]) + .describe("An array of logger types.") + .register(configRegistry, { + defaultValueDescription: '`"disk,mcp"` see below*', + }), + logPath: z4 + .string() + .default(getLogPath()) + .describe("Folder to store logs.") + .register(configRegistry, { defaultValueDescription: "see below*" }), + disabledTools: z4 + .preprocess((val: string | string[] | undefined) => commaSeparatedToArray(val), z4.array(z4.string())) + .default([]) + .describe("An array of tool names, operation types, and/or categories of tools that will be disabled."), + confirmationRequiredTools: z4 + .preprocess((val: string | string[] | undefined) => commaSeparatedToArray(val), z4.array(z4.string())) + .default([ + "atlas-create-access-list", + "atlas-create-db-user", + "drop-database", + "drop-collection", + "delete-many", + "drop-index", + ]) + .describe( + "An array of tool names that require user confirmation before execution. Requires the client to support elicitation." + ), + readOnly: z4 + .boolean() + .default(false) + .describe( + "When set to true, only allows read, connect, and metadata operation types, disabling create/update/delete operations." + ), + indexCheck: z4 + .boolean() + .default(false) + .describe( + "When set to true, enforces that query operations must use an index, rejecting queries that perform a collection scan." + ), + telemetry: z4 + .enum(["enabled", "disabled"]) + .default("enabled") + .describe("When set to disabled, disables telemetry collection."), + transport: z4.enum(["stdio", "http"]).default("stdio").describe("Either 'stdio' or 'http'."), + httpPort: z4.coerce + .number() + .int() + .min(1, "Invalid httpPort: must be at least 1") + .max(65535, "Invalid httpPort: must be at most 65535") + .default(3000) + .describe("Port number for the HTTP server (only used when transport is 'http')."), + httpHost: z4 + .string() + .default("127.0.0.1") + .describe("Host address to bind the HTTP server to (only used when transport is 'http')."), + httpHeaders: z4 + .object({}) + .passthrough() + .default({}) + .describe( + "Header that the HTTP server will validate when making requests (only used when transport is 'http')." + ), + idleTimeoutMs: z4.coerce + .number() + .default(600_000) + .describe("Idle timeout for a client to disconnect (only applies to http transport)."), + notificationTimeoutMs: z4.coerce + .number() + .default(540_000) + .describe("Notification timeout for a client to be aware of disconnect (only applies to http transport)."), + maxBytesPerQuery: z4.coerce + .number() + .default(16_777_216) + .describe( + "The maximum size in bytes for results from a find or aggregate tool call. This serves as an upper bound for the responseBytesLimit parameter in those tools." + ), + maxDocumentsPerQuery: z4.coerce + .number() + .default(100) + .describe( + "The maximum number of documents that can be returned by a find or aggregate tool call. For the find tool, the effective limit will be the smaller of this value and the tool's limit parameter." + ), + exportsPath: z4 + .string() + .default(getExportsPath()) + .describe("Folder to store exported data files.") + .register(configRegistry, { defaultValueDescription: "see below*" }), + exportTimeoutMs: z4.coerce + .number() + .default(300_000) + .describe("Time in milliseconds after which an export is considered expired and eligible for cleanup."), + exportCleanupIntervalMs: z4.coerce + .number() + .default(120_000) + .describe("Time in milliseconds between export cleanup cycles that remove expired export files."), + atlasTemporaryDatabaseUserLifetimeMs: z4.coerce + .number() + .default(14_400_000) + .describe( + "Time in milliseconds that temporary database users created when connecting to MongoDB Atlas clusters will remain active before being automatically deleted." + ), + voyageApiKey: z4 + .string() + .default("") + .describe( + "API key for Voyage AI embeddings service (required for vector search operations with text-to-embedding conversion)." + ) + .register(configRegistry, { isSecret: true }), + disableEmbeddingsValidation: z4 + .boolean() + .default(false) + .describe("When set to true, disables validation of embeddings dimensions."), + vectorSearchDimensions: z4.coerce + .number() + .default(1024) + .describe("Default number of dimensions for vector search embeddings."), + vectorSearchSimilarityFunction: z4 + .enum(similarityValues) + .default("euclidean") + .describe("Default similarity function for vector search: 'euclidean', 'cosine', or 'dotProduct'."), + previewFeatures: z4 + .preprocess( + (val: string | string[] | undefined) => commaSeparatedToArray(val), + z4.array(z4.enum(previewFeatureValues)) + ) + .default([]) + .describe("An array of preview features that are enabled."), +}); + +export const defaultUserConfig = UserConfigSchema.parse({}); diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index 5d583c6d..6005664d 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -4,7 +4,7 @@ import { ConnectionString } from "mongodb-connection-string-url"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { type ConnectionInfo } from "@mongosh/arg-parser"; import type { DeviceId } from "../helpers/deviceId.js"; -import { type UserConfig } from "./config.js"; +import { type UserConfig } from "./config/userConfig.js"; import { MongoDBError, ErrorCodes } from "./errors.js"; import { type LoggerBase, LogId } from "./logger.js"; import { packageInfo } from "./packageInfo.js"; diff --git a/src/common/exportsManager.ts b/src/common/exportsManager.ts index f8ce9450..82727ead 100644 --- a/src/common/exportsManager.ts +++ b/src/common/exportsManager.ts @@ -10,7 +10,7 @@ import { Transform } from "stream"; import { pipeline } from "stream/promises"; import type { MongoLogId } from "mongodb-log-writer"; -import type { UserConfig } from "./config.js"; +import type { UserConfig } from "./config/userConfig.js"; import type { LoggerBase } from "./logger.js"; import { LogId } from "./logger.js"; diff --git a/src/common/search/embeddingsProvider.ts b/src/common/search/embeddingsProvider.ts index 96b3ea61..7a40f358 100644 --- a/src/common/search/embeddingsProvider.ts +++ b/src/common/search/embeddingsProvider.ts @@ -1,7 +1,7 @@ import { createVoyage } from "voyage-ai-provider"; import type { VoyageProvider } from "voyage-ai-provider"; import { embedMany } from "ai"; -import type { UserConfig } from "../config.js"; +import type { UserConfig } from "../config/userConfig.js"; import assert from "assert"; import { createFetch } from "@mongodb-js/devtools-proxy-support"; import { diff --git a/src/common/search/vectorSearchEmbeddingsManager.ts b/src/common/search/vectorSearchEmbeddingsManager.ts index 7384f1b0..3a830a15 100644 --- a/src/common/search/vectorSearchEmbeddingsManager.ts +++ b/src/common/search/vectorSearchEmbeddingsManager.ts @@ -1,6 +1,6 @@ import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { BSON, type Document } from "bson"; -import type { UserConfig } from "../config.js"; +import type { UserConfig } from "../config/userConfig.js"; import type { ConnectionManager } from "../connectionManager.js"; import z from "zod"; import { ErrorCodes, MongoDBError } from "../errors.js"; diff --git a/src/index.ts b/src/index.ts index c4013b4e..fd030ede 100644 --- a/src/index.ts +++ b/src/index.ts @@ -35,9 +35,10 @@ function enableFipsIfRequested(): void { enableFipsIfRequested(); -import { ConsoleLogger, LogId } from "./common/logger.js"; -import { config } from "./common/config.js"; import crypto from "crypto"; +import { ConsoleLogger, LogId } from "./common/logger.js"; +import { createUserConfig } from "./common/config/createUserConfig.js"; +import { type UserConfig } from "./common/config/userConfig.js"; import { packageInfo } from "./common/packageInfo.js"; import { StdioRunner } from "./transports/stdio.js"; import { StreamableHttpRunner } from "./transports/streamableHttp.js"; @@ -47,8 +48,9 @@ import { Keychain } from "./common/keychain.js"; async function main(): Promise { systemCA().catch(() => undefined); // load system CA asynchronously as in mongosh - assertHelpMode(); - assertVersionMode(); + const config = createUserConfig(); + assertHelpMode(config); + assertVersionMode(config); const transportRunner = config.transport === "stdio" @@ -131,7 +133,7 @@ main().catch((error: unknown) => { process.exit(1); }); -function assertHelpMode(): void | never { +function assertHelpMode(config: UserConfig): void | never { if (config.help) { console.log("For usage information refer to the README.md:"); console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start"); @@ -139,7 +141,7 @@ function assertHelpMode(): void | never { } } -function assertVersionMode(): void | never { +function assertVersionMode(config: UserConfig): void | never { if (config.version) { console.log(packageInfo.version); process.exit(0); diff --git a/src/lib.ts b/src/lib.ts index 2ae4b1bb..44ee47c6 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -1,6 +1,6 @@ export { Server, type ServerOptions } from "./server.js"; export { Session, type SessionOptions } from "./common/session.js"; -export { type UserConfig } from "./common/config.js"; +export { type UserConfig } from "./common/config/userConfig.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"; diff --git a/src/resources/common/config.ts b/src/resources/common/config.ts index 684c03f3..932f8657 100644 --- a/src/resources/common/config.ts +++ b/src/resources/common/config.ts @@ -1,5 +1,5 @@ import { ReactiveResource } from "../resource.js"; -import type { UserConfig } from "../../common/config.js"; +import type { UserConfig } from "../../common/config/userConfig.js"; import type { Telemetry } from "../../telemetry/telemetry.js"; import type { Session } from "../../lib.js"; import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; diff --git a/src/resources/resource.ts b/src/resources/resource.ts index a9cb702a..ef91a64d 100644 --- a/src/resources/resource.ts +++ b/src/resources/resource.ts @@ -1,6 +1,6 @@ import type { Server } from "../server.js"; import type { Session } from "../common/session.js"; -import type { UserConfig } from "../common/config.js"; +import type { UserConfig } from "../common/config/userConfig.js"; import type { Telemetry } from "../telemetry/telemetry.js"; import type { SessionEvents } from "../common/session.js"; import type { ReadResourceCallback, ResourceMetadata } from "@modelcontextprotocol/sdk/server/mcp.js"; diff --git a/src/server.ts b/src/server.ts index a276a4c7..7f5d274e 100644 --- a/src/server.ts +++ b/src/server.ts @@ -6,7 +6,7 @@ import { Resources } from "./resources/resources.js"; import type { LogLevel } from "./common/logger.js"; import { LogId, McpLogger } from "./common/logger.js"; import type { Telemetry } from "./telemetry/telemetry.js"; -import type { UserConfig } from "./common/config.js"; +import type { UserConfig } from "./common/config/userConfig.js"; import { type ServerEvent } from "./telemetry/types.js"; import { type ServerCommand } from "./telemetry/types.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 6a9db5c1..4d46fafe 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -1,6 +1,6 @@ import type { Session } from "../common/session.js"; import type { BaseEvent, CommonProperties } from "./types.js"; -import type { UserConfig } from "../common/config.js"; +import type { UserConfig } from "../common/config/userConfig.js"; import { LogId } from "../common/logger.js"; import type { ApiClient } from "../common/atlas/apiClient.js"; import { MACHINE_METADATA } from "./constants.js"; diff --git a/src/tools/tool.ts b/src/tools/tool.ts index ac650215..bd0708f5 100644 --- a/src/tools/tool.ts +++ b/src/tools/tool.ts @@ -6,7 +6,7 @@ import type { Session } from "../common/session.js"; import { LogId } from "../common/logger.js"; import type { Telemetry } from "../telemetry/telemetry.js"; import type { ConnectionMetadata, TelemetryToolMetadata, ToolEvent } from "../telemetry/types.js"; -import type { UserConfig } from "../common/config.js"; +import type { UserConfig } from "../common/config/userConfig.js"; import type { Server } from "../server.js"; import type { Elicitation } from "../elicitation.js"; import type { PreviewFeature } from "../common/schemas.js"; diff --git a/src/transports/base.ts b/src/transports/base.ts index 77d67c97..83e76a92 100644 --- a/src/transports/base.ts +++ b/src/transports/base.ts @@ -1,4 +1,4 @@ -import type { UserConfig } from "../common/config.js"; +import type { UserConfig } from "../common/config/userConfig.js"; import { packageInfo } from "../common/packageInfo.js"; import { Server } from "../server.js"; import { Session } from "../common/session.js"; diff --git a/tests/integration/common/connectionManager.oidc.test.ts b/tests/integration/common/connectionManager.oidc.test.ts index f6fd9fee..53d449aa 100644 --- a/tests/integration/common/connectionManager.oidc.test.ts +++ b/tests/integration/common/connectionManager.oidc.test.ts @@ -7,7 +7,7 @@ import type { MongoDBIntegrationTestCase } from "../tools/mongodb/mongodbHelpers import { describeWithMongoDB, isCommunityServer, getServerVersion } from "../tools/mongodb/mongodbHelpers.js"; import { defaultTestConfig, responseAsText, timeout, waitUntil } from "../helpers.js"; import type { ConnectionStateConnected, ConnectionStateConnecting } from "../../../src/common/connectionManager.js"; -import type { UserConfig } from "../../../src/common/config.js"; +import type { UserConfig } from "../../../src/common/config/userConfig.js"; import path from "path"; import type { OIDCMockProviderConfig } from "@mongodb-js/oidc-mock-provider"; import { OIDCMockProvider } from "@mongodb-js/oidc-mock-provider"; diff --git a/tests/integration/common/connectionManager.test.ts b/tests/integration/common/connectionManager.test.ts index ec7cf829..5dbf1cad 100644 --- a/tests/integration/common/connectionManager.test.ts +++ b/tests/integration/common/connectionManager.test.ts @@ -5,7 +5,7 @@ import type { } from "../../../src/common/connectionManager.js"; import { MCPConnectionManager } from "../../../src/common/connectionManager.js"; -import type { UserConfig } from "../../../src/common/config.js"; +import type { UserConfig } from "../../../src/common/config/userConfig.js"; import { describeWithMongoDB } from "../tools/mongodb/mongodbHelpers.js"; import { describe, beforeEach, expect, it, vi, afterEach } from "vitest"; import { type TestConnectionManager } from "../../utils/index.js"; diff --git a/tests/integration/elicitation.test.ts b/tests/integration/elicitation.test.ts index d16df9b9..ca3e364d 100644 --- a/tests/integration/elicitation.test.ts +++ b/tests/integration/elicitation.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import { describe, it, expect, afterEach } from "vitest"; -import { type UserConfig } from "../../src/common/config.js"; +import { type UserConfig } from "../../src/common/config/userConfig.js"; import { defaultTestConfig } from "./helpers.js"; import { Elicitation } from "../../src/elicitation.js"; import { createMockElicitInput } from "../utils/elicitationMocks.js"; diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 3e93a531..a44a940c 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -6,7 +6,7 @@ import { Telemetry } from "../../src/telemetry/telemetry.js"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "./inMemoryTransport.js"; -import { type UserConfig, config } from "../../src/common/config.js"; +import { type UserConfig } from "../../src/common/config/userConfig.js"; import { McpError, ResourceUpdatedNotificationSchema } from "@modelcontextprotocol/sdk/types.js"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import type { ConnectionManager, ConnectionState } from "../../src/common/connectionManager.js"; @@ -18,6 +18,7 @@ import { Elicitation } from "../../src/elicitation.js"; import type { MockClientCapabilities, createMockElicitInput } from "../utils/elicitationMocks.js"; import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; import { defaultCreateAtlasLocalClient } from "../../src/common/atlasLocal.js"; +import { defaultUserConfig } from "../../src/common/config/userConfig.js"; interface Parameter { name: string; @@ -42,7 +43,7 @@ export interface IntegrationTest { mcpServer: () => Server; } export const defaultTestConfig: UserConfig = { - ...config, + ...defaultUserConfig, telemetry: "disabled", loggers: ["stderr"], }; diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 2a3f18b4..9cc88a78 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -1,6 +1,5 @@ import { Telemetry } from "../../src/telemetry/telemetry.js"; import { Session } from "../../src/common/session.js"; -import { config } from "../../src/common/config.js"; import { DeviceId } from "../../src/helpers/deviceId.js"; import { describe, expect, it } from "vitest"; import { CompositeLogger } from "../../src/common/logger.js"; @@ -8,8 +7,11 @@ import { MCPConnectionManager } from "../../src/common/connectionManager.js"; import { ExportsManager } from "../../src/common/exportsManager.js"; import { Keychain } from "../../src/common/keychain.js"; import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; +import { defaultTestConfig } from "./helpers.js"; +import { type UserConfig } from "../../src/common/config/userConfig.js"; describe("Telemetry", () => { + const config: UserConfig = { ...defaultTestConfig, telemetry: "enabled" }; it("should resolve the actual device ID", async () => { const logger = new CompositeLogger(); diff --git a/tests/integration/tools/mongodb/mongodbHelpers.ts b/tests/integration/tools/mongodb/mongodbHelpers.ts index 850dd7d5..8327f070 100644 --- a/tests/integration/tools/mongodb/mongodbHelpers.ts +++ b/tests/integration/tools/mongodb/mongodbHelpers.ts @@ -10,7 +10,7 @@ import { defaultTestConfig, getDataFromUntrustedContent, } from "../../helpers.js"; -import type { UserConfig } from "../../../../src/common/config.js"; +import type { UserConfig } from "../../../../src/common/config/userConfig.js"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { EJSON } from "bson"; import { MongoDBClusterProcess } from "./mongodbClusterProcess.js"; diff --git a/tests/integration/tools/mongodb/mongodbTool.test.ts b/tests/integration/tools/mongodb/mongodbTool.test.ts index fb512c2f..0ab1fc9f 100644 --- a/tests/integration/tools/mongodb/mongodbTool.test.ts +++ b/tests/integration/tools/mongodb/mongodbTool.test.ts @@ -4,7 +4,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { MongoDBToolBase } from "../../../../src/tools/mongodb/mongodbTool.js"; import { type ToolBase, type ToolConstructorParams, type OperationType } from "../../../../src/tools/tool.js"; -import { type UserConfig } from "../../../../src/common/config.js"; +import { type UserConfig } from "../../../../src/common/config/userConfig.js"; import { MCPConnectionManager } from "../../../../src/common/connectionManager.js"; import { Session } from "../../../../src/common/session.js"; import { CompositeLogger } from "../../../../src/common/logger.js"; diff --git a/tests/integration/transports/streamableHttp.test.ts b/tests/integration/transports/streamableHttp.test.ts index 7f57135d..a901a00b 100644 --- a/tests/integration/transports/streamableHttp.test.ts +++ b/tests/integration/transports/streamableHttp.test.ts @@ -2,23 +2,22 @@ import { StreamableHttpRunner } from "../../../src/transports/streamableHttp.js" import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { describe, expect, it, beforeAll, afterAll, beforeEach, afterEach } from "vitest"; -import { config } from "../../../src/common/config.js"; import type { LoggerType, LogLevel, LogPayload } from "../../../src/common/logger.js"; import { LoggerBase, LogId } from "../../../src/common/logger.js"; import { createMCPConnectionManager } from "../../../src/common/connectionManager.js"; import { Keychain } from "../../../src/common/keychain.js"; +import { defaultTestConfig } from "../helpers.js"; +import { type UserConfig } from "../../../src/common/config/userConfig.js"; describe("StreamableHttpRunner", () => { let runner: StreamableHttpRunner; - let oldTelemetry: "enabled" | "disabled"; - let oldLoggers: ("stderr" | "disk" | "mcp")[]; + let config: UserConfig; beforeAll(() => { - oldTelemetry = config.telemetry; - oldLoggers = config.loggers; - config.telemetry = "disabled"; - config.loggers = ["stderr"]; - config.httpPort = 0; // Use a random port for testing + config = { + ...defaultTestConfig, + httpPort: 0, // Use a random port for testing + }; }); const headerTestCases: { headers: Record; description: string }[] = [ @@ -36,9 +35,6 @@ describe("StreamableHttpRunner", () => { afterAll(async () => { await runner.close(); - config.telemetry = oldTelemetry; - config.loggers = oldLoggers; - config.httpHeaders = {}; }); const clientHeaderTestCases = [ @@ -110,7 +106,6 @@ describe("StreamableHttpRunner", () => { const runners: StreamableHttpRunner[] = []; try { for (let i = 0; i < 3; i++) { - config.httpPort = 0; // Use a random port for each runner const runner = new StreamableHttpRunner({ userConfig: config }); await runner.start(); runners.push(runner); @@ -163,9 +158,6 @@ describe("StreamableHttpRunner", () => { describe("with telemetry properties", () => { afterEach(async () => { await runner.close(); - config.telemetry = oldTelemetry; - config.loggers = oldLoggers; - config.httpHeaders = {}; }); it("merges them with the base properties", async () => { diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index 827a5d6b..65f744cd 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -1,17 +1,29 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import type { UserConfig } from "../../../src/common/config.js"; -import { - setupUserConfig, - registerKnownSecretsInRootKeychain, - warnAboutDeprecatedOrUnknownCliArgs, - UserConfigSchema, - warnIfVectorSearchNotEnabledCorrectly, -} from "../../../src/common/config.js"; +import { describe, it, expect, vi, beforeEach, afterEach, type MockedFunction } from "vitest"; +import { type UserConfig, UserConfigSchema } from "../../../src/common/config/userConfig.js"; +import { type CreateUserConfigHelpers, createUserConfig } from "../../../src/common/config/createUserConfig.js"; import { getLogPath, getExportsPath } from "../../../src/common/config/configUtils.js"; -import type { CliOptions } from "@mongosh/arg-parser"; import { Keychain } from "../../../src/common/keychain.js"; import type { Secret } from "../../../src/common/keychain.js"; -import { defaultTestConfig } from "../../integration/helpers.js"; + +function createEnvironment(): { + setVariable: (this: void, variable: string, value: unknown) => void; + clearVariables(this: void): void; +} { + const registeredEnvVariables: string[] = []; + + return { + setVariable(variable: string, value: unknown): void { + (process.env as Record)[variable] = value; + registeredEnvVariables.push(variable); + }, + + clearVariables(): void { + for (const variable of registeredEnvVariables) { + delete (process.env as Record)[variable]; + } + }, + }; +} describe("config", () => { it("should generate defaults from UserConfigSchema that match expected values", () => { @@ -50,20 +62,58 @@ describe("config", () => { disableEmbeddingsValidation: false, previewFeatures: [], }; - expect(UserConfigSchema.parse({})).toStrictEqual(expectedDefaults); }); + it("should generate defaults when no config sources are populated", () => { + const expectedDefaults = { + apiBaseUrl: "https://cloud.mongodb.com/", + logPath: getLogPath(), + exportsPath: getExportsPath(), + exportTimeoutMs: 5 * 60 * 1000, // 5 minutes + exportCleanupIntervalMs: 2 * 60 * 1000, // 2 minutes + disabledTools: [], + telemetry: "enabled", + readOnly: false, + indexCheck: false, + confirmationRequiredTools: [ + "atlas-create-access-list", + "atlas-create-db-user", + "drop-database", + "drop-collection", + "delete-many", + "drop-index", + ], + transport: "stdio", + httpPort: 3000, + httpHost: "127.0.0.1", + loggers: ["disk", "mcp"], + idleTimeoutMs: 10 * 60 * 1000, // 10 minutes + notificationTimeoutMs: 9 * 60 * 1000, // 9 minutes + httpHeaders: {}, + maxDocumentsPerQuery: 100, + maxBytesPerQuery: 16 * 1024 * 1024, // ~16 mb + atlasTemporaryDatabaseUserLifetimeMs: 4 * 60 * 60 * 1000, // 4 hours + voyageApiKey: "", + vectorSearchDimensions: 1024, + vectorSearchSimilarityFunction: "euclidean", + disableEmbeddingsValidation: false, + previewFeatures: [], + }; + expect(createUserConfig()).toStrictEqual(expectedDefaults); + }); + describe("env var parsing", () => { + const { setVariable, clearVariables } = createEnvironment(); + + afterEach(() => { + clearVariables(); + }); + describe("mongodb urls", () => { it("should not try to parse a multiple-host urls", () => { - const actual = setupUserConfig({ - env: { - MDB_MCP_CONNECTION_STRING: "mongodb://user:password@host1,host2,host3/", - }, - cli: [], - }); - + setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://user:password@host1,host2,host3/"); + const actual = createUserConfig(); expect(actual.connectionString).toEqual("mongodb://user:password@host1,host2,host3/"); }); }); @@ -92,34 +142,24 @@ describe("config", () => { for (const { envVar, property, value } of testCases) { it(`should map ${envVar} to ${property} with value "${value}"`, () => { - const actual = setupUserConfig({ - cli: [], - env: { - [envVar]: String(value), - }, - }); - + setVariable(envVar, value); + const actual = createUserConfig(); expect(actual[property]).toBe(value); }); } }); describe("array cases", () => { - const testCases = { - MDB_MCP_DISABLED_TOOLS: "disabledTools", - MDB_MCP_LOGGERS: "loggers", - } as const; - - for (const [envVar, config] of Object.entries(testCases)) { - it(`should map ${envVar} to ${config}`, () => { - const actual = setupUserConfig({ - cli: [], - env: { - [envVar]: "disk,mcp", - }, - }); + const testCases = [ + { envVar: "MDB_MCP_DISABLED_TOOLS", property: "disabledTools", value: "find,export" }, + { envVar: "MDB_MCP_LOGGERS", property: "loggers", value: "disk,mcp" }, + ] as const; - expect(actual[config]).toEqual(["disk", "mcp"]); + for (const { envVar, property, value } of testCases) { + it(`should map ${envVar} to ${property}`, () => { + setVariable(envVar, value); + const actual = createUserConfig(); + expect(actual[property]).toEqual(value.split(",")); }); } }); @@ -127,9 +167,8 @@ describe("config", () => { describe("cli parsing", () => { it("should not try to parse a multiple-host urls", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--connectionString", "mongodb://user:password@host1,host2,host3/"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["--connectionString", "mongodb://user:password@host1,host2,host3/"], }); expect(actual.connectionString).toEqual("mongodb://user:password@host1,host2,host3/"); @@ -309,11 +348,9 @@ describe("config", () => { for (const { cli, expected } of testCases) { it(`should parse '${cli.join(" ")}' to ${JSON.stringify(expected)}`, () => { - const actual = setupUserConfig({ - cli: ["myself", "--", ...cli], - env: {}, + const actual = createUserConfig({ + cliArguments: cli, }); - expect(actual).toStrictEqual({ ...UserConfigSchema.parse({}), ...expected, @@ -408,11 +445,9 @@ describe("config", () => { for (const { cli, expected } of testCases) { it(`should parse '${cli.join(" ")}' to ${JSON.stringify(expected)}`, () => { - const actual = setupUserConfig({ - cli: ["myself", "--", ...cli], - env: {}, + const actual = createUserConfig({ + cliArguments: cli, }); - for (const [key, value] of Object.entries(expected)) { expect(actual[key as keyof UserConfig]).toBe(value); } @@ -434,11 +469,9 @@ describe("config", () => { for (const { cli, expected } of testCases) { it(`should parse '${cli.join(" ")}' to ${JSON.stringify(expected)}`, () => { - const actual = setupUserConfig({ - cli: ["myself", "--", ...cli], - env: {}, + const actual = createUserConfig({ + cliArguments: cli, }); - for (const [key, value] of Object.entries(expected)) { expect(actual[key as keyof UserConfig]).toEqual(value); } @@ -448,430 +481,460 @@ describe("config", () => { }); describe("precedence rules", () => { + const { setVariable, clearVariables } = createEnvironment(); + + afterEach(() => { + clearVariables(); + }); + it("cli arguments take precedence over env vars", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--connectionString", "mongodb://localhost"], - env: { MDB_MCP_CONNECTION_STRING: "mongodb://crazyhost" }, + setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://crazyhost"); + const actual = createUserConfig({ + cliArguments: ["--connectionString", "mongodb://localhost"], }); - expect(actual.connectionString).toBe("mongodb://localhost"); }); it("any cli argument takes precedence over defaults", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--connectionString", "mongodb://localhost"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["--connectionString", "mongodb://localhost"], }); - expect(actual.connectionString).toBe("mongodb://localhost"); }); it("any env var takes precedence over defaults", () => { - const actual = setupUserConfig({ - cli: [], - env: { MDB_MCP_CONNECTION_STRING: "mongodb://localhost" }, - }); - + setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://localhost"); + const actual = createUserConfig(); expect(actual.connectionString).toBe("mongodb://localhost"); }); }); describe("consolidation", () => { it("positional argument for url has precedence over --connectionString", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "mongodb://localhost", "--connectionString", "toRemove"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["mongodb://localhost", "--connectionString", "mongodb://toRemoveHost"], }); + // the shell specifies directConnection=true and serverSelectionTimeoutMS=2000 by default + expect(actual.connectionString).toBe( + "mongodb://localhost/?directConnection=true&serverSelectionTimeoutMS=2000" + ); + }); + it("positional argument is always considered", () => { + const actual = createUserConfig({ + cliArguments: ["mongodb://localhost"], + }); // the shell specifies directConnection=true and serverSelectionTimeoutMS=2000 by default expect(actual.connectionString).toBe( "mongodb://localhost/?directConnection=true&serverSelectionTimeoutMS=2000" ); - expect(actual.connectionSpecifier).toBe("mongodb://localhost"); }); }); describe("validation", () => { describe("transport", () => { it("should support http", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--transport", "http"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["--transport", "http"], }); - expect(actual.transport).toEqual("http"); }); it("should support stdio", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--transport", "stdio"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["--transport", "stdio"], }); - expect(actual.transport).toEqual("stdio"); }); it("should not support sse", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--transport", "sse"], - env: {}, - }) - ).toThrowError( - 'Invalid configuration for the following fields:\ntransport - Invalid option: expected one of "stdio"|"http"' + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--transport", "sse"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + 'Invalid configuration for the following fields:\ntransport - Invalid option: expected one of "stdio"|"http"' + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("should not support arbitrary values", () => { const value = Math.random() + "transport"; - - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--transport", value], - env: {}, - }) - ).toThrowError( - `Invalid configuration for the following fields:\ntransport - Invalid option: expected one of "stdio"|"http"` + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--transport", value], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + 'Invalid configuration for the following fields:\ntransport - Invalid option: expected one of "stdio"|"http"' + ) ); + expect(onExitFn).toBeCalledWith(1); }); }); describe("telemetry", () => { it("can be enabled", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--telemetry", "enabled"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["--telemetry", "enabled"], }); - expect(actual.telemetry).toEqual("enabled"); }); it("can be disabled", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--telemetry", "disabled"], - env: {}, + const actual = createUserConfig({ + cliArguments: ["--telemetry", "disabled"], }); - expect(actual.telemetry).toEqual("disabled"); }); it("should not support the boolean true value", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--telemetry", "true"], - env: {}, - }) - ).toThrowError( - 'Invalid configuration for the following fields:\ntelemetry - Invalid option: expected one of "enabled"|"disabled"' + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--telemetry", "true"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + 'Invalid configuration for the following fields:\ntelemetry - Invalid option: expected one of "enabled"|"disabled"' + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("should not support the boolean false value", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--telemetry", "false"], - env: {}, - }) - ).toThrowError( - 'Invalid configuration for the following fields:\ntelemetry - Invalid option: expected one of "enabled"|"disabled"' + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--telemetry", "false"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + 'Invalid configuration for the following fields:\ntelemetry - Invalid option: expected one of "enabled"|"disabled"' + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("should not support arbitrary values", () => { const value = Math.random() + "telemetry"; - - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--telemetry", value], - env: {}, - }) - ).toThrowError( - `Invalid configuration for the following fields:\ntelemetry - Invalid option: expected one of "enabled"|"disabled"` + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--telemetry", value], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + 'Invalid configuration for the following fields:\ntelemetry - Invalid option: expected one of "enabled"|"disabled"' + ) ); + expect(onExitFn).toBeCalledWith(1); }); }); describe("httpPort", () => { it("must be above 1", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--httpPort", "0"], - env: {}, - }) - ).toThrowError( - "Invalid configuration for the following fields:\nhttpPort - Invalid httpPort: must be at least 1" + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--httpPort", "0"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + "Invalid configuration for the following fields:\nhttpPort - Invalid httpPort: must be at least 1" + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("must be below 65535 (OS limit)", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--httpPort", "89527345"], - env: {}, - }) - ).toThrowError( - "Invalid configuration for the following fields:\nhttpPort - Invalid httpPort: must be at most 65535" + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--httpPort", "89527345"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + "Invalid configuration for the following fields:\nhttpPort - Invalid httpPort: must be at most 65535" + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("should not support non numeric values", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--httpPort", "portAventura"], - env: {}, - }) - ).toThrowError( - "Invalid configuration for the following fields:\nhttpPort - Invalid input: expected number, received NaN" + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--httpPort", "portAventura"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + "Invalid configuration for the following fields:\nhttpPort - Invalid input: expected number, received NaN" + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("should support numeric values", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--httpPort", "8888"], - env: {}, - }); - + const actual = createUserConfig({ cliArguments: ["--httpPort", "8888"] }); expect(actual.httpPort).toEqual(8888); }); }); describe("loggers", () => { it("must not be empty", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--loggers", ""], - env: {}, - }) - ).toThrowError("Invalid configuration for the following fields:\nloggers - Cannot be an empty array"); + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--loggers", ""], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + "Invalid configuration for the following fields:\nloggers - Cannot be an empty array" + ) + ); + expect(onExitFn).toBeCalledWith(1); }); it("must not allow duplicates", () => { - expect(() => - setupUserConfig({ - cli: ["myself", "--", "--loggers", "disk,disk,disk"], - env: {}, - }) - ).toThrowError( - "Invalid configuration for the following fields:\nloggers - Duplicate loggers found in config" + const onErrorFn = vi.fn(); + const onExitFn = vi.fn(); + createUserConfig({ + onError: onErrorFn, + closeProcess: onExitFn, + cliArguments: ["--loggers", "disk,disk,disk"], + }); + expect(onErrorFn).toBeCalledWith( + expect.stringContaining( + "Invalid configuration for the following fields:\nloggers - Duplicate loggers found in config" + ) ); + expect(onExitFn).toBeCalledWith(1); }); it("allows mcp logger", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--loggers", "mcp"], - env: {}, - }); - + const actual = createUserConfig({ cliArguments: ["--loggers", "mcp"] }); expect(actual.loggers).toEqual(["mcp"]); }); it("allows disk logger", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--loggers", "disk"], - env: {}, - }); - + const actual = createUserConfig({ cliArguments: ["--loggers", "disk"] }); expect(actual.loggers).toEqual(["disk"]); }); it("allows stderr logger", () => { - const actual = setupUserConfig({ - cli: ["myself", "--", "--loggers", "stderr"], - env: {}, - }); - + const actual = createUserConfig({ cliArguments: ["--loggers", "stderr"] }); expect(actual.loggers).toEqual(["stderr"]); }); }); }); }); -describe("CLI arguments", () => { +describe("Warning and Error messages", () => { + let warn: MockedFunction; + let error: MockedFunction; + let exit: MockedFunction; const referDocMessage = "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server."; - type TestCase = { readonly cliArg: keyof (CliOptions & UserConfig); readonly warning: string }; - const testCases = [ - { - cliArg: "connectionString", - warning: - "Warning: The --connectionString argument is deprecated. Prefer using the MDB_MCP_CONNECTION_STRING environment variable or the first positional argument for the connection string.", - }, - ] as TestCase[]; - - for (const { cliArg, warning } of testCases) { - describe(`deprecation behaviour of ${cliArg}`, () => { - let cliArgs: CliOptions & UserConfig & { _?: string[] }; - let warn: (msg: string) => void; - let exit: (status: number) => void | never; - - beforeEach(() => { - cliArgs = { [cliArg]: "RandomString" } as unknown as CliOptions & UserConfig & { _?: string[] }; - warn = vi.fn(); - exit = vi.fn(); + beforeEach(() => { + warn = vi.fn(); + error = vi.fn(); + exit = vi.fn(); + }); - warnAboutDeprecatedOrUnknownCliArgs(cliArgs as unknown as Record, { warn, exit }); - }); + describe("Deprecated CLI arguments", () => { + const testCases = [ + { + cliArg: "--connectionString", + value: "mongodb://localhost:27017", + warning: + "Warning: The --connectionString argument is deprecated. Prefer using the MDB_MCP_CONNECTION_STRING environment variable or the first positional argument for the connection string.", + }, + ] as const; + + for (const { cliArg, value, warning } of testCases) { + describe(`deprecation behaviour of ${cliArg}`, () => { + beforeEach(() => { + createUserConfig({ onWarning: warn, closeProcess: exit, cliArguments: [cliArg, value] }); + }); - it(`warns the usage of ${cliArg} as it is deprecated`, () => { - expect(warn).toHaveBeenCalledWith(warning); - }); + it(`warns the usage of ${cliArg} as it is deprecated`, () => { + expect(warn).toHaveBeenCalledWith(expect.stringContaining(warning)); + }); - it(`shows the reference message when ${cliArg} was passed`, () => { - expect(warn).toHaveBeenCalledWith(referDocMessage); - }); + it(`shows the reference message when ${cliArg} was passed`, () => { + expect(warn).toHaveBeenCalledWith(expect.stringContaining(referDocMessage)); + }); - it(`should not exit the process`, () => { - expect(exit).not.toHaveBeenCalled(); + it(`should not exit the process`, () => { + expect(exit).not.toHaveBeenCalled(); + }); }); - }); - } + } + }); describe("invalid arguments", () => { - let warn: (msg: string) => void; - let exit: (status: number) => void | never; - - beforeEach(() => { - warn = vi.fn(); - exit = vi.fn(); - }); - - it("should show a warning when an argument is not known", () => { - warnAboutDeprecatedOrUnknownCliArgs( - { - wakanda: "", - }, - { warn, exit } - ); + it("should show an error when an argument is not known and exit the process", () => { + createUserConfig({ + cliArguments: ["--wakanda", "forever"], + onWarning: warn, + onError: error, + closeProcess: exit, + }); - expect(warn).toHaveBeenCalledWith("Warning: Invalid command line argument 'wakanda'."); - expect(warn).toHaveBeenCalledWith( - "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server." + expect(error).toHaveBeenCalledWith( + expect.stringContaining("Error: Invalid command line argument '--wakanda'.") ); - }); - - it("should exit the process on unknown cli args", () => { - warnAboutDeprecatedOrUnknownCliArgs( - { - wakanda: "", - }, - { warn, exit } + expect(error).toHaveBeenCalledWith( + expect.stringContaining( + "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server." + ) ); - expect(exit).toHaveBeenCalledWith(1); }); it("should show a suggestion when is a simple typo", () => { - warnAboutDeprecatedOrUnknownCliArgs( - { - readonli: "", - }, - { warn, exit } - ); - - expect(warn).toHaveBeenCalledWith( - "Warning: Invalid command line argument 'readonli'. Did you mean 'readOnly'?" + createUserConfig({ + cliArguments: ["--readonli", ""], + onWarning: warn, + onError: error, + closeProcess: exit, + }); + expect(error).toHaveBeenCalledWith( + expect.stringContaining("Error: Invalid command line argument '--readonli'. Did you mean '--readOnly'?") ); - expect(warn).toHaveBeenCalledWith( - "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server." + expect(error).toHaveBeenCalledWith( + expect.stringContaining( + "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server." + ) ); + expect(exit).toHaveBeenCalledWith(1); }); it("should show a suggestion when the only change is on the case", () => { - warnAboutDeprecatedOrUnknownCliArgs( - { - readonly: "", - }, - { warn, exit } - ); + createUserConfig({ + cliArguments: ["--readonly", ""], + onWarning: warn, + onError: error, + closeProcess: exit, + }); - expect(warn).toHaveBeenCalledWith( - "Warning: Invalid command line argument 'readonly'. Did you mean 'readOnly'?" + expect(error).toHaveBeenCalledWith( + expect.stringContaining("Error: Invalid command line argument '--readonly'. Did you mean '--readOnly'?") ); - expect(warn).toHaveBeenCalledWith( - "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server." + expect(error).toHaveBeenCalledWith( + expect.stringContaining( + "- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server." + ) ); + expect(exit).toHaveBeenCalledWith(1); }); }); - describe("warnIfVectorSearchNotEnabledCorrectly", () => { + describe("vector search misconfiguration", () => { it("should warn if vectorSearch is enabled but embeddings provider is not configured", () => { - const warnStub = vi.fn(); - warnIfVectorSearchNotEnabledCorrectly( - { - ...defaultTestConfig, - previewFeatures: ["vectorSearch"], - }, - warnStub - ); - expect(warnStub).toBeCalledWith(`\ + createUserConfig({ + cliArguments: ["--previewFeatures", "vectorSearch"], + onWarning: warn, + onError: error, + closeProcess: exit, + }); + expect(warn).toBeCalledWith(`\ Warning: Vector search is enabled but no embeddings provider is configured. - Set an embeddings provider configuration option to enable auto-embeddings during document insertion and text-based queries with $vectorSearch.\ `); }); it("should warn if vectorSearch is not enabled but embeddings provider is configured", () => { - const warnStub = vi.fn(); - warnIfVectorSearchNotEnabledCorrectly( - { - ...defaultTestConfig, - voyageApiKey: "random-key", - }, - warnStub - ); - expect(warnStub).toBeCalledWith(`\ + createUserConfig({ + cliArguments: ["--voyageApiKey", "1FOO"], + onWarning: warn, + onError: error, + closeProcess: exit, + }); + + expect(warn).toBeCalledWith(`\ Warning: An embeddings provider is configured but the 'vectorSearch' preview feature is not enabled. - Enable vector search by adding 'vectorSearch' to the 'previewFeatures' configuration option, or remove the embeddings provider configuration if not needed.\ `); }); it("should not warn if vectorSearch is enabled correctly", () => { - const warnStub = vi.fn(); - warnIfVectorSearchNotEnabledCorrectly( - { - ...defaultTestConfig, - voyageApiKey: "random-key", - previewFeatures: ["vectorSearch"], - }, - warnStub - ); - expect(warnStub).not.toBeCalled(); + createUserConfig({ + cliArguments: ["--voyageApiKey", "1FOO", "--previewFeatures", "vectorSearch"], + onWarning: warn, + onError: error, + closeProcess: exit, + }); + expect(warn).not.toBeCalled(); }); }); +}); - describe("keychain management", () => { - type TestCase = { readonly cliArg: keyof UserConfig; secretKind: Secret["kind"] }; - const testCases = [ - { cliArg: "apiClientId", secretKind: "user" }, - { cliArg: "apiClientSecret", secretKind: "password" }, - { cliArg: "awsAccessKeyId", secretKind: "password" }, - { cliArg: "awsIamSessionToken", secretKind: "password" }, - { cliArg: "awsSecretAccessKey", secretKind: "password" }, - { cliArg: "awsSessionToken", secretKind: "password" }, - { cliArg: "password", secretKind: "password" }, - { cliArg: "tlsCAFile", secretKind: "url" }, - { cliArg: "tlsCRLFile", secretKind: "url" }, - { cliArg: "tlsCertificateKeyFile", secretKind: "url" }, - { cliArg: "tlsCertificateKeyFilePassword", secretKind: "password" }, - { cliArg: "username", secretKind: "user" }, - ] as TestCase[]; - let keychain: Keychain; - - beforeEach(() => { - keychain = Keychain.root; - keychain.clearAllSecrets(); - }); - - afterEach(() => { - keychain.clearAllSecrets(); - }); +describe("keychain management", () => { + type TestCase = { readonly cliArg: keyof UserConfig; secretKind: Secret["kind"] }; + const testCases = [ + { cliArg: "apiClientId", secretKind: "user" }, + { cliArg: "apiClientSecret", secretKind: "password" }, + // Note: These arguments were part of original test cases before + // refactor of Config but because now we use yargs-parser to strictly + // parse the config and do not allow unknown arguments to creep into the + // final results, these arguments never end up in the config. It is + // because we have the mongosh OPTIONS copied over from the repo and the + // copied object does not contain these are parse targets. + + // TODO: Whenever we finish importing OPTIONS from mongosh these test + // cases should be good to be enabled again. + + // { cliArg: "awsAccessKeyId", secretKind: "password" }, + // { cliArg: "awsIamSessionToken", secretKind: "password" }, + // { cliArg: "awsSecretAccessKey", secretKind: "password" }, + // { cliArg: "awsSessionToken", secretKind: "password" }, + { cliArg: "password", secretKind: "password" }, + { cliArg: "tlsCAFile", secretKind: "url" }, + { cliArg: "tlsCRLFile", secretKind: "url" }, + { cliArg: "tlsCertificateKeyFile", secretKind: "url" }, + { cliArg: "tlsCertificateKeyFilePassword", secretKind: "password" }, + { cliArg: "username", secretKind: "user" }, + ] as TestCase[]; + let keychain: Keychain; - for (const { cliArg, secretKind } of testCases) { - it(`should register ${cliArg} as a secret of kind ${secretKind} in the root keychain`, () => { - registerKnownSecretsInRootKeychain({ [cliArg]: cliArg }); + beforeEach(() => { + keychain = Keychain.root; + keychain.clearAllSecrets(); + }); - expect(keychain.allSecrets).toEqual([{ value: cliArg, kind: secretKind }]); - }); - } + afterEach(() => { + keychain.clearAllSecrets(); }); + + for (const { cliArg, secretKind } of testCases) { + it(`should register ${cliArg} as a secret of kind ${secretKind} in the root keychain`, () => { + createUserConfig({ cliArguments: [`--${cliArg}`, cliArg], onError: console.error }); + expect(keychain.allSecrets).toEqual([{ value: cliArg, kind: secretKind }]); + }); + } }); diff --git a/tests/unit/common/exportsManager.test.ts b/tests/unit/common/exportsManager.test.ts index bfc1eba1..17851848 100644 --- a/tests/unit/common/exportsManager.test.ts +++ b/tests/unit/common/exportsManager.test.ts @@ -7,9 +7,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ExportsManagerConfig } from "../../../src/common/exportsManager.js"; import { ensureExtension, isExportExpired, ExportsManager } from "../../../src/common/exportsManager.js"; import type { AvailableExport } from "../../../src/common/exportsManager.js"; -import { config } from "../../../src/common/config.js"; import { ROOT_DIR } from "../../accuracy/sdk/constants.js"; -import { timeout } from "../../integration/helpers.js"; +import { defaultTestConfig, timeout } from "../../integration/helpers.js"; import type { EJSONOptions } from "bson"; import { EJSON, ObjectId } from "bson"; import { CompositeLogger } from "../../../src/common/logger.js"; @@ -18,8 +17,8 @@ const logger = new CompositeLogger(); const exportsPath = path.join(ROOT_DIR, "tests", "tmp", `exports-${Date.now()}`); const exportsManagerConfig: ExportsManagerConfig = { exportsPath, - exportTimeoutMs: config.exportTimeoutMs, - exportCleanupIntervalMs: config.exportCleanupIntervalMs, + exportTimeoutMs: defaultTestConfig.exportTimeoutMs, + exportCleanupIntervalMs: defaultTestConfig.exportCleanupIntervalMs, } as const; function getExportNameAndPath({ diff --git a/tests/unit/common/roles.test.ts b/tests/unit/common/roles.test.ts index e9eac0f2..71fb0c9a 100644 --- a/tests/unit/common/roles.test.ts +++ b/tests/unit/common/roles.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; import { getDefaultRoleFromConfig } from "../../../src/common/atlas/roles.js"; -import { UserConfigSchema, type UserConfig } from "../../../src/common/config.js"; +import { UserConfigSchema, type UserConfig } from "../../../src/common/config/userConfig.js"; describe("getDefaultRoleFromConfig", () => { const defaultConfig: UserConfig = UserConfigSchema.parse({}); diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts index c4a1c370..d91ed7dc 100644 --- a/tests/unit/common/session.test.ts +++ b/tests/unit/common/session.test.ts @@ -2,7 +2,6 @@ import type { Mocked, MockedFunction } from "vitest"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { Session } from "../../../src/common/session.js"; -import { config } from "../../../src/common/config.js"; import { CompositeLogger } from "../../../src/common/logger.js"; import { MCPConnectionManager } from "../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../src/common/exportsManager.js"; @@ -10,6 +9,7 @@ import { DeviceId } from "../../../src/helpers/deviceId.js"; import { Keychain } from "../../../src/common/keychain.js"; import { VectorSearchEmbeddingsManager } from "../../../src/common/search/vectorSearchEmbeddingsManager.js"; import { ErrorCodes, MongoDBError } from "../../../src/common/errors.js"; +import { defaultTestConfig } from "../../integration/helpers.js"; vi.mock("@mongosh/service-provider-node-driver"); @@ -24,16 +24,16 @@ describe("Session", () => { const logger = new CompositeLogger(); mockDeviceId = MockDeviceId; - const connectionManager = new MCPConnectionManager(config, logger, mockDeviceId); + const connectionManager = new MCPConnectionManager(defaultTestConfig, logger, mockDeviceId); session = new Session({ apiClientId: "test-client-id", apiBaseUrl: "https://api.test.com", logger, - exportsManager: ExportsManager.init(config, logger), + exportsManager: ExportsManager.init(defaultTestConfig, logger), connectionManager: connectionManager, keychain: new Keychain(), - vectorSearchEmbeddingsManager: new VectorSearchEmbeddingsManager(config, connectionManager), + vectorSearchEmbeddingsManager: new VectorSearchEmbeddingsManager(defaultTestConfig, connectionManager), }); MockNodeDriverServiceProvider.connect = vi.fn().mockResolvedValue({} as unknown as NodeDriverServiceProvider); diff --git a/tests/unit/resources/common/debug.test.ts b/tests/unit/resources/common/debug.test.ts index db4024fd..3fade367 100644 --- a/tests/unit/resources/common/debug.test.ts +++ b/tests/unit/resources/common/debug.test.ts @@ -2,36 +2,36 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { DebugResource } from "../../../../src/resources/common/debug.js"; import { Session } from "../../../../src/common/session.js"; import { Telemetry } from "../../../../src/telemetry/telemetry.js"; -import { config } from "../../../../src/common/config.js"; import { CompositeLogger } from "../../../../src/common/logger.js"; import { MCPConnectionManager } from "../../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../../src/common/exportsManager.js"; import { DeviceId } from "../../../../src/helpers/deviceId.js"; import { Keychain } from "../../../../src/common/keychain.js"; import { VectorSearchEmbeddingsManager } from "../../../../src/common/search/vectorSearchEmbeddingsManager.js"; +import { defaultTestConfig } from "../../../integration/helpers.js"; describe("debug resource", () => { const logger = new CompositeLogger(); const deviceId = DeviceId.create(logger); - const connectionManager = new MCPConnectionManager(config, logger, deviceId); + const connectionManager = new MCPConnectionManager(defaultTestConfig, logger, deviceId); const session = vi.mocked( new Session({ apiBaseUrl: "", logger, - exportsManager: ExportsManager.init(config, logger), + exportsManager: ExportsManager.init(defaultTestConfig, logger), connectionManager, keychain: new Keychain(), - vectorSearchEmbeddingsManager: new VectorSearchEmbeddingsManager(config, connectionManager), + vectorSearchEmbeddingsManager: new VectorSearchEmbeddingsManager(defaultTestConfig, connectionManager), }) ); - const telemetry = Telemetry.create(session, { ...config, telemetry: "disabled" }, deviceId); + const telemetry = Telemetry.create(session, { ...defaultTestConfig, telemetry: "disabled" }, deviceId); - let debugResource: DebugResource = new DebugResource(session, config, telemetry); + let debugResource: DebugResource = new DebugResource(session, defaultTestConfig, telemetry); beforeEach(() => { - debugResource = new DebugResource(session, config, telemetry); + debugResource = new DebugResource(session, defaultTestConfig, telemetry); }); it("should be connected when a connected event happens", async () => { diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 8b4a0c9c..022e24eb 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -3,13 +3,13 @@ import type { Session } from "../../src/common/session.js"; import { Telemetry } from "../../src/telemetry/telemetry.js"; import type { BaseEvent, CommonProperties, TelemetryEvent, TelemetryResult } from "../../src/telemetry/types.js"; import { EventCache } from "../../src/telemetry/eventCache.js"; -import { config } from "../../src/common/config.js"; import { afterEach, beforeEach, describe, it, vi, expect } from "vitest"; import { NullLogger } from "../../tests/utils/index.js"; import type { MockedFunction } from "vitest"; import type { DeviceId } from "../../src/helpers/deviceId.js"; -import { expectDefined } from "../integration/helpers.js"; +import { defaultTestConfig, expectDefined } from "../integration/helpers.js"; import { Keychain } from "../../src/common/keychain.js"; +import { type UserConfig } from "../../src/common/config/userConfig.js"; // Mock the ApiClient to avoid real API calls vi.mock("../../src/common/atlas/apiClient.js"); @@ -32,6 +32,7 @@ describe("Telemetry", () => { let session: Session; let telemetry: Telemetry; let mockDeviceId: DeviceId; + let config: UserConfig; // Helper function to create properly typed test events function createTestEvent(options?: { @@ -112,6 +113,7 @@ describe("Telemetry", () => { } beforeEach(() => { + config = { ...defaultTestConfig, telemetry: "enabled" }; // Reset mocks before each test vi.clearAllMocks(); diff --git a/tests/unit/toolBase.test.ts b/tests/unit/toolBase.test.ts index e6d7c58e..b88bebae 100644 --- a/tests/unit/toolBase.test.ts +++ b/tests/unit/toolBase.test.ts @@ -12,7 +12,7 @@ import type { import { ToolBase } from "../../src/tools/tool.js"; import type { CallToolResult, ToolAnnotations } from "@modelcontextprotocol/sdk/types.js"; import type { Session } from "../../src/common/session.js"; -import type { UserConfig } from "../../src/common/config.js"; +import type { UserConfig } from "../../src/common/config/userConfig.js"; import type { Telemetry } from "../../src/telemetry/telemetry.js"; import type { Elicitation } from "../../src/elicitation.js"; import type { CompositeLogger } from "../../src/common/logger.js"; From c39361d1caf0f167219cbde5c6972ecf07d392b2 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 23:11:25 +0100 Subject: [PATCH 08/16] chore: fix new tests --- tests/integration/customTools.test.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/integration/customTools.test.ts b/tests/integration/customTools.test.ts index ac99cdae..955999d3 100644 --- a/tests/integration/customTools.test.ts +++ b/tests/integration/customTools.test.ts @@ -3,18 +3,14 @@ import { ToolBase, type ToolArgs } from "../../src/tools/index.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; import type { TelemetryToolMetadata } from "../../src/telemetry/types.js"; -import { defaultTestConfig, driverOptions, setupIntegrationTest } from "./helpers.js"; +import { defaultTestConfig, setupIntegrationTest } from "./helpers.js"; describe("Custom Tools", () => { - const { mcpClient, mcpServer } = setupIntegrationTest( - () => ({ ...defaultTestConfig }), - () => driverOptions, - { - serverOptions: { - tools: [CustomGreetingTool, CustomCalculatorTool], - }, - } - ); + const { mcpClient, mcpServer } = setupIntegrationTest(() => ({ ...defaultTestConfig }), { + serverOptions: { + tools: [CustomGreetingTool, CustomCalculatorTool], + }, + }); it("should register custom tools instead of default tools", async () => { // Check that custom tools are registered From b26e894529a32f2199561e74641cec2b55eede04 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 23:15:46 +0100 Subject: [PATCH 09/16] chore: add connection specifier parse test --- tests/unit/common/config.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index 65f744cd..722b0893 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -174,6 +174,15 @@ describe("config", () => { expect(actual.connectionString).toEqual("mongodb://user:password@host1,host2,host3/"); }); + it("positional connection specifier gets accounted for even without other connection sources", () => { + // Note that neither connectionString argument nor env variable is + // provided. + const actual = createUserConfig({ + cliArguments: ["mongodb://host1:27017"], + }); + expect(actual.connectionString).toEqual("mongodb://host1:27017/?directConnection=true"); + }); + describe("string use cases", () => { const testCases = [ { @@ -487,6 +496,14 @@ describe("config", () => { clearVariables(); }); + it("positional argument takes precedence over all", () => { + setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://crazyhost1"); + const actual = createUserConfig({ + cliArguments: ["mongodb://crazyhost2", "--connectionString", "mongodb://localhost"], + }); + expect(actual.connectionString).toBe("mongodb://crazyhost2/?directConnection=true"); + }); + it("cli arguments take precedence over env vars", () => { setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://crazyhost"); const actual = createUserConfig({ From 98cc28f39eec0394c438410dbfc7f54fa8877b59 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 17 Nov 2025 23:18:59 +0100 Subject: [PATCH 10/16] chore: remove unused defaultUserConfig --- src/common/config/userConfig.ts | 2 -- tests/integration/helpers.ts | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/common/config/userConfig.ts b/src/common/config/userConfig.ts index f820c834..15f906a9 100644 --- a/src/common/config/userConfig.ts +++ b/src/common/config/userConfig.ts @@ -169,5 +169,3 @@ export const UserConfigSchema = z4.object({ .default([]) .describe("An array of preview features that are enabled."), }); - -export const defaultUserConfig = UserConfigSchema.parse({}); diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index a44a940c..fe01cd78 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -18,7 +18,7 @@ import { Elicitation } from "../../src/elicitation.js"; import type { MockClientCapabilities, createMockElicitInput } from "../utils/elicitationMocks.js"; import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; import { defaultCreateAtlasLocalClient } from "../../src/common/atlasLocal.js"; -import { defaultUserConfig } from "../../src/common/config/userConfig.js"; +import { UserConfigSchema } from "../../src/common/config/userConfig.js"; interface Parameter { name: string; @@ -43,7 +43,7 @@ export interface IntegrationTest { mcpServer: () => Server; } export const defaultTestConfig: UserConfig = { - ...defaultUserConfig, + ...UserConfigSchema.parse({}), telemetry: "disabled", loggers: ["stderr"], }; From ff9e1239b87a37c5f919ef2caeda9e96f0717bcb Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 18 Nov 2025 00:02:21 +0100 Subject: [PATCH 11/16] chore: address copilot suggestions --- src/common/config/configUtils.ts | 6 ++--- src/common/config/createUserConfig.ts | 35 +++++++++++++-------------- tests/unit/common/config.test.ts | 21 ++++++++-------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/common/config/configUtils.ts b/src/common/config/configUtils.ts index a13e4b66..11db9ee4 100644 --- a/src/common/config/configUtils.ts +++ b/src/common/config/configUtils.ts @@ -42,10 +42,10 @@ export function isConnectionSpecifier(arg: string | undefined): boolean { arg !== undefined && (arg.startsWith("mongodb://") || arg.startsWith("mongodb+srv://") || - // Strings starting with a hyphen `-` are generally a sign of CLI - // flag so we exclude them from the possibility of being a + // Strings starting with double hyphens `--` are generally a sign of + // CLI flag so we exclude them from the possibility of being a // connection specifier. - !(arg.endsWith(".js") || arg.endsWith(".mongodb") || arg.startsWith("-"))) + !(arg.endsWith(".js") || arg.endsWith(".mongodb") || arg.startsWith("--"))) ); } diff --git a/src/common/config/createUserConfig.ts b/src/common/config/createUserConfig.ts index 196d2f50..12a02ac4 100644 --- a/src/common/config/createUserConfig.ts +++ b/src/common/config/createUserConfig.ts @@ -108,15 +108,23 @@ function parseUserConfigSources(cliArguments: string[]): { // no-op statement so ESLint does not complain. void endOfFlagArguments; + // A connectionSpecifier can be one of: + // - database name + // - host name + // - ip address + // - replica set specifier + // - complete connection string + let connectionSpecifier: string | undefined = undefined; const [maybeConnectionSpecifier, ...unknownArguments] = positionalAndUnknownArguments; - // If the extracted connection specifier is not a connection specifier - // indeed, then we push it back to the unknown arguments list. This might - // happen for example when an unknown argument is provided without ever - // specifying a positional argument. - if (typeof maybeConnectionSpecifier !== "string" || !isConnectionSpecifier(maybeConnectionSpecifier)) { - if (maybeConnectionSpecifier) { - unknownArguments.unshift(maybeConnectionSpecifier); - } + + if (typeof maybeConnectionSpecifier === "string" && isConnectionSpecifier(maybeConnectionSpecifier)) { + connectionSpecifier = maybeConnectionSpecifier; + } else if (maybeConnectionSpecifier !== undefined) { + // If the extracted connection specifier is not a connection specifier + // indeed, then we push it back to the unknown arguments list. This might + // happen for example when an unknown argument is provided without ever + // specifying a positional argument. + unknownArguments.unshift(maybeConnectionSpecifier); } return { @@ -142,16 +150,7 @@ function parseUserConfigSources(cliArguments: string[]): { }) .filter((argument) => typeof argument === "string"), userAndArgsParserConfig: parsedUserAndArgsParserConfig, - // A connectionSpecifier can be one of: - // - database name - // - host name - // - ip address - // - replica set specifier - // - complete connection string - connectionSpecifier: - typeof maybeConnectionSpecifier === "string" && isConnectionSpecifier(maybeConnectionSpecifier) - ? String(maybeConnectionSpecifier) - : undefined, + connectionSpecifier, }; } diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index 722b0893..372445c7 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -916,16 +916,17 @@ describe("keychain management", () => { const testCases = [ { cliArg: "apiClientId", secretKind: "user" }, { cliArg: "apiClientSecret", secretKind: "password" }, - // Note: These arguments were part of original test cases before - // refactor of Config but because now we use yargs-parser to strictly - // parse the config and do not allow unknown arguments to creep into the - // final results, these arguments never end up in the config. It is - // because we have the mongosh OPTIONS copied over from the repo and the - // copied object does not contain these are parse targets. - - // TODO: Whenever we finish importing OPTIONS from mongosh these test - // cases should be good to be enabled again. - + /* + * Note: These arguments were part of original test cases before + * refactor of Config but because now we use yargs-parser to strictly + * parse the config and do not allow unknown arguments to creep into the + * final results, these arguments never end up in the config. It is + * because we have the mongosh OPTIONS copied over from the repo and the + * copied object does not contain these as parse targets. + * + * TODO: Whenever we finish importing OPTIONS from mongosh these test + * cases should be good to be enabled again. + */ // { cliArg: "awsAccessKeyId", secretKind: "password" }, // { cliArg: "awsIamSessionToken", secretKind: "password" }, // { cliArg: "awsSecretAccessKey", secretKind: "password" }, From ba7e498131566db7eff0dd5045e6538c9b2cca13 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 18 Nov 2025 08:23:42 +0100 Subject: [PATCH 12/16] chore: fix only zod/v4 rule to point to right file The last refactor moved exports around and now that the UserSchema is in config/userConfig.ts, the eslint rule needed updating as well. --- eslint-rules/enforce-zod-v4.js | 3 +-- eslint-rules/enforce-zod-v4.test.js | 12 ++++++------ src/common/config/userConfig.ts | 1 - 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/eslint-rules/enforce-zod-v4.js b/eslint-rules/enforce-zod-v4.js index 86f33d75..7afa38ec 100644 --- a/eslint-rules/enforce-zod-v4.js +++ b/eslint-rules/enforce-zod-v4.js @@ -2,8 +2,7 @@ import path from "path"; // The file that is allowed to import from zod/v4 -const configFilePath = path.resolve(import.meta.dirname, "../src/common/config.ts"); -const schemasFilePath = path.resolve(import.meta.dirname, "../src/common/schemas.ts"); +const configFilePath = path.resolve(import.meta.dirname, "../src/common/config/userConfig.ts"); // Ref: https://eslint.org/docs/latest/extend/custom-rules export default { diff --git a/eslint-rules/enforce-zod-v4.test.js b/eslint-rules/enforce-zod-v4.test.js index 835fd86a..287f52c3 100644 --- a/eslint-rules/enforce-zod-v4.test.js +++ b/eslint-rules/enforce-zod-v4.test.js @@ -15,19 +15,19 @@ const ruleTester = new RuleTester({ }); describe("enforce-zod-v4", () => { - it("should allow zod/v4 imports in config.ts", () => { + it("should allow zod/v4 imports in userConfig.ts", () => { ruleTester.run("enforce-zod-v4", rule, { valid: [ { - filename: resolve("src/common/config.ts"), + filename: resolve("src/common/config/userConfig.ts"), code: 'import { z } from "zod/v4";\n', }, { - filename: resolve("src/common/config.ts"), + filename: resolve("src/common/config/userConfig.ts"), code: 'import * as z from "zod/v4";\n', }, { - filename: resolve("src/common/config.ts"), + filename: resolve("src/common/config/userConfig.ts"), code: 'import type { ZodType } from "zod/v4";\n', }, ], @@ -63,7 +63,7 @@ describe("enforce-zod-v4", () => { code: 'import { something } from "some-package";\n', }, { - filename: resolve("src/common/config.ts"), + filename: resolve("src/common/config/userConfig.ts"), code: 'import path from "path";\n', }, { @@ -127,7 +127,7 @@ describe("enforce-zod-v4", () => { ruleTester.run("enforce-zod-v4", rule, { valid: [ { - filename: resolve("src/common/config.ts"), + filename: resolve("src/common/config/userConfig.ts"), code: `import { z } from "zod/v4"; import path from "path"; import type { UserConfig } from "./types.js"; diff --git a/src/common/config/userConfig.ts b/src/common/config/userConfig.ts index 15f906a9..938155ab 100644 --- a/src/common/config/userConfig.ts +++ b/src/common/config/userConfig.ts @@ -1,4 +1,3 @@ -// eslint-disable-next-line enforce-zod-v4/enforce-zod-v4 import { z as z4 } from "zod/v4"; import { type CliOptions } from "@mongosh/arg-parser"; import { type ConfigFieldMeta, commaSeparatedToArray, getExportsPath, getLogPath } from "./configUtils.js"; From b8e490e01b1891c1d0e45bba348518c792ca2f1c Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 18 Nov 2025 08:26:37 +0100 Subject: [PATCH 13/16] chore: remove no-config-import rule There is no free floating config object anymore so we don't have to worry about accidentally importing wrong config. --- eslint-rules/no-config-imports.js | 75 -------------------------- eslint-rules/no-config-imports.test.js | 69 ------------------------ eslint.config.js | 7 --- 3 files changed, 151 deletions(-) delete mode 100644 eslint-rules/no-config-imports.js delete mode 100644 eslint-rules/no-config-imports.test.js diff --git a/eslint-rules/no-config-imports.js b/eslint-rules/no-config-imports.js deleted file mode 100644 index 5c4efb7c..00000000 --- a/eslint-rules/no-config-imports.js +++ /dev/null @@ -1,75 +0,0 @@ -"use strict"; -import path from "path"; - -// The file from which we wish to discourage importing values -const configFilePath = path.resolve(import.meta.dirname, "../src/common/config.js"); - -// Files that are allowed to import value exports from config.ts -const allowedConfigValueImportFiles = [ - // Main entry point that injects the config - "src/index.ts", - // Config resource definition that works with the some config values - "src/resources/common/config.ts", - // The file exports, a factory function to create MCPConnectionManager and - // it relies on driver options generator and default driver options from - // config file. - "src/common/connectionManager.ts", -]; - -// Ref: https://eslint.org/docs/latest/extend/custom-rules -export default { - meta: { - type: "problem", - docs: { - description: - "Disallows value imports from config.ts, with a few exceptions, to enforce dependency injection of the config.", - recommended: true, - }, - fixable: null, - messages: { - noConfigImports: - "Value imports from config.ts are not allowed. Use dependency injection instead. Only type imports are permitted.", - }, - }, - create(context) { - const currentFilePath = path.resolve(context.getFilename()); - - const isCurrentFileAllowedToImport = allowedConfigValueImportFiles.some((allowedFile) => { - const resolvedAllowedFile = path.resolve(allowedFile); - return currentFilePath === resolvedAllowedFile; - }); - - if (isCurrentFileAllowedToImport) { - return {}; - } - - return { - ImportDeclaration(node) { - const importPath = node.source.value; - - // If the path is not relative, very likely its targeting a - // node_module so we skip it. And also if the entire import is - // marked with a type keyword. - if (typeof importPath !== "string" || !importPath.startsWith(".") || node.importKind === "type") { - return; - } - - const currentDir = path.dirname(currentFilePath); - const resolvedImportPath = path.resolve(currentDir, importPath); - - if (resolvedImportPath === configFilePath) { - const hasValueImportFromConfig = node.specifiers.some((specifier) => { - return specifier.importKind !== "type"; - }); - - if (hasValueImportFromConfig) { - context.report({ - node, - messageId: "noConfigImports", - }); - } - } - }, - }; - }, -}; diff --git a/eslint-rules/no-config-imports.test.js b/eslint-rules/no-config-imports.test.js deleted file mode 100644 index d94e952d..00000000 --- a/eslint-rules/no-config-imports.test.js +++ /dev/null @@ -1,69 +0,0 @@ -import path from "path"; -import { RuleTester } from "eslint"; -import { describe, it } from "vitest"; -import tsParser from "@typescript-eslint/parser"; -import rule from "./no-config-imports.js"; - -const ROOT = process.cwd(); -const resolve = (p) => path.resolve(ROOT, p); - -const ruleTester = new RuleTester({ - languageOptions: { - parser: tsParser, - parserOptions: { ecmaVersion: 2022, sourceType: "module" }, - }, -}); - -describe("no-config-imports", () => { - it("should not report any violations", () => { - ruleTester.run("no-config-imports", rule, { - valid: [ - { - filename: resolve("src/some/module.ts"), - code: 'import type { UserConfig } from "../common/config.js";\n', - }, - { - filename: resolve("src/some/module.ts"), - code: 'import { something } from "../common/logger.js";\n', - }, - { - filename: resolve("src/some/module.ts"), - code: 'import type * as Cfg from "../common/config.js";\n', - }, - { - filename: resolve("src/index.ts"), - code: 'import { driverOptions } from "../common/config.js";\n', - }, - ], - invalid: [], - }); - }); - - it("should report rule violations", () => { - ruleTester.run("no-config-imports", rule, { - valid: [], - invalid: [ - { - filename: resolve("src/another/module.ts"), - code: 'import { driverOptions } from "../common/config.js";\n', - errors: [{ messageId: "noConfigImports" }], - }, - { - filename: resolve("src/another/module.ts"), - code: 'import configDefault from "../common/config.js";\n', - errors: [{ messageId: "noConfigImports" }], - }, - { - filename: resolve("src/another/module.ts"), - code: 'import * as cfg from "../common/config.js";\n', - errors: [{ messageId: "noConfigImports" }], - }, - { - filename: resolve("src/another/module.ts"), - code: 'import { type UserConfig, driverOptions } from "../common/config.js";\n', - errors: [{ messageId: "noConfigImports" }], - }, - ], - }); - }); -}); diff --git a/eslint.config.js b/eslint.config.js index f9d4f308..88dfd146 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -5,7 +5,6 @@ import globals from "globals"; import tseslint from "typescript-eslint"; import eslintPluginPrettierRecommended from "eslint-plugin-prettier/recommended"; import vitestPlugin from "@vitest/eslint-plugin"; -import noConfigImports from "./eslint-rules/no-config-imports.js"; import enforceZodV4 from "./eslint-rules/enforce-zod-v4.js"; const testFiles = ["tests/**/*.test.ts", "tests/**/*.ts"]; @@ -68,11 +67,6 @@ export default defineConfig([ { files: ["src/**/*.ts"], plugins: { - "no-config-imports": { - rules: { - "no-config-imports": noConfigImports, - }, - }, "enforce-zod-v4": { rules: { "enforce-zod-v4": enforceZodV4, @@ -80,7 +74,6 @@ export default defineConfig([ }, }, rules: { - "no-config-imports/no-config-imports": "error", "enforce-zod-v4/enforce-zod-v4": "error", }, }, From 75d122167a0c832566ec63140114ad372ce56f0b Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 18 Nov 2025 09:44:57 +0100 Subject: [PATCH 14/16] chore: use default driver options for connection strings Last change missed using default driver options for connection info without driver options. We are bringing that back now. Also the ensureConnected is modified to use correct connectionInfo. --- src/common/connectionManager.ts | 29 +++++++++++++++++++++++----- src/tools/mongodb/connect/connect.ts | 2 +- src/tools/mongodb/mongodbTool.ts | 11 ++++++----- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index 6005664d..e1324917 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -2,7 +2,7 @@ import { EventEmitter } from "events"; import type { MongoClientOptions } from "mongodb"; import { ConnectionString } from "mongodb-connection-string-url"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; -import { type ConnectionInfo } from "@mongosh/arg-parser"; +import { generateConnectionInfoFromCliArgs, type ConnectionInfo } from "@mongosh/arg-parser"; import type { DeviceId } from "../helpers/deviceId.js"; import { type UserConfig } from "./config/userConfig.js"; import { MongoDBError, ErrorCodes } from "./errors.js"; @@ -33,6 +33,20 @@ export interface ConnectionState { } const MCP_TEST_DATABASE = "#mongodb-mcp"; + +export const defaultDriverOptions: ConnectionInfo["driverOptions"] = { + readConcern: { + level: "local", + }, + readPreference: "secondaryPreferred", + writeConcern: { + w: "majority", + }, + timeoutMS: 30_000, + proxy: { useEnvironmentVariableProxies: true }, + applyProxyToOIDC: true, +}; + export class ConnectionStateConnected implements ConnectionState { public tag = "connected" as const; @@ -171,10 +185,15 @@ export class MCPConnectionManager extends ConnectionManager { components: appNameComponents, }); - const connectionInfo = { - connectionString: settings.connectionString, - driverOptions: settings.driverOptions ?? {}, - }; + const connectionInfo: ConnectionInfo = settings.driverOptions + ? { + connectionString: settings.connectionString, + driverOptions: settings.driverOptions, + } + : generateConnectionInfoFromCliArgs({ + ...defaultDriverOptions, + connectionSpecifier: settings.connectionString, + }); if (connectionInfo.driverOptions.oidc) { connectionInfo.driverOptions.oidc.allowedFlows ??= ["auth-code"]; diff --git a/src/tools/mongodb/connect/connect.ts b/src/tools/mongodb/connect/connect.ts index 601fcb36..87219d0d 100644 --- a/src/tools/mongodb/connect/connect.ts +++ b/src/tools/mongodb/connect/connect.ts @@ -68,7 +68,7 @@ export class ConnectTool extends MongoDBToolBase { break; } - await this.connectToMongoDB(connectionString); + await this.session.connectToMongoDB({ connectionString }); this.updateMetadata(); return { diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index e28338b3..792e744d 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -8,6 +8,7 @@ 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"; +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; export const DbOperationArgs = { database: z.string().describe("Database name"), @@ -29,7 +30,11 @@ export abstract class MongoDBToolBase extends ToolBase { if (this.config.connectionString) { try { - await this.connectToMongoDB(this.config.connectionString); + const connectionInfo = generateConnectionInfoFromCliArgs({ + ...this.config, + connectionSpecifier: this.config.connectionString, + }); + await this.session.connectToMongoDB(connectionInfo); } catch (error) { this.session.logger.error({ id: LogId.mongodbConnectFailure, @@ -108,10 +113,6 @@ export abstract class MongoDBToolBase extends ToolBase { return super.handleError(error, args); } - protected connectToMongoDB(connectionString: string): Promise { - return this.session.connectToMongoDB({ connectionString }); - } - /** * Resolves the tool metadata from the arguments passed to the mongoDB tools. * From dd0d98b6f0172bcaa94bf431c8bed008fab0ea85 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 18 Nov 2025 11:03:42 +0100 Subject: [PATCH 15/16] chore: address PR feedback around createUserConfig 1. Use eslint disable instead of void marking the unused expression. 2. Make deprecatedCliArgumentWarnings a string instead of list of strings --- src/common/config/createUserConfig.ts | 32 ++++++++++----------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/common/config/createUserConfig.ts b/src/common/config/createUserConfig.ts index 12a02ac4..335eeed6 100644 --- a/src/common/config/createUserConfig.ts +++ b/src/common/config/createUserConfig.ts @@ -32,7 +32,7 @@ export function createUserConfig({ closeProcess = defaultUserConfigHelpers.closeProcess, cliArguments = defaultUserConfigHelpers.cliArguments, }: Partial = defaultUserConfigHelpers): UserConfig { - const { unknownCliArgumentErrors, deprecatedCliArgumentWarnings, userAndArgsParserConfig, connectionSpecifier } = + const { unknownCliArgumentErrors, deprecatedCliArgumentWarning, userAndArgsParserConfig, connectionSpecifier } = parseUserConfigSources(cliArguments); if (unknownCliArgumentErrors.length) { @@ -44,9 +44,9 @@ ${unknownCliArgumentErrors.join("\n")} return closeProcess(1); } - if (deprecatedCliArgumentWarnings.length) { + if (deprecatedCliArgumentWarning) { const deprecatedMessages = ` -${deprecatedCliArgumentWarnings.join("\n")} +${deprecatedCliArgumentWarning} - Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server. `; onWarning(deprecatedMessages); @@ -79,13 +79,17 @@ ${deprecatedCliArgumentWarnings.join("\n")} function parseUserConfigSources(cliArguments: string[]): { unknownCliArgumentErrors: string[]; - deprecatedCliArgumentWarnings: string[]; + deprecatedCliArgumentWarning: string | undefined; userAndArgsParserConfig: Record; connectionSpecifier: string | undefined; } { const { _: positionalAndUnknownArguments, - "--": endOfFlagArguments, + // We don't make use of end of flag arguments but also don't want them to + // end up alongside unknown arguments so we are extracting them and having a + // no-op statement so ESLint does not complain. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + "--": _endOfFlagArguments, ...parsedUserAndArgsParserConfig } = argv(cliArguments, { ...OPTIONS, @@ -103,11 +107,6 @@ function parseUserConfigSources(cliArguments: string[]): { }, }); - // We don't make use of end of flag arguments but also don't want them to - // end up alongside unknown arguments so we are extracting them and having a - // no-op statement so ESLint does not complain. - void endOfFlagArguments; - // A connectionSpecifier can be one of: // - database name // - host name @@ -139,16 +138,9 @@ function parseUserConfigSources(cliArguments: string[]): { return `Error: Invalid command line argument '${argument}'.`; }), - deprecatedCliArgumentWarnings: cliArguments - .filter((argument) => argument.startsWith("--connectionString")) - .map((argument) => { - if (argument.startsWith("--connectionString")) { - return "Warning: The --connectionString argument is deprecated. Prefer using the MDB_MCP_CONNECTION_STRING environment variable or the first positional argument for the connection string."; - } - - return undefined; - }) - .filter((argument) => typeof argument === "string"), + deprecatedCliArgumentWarning: cliArguments.find((argument) => argument.startsWith("--connectionString")) + ? "Warning: The --connectionString argument is deprecated. Prefer using the MDB_MCP_CONNECTION_STRING environment variable or the first positional argument for the connection string." + : undefined, userAndArgsParserConfig: parsedUserAndArgsParserConfig, connectionSpecifier, }; From 8e32474a430d4647fadc1f1e4a5b1e4abfa46039 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 18 Nov 2025 11:16:59 +0100 Subject: [PATCH 16/16] chore: create Session.connectToConfiguredConnection Creates a dedicated method on Session to connect to the connection configured in the config. This involves generating proper connection info. --- src/common/session.ts | 28 ++++++++++++------- src/server.ts | 7 +---- src/tools/mongodb/mongodbTool.ts | 7 +---- src/transports/base.ts | 4 +-- tests/integration/helpers.ts | 4 +-- tests/integration/telemetry.test.ts | 2 +- .../tools/mongodb/mongodbTool.test.ts | 4 +-- tests/unit/common/session.test.ts | 7 +++-- tests/unit/resources/common/debug.test.ts | 2 +- 9 files changed, 30 insertions(+), 35 deletions(-) diff --git a/src/common/session.ts b/src/common/session.ts index 4a8a0213..16920bc4 100644 --- a/src/common/session.ts +++ b/src/common/session.ts @@ -19,11 +19,11 @@ import type { ExportsManager } from "./exportsManager.js"; import type { Client } from "@mongodb-js/atlas-local"; import type { Keychain } from "./keychain.js"; import type { VectorSearchEmbeddingsManager } from "./search/vectorSearchEmbeddingsManager.js"; +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; +import { type UserConfig } from "../common/config/userConfig.js"; export interface SessionOptions { - apiBaseUrl: string; - apiClientId?: string; - apiClientSecret?: string; + userConfig: UserConfig; logger: CompositeLogger; exportsManager: ExportsManager; connectionManager: ConnectionManager; @@ -40,6 +40,7 @@ export type SessionEvents = { }; export class Session extends EventEmitter { + private readonly userConfig: UserConfig; readonly sessionId: string = new ObjectId().toString(); readonly exportsManager: ExportsManager; readonly connectionManager: ConnectionManager; @@ -57,9 +58,7 @@ export class Session extends EventEmitter { public logger: CompositeLogger; constructor({ - apiBaseUrl, - apiClientId, - apiClientSecret, + userConfig, logger, connectionManager, exportsManager, @@ -69,17 +68,18 @@ export class Session extends EventEmitter { }: SessionOptions) { super(); + this.userConfig = userConfig; this.keychain = keychain; this.logger = logger; const credentials: ApiClientCredentials | undefined = - apiClientId && apiClientSecret + userConfig.apiClientId && userConfig.apiClientSecret ? { - clientId: apiClientId, - clientSecret: apiClientSecret, + clientId: userConfig.apiClientId, + clientSecret: userConfig.apiClientSecret, } : undefined; - this.apiClient = new ApiClient({ baseUrl: apiBaseUrl, credentials }, logger); + this.apiClient = new ApiClient({ baseUrl: userConfig.apiBaseUrl, credentials }, logger); this.atlasLocalClient = atlasLocalClient; this.exportsManager = exportsManager; this.connectionManager = connectionManager; @@ -144,6 +144,14 @@ export class Session extends EventEmitter { this.emit("close"); } + async connectToConfiguredConnection(): Promise { + const connectionInfo = generateConnectionInfoFromCliArgs({ + ...this.userConfig, + connectionSpecifier: this.userConfig.connectionString, + }); + await this.connectToMongoDB(connectionInfo); + } + async connectToMongoDB(settings: ConnectionSettings): Promise { await this.connectionManager.connect({ ...settings }); } diff --git a/src/server.ts b/src/server.ts index 7f5d274e..3ae209d4 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,4 +1,3 @@ -import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { Session } from "./common/session.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; @@ -328,11 +327,7 @@ export class Server { context: "server", message: `Detected a MongoDB connection string in the configuration, trying to connect...`, }); - const connectionInfo = generateConnectionInfoFromCliArgs({ - ...this.userConfig, - connectionSpecifier: this.userConfig.connectionString, - }); - await this.session.connectToMongoDB(connectionInfo); + await this.session.connectToConfiguredConnection(); } catch (error) { // We don't throw an error here because we want to allow the server to start even if the connection string is invalid. this.session.logger.error({ diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index 792e744d..1d2969f3 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -8,7 +8,6 @@ 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"; -import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; export const DbOperationArgs = { database: z.string().describe("Database name"), @@ -30,11 +29,7 @@ export abstract class MongoDBToolBase extends ToolBase { if (this.config.connectionString) { try { - const connectionInfo = generateConnectionInfoFromCliArgs({ - ...this.config, - connectionSpecifier: this.config.connectionString, - }); - await this.session.connectToMongoDB(connectionInfo); + await this.session.connectToConfiguredConnection(); } catch (error) { this.session.logger.error({ id: LogId.mongodbConnectFailure, diff --git a/src/transports/base.ts b/src/transports/base.ts index 83e76a92..4643a31d 100644 --- a/src/transports/base.ts +++ b/src/transports/base.ts @@ -95,9 +95,7 @@ export abstract class TransportRunnerBase { }); const session = new Session({ - apiBaseUrl: this.userConfig.apiBaseUrl, - apiClientId: this.userConfig.apiClientId, - apiClientSecret: this.userConfig.apiClientSecret, + userConfig: this.userConfig, atlasLocalClient: await this.atlasLocalClient, logger, exportsManager, diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index fe01cd78..41e585a7 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -96,9 +96,7 @@ export function setupIntegrationTest( const connectionManager = new MCPConnectionManager(userConfig, logger, deviceId); const session = new Session({ - apiBaseUrl: userConfig.apiBaseUrl, - apiClientId: userConfig.apiClientId, - apiClientSecret: userConfig.apiClientSecret, + userConfig, logger, exportsManager, connectionManager, diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 9cc88a78..84334136 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -21,7 +21,7 @@ describe("Telemetry", () => { const telemetry = Telemetry.create( new Session({ - apiBaseUrl: "", + userConfig: defaultTestConfig, logger, exportsManager: ExportsManager.init(config, logger), connectionManager: connectionManager, diff --git a/tests/integration/tools/mongodb/mongodbTool.test.ts b/tests/integration/tools/mongodb/mongodbTool.test.ts index 0ab1fc9f..56fd86bd 100644 --- a/tests/integration/tools/mongodb/mongodbTool.test.ts +++ b/tests/integration/tools/mongodb/mongodbTool.test.ts @@ -99,9 +99,7 @@ describe("MongoDBTool implementations", () => { deviceId = DeviceId.create(logger); const connectionManager = new MCPConnectionManager(userConfig, logger, deviceId); const session = new Session({ - apiBaseUrl: userConfig.apiBaseUrl, - apiClientId: userConfig.apiClientId, - apiClientSecret: userConfig.apiClientSecret, + userConfig, logger, exportsManager, connectionManager, diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts index d91ed7dc..f4eeacc7 100644 --- a/tests/unit/common/session.test.ts +++ b/tests/unit/common/session.test.ts @@ -27,8 +27,11 @@ describe("Session", () => { const connectionManager = new MCPConnectionManager(defaultTestConfig, logger, mockDeviceId); session = new Session({ - apiClientId: "test-client-id", - apiBaseUrl: "https://api.test.com", + userConfig: { + ...defaultTestConfig, + apiClientId: "test-client-id", + apiBaseUrl: "https://api.test.com", + }, logger, exportsManager: ExportsManager.init(defaultTestConfig, logger), connectionManager: connectionManager, diff --git a/tests/unit/resources/common/debug.test.ts b/tests/unit/resources/common/debug.test.ts index 3fade367..d160c662 100644 --- a/tests/unit/resources/common/debug.test.ts +++ b/tests/unit/resources/common/debug.test.ts @@ -17,7 +17,7 @@ describe("debug resource", () => { const session = vi.mocked( new Session({ - apiBaseUrl: "", + userConfig: defaultTestConfig, logger, exportsManager: ExportsManager.init(defaultTestConfig, logger), connectionManager,