chore: adopt strict validation for all command line arguments MCP-298 #777

gagik · 2025-12-02T18:35:33Z

This will not work until mongodb-js/mongosh#2589 is released but making it available for early reviews.

This gets rid of our hardcoded dependency on OPTIONS for both internal and mongosh-related argument parsing and instead relies completely on our (and mongosh's) Zod schema.

This provides the following advantages:

We no longer need to manually append the OPTIONS array when adding new config fields which is easy to forget. This actually revealed that we had forgotten to add i.e. vectorSearchDimensions to this array before because of the redundancy involved.
We can reliably validate both our and mongosh argument types.
We can ensure we're keep up with changes to mongosh's connection string cases.
We get "out-of-the-box" support for more complex types like objects with --object.field=3 which would otherwise have require us to manually specify different scenarios in the OPTIONS array.

As a consequence of some of these advantages, this PR also...:

Adds support to set httpHeaders using the CLI (or env), either as --httpHeaders='{ "test": "1" }' or --httpHeaders.test='1'!
Better alignment with some of the mongosh logic - for example ssl-related fields actually get replaced with tls equivalents; and some fields are also unsupported. We would correctly throw an error in this case (which we weren't doing before).
We likely got some new type checking options now.

Some considerations:

For the future, we might want to consider adopting a subset of mongosh's arguments; in that case it's important to make sure that this subset can cover all usecases.
There's a general risk of clashing names. For example readonly exists in both. By default our configuration takes precedence; just something to keep in mind

Copilot

Pull request overview

This PR adopts strict validation for command-line arguments by relying entirely on Zod schemas instead of hardcoded OPTIONS configurations, eliminating redundancy and enabling type-safe argument parsing for both internal and mongosh-related arguments.

Key changes include:

Removal of manual OPTIONS array and the argsParserOptions.ts file
Integration with @mongosh/arg-parser for unified CLI argument parsing
Support for complex types like objects via CLI (e.g., --httpHeaders='{"test":"1"}' or --httpHeaders.test='1')
Better alignment with mongosh's connection string handling (e.g., ssl arguments map to tls equivalents)
Improved error handling that returns structured validation results instead of directly calling process.exit()

Reviewed changes

Copilot reviewed 16 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`tsconfig.cjs.json`	Updated module system from `commonjs` to `node16`
`tests/utils/index.ts`	Added `createEnvironment` helper for managing test environment variables
`tests/unit/common/config.test.ts`	Refactored tests to use new `createUserConfig` API returning structured results instead of throwing errors
`tests/e2e/e2eUtils.ts`	Added CLI runner utility for e2e tests
`tests/e2e/cli.test.ts`	Converted warning/error tests from unit to e2e tests
`src/index.ts`	Updated to handle structured validation results from `createUserConfig`
`src/common/config/userConfig.ts`	Introduced `UserConfigSchemaWithCliOptions` merging internal and mongosh schemas
`src/common/config/createUserConfig.ts`	Replaced manual argument parsing with `parseArgsWithCliOptions` from mongosh
`src/common/config/configUtils.ts`	Removed `isConnectionSpecifier` function, now handled by mongosh
`src/common/config/argsParserOptions.ts`	Deleted file - manual `OPTIONS` array no longer needed
`server.json`	Updated format fields to correct types (boolean/number instead of string) and added missing configuration options
`scripts/generateArguments.ts`	Updated to derive argument types from Zod schemas instead of `OPTIONS`
`scripts/apply.ts`	Migrated from `yargs-parser` to `@mongosh/arg-parser`
`package.json`	Removed `yargs-parser` dependency
`eslint-rules/enforce-zod-v4.js`	Added `createUserConfig.ts` to allowed files for zod/v4 imports
`README.md`	Updated configuration table with new/corrected options

src/common/config/createUserConfig.ts

tsconfig.cjs.json

scripts/generateArguments.ts

src/common/config/createUserConfig.ts

gagik · 2025-12-02T18:37:01Z

.gitignore

+.DS_Store
+
+# Development tool files
+.yalc


I like yalc so would be great if this is gitgnored

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

gagik · 2025-12-03T09:58:14Z

tests/unit/common/config.test.ts

-    });
-});
-
-describe("Warning and Error messages", () => {


moved to e2e tests instead

gagik · 2025-12-03T09:59:18Z

src/common/config/createUserConfig.ts

- Refer to https://www.mongodb.com/docs/mcp-server/get-started/ for setting up the MCP Server.
-`;
-        onError(errorMessage);
-        return closeProcess(1);


changed this callback-based logic to instead be returning information to be used at a higher level

gagik · 2025-12-03T09:59:58Z

tests/unit/common/config.test.ts

                {
                    cli: ["--sslCertificateSelector", "pem=pom"],
-                    expected: { sslCertificateSelector: "pem=pom" },
+                    expected: { tlsCertificateSelector: "pem=pom" },


this is what the actually happens in mongosh since ssl arguments are deprecated

gagik · 2025-12-03T10:01:01Z

server.json

          "description": "Idle timeout for a client to disconnect (only applies to http transport).",
          "isRequired": false,
-          "format": "string",
+          "format": "number",


seems like there were some issues with the server.json generation before 🤷

himanshusinghs · 2025-12-03T13:03:05Z

src/common/config/userConfig.ts

+export const UserConfigSchemaWithCliOptions = z4.object({
+    ...UserConfigSchema.shape,
+    ...CliOptionsSchema.shape,
+});


We need to at-least deprecate nodb here otherwise that will hinder the rightful parsing of a connection specifier.

@gagik did you find a way to deprecate this? I notice that we now have a warning message but the argument still end up deflecting the positional connection string.

deprecation to me means the behavior continues to work. We warn them to remove this so we can eventually throw an error. Otherwise we're breaking behavior

src/common/config/userConfig.ts

himanshusinghs

These are amazing changes @gagik. I left one additional note for tsconfig consideration.

himanshusinghs · 2025-12-03T14:13:14Z

scripts/generateArguments.ts

 import { join, dirname } from "path";
 import { fileURLToPath } from "url";
-import { UserConfigSchema, configRegistry } from "../src/common/config/userConfig.js";
+import { UserConfigSchemaWithCliOptions, configRegistry } from "../src/common/config/userConfig.js";


We're using the UserConfig schema together with CliOptions schema, yet, in the final server.json I don't see the entries from CliOptions schema? We should expect those right?

hmmmm..... I guess in server.json we should; should we have any fields in the README?

let's follow-up on this. otherwise we're going to dump a bunch of unrelated fields into it

Yea - I am also unsure if they deserve a place in server.json and README but worth noting that somehow with this script they don't end up there. Happy to address this as a follow up.

this is existing behavior and I think the code is self-explanatory, let's follow up on this later

src/index.ts

tsconfig.cjs.json

…gagik/parse-mongosh

coveralls · 2025-12-03T17:08:05Z

Pull Request Test Coverage Report for Build 19928292758

Warning: 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.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

82 of 121 (67.77%) changed or added relevant lines in 8 files are covered.
18 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.4%) to 80.172%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
eslint.config.js	0	1	0.0%
src/index.ts	0	16	0.0%
src/common/config/createUserConfig.ts	62	84	73.81%

Files with Coverage Reduction	New Missed Lines	%
src/common/config/createUserConfig.ts	7	75.17%
src/common/config/configUtils.ts	11	84.66%

Totals
Change from base Build 19925770219:	-0.4%
Covered Lines:	6640
Relevant Lines:	8200

💛 - Coveralls

src/index.ts

Co-authored-by: Himanshu Singh <himanshu.singhs@outlook.in>

gagik · 2025-12-04T11:03:33Z

src/common/config/createUserConfig.ts

    }

+    if (parsed.nodb) {
+        return {


This ended up being more manual because of some mongosh type integrity stuff. We will need to follow-up to make this and more arguments use a subset of functionality

…gagik/parse-mongosh

himanshusinghs

Noice 🚀

cveticm

✅ LGTM and thanks for the descriptive summary, gave great context on the issue!

chore: adopt strict validation for all command line arguments MCP-298

d8f2e9d

gagik requested a review from a team as a code owner December 2, 2025 18:35

Copilot AI review requested due to automatic review settings December 2, 2025 18:35

gagik marked this pull request as draft December 2, 2025 18:35

Copilot AI reviewed Dec 2, 2025

View reviewed changes

src/common/config/createUserConfig.ts Outdated Show resolved Hide resolved

tsconfig.cjs.json Show resolved Hide resolved

scripts/generateArguments.ts Show resolved Hide resolved

src/common/config/createUserConfig.ts Show resolved Hide resolved

gagik commented Dec 2, 2025

View reviewed changes

gagik and others added 2 commits December 2, 2025 19:44

chore: push warnings

866b8fb

Update scripts/generateArguments.ts

228da47

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

gagik commented Dec 3, 2025

View reviewed changes

tests/unit/common/config.test.ts

});

});

describe("Warning and Error messages", () => {

Copy link

Collaborator Author

gagik Dec 3, 2025

Choose a reason for hiding this comment

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

moved to e2e tests instead

gagik commented Dec 3, 2025

View reviewed changes

gagik marked this pull request as ready for review December 3, 2025 12:17

himanshusinghs reviewed Dec 3, 2025

View reviewed changes

src/common/config/userConfig.ts Show resolved Hide resolved

himanshusinghs reviewed Dec 3, 2025

View reviewed changes

gagik added 4 commits December 3, 2025 16:51

chore: use the actual package

19e9a44

Merge branch 'main' of github.com:mongodb-js/mongodb-mcp-server into …

0046732

…gagik/parse-mongosh

chore: wrap objects in string

437e719

chore: fix cjs build

b2dd137

gagik added 3 commits December 3, 2025 18:10

chore: fix types, eslint

b33f697

chore: use execpath

1068d03

chore: warn on --nodb

da4bb95

himanshusinghs reviewed Dec 3, 2025

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

Update src/index.ts

e3dcf83

Co-authored-by: Himanshu Singh <himanshu.singhs@outlook.in>

gagik force-pushed the gagik/parse-mongosh branch 2 times, most recently from a5438af to b009bb3 Compare December 4, 2025 11:02

gagik commented Dec 4, 2025

View reviewed changes

chore: throw on nodb

9263486

gagik force-pushed the gagik/parse-mongosh branch from b009bb3 to 9263486 Compare December 4, 2025 11:06

gagik added 2 commits December 4, 2025 12:16

chore: add deepinspect to test

29c0b34

Merge branch 'main' of github.com:mongodb-js/mongodb-mcp-server into …

e4427c6

…gagik/parse-mongosh

himanshusinghs approved these changes Dec 4, 2025

View reviewed changes

chore: fix type, format

52b5c39

gagik force-pushed the gagik/parse-mongosh branch from fad86b9 to 52b5c39 Compare December 4, 2025 11:39

gagik added 2 commits December 4, 2025 12:44

chore: add mode: form

7ebf7f1

chore: add more mode: form

305c353

gagik enabled auto-merge (squash) December 4, 2025 12:06

gagik merged commit 8b9cc4e into main Dec 4, 2025
17 checks passed

gagik deleted the gagik/parse-mongosh branch December 4, 2025 12:06

cveticm reviewed Dec 4, 2025

View reviewed changes

chore: adopt strict validation for all command line arguments MCP-298 #777

chore: adopt strict validation for all command line arguments MCP-298 #777

Conversation

gagik commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gagik Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

himanshusinghs left a comment

Choose a reason for hiding this comment

Uh oh!

himanshusinghs Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coveralls commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 19928292758

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshusinghs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cveticm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gagik commented Dec 2, 2025 •

edited

Loading

gagik Dec 3, 2025 •

edited

Loading

himanshusinghs Dec 3, 2025 •

edited

Loading

coveralls commented Dec 3, 2025 •

edited

Loading