mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 02:06:43 +00:00
fix(mcp): prevent custom server config races and parsing errors (#111761)
* fix(mcp): harden custom server management * fix(ui): keep config patch types private
This commit is contained in:
+1
-1
@@ -416,7 +416,7 @@ Notes:
|
||||
|
||||
- `list` sorts server names.
|
||||
- `show` without a name prints the full configured MCP server object.
|
||||
- `status` classifies configured transports without connecting. `--verbose` includes resolved launch, timeout, OAuth, filter, and parallel-call details, including when stored OAuth tokens require additional authorization.
|
||||
- `status` classifies configured transports without connecting. `--verbose` includes resolved launch, timeout, OAuth, filter, and parallel-call details, including when stored OAuth tokens require additional authorization. Credential-bearing stdio arguments are redacted in text and JSON output.
|
||||
- `doctor` performs static checks without connecting. Add `--probe` when the command should also verify that enabled servers connect.
|
||||
- `probe` connects and reports tool counts, resources/prompts support, list-change support, and diagnostics.
|
||||
- `add` accepts stdio flags such as `--command`, `--arg`, `--env`, and `--cwd`, or HTTP flags such as `--url`, `--transport`, `--header`, `--auth oauth`, TLS, timeout, and tool-selection flags.
|
||||
|
||||
@@ -327,8 +327,8 @@ Typical workflow:
|
||||
1. Open **MCP** from the sidebar.
|
||||
2. Check the summary cards for total, enabled, OAuth, and filtered server counts.
|
||||
3. Review each server row for transport, enablement, auth, filters, timeouts, and command hints.
|
||||
4. Add, enable, disable, or remove servers directly on the MCP page. Use the **Plugins** page for one-click connectors and discovery.
|
||||
5. Edit the scoped `mcp` config section for server definitions, headers, TLS/mTLS paths, OAuth metadata, tool filters, and Codex projection metadata.
|
||||
4. Add, enable, disable, or remove servers directly on the MCP page. Choose Streamable HTTP, SSE, or stdio explicitly; stdio command lines accept quoted arguments such as paths with spaces. Use the **Plugins** page for one-click connectors and discovery.
|
||||
5. Edit the scoped `mcp` config section for advanced server fields such as environment variables, working directories, headers, TLS/mTLS paths, OAuth metadata, tool filters, and Codex projection metadata.
|
||||
6. Use **Save** for a config write, or **Save & Publish** when the running Gateway should apply the changed config.
|
||||
7. Run `openclaw mcp status --verbose`, `openclaw mcp doctor --probe`, or `openclaw mcp reload` from a terminal for static diagnostics, live proof, or cached-runtime disposal.
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
* Accepts OpenClaw and upstream MCP config field names, keeping only
|
||||
* command/args/env/cwd needed to spawn a stdio server.
|
||||
*/
|
||||
import { redactSensitiveArgv } from "../config/redact-argv.js";
|
||||
import { isMcpConfigRecord, toMcpEnvRecord, toMcpStringArray } from "./mcp-config-shared.js";
|
||||
|
||||
/** Normalized stdio MCP server launch config. */
|
||||
@@ -53,8 +54,8 @@ export function resolveStdioMcpServerLaunchConfig(
|
||||
|
||||
/** Describe a stdio MCP launch config for diagnostics. */
|
||||
export function describeStdioMcpServerLaunchConfig(config: StdioMcpServerLaunchConfig): string {
|
||||
const args =
|
||||
Array.isArray(config.args) && config.args.length > 0 ? ` ${config.args.join(" ")}` : "";
|
||||
const redactedArgs = Array.isArray(config.args) ? redactSensitiveArgv(config.args) : [];
|
||||
const args = redactedArgs.length > 0 ? ` ${redactedArgs.join(" ")}` : "";
|
||||
const cwd = config.cwd ? ` (cwd=${config.cwd})` : "";
|
||||
return `${config.command}${args}${cwd}`;
|
||||
}
|
||||
|
||||
@@ -477,6 +477,36 @@ describe("mcp cli", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("redacts stdio argv credentials from MCP status output", async () => {
|
||||
await withTempHome("openclaw-cli-mcp-home-", async () => {
|
||||
const workspaceDir = await createWorkspace();
|
||||
vi.spyOn(process, "cwd").mockReturnValue(workspaceDir);
|
||||
|
||||
await runMcpCommand([
|
||||
"mcp",
|
||||
"set",
|
||||
"local",
|
||||
'{"command":"node","args":["server.mjs","--api-key","test-api-key","--token=test-token"]}',
|
||||
]);
|
||||
mockLog.mockClear();
|
||||
|
||||
await runMcpCommand(["mcp", "status", "--json"]);
|
||||
const jsonOutput = lastLogLine();
|
||||
expect(jsonOutput).not.toContain("test-api-key");
|
||||
expect(jsonOutput).not.toContain("test-token");
|
||||
expect(JSON.parse(jsonOutput).servers[0].launch).toBe(
|
||||
"node server.mjs --api-key *** --token=***",
|
||||
);
|
||||
|
||||
mockLog.mockClear();
|
||||
await runMcpCommand(["mcp", "status", "--verbose"]);
|
||||
const verboseOutput = mockLog.mock.calls.map(([line]) => String(line)).join("\n");
|
||||
expect(verboseOutput).toContain("launch: node server.mjs --api-key *** --token=***");
|
||||
expect(verboseOutput).not.toContain("test-api-key");
|
||||
expect(verboseOutput).not.toContain("test-token");
|
||||
});
|
||||
});
|
||||
|
||||
it("includes OAuth credential status in MCP status output", async () => {
|
||||
await withTempHome("openclaw-cli-mcp-home-", async () => {
|
||||
const workspaceDir = await createWorkspace();
|
||||
|
||||
@@ -86,9 +86,4 @@ export type McpConfig = {
|
||||
/** Dedicated listener port. Defaults to the Gateway port plus one. */
|
||||
sandboxPort?: number;
|
||||
};
|
||||
/**
|
||||
* Idle TTL for session-scoped bundled MCP runtimes, in milliseconds.
|
||||
*
|
||||
* Defaults to 10 minutes. Set to 0 to disable idle eviction.
|
||||
*/
|
||||
};
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { isRecord } from "@openclaw/normalization-core/record-coerce";
|
||||
// Plugin MCP tool handlers route plugin tool calls through the active runtime.
|
||||
import {
|
||||
@@ -82,7 +83,7 @@ export function createPluginToolsMcpHandlers(tools: AnyAgentTool[]) {
|
||||
};
|
||||
}
|
||||
try {
|
||||
const result = await tool.execute(`mcp-${Date.now()}`, params.arguments ?? {}, signal);
|
||||
const result = await tool.execute(`mcp-${randomUUID()}`, params.arguments ?? {}, signal);
|
||||
const rawContent =
|
||||
result && typeof result === "object" && "content" in result
|
||||
? (result as { content?: unknown }).content
|
||||
|
||||
@@ -46,7 +46,9 @@ describe("plugin tools MCP client bridge", () => {
|
||||
});
|
||||
|
||||
expect(execute).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/^mcp-\d+$/),
|
||||
expect.stringMatching(
|
||||
/^mcp-[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/u,
|
||||
),
|
||||
{ query: "ORBIT-9 codename", maxResults: 3 },
|
||||
expect.any(AbortSignal),
|
||||
undefined,
|
||||
|
||||
@@ -215,17 +215,17 @@ describe("plugin tools MCP server", () => {
|
||||
const executeCall = requireFirstMockCall(execute.mock.calls, "plugin tool execute");
|
||||
const requestId = executeCall[0];
|
||||
expect(typeof requestId).toBe("string");
|
||||
expect((requestId as string).startsWith("mcp-")).toBe(true);
|
||||
expect(Number.isSafeInteger(Number((requestId as string).slice("mcp-".length)))).toBe(true);
|
||||
expect(requestId).toMatch(
|
||||
/^mcp-[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/u,
|
||||
);
|
||||
expect(executeCall[1]).toEqual({ query: "remember this" });
|
||||
expect(executeCall[2]).toBeUndefined();
|
||||
expect(executeCall[3]).toBeUndefined();
|
||||
expect(result.content).toEqual([{ type: "text", text: "Stored." }]);
|
||||
});
|
||||
|
||||
it("releases execution tracking after repeated direct MCP calls", async () => {
|
||||
let now = 1_000;
|
||||
vi.spyOn(Date, "now").mockImplementation(() => now++);
|
||||
it("uses unique ids and releases execution tracking after repeated direct MCP calls", async () => {
|
||||
vi.spyOn(Date, "now").mockReturnValue(1_000);
|
||||
const executeSuccess = vi.fn().mockResolvedValue({ content: "Stored." });
|
||||
const executeFailure = vi.fn().mockRejectedValue(new Error("unavailable"));
|
||||
const handlers = createPluginToolsMcpHandlers([
|
||||
@@ -250,8 +250,12 @@ describe("plugin tools MCP server", () => {
|
||||
|
||||
expect(executeSuccess).toHaveBeenCalledTimes(32);
|
||||
expect(executeFailure).toHaveBeenCalledTimes(32);
|
||||
for (const [toolCallId] of [...executeSuccess.mock.calls, ...executeFailure.mock.calls]) {
|
||||
expect(consumeTrackedToolExecutionStarted(String(toolCallId))).toBeUndefined();
|
||||
const toolCallIds = [...executeSuccess.mock.calls, ...executeFailure.mock.calls].map(
|
||||
([toolCallId]) => String(toolCallId),
|
||||
);
|
||||
expect(new Set(toolCallIds).size).toBe(toolCallIds.length);
|
||||
for (const toolCallId of toolCallIds) {
|
||||
expect(consumeTrackedToolExecutionStarted(toolCallId)).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { html, type TemplateResult } from "lit";
|
||||
import { t } from "../i18n/index.ts";
|
||||
import type { McpServerTransport } from "../lib/config/mcp-servers.ts";
|
||||
|
||||
export type McpServerForm = {
|
||||
name: string;
|
||||
transport: McpServerTransport;
|
||||
target: string;
|
||||
};
|
||||
|
||||
@@ -19,9 +21,11 @@ export function renderMcpServerForm(props: {
|
||||
const form = event.currentTarget as HTMLFormElement;
|
||||
const data = new FormData(form);
|
||||
const name = data.get("mcp-name");
|
||||
const transport = data.get("mcp-transport");
|
||||
const target = data.get("mcp-target");
|
||||
props.onSubmit({
|
||||
name: typeof name === "string" ? name.trim() : "",
|
||||
transport: transport === "sse" || transport === "stdio" ? transport : "streamable-http",
|
||||
target: typeof target === "string" ? target.trim() : "",
|
||||
});
|
||||
};
|
||||
@@ -40,6 +44,19 @@ export function renderMcpServerForm(props: {
|
||||
?disabled=${disabled}
|
||||
/>
|
||||
</label>
|
||||
<label>
|
||||
<span>${t("mcpServers.transportLabel")}</span>
|
||||
<select
|
||||
name="mcp-transport"
|
||||
class="settings-select"
|
||||
title=${props.blockedReason ?? ""}
|
||||
?disabled=${disabled}
|
||||
>
|
||||
<option value="streamable-http">${t("mcpServers.transportStreamableHttp")}</option>
|
||||
<option value="sse">${t("mcpServers.transportSse")}</option>
|
||||
<option value="stdio">${t("mcpServers.transportStdio")}</option>
|
||||
</select>
|
||||
</label>
|
||||
<label class="mcp-server-form__target">
|
||||
<span>${t("mcpServers.targetLabel")}</span>
|
||||
<input
|
||||
|
||||
@@ -61,13 +61,29 @@ function createRuntimeConfig(config: Record<string, unknown>): RuntimeConfigHarn
|
||||
(options: { raw: Record<string, unknown>; note: string }) => Promise<boolean>
|
||||
>(async () => true);
|
||||
const listeners = new Set<() => void>();
|
||||
const runtimeConfig = {
|
||||
state: {
|
||||
configSnapshot: { sourceConfig: config, hash: "base" },
|
||||
lastError: null,
|
||||
const state = {
|
||||
configSnapshot: { sourceConfig: config, hash: "base" },
|
||||
lastError: null as string | null,
|
||||
};
|
||||
const patchFromSnapshot = vi.fn(
|
||||
async (
|
||||
build: (
|
||||
config: Readonly<Record<string, unknown>>,
|
||||
) => { options: { raw: Record<string, unknown>; note: string } } | { error: string },
|
||||
) => {
|
||||
const built = build(config);
|
||||
if ("error" in built) {
|
||||
state.lastError = built.error;
|
||||
return false;
|
||||
}
|
||||
return patch(built.options);
|
||||
},
|
||||
);
|
||||
const runtimeConfig = {
|
||||
state,
|
||||
ensureLoaded,
|
||||
patch,
|
||||
patchFromSnapshot,
|
||||
refresh: vi.fn(async () => undefined),
|
||||
subscribe(listener: () => void) {
|
||||
listeners.add(listener);
|
||||
@@ -112,12 +128,21 @@ function actionButton(container: Element, label: string): HTMLButtonElement {
|
||||
return expectDefined(button, `${label} button`);
|
||||
}
|
||||
|
||||
async function submitAddForm(card: McpServersCard, name: string, target: string) {
|
||||
async function submitAddForm(
|
||||
card: McpServersCard,
|
||||
name: string,
|
||||
target: string,
|
||||
transport: "streamable-http" | "sse" | "stdio" = "streamable-http",
|
||||
) {
|
||||
actionButton(card, "Add server").click();
|
||||
await card.updateComplete;
|
||||
const form = expectDefined(card.querySelector<HTMLFormElement>(".mcp-server-form"), "MCP form");
|
||||
expectDefined(form.querySelector<HTMLInputElement>('[name="mcp-name"]'), "name input").value =
|
||||
name;
|
||||
expectDefined(
|
||||
form.querySelector<HTMLSelectElement>('[name="mcp-transport"]'),
|
||||
"transport select",
|
||||
).value = transport;
|
||||
expectDefined(form.querySelector<HTMLInputElement>('[name="mcp-target"]'), "target input").value =
|
||||
target;
|
||||
form.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true }));
|
||||
@@ -158,6 +183,11 @@ describe("openclaw-mcp-servers-card", () => {
|
||||
supportsParallelToolCalls: true,
|
||||
clientCert: "/tmp/client.pem",
|
||||
},
|
||||
mixed: {
|
||||
command: "node",
|
||||
url: "https://mcp.example.com/mcp?token=test-token",
|
||||
transport: "streamable-http",
|
||||
},
|
||||
// Config files can carry names the add form would reject; the
|
||||
// command snippet must stay shell-safe for copy/paste.
|
||||
"docs; echo unsafe": { url: "https://mcp.example.com/mcp" },
|
||||
@@ -168,7 +198,7 @@ describe("openclaw-mcp-servers-card", () => {
|
||||
|
||||
const docs = expectDefined(card.querySelector('[data-mcp-name="docs"]'), "docs row");
|
||||
expect(docs.textContent).toContain("https://mcp.example.com/mcp?keep=visible&token=***");
|
||||
expect(docs.textContent).toContain("http · oauth · tool filter · TLS verify off");
|
||||
expect(docs.textContent).toContain("sse · oauth · tool filter · TLS verify off");
|
||||
expect(docs.textContent).toContain("openclaw mcp login docs");
|
||||
expect(docs.textContent).not.toContain("test-token");
|
||||
|
||||
@@ -179,6 +209,12 @@ describe("openclaw-mcp-servers-card", () => {
|
||||
expect(local.textContent).not.toContain("server.js");
|
||||
expect(local.textContent).not.toContain("test-token");
|
||||
|
||||
const mixed = expectDefined(card.querySelector('[data-mcp-name="mixed"]'), "mixed row");
|
||||
expect(mixed.textContent).toContain("node");
|
||||
expect(mixed.textContent).toContain("stdio");
|
||||
expect(mixed.textContent).not.toContain("mcp.example.com");
|
||||
expect(mixed.textContent).not.toContain("test-token");
|
||||
|
||||
const hostile = expectDefined(
|
||||
card.querySelector('[data-mcp-name="docs; echo unsafe"]'),
|
||||
"hostile-name row",
|
||||
@@ -196,24 +232,39 @@ describe("openclaw-mcp-servers-card", () => {
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "streamable HTTP URL",
|
||||
target: "https://mcp.context7.com/mcp",
|
||||
expected: { url: "https://mcp.context7.com/mcp", transport: "streamable-http" },
|
||||
label: "streamable HTTP URL whose path ends in /sse",
|
||||
transport: "streamable-http" as const,
|
||||
target: "https://mcp.context7.com/sse",
|
||||
expected: { url: "https://mcp.context7.com/sse", transport: "streamable-http" },
|
||||
},
|
||||
{
|
||||
label: "SSE URL",
|
||||
target: "https://mcp.example.com/sse?token=test",
|
||||
expected: { url: "https://mcp.example.com/sse?token=test", transport: "sse" },
|
||||
label: "SSE URL with an arbitrary endpoint path",
|
||||
transport: "sse" as const,
|
||||
target: "https://mcp.example.com/events?token=test",
|
||||
expected: { url: "https://mcp.example.com/events?token=test", transport: "sse" },
|
||||
},
|
||||
{
|
||||
label: "stdio command",
|
||||
target: "npx some-mcp-server --stdio",
|
||||
expected: { command: "npx", args: ["some-mcp-server", "--stdio"] },
|
||||
label: "stdio command with a quoted argument",
|
||||
transport: "stdio" as const,
|
||||
target: 'npx some-mcp-server "/Users/alice/My Files"',
|
||||
expected: { command: "npx", args: ["some-mcp-server", "/Users/alice/My Files"] },
|
||||
},
|
||||
])("adds a server from a $label", async ({ target, expected }) => {
|
||||
{
|
||||
label: "stdio command with a UNC path",
|
||||
transport: "stdio" as const,
|
||||
target: '"\\\\server\\share\\mcp.exe" --stdio',
|
||||
expected: { command: "\\\\server\\share\\mcp.exe", args: ["--stdio"] },
|
||||
},
|
||||
{
|
||||
label: "stdio command with a quoted Windows directory ending in a backslash",
|
||||
transport: "stdio" as const,
|
||||
target: 'server.exe "C:\\Program Files\\\\"',
|
||||
expected: { command: "server.exe", args: ["C:\\Program Files\\"] },
|
||||
},
|
||||
])("adds a server from a $label", async ({ transport, target, expected }) => {
|
||||
const { card, harness } = await mountCard();
|
||||
|
||||
await submitAddForm(card, "context7", target);
|
||||
await submitAddForm(card, "context7", target, transport);
|
||||
|
||||
await waitForFast(() => expect(harness.patch).toHaveBeenCalledOnce());
|
||||
expect(firstPatchCall(harness)).toEqual({
|
||||
@@ -239,6 +290,33 @@ describe("openclaw-mcp-servers-card", () => {
|
||||
expect(harness.patch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "stdio URL",
|
||||
transport: "stdio" as const,
|
||||
target: "https://mcp.example.com/mcp",
|
||||
},
|
||||
{
|
||||
label: "HTTP command",
|
||||
transport: "streamable-http" as const,
|
||||
target: "npx some-mcp-server",
|
||||
},
|
||||
{
|
||||
label: "unterminated stdio quote",
|
||||
transport: "stdio" as const,
|
||||
target: 'npx some-mcp-server "unfinished',
|
||||
},
|
||||
])("rejects a mismatched or malformed $label", async ({ transport, target }) => {
|
||||
const { card, harness } = await mountCard();
|
||||
|
||||
await submitAddForm(card, "docs", target, transport);
|
||||
|
||||
await waitForFast(() =>
|
||||
expect(card.querySelector('[role="alert"]')?.textContent).toContain("valid command line"),
|
||||
);
|
||||
expect(harness.patch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a duplicate name before patching", async () => {
|
||||
const { card, harness } = await mountCard({
|
||||
config: { mcp: { servers: { docs: { url: "https://mcp.example.com/mcp" } } } },
|
||||
|
||||
@@ -124,7 +124,7 @@ class McpServersCard extends OpenClawLightDomElement {
|
||||
this.message = { kind: "error", text: t("mcpServers.nameInvalid") };
|
||||
return;
|
||||
}
|
||||
const config = parseMcpTarget(form.target);
|
||||
const config = parseMcpTarget(form.target, form.transport);
|
||||
if (!config) {
|
||||
this.message = { kind: "error", text: t("mcpServers.targetInvalid") };
|
||||
return;
|
||||
|
||||
@@ -1946,9 +1946,13 @@ export const en: TranslationMap = {
|
||||
add: "Add server",
|
||||
adding: "Adding…",
|
||||
nameLabel: "Name",
|
||||
transportLabel: "Transport",
|
||||
transportStreamableHttp: "Streamable HTTP",
|
||||
transportSse: "SSE",
|
||||
transportStdio: "Stdio",
|
||||
targetLabel: "URL or command",
|
||||
nameInvalid: "Server names use letters, numbers, dots, dashes, or underscores.",
|
||||
targetInvalid: "Enter an http(s) URL or a command line.",
|
||||
targetInvalid: "Enter a URL for HTTP transports or a valid command line for stdio.",
|
||||
nameTaken: "An MCP server named “{name}” already exists.",
|
||||
missing: "MCP server “{name}” was not found in the configuration.",
|
||||
missingTransport: "missing transport",
|
||||
|
||||
@@ -1972,11 +1972,18 @@ describe("config form auto-save", () => {
|
||||
// Patch during the debounce window: the draft must be flushed as a real
|
||||
// save before the patch, not silently dropped with its timer.
|
||||
runtimeConfig.patchForm(["count"], 2);
|
||||
let patchBaseCount: unknown;
|
||||
await expect(
|
||||
runtimeConfig.patch({ raw: { other: true }, note: "test patch after autosave" }),
|
||||
runtimeConfig.patchFromSnapshot((config) => {
|
||||
patchBaseCount = config.count;
|
||||
return {
|
||||
options: { raw: { other: true }, note: "test patch after autosave" },
|
||||
};
|
||||
}),
|
||||
).resolves.toBe(true);
|
||||
|
||||
expect(order).toEqual(["config.set", "config.patch"]);
|
||||
expect(patchBaseCount).toBe(2);
|
||||
expect(runtimeConfig.state.configFormDirty).toBe(false);
|
||||
runtimeConfig.dispose();
|
||||
});
|
||||
|
||||
+36
-18
@@ -107,6 +107,7 @@ export type RuntimeConfigCapability = {
|
||||
ensureAgentEntry: (agentId: string) => number;
|
||||
stageDefaultAgent: (agentId: string) => boolean;
|
||||
patch: (options: ConfigPatchOptions) => Promise<boolean>;
|
||||
patchFromSnapshot: (build: ConfigPatchBuilder) => Promise<boolean>;
|
||||
lookupSchemaPath: (path: string) => Promise<unknown>;
|
||||
subscribe: (listener: (state: ConfigState) => void) => () => void;
|
||||
dispose: () => void;
|
||||
@@ -123,6 +124,9 @@ type ConfigPatchOptions = {
|
||||
replacePaths?: string[];
|
||||
};
|
||||
|
||||
type ConfigPatchBuildResult = { options: ConfigPatchOptions } | { error: string };
|
||||
type ConfigPatchBuilder = (config: Readonly<Record<string, unknown>>) => ConfigPatchBuildResult;
|
||||
|
||||
type ConfigGatewayClient = {
|
||||
request<T = unknown>(method: string, params?: unknown): Promise<T>;
|
||||
};
|
||||
@@ -1582,6 +1586,30 @@ export function createRuntimeConfigCapability(
|
||||
publish();
|
||||
});
|
||||
|
||||
const queueConfigPatch = (resolveOptions: () => ConfigPatchBuildResult): Promise<boolean> => {
|
||||
cancelAppliedRefresh();
|
||||
if (autoSaveTimer) {
|
||||
cancelScheduledAutoSave();
|
||||
runAutoSave();
|
||||
}
|
||||
return afterPendingWritesSettled(async () => {
|
||||
// A drained autosave can start its own refresh while this patch waits.
|
||||
cancelAppliedRefresh();
|
||||
try {
|
||||
const resolved = resolveOptions();
|
||||
if ("error" in resolved) {
|
||||
state.lastError = resolved.error;
|
||||
return false;
|
||||
}
|
||||
return await patchConfig(state, resolved.options);
|
||||
} finally {
|
||||
reconcileAppliedRefresh();
|
||||
}
|
||||
}).finally(() => {
|
||||
scheduleAutoSave();
|
||||
});
|
||||
};
|
||||
|
||||
return {
|
||||
get state() {
|
||||
return state;
|
||||
@@ -1712,24 +1740,14 @@ export function createRuntimeConfigCapability(
|
||||
// Unlike save/apply, a patch does not submit the form draft — flush a
|
||||
// scheduled autosave into a flight first (the settle below drains it) and
|
||||
// re-arm the debounce after so a dirty form is never left timer-less.
|
||||
patch: (options) => {
|
||||
cancelAppliedRefresh();
|
||||
if (autoSaveTimer) {
|
||||
cancelScheduledAutoSave();
|
||||
runAutoSave();
|
||||
}
|
||||
return afterPendingWritesSettled(async () => {
|
||||
// A drained autosave can start its own refresh while this patch waits.
|
||||
cancelAppliedRefresh();
|
||||
try {
|
||||
return await patchConfig(state, options);
|
||||
} finally {
|
||||
reconcileAppliedRefresh();
|
||||
}
|
||||
}).finally(() => {
|
||||
scheduleAutoSave();
|
||||
});
|
||||
},
|
||||
patch: (options) => queueConfigPatch(() => ({ options })),
|
||||
patchFromSnapshot: (build) =>
|
||||
queueConfigPatch(() => {
|
||||
const config = resolveEditableSnapshotConfig(state.configSnapshot);
|
||||
return config
|
||||
? build(config)
|
||||
: { error: "Configuration is unavailable; refresh and try again." };
|
||||
}),
|
||||
lookupSchemaPath: (path) => run(() => lookupConfigSchemaPath(state, path)),
|
||||
subscribe(listener) {
|
||||
listeners.add(listener);
|
||||
|
||||
@@ -1,14 +1,16 @@
|
||||
import { redactSensitiveUrlLikeString } from "@openclaw/net-policy/redact-sensitive-url";
|
||||
import { asNullableRecord as asRecord } from "@openclaw/normalization-core/record-coerce";
|
||||
import { t } from "../../i18n/index.ts";
|
||||
import { resolveEditableSnapshotConfig, type RuntimeConfigCapability } from "./index.ts";
|
||||
import type { RuntimeConfigCapability } from "./index.ts";
|
||||
|
||||
export const MCP_SERVER_NAME_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
|
||||
|
||||
export type McpServerTransport = "streamable-http" | "sse" | "stdio";
|
||||
|
||||
export type McpServerSummary = {
|
||||
name: string;
|
||||
enabled: boolean;
|
||||
transport: "http" | "stdio" | "invalid";
|
||||
transport: McpServerTransport | "invalid";
|
||||
target: string;
|
||||
auth: string | null;
|
||||
toolFilter: boolean;
|
||||
@@ -18,14 +20,93 @@ export type McpServerSummary = {
|
||||
|
||||
export type McpServersPatchBuildResult = { patch: Record<string, unknown> } | { error: string };
|
||||
|
||||
export function parseMcpTarget(target: string): Record<string, unknown> | null {
|
||||
if (/^https?:\/\//i.test(target)) {
|
||||
// The runtime defaults URL-only servers to SSE; modern MCP endpoints are
|
||||
// streamable HTTP unless the /sse path convention says otherwise.
|
||||
const transport = /\/sse\/?$/i.test(target.split("?")[0] ?? target) ? "sse" : "streamable-http";
|
||||
return { url: target, transport };
|
||||
function splitMcpCommandLine(value: string): string[] | null {
|
||||
const parts: string[] = [];
|
||||
let current = "";
|
||||
let quote: "'" | '"' | null = null;
|
||||
let tokenStarted = false;
|
||||
|
||||
for (let index = 0; index < value.length; index += 1) {
|
||||
const char = value[index] ?? "";
|
||||
if (char === "\\" && quote === '"') {
|
||||
let slashCount = 1;
|
||||
while (value[index + slashCount] === "\\") {
|
||||
slashCount += 1;
|
||||
}
|
||||
const next = value[index + slashCount];
|
||||
if (next === '"') {
|
||||
current += "\\".repeat(Math.floor(slashCount / 2));
|
||||
if (slashCount % 2 === 0) {
|
||||
quote = null;
|
||||
} else {
|
||||
current += '"';
|
||||
}
|
||||
index += slashCount;
|
||||
} else {
|
||||
current += "\\".repeat(slashCount);
|
||||
index += slashCount - 1;
|
||||
}
|
||||
tokenStarted = true;
|
||||
continue;
|
||||
}
|
||||
if (char === "\\" && quote === null) {
|
||||
const next = value[index + 1];
|
||||
if (next && (next === '"' || next === "'" || /\s/u.test(next))) {
|
||||
current += next;
|
||||
tokenStarted = true;
|
||||
index += 1;
|
||||
} else {
|
||||
current += char;
|
||||
tokenStarted = true;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (quote) {
|
||||
if (char === quote) {
|
||||
quote = null;
|
||||
} else {
|
||||
current += char;
|
||||
}
|
||||
tokenStarted = true;
|
||||
continue;
|
||||
}
|
||||
if (char === "'" || char === '"') {
|
||||
quote = char;
|
||||
tokenStarted = true;
|
||||
continue;
|
||||
}
|
||||
if (/\s/u.test(char)) {
|
||||
if (tokenStarted) {
|
||||
parts.push(current);
|
||||
current = "";
|
||||
tokenStarted = false;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
current += char;
|
||||
tokenStarted = true;
|
||||
}
|
||||
const [command, ...args] = target.trim().split(/\s+/u);
|
||||
|
||||
if (quote) {
|
||||
return null;
|
||||
}
|
||||
if (tokenStarted) {
|
||||
parts.push(current);
|
||||
}
|
||||
return parts;
|
||||
}
|
||||
|
||||
export function parseMcpTarget(
|
||||
target: string,
|
||||
transport: McpServerTransport,
|
||||
): Record<string, unknown> | null {
|
||||
if (transport !== "stdio") {
|
||||
return /^https?:\/\//i.test(target) ? { url: target, transport } : null;
|
||||
}
|
||||
if (/^https?:\/\//i.test(target)) {
|
||||
return null;
|
||||
}
|
||||
const [command, ...args] = splitMcpCommandLine(target.trim()) ?? [];
|
||||
if (!command) {
|
||||
return null;
|
||||
}
|
||||
@@ -46,11 +127,20 @@ export function summarizeMcpServers(
|
||||
// Command only: stdio args routinely carry tokens, and this projection
|
||||
// is visible to read-only operators.
|
||||
const command = typeof server.command === "string" ? server.command : "";
|
||||
const transport = command
|
||||
? ("stdio" as const)
|
||||
: url
|
||||
? server.transport === "streamable-http"
|
||||
? ("streamable-http" as const)
|
||||
: server.transport === undefined || server.transport === "sse"
|
||||
? ("sse" as const)
|
||||
: ("invalid" as const)
|
||||
: ("invalid" as const);
|
||||
return {
|
||||
name,
|
||||
enabled: server.enabled !== false,
|
||||
transport: url ? ("http" as const) : command ? ("stdio" as const) : ("invalid" as const),
|
||||
target: url ? redactSensitiveUrlLikeString(url) : command,
|
||||
transport,
|
||||
target: command || redactSensitiveUrlLikeString(url),
|
||||
auth: typeof server.auth === "string" ? server.auth : null,
|
||||
toolFilter: Boolean(server.toolFilter),
|
||||
parallel: server.supportsParallelToolCalls === true,
|
||||
@@ -98,8 +188,9 @@ export function buildRemoveMcpServerPatch(
|
||||
|
||||
/**
|
||||
* Apply one mutation to config.mcp.servers through the shared config seam.
|
||||
* config.patch uses RFC 7396 merge semantics, so `buildPatch` must return a
|
||||
* minimal fragment (with explicit nulls for deletions), never a full config.
|
||||
* The builder runs after pending editor writes settle so duplicate/existence
|
||||
* decisions use the same snapshot as the patch base hash. config.patch uses
|
||||
* RFC 7396 semantics, so return a minimal fragment with explicit deletion nulls.
|
||||
*/
|
||||
export async function patchMcpServers(
|
||||
runtimeConfig: RuntimeConfigCapability,
|
||||
@@ -110,18 +201,17 @@ export async function patchMcpServers(
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> {
|
||||
try {
|
||||
await runtimeConfig.ensureLoaded();
|
||||
const base = resolveEditableSnapshotConfig(runtimeConfig.state.configSnapshot);
|
||||
if (!base) {
|
||||
return { ok: false, error: t("mcpServers.configUnavailable") };
|
||||
}
|
||||
const servers = asRecord(asRecord(base.mcp)?.servers) ?? {};
|
||||
const built = options.buildPatch(servers);
|
||||
if ("error" in built) {
|
||||
return { ok: false, error: built.error };
|
||||
}
|
||||
const patched = await runtimeConfig.patch({
|
||||
raw: { mcp: { servers: built.patch } },
|
||||
note: options.note,
|
||||
const patched = await runtimeConfig.patchFromSnapshot((base) => {
|
||||
const servers = asRecord(asRecord(base.mcp)?.servers) ?? {};
|
||||
const built = options.buildPatch(servers);
|
||||
return "error" in built
|
||||
? built
|
||||
: {
|
||||
options: {
|
||||
raw: { mcp: { servers: built.patch } },
|
||||
note: options.note,
|
||||
},
|
||||
};
|
||||
});
|
||||
if (!patched) {
|
||||
return {
|
||||
|
||||
@@ -132,6 +132,7 @@ type RuntimeConfigTestHarness = {
|
||||
patch: ReturnType<
|
||||
typeof vi.fn<(options: { raw: Record<string, unknown>; note: string }) => Promise<boolean>>
|
||||
>;
|
||||
patchFromSnapshot: ApplicationContext["runtimeConfig"]["patchFromSnapshot"];
|
||||
subscribe: (listener: (state: RuntimeConfigTestState) => void) => () => void;
|
||||
};
|
||||
notify: () => void;
|
||||
@@ -142,13 +143,23 @@ function createRuntimeConfigHarness(
|
||||
runtimeConfigState: RuntimeConfigTestState,
|
||||
): RuntimeConfigTestHarness {
|
||||
const listeners = new Set<(state: RuntimeConfigTestState) => void>();
|
||||
const patch = vi.fn<
|
||||
(options: { raw: Record<string, unknown>; note: string }) => Promise<boolean>
|
||||
>(async () => true);
|
||||
const runtimeConfig = {
|
||||
state: runtimeConfigState,
|
||||
refresh: refreshConfig,
|
||||
ensureLoaded: vi.fn(async () => undefined),
|
||||
patch: vi.fn<(options: { raw: Record<string, unknown>; note: string }) => Promise<boolean>>(
|
||||
async () => true,
|
||||
),
|
||||
patch,
|
||||
patchFromSnapshot: vi.fn(async (build) => {
|
||||
const config = runtimeConfigState.configSnapshot?.sourceConfig ?? {};
|
||||
const built = build(config);
|
||||
if ("error" in built) {
|
||||
runtimeConfigState.lastError = built.error;
|
||||
return false;
|
||||
}
|
||||
return patch(built.options);
|
||||
}),
|
||||
subscribe(listener: (state: RuntimeConfigTestState) => void) {
|
||||
listeners.add(listener);
|
||||
return () => listeners.delete(listener);
|
||||
|
||||
@@ -869,7 +869,7 @@ class PluginsPage extends OpenClawLightDomElement {
|
||||
this.mcpMessage = { kind: "error", text: t("mcpServers.nameInvalid") };
|
||||
return;
|
||||
}
|
||||
const config = parseMcpTarget(form.target);
|
||||
const config = parseMcpTarget(form.target, form.transport);
|
||||
if (!config) {
|
||||
this.mcpMessage = { kind: "error", text: t("mcpServers.targetInvalid") };
|
||||
return;
|
||||
|
||||
@@ -327,7 +327,7 @@ describe("renderPlugins", () => {
|
||||
{
|
||||
name: "github",
|
||||
enabled: true,
|
||||
transport: "http",
|
||||
transport: "streamable-http",
|
||||
target: "https://api.githubcopilot.com/mcp/",
|
||||
auth: "oauth",
|
||||
toolFilter: false,
|
||||
@@ -356,6 +356,7 @@ describe("renderPlugins", () => {
|
||||
form.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true }));
|
||||
expect(onMcpAdd).toHaveBeenCalledWith({
|
||||
name: "context7",
|
||||
transport: "streamable-http",
|
||||
target: "https://mcp.context7.com/mcp",
|
||||
});
|
||||
});
|
||||
@@ -454,7 +455,7 @@ describe("renderPlugins", () => {
|
||||
{
|
||||
name: "github",
|
||||
enabled: true,
|
||||
transport: "http",
|
||||
transport: "streamable-http",
|
||||
target: "https://x",
|
||||
auth: "oauth",
|
||||
toolFilter: false,
|
||||
|
||||
@@ -715,7 +715,7 @@
|
||||
* serves both the MCP settings page and the Plugins inventory. */
|
||||
.mcp-server-form {
|
||||
display: grid;
|
||||
grid-template-columns: minmax(140px, 220px) minmax(0, 1fr) auto;
|
||||
grid-template-columns: minmax(140px, 220px) minmax(150px, 190px) minmax(0, 1fr) auto;
|
||||
gap: var(--space-2);
|
||||
align-items: end;
|
||||
padding: var(--space-3) var(--space-4);
|
||||
|
||||
Reference in New Issue
Block a user