Skip to content

chore: disable the connect tool #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/code_health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
path: coverage/lcov.info

coverage:
name: Run MongoDB tests
name: Report Coverage
if: always() && github.event.pull_request.user.login != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest
needs: [run-tests, run-atlas-tests]
Expand Down
7 changes: 3 additions & 4 deletions src/common/atlas/apiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface ApiClientCredentials {

export interface ApiClientOptions {
credentials?: ApiClientCredentials;
baseUrl?: string;
baseUrl: string;
userAgent?: string;
}

Expand Down Expand Up @@ -63,12 +63,11 @@ export class ApiClient {
},
};

constructor(options?: ApiClientOptions) {
constructor(options: ApiClientOptions) {
this.options = {
...options,
baseUrl: options?.baseUrl || "https://cloud.mongodb.com/",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code was structured like that originally we moved https://cloud.mongodb.com/ to be part of apiClient to keep everything about the API in one place, any reason to move it back?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes more sense to have the defaults for the config options together rather than disconnected like that. That way we can see that the baseUrl option is actually always set and the exact value it is set to.

userAgent:
options?.userAgent ||
options.userAgent ||
`AtlasMCP/${packageInfo.version} (${process.platform}; ${process.arch}; ${process.env.HOSTNAME || "unknown"})`,
};

Expand Down
17 changes: 10 additions & 7 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,29 @@ import argv from "yargs-parser";

import { ReadConcernLevel, ReadPreferenceMode, W } from "mongodb";

export interface ConnectOptions {
readConcern: ReadConcernLevel;
readPreference: ReadPreferenceMode;
writeConcern: W;
timeoutMS: number;
}

// If we decide to support non-string config options, we'll need to extend the mechanism for parsing
// env variables.
export interface UserConfig {
apiBaseUrl?: string;
apiBaseUrl: string;
apiClientId?: string;
apiClientSecret?: string;
telemetry?: "enabled" | "disabled";
logPath: string;
connectionString?: string;
connectOptions: {
readConcern: ReadConcernLevel;
readPreference: ReadPreferenceMode;
writeConcern: W;
timeoutMS: number;
};
connectOptions: ConnectOptions;
disabledTools: Array<string>;
readOnly?: boolean;
}

const defaults: UserConfig = {
apiBaseUrl: "https://cloud.mongodb.com/",
logPath: getLogPath(),
connectOptions: {
readConcern: "local",
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ try {
name: packageInfo.mcpServerName,
version: packageInfo.version,
});

const server = new Server({
mcpServer,
session,
Expand Down
29 changes: 28 additions & 1 deletion src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class Server {
this.userConfig = userConfig;
}

async connect(transport: Transport) {
async connect(transport: Transport): Promise<void> {
this.mcpServer.server.registerCapabilities({ logging: {} });

this.registerTools();
Expand Down Expand Up @@ -88,6 +88,8 @@ export class Server {
const closeTime = Date.now();
this.emitServerEvent("stop", Date.now() - closeTime, error);
};

await this.validateConfig();
}

async close(): Promise<void> {
Expand Down Expand Up @@ -183,4 +185,29 @@ export class Server {
);
}
}

private async validateConfig(): Promise<void> {
const isAtlasConfigured = this.userConfig.apiClientId && this.userConfig.apiClientSecret;
const isMongoDbConfigured = this.userConfig.connectionString;
if (!isAtlasConfigured && !isMongoDbConfigured) {
console.error(
"Either Atlas Client Id or a MongoDB connection string must be configured - you can provide them as environment variables or as startup arguments. \n" +
"Provide the Atlas credentials as `MDB_MCP_API_CLIENT_ID` and `MDB_MCP_API_CLIENT_SECRET` environment variables or as `--apiClientId` and `--apiClientSecret` startup arguments. \n" +
"Provide the MongoDB connection string as `MDB_MCP_CONNECTION_STRING` environment variable or as `--connectionString` startup argument."
);
throw new Error("Either Atlas Client Id or a MongoDB connection string must be configured");
}

if (this.userConfig.connectionString) {
try {
await this.session.connectToMongoDB(this.userConfig.connectionString, this.userConfig.connectOptions);
} catch (error) {
console.error(
"Failed to connect to MongoDB instance using the connection string from the config: ",
error
);
throw new Error("Failed to connect to MongoDB instance using the connection string from the config");
}
}
}
}
22 changes: 20 additions & 2 deletions src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver
import { ApiClient, ApiClientCredentials } from "./common/atlas/apiClient.js";
import { Implementation } from "@modelcontextprotocol/sdk/types.js";
import EventEmitter from "events";
import { ConnectOptions } from "./config.js";

export interface SessionOptions {
apiBaseUrl?: string;
apiBaseUrl: string;
apiClientId?: string;
apiClientSecret?: string;
}
Expand All @@ -20,7 +21,7 @@ export class Session extends EventEmitter<{
version: string;
};

constructor({ apiBaseUrl, apiClientId, apiClientSecret }: SessionOptions = {}) {
constructor({ apiBaseUrl, apiClientId, apiClientSecret }: SessionOptions) {
super();

const credentials: ApiClientCredentials | undefined =
Expand Down Expand Up @@ -58,4 +59,21 @@ export class Session extends EventEmitter<{
this.emit("close");
}
}

async connectToMongoDB(connectionString: string, connectOptions: ConnectOptions): Promise<void> {
const provider = await NodeDriverServiceProvider.connect(connectionString, {
productDocsLink: "https://docs.mongodb.com/todo-mcp",
productName: "MongoDB MCP",
readConcern: {
level: connectOptions.readConcern,
},
readPreference: connectOptions.readPreference,
writeConcern: {
w: connectOptions.writeConcern,
},
timeoutMS: connectOptions.timeoutMS,
});

this.serviceProvider = provider;
}
}
17 changes: 2 additions & 15 deletions src/tools/mongodb/mongodbTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,7 @@ export abstract class MongoDBToolBase extends ToolBase {
return super.handleError(error, args);
}

protected async connectToMongoDB(connectionString: string): Promise<void> {
const provider = await NodeDriverServiceProvider.connect(connectionString, {
productDocsLink: "https://docs.mongodb.com/todo-mcp",
productName: "MongoDB MCP",
readConcern: {
level: this.config.connectOptions.readConcern,
},
readPreference: this.config.connectOptions.readPreference,
writeConcern: {
w: this.config.connectOptions.writeConcern,
},
timeoutMS: this.config.connectOptions.timeoutMS,
});

this.session.serviceProvider = provider;
protected connectToMongoDB(connectionString: string): Promise<void> {
return this.session.connectToMongoDB(connectionString, this.config.connectOptions);
}
}
6 changes: 4 additions & 2 deletions src/tools/mongodb/tools.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ConnectTool } from "./metadata/connect.js";
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/141 - reenable when the connect tool is reenabled
// import { ConnectTool } from "./metadata/connect.js";
import { ListCollectionsTool } from "./metadata/listCollections.js";
import { CollectionIndexesTool } from "./read/collectionIndexes.js";
import { ListDatabasesTool } from "./metadata/listDatabases.js";
Expand All @@ -20,7 +21,8 @@ import { CreateCollectionTool } from "./create/createCollection.js";
import { LogsTool } from "./metadata/logs.js";

export const MongoDbTools = [
ConnectTool,
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/141 - reenable when the connect tool is reenabled
// ConnectTool,
ListCollectionsTool,
ListDatabasesTool,
CollectionIndexesTool,
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
import { InMemoryTransport } from "./inMemoryTransport.js";
import { Server } from "../../src/server.js";
import { config, UserConfig } from "../../src/config.js";
import { UserConfig } from "../../src/config.js";
import { McpError } from "@modelcontextprotocol/sdk/types.js";
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { Session } from "../../src/session.js";
Expand All @@ -20,7 +20,7 @@ export interface IntegrationTest {
mcpServer: () => Server;
}

export function setupIntegrationTest(getUserConfig: () => UserConfig = () => config): IntegrationTest {
export function setupIntegrationTest(getUserConfig: () => UserConfig): IntegrationTest {
let mcpClient: Client | undefined;
let mcpServer: Server | undefined;

Expand Down
28 changes: 16 additions & 12 deletions tests/integration/server.test.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
import { expectDefined, setupIntegrationTest } from "./helpers.js";
import { config } from "../../src/config.js";
import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";

describe("Server integration test", () => {
describe("without atlas", () => {
const integration = setupIntegrationTest(() => ({
describeWithMongoDB(
"without atlas",
(integration) => {
it("should return positive number of tools and have no atlas tools", async () => {
const tools = await integration.mcpClient().listTools();
expectDefined(tools);
expect(tools.tools.length).toBeGreaterThan(0);

const atlasTools = tools.tools.filter((tool) => tool.name.startsWith("atlas-"));
expect(atlasTools.length).toBeLessThanOrEqual(0);
});
},
() => ({
...config,
apiClientId: undefined,
apiClientSecret: undefined,
}));
})
);

it("should return positive number of tools and have no atlas tools", async () => {
const tools = await integration.mcpClient().listTools();
expectDefined(tools);
expect(tools.tools.length).toBeGreaterThan(0);

const atlasTools = tools.tools.filter((tool) => tool.name.startsWith("atlas-"));
expect(atlasTools.length).toBeLessThanOrEqual(0);
});
});
describe("with atlas", () => {
const integration = setupIntegrationTest(() => ({
...config,
Expand Down
8 changes: 7 additions & 1 deletion tests/integration/tools/atlas/atlasHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ObjectId } from "mongodb";
import { Group } from "../../../../src/common/atlas/openapi.js";
import { ApiClient } from "../../../../src/common/atlas/apiClient.js";
import { setupIntegrationTest, IntegrationTest } from "../../helpers.js";
import { config } from "../../../../src/config.js";

export type IntegrationTestFunction = (integration: IntegrationTest) => void;

Expand All @@ -11,7 +12,12 @@ export function sleep(ms: number) {

export function describeWithAtlas(name: string, fn: IntegrationTestFunction) {
const testDefinition = () => {
const integration = setupIntegrationTest();
const integration = setupIntegrationTest(() => ({
...config,
apiClientId: process.env.MDB_MCP_API_CLIENT_ID,
apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET,
}));

describe(name, () => {
fn(integration);
});
Expand Down
84 changes: 46 additions & 38 deletions tests/integration/tools/mongodb/metadata/connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { describeWithMongoDB } from "../mongodbHelpers.js";
import { getResponseContent, validateThrowsForInvalidArguments, validateToolMetadata } from "../../../helpers.js";
import { config } from "../../../../../src/config.js";

// These tests are temporarily skipped because the connect tool is disabled for the initial release.
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/141 - reenable when the connect tool is reenabled
describeWithMongoDB(
"switchConnection tool",
(integration) => {
Expand Down Expand Up @@ -75,50 +77,56 @@ describeWithMongoDB(
(mdbIntegration) => ({
...config,
connectionString: mdbIntegration.connectionString(),
})
}),
describe.skip
);
describeWithMongoDB("Connect tool", (integration) => {
validateToolMetadata(integration, "connect", "Connect to a MongoDB instance", [
{
name: "connectionString",
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
type: "string",
required: true,
},
]);
describeWithMongoDB(
"Connect tool",
(integration) => {
validateToolMetadata(integration, "connect", "Connect to a MongoDB instance", [
{
name: "connectionString",
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
type: "string",
required: true,
},
]);

validateThrowsForInvalidArguments(integration, "connect", [{}, { connectionString: 123 }]);
validateThrowsForInvalidArguments(integration, "connect", [{}, { connectionString: 123 }]);

it("doesn't have the switch-connection tool registered", async () => {
const { tools } = await integration.mcpClient().listTools();
const tool = tools.find((tool) => tool.name === "switch-connection");
expect(tool).toBeUndefined();
});
it("doesn't have the switch-connection tool registered", async () => {
const { tools } = await integration.mcpClient().listTools();
const tool = tools.find((tool) => tool.name === "switch-connection");
expect(tool).toBeUndefined();
});

describe("with connection string", () => {
it("connects to the database", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: {
connectionString: integration.connectionString(),
},
describe("with connection string", () => {
it("connects to the database", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: {
connectionString: integration.connectionString(),
},
});
const content = getResponseContent(response.content);
expect(content).toContain("Successfully connected");
});
const content = getResponseContent(response.content);
expect(content).toContain("Successfully connected");
});
});

describe("with invalid connection string", () => {
it("returns error message", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionString: "mongodb://localhost:12345" },
});
const content = getResponseContent(response.content);
expect(content).toContain("Error running connect");
describe("with invalid connection string", () => {
it("returns error message", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionString: "mongodb://localhost:12345" },
});
const content = getResponseContent(response.content);
expect(content).toContain("Error running connect");

// Should not suggest using the config connection string (because we don't have one)
expect(content).not.toContain("Your config lists a different connection string");
// Should not suggest using the config connection string (because we don't have one)
expect(content).not.toContain("Your config lists a different connection string");
});
});
});
});
},
() => config,
describe.skip
);
Loading
Loading