chore: add arg-parser and put the config under test MCP-86 #429
+959
−81
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the configuration management system to use the @mongosh/arg-parser library for generating MongoDB connection strings. This change aligns with how mongosh handles connection arguments and ensures support for additional authentication mechanisms. The refactoring removes several deprecated flags and types that are no longer necessary with the consolidated approach.
- Uses mongosh argument parser to standardize connection string generation and authentication support
- Consolidates configuration by removing the separate
ConnectOptionsinterface and moving connection options to driver-level configuration - Removes hardcoded connection options from test files and connection calls throughout the codebase
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/common/config.ts | Major refactor to use mongosh arg-parser with new configuration parsing and driver options setup |
| tests/unit/common/config.test.ts | Comprehensive test suite for the new configuration parsing functionality |
| src/index.ts | Added FIPS mode support, help/version modes, and system CA loading |
| tests/unit/common/session.test.ts | Removed deprecated connectOptions usage in tests |
| tests/integration/tools/mongodb/connect/connect.test.ts | Removed deprecated connectOptions usage in integration tests |
| tests/integration/common/connectionManager.test.ts | Removed deprecated connectOptions usage and import |
| src/tools/mongodb/mongodbTool.ts | Simplified connection call by removing connectOptions spread |
| src/tools/atlas/connect/connectCluster.ts | Simplified connection call by removing connectOptions spread |
| src/server.ts | Simplified connection call by removing connectOptions spread |
| src/resources/common/config.ts | Updated to use new driverOptions export instead of connectOptions |
| src/lib.ts | Removed ConnectOptions from exports |
| src/common/connectionManager.ts | Refactored to use new driverOptions configuration |
| package.json | Added @mongosh/arg-parser dependency |
himanshusinghs
requested changes
Aug 8, 2025
Pull Request Test Coverage Report for Build 16880817215Details
💛 - Coveralls |
gagik
reviewed
Aug 11, 2025
gagik
reviewed
Aug 11, 2025
gagik
reviewed
Aug 11, 2025
This commit is biggish because it adds a big chunk of tests that weren't there before. While it would be arguably that testing every flag might be too much, because I'm changing how arguments are parsed, I want to have this as a safeline.
For help we are refering to the README.md, if we want something more fancy we porobably want to have better documentation generation so we don't maintain two files (here and the README.md)
The driver options can not be sent by the agent, so we are going to use the ones specified in the CLI arguments / ENV Vars. This is relevant because new connections should use any inherited configuration like TLS settings or FIPS by default.
This is necessary when connecting for the first time.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This diverges a bit on how mongosh does it, but it should be safe enough.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Use mongosh argument parser to generate the connection string that is going to be used to connect to MongoDB, aligning with how mongosh does it and making sure we support additional authentication mechanisms. It removes a bunch of flags and types not necessary now with the consolidation.
There will be integration tests for each of the supported connection types when playing each of their tasks. Specific support is out of the scope of this PR.
Checklist