Skip to content

feat: port CLI, TUI, and launcher from v1.5 into v2#1452

Open
BobDickinson wants to merge 7 commits into
v2/mainfrom
v2-cli-tui
Open

feat: port CLI, TUI, and launcher from v1.5 into v2#1452
BobDickinson wants to merge 7 commits into
v2/mainfrom
v2-cli-tui

Conversation

@BobDickinson

Copy link
Copy Markdown
Contributor

Summary

Ports the CLI, TUI, and launcher from v1.5 into v2 with the smallest practical blast radius. The goal is to get all three clients building, testable, and invocable from a single mcp-inspector entrypoint — not to fully “v2-ify” them yet.

This is the “drag the rest of the clients kicking and screaming into v2” PR. Follow-ups will tackle deeper v2 integration (catalog UX, CI hardening, TUI parity, session-oriented CLI, etc.).

Closes #1246.

What changed

New clients (ported from v1.5)

  • clients/cli/ — one-shot CLI (runCli), 86 ported integration tests
  • clients/tui/ — Ink TUI (runTui), minimal smoke tests
  • clients/launcher/ — mode router (--web / --cli / --tui; default web), argv forwarding

Packaging & build

  • Root @modelcontextprotocol/inspector becomes a fat npm package: merged runtime deps, prepack → full build, mcp-inspector bin
  • tsup bundles @inspector/core into CLI/TUI/web-runner build/index.js (core is source-only in v2, not a separate published package)
  • Launcher dynamically imports sibling clients/{web,cli,tui}/build/index.js — no file: workspace deps

Web changes (required for launcher)

  • runWeb(argv) — programmatic web entry for mcp-inspector --web / --web --dev
  • tsup.runner.config.ts + clients/web/build/index.js — Node runner separate from Vite SPA build
  • web-server-config.ts — shared config builder for dev plugin, prod Hono, and launcher

Config / catalog (minimal)

  • withDefaultConfigPath() in core/mcp/node/config.ts — CLI and TUI fall back to ~/.mcp-inspector/mcp.json when no --config and no ad-hoc target
  • TUI uses mcpConfigToServerEntries() (correct v2 settings path); CLI still uses bare MCPServerConfig from file (known gap)

Infra & docs

  • vitest.shared.mts — shared @inspector/core / test-server aliases for CLI/TUI Vitest
  • CI — CLI + TUI tests, launcher --help smoke, Storybook (existing web pipeline unchanged in spirit)
  • specification/v2_cli_tui_launcher.md — architecture spec for this port
  • specification/v2_catalog_launch_config.md — catalog/launch design (mostly future work; cross-linked)

What we intentionally did not solve

Area Status
--catalog / servers/import Not implemented — web still uses default catalog only (#1348)
Web catalog switching via launcher runWeb --config sets legacy initialMcpConfig only; no mcpConfigPath on backend
CLI config-file settings Disk headers/timeouts/OAuth not lifted into serverSettings (TUI does lift them)
runWeb --header Accepted but warns only
Session-oriented CLI v2 Still one-shot per invocation (#1432)
TUI feature/test parity 2 smoke tests; no Ink/OAuth/catalog-CRUD coverage (#1183)
CLI/TUI coverage gates Core coverage stays on web suite; CLI runs out-of-process
Root validate Still web-only; does not chain CLI/TUI build+test
Prod web via launcher Requires pre-built clients/web/dist/ (CI builds web; launcher smoke does not start prod web end-to-end)

Test plan

  • npm run test:coverage (web unit + integration)
  • npm run test:cli (CLI tests)
  • npm run test:tui (TUI tests)
  • npm run test:storybook (Storybook play tests)
  • Launcher smoke: --help, --cli --help, --tui --help
  • npm run build (web + cli + tui + launcher)
  • Manual: mcp-inspector --cli node … --method tools/list
  • Manual: mcp-inspector --tui with default ~/.mcp-inspector/mcp.json
  • Manual: mcp-inspector --web --dev

Docs

  • Primary spec: specification/v2_cli_tui_launcher.md
  • Catalog/launch (deferred): specification/v2_catalog_launch_config.md
  • Updated: AGENTS.md (project structure, test commands)

@cliffhall

Copy link
Copy Markdown
Member

@claude review

@claude

claude Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 3m 47s —— View job


Review — PR #1452: port CLI, TUI, launcher from v1.5 into v2

I read the diff against origin/v2/main, focusing on the new integration glue (launcher, runWeb, core config/logging changes, packaging, CI) rather than the bulk-ported TUI/CLI source. Overall this is a clean, well-scoped port — the intentional gaps are documented, runWeb and config have solid tests, and extracting vitest.shared.mts removes real duplication. A few things worth addressing before merge.

Correctness

1. Default-config fallback is baked into shared resolveServerConfigs — global behavior change (core/mcp/node/config.ts:242)
withDefaultConfigPath(options) now runs inside resolveServerConfigs, so the ~/.mcp-inspector/mcp.json fallback applies to every caller, not just CLI/TUI. The previous "Target is required" error for an empty target is now "Config file not found" (the test was updated to match). The web backend escapes this only because run-web.ts guards the call behind hasServerInput. This couples the shared resolver to a CLI/TUI convenience, and loadTuiServers already calls withDefaultConfigPath explicitly (tui-servers.ts:67) and then calls resolveServerConfigs, so it's applied twice. Consider applying the default only at the CLI/TUI entry points and leaving resolveServerConfigs behavior unchanged for shared callers. Fix this →

2. Dead try/catch in TUI entry (clients/tui/tui.tsx:131-139)

try {
  callbackUrlConfig = parseCallbackUrl(options.callbackUrl);
} catch (err) {
  if (err instanceof Error) throw err;
  throw err;   // both branches rethrow — the catch does nothing
}

The try/catch and the conditional are no-ops. Call parseCallbackUrl directly. Fix this →

Minor / nits

3. Launcher strips all occurrences of the mode flag (clients/launcher/src/index.ts:42)
process.argv.filter((arg) => arg !== modeFlag) removes every token equal to --web/--cli/--tui. A forwarded argument value that happens to equal that string (e.g. a header or positional) would be silently dropped. Index-based removal of the first match would be safer. Related: if both --cli and --tui are passed, only the selected mode's flag is filtered and the other is forwarded to an app whose Commander may reject it.

4. CI launcher smoke depends on an implicit build (.github/workflows/main.yml)
The smoke step runs --cli --help, which dynamically imports clients/cli/build/index.js, but the workflow only explicitly builds tui and launcher. It works today because the earlier "Run CLI tests" step's pretest builds the CLI bin — but that's a fragile ordering dependency. An explicit npm run build:cli before the smoke would be more robust.

5. engines.node >= 22.7.5 vs target: 'node20' — all three tsup.config.ts target node20 while the root package requires Node ≥ 22.7.5. Harmless, but bumping the target to node22 would keep them consistent.

6. vite + @vitejs/plugin-react as production dependencies (package.json) — the fat package now pulls the full Vite toolchain on every install, including for CLI/TUI-only users. Expected given --web --dev, but worth a conscious confirmation given the install footprint.

Acknowledged gaps (no action needed)

The PR body already tracks these — TUI has only 2 smoke tests (#1183), CLI config-file settings aren't lifted into serverSettings, --catalog is deferred (#1348), root validate stays web-only. All reasonable for a "get it building and invokable" port.

None of the above are blocking on their own; #1 and #2 are the two I'd recommend addressing in this PR.
· branch v2-cli-tui

Keep resolveServerConfigs as an explicit-options resolver; CLI uses the launch wrapper so catalog defaults apply only at entry points, not for all callers. Web still resolves explicit server args only today; a follow-up can wire it through the same launch helper when catalog/session argv is unified.
Read --web/--cli/--tui only from a contiguous prefix after the script name; forward all following tokens unchanged so homonyms can appear in app/server args.

Reject multiple mode flags in the prefix; do not strip or warn on later matches.
…t Vite deps

Build CLI before launcher smoke so the step does not rely on test pretest ordering. Align tsup targets with engines.node (>=22.7.5). Document why Vite stays in published runtime dependencies for mcp-inspector --web --dev.
@BobDickinson

Copy link
Copy Markdown
Contributor Author

Addressed all PR review comments from Claude.

The "vite + @vitejs/plugin-react as production dependencies" was as intended, docs updated to reflect that.

@cliffhall

Copy link
Copy Markdown
Member

@claude review

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 1m 12s —— View job


Re-review — PR #1452

I re-read the diff against origin/v2/main, focusing on the four items I flagged in the first pass plus the new "Harden packaging" / "argv forwarding" commits. All four prior findings are cleanly resolved. This is good to merge from my side; one tiny non-blocking redundancy noted below.

Previously-flagged items — all addressed ✅

1. Default-config fallback coupling — Resolved correctly. resolveServerConfigs no longer calls withDefaultConfigPath; the fallback now lives in a dedicated launch-time wrapper resolveLaunchServerConfigs() (core/mcp/node/config.ts:309), so shared/web callers keep their original behavior and only CLI/TUI opt into the ~/.mcp-inspector/mcp.json default. Clean separation.

2. Dead try/catch in TUI entry — Removed. clients/tui/tui.tsx:131 now calls parseCallbackUrl(options.callbackUrl) directly.

3. Launcher strips all occurrences of the mode flag — Reworked into a much safer prefix-zone parser (clients/launcher/src/parse-launcher-argv.ts). Mode flags are only read from a contiguous prefix of argv[2…]; the first non-mode token ends the zone and everything after is forwarded unchanged, so a forwarded value equal to --cli/--web/--tui is no longer dropped. It also rejects passing more than one mode flag (Specify at most one of …). Nice.

4. CI launcher smoke depends on implicit build — Fixed. The workflow now has an explicit Build CLI, TUI, and launcher step (npm run build:cli && npm run build:tui && npm run build:launcher) before the smoke step, so --cli --help no longer relies on the CLI pretest ordering.

The node22 tsup target bump (item 5) and the Vite-deps documentation (item 6) were also handled, per the commit log and the author's note.

One minor non-blocking nit

Redundant withDefaultConfigPath in the ad-hoc branch of loadTuiServers (clients/tui/src/tui-servers.ts:67 + :88)
loadTuiServers applies withDefaultConfigPath(serverOptions) at line 67. If that injects a default config path, hasConfigPath becomes true and we take the config-file branch — so the only way to reach line 88 is with an ad-hoc target, where resolveLaunchServerConfigs calls withDefaultConfigPath again internally. It's a harmless no-op (the second call early-returns because hasAdHocServerOptions is true), but line 88 could call resolveServerConfigs(serverOptions, "multi") directly since the default is already resolved, making the intent clearer. Purely cosmetic. Tidy this →

Acknowledged gaps (no action needed)

Same as before and still reasonable for a "get it building and invokable" port — TUI smoke-only tests (#1183), CLI config-file settings not lifted into serverSettings, --catalog deferred (#1348), root validate stays web-only.

LGTM. 🚀
· branch v2-cli-tui

Default catalog is already applied via withDefaultConfigPath above; the ad-hoc branch only needs the resolver, not resolveLaunchServerConfigs's redundant pass.
@BobDickinson

Copy link
Copy Markdown
Contributor Author

Addressed remaining issue: withDefaultConfigPath in the ad-hoc branch of loadTuiServers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port v1.5 CLI / TUI / launcher to v2 (clients/cli, clients/tui, clients/launcher)

2 participants