From 2b2e1575111ec8911ccec63a3c916a36ee5b15b5 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 15 Aug 2026 17:39:43 -0400 Subject: [PATCH] fix: gate the connect-time logging/setLevel on the negotiated era (#1990) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `logging/setLevel` is a legacy-era method, but connect() called it on any server advertising the `logging` capability, era regardless. The modern wire era rejects it outright, and because the call sits in the post-handshake sequence that rejection failed the *connection* — so the CLI (the one caller passing `initialLoggingLevel`, hardcoded to "debug") could not run any method at all against a modern server that advertised logging. Gate it on `protocolEra !== "modern"`. Modern has no session-scoped level to set: the equivalent is the per-request `io.modelcontextprotocol/logLevel` `_meta` opt-in, already driven by `modernLogLevel` from the server settings. So modern skips the call rather than translating it. Legacy is unchanged. The guard lives next to the capability check in inspectorClient.ts rather than in the CLI, so any future caller passing `initialLoggingLevel` is covered too. Signed-off-by: cliffhall --- ...nspectorClient-initial-logging-era.test.ts | 127 ++++++++++++++++++ core/mcp/inspectorClient.ts | 19 ++- 2 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 clients/web/src/test/core/mcp/inspectorClient-initial-logging-era.test.ts diff --git a/clients/web/src/test/core/mcp/inspectorClient-initial-logging-era.test.ts b/clients/web/src/test/core/mcp/inspectorClient-initial-logging-era.test.ts new file mode 100644 index 00000000..1f5fa165 --- /dev/null +++ b/clients/web/src/test/core/mcp/inspectorClient-initial-logging-era.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect } from "vitest"; +import type { JSONRPCMessage, Transport } from "@modelcontextprotocol/client"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { + MODERN_PROTOCOL_VERSION, + eraToVersionNegotiation, +} from "@inspector/core/mcp/types.js"; + +/** + * Regression coverage for #1990: the connect-time `initialLoggingLevel` must be + * gated on the negotiated era, not on the server capability alone. + * + * `logging/setLevel` is a legacy-era method, and the modern wire era rejects it + * outright ("Method 'logging/setLevel' is not supported by the negotiated + * protocol version"). Because the call sat in `connect()`'s post-handshake + * sequence, that rejection failed the *connection* — so the CLI (the one caller + * that passes `initialLoggingLevel`, hardcoded to "debug" in + * `clients/cli/src/cli.ts`) could not run *any* method against a modern server + * that advertised `logging`, logging-related or not. + * + * The two cases below are the era split: legacy still sends the request with the + * configured level, modern connects cleanly and never puts the method on the + * wire. The modern half asserts on the sent methods rather than on connect + * succeeding alone — a server that happens to answer the method would hide the + * regression behind a green connect. + * + * A fake transport carries both. The modern leg needs no `initialize` at all: + * the SDK negotiates with a `server/discover` probe and adopts that result's + * capabilities directly, which is also what lets one small class serve both eras + * by answering whichever handshake arrives. + */ +class CapabilityAdvertisingTransport implements Transport { + onmessage?: (message: JSONRPCMessage) => void; + onclose?: () => void; + onerror?: (error: Error) => void; + + /** Every method this client put on the wire, in order. */ + readonly sentMethods: string[] = []; + + /** The params of the `logging/setLevel` request, if one was sent. */ + loggingSetLevelParams?: Record; + + async start(): Promise {} + + async close(): Promise { + this.onclose?.(); + } + + async send(message: JSONRPCMessage): Promise { + if (!("method" in message)) return; + this.sentMethods.push(message.method); + + // The modern era's handshake: one probe, answered with the capabilities the + // client then treats as the server's. + if (message.method === "server/discover" && "id" in message) { + this.reply(message.id, { + supportedVersions: [MODERN_PROTOCOL_VERSION], + capabilities: { logging: {} }, + }); + return; + } + + // The legacy era's handshake. + if (message.method === "initialize" && "id" in message) { + const params = message.params as { protocolVersion: string }; + this.reply(message.id, { + protocolVersion: params.protocolVersion, + capabilities: { logging: {} }, + serverInfo: { name: "logging-server", version: "1.0.0" }, + }); + return; + } + + if (message.method === "logging/setLevel" && "id" in message) { + this.loggingSetLevelParams = message.params as Record; + this.reply(message.id, {}); + return; + } + } + + private reply(id: string | number, result: Record): void { + // The SDK resolves the request from its own microtask; delivering + // synchronously from inside `send()` is the earliest any server could. + this.onmessage?.({ jsonrpc: "2.0", id, result }); + } +} + +async function connectWith( + era: "legacy" | "modern", +): Promise { + const transport = new CapabilityAdvertisingTransport(); + const client = new InspectorClient( + { type: "streamable-http", url: "https://mcp.example/mcp" }, + { + environment: { transport: () => ({ transport }) }, + versionNegotiation: eraToVersionNegotiation(era), + initialLoggingLevel: "debug", + }, + ); + + await client.connect(); + expect(client.getProtocolEra()).toBe(era); + await client.disconnect(); + + return transport; +} + +describe("InspectorClient connect() initial logging level, by era (#1990)", () => { + it("sends logging/setLevel on a legacy connection", async () => { + const transport = await connectWith("legacy"); + + expect(transport.sentMethods).toContain("logging/setLevel"); + expect(transport.loggingSetLevelParams).toMatchObject({ level: "debug" }); + }); + + it("never sends logging/setLevel on a modern connection", async () => { + const transport = await connectWith("modern"); + + // Modern has no session-scoped level: the equivalent is the per-request + // `io.modelcontextprotocol/logLevel` `_meta` opt-in, driven by the + // `modernLogLevel` server setting rather than by `initialLoggingLevel`. + expect(transport.sentMethods).not.toContain("logging/setLevel"); + // The server did advertise logging, so the capability check alone would + // have let the call through — the era gate is what stops it. + expect(transport.sentMethods).toContain("server/discover"); + }); +}); diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index b0b809f3..5d8aa86a 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -1936,8 +1936,23 @@ export class InspectorClient extends InspectorClientEventTarget { // capabilities and wipe tools/prompts/resources to empty on every connect. await this.fetchServerInfo(); - // Set initial logging level if configured and server supports it - if (this.initialLoggingLevel && this.capabilities?.logging) { + // Set initial logging level if configured and server supports it. + // + // Era-gated (#1990): `logging/setLevel` is a legacy-era method, and the + // modern wire era rejects it outright — so an unconditional call here + // failed the *connect* of every modern server advertising `logging`, + // taking down commands that have nothing to do with logging. Modern has + // no session-scoped level at all: the equivalent is the per-request + // `io.modelcontextprotocol/logLevel` `_meta` opt-in, which + // `modernLogLevel` already drives from the server settings (see + // {@link setModernLogLevel}). So the right behavior on modern is to skip + // this call rather than translate it — `initialLoggingLevel` names a + // mechanism that era does not have. + if ( + this.initialLoggingLevel && + this.capabilities?.logging && + this.protocolEra !== "modern" + ) { await this.client.setLoggingLevel( this.initialLoggingLevel, this.getRequestOptions(),