diff --git a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx index 2d72e026..85643475 100644 --- a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx +++ b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx @@ -158,6 +158,26 @@ describe("JsonObjectInput", () => { return el; } + it("gives two instances distinct DOM ids", () => { + // react-ace uses `name` as the container id; a fixed value would put + // duplicate ids in the document as soon as a second editor renders. + renderWithMantine( + <> + {}} /> + {}} /> + , + ); + const containerIds = Array.from( + document.querySelectorAll(".ace_editor"), + ).map((n) => n.id); + const inputIds = Array.from( + document.querySelectorAll("textarea.ace_text-input"), + ).map((n) => n.id); + expect(containerIds).toHaveLength(2); + expect(new Set([...containerIds, ...inputIds]).size).toBe(4); + expect(containerIds.concat(inputIds).every((id) => id !== "")).toBe(true); + }); + it("lets the visible label focus the editor", () => { renderWithMantine( ` points at. + name={`${wrapperId}-editor`} value={draft} onChange={handleChange} width="100%" diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 4abb41e7..2e03a93b 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -732,7 +732,12 @@ export function ServerSettingsForm({ Metadata merged into the _meta field of every MCP request to this server, as a JSON object. Values may be any JSON — objects, arrays, numbers, booleans, null — not just - strings. Leave it as {"{}"} to send none. + strings. Leave it as {"{}"} to send none. The keys + the protocol reserves keep their own shape:{" "} + progressToken must be a string or an integer, and{" "} + io.modelcontextprotocol/related-task an object with a + string taskId. A reserved key that does not match is + dropped from the request rather than sent. { }, ); + it.each([ + ["an array", [1]], + ["a string", "t1"], + ["an object with a non-string taskId", { taskId: 5 }], + ])( + "drops a related-task member that is %s, keeping its siblings", + async (_label, badRelatedTask) => { + // `progressToken` is not the only reserved member: the pinned SDK also + // constrains `io.modelcontextprotocol/related-task` to + // `{ taskId: string }`. A bad one makes a conforming server reject the + // whole request, so it is dropped the same way. + client = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { + environment: { transport: createTransportNode }, + progress: false, + defaultMetadata: { + "io.modelcontextprotocol/related-task": badRelatedTask, + keep: "me", + }, + }, + ); + const messageLogState = new MessageLogState(client); + await client.connect(); + await client.listTools(); + + const req = messageLogState + .getMessages() + .find( + (m) => + m.direction === "request" && + (m.message as { method?: string }).method === "tools/list", + ); + const meta = metaOf(req!); + expect(meta).not.toHaveProperty("io.modelcontextprotocol/related-task"); + expect(meta.keep).toBe("me"); + messageLogState.destroy(); + }, + ); + + it("keeps a well-formed related-task member", async () => { + client = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { + environment: { transport: createTransportNode }, + progress: false, + defaultMetadata: { + "io.modelcontextprotocol/related-task": { taskId: "task-1" }, + }, + }, + ); + const messageLogState = new MessageLogState(client); + await client.connect(); + await client.listTools(); + + const req = messageLogState + .getMessages() + .find( + (m) => + m.direction === "request" && + (m.message as { method?: string }).method === "tools/list", + ); + expect(metaOf(req!)["io.modelcontextprotocol/related-task"]).toEqual({ + taskId: "task-1", + }); + messageLogState.destroy(); + }); + it("warns with the rejected progressToken's type, never its value", async () => { // This logger is persisted by real clients (the TUI writes it to // `~/.mcp-inspector/auth.log`), and an invalid `progressToken` can now be diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 98290f1e..09c8fa89 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -1,4 +1,10 @@ import { Client } from "@modelcontextprotocol/client"; +// The protocol's own schemas for the reserved `_meta` members, so the client +// validates against the SDK rather than a restatement of it that can drift. +import { + ProgressTokenSchema, + RequestMetaSchema, +} from "@modelcontextprotocol/core"; import type { MCPServerConfig, StderrLogEntry, @@ -382,23 +388,53 @@ function describeJsonType(value: unknown): string { /** * Whether a `_meta.progressToken` value is one the protocol allows. * - * The spec (and the SDK's `ProgressTokenSchema`, - * `z.union([z.string(), z.number().int()])`) constrains this reserved member to - * a string or an **integer** — a float is as invalid as an object. Everything - * else in `_meta` may be any JSON (#1910), which is exactly why this one key - * needs its own check. - * - * `isSafeInteger`, not `isInteger`: zod 4's `.int()` rejects anything past - * `Number.MAX_SAFE_INTEGER`, where the float64 representation is no longer the - * integer it was written as. `Number.isInteger(2 ** 53)` is `true`, so the - * looser check would pass a token the server then rejects — verified against - * the pinned zod. + * Delegates to the SDK's own `ProgressTokenSchema` rather than restating it. + * A hand-rolled check drifts: this was `Number.isInteger` until review pointed + * out that zod 4's `.int()` rejects anything past `Number.MAX_SAFE_INTEGER`, + * so an unsafe integer passed here and was rejected by the server instead. */ function isProgressToken(value: unknown): value is ProgressToken { - return ( - typeof value === "string" || - (typeof value === "number" && Number.isSafeInteger(value)) - ); + return ProgressTokenSchema.safeParse(value).success; +} + +/** + * Drop the reserved `_meta` members whose value the protocol will not accept. + * + * Most of `_meta` is arbitrary JSON (#1910), but a few keys are **reserved** + * and carry their own schema — `progressToken` (`string | integer`) and + * `io.modelcontextprotocol/related-task` (`{ taskId: string }`) in the pinned + * SDK. Before the widening every value was a string, so only `progressToken` + * could even be wrong; now any of them can, and one bad reserved member makes a + * conforming server reject the **whole request** rather than ignore the key. + * + * Validated with the SDK's `RequestMetaSchema` instead of a hand-maintained + * list, which is the point: the schema is loose, so unknown keys pass through + * untouched, and a member the SDK constrains in a later release is covered here + * without anyone remembering to add it. Zod reports the offending top-level key + * as `issue.path[0]`, so only that member is removed — siblings survive, and + * the request stays well-formed because every reserved member is optional. + */ +function dropInvalidReservedMeta( + meta: RequestMetadata, + logger: InspectorLogger, +): RequestMetadata { + const result = RequestMetaSchema.safeParse(meta); + if (result.success) return meta; + + const cleaned = { ...meta }; + for (const issue of result.error.issues) { + const key = issue.path[0]; + if (typeof key !== "string" || !(key in cleaned)) continue; + // The key and the reason, never the value: `_meta` can carry credentials, + // and this logger is persisted by real clients (the TUI writes it to + // `~/.mcp-inspector/auth.log`). + logger.warn( + { key, received: describeJsonType(cleaned[key]) }, + "Dropping reserved `_meta` member — it does not match the protocol schema.", + ); + delete cleaned[key]; + } + return cleaned; } const MODERN_RECONNECT_BASE_MS = 500; @@ -1085,27 +1121,11 @@ export class InspectorClient extends InspectorClientEventTarget { ...(logMeta ?? {}), ...(callMetadata ?? {}), }; - // `_meta` values are arbitrary JSON (#1910), but `progressToken` is a - // reserved member the spec constrains to `string | integer` - // (`ProgressTokenSchema = z.union([z.string(), z.number().int()])`). Now - // that a value can be an object, an array or a float, a bad one would ride - // out on the wire and a conforming server would reject the whole request — - // so drop the member rather than send something invalid. It is optional, so - // the request stays well-formed and simply asks for no progress. Warn, - // because a silently dropped key the user typed is worse than a noisy one. - if ("progressToken" in merged && !isProgressToken(merged.progressToken)) { - // Log the rejected value's *type*, never the value. `_meta` now carries - // arbitrary JSON, so an invalid `progressToken` can be an object holding - // credentials — and this logger is persisted by real clients (the TUI - // writes it to `~/.mcp-inspector/auth.log`). The type is what makes the - // warning actionable anyway; the value would only leak. - this.logger.warn( - { received: describeJsonType(merged.progressToken) }, - "Dropping `_meta.progressToken` — expected a string or an integer.", - ); - delete merged.progressToken; - } - return Object.keys(merged).length > 0 ? merged : undefined; + // Reserved members carry their own schemas even though the rest of `_meta` + // is arbitrary JSON, and one bad reserved value makes a conforming server + // reject the whole request. See `dropInvalidReservedMeta`. + const sanitized = dropInvalidReservedMeta(merged, this.logger); + return Object.keys(sanitized).length > 0 ? sanitized : undefined; } /** diff --git a/docs/mcp-server-configuration.md b/docs/mcp-server-configuration.md index 67f98b12..9005411d 100644 --- a/docs/mcp-server-configuration.md +++ b/docs/mcp-server-configuration.md @@ -163,21 +163,21 @@ The file is the familiar MCP client-config shape — an `mcpServers` object keye These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on write when it equals its default**, so a round-trip through the Inspector keeps the file diff minimal. -| Field | Default | Meaning | -| -------------------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | -| `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. Web and TUI only today; see the CLI note below | -| `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 | -| `paginatedLists` | `false` | Fetch tools/resources/prompts one page at a time instead of auto-aggregating | -| `advertisedExtensions` | — | Per-extension overrides for what the Inspector declares in `capabilities.extensions` | -| `maxFetchRequests` | `1000` | Network-log retention for this server (`DEFAULT_MAX_FETCH_REQUESTS`); `0` means unlimited | -| `oauth` | — | `{ clientId, clientSecret, scopes, authorizationParams, authorizationUrl, tokenUrl, enterpriseManaged, onInsufficientScope }` | +| Field | Default | Meaning | +| -------------------------------------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `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 | +| `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 | +| `paginatedLists` | `false` | Fetch tools/resources/prompts one page at a time instead of auto-aggregating | +| `advertisedExtensions` | — | Per-extension overrides for what the Inspector declares in `capabilities.extensions` | +| `maxFetchRequests` | `1000` | Network-log retention for this server (`DEFAULT_MAX_FETCH_REQUESTS`); `0` means unlimited | +| `oauth` | — | `{ clientId, clientSecret, scopes, authorizationParams, authorizationUrl, tokenUrl, enterpriseManaged, onInsufficientScope }` | -`metadata` is a JSON **object**, and its values may be any JSON — an object, an array, a number, a boolean, `null` — not only a string. Nothing in the MCP spec restricts `_meta` value types, and the SDK models the field as a passthrough object, so the Inspector does not narrow it either ([#1910](https://github.com/modelcontextprotocol/inspector/issues/1910)). Edit it in Server Settings → Request Metadata, which is a JSON editor rather than the key/value rows `headers` and `env` use; text that is not a JSON object is flagged inline and not applied. +`metadata` is a JSON **object**, and its values may be any JSON — an object, an array, a number, a boolean, `null` — not only a string. The MCP spec does not restrict `_meta` value types in general, and the SDK models the field as a passthrough object, so the Inspector does not narrow it either ([#1910](https://github.com/modelcontextprotocol/inspector/issues/1910)). Edit it in Server Settings → Request Metadata, which is a JSON editor rather than the key/value rows `headers` and `env` use; text that is not a JSON object is flagged inline and not applied. ```jsonc "metadata": { @@ -187,6 +187,8 @@ 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 an integer) and `io.modelcontextprotocol/related-task` (`{ "taskId": "" }`). 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. > **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.