mirror of https://github.com/openclaw/openclaw.git
fix: port safer bundle MCP naming onto latest main (#49505) (thanks @ziomancer)
This commit is contained in:
Peter Steinberger
parent
004bffa1c3
commit
147c2c7389
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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"]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
return new Promise<void>((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>): 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>;
|
||||
}): 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<string, McpServerCatalog> = {};
|
||||
const tools: McpCatalogTool[] = [];
|
||||
const usedServerNames = new Set<string>();
|
||||
|
||||
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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue