fix(core): register peer-request handlers before connect (#1797)

clients/cli/README.md

@@ -78,7 +78,9 @@ npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --tr
 npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --transport http --method tools/list --header "X-API-Key: your-api-key"
 ```
 
-When a server is loaded from a `--catalog`/`--config` file, its per-server settings (headers, connection/request timeouts, and OAuth) are applied to the connection — the same resolution the TUI uses. A `--header` flag overrides the file's headers for that run while leaving the file's timeouts and OAuth in place.
+When a server is loaded from a `--catalog`/`--config` file, its per-server settings (headers, connection/request timeouts, OAuth, and `roots`) are applied to the connection — the same resolution the TUI uses. A `--header` flag overrides the file's headers for that run while leaving the file's timeouts and OAuth in place.
+
+The file is the only durable way to give a run its roots: there is no roots flag, and `--method roots/set` applies only to that one short-lived connection. Roots configured for a server (the same field the web UI's Server Settings writes) are advertised at connect, so a server that asks for `roots/list` — `@modelcontextprotocol/server-filesystem` does, to learn its allowed directories — gets them.
 
 **Environment-variable semantics.** `MCP_CATALOG_PATH` is honored only when no ad-hoc target is given (positional command, `--server-url`, or `--transport`) — so a shell that exports it can still run one-off ad-hoc invocations without hitting the catalog/ad-hoc conflict. `MCP_STORAGE_DIR` sets the storage directory used by the OAuth persist backend (`<MCP_STORAGE_DIR>/oauth.json`); the per-file `MCP_INSPECTOR_OAUTH_STATE_PATH` override still takes precedence over it.
 

clients/cli/__tests__/cli.test.ts

@@ -20,6 +20,7 @@ import {
   getTestMcpServerCommand,
   createTestServerHttp,
   createEchoTool,
+  createListRootsTool,
   createTestServerInfo,
 } from "@modelcontextprotocol/inspector-test-server";
 import type { MCPServerConfig } from "@modelcontextprotocol/inspector-core/mcp/index.js";
@@ -325,6 +326,89 @@ describe("CLI Tests", () => {
     });
   });
 
+  describe("Roots capability (#1797)", () => {
+    it("answers a server's roots/list instead of -32601", async () => {
+      // The CLI used to omit `roots` when constructing its InspectorClient, so
+      // the capability was never advertised and no `roots/list` handler was
+      // registered — a server that asked got -32601 Method not found. (And
+      // `--method roots/set` could not announce the change: the SDK refuses
+      // `roots/list_changed` from a client that never declared it, so nothing
+      // reached the wire.) `cli.ts` now always passes the `roots` option —
+      // empty on this ad-hoc path, since there is no config file.
+      const server = createTestServerHttp({
+        serverInfo: createTestServerInfo(),
+        tools: [createListRootsTool()],
+      });
+      try {
+        await server.start();
+
+        const result = await runCli([
+          server.url,
+          "--cli",
+          "--method",
+          "tools/call",
+          "--tool-name",
+          "list_roots",
+        ]);
+
+        expectCliSuccess(result);
+        const json = expectValidJson(result);
+        // The tool asks the client for roots and renders what it got. An
+        // unregistered handler surfaces as an isError result quoting -32601.
+        expect(json.isError).toBeFalsy();
+        expect(JSON.stringify(json)).toContain("Roots:");
+        expect(JSON.stringify(json)).not.toContain("-32601");
+      } finally {
+        await server.stop();
+      }
+    });
+
+    it("answers with the roots configured for the server in the config file", async () => {
+      // Answering `roots/list` is only half the fix: the roots a user
+      // configured in mcp.json have to be the answer. They ride in on
+      // `serverSettings.roots` (lifted by `mcpConfigToServerEntries`), which is
+      // what web passes too — a CLI that seeded `[]` would let a server fall
+      // back to its own defaults, the outcome #1797 is about.
+      const server = createTestServerHttp({
+        serverInfo: createTestServerInfo(),
+        tools: [createListRootsTool()],
+      });
+      let configPath: string | undefined;
+      try {
+        await server.start();
+        configPath = createTestConfig({
+          mcpServers: {
+            web: {
+              type: "streamable-http",
+              url: server.url,
+              roots: [{ uri: "file:///configured", name: "Configured" }],
+            } as unknown as MCPServerConfig,
+          },
+        });
+
+        const result = await runCli([
+          "--config",
+          configPath,
+          "--server",
+          "web",
+          "--cli",
+          "--method",
+          "tools/call",
+          "--tool-name",
+          "list_roots",
+        ]);
+
+        expectCliSuccess(result);
+        const json = expectValidJson(result);
+        expect(json.isError).toBeFalsy();
+        expect(JSON.stringify(json)).toContain("file:///configured");
+      } finally {
+        await server.stop();
+        if (configPath) deleteConfigFile(configPath);
+      }
+    });
+  });
+
   describe("Config-file settings lifting (#1482)", () => {
     it("applies a config file's custom header on a tools/call over HTTP", async () => {
       const server = createTestServerHttp({

clients/cli/__tests__/run-method.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, afterEach } from "vitest";
+import { describe, it, expect, afterEach, vi } from "vitest";
 import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
 import { InspectorClient } from "@inspector/core/mcp/index.js";
 import { createTransportNode } from "@inspector/core/mcp/node/index.js";
@@ -25,6 +25,9 @@ describe("runMethod", () => {
         progress: false,
         sample: false,
         elicit: false,
+        // Mirrors `cli.ts`, which always passes the option so the capability is
+        // negotiated and the `roots/list` handler registered (#1797).
+        roots: [],
       },
     );
     await client.connect();
@@ -92,6 +95,33 @@ describe("runMethod", () => {
     }
   });
 
+  it("roots/set drops a malformed root rather than advertising it", async () => {
+    // `--roots-json` only checks that the payload parses to an array, so a
+    // root with no `uri` used to be stored and then handed to the server in
+    // the `roots/list` reply — an invalid Root on the wire, echoed back to the
+    // user as success. `setRoots` normalizes through `cleanRoots` now (#1797).
+    const c = await connectStdio();
+    // cleanRoots reports the drop; suppress it here and assert it happened, so
+    // the test also covers that a dropped root is not dropped silently.
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    try {
+      const set = await runMethod(c, {
+        method: "roots/set",
+        rootsJson: JSON.stringify([
+          { name: "no uri" },
+          { uri: "file:///keep" },
+        ]),
+      });
+      expect(set.kind).toBe("result");
+      if (set.kind === "result") {
+        expect(set.result.roots).toEqual([{ uri: "file:///keep" }]);
+      }
+      expect(warn).toHaveBeenCalled();
+    } finally {
+      warn.mockRestore();
+    }
+  });
+
   it("consumeMethodOutcome writes result json", async () => {
     let stdout = "";
     const original = process.stdout.write;

clients/cli/src/cli.ts

@@ -15,6 +15,7 @@ import { listServerEntries, showServerEntry } from "./handlers/servers-list.js";
 import { writeFormattedResult } from "./handlers/format-output.js";
 import { clearStoredAuthForRelogin } from "./clear-stored-auth-for-relogin.js";
 import { InspectorClient } from "@inspector/core/mcp/index.js";
+import { cleanRoots } from "@inspector/core/mcp/serverList.js";
 import {
   createTransportNode,
   loadServerEntries,
@@ -152,6 +153,15 @@ async function callMethod(
     progress: false,
     sample: false,
     elicit: false,
+    // Advertise the roots configured for this server in mcp.json, exactly as
+    // web does (`App.tsx`) so both answer `roots/list` with the same content.
+    // Passing the option (even empty) is what negotiates `capabilities.roots`
+    // at `initialize` and registers the `roots/list` handler. Omitting it meant
+    // a server that asks for roots on its own — `server-filesystem` does, at
+    // `initialize` — got -32601, and `--method roots/set` could not announce
+    // 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 ?? []),
     serverSettings,
     // Per-server protocol era (SEP §7.8) from mcp.json → SDK versionNegotiation.
     // Absent era defaults to legacy in the InspectorClient constructor (#1626).

clients/tui/src/App.tsx

@@ -20,6 +20,7 @@ import type {
   GetPromptResult,
 } from "@modelcontextprotocol/client";
 import { InspectorClient } from "@inspector/core/mcp/index.js";
+import { cleanRoots } from "@inspector/core/mcp/serverList.js";
 import { eraToVersionNegotiation } from "@inspector/core/mcp/types.js";
 import {
   ManagedToolsState,
@@ -304,6 +305,11 @@ function App({
               defaultMetadata,
             }),
           ...(savedSettings && { serverSettings: savedSettings }),
+          // Advertise the roots configured for this server in mcp.json, as web
+          // and the CLI do. The TUI has no roots editor, but a user who set
+          // roots in the web UI expects them to apply to the same server here
+          // (#1797).
+          roots: cleanRoots(savedSettings?.roots ?? []),
           // Per-server protocol era (SEP §7.8) from mcp.json → SDK
           // versionNegotiation; absent era defaults to legacy (#1626).
           ...(savedSettings?.protocolEra && {

clients/web/src/App.tsx

@@ -44,9 +44,9 @@ import type {
 } from "@inspector/core/mcp/types.js";
 import {
   DEFAULT_MAX_FETCH_REQUESTS,
-  DEFAULT_MODERN_LOG_LEVEL,
   DEFAULT_TASK_TTL_MS,
   eraToVersionNegotiation,
+  resolveModernLogLevel,
 } from "@inspector/core/mcp/types.js";
 import {
   API_SERVER_ENV_VARS,
@@ -1216,7 +1216,27 @@ function App() {
     setConsoleUi(EMPTY_CONSOLE_UI);
     setProgressByTaskId({});
     setCurrentLogLevel("info");
-    setModernLogLevel(null);
+    // Re-seed rather than blank: the client restores its own opt-in from the
+    // server setting at connect (`resetSessionState`), so blanking here would
+    // leave the control reading Off while every modern request still carries
+    // the level — visible on the auth-recovery path, which reconnects the same
+    // client instance rather than rebuilding it (#1629, #1797).
+    // `activeServerIdRef` is synced in a passive effect, so it still holds the
+    // outgoing server's id when this runs from `onDisconnect` — which is what
+    // lets the re-seed find its settings. Clearing that ref eagerly would take
+    // the no-server branch below and silently drop this to Off.
+    // Branch on the *server*, not its settings: an entry with no settings node
+    // is the common case (`mcp.json` written by hand, never opened in Server
+    // Settings), and there the default is right — it is what the seed and the
+    // client both use. Only "no server at all" means Off.
+    const activeServer = serversRef.current.find(
+      (s) => s.id === activeServerIdRef.current,
+    );
+    setModernLogLevel(
+      activeServer
+        ? (resolveModernLogLevel(activeServer.settings) ?? null)
+        : null,
+    );
     setPendingStepUp(null);
     setPendingReauth(null);
     setReAuthBanner(null);
@@ -2337,9 +2357,7 @@ function App() {
       // setting so the Logs-tab control reflects what the client stamps by
       // default (the client was seeded the same way in its constructor). "off"
       // means not opted in (null). Only affects modern connections.
-      const seededModernLevel =
-        savedSettings?.modernLogLevel ?? DEFAULT_MODERN_LOG_LEVEL;
-      setModernLogLevel(seededModernLevel === "off" ? null : seededModernLevel);
+      setModernLogLevel(resolveModernLogLevel(savedSettings) ?? null);
       setManagedToolsState(new ManagedToolsState(client));
       setPagedToolsState(new PagedToolsState(client));
       setPagedPromptsState(new PagedPromptsState(client));