-
Notifications
You must be signed in to change notification settings - Fork 167
feat: introduces dry mode for CLI MCP-297 #747
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
Changes from all commits
6086e74
709f4ee
ef465af
51a3b2d
fe83ace
14891a5
3438d58
5884a5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,13 +44,23 @@ import { StdioRunner } from "./transports/stdio.js"; | |
| import { StreamableHttpRunner } from "./transports/streamableHttp.js"; | ||
| import { systemCA } from "@mongodb-js/devtools-proxy-support"; | ||
| import { Keychain } from "./common/keychain.js"; | ||
| import { DryRunModeRunner } from "./transports/dryModeRunner.js"; | ||
|
|
||
| async function main(): Promise<void> { | ||
| systemCA().catch(() => undefined); // load system CA asynchronously as in mongosh | ||
|
|
||
| const config = createUserConfig(); | ||
| assertHelpMode(config); | ||
| assertVersionMode(config); | ||
| if (config.help) { | ||
| handleHelpRequest(); | ||
| } | ||
|
|
||
| if (config.version) { | ||
| handleVersionRequest(); | ||
| } | ||
|
|
||
| if (config.dryRun) { | ||
| await handleDryRunRequest(config); | ||
| } | ||
|
|
||
| const transportRunner = | ||
| config.transport === "stdio" | ||
|
|
@@ -133,17 +143,35 @@ main().catch((error: unknown) => { | |
| process.exit(1); | ||
| }); | ||
|
|
||
| function assertHelpMode(config: UserConfig): void | never { | ||
| if (config.help) { | ||
| console.log("For usage information refer to the README.md:"); | ||
| console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start"); | ||
| process.exit(0); | ||
| } | ||
| function handleHelpRequest(): never { | ||
| console.log("For usage information refer to the README.md:"); | ||
| console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start"); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| function assertVersionMode(config: UserConfig): void | never { | ||
| if (config.version) { | ||
| console.log(packageInfo.version); | ||
| function handleVersionRequest(): never { | ||
| console.log(packageInfo.version); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| export async function handleDryRunRequest(config: UserConfig): Promise<never> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this function might as well be moved to dryModeRunner.ts
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. although I also am fine with keeping as is because of usage of
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe these functions shouldn't process.exit themselves and instead return and then a larger function would process.exit?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. |
||
| try { | ||
| const runner = new DryRunModeRunner({ | ||
| userConfig: config, | ||
| logger: { | ||
| log(message): void { | ||
| console.log(message); | ||
| }, | ||
| error(message): void { | ||
| console.error(message); | ||
| }, | ||
| }, | ||
| }); | ||
| await runner.start(); | ||
himanshusinghs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| await runner.close(); | ||
| process.exit(0); | ||
| } catch (error) { | ||
| console.error(`Fatal error running server in dry run mode: ${error as string}`); | ||
| process.exit(1); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { InMemoryTransport } from "./inMemoryTransport.js"; | ||
| import { TransportRunnerBase, type TransportRunnerConfig } from "./base.js"; | ||
| import { type Server } from "../server.js"; | ||
|
|
||
| export type DryRunModeTestHelpers = { | ||
| logger: { | ||
| log(this: void, message: string): void; | ||
| error(this: void, message: string): void; | ||
| }; | ||
| }; | ||
|
|
||
| type DryRunModeRunnerConfig = TransportRunnerConfig & DryRunModeTestHelpers; | ||
|
|
||
| export class DryRunModeRunner extends TransportRunnerBase { | ||
| private server: Server | undefined; | ||
| private consoleLogger: DryRunModeTestHelpers["logger"]; | ||
|
|
||
| constructor({ logger, ...transportRunnerConfig }: DryRunModeRunnerConfig) { | ||
| super(transportRunnerConfig); | ||
| this.consoleLogger = logger; | ||
| } | ||
|
|
||
| override async start(): Promise<void> { | ||
| this.server = await this.setupServer(); | ||
| const transport = new InMemoryTransport(); | ||
|
|
||
| await this.server.connect(transport); | ||
| this.dumpConfig(); | ||
| this.dumpTools(); | ||
| } | ||
|
|
||
| override async closeTransport(): Promise<void> { | ||
| await this.server?.close(); | ||
| } | ||
|
|
||
| private dumpConfig(): void { | ||
| this.consoleLogger.log("Configuration:"); | ||
| this.consoleLogger.log(JSON.stringify(this.userConfig, null, 2)); | ||
| } | ||
|
|
||
| private dumpTools(): void { | ||
| const tools = | ||
| this.server?.tools | ||
| .filter((tool) => tool.isEnabled()) | ||
| .map((tool) => ({ | ||
| name: tool.name, | ||
| category: tool.category, | ||
| })) ?? []; | ||
| this.consoleLogger.log("Enabled tools:"); | ||
| this.consoleLogger.log(JSON.stringify(tools, null, 2)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import path from "path"; | ||
| import { execFile } from "child_process"; | ||
| import { promisify } from "util"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import packageJson from "../../package.json" with { type: "json" }; | ||
|
|
||
| const execFileAsync = promisify(execFile); | ||
| const CLI_PATH = path.join(import.meta.dirname, "..", "..", "dist", "index.js"); | ||
|
|
||
| describe("CLI entrypoint", () => { | ||
| it("should handle version request", async () => { | ||
| const { stdout, stderr } = await execFileAsync(process.execPath, [CLI_PATH, "--version"]); | ||
| expect(stdout).toContain(packageJson.version); | ||
| expect(stderr).toEqual(""); | ||
| }); | ||
|
|
||
| it("should handle help request", async () => { | ||
| const { stdout, stderr } = await execFileAsync(process.execPath, [CLI_PATH, "--help"]); | ||
| expect(stdout).toContain("For usage information refer to the README.md"); | ||
| expect(stderr).toEqual(""); | ||
| }); | ||
|
|
||
| it("should handle dry run request", async () => { | ||
| const { stdout } = await execFileAsync(process.execPath, [CLI_PATH, "--dryRun"]); | ||
| expect(stdout).toContain("Configuration:"); | ||
| expect(stdout).toContain("Enabled tools:"); | ||
| // We don't do stderr assertions because in our CI, for docker-less env | ||
| // atlas local tools push message on stderr stream. | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; | ||
| import { DryRunModeRunner, type DryRunModeTestHelpers } from "../../../src/transports/dryModeRunner.js"; | ||
| import { type UserConfig } from "../../../src/common/config/userConfig.js"; | ||
| import { type TransportRunnerConfig } from "../../../src/transports/base.js"; | ||
| import { defaultTestConfig } from "../../integration/helpers.js"; | ||
|
|
||
| describe("DryModeRunner", () => { | ||
| let loggerMock: DryRunModeTestHelpers["logger"]; | ||
| let runnerConfig: TransportRunnerConfig; | ||
|
|
||
| beforeEach(() => { | ||
| loggerMock = { | ||
| log: vi.fn(), | ||
| error: vi.fn(), | ||
| }; | ||
| runnerConfig = { | ||
| userConfig: defaultTestConfig, | ||
| } as TransportRunnerConfig; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it.each([{ transport: "http", httpHost: "127.0.0.1", httpPort: "3001" }, { transport: "stdio" }] as Array< | ||
| Partial<UserConfig> | ||
| >)("should handle dry run request for transport - $transport", async (partialConfig) => { | ||
| runnerConfig.userConfig = { | ||
| ...runnerConfig.userConfig, | ||
| ...partialConfig, | ||
| dryRun: true, | ||
| }; | ||
| const runner = new DryRunModeRunner({ logger: loggerMock, ...runnerConfig }); | ||
| await runner.start(); | ||
| expect(loggerMock.log).toHaveBeenNthCalledWith(1, "Configuration:"); | ||
| expect(loggerMock.log).toHaveBeenNthCalledWith(2, JSON.stringify(runnerConfig.userConfig, null, 2)); | ||
| expect(loggerMock.log).toHaveBeenNthCalledWith(3, "Enabled tools:"); | ||
| expect(loggerMock.log).toHaveBeenNthCalledWith(4, expect.stringContaining('"name": "connect"')); | ||
| // Because switch-connection is not enabled by default | ||
| expect(loggerMock.log).toHaveBeenNthCalledWith(4, expect.not.stringContaining('"name": "switch-connection"')); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.