feat(cli,tui): normalize --catalog / --config with web; update --help (#1347)

[cliffhall]

Closes #1347.

Brings the CLI and TUI in line with the web catalog vs. read-only session model introduced in #1481/#1483, and closes the outstanding --help acceptance item. The original default-fallback ask (getDefaultMcpConfigPath() when no flag is given) was already delivered with the CLI/TUI port; this PR normalizes the --catalog / --config vocabulary and behavior everywhere.

What changed

Core — core/mcp/node/config.ts

• Add catalogPath to ServerConfigOptions (the 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; a read-only --config still errors if absent.
• serverSourceConflict() — shared conflict matrix (mirrors run-web): --catalog/--config are mutually exclusive, and neither combines with an ad-hoc target.
• readServerListFile() — public loader for runners (TUI) that need the raw server map.
• Rename withDefaultConfigPath → withDefaultCatalogPath; the no-flag default now fills the writable catalog slot, not --config.

CLI / TUI

• Add --catalog <path> (+ MCP_CATALOG_PATH) and re-document --config as a read-only session in --help.
• Enforce the shared conflict matrix before resolution.

Decisions (confirmed with maintainer)

• Seed-on-missing: match web. A missing writable catalog (or the default) is seeded empty; a missing --config errors.
• Focused slice. Real writable persistence/CRUD threading remains deferred to CLI: lift config-file headers/timeouts/OAuth into serverSettings (parity with TUI) #1482 / Inspector mcpi client #1432 — CLI/TUI are read consumers today, so the writable/read-only split currently surfaces only as seed- vs. error-on-missing. Vocabulary, flags, conflict matrix, help text, and the seed decision land here.

Tests

• Core resolution matrix: resolveServerSource, serverSourceConflict, seed-on-missing (writable), error-on-missing (read-only), readServerListFile, withDefaultCatalogPath, and catalog branches of resolveServerConfigs (single + multi).
• CLI: --catalog success + --catalog/--config and --catalog/ad-hoc conflict integration tests.
• TUI: loadTuiServers unit tests (catalog seed, config error, conflicts, ad-hoc, header merge).
• Docs: launcher README CLI/TUI flag table; CLI/TUI READMEs.

npm run validate passes end-to-end (web format/lint/build/coverage, CLI + TUI suites, launcher build, smoke:launcher, smoke:web).

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

Review: PR #1499 — normalize --catalog / --config for CLI & TUI

• Read core config changes (core/mcp/node/config.ts)
• Read CLI / TUI wiring
• Read tests & smoke scripts
• Post review

---

Overall this is a clean, well-scoped slice. The catalog-vs-read-only-session model is faithfully ported from the web backend, the seed-on-missing decision is implemented symmetrically (writable seeds, read-only errors), and test/smoke coverage is genuinely thorough — resolveServerSource, serverSourceConflict, seed vs. error paths, readServerListFile, single/multi resolution, plus real end-to-end stdio connect in smoke-cli.mjs. The vocabulary, help text, and conflict matrix all line up. Nice work.

A few small things, none blocking:

1. serverSourceConflict doesn't mirror web's --header rule (doc accuracy)

core/mcp/node/config.ts:296 says it "mirrors the web run-web conflict matrix," but run-web.ts:135–140 also rejects --header combined with --catalog/--config, whereas this matrix does not. That divergence is actually correct and intentional for CLI/TUI — loadTuiServers explicitly merges --header into per-server settings via mergeSettings (clients/tui/src/tui-servers.ts:91), and the CLI applies headers via headersToServerSettings. So the behavior is right; only the "mirrors" wording slightly overstates it. Consider tweaking the comment to "mirrors the source-selection portion of the web conflict matrix" so a future reader doesn't try to "fix" the missing --header branch.

2. CLI vs. TUI disagree on empty-string --catal + env fallback

• CLI (clients/cli/src/cli.ts:396): catalogPath: options.catalog ?? process.env.MCP_CATALOG_PATH
• TUI (clients/tui/tui.tsx:82): catalogPath: options.catalog?.trim() || process.env.MCP_CATALOG_PATH

With an explicit empty --catalog "": the CLI keeps "" (?? only guards undefined/null) and never falls back to MCP_CATALOG_PATH; the TUI's ?.trim() || collapses "" and does fall back to the env var. A genuine edge case, but the two front-ends should resolve the same flags identically. Aligning the CLI to options.catalog?.trim() || process.env.MCP_CATALOG_PATH would make them consistent (and the downstream ?.trim() conflict check already tolerates either). Fix this →

3. TUI applies --header to every catalog server (minor)

In loadTuiServers, when a catalog/config holds multiple servers, the same --header set is merged into all of them (tui-servers.ts:85–93). That's a reasonable default for the common single-server case, but it means a shared Authorization header would be broadcast to every server in a multi-server catalog. Probably fine for now given headers-in-catalog is the long-term path, but worth a one-line comment noting the broadcast semantics so it's a deliberate choice rather than a surprise.

Nits (optional)

• readServerListContent (config.ts:112) returns JSON.stringify({ mcpServers: {} }) (compact) for the seeded case while seedEmptyCatalog writes the pretty-printed +\n variant to disk. Functionally identical after parse — just noting the two literals could share a constant.
• seedEmptyCatalog writes mode 0o600. Right for the default ~/.mcp-inspector/mcp.json; for a user-supplied --catalog on a shared path it's stricter than the umask default. Almost certainly the safe choice — flagging only for awareness.

No correctness bugs spotted in the resolution matrix or seed/error logic, and the deferred-persistence scope is clearly documented. 👍
· v2/1347-normalize-catalog-config-cli-tui

[cliffhall]

Thanks for the review! Addressed in f496577 (all non-blocking):

1. serverSourceConflict wording — reworded the comment to say it mirrors only the source-selection portion of web's matrix, and explicitly notes it omits web's --header rejection on purpose (CLI/TUI merge --header into per-server settings). Behavior unchanged, as you confirmed.
2. CLI vs TUI empty-string --catalog — good catch, real divergence. Aligned the CLI to options.catalog?.trim() || process.env.MCP_CATALOG_PATH so an explicit --catalog "" now falls back to MCP_CATALOG_PATH identically in both front-ends.
3. TUI --header broadcast — added a comment marking it as a deliberate broadcast (fine for the common single-server case; multi-server files should use per-server headers).
4. Nit (shared literal) — extracted EMPTY_CATALOG_CONTENT and use it in both seedEmptyCatalog and readServerListContent.

Left 0o600 as-is (the safe choice you flagged). npm run validate is green end-to-end (web coverage, CLI/TUI suites, all three launcher smokes).

[cliffhall]

@claude review

[claude]

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

---

Re-review: PR #1499 — normalize --catalog / --config for CLI & TUI

• Read updated core config (core/mcp/node/config.ts)
• Read CLI / TUI wiring updates
• Verify prior review points addressed
• Post review

---

I re-read the diff against origin/v2/main, focusing on the follow-up commit f4965777. All four points from the last pass are addressed cleanly, and I found no new issues.

Prior feedback — verified resolved

1. serverSourceConflict wording ✅ — core/mcp/node/config.ts:290-298 now says it mirrors only the source-selection portion of web's matrix and explicitly documents that it omits web's --header rejection (because CLI/TUI merge headers into per-server settings). Accurate now.
2. CLI/TUI empty-string --catalog parity ✅ — clients/cli/src/cli.ts:398 uses options.catalog?.trim() || process.env.MCP_CATALOG_PATH, matching clients/tui/tui.tsx:82. The inline comment explains the ?.trim() || vs ?? choice. An explicit --catalog "" now falls back to the env var identically in both front-ends.
3. TUI --header broadcast ✅ — clients/tui/src/tui-servers.ts:91-93 marks the broadcast as deliberate, with the right guidance (fine for single-server; multi-server files should carry per-server headers).
4. Shared literal nit ✅ — EMPTY_CATALOG_CONTENT (config.ts:89) is now the single source used by both seedEmptyCatalog (:98) and readServerListContent (:111); no more divergent compact/pretty literals.
5. 0o600 — left as-is, the safe choice flagged previously.

New observations (non-blocking)

• configPath isn't .trim()-normalized in the CLI. cli.ts:399 does configPath: options.config while the TUI (tui.tsx:83) does options.config?.trim() || undefined. The downstream resolveServerSource / serverSourceConflict both ?.trim() before testing, so behavior is identical for whitespace input — but it's a small asymmetry with the catalog slot right above it (which is trimmed). Trimming here too would make the two slots and the two front-ends fully uniform. Purely cosmetic.
• loadTuiServers length-0 guard is effectively dead (tui-servers.ts:101). In the no-source path, resolveServerConfigs(_, "multi") always returns exactly one ad-hoc config (or throws inside buildConfigFromOptions), so configs.length === 0 can't trigger. Harmless safety net — fine to keep, just noting it isn't exercised.

Coverage

The new tui-servers.test.ts covers the full matrix — read-only load, writable seed, missing-config error, both conflicts, ad-hoc build, and header merge — and the core config.test.ts additions exercise resolveServerSource / serverSourceConflict / seed-vs-error / readServerListFile / withDefaultCatalogPath. Combined with the real stdio connect in smoke-cli.mjs, this is thorough.

LGTM — the slice is well-scoped, the seed-vs-error split is symmetric, deferred persistence is clearly documented, and the prior feedback is fully resolved. (I can't formally approve PRs, but I have no blocking concerns.) 👍
· v2/1347-normalize-catalog-config-cli-tui

[cliffhall]

Thanks — addressed the two new observations in 899c20a:

• CLI configPath trim — applied the one-liner: options.config?.trim() || undefined, so it matches the --catalog slot above it and the TUI. Behavior unchanged (downstream already trims).
• loadTuiServers length-0 guard — keeping it as you suggested; it's a defensive net for the no-source/ad-hoc path. Left a mental note that it isn't currently exercisable.

CLI suite (38) + smoke:cli green after the change.

[cliffhall]

@claude review

[claude]

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

---

Re-review: PR #1499 — normalize --catalog / --config for CLI & TUI

• Read core config (core/mcp/node/config.ts)
• Read CLI / TUI wiring
• Verify prior feedback resolved
• Post review

---

Third pass over the diff against origin/v2/main, including commit 899c20a6. The two observations from the last round are addressed, and I cross-checked the CLI/TUI source-selection behavior against run-web.ts — they now line up. No blocking issues.

Prior feedback — verified resolved

1. CLI configPath trim ✅ — cli.ts:399 is now options.config?.trim() || undefined, matching the --catalog slot above it and the TUI (tui.tsx:83).
2. loadTuiServers length-0 guard ✅ — kept as the documented defensive net for the no-source/ad-hoc path; acknowledged as not currently exercisable.

Cross-check against web (confirms consistency)

I verified the conflict + source-selection logic against clients/web/server/run-web.ts:120-162:

• Both treat --catalog/--config as mutually exclusive and reject either with an ad-hoc target — identical messages (config.ts:300-308 vs run-web.ts:131-138). ✅
• Both resolve catalog via opts.catalog ?? process.env.MCP_CATALOG_PATH, and both treat an ambient MCP_CATALOG_PATH as hasCatalog. So setting that env var and also passing --config produces the mutual-exclusion error in all three front-ends — consistent by design, not a CLI/TUI-only quirk. ✅
• The deliberate divergence (CLI/TUI allow --header alongside a catalog/config; web rejects it) is now correctly documented in serverSourceConflict (config.ts:290-298) and matched by mergeSettings/headersToServerSettings. ✅

Minor observations (non-blocking, no action needed)

• Env-var fallback isn't .trim()-ed. cli.ts:398 / tui.tsx:82 use the explicit flag trimmed (options.catalog?.trim()) but the env fallback raw (process.env.MCP_CATALOG_PATH). A MCP_CATALOG_PATH=" " (whitespace) would survive the ||, but every downstream consumer (resolveServerSource, serverSourceConflict, withDefaultCatalogPath) re-checks with ?.trim(), so the effective behavior is correct. Purely cosmetic; matches web's untrimmed ?? too, so I'd leave it.
• EMPTY_CATALOG_CONTENT mode 0o600 — still the safe default for the writable catalog, agreed from the first pass.

Coverage

tui-servers.test.ts exercises the full matrix (read-only load, writable seed, missing-config error, both conflicts, ad-hoc build, header merge), the core config.test.ts additions cover resolveServerSource/serverSourceConflict/seed-vs-error/readServerListFile/withDefaultCatalogPath, and smoke-cli.mjs adds a real stdio connect plus the seed/error/conflict assertions end-to-end. Thorough.

LGTM — well-scoped, faithful to the web model, prior feedback fully resolved, deferred persistence clearly fenced off. I can't formally approve, but I have no blocking concerns. 👍
· v2/1347-normalize-catalog-config-cli-tui

[cliffhall]

Thanks for the thorough three passes (and the cross-check against run-web.ts — good to have the three front-ends confirmed consistent). Leaving the two remaining items as you recommended:

• Env-var fallback trim — leaving process.env.MCP_CATALOG_PATH untrimmed; downstream ?.trim() makes it correct and it matches web's untrimmed ??.
• 0o600 — keeping the safe default.

No further changes; PR is ready for maintainer review.