Skip to content

Commit 355fbf2

Browse files
authored
chore: disable the connect tool (#142)
1 parent cdf992f commit 355fbf2

File tree

13 files changed

+157
-95
lines changed

13 files changed

+157
-95
lines changed

.github/workflows/code_health.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ jobs:
6363
path: coverage/lcov.info
6464

6565
coverage:
66-
name: Run MongoDB tests
66+
name: Report Coverage
6767
if: always() && github.event.pull_request.user.login != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository
6868
runs-on: ubuntu-latest
6969
needs: [run-tests, run-atlas-tests]

src/common/atlas/apiClient.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export interface ApiClientCredentials {
1515

1616
export interface ApiClientOptions {
1717
credentials?: ApiClientCredentials;
18-
baseUrl?: string;
18+
baseUrl: string;
1919
userAgent?: string;
2020
}
2121

@@ -63,12 +63,11 @@ export class ApiClient {
6363
},
6464
};
6565

66-
constructor(options?: ApiClientOptions) {
66+
constructor(options: ApiClientOptions) {
6767
this.options = {
6868
...options,
69-
baseUrl: options?.baseUrl || "https://cloud.mongodb.com/",
7069
userAgent:
71-
options?.userAgent ||
70+
options.userAgent ||
7271
`AtlasMCP/${packageInfo.version} (${process.platform}; ${process.arch}; ${process.env.HOSTNAME || "unknown"})`,
7372
};
7473

src/config.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,29 @@ import argv from "yargs-parser";
44

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

7+
export interface ConnectOptions {
8+
readConcern: ReadConcernLevel;
9+
readPreference: ReadPreferenceMode;
10+
writeConcern: W;
11+
timeoutMS: number;
12+
}
13+
714
// If we decide to support non-string config options, we'll need to extend the mechanism for parsing
815
// env variables.
916
export interface UserConfig {
10-
apiBaseUrl?: string;
17+
apiBaseUrl: string;
1118
apiClientId?: string;
1219
apiClientSecret?: string;
1320
telemetry?: "enabled" | "disabled";
1421
logPath: string;
1522
connectionString?: string;
16-
connectOptions: {
17-
readConcern: ReadConcernLevel;
18-
readPreference: ReadPreferenceMode;
19-
writeConcern: W;
20-
timeoutMS: number;
21-
};
23+
connectOptions: ConnectOptions;
2224
disabledTools: Array<string>;
2325
readOnly?: boolean;
2426
}
2527

2628
const defaults: UserConfig = {
29+
apiBaseUrl: "https://cloud.mongodb.com/",
2730
logPath: getLogPath(),
2831
connectOptions: {
2932
readConcern: "local",

src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ try {
1818
name: packageInfo.mcpServerName,
1919
version: packageInfo.version,
2020
});
21+
2122
const server = new Server({
2223
mcpServer,
2324
session,

src/server.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export class Server {
3333
this.userConfig = userConfig;
3434
}
3535

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

3939
this.registerTools();
@@ -88,6 +88,8 @@ export class Server {
8888
const closeTime = Date.now();
8989
this.emitServerEvent("stop", Date.now() - closeTime, error);
9090
};
91+
92+
await this.validateConfig();
9193
}
9294

9395
async close(): Promise<void> {
@@ -183,4 +185,29 @@ export class Server {
183185
);
184186
}
185187
}
188+
189+
private async validateConfig(): Promise<void> {
190+
const isAtlasConfigured = this.userConfig.apiClientId && this.userConfig.apiClientSecret;
191+
const isMongoDbConfigured = this.userConfig.connectionString;
192+
if (!isAtlasConfigured && !isMongoDbConfigured) {
193+
console.error(
194+
"Either Atlas Client Id or a MongoDB connection string must be configured - you can provide them as environment variables or as startup arguments. \n" +
195+
"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" +
196+
"Provide the MongoDB connection string as `MDB_MCP_CONNECTION_STRING` environment variable or as `--connectionString` startup argument."
197+
);
198+
throw new Error("Either Atlas Client Id or a MongoDB connection string must be configured");
199+
}
200+
201+
if (this.userConfig.connectionString) {
202+
try {
203+
await this.session.connectToMongoDB(this.userConfig.connectionString, this.userConfig.connectOptions);
204+
} catch (error) {
205+
console.error(
206+
"Failed to connect to MongoDB instance using the connection string from the config: ",
207+
error
208+
);
209+
throw new Error("Failed to connect to MongoDB instance using the connection string from the config");
210+
}
211+
}
212+
}
186213
}

src/session.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver
22
import { ApiClient, ApiClientCredentials } from "./common/atlas/apiClient.js";
33
import { Implementation } from "@modelcontextprotocol/sdk/types.js";
44
import EventEmitter from "events";
5+
import { ConnectOptions } from "./config.js";
56

67
export interface SessionOptions {
7-
apiBaseUrl?: string;
8+
apiBaseUrl: string;
89
apiClientId?: string;
910
apiClientSecret?: string;
1011
}
@@ -20,7 +21,7 @@ export class Session extends EventEmitter<{
2021
version: string;
2122
};
2223

23-
constructor({ apiBaseUrl, apiClientId, apiClientSecret }: SessionOptions = {}) {
24+
constructor({ apiBaseUrl, apiClientId, apiClientSecret }: SessionOptions) {
2425
super();
2526

2627
const credentials: ApiClientCredentials | undefined =
@@ -58,4 +59,21 @@ export class Session extends EventEmitter<{
5859
this.emit("close");
5960
}
6061
}
62+
63+
async connectToMongoDB(connectionString: string, connectOptions: ConnectOptions): Promise<void> {
64+
const provider = await NodeDriverServiceProvider.connect(connectionString, {
65+
productDocsLink: "https://docs.mongodb.com/todo-mcp",
66+
productName: "MongoDB MCP",
67+
readConcern: {
68+
level: connectOptions.readConcern,
69+
},
70+
readPreference: connectOptions.readPreference,
71+
writeConcern: {
72+
w: connectOptions.writeConcern,
73+
},
74+
timeoutMS: connectOptions.timeoutMS,
75+
});
76+
77+
this.serviceProvider = provider;
78+
}
6179
}

src/tools/mongodb/mongodbTool.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,7 @@ export abstract class MongoDBToolBase extends ToolBase {
7070
return super.handleError(error, args);
7171
}
7272

73-
protected async connectToMongoDB(connectionString: string): Promise<void> {
74-
const provider = await NodeDriverServiceProvider.connect(connectionString, {
75-
productDocsLink: "https://docs.mongodb.com/todo-mcp",
76-
productName: "MongoDB MCP",
77-
readConcern: {
78-
level: this.config.connectOptions.readConcern,
79-
},
80-
readPreference: this.config.connectOptions.readPreference,
81-
writeConcern: {
82-
w: this.config.connectOptions.writeConcern,
83-
},
84-
timeoutMS: this.config.connectOptions.timeoutMS,
85-
});
86-
87-
this.session.serviceProvider = provider;
73+
protected connectToMongoDB(connectionString: string): Promise<void> {
74+
return this.session.connectToMongoDB(connectionString, this.config.connectOptions);
8875
}
8976
}

src/tools/mongodb/tools.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { ConnectTool } from "./metadata/connect.js";
1+
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/141 - reenable when the connect tool is reenabled
2+
// import { ConnectTool } from "./metadata/connect.js";
23
import { ListCollectionsTool } from "./metadata/listCollections.js";
34
import { CollectionIndexesTool } from "./read/collectionIndexes.js";
45
import { ListDatabasesTool } from "./metadata/listDatabases.js";
@@ -20,7 +21,8 @@ import { CreateCollectionTool } from "./create/createCollection.js";
2021
import { LogsTool } from "./metadata/logs.js";
2122

2223
export const MongoDbTools = [
23-
ConnectTool,
24+
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/141 - reenable when the connect tool is reenabled
25+
// ConnectTool,
2426
ListCollectionsTool,
2527
ListDatabasesTool,
2628
CollectionIndexesTool,

tests/integration/helpers.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
22
import { InMemoryTransport } from "./inMemoryTransport.js";
33
import { Server } from "../../src/server.js";
4-
import { config, UserConfig } from "../../src/config.js";
4+
import { UserConfig } from "../../src/config.js";
55
import { McpError } from "@modelcontextprotocol/sdk/types.js";
66
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
77
import { Session } from "../../src/session.js";
@@ -20,7 +20,7 @@ export interface IntegrationTest {
2020
mcpServer: () => Server;
2121
}
2222

23-
export function setupIntegrationTest(getUserConfig: () => UserConfig = () => config): IntegrationTest {
23+
export function setupIntegrationTest(getUserConfig: () => UserConfig): IntegrationTest {
2424
let mcpClient: Client | undefined;
2525
let mcpServer: Server | undefined;
2626

tests/integration/server.test.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
11
import { expectDefined, setupIntegrationTest } from "./helpers.js";
22
import { config } from "../../src/config.js";
3+
import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";
34

45
describe("Server integration test", () => {
5-
describe("without atlas", () => {
6-
const integration = setupIntegrationTest(() => ({
6+
describeWithMongoDB(
7+
"without atlas",
8+
(integration) => {
9+
it("should return positive number of tools and have no atlas tools", async () => {
10+
const tools = await integration.mcpClient().listTools();
11+
expectDefined(tools);
12+
expect(tools.tools.length).toBeGreaterThan(0);
13+
14+
const atlasTools = tools.tools.filter((tool) => tool.name.startsWith("atlas-"));
15+
expect(atlasTools.length).toBeLessThanOrEqual(0);
16+
});
17+
},
18+
() => ({
719
...config,
820
apiClientId: undefined,
921
apiClientSecret: undefined,
10-
}));
22+
})
23+
);
1124

12-
it("should return positive number of tools and have no atlas tools", async () => {
13-
const tools = await integration.mcpClient().listTools();
14-
expectDefined(tools);
15-
expect(tools.tools.length).toBeGreaterThan(0);
16-
17-
const atlasTools = tools.tools.filter((tool) => tool.name.startsWith("atlas-"));
18-
expect(atlasTools.length).toBeLessThanOrEqual(0);
19-
});
20-
});
2125
describe("with atlas", () => {
2226
const integration = setupIntegrationTest(() => ({
2327
...config,

tests/integration/tools/atlas/atlasHelpers.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ObjectId } from "mongodb";
22
import { Group } from "../../../../src/common/atlas/openapi.js";
33
import { ApiClient } from "../../../../src/common/atlas/apiClient.js";
44
import { setupIntegrationTest, IntegrationTest } from "../../helpers.js";
5+
import { config } from "../../../../src/config.js";
56

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

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

1213
export function describeWithAtlas(name: string, fn: IntegrationTestFunction) {
1314
const testDefinition = () => {
14-
const integration = setupIntegrationTest();
15+
const integration = setupIntegrationTest(() => ({
16+
...config,
17+
apiClientId: process.env.MDB_MCP_API_CLIENT_ID,
18+
apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET,
19+
}));
20+
1521
describe(name, () => {
1622
fn(integration);
1723
});

tests/integration/tools/mongodb/metadata/connect.test.ts

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { describeWithMongoDB } from "../mongodbHelpers.js";
22
import { getResponseContent, validateThrowsForInvalidArguments, validateToolMetadata } from "../../../helpers.js";
33
import { config } from "../../../../../src/config.js";
44

5+
// These tests are temporarily skipped because the connect tool is disabled for the initial release.
6+
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/141 - reenable when the connect tool is reenabled
57
describeWithMongoDB(
68
"switchConnection tool",
79
(integration) => {
@@ -75,50 +77,56 @@ describeWithMongoDB(
7577
(mdbIntegration) => ({
7678
...config,
7779
connectionString: mdbIntegration.connectionString(),
78-
})
80+
}),
81+
describe.skip
7982
);
80-
describeWithMongoDB("Connect tool", (integration) => {
81-
validateToolMetadata(integration, "connect", "Connect to a MongoDB instance", [
82-
{
83-
name: "connectionString",
84-
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
85-
type: "string",
86-
required: true,
87-
},
88-
]);
83+
describeWithMongoDB(
84+
"Connect tool",
85+
(integration) => {
86+
validateToolMetadata(integration, "connect", "Connect to a MongoDB instance", [
87+
{
88+
name: "connectionString",
89+
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
90+
type: "string",
91+
required: true,
92+
},
93+
]);
8994

90-
validateThrowsForInvalidArguments(integration, "connect", [{}, { connectionString: 123 }]);
95+
validateThrowsForInvalidArguments(integration, "connect", [{}, { connectionString: 123 }]);
9196

92-
it("doesn't have the switch-connection tool registered", async () => {
93-
const { tools } = await integration.mcpClient().listTools();
94-
const tool = tools.find((tool) => tool.name === "switch-connection");
95-
expect(tool).toBeUndefined();
96-
});
97+
it("doesn't have the switch-connection tool registered", async () => {
98+
const { tools } = await integration.mcpClient().listTools();
99+
const tool = tools.find((tool) => tool.name === "switch-connection");
100+
expect(tool).toBeUndefined();
101+
});
97102

98-
describe("with connection string", () => {
99-
it("connects to the database", async () => {
100-
const response = await integration.mcpClient().callTool({
101-
name: "connect",
102-
arguments: {
103-
connectionString: integration.connectionString(),
104-
},
103+
describe("with connection string", () => {
104+
it("connects to the database", async () => {
105+
const response = await integration.mcpClient().callTool({
106+
name: "connect",
107+
arguments: {
108+
connectionString: integration.connectionString(),
109+
},
110+
});
111+
const content = getResponseContent(response.content);
112+
expect(content).toContain("Successfully connected");
105113
});
106-
const content = getResponseContent(response.content);
107-
expect(content).toContain("Successfully connected");
108114
});
109-
});
110115

111-
describe("with invalid connection string", () => {
112-
it("returns error message", async () => {
113-
const response = await integration.mcpClient().callTool({
114-
name: "connect",
115-
arguments: { connectionString: "mongodb://localhost:12345" },
116-
});
117-
const content = getResponseContent(response.content);
118-
expect(content).toContain("Error running connect");
116+
describe("with invalid connection string", () => {
117+
it("returns error message", async () => {
118+
const response = await integration.mcpClient().callTool({
119+
name: "connect",
120+
arguments: { connectionString: "mongodb://localhost:12345" },
121+
});
122+
const content = getResponseContent(response.content);
123+
expect(content).toContain("Error running connect");
119124

120-
// Should not suggest using the config connection string (because we don't have one)
121-
expect(content).not.toContain("Your config lists a different connection string");
125+
// Should not suggest using the config connection string (because we don't have one)
126+
expect(content).not.toContain("Your config lists a different connection string");
127+
});
122128
});
123-
});
124-
});
129+
},
130+
() => config,
131+
describe.skip
132+
);

0 commit comments

Comments
 (0)