-
Notifications
You must be signed in to change notification settings - Fork 72
chore: better connect tool guidance with Atlas MCP-48 #349
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the connect tooling by adding a dedicated "connect" operation type, updating the tool registration flow, and improving LLM hints for connection errors. It also expands integration tests for both MongoDB and Atlas connect scenarios, and updates documentation to include the new connect operation.
- Introduce a "connect" operation type and update
ToolBase
/MongoDBToolBase
registration and error handling to surface dynamic connect hints. - Add and update integration tests for MongoDB and Atlas connect behaviors, including a disabled-tools scenario.
- Refactor
Server.registerTools
to track registered tools and update README to include "connect" in operation types and read-only mode.
Reviewed Changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/integration/tools/mongodb/connect/connect.test.ts | Update imports, add test for disabled connect tool scenario |
tests/integration/tools/atlas/clusters.test.ts | Add “not connected” Atlas cluster test with connect hints |
src/tools/tool.ts | Add "connect" to OperationType , change register to return boolean |
src/tools/mongodb/mongodbTool.ts | Inject dynamic connect-tool hints in error responses |
src/server.ts | Expose tools list, adjust registration to use boolean return |
README.md | Document "connect" operation type and read-only mode changes |
Co-authored-by: Copilot <[email protected]>
This reverts commit 9cdf294.
@@ -301,10 +301,11 @@ Operation types: | |||
- `delete` - Tools that delete resources, such as delete document, drop collection, etc. | |||
- `read` - Tools that read resources, such as find, aggregate, list clusters, etc. | |||
- `metadata` - Tools that read metadata, such as list databases, list collections, collection schema, etc. | |||
- `connect` - Tools that allow you to connect or switch the connection to a MongoDB instance. If this is disabled, you will need to provide a connection string through the config when starting the server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a new tool operation type to more clearly cluster the connection tools and allow users to disable them if they want to be 100% certain they're always connected to the same cluster (this was another recommendation by the IS team).
Right now, I'm keeping that enabled when working with --readOnly
, but am open to be convinced otherwise.
export type ToolCategory = "mongodb" | "atlas"; | ||
export type TelemetryToolMetadata = { | ||
projectId?: string; | ||
orgId?: string; | ||
}; | ||
|
||
export abstract class ToolBase { | ||
protected abstract name: string; | ||
public abstract name: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are now public, because I wanted to avoid hardcoding the tool names
Pull Request Test Coverage Report for Build 16217587845Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, no concerns about the new operation type!
Proposed changes
We're updating the response when trying to run a data-plane tool before we're connected. The main difference is that now we're actively hinting to the LLM that it needs to use the atlas-cluster-connect tool if it's available and providing some extra context in general. I've tested the results manually with Cursor, Claude, and VSCode and they all seem to be pretty good about picking up the hints.
This should resolve the issue reported by the IS team where the model was hallucinating connection strings instead of suggesting to connect to an Atlas cluster.
Checklist