Skip to content

fix: tweak the connect tool to prefer preconfigured connection string #88

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.

Sign up for GitHub

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 2 commits into from
Apr 22, 2025
Merged

fix: tweak the connect tool to prefer preconfigured connection string #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61ff695
fix: tweak the connect tool arguments to instruct the model to prefer…
nirinchev Apr 22, 2025
f2f9e65
fix tests
nirinchev Apr 22, 2025
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
24 changes: 24 additions & 0 deletions src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { AtlasTools } from "./tools/atlas/tools.js";
import { MongoDbTools } from "./tools/mongodb/tools.js";
import logger, { initializeLogger } from "./logger.js";
import { mongoLogId } from "mongodb-log-writer";
import config from "./config.js";

export class Server {
public readonly session: Session;
Expand All @@ -19,6 +20,7 @@ export class Server {
this.mcpServer.server.registerCapabilities({ logging: {} });

this.registerTools();
this.registerResources();

await initializeLogger(this.mcpServer);

Expand All @@ -37,4 +39,26 @@ export class Server {
new tool(this.session).register(this.mcpServer);
}
}

private registerResources() {
if (config.connectionString) {
this.mcpServer.resource(
"connection-string",
"config://connection-string",
{
description: "Preconfigured connection string that will be used as a default in the `connect` tool",
},
(uri) => {
return {
contents: [
{
text: `Preconfigured connection string: ${config.connectionString}`,
uri: uri.href,
},
],
};
}
);
}
}
}
46 changes: 27 additions & 19 deletions src/tools/mongodb/metadata/connect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,38 +9,46 @@ export class ConnectTool extends MongoDBToolBase {
protected name = "connect";
protected description = "Connect to a MongoDB instance";
protected argsShape = {
connectionStringOrClusterName: z
.string()
options: z
.array(
z
.union([
z.object({
connectionString: z
.string()
.describe("MongoDB connection string (in the mongodb:// or mongodb+srv:// format)"),
}),
z.object({
clusterName: z.string().describe("MongoDB cluster name"),
}),
])
.optional()
)
.optional()
.describe("MongoDB connection string (in the mongodb:// or mongodb+srv:// format) or cluster name"),
.describe(
"Options for connecting to MongoDB. If not provided, the connection string from the config://connection-string resource will be used. If the user hasn't specified Atlas cluster name or a connection string explicitly and the `config://connection-string` resource is present, always invoke this with no arguments."
),
};

protected operationType: OperationType = "metadata";

protected async execute({
connectionStringOrClusterName,
}: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> {
connectionStringOrClusterName ??= config.connectionString;
if (!connectionStringOrClusterName) {
protected async execute({ options: optionsArr }: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> {
const options = optionsArr?.[0];
let connectionString: string;
if (!options && !config.connectionString) {
return {
content: [
{ type: "text", text: "No connection details provided." },
{ type: "text", text: "Please provide either a connection string or a cluster name" },
{
type: "text",
text: "Alternatively, you can use the default deployment at mongodb://localhost:27017",
},
],
};
}

let connectionString: string;

if (
connectionStringOrClusterName.startsWith("mongodb://") ||
connectionStringOrClusterName.startsWith("mongodb+srv://")
) {
connectionString = connectionStringOrClusterName;
if (!options) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
connectionString = config.connectionString!;
} else if ("connectionString" in options) {
connectionString = options.connectionString;
} else {
// TODO: https://github.com/mongodb-js/mongodb-mcp-server/issues/19
// We don't support connecting via cluster name since we'd need to obtain the user credentials
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export function setupIntegrationTest(): {
connectMcpClient: async () => {
await getMcpClient().callTool({
name: "connect",
arguments: { connectionStringOrClusterName: getConnectionString() },
arguments: { options: [{ connectionString: getConnectionString() }] },
});
},
randomDbName: () => randomDbName,
Expand Down
42 changes: 33 additions & 9 deletions tests/integration/tools/mongodb/metadata/connect.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ describe("Connect tool", () => {

validateParameters(connectTool, [
{
name: "connectionStringOrClusterName",
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format) or cluster name",
type: "string",
name: "options",
description:
"Options for connecting to MongoDB. If not provided, the connection string from the config://connection-string resource will be used. If the user hasn't specified Atlas cluster name or a connection string explicitly and the `config://connection-string` resource is present, always invoke this with no arguments.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity, does this work well in practice? I'm afraid this may be specific to models being used though don't have a better solution for this either.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I tested it, it seemed to work well with Claude, Cursor, and Windsurf, though would be nice if others can verify it works as expected in their setups.

type: "array",
required: false,
},
]);
Expand All @@ -27,15 +28,20 @@ describe("Connect tool", () => {
const response = await integration.mcpClient().callTool({ name: "connect", arguments: {} });
const content = getResponseContent(response.content);
expect(content).toContain("No connection details provided");
expect(content).toContain("mongodb://localhost:27017");
});
});

describe("with connection string", () => {
it("connects to the database", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionStringOrClusterName: integration.connectionString() },
arguments: {
options: [
{
connectionString: integration.connectionString(),
},
],
},
});
const content = getResponseContent(response.content);
expect(content).toContain("Successfully connected");
Expand All @@ -47,7 +53,7 @@ describe("Connect tool", () => {
it("returns error message", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionStringOrClusterName: "mongodb://localhost:12345" },
arguments: { options: [{ connectionString: "mongodb://localhost:12345" }] },
});
const content = getResponseContent(response.content);
expect(content).toContain("Error running connect");
Expand All @@ -74,7 +80,13 @@ describe("Connect tool", () => {
const newConnectionString = `${integration.connectionString()}?appName=foo-bar`;
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionStringOrClusterName: newConnectionString },
arguments: {
options: [
{
connectionString: newConnectionString,
},
],
},
});
const content = getResponseContent(response.content);
expect(content).toContain("Successfully connected");
Expand All @@ -85,7 +97,13 @@ describe("Connect tool", () => {
it("suggests the config connection string if set", async () => {
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionStringOrClusterName: "mongodb://localhost:12345" },
arguments: {
options: [
{
connectionString: "mongodb://localhost:12345",
},
],
},
});
const content = getResponseContent(response.content);
expect(content).toContain("Failed to connect to MongoDB at 'mongodb://localhost:12345'");
Expand All @@ -98,7 +116,13 @@ describe("Connect tool", () => {
config.connectionString = "mongodb://localhost:12345";
const response = await integration.mcpClient().callTool({
name: "connect",
arguments: { connectionStringOrClusterName: "mongodb://localhost:12345" },
arguments: {
options: [
{
connectionString: "mongodb://localhost:12345",
},
],
},
});

const content = getResponseContent(response.content);
Expand Down
Loading