From 73b651ff21be01d32890a7de1f5fd712964293ff Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Fri, 19 Jun 2026 17:30:26 -0400 Subject: [PATCH] feat(cli,tui): normalize --catalog / --config with web; update --help (#1347) (#1499) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli,tui): normalize --catalog / --config with web; update --help (#1347) Bring the CLI and TUI in line with the web catalog vs read-only session model introduced in #1481/#1483, and close the outstanding --help item from #1347. Core (core/mcp/node/config.ts): - Add `catalogPath` to ServerConfigOptions (writable catalog slot). - `resolveServerSource()` picks the active source: `--catalog` (writable, seed-if-missing) wins over `--config` (read-only, error-if-missing). - Writable sources seed an empty `{"mcpServers":{}}` on first run (matches the web backend) instead of erroring; read-only `--config` still errors. - `serverSourceConflict()` shared conflict matrix (mirrors run-web): --catalog/--config mutually exclusive; neither combines with an ad-hoc target. - `readServerListFile()` public loader for runners (TUI) needing the raw map. - Rename `withDefaultConfigPath` → `withDefaultCatalogPath`; the no-flag default now fills the writable catalog slot, not --config. CLI / TUI: - Add `--catalog ` (+ MCP_CATALOG_PATH) and re-document `--config` as a read-only session in --help. - Enforce the shared conflict matrix before resolution. Full writable persistence/CRUD threading remains deferred to #1482 / #1432 (CLI/TUI are read consumers today; the split surfaces as seed- vs error-on-missing). Tests: core resolution matrix (source/conflict/seed/read-only), CLI catalog + conflict integration tests, TUI loadTuiServers unit tests. READMEs updated. Co-Authored-By: Claude Opus 4.8 (1M context) * test(launcher): add smoke:cli + smoke:tui e2e launcher smokes (#1347) The launcher smokes only checked `--help` for --cli/--tui; nothing exercised the launcher → CLI/TUI → core path on the built artifact (only smoke:web did, for web). Add two end-to-end smokes wired into validate:launcher between smoke:launcher and smoke:web: - smoke:cli (scripts/smoke-cli.mjs): runs `--cli` against the bundled stdio test server via a temp --catalog and asserts tools/list returns the tools, plus the #1347 resolution paths — default-catalog seed-on-missing, read-only --config error-without-seed, and the --catalog/--config conflict. - smoke:tui (scripts/smoke-tui.mjs): launches `--tui --catalog ` and asserts the Ink app renders its first frame ("MCP Servers") within a timeout, then SIGTERMs it (shallow boot/render check). Both build test-servers/build on demand if missing. Docs updated (AGENTS.md validate description, launcher README). Co-Authored-By: Claude Opus 4.8 (1M context) * refactor: address review on #1499 (comment accuracy, CLI/TUI flag parity) Review feedback from @claude on PR #1499 (all non-blocking): - Align CLI catalogPath resolution with the TUI: `options.catalog?.trim() || process.env.MCP_CATALOG_PATH` so an explicit empty `--catalog ""` falls back to MCP_CATALOG_PATH identically in both front-ends (CLI used `??`, which only guards undefined/null). - Clarify the `serverSourceConflict` comment: it mirrors only the source-selection portion of web's matrix and deliberately omits web's `--header` rejection, since CLI/TUI merge --header into per-server settings. - Note the deliberate --header broadcast semantics in loadTuiServers. - Share an EMPTY_CATALOG_CONTENT constant between seedEmptyCatalog and readServerListContent instead of two separate literals. Co-Authored-By: Claude Opus 4.8 (1M context) * refactor(cli): trim --config to match --catalog and the TUI (#1499 review) Cosmetic uniformity follow-up from the re-review: configPath now uses `options.config?.trim() || undefined`, matching the catalog slot above it and the TUI. Behavior is unchanged (resolveServerSource/serverSourceConflict already trim downstream). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- AGENTS.md | 3 +- clients/cli/README.md | 2 +- clients/cli/__tests__/cli.test.ts | 67 +++++ clients/cli/src/cli.ts | 28 ++- clients/launcher/README.md | 48 +++- clients/tui/README.md | 9 +- clients/tui/__tests__/tui-servers.test.ts | 102 ++++++++ clients/tui/src/tui-servers.ts | 34 ++- clients/tui/tui.tsx | 8 +- .../web/src/test/core/mcp/node/config.test.ts | 234 +++++++++++++++++- core/mcp/node/config.ts | 193 ++++++++++++--- core/mcp/node/index.ts | 7 +- package.json | 4 +- scripts/smoke-cli.mjs | 170 +++++++++++++ scripts/smoke-tui.mjs | 134 ++++++++++ 15 files changed, 962 insertions(+), 81 deletions(-) create mode 100644 clients/tui/__tests__/tui-servers.test.ts create mode 100644 scripts/smoke-cli.mjs create mode 100644 scripts/smoke-tui.mjs diff --git a/AGENTS.md b/AGENTS.md index 0d81de80..214c120d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,8 +134,9 @@ All work should be driven by items on the project board. ### Lint-fixed, Formatted code - ALWAYS do `npm run format` before committing — it auto-fixes any Prettier issues. `validate` runs `format:check` (the non-fixing variant) and will fail in CI on any unformatted file, so always run the auto-fixer first rather than letting `format:check` catch it. -- ALWAYS do `npm run validate` before pushing any changes — from the repo root it mirrors what CI enforces across every client: web `format:check` + `lint` + `build` + `test:coverage` (`validate:web`), then the CLI and TUI test suites (`test:cli` / `test:tui`), then a build of CLI + TUI + launcher, the launcher `--help` smoke (`smoke:launcher`), and the prod web end-to-end smoke (`smoke:web`) — together `validate:launcher`. The sub-scripts (`validate:web`, `validate:launcher`, `smoke:launcher`, `smoke:web`) can be run individually when iterating on one client. Storybook is the only CI step left out (see below). +- ALWAYS do `npm run validate` before pushing any changes — from the repo root it mirrors what CI enforces across every client: web `format:check` + `lint` + `build` + `test:coverage` (`validate:web`), then the CLI and TUI test suites (`test:cli` / `test:tui`), then a build of CLI + TUI + launcher, the launcher `--help` smoke (`smoke:launcher`), the prod CLI and TUI end-to-end smokes (`smoke:cli` / `smoke:tui`), and the prod web end-to-end smoke (`smoke:web`) — together `validate:launcher`. The sub-scripts (`validate:web`, `validate:launcher`, `smoke:launcher`, `smoke:cli`, `smoke:tui`, `smoke:web`) can be run individually when iterating on one client. Storybook is the only CI step left out (see below). - `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`. +- `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog ` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. Both build `test-servers/build` on demand if it's missing. - Also run `npm run test:storybook` before pushing — it executes every story's `play` function in headless Chromium via `@vitest/browser-playwright` (~10s). CI runs this as a separate step after the unit/lint/build checks; failures block merge. It is kept out of `validate` because it needs the Playwright browser binary and is much slower than the unit suite. ### Typescript instructions diff --git a/clients/cli/README.md b/clients/cli/README.md index fd3d8f87..d8b753a4 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -82,7 +82,7 @@ npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --tr ### MCP server (which server to connect to) -Options that specify the MCP server (config file, ad-hoc command/URL, env vars, headers) are shared by the Web, CLI, and TUI and are documented in [MCP server configuration](../../docs/mcp-server-configuration.md): `--config`, `--server`, `-e`, `--cwd`, `--header`, `--transport`, `--server-url`, and the positional `[target...]`. +Options that specify the MCP server (catalog/config file, ad-hoc command/URL, env vars, headers) are shared by the Web, CLI, and TUI and are documented in [MCP server configuration](../../docs/mcp-server-configuration.md): `--catalog` (writable catalog, seeded if missing; default `~/.mcp-inspector/mcp.json` or `MCP_CATALOG_PATH`), `--config` (read-only session, errors if absent), `--server`, `-e`, `--cwd`, `--header`, `--transport`, `--server-url`, and the positional `[target...]`. `--catalog` and `--config` are mutually exclusive, and neither combines with an ad-hoc target. ### CLI-specific (what to invoke) diff --git a/clients/cli/__tests__/cli.test.ts b/clients/cli/__tests__/cli.test.ts index c7ea6674..94da83f9 100644 --- a/clients/cli/__tests__/cli.test.ts +++ b/clients/cli/__tests__/cli.test.ts @@ -253,6 +253,73 @@ describe("CLI Tests", () => { }); }); + describe("Catalog File", () => { + it("should use a writable catalog with CLI mode", async () => { + const catalogPath = createSampleTestConfig(); + try { + const result = await runCli([ + "--catalog", + catalogPath, + "--server", + "test-stdio", + "--cli", + "--method", + "tools/list", + ]); + + expectCliSuccess(result); + const json = expectValidJson(result); + expect(json).toHaveProperty("tools"); + expect(Array.isArray(json.tools)).toBe(true); + } finally { + deleteConfigFile(catalogPath); + } + }); + + it("should fail when --catalog and --config are combined", async () => { + const catalogPath = createSampleTestConfig(); + try { + const result = await runCli([ + "--catalog", + catalogPath, + "--config", + catalogPath, + "--server", + "test-stdio", + "--cli", + "--method", + "tools/list", + ]); + + expectCliFailure(result); + expect(result.stderr).toMatch(/mutually exclusive/); + } finally { + deleteConfigFile(catalogPath); + } + }); + + it("should fail when --catalog is combined with an ad-hoc target", async () => { + const catalogPath = createSampleTestConfig(); + const { command, args } = getTestMcpServerCommand(); + try { + const result = await runCli([ + command, + ...args, + "--catalog", + catalogPath, + "--cli", + "--method", + "tools/list", + ]); + + expectCliFailure(result); + expect(result.stderr).toMatch(/--catalog cannot be combined/); + } finally { + deleteConfigFile(catalogPath); + } + }); + }); + 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 127d4c51..f648745f 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -19,6 +19,8 @@ import { import { createTransportNode, resolveLaunchServerConfigs, + serverSourceConflict, + hasAdHocServerOptions, parseKeyValuePair as parseEnvPair, parseHeaderPair, } from "@inspector/core/mcp/node/index.js"; @@ -288,8 +290,15 @@ function parseArgs(argv?: string[]): { "[target...]", "Command and arguments or URL of the MCP server (or use --config and --server)", ) - .option("--config ", "Config file path") - .option("--server ", "Server name from config file") + .option( + "--catalog ", + "Writable catalog file (created if missing; default: ~/.mcp-inspector/mcp.json, or MCP_CATALOG_PATH)", + ) + .option( + "--config ", + "Read-only session config file (served as-is, never written or seeded; errors if absent)", + ) + .option("--server ", "Server name from config/catalog file") .option( "-e ", "Environment variables for the server (KEY=VALUE)", @@ -364,6 +373,7 @@ function parseArgs(argv?: string[]): { program.parse(preArgs); const options = program.opts() as { + catalog?: string; config?: string; server?: string; e?: Record; @@ -383,7 +393,10 @@ function parseArgs(argv?: string[]): { }; const serverOptions = { - configPath: options.config, + // `?.trim() ||` (not `??`) so an explicit empty `--catalog ""` still falls + // back to MCP_CATALOG_PATH — keeps CLI and TUI flag resolution identical. + catalogPath: options.catalog?.trim() || process.env.MCP_CATALOG_PATH, + configPath: options.config?.trim() || undefined, serverName: options.server, target: targetArgs.length > 0 ? targetArgs : undefined, transport: options.transport, @@ -392,6 +405,15 @@ function parseArgs(argv?: string[]): { env: options.e, }; + 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) { diff --git a/clients/launcher/README.md b/clients/launcher/README.md index 490139d0..78a90e88 100644 --- a/clients/launcher/README.md +++ b/clients/launcher/README.md @@ -15,17 +15,36 @@ All configuration parsing, config-file loading, and server setup are handled by `mcp-inspector --web` chooses which server list the UI shows and whether it is editable (see [specification/v2_catalog_launch_config.md](../../specification/v2_catalog_launch_config.md)): -| Invocation | Server list | Editable in UI? | -|------------|-------------|-----------------| -| `mcp-inspector --web` | Default catalog `~/.mcp-inspector/mcp.json` | Yes | -| `mcp-inspector --web --catalog ` (or `MCP_CATALOG_PATH=`) | That file as the active catalog (created/seeded if missing) | Yes | -| `mcp-inspector --web --config ` | That file as a **read-only session** — shown but never written, seeded, or migrated (safe for a foreign config) | No | -| `mcp-inspector --web --server-url --transport http --header "Name: Value"` (or a positional command) | One ad-hoc server held in memory, connectable with the given `--header`s | No | +| Invocation | Server list | Editable in UI? | +| ---------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- | --------------- | +| `mcp-inspector --web` | Default catalog `~/.mcp-inspector/mcp.json` | Yes | +| `mcp-inspector --web --catalog ` (or `MCP_CATALOG_PATH=`) | That file as the active catalog (created/seeded if missing) | Yes | +| `mcp-inspector --web --config ` | That file as a **read-only session** — shown but never written, seeded, or migrated (safe for a foreign config) | No | +| `mcp-inspector --web --server-url --transport http --header "Name: Value"` (or a positional command) | One ad-hoc server held in memory, connectable with the given `--header`s | No | Rules: `--catalog` and `--config` are mutually exclusive; neither combines with an ad-hoc target or `--header`; `--header` requires an ad-hoc HTTP/SSE server and is applied to that connection (it is no longer a warn-only no-op). +## CLI and TUI server-list flags (`--cli` / `--tui`) + +The CLI and TUI use the same `--catalog` / `--config` vocabulary as `--web`, +resolved by the shared `core/mcp/node/config.ts` helpers: + +| Invocation | Server list source | +| ------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------- | +| `mcp-inspector --cli` / `--tui` (no source flag, no ad-hoc target) | Default writable catalog `~/.mcp-inspector/mcp.json` (created/seeded empty if missing) | +| `--catalog ` (or `MCP_CATALOG_PATH=`) | That file as a **writable catalog** — seeded empty if missing | +| `--config ` | That file as a **read-only session** — served as-is, never written or seeded; **errors if absent** | +| positional command / `--server-url ` | One ad-hoc server | + +Rules (shared `serverSourceConflict`): `--catalog` and `--config` are mutually +exclusive, and neither combines with an ad-hoc command/URL target. The CLI/TUI +do not perform catalog CRUD yet — they are read consumers — so the +writable/read-only split currently surfaces only as **seed-if-missing** +(`--catalog`/default) vs **error-if-missing** (`--config`). Full writable +persistence is tracked in #1482 / #1432. + ### Production web build (`--web`, no `--dev`) Prod `--web` serves static assets from `clients/web/dist/`, which only exists @@ -42,6 +61,23 @@ end-to-end with `npm run smoke:web` (`scripts/smoke-web.mjs`): it starts `mcp-inspector --web` against the built `dist/` and asserts `GET /` returns the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. +### CLI and TUI smokes + +`validate:launcher` also runs end-to-end smokes for the other two modes through +the built launcher artifact (beyond the `--help` checks in `smoke:launcher`): + +- `npm run smoke:cli` (`scripts/smoke-cli.mjs`) — runs `mcp-inspector --cli` + against the bundled stdio test server via a temp `--catalog` and asserts + `tools/list` returns the server's tools, plus the `--catalog`/`--config` + resolution paths (default-catalog seed-on-missing, read-only `--config` + error-without-seed, `--catalog`/`--config` conflict). +- `npm run smoke:tui` (`scripts/smoke-tui.mjs`) — launches + `mcp-inspector --tui --catalog ` and asserts the Ink app renders its + first frame within a timeout, then shuts it down (a shallow boot/render + check, not full interaction). + +Both build `test-servers/build` on demand if it is missing. + ## Publishing The root `@modelcontextprotocol/inspector` package ships as one fat tarball: `npm run build` at the repo root builds all clients, then `prepack` runs before `npm publish`. Runtime dependencies are declared on the root `package.json`; client builds bundle `@inspector/core` and externalize npm packages resolved from the root install. diff --git a/clients/tui/README.md b/clients/tui/README.md index e050f9b4..41f5841e 100644 --- a/clients/tui/README.md +++ b/clients/tui/README.md @@ -14,10 +14,13 @@ npx @modelcontextprotocol/inspector --tui node build/index.js ### With Configuration Files -The TUI can load all servers from an MCP config file: +The TUI can load all servers from an MCP catalog/config file. With no source +flag it uses the default writable catalog `~/.mcp-inspector/mcp.json` (seeded +empty if missing): ```bash -npx @modelcontextprotocol/inspector --tui --config mcp.json +npx @modelcontextprotocol/inspector --tui --catalog mcp.json # writable catalog (seeded if missing) +npx @modelcontextprotocol/inspector --tui --config mcp.json # read-only session (errors if absent) ``` (It does not use `--server`; all servers in the file are available in the TUI.) @@ -26,7 +29,7 @@ npx @modelcontextprotocol/inspector --tui --config mcp.json ### MCP server (which server(s) to connect to) -Options that specify the MCP server(s) (config file, ad-hoc command/URL, env vars, headers) are shared by the Web, CLI, and TUI and are documented in [MCP server configuration](../../docs/mcp-server-configuration.md): `--config`, `-e`, `--cwd`, `--header`, `--transport`, `--server-url`, and the positional `[target...]`. +Options that specify the MCP server(s) (catalog/config file, ad-hoc command/URL, env vars, headers) are shared by the Web, CLI, and TUI and are documented in [MCP server configuration](../../docs/mcp-server-configuration.md): `--catalog` (writable catalog, seeded if missing; default `~/.mcp-inspector/mcp.json` or `MCP_CATALOG_PATH`), `--config` (read-only session, errors if absent), `-e`, `--cwd`, `--header`, `--transport`, `--server-url`, and the positional `[target...]`. `--catalog` and `--config` are mutually exclusive, and neither combines with an ad-hoc target. ### TUI-specific (OAuth for HTTP servers) diff --git a/clients/tui/__tests__/tui-servers.test.ts b/clients/tui/__tests__/tui-servers.test.ts new file mode 100644 index 00000000..722e2eb9 --- /dev/null +++ b/clients/tui/__tests__/tui-servers.test.ts @@ -0,0 +1,102 @@ +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 { loadTuiServers } from "../src/tui-servers.js"; + +describe("loadTuiServers", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "tui-servers-test-")); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + it("loads named servers from a read-only --config file", () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { foo: { command: "node", args: ["foo.js"] } }, + }), + ); + const servers = loadTuiServers({ configPath }); + expect(Object.keys(servers)).toEqual(["foo"]); + expect(servers.foo?.config).toMatchObject({ + type: "stdio", + command: "node", + }); + }); + + it("seeds an empty writable catalog when --catalog is missing", () => { + const catalogPath = join(tempDir, "catalog.json"); + const servers = loadTuiServers({ 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(() => loadTuiServers({ 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(() => loadTuiServers({ 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(() => + loadTuiServers({ catalogPath, target: ["my-server"] }), + ).toThrow(/--catalog cannot be combined/); + }); + + it("builds a single ad-hoc server from a positional target", () => { + const servers = loadTuiServers({ target: ["my-server", "--flag"] }); + expect(Object.keys(servers)).toEqual(["default"]); + expect(servers.default?.config).toMatchObject({ + type: "stdio", + command: "my-server", + args: ["--flag"], + }); + }); + + it("merges --header into per-server settings for catalog servers", () => { + const catalogPath = join(tempDir, "catalog.json"); + writeFileSync( + catalogPath, + JSON.stringify({ + mcpServers: { web: { type: "streamable-http", url: "http://x/mcp" } }, + }), + ); + const servers = loadTuiServers({ + catalogPath, + headers: { Authorization: "Bearer t" }, + }); + expect(servers.web?.settings?.headers).toEqual([ + { key: "Authorization", value: "Bearer t" }, + ]); + }); +}); diff --git a/clients/tui/src/tui-servers.ts b/clients/tui/src/tui-servers.ts index 8af17fdf..c0e48bac 100644 --- a/clients/tui/src/tui-servers.ts +++ b/clients/tui/src/tui-servers.ts @@ -1,16 +1,17 @@ -import { readFileSync } from "node:fs"; -import { resolve } from "node:path"; import type { InspectorServerSettings, MCPServerConfig, - MCPConfig, 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, - withDefaultConfigPath, + resolveServerSource, + serverSourceConflict, + readServerListFile, + hasAdHocServerOptions, + withDefaultCatalogPath, type ServerConfigOptions, } from "@inspector/core/mcp/node/config.js"; @@ -64,13 +65,21 @@ function mergeSettings( export function loadTuiServers( serverOptions: ServerConfigOptions & { headers?: Record }, ): Record { - serverOptions = withDefaultConfigPath(serverOptions); - const hasConfigPath = Boolean(serverOptions.configPath?.trim()); + serverOptions = withDefaultCatalogPath(serverOptions); - if (hasConfigPath) { - const configPath = serverOptions.configPath!; - const resolvedPath = resolve(process.cwd(), configPath); - const config = JSON.parse(readFileSync(resolvedPath, "utf-8")) as MCPConfig; + 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) { @@ -79,6 +88,9 @@ export function loadTuiServers( 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), }; } @@ -88,7 +100,7 @@ export function loadTuiServers( const configs = resolveServerConfigs(serverOptions, "multi"); if (configs.length === 0) { throw new Error( - "At least one server is required. Use --config or ad-hoc target (command/URL).", + "At least one server is required. Use --catalog/--config or an ad-hoc target (command/URL).", ); } return { diff --git a/clients/tui/tui.tsx b/clients/tui/tui.tsx index bc1628e8..b9a27b1e 100644 --- a/clients/tui/tui.tsx +++ b/clients/tui/tui.tsx @@ -15,9 +15,13 @@ export async function runTui(args?: string[]): Promise { program .name("mcp-inspector-tui") .description("Terminal UI for MCP Inspector") + .option( + "--catalog ", + "Writable catalog file (created if missing; default: ~/.mcp-inspector/mcp.json, or MCP_CATALOG_PATH)", + ) .option( "--config ", - "Path to MCP servers config file (or use ad-hoc server options below)", + "Read-only session config file (served as-is, never written or seeded; errors if absent)", ) .option( "-e ", @@ -60,6 +64,7 @@ export async function runTui(args?: string[]): Promise { .parse(args ?? process.argv); const options = program.opts() as { + catalog?: string; config?: string; e?: Record; cwd?: string; @@ -74,6 +79,7 @@ export async function runTui(args?: string[]): Promise { const targetArgs = program.args as string[]; const serverOptions = { + catalogPath: options.catalog?.trim() || process.env.MCP_CATALOG_PATH, configPath: options.config?.trim() || undefined, target: targetArgs.length > 0 ? targetArgs : undefined, cwd: options.cwd?.trim() || undefined, 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 447cf7e1..7a1fa12c 100644 --- a/clients/web/src/test/core/mcp/node/config.test.ts +++ b/clients/web/src/test/core/mcp/node/config.test.ts @@ -1,13 +1,23 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdirSync, mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + writeFileSync, + rmSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { parseKeyValuePair, parseHeaderPair, - withDefaultConfigPath, + withDefaultCatalogPath, resolveServerConfigs, resolveLaunchServerConfigs, + resolveServerSource, + serverSourceConflict, + readServerListFile, getNamedServerConfigs, } from "@inspector/core/mcp/node/config.js"; @@ -59,7 +69,99 @@ describe("parseHeaderPair", () => { }); }); -describe("withDefaultConfigPath", () => { +describe("resolveServerSource", () => { + it("returns the writable catalog source when catalogPath is set", () => { + expect(resolveServerSource({ catalogPath: "/tmp/cat.json" })).toEqual({ + path: "/tmp/cat.json", + writable: true, + }); + }); + + it("returns the read-only config source when only configPath is set", () => { + expect(resolveServerSource({ configPath: "/tmp/cfg.json" })).toEqual({ + path: "/tmp/cfg.json", + writable: false, + }); + }); + + it("prefers the catalog over the config when both are set", () => { + expect( + resolveServerSource({ + catalogPath: "/tmp/cat.json", + configPath: "/tmp/cfg.json", + }), + ).toEqual({ path: "/tmp/cat.json", writable: true }); + }); + + it("returns null when neither catalog nor config is set", () => { + expect(resolveServerSource({ target: ["cmd"] })).toBeNull(); + expect(resolveServerSource({})).toBeNull(); + }); + + it("ignores blank/whitespace-only paths", () => { + expect( + resolveServerSource({ catalogPath: " ", configPath: " " }), + ).toBeNull(); + }); +}); + +describe("serverSourceConflict", () => { + it("rejects --catalog and --config together", () => { + expect( + serverSourceConflict({ + hasCatalog: true, + hasConfig: true, + hasAdHoc: false, + }), + ).toMatch(/mutually exclusive/); + }); + + it("rejects --catalog with an ad-hoc target", () => { + expect( + serverSourceConflict({ + hasCatalog: true, + hasConfig: false, + hasAdHoc: true, + }), + ).toMatch(/--catalog cannot be combined/); + }); + + it("rejects --config with an ad-hoc target", () => { + expect( + serverSourceConflict({ + hasCatalog: false, + hasConfig: true, + hasAdHoc: true, + }), + ).toMatch(/--config cannot be combined/); + }); + + it("allows a lone catalog, a lone config, or a lone ad-hoc target", () => { + expect( + serverSourceConflict({ + hasCatalog: true, + hasConfig: false, + hasAdHoc: false, + }), + ).toBeNull(); + expect( + serverSourceConflict({ + hasCatalog: false, + hasConfig: true, + hasAdHoc: false, + }), + ).toBeNull(); + expect( + serverSourceConflict({ + hasCatalog: false, + hasConfig: false, + hasAdHoc: true, + }), + ).toBeNull(); + }); +}); + +describe("withDefaultCatalogPath", () => { let tempDir: string; beforeEach(() => { @@ -70,31 +172,77 @@ describe("withDefaultConfigPath", () => { rmSync(tempDir, { recursive: true, force: true }); }); - it("leaves options unchanged when --config or ad-hoc target is present", () => { - expect(withDefaultConfigPath({ configPath: "/tmp/mcp.json" })).toEqual({ + it("leaves options unchanged when --catalog, --config, or ad-hoc target is present", () => { + expect(withDefaultCatalogPath({ catalogPath: "/tmp/cat.json" })).toEqual({ + catalogPath: "/tmp/cat.json", + }); + expect(withDefaultCatalogPath({ configPath: "/tmp/mcp.json" })).toEqual({ configPath: "/tmp/mcp.json", }); - expect(withDefaultConfigPath({ target: ["node", "server.js"] })).toEqual({ + expect(withDefaultCatalogPath({ target: ["node", "server.js"] })).toEqual({ target: ["node", "server.js"], }); }); - it("injects the default catalog path when no config or ad-hoc target is given", () => { + it("injects the default catalog path into the writable catalog slot when nothing is given", () => { const prevHome = process.env.HOME; const homeDir = join(tempDir, "home"); mkdirSync(homeDir, { recursive: true }); process.env.HOME = homeDir; try { - const options = withDefaultConfigPath({}); - expect(options.configPath).toBe( + const options = withDefaultCatalogPath({}); + expect(options.catalogPath).toBe( join(homeDir, ".mcp-inspector", "mcp.json"), ); + expect(options.configPath).toBeUndefined(); } finally { process.env.HOME = prevHome; } }); }); +describe("readServerListFile", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "inspector-config-test-")); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + it("seeds an empty catalog on disk when a writable source is missing", () => { + const catalogPath = join(tempDir, "nested", "mcp.json"); + const config = readServerListFile(catalogPath, true); + expect(config).toEqual({ mcpServers: {} }); + expect(existsSync(catalogPath)).toBe(true); + expect(JSON.parse(readFileSync(catalogPath, "utf-8"))).toEqual({ + mcpServers: {}, + }); + }); + + it("throws when a read-only source is missing (never seeds)", () => { + const configPath = join(tempDir, "absent.json"); + expect(() => readServerListFile(configPath, false)).toThrow( + /Config file not found/, + ); + expect(existsSync(configPath)).toBe(false); + }); + + it("reads and normalizes server types from an existing file", () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { a: { type: "http", url: "http://example.com/mcp" } }, + }), + ); + const config = readServerListFile(configPath, false); + expect(config.mcpServers.a).toMatchObject({ type: "streamable-http" }); + }); +}); + describe("resolveLaunchServerConfigs", () => { let tempDir: string; @@ -106,13 +254,37 @@ describe("resolveLaunchServerConfigs", () => { rmSync(tempDir, { recursive: true, force: true }); }); - it("uses the default catalog when no target or --config is given", () => { + it("seeds the default catalog and reports no servers when it is missing (single mode)", () => { const prevHome = process.env.HOME; - process.env.HOME = tempDir; + const homeDir = join(tempDir, "home"); + mkdirSync(homeDir, { recursive: true }); + process.env.HOME = homeDir; + const defaultConfig = join(homeDir, ".mcp-inspector", "mcp.json"); try { expect(() => resolveLaunchServerConfigs({ target: [] }, "single"), - ).toThrow(/Config file not found/); + ).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); + expect(JSON.parse(readFileSync(defaultConfig, "utf-8"))).toEqual({ + mcpServers: {}, + }); + } finally { + process.env.HOME = prevHome; + } + }); + + it("returns an empty list from a seeded default catalog (multi mode)", () => { + const prevHome = process.env.HOME; + const homeDir = join(tempDir, "home"); + mkdirSync(homeDir, { recursive: true }); + process.env.HOME = homeDir; + try { + expect(resolveLaunchServerConfigs({}, "multi")).toEqual([]); + expect(existsSync(join(homeDir, ".mcp-inspector", "mcp.json"))).toBe( + true, + ); } finally { process.env.HOME = prevHome; } @@ -306,7 +478,7 @@ describe("resolveServerConfigs — single mode", () => { ); }); - it("throws when the config file is missing", () => { + it("throws when the read-only config file is missing", () => { expect(() => resolveServerConfigs( { configPath: join(tempDir, "nonexistent.json") }, @@ -315,6 +487,27 @@ describe("resolveServerConfigs — single mode", () => { ).toThrow(/Config file not found/); }); + it("seeds a missing writable catalog and then reports no servers", () => { + const catalogPath = join(tempDir, "seeded.json"); + expect(() => resolveServerConfigs({ catalogPath }, "single")).toThrow( + /No servers found/, + ); + expect(existsSync(catalogPath)).toBe(true); + }); + + it("loads a named server from a writable catalog", () => { + const catalogPath = join(tempDir, "cat.json"); + writeFileSync( + catalogPath, + JSON.stringify({ mcpServers: { foo: { command: "node" } } }), + ); + const [config] = resolveServerConfigs( + { catalogPath, serverName: "foo" }, + "single", + ); + expect(config).toMatchObject({ type: "stdio", command: "node" }); + }); + it("throws when the config file has no mcpServers element", () => { writeFileSync(configPath, JSON.stringify({})); expect(() => resolveServerConfigs({ configPath }, "single")).toThrow( @@ -440,6 +633,21 @@ describe("resolveServerConfigs — multi mode", () => { expect(configs[0]?.type).toBe("stdio"); }); + it("returns all servers from a writable catalog, seeding it if missing", () => { + const catalogPath = join(tempDir, "cat.json"); + // Missing → seeded empty → no servers. + expect(resolveServerConfigs({ catalogPath }, "multi")).toEqual([]); + expect(existsSync(catalogPath)).toBe(true); + // Now populated → returned. + writeFileSync( + catalogPath, + JSON.stringify({ mcpServers: { a: { command: "a" } } }), + ); + const configs = resolveServerConfigs({ catalogPath }, "multi"); + expect(configs).toHaveLength(1); + expect(configs[0]?.type).toBe("stdio"); + }); + it("requires a target when no config path is given", () => { expect(() => resolveServerConfigs({}, "multi")).toThrow( /Target is required/, diff --git a/core/mcp/node/config.ts b/core/mcp/node/config.ts index b722f86a..442a85a5 100644 --- a/core/mcp/node/config.ts +++ b/core/mcp/node/config.ts @@ -1,5 +1,5 @@ -import { existsSync, readFileSync } from "fs"; -import { resolve } from "path"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; +import { dirname, resolve } from "path"; import { getDefaultMcpConfigPath } from "../../storage/store-io.js"; import type { MCPConfig, @@ -15,6 +15,17 @@ import { normalizeServerType } from "../serverList.js"; * Core exports this type so runners can type the subset they pass in. */ export interface ServerConfigOptions { + /** + * Writable catalog file (`--catalog` / `MCP_CATALOG_PATH`): seeded as an empty + * catalog if it doesn't exist, then served. This is the slot the no-flag + * default (`~/.mcp-inspector/mcp.json`) fills. Wins over `configPath`. + */ + catalogPath?: string; + /** + * Read-only session file (`--config`): served as-is and never written, seeded, + * or migrated. Errors if absent (so a typo'd path is a clear error, not a + * silently empty list). Used when `catalogPath` is unset. + */ configPath?: string; serverName?: string; /** Command + args for stdio, or [url] for SSE/HTTP. Positional / args after -- */ @@ -74,22 +85,53 @@ export function parseHeaderPair( return { ...previous, [key]: val }; } +/** On-disk contents of a freshly seeded empty catalog (pretty-printed). */ +const EMPTY_CATALOG_CONTENT = `${JSON.stringify({ mcpServers: {} }, null, 2)}\n`; + +/** + * Write an empty catalog (`{ "mcpServers": {} }`) to `resolvedPath`, creating + * parent directories. Seeds a writable catalog on first run so CLI/TUI match + * the web backend instead of erroring on a missing default file. + */ +function seedEmptyCatalog(resolvedPath: string): void { + mkdirSync(dirname(resolvedPath), { recursive: true }); + writeFileSync(resolvedPath, EMPTY_CATALOG_CONTENT, { mode: 0o600 }); +} + +/** + * Read the raw contents of a server-list file. A missing *writable* catalog is + * seeded as an empty catalog and its contents returned; a missing *read-only* + * config throws so a typo'd path is a clear error, not a silently empty list. + */ +function readServerListContent(configPath: string, writable: boolean): string { + const resolvedPath = resolve(process.cwd(), configPath); + if (!existsSync(resolvedPath)) { + if (writable) { + seedEmptyCatalog(resolvedPath); + return EMPTY_CATALOG_CONTENT; + } + throw new Error(`Config file not found: ${resolvedPath}`); + } + return readFileSync(resolvedPath, "utf-8"); +} + /** * Loads and validates an MCP servers configuration file. - * Checks file existence before reading. Normalizes each server's type - * (missing → "stdio", "http" → "streamable-http"). + * Seeds an empty catalog when `writable` and the file is missing; otherwise a + * missing file throws. Normalizes each server's type (missing → "stdio", + * "http" → "streamable-http"). * * @param configPath - Path to the config file (relative to process.cwd() or absolute) + * @param writable - true for a `--catalog`/default source (seed-if-missing); false for `--config` (error-if-missing) * @returns The parsed MCPConfig with normalized server types - * @throws Error if the file is missing, cannot be loaded, parsed, or is invalid + * @throws Error if a read-only file is missing, or any file cannot be loaded, parsed, or is invalid */ -function loadMcpServersConfig(configPath: string): MCPConfig { +function loadMcpServersConfig( + configPath: string, + writable: boolean, +): MCPConfig { try { - const resolvedPath = resolve(process.cwd(), configPath); - if (!existsSync(resolvedPath)) { - throw new Error(`Config file not found: ${resolvedPath}`); - } - const configContent = readFileSync(resolvedPath, "utf-8"); + const configContent = readServerListContent(configPath, writable); const config = JSON.parse(configContent) as MCPConfig; if (!config.mcpServers) { @@ -118,8 +160,9 @@ function loadMcpServersConfig(configPath: string): MCPConfig { function loadServerFromConfig( configPath: string, serverName: string, + writable: boolean, ): MCPServerConfig { - const config = loadMcpServersConfig(configPath); + const config = loadMcpServersConfig(configPath, writable); if (!config.mcpServers[serverName]) { const available = Object.keys(config.mcpServers).join(", "); throw new Error( @@ -220,14 +263,81 @@ export function hasAdHocServerOptions(options: ServerConfigOptions): boolean { ); } -/** When no --config and no ad-hoc target, use ~/.mcp-inspector/mcp.json (same as web). */ -export function withDefaultConfigPath( +/** + * Identify the active server-list source and whether it's writable. + * `--catalog` (writable: seeded if missing) wins over `--config` (read-only: + * served as-is, never seeded/written). Returns null when neither is set + * (ad-hoc / no source). + */ +export function resolveServerSource( + options: ServerConfigOptions, +): { path: string; writable: boolean } | null { + if (options.catalogPath?.trim()) { + return { path: options.catalogPath, writable: true }; + } + if (options.configPath?.trim()) { + return { path: options.configPath, writable: false }; + } + return null; +} + +export interface ServerSourceFlags { + hasCatalog: boolean; + hasConfig: boolean; + hasAdHoc: boolean; +} + +/** + * Validate the `--catalog` / `--config` / ad-hoc combination shared by all + * runners (mirrors the *source-selection* portion of the web `run-web` conflict + * matrix). It deliberately omits web's `--header` + `--catalog`/`--config` + * rejection: unlike web, the CLI/TUI merge `--header` into per-server settings + * (`headersToServerSettings` / `mergeSettings`), so headers are allowed + * alongside a catalog/config here. Returns an error message for an illegal + * combination, or null when the flags are coherent. + */ +export function serverSourceConflict(flags: ServerSourceFlags): string | null { + if (flags.hasCatalog && flags.hasConfig) { + return "--catalog and --config are mutually exclusive. --catalog is the writable catalog; --config is a read-only session file."; + } + if (flags.hasCatalog && flags.hasAdHoc) { + return "--catalog cannot be combined with an ad-hoc server URL/command."; + } + if (flags.hasConfig && flags.hasAdHoc) { + return "--config cannot be combined with an ad-hoc server URL/command. --config selects a read-only session file."; + } + return null; +} + +/** + * Public catalog/config loader for runners that need the raw server map (e.g. + * the TUI, which derives per-server settings via `mcpConfigToServerEntries`). + * Applies the same seed-if-writable / error-if-read-only semantics as + * `resolveServerConfigs`; server `type` fields are normalized. + */ +export function readServerListFile( + configPath: string, + writable: boolean, +): MCPConfig { + return loadMcpServersConfig(configPath, writable); +} + +/** + * When no `--catalog`, no `--config`, and no ad-hoc target is given, default the + * writable catalog to ~/.mcp-inspector/mcp.json (the same file the web backend + * uses). Fills the *catalog* (writable, seed-if-missing) slot — not `--config`. + */ +export function withDefaultCatalogPath( options: ServerConfigOptions, ): ServerConfigOptions { - if (options.configPath?.trim() || hasAdHocServerOptions(options)) { + if ( + options.catalogPath?.trim() || + options.configPath?.trim() || + hasAdHocServerOptions(options) + ) { return options; } - return { ...options, configPath: getDefaultMcpConfigPath() }; + return { ...options, catalogPath: getDefaultMcpConfigPath() }; } /** @@ -239,14 +349,15 @@ export function resolveServerConfigs( options: ServerConfigOptions, mode: ResolveServerConfigsMode, ): MCPServerConfig[] { - const hasConfigPath = Boolean(options.configPath?.trim()); + const source = resolveServerSource(options); const hasAdHoc = hasAdHocServerOptions(options); if (mode === "single") { - if (hasConfigPath && options.serverName) { + if (source && options.serverName) { const config = loadServerFromConfig( - options.configPath!, + source.path, options.serverName, + source.writable, ); return [ applyOverrides(config, { @@ -255,9 +366,8 @@ export function resolveServerConfigs( }), ]; } - if (hasConfigPath && !options.serverName) { - const configPath = options.configPath!; - const mcpConfig = loadMcpServersConfig(configPath); + if (source && !options.serverName) { + const mcpConfig = loadMcpServersConfig(source.path, source.writable); const servers = Object.keys(mcpConfig.mcpServers); if (servers.length === 0) throw new Error("No servers found in config file"); @@ -268,7 +378,11 @@ export function resolveServerConfigs( } const serverName = servers[0]; if (!serverName) throw new Error("No servers found in config file"); - const config = loadServerFromConfig(configPath, serverName); + const config = loadServerFromConfig( + source.path, + serverName, + source.writable, + ); return [ applyOverrides(config, { env: options.env, @@ -280,14 +394,13 @@ export function resolveServerConfigs( } if (mode === "multi") { - if (hasConfigPath && hasAdHoc) { + if (source && hasAdHoc) { throw new Error( "In multi-server mode with a config file, do not pass --transport, --server-url, or positional command/URL. Use only --config with optional -e, --cwd.", ); } - if (hasConfigPath && options.configPath) { - const configPath = options.configPath; - const mcpConfig = loadMcpServersConfig(configPath); + if (source) { + const mcpConfig = loadMcpServersConfig(source.path, source.writable); const configs = Object.values(mcpConfig.mcpServers).map((c) => applyOverrides({ ...c } as MCPServerConfig, { env: options.env, @@ -303,31 +416,31 @@ export function resolveServerConfigs( } /** - * Launch-time resolver for CLI/TUI: applies the default catalog path when no - * `--config` or ad-hoc target is given, then delegates to `resolveServerConfigs`. + * 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(withDefaultConfigPath(options), mode); + return resolveServerConfigs(withDefaultCatalogPath(options), mode); } /** - * Returns named server configs from a config file (multi-server). Use when the caller - * needs server names (e.g. TUI). Errors if config path is missing or if ad-hoc options - * (target, transport, serverUrl) are also provided. + * Returns named server configs from a catalog/config file (multi-server). Use + * when the caller needs server names. Resolves the active source via + * `resolveServerSource` (a writable `--catalog` is seeded if missing; a + * read-only `--config` errors if absent). Errors if neither source is set or if + * ad-hoc options (target, transport, serverUrl) are also provided. */ export function getNamedServerConfigs( options: ServerConfigOptions, ): Record { - const hasConfigPath = Boolean(options.configPath?.trim()); - const hasAdHoc = - (options.target && options.target.length > 0) || - Boolean(options.transport) || - Boolean(options.serverUrl); + const source = resolveServerSource(options); + const hasAdHoc = hasAdHocServerOptions(options); - if (!hasConfigPath) { + if (!source) { throw new Error("Config path is required for getNamedServerConfigs."); } if (hasAdHoc) { @@ -336,7 +449,7 @@ export function getNamedServerConfigs( ); } - const mcpConfig = loadMcpServersConfig(options.configPath!); + const mcpConfig = loadMcpServersConfig(source.path, source.writable); const result: Record = {}; for (const [name, config] of Object.entries(mcpConfig.mcpServers)) { result[name] = applyOverrides( diff --git a/core/mcp/node/index.ts b/core/mcp/node/index.ts index 2c970cb2..ec4d79e1 100644 --- a/core/mcp/node/index.ts +++ b/core/mcp/node/index.ts @@ -1,11 +1,16 @@ export { parseKeyValuePair, parseHeaderPair, - withDefaultConfigPath, + withDefaultCatalogPath, resolveServerConfigs, resolveLaunchServerConfigs, getNamedServerConfigs, + resolveServerSource, + serverSourceConflict, + hasAdHocServerOptions, + readServerListFile, type ServerConfigOptions, type ResolveServerConfigsMode, + type ServerSourceFlags, } from "./config.js"; export { createTransportNode } from "./transport.js"; diff --git a/package.json b/package.json index c7f488c9..738714dc 100644 --- a/package.json +++ b/package.json @@ -39,8 +39,10 @@ "test:tui": "cd clients/tui && npm run test", "validate": "npm run validate:web && npm run test:cli && npm run test:tui && npm run validate:launcher", "validate:web": "cd clients/web && npm run validate", - "validate:launcher": "npm run build:cli && npm run build:tui && npm run build:launcher && npm run smoke:launcher && npm run smoke:web", + "validate:launcher": "npm run build:cli && npm run build:tui && npm run build:launcher && npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web", "smoke:launcher": "node clients/launcher/build/index.js --help && node clients/launcher/build/index.js --cli --help && node clients/launcher/build/index.js --tui --help", + "smoke:cli": "node scripts/smoke-cli.mjs", + "smoke:tui": "node scripts/smoke-tui.mjs", "smoke:web": "node scripts/smoke-web.mjs", "test": "cd ./clients/web/ && npm run test && cd ../cli && npm run test && cd ../tui && npm run test", "test:coverage": "cd ./clients/web/ && npm run test:coverage", diff --git a/scripts/smoke-cli.mjs b/scripts/smoke-cli.mjs new file mode 100644 index 00000000..70a95a43 --- /dev/null +++ b/scripts/smoke-cli.mjs @@ -0,0 +1,170 @@ +#!/usr/bin/env node +/** + * End-to-end smoke test for the prod CLI launcher path (#1347). + * + * `npm run smoke:launcher` only checks `--cli --help`; it never connects to a + * server, so the launcher → CLI → core → stdio-transport path and the + * `--catalog` / `--config` resolution introduced in #1347 went unverified by + * the launcher smokes. This script drives the built `clients/launcher/build` + * artifact in `--cli` mode and asserts, end to end: + * + * 1. `--catalog ` + `--server` runs `tools/list` and returns the + * bundled test server's tools (real connect over stdio). + * 2. The default writable catalog is seeded empty on first run (and reports + * "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. + * + * Exits non-zero (failing CI / `npm run validate`) on any mismatch. + * + * Expects `clients/launcher/build` and `clients/cli/build` to be built first + * (the validate / CI ordering guarantees this). The bundled stdio test server + * (`test-servers/build`) is built on demand here if missing. + */ + +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, existsSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +const repoRoot = resolve(import.meta.dirname, ".."); +const launcher = join(repoRoot, "clients", "launcher", "build", "index.js"); +const testServer = join( + repoRoot, + "test-servers", + "build", + "test-server-stdio.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. */ +function ensureTestServer() { + if (existsSync(testServer)) 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)) { + fail( + "could not build the stdio test server (test-servers/build/test-server-stdio.js). " + + "Run `npm run test-servers:build` from clients/cli.", + ); + } +} + +/** Run the launcher in --cli mode. Returns { status, stdout, stderr }. */ +function runCli(args, extraEnv = {}) { + const r = spawnSync(process.execPath, [launcher, "--cli", ...args], { + cwd: repoRoot, + env: { ...process.env, ...extraEnv }, + encoding: "utf-8", + }); + return { status: r.status, stdout: r.stdout ?? "", stderr: r.stderr ?? "" }; +} + +if (!existsSync(launcher)) { + fail(`launcher build not found at ${launcher} — run \`npm run build\` first`); +} +ensureTestServer(); + +const work = mkdtempSync(join(tmpdir(), "smoke-cli-")); +try { + // 1) Populated --catalog → tools/list returns the test server's tools. + const catalogPath = join(work, "catalog.json"); + writeFileSync( + catalogPath, + JSON.stringify({ + mcpServers: { + test: { type: "stdio", command: process.execPath, args: [testServer] }, + }, + }), + ); + const list = runCli([ + "--catalog", + catalogPath, + "--server", + "test", + "--method", + "tools/list", + ]); + if (list.status !== 0) { + fail( + `tools/list via --catalog exited ${list.status}\n${list.stderr || list.stdout}`, + ); + } + let parsed; + try { + parsed = JSON.parse(list.stdout); + } catch { + fail(`tools/list did not return JSON on stdout:\n${list.stdout}`); + } + const toolNames = (parsed.tools ?? []).map((t) => t.name); + if (!toolNames.includes("echo")) { + fail( + `tools/list missing expected "echo" tool; got: ${toolNames.join(", ")}`, + ); + } + + // 2) Default writable catalog is seeded empty on first run (HOME redirected). + const fakeHome = join(work, "home"); + const seed = runCli(["--method", "tools/list"], { + HOME: fakeHome, + USERPROFILE: fakeHome, + }); + if (seed.status === 0) { + fail("default catalog with no servers should have exited non-zero"); + } + if (!/No servers found/.test(seed.stderr)) { + fail( + `expected "No servers found" from seeded default catalog; got:\n${seed.stderr}`, + ); + } + if (!existsSync(join(fakeHome, ".mcp-inspector", "mcp.json"))) { + fail("default catalog was not seeded at ~/.mcp-inspector/mcp.json"); + } + + // 3) Read-only --config that is missing errors and is NOT seeded. + const missingConfig = join(work, "missing.json"); + const ro = runCli(["--config", missingConfig, "--method", "tools/list"]); + if (ro.status === 0) { + fail("missing --config should have exited non-zero"); + } + if (!/Config file not found/.test(ro.stderr)) { + fail( + `expected "Config file not found" for missing --config; got:\n${ro.stderr}`, + ); + } + if (existsSync(missingConfig)) { + fail("read-only --config must never be seeded, but the file was created"); + } + + // 4) --catalog + --config is rejected. + const conflict = runCli([ + "--catalog", + catalogPath, + "--config", + catalogPath, + "--method", + "tools/list", + ]); + if (conflict.status === 0) { + fail("--catalog with --config should have exited non-zero"); + } + if (!/mutually exclusive/.test(conflict.stderr)) { + fail( + `expected "mutually exclusive" for --catalog + --config; got:\n${conflict.stderr}`, + ); + } + + console.log( + "smoke:cli OK — tools/list over stdio via --catalog; default-catalog seed; " + + "read-only --config error (no seed); --catalog/--config conflict", + ); +} finally { + rmSync(work, { recursive: true, force: true }); +} diff --git a/scripts/smoke-tui.mjs b/scripts/smoke-tui.mjs new file mode 100644 index 00000000..71f4e181 --- /dev/null +++ b/scripts/smoke-tui.mjs @@ -0,0 +1,134 @@ +#!/usr/bin/env node +/** + * Boot smoke test for the prod TUI launcher path (#1347). + * + * `npm run smoke:launcher` only checks `--tui --help`; it never actually starts + * the Ink app, so the launcher → TUI → core path and `--catalog` loading went + * unverified by the launcher smokes. This script launches the built launcher in + * `--tui` mode against a temp `--catalog`, waits for the app to render its first + * frame (the "MCP Servers" panel) within a timeout, then sends SIGTERM and + * exits. + * + * It asserts the TUI *boots and renders* without crashing — not full + * interaction (driving an Ink UI deterministically in CI is flaky, so this is + * intentionally a shallow render check). Exits non-zero on a crash-before-render + * or a render timeout. + * + * Expects `clients/launcher/build` and `clients/tui/build` to be built first + * (the validate / CI ordering guarantees this). The bundled stdio test server + * (`test-servers/build`) is built on demand here if missing. + */ + +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"; + +const repoRoot = resolve(import.meta.dirname, ".."); +const launcher = join(repoRoot, "clients", "launcher", "build", "index.js"); +const testServer = join( + repoRoot, + "test-servers", + "build", + "test-server-stdio.js", +); +const RENDER_MARKER = "MCP Servers"; +const TIMEOUT_MS = Number(process.env.SMOKE_TUI_TIMEOUT_MS ?? 15000); + +function fail(message) { + console.error(`smoke:tui FAILED — ${message}`); + process.exit(1); +} + +function ensureTestServer() { + if (existsSync(testServer)) return; + console.log("smoke:tui — 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)) { + fail( + "could not build the stdio test server (test-servers/build/test-server-stdio.js). " + + "Run `npm run test-servers:build` from clients/cli.", + ); + } +} + +if (!existsSync(launcher)) { + fail(`launcher build not found at ${launcher} — run \`npm run build\` first`); +} +ensureTestServer(); + +const work = mkdtempSync(join(tmpdir(), "smoke-tui-")); +const catalogPath = join(work, "catalog.json"); +writeFileSync( + catalogPath, + JSON.stringify({ + mcpServers: { + test: { type: "stdio", command: process.execPath, args: [testServer] }, + }, + }), +); + +const child = spawn( + process.execPath, + [launcher, "--tui", "--catalog", catalogPath], + { + cwd: repoRoot, + // Redirect HOME so the TUI's storage never touches the real ~/.mcp-inspector. + env: { ...process.env, HOME: work, USERPROFILE: work }, + stdio: ["ignore", "pipe", "pipe"], + }, +); + +let output = ""; +let settled = false; + +function cleanup() { + rmSync(work, { recursive: true, force: true }); +} + +function done(code, message) { + if (settled) return; + settled = true; + clearTimeout(timer); + if (!child.killed) child.kill("SIGTERM"); + cleanup(); + if (code === 0) { + console.log(`smoke:tui OK — ${message}`); + } else { + console.error(`smoke:tui FAILED — ${message}`); + } + process.exit(code); +} + +function onData(chunk) { + output += chunk.toString(); + if (output.includes(RENDER_MARKER)) { + done(0, `rendered "${RENDER_MARKER}" panel from --catalog within timeout`); + } +} + +child.stdout.on("data", onData); +child.stderr.on("data", onData); + +child.on("exit", (code) => { + if (settled) return; + // Exiting before the render marker appeared is a failure (crash on boot). + done( + 1, + `TUI exited (code ${code}) before rendering "${RENDER_MARKER}"\n${output.slice(0, 800)}`, + ); +}); + +child.on("error", (err) => { + done(1, `failed to spawn TUI: ${err.message}`); +}); + +const timer = setTimeout(() => { + done( + 1, + `TUI did not render "${RENDER_MARKER}" within ${TIMEOUT_MS}ms\n${output.slice(0, 800)}`, + ); +}, TIMEOUT_MS);