diff --git a/CHANGELOG.md b/CHANGELOG.md index 271ccc4c06f..4b15988d0db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Docs: https://docs.openclaw.ai - LINE/outbound media: add LINE image, video, and audio outbound sends on the LINE-specific delivery path, including explicit preview/tracking handling for videos while keeping generic media sends on the existing image-only route. (#45826) Thanks @masatohoshino. - WhatsApp/reactions: agents can now react with emoji on incoming WhatsApp messages, enabling more natural conversational interactions like acknowledging a photo with ❤️ instead of typing a reply. Thanks @mcaxtr. - MCP: add remote HTTP/SSE server support for `mcp.servers` URL configs, including auth headers and safer config redaction for MCP credentials. (#50396) Thanks @dhananjai1729. +- Agents/MCP: materialize bundle MCP tools with provider-safe names (`serverName__toolName`), support optional `streamable-http` transport selection plus per-server connection timeouts, and preserve real tool results from aborted/error turns unless truncation explicitly drops them. (#49505) Thanks @ziomancer. - Plugins/hooks: add a `before_install` hook with structured request provenance, built-in scan status, and install-target metadata so external security scanners and policy engines can review and block skill, plugin package, plugin bundle, and single-file plugin installs. (#56050) thanks @odysseus0. - ACP/plugins: add an explicit default-off ACPX plugin-tools MCP bridge config, document the trust boundary, and harden the built-in bridge packaging/logging path so global installs and stdio MCP sessions work reliably. (#56867) Thanks @joe2643. diff --git a/docs/plugins/bundles.md b/docs/plugins/bundles.md index 2523bd6e091..8e18d5c4485 100644 --- a/docs/plugins/bundles.md +++ b/docs/plugins/bundles.md @@ -126,7 +126,7 @@ MCP servers can use stdio or HTTP transport: } ``` -**HTTP** connects to a running MCP server over `streamable-http` or `sse`: +**HTTP** connects to a running MCP server over `sse` by default, or `streamable-http` when requested: ```json { @@ -145,7 +145,7 @@ MCP servers can use stdio or HTTP transport: } ``` -- `transport` must be `"streamable-http"` or `"sse"`; there is no auto-detection +- `transport` may be set to `"streamable-http"` or `"sse"`; when omitted, OpenClaw uses `sse` - only `http:` and `https:` URL schemes are allowed - `headers` values support `${ENV_VAR}` interpolation - a server entry with both `command` and `url` is rejected diff --git a/src/agents/mcp-transport.ts b/src/agents/mcp-transport.ts index 38328107969..baf06e84d86 100644 --- a/src/agents/mcp-transport.ts +++ b/src/agents/mcp-transport.ts @@ -1,5 +1,6 @@ import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import { logDebug, logWarn } from "../logger.js"; import { describeSseMcpServerLaunchConfig, resolveSseMcpServerLaunchConfig } from "./mcp-sse.js"; @@ -11,6 +12,7 @@ import { export type ResolvedMcpTransport = { transport: Transport; description: string; + transportType: "stdio" | "sse" | "streamable-http"; detachStderr?: () => void; }; @@ -86,6 +88,35 @@ function resolveSseTransport(serverName: string, rawServer: unknown): ResolvedMc eventSourceInit: hasHeaders ? { fetch: buildSseEventSourceFetch(headers) } : undefined, }), description: describeSseMcpServerLaunchConfig(sseLaunch.config), + transportType: "sse", + }; +} + +function resolveStreamableHttpTransport( + serverName: string, + rawServer: unknown, +): ResolvedMcpTransport | null { + const launch = resolveSseMcpServerLaunchConfig(rawServer, { + onDroppedHeader: (key) => { + logWarn( + `bundle-mcp: server "${serverName}": header "${key}" has an unsupported value type and was ignored.`, + ); + }, + onMalformedHeaders: () => { + logWarn( + `bundle-mcp: server "${serverName}": "headers" must be a JSON object; the value was ignored.`, + ); + }, + }); + if (!launch.ok) { + return null; + } + return { + transport: new StreamableHTTPClientTransport(new URL(launch.config.url), { + requestInit: launch.config.headers ? { headers: launch.config.headers } : undefined, + }), + description: describeSseMcpServerLaunchConfig(launch.config), + transportType: "streamable-http", }; } @@ -93,6 +124,12 @@ export function resolveMcpTransport( serverName: string, rawServer: unknown, ): ResolvedMcpTransport | null { + const requestedTransport = + rawServer && + typeof rawServer === "object" && + typeof (rawServer as { transport?: unknown }).transport === "string" + ? ((rawServer as { transport?: string }).transport ?? "").trim().toLowerCase() + : ""; const stdioLaunch = resolveStdioMcpServerLaunchConfig(rawServer); if (stdioLaunch.ok) { const transport = new StdioClientTransport({ @@ -105,10 +142,29 @@ export function resolveMcpTransport( return { transport, description: describeStdioMcpServerLaunchConfig(stdioLaunch.config), + transportType: "stdio", detachStderr: attachStderrLogging(serverName, transport), }; } + if ( + requestedTransport && + requestedTransport !== "sse" && + requestedTransport !== "streamable-http" + ) { + logWarn( + `bundle-mcp: skipped server "${serverName}" because transport "${requestedTransport}" is not supported.`, + ); + return null; + } + + if (requestedTransport === "streamable-http") { + const httpTransport = resolveStreamableHttpTransport(serverName, rawServer); + if (httpTransport) { + return httpTransport; + } + } + const sseTransport = resolveSseTransport(serverName, rawServer); if (sseTransport) { return sseTransport; diff --git a/src/agents/pi-bundle-mcp-tools.test.ts b/src/agents/pi-bundle-mcp-tools.test.ts index 2f71d926f92..3cc68ba2236 100644 --- a/src/agents/pi-bundle-mcp-tools.test.ts +++ b/src/agents/pi-bundle-mcp-tools.test.ts @@ -155,7 +155,7 @@ describe("createBundleMcpToolRuntime", () => { }); try { - expect(runtime.tools.map((tool) => tool.name)).toEqual(["bundle_probe"]); + expect(runtime.tools.map((tool) => tool.name)).toEqual(["bundleProbe__bundle_probe"]); const result = await runtime.tools[0].execute("call-bundle-probe", {}, undefined, undefined); expect(result.content[0]).toMatchObject({ type: "text", @@ -170,7 +170,7 @@ describe("createBundleMcpToolRuntime", () => { } }); - it("skips bundle MCP tools that collide with existing tool names", async () => { + it("disambiguates bundle MCP tools that collide with existing tool names", async () => { const workspaceDir = await makeTempDir("openclaw-bundle-mcp-tools-"); const pluginRoot = path.join(workspaceDir, ".openclaw", "extensions", "bundle-probe"); const serverScriptPath = path.join(pluginRoot, "servers", "bundle-probe.mjs"); @@ -186,11 +186,11 @@ describe("createBundleMcpToolRuntime", () => { }, }, }, - reservedToolNames: ["bundle_probe"], + reservedToolNames: ["bundleProbe__bundle_probe"], }); try { - expect(runtime.tools).toEqual([]); + expect(runtime.tools.map((tool) => tool.name)).toEqual(["bundleProbe__bundle_probe-2"]); } finally { await runtime.dispose(); } @@ -219,7 +219,7 @@ describe("createBundleMcpToolRuntime", () => { }); try { - expect(runtime.tools.map((tool) => tool.name)).toEqual(["bundle_probe"]); + expect(runtime.tools.map((tool) => tool.name)).toEqual(["configuredProbe__bundle_probe"]); const result = await runtime.tools[0].execute( "call-configured-probe", {}, @@ -285,6 +285,7 @@ describe("createBundleMcpToolRuntime", () => { servers: { sseProbe: { url: `http://127.0.0.1:${port}/sse`, + transport: "sse", }, }, }, @@ -292,7 +293,7 @@ describe("createBundleMcpToolRuntime", () => { }); try { - expect(runtime.tools.map((tool) => tool.name)).toEqual(["sse_probe"]); + expect(runtime.tools.map((tool) => tool.name)).toEqual(["sseProbe__sse_probe"]); const result = await runtime.tools[0].execute("call-sse-probe", {}, undefined, undefined); expect(result.content[0]).toMatchObject({ type: "text", @@ -352,8 +353,8 @@ describe("createBundleMcpToolRuntime", () => { }); expect(runtimeA).toBe(runtimeB); - expect(materializedA.tools.map((tool) => tool.name)).toEqual(["bundle_probe"]); - expect(materializedB.tools.map((tool) => tool.name)).toEqual(["bundle_probe"]); + expect(materializedA.tools.map((tool) => tool.name)).toEqual(["bundleProbe__bundle_probe"]); + expect(materializedB.tools.map((tool) => tool.name)).toEqual(["bundleProbe__bundle_probe"]); expect(await fs.readFile(startupCounterPath, "utf8")).toBe("1"); expect(__testing.getCachedSessionIds()).toEqual(["session-a"]); }); diff --git a/src/agents/pi-bundle-mcp-tools.ts b/src/agents/pi-bundle-mcp-tools.ts index c4344c1ede9..efcf114d62c 100644 --- a/src/agents/pi-bundle-mcp-tools.ts +++ b/src/agents/pi-bundle-mcp-tools.ts @@ -1,11 +1,13 @@ import crypto from "node:crypto"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import type { OpenClawConfig } from "../config/config.js"; import { logWarn } from "../logger.js"; import { resolveGlobalSingleton } from "../shared/global-singleton.js"; +import { redactSensitiveUrlLikeString } from "../shared/net/redact-sensitive-url.js"; import { loadEmbeddedPiMcpConfig } from "./embedded-pi-mcp.js"; import { isMcpConfigRecord } from "./mcp-config-shared.js"; import { resolveMcpTransport } from "./mcp-transport.js"; @@ -20,6 +22,7 @@ type BundleMcpSession = { serverName: string; client: Client; transport: Transport; + transportType: "stdio" | "sse" | "streamable-http"; detachStderr?: () => void; }; @@ -34,6 +37,7 @@ export type McpServerCatalog = { export type McpCatalogTool = { serverName: string; + safeServerName: string; toolName: string; title?: string; description?: string; @@ -76,6 +80,95 @@ export type SessionMcpRuntimeManager = { }; const SESSION_MCP_RUNTIME_MANAGER_KEY = Symbol.for("openclaw.sessionMcpRuntimeManager"); +const DEFAULT_CONNECTION_TIMEOUT_MS = 30_000; +const TOOL_NAME_SAFE_RE = /[^A-Za-z0-9_-]/g; +const TOOL_NAME_SEPARATOR = "__"; +const TOOL_NAME_MAX_PREFIX = 30; +const TOOL_NAME_MAX_TOTAL = 64; + +function connectWithTimeout( + client: Client, + transport: Transport, + timeoutMs: number, +): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new Error(`MCP server connection timed out after ${timeoutMs}ms`)), + timeoutMs, + ); + client.connect(transport).then( + (value) => { + clearTimeout(timer); + resolve(value); + }, + (error) => { + clearTimeout(timer); + reject(error); + }, + ); + }); +} + +function redactErrorUrls(error: unknown): string { + return redactSensitiveUrlLikeString(String(error)); +} + +function sanitizeServerName(raw: string, usedNames: Set): string { + const cleaned = raw.trim().replace(TOOL_NAME_SAFE_RE, "-"); + const base = + cleaned.length > TOOL_NAME_MAX_PREFIX + ? cleaned.slice(0, TOOL_NAME_MAX_PREFIX) + : cleaned || "mcp"; + let candidate = base; + let n = 2; + while (usedNames.has(candidate.toLowerCase())) { + const suffix = `-${n}`; + candidate = `${base.slice(0, Math.max(1, TOOL_NAME_MAX_PREFIX - suffix.length))}${suffix}`; + n += 1; + } + usedNames.add(candidate.toLowerCase()); + return candidate; +} + +function sanitizeToolName(raw: string): string { + const cleaned = raw.trim().replace(TOOL_NAME_SAFE_RE, "-"); + return cleaned || "tool"; +} + +function buildSafeToolName(params: { + serverName: string; + toolName: string; + reservedNames: Set; +}): string { + const cleanedToolName = sanitizeToolName(params.toolName); + const maxToolChars = Math.max( + 1, + TOOL_NAME_MAX_TOTAL - params.serverName.length - TOOL_NAME_SEPARATOR.length, + ); + const truncatedToolName = cleanedToolName.slice(0, maxToolChars); + let candidateToolName = truncatedToolName || "tool"; + let candidate = `${params.serverName}${TOOL_NAME_SEPARATOR}${candidateToolName}`; + let n = 2; + while (params.reservedNames.has(candidate.toLowerCase())) { + const suffix = `-${n}`; + candidateToolName = `${(truncatedToolName || "tool").slice(0, Math.max(1, maxToolChars - suffix.length))}${suffix}`; + candidate = `${params.serverName}${TOOL_NAME_SEPARATOR}${candidateToolName}`; + n += 1; + } + return candidate; +} + +function getConnectionTimeoutMs(rawServer: unknown): number { + if ( + rawServer && + typeof rawServer === "object" && + typeof (rawServer as { connectionTimeoutMs?: unknown }).connectionTimeoutMs === "number" && + (rawServer as { connectionTimeoutMs: number }).connectionTimeoutMs > 0 + ) { + return (rawServer as { connectionTimeoutMs: number }).connectionTimeoutMs; + } + return DEFAULT_CONNECTION_TIMEOUT_MS; +} async function listAllTools(client: Client) { const tools: ListedTool[] = []; @@ -138,6 +231,9 @@ function toAgentToolResult(params: { async function disposeSession(session: BundleMcpSession) { session.detachStderr?.(); + if (session.transportType === "streamable-http") { + await (session.transport as StreamableHTTPClientTransport).terminateSession().catch(() => {}); + } await session.client.close().catch(() => {}); await session.transport.close().catch(() => {}); } @@ -220,6 +316,7 @@ export function createSessionMcpRuntime(params: { const servers: Record = {}; const tools: McpCatalogTool[] = []; + const usedServerNames = new Set(); try { for (const [serverName, rawServer] of Object.entries(loaded.mcpServers)) { @@ -228,6 +325,12 @@ export function createSessionMcpRuntime(params: { if (!resolved) { continue; } + const safeServerName = sanitizeServerName(serverName, usedServerNames); + if (safeServerName !== serverName) { + logWarn( + `bundle-mcp: server key "${serverName}" registered as "${safeServerName}" for provider-safe tool names.`, + ); + } const client = new Client( { @@ -240,13 +343,14 @@ export function createSessionMcpRuntime(params: { serverName, client, transport: resolved.transport, + transportType: resolved.transportType, detachStderr: resolved.detachStderr, }; sessions.set(serverName, session); try { failIfDisposed(); - await client.connect(resolved.transport); + await connectWithTimeout(client, resolved.transport, getConnectionTimeoutMs(rawServer)); failIfDisposed(); const listedTools = await listAllTools(client); failIfDisposed(); @@ -262,6 +366,7 @@ export function createSessionMcpRuntime(params: { } tools.push({ serverName, + safeServerName, toolName, title: tool.title, description: tool.description?.trim() || undefined, @@ -272,7 +377,7 @@ export function createSessionMcpRuntime(params: { } catch (error) { if (!disposed) { logWarn( - `bundle-mcp: failed to start server "${serverName}" (${resolved.description}): ${String(error)}`, + `bundle-mcp: failed to start server "${serverName}" (${resolved.description}): ${redactErrorUrls(error)}`, ); } await disposeSession(session); @@ -356,19 +461,23 @@ export async function materializeBundleMcpToolsForRun(params: { const tools: AnyAgentTool[] = []; for (const tool of catalog.tools) { - const normalizedName = tool.toolName.trim().toLowerCase(); - if (!normalizedName) { + const originalName = tool.toolName.trim(); + if (!originalName) { continue; } - if (reservedNames.has(normalizedName)) { + const safeToolName = buildSafeToolName({ + serverName: tool.safeServerName, + toolName: originalName, + reservedNames, + }); + if (safeToolName !== `${tool.safeServerName}${TOOL_NAME_SEPARATOR}${originalName}`) { logWarn( - `bundle-mcp: skipped tool "${tool.toolName}" from server "${tool.serverName}" because the name already exists.`, + `bundle-mcp: tool "${tool.toolName}" from server "${tool.serverName}" registered as "${safeToolName}" to keep the tool name provider-safe.`, ); - continue; } - reservedNames.add(normalizedName); + reservedNames.add(safeToolName.toLowerCase()); tools.push({ - name: tool.toolName, + name: safeToolName, label: tool.title ?? tool.toolName, description: tool.description || tool.fallbackDescription, parameters: tool.inputSchema,