feat: Setting MCP up with AI Tooling CLOUDP-384442#952
feat: Setting MCP up with AI Tooling CLOUDP-384442#952
Conversation
rubydong
force-pushed
the
feature/setup-mcp-with-ai
branch
from
880ed5b to
c7874e3
Compare
rubydong
changed the title
rubydong
changed the title
nirinchev
left a comment
nirinchev
left a comment
There was a problem hiding this comment.
Did a first pass - looks solid overall, though I flagged some opportunities to clean up the architecture.
src/index.ts
Outdated
| if (config.previewFeatures.includes("setup") && isSetupRequested) { | ||
| await handleSetupRequest(); | ||
| } |
There was a problem hiding this comment.
On a second thought, we probably want to handle the case when the preview feature isn't enabled, otherwise we're silently ignoring the first argument (I know this is super nitty as we'll remove the FF quite soon). Something like:
if (isSetupRequested) {
if (config.previewFeatures.includes("setup")) {
await handleSetupRequest();
} else {
console.error("Automated MCP setup is in preview. If you want to run the setup flow, enable the 'setup' preview feature");
process.exit(1);
}
}There was a problem hiding this comment.
is the plan to remove the feature flag right before it releases to the public? just wondering about the timeline for my own knowledge
There was a problem hiding this comment.
That's what I was thinking - if you want to keep it for a while and circulate the release internally for testing, that's also fine. We can remove the FF in a follow-up release.
There was a problem hiding this comment.
that works! https://jira.mongodb.org/browse/CLOUDP-386891 i have this ticket to remove it later
Pull Request Test Coverage Report for Build 23072518523Details
💛 - Coveralls |
rubydong
force-pushed
the
feature/setup-mcp-with-ai
branch
from
f069493 to
c6e4132
Compare
src/setup/AiTool.ts
Outdated
| if (!fs.existsSync(configDir)) { | ||
| fs.mkdirSync(configDir, { recursive: true }); | ||
| } | ||
| fs.writeFileSync(configPath, JSON.stringify(opencodeConfig, null, 2)); |
There was a problem hiding this comment.
This is problematic - many editors allow comments their configs, even though technically, that's not part of the json spec. By deserializing and serializing, we would be deleting those, which is not acceptable. I know it's quite a bit more involved, but we should look into adding a helper that inserts the additional config without overwriting the entire file.
There was a problem hiding this comment.
to be sure, you're not talking about comments within this kind of block but for the rest of the file?
{
"mcpServers": {
"mongodb-mcp-server": {
"command": "npx",
"args": [
"-y",
"mongodb-mcp-server@latest"
],
"env": {
"MDB_MCP_CONNECTION_STRING": ""
}
}
}
}
There was a problem hiding this comment.
Including this kind of block - e.g. I can add a comment above the connection string env variable that it's for a staging cluster. Essentially, we should not assume that it's safe to deserialize and then serialize user-generated .json files. We can deserialize if we need to process the contents in a structured way and we can serialize our own section, but then we need to insert our changes without overwriting the rest of the file.
There was a problem hiding this comment.
got it, i'll take some time to figure that portion out thanks for the feedback!
There was a problem hiding this comment.
if i cannot read their existing config though can i create a new one?
There was a problem hiding this comment.
Absolutely - if there's nothing, we should create a new one. It's just the case where there's an existing config, we shouldn't assume we can overwrite its entire content.
There was a problem hiding this comment.
hey Nikola, could you chime in with your thoughts regarding openex here https://mongodb.slack.com/archives/C0AGKDAGVG8/p1773200931654289
is it worthwhile to keep it in and have a not very clean solution to preserve the comments?
rubydong
force-pushed
the
feature/setup-mcp-with-ai
branch
from
c8890e2 to
a9beca2
Compare
rubydong
force-pushed
the
feature/setup-mcp-with-ai
branch
from
6c25676 to
4d8f6f1
Compare
rubydong
force-pushed
the
feature/setup-mcp-with-ai
branch
2 times, most recently
from
2a6f754 to
5710a8c
Compare
rubydong
force-pushed
the
feature/setup-mcp-with-ai
branch
from
5710a8c to
6ad27ce
Compare
nirinchev
left a comment
nirinchev
left a comment
There was a problem hiding this comment.
I think we're close to done here - needs a merge with main to resolve conflicts and testing on all the platforms, but otherwise, let's merge it.
| : `xdg-open "${configPath}"`; | ||
|
|
||
| // Windows: open path in default app (for tools without a dedicated editor) | ||
| const getOpenCommandWindowsDefault = (configPath: string): string => `start "" "${configPath}"`; |
There was a problem hiding this comment.
Just tested this out in parallels windows and seems to work for me.
There was a problem hiding this comment.
that's so odd 😭 i commented out the logic for now because it just won't work on my windows but will swing back
|
|
||
| export const runSetup = async (config: UserConfig): Promise<void> => { | ||
| try { | ||
| console.log(chalk.hex("#00ED64")(banner) + "\n"); |
There was a problem hiding this comment.
[not blocking] Consider splitting this function into steps so it's easier to follow what's going on. Could look something like
printLogo();
validateNodeVersion();
const platform = validatePlatform();
printInstructions();
const tool = promptForAITool();
const readOnly = promptForReadonly();
const connectionString = promptForConnectionString();
// ...
That way, it'd be easier to follow the flow without the noise of the console.logs and prompting. And the functions themselves can be self-contained in terms of input, output, and error handling, so the main function is just moving from one step to the next.
4 participants
Proposed changes
Additional Info
Screen.Recording.2026-03-12.at.11.07.20.AM.mov
Checklist