diff --git a/clients/cli/README.md b/clients/cli/README.md index 98eed8f60..7a6d9a22d 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -80,6 +80,8 @@ npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --tr When a server is loaded from a `--catalog`/`--config` file, its per-server settings (headers, connection/request timeouts, and OAuth) are applied to the connection — the same resolution the TUI uses. A `--header` flag overrides the file's headers for that run while leaving the file's timeouts and OAuth in place. +**Environment-variable semantics.** `MCP_CATALOG_PATH` is honored only when no ad-hoc target is given (positional command, `--server-url`, or `--transport`) — so a shell that exports it can still run one-off ad-hoc invocations without hitting the catalog/ad-hoc conflict. `MCP_STORAGE_DIR` sets the storage directory used by the OAuth persist backend (`/oauth.json`); the per-file `MCP_INSPECTOR_OAUTH_STATE_PATH` override still takes precedence over it. + ### HTTP proxy support Connections to remote HTTP/SSE servers honor the conventional proxy environment variables: `HTTPS_PROXY` / `HTTP_PROXY` (and their lowercase forms) select the proxy, and `NO_PROXY` exempts hosts. This applies to the Node transport shared by the CLI and the web backend — no inspector-specific flag is needed. When a proxy variable is set, outbound requests are routed through undici's `EnvHttpProxyAgent`. @@ -96,15 +98,17 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en | Option | Description | | ----------------------------- | ----------------------------------------------------------------------------------------- | -| `--method ` | MCP method to invoke (e.g. `tools/list`, `tools/call`, `resources/list`, `prompts/list`). | +| `--method ` | MCP method to invoke. Supports `initialize` (connect-only probe → `{serverInfo, protocolVersion, capabilities, instructions}`), `tools/list`, `tools/call`, `resources/list`, `resources/read`, `resources/templates/list`, `prompts/list`, `prompts/get`, `logging/setLevel`. | | `--tool-name ` | Tool name (for `tools/call`). | -| `--tool-arg ` | Tool argument; repeat for multiple. Use `key='{"json":true}'` for JSON. | +| `--tool-arg ` | Tool argument; repeat for multiple. Use `key='{"json":true}'` for JSON. Values are coerced (JSON-parsed, so `count=1` becomes a number). | +| `--tool-args-json ` | Tool arguments as a single JSON object (e.g. `'{"zip":"10001"}'`). Passed verbatim — no `key=value` coercion, so `"012"` stays a string. Mutually exclusive with `--tool-arg`. | | `--uri ` | Resource URI (for `resources/read`). | | `--prompt-name ` | Prompt name (for `prompts/get`). | | `--prompt-args ` | Prompt arguments; repeat for multiple. | | `--log-level ` | Logging level for `logging/setLevel` (e.g. `debug`, `info`). | | `--metadata ` | General metadata (key=value); applied to all methods. | | `--tool-metadata ` | Tool-specific metadata for `tools/call`. | +| `--connect-timeout ` | Connection timeout in ms. Defaults to `15000` for ad-hoc `--server-url`/target runs (so a black-holed host fails fast) and to the file-level timeout for `--catalog`/`--config` runs. `0` disables the timeout. | ### CLI-specific (OAuth for HTTP servers) diff --git a/clients/cli/__tests__/programmatic-ergonomics.test.ts b/clients/cli/__tests__/programmatic-ergonomics.test.ts new file mode 100644 index 000000000..63ea39cbd --- /dev/null +++ b/clients/cli/__tests__/programmatic-ergonomics.test.ts @@ -0,0 +1,202 @@ +import { describe, it, expect } from "vitest"; +import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; +import { runCli } from "./helpers/cli-runner.js"; +import { DEFAULT_CONNECT_TIMEOUT_MS, withConnectTimeout } from "../src/cli.js"; +import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; +import { + expectCliFailure, + expectCliSuccess, + expectOutputContains, +} from "./helpers/assertions.js"; + +describe("withConnectTimeout", () => { + const baseSettings: InspectorServerSettings = { + headers: [{ key: "X-Test", value: "1" }], + metadata: [], + env: [], + connectionTimeout: 42, + requestTimeout: 0, + taskTtl: 60000, + maxFetchRequests: 1000, + autoRefreshOnListChanged: false, + roots: [], + }; + + it("returns the settings unchanged when no timeout is given", () => { + expect(withConnectTimeout(baseSettings, undefined)).toBe(baseSettings); + expect(withConnectTimeout(undefined, undefined)).toBeUndefined(); + }); + + it("overrides an existing settings' connectionTimeout", () => { + const result = withConnectTimeout(baseSettings, 5000); + expect(result).not.toBe(baseSettings); + expect(result?.connectionTimeout).toBe(5000); + // Other fields are preserved. + expect(result?.headers).toEqual(baseSettings.headers); + }); + + it("builds a minimal settings object when none was provided", () => { + const result = withConnectTimeout(undefined, DEFAULT_CONNECT_TIMEOUT_MS); + expect(result?.connectionTimeout).toBe(DEFAULT_CONNECT_TIMEOUT_MS); + expect(result?.headers).toEqual([]); + expect(result?.requestTimeout).toBe(0); + }); +}); + +/** + * Covers the programmatic-ergonomics flags from #1573: `--method initialize`, + * `--tool-args-json`, `--connect-timeout`, and the MCP_CATALOG_PATH ad-hoc + * gate. All connect to the bundled stdio test server. + */ +describe("--method initialize", () => { + it("connects and emits the cached InitializeResult fields", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([command, ...args, "--method", "initialize"]); + expectCliSuccess(result); + const json = JSON.parse(result.stdout) as { + serverInfo: { name: string }; + protocolVersion: string; + capabilities: Record; + }; + expect(json.serverInfo.name).toBeTruthy(); + expect(typeof json.protocolVersion).toBe("string"); + expect(typeof json.capabilities).toBe("object"); + }); +}); + +describe("--tool-args-json", () => { + it("passes the JSON object verbatim (string values stay strings)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-args-json", + JSON.stringify({ message: "012" }), + ]); + expectCliSuccess(result); + // echo returns its message; with --tool-arg message=012 the value would + // coerce to the number 12 (and fail the string schema), but + // --tool-args-json preserves the string "012". + expect(result.stdout).toContain("012"); + }); + + it("rejects --tool-args-json combined with --tool-arg", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "x=1", + "--tool-args-json", + "{}", + ]); + expectCliFailure(result); + expectOutputContains(result, "cannot be combined with --tool-arg"); + }); + + it("rejects malformed JSON", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-args-json", + "{not json}", + ]); + expectCliFailure(result); + expectOutputContains(result, "not valid JSON"); + }); + + it("rejects a non-object value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-args-json", + "[1,2]", + ]); + expectCliFailure(result); + expectOutputContains(result, "must be a JSON object"); + }); +}); + +describe("MCP_CATALOG_PATH with an ad-hoc target", () => { + it("does not conflict with an ad-hoc target (env catalog is ignored)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([command, ...args, "--method", "tools/list"], { + env: { MCP_CATALOG_PATH: "/no/such/catalog.json" }, + }); + expectCliSuccess(result); + }); +}); + +describe("--connect-timeout", () => { + it("accepts a numeric value and connects within it", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--connect-timeout", + "5000", + "--method", + "tools/list", + ]); + expectCliSuccess(result); + }); + + it("accepts 0 (no timeout) and connects", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--connect-timeout", + "0", + "--method", + "tools/list", + ]); + expectCliSuccess(result); + }); + + it("rejects a negative value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--connect-timeout", + "-1", + "--method", + "tools/list", + ]); + expectCliFailure(result); + expectOutputContains(result, "non-negative number"); + }); + + it("rejects a non-numeric value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--connect-timeout", + "soon", + "--method", + "tools/list", + ]); + expectCliFailure(result); + expectOutputContains(result, "non-negative number"); + }); +}); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 3d858815b..df65a95d3 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -6,6 +6,10 @@ import type { MCPServerConfig, InspectorClientEnvironment, } from "@inspector/core/mcp/types.js"; +import { + DEFAULT_MAX_FETCH_REQUESTS, + DEFAULT_TASK_TTL_MS, +} from "@inspector/core/mcp/types.js"; import { InspectorClient } from "@inspector/core/mcp/index.js"; import { ManagedToolsState, @@ -56,6 +60,14 @@ export const validLogLevels: LoggingLevel[] = Object.values( /** Client identity name the CLI reports to servers. */ const CLI_CLIENT_NAME = "inspector-cli"; +/** + * Default connect timeout (ms) for ad-hoc server invocations. Without this an + * unreachable server (e.g. a partner edge that drops the SYN) hangs the CLI + * indefinitely; the value is generous enough for cold-start OAuth discovery + * round-trips while still failing fast on a black-holed host. + */ +export const DEFAULT_CONNECT_TIMEOUT_MS = 15000; + type MethodArgs = { method?: string; promptName?: string; @@ -229,6 +241,16 @@ async function callMethod( args.metadata, ); result = invocation.result; + } else if (args.method === "initialize") { + // Connect-only probe: emit the cached InitializeResult fields so a + // caller can read serverInfo / protocolVersion / capabilities / + // instructions without picking a list method. + result = { + serverInfo: inspectorClient.getServerInfo(), + protocolVersion: inspectorClient.getProtocolVersion(), + capabilities: inspectorClient.getCapabilities(), + instructions: inspectorClient.getInstructions(), + }; } else if (args.method === "logging/setLevel") { if (!args.logLevel) { throw new Error( @@ -240,7 +262,7 @@ async function callMethod( result = {}; } else { throw new Error( - `Unsupported method: ${args.method}. Supported methods include: tools/list, tools/call, resources/list, resources/read, resources/templates/list, prompts/list, prompts/get, logging/setLevel`, + `Unsupported method: ${args.method}. Supported methods include: initialize, tools/list, tools/call, resources/list, resources/read, resources/templates/list, prompts/list, prompts/get, logging/setLevel`, ); } @@ -274,6 +296,32 @@ async function callMethod( } } +/** + * Apply a connection timeout to a resolved server's settings, building a + * minimal {@link InspectorServerSettings} when none came from the file. Ad-hoc + * invocations get {@link DEFAULT_CONNECT_TIMEOUT_MS} so a black-holed host + * fails fast; catalog/config invocations keep their file-level timeout unless + * `--connect-timeout` is passed explicitly. + */ +export function withConnectTimeout( + settings: InspectorServerSettings | undefined, + connectionTimeout: number | undefined, +): InspectorServerSettings | undefined { + if (connectionTimeout === undefined) return settings; + if (settings) return { ...settings, connectionTimeout }; + return { + headers: [], + metadata: [], + env: [], + connectionTimeout, + requestTimeout: 0, + taskTtl: DEFAULT_TASK_TTL_MS, + maxFetchRequests: DEFAULT_MAX_FETCH_REQUESTS, + autoRefreshOnListChanged: false, + roots: [], + }; +} + function parseKeyValuePair( value: string, previous: Record = {}, @@ -432,6 +480,21 @@ async function parseArgs(argv?: string[]): Promise<{ parseKeyValuePair, {}, ) + .option( + "--connect-timeout ", + `Connection timeout in ms (default ${DEFAULT_CONNECT_TIMEOUT_MS} for ad-hoc --server-url / target invocations; 0 = no timeout).`, + (v: string) => { + const n = Number(v); + if (!Number.isFinite(n) || n < 0) { + throw new Error(`--connect-timeout must be a non-negative number.`); + } + return n; + }, + ) + .option( + "--tool-args-json ", + 'Tool arguments as a single JSON object (e.g. \'{"zip":"10001"}\'). Values are passed verbatim — no key=value coercion. Mutually exclusive with --tool-arg.', + ) .option( "--client-config ", "Install-level client config (default: ~/.mcp-inspector/storage/client.json, or MCP_CLIENT_CONFIG_PATH)", @@ -473,6 +536,8 @@ async function parseArgs(argv?: string[]): Promise<{ transport?: "sse" | "http" | "stdio"; serverUrl?: string; header?: Record; + connectTimeout?: number; + toolArgsJson?: string; clientConfig?: string; clientId?: string; clientSecret?: string; @@ -480,10 +545,19 @@ async function parseArgs(argv?: string[]): Promise<{ callbackUrl?: string; }; + // Honour MCP_CATALOG_PATH only when no ad-hoc target is given. Applying it + // unconditionally meant a homespace that exports the env var could never run + // `--server-url …` (serverSourceConflict rejects catalog + ad-hoc). + const adHoc = + targetArgs.length > 0 || + Boolean(options.transport) || + Boolean(options.serverUrl?.trim()); + const envCatalog = adHoc ? undefined : process.env.MCP_CATALOG_PATH; + const serverOptions = { // `?.trim() ||` (not `??`) so an explicit empty `--catalog ""` still falls // back to MCP_CATALOG_PATH — keeps CLI and TUI flag resolution identical. - catalogPath: options.catalog?.trim() || process.env.MCP_CATALOG_PATH, + catalogPath: options.catalog?.trim() || envCatalog, configPath: options.config?.trim() || undefined, target: targetArgs.length > 0 ? targetArgs : undefined, transport: options.transport, @@ -499,9 +573,14 @@ async function parseArgs(argv?: string[]): Promise<{ // enforces the conflict matrix, and lifts disk headers/timeouts/OAuth into // per-server settings. `--server` selects one when the file has several. const entries = await loadServerEntries(serverOptions); - const { config: serverConfig, settings: serverSettings } = selectServerEntry( - entries, - options.server, + const selected = selectServerEntry(entries, options.server); + const serverConfig = selected.config; + // Ad-hoc invocations get a default connect timeout so a black-holed host + // fails fast; catalog/config runs keep their file-level timeout unless + // `--connect-timeout` is passed explicitly. + const serverSettings = withConnectTimeout( + selected.settings, + options.connectTimeout ?? (adHoc ? DEFAULT_CONNECT_TIMEOUT_MS : undefined), ); if (!options.method) { @@ -510,10 +589,37 @@ async function parseArgs(argv?: string[]): Promise<{ ); } + // --tool-args-json passes arguments verbatim with no key=value coercion (so + // `"012"` stays a string and nested objects work without shell escaping). + let toolArg = options.toolArg; + if (options.toolArgsJson !== undefined) { + if (toolArg && Object.keys(toolArg).length > 0) { + throw new Error( + "--tool-args-json cannot be combined with --tool-arg; pick one.", + ); + } + let parsed: unknown; + try { + parsed = JSON.parse(options.toolArgsJson); + } catch (e) { + throw new Error( + `--tool-args-json is not valid JSON: ${(e as Error).message}`, + ); + } + if ( + parsed === null || + typeof parsed !== "object" || + Array.isArray(parsed) + ) { + throw new Error("--tool-args-json must be a JSON object."); + } + toolArg = parsed as Record; + } + const methodArgs: MethodArgs & { method: string } = { method: options.method, toolName: options.toolName, - toolArg: options.toolArg, + toolArg, uri: options.uri, promptName: options.promptName, promptArgs: options.promptArgs, diff --git a/clients/web/src/test/integration/auth/node/storage.test.ts b/clients/web/src/test/integration/auth/node/storage.test.ts index 0bb328ad8..d024fadc8 100644 --- a/clients/web/src/test/integration/auth/node/storage.test.ts +++ b/clients/web/src/test/integration/auth/node/storage.test.ts @@ -630,3 +630,46 @@ describe("NodeOAuthStorage with custom storagePath", () => { } }); }); + +describe("getStateFilePath resolution", () => { + let savedStatePath: string | undefined; + let savedStorageDir: string | undefined; + + beforeEach(() => { + savedStatePath = process.env.MCP_INSPECTOR_OAUTH_STATE_PATH; + savedStorageDir = process.env.MCP_STORAGE_DIR; + delete process.env.MCP_INSPECTOR_OAUTH_STATE_PATH; + delete process.env.MCP_STORAGE_DIR; + }); + + afterEach(() => { + if (savedStatePath === undefined) + delete process.env.MCP_INSPECTOR_OAUTH_STATE_PATH; + else process.env.MCP_INSPECTOR_OAUTH_STATE_PATH = savedStatePath; + if (savedStorageDir === undefined) delete process.env.MCP_STORAGE_DIR; + else process.env.MCP_STORAGE_DIR = savedStorageDir; + }); + + it("prefers an explicit customPath over every env var", () => { + process.env.MCP_INSPECTOR_OAUTH_STATE_PATH = "/env/oauth.json"; + process.env.MCP_STORAGE_DIR = "/env/dir"; + expect(getStateFilePath("/explicit/path.json")).toBe("/explicit/path.json"); + }); + + it("uses MCP_INSPECTOR_OAUTH_STATE_PATH over MCP_STORAGE_DIR", () => { + process.env.MCP_INSPECTOR_OAUTH_STATE_PATH = "/env/oauth.json"; + process.env.MCP_STORAGE_DIR = "/env/dir"; + expect(getStateFilePath()).toBe("/env/oauth.json"); + }); + + it("uses /oauth.json when only the storage dir is set", () => { + process.env.MCP_STORAGE_DIR = path.join("/env", "storage"); + expect(getStateFilePath()).toBe(path.join("/env", "storage", "oauth.json")); + }); + + it("falls back to the default path when no override is set", () => { + expect(getStateFilePath()).toContain( + path.join(".mcp-inspector", "storage", "oauth.json"), + ); + }); +}); diff --git a/core/auth/node/storage-node.ts b/core/auth/node/storage-node.ts index c2039628c..cad693e77 100644 --- a/core/auth/node/storage-node.ts +++ b/core/auth/node/storage-node.ts @@ -10,17 +10,32 @@ import { const DEFAULT_STATE_PATH = getStoreFilePath(getDefaultStorageDir(), "oauth"); /** - * Get path to OAuth state file. Resolution order: explicit `customPath`, then - * the `MCP_INSPECTOR_OAUTH_STATE_PATH` environment variable (so tests and - * scripted runs can point at an isolated fixture without touching - * `~/.mcp-inspector`), then the default `~/.mcp-inspector/storage/oauth.json`. + * Get path to OAuth state file. Resolution order: + * 1. explicit `customPath` + * 2. `MCP_INSPECTOR_OAUTH_STATE_PATH` — the per-file override (so tests and + * scripted runs can point at an isolated fixture without touching + * `~/.mcp-inspector`) + * 3. `/oauth.json` — the storage-directory override for the + * OAuth state file specifically, so the CLI's stored-auth flows and the web + * backend agree on where `oauth.json` lives when the env var is set. Note + * this branch scopes `MCP_STORAGE_DIR` to the OAuth backend only; it does + * NOT relocate `client.json` / `mcp.json` (those still resolve from + * `getDefaultStorageDir()`, which reads `HOME`/`USERPROFILE`). Resolving it + * here (rather than in `getDefaultStorageDir()`) is deliberate: the default + * path constant above is evaluated at module load, so a runtime-set + * `MCP_STORAGE_DIR` — how the CLI stored-auth tests and the `--wait-for-auth` + * flow set it — is only honoured by reading the env var at call time. + * 4. the default `~/.mcp-inspector/storage/oauth.json` */ export function getStateFilePath(customPath?: string): string { - return ( - customPath ?? - process.env.MCP_INSPECTOR_OAUTH_STATE_PATH ?? - DEFAULT_STATE_PATH - ); + if (customPath) return customPath; + if (process.env.MCP_INSPECTOR_OAUTH_STATE_PATH) { + return process.env.MCP_INSPECTOR_OAUTH_STATE_PATH; + } + if (process.env.MCP_STORAGE_DIR) { + return getStoreFilePath(process.env.MCP_STORAGE_DIR, "oauth"); + } + return DEFAULT_STATE_PATH; } const memoryCache = new Map();