From af9359920bfad7da01083fb5c0e41f590e487365 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 12:51:28 -0400 Subject: [PATCH 1/6] =?UTF-8?q?feat(cli):=20programmatic=20ergonomics=20?= =?UTF-8?q?=E2=80=94=20initialize,=20tool-args-json,=20connect-timeout,=20?= =?UTF-8?q?env-var=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the #1573 programmatic-ergonomics surface to the CLI, re-implemented informed by PR #1510 and adapted to the v2/main OAuth-wired cli.ts: - `--method initialize` — connect-only probe emitting `{serverInfo, protocolVersion, capabilities, instructions}`. - `--tool-args-json '{"k":"v"}'` — tool args passed verbatim as JSON (no key=value coercion); mutually exclusive with `--tool-arg`, with clear errors for combined use, malformed JSON, and non-object values. - `--connect-timeout ` — default 15000 for ad-hoc `--server-url`/target runs, file-level timeout otherwise; `0` disables. Backed by the exported `withConnectTimeout()` helper. - MCP_CATALOG_PATH is ignored when an ad-hoc target is given, so a shell that exports it can still run one-off ad-hoc invocations. - MCP_STORAGE_DIR is honored by core `getStateFilePath` (`/oauth.json`), below the per-file MCP_INSPECTOR_OAUTH_STATE_PATH override. Tests: new programmatic-ergonomics.test.ts (CLI, in-process runCli + direct withConnectTimeout unit tests) and getStateFilePath resolution cases in the web storage integration suite. Per-file coverage gate stays ≥90. README: documents the new flags and env-var semantics. Closes #1573 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- clients/cli/README.md | 8 +- .../__tests__/programmatic-ergonomics.test.ts | 202 ++++++++++++++++++ clients/cli/src/cli.ts | 118 +++++++++- .../integration/auth/node/storage.test.ts | 43 ++++ core/auth/node/storage-node.ts | 26 ++- 5 files changed, 380 insertions(+), 17 deletions(-) create mode 100644 clients/cli/__tests__/programmatic-ergonomics.test.ts 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..6957bbdbf 100644 --- a/core/auth/node/storage-node.ts +++ b/core/auth/node/storage-node.ts @@ -10,17 +10,25 @@ 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 honoured + * across the app, so setting it once points every persist backend at the + * same directory + * 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(); From 45380f497598c9183f51f15151947ce143536c90 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 13:09:49 -0400 Subject: [PATCH 2/6] feat(cli): --app-info probe + --format json envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the #1574 app-probing and machine-readable output surface, re-implemented informed by PR #1510 and adapted to the v2/main OAuth-wired cli.ts. - `--method tools/call --tool-name --app-info` — prints one JSON line describing the tool's app posture (hasApp, resourceUri, csp, permissions, domain, …) WITHOUT invoking the tool; exit 0 when it has an app, 2 (no_app) when it doesn't. A missing tool exits 5 (tool_not_found) — distinct from the no-app exit so a typo isn't mistaken for "no app". - `--method tools/list --app-info` — one NDJSON line per tool over a single connection (`| jq -c 'select(.hasApp)'`). - `--format json` — a single stdout envelope (`{result}` plus `{appInfo}` as a sibling key for App tools), no banner, so any method pipes into jq. - A tools/call returning isError:true now exits 5 (tool_is_error) after printing its payload, so && chains stop on a failed call. Implementation: `collectAppInfo()` (core `extractAppInfo` + a tolerated resource read that reports `resourceError`), an NDJSON loop in tools/list, and a single `emitResult()` that owns format + exit-code handling. `runMethod` now returns a `MethodOutcome` discriminated union so the NDJSON path can signal "already emitted". Tests: app-info.test.ts (exit-0/2/5 + NDJSON + no-invoke), format-json.test.ts (envelope shapes + bad --format), emit-result.test.ts (direct unit coverage of the isError→exit-5 and resourceError branches via exported emitResult/ collectAppInfo). Updated the tools.test "nonexistent tool" case to the new exit-5 tool_not_found contract. Per-file coverage ≥90 on all four dimensions (cli.ts 100% stmts/funcs/lines). README documents the flags, NDJSON shape, and exit semantics. Closes #1574 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- clients/cli/README.md | 26 +++ clients/cli/__tests__/app-info.test.ts | 155 +++++++++++++++++ clients/cli/__tests__/emit-result.test.ts | 110 +++++++++++++ clients/cli/__tests__/format-json.test.ts | 112 +++++++++++++ clients/cli/__tests__/tools.test.ts | 12 +- clients/cli/src/cli.ts | 192 ++++++++++++++++++++-- 6 files changed, 590 insertions(+), 17 deletions(-) create mode 100644 clients/cli/__tests__/app-info.test.ts create mode 100644 clients/cli/__tests__/emit-result.test.ts create mode 100644 clients/cli/__tests__/format-json.test.ts diff --git a/clients/cli/README.md b/clients/cli/README.md index 7a6d9a22d..f0b2b75a8 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -109,6 +109,32 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en | `--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. | +| `--app-info` | Probe a tool's MCP App UI metadata without invoking it. With `--method tools/call --tool-name `: prints one JSON line (`hasApp`, `resourceUri`, `csp`, `permissions`, `domain`, …) and exits `0` if the tool has an app or `2` (`no_app`) if not. With `--method tools/list`: emits NDJSON — one app-info line per tool over a single connection. | +| `--format ` | Output format. `text` (default) pretty-prints the result. `json` emits a single JSON object on stdout (`{ "result": … }`, plus `{ "appInfo": … }` as a sibling key for App tools) with no banners, so the whole output pipes cleanly into `jq`. | + +#### App probing (`--app-info`) and machine-readable output (`--format json`) + +`--app-info` inspects a tool's [MCP App](https://modelcontextprotocol.io) UI posture **without calling the tool**, so a pipeline can decide whether to open a browser before touching one: + +```bash +# Probe one tool. Exits 0 (has app) or 2 (no_app). One JSON line on stdout. +mcp-inspector --cli --method tools/call --tool-name my_tool --app-info +# → {"hasApp":true,"toolName":"my_tool","resourceUri":"ui://…","csp":{…},"permissions":{…},"prefersBorder":true,"resourceMimeType":"text/html"} + +# Probe every tool at once — NDJSON, one line per tool, single connection. +mcp-inspector --cli --method tools/list --app-info | jq -c 'select(.hasApp)' +``` + +Exit semantics: a tool that **has** an app exits `0`; one with **no** app exits `2` (`no_app`); a **missing** tool exits `5` (`tool_not_found`) — distinct so a typo isn't mistaken for "no app". A resource-read failure during the probe is tolerated and reported in a `resourceError` field rather than aborting. + +`--format json` wraps any method's output in a single stdout envelope with no banners, so App tools and plain tools both pipe cleanly into `jq`: + +```bash +mcp-inspector --cli --method tools/call --tool-name my_app_tool --format json +# → {"result":{…tool result…},"appInfo":{"hasApp":true,"resourceUri":"ui://…",…}} +``` + +A `tools/call` that returns `isError:true` still prints its payload but exits `5` (`tool_is_error`) so `&&` chains don't proceed on a failed call. ### CLI-specific (OAuth for HTTP servers) diff --git a/clients/cli/__tests__/app-info.test.ts b/clients/cli/__tests__/app-info.test.ts new file mode 100644 index 000000000..88a40c468 --- /dev/null +++ b/clients/cli/__tests__/app-info.test.ts @@ -0,0 +1,155 @@ +import { describe, it, expect } from "vitest"; +import { runCli } from "./helpers/cli-runner.js"; +import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; + +/** + * The default stdio test server advertises exactly one MCP App tool + * (`mcp_app_demo` + its `mcp_app_demo_widget` UI resource). These cover both + * the positive probe (csp/permissions/exit-0) and the negative path + * (`{hasApp:false}` + exit 2), plus the exit-5 tool-not-found distinction and + * the NDJSON `tools/list --app-info` shape. The `extractAppInfo` unit is + * covered in the web core suite; here we exercise the CLI wiring end-to-end. + */ +describe("--app-info", () => { + it("rejects --app-info on a method other than tools/call or tools/list", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "resources/list", + "--app-info", + ]); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain( + "--app-info requires --method tools/call (with --tool-name) or --method tools/list", + ); + }); + + it("emits NDJSON (one app-info line per tool) on --method tools/list --app-info", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/list", + "--app-info", + ]); + expect(result.exitCode).toBe(0); + const lines = result.stdout.trim().split("\n"); + expect(lines.length).toBeGreaterThan(1); + const infos = lines.map( + (l) => JSON.parse(l) as { hasApp: boolean; toolName: string }, + ); + // Every line is a valid app-info object with a toolName. + expect(infos.every((i) => typeof i.toolName === "string")).toBe(true); + // The fixture's `mcp_app_demo` is the (only) App tool. + const demo = infos.find((i) => i.toolName === "mcp_app_demo"); + expect(demo?.hasApp).toBe(true); + expect(infos.filter((i) => i.hasApp).length).toBe(1); + }); + + it("emits {hasApp:false} as one JSON line and exits 2 for a non-App tool", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--app-info", + ]); + expect(result.exitCode).toBe(2); + const line = result.stdout.trim().split("\n")[0]; + const info = JSON.parse(line) as { hasApp: boolean; toolName: string }; + expect(info).toEqual({ hasApp: false, toolName: "echo" }); + expect(result.stderr).toContain("has no MCP App UI resource"); + }); + + it("exits 5 (TOOL_ERROR, code:tool_not_found) when the tool is not found — distinct from the no-app exit-2", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "no-such-tool", + "--app-info", + ]); + expect(result.exitCode).toBe(5); + expect(result.stdout).toBe(""); + const envelope = JSON.parse(result.stderr.trim()) as { + error: { code: string }; + }; + expect(envelope.error.code).toBe("tool_not_found"); + }); + + it("emits the resource-side csp/permissions and exits 0 for an App tool", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "mcp_app_demo", + "--app-info", + ]); + expect(result.exitCode).toBe(0); + const info = JSON.parse(result.stdout.trim().split("\n")[0]) as { + hasApp: boolean; + toolName: string; + resourceUri: string; + csp: unknown; + permissions: unknown; + prefersBorder: boolean; + resourceMimeType: string; + }; + expect(info.hasApp).toBe(true); + expect(info.toolName).toBe("mcp_app_demo"); + expect(info.resourceUri).toBe("ui://demo/widget.html"); + expect(info.csp).toEqual({ connectDomains: [], resourceDomains: [] }); + expect(info.permissions).toEqual({ clipboard: false }); + expect(info.prefersBorder).toBe(true); + expect(info.resourceMimeType).toBe("text/html"); + }); + + it("does not collect app-info on a plain text-mode tools/call", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "mcp_app_demo", + "--tool-arg", + "title=hello", + ]); + expect(result.exitCode).toBe(0); + expect(result.stdout).not.toContain("--- MCP App Info ---"); + expect(result.stdout).not.toContain("hasApp"); + // stdout is a single JSON value (the tool result) so `| jq` works. + expect(() => JSON.parse(result.stdout)).not.toThrow(); + }); + + it("does not invoke the tool when --app-info is set", async () => { + // get_sum requires numeric a/b args; without --app-info this would fail + // with a tool error. With --app-info the tool is never called, so the + // only outcome is the no-app exit-2. + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "get_sum", + "--app-info", + ]); + expect(result.exitCode).toBe(2); + expect(result.output).not.toContain("isError"); + }); +}); diff --git a/clients/cli/__tests__/emit-result.test.ts b/clients/cli/__tests__/emit-result.test.ts new file mode 100644 index 000000000..1ea588548 --- /dev/null +++ b/clients/cli/__tests__/emit-result.test.ts @@ -0,0 +1,110 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { emitResult, collectAppInfo } from "../src/cli.js"; +import { CliExitCodeError } from "../src/error-handler.js"; +import type { InspectorClient } from "@inspector/core/mcp/index.js"; + +/** + * Direct unit coverage for the two output-side branches that the end-to-end + * CLI tests can't reach with the default stdio fixture: a tool that returns + * `isError:true` (exit 5), and a UI-resource read that fails during an + * `--app-info` probe (`resourceError`). Both functions are exported from + * `cli.ts` purely for this test, mirroring the `withConnectTimeout` pattern. + */ +describe("emitResult", () => { + let stdout: string; + let originalWrite: typeof process.stdout.write; + + beforeEach(() => { + stdout = ""; + originalWrite = process.stdout.write; + process.stdout.write = ((chunk: unknown, ...rest: unknown[]): boolean => { + stdout += typeof chunk === "string" ? chunk : String(chunk); + const cb = rest.find((r) => typeof r === "function") as + | (() => void) + | undefined; + cb?.(); + return true; + }) as typeof process.stdout.write; + }); + + afterEach(() => { + process.stdout.write = originalWrite; + }); + + it("throws TOOL_ERROR (code:tool_is_error) when the result has isError:true", async () => { + const promise = emitResult({ isError: true }, undefined, { + toolName: "boom", + }); + await expect(promise).rejects.toBeInstanceOf(CliExitCodeError); + await promise.catch((e: CliExitCodeError) => { + expect(e.exitCode).toBe(5); + expect(e.envelope?.code).toBe("tool_is_error"); + }); + // The payload is still printed before the throw. + expect(stdout).toContain("isError"); + }); + + it("does not throw for a successful result (text mode)", async () => { + await expect( + emitResult({ ok: true }, undefined, { format: "text" }), + ).resolves.toBeUndefined(); + expect(JSON.parse(stdout.trim())).toEqual({ ok: true }); + }); + + it("wraps result + appInfo under a json envelope", async () => { + await emitResult( + { value: 1 }, + { hasApp: true, toolName: "t", resourceUri: "ui://x" }, + { format: "json" }, + ); + expect(JSON.parse(stdout.trim())).toEqual({ + result: { value: 1 }, + appInfo: { hasApp: true, toolName: "t", resourceUri: "ui://x" }, + }); + }); +}); + +describe("collectAppInfo", () => { + it("returns base info for a non-App tool without reading a resource", async () => { + const client = { + readResource: vi.fn(), + } as unknown as Pick; + const info = await collectAppInfo( + client, + { name: "plain", inputSchema: { type: "object" as const } }, + undefined, + ); + expect(info).toEqual({ hasApp: false, toolName: "plain" }); + expect( + (client.readResource as ReturnType).mock.calls.length, + ).toBe(0); + }); + + it("captures resourceError when the UI resource read fails", async () => { + const client = { + readResource: vi.fn().mockRejectedValue(new Error("unreadable")), + } as unknown as Pick; + const tool = { + name: "app", + inputSchema: { type: "object" as const }, + _meta: { ui: { resourceUri: "ui://app/widget.html" } }, + }; + const info = await collectAppInfo(client, tool, undefined); + expect(info.hasApp).toBe(true); + expect(info.resourceUri).toBe("ui://app/widget.html"); + expect(info.resourceError).toBe("unreadable"); + }); + + it("stringifies a non-Error rejection into resourceError", async () => { + const client = { + readResource: vi.fn().mockRejectedValue("string failure"), + } as unknown as Pick; + const tool = { + name: "app", + inputSchema: { type: "object" as const }, + _meta: { ui: { resourceUri: "ui://app/widget.html" } }, + }; + const info = await collectAppInfo(client, tool, undefined); + expect(info.resourceError).toBe("string failure"); + }); +}); diff --git a/clients/cli/__tests__/format-json.test.ts b/clients/cli/__tests__/format-json.test.ts new file mode 100644 index 000000000..89577dd10 --- /dev/null +++ b/clients/cli/__tests__/format-json.test.ts @@ -0,0 +1,112 @@ +import { describe, it, expect } from "vitest"; +import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; +import { runCli } from "./helpers/cli-runner.js"; +import { + expectCliFailure, + expectCliSuccess, + expectOutputContains, +} from "./helpers/assertions.js"; + +/** + * Covers the `--format json` envelope from #1574: a single JSON object on + * stdout (`result`, plus `appInfo` as a sibling key for App tools), no banner. + */ +describe("--format json", () => { + it("wraps tools/list output in a single {result} envelope", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/list", + "--format", + "json", + ]); + expectCliSuccess(result); + expect(result.stdout.trim().split("\n").length).toBe(1); + const env = JSON.parse(result.stdout) as { result: { tools: unknown[] } }; + expect(Array.isArray(env.result.tools)).toBe(true); + }); + + it("emits {result, appInfo} as one JSON object for an App tool (no banner)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "mcp_app_demo", + "--tool-arg", + "title=hello", + "--format", + "json", + ]); + expectCliSuccess(result); + expect(result.stdout).not.toContain("--- MCP App Info ---"); + const env = JSON.parse(result.stdout) as { + result: unknown; + appInfo: { hasApp: boolean; resourceUri: string }; + }; + expect(env.result).toBeTruthy(); + expect(env.appInfo.hasApp).toBe(true); + expect(env.appInfo.resourceUri).toBe("ui://demo/widget.html"); + }); + + it("omits appInfo for a non-App tool", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=hi", + "--format", + "json", + ]); + expectCliSuccess(result); + const env = JSON.parse(result.stdout) as { + result: unknown; + appInfo?: unknown; + }; + expect(env.result).toBeTruthy(); + expect(env.appInfo).toBeUndefined(); + }); + + it("wraps --app-info output under {appInfo} and exits 2 for a non-App tool", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--app-info", + "--format", + "json", + ]); + expect(result.exitCode).toBe(2); + const env = JSON.parse(result.stdout.trim()) as { + appInfo: { hasApp: boolean }; + }; + expect(env.appInfo.hasApp).toBe(false); + }); + + it("rejects an unknown --format value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/list", + "--format", + "yaml", + ]); + expectCliFailure(result); + expectOutputContains(result, "--format must be 'text' or 'json'"); + }); +}); diff --git a/clients/cli/__tests__/tools.test.ts b/clients/cli/__tests__/tools.test.ts index 3a7ea6a17..8ca50f474 100644 --- a/clients/cli/__tests__/tools.test.ts +++ b/clients/cli/__tests__/tools.test.ts @@ -4,7 +4,6 @@ import { expectCliSuccess, expectCliFailure, expectValidJson, - expectJsonError, } from "./helpers/assertions.js"; import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; @@ -378,8 +377,15 @@ describe("Tool Tests", () => { "message=test", ]); - // CLI returns exit code 0 but includes isError: true in JSON (server returns error) - expectJsonError(result); + // A tool that does not exist on the server exits TOOL_ERROR (5) with a + // stable `tool_not_found` code, distinct from a tool that ran and + // returned isError:true. The error envelope is on stderr; stdout is empty. + expect(result.exitCode).toBe(5); + expect(result.stdout).toBe(""); + const envelope = JSON.parse(result.stderr.trim()) as { + error: { code: string }; + }; + expect(envelope.error.code).toBe("tool_not_found"); }); it("should fail when tool name is missing", async () => { diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index df65a95d3..ba225a8fe 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -25,6 +25,9 @@ import { parseHeaderPair, } from "@inspector/core/mcp/node/index.js"; import type { JsonValue } from "@inspector/core/mcp/index.js"; +import { extractAppInfo } from "@inspector/core/mcp/apps.js"; +import type { AppInfo } from "@inspector/core/mcp/apps.js"; +import { CliExitCodeError, EXIT_CODES } from "./error-handler.js"; import { ConsoleNavigation, MutableRedirectUrlProvider, @@ -68,6 +71,8 @@ const CLI_CLIENT_NAME = "inspector-cli"; */ export const DEFAULT_CONNECT_TIMEOUT_MS = 15000; +type OutputFormat = "text" | "json"; + type MethodArgs = { method?: string; promptName?: string; @@ -78,8 +83,27 @@ type MethodArgs = { toolArg?: Record; toolMeta?: Record; metadata?: Record; + appInfo?: boolean; + format?: OutputFormat; }; +/** + * {@link AppInfo} plus a CLI-only `resourceError` so a `resources/read` failure + * during the probe is reported instead of being silently swallowed (which would + * make "no CSP declared" indistinguishable from "resource unreadable"). + */ +export type CliAppInfo = AppInfo & { resourceError?: string }; + +/** + * Discriminated outcome from {@link callMethod}'s per-method runner. Most + * methods return a `result` (with optional collected `appInfo`) for + * {@link emitResult} to format; the `tools/list --app-info` NDJSON path writes + * its lines itself and reports `emitted` so the caller skips a second write. + */ +type MethodOutcome = + | { kind: "result"; result: McpResponse; appInfo?: CliAppInfo } + | { kind: "emitted" }; + async function callMethod( serverConfig: MCPServerConfig, serverSettings: InspectorServerSettings | undefined, @@ -132,8 +156,9 @@ async function callMethod( null; let managedPromptsState: ManagedPromptsState | null = null; - const runMethod = async (): Promise => { + const runMethod = async (): Promise => { let result: McpResponse; + let appInfo: CliAppInfo | undefined; if (args.method === "tools/list" || args.method === "tools/call") { managedToolsState = new ManagedToolsState(inspectorClient); @@ -158,7 +183,21 @@ async function callMethod( } if (args.method === "tools/list") { - result = { tools: managedToolsState!.getTools() }; + const tools = managedToolsState!.getTools(); + if (args.appInfo) { + // NDJSON: one app-info line per tool, all on a single connection. A + // caller that wants only the App tools can `| jq -c 'select(.hasApp)'`. + for (const tool of tools) { + const info = await collectAppInfo( + inspectorClient, + tool, + args.metadata, + ); + await awaitableLog(JSON.stringify(info) + "\n"); + } + return { kind: "emitted" }; + } + result = { tools }; } else if (args.method === "tools/call") { if (!args.toolName) { throw new Error( @@ -170,15 +209,28 @@ async function callMethod( .getTools() .find((t) => t.name === args.toolName); if (!tool) { - result = { - content: [ - { - type: "text" as const, - text: `Tool '${args.toolName}' not found.`, - }, - ], - isError: true, - }; + // Distinct from `isError:true` and (for --app-info) from "tool has no + // app": the named tool does not exist on the server. Exit TOOL_ERROR + // with `code: "tool_not_found"` so a caller can tell a typo/rename + // apart from a real tool failure or a no-app probe result. + throw new CliExitCodeError( + EXIT_CODES.TOOL_ERROR, + `Tool '${args.toolName}' not found on server.`, + { code: "tool_not_found" }, + ); + } + + // Only collect app-info when the caller asked for it (`--app-info` or + // `--format json`); a plain text-mode `tools/call` shouldn't fail just + // because the tool's `_meta.ui.resourceUri` is malformed or its resource + // is unreadable. + if (args.appInfo || args.format === "json") { + appInfo = await collectAppInfo(inspectorClient, tool, args.metadata); + } + if (args.appInfo) { + // --app-info: probe-only — emit the app metadata and skip the tool + // call entirely. The no-app exit code is handled in emitResult. + result = { ...appInfo }; } else { const invocation = await inspectorClient.callTool( tool, @@ -266,7 +318,7 @@ async function callMethod( ); } - return result; + return { kind: "result", result, appInfo }; }; try { @@ -278,7 +330,7 @@ async function callMethod( serverSettings, ); - const result = await withCliAuthRecoveryRetry( + const outcome = await withCliAuthRecoveryRetry( inspectorClient, redirectUrlProvider, callbackUrlConfig, @@ -286,7 +338,11 @@ async function callMethod( runMethod, ); - await awaitableLog(JSON.stringify(result, null, 2)); + // The NDJSON `tools/list --app-info` path already wrote its lines; every + // other method hands its result to emitResult for format/exit handling. + if (outcome.kind === "result") { + await emitResult(outcome.result, outcome.appInfo, args); + } } finally { managedToolsState?.destroy(); managedResourcesState?.destroy(); @@ -296,6 +352,86 @@ async function callMethod( } } +/** + * Write the method result (and any app-info) to stdout, honouring `--format` + * and `--app-info`, then map `isError`/no-app outcomes onto the exit-code map. + * Extracted from `callMethod` so the format/exit handling is in one place. + */ +export async function emitResult( + result: McpResponse, + appInfo: CliAppInfo | undefined, + args: MethodArgs, +): Promise { + const json = args.format === "json"; + + if (args.appInfo) { + const info: CliAppInfo = appInfo ?? { + hasApp: false, + toolName: args.toolName ?? "", + }; + // Single-line JSON either way; --format json wraps it under an `appInfo` + // key so the envelope shape is uniform with the non-probe path. + await awaitableLog(JSON.stringify(json ? { appInfo: info } : info) + "\n"); + if (!info.hasApp) { + throw new CliExitCodeError( + EXIT_CODES.NO_APP, + `Tool '${args.toolName}' has no MCP App UI resource (_meta.ui.resourceUri).`, + ); + } + return; + } + + if (json) { + // One JSON object on stdout — `result` plus, when present, `appInfo` as a + // sibling key. No `--- MCP App Info ---` banner, so `| jq` works for App + // tools as well as plain ones. + const envelope: Record = { result }; + if (appInfo?.hasApp) envelope.appInfo = appInfo; + await awaitableLog(JSON.stringify(envelope) + "\n"); + } else { + // Text mode emits the result only; app-info is not collected on this path + // (use `--format json` or `--app-info` to get it). + await awaitableLog(JSON.stringify(result, null, 2) + "\n"); + } + + // A tool that returned `isError:true` (or whose call failed) is still printed + // above so the caller sees the payload, but the process exits TOOL_ERROR so + // `&&` chains don't proceed on a failed call. + if ((result as { isError?: unknown }).isError === true) { + throw new CliExitCodeError( + EXIT_CODES.TOOL_ERROR, + `Tool '${args.toolName}' returned isError:true.`, + { code: "tool_is_error" }, + ); + } +} + +/** + * Build the CLI's app-info for a tool: extract the tool-side `_meta.ui` and, + * when the tool advertises a UI resource, follow it with a `resources/read` so + * the resource-side csp/permissions/domain are included. A read failure is + * tolerated — the tool-side info is still returned with `resourceError` set, + * since "tool says it has an app but the resource is unreadable" is itself a + * useful probe result. + */ +export async function collectAppInfo( + client: Pick, + tool: Parameters[0], + metadata: Record | undefined, +): Promise { + const base = extractAppInfo(tool); + if (!base.hasApp || base.resourceUri === undefined) return base; + try { + const read = await client.readResource(base.resourceUri, metadata); + return extractAppInfo(tool, read.result); + } catch (e) { + return { + ...base, + resourceError: e instanceof Error ? e.message : String(e), + }; + } +} + /** * Apply a connection timeout to a resolved server's settings, building a * minimal {@link InspectorServerSettings} when none came from the file. Ad-hoc @@ -480,6 +616,10 @@ async function parseArgs(argv?: string[]): Promise<{ parseKeyValuePair, {}, ) + .option( + "--app-info", + "Probe the tool's MCP App UI metadata (resourceUri, csp, permissions, domain) and emit it as one JSON line; exit 2 when the tool has no app. Use with --method tools/call --tool-name (the tool itself is not invoked) or --method tools/list (one NDJSON line per tool).", + ) .option( "--connect-timeout ", `Connection timeout in ms (default ${DEFAULT_CONNECT_TIMEOUT_MS} for ad-hoc --server-url / target invocations; 0 = no timeout).`, @@ -491,6 +631,16 @@ async function parseArgs(argv?: string[]): Promise<{ return n; }, ) + .option( + "--format ", + "Output format: text (default; pretty-printed) or json (one JSON object on stdout, no banners).", + (v: string): OutputFormat => { + if (v !== "text" && v !== "json") { + throw new Error(`--format must be 'text' or 'json'.`); + } + return v; + }, + ) .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.', @@ -536,7 +686,9 @@ async function parseArgs(argv?: string[]): Promise<{ transport?: "sse" | "http" | "stdio"; serverUrl?: string; header?: Record; + appInfo?: boolean; connectTimeout?: number; + format?: OutputFormat; toolArgsJson?: string; clientConfig?: string; clientId?: string; @@ -589,6 +741,16 @@ async function parseArgs(argv?: string[]): Promise<{ ); } + if ( + options.appInfo && + options.method !== "tools/call" && + options.method !== "tools/list" + ) { + throw new Error( + "--app-info requires --method tools/call (with --tool-name) or --method tools/list.", + ); + } + // --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; @@ -640,6 +802,8 @@ async function parseArgs(argv?: string[]): Promise<{ ]), ) : undefined, + appInfo: options.appInfo === true, + format: options.format, }; return { From d81336d2402d38b1d689884eb99cd49fd37fb48f Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 13:24:53 -0400 Subject: [PATCH 3/6] =?UTF-8?q?feat(cli):=20stored-auth=20suite=20?= =?UTF-8?q?=E2=80=94=20--use-stored-auth,=20--wait-for-auth,=20--list-stor?= =?UTF-8?q?ed-auth,=20--print-handoff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the #1575 web→CLI OAuth handoff surface, re-implemented informed by PR #1510 and adapted to v2/main. - `--use-stored-auth` — read the shared oauth.json and inject `Authorization: Bearer ` for --server-url; exit 3 (no_stored_token), listing stored keys, when none matches. - `--wait-for-auth ` — poll oauth.json (500ms) until a token for --server-url appears, then proceed; timeout → exit 3 (auth_wait_timeout). - `--list-stored-auth` — print `{oauthStatePath, storedServerUrls}` and exit (short-circuit, no connect). - `--print-handoff` — emit `{serverUrl, deepLink, portForwardCmd, oauthStatePath, apiToken, note}` for --server-url and exit. Adaptations vs. the reference: - State-path resolution reuses core `getStateFilePath()` (the MCP_INSPECTOR_OAUTH_STATE_PATH → /oauth.json → default precedence added in #1573) instead of a CLI-local resolver. - The state file is read through the shared isomorphic `parseOAuthPersistBlob`, so both the plain `{servers,idpSessions}` layout the web backend writes and the legacy `{state,version}` envelope are accepted. - `parseArgs` now returns a ParseResult union so the short-circuit modes return without a server connection. Blind injection: the token is passed as-is; a stale one surfaces as HTTP 401 → exit 3. Documented in the README along with all four flags, the state-path precedence, and a follow-up note that --print-handoff's deepLink shape is interim pending #1576's canonical handoff format. Tests: stored-auth.test.ts drives injection, --header coexistence, missing --server-url, no-token exit-3, URL-normalisation matching, MCP_STORAGE_DIR resolution, listing, handoff (+note), wait-for-auth polling/timeout, and normalizeServerUrl units — all against a real in-process HTTP test server. Per-file coverage ≥90 on all four dimensions (cli.ts 100% stmts/funcs/lines). Closes #1575 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- clients/cli/README.md | 26 ++ clients/cli/__tests__/stored-auth.test.ts | 369 ++++++++++++++++++++++ clients/cli/src/cli.ts | 247 ++++++++++++++- 3 files changed, 630 insertions(+), 12 deletions(-) create mode 100644 clients/cli/__tests__/stored-auth.test.ts diff --git a/clients/cli/README.md b/clients/cli/README.md index f0b2b75a8..0cc4d3d94 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -185,6 +185,32 @@ npx @modelcontextprotocol/inspector --cli --catalog mcp.json --server my-http-se See [EMA / enterprise-managed auth](../../specification/v2_auth_ema.md) and [OAuth smoke testing](../../specification/v2_auth_smoke_testing.md) (§3 Stytch/CIMD; [§5 mid-session manual validation](../../specification/v2_auth_smoke_testing.md#5-mid-session-auth--step-up--manual-validation) — CLI **C1–C2**). +#### Stored-auth (web → CLI handoff) + +For the common case where OAuth was already completed in the **web inspector on the same machine**, the CLI can reuse the resulting token instead of running its own interactive flow. It reads the shared OAuth state file (the `oauth.json` the web backend writes) directly from disk and injects `Authorization: Bearer ` for `--server-url`. + +| Option | Description | +| ----------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--use-stored-auth` | Read the stored access token for `--server-url` and inject it as `Authorization: Bearer`. Exits `3` (`no_stored_token`) — listing the stored keys — when no token matches. Requires `--server-url`. | +| `--wait-for-auth ` | Poll the OAuth state file (500 ms interval) until a token for `--server-url` appears, then proceed as if `--use-stored-auth` were set. Times out at `` with exit `3` (`auth_wait_timeout`). Use after handing off to a human to complete OAuth in a browser. | +| `--list-stored-auth` | Print `{ oauthStatePath, storedServerUrls }` (the server keys that currently have a token) and exit. No server connection is made. | +| `--print-handoff` | Print a JSON handoff block (`deepLink`, `portForwardCmd`, `oauthStatePath`, `apiToken`) for `--server-url` and exit — everything a script/remote VM needs to drive the browser-side OAuth dance. Requires `--server-url`. | + +**State-file resolution** follows `MCP_INSPECTOR_OAUTH_STATE_PATH` → `/oauth.json` → `~/.mcp-inspector/storage/oauth.json` — the same precedence the rest of the Inspector uses, so the CLI and web backend agree on the file. Server keys are canonicalised with `new URL().href` (the scheme the web store writes), so a trailing-slash or case mismatch between the URL a human opened and the one the agent passed still resolves. + +**Blind injection.** The token is injected as-is; the CLI does not validate or refresh it. A stale/expired token surfaces as an HTTP `401` on the first request → exit `3` (`auth_required`). Re-complete the flow in the web inspector (or use `--wait-for-auth`) and retry. + +> The `deepLink` shape emitted by `--print-handoff` is interim, pending the web deep-link auto-connect work ([#1576](https://github.com/modelcontextprotocol/inspector/issues/1576)); it will be reconciled with that issue's canonical handoff format once it lands. + +```bash +# On a remote VM: print what a human needs to complete OAuth in their browser. +mcp-inspector --cli --server-url https://api.example/mcp --print-handoff + +# Then block until the token lands and run the call with it. +mcp-inspector --cli --transport http --server-url https://api.example/mcp \ + --wait-for-auth 120 --method tools/list +``` + ## Exit codes & error envelopes Every non-zero exit maps to a stable failure class, so a programmatic caller diff --git a/clients/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts new file mode 100644 index 000000000..22a8d93ba --- /dev/null +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -0,0 +1,369 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { tmpdir } from "node:os"; +import { runCli } from "./helpers/cli-runner.js"; +import { expectCliFailure, expectCliSuccess } from "./helpers/assertions.js"; +import { normalizeServerUrl } from "../src/cli.js"; +import { + createTestServerHttp, + createEchoTool, + createTestServerInfo, +} from "@modelcontextprotocol/inspector-test-server"; + +/** + * Writes an oauth.json fixture in the plain `{ servers, idpSessions }` layout + * the web backend persists (also accepted: the legacy `{ state, version }` + * envelope). The CLI reads it through the shared `parseOAuthPersistBlob`, so + * both sides agree on the format and the URL-normalised server key. + */ +function writeOAuthFixture(servers: Record): string { + const dir = mkdtempSync(join(tmpdir(), "inspector-cli-stored-auth-")); + const file = join(dir, "oauth.json"); + writeFileSync(file, JSON.stringify({ servers, idpSessions: {} }), "utf8"); + return file; +} + +describe("normalizeServerUrl", () => { + it("canonicalises a URL via new URL().href (lowercases scheme/host)", () => { + expect(normalizeServerUrl("HTTP://Example.COM/Mcp")).toBe( + "http://example.com/Mcp", + ); + }); + + it("returns the raw string when the value is not a parseable URL", () => { + expect(normalizeServerUrl("not a url")).toBe("not a url"); + }); +}); + +describe("--use-stored-auth", () => { + let server: ReturnType; + let serverUrl: string; + let fixturePath: string; + const TOKEN = "stored-access-token-abc"; + + beforeAll(async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + serverUrl = server.url; + fixturePath = writeOAuthFixture({ + [serverUrl]: { tokens: { access_token: TOKEN, token_type: "Bearer" } }, + }); + }); + + afterAll(async () => { + await server.stop(); + rmSync(fixturePath, { force: true }); + }); + + it("injects the stored token as Authorization: Bearer on the outgoing request", async () => { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliSuccess(result); + const recorded = server.getRecordedRequests(); + expect(recorded.length).toBeGreaterThan(0); + const last = recorded[recorded.length - 1]!; + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + }); + + it("merges with --header (explicit headers + stored auth coexist)", async () => { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--header", + "X-Trace: abc", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + expect(last.headers?.["x-trace"]).toBe("abc"); + }); + + it("errors clearly when --server-url is missing", async () => { + const result = await runCli( + ["--use-stored-auth", "--method", "tools/list"], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliFailure(result); + expect(result.stderr).toContain("--use-stored-auth requires --server-url"); + }); + + it("errors with exit 3 (AUTH_REQUIRED) and lists stored keys when no token matches", async () => { + const other = writeOAuthFixture({ + "https://other.example/mcp": { + tokens: { access_token: "x", token_type: "Bearer" }, + }, + }); + try { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: other } }, + ); + expect(result.exitCode).toBe(3); + const env = JSON.parse(result.stderr.trim()) as { + error: { code: string; message: string }; + }; + expect(env.error.code).toBe("no_stored_token"); + expect(env.error.message).toContain("No stored OAuth token"); + expect(env.error.message).toContain("https://other.example/mcp"); + } finally { + rmSync(other, { force: true }); + } + }); + + it("matches a stored key even when --server-url differs by URL normalisation", async () => { + // Store under the normalised form; pass the upper-cased scheme on the + // command line. new URL().href lowercases the scheme, so the lookup still + // resolves. + const normalised = normalizeServerUrl(serverUrl); + const upper = serverUrl.replace("http://", "HTTP://"); + const fixture = writeOAuthFixture({ + [normalised]: { tokens: { access_token: TOKEN, token_type: "Bearer" } }, + }); + try { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + upper, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture } }, + ); + expectCliSuccess(result); + } finally { + rmSync(fixture, { force: true }); + } + }); + + it("honours MCP_STORAGE_DIR when MCP_INSPECTOR_OAUTH_STATE_PATH is unset", async () => { + const dir = dirname(fixturePath); + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_STORAGE_DIR: dir } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + }); +}); + +describe("--list-stored-auth", () => { + it("prints the stored server URLs and the resolved state path", async () => { + const fixture = writeOAuthFixture({ + "https://a.example/mcp": { + tokens: { access_token: "t1", token_type: "Bearer" }, + }, + "https://b.example/mcp": { tokens: {} }, + }); + try { + const result = await runCli(["--list-stored-auth"], { + env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture }, + }); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { + oauthStatePath: string; + storedServerUrls: string[]; + }; + expect(out.oauthStatePath).toBe(fixture); + expect(out.storedServerUrls).toEqual(["https://a.example/mcp"]); + } finally { + rmSync(fixture, { force: true }); + } + }); + + it("emits an empty list when the state file is absent", async () => { + const result = await runCli(["--list-stored-auth"], { + env: { MCP_INSPECTOR_OAUTH_STATE_PATH: "/no/such/file.json" }, + }); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { storedServerUrls: string[] }; + expect(out.storedServerUrls).toEqual([]); + }); +}); + +describe("--print-handoff", () => { + it("emits a JSON handoff block with deepLink, port-forward command, and the resolved state path", async () => { + const result = await runCli( + ["--print-handoff", "--server-url", "https://x.example/mcp"], + { + env: { + MCP_INSPECTOR_API_TOKEN: "tok123", + CLIENT_PORT: "16274", + MCP_SANDBOX_PORT: "16275", + MCP_STORAGE_DIR: "/tmp/inspector-storage", + MCP_INSPECTOR_OAUTH_STATE_PATH: "", + }, + }, + ); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { + serverUrl: string; + deepLink: string; + portForwardCmd: string; + oauthStatePath: string; + apiToken: string; + }; + expect(out.serverUrl).toBe("https://x.example/mcp"); + expect(out.deepLink).toContain("autoConnect=tok123"); + expect(out.deepLink).toContain("serverUrl=https%3A%2F%2Fx.example%2Fmcp"); + expect(out.portForwardCmd).toContain("--tcp 16274:16274"); + expect(out.portForwardCmd).toContain("--tcp 16275:16275"); + expect(out.oauthStatePath).toBe( + join("/tmp/inspector-storage", "oauth.json"), + ); + expect(out.apiToken).toBe("tok123"); + }); + + it("includes a note when MCP_INSPECTOR_API_TOKEN is unset", async () => { + const result = await runCli( + ["--print-handoff", "--server-url", "https://x.example/mcp"], + { env: { MCP_INSPECTOR_API_TOKEN: "" } }, + ); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { + apiToken: string | null; + note?: string; + }; + expect(out.apiToken).toBeNull(); + expect(out.note).toContain("MCP_INSPECTOR_API_TOKEN is not set"); + }); + + it("requires --server-url", async () => { + const result = await runCli(["--print-handoff"]); + expectCliFailure(result); + expect(result.stderr).toContain("--print-handoff requires --server-url"); + }); +}); + +describe("--wait-for-auth", () => { + let server: ReturnType; + let serverUrl: string; + + beforeAll(async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + serverUrl = server.url; + }); + + afterAll(async () => { + await server.stop(); + }); + + it("polls until the token appears, then proceeds with it", async () => { + const dir = mkdtempSync(join(tmpdir(), "inspector-cli-wait-")); + const file = join(dir, "oauth.json"); + // Write the token after a short delay so the first poll misses. + setTimeout(() => { + writeFileSync( + file, + JSON.stringify({ + servers: { + [normalizeServerUrl(serverUrl)]: { + tokens: { access_token: "waited-tok", token_type: "Bearer" }, + }, + }, + idpSessions: {}, + }), + "utf8", + ); + }, 200); + try { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--wait-for-auth", + "5", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: file } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe("Bearer waited-tok"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("times out with exit 3 (AUTH_REQUIRED) when no token appears", async () => { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--wait-for-auth", + "1", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: "/no/such/file.json" } }, + ); + expect(result.exitCode).toBe(3); + const env = JSON.parse(result.stderr.trim()) as { + error: { code: string }; + }; + expect(env.error.code).toBe("auth_wait_timeout"); + }); + + it("rejects a non-positive timeout", async () => { + const result = await runCli([ + "--server-url", + "https://x.example/mcp", + "--wait-for-auth", + "0", + "--method", + "tools/list", + ]); + expectCliFailure(result); + expect(result.stderr).toContain("positive number of seconds"); + }); +}); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index ba225a8fe..193fa24f2 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -27,6 +27,8 @@ import { import type { JsonValue } from "@inspector/core/mcp/index.js"; import { extractAppInfo } from "@inspector/core/mcp/apps.js"; import type { AppInfo } from "@inspector/core/mcp/apps.js"; +import { getStateFilePath } from "@inspector/core/auth/node/storage-node.js"; +import { parseOAuthPersistBlob } from "@inspector/core/auth/oauth-persist.js"; import { CliExitCodeError, EXIT_CODES } from "./error-handler.js"; import { ConsoleNavigation, @@ -432,6 +434,123 @@ export async function collectAppInfo( } } +/** + * Canonicalise a server URL the same way the web inspector does before storing + * OAuth state (`new URL().href` lowercases the host, normalises the scheme, and + * adds a trailing `/` for bare-origin URLs). The CLI must look up by the same + * key the web side wrote, so a trailing-slash or case mismatch doesn't miss a + * token that's sitting one key over. Falls back to the raw string when the URL + * can't be parsed (e.g. an ad-hoc non-URL target). + */ +export function normalizeServerUrl(serverUrl: string): string { + try { + return new URL(serverUrl).href; + } catch { + return serverUrl; + } +} + +/** The stored-server map shape the CLI reads out of the OAuth state file. */ +type StoredServers = Record; + +/** + * Read the OAuth state file directly (bypassing the Zustand store cache) so + * each call sees the current on-disk state — required for `--wait-for-auth` + * polling. Returns the `servers` map, or an empty object when the file is + * absent or unreadable. Uses the shared {@link parseOAuthPersistBlob} so both + * the plain `{servers,idpSessions}` and legacy `{state,version}` layouts are + * accepted, matching whatever the web backend wrote. + */ +async function readOAuthServers(statePath: string): Promise { + const { readFile } = await import("node:fs/promises"); + try { + const text = await readFile(statePath, "utf8"); + const snapshot = parseOAuthPersistBlob(text); + return (snapshot?.servers as StoredServers | undefined) ?? {}; + } catch { + return {}; + } +} + +/** + * Look up a stored access token for `serverUrl`, trying the URL-normalised key + * first (how the web store writes it) and the raw string second. + */ +function findStoredToken( + servers: StoredServers, + serverUrl: string, +): string | undefined { + const key = normalizeServerUrl(serverUrl); + return ( + servers[key]?.tokens?.access_token ?? + servers[serverUrl]?.tokens?.access_token + ); +} + +/** + * Poll the OAuth state file until a token for `serverUrl` appears (or the + * timeout elapses). Used by `--wait-for-auth` so an automated caller can hand + * off to a human for the OAuth dance and resume once the token lands. The + * lookup is normalised, so a trailing-slash mismatch between the URL the human + * opened and the one the agent passed still resolves. + */ +async function waitForStoredToken( + serverUrl: string, + statePath: string, + timeoutSec: number, +): Promise { + const key = normalizeServerUrl(serverUrl); + const deadline = Date.now() + timeoutSec * 1000; + for (;;) { + const servers = await readOAuthServers(statePath); + const token = findStoredToken(servers, serverUrl); + if (token) return token; + if (Date.now() >= deadline) { + const stored = Object.keys(servers); + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `--wait-for-auth timed out after ${timeoutSec}s; no stored OAuth token for ${key} in ${statePath}.` + + (stored.length > 0 + ? ` Stored keys: ${stored.join(", ")}.` + : " No tokens stored yet."), + { code: "auth_wait_timeout", url: serverUrl }, + ); + } + await new Promise((r) => setTimeout(r, 500)); + } +} + +/** + * Build the JSON `--print-handoff` emits: everything an automated caller needs + * to relay to a human so they can complete OAuth in a browser and have the + * token land where the CLI will find it. + * + * NOTE: the `deepLink` query shape (serverUrl/transport/autoConnect) is the + * interim format pending the web deep-link auto-connect work (#1576); reconcile + * with that issue's canonical handoff format once it lands. + */ +function buildHandoff(serverUrl: string, statePath: string): McpResponse { + const host = process.env.HOST || "127.0.0.1"; + const clientPort = process.env.CLIENT_PORT || "6274"; + const sandboxPort = process.env.MCP_SANDBOX_PORT || "6275"; + // Treat an empty MCP_INSPECTOR_API_TOKEN the same as unset — an empty token + // can't satisfy the deep-link autoConnect gate. + const apiToken = process.env.MCP_INSPECTOR_API_TOKEN || undefined; + const params = new URLSearchParams({ serverUrl, transport: "http" }); + if (apiToken) params.set("autoConnect", apiToken); + return { + serverUrl: normalizeServerUrl(serverUrl), + deepLink: `http://${host}:${clientPort}/?${params.toString()}`, + portForwardCmd: `coder port-forward --tcp ${clientPort}:${clientPort} --tcp ${sandboxPort}:${sandboxPort}`, + oauthStatePath: statePath, + apiToken: apiToken ?? null, + note: + apiToken === undefined + ? "MCP_INSPECTOR_API_TOKEN is not set; the deep-link autoConnect gate will reject — launch the web inspector with a known token first." + : undefined, + }; +} + /** * Apply a connection timeout to a resolved server's settings, building a * minimal {@link InspectorServerSettings} when none came from the file. Ad-hoc @@ -482,16 +601,23 @@ function parseKeyValuePair( return { ...previous, [key as string]: parsedValue }; } -async function parseArgs(argv?: string[]): Promise<{ - serverConfig: MCPServerConfig; - serverSettings: InspectorServerSettings | undefined; - methodArgs: MethodArgs & { method: string }; - clientConfigPath?: string; - clientId?: string; - clientSecret?: string; - clientMetadataUrl?: string; - callbackUrl?: string; -}> { +type ParseResult = + | { + shortCircuit?: undefined; + serverConfig: MCPServerConfig; + serverSettings: InspectorServerSettings | undefined; + methodArgs: MethodArgs & { method: string }; + clientConfigPath?: string; + clientId?: string; + clientSecret?: string; + clientMetadataUrl?: string; + callbackUrl?: string; + } + // Short-circuit modes (`--list-stored-auth`, `--print-handoff`) do their own + // output and need no server connection; runCli returns immediately. + | { shortCircuit: true }; + +async function parseArgs(argv?: string[]): Promise { const program = new Command(); // On a parse/usage ERROR (exitCode !== 0), throw the CommanderError instead // of letting commander call process.exit(). The binary entry (index.ts) still @@ -664,6 +790,31 @@ async function parseArgs(argv?: string[]): Promise<{ .option( "--callback-url ", `OAuth redirect/callback listener URL (default: ${DEFAULT_RUNNER_OAUTH_CALLBACK_URL}, or MCP_OAUTH_CALLBACK_URL)`, + ) + .option( + "--use-stored-auth", + "Read the OAuth access token for --server-url from the OAuth state file (written by the web inspector) and inject it as Authorization: Bearer.", + ) + .option( + "--wait-for-auth ", + "Poll the OAuth state file until a token for --server-url appears (or the timeout elapses), then proceed as if --use-stored-auth were set. Use after handing off to a human to complete OAuth in a browser.", + (v: string) => { + const n = Number(v); + if (!Number.isFinite(n) || n <= 0) { + throw new Error( + `--wait-for-auth must be a positive number of seconds.`, + ); + } + return n; + }, + ) + .option( + "--list-stored-auth", + "Print the server URLs that have a stored OAuth token (one JSON object on stdout) and exit. No server connection is made.", + ) + .option( + "--print-handoff", + "Print a JSON handoff block (deepLink, portForwardCmd, oauthStatePath, apiToken) for --server-url and exit. No server connection is made.", ); program.parse(preArgs); @@ -695,8 +846,38 @@ async function parseArgs(argv?: string[]): Promise<{ clientSecret?: string; clientMetadataUrl?: string; callbackUrl?: string; + useStoredAuth?: boolean; + waitForAuth?: number; + listStoredAuth?: boolean; + printHandoff?: boolean; }; + // State-path precedence (getStateFilePath): MCP_INSPECTOR_OAUTH_STATE_PATH → + // /oauth.json → ~/.mcp-inspector/storage/oauth.json — the + // same file the web backend writes, so tokens are shared across surfaces. + const oauthStatePath = getStateFilePath(); + + // Short-circuit modes that need no server connection. + if (options.listStoredAuth) { + const servers = await readOAuthServers(oauthStatePath); + const withToken = Object.entries(servers) + .filter(([, v]) => Boolean(v.tokens?.access_token)) + .map(([k]) => k); + await awaitableLog( + JSON.stringify({ oauthStatePath, storedServerUrls: withToken }) + "\n", + ); + return { shortCircuit: true }; + } + if (options.printHandoff) { + if (!options.serverUrl) { + throw new Error("--print-handoff requires --server-url"); + } + await awaitableLog( + JSON.stringify(buildHandoff(options.serverUrl, oauthStatePath)) + "\n", + ); + return { shortCircuit: true }; + } + // 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). @@ -718,9 +899,48 @@ async function parseArgs(argv?: string[]): Promise<{ env: options.e, // `--header` is merged into the resolved server's settings (overriding any // file-level headers); file timeouts/OAuth are preserved. See #1482. - headers: options.header, + headers: options.header as Record | undefined, }; + if (options.waitForAuth !== undefined || options.useStoredAuth) { + if (!options.serverUrl) { + throw new Error( + `${options.waitForAuth !== undefined ? "--wait-for-auth" : "--use-stored-auth"} requires --server-url`, + ); + } + // Read the OAuth state file directly so the lookup is normalised the same + // way the web inspector wrote it (`new URL().href`), and so `--wait-for- + // auth` sees fresh on-disk state on each poll. Header injection is the + // prototype path — the token is passed blindly, so a stale token surfaces + // as HTTP 401 → exit 3 (auth_required). + let token: string; + if (options.waitForAuth !== undefined) { + token = await waitForStoredToken( + options.serverUrl, + oauthStatePath, + options.waitForAuth, + ); + } else { + const servers = await readOAuthServers(oauthStatePath); + const found = findStoredToken(servers, options.serverUrl); + if (!found) { + const key = normalizeServerUrl(options.serverUrl); + const stored = Object.keys(servers); + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `No stored OAuth token for ${key} in ${oauthStatePath}. Complete the OAuth flow in the web inspector first.` + + (stored.length > 0 ? ` Stored keys: ${stored.join(", ")}.` : ""), + { code: "no_stored_token", url: options.serverUrl }, + ); + } + token = found; + } + serverOptions.headers = { + ...(serverOptions.headers ?? {}), + Authorization: `Bearer ${token}`, + }; + } + // Shared with the TUI: resolves the catalog/config source (or ad-hoc target), // enforces the conflict matrix, and lifts disk headers/timeouts/OAuth into // per-server settings. `--server` selects one when the file has several. @@ -819,6 +1039,9 @@ async function parseArgs(argv?: string[]): Promise<{ } export async function runCli(argv?: string[]): Promise { + const parsed = await parseArgs(argv ?? process.argv); + // `--list-stored-auth` / `--print-handoff` already wrote their output. + if (parsed.shortCircuit) return; const { serverConfig, serverSettings, @@ -828,7 +1051,7 @@ export async function runCli(argv?: string[]): Promise { clientSecret, clientMetadataUrl, callbackUrl, - } = await parseArgs(argv ?? process.argv); + } = parsed; const clientConfig = await loadRunnerClientConfig({ clientConfigPath }); const callbackUrlConfig = parseRunnerOAuthCallbackUrl(callbackUrl); await callMethod( From adf41d3af93361a715442345b9bbdd82da9089a4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 15:24:01 -0400 Subject: [PATCH 4/6] docs(core): scope MCP_STORAGE_DIR comment in getStateFilePath to the OAuth backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review of PR #1651: the doc comment claimed MCP_STORAGE_DIR was honoured "across the app, so setting it once points every persist backend at the same directory", but the branch only relocates oauth.json. Narrow the comment to state it is OAuth-backend-only (client.json/mcp.json still resolve from getDefaultStorageDir → HOME) and record why the resolution lives here rather than in getDefaultStorageDir (the default-path constant is evaluated at module load, so a runtime-set MCP_STORAGE_DIR — as the CLI stored-auth / --wait-for-auth flows set it — must be read at call time). Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- core/auth/node/storage-node.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/core/auth/node/storage-node.ts b/core/auth/node/storage-node.ts index 6957bbdbf..cad693e77 100644 --- a/core/auth/node/storage-node.ts +++ b/core/auth/node/storage-node.ts @@ -15,9 +15,16 @@ const DEFAULT_STATE_PATH = getStoreFilePath(getDefaultStorageDir(), "oauth"); * 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 honoured - * across the app, so setting it once points every persist backend at the - * same directory + * 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 { From 38f494f137555744a70d1ca010d18080ed7541b6 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 15:45:12 -0400 Subject: [PATCH 5/6] review(cli): per-tool-tolerant app-info listing + drop dead helper (PR #1653) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review of PR #1653: - collectAppInfo now never throws: a malformed `_meta.ui.resourceUri` (extractAppInfo throws) folds into `{hasApp:false, resourceError}` just like a resources/read failure. This makes `tools/list --app-info` per-tool tolerant — one bad tool no longer aborts the whole NDJSON listing with a generic exit-1. Guarding inside collectAppInfo (vs. a loop-local try/catch) keeps the branch unit-testable; added an emit-result.test.ts case for the malformed-URI path. - Documented that `tools/list --app-info` always emits NDJSON regardless of `--format` (the per-tool list shape is fixed; --format json only reshapes the single-result paths) — in both the code comment and the README. - Removed the now-unused `expectJsonError` helper (its only caller moved to the exit-5 tool_not_found contract). Coverage stays ≥90 on all four dimensions (cli.ts 100% stmts/funcs/lines). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- clients/cli/README.md | 4 ++- clients/cli/__tests__/emit-result.test.ts | 22 ++++++++++++++++ clients/cli/__tests__/helpers/assertions.ts | 9 ------- clients/cli/src/cli.ts | 29 +++++++++++++++++---- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/clients/cli/README.md b/clients/cli/README.md index f0b2b75a8..10923437c 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -125,7 +125,7 @@ mcp-inspector --cli --method tools/call --tool-name my_tool --app-info mcp-inspector --cli --method tools/list --app-info | jq -c 'select(.hasApp)' ``` -Exit semantics: a tool that **has** an app exits `0`; one with **no** app exits `2` (`no_app`); a **missing** tool exits `5` (`tool_not_found`) — distinct so a typo isn't mistaken for "no app". A resource-read failure during the probe is tolerated and reported in a `resourceError` field rather than aborting. +Exit semantics: a tool that **has** an app exits `0`; one with **no** app exits `2` (`no_app`); a **missing** tool exits `5` (`tool_not_found`) — distinct so a typo isn't mistaken for "no app". A probe failure (an unreadable UI resource, or a malformed `_meta.ui.resourceUri`) is tolerated and reported in a `resourceError` field rather than aborting — so in `tools/list --app-info` one bad tool never kills the rest of the listing. `--format json` wraps any method's output in a single stdout envelope with no banners, so App tools and plain tools both pipe cleanly into `jq`: @@ -134,6 +134,8 @@ mcp-inspector --cli --method tools/call --tool-name my_app_tool --forma # → {"result":{…tool result…},"appInfo":{"hasApp":true,"resourceUri":"ui://…",…}} ``` +> `tools/list --app-info` always emits NDJSON (one raw app-info object per line) **regardless of `--format`** — the per-tool list shape is fixed. `--format json` only reshapes the single-result paths (`tools/call`, `tools/list` without `--app-info`, etc.) into the `{result[, appInfo]}` envelope. + A `tools/call` that returns `isError:true` still prints its payload but exits `5` (`tool_is_error`) so `&&` chains don't proceed on a failed call. ### CLI-specific (OAuth for HTTP servers) diff --git a/clients/cli/__tests__/emit-result.test.ts b/clients/cli/__tests__/emit-result.test.ts index 1ea588548..df4e8fc6f 100644 --- a/clients/cli/__tests__/emit-result.test.ts +++ b/clients/cli/__tests__/emit-result.test.ts @@ -107,4 +107,26 @@ describe("collectAppInfo", () => { const info = await collectAppInfo(client, tool, undefined); expect(info.resourceError).toBe("string failure"); }); + + it("folds a malformed _meta.ui.resourceUri into {hasApp:false, resourceError} instead of throwing", async () => { + // extractAppInfo throws when the advertised UI URI is not a `ui://` string; + // collectAppInfo must tolerate that so the tools/list --app-info NDJSON loop + // stays per-tool robust. readResource must never be reached here. + const readResource = vi.fn(); + const client = { readResource } as unknown as Pick< + InspectorClient, + "readResource" + >; + const tool = { + name: "bad-app", + inputSchema: { type: "object" as const }, + _meta: { ui: { resourceUri: "http://not-a-ui-uri" } }, + }; + const info = await collectAppInfo(client, tool, undefined); + expect(info.hasApp).toBe(false); + expect(info.toolName).toBe("bad-app"); + expect(typeof info.resourceError).toBe("string"); + expect(info.resourceError!.length).toBeGreaterThan(0); + expect(readResource.mock.calls.length).toBe(0); + }); }); diff --git a/clients/cli/__tests__/helpers/assertions.ts b/clients/cli/__tests__/helpers/assertions.ts index 3631c00bf..03da5f7e3 100644 --- a/clients/cli/__tests__/helpers/assertions.ts +++ b/clients/cli/__tests__/helpers/assertions.ts @@ -43,15 +43,6 @@ export function expectValidJson(result: CliResult) { return JSON.parse(result.stdout); } -/** - * Assert that output contains JSON with error flag - */ -export function expectJsonError(result: CliResult) { - const json = expectValidJson(result); - expect(json.isError).toBe(true); - return json; -} - /** * Assert that output contains expected JSON structure */ diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index ba225a8fe..8387a796d 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -187,6 +187,11 @@ async function callMethod( if (args.appInfo) { // NDJSON: one app-info line per tool, all on a single connection. A // caller that wants only the App tools can `| jq -c 'select(.hasApp)'`. + // collectAppInfo never throws — a tool with a malformed `_meta.ui` + // surfaces as `{hasApp:false, resourceError}` — so one bad tool can't + // abort the whole listing. Emitted verbatim as NDJSON regardless of + // --format (the list-probe shape is fixed; --format json only reshapes + // the single-result paths). for (const tool of tools) { const info = await collectAppInfo( inspectorClient, @@ -409,17 +414,31 @@ export async function emitResult( /** * Build the CLI's app-info for a tool: extract the tool-side `_meta.ui` and, * when the tool advertises a UI resource, follow it with a `resources/read` so - * the resource-side csp/permissions/domain are included. A read failure is - * tolerated — the tool-side info is still returned with `resourceError` set, - * since "tool says it has an app but the resource is unreadable" is itself a - * useful probe result. + * the resource-side csp/permissions/domain are included. + * + * Never throws — the two failure modes both fold into a `{hasApp:false, + * resourceError}` result rather than propagating: + * - a malformed `_meta.ui.resourceUri` (extractAppInfo throws), so the + * `tools/list --app-info` NDJSON loop stays per-tool tolerant (one bad tool + * can't abort the whole listing); + * - a `resources/read` failure, since "tool says it has an app but the + * resource is unreadable" is itself a useful probe result. */ export async function collectAppInfo( client: Pick, tool: Parameters[0], metadata: Record | undefined, ): Promise { - const base = extractAppInfo(tool); + let base: AppInfo; + try { + base = extractAppInfo(tool); + } catch (e) { + return { + hasApp: false, + toolName: tool.name, + resourceError: e instanceof Error ? e.message : String(e), + }; + } if (!base.hasApp || base.resourceUri === undefined) return base; try { const read = await client.readResource(base.resourceUri, metadata); From 133b90418ac941ee60d8bcb231579b6f4add1e94 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 16:05:51 -0400 Subject: [PATCH 6/6] =?UTF-8?q?review(cli):=20stored-auth=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20TODO(#1576),=20wait-for-auth=20guard=20test,=20docs?= =?UTF-8?q?=20(PR=20#1655)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review of PR #1655: - Added an explicit TODO(#1576) on buildHandoff's deepLink: `transport:"http"` is hardcoded even for SSE, and `autoConnect=` is not one of the three token sources the web app reads (per CLAUDE.md) — both to be reconciled with #1576's canonical handoff format. - Added a test for `--wait-for-auth` without `--server-url`, covering the previously-untested `--wait-for-auth` arm of the shared missing-server-url guard's message ternary. - Documented that `--list-stored-auth` / `--print-handoff` are mutually exclusive short-circuit modes (list takes precedence) and that a stored refresh_token is not yet used (noted as a follow-up in code + README). No behavior change; coverage stays ≥90 on all four dimensions (cli.ts 100% stmts/funcs/lines). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- clients/cli/README.md | 4 +++- clients/cli/__tests__/stored-auth.test.ts | 11 +++++++++++ clients/cli/src/cli.ts | 11 +++++++++-- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/clients/cli/README.md b/clients/cli/README.md index c2708225f..3563e870e 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -200,7 +200,9 @@ For the common case where OAuth was already completed in the **web inspector on **State-file resolution** follows `MCP_INSPECTOR_OAUTH_STATE_PATH` → `/oauth.json` → `~/.mcp-inspector/storage/oauth.json` — the same precedence the rest of the Inspector uses, so the CLI and web backend agree on the file. Server keys are canonicalised with `new URL().href` (the scheme the web store writes), so a trailing-slash or case mismatch between the URL a human opened and the one the agent passed still resolves. -**Blind injection.** The token is injected as-is; the CLI does not validate or refresh it. A stale/expired token surfaces as an HTTP `401` on the first request → exit `3` (`auth_required`). Re-complete the flow in the web inspector (or use `--wait-for-auth`) and retry. +**Blind injection.** The token is injected as-is; the CLI does not validate or refresh it (a stored `refresh_token` is not yet used — a natural follow-up). A stale/expired access token surfaces as an HTTP `401` on the first request → exit `3` (`auth_required`). Re-complete the flow in the web inspector (or use `--wait-for-auth`) and retry. + +**Short-circuit modes.** `--list-stored-auth` and `--print-handoff` each print their output and exit without connecting to a server; they ignore the method/target flags. They are mutually exclusive — if both are passed, `--list-stored-auth` takes precedence. > The `deepLink` shape emitted by `--print-handoff` is interim, pending the web deep-link auto-connect work ([#1576](https://github.com/modelcontextprotocol/inspector/issues/1576)); it will be reconciled with that issue's canonical handoff format once it lands. diff --git a/clients/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts index 22a8d93ba..69be24f59 100644 --- a/clients/cli/__tests__/stored-auth.test.ts +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -109,6 +109,17 @@ describe("--use-stored-auth", () => { expect(result.stderr).toContain("--use-stored-auth requires --server-url"); }); + it("errors clearly when --wait-for-auth is set without --server-url", async () => { + // Exercises the `--wait-for-auth` arm of the shared missing-server-url + // guard's message ternary (the --use-stored-auth arm is covered above). + const result = await runCli( + ["--wait-for-auth", "5", "--method", "tools/list"], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliFailure(result); + expect(result.stderr).toContain("--wait-for-auth requires --server-url"); + }); + it("errors with exit 3 (AUTH_REQUIRED) and lists stored keys when no token matches", async () => { const other = writeOAuthFixture({ "https://other.example/mcp": { diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 40b5d8a6e..059338db5 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -555,6 +555,11 @@ function buildHandoff(serverUrl: string, statePath: string): McpResponse { // Treat an empty MCP_INSPECTOR_API_TOKEN the same as unset — an empty token // can't satisfy the deep-link autoConnect gate. const apiToken = process.env.MCP_INSPECTOR_API_TOKEN || undefined; + // TODO(#1576): interim deep-link shape. `transport: "http"` is hardcoded even + // for SSE servers, and `autoConnect=` is NOT one of the three token + // sources the web app reads (window.__INSPECTOR_API_TOKEN__ / + // ?MCP_INSPECTOR_API_TOKEN / sessionStorage — see CLAUDE.md). Reconcile both + // with #1576's canonical handoff/deep-link format once it lands. const params = new URLSearchParams({ serverUrl, transport: "http" }); if (apiToken) params.set("autoConnect", apiToken); return { @@ -930,8 +935,10 @@ async function parseArgs(argv?: string[]): Promise { // Read the OAuth state file directly so the lookup is normalised the same // way the web inspector wrote it (`new URL().href`), and so `--wait-for- // auth` sees fresh on-disk state on each poll. Header injection is the - // prototype path — the token is passed blindly, so a stale token surfaces - // as HTTP 401 → exit 3 (auth_required). + // prototype path — only the access token is injected, blindly; a stale one + // surfaces as HTTP 401 → exit 3 (auth_required). Follow-up: when the stored + // `tokens.refresh_token` is present and the access token is expired, mint a + // fresh one instead of injecting the stale token. let token: string; if (options.waitForAuth !== undefined) { token = await waitForStoredToken(