feat: introduces dry mode for CLI MCP-297 #747

himanshusinghs · 2025-11-24T12:17:53Z

Proposed changes

Introduces dry mode for dumping used configuration and enabled tools.

Checklist

I have signed the MongoDB CLA

gagik · 2025-11-24T16:10:46Z

src/common/config/argsParserOptions.ts

        "apiDeprecationErrors",
        "apiStrict",
        "disableEmbeddingsValidation",
+        "dry",


I don't think this should live here

TIL we have been doing this for all our variables... that's not great :(

Should this be dryRun instead? dry sounds a bit awkward.

I don't think this should live here
yargs-parser parses the arguments considering these OPTIONS provided to it and gathers the rest (ones that are not part of OPTIONS) as unknown arguments which we use further to post warnings about unknown arguments passed.

The alternative is to not pass yargs-parser these OPTIONS and do the verification manually which I don't think bring much value.

dry sounds a bit awkward.

more like dry sounds a bit dry? 😄 I have updated the name to dryRun instead

gagik · 2025-11-24T16:11:45Z

src/transports/dryModeRunner.ts

+export class DryModeRunner extends TransportRunnerBase {
+    private server: Server | undefined;
+    private exitProcess: DryModeTestHelpers["exit"];
+    private consoleLogger: DryModeTestHelpers["logger"];


Suggested change

private consoleLogger: DryModeTestHelpers["logger"];

private logger: DryModeTestHelpers["logger"];

(nit) this also isn't dependent on a console

but it is dependent on console :) logger is a base class property which I did not want to override because otherwise I would have to conform to the interface.

And because in dryMode we plan to exit as soon as we dump, there is not much sense in logging via Session loggers. This should ideally work the same way as our warnings do and they work through console logging.

gagik · 2025-11-24T16:13:36Z

src/transports/dryModeRunner.ts

+        this.consoleLogger.log(JSON.stringify(tools, null, 2));
+    }
+
+    static async assertDryMode(


this isn't really an assertion, is it?
I think we should move this logic into index.ts

new this(...)

I never knew you could do that 😃 JS is cool and terrible

this isn't really an assertion, is it?

thanks for pointing this. I have renamed the helpers to be more aligned with what they do.

gagik · 2025-11-24T16:14:17Z

src/transports/dryModeRunner.ts

+        this.consoleLogger = logger;
+    }
+
+    async start(): Promise<void> {


why not just have this do everything? i.e. combine all the dump functions into it.

I followed on the suggestion and have combined dump calls within the start method itself.

Copilot

Pull request overview

This PR introduces a dry-run mode for the CLI that dumps the server's configuration and enabled tools to the console without starting the server. This is useful for debugging configuration issues and understanding which tools are available.

Key changes:

Added --dryRun CLI flag and corresponding configuration option
Created DryRunModeRunner class to handle dry-run execution
Added comprehensive test coverage for the new functionality

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/common/config/userConfig.ts`	Added `dryRun` boolean configuration field with default value of `false`
`src/common/config/argsParserOptions.ts`	Registered `dryRun` as a valid CLI argument option
`src/index.ts`	Refactored help/version handlers and added dry-run request handling logic
`src/transports/dryModeRunner.ts`	New runner class that connects to an in-memory transport and dumps configuration/tools
`tests/unit/transports/dryModeRunner.test.ts`	Unit tests for the `DryRunModeRunner` class
`tests/unit/common/config.test.ts`	Updated expected defaults to include `dryRun: false`
`tests/e2e/cli.test.ts`	New e2e tests for CLI entrypoint including dry-run validation
`tests/integration/helpers.ts`	Updated import path for `InMemoryTransport`
`tests/integration/tools/mongodb/mongodbTool.test.ts`	Updated import path for `InMemoryTransport`
`package.json`	Changed `pretest:accuracy` hook to general `pretest` to ensure build runs before all tests

tests/unit/transports/dryModeRunner.test.ts

tests/integration/helpers.ts

src/transports/dryModeRunner.ts

tests/e2e/cli.test.ts

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

src/index.ts

coveralls · 2025-11-25T11:45:24Z

Pull Request Test Coverage Report for Build 19697411085

Details

38 of 76 (50.0%) changed or added relevant lines in 4 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.3%) to 80.141%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/transports/dryModeRunner.ts	31	33	93.94%
src/index.ts	0	36	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/index.ts	1	0.65%

Totals
Change from base Build 19677845049:	0.3%
Covered Lines:	6477
Relevant Lines:	7987

💛 - Coveralls

src/index.ts

gagik · 2025-11-25T18:19:43Z

src/index.ts

+    process.exit(0);
+}
+
+export async function handleDryRunRequest(config: UserConfig): Promise<never> {


I think this function might as well be moved to dryModeRunner.ts

although I also am fine with keeping as is because of usage of process.exit

maybe these functions shouldn't process.exit themselves and instead return and then a larger function would process.exit?

I think this function might as well be moved to [dryModeRunner.ts]

It was originally there but I moved it to index file based on the suggestion here. For me, either is fine but I still moved it to index only to have all the CLI args handling in one place.

I think its fine for them to take control of exiting - that's a pattern I have seen commander where different args have different action paths and they all independently terminate the process.

gagik

LGTM, one note about process.exit

1. assert function are not assertions so fixed them with relevant names 2. moved static method logic to cli entry point and added e2e for test cases

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

himanshusinghs requested a review from a team as a code owner November 24, 2025 12:17

himanshusinghs force-pushed the feat/MCP-297-dry-run-mode branch from 458d2d4 to 9c54a3b Compare November 24, 2025 14:36

himanshusinghs changed the base branch from feat/MCP-292-config-file to main November 24, 2025 14:36

gagik reviewed Nov 24, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 25, 2025 11:33

Copilot AI reviewed Nov 25, 2025

View reviewed changes

tests/unit/transports/dryModeRunner.test.ts Outdated Show resolved Hide resolved

tests/integration/helpers.ts Show resolved Hide resolved

src/transports/dryModeRunner.ts Outdated Show resolved Hide resolved

himanshusinghs force-pushed the feat/MCP-297-dry-run-mode branch from 3df1573 to 2b38a3f Compare November 25, 2025 11:34

github-advanced-security bot found potential problems Nov 25, 2025

View reviewed changes

tests/e2e/cli.test.ts Fixed Show fixed Hide fixed

tests/e2e/cli.test.ts Fixed Show fixed Hide fixed

tests/e2e/cli.test.ts Fixed Show fixed Hide fixed

himanshusinghs requested review from Copilot and gagik November 25, 2025 11:44

Copilot AI reviewed Nov 25, 2025

View reviewed changes

src/index.ts Show resolved Hide resolved

himanshusinghs requested a review from nirinchev November 25, 2025 14:37

nirinchev reviewed Nov 25, 2025

View reviewed changes

src/index.ts Show resolved Hide resolved

himanshusinghs force-pushed the feat/MCP-297-dry-run-mode branch from 70a1e58 to 450dee7 Compare November 25, 2025 16:11

himanshusinghs requested a review from nirinchev November 25, 2025 16:45

gagik reviewed Nov 25, 2025

View reviewed changes

gagik approved these changes Nov 25, 2025

View reviewed changes

himanshusinghs and others added 8 commits November 26, 2025 09:23

feat: introduces dry mode for CLI

6086e74

chore: fix config expectation

709f4ee

chore: address PR feedback

ef465af

1. assert function are not assertions so fixed them with relevant names 2. moved static method logic to cli entry point and added e2e for test cases

chore: dump only enabled tools

51a3b2d

Update tests/unit/transports/dryModeRunner.test.ts

fe83ace

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

chore: address copilot suggestions

14891a5

chore: remove stderr assertions

3438d58

chore: stop runner as well

5884a5a

himanshusinghs force-pushed the feat/MCP-297-dry-run-mode branch from 450dee7 to 5884a5a Compare November 26, 2025 08:33

nirinchev approved these changes Nov 26, 2025

View reviewed changes

himanshusinghs merged commit 17cdcd1 into main Nov 26, 2025
18 checks passed

himanshusinghs deleted the feat/MCP-297-dry-run-mode branch November 26, 2025 11:07

	private consoleLogger: DryModeTestHelpers["logger"];
	private logger: DryModeTestHelpers["logger"];

feat: introduces dry mode for CLI MCP-297 #747

feat: introduces dry mode for CLI MCP-297 #747

Uh oh!

Conversation

himanshusinghs commented Nov 24, 2025

Proposed changes

Checklist

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!

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!

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

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

Uh oh!

Pull Request Test Coverage Report for Build 19697411085

Details

💛 - Coveralls

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!

himanshusinghs Nov 26, 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!

gagik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coveralls commented Nov 25, 2025 •

edited

Loading

himanshusinghs Nov 26, 2025 •

edited

Loading