From e5c2124b3132f7ca4a8517c0ef66f31897381a5c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 04:46:53 -0400 Subject: [PATCH 1/5] feat(cli,tui): --skill-catalog-max-skills/--skill-catalog-max-bytes flags (#2420) Web exposes the per-server skills catalog budget (skillCatalogMaxSkills / skillCatalogMaxBytes) in Server Settings; the CLI and TUI already honored the values from a catalog/config file entry but had no flag for them. Add both flags to the CLI and TUI, validated as positive integers by a shared core coerce (skillCatalogLimitParser), and overlay them onto the resolved server settings in loadServerEntries the way --protocol-era is, so verifySkills reads them through getServerSettings(). Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/cli/README.md | 2 + .../__tests__/programmatic-ergonomics.test.ts | 63 +++++++++++++++++ clients/cli/src/cli.ts | 17 +++++ clients/tui/README.md | 2 + clients/tui/tui.tsx | 15 ++++ .../web/src/test/core/mcp/node/config.test.ts | 23 +++++++ .../src/test/core/mcp/node/servers.test.ts | 68 +++++++++++++++++++ core/mcp/node/config.ts | 26 +++++++ core/mcp/node/index.ts | 1 + core/mcp/node/servers.ts | 37 +++++++--- docs/mcp-server-configuration.md | 1 + 11 files changed, 246 insertions(+), 9 deletions(-) diff --git a/clients/cli/README.md b/clients/cli/README.md index 78abe7a46b..c6799c4696 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -125,6 +125,8 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en | `--strict` | With `--method tools/list`: report tool-schema portability problems in full (path, issue, suggested fix) on stderr, and exit `6` if any is error-severity. Without it, a one-line count is printed instead. See [Schema portability](#schema-portability---strict). | | `--verify` | With `--method skills/list` or `--method skills/get`: run the SEP-2640 conformance, digest and frontmatter checks over the skills returned, emit one JSON report per skill on stdout, and exit `7` if any fails. See [Skill verification](#skill-verification---verify). | | `--require-digests` | With `--verify`: exit `9` when a skill advertises no digests (`resources: "dynamic"`), instead of reporting it `unverifiable` and exiting `0`. See [Skill verification](#skill-verification---verify). | +| `--skill-catalog-max-skills ` | The skills catalog budget for `--verify`: the most skills whose files one run reads (default `256`). Positive integer; anything else is rejected before connecting. Overrides the server's [`skillCatalogMaxSkills`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields) from the catalog/config file — the same setting the web client exposes per server in Server Settings. Skills past the budget are reported `incomplete` (exit `8`). | +| `--skill-catalog-max-bytes ` | The skills catalog budget for `--verify`: the most bytes one run reads across all skills (default `67108864`, 64 MiB). Positive integer. Overrides the server's [`skillCatalogMaxBytes`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields). | | `--format ` | Output format. `text` (default) pretty-prints the result. `json` emits a single JSON object on stdout (`{ "result": … }`, plus `{ "appInfo": … }` as a sibling key for App tools) with no banners, so the whole output pipes cleanly into `jq`. | | `--relogin` | Delete stored OAuth for this server URL from the shared store before connect; interactive login still only runs if the server requires auth. Requires an HTTP/SSE URL (rejected for stdio). Conflicts with `--stored-auth-only` / `--use-stored-auth` / `--wait-for-auth` / catalog short-circuits. | | `--no-revoke` | With `--relogin`, skip the [RFC 7009](https://datatracker.ietf.org/doc/html/rfc7009) revocation request that would otherwise end the grant at the authorization server when the local state is deleted. The per-server `oauth.revokeOnClear` setting is the persistent form of the same opt-out; either one is enough to skip it. See [Revoking on `--relogin`](#revoking-on---relogin). | diff --git a/clients/cli/__tests__/programmatic-ergonomics.test.ts b/clients/cli/__tests__/programmatic-ergonomics.test.ts index 8459644e81..24503317bc 100644 --- a/clients/cli/__tests__/programmatic-ergonomics.test.ts +++ b/clients/cli/__tests__/programmatic-ergonomics.test.ts @@ -299,6 +299,69 @@ describe("--protocol-era", () => { }); }); +describe("--skill-catalog-max-skills / --skill-catalog-max-bytes (#2420)", () => { + // The flags must reach `verifySkills`' run-level budget, not just parse. A + // budget of one skill leaves every later skill unread, which the report + // records as an `incomplete` outcome — without the flag the fixture's whole + // catalog fits the default budget and no skill is incomplete. + it("caps the --verify run at the flag's skill budget", async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + skills: true, + }); + try { + await server.start(); + const verify = [ + server.url, + "--transport", + "http", + "--method", + "skills/list", + "--verify", + ]; + + const unbounded = await runCli(verify); + expect(reportOutcomes(unbounded.stdout)).not.toContain("incomplete"); + + const capped = await runCli([ + ...verify, + "--skill-catalog-max-skills", + "1", + ]); + expect(reportOutcomes(capped.stdout)).toContain("incomplete"); + } finally { + await server.stop(); + } + }); + + it.each([ + ["--skill-catalog-max-skills", "0"], + ["--skill-catalog-max-bytes", "1.5"], + ])("rejects %s %s before connecting", async (flag, value) => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + flag, + value, + "--method", + "tools/list", + ]); + expectCliFailure(result); + expect(result.stderr).toContain( + `Invalid ${flag}: ${value}. Expected a positive integer.`, + ); + }); +}); + +/** The `outcome` of each NDJSON `--verify` report line. */ +function reportOutcomes(stdout: string): string[] { + return stdout + .trim() + .split("\n") + .map((line) => (JSON.parse(line) as { outcome: string }).outcome); +} + describe("MCP_CATALOG_PATH with an ad-hoc target", () => { it("does not conflict with an ad-hoc target (env catalog is ignored)", async () => { const { command, args } = getTestMcpServerCommand(); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 293df651f1..7fb20b9313 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -26,6 +26,7 @@ import { parseKeyValuePair as parseEnvPair, parseHeaderPair, parseProtocolEra, + skillCatalogLimitParser, } from "@inspector/core/mcp/node/index.js"; import type { JsonValue } from "@inspector/core/mcp/index.js"; import type { StrictJsonValue } from "@inspector/core/json/jsonUtils.js"; @@ -855,6 +856,16 @@ async function parseArgs(argv?: string[]): Promise { "Protocol era to negotiate: legacy, auto, or modern. Overrides the file-level protocolEra for --catalog/--config runs; ad-hoc --server-url / target runs otherwise default to legacy.", parseProtocolEra, ) + .option( + "--skill-catalog-max-skills ", + "Skills catalog budget for --verify: the most skills one run reads (positive integer; default 256). Overrides the server's skillCatalogMaxSkills in the catalog/config file.", + skillCatalogLimitParser("--skill-catalog-max-skills"), + ) + .option( + "--skill-catalog-max-bytes ", + "Skills catalog budget for --verify: the most bytes one run reads across all skills (positive integer; default 67108864, 64 MiB). Overrides the server's skillCatalogMaxBytes in the catalog/config file.", + skillCatalogLimitParser("--skill-catalog-max-bytes"), + ) .option( "--format ", "Output format: text (default; pretty-printed) or json (one JSON object on stdout, no banners).", @@ -955,6 +966,8 @@ async function parseArgs(argv?: string[]): Promise { cursor?: string; connectTimeout?: number; protocolEra?: ServerProtocolEra; + skillCatalogMaxSkills?: number; + skillCatalogMaxBytes?: number; format?: OutputFormat; toolArgsJson?: string; clientConfig?: string; @@ -1125,6 +1138,10 @@ async function parseArgs(argv?: string[]): Promise { // `--protocol-era` feeds `settings.protocolEra` the same way, so an ad-hoc // launch can pick a non-legacy era without an mcp.json entry (#2208). protocolEra: options.protocolEra, + // `--skill-catalog-max-*` feed the per-server skills catalog budget the + // web sets in Server Settings, read by `verifySkills` (#2420). + skillCatalogMaxSkills: options.skillCatalogMaxSkills, + skillCatalogMaxBytes: options.skillCatalogMaxBytes, }; // Catalog list / show — no MCP connection. Run before stored-auth refresh so diff --git a/clients/tui/README.md b/clients/tui/README.md index ba2c3c083d..cf60bd3c03 100644 --- a/clients/tui/README.md +++ b/clients/tui/README.md @@ -31,6 +31,8 @@ npx @modelcontextprotocol/inspector --tui --config mcp.json # read-only sessi 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 **empty** if missing; default `~/.mcp-inspector/mcp.json` or `MCP_CATALOG_PATH`), `--config` (read-only session, errors if absent), `-e`, `--cwd`, `--header`, `--protocol-era` (`legacy`/`auto`/`modern`; sets the era an ad-hoc server negotiates, or overrides a file's `protocolEra`), `--transport`, `--server-url`, and the positional `[target...]`. `--catalog` and `--config` are mutually exclusive, and neither combines with an ad-hoc target. +`--skill-catalog-max-skills ` and `--skill-catalog-max-bytes ` set the skills catalog budget the **Skills** pane's verification reads within (defaults `256` skills and `67108864` bytes, 64 MiB) — the same per-server setting the web client exposes in Server Settings. Each takes a positive integer (anything else is rejected at startup) and overrides the file's [`skillCatalogMaxSkills` / `skillCatalogMaxBytes`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields), applied to every server loaded, like `--header`. + ### TUI-specific (OAuth for HTTP servers) The TUI supports OAuth for **SSE** and **Streamable HTTP** servers. Per-server OAuth fields in `mcp.json` (static client id/secret, scopes, enterprise-managed flag) are applied automatically when loaded from `--catalog` or `--config`. Install-wide settings (CIMD, enterprise IdP) come from **`~/.mcp-inspector/storage/client.json`** — the same file the web **Client Settings** dialog writes. You can point at a different file with `--client-config` or `MCP_CLIENT_CONFIG_PATH`. diff --git a/clients/tui/tui.tsx b/clients/tui/tui.tsx index 1768614868..790e0fd7d1 100644 --- a/clients/tui/tui.tsx +++ b/clients/tui/tui.tsx @@ -6,6 +6,7 @@ import { parseKeyValuePair, parseHeaderPair, parseProtocolEra, + skillCatalogLimitParser, } from "@inspector/core/mcp/node/index.js"; import type { ServerProtocolEra } from "@inspector/core/mcp/types.js"; import { loadRunnerClientConfig } from "@inspector/core/client/runner.js"; @@ -48,6 +49,16 @@ export async function runTui(args?: string[]): Promise { "Protocol era to negotiate: legacy, auto, or modern (overrides the file's protocolEra; default legacy)", parseProtocolEra, ) + .option( + "--skill-catalog-max-skills ", + "Skills catalog budget for verification: the most skills one run reads (positive integer; overrides the file's skillCatalogMaxSkills; default 256)", + skillCatalogLimitParser("--skill-catalog-max-skills"), + ) + .option( + "--skill-catalog-max-bytes ", + "Skills catalog budget for verification: the most bytes one run reads (positive integer; overrides the file's skillCatalogMaxBytes; default 64 MiB)", + skillCatalogLimitParser("--skill-catalog-max-bytes"), + ) .option( "--client-id ", "OAuth client ID (static client) for HTTP servers", @@ -86,6 +97,8 @@ export async function runTui(args?: string[]): Promise { cwd?: string; header?: Record; protocolEra?: ServerProtocolEra; + skillCatalogMaxSkills?: number; + skillCatalogMaxBytes?: number; clientId?: string; clientSecret?: string; clientMetadataUrl?: string; @@ -104,6 +117,8 @@ export async function runTui(args?: string[]): Promise { env: options.e, headers: options.header, protocolEra: options.protocolEra, + skillCatalogMaxSkills: options.skillCatalogMaxSkills, + skillCatalogMaxBytes: options.skillCatalogMaxBytes, transport: options.transport, serverUrl: options.serverUrl?.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 7e1e1689ec..70c2b3bd85 100644 --- a/clients/web/src/test/core/mcp/node/config.test.ts +++ b/clients/web/src/test/core/mcp/node/config.test.ts @@ -13,6 +13,7 @@ import { parseKeyValuePair, parseHeaderPair, parseProtocolEra, + skillCatalogLimitParser, withDefaultCatalogPath, resolveServerConfigs, resolveServerSource, @@ -57,6 +58,28 @@ describe("parseProtocolEra", () => { }); }); +describe("skillCatalogLimitParser", () => { + const parse = skillCatalogLimitParser("--skill-catalog-max-skills"); + + it.each([ + ["1", 1], + ["256", 256], + [" 42 ", 42], + ["67108864", 67108864], + ])("accepts %j as %d", (value, expected) => { + expect(parse(value)).toBe(expected); + }); + + it.each(["0", "-1", "1.5", "1e3", "0x10", "", "abc", "9007199254740993"])( + "rejects %j, naming the flag", + (value) => { + expect(() => parse(value)).toThrow( + `Invalid --skill-catalog-max-skills: ${value}. Expected a positive integer.`, + ); + }, + ); +}); + describe("parseHeaderPair", () => { it("splits on the first colon and trims both sides", () => { expect(parseHeaderPair("X-Test: value")).toEqual({ diff --git a/clients/web/src/test/core/mcp/node/servers.test.ts b/clients/web/src/test/core/mcp/node/servers.test.ts index 3cd372d8d7..f6a9b2ea32 100644 --- a/clients/web/src/test/core/mcp/node/servers.test.ts +++ b/clients/web/src/test/core/mcp/node/servers.test.ts @@ -297,6 +297,74 @@ describe("loadServerEntries", () => { }); }); + it("overrides the disk skills catalog budget with --skill-catalog-max-*, preserving the rest", async () => { + const configPath = join(tempDir, "mcp.json"); + writeFileSync( + configPath, + JSON.stringify({ + mcpServers: { + web: { + type: "streamable-http", + url: "http://x/mcp", + skillCatalogMaxSkills: 10, + skillCatalogMaxBytes: 2048, + requestTimeout: 9000, + }, + }, + }), + ); + + const fromFile = await loadServerEntries({ configPath }); + expect(fromFile.web?.settings).toMatchObject({ + skillCatalogMaxSkills: 10, + skillCatalogMaxBytes: 2048, + }); + + const servers = await loadServerEntries({ + configPath, + skillCatalogMaxSkills: 3, + skillCatalogMaxBytes: 4096, + }); + expect(servers.web?.settings).toMatchObject({ + skillCatalogMaxSkills: 3, + skillCatalogMaxBytes: 4096, + requestTimeout: 9000, + }); + + const onlyBytes = await loadServerEntries({ + configPath, + skillCatalogMaxBytes: 1, + }); + expect(onlyBytes.web?.settings).toMatchObject({ + skillCatalogMaxSkills: 10, + skillCatalogMaxBytes: 1, + }); + }); + + it("applies --skill-catalog-max-* to an ad-hoc URL with no other settings", async () => { + const servers = await loadServerEntries({ + serverUrl: "http://x/mcp", + transport: "http", + skillCatalogMaxSkills: 5, + }); + expect(servers.default?.settings).toMatchObject({ + skillCatalogMaxSkills: 5, + headers: [], + connectionTimeout: DEFAULT_CONNECTION_TIMEOUT_MS, + }); + expect(servers.default?.settings?.skillCatalogMaxBytes).toBeUndefined(); + + const bytesOnly = await loadServerEntries({ + serverUrl: "http://x/mcp", + transport: "http", + skillCatalogMaxBytes: 7, + }); + expect(bytesOnly.default?.settings).toMatchObject({ + skillCatalogMaxBytes: 7, + headers: [], + }); + }); + it("gives an ad-hoc target with neither flag no settings (legacy default)", async () => { const servers = await loadServerEntries({ target: ["my-server"] }); expect(servers.default?.settings).toBeUndefined(); diff --git a/core/mcp/node/config.ts b/core/mcp/node/config.ts index 3f23913f0d..9dec815b15 100644 --- a/core/mcp/node/config.ts +++ b/core/mcp/node/config.ts @@ -9,6 +9,7 @@ import type { StreamableHttpServerConfig, } from "../types.js"; import { isProtocolEra, normalizeServerType } from "../serverList.js"; +import { isSkillCatalogLimit } from "../skills.js"; import type { ServerProtocolEra } from "../types.js"; import { toRecord } from "../../json/jsonUtils.js"; @@ -101,6 +102,31 @@ export function parseProtocolEra(value: string): ServerProtocolEra { return value; } +/** + * Build the Commander coerce for a skills-catalog budget flag + * (`--skill-catalog-max-skills` / `--skill-catalog-max-bytes`), so the CLI and + * TUI can set the per-server budget the web exposes in Server Settings (#2420). + * Accepts only a plain run of decimal digits denoting a positive safe integer — + * the values {@link isSkillCatalogLimit} keeps when it reads `mcp.json` — so + * `0`, `-1`, `1.5`, `1e3` and `0x10` are rejected up front, rather than falling + * back to the default the way a hand-edited file value does. `flag` names the + * option in the error. Pure function; no Commander dependency. + */ +export function skillCatalogLimitParser( + flag: string, +): (value: string) => number { + return (value: string): number => { + const trimmed = value.trim(); + const n = /^\d+$/.test(trimmed) ? Number(trimmed) : NaN; + if (!isSkillCatalogLimit(n)) { + throw new Error( + `Invalid ${flag}: ${value}. Expected a positive integer.`, + ); + } + return n; + }; +} + /** On-disk contents of a freshly seeded empty catalog (pretty-printed). */ const EMPTY_CATALOG_CONTENT = `${JSON.stringify({ mcpServers: {} }, null, 2)}\n`; diff --git a/core/mcp/node/index.ts b/core/mcp/node/index.ts index f8fa91fcf1..339cbf6487 100644 --- a/core/mcp/node/index.ts +++ b/core/mcp/node/index.ts @@ -2,6 +2,7 @@ export { parseKeyValuePair, parseHeaderPair, parseProtocolEra, + skillCatalogLimitParser, withDefaultCatalogPath, resolveServerConfigs, getNamedServerConfigs, diff --git a/core/mcp/node/servers.ts b/core/mcp/node/servers.ts index 8b7a2e348e..dad321c11d 100644 --- a/core/mcp/node/servers.ts +++ b/core/mcp/node/servers.ts @@ -42,6 +42,10 @@ export type ServerLoadOptions = ServerConfigOptions & { /** `--protocol-era`: overrides the file's `protocolEra` (or the legacy * default) the way `--header` overrides its headers (#2208). */ protocolEra?: ServerProtocolEra; + /** `--skill-catalog-max-skills` / `--skill-catalog-max-bytes`: override the + * file's per-server skills catalog budget the same way (#2420). */ + skillCatalogMaxSkills?: number; + skillCatalogMaxBytes?: number; /** Test injection; defaults to the selected store (see * `secret-store-selection.ts`) for catalog/config loads. */ secretStore?: SecretStore; @@ -81,14 +85,17 @@ function defaultServerSettings(): InspectorServerSettings { } /** - * Overlay `--header` / `--protocol-era` onto the settings lifted from the file - * (or onto nothing, for an ad-hoc target). Only those two fields are overridden - * — timeouts, OAuth, and the rest of the file's settings are preserved. Returns - * `base` untouched when neither flag was given. + * Overlay `--header` / `--protocol-era` / `--skill-catalog-max-*` onto the + * settings lifted from the file (or onto nothing, for an ad-hoc target). Only + * those fields are overridden — timeouts, OAuth, and the rest of the file's + * settings are preserved. Returns `base` untouched when no such flag was given. */ function mergeSettings( base: InspectorServerSettings | undefined, - overrides: Pick, + overrides: Pick< + ServerLoadOptions, + "headers" | "protocolEra" | "skillCatalogMaxSkills" | "skillCatalogMaxBytes" + >, ): InspectorServerSettings | undefined { let settings = base; const fromHeaders = headersToServerSettings(overrides.headers); @@ -103,6 +110,18 @@ function mergeSettings( protocolEra: overrides.protocolEra, }; } + if (overrides.skillCatalogMaxSkills !== undefined) { + settings = { + ...(settings ?? defaultServerSettings()), + skillCatalogMaxSkills: overrides.skillCatalogMaxSkills, + }; + } + if (overrides.skillCatalogMaxBytes !== undefined) { + settings = { + ...(settings ?? defaultServerSettings()), + skillCatalogMaxBytes: overrides.skillCatalogMaxBytes, + }; + } return settings; } @@ -144,10 +163,10 @@ export async function loadServerEntries( env: serverOptions.env, cwd: serverOptions.cwd, }), - // Deliberate broadcast: a single `--header` set (and `--protocol-era`) - // is merged into EVERY server in the catalog/config (fine for the - // common single-server case; for multi-server files, prefer per-server - // settings in the file itself). + // Deliberate broadcast: a single `--header` set (and `--protocol-era`, + // `--skill-catalog-max-*`) is merged into EVERY server in the + // catalog/config (fine for the common single-server case; for + // multi-server files, prefer per-server settings in the file itself). settings: mergeSettings(entry.settings, serverOptions), }; } diff --git a/docs/mcp-server-configuration.md b/docs/mcp-server-configuration.md index 3a9c5add14..18379f0dcd 100644 --- a/docs/mcp-server-configuration.md +++ b/docs/mcp-server-configuration.md @@ -126,6 +126,7 @@ Without a `--` on the line the target is only the leading run of **non-dash** to | `-e ` | Environment variable for a stdio server; repeatable | | | `--header "Name: Value"` | HTTP header for an HTTP/SSE server; repeatable | On web, requires an ad-hoc HTTP/SSE server | | `--protocol-era ` | `legacy`, `auto`, or `modern` — the era to negotiate | Sets [`protocolEra`](#inspector-specific-per-server-fields) without a file, on any transport. CLI/TUI also override a file's `protocolEra` with it; web, like `--header`, requires an ad-hoc target | +| `--skill-catalog-max-skills ` / `--skill-catalog-max-bytes ` | Skills catalog budget for a verification run; positive integer | **CLI/TUI only.** Set or override [`skillCatalogMaxSkills` / `skillCatalogMaxBytes`](#inspector-specific-per-server-fields) for every loaded server; web sets them per server in Server Settings instead | | `[target...]` | Positional command or URL for one ad-hoc server | | **`MCP_CATALOG_PATH` and ad-hoc targets differ by client.** The **CLI** ignores the env var when an ad-hoc target is given (a positional command, `--server-url`, or `--transport`), so a shell that exports it can still run one-off ad-hoc invocations without tripping the catalog/ad-hoc conflict. **Web and TUI read it unconditionally** — with it exported, an ad-hoc invocation such as `mcp-inspector --tui node build/index.js` is rejected as `--catalog cannot be combined with an ad-hoc server URL/command`. Unset the variable for that invocation on those two surfaces. From 58a58a945f1f4b79bff2b85ded4e8e59656ad3b2 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 05:06:23 -0400 Subject: [PATCH 2/5] fix(core): name the decimal-digit and safe-integer limits in the budget flag error (#2420) Copilot review: `1e3` and `9007199254740993` are positive integers that the parser rejects by design, so 'Expected a positive integer' did not tell the user how to correct them. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/cli/__tests__/programmatic-ergonomics.test.ts | 2 +- clients/web/src/test/core/mcp/node/config.test.ts | 2 +- core/mcp/node/config.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/cli/__tests__/programmatic-ergonomics.test.ts b/clients/cli/__tests__/programmatic-ergonomics.test.ts index 24503317bc..f57c91359b 100644 --- a/clients/cli/__tests__/programmatic-ergonomics.test.ts +++ b/clients/cli/__tests__/programmatic-ergonomics.test.ts @@ -349,7 +349,7 @@ describe("--skill-catalog-max-skills / --skill-catalog-max-bytes (#2420)", () => ]); expectCliFailure(result); expect(result.stderr).toContain( - `Invalid ${flag}: ${value}. Expected a positive integer.`, + `Invalid ${flag}: ${value}. Expected a positive integer written as plain decimal digits, at most 9007199254740991.`, ); }); }); 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 70c2b3bd85..f450d68155 100644 --- a/clients/web/src/test/core/mcp/node/config.test.ts +++ b/clients/web/src/test/core/mcp/node/config.test.ts @@ -74,7 +74,7 @@ describe("skillCatalogLimitParser", () => { "rejects %j, naming the flag", (value) => { expect(() => parse(value)).toThrow( - `Invalid --skill-catalog-max-skills: ${value}. Expected a positive integer.`, + `Invalid --skill-catalog-max-skills: ${value}. Expected a positive integer written as plain decimal digits, at most 9007199254740991.`, ); }, ); diff --git a/core/mcp/node/config.ts b/core/mcp/node/config.ts index 9dec815b15..36ceca7b7b 100644 --- a/core/mcp/node/config.ts +++ b/core/mcp/node/config.ts @@ -120,7 +120,7 @@ export function skillCatalogLimitParser( const n = /^\d+$/.test(trimmed) ? Number(trimmed) : NaN; if (!isSkillCatalogLimit(n)) { throw new Error( - `Invalid ${flag}: ${value}. Expected a positive integer.`, + `Invalid ${flag}: ${value}. Expected a positive integer written as plain decimal digits, at most ${Number.MAX_SAFE_INTEGER}.`, ); } return n; From 93cf3ed323b42de61ff4e162e6c94992156ea2dc Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 05:36:44 -0400 Subject: [PATCH 3/5] docs(tui): say the skills budget flags do not bound the one-skill Skills pane (#2420) Copilot review: SkillsTab verifies one skill per gesture, and the catalog budget is a run-level bound, so the TUI flags (like the file values) have no observable effect there today. Say so in the README and --help rather than claiming the pane reads within the budget; making the pane verify the whole catalog is tracked in #2590. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/tui/README.md | 2 +- clients/tui/tui.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/tui/README.md b/clients/tui/README.md index cf60bd3c03..8f28871bf4 100644 --- a/clients/tui/README.md +++ b/clients/tui/README.md @@ -31,7 +31,7 @@ npx @modelcontextprotocol/inspector --tui --config mcp.json # read-only sessi 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 **empty** if missing; default `~/.mcp-inspector/mcp.json` or `MCP_CATALOG_PATH`), `--config` (read-only session, errors if absent), `-e`, `--cwd`, `--header`, `--protocol-era` (`legacy`/`auto`/`modern`; sets the era an ad-hoc server negotiates, or overrides a file's `protocolEra`), `--transport`, `--server-url`, and the positional `[target...]`. `--catalog` and `--config` are mutually exclusive, and neither combines with an ad-hoc target. -`--skill-catalog-max-skills ` and `--skill-catalog-max-bytes ` set the skills catalog budget the **Skills** pane's verification reads within (defaults `256` skills and `67108864` bytes, 64 MiB) — the same per-server setting the web client exposes in Server Settings. Each takes a positive integer (anything else is rejected at startup) and overrides the file's [`skillCatalogMaxSkills` / `skillCatalogMaxBytes`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields), applied to every server loaded, like `--header`. +`--skill-catalog-max-skills ` and `--skill-catalog-max-bytes ` set the per-server skills catalog budget (defaults `256` skills and `67108864` bytes, 64 MiB), the same setting the web client exposes in Server Settings. Each takes a positive integer (anything else is rejected at startup) and overrides the file's [`skillCatalogMaxSkills` / `skillCatalogMaxBytes`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields) for every server loaded, like `--header`. ⚠️ The budget bounds a verification run that covers **several** skills. The **Skills** pane verifies one skill per **Enter**, which always reads that skill in full, so today neither the flags nor the file values change what the pane does ([#2590](https://github.com/modelcontextprotocol/inspector/issues/2590)). To verify a whole catalog under the budget, use the CLI's `--verify`. ### TUI-specific (OAuth for HTTP servers) diff --git a/clients/tui/tui.tsx b/clients/tui/tui.tsx index 790e0fd7d1..565af5b459 100644 --- a/clients/tui/tui.tsx +++ b/clients/tui/tui.tsx @@ -51,12 +51,12 @@ export async function runTui(args?: string[]): Promise { ) .option( "--skill-catalog-max-skills ", - "Skills catalog budget for verification: the most skills one run reads (positive integer; overrides the file's skillCatalogMaxSkills; default 256)", + "Skills catalog budget: the most skills one multi-skill verification run reads (positive integer; overrides the file's skillCatalogMaxSkills; default 256). The Skills pane verifies one skill at a time, so it is not bounded by this today (#2590)", skillCatalogLimitParser("--skill-catalog-max-skills"), ) .option( "--skill-catalog-max-bytes ", - "Skills catalog budget for verification: the most bytes one run reads (positive integer; overrides the file's skillCatalogMaxBytes; default 64 MiB)", + "Skills catalog budget: the most bytes one multi-skill verification run reads (positive integer; overrides the file's skillCatalogMaxBytes; default 64 MiB). Not applied by the one-skill-at-a-time Skills pane today (#2590)", skillCatalogLimitParser("--skill-catalog-max-bytes"), ) .option( From 783e3e92f936330025266e1ddf480dba318b7597 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 05:38:42 -0400 Subject: [PATCH 4/5] test(tui): cover runTui forwarding of --skill-catalog-max-* to the loader (#2420) Copilot review: tui.tsx is outside the TUI coverage include and tui-servers.test.ts calls the loader directly, so a misspelt option or a dropped forwarding field would have passed. Mock the loader to capture the options runTui builds and assert both flags arrive parsed. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/tui/__tests__/tui-entry-flags.test.ts | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 clients/tui/__tests__/tui-entry-flags.test.ts diff --git a/clients/tui/__tests__/tui-entry-flags.test.ts b/clients/tui/__tests__/tui-entry-flags.test.ts new file mode 100644 index 0000000000..a68cc609b3 --- /dev/null +++ b/clients/tui/__tests__/tui-entry-flags.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect, vi } from "vitest"; +import type { ServerLoadOptions } from "@inspector/core/mcp/node/servers.js"; + +/** + * `runTui`'s Commander registration and the forwarding of parsed flags into + * the shared server loader (#2420). `tui.tsx` sits outside the TUI coverage + * `include` (`src/**`), and `tui-servers.test.ts` calls the loader directly, + * so without this a misspelt option or a dropped forwarding field would pass + * every other test. + * + * The loader is mocked to throw a sentinel carrying the options it received, + * which stops `runTui` before it touches stdout or renders Ink. + */ +class Captured extends Error { + constructor(readonly options: ServerLoadOptions) { + super("captured"); + } +} + +vi.mock("../src/tui-servers.js", () => ({ + loadTuiServers: (options: ServerLoadOptions) => { + throw new Captured(options); + }, +})); + +async function loaderOptionsFor(argv: string[]): Promise { + const { runTui } = await import("../tui.js"); + try { + await runTui(["node", "mcp-inspector-tui", ...argv]); + } catch (err) { + if (err instanceof Captured) return err.options; + throw err; + } + throw new Error("runTui returned without calling the server loader"); +} + +describe("runTui --skill-catalog-max-* forwarding (#2420)", () => { + it("forwards both budget flags, parsed as numbers, to the server loader", async () => { + const options = await loaderOptionsFor([ + "--server-url", + "http://127.0.0.1:1/mcp", + "--transport", + "http", + "--skill-catalog-max-skills", + "3", + "--skill-catalog-max-bytes", + "4096", + ]); + expect(options.skillCatalogMaxSkills).toBe(3); + expect(options.skillCatalogMaxBytes).toBe(4096); + }); + + it("leaves both unset when the flags are absent", async () => { + const options = await loaderOptionsFor([ + "--server-url", + "http://127.0.0.1:1/mcp", + "--transport", + "http", + ]); + expect(options.skillCatalogMaxSkills).toBeUndefined(); + expect(options.skillCatalogMaxBytes).toBeUndefined(); + }); +}); From f8b51d07798134618db024feaf9cdc2c19350018 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 06:17:48 -0400 Subject: [PATCH 5/5] fix(cli): require --verify for --skill-catalog-max-* (#2420) Copilot review: the budget is read only by verifySkills, so accepting the flags on a run without --verify would make a job look bounded when nothing is bounded. Reject them there, as --require-digests already is. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/cli/README.md | 4 ++-- .../__tests__/programmatic-ergonomics.test.ts | 17 +++++++++++++++++ clients/cli/src/cli.ts | 9 +++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/clients/cli/README.md b/clients/cli/README.md index c6799c4696..e9ba9b913b 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -125,8 +125,8 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en | `--strict` | With `--method tools/list`: report tool-schema portability problems in full (path, issue, suggested fix) on stderr, and exit `6` if any is error-severity. Without it, a one-line count is printed instead. See [Schema portability](#schema-portability---strict). | | `--verify` | With `--method skills/list` or `--method skills/get`: run the SEP-2640 conformance, digest and frontmatter checks over the skills returned, emit one JSON report per skill on stdout, and exit `7` if any fails. See [Skill verification](#skill-verification---verify). | | `--require-digests` | With `--verify`: exit `9` when a skill advertises no digests (`resources: "dynamic"`), instead of reporting it `unverifiable` and exiting `0`. See [Skill verification](#skill-verification---verify). | -| `--skill-catalog-max-skills ` | The skills catalog budget for `--verify`: the most skills whose files one run reads (default `256`). Positive integer; anything else is rejected before connecting. Overrides the server's [`skillCatalogMaxSkills`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields) from the catalog/config file — the same setting the web client exposes per server in Server Settings. Skills past the budget are reported `incomplete` (exit `8`). | -| `--skill-catalog-max-bytes ` | The skills catalog budget for `--verify`: the most bytes one run reads across all skills (default `67108864`, 64 MiB). Positive integer. Overrides the server's [`skillCatalogMaxBytes`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields). | +| `--skill-catalog-max-skills ` | With `--verify` (required; rejected without it, since it would bound nothing): the skills catalog budget, the most skills whose files one run reads (default `256`). Positive integer; anything else is rejected before connecting. Overrides the server's [`skillCatalogMaxSkills`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields) from the catalog/config file — the same setting the web client exposes per server in Server Settings. Skills past the budget are reported `incomplete` (exit `8`). | +| `--skill-catalog-max-bytes ` | With `--verify` (required): the skills catalog budget, the most bytes one run reads across all skills (default `67108864`, 64 MiB). Positive integer. Overrides the server's [`skillCatalogMaxBytes`](../../docs/mcp-server-configuration.md#inspector-specific-per-server-fields). | | `--format ` | Output format. `text` (default) pretty-prints the result. `json` emits a single JSON object on stdout (`{ "result": … }`, plus `{ "appInfo": … }` as a sibling key for App tools) with no banners, so the whole output pipes cleanly into `jq`. | | `--relogin` | Delete stored OAuth for this server URL from the shared store before connect; interactive login still only runs if the server requires auth. Requires an HTTP/SSE URL (rejected for stdio). Conflicts with `--stored-auth-only` / `--use-stored-auth` / `--wait-for-auth` / catalog short-circuits. | | `--no-revoke` | With `--relogin`, skip the [RFC 7009](https://datatracker.ietf.org/doc/html/rfc7009) revocation request that would otherwise end the grant at the authorization server when the local state is deleted. The per-server `oauth.revokeOnClear` setting is the persistent form of the same opt-out; either one is enough to skip it. See [Revoking on `--relogin`](#revoking-on---relogin). | diff --git a/clients/cli/__tests__/programmatic-ergonomics.test.ts b/clients/cli/__tests__/programmatic-ergonomics.test.ts index f57c91359b..579e83c47f 100644 --- a/clients/cli/__tests__/programmatic-ergonomics.test.ts +++ b/clients/cli/__tests__/programmatic-ergonomics.test.ts @@ -352,6 +352,23 @@ describe("--skill-catalog-max-skills / --skill-catalog-max-bytes (#2420)", () => `Invalid ${flag}: ${value}. Expected a positive integer written as plain decimal digits, at most 9007199254740991.`, ); }); + + it.each(["--skill-catalog-max-skills", "--skill-catalog-max-bytes"])( + "rejects %s without --verify, where it would bound nothing", + async (flag) => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + flag, + "5", + "--method", + "tools/list", + ]); + expectCliFailure(result); + expect(result.stderr).toContain(`${flag} requires --verify.`); + }, + ); }); /** The `outcome` of each NDJSON `--verify` report line. */ diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 7fb20b9313..736ed848c3 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -1054,6 +1054,15 @@ async function parseArgs(argv?: string[]): Promise { if (options.requireDigests && !options.verify) { throw new Error("--require-digests requires --verify."); } + // The skills catalog budget is read only by `verifySkills`, so without + // `--verify` it bounds nothing — and a job that set it would look bounded + // when it is not (#2420). + if (options.skillCatalogMaxSkills !== undefined && !options.verify) { + throw new Error("--skill-catalog-max-skills requires --verify."); + } + if (options.skillCatalogMaxBytes !== undefined && !options.verify) { + throw new Error("--skill-catalog-max-bytes requires --verify."); + } // `--advertise-apps` is checked here for the same reason: it shapes the // `initialize` handshake, and the short-circuit paths below never open an