fix(mcp): bound tools/list during catalog discovery (#85063)

Summary: - The branch adds a 1500 ms internal timeout to bundled MCP `tools/list` catalog discovery, adds slow and hung stdio MCP regression tests, and records the fix in `CHANGELOG.md`. - PR surface: Source +2, Tests +216, Docs +1. Total +219 across 3 files. - Reproducibility: yes. The current-main source path is high confidence: bundled MCP connects successfully, then calls `client.listTools` without request options, and the upstream SDK defaults that request to 60000 ms. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(mcp): use internal tools list timeout - PR branch already contained follow-up commit before automerge: fix(mcp): bound tools/list during catalog discovery - PR branch already contained follow-up commit before automerge: fix(clawsweeper): address review for automerge-openclaw-openclaw-8506… Validation: - ClawSweeper review passed for head bbbfb9f059. - Required merge gates passed before the squash merge. Prepared head SHA: bbbfb9f059 Review: https://github.com/openclaw/openclaw/pull/85063#issuecomment-4511554739 Co-authored-by: nxmxbbd <32288+nxmxbbd@users.noreply.github.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
2026-07-21 10:16:44 +00:00 · 2026-05-24 18:37:28 +00:00
parent dfa1a51225
commit 07f500aa56
3 changed files with 222 additions and 3 deletions
@@ -8,6 +8,7 @@ Docs: https://docs.openclaw.ai

 ### Fixes

+- Agents/MCP: bound bundled MCP `tools/list` catalog discovery so hung MCP servers do not block session tool materialization. (#85063) Thanks @nxmxbbd.
 - Channels/iMessage: advance the startup catchup cursor from live-handled rows after a completed catchup pass, including rows received while catchup is still running, so restarts do not replay them. (#85475) Thanks @TurboTheTurtle.
 - Tests: mount the shared Windows command helper into bare Docker E2E harness containers so published upgrade-survivor config walks can start on Linux.
 - Tests: keep the plugin binding command escape Docker smoke focused on its intended Vitest cases and skip source-only install lifecycle scripts.
@@ -1,4 +1,8 @@
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
 import { afterEach, describe, expect, it, vi } from "vitest";
+import { writeExecutable } from "./bundle-mcp-shared.test-harness.js";
 import { createBundleMcpJsonSchemaValidator } from "./pi-bundle-mcp-runtime.js";
 import { cleanupBundleMcpHarness } from "./pi-bundle-mcp-test-harness.js";
 import {
@@ -21,6 +25,129 @@ type RuntimeFactoryOptions = NonNullable<
  Parameters<typeof testing.createSessionMcpRuntimeManager>[0]
 >;
 type RuntimeFactory = NonNullable<RuntimeFactoryOptions["createRuntime"]>;
+const LIST_TOOLS_SERVER_LOG_TIMEOUT_MS = 2_000;
+const LIST_TOOLS_TEST_DEADLINE_MS = 4_000;
+
+async function writeListToolsMcpServer(params: {
+  filePath: string;
+  logPath: string;
+  delayMs?: number;
+  hang?: boolean;
+}): Promise<void> {
+  await writeExecutable(
+    params.filePath,
+    `#!/usr/bin/env node
+import fs from "node:fs/promises";
+
+const logPath = ${JSON.stringify(params.logPath)};
+const delayMs = ${params.delayMs ?? 0};
+const hang = ${params.hang === true};
+
+let buffer = "";
+let pendingTimer;
+let keepAlive;
+function log(line) {
+  void fs.appendFile(logPath, line + "\\n", "utf8").catch(() => {});
+}
+function send(message) {
+  process.stdout.write(JSON.stringify(message) + "\\n");
+}
+function handle(message) {
+  if (!message || typeof message !== "object") {
+    return;
+  }
+  log("recv " + String(message.method ?? "unknown"));
+  if (message.method === "initialize") {
+    send({
+      jsonrpc: "2.0",
+      id: message.id,
+      result: {
+        protocolVersion: message.params?.protocolVersion ?? "2025-03-26",
+        capabilities: { tools: {} },
+        serverInfo: { name: "test-list-tools", version: "1.0.0" },
+      },
+    });
+    return;
+  }
+  if (message.method === "notifications/initialized") {
+    return;
+  }
+  if (message.method === "tools/list") {
+    if (hang) {
+      log("hang tools/list");
+      keepAlive = setInterval(() => {}, 1000);
+      return;
+    }
+    log("delay tools/list " + delayMs);
+    pendingTimer = setTimeout(() => {
+      send({
+        jsonrpc: "2.0",
+        id: message.id,
+        result: {
+          tools: [
+            {
+              name: "slow_tool",
+              description: "Returned after a slow catalog response.",
+              inputSchema: { type: "object", properties: {} },
+            },
+          ],
+        },
+      });
+    }, delayMs);
+  }
+}
+process.stdin.setEncoding("utf8");
+function shutdown() {
+  if (pendingTimer) {
+    clearTimeout(pendingTimer);
+  }
+  if (keepAlive) {
+    clearInterval(keepAlive);
+  }
+  process.exit(0);
+}
+process.stdin.on("data", (chunk) => {
+  buffer += chunk;
+  while (true) {
+    const newline = buffer.indexOf("\\n");
+    if (newline < 0) {
+      return;
+    }
+    const line = buffer.slice(0, newline).replace(/\\r$/, "");
+    buffer = buffer.slice(newline + 1);
+    if (line.trim()) {
+      handle(JSON.parse(line));
+    }
+  }
+});
+process.stdin.on("end", shutdown);
+process.on("SIGTERM", shutdown);
+process.on("SIGINT", shutdown);`,
+  );
+}
+
+async function waitForFileText(
+  filePath: string,
+  expectedText: string,
+  timeoutMs: number,
+): Promise<void> {
+  const deadline = Date.now() + timeoutMs;
+  let lastText = "";
+  while (Date.now() < deadline) {
+    try {
+      lastText = await fs.readFile(filePath, "utf8");
+      if (lastText.includes(expectedText)) {
+        return;
+      }
+    } catch {
+      // The server may not have written the log file yet.
+    }
+    await new Promise((resolve) => setTimeout(resolve, 10));
+  }
+  throw new Error(
+    `Timed out waiting for ${expectedText} in ${filePath}; saw ${JSON.stringify(lastText)}`,
+  );
+}

 function makeRuntime(
  tools: Array<{ toolName: string; description: string }>,
@@ -181,6 +308,95 @@ describe("session MCP runtime", () => {
    expect(activeLeases).toBe(0);
  });

+  it("keeps MCP tools/list responses that exceed the connection timeout but finish within the internal catalog timeout", async () => {
+    const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "bundle-mcp-slow-listtools-"));
+    const serverPath = path.join(tempDir, "slow-list-tools.mjs");
+    const logPath = path.join(tempDir, "server.log");
+    await writeListToolsMcpServer({
+      filePath: serverPath,
+      logPath,
+      delayMs: 750,
+    });
+
+    const runtime = await getOrCreateSessionMcpRuntime({
+      sessionId: "session-slow-listtools-server-timeout",
+      sessionKey: "agent:test:session-slow-listtools-server-timeout",
+      workspaceDir: "/workspace",
+      cfg: {
+        mcp: {
+          servers: {
+            slowListTools: {
+              command: process.execPath,
+              args: [serverPath],
+              connectionTimeoutMs: 500,
+            },
+          },
+        },
+      },
+    });
+
+    try {
+      const catalog = await runtime.getCatalog();
+
+      expect(catalog.tools.map((tool) => tool.toolName)).toEqual(["slow_tool"]);
+      expect(catalog.servers.slowListTools).toMatchObject({
+        serverName: "slowListTools",
+        toolCount: 1,
+      });
+      await expect(fs.readFile(logPath, "utf8")).resolves.toContain("delay tools/list 750");
+    } finally {
+      await runtime.dispose();
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
+  });
+
+  it("times out default-config hung bundle MCP tools/list using the internal catalog timeout", async () => {
+    const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "bundle-mcp-listtools-timeout-"));
+    const serverPath = path.join(tempDir, "hanging-list-tools.mjs");
+    const logPath = path.join(tempDir, "server.log");
+    await writeListToolsMcpServer({ filePath: serverPath, logPath, hang: true });
+
+    const runtime = await getOrCreateSessionMcpRuntime({
+      sessionId: "session-listtools-server-timeout",
+      sessionKey: "agent:test:session-listtools-server-timeout",
+      workspaceDir: "/workspace",
+      cfg: {
+        mcp: {
+          servers: {
+            hangingListTools: {
+              command: process.execPath,
+              args: [serverPath],
+            },
+          },
+        },
+      },
+    });
+    const catalogResult = runtime.getCatalog().then(
+      (catalog) => ({ status: "resolved" as const, catalog }),
+      (error: unknown) => ({ status: "rejected" as const, error }),
+    );
+
+    try {
+      await waitForFileText(logPath, "recv tools/list", LIST_TOOLS_SERVER_LOG_TIMEOUT_MS);
+      const result = await Promise.race([
+        catalogResult,
+        new Promise<{ status: "pending" }>((resolve) => {
+          setTimeout(() => resolve({ status: "pending" }), LIST_TOOLS_TEST_DEADLINE_MS);
+        }),
+      ]);
+
+      expect(result.status).toBe("resolved");
+      if (result.status === "resolved") {
+        expect(result.catalog.tools).toEqual([]);
+        expect(result.catalog.servers).toEqual({});
+      }
+    } finally {
+      await runtime.dispose();
+      await Promise.race([catalogResult, new Promise((resolve) => setTimeout(resolve, 1000))]);
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
+  });
+
  it("reuses repeated materialization and recreates after explicit disposal", async () => {
    const created: SessionMcpRuntime[] = [];
    const disposed: string[] = [];
@@ -47,6 +47,7 @@ const SESSION_MCP_RUNTIME_MANAGER_KEY = Symbol.for("openclaw.sessionMcpRuntimeMa
 const DRAFT_2020_12_SCHEMA = "https://json-schema.org/draft/2020-12/schema";
 const DEFAULT_SESSION_MCP_RUNTIME_IDLE_TTL_MS = 10 * 60 * 1000;
 const SESSION_MCP_RUNTIME_SWEEP_INTERVAL_MS = 60 * 1000;
+const BUNDLE_MCP_CATALOG_LIST_TIMEOUT_MS = 1_500;

 type Ajv2020Like = {
  compile: (schema: JsonSchemaType) => ValidateFunction;
@@ -119,11 +120,12 @@ function redactErrorUrls(error: unknown): string {
  return redactSensitiveUrlLikeString(String(error));
 }

-async function listAllTools(client: Client) {
+async function listAllTools(client: Client, timeoutMs: number) {
  const tools: ListedTool[] = [];
  let cursor: string | undefined;
  do {
-    const page = await client.listTools(cursor ? { cursor } : undefined);
+    const params = cursor ? { cursor } : undefined;
+    const page = await client.listTools(params, { timeout: timeoutMs });
    tools.push(...page.tools);
    cursor = page.nextCursor;
  } while (cursor);
@@ -260,7 +262,7 @@ export function createSessionMcpRuntime(params: {
            failIfDisposed();
            await connectWithTimeout(client, resolved.transport, resolved.connectionTimeoutMs);
            failIfDisposed();
-            const listedTools = await listAllTools(client);
+            const listedTools = await listAllTools(client, BUNDLE_MCP_CATALOG_LIST_TIMEOUT_MS);
            failIfDisposed();
            servers[serverName] = {
              serverName,