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
7 changes: 5 additions & 2 deletions src/common/atlas/accessListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ export async function makeCurrentIpAccessListEntry(
* If the IP is already present, this is a no-op.
* @param apiClient The Atlas API client instance
* @param projectId The Atlas project ID
* @returns Promise<boolean> - true if a new IP access list entry was created, false if it already existed
*/
export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectId: string): Promise<void> {
export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectId: string): Promise<boolean> {
const entry = await makeCurrentIpAccessListEntry(apiClient, projectId, DEFAULT_ACCESS_LIST_COMMENT);
try {
await apiClient.createProjectIpAccessList({
Expand All @@ -35,6 +36,7 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI
context: "accessListUtils",
message: `IP access list created: ${JSON.stringify(entry)}`,
});
return true;
} catch (err) {
if (err instanceof ApiClientError && err.response?.status === 409) {
// 409 Conflict: entry already exists, log info
Expand All @@ -43,12 +45,13 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI
context: "accessListUtils",
message: `IP address ${entry.ipAddress} is already present in the access list for project ${projectId}.`,
});
return;
return false;
}
apiClient.logger.warning({
id: LogId.atlasIpAccessListAddFailure,
context: "accessListUtils",
message: `Error adding IP access list: ${err instanceof Error ? err.message : String(err)}`,
});
}
return false;
}
91 changes: 63 additions & 28 deletions src/tools/atlas/connect/connectCluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import type { AtlasClusterConnectionInfo } from "../../../common/connectionManag
import { getDefaultRoleFromConfig } from "../../../common/atlas/roles.js";

const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours
const addedIpAccessListMessage =
"Note: Your current IP address has been added to the Atlas project's IP access list to enable secure connection.";

const createdUserMessage =
"Note: A temporary user has been created to enable secure connection to the cluster. For more information, see https://dochub.mongodb.org/core/mongodb-mcp-server-tools-considerations";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Too lazy to test it, but during manual testing, did you experience the model correctly relaying the url to users?

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 couldn't find the LLM propagating the link. But it does show up in the tool call logs if the toggle is clicked

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2025-09-01 at 20 56 55

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we try tweaking the response and seeing what the output is? E.g. we could add something like: Note to LLM Agent: it is important to include the following link in your response to the user in case they want to get more information about the temporary user created

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that did the trick!
Screenshot 2025-09-03 at 13 42 54


function sleep(ms: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, ms));
Expand Down Expand Up @@ -62,7 +67,7 @@ export class ConnectClusterTool extends AtlasToolBase {
private async prepareClusterConnection(
projectId: string,
clusterName: string
): Promise<{ connectionString: string; atlas: AtlasClusterConnectionInfo }> {
): Promise<{ connectionString: string; atlas: AtlasClusterConnectionInfo; userCreated: boolean }> {
const cluster = await inspectCluster(this.session.apiClient, projectId, clusterName);

if (!cluster.connectionString) {
Expand Down Expand Up @@ -110,7 +115,7 @@ export class ConnectClusterTool extends AtlasToolBase {
cn.password = password;
cn.searchParams.set("authSource", "admin");

return { connectionString: cn.toString(), atlas: connectedAtlasCluster };
return { connectionString: cn.toString(), atlas: connectedAtlasCluster, userCreated: true };
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like we're always returning true for userCreated - am I missing something?

Copy link
Collaborator Author

@blva blva Sep 1, 2025

Choose a reason for hiding this comment

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

no, if the connection state doesn't require preparation (e.g. is already connected*) then we'd not execute this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right - my point is that prepareClusterConnection always returns userCreated: true (or at least it's not obvious in which case it will return false for userCreated. In the already connected state, we wouldn't invoke this method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, got it! yes, good point since we error if something goes wrong. I can remove that from now, I think we'll need to do more work in the connectCluster anyways but this is out of scope here, so let me simplify

}

private async connectToCluster(connectionString: string, atlas: AtlasClusterConnectionInfo): Promise<void> {
Expand Down Expand Up @@ -190,19 +195,35 @@ export class ConnectClusterTool extends AtlasToolBase {
}

protected async execute({ projectId, clusterName }: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> {
await ensureCurrentIpInAccessList(this.session.apiClient, projectId);
const ipAccessListUpdated = await ensureCurrentIpInAccessList(this.session.apiClient, projectId);
let createdUser = false;

for (let i = 0; i < 60; i++) {
const state = this.queryConnection(projectId, clusterName);
switch (state) {
case "connected": {
return {
content: [
{
type: "text",
text: `Connected to cluster "${clusterName}".`,
},
],
};
const content: CallToolResult["content"] = [
{
type: "text",
text: `Connected to cluster "${clusterName}".`,
},
];

if (ipAccessListUpdated) {
content.push({
type: "text",
text: addedIpAccessListMessage,
});
}

if (createdUser) {
content.push({
type: "text",
text: createdUserMessage,
});
}

return { content };
}
case "connecting":
case "unknown": {
Expand All @@ -212,8 +233,12 @@ export class ConnectClusterTool extends AtlasToolBase {
case "disconnected":
default: {
await this.session.disconnect();
const { connectionString, atlas } = await this.prepareClusterConnection(projectId, clusterName);
const { connectionString, atlas, userCreated } = await this.prepareClusterConnection(
projectId,
clusterName
);

createdUser = userCreated;
// try to connect for about 5 minutes asynchronously
void this.connectToCluster(connectionString, atlas).catch((err: unknown) => {
const error = err instanceof Error ? err : new Error(String(err));
Expand All @@ -230,21 +255,31 @@ export class ConnectClusterTool extends AtlasToolBase {
await sleep(500);
}

return {
content: [
{
type: "text" as const,
text: `Attempting to connect to cluster "${clusterName}"...`,
},
{
type: "text" as const,
text: `Warning: Provisioning a user and connecting to the cluster may take more time, please check again in a few seconds.`,
},
{
type: "text" as const,
text: `Warning: Make sure your IP address was enabled in the allow list setting of the Atlas cluster.`,
},
],
};
const content: CallToolResult["content"] = [
{
type: "text" as const,
text: `Attempting to connect to cluster "${clusterName}"...`,
},
{
type: "text" as const,
text: `Warning: Provisioning a user and connecting to the cluster may take more time, please check again in a few seconds.`,
},
];

if (ipAccessListUpdated) {
content.push({
type: "text" as const,
text: addedIpAccessListMessage,
});
}

if (createdUser) {
content.push({
type: "text" as const,
text: createdUserMessage,
});
}

return { content };
}
}
5 changes: 5 additions & 0 deletions tests/integration/tools/atlas/atlasHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ export function describeWithAtlas(name: string, fn: IntegrationTestFunction): vo

interface ProjectTestArgs {
getProjectId: () => string;
getIpAddress: () => string;
}

type ProjectTestFunction = (args: ProjectTestArgs) => void;

export function withProject(integration: IntegrationTest, fn: ProjectTestFunction): SuiteCollector<object> {
return describe("with project", () => {
let projectId: string = "";
let ipAddress: string = "";

beforeAll(async () => {
const apiClient = integration.mcpServer().session.apiClient;
Expand All @@ -49,6 +51,8 @@ export function withProject(integration: IntegrationTest, fn: ProjectTestFunctio
await apiClient.validateAccessToken();
try {
const group = await createProject(apiClient);
const ipInfo = await apiClient.getIpInfo();
ipAddress = ipInfo.currentIpv4Address;
projectId = group.id;
} catch (error) {
console.error("Failed to create project:", error);
Expand All @@ -72,6 +76,7 @@ export function withProject(integration: IntegrationTest, fn: ProjectTestFunctio

const args = {
getProjectId: (): string => projectId,
getIpAddress: (): string => ipAddress,
};

fn(args);
Expand Down
25 changes: 18 additions & 7 deletions tests/integration/tools/atlas/clusters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ async function waitCluster(
}

describeWithAtlas("clusters", (integration) => {
withProject(integration, ({ getProjectId }) => {
withProject(integration, ({ getProjectId, getIpAddress }) => {
const clusterName = "ClusterTest-" + randomId;

afterAll(async () => {
Expand Down Expand Up @@ -162,6 +162,7 @@ describeWithAtlas("clusters", (integration) => {
describe("atlas-connect-cluster", () => {
beforeAll(async () => {
const projectId = getProjectId();
const ipAddress = getIpAddress();
await waitCluster(integration.mcpServer().session, projectId, clusterName, (cluster) => {
return (
cluster.stateName === "IDLE" &&
Expand All @@ -177,7 +178,7 @@ describeWithAtlas("clusters", (integration) => {
body: [
{
comment: "MCP test",
cidrBlock: "0.0.0.0/0",
ipAddress: ipAddress,
},
],
});
Expand All @@ -196,6 +197,7 @@ describeWithAtlas("clusters", (integration) => {

it("connects to cluster", async () => {
const projectId = getProjectId();
let connected = false;

for (let i = 0; i < 10; i++) {
const response = await integration.mcpClient().callTool({
Expand All @@ -205,16 +207,25 @@ describeWithAtlas("clusters", (integration) => {

const elements = getResponseElements(response.content);
expect(elements.length).toBeGreaterThanOrEqual(1);
if (
elements[0]?.text.includes("Cluster is already connected.") ||
elements[0]?.text.includes(`Connected to cluster "${clusterName}"`)
) {
break; // success
if (elements[0]?.text.includes(`Connected to cluster "${clusterName}"`)) {
connected = true;

// assert that some of the element s have the message
expect(
elements.some((element) =>
element.text.includes(
"Note: A temporary user has been created to enable secure connection to the cluster. For more information, see https://dochub.mongodb.org/core/mongodb-mcp-server-tools-considerations"
)
)
).toBe(true);

break;
} else {
expect(elements[0]?.text).toContain(`Attempting to connect to cluster "${clusterName}"...`);
}
await sleep(500);
}
expect(connected).toBe(true);
});

describe("when not connected", () => {
Expand Down
Loading