MUL-2778 feat(agent): wire mcp_config through OpenClaw runtime (#3450)

* MUL-2778 feat(agent): wire mcp_config through OpenClaw runtime

The MCP config tab (#3419) lets admins save mcp_config on an agent, and
recent work (#3439) plumbed it through the three ACP runtimes. OpenClaw
still ignored the field, leaving the Tab silently inert for any
OpenClaw-backed agent.

Translate the agent's Claude-style `{"mcpServers": {...}}` into the
per-task OpenClaw wrapper's `mcp.servers` block — OpenClaw resolves MCP
via its own config schema rather than ExecOptions, so the existing
OPENCLAW_CONFIG_PATH preparer is the right seam. Fail closed on
malformed JSON / entries missing `command` or `url`, matching the
fail-closed posture the preparer already uses for the agents.list step.
Null / absent mcp_config leaves the wrapper free of an `mcp` key so the
user's global mcp.servers flows through untouched; an explicit empty
managed set (`{}` / `{"mcpServers":{}}`) is honoured as "admin saved no
servers" mirroring `hasManagedCodexMcpConfig`.

Strict-mode replacement (drop user-only servers entirely) would require
OpenClaw to do a per-key replace rather than a deep merge at
`mcp.servers`; the comment documents that caveat rather than relying on
undocumented behaviour.

Also adds `openclaw` to `MCP_SUPPORTED_PROVIDERS` so the MCP Tab
actually surfaces in the agent overview pane, and pins the new
visibility case with a renderPane test.

Co-authored-by: multica-agent <github@multica.ai>

* MUL-2778 fix(agent): make openclaw mcp_config strict-replace via sanitized snapshot

Elon flagged on #3450 that the previous wiring let user-only mcp.servers
leak through the wrapper's `$include` of the live user config: deep-merge
at `mcp.servers` keeps user-only names, and the strict-empty case
(`{ "mcpServers": {} }`) silently inherited user globals.

Switch the strict-replace path to write a sanitized snapshot of the
user's fully resolved config (via `openclaw config get --json`) with the
`mcp` block stripped, then have the wrapper `$include` the snapshot
instead of the live user file. With the user's `mcp` gone from the
$include resolution, the wrapper's `mcp.servers` is the only definition
the embedded OpenClaw sees — managed only, including the explicit empty
set.

The snapshot lives in envRoot at 0o600 alongside the wrapper so the GC
reaper sweeps it with the rest of the task scratch, and no extra
OPENCLAW_INCLUDE_ROOTS entry is needed (same-dir $include).

Fail-closed on `config get --json` errors so the daemon never silently
falls back to the leaky $include path. The inherit branch (null
mcp_config) still uses the live user file directly — no extra CLI
roundtrip and no snapshot is written.

New tests pin the contract Elon's review required:

- TestPrepareOpenclawConfigStrictReplacesUserMcpServers: user has
  global_one + shared, managed has shared + managed_only → wrapper has
  exactly {shared (managed value), managed_only}; global_one does NOT
  leak; snapshot file has the user's `mcp` stripped while preserving
  gateway / providers / API keys.
- TestPrepareOpenclawConfigStrictEmptyManagedSetDropsUserMcp: empty
  managed set drops user's global_one (both `{}` and
  `{"mcpServers":{}}` cases).
- TestPrepareOpenclawConfigNullMcpConfigKeepsUserInclude: null path
  inherits the live user config, writes no snapshot, makes no extra CLI
  call.
- TestPrepareOpenclawConfigFailsClosedOnResolvedConfigError: errors
  during `config get --json` surface; no stale wrapper or snapshot.
- TestPrepareOpenclawConfigManagedSetFreshInstall: fresh install with
  managed mcp_config skips the snapshot dance entirely.

Also tightens en + zh-Hans MCP Tab copy to mention OpenClaw goes via the
per-task wrapper, and to use OpenClaw's own `transport` field rather
than Claude's `type` for HTTP/SSE entries.

Co-authored-by: multica-agent <github@multica.ai>

* MUL-2778 fix(agent): narrow openclaw snapshot strip to mcp.servers only

Elon's third-round must-fix: the previous strict-replace snapshot deleted
the entire `mcp` block, which wiped out non-server settings under `mcp`
like `sessionIdleTtlMs`. Those are documented OpenClaw config keys
(https://docs.openclaw.ai/gateway/configuration-reference#mcp) outside
the MCP Tab's scope — the agent's saved mcp_config only manages server
definitions, so other `mcp.*` tuning the user set must survive.

Replace the blanket `delete(resolved, "mcp")` with a stripUserMcpServers
helper that:

- deletes only `mcp.servers` when `mcp` is an object
- drops the parent `mcp` key only when the object is empty after the
  strip (so we don't emit `mcp: {}` placeholders)
- leaves non-object `mcp` values untouched (we only know how to strip
  servers from the documented shape)

Pinned with TestPrepareOpenclawConfigStrictPreservesNonServerMcpKeys:
user resolved has both `mcp.sessionIdleTtlMs: 300000` and
`mcp.servers.global_one`; after the strict path runs the snapshot
keeps the TTL and drops the servers map, and the wrapper's
`mcp.servers` is exactly the managed set with no leak.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-05-28 18:43:02 +08:00
committed by GitHub
parent 16336ad33c
commit fa076d38f2
8 changed files with 677 additions and 17 deletions

View File

@@ -2,8 +2,11 @@
// forwards MCP servers to the underlying CLI. The MCP config tab is hidden
// for every other provider so a user can't save a value the runtime will
// silently ignore. Keep this list in sync with the backends in
// `server/pkg/agent/` that read `ExecOptions.McpConfig`.
const MCP_SUPPORTED_PROVIDERS = new Set(["claude", "codex"]);
// `server/pkg/agent/` that read `ExecOptions.McpConfig`, plus the OpenClaw
// per-task wrapper preparer in `server/internal/daemon/execenv/` which
// materialises `mcp.servers` into the synthesised config rather than going
// through ExecOptions.
const MCP_SUPPORTED_PROVIDERS = new Set(["claude", "codex", "openclaw"]);
export function providerSupportsMcpConfig(provider: string | undefined | null): boolean {
if (!provider) return false;

View File

@@ -109,6 +109,14 @@ describe("AgentOverviewPane MCP tab visibility", () => {
expect(screen.getByRole("button", { name: /^MCP$/i })).toBeInTheDocument();
});
it("renders the MCP tab when the agent runs on the OpenClaw runtime", () => {
// OpenClaw materialises mcp_config via the per-task wrapper config
// (OPENCLAW_CONFIG_PATH) rather than ExecOptions, but the Tab must still
// surface so admins can save the managed set.
renderPane([makeRuntime("openclaw")]);
expect(screen.getByRole("button", { name: /^MCP$/i })).toBeInTheDocument();
});
it("hides the MCP tab for providers whose backend does not read mcp_config", () => {
// Saving an MCP config on e.g. Gemini would be a silent no-op at run
// time — that's the bug this hiding logic is meant to prevent.

View File

@@ -296,7 +296,7 @@
"save_failed_toast": "Failed to save custom arguments"
},
"mcp_config": {
"intro": "MCP server configuration forwarded to the runtime CLI (Claude via --mcp-config, Codex via $CODEX_HOME/config.toml). Stored verbatim and may contain secrets — only the agent owner and workspace admins can read it. Leave empty to fall back to the CLI's own default.",
"intro": "MCP server configuration forwarded to the runtime CLI (Claude via --mcp-config, Codex via $CODEX_HOME/config.toml, OpenClaw via the per-task config wrapper). Stored verbatim and may contain secrets — only the agent owner and workspace admins can read it. Leave empty to fall back to the CLI's own default. For OpenClaw HTTP/SSE entries use OpenClaw's own field name (e.g. \"transport\": \"streamable-http\") instead of Claude's \"type\".",
"placeholder": "{\n \"mcpServers\": {\n \"fetch\": {\n \"command\": \"uvx\",\n \"args\": [\"mcp-server-fetch\"]\n }\n }\n}",
"editor_aria": "MCP config JSON editor",
"clear_action": "Clear",

View File

@@ -290,7 +290,7 @@
"save_failed_toast": "保存自定义参数失败"
},
"mcp_config": {
"intro": "转发给运行时 CLI 的 MCP 服务器配置Claude 通过 --mcp-configCodex 通过 $CODEX_HOME/config.toml。原样保存可能包含密钥 —— 只有智能体所有者和工作区管理员可以读取。留空则回退到 CLI 自身的默认设置。",
"intro": "转发给运行时 CLI 的 MCP 服务器配置Claude 通过 --mcp-configCodex 通过 $CODEX_HOME/config.tomlOpenClaw 通过每个任务的配置 wrapper)。原样保存,可能包含密钥 —— 只有智能体所有者和工作区管理员可以读取。留空则回退到 CLI 自身的默认设置。OpenClaw 的 HTTP/SSE 条目需要使用 OpenClaw 自己的字段名(例如 \"transport\": \"streamable-http\"),而不是 Claude 的 \"type\"。",
"placeholder": "{\n \"mcpServers\": {\n \"fetch\": {\n \"command\": \"uvx\",\n \"args\": [\"mcp-server-fetch\"]\n }\n }\n}",
"editor_aria": "MCP 配置 JSON 编辑器",
"clear_action": "清空",

View File

@@ -2540,12 +2540,17 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i
// WorkDir is the user's own path (always present) but the reuse path
// loses the envRoot association the GC loop needs, and re-running
// Prepare against a stable user path is cheap (no clone, no copy).
var agentMcpConfig json.RawMessage
if task.Agent != nil {
agentMcpConfig = task.Agent.McpConfig
}
if task.PriorWorkDir != "" && localAssignment == nil {
env = execenv.Reuse(execenv.ReuseParams{
WorkDir: task.PriorWorkDir,
Provider: provider,
CodexVersion: codexVersion,
OpenclawBin: openclawBin,
McpConfig: agentMcpConfig,
Task: taskCtx,
}, d.logger)
}
@@ -2559,6 +2564,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i
Provider: provider,
CodexVersion: codexVersion,
OpenclawBin: openclawBin,
McpConfig: agentMcpConfig,
Task: taskCtx,
}
if localAssignment != nil {

View File

@@ -39,6 +39,11 @@ type PrepareParams struct {
Provider string // agent provider (determines runtime config and skill injection paths)
CodexVersion string // detected Codex CLI version (only used when Provider == "codex")
OpenclawBin string // resolved openclaw CLI path (only used when Provider == "openclaw"); empty = look up on PATH
// McpConfig is the agent's saved `mcp_config` JSON, forwarded to the
// provider-specific config preparer when that provider materialises MCP
// via a per-task config file. Only OpenClaw consumes it here today; other
// providers wire MCP via ExecOptions.McpConfig in the agent backend.
McpConfig json.RawMessage
// LocalWorkDir, when non-empty, redirects the agent's working directory
// to a user-supplied absolute path instead of the synthesised envRoot/
// workdir. The path is NOT copied or mounted — the agent operates on
@@ -223,7 +228,10 @@ func Prepare(params PrepareParams, logger *slog.Logger) (*Environment, error) {
// silently degrading to a minimal config would mask it by booting
// OpenClaw without the agents / providers / API keys it expects.
if params.Provider == "openclaw" {
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: params.OpenclawBin})
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: params.OpenclawBin,
McpConfig: params.McpConfig,
})
if err != nil {
return nil, fmt.Errorf("execenv: prepare openclaw config: %w", err)
}
@@ -243,6 +251,11 @@ type ReuseParams struct {
Provider string
CodexVersion string // only used when Provider == "codex"
OpenclawBin string // only used when Provider == "openclaw"; empty = PATH lookup
// McpConfig is the agent's saved `mcp_config` JSON. Reused on reuse so a
// freshly-saved managed set re-materialises into the wrapper before the
// task starts — without this a stale wrapper from a prior run would keep
// the old MCP set in play.
McpConfig json.RawMessage
// LocalDirectory is true when the reused WorkDir is a user-supplied
// directory (the local_directory flow). The flag is propagated into
// the returned Environment so downstream callers (notably the GC
@@ -321,7 +334,10 @@ func Reuse(params ReuseParams, logger *slog.Logger) *Environment {
// reuse rather than degrade to a minimal config that boots OpenClaw
// without the registered agents.
if params.Provider == "openclaw" {
result, err := prepareOpenclawConfig(env.RootDir, params.WorkDir, OpenclawConfigPrep{OpenclawBin: params.OpenclawBin})
result, err := prepareOpenclawConfig(env.RootDir, params.WorkDir, OpenclawConfigPrep{
OpenclawBin: params.OpenclawBin,
McpConfig: params.McpConfig,
})
if err != nil {
logger.Warn("execenv: refresh openclaw config failed", "error", err)
return nil

View File

@@ -1,6 +1,7 @@
package execenv
import (
"bytes"
"context"
"encoding/json"
"errors"
@@ -8,6 +9,7 @@ import (
"os"
"os/exec"
"path/filepath"
"sort"
"strings"
"time"
)
@@ -18,6 +20,14 @@ import (
// the rest of the task env.
const openclawConfigFile = "openclaw-config.json"
// openclawUserSnapshotFile is the sanitized copy of the user's fully
// resolved openclaw config the wrapper $includes when the agent has a
// managed mcp_config. It is the user's config minus the `mcp` block so the
// wrapper's managed `mcp.servers` is the only MCP definition visible to
// OpenClaw — true strict-replace, not deep-merge-by-name. Lives in envRoot
// at 0o600 next to the wrapper.
const openclawUserSnapshotFile = "openclaw-user-snapshot.json"
// openclawCLITimeout caps each `openclaw config ...` invocation during task
// setup. The CLI is fast (<200ms normal); 5s leaves headroom for a cold
// node start without letting a hung CLI stall task dispatch indefinitely.
@@ -32,6 +42,13 @@ type OpenclawConfigPrep struct {
OpenclawBin string
// Timeout caps each CLI invocation. Zero falls back to openclawCLITimeout.
Timeout time.Duration
// McpConfig is the agent's saved `mcp_config` JSON (Claude-style
// `{"mcpServers": {"<name>": {...}}}`). When non-null the wrapper pins
// `mcp.servers` to the managed set so OpenClaw resolves MCP from the
// daemon's authoritative list instead of the user's global `mcp.servers`.
// Null / empty means inherit the user's global config — same three-state
// semantics codex uses (`hasManagedCodexMcpConfig`).
McpConfig json.RawMessage
}
// OpenclawConfigResult is what prepareOpenclawConfig returns to its callers
@@ -124,7 +141,58 @@ func prepareOpenclawConfig(envRoot, workDir string, opts OpenclawConfigPrep) (Op
}
}
cfg := buildPerTaskOpenclawConfig(activePath, exists, resolvedList, workDir)
// Parse the agent's managed mcp_config (if any) before writing the wrapper
// so a malformed value fails the prepare step rather than crashing the
// openclaw subprocess later. Same fail-closed posture as Codex's
// ensureCodexMcpConfig — silent fallback to the user's global mcp.servers
// would be indistinguishable from "the managed set applied" and is exactly
// the surprise the MCP Tab is supposed to remove.
managedMcp, hasManagedMcp, err := openclawManagedMcpServers(opts.McpConfig)
if err != nil {
return OpenclawConfigResult{}, fmt.Errorf("render openclaw mcp_config: %w", err)
}
// **Strict replace for managed mcp_config.** When the agent has a managed
// set, deep-merging the wrapper's `mcp.servers` against the user's active
// config via `$include` would let user-only entries leak in (and an empty
// managed set would not actually clear inherited servers). To enforce the
// Codex-style "managed wins, user globals invisible" contract, fetch the
// user's resolved config, drop just the `mcp.servers` map (keep other
// `mcp.*` settings like `sessionIdleTtlMs`), write a sanitized snapshot
// in envRoot, and $include the snapshot instead of the live user file.
// The wrapper's `mcp.servers` then becomes the only MCP server definition
// the snapshot's resolution can yield, while the user's surrounding `mcp`
// tuning still flows through.
snapshotPath := ""
if hasManagedMcp && exists {
resolved, ferr := openclawResolvedFullConfig(bin, timeout)
if ferr != nil {
return OpenclawConfigResult{}, fmt.Errorf("read openclaw resolved config: %w", ferr)
}
if resolved == nil {
// CLI reports the file exists but `config get --json` returned
// nothing structured. Treat as no user-config-to-strip: the
// wrapper will carry managed mcp.servers as the sole source.
exists = false
activePath = ""
} else {
stripUserMcpServers(resolved)
snapBytes, merr := json.MarshalIndent(resolved, "", " ")
if merr != nil {
return OpenclawConfigResult{}, fmt.Errorf("marshal openclaw user snapshot: %w", merr)
}
snapshotPath = filepath.Join(envRoot, openclawUserSnapshotFile)
// 0o600 — the snapshot is now a flat copy of the user's resolved
// config and may carry API keys / model-provider tokens that
// $include used to keep on disk in the user's own file. Lock the
// snapshot to the daemon owner; only the openclaw child reads it.
if werr := os.WriteFile(snapshotPath, snapBytes, 0o600); werr != nil {
return OpenclawConfigResult{}, fmt.Errorf("write openclaw user snapshot: %w", werr)
}
}
}
cfg := buildPerTaskOpenclawConfig(activePath, exists, snapshotPath, resolvedList, workDir, managedMcp, hasManagedMcp)
data, err := json.MarshalIndent(cfg, "", " ")
if err != nil {
@@ -138,11 +206,13 @@ func prepareOpenclawConfig(envRoot, workDir string, opts OpenclawConfigPrep) (Op
return OpenclawConfigResult{}, fmt.Errorf("write openclaw config: %w", err)
}
result := OpenclawConfigResult{ConfigPath: outPath}
if exists {
// Only emit an include root when we actually emit a $include line
// (i.e. the user has an on-disk config). On fresh install the
// wrapper is self-contained and OpenClaw never needs to step out
// of envRoot, so no extra root is required.
if snapshotPath != "" {
// Sanitized snapshot lives in envRoot alongside the wrapper, so the
// $include never crosses directories — daemon does not need to grant
// an extra OPENCLAW_INCLUDE_ROOTS entry.
} else if exists {
// Live user config is in its own directory; tell the daemon to grant
// it so OpenClaw's include-confinement check passes.
result.IncludeRoot = filepath.Dir(activePath)
}
return result, nil
@@ -163,7 +233,22 @@ func prepareOpenclawConfig(envRoot, workDir string, opts OpenclawConfigPrep) (Op
// config containing only the workspace override. There is no user data to
// $include here, so this is not the silent-fallback case the reviewer
// flagged.
func buildPerTaskOpenclawConfig(activePath string, exists bool, resolvedList []any, workDir string) map[string]any {
//
// snapshotPath, when non-empty, points at a sanitized copy of the user's
// resolved config (mcp stripped) sitting in envRoot. It is the $include
// target whenever the agent has a managed mcp_config — the live user file
// would otherwise leak global `mcp.servers` past the wrapper. When
// snapshotPath is empty the wrapper falls back to $include'ing the active
// path so secrets / nested includes stay in the user's own file (no
// managed mcp means there is nothing to enforce strictness against).
//
// hasManagedMcp distinguishes "agent has a managed mcp_config (possibly an
// empty set)" from "agent inherits the user's global mcp.servers". When
// true we pin `mcp.servers` to managedMcp on the wrapper. Because the
// snapshot $include has already dropped the user's `mcp` block, the
// resulting view of `mcp.servers` is exactly the managed set — including
// `{}` for "admin saved no servers" (mirrors `hasManagedCodexMcpConfig`).
func buildPerTaskOpenclawConfig(activePath string, exists bool, snapshotPath string, resolvedList []any, workDir string, managedMcp map[string]any, hasManagedMcp bool) map[string]any {
agents := map[string]any{
"defaults": map[string]any{"workspace": workDir},
}
@@ -173,10 +258,25 @@ func buildPerTaskOpenclawConfig(activePath string, exists bool, resolvedList []a
cfg := map[string]any{
"agents": agents,
}
if exists {
// Array form (not single-file form) so OpenClaw deep-merges the
// included object with our sibling keys rather than letting the
// include replace the whole containing object.
if hasManagedMcp {
// Always emit `mcp.servers` (even when empty) so the wrapper's intent
// — "admin manages this set" — is grep-able on disk and visible to
// OpenClaw's loader. The snapshot $include has already dropped the
// user's `mcp` block, so this becomes the only definition.
servers := managedMcp
if servers == nil {
servers = map[string]any{}
}
cfg["mcp"] = map[string]any{"servers": servers}
}
switch {
case snapshotPath != "":
// Sanitized snapshot path; strict-replace flow for managed mcp_config.
// Array form so OpenClaw deep-merges the snapshot's content with our
// sibling keys (agents overrides, mcp.servers) rather than letting the
// include replace the whole wrapper.
cfg["$include"] = []any{snapshotPath}
case exists:
cfg["$include"] = []any{activePath}
}
return cfg
@@ -213,6 +313,28 @@ func rewriteAgentsListWorkspaces(list []any, workDir string) []any {
return out
}
// stripUserMcpServers removes only `mcp.servers` from a resolved user
// config, leaving every other key under `mcp` (e.g. `sessionIdleTtlMs`)
// intact. The wrapper's managed `mcp.servers` becomes the sole server
// definition while the user's surrounding MCP tuning still applies — see
// https://docs.openclaw.ai/gateway/configuration-reference#mcp for the
// full list of sibling settings the snapshot should preserve.
//
// If the resulting `mcp` block has no keys left, the parent `mcp` key is
// dropped too so the snapshot doesn't carry an empty placeholder. Any
// non-object value for `mcp` is left as-is; we only know how to strip
// servers from the documented object shape.
func stripUserMcpServers(resolved map[string]any) {
mcp, ok := resolved["mcp"].(map[string]any)
if !ok {
return
}
delete(mcp, "servers")
if len(mcp) == 0 {
delete(resolved, "mcp")
}
}
// openclawActiveConfigPath runs `openclaw config file` to discover the path
// the openclaw CLI considers active. Returns (absolutePath, exists, error).
//
@@ -260,6 +382,35 @@ func openclawActiveConfigPath(bin string, timeout time.Duration) (string, bool,
return path, true, nil
}
// openclawResolvedFullConfig fetches the user's fully resolved openclaw
// config via `openclaw config get --json` (no key path — root). The CLI's
// loader handles JSON5 / $include / env-substitution and emits a flat JSON
// object, which is what we need to write a sanitized snapshot that the
// wrapper can $include without inheriting the user's `mcp.servers`.
//
// Returns (nil, nil) when the CLI prints empty / null output for the root
// — interpreted as "no resolvable user config" by the caller, which then
// falls through to the fresh-install code path. Any other failure
// surfaces as an error so the daemon fails closed instead of silently
// degrading to a leaky non-strict wrapper.
func openclawResolvedFullConfig(bin string, timeout time.Duration) (map[string]any, error) {
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
out, err := openclawExec(ctx, bin, "config", "get", "--json")
if err != nil {
return nil, err
}
trimmed := strings.TrimSpace(out)
if trimmed == "" || trimmed == "null" {
return nil, nil
}
var cfg map[string]any
if err := json.Unmarshal([]byte(trimmed), &cfg); err != nil {
return nil, fmt.Errorf("parse `openclaw config get --json` output: %w", err)
}
return cfg, nil
}
// openclawResolvedAgentsList fetches the user's resolved agents.list via
// `openclaw config get agents.list --json`. The CLI returns the post-
// include, post-env-substitution view of the array, which is exactly the
@@ -315,6 +466,61 @@ func execOpenclawCLI(ctx context.Context, bin string, args ...string) (string, e
return string(raw), nil
}
// openclawManagedMcpServers parses the agent's `mcp_config` JSON and returns
// the map of server name → server config that the wrapper should emit at
// `mcp.servers`. The second return is `true` when the agent has a managed
// mcp_config saved (non-null) — including the explicit empty set
// `{}` / `{"mcpServers":{}}` — and `false` when the field is null/absent so
// the user's global config flows through unmodified.
//
// Input shape mirrors the rest of Multica: Claude-style
// `{"mcpServers": {"<name>": {...}}}`. The server-entry fields pass through
// verbatim. OpenClaw's stdio schema uses the same camelCase keys (`command`,
// `args`, `env`) as Claude; HTTP/SSE entries should set OpenClaw's
// `transport` field directly (e.g. `"transport": "streamable-http"`) rather
// than Claude's `type` since OpenClaw does not recognise the latter.
//
// Each entry must declare either `command` (stdio) or `url` (http/sse); any
// other shape returns an error so the launch fails closed with an actionable
// message rather than running with a server OpenClaw will refuse to start.
func openclawManagedMcpServers(raw json.RawMessage) (map[string]any, bool, error) {
trimmed := bytes.TrimSpace(raw)
if len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) {
return nil, false, nil
}
var parsed struct {
McpServers map[string]json.RawMessage `json:"mcpServers"`
}
if err := json.Unmarshal(trimmed, &parsed); err != nil {
return nil, false, fmt.Errorf("parse mcp_config json: %w", err)
}
if len(parsed.McpServers) == 0 {
return map[string]any{}, true, nil
}
names := make([]string, 0, len(parsed.McpServers))
for name := range parsed.McpServers {
names = append(names, name)
}
sort.Strings(names)
out := make(map[string]any, len(names))
for _, name := range names {
var entry map[string]any
if err := json.Unmarshal(parsed.McpServers[name], &entry); err != nil {
return nil, false, fmt.Errorf("mcp_servers.%s: %w", name, err)
}
if entry == nil {
return nil, false, fmt.Errorf("mcp_servers.%s must be a JSON object", name)
}
command, _ := entry["command"].(string)
url, _ := entry["url"].(string)
if strings.TrimSpace(command) == "" && strings.TrimSpace(url) == "" {
return nil, false, fmt.Errorf("mcp_servers.%s must declare either `command` (stdio) or `url` (http/sse)", name)
}
out[name] = entry
}
return out, true, nil
}
// isOpenclawKeyMissing returns true when the CLI error indicates the asked-
// for path simply isn't set, as opposed to a real failure (bad config,
// CLI bug, missing binary). The CLI's "key not found" exit text has varied

View File

@@ -428,6 +428,427 @@ func TestPrepareOpenclawConfigWrapperLoadableUnderIncludeConfinement(t *testing.
}
}
// TestPrepareOpenclawConfigStrictReplacesUserMcpServers — the headline
// assertion for Elon's strict-replace must-fix on PR #3450. When the user
// has a global `mcp.servers.global_one` AND the agent has a managed
// `mcp.servers.shared + managed_only`, the wrapper must NOT $include the
// live user config (which would leak global_one) and must instead
// $include a sanitized snapshot that has the user's `mcp` block stripped.
// The wrapper itself carries managed servers and nothing else.
func TestPrepareOpenclawConfigStrictReplacesUserMcpServers(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
userCfgPath := filepath.Join(t.TempDir(), "openclaw.json")
if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil {
t.Fatalf("write user cfg: %v", err)
}
// The resolved user config the CLI would return: a user global
// mcp.servers + some other non-mcp content the snapshot must preserve.
resolvedUser := `{
"mcp": {"servers": {
"global_one": {"command": "/bin/echo", "args": ["user"]},
"shared": {"command": "/bin/old-version"}
}},
"gateway": {"port": 18789},
"providers": {"anthropic": {"apiKey": "sk-user-secret"}}
}`
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: userCfgPath},
"config get --json": {stdout: resolvedUser},
"config get agents.list --json": {stdout: "null"},
})
mcpConfig := json.RawMessage(`{
"mcpServers": {
"shared": {"command": "/bin/new-version"},
"managed_only": {"url": "https://mcp.example.com", "transport": "streamable-http"}
}
}`)
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: mcpConfig,
})
if err != nil {
t.Fatalf("prepareOpenclawConfig: %v", err)
}
got := mustReadJSON(t, result.ConfigPath)
mcp, ok := got["mcp"].(map[string]any)
if !ok {
t.Fatalf("wrapper missing mcp block: %v", got)
}
servers, ok := mcp["servers"].(map[string]any)
if !ok {
t.Fatalf("mcp.servers is not an object: %v", mcp)
}
if len(servers) != 2 {
t.Errorf("mcp.servers has %d entries, want 2 (managed only — global_one must not leak): %v", len(servers), servers)
}
if _, leaked := servers["global_one"]; leaked {
t.Errorf("mcp.servers.global_one leaked into wrapper from user config: %v", servers)
}
if shared, ok := servers["shared"].(map[string]any); !ok || shared["command"] != "/bin/new-version" {
t.Errorf("mcp.servers.shared = %v, want managed `command: /bin/new-version` (managed overrides user same-name)", shared)
}
if managed, ok := servers["managed_only"].(map[string]any); !ok || managed["url"] != "https://mcp.example.com" {
t.Errorf("mcp.servers.managed_only missing or wrong shape: %v", managed)
}
// The wrapper's $include must point at the sanitized snapshot, NOT the
// live user config — otherwise OpenClaw would deep-merge user.mcp back in.
include, _ := got["$include"].([]any)
if len(include) != 1 {
t.Fatalf("wrapper $include has %d entries, want 1: %v", len(include), include)
}
snapshotPath, _ := include[0].(string)
if snapshotPath == userCfgPath {
t.Fatalf("wrapper $includes the live user config (%q) — strict replace requires the sanitized snapshot", userCfgPath)
}
wantSnapshot := filepath.Join(envRoot, openclawUserSnapshotFile)
if snapshotPath != wantSnapshot {
t.Errorf("$include = %q, want sanitized snapshot %q", snapshotPath, wantSnapshot)
}
// Snapshot must exist, must drop the `mcp` block, and must preserve the
// non-mcp keys (gateway, providers, secrets) so OpenClaw still has API
// keys and other config the user relied on.
snap := mustReadJSON(t, snapshotPath)
if _, present := snap["mcp"]; present {
t.Errorf("snapshot still contains an `mcp` block — strict replace not enforced: %v", snap["mcp"])
}
if gw, ok := snap["gateway"].(map[string]any); !ok || gw["port"] != float64(18789) {
t.Errorf("snapshot lost gateway.port carryover: %v", snap["gateway"])
}
if _, ok := snap["providers"].(map[string]any); !ok {
t.Errorf("snapshot lost providers carryover: %v", snap)
}
// The snapshot lives in envRoot alongside the wrapper, so the daemon
// does NOT need to grant an OPENCLAW_INCLUDE_ROOTS entry for it.
if result.IncludeRoot != "" {
t.Errorf("IncludeRoot = %q, want empty (snapshot lives in envRoot, no cross-dir include)", result.IncludeRoot)
}
}
// TestPrepareOpenclawConfigStrictPreservesNonServerMcpKeys — Elon's
// follow-up must-fix: the strict-replace path must scope only to
// `mcp.servers`, not the entire `mcp` block. OpenClaw config has
// sibling settings under `mcp` (e.g. `sessionIdleTtlMs` — see
// https://docs.openclaw.ai/gateway/configuration-reference#mcp). The
// previous implementation deleted the whole `mcp` block which silently
// reset those siblings to OpenClaw's defaults. This test fixes that
// scope: managed-MCP path drops `mcp.servers` but leaves
// `mcp.sessionIdleTtlMs` intact in the snapshot.
func TestPrepareOpenclawConfigStrictPreservesNonServerMcpKeys(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
userCfgPath := filepath.Join(t.TempDir(), "openclaw.json")
if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil {
t.Fatalf("write user cfg: %v", err)
}
// User's resolved config has BOTH `mcp.servers` (must be stripped) and
// `mcp.sessionIdleTtlMs` (must survive). The snapshot is what OpenClaw
// loads via the wrapper's $include, so only the snapshot's `mcp` block
// is consulted for non-server settings.
resolvedUser := `{
"mcp": {
"sessionIdleTtlMs": 300000,
"servers": {"global_one": {"command": "/bin/echo"}}
},
"gateway": {"port": 18789}
}`
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: userCfgPath},
"config get --json": {stdout: resolvedUser},
"config get agents.list --json": {stdout: "null"},
})
mcpConfig := json.RawMessage(`{"mcpServers": {"managed_only": {"command": "uvx", "args": ["m"]}}}`)
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: mcpConfig,
})
if err != nil {
t.Fatalf("prepareOpenclawConfig: %v", err)
}
snapPath := filepath.Join(envRoot, openclawUserSnapshotFile)
snap := mustReadJSON(t, snapPath)
snapMcp, ok := snap["mcp"].(map[string]any)
if !ok {
t.Fatalf("snapshot lost the mcp block entirely; mcp.sessionIdleTtlMs should have survived: %v", snap)
}
if _, leaked := snapMcp["servers"]; leaked {
t.Errorf("snapshot still has mcp.servers; strict scope must drop it: %v", snapMcp)
}
// json.Unmarshal decodes JSON numbers as float64.
if ttl, ok := snapMcp["sessionIdleTtlMs"].(float64); !ok || ttl != 300000 {
t.Errorf("snapshot lost mcp.sessionIdleTtlMs (should be preserved): %v", snapMcp)
}
// Wrapper still emits the managed-only server set on top, so the
// effective view post-include is exactly the managed set.
got := mustReadJSON(t, result.ConfigPath)
wrapperMcp, _ := got["mcp"].(map[string]any)
servers, _ := wrapperMcp["servers"].(map[string]any)
if _, ok := servers["managed_only"]; !ok {
t.Errorf("wrapper missing managed_only: %v", servers)
}
if _, leaked := servers["global_one"]; leaked {
t.Errorf("global_one leaked into wrapper: %v", servers)
}
}
// TestPrepareOpenclawConfigStrictEmptyManagedSetDropsUserMcp — empty
// managed set `{}` must drop the user's global mcp.servers too. Without
// strict replace, OpenClaw would still resolve user-only servers via the
// $include and the admin's "saved no servers" intent would be silently
// overridden.
func TestPrepareOpenclawConfigStrictEmptyManagedSetDropsUserMcp(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
userCfgPath := filepath.Join(t.TempDir(), "openclaw.json")
if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil {
t.Fatalf("write user cfg: %v", err)
}
resolvedUser := `{"mcp": {"servers": {"global_one": {"command": "/bin/echo"}}}}`
cases := map[string]json.RawMessage{
"object_empty": json.RawMessage(`{}`),
"mcp_servers_empty_map": json.RawMessage(`{"mcpServers": {}}`),
}
for name, raw := range cases {
t.Run(name, func(t *testing.T) {
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: userCfgPath},
"config get --json": {stdout: resolvedUser},
"config get agents.list --json": {stdout: "null"},
})
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: raw,
})
if err != nil {
t.Fatalf("prepareOpenclawConfig: %v", err)
}
got := mustReadJSON(t, result.ConfigPath)
mcp, ok := got["mcp"].(map[string]any)
if !ok {
t.Fatalf("wrapper missing mcp block (managed empty must still be present): %v", got)
}
servers, ok := mcp["servers"].(map[string]any)
if !ok {
t.Fatalf("mcp.servers is not an object: %v", mcp)
}
if len(servers) != 0 {
t.Errorf("mcp.servers has %d entries on managed-empty, want 0 (global_one must not leak): %v", len(servers), servers)
}
// And the snapshot must have dropped the user's mcp block, so the
// $include resolves with no mcp at all.
snapPath := filepath.Join(envRoot, openclawUserSnapshotFile)
snap := mustReadJSON(t, snapPath)
if _, present := snap["mcp"]; present {
t.Errorf("snapshot still has `mcp` — strict empty must drop the user block: %v", snap["mcp"])
}
})
}
}
// TestPrepareOpenclawConfigNullMcpConfigKeepsUserInclude — when the agent
// has no managed mcp_config (`null` / absent), the wrapper must NOT write
// a sanitized snapshot and must $include the live user config so the
// user's global mcp.servers and other config still flow through. This is
// the "inherit defaults" branch — must remain a no-op vs. the previous
// implementation.
func TestPrepareOpenclawConfigNullMcpConfigKeepsUserInclude(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
userCfgDir := t.TempDir()
userCfgPath := filepath.Join(userCfgDir, "openclaw.json")
if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil {
t.Fatalf("write user cfg: %v", err)
}
cases := map[string]json.RawMessage{
"nil": nil,
"empty": json.RawMessage(""),
"null": json.RawMessage("null"),
}
for name, raw := range cases {
t.Run(name, func(t *testing.T) {
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: userCfgPath},
"config get agents.list --json": {stdout: "null"},
// Note: no `config get --json` stub — the inherit path must
// not call it (would burn an extra CLI roundtrip per task).
})
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: raw,
})
if err != nil {
t.Fatalf("prepareOpenclawConfig: %v", err)
}
got := mustReadJSON(t, result.ConfigPath)
if _, present := got["mcp"]; present {
t.Errorf("wrapper has mcp block when mcp_config = %q: %v", name, got["mcp"])
}
include, _ := got["$include"].([]any)
if len(include) != 1 || include[0] != userCfgPath {
t.Errorf("$include = %v, want live user config %q on inherit path", got["$include"], userCfgPath)
}
if _, err := os.Stat(filepath.Join(envRoot, openclawUserSnapshotFile)); !os.IsNotExist(err) {
t.Errorf("inherit path wrote a snapshot file (should not): err=%v", err)
}
if result.IncludeRoot != userCfgDir {
t.Errorf("IncludeRoot = %q, want %q (cross-dir hop for live $include)", result.IncludeRoot, userCfgDir)
}
})
}
}
// TestPrepareOpenclawConfigManagedSetFreshInstall — managed mcp_config on
// a fresh install (no on-disk user config) must NOT call `config get
// --json` (there is nothing to snapshot) and must write a wrapper that
// carries managed servers as the sole MCP definition with no $include.
func TestPrepareOpenclawConfigManagedSetFreshInstall(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
missingPath := filepath.Join(t.TempDir(), "openclaw.json")
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: missingPath},
// No `config get --json` stub — fresh install must not call it.
})
mcpConfig := json.RawMessage(`{"mcpServers": {"context7": {"command": "uvx", "args": ["context7-mcp"]}}}`)
result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: mcpConfig,
})
if err != nil {
t.Fatalf("prepareOpenclawConfig: %v", err)
}
got := mustReadJSON(t, result.ConfigPath)
mcp, ok := got["mcp"].(map[string]any)
if !ok {
t.Fatalf("wrapper missing mcp block: %v", got)
}
servers, ok := mcp["servers"].(map[string]any)
if !ok {
t.Fatalf("mcp.servers is not an object: %v", mcp)
}
entry, _ := servers["context7"].(map[string]any)
if entry == nil || entry["command"] != "uvx" {
t.Errorf("context7 entry missing/wrong on fresh install: %v", servers)
}
args, _ := entry["args"].([]any)
if len(args) != 1 || args[0] != "context7-mcp" {
t.Errorf("context7.args = %v", args)
}
if _, present := got["$include"]; present {
t.Errorf("fresh install should not emit $include: %v", got["$include"])
}
}
// TestPrepareOpenclawConfigFailsClosedOnResolvedConfigError — when the
// user has a config on disk and the agent has managed mcp_config but
// `openclaw config get --json` errors, the preparer must NOT fall back to
// `$include`ing the live user file (which would leak global mcp.servers).
// Fail closed instead, mirroring the existing fail-closed posture.
func TestPrepareOpenclawConfigFailsClosedOnResolvedConfigError(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
userCfgPath := filepath.Join(t.TempDir(), "openclaw.json")
if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil {
t.Fatalf("write user cfg: %v", err)
}
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: userCfgPath},
"config get agents.list --json": {stdout: "null"},
"config get --json": {err: errors.New("openclaw: schema validation failed")},
})
mcpConfig := json.RawMessage(`{"mcpServers": {"context7": {"command": "uvx"}}}`)
_, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: mcpConfig,
})
if err == nil {
t.Fatal("prepareOpenclawConfig succeeded when `config get --json` errored; expected fail closed")
}
if !strings.Contains(err.Error(), "resolved config") {
t.Errorf("error %q does not name the resolved-config step", err.Error())
}
// No stale wrapper / snapshot left behind.
if _, err := os.Stat(filepath.Join(envRoot, openclawConfigFile)); !os.IsNotExist(err) {
t.Errorf("wrapper exists after fail-closed: %v", err)
}
if _, err := os.Stat(filepath.Join(envRoot, openclawUserSnapshotFile)); !os.IsNotExist(err) {
t.Errorf("snapshot exists after fail-closed: %v", err)
}
}
// TestPrepareOpenclawConfigFailsClosedOnMalformedMcpConfig — keeping with
// the fail-closed posture used for the rest of the preparer: a malformed
// mcp_config must not write any wrapper file, so the daemon surfaces the
// error instead of booting OpenClaw with an empty / inherited MCP set the
// admin didn't expect.
func TestPrepareOpenclawConfigFailsClosedOnMalformedMcpConfig(t *testing.T) {
envRoot := t.TempDir()
workDir := filepath.Join(envRoot, "workdir")
if err := os.MkdirAll(workDir, 0o755); err != nil {
t.Fatalf("mkdir workdir: %v", err)
}
userCfgPath := filepath.Join(t.TempDir(), "openclaw.json")
if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil {
t.Fatalf("write user cfg: %v", err)
}
cases := map[string]json.RawMessage{
"unparseable_json": json.RawMessage(`{not-json}`),
"entry_missing_command": json.RawMessage(`{"mcpServers": {"bad": {}}}`),
"entry_wrong_shape": json.RawMessage(`{"mcpServers": {"bad": "not-an-object"}}`),
}
for name, raw := range cases {
t.Run(name, func(t *testing.T) {
stub := installOpenclawStub(t, map[string]openclawResponse{
"config file": {stdout: userCfgPath},
"config get agents.list --json": {stdout: "null"},
})
_, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{
OpenclawBin: stub.bin,
McpConfig: raw,
})
if err == nil {
t.Fatalf("prepareOpenclawConfig succeeded on %s; expected fail closed", name)
}
if !strings.Contains(err.Error(), "mcp_config") && !strings.Contains(err.Error(), "mcp_servers") {
t.Errorf("error %q does not name the mcp_config step", err.Error())
}
})
}
}
// TestPrepareOpenclawSkillWriteMatchesScanPath is the regression test the
// MUL-2219 DoD calls out: the directory Multica writes skills into MUST be
// the same directory the OpenClaw scanner reads from. We assert this by