Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
48 changes: 24 additions & 24 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"node": "^20.19.0 || ^22.12.0 || >= 23.0.0"
},
"optionalDependencies": {
"@mongodb-js-preview/atlas-local": "^0.0.0-preview.3",
"@mongodb-js-preview/atlas-local": "^0.0.0-preview.6",
"kerberos": "^2.2.2"
}
}
15 changes: 12 additions & 3 deletions src/common/connectionErrorHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,18 @@ export const connectionErrorHandler: ConnectionErrorHandler = (error, { availabl
// Find the first Atlas connect tool if available and suggest to the LLM to use it.
// Note: if we ever have multiple Atlas connect tools, we may want to refine this logic to select the most appropriate one.
const atlasConnectTool = connectTools?.find((t) => t.category === "atlas");
const llmConnectHint = atlasConnectTool
? `Note to LLM: prefer using the "${atlasConnectTool.name}" tool to connect to an Atlas cluster over using a connection string. Make sure to ask the user to specify a cluster name they want to connect to or ask them if they want to use the "list-clusters" tool to list all their clusters. Do not invent cluster names or connection strings unless the user has explicitly specified them. If they've previously connected to MongoDB using MCP, you can ask them if they want to reconnect using the same cluster/connection.`
: "Note to LLM: do not invent connection strings and explicitly ask the user to provide one. If they have previously connected to MongoDB using MCP, you can ask them if they want to reconnect using the same connection string.";
const atlasLocalConnectTool = connectTools?.find((t) => t.category === "atlas-local");

let llmConnectHint: string;

if (atlasConnectTool) {
llmConnectHint = `Note to LLM: prefer using the "${atlasConnectTool.name}" tool to connect to an Atlas cluster over using a connection string. Make sure to ask the user to specify a cluster name they want to connect to or ask them if they want to use the "list-clusters" tool to list all their clusters. Do not invent cluster names or connection strings unless the user has explicitly specified them. If they've previously connected to MongoDB using MCP, you can ask them if they want to reconnect using the same cluster/connection.`;
} else if (atlasLocalConnectTool) {
llmConnectHint = `Note to LLM: prefer using the "${atlasLocalConnectTool.name}" tool for connecting to local MongoDB deployments. Ask the user to specify a deployment name or use "atlas-local-list-deployments" to show available local deployments. Do not invent deployment names unless the user has explicitly specified them. If they've previously connected to a local MongoDB deployment using MCP, you can ask them if they want to reconnect using the same deployment.`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm... I wonder if we want to be that prescriptive - my understanding is that this tool would be available as long as the user has docker installed, regardless of whether they're connecting to a local deployment or not. I guess we'd need to vibe check this in the real world.

Copy link
Collaborator Author

@cveticm cveticm Oct 7, 2025

Choose a reason for hiding this comment

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

@nirinchev Thoughts on these actions?:

  1. Change the logic such that both atlasConnectTool and atlasLocalConnectTool hints can be provided if both tools are available. (currently only atlasConnectTool hint is printed if the tool is avb)
  2. Change atlasLocalConnectTool hint to not have preference for it - since the connectTool can also be used to connect to local deployments if given the connection string.
Suggested change
llmConnectHint = `Note to LLM: prefer using the "${atlasLocalConnectTool.name}" tool for connecting to local MongoDB deployments. Ask the user to specify a deployment name or use "atlas-local-list-deployments" to show available local deployments. Do not invent deployment names unless the user has explicitly specified them. If they've previously connected to a local MongoDB deployment using MCP, you can ask them if they want to reconnect using the same deployment.`;
llmConnectHint = `Note to LLM: For Atlas Local deployments, ask the user to either provide a connection string, specify a deployment name, or use "atlas-local-list-deployments" to show available local deployments. If a deployment name is provided, prefer using the "${atlasLocalConnectTool.name}" tool. If a connection string is provided, prefer using the "${connectTool.name}" tool. Do not invent deployment names or connection strings unless the user has explicitly specified them. If they've previously connected to an Atlas Local deployment using MCP, you can ask them if they want to reconnect using the same deployment.`;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds reasonable to me!

} else {
llmConnectHint =
"Note to LLM: do not invent connection strings and explicitly ask the user to provide one. If they have previously connected to MongoDB using MCP, you can ask them if they want to reconnect using the same connection string.";
}

const connectToolsNames = connectTools?.map((t) => `"${t.name}"`).join(", ");
const additionalPromptForConnectivity: { type: "text"; text: string }[] = [];
Expand Down
34 changes: 34 additions & 0 deletions src/tools/atlasLocal/connect/connectDeployment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
import { AtlasLocalToolBase } from "../atlasLocalTool.js";
import type { OperationType, ToolArgs } from "../../tool.js";
import type { Client } from "@mongodb-js-preview/atlas-local";
import { z } from "zod";

export class ConnectDeploymentTool extends AtlasLocalToolBase {
public name = "atlas-local-connect-deployment";
protected description = "Connect to a MongoDB Atlas Local deployment";
public operationType: OperationType = "connect";
protected argsShape = {
deploymentIdOrName: z.string().describe("Name or ID of the deployment to connect to"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this take id if we're not surfacing the id in list-deployments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tool can handle both id and name but I agree, since we don't expose the container id (even in cli) I changed the input name and description to avoid confusion

};

protected async executeWithAtlasLocalClient(
client: Client,
{ deploymentIdOrName }: ToolArgs<typeof this.argsShape>
): Promise<CallToolResult> {
// Get the connection string for the deployment
const connectionString = await client.getConnectionString(deploymentIdOrName);

// Connect to the deployment
await this.session.connectToMongoDB({ connectionString });

return {
content: [
{
type: "text",
text: `Successfully connected to Atlas Local deployment "${deploymentIdOrName}".`,
},
],
};
}
}
3 changes: 2 additions & 1 deletion src/tools/atlasLocal/tools.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { DeleteDeploymentTool } from "./delete/deleteDeployment.js";
import { ListDeploymentsTool } from "./read/listDeployments.js";
import { CreateDeploymentTool } from "./create/createDeployment.js";
import { ConnectDeploymentTool } from "./connect/connectDeployment.js";

export const AtlasLocalTools = [ListDeploymentsTool, DeleteDeploymentTool, CreateDeploymentTool];
export const AtlasLocalTools = [ListDeploymentsTool, DeleteDeploymentTool, CreateDeploymentTool, ConnectDeploymentTool];
70 changes: 70 additions & 0 deletions tests/accuracy/connectDeployment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { describeAccuracyTests } from "./sdk/describeAccuracyTests.js";
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";

describeAccuracyTests([
{
prompt: "Connect to the local MongoDB cluster called 'my-database'",
expectedToolCalls: [
{
toolName: "atlas-local-connect-deployment",
parameters: {
deploymentIdOrName: "my-database",
},
},
],
},
{
prompt: "Connect to the local MongoDB atlas database called 'my-instance'",
expectedToolCalls: [
{
toolName: "atlas-local-connect-deployment",
parameters: {
deploymentIdOrName: "my-instance",
},
},
],
},
{
prompt: "If and only if, the local MongoDB deployment 'local-mflix' exists, then connect to it",
mockedTools: {
"atlas-local-list-deployments": (): CallToolResult => ({
content: [
{ type: "text", text: "Found 1 deployment:" },
{
type: "text",
text: "Deployment Name | State | MongoDB Version\n----------------|----------------|----------------\nlocal-mflix | Running | 6.0",
},
],
}),
},
expectedToolCalls: [
{
toolName: "atlas-local-list-deployments",
parameters: {},
},
{
toolName: "atlas-local-connect-deployment",
parameters: {
deploymentIdOrName: "local-mflix",
},
},
],
},
{
prompt: "Connect to a new local MongoDB cluster named 'local-mflix'",
expectedToolCalls: [
{
toolName: "atlas-local-create-deployment",
parameters: {
deploymentName: "local-mflix",
},
},
{
toolName: "atlas-local-connect-deployment",
parameters: {
deploymentIdOrName: "local-mflix",
},
},
],
},
]);
98 changes: 98 additions & 0 deletions tests/integration/tools/atlas-local/connectDeployment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import {
defaultDriverOptions,
defaultTestConfig,
expectDefined,
getResponseElements,
setupIntegrationTest,
waitUntilMcpClientIsSet,
} from "../../helpers.js";
import { describe, expect, it } from "vitest";

const isMacOSInGitHubActions = process.platform === "darwin" && process.env.GITHUB_ACTIONS === "true";

// Docker is not available on macOS in GitHub Actions
// That's why we skip the tests on macOS in GitHub Actions
describe("atlas-local-connect-deployment", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we move this .skipIf here?

const integration = setupIntegrationTest(
() => defaultTestConfig,
() => defaultDriverOptions
);

it.skipIf(isMacOSInGitHubActions)("should have the atlas-local-connect-deployment tool", async ({ signal }) => {
await waitUntilMcpClientIsSet(integration.mcpServer(), signal);

const { tools } = await integration.mcpClient().listTools();
const connectDeployment = tools.find((tool) => tool.name === "atlas-local-connect-deployment");
expectDefined(connectDeployment);
});

it.skipIf(!isMacOSInGitHubActions)(
"[MacOS in GitHub Actions] should not have the atlas-local-connect-deployment tool",
async ({ signal }) => {
// This should throw an error because the client is not set within the timeout of 5 seconds (default)
await expect(waitUntilMcpClientIsSet(integration.mcpServer(), signal)).rejects.toThrow();

const { tools } = await integration.mcpClient().listTools();
const connectDeployment = tools.find((tool) => tool.name === "atlas-local-connect-deployment");
expect(connectDeployment).toBeUndefined();
}
);

it.skipIf(isMacOSInGitHubActions)("should have correct metadata", async ({ signal }) => {
await waitUntilMcpClientIsSet(integration.mcpServer(), signal);
const { tools } = await integration.mcpClient().listTools();
const connectDeployment = tools.find((tool) => tool.name === "atlas-local-connect-deployment");
expectDefined(connectDeployment);
expect(connectDeployment.inputSchema.type).toBe("object");
expectDefined(connectDeployment.inputSchema.properties);
expect(connectDeployment.inputSchema.properties).toHaveProperty("deploymentIdOrName");
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider using the validateToolMetadata helper to simplify tests like this.


it.skipIf(isMacOSInGitHubActions)(
"should return 'no such container' error when connecting to non-existent deployment",
async ({ signal }) => {
await waitUntilMcpClientIsSet(integration.mcpServer(), signal);

const response = await integration.mcpClient().callTool({
name: "atlas-local-connect-deployment",
arguments: { deploymentIdOrName: "non-existent" },
});
const elements = getResponseElements(response.content);
expect(elements.length).toBeGreaterThanOrEqual(1);
expect(elements[0]?.text).toContain(
"Docker responded with status code 404: No such container: non-existent"
Copy link
Collaborator

Choose a reason for hiding this comment

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

q: should we be translating these errors? I don't imagine customers care too deeply about docker containers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sounds good, added error handling to atlas local tools. This can be expanded in future to translate more errors

);
}
);

it.skipIf(isMacOSInGitHubActions)("should connect to a deployment when calling the tool", async ({ signal }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider creating a separate suite for this case with its own setup/cleanup. Something like:

describe("when local deployments are available", () => {
  beforeEach(() => {
    // create deployment
  });

  afterEach(() => {
    // cleanup deployment
  });

  it("should connect to a deployment...", () => {
    // test without the setup
  });
});

Also, might be a good idea to test when there are more than one deployments that we connect to the correct one. As things stand today, all tests will pass if we disregard the deployment id/name argument and instead connect to the first available deployment.

await waitUntilMcpClientIsSet(integration.mcpServer(), signal);
// Create a deployment
const deploymentName = `test-deployment-${Date.now()}`;
await integration.mcpClient().callTool({
name: "atlas-local-create-deployment",
arguments: { deploymentName },
});

// Connect to the deployment
const response = await integration.mcpClient().callTool({
name: "atlas-local-connect-deployment",
arguments: { deploymentIdOrName: deploymentName },
});
const elements = getResponseElements(response.content);
expect(elements.length).toBeGreaterThanOrEqual(1);
expect(elements[0]?.text).toContain(
'Successfully connected to Atlas Local deployment "' + deploymentName + '".'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'Successfully connected to Atlas Local deployment "' + deploymentName + '".'
`Successfully connected to Atlas Local deployment "${deploymentName}".`

);

// cleanup
try {
await integration.mcpClient().callTool({
name: "atlas-local-delete-deployment",
arguments: { deploymentName },
});
} catch (error) {
console.warn(`Failed to delete deployment ${deploymentName}:`, error);
}
});
});
2 changes: 1 addition & 1 deletion tests/integration/tools/atlas/clusters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ describeWithAtlas("clusters", (integration) => {
"You need to connect to a MongoDB instance before you can access its data."
);
expect(elements[1]?.text).toContain(
'Please use one of the following tools: "atlas-connect-cluster", "connect" to connect to a MongoDB instance'
'Please use one of the following tools: "atlas-connect-cluster", "atlas-local-connect-deployment", "connect" to connect to a MongoDB instance'
Copy link
Collaborator Author

@cveticm cveticm Oct 6, 2025

Choose a reason for hiding this comment

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

added to fix test fail

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm... should this be the case across the board? I.e. on macOS GHA runners, I'd expect this tool to be unavailable, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point @nirinchev , i was only considering how it runs in cicd. Changing it to expect either depending on testing environment

);
});
});
Expand Down
Loading