Skip to content

Conversation

jeroenvervaeke
Copy link
Member

Proposed changes

Ticket: [MCP-158] [mongodb-mcp-server] Implement listLocalAtlasClusters tool

Checklist

@jeroenvervaeke jeroenvervaeke requested a review from a team as a code owner September 4, 2025 15:09
src/server.ts Outdated
this.registerToolInstances(AtlasLocalTools);
} catch (error) {
// If Atlas Local tools are disabled, don't log an error
if (this.userConfig.disabledTools.includes("atlas-local")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be done even before the import at line 207? Why do we want to attempt this if the tools are disabled? Or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I missed this. Updated

const client = AtlasLocalClient.connect();

// Set Atlas Local client
this.session.setAtlasLocalClient(client);
Copy link
Collaborator

Choose a reason for hiding this comment

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

for streamable http, we'll have multiple sessions that will connect to the server and manage the local atlas env, do we think this is the best place to add set client or should we set it somewhere else? if you think we need a single client, you could check the transport file base.ts and how we inject DeviceId (long-running operation that blocks other calls) or Telemetry

Copy link
Member Author

Choose a reason for hiding this comment

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

It is ok to have multiple instances of the client, it should not cause any issues

@blva
Copy link
Collaborator

blva commented Sep 8, 2025

Could we also decouple the atlas local tests? in code-health we can have a Run Atlas Local tests and update the command in Run Atlas tests

Copy link
Collaborator

@blva blva left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes and context, LGTM as we can split the tests as a follow-up

@jeroenvervaeke jeroenvervaeke merged commit 8a2db27 into feat-MCP-40 Sep 9, 2025
11 of 15 checks passed
@jeroenvervaeke jeroenvervaeke deleted the MCP-158 branch September 9, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants