diff --git a/clients/cli/README.md b/clients/cli/README.md index d8b753a4..57d40855 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -78,6 +78,8 @@ npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --tr npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --transport http --method tools/list --header "X-API-Key: your-api-key" ``` +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. + ## Options ### MCP server (which server to connect to) diff --git a/clients/cli/__tests__/cli.test.ts b/clients/cli/__tests__/cli.test.ts index 94da83f9..c833bf48 100644 --- a/clients/cli/__tests__/cli.test.ts +++ b/clients/cli/__tests__/cli.test.ts @@ -18,6 +18,7 @@ import { createEchoTool, createTestServerInfo, } from "@modelcontextprotocol/inspector-test-server"; +import type { MCPServerConfig } from "@modelcontextprotocol/inspector-core/mcp/index.js"; describe("CLI Tests", () => { describe("Basic CLI Mode", () => { @@ -320,6 +321,100 @@ describe("CLI Tests", () => { }); }); + describe("Config-file settings lifting (#1482)", () => { + it("applies a config file's custom header on a tools/call over HTTP", async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + let configPath: string | undefined; + try { + await server.start(); + // Write a config whose server carries disk-level headers + a timeout. + // `headers`/`requestTimeout` are Inspector-extension fields not present + // on MCPServerConfig, so build the entry as a loose record. + configPath = createTestConfig({ + mcpServers: { + web: { + type: "streamable-http", + url: server.url, + headers: { "X-Custom-Token": "secret-123" }, + requestTimeout: 8000, + } as unknown as MCPServerConfig, + }, + }); + + const result = await runCli([ + "--config", + configPath, + "--server", + "web", + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=hi", + ]); + + expectCliSuccess(result); + // The server should have received a request carrying the disk header + // (Express lower-cases header names), proving the CLI lifted it from + // the config file into the connection's settings. + const sawHeader = server + .getRecordedRequests() + .some((r) => r.headers["x-custom-token"] === "secret-123"); + expect(sawHeader).toBe(true); + } finally { + await server.stop(); + if (configPath) deleteConfigFile(configPath); + } + }); + + it("overrides a config file's header with --header", async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + let configPath: string | undefined; + try { + await server.start(); + configPath = createTestConfig({ + mcpServers: { + web: { + type: "streamable-http", + url: server.url, + headers: { "X-Custom-Token": "from-disk" }, + } as unknown as MCPServerConfig, + }, + }); + + const result = await runCli([ + "--config", + configPath, + "--server", + "web", + "--header", + "X-Custom-Token: from-cli", + "--cli", + "--method", + "tools/list", + ]); + + expectCliSuccess(result); + const tokens = server + .getRecordedRequests() + .map((r) => r.headers["x-custom-token"]); + expect(tokens).toContain("from-cli"); + expect(tokens).not.toContain("from-disk"); + } finally { + await server.stop(); + if (configPath) deleteConfigFile(configPath); + } + }); + }); + describe("Resource Options", () => { it("should read resource with URI", async () => { const { command, args } = getTestMcpServerCommand(); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index f648745f..be592401 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -8,7 +8,6 @@ import type { InspectorServerSettings, MCPServerConfig, } from "@inspector/core/mcp/types.js"; -import { DEFAULT_TASK_TTL_MS } from "@inspector/core/mcp/types.js"; import { InspectorClient } from "@inspector/core/mcp/index.js"; import { ManagedToolsState, @@ -18,9 +17,8 @@ import { } from "@inspector/core/mcp/state/index.js"; import { createTransportNode, - resolveLaunchServerConfigs, - serverSourceConflict, - hasAdHocServerOptions, + loadServerEntries, + selectServerEntry, parseKeyValuePair as parseEnvPair, parseHeaderPair, } from "@inspector/core/mcp/node/index.js"; @@ -46,23 +44,6 @@ type MethodArgs = { metadata?: Record; }; -function headersToServerSettings( - headers?: Record, -): InspectorServerSettings | undefined { - if (!headers || Object.keys(headers).length === 0) { - return undefined; - } - return { - headers: Object.entries(headers).map(([key, value]) => ({ key, value })), - metadata: [], - connectionTimeout: 0, - requestTimeout: 0, - taskTtl: DEFAULT_TASK_TTL_MS, - autoRefreshOnListChanged: false, - roots: [], - }; -} - async function callMethod( serverConfig: MCPServerConfig, serverSettings: InspectorServerSettings | undefined, @@ -397,30 +378,24 @@ function parseArgs(argv?: string[]): { // back to MCP_CATALOG_PATH — keeps CLI and TUI flag resolution identical. catalogPath: options.catalog?.trim() || process.env.MCP_CATALOG_PATH, configPath: options.config?.trim() || undefined, - serverName: options.server, target: targetArgs.length > 0 ? targetArgs : undefined, transport: options.transport, serverUrl: options.serverUrl, cwd: options.cwd, 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, }; - const conflict = serverSourceConflict({ - hasCatalog: Boolean(serverOptions.catalogPath?.trim()), - hasConfig: Boolean(serverOptions.configPath?.trim()), - hasAdHoc: hasAdHocServerOptions(serverOptions), - }); - if (conflict) { - throw new Error(conflict); - } - - const configs = resolveLaunchServerConfigs(serverOptions, "single"); - const serverConfig = configs[0]; - if (!serverConfig) { - throw new Error( - "Could not resolve server config. Specify a URL or command, or use --config and --server.", - ); - } + // 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. + const entries = loadServerEntries(serverOptions); + const { config: serverConfig, settings: serverSettings } = selectServerEntry( + entries, + options.server, + ); if (!options.method) { throw new Error( @@ -456,7 +431,7 @@ function parseArgs(argv?: string[]): { return { serverConfig, - serverSettings: headersToServerSettings(options.header), + serverSettings, methodArgs, }; } diff --git a/clients/tui/src/tui-servers.ts b/clients/tui/src/tui-servers.ts index c0e48bac..b861e85f 100644 --- a/clients/tui/src/tui-servers.ts +++ b/clients/tui/src/tui-servers.ts @@ -1,112 +1,12 @@ -import type { - InspectorServerSettings, - MCPServerConfig, - StdioServerConfig, -} from "@inspector/core/mcp/types.js"; -import { DEFAULT_TASK_TTL_MS } from "@inspector/core/mcp/types.js"; -import { mcpConfigToServerEntries } from "@inspector/core/mcp/serverList.js"; -import { - resolveServerConfigs, - resolveServerSource, - serverSourceConflict, - readServerListFile, - hasAdHocServerOptions, - withDefaultCatalogPath, - type ServerConfigOptions, -} from "@inspector/core/mcp/node/config.js"; +// The catalog/config → per-server-settings resolution is shared with the CLI in +// `@inspector/core/mcp/node` (see issue #1482). This module just re-exports it +// under the TUI's historical names so the TUI keeps a single import surface. +import type { ResolvedServer } from "@inspector/core/mcp/node/servers.js"; -export type TuiServer = { - config: MCPServerConfig; - settings?: InspectorServerSettings; -}; +export { + headersToServerSettings, + loadServerEntries as loadTuiServers, +} from "@inspector/core/mcp/node/servers.js"; -export function headersToServerSettings( - headers?: Record, -): InspectorServerSettings | undefined { - if (!headers || Object.keys(headers).length === 0) { - return undefined; - } - return { - headers: Object.entries(headers).map(([key, value]) => ({ key, value })), - metadata: [], - connectionTimeout: 0, - requestTimeout: 0, - taskTtl: DEFAULT_TASK_TTL_MS, - autoRefreshOnListChanged: false, - roots: [], - }; -} - -function applyStdioOverrides( - config: MCPServerConfig, - overrides: { env?: Record; cwd?: string }, -): MCPServerConfig { - if (config.type !== "stdio") return config; - const c = { ...config } as StdioServerConfig; - if (overrides.env && Object.keys(overrides.env).length > 0) { - c.env = { ...(c.env ?? {}), ...overrides.env }; - } - if (overrides.cwd?.trim()) { - c.cwd = overrides.cwd.trim(); - } - return c; -} - -function mergeSettings( - base: InspectorServerSettings | undefined, - headers?: Record, -): InspectorServerSettings | undefined { - const fromHeaders = headersToServerSettings(headers); - if (!fromHeaders) return base; - if (!base) return fromHeaders; - return { ...base, headers: fromHeaders.headers }; -} - -export function loadTuiServers( - serverOptions: ServerConfigOptions & { headers?: Record }, -): Record { - serverOptions = withDefaultCatalogPath(serverOptions); - - const conflict = serverSourceConflict({ - hasCatalog: Boolean(serverOptions.catalogPath?.trim()), - hasConfig: Boolean(serverOptions.configPath?.trim()), - hasAdHoc: hasAdHocServerOptions(serverOptions), - }); - if (conflict) { - throw new Error(conflict); - } - - const source = resolveServerSource(serverOptions); - - if (source) { - const config = readServerListFile(source.path, source.writable); - const entries = mcpConfigToServerEntries(config); - const result: Record = {}; - for (const entry of entries) { - result[entry.name] = { - config: applyStdioOverrides(entry.config, { - env: serverOptions.env, - cwd: serverOptions.cwd, - }), - // Deliberate broadcast: a single `--header` set is merged into EVERY - // server in the catalog/config (fine for the common single-server case; - // for multi-server files, prefer per-server headers in the file itself). - settings: mergeSettings(entry.settings, serverOptions.headers), - }; - } - return result; - } - - const configs = resolveServerConfigs(serverOptions, "multi"); - if (configs.length === 0) { - throw new Error( - "At least one server is required. Use --catalog/--config or an ad-hoc target (command/URL).", - ); - } - return { - default: { - config: configs[0]!, - settings: headersToServerSettings(serverOptions.headers), - }, - }; -} +/** A server resolved for the TUI: transport config plus per-server settings. */ +export type TuiServer = ResolvedServer; diff --git a/clients/web/src/test/core/mcp/node/config.test.ts b/clients/web/src/test/core/mcp/node/config.test.ts index 7a1fa12c..72983440 100644 --- a/clients/web/src/test/core/mcp/node/config.test.ts +++ b/clients/web/src/test/core/mcp/node/config.test.ts @@ -14,7 +14,6 @@ import { parseHeaderPair, withDefaultCatalogPath, resolveServerConfigs, - resolveLaunchServerConfigs, resolveServerSource, serverSourceConflict, readServerListFile, @@ -243,9 +242,17 @@ describe("readServerListFile", () => { }); }); -describe("resolveLaunchServerConfigs", () => { +describe("default-catalog launch resolution (withDefaultCatalogPath + resolveServerConfigs)", () => { + // The CLI/TUI launch path: apply the default writable catalog when no source + // or ad-hoc target is given, then resolve. (Both clients now compose these + // via loadServerEntries; this exercises the underlying config primitives.) let tempDir: string; + const resolveLaunch = ( + options: Parameters[0], + mode: Parameters[1], + ) => resolveServerConfigs(withDefaultCatalogPath(options), mode); + beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), "inspector-config-test-")); }); @@ -261,9 +268,9 @@ describe("resolveLaunchServerConfigs", () => { process.env.HOME = homeDir; const defaultConfig = join(homeDir, ".mcp-inspector", "mcp.json"); try { - expect(() => - resolveLaunchServerConfigs({ target: [] }, "single"), - ).toThrow(/No servers found/); + expect(() => resolveLaunch({ target: [] }, "single")).toThrow( + /No servers found/, + ); // The writable default catalog is seeded on first run (matches web), // rather than erroring with "Config file not found". expect(existsSync(defaultConfig)).toBe(true); @@ -281,7 +288,7 @@ describe("resolveLaunchServerConfigs", () => { mkdirSync(homeDir, { recursive: true }); process.env.HOME = homeDir; try { - expect(resolveLaunchServerConfigs({}, "multi")).toEqual([]); + expect(resolveLaunch({}, "multi")).toEqual([]); expect(existsSync(join(homeDir, ".mcp-inspector", "mcp.json"))).toBe( true, ); @@ -302,7 +309,7 @@ describe("resolveLaunchServerConfigs", () => { ); process.env.HOME = homeDir; try { - const configs = resolveLaunchServerConfigs({}, "multi"); + const configs = resolveLaunch({}, "multi"); expect(configs).toHaveLength(1); expect(configs[0]).toMatchObject({ type: "stdio", command: "a" }); } finally { diff --git a/clients/web/src/test/core/mcp/node/servers.test.ts b/clients/web/src/test/core/mcp/node/servers.test.ts new file mode 100644 index 00000000..dd87d539 --- /dev/null +++ b/clients/web/src/test/core/mcp/node/servers.test.ts @@ -0,0 +1,240 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { + existsSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + headersToServerSettings, + loadServerEntries, + selectServerEntry, + type ResolvedServer, +} from "@inspector/core/mcp/node/servers.js"; + +describe("headersToServerSettings", () => { + it("returns undefined when no headers are given", () => { + expect(headersToServerSettings()).toBeUndefined(); + expect(headersToServerSettings({})).toBeUndefined(); + }); + + it("builds a settings object carrying the headers as key/value pairs", () => { + const settings = headersToServerSettings({ Authorization: "Bearer t" }); + expect(settings?.headers).toEqual([ + { key: "Authorization", value: "Bearer t" }, + ]); + expect(settings?.metadata).toEqual([]); + expect(settings?.roots).toEqual([]); + }); +}); + +describe("loadServerEntries", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "server-entries-test-")); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + it("lifts disk headers, timeouts, and OAuth into per-server settings", () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { + web: { + type: "streamable-http", + url: "http://x/mcp", + headers: { Authorization: "Bearer disk" }, + connectionTimeout: 5000, + requestTimeout: 9000, + oauth: { + clientId: "client-abc", + clientSecret: "shh", + scopes: ["a", "b"], + }, + }, + }, + }), + ); + + const servers = loadServerEntries({ configPath }); + const settings = servers.web?.settings; + expect(settings?.headers).toEqual([ + { key: "Authorization", value: "Bearer disk" }, + ]); + expect(settings?.connectionTimeout).toBe(5000); + expect(settings?.requestTimeout).toBe(9000); + expect(settings?.oauthClientId).toBe("client-abc"); + expect(settings?.oauthClientSecret).toBe("shh"); + expect(settings?.oauthScopes).toEqual(["a", "b"]); + }); + + it("merges --header over disk headers while preserving disk timeouts", () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { + web: { + type: "streamable-http", + url: "http://x/mcp", + headers: { Authorization: "Bearer disk" }, + requestTimeout: 9000, + }, + }, + }), + ); + + const servers = loadServerEntries({ + configPath, + headers: { Authorization: "Bearer cli" }, + }); + expect(servers.web?.settings?.headers).toEqual([ + { key: "Authorization", value: "Bearer cli" }, + ]); + // Disk timeout survives the header override. + expect(servers.web?.settings?.requestTimeout).toBe(9000); + }); + + it("merges --header into a server that has no disk settings", () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { + web: { type: "streamable-http", url: "http://x/mcp" }, + }, + }), + ); + + const servers = loadServerEntries({ + configPath, + headers: { Authorization: "Bearer cli" }, + }); + expect(servers.web?.settings?.headers).toEqual([ + { key: "Authorization", value: "Bearer cli" }, + ]); + }); + + it("applies env and cwd overrides to stdio configs only", () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { + local: { command: "node", args: ["server.js"], env: { A: "1" } }, + web: { type: "streamable-http", url: "http://x/mcp" }, + }, + }), + ); + + const servers = loadServerEntries({ + configPath, + env: { B: "2" }, + cwd: "/tmp/work", + }); + expect(servers.local?.config).toMatchObject({ + type: "stdio", + command: "node", + env: { A: "1", B: "2" }, + cwd: "/tmp/work", + }); + // Non-stdio config is left untouched by env/cwd overrides. + expect(servers.web?.config).toMatchObject({ + type: "streamable-http", + url: "http://x/mcp", + }); + }); + + it("seeds an empty writable catalog when --catalog is missing", () => { + const catalogPath = join(tempDir, "catalog.json"); + const servers = loadServerEntries({ catalogPath }); + expect(servers).toEqual({}); + expect(existsSync(catalogPath)).toBe(true); + expect(JSON.parse(readFileSync(catalogPath, "utf-8"))).toEqual({ + mcpServers: {}, + }); + }); + + it("throws when a read-only --config file is missing (never seeds)", () => { + const configPath = join(tempDir, "absent.json"); + expect(() => loadServerEntries({ configPath })).toThrow( + /Config file not found/, + ); + expect(existsSync(configPath)).toBe(false); + }); + + it("rejects --catalog and --config together", () => { + const catalogPath = join(tempDir, "catalog.json"); + const configPath = join(tempDir, "config.json"); + writeFileSync(catalogPath, JSON.stringify({ mcpServers: {} })); + writeFileSync(configPath, JSON.stringify({ mcpServers: {} })); + expect(() => loadServerEntries({ catalogPath, configPath })).toThrow( + /mutually exclusive/, + ); + }); + + it("rejects --catalog combined with an ad-hoc target", () => { + const catalogPath = join(tempDir, "catalog.json"); + writeFileSync(catalogPath, JSON.stringify({ mcpServers: {} })); + expect(() => + loadServerEntries({ catalogPath, target: ["my-server"] }), + ).toThrow(/--catalog cannot be combined/); + }); + + it("builds a single ad-hoc server from a positional target", () => { + const servers = loadServerEntries({ + target: ["my-server", "--flag"], + headers: { Authorization: "Bearer t" }, + }); + expect(Object.keys(servers)).toEqual(["default"]); + expect(servers.default?.config).toMatchObject({ + type: "stdio", + command: "my-server", + args: ["--flag"], + }); + expect(servers.default?.settings?.headers).toEqual([ + { key: "Authorization", value: "Bearer t" }, + ]); + }); +}); + +describe("selectServerEntry", () => { + const a: ResolvedServer = { + config: { type: "stdio", command: "a" }, + }; + const b: ResolvedServer = { + config: { type: "stdio", command: "b" }, + }; + + it("returns the named entry when it exists", () => { + expect(selectServerEntry({ a, b }, "b")).toBe(b); + }); + + it("throws listing available servers when the name is unknown", () => { + expect(() => selectServerEntry({ a, b }, "missing")).toThrow( + /Server 'missing' not found.*Available servers: a, b/, + ); + }); + + it("returns the only entry when no name is given", () => { + expect(selectServerEntry({ a })).toBe(a); + }); + + it("throws when the source is empty and no name is given", () => { + expect(() => selectServerEntry({})).toThrow(/No servers found/); + }); + + it("throws asking for --server when several exist and no name is given", () => { + expect(() => selectServerEntry({ a, b })).toThrow( + /Multiple servers found.*--server.*Available servers: a, b/, + ); + }); +}); diff --git a/core/mcp/node/config.ts b/core/mcp/node/config.ts index 442a85a5..7d05b5d3 100644 --- a/core/mcp/node/config.ts +++ b/core/mcp/node/config.ts @@ -235,7 +235,7 @@ function buildConfigFromOptions(options: ServerConfigOptions): MCPServerConfig { * carry no overridable per-request fields here — custom headers live in * `InspectorServerSettings.headers` (the persisted per-server settings node), * not on `MCPServerConfig`. */ -function applyOverrides( +export function applyOverrides( config: MCPServerConfig, overrides: { env?: Record; @@ -247,7 +247,7 @@ function applyOverrides( if (overrides.env && Object.keys(overrides.env).length > 0) { c.env = { ...(c.env ?? {}), ...overrides.env }; } - if (overrides.cwd) c.cwd = overrides.cwd; + if (overrides.cwd?.trim()) c.cwd = overrides.cwd.trim(); return c; } return config; @@ -415,18 +415,6 @@ export function resolveServerConfigs( return []; } -/** - * Launch-time resolver for CLI/TUI: applies the default writable catalog path - * when no `--catalog`, `--config`, or ad-hoc target is given, then delegates to - * `resolveServerConfigs`. - */ -export function resolveLaunchServerConfigs( - options: ServerConfigOptions, - mode: ResolveServerConfigsMode, -): MCPServerConfig[] { - return resolveServerConfigs(withDefaultCatalogPath(options), mode); -} - /** * Returns named server configs from a catalog/config file (multi-server). Use * when the caller needs server names. Resolves the active source via diff --git a/core/mcp/node/index.ts b/core/mcp/node/index.ts index ec4d79e1..2162a724 100644 --- a/core/mcp/node/index.ts +++ b/core/mcp/node/index.ts @@ -3,7 +3,6 @@ export { parseHeaderPair, withDefaultCatalogPath, resolveServerConfigs, - resolveLaunchServerConfigs, getNamedServerConfigs, resolveServerSource, serverSourceConflict, @@ -13,4 +12,11 @@ export { type ResolveServerConfigsMode, type ServerSourceFlags, } from "./config.js"; +export { + headersToServerSettings, + loadServerEntries, + selectServerEntry, + type ResolvedServer, + type ServerLoadOptions, +} from "./servers.js"; export { createTransportNode } from "./transport.js"; diff --git a/core/mcp/node/servers.ts b/core/mcp/node/servers.ts new file mode 100644 index 00000000..0ee89e20 --- /dev/null +++ b/core/mcp/node/servers.ts @@ -0,0 +1,162 @@ +import type { InspectorServerSettings, MCPServerConfig } from "../types.js"; +import { DEFAULT_MAX_FETCH_REQUESTS, DEFAULT_TASK_TTL_MS } from "../types.js"; +import { mcpConfigToServerEntries } from "../serverList.js"; +import { + applyOverrides, + resolveServerConfigs, + resolveServerSource, + serverSourceConflict, + readServerListFile, + hasAdHocServerOptions, + withDefaultCatalogPath, + type ServerConfigOptions, +} from "./config.js"; + +/** + * A server resolved from a catalog/config file or an ad-hoc target, paired with + * the per-server settings (headers, timeouts, OAuth, etc.) lifted from the file. + * `config` is the SDK-facing transport config; `settings` is the v2 + * `InspectorServerSettings` model (post-#1358 top-level shape). + */ +export type ResolvedServer = { + config: MCPServerConfig; + settings?: InspectorServerSettings; +}; + +/** Loader options: the shared source flags plus a single `--header` set that is + * broadcast into every resolved server's settings. */ +export type ServerLoadOptions = ServerConfigOptions & { + headers?: Record; +}; + +/** + * Build an `InspectorServerSettings` carrying only `--header` overrides (all + * other fields defaulted). Returns undefined when no headers are given so it can + * be used as a no-op base in `mergeSettings`. + */ +export function headersToServerSettings( + headers?: Record, +): InspectorServerSettings | undefined { + if (!headers || Object.keys(headers).length === 0) { + return undefined; + } + return { + headers: Object.entries(headers).map(([key, value]) => ({ key, value })), + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: DEFAULT_TASK_TTL_MS, + maxFetchRequests: DEFAULT_MAX_FETCH_REQUESTS, + autoRefreshOnListChanged: false, + roots: [], + }; +} + +/** + * Overlay CLI `--header` values onto the settings lifted from the file. Only the + * `headers` field is overridden — timeouts, OAuth, and the rest of the file's + * settings are preserved. + */ +function mergeSettings( + base: InspectorServerSettings | undefined, + headers?: Record, +): InspectorServerSettings | undefined { + const fromHeaders = headersToServerSettings(headers); + if (!fromHeaders) return base; + if (!base) return fromHeaders; + return { ...base, headers: fromHeaders.headers }; +} + +/** + * Resolve every server from a catalog/config file (or a single ad-hoc target) + * into `{ config, settings }`, lifting disk-level headers/timeouts/OAuth into + * `InspectorServerSettings` via `mcpConfigToServerEntries`. Shared by the CLI + * and TUI so both apply the same file→settings resolution (issue #1482). + * + * Applies the default writable catalog when no source/ad-hoc target is given, + * enforces the `--catalog`/`--config`/ad-hoc conflict matrix, and seeds an empty + * writable catalog on first run (a missing read-only `--config` still errors). + */ +export function loadServerEntries( + serverOptions: ServerLoadOptions, +): Record { + serverOptions = withDefaultCatalogPath(serverOptions); + + const conflict = serverSourceConflict({ + hasCatalog: Boolean(serverOptions.catalogPath?.trim()), + hasConfig: Boolean(serverOptions.configPath?.trim()), + hasAdHoc: hasAdHocServerOptions(serverOptions), + }); + if (conflict) { + throw new Error(conflict); + } + + const source = resolveServerSource(serverOptions); + + if (source) { + const config = readServerListFile(source.path, source.writable); + const entries = mcpConfigToServerEntries(config); + const result: Record = {}; + for (const entry of entries) { + result[entry.name] = { + config: applyOverrides(entry.config, { + env: serverOptions.env, + cwd: serverOptions.cwd, + }), + // Deliberate broadcast: a single `--header` set is merged into EVERY + // server in the catalog/config (fine for the common single-server case; + // for multi-server files, prefer per-server headers in the file itself). + settings: mergeSettings(entry.settings, serverOptions.headers), + }; + } + return result; + } + + const configs = resolveServerConfigs(serverOptions, "multi"); + if (configs.length === 0) { + throw new Error( + "At least one server is required. Use --catalog/--config or an ad-hoc target (command/URL).", + ); + } + return { + default: { + config: configs[0]!, + settings: headersToServerSettings(serverOptions.headers), + }, + }; +} + +/** + * Pick a single resolved server for runners that connect to exactly one (the + * CLI). With `serverName`, returns that entry or errors listing the available + * names; otherwise requires exactly one server in the source. + * + * The unknown-name error is source-agnostic ("not found" rather than "not found + * in config file") because `entries` may come from a file *or* a single ad-hoc + * target — e.g. `--server foo` alongside a positional command resolves to just + * `{ default }`, where "in config file" would be misleading. + */ +export function selectServerEntry( + entries: Record, + serverName?: string, +): ResolvedServer { + const names = Object.keys(entries); + if (serverName) { + const entry = entries[serverName]; + if (!entry) { + throw new Error( + `Server '${serverName}' not found. Available servers: ${names.join(", ")}`, + ); + } + return entry; + } + if (names.length === 0) { + throw new Error("No servers found in config file"); + } + if (names.length > 1) { + throw new Error( + `Multiple servers found in config file. Please specify one with --server. Available servers: ${names.join(", ")}`, + ); + } + return entries[names[0]!]!; +} diff --git a/scripts/smoke-cli.mjs b/scripts/smoke-cli.mjs index 70a95a43..6649740d 100644 --- a/scripts/smoke-cli.mjs +++ b/scripts/smoke-cli.mjs @@ -14,6 +14,15 @@ * "no servers"), rather than erroring "Config file not found". * 3. A read-only `--config` that is missing errors and is NOT seeded. * 4. `--catalog` + `--config` is rejected as mutually exclusive. + * 5. An unknown `--server` errors with the source-agnostic "not found. + * Available servers: …" message (#1482). + * 6. A multi-server catalog selects via `--server` and errors (asking for a + * selection) when it is omitted. + * 7. `--header` merges into the resolved server's settings without breaking + * the connect path (the file→settings lift from #1482). + * 8. Over an HTTP transport (in-process test server), a config-file `headers` + * object is lifted onto the wire, and a CLI `--header` overrides it — the + * headline lift→transport path of #1482, verified end to end. * * Exits non-zero (failing CI / `npm run validate`) on any mismatch. * @@ -22,10 +31,11 @@ * (`test-servers/build`) is built on demand here if missing. */ -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { mkdtempSync, rmSync, existsSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; const repoRoot = resolve(import.meta.dirname, ".."); const launcher = join(repoRoot, "clients", "launcher", "build", "index.js"); @@ -35,24 +45,34 @@ const testServer = join( "build", "test-server-stdio.js", ); +const httpTestServerModule = join( + repoRoot, + "test-servers", + "build", + "test-server-http.js", +); function fail(message) { console.error(`smoke:cli FAILED — ${message}`); process.exit(1); } -/** Build the bundled stdio test server if it isn't present yet. */ +/** Build the bundled test servers (stdio + http) if they aren't present yet. */ function ensureTestServer() { - if (existsSync(testServer)) return; + if (existsSync(testServer) && existsSync(httpTestServerModule)) return; console.log("smoke:cli — building test-servers (missing build output)..."); const r = spawnSync("npx", ["tsc", "-p", "test-servers", "--noCheck"], { cwd: repoRoot, stdio: "inherit", }); - if (r.status !== 0 || !existsSync(testServer)) { + if ( + r.status !== 0 || + !existsSync(testServer) || + !existsSync(httpTestServerModule) + ) { fail( - "could not build the stdio test server (test-servers/build/test-server-stdio.js). " + - "Run `npm run test-servers:build` from clients/cli.", + "could not build the test servers (test-servers/build/test-server-stdio.js, " + + "test-server-http.js). Run `npm run test-servers:build` from clients/cli.", ); } } @@ -67,6 +87,28 @@ function runCli(args, extraEnv = {}) { return { status: r.status, stdout: r.stdout ?? "", stderr: r.stderr ?? "" }; } +/** + * Async variant of `runCli` using non-blocking `spawn`. Required for the HTTP + * case: the test server runs in THIS process's event loop, so a blocking + * `spawnSync` would deadlock (the CLI child's request could never be serviced). + * Returns { status, stdout, stderr }. + */ +function runCliAsync(args, extraEnv = {}) { + return new Promise((resolveRun) => { + const child = spawn(process.execPath, [launcher, "--cli", ...args], { + cwd: repoRoot, + env: { ...process.env, ...extraEnv }, + }); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf-8"); + child.stderr.setEncoding("utf-8"); + child.stdout.on("data", (d) => (stdout += d)); + child.stderr.on("data", (d) => (stderr += d)); + child.on("close", (status) => resolveRun({ status, stdout, stderr })); + }); +} + if (!existsSync(launcher)) { fail(`launcher build not found at ${launcher} — run \`npm run build\` first`); } @@ -161,9 +203,212 @@ try { ); } + // 5) Unknown --server errors with the source-agnostic message (#1482): the + // entry may come from a file or a single ad-hoc target, so it is "not + // found" rather than "not found in config file". + const unknownServer = runCli([ + "--catalog", + catalogPath, + "--server", + "nope", + "--method", + "tools/list", + ]); + if (unknownServer.status === 0) { + fail("--server with an unknown name should have exited non-zero"); + } + if ( + !/Server 'nope' not found\. Available servers: test/.test( + unknownServer.stderr, + ) + ) { + fail( + `expected "Server 'nope' not found. Available servers: test"; got:\n${unknownServer.stderr}`, + ); + } + + // 6) Multi-server catalog: --server selects one; omitting it errors asking for + // a selection (selectServerEntry over a multi-entry source). + const multiCatalogPath = join(work, "multi-catalog.json"); + writeFileSync( + multiCatalogPath, + JSON.stringify({ + mcpServers: { + one: { type: "stdio", command: process.execPath, args: [testServer] }, + two: { type: "stdio", command: process.execPath, args: [testServer] }, + }, + }), + ); + const ambiguous = runCli([ + "--catalog", + multiCatalogPath, + "--method", + "tools/list", + ]); + if (ambiguous.status === 0) { + fail("multi-server catalog without --server should have exited non-zero"); + } + if (!/Multiple servers found/.test(ambiguous.stderr)) { + fail( + `expected "Multiple servers found" without --server; got:\n${ambiguous.stderr}`, + ); + } + const selected = runCli([ + "--catalog", + multiCatalogPath, + "--server", + "two", + "--method", + "tools/list", + ]); + if (selected.status !== 0) { + fail( + `--server two against a multi-server catalog exited ${selected.status}\n${selected.stderr || selected.stdout}`, + ); + } + let selectedParsed; + try { + selectedParsed = JSON.parse(selected.stdout); + } catch { + fail(`--server two did not return JSON on stdout:\n${selected.stdout}`); + } + if (!(selectedParsed.tools ?? []).some((t) => t.name === "echo")) { + fail(`--server two tools/list missing expected "echo" tool`); + } + + // 7) --header is merged into the resolved server's settings and broadcast to + // every entry (#1482). Over stdio the header is inert, so the real check is + // that the merge path does not break the connect: tools/list still works. + const withHeader = runCli([ + "--catalog", + catalogPath, + "--server", + "test", + "--header", + "X-Smoke: 1", + "--method", + "tools/list", + ]); + if (withHeader.status !== 0) { + fail( + `tools/list with --header exited ${withHeader.status}\n${withHeader.stderr || withHeader.stdout}`, + ); + } + let withHeaderParsed; + try { + withHeaderParsed = JSON.parse(withHeader.stdout); + } catch { + fail( + `tools/list with --header did not return JSON on stdout:\n${withHeader.stdout}`, + ); + } + if (!(withHeaderParsed.tools ?? []).some((t) => t.name === "echo")) { + fail(`tools/list with --header missing expected "echo" tool`); + } + + // 8) HTTP transport — the headline of #1482: a config-file `headers` object is + // lifted into InspectorServerSettings and actually sent ON THE WIRE, and a + // CLI `--header` overrides it. Unlike stdio (where headers are inert), this + // proves the full lift→transport path. The HTTP test server runs IN-PROCESS + // so we can read back the headers it recorded; the CLI connects to it as a + // child via `runCliAsync` (a blocking spawnSync would deadlock the server's + // event loop). `getRecordedRequests()` lowercases header names (Node http). + const { createTestServerHttp } = await import( + pathToFileURL(httpTestServerModule).href + ); + const { createEchoTool, createTestServerInfo } = await import( + pathToFileURL( + join(repoRoot, "test-servers", "build", "test-server-fixtures.js"), + ).href + ); + const httpServer = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + serverType: "streamable-http", + }); + const httpPort = await httpServer.start(); + try { + const httpConfigPath = join(work, "http-config.json"); + const writeHttpConfig = (headers) => + writeFileSync( + httpConfigPath, + JSON.stringify({ + mcpServers: { + http: { + type: "http", + url: `http://localhost:${httpPort}/mcp`, + headers, + }, + }, + }), + ); + + // 8a) File-level header is lifted onto the wire. + writeHttpConfig({ "X-Smoke-Header": "from-config" }); + const httpList = await runCliAsync([ + "--config", + httpConfigPath, + "--server", + "http", + "--method", + "tools/list", + ]); + if (httpList.status !== 0) { + fail( + `HTTP tools/list exited ${httpList.status}\n${httpList.stderr || httpList.stdout}`, + ); + } + const headerValues = () => + httpServer + .getRecordedRequests() + .map((r) => r.headers?.["x-smoke-header"]) + .filter(Boolean); + if (!headerValues().includes("from-config")) { + fail( + `expected config-file header "X-Smoke-Header: from-config" on the wire; ` + + `recorded values: ${JSON.stringify(headerValues())}`, + ); + } + + // 8b) CLI --header overrides the file header on the wire (mergeSettings). + httpServer.clearRecordings(); + const httpOverride = await runCliAsync([ + "--config", + httpConfigPath, + "--server", + "http", + "--header", + "X-Smoke-Header: from-cli", + "--method", + "tools/list", + ]); + if (httpOverride.status !== 0) { + fail( + `HTTP tools/list with --header override exited ${httpOverride.status}\n${httpOverride.stderr || httpOverride.stdout}`, + ); + } + const overrideValues = headerValues(); + if (!overrideValues.includes("from-cli")) { + fail( + `expected --header override "X-Smoke-Header: from-cli" on the wire; ` + + `recorded values: ${JSON.stringify(overrideValues)}`, + ); + } + if (overrideValues.includes("from-config")) { + fail( + `--header should override the file header, but "from-config" still reached the wire; ` + + `recorded values: ${JSON.stringify(overrideValues)}`, + ); + } + } finally { + await httpServer.stop(); + } + console.log( "smoke:cli OK — tools/list over stdio via --catalog; default-catalog seed; " + - "read-only --config error (no seed); --catalog/--config conflict", + "read-only --config error (no seed); --catalog/--config conflict; " + + "unknown --server error; multi-server --server selection; --header merge; " + + "HTTP config-header lift + --header override on the wire", ); } finally { rmSync(work, { recursive: true, force: true }); diff --git a/specification/v2_catalog_launch_config.md b/specification/v2_catalog_launch_config.md index 23e7b9fc..ad43cc6d 100644 --- a/specification/v2_catalog_launch_config.md +++ b/specification/v2_catalog_launch_config.md @@ -3,35 +3,36 @@ ### [Brief](README.md) | [V1 Problems](v1_problems.md) | [V2 Scope](v2_scope.md) | V2 Tech Stack | [V2 UX](v2_ux.md) #### [Web Client](v2_web_client.md) | [CLI, TUI, Launcher](v2_cli_tui_launcher.md) | [Server](v2_server.md) | [Storage](v2_storage.md) + ##### [Overview](v2_storage.md) | [Server List File](v2_servers_file.md) | Catalog and Launch ## Table of Contents - * [Summary](#summary) - * [Goals](#goals) - * [Non-goals](#non-goals) - * [Related specifications](#related-specifications) - * [Terminology](#terminology) - * [User intents (UC1–UC5)](#user-intents-uc1uc5) - * [Flag reference](#flag-reference) - * [Background (v1.5)](#background-v15) - * [Normalization](#normalization) - * [Cross-client list contract](#cross-client-list-contract) - * [Launch behavior by client](#launch-behavior-by-client) - * [Import (UC2)](#import-uc2) - * [Write-back policy](#write-back-policy) - * [Web launch behavior](#web-launch-behavior) - * [Target API (core)](#target-api-core) - * [Open decisions](#open-decisions) - * [Known gaps](#known-gaps) - * [Decision matrix](#decision-matrix) - * [Engineering follow-ups](#engineering-follow-ups) - * [Related GitHub issues](#related-github-issues) - * [References](#references) +- [Summary](#summary) +- [Goals](#goals) +- [Non-goals](#non-goals) +- [Related specifications](#related-specifications) +- [Terminology](#terminology) +- [User intents (UC1–UC5)](#user-intents-uc1uc5) +- [Flag reference](#flag-reference) +- [Background (v1.5)](#background-v15) +- [Normalization](#normalization) +- [Cross-client list contract](#cross-client-list-contract) +- [Launch behavior by client](#launch-behavior-by-client) +- [Import (UC2)](#import-uc2) +- [Write-back policy](#write-back-policy) +- [Web launch behavior](#web-launch-behavior) +- [Target API (core)](#target-api-core) +- [Open decisions](#open-decisions) +- [Known gaps](#known-gaps) +- [Decision matrix](#decision-matrix) +- [Engineering follow-ups](#engineering-follow-ups) +- [Related GitHub issues](#related-github-issues) +- [References](#references) ## Summary -v2 treats `~/.mcp-inspector/mcp.json` as the default **catalog** — a persistent, Inspector-owned grouping of MCP servers. The web client loads and mutates it via `/api/servers`. CLI and TUI resolve servers from that path when launch args omit both `--config` and ad-hoc targets (`resolveLaunchServerConfigs()` / `withDefaultConfigPath()` in `core/mcp/node/config.ts`). +v2 treats `~/.mcp-inspector/mcp.json` as the default **catalog** — a persistent, Inspector-owned grouping of MCP servers. The web client loads and mutates it via `/api/servers`. CLI and TUI resolve servers from that path when launch args omit both `--catalog`/`--config` and ad-hoc targets (the shared `loadServerEntries()` applies `withDefaultCatalogPath()`, in `core/mcp/node/servers.ts` / `config.ts`). This specification defines **launch-time configuration** across web, CLI, and TUI: how `--catalog`, `--config`, `--server`, and ad-hoc flags select a server list; which paths are writable; and how import differs from session launch. On-disk catalog format and web CRUD are specified in [Server List File](v2_servers_file.md). CLI/TUI/launcher build and per-client implementation gaps are in [CLI, TUI, and Launcher](v2_cli_tui_launcher.md). @@ -56,26 +57,26 @@ This specification defines **launch-time configuration** across web, CLI, and TU ## Related specifications -| Document | Owns | -|----------|------| -| [v2_servers_file.md](v2_servers_file.md) | Catalog path, on-disk shape, web CRUD, `/api/servers`, per-server settings | -| [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md) | Bins, tsup, tests, `runWeb`, CLI config-file settings gap (G1) | -| [v2_ux.md](v2_ux.md) | Import config vs Import server.json menu UX | -| v1.5 `docs/mcp-server-configuration.md` | Baseline shared launch flags (port to v2 pending, G6) | +| Document | Owns | +| ------------------------------------------------ | ------------------------------------------------------------------------------- | +| [v2_servers_file.md](v2_servers_file.md) | Catalog path, on-disk shape, web CRUD, `/api/servers`, per-server settings | +| [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md) | Bins, tsup, tests, `runWeb`, CLI config-file settings lift (G1, resolved #1482) | +| [v2_ux.md](v2_ux.md) | Import config vs Import server.json menu UX | +| v1.5 `docs/mcp-server-configuration.md` | Baseline shared launch flags (port to v2 pending, G6) | --- ## Terminology -| Term | Meaning | -|------|---------| -| **Catalog** | Inspector-owned `mcp.json` used as a persistent server grouping. Default: `~/.mcp-inspector/mcp.json`. Active catalog selected by `--catalog ` or default (UC1/UC4). CRUD when `writable: true`. | -| **`--catalog`** | Selects the **active catalog** for this run (`writable: true`). Omitted → default path (UC1). | -| **Alternate config file** | Any JSON passed via `--config ` without catalog intent. **Read-only, run-scoped** server set (UC3). Inspector does not write to this path. | -| **Canonical catalog format** | Normalized `MCPConfig` / `ServerEntry[]` after `normalizeServerType`, `normalizeMcpServers`, and `mcpConfigToServerEntries` — the only shape written to a catalog. | -| **Catalog lookup** | `--server ` with no `--config` and no ad-hoc target. `resolveLaunchServerConfigs()` injects the catalog path via `withDefaultConfigPath()`, then the named entry is resolved. | -| **Ad-hoc / inline config** | Positional command/URL and/or `--transport`, `--server-url`, `-e`, `--cwd`, `--header` without `--config`. Ephemeral unless the user persists via CRUD or import. | -| **Launch-time initial config (web)** | `initialMcpConfig` from `runWeb(argv)` → `GET /api/config`. Legacy v1.5 channel; largely unused by the v2 web UI today (G2). | +| Term | Meaning | +| ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Catalog** | Inspector-owned `mcp.json` used as a persistent server grouping. Default: `~/.mcp-inspector/mcp.json`. Active catalog selected by `--catalog ` or default (UC1/UC4). CRUD when `writable: true`. | +| **`--catalog`** | Selects the **active catalog** for this run (`writable: true`). Omitted → default path (UC1). | +| **Alternate config file** | Any JSON passed via `--config ` without catalog intent. **Read-only, run-scoped** server set (UC3). Inspector does not write to this path. | +| **Canonical catalog format** | Normalized `MCPConfig` / `ServerEntry[]` after `normalizeServerType`, `normalizeMcpServers`, and `mcpConfigToServerEntries` — the only shape written to a catalog. | +| **Catalog lookup** | `--server ` with no `--catalog`/`--config` and no ad-hoc target. `loadServerEntries()` injects the catalog path via `withDefaultCatalogPath()`, then `selectServerEntry()` resolves the named entry. | +| **Ad-hoc / inline config** | Positional command/URL and/or `--transport`, `--server-url`, `-e`, `--cwd`, `--header` without `--config`. Ephemeral unless the user persists via CRUD or import. | +| **Launch-time initial config (web)** | `initialMcpConfig` from `runWeb(argv)` → `GET /api/config`. Legacy v1.5 channel; largely unused by the v2 web UI today (G2). | --- @@ -87,11 +88,11 @@ Five distinct user goals. They share flags (`--config`, `--server`, `--catalog`) **Intent:** Use the usual server list at `~/.mcp-inspector/mcp.json` — the default active catalog (UC4 with default path). Add, edit, remove, and configure settings through the UX; CLI invokes methods against a named catalog entry. -| | Today | Target | -|---|--------|--------| -| **Web** | Works — `useServers` + `/api/servers` CRUD | Same | -| **TUI** | Loads default catalog via `withDefaultConfigPath` in `loadTuiServers`; **no CRUD UX yet** | Catalog CRUD in TUI (planned) | -| **CLI** | `--server foo --method …` with no `--config` resolves against default catalog | Same; lift settings from catalog entries (G1) | +| | Today | Target | +| ------- | --------------------------------------------------------------------------------------------- | -------------------------------------------------------------- | +| **Web** | Works — `useServers` + `/api/servers` CRUD | Same | +| **TUI** | Loads default catalog via `withDefaultCatalogPath` in `loadServerEntries`; **no CRUD UX yet** | Catalog CRUD in TUI (planned) | +| **CLI** | `--server foo --method …` with no `--config` resolves against default catalog | Same; settings lifted from catalog entries (G1 resolved #1482) | ```bash mcp-inspector --web @@ -105,10 +106,10 @@ The catalog is both the **data source** and the **write target** (`writable: tru **Intent:** Add servers **into** an Inspector catalog (default or `--catalog` target). Distinct from “launch and use for this session” (UC3). Import is a **catalog write**, not a launch mode. -| Source | CLI shape | Stored | -|--------|-----------|--------| -| **A — `mcpServers` file** | `--config /path/to/source.json` [ `--server ` ] `--method servers/import` | One or all entries, canonicalized | -| **B — Ad-hoc parts** | Same argv as ad-hoc launch (`[target...]`, `--transport`, `--server-url`, `-e`, `--cwd`, `--header`, …); no `--config`. Method → `servers/import`. | One entry; **`--server `** = catalog map key (required for B) | +| Source | CLI shape | Stored | +| ------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | +| **A — `mcpServers` file** | `--config /path/to/source.json` [ `--server ` ] `--method servers/import` | One or all entries, canonicalized | +| **B — Ad-hoc parts** | Same argv as ad-hoc launch (`[target...]`, `--transport`, `--server-url`, `-e`, `--cwd`, `--header`, …); no `--config`. Method → `servers/import`. | One entry; **`--server `** = catalog map key (required for B) | ```bash # A: import all from a foreign mcp.json @@ -148,11 +149,11 @@ mcp-inspector --cli --catalog ./project/.mcp-inspector/mcp.json \ **Not UC2:** `mcp-inspector --cli node ./server.js --method tools/list` without `servers/import` is UC3/UC1 (use now), not import. -| | Today | Target | -|---|--------|--------| -| **CLI** | Not implemented | `servers/import` for A and B via `importServersIntoCatalog` in core | +| | Today | Target | +| ------- | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **CLI** | Not implemented | `servers/import` for A and B via `importServersIntoCatalog` in core | | **Web** | Import menu items stubbed | Interactive import per [v2_ux.md](v2_ux.md); [#1348](https://github.com/modelcontextprotocol/inspector/issues/1348), [#1435](https://github.com/modelcontextprotocol/inspector/issues/1435) | -| **TUI** | N/A | No requirement | +| **TUI** | N/A | No requirement | See [Import (UC2)](#import-uc2) for behavior rules. @@ -160,11 +161,11 @@ See [Import (UC2)](#import-uc2) for behavior rules. **Intent:** Run against **another** server set for **this session only** — ad-hoc (one server from command/URL) or file (all servers in TUI; one selected by `--server` for CLI). User sees those servers in list UI; Add/Edit/Remove must not mutate the file (`writable: false`). -| | Today | Target | -|---|--------|--------| -| **TUI** | Works for `--config ` and ad-hoc via `loadTuiServers` | Same + explicit `writable: false` for non-catalog paths | +| | Today | Target | +| ------- | -------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ | +| **TUI** | Works for `--config ` and ad-hoc via `loadTuiServers` | Same + explicit `writable: false` for non-catalog paths | | **Web** | UI always shows default catalog; `runWeb --config` only sets legacy `initialMcpConfig` | Session `mcpConfigPath` → `/api/servers` serves that file; `writable: false`; disable CRUD | -| **CLI** | Single resolved server (file + `--server`, ad-hoc, or default catalog) | Same; shared normalizer + settings lift | +| **CLI** | Single resolved server (file + `--server`, ad-hoc, or default catalog) | Same; shared normalizer + settings lift | ```bash mcp-inspector --tui --config ~/Library/Application\ Support/Claude/claude_desktop_config.json @@ -189,11 +190,11 @@ mcp-inspector --cli --catalog ~/work/mcp.json --server api --method tools/list **Distinction from UC3:** UC3 uses **`--config`** (or ad-hoc) for a foreign/session file — show servers, **do not write**. UC4 uses **`--catalog`** — this file **is** the Inspector catalog for this run. -| | Today | Target | -|---|--------|--------| -| **Web** | Only default catalog — backend always `getDefaultMcpConfigPath()` | `runWeb --catalog ` → `mcpConfigPath`; CRUD on active catalog | -| **TUI** | `--config` loads a file; no catalog vs session distinction | `--catalog` = writable active catalog; `--config` without `--catalog` → UC3 | -| **CLI** | `--config` for one-shot reads; default path when only `--server` | `--catalog` scopes UC1/UC2/UC5; `--config` remains session-only | +| | Today | Target | +| ------- | ----------------------------------------------------------------- | --------------------------------------------------------------------------- | +| **Web** | Only default catalog — backend always `getDefaultMcpConfigPath()` | `runWeb --catalog ` → `mcpConfigPath`; CRUD on active catalog | +| **TUI** | `--config` loads a file; no catalog vs session distinction | `--catalog` = writable active catalog; `--config` without `--catalog` → UC3 | +| **CLI** | `--config` for one-shot reads; default path when only `--server` | `--catalog` scopes UC1/UC2/UC5; `--config` remains session-only | **Env alternative:** `MCP_CATALOG_PATH` — same semantics as `--catalog` for scripts/CI. @@ -201,21 +202,21 @@ mcp-inspector --cli --catalog ~/work/mcp.json --server api --method tools/list **Intent:** Among loaded servers (catalog or session), focus one entry — required for CLI (one server), useful for web/TUI. -| | Today | Target | -|---|--------|--------| -| **CLI** | `--server` with catalog (UC1) or `--config` file (UC3) | Same | -| **TUI** | All servers shown; no `--server` flag | Optional highlight / initial selection | -| **Web** | Ignores `--server` for list/selection | `defaultServerId` from launch + optional auto-connect ([#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)) | +| | Today | Target | +| ------- | ------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------ | +| **CLI** | `--server` with catalog (UC1) or `--config` file (UC3) | Same | +| **TUI** | All servers shown; no `--server` flag | Optional highlight / initial selection | +| **Web** | Ignores `--server` for list/selection | `defaultServerId` from launch + optional auto-connect ([#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)) | ### Use-case map (quick reference) -| UC | User goal | How selected | Writable? | Primary clients | Import? | -|----|-----------|--------------|-----------|-----------------|---------| -| **1** | Manage default catalog | (no `--catalog`) | Yes | Web (now), TUI (CRUD planned), CLI | No | -| **2** | Add servers to a catalog | `--method servers/import` | Writes **active catalog** only | CLI; web UI | Yes | -| **3** | Session with external config | `--config` or ad-hoc | **No** | TUI (now), Web (gap), CLI | No | -| **4** | Switch / use another catalog | `--catalog ` | Yes (that path) | Web/TUI (gaps) | Via UC2 | -| **5** | Pick one server by name | `--server` | — | CLI (required), Web/TUI (hint) | — | +| UC | User goal | How selected | Writable? | Primary clients | Import? | +| ----- | ---------------------------- | ------------------------- | ------------------------------ | ---------------------------------- | ------- | +| **1** | Manage default catalog | (no `--catalog`) | Yes | Web (now), TUI (CRUD planned), CLI | No | +| **2** | Add servers to a catalog | `--method servers/import` | Writes **active catalog** only | CLI; web UI | Yes | +| **3** | Session with external config | `--config` or ad-hoc | **No** | TUI (now), Web (gap), CLI | No | +| **4** | Switch / use another catalog | `--catalog ` | Yes (that path) | Web/TUI (gaps) | Via UC2 | +| **5** | Pick one server by name | `--server` | — | CLI (required), Web/TUI (hint) | — | **Design principle:** UC1 is UC4 with the default path. Catalog modes (UC1/UC4): `writable: true`. Session mode (UC3): `writable: false`. UC2 mutates the active catalog only — not a launch side-effect on all clients. @@ -225,21 +226,21 @@ mcp-inspector --cli --catalog ~/work/mcp.json --server api --method tools/list Shared launch flags (full detail in v1.5 `docs/mcp-server-configuration.md`, port pending): -| Flag | UC1/UC4 (catalog) | UC3 (session) | UC2 (import) | -|------|-------------------|---------------|--------------| -| `--catalog ` | Active writable catalog | — | Target catalog for write | -| `--config ` | — | Read-only session file | **Source** file (A only) | -| `--server ` | Catalog entry id (UC5) | Entry in session file | Source name (A) or new catalog id (B) | -| `[target...]`, `-e`, `--cwd`, `--transport`, `--server-url`, `--header` | Ad-hoc overrides on connect | Ad-hoc session server | Same argv as launch (B) | +| Flag | UC1/UC4 (catalog) | UC3 (session) | UC2 (import) | +| ----------------------------------------------------------------------- | --------------------------- | ---------------------- | ------------------------------------- | +| `--catalog ` | Active writable catalog | — | Target catalog for write | +| `--config ` | — | Read-only session file | **Source** file (A only) | +| `--server ` | Catalog entry id (UC5) | Entry in session file | Source name (A) or new catalog id (B) | +| `[target...]`, `-e`, `--cwd`, `--transport`, `--server-url`, `--header` | Ad-hoc overrides on connect | Ad-hoc session server | Same argv as launch (B) | **`--server` overload (by context — no new flags):** -| Context | `--server` means | -|---------|------------------| -| `servers/import` + `--config` | Entry to copy **from source file** | -| `servers/import` + ad-hoc (no `--config`) | Catalog **id** for new entry | -| `tools/list` (etc.) + no `--config` | Lookup **from active catalog** | -| `tools/list` + `--config` | Lookup **from session file** | +| Context | `--server` means | +| ----------------------------------------- | ---------------------------------- | +| `servers/import` + `--config` | Entry to copy **from source file** | +| `servers/import` + ad-hoc (no `--config`) | Catalog **id** for new entry | +| `tools/list` (etc.) + no `--config` | Lookup **from active catalog** | +| `tools/list` + `--config` | Lookup **from session file** | Optional import flags (target): `--on-conflict skip|error|replace`. @@ -247,7 +248,7 @@ Optional import flags (target): `--on-conflict skip|error|replace`. ## Background (v1.5) -v1.5 (`docs/mcp-server-configuration.md`) had no default catalog: omitting `--config` and positional args failed resolution. Config-file `headers` lived on `MCPServerConfig`. Web had no file-backed list — launcher argv became `initialMcpConfig` for form pre-fill only. v2 adds the persistent catalog ([Server List File](v2_servers_file.md)), flat on-disk settings (post-#1358), and `resolveLaunchServerConfigs()` for CLI/TUI launch resolution. +v1.5 (`docs/mcp-server-configuration.md`) had no default catalog: omitting `--config` and positional args failed resolution. Config-file `headers` lived on `MCPServerConfig`. Web had no file-backed list — launcher argv became `initialMcpConfig` for form pre-fill only. v2 adds the persistent catalog ([Server List File](v2_servers_file.md)), flat on-disk settings (post-#1358), and the shared `loadServerEntries()` for CLI/TUI launch resolution (lifting disk settings into `InspectorServerSettings`). --- @@ -255,12 +256,12 @@ v1.5 (`docs/mcp-server-configuration.md`) had no default catalog: omitting `--co External config files (Claude Desktop, Cursor, hand-edited JSON) are not reliably conformant. **Read path:** always parse → normalize → lift; never connect from raw file bytes. -| Stage | Where | What | -|-------|-------|------| -| Parse | `loadMcpServersConfig` / backend JSON parse | Structural check; tolerate unknown keys | -| Normalize transport | `normalizeServerType` (`core/mcp/serverList.ts`) | Missing type → `stdio`; `http` → `streamable-http` | -| Normalize catalog entries | `normalizeMcpServers` (`core/mcp/remote/node/server.ts`) | Legacy `settings` hard-cutover; secret migration | -| Lift to runtime | `mcpConfigToServerEntries` | Map key → `id`; flat disk fields → `InspectorServerSettings` | +| Stage | Where | What | +| ------------------------- | -------------------------------------------------------- | ------------------------------------------------------------ | +| Parse | `loadMcpServersConfig` / backend JSON parse | Structural check; tolerate unknown keys | +| Normalize transport | `normalizeServerType` (`core/mcp/serverList.ts`) | Missing type → `stdio`; `http` → `streamable-http` | +| Normalize catalog entries | `normalizeMcpServers` (`core/mcp/remote/node/server.ts`) | Legacy `settings` hard-cutover; secret migration | +| Lift to runtime | `mcpConfigToServerEntries` | Map key → `id`; flat disk fields → `InspectorServerSettings` | **Write path (catalog only):** `serverEntriesToMcpConfig` / `buildStoredEntry` / `writeStoreFile` per [Server List File](v2_servers_file.md). @@ -283,13 +284,13 @@ Required inputs: Centralization belongs in **core**, not the launcher. Today: -| Piece | Today | Target | -|-------|--------|--------| -| **Canonical conversion** | `mcpConfigToServerEntries` | Same | -| **TUI** | `loadTuiServers` — full settings; no `writable` | `resolveServerList`; gate UI on `writable` | -| **Web** | `useServers` → always default catalog; CRUD always on | `/api/servers` returns session or catalog list + `writable` | -| **CLI** | Single server via `resolveLaunchServerConfigs` | `resolveSingleServer` from same normalizer | -| **Launcher** | Forwards argv | Unchanged | +| Piece | Today | Target | +| ------------------------ | ----------------------------------------------------------------------------- | ----------------------------------------------------------- | +| **Canonical conversion** | `mcpConfigToServerEntries` | Same | +| **TUI** | `loadTuiServers` — full settings; no `writable` | `resolveServerList`; gate UI on `writable` | +| **Web** | `useServers` → always default catalog; CRUD always on | `/api/servers` returns session or catalog list + `writable` | +| **CLI** | Single server via `loadServerEntries` + `selectServerEntry` (settings lifted) | `resolveSingleServer` from same normalizer | +| **Launcher** | Forwards argv | Unchanged | Import is not a list-client concern — catalog mutation via UC2 or web CRUD when `writable`. @@ -303,12 +304,12 @@ Implemented (#1481/#1483): `runWeb` selects the backend's catalog source and a session-wide `writable` flag (`/api/config`); the web UI hides catalog CRUD when `writable: false`. -| Launch | Catalog (`/api/servers`) | `writable` | Connect | -|--------|--------------------------|-----------|---------| -| No server args | Default catalog via `useServers` | `true` | User selects catalog server | -| `--catalog ` / `MCP_CATALOG_PATH` | That file (seed/CRUD/watch) | `true` | User selects a server | -| `--config ` | That file, served read-only (never written/seeded/migrated) | `false` | User selects; CRUD hidden | -| Ad-hoc server flags (`--server-url`/command, `--header`) | One server held **in memory** (no file) | `false` | User connects; `--header` applied; CRUD hidden | +| Launch | Catalog (`/api/servers`) | `writable` | Connect | +| -------------------------------------------------------- | ----------------------------------------------------------- | ---------- | ---------------------------------------------- | +| No server args | Default catalog via `useServers` | `true` | User selects catalog server | +| `--catalog ` / `MCP_CATALOG_PATH` | That file (seed/CRUD/watch) | `true` | User selects a server | +| `--config ` | That file, served read-only (never written/seeded/migrated) | `false` | User selects; CRUD hidden | +| Ad-hoc server flags (`--server-url`/command, `--header`) | One server held **in memory** (no file) | `false` | User connects; `--header` applied; CRUD hidden | `--catalog`/`--config` are mutually exclusive, and neither combines with an ad-hoc target or `--header`; `--header` requires an ad-hoc HTTP/SSE server. @@ -317,37 +318,26 @@ ad-hoc target or `--header`; `--header` requires an ad-hoc HTTP/SSE server. ### CLI and TUI -CLI connects to exactly one resolved config per invocation; TUI shows all entries from the resolved list. Resolution paths, settings lift, and port gaps are documented in [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). Key remaining catalog-specific gap: **G1** (CLI drops disk settings on catalog/file lookup). (Web's **G4** `--header` and **G2** list-seeding are resolved — see above.) +CLI connects to exactly one resolved config per invocation; TUI shows all entries from the resolved list. Resolution paths, settings lift, and port gaps are documented in [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). (Web's **G4** `--header` and **G2** list-seeding are resolved — see above; **G1**, the CLI dropping disk settings, is resolved by #1482 — see below.) -### Shared resolvers (`resolveLaunchServerConfigs` / `resolveServerConfigs`) +### Shared resolver (`loadServerEntries` / `selectServerEntry`) -CLI and TUI call `resolveLaunchServerConfigs()` at launch; it applies `withDefaultConfigPath()` then delegates to `resolveServerConfigs()`: +CLI and TUI both call the shared `loadServerEntries()` in `core/mcp/node/servers.ts` at launch (the TUI's `loadTuiServers` is now a thin re-export). It applies `withDefaultCatalogPath()`, enforces the conflict matrix, and lifts disk settings via `mcpConfigToServerEntries()`: ``` -withDefaultConfigPath(options) - → if no --config and no ad-hoc target: configPath = ~/.mcp-inspector/mcp.json - -resolveServerConfigs(options, mode) — explicit options only; no default injection +loadServerEntries(options) → Record + withDefaultCatalogPath(options) + → if no --catalog/--config and no ad-hoc target: catalogPath = ~/.mcp-inspector/mcp.json + source (catalog/config) → mcpConfigToServerEntries (settings lifted), --header merged per entry + ad-hoc → resolveServerConfigs(..., "multi") → single { default } entry, --header as settings ``` -**single mode (CLI):** - -``` -configPath + serverName → loadServerFromConfig (MCPServerConfig only — G1) -configPath, no serverName → sole entry or error if multiple -ad-hoc → buildConfigFromOptions -``` - -**multi mode (TUI):** - -``` -configPath → all entries via mcpConfigToServerEntries (settings lifted) in loadTuiServers -ad-hoc → resolveServerConfigs(..., "multi") in loadTuiServers -``` +- **CLI:** `selectServerEntry(entries, --server)` picks one (named, or the sole entry, else errors). Disk `headers`/timeouts/OAuth are now applied (#1482 closes **G1**); `--header` overrides the file's headers. +- **TUI:** uses the full `entries` record (all servers shown). Web `runWeb` uses `resolveServerConfigs()` directly (explicit server argv only); catalog path is backend-owned until launch-config unification. -`loadServerFromConfig` returns bare `MCPServerConfig`. Only `mcpConfigToServerEntries()` lifts flat disk fields into `InspectorServerSettings`. +`mcpConfigToServerEntries()` is the single place that lifts flat disk fields into `InspectorServerSettings`; both clients now flow through it. --- @@ -355,11 +345,11 @@ Web `runWeb` uses `resolveServerConfigs()` directly (explicit server argv only); **`servers/import`** on CLI only (plus web Import menu for interactive use). Targets the **active catalog** (default or `--catalog`). No launcher `--import` flag — `mcp-inspector --cli … --method servers/import` is sufficient. -| Approach | Rationale | -|----------|-----------| -| **CLI method** | Explicit, scriptable, reuses `--config` / ad-hoc argv parsing; same normalization as `POST /api/servers` | -| **Not launcher flag** | Launcher routes only; catalog mutation blurs responsibilities | -| **Not all clients at startup** | TUI/web import is UI concern (paste/upload) | +| Approach | Rationale | +| ------------------------------ | -------------------------------------------------------------------------------------------------------- | +| **CLI method** | Explicit, scriptable, reuses `--config` / ad-hoc argv parsing; same normalization as `POST /api/servers` | +| **Not launcher flag** | Launcher routes only; catalog mutation blurs responsibilities | +| **Not all clients at startup** | TUI/web import is UI concern (paste/upload) | **Behavior:** @@ -377,15 +367,15 @@ Web **Import config** vs **Import server.json** menu semantics: [v2_ux.md](v2_ux No silent write-back to **any** file on launch. Catalog mutations go through `/api/servers`, `servers/import`, or explicit user CRUD. -| Action | Writes catalog? | Writes session `--config` file? | -|--------|-----------------|--------------------------------| -| Launch ad-hoc command/URL | No | No | -| Launch `--server` + overrides (`-e`, `--cwd`, `--header`) | No | No | -| Launch `--config /other.json` | No | **No** (read-only) | -| User Add/Edit/Settings (web UI) | Yes (canonical serialize) | No | -| Import config / server.json (web UI, future) | Yes | No | -| `--cli --method servers/import` | Yes (active catalog) | No | -| Future explicit `--save-as ` | Opt-in only | No | +| Action | Writes catalog? | Writes session `--config` file? | +| --------------------------------------------------------- | ------------------------- | ------------------------------- | +| Launch ad-hoc command/URL | No | No | +| Launch `--server` + overrides (`-e`, `--cwd`, `--header`) | No | No | +| Launch `--config /other.json` | No | **No** (read-only) | +| User Add/Edit/Settings (web UI) | Yes (canonical serialize) | No | +| Import config / server.json (web UI, future) | Yes | No | +| `--cli --method servers/import` | Yes (active catalog) | No | +| Future explicit `--save-as ` | Opt-in only | No | **Runtime overlay model (CLI/TUI):** @@ -402,12 +392,12 @@ catalog entry (config + settings) **Target model: catalog-first (B1).** The catalog always exists in the UI. Launch args influence selection and connect ergonomics; they do not replace the catalog or auto-persist ad-hoc servers. -| Behavior | Rule | -|----------|------| -| Catalog UI | Always show full resolved catalog (or UC3 session list when wired) | -| `--server ` | Set `activeServerId` / `defaultServerId` to matching catalog entry | -| Ad-hoc inline config | Temporary “session server” or “Quick connect” — **do not** auto-`POST /api/servers` | -| Auto-connect | Optional `--connect` or `?autoConnect=true` ([#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)) | +| Behavior | Rule | +| -------------------- | -------------------------------------------------------------------------------------------------------------------- | +| Catalog UI | Always show full resolved catalog (or UC3 session list when wired) | +| `--server ` | Set `activeServerId` / `defaultServerId` to matching catalog entry | +| Ad-hoc inline config | Temporary “session server” or “Quick connect” — **do not** auto-`POST /api/servers` | +| Auto-connect | Optional `--connect` or `?autoConnect=true` ([#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)) | **Rejected alternatives:** launch-only UI hiding catalog (B2); v1.5 form pre-fill without catalog (B3). @@ -454,12 +444,12 @@ importServersIntoCatalog( ## Open decisions -| Topic | Status | Notes | -|-------|--------|-------| -| Import collision default | TBD | `skip` vs `error` vs `replace` for duplicate catalog ids | -| `MCP_CATALOG_PATH` env name | Proposed | Alias for `--catalog` | -| Ad-hoc web UX detail | Partially decided | B1 catalog-first; exact “Quick connect” vs banner TBD | -| TUI `--server` initial selection | Open | Optional highlight when UC5 used from launcher | +| Topic | Status | Notes | +| -------------------------------- | ----------------- | -------------------------------------------------------- | +| Import collision default | TBD | `skip` vs `error` vs `replace` for duplicate catalog ids | +| `MCP_CATALOG_PATH` env name | Proposed | Alias for `--catalog` | +| Ad-hoc web UX detail | Partially decided | B1 catalog-first; exact “Quick connect” vs banner TBD | +| TUI `--server` initial selection | Open | Optional highlight when UC5 used from launcher | Resolved (documented above): catalog vs session flag split; no launch write-back; CLI-only `servers/import`; core-owned `resolveServerList`; web catalog-first (B1). @@ -467,18 +457,18 @@ Resolved (documented above): catalog vs session flag split; no launch write-back ## Known gaps -| # | Gap | Severity | Clients | -|---|-----|----------|---------| -| G1 | CLI catalog/file lookup drops settings — `loadServerFromConfig` not `mcpConfigToServerEntries` | Functional | CLI | -| G2 | ~~Web ignores launch-time server input for the list~~ — **resolved for web (#1481):** `runWeb` now points the backend catalog at `--catalog` (writable) / `--config` (read-only session), or serves an in-memory ad-hoc server, and `/api/config` carries `writable` so the UI gates CRUD. Selection/auto-connect (`--server` / `defaultServerId`) is still open (G8). | Web | -| G3 | ~~`InitialConfigPayload` has no settings — cannot pass headers at web launch~~ — **resolved for ad-hoc headers (#1483):** ad-hoc `--header` rides on the in-memory session entry (`headers` → `settings.headers`); a general settings channel on `InitialConfigPayload` is still unbuilt. | Web + launcher | -| G4 | ~~`runWeb --header` warns only~~ — **resolved (#1483):** applied to the seeded ad-hoc connection (or a clear error when it can't apply). | Web launcher | -| G5 | Two config pipelines — `config.ts` vs `serverList.ts` | Design debt | CLI vs TUI vs web | -| G6 | v1.5 `docs/mcp-server-configuration.md` not ported to v2 | Docs | All | -| G7 | CLI test expects `--server` without `--config` to fail — predates default catalog | Tests | CLI | -| G8 | No auto-connect ([#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)) | Enhancement | Web | -| G9 | Import config / Import server.json menu stubbed | UX | Web | -| G10 | CLI thin parse path — no full `normalizeMcpServers` / settings lift on catalog GET parity | Consistency | CLI | +| # | Gap | Severity | Clients | +| --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------- | ----------------- | +| G1 | ~~CLI catalog/file lookup drops settings — `loadServerFromConfig` not `mcpConfigToServerEntries`~~ — **resolved (#1482):** CLI now resolves via the shared `loadServerEntries()` → `mcpConfigToServerEntries()`, lifting disk `headers`/timeouts/OAuth; `--header` overrides the file's headers. | Functional | CLI | +| G2 | ~~Web ignores launch-time server input for the list~~ — **resolved for web (#1481):** `runWeb` now points the backend catalog at `--catalog` (writable) / `--config` (read-only session), or serves an in-memory ad-hoc server, and `/api/config` carries `writable` so the UI gates CRUD. Selection/auto-connect (`--server` / `defaultServerId`) is still open (G8). | Web | +| G3 | ~~`InitialConfigPayload` has no settings — cannot pass headers at web launch~~ — **resolved for ad-hoc headers (#1483):** ad-hoc `--header` rides on the in-memory session entry (`headers` → `settings.headers`); a general settings channel on `InitialConfigPayload` is still unbuilt. | Web + launcher | +| G4 | ~~`runWeb --header` warns only~~ — **resolved (#1483):** applied to the seeded ad-hoc connection (or a clear error when it can't apply). | Web launcher | +| G5 | Two config pipelines — `config.ts` vs `serverList.ts` | Design debt | CLI vs TUI vs web | +| G6 | v1.5 `docs/mcp-server-configuration.md` not ported to v2 | Docs | All | +| G7 | CLI test expects `--server` without `--config` to fail — predates default catalog | Tests | CLI | +| G8 | No auto-connect ([#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)) | Enhancement | Web | +| G9 | Import config / Import server.json menu stubbed | UX | Web | +| G10 | CLI thin parse path — no full `normalizeMcpServers` / settings lift on catalog GET parity | Consistency | CLI | G1, G4, and launcher details: [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). @@ -486,22 +476,22 @@ G1, G4, and launcher details: [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). ## Decision matrix -| User launches with… | File read | Canonicalize on read? | Catalog UI (web) | Writable? | -|---------------------|-----------|----------------------|------------------|-----------| -| Nothing (defaults) | Default catalog | Yes | Full list | Catalog only | -| `--server foo` | Default catalog | Yes | Full list; should select `foo` | Catalog only | -| `--config other.json` | Alternate file (UC3) | Yes | Session list (target); today default catalog | **No** | -| `--catalog ./proj/mcp.json` | Project catalog | Yes | That catalog (target) | That path | -| Ad-hoc `node srv.js` | None | N/A | Full catalog + session hint (target) | Catalog only if user saves | -| Web Add / Edit / Import | N/A | Yes before write | Full list | Catalog only | -| `servers/import` | Source (A) or ad-hoc (B) | Yes before write | N/A (CLI) | Active catalog | +| User launches with… | File read | Canonicalize on read? | Catalog UI (web) | Writable? | +| --------------------------- | ------------------------ | --------------------- | -------------------------------------------- | -------------------------- | +| Nothing (defaults) | Default catalog | Yes | Full list | Catalog only | +| `--server foo` | Default catalog | Yes | Full list; should select `foo` | Catalog only | +| `--config other.json` | Alternate file (UC3) | Yes | Session list (target); today default catalog | **No** | +| `--catalog ./proj/mcp.json` | Project catalog | Yes | That catalog (target) | That path | +| Ad-hoc `node srv.js` | None | N/A | Full catalog + session hint (target) | Catalog only if user saves | +| Web Add / Edit / Import | N/A | Yes before write | Full list | Catalog only | +| `servers/import` | Source (A) or ad-hoc (B) | Yes before write | N/A (CLI) | Active catalog | --- ## Engineering follow-ups 1. **Core:** `resolveServerList`, `resolveSingleServer`, `importServersIntoCatalog`. -2. **CLI:** resolve through shared normalizer; implement `--method servers/import` (G1, G10). +2. **CLI:** resolve through shared normalizer; implement `--method servers/import` (G10; G1 resolved #1482). 3. **Web:** `/api/servers` serves resolved session or catalog list + `writable` + `defaultServerId` (G2). 4. **TUI:** `resolveServerList`; respect `writable` (no CRUD on UC3 files). 5. **Launcher / web:** extend `InitialConfigPayload` for settings at launch (G3, G4). @@ -512,18 +502,19 @@ G1, G4, and launcher details: [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). ## Related GitHub issues -| Issue | Status | Relevance | -|-------|--------|-----------| -| [#1481](https://github.com/modelcontextprotocol/inspector/issues/1481) — launcher `--config` drives the web catalog | Implemented | `--catalog` (writable) / `--config` (read-only session) / ad-hoc in-memory → backend `mcpConfigPath`/`initialServers` + `writable` on `/api/config`; UI gates CRUD | -| [#1483](https://github.com/modelcontextprotocol/inspector/issues/1483) — web `--header` warns only | Implemented | `--header` lifted onto the in-memory ad-hoc entry (`settings.headers`); errors clearly when it can't apply | -| [#1347](https://github.com/modelcontextprotocol/inspector/issues/1347) — default `--config` | Open (likely closable) | Implemented via `resolveLaunchServerConfigs()` | -| [#1246](https://github.com/modelcontextprotocol/inspector/issues/1246) — port CLI/TUI/launcher | Done in worktree | Parent of default-catalog behavior | -| [#1183](https://github.com/modelcontextprotocol/inspector/issues/1183) — auto-connect | Open | UC5 web ergonomics | -| [#1348](https://github.com/modelcontextprotocol/inspector/issues/1348) — import from other clients | Open | UC2 web UI | -| [#1435](https://github.com/modelcontextprotocol/inspector/issues/1435) — registry import | Open | UC2 registry path | -| [#1432](https://github.com/modelcontextprotocol/inspector/issues/1432) — CLI v2 | Open | Session CLI umbrella | -| [#1352](https://github.com/modelcontextprotocol/inspector/pull/1352) / [#1358](https://github.com/modelcontextprotocol/inspector/pull/1358) | Merged | Flat settings on disk | -| [#1356](https://github.com/modelcontextprotocol/inspector/pull/1356) | Merged | Secrets in keychain | +| Issue | Status | Relevance | +| ------------------------------------------------------------------------------------------------------------------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| [#1481](https://github.com/modelcontextprotocol/inspector/issues/1481) — launcher `--config` drives the web catalog | Implemented | `--catalog` (writable) / `--config` (read-only session) / ad-hoc in-memory → backend `mcpConfigPath`/`initialServers` + `writable` on `/api/config`; UI gates CRUD | +| [#1483](https://github.com/modelcontextprotocol/inspector/issues/1483) — web `--header` warns only | Implemented | `--header` lifted onto the in-memory ad-hoc entry (`settings.headers`); errors clearly when it can't apply | +| [#1347](https://github.com/modelcontextprotocol/inspector/issues/1347) — default `--config` + `--catalog`/`--config` normalization | Closed (#1499) | Default catalog + normalized flags; CLI/TUI resolve via the shared `loadServerEntries()` | +| [#1482](https://github.com/modelcontextprotocol/inspector/issues/1482) — CLI lift config-file settings (G1) | Closed (#1500) | CLI routes file-sourced servers through `loadServerEntries()`/`mcpConfigToServerEntries()`; disk headers/timeouts/OAuth applied | +| [#1246](https://github.com/modelcontextprotocol/inspector/issues/1246) — port CLI/TUI/launcher | Done in worktree | Parent of default-catalog behavior | +| [#1183](https://github.com/modelcontextprotocol/inspector/issues/1183) — auto-connect | Open | UC5 web ergonomics | +| [#1348](https://github.com/modelcontextprotocol/inspector/issues/1348) — import from other clients | Open | UC2 web UI | +| [#1435](https://github.com/modelcontextprotocol/inspector/issues/1435) — registry import | Open | UC2 registry path | +| [#1432](https://github.com/modelcontextprotocol/inspector/issues/1432) — CLI v2 | Open | Session CLI umbrella | +| [#1352](https://github.com/modelcontextprotocol/inspector/pull/1352) / [#1358](https://github.com/modelcontextprotocol/inspector/pull/1358) | Merged | Flat settings on disk | +| [#1356](https://github.com/modelcontextprotocol/inspector/pull/1356) | Merged | Secrets in keychain | --- @@ -532,10 +523,11 @@ G1, G4, and launcher details: [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). - [v2_servers_file.md](v2_servers_file.md) — catalog format, CRUD, settings on disk - [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md) — CLI/TUI/launcher architecture and port gaps - [v2_ux.md](v2_ux.md) — Import config vs Import server.json UX -- `core/mcp/node/config.ts` — `withDefaultConfigPath`, `resolveLaunchServerConfigs`, `resolveServerConfigs` +- `core/mcp/node/servers.ts` — `loadServerEntries`, `selectServerEntry` (shared CLI/TUI catalog + settings load path) +- `core/mcp/node/config.ts` — `withDefaultCatalogPath`, `resolveServerConfigs`, `serverSourceConflict` - `core/mcp/serverList.ts` — `mcpConfigToServerEntries` -- `clients/tui/src/tui-servers.ts` — TUI catalog + settings load path -- `clients/cli/src/cli.ts` — single-config resolve; settings gap (G1) +- `clients/tui/src/tui-servers.ts` — thin re-export of the shared `loadServerEntries` +- `clients/cli/src/cli.ts` — single-config resolve via `loadServerEntries`/`selectServerEntry` (G1 resolved #1482) - `clients/web/server/run-web.ts` — launcher argv → `initialMcpConfig` - `clients/web/src/App.tsx` — catalog via `useServers` - `clients/web/src/components/groups/ImportServerJsonPanel/` — registry panel (unwired) diff --git a/specification/v2_cli_tui_launcher.md b/specification/v2_cli_tui_launcher.md index 34c3d5fa..a8b5eecc 100644 --- a/specification/v2_cli_tui_launcher.md +++ b/specification/v2_cli_tui_launcher.md @@ -13,7 +13,7 @@ This document describes how those clients are built, wired, and tested today, an ## Goals - One published entry binary (`mcp-inspector`) that forwards argv to the selected client. -- CLI and TUI use the same `InspectorClient`, managed-state classes, and `resolveLaunchServerConfigs()` path as v1.5 — adapted to v2's `InspectorServerSettings` model (post-#1358). +- CLI and TUI use the same `InspectorClient`, managed-state classes, and shared `loadServerEntries()` resolution path — adapted to v2's `InspectorServerSettings` model (post-#1358). - Bundle `core/` into CLI/TUI Node artifacts with **tsup** (analogous to Vite bundling core for the browser). - Port v1.5 CLI integration tests (86 cases) and TUI smoke tests without duplicating `core/` unit tests (web suite remains canonical for core). - CI runs CLI and TUI tests plus a launcher `--help` smoke step on every PR. @@ -31,12 +31,12 @@ This document describes how those clients are built, wired, and tested today, an ## Packaging and entrypoints -| Artifact | Path | Build | Published bin | -|----------|------|-------|---------------| -| Launcher | `clients/launcher/` | `tsc` → `build/index.js` | Root `mcp-inspector` → `clients/launcher/build/index.js` | -| CLI | `clients/cli/` | `tsup` → `build/index.js` | `mcp-inspector-cli` (client package only) | -| TUI | `clients/tui/` | `tsup` → `build/index.js` | `mcp-inspector-tui` (client package only) | -| Web runner | `clients/web/server/run-web.ts` | `tsup` (`build:runner`) → `clients/web/build/index.js` | `mcp-inspector-web` (client package only) | +| Artifact | Path | Build | Published bin | +| ---------- | ------------------------------- | ------------------------------------------------------ | -------------------------------------------------------- | +| Launcher | `clients/launcher/` | `tsc` → `build/index.js` | Root `mcp-inspector` → `clients/launcher/build/index.js` | +| CLI | `clients/cli/` | `tsup` → `build/index.js` | `mcp-inspector-cli` (client package only) | +| TUI | `clients/tui/` | `tsup` → `build/index.js` | `mcp-inspector-tui` (client package only) | +| Web runner | `clients/web/server/run-web.ts` | `tsup` (`build:runner`) → `clients/web/build/index.js` | `mcp-inspector-web` (client package only) | Root `package.json` (`@modelcontextprotocol/inspector` v2) publishes a **fat package**: merged runtime `dependencies`, `files` manifest listing each client's `build/` (and web `dist/`), and `prepack` → full `npm run build`. On the dev side, a `postinstall` runs `scripts/install-clients.mjs`, which cascades `npm install` into each client so one `npm install` at the repo root sets up every client (also exposed as `npm run install:clients`); it no-ops when the package is installed as a dependency. The launcher does not declare `file:` sibling dependencies; it dynamically imports `../web/build/index.js`, `../cli/build/index.js`, or `../tui/build/index.js` relative to its own `build/` directory. @@ -73,22 +73,22 @@ Root scripts `inspector`, `web`, and `web:dev` are thin wrappers around the laun All three clients import from `@inspector/core/...` (mapped to `../../core/` source). -| Concern | Web | CLI / TUI | Launcher | -|---------|-----|-----------|----------| -| Dev typecheck | `tsconfig.app.json` paths | per-client `tsconfig.json` paths | `tsconfig.json` (no core) | -| Runtime bundle | Vite alias | tsup `noExternal: [/^@inspector\/core/]` + esbuild alias | n/a | -| Tests | Vitest projects in `clients/web/` | Vitest + `vitest.shared.mts` aliases | none | +| Concern | Web | CLI / TUI | Launcher | +| -------------- | --------------------------------- | -------------------------------------------------------- | ------------------------- | +| Dev typecheck | `tsconfig.app.json` paths | per-client `tsconfig.json` paths | `tsconfig.json` (no core) | +| Runtime bundle | Vite alias | tsup `noExternal: [/^@inspector\/core/]` + esbuild alias | n/a | +| Tests | Vitest projects in `clients/web/` | Vitest + `vitest.shared.mts` aliases | none | `vitest.shared.mts` at repo root centralizes `@inspector/core` and test-server aliases plus bare-module pins (`react`, `pino`, SDK, etc.) so CLI/TUI Vitest configs stay aligned with web. **Resolved design choices:** -| Topic | Decision | -|-------|----------| -| Core package | No separate `inspector-core` npm package; source-only `core/` | -| CLI/TUI build | tsup bundles `@inspector/core` into `build/index.js` | -| Core tests | Not duplicated under cli/tui; web unit + integration suites cover `core/` | -| Default config path | `resolveLaunchServerConfigs()` applies `withDefaultConfigPath()` → `~/.mcp-inspector/mcp.json` when no `--config` and no ad-hoc target | +| Topic | Decision | +| ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | +| Core package | No separate `inspector-core` npm package; source-only `core/` | +| CLI/TUI build | tsup bundles `@inspector/core` into `build/index.js` | +| Core tests | Not duplicated under cli/tui; web unit + integration suites cover `core/` | +| Default config path | `loadServerEntries()` applies `withDefaultCatalogPath()` → `~/.mcp-inspector/mcp.json` when no `--catalog`/`--config` and no ad-hoc target | --- @@ -100,11 +100,11 @@ All three clients import from `@inspector/core/...` (mapped to `../../core/` sou **Server resolution:** -1. Positional `[target...]` (command/URL) and/or `--config` + `--server`, plus `-e`, `--cwd`, `--transport`, `--server-url`. -2. `resolveLaunchServerConfigs(serverOptions, "single")` in `core/mcp/node/config.ts` — applies default catalog path when appropriate. -3. Ad-hoc `--header` pairs map to `InspectorServerSettings` and pass into `InspectorClient` via `serverSettings` (not `MCPServerConfig.headers`). +1. Positional `[target...]` (command/URL) and/or `--catalog`/`--config` + `--server`, plus `-e`, `--cwd`, `--transport`, `--server-url`, `--header`. +2. `loadServerEntries(serverOptions)` in `core/mcp/node/servers.ts` — applies the default catalog path when appropriate and lifts disk settings; `selectServerEntry(entries, --server)` picks the single server to connect. +3. Disk `headers`/timeouts/OAuth (post-#1358 flat entry shape) and ad-hoc `--header` pairs both map to `InspectorServerSettings` and pass into `InspectorClient` via `serverSettings` (not `MCPServerConfig.headers`). A launch-time `--header` overrides the file's headers. -**Gap — config-file settings:** when loading from a config file, CLI uses `loadServerFromConfig()` / bare `MCPServerConfig` only. Persisted `headers`, timeouts, and OAuth fields on disk (post-#1358 flat entry shape) are **not** lifted into `serverSettings`. TUI uses `mcpConfigToServerEntries()` and does lift them. Ad-hoc `--header` on CLI works; file-backed settings do not. +**Config-file settings (resolved #1482):** the CLI now routes file-sourced servers through the same `loadServerEntries()` → `mcpConfigToServerEntries()` path as the TUI, so persisted `headers`, timeouts, and OAuth are lifted into `serverSettings`. (Previously the CLI used bare `MCPServerConfig` and dropped them.) **Tests:** 86 cases in `clients/cli/__tests__/` spawn `node build/index.js` via `cli-runner.ts` (`pretest` builds test-servers + CLI). Core behavior is exercised; Vitest coverage on `src/cli.ts` is invisible to the coverage instrumenter because tests run out-of-process (see [Known gaps](#known-gaps)). @@ -116,10 +116,10 @@ All three clients import from `@inspector/core/...` (mapped to `../../core/` sou **Entry:** `clients/tui/index.ts` exports `runTui(argv)`; `tui.tsx` parses argv and renders `App`. -**Server resolution:** `loadTuiServers()` in `clients/tui/src/tui-servers.ts`: +**Server resolution:** `loadTuiServers()` in `clients/tui/src/tui-servers.ts` — a thin re-export of the shared `loadServerEntries()` in `core/mcp/node/servers.ts`: - **Config file path:** reads JSON, runs `mcpConfigToServerEntries()` — returns `{ config, settings }` per server (correct post-#1358 path; matches web `useServers`). -- **Catalog file:** applies `withDefaultConfigPath()` before reading the file (default catalog when no `--config` or ad-hoc target). +- **Catalog file:** applies `withDefaultCatalogPath()` before reading the file (default catalog when no `--catalog`/`--config` or ad-hoc target). - **Ad-hoc:** `resolveServerConfigs(..., "multi")` for a single inline server (default catalog already applied above); merges launch-time `--header` into `settings`. **Core hooks:** TUI uses the same managed-state and `useInspectorClient` patterns as web (`ManagedToolsState`, etc.) inside Ink components. @@ -151,15 +151,15 @@ Day-to-day web development still uses `cd clients/web && npm run dev` (Vite CLI CLI, TUI, and `runWeb` share launch-time server options documented in v1.5's `docs/mcp-server-configuration.md` (port to v2 pending). Common flags: -| Flag | Purpose | -|------|---------| -| `--config ` | MCP servers JSON file | -| `--server ` | Named entry within config (required when file has multiple servers) | -| `[target...]` | Ad-hoc stdio command/args or HTTP URL | -| `-e KEY=VALUE` | Stdio environment overrides | -| `--cwd ` | Stdio working directory | -| `--transport` | `stdio`, `sse`, or `http` | -| `--server-url ` | HTTP/SSE endpoint | +| Flag | Purpose | +| ------------------------ | -------------------------------------------------------------------------------- | +| `--config ` | MCP servers JSON file | +| `--server ` | Named entry within config (required when file has multiple servers) | +| `[target...]` | Ad-hoc stdio command/args or HTTP URL | +| `-e KEY=VALUE` | Stdio environment overrides | +| `--cwd ` | Stdio working directory | +| `--transport` | `stdio`, `sse`, or `http` | +| `--server-url ` | HTTP/SSE endpoint | | `--header "Name: Value"` | Launch-time HTTP headers (CLI/TUI ad-hoc → `serverSettings`; web → warning only) | CLI additionally requires `--method` and method-specific args (`--tool-name`, `--uri`, etc.). Semantics for catalog vs session config, `--catalog`, and `servers/import` are in [v2_catalog_launch_config.md](v2_catalog_launch_config.md). @@ -168,12 +168,12 @@ CLI additionally requires `--method` and method-specific args (`--tool-name`, `- ## Testing and CI -| Suite | Location | Count | How | -|-------|----------|-------|-----| -| CLI | `clients/cli/__tests__/` | 86 | Subprocess `node build/index.js` | -| TUI | `clients/tui/__tests__/` | 2 | In-process imports | -| Launcher | CI smoke only | 3 | `--help`, `--cli --help`, `--tui --help` | -| Core | `clients/web/src/test/` | — | Canonical; not duplicated in cli/tui | +| Suite | Location | Count | How | +| -------- | ------------------------ | ----- | ---------------------------------------- | +| CLI | `clients/cli/__tests__/` | 86 | Subprocess `node build/index.js` | +| TUI | `clients/tui/__tests__/` | 2 | In-process imports | +| Launcher | CI smoke only | 3 | `--help`, `--cli --help`, `--tui --help` | +| Core | `clients/web/src/test/` | — | Canonical; not duplicated in cli/tui | **Root orchestration:** `npm run test` chains web unit tests, then `clients/cli` and `clients/tui` tests. `npm run test:coverage` runs web coverage only. @@ -210,17 +210,17 @@ Root `npm run validate` currently runs web validate only; extending it to CLI/TU ## Known gaps -| Area | Gap | Tracking | -|------|-----|----------| -| **CLI config-file settings** | Disk `headers` / timeouts / OAuth not lifted when `--config` is used | This spec; [catalog doc](v2_catalog_launch_config.md) G1 | -| **Web launch headers** | `runWeb --header` warns but does not apply settings to UI | This spec; [#1246](https://github.com/modelcontextprotocol/inspector/issues/1246) | -| **TUI list-changed UX** | No stale-list indicator when `autoRefreshOnListChanged` is false | [#1402](https://github.com/modelcontextprotocol/inspector/issues/1402) | -| **TUI test coverage** | Only `tabsConfig` shape tested | Expand `tui-servers.ts`, Ink flows | -| **CLI coverage gates** | Subprocess tests don't instrument `src/cli.ts` | In-process `runCli()` runner + thin binary E2E suite | -| **Prod web via launcher** | Requires pre-built `clients/web/dist/` | Document or wire into launcher prebuild | -| **Import / `--catalog`** | `servers/import`, `--catalog` not implemented | [catalog doc](v2_catalog_launch_config.md); [#1348](https://github.com/modelcontextprotocol/inspector/issues/1348) | -| **README refresh** | Client READMEs may lag v2 install/build commands | `clients/*/README.md`, `AGENTS.md` | -| **Root validate** | Does not build/test CLI/TUI | Root `package.json` | +| Area | Gap | Tracking | +| ---------------------------- | -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ | +| **CLI config-file settings** | Disk `headers` / timeouts / OAuth not lifted when `--config` is used | This spec; [catalog doc](v2_catalog_launch_config.md) G1 | +| **Web launch headers** | `runWeb --header` warns but does not apply settings to UI | This spec; [#1246](https://github.com/modelcontextprotocol/inspector/issues/1246) | +| **TUI list-changed UX** | No stale-list indicator when `autoRefreshOnListChanged` is false | [#1402](https://github.com/modelcontextprotocol/inspector/issues/1402) | +| **TUI test coverage** | Only `tabsConfig` shape tested | Expand `tui-servers.ts`, Ink flows | +| **CLI coverage gates** | Subprocess tests don't instrument `src/cli.ts` | In-process `runCli()` runner + thin binary E2E suite | +| **Prod web via launcher** | Requires pre-built `clients/web/dist/` | Document or wire into launcher prebuild | +| **Import / `--catalog`** | `servers/import`, `--catalog` not implemented | [catalog doc](v2_catalog_launch_config.md); [#1348](https://github.com/modelcontextprotocol/inspector/issues/1348) | +| **README refresh** | Client READMEs may lag v2 install/build commands | `clients/*/README.md`, `AGENTS.md` | +| **Root validate** | Does not build/test CLI/TUI | Root `package.json` | --- @@ -228,9 +228,10 @@ Root `npm run validate` currently runs web validate only; extending it to CLI/TU - `clients/launcher/src/index.ts` — mode routing and dynamic import - `clients/cli/src/cli.ts` — CLI parsing, `InspectorClient` one-shot flow -- `clients/tui/src/tui-servers.ts` — config + settings load path (correct for v2) +- `clients/tui/src/tui-servers.ts` — thin re-export of the shared `loadServerEntries` - `clients/web/server/run-web.ts` — launcher web entry -- `core/mcp/node/config.ts` — `resolveLaunchServerConfigs`, `resolveServerConfigs`, `withDefaultConfigPath` +- `core/mcp/node/servers.ts` — `loadServerEntries`, `selectServerEntry` (shared CLI/TUI config + settings load path) +- `core/mcp/node/config.ts` — `resolveServerConfigs`, `withDefaultCatalogPath`, `serverSourceConflict` - `core/mcp/serverList.ts` — `mcpConfigToServerEntries` - `vitest.shared.mts` — shared Vitest aliases - [v2_catalog_launch_config.md](v2_catalog_launch_config.md) — catalog and launch config, import, UC1–UC5