* 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 <path>` (+ 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) <noreply@anthropic.com> * 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 <temp>` 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
36b48e61a2
commit
73b651ff21
1 changed files with 962 additions and 81 deletions
|
|
@ -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/,
|
||||
|
|
|
|||
Loading…
Reference in a new issue