fix(cli): apply a server's persisted mcp.json metadata
The web client and the TUI both read `savedSettings.metadata` and pass it
as `InspectorClientOptions.defaultMetadata`. The CLI never did, and
`InspectorClient` only reads the option rather than falling back to
`serverSettings.metadata` — so a per-server `metadata` entry was silently
inert for `--cli` runs.
Pass it beside the existing `roots:`, guarded on a non-empty map the same
way web and the TUI guard theirs, so `{}` still means "no defaults".
Precedence is unchanged: call-time keys override defaults in `mergeMeta`,
so `--metadata` stays per-invocation and wins on a key collision.
Covered by three catalog-driven tests asserting against the test server's
recorded wire requests. `createTestConfig` is retyped `StoredMCPServer` —
the accurate on-disk shape — since a fixture that cannot express the
per-server keys cannot cover the settings that reach the client.
Closes #2093
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
9d0448b644
commit
7da20dcfd0
5 changed files with 168 additions and 7 deletions
|
|
@ -116,7 +116,7 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en
|
|||
| `--prompt-name <name>` | Prompt name (for `prompts/get`). |
|
||||
| `--prompt-args <key=value>` | Prompt arguments; repeat for multiple. |
|
||||
| `--log-level <level>` | Logging level for `logging/setLevel` (e.g. `debug`, `info`). |
|
||||
| `--metadata <key=value>` | General `_meta` entries (key=value); applied to all methods. The value is JSON-parsed when it parses, so `trace={"id":"abc"}` sends a real object and `n=3` a number; anything that is not valid JSON is sent as the literal string. |
|
||||
| `--metadata <key=value>` | General `_meta` entries (key=value); applied to all methods. The value is JSON-parsed when it parses, so `trace={"id":"abc"}` sends a real object and `n=3` a number; anything that is not valid JSON is sent as the literal string. Merged **over** the server's persisted `metadata` from `mcp.json`, which is sent on every request without this flag (#2093). |
|
||||
| `--tool-metadata <key=value>` | Tool-specific `_meta` entries for `tools/call`. Same JSON-parsed value handling as `--metadata`. |
|
||||
| `--connect-timeout <ms>` | Connection timeout in ms. Defaults to `15000` for ad-hoc `--server-url`/target runs (so a black-holed host fails fast) and to the file-level timeout for `--catalog`/`--config` runs. `0` disables the timeout. |
|
||||
| `--app-info` | Probe a tool's MCP App UI metadata without invoking it. With `--method tools/call --tool-name <name>`: prints one JSON line (`hasApp`, `resourceUri`, `csp`, `permissions`, `domain`, …) and exits `0` if the tool has an app or `2` (`no_app`) if not. With `--method tools/list`: emits NDJSON — one app-info line per tool over a single connection. |
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import * as path from "path";
|
|||
import * as os from "os";
|
||||
import * as crypto from "crypto";
|
||||
import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
|
||||
import type { MCPServerConfig } from "@inspector/core/mcp/index.js";
|
||||
import type { StoredMCPServer } from "@inspector/core/mcp/types.js";
|
||||
|
||||
/**
|
||||
* Sentinel value for tests that don't need a real server
|
||||
|
|
@ -61,10 +61,16 @@ function cleanupTempDir(dir: string) {
|
|||
}
|
||||
|
||||
/**
|
||||
* Create a test config file
|
||||
* Create a test config file.
|
||||
*
|
||||
* Entries are `StoredMCPServer`, not bare `MCPServerConfig` — the on-disk
|
||||
* `mcp.json` shape carries the Inspector-specific per-server keys (`metadata`,
|
||||
* `headers`, `roots`, `protocolEra`, …) directly alongside `type`/`url`, and a
|
||||
* fixture that cannot express them cannot cover the settings that reach the
|
||||
* client from the catalog (#2093).
|
||||
*/
|
||||
export function createTestConfig(config: {
|
||||
mcpServers: Record<string, MCPServerConfig>;
|
||||
mcpServers: Record<string, StoredMCPServer>;
|
||||
}): string {
|
||||
const tempDir = createTempDir("mcp-inspector-config-");
|
||||
const configPath = path.join(tempDir, "config.json");
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { runCli } from "./helpers/cli-runner.js";
|
||||
import type { StrictJsonObject } from "@inspector/core/json/jsonUtils.js";
|
||||
import {
|
||||
expectCliSuccess,
|
||||
expectCliFailure,
|
||||
|
|
@ -11,7 +12,11 @@ import {
|
|||
createAddTool,
|
||||
createTestServerInfo,
|
||||
} from "@modelcontextprotocol/inspector-test-server";
|
||||
import { NO_SERVER_SENTINEL } from "./helpers/fixtures.js";
|
||||
import {
|
||||
NO_SERVER_SENTINEL,
|
||||
createTestConfig,
|
||||
deleteConfigFile,
|
||||
} from "./helpers/fixtures.js";
|
||||
|
||||
describe("Metadata Tests", () => {
|
||||
describe("General Metadata", () => {
|
||||
|
|
@ -1028,4 +1033,143 @@ describe("Metadata Tests", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
describe("Per-server metadata from mcp.json (#2093)", () => {
|
||||
/**
|
||||
* Write a one-server catalog pointing at `url`, carrying `metadata` as the
|
||||
* on-disk per-server key. Returns the catalog path; the caller deletes it.
|
||||
*/
|
||||
function writeCatalogWithMetadata(
|
||||
url: string,
|
||||
metadata: StrictJsonObject,
|
||||
): string {
|
||||
return createTestConfig({
|
||||
mcpServers: {
|
||||
"meta-server": {
|
||||
type: "streamable-http",
|
||||
url,
|
||||
metadata,
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
it("applies a server's persisted metadata to every request", async () => {
|
||||
// The setting belongs to the server, not to the client that reads it —
|
||||
// web and the TUI already honored it, and the CLI silently did not,
|
||||
// because `InspectorClient` reads `defaultMetadata` rather than falling
|
||||
// back to `serverSettings.metadata`.
|
||||
const server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
});
|
||||
let catalogPath: string | undefined;
|
||||
|
||||
try {
|
||||
await server.start();
|
||||
catalogPath = writeCatalogWithMetadata(server.url, { tenant: "acme" });
|
||||
|
||||
const result = await runCli([
|
||||
"--catalog",
|
||||
catalogPath,
|
||||
"--server",
|
||||
"meta-server",
|
||||
"--cli",
|
||||
"--method",
|
||||
"tools/list",
|
||||
]);
|
||||
|
||||
expectCliSuccess(result);
|
||||
expect(expectValidJson(result)).toHaveProperty("tools");
|
||||
|
||||
const toolsListRequest = server
|
||||
.getRecordedRequests()
|
||||
.find((r) => r.method === "tools/list");
|
||||
expect(toolsListRequest).toBeDefined();
|
||||
expect(toolsListRequest?.metadata).toEqual({ tenant: "acme" });
|
||||
} finally {
|
||||
if (catalogPath) deleteConfigFile(catalogPath);
|
||||
await server.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("merges --metadata over the persisted defaults", async () => {
|
||||
// `--metadata` is per-invocation and stays that way: non-colliding keys
|
||||
// merge with the catalog's, and a colliding one wins (call-time keys
|
||||
// override defaults in `InspectorClient.mergeMeta`).
|
||||
const server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
});
|
||||
let catalogPath: string | undefined;
|
||||
|
||||
try {
|
||||
await server.start();
|
||||
catalogPath = writeCatalogWithMetadata(server.url, {
|
||||
tenant: "acme",
|
||||
region: "eu",
|
||||
});
|
||||
|
||||
const result = await runCli([
|
||||
"--catalog",
|
||||
catalogPath,
|
||||
"--server",
|
||||
"meta-server",
|
||||
"--cli",
|
||||
"--method",
|
||||
"tools/list",
|
||||
"--metadata",
|
||||
"tenant=override",
|
||||
]);
|
||||
|
||||
expectCliSuccess(result);
|
||||
|
||||
const toolsListRequest = server
|
||||
.getRecordedRequests()
|
||||
.find((r) => r.method === "tools/list");
|
||||
expect(toolsListRequest?.metadata).toEqual({
|
||||
tenant: "override",
|
||||
region: "eu",
|
||||
});
|
||||
} finally {
|
||||
if (catalogPath) deleteConfigFile(catalogPath);
|
||||
await server.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("sends no _meta when the persisted metadata is empty", async () => {
|
||||
// `{}` means "no defaults" — an empty map must not put a bare `_meta` on
|
||||
// the wire, matching what the option-less CLI has always sent.
|
||||
const server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
});
|
||||
let catalogPath: string | undefined;
|
||||
|
||||
try {
|
||||
await server.start();
|
||||
catalogPath = writeCatalogWithMetadata(server.url, {});
|
||||
|
||||
const result = await runCli([
|
||||
"--catalog",
|
||||
catalogPath,
|
||||
"--server",
|
||||
"meta-server",
|
||||
"--cli",
|
||||
"--method",
|
||||
"tools/list",
|
||||
]);
|
||||
|
||||
expectCliSuccess(result);
|
||||
|
||||
const toolsListRequest = server
|
||||
.getRecordedRequests()
|
||||
.find((r) => r.method === "tools/list");
|
||||
expect(toolsListRequest).toBeDefined();
|
||||
expect(toolsListRequest?.metadata).toBeUndefined();
|
||||
} finally {
|
||||
if (catalogPath) deleteConfigFile(catalogPath);
|
||||
await server.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -169,6 +169,17 @@ async function callMethod(
|
|||
// the change at all: the SDK refuses `roots/list_changed` from a client
|
||||
// that never declared it, which `setRoots` logged as a send failure (#1797).
|
||||
roots: cleanRoots(serverSettings?.roots ?? []),
|
||||
// Per-server default `_meta` from mcp.json, exactly as web (`App.tsx`) and
|
||||
// the TUI pass it — the setting belongs to the server, not to the client
|
||||
// that happens to read it, and `InspectorClient` only reads the option
|
||||
// rather than falling back to `serverSettings.metadata` (#2093). Already a
|
||||
// JSON object (#1910), so there is no pair-array flattening left to do;
|
||||
// `{}` means "no defaults". `--metadata` stays per-invocation and wins on a
|
||||
// key collision, since call-time keys override defaults in `mergeMeta`.
|
||||
...(serverSettings?.metadata &&
|
||||
Object.keys(serverSettings.metadata).length > 0 && {
|
||||
defaultMetadata: serverSettings.metadata,
|
||||
}),
|
||||
serverSettings,
|
||||
// Per-server protocol era (SEP §7.8) from mcp.json → SDK versionNegotiation.
|
||||
// Absent era defaults to legacy in the InspectorClient constructor (#1626).
|
||||
|
|
|
|||
|
|
@ -168,7 +168,7 @@ These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on w
|
|||
| `protocolEra` | `"legacy"` | `"legacy"` \| `"auto"` \| `"modern"` — which protocol era to negotiate, orthogonal to the transport |
|
||||
| `modernLogLevel` | `"debug"` | Per-request log level stamped on modern connections, or `"off"`. Legacy connections ignore it |
|
||||
| `roots` | — | Roots advertised via the `roots` client capability; each is `{ uri, name? }` |
|
||||
| `metadata` | — | Default `_meta` payload merged into every outgoing request — a JSON object; values may be any JSON except for the reserved keys noted below. Web and TUI only today; see the CLI note below |
|
||||
| `metadata` | — | Default `_meta` payload merged into every outgoing request — a JSON object; values may be any JSON except for the reserved keys noted below. Sent by all three clients |
|
||||
| `connectionTimeout` / `requestTimeout` | — | Timeouts in ms |
|
||||
| `taskTtl` | `60000` | TTL in ms for tasks created via "Run as task" (`DEFAULT_TASK_TTL_MS`) |
|
||||
| `autoRefreshOnListChanged` | `false` | Refresh lists automatically on `*/list_changed` instead of only flagging the indicator |
|
||||
|
|
@ -189,7 +189,7 @@ These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on w
|
|||
|
||||
> **A few keys are reserved and keep their own shape.** "Any JSON" holds for keys you invent, not for the ones the protocol defines. In the current SDK those are `progressToken` (a string or a **safe** integer — one JavaScript represents exactly, so nothing past `Number.MAX_SAFE_INTEGER`) and `io.modelcontextprotocol/related-task` (`{ "taskId": "<string>" }`). A reserved key whose value does not match is **dropped from the outgoing request**, with a warning naming the key — because a conforming server rejects the _entire_ request over one bad reserved member, rather than ignoring that key. Everything else in the payload is sent unchanged.
|
||||
|
||||
> **Not yet applied by the CLI.** The web client and the TUI both send this payload on every request. The CLI does not — it never passes the persisted value to its client, so a `metadata` entry is currently inert for `--cli` runs ([#2093](https://github.com/modelcontextprotocol/inspector/issues/2093)). Use the per-invocation `--metadata` flag there in the meantime.
|
||||
> **All three clients send it.** Web, TUI, and CLI alike merge this payload into every outgoing request — the setting belongs to the server, not to the client that happens to read it. The CLI was the exception until [#2093](https://github.com/modelcontextprotocol/inspector/issues/2093): it never passed the persisted value to its client, so a `metadata` entry was silently inert for `--cli` runs. The CLI's per-invocation `--metadata` flag still applies on top, and its keys win on a collision.
|
||||
|
||||
> **Reading an older file.** Before #1910 this field was a `[{ "key": …, "value": … }]` pair array. That shape is still **read**, so an existing `mcp.json` keeps working unchanged; it is never written back, so the field is rewritten as an object the next time the Inspector saves the entry.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue