Skip to content

Commit f753361

Browse files
authored
fix(agent): preserve existing MCP servers during managed updates (#325)
- adopt existing runtime MCP servers when management is first enabled - expose a single server view with server-side batch add and delete operations - align the MCP UI, API documentation, and tests with the managed state flow
1 parent 8a1c198 commit f753361

15 files changed

Lines changed: 538 additions & 323 deletions

docs/api.md

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -377,27 +377,15 @@ Notes:
377377

378378
#### `GET /api/v1/agents/{id}/mcp-servers`
379379

380-
Returns the agent's editable MCP server configuration. Unlike generic Agent
381-
responses, this configuration-page endpoint returns raw values so a user can
382-
review and edit existing tokens. `desired` and `actual` are each direct
383-
server-name-to-server-config maps when non-null; neither is nested under
384-
another MCP field. `desired` preserves the distinction between `null` (no
385-
managed set) and `{}` (an explicitly managed empty set). If the native runtime
386-
configuration cannot be read, the endpoint still returns `desired`, sets
387-
`actual` to `null`, and includes an optional `actual_error` message.
380+
Returns the agent's MCP server configuration as a direct raw
381+
server-name-to-server-config map in `servers`. Unlike generic Agent responses,
382+
this endpoint does not redact configured token values.
388383

389384
```json
390385
{
391386
"agent_id": "u-alice",
392387
"runtime_kind": "openclaw_sandbox",
393-
"desired": {
394-
"context7": {
395-
"command": "uvx",
396-
"args": ["context7-mcp"],
397-
"env": { "CONTEXT7_API_KEY": "secret" }
398-
}
399-
},
400-
"actual": {
388+
"servers": {
401389
"context7": {
402390
"command": "uvx",
403391
"args": ["context7-mcp"],
@@ -407,17 +395,17 @@ configuration cannot be read, the endpoint still returns `desired`, sets
407395
}
408396
```
409397

410-
#### `PUT /api/v1/agents/{id}/mcp-servers`
411-
412-
Replaces the agent's `mcpServers` direct map. Sending `null` clears the managed
413-
set. The response has the same raw `desired`/`actual` shape as the `GET`
414-
endpoint.
415-
416398
#### `POST /api/v1/agents/{id}/mcp-servers:batchAdd`
417399

418400
Accepts `{ "names": ["..."] }` and merges the named server definitions from
419-
the MCP catalog into the agent's `mcpServers` map. The response has the same raw
420-
`desired`/`actual` shape as the `GET` endpoint.
401+
the MCP catalog into the agent's managed MCP servers. The response has the same
402+
shape as the `GET` endpoint.
403+
404+
#### `POST /api/v1/agents/{id}/mcp-servers:batchDelete`
405+
406+
Accepts `{ "names": ["..."] }` and removes the named servers from the
407+
agent's managed MCP servers. The response has the same shape as the `GET`
408+
endpoint.
421409

422410
### MCP server catalog
423411

docs/api.zh.md

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -367,20 +367,13 @@ OpenClaw、PicoClaw 和 Codex CLI agent 通过顶层 `mcpServers` 字段配置 M
367367

368368
#### `GET /api/v1/agents/{id}/mcp-servers`
369369

370-
返回 Agent 可编辑的 MCP server 配置。与通用 Agent 响应不同,这个配置页面接口会返回原始值,以便用户查看和编辑已配置的 token。非空的 `desired``actual` 都是从 server 名称到 server 配置的直接映射,不再嵌套在其他 MCP 字段中。`desired` 会保留 `null`(未托管集合)与 `{}`(显式托管空集合)的区别;如果无法读取 runtime 原生配置,接口仍会返回 `desired`,将 `actual` 设为 `null`,并在可选 `actual_error` 中给出失败原因
370+
返回 Agent MCP server 配置。`servers` 是从 server 名称到 server 配置的直接原始映射。与通用 Agent 响应不同,这个接口不会脱敏已配置的 token
371371

372372
```json
373373
{
374374
"agent_id": "u-alice",
375375
"runtime_kind": "openclaw_sandbox",
376-
"desired": {
377-
"context7": {
378-
"command": "uvx",
379-
"args": ["context7-mcp"],
380-
"env": { "CONTEXT7_API_KEY": "secret" }
381-
}
382-
},
383-
"actual": {
376+
"servers": {
384377
"context7": {
385378
"command": "uvx",
386379
"args": ["context7-mcp"],
@@ -390,13 +383,13 @@ OpenClaw、PicoClaw 和 Codex CLI agent 通过顶层 `mcpServers` 字段配置 M
390383
}
391384
```
392385

393-
#### `PUT /api/v1/agents/{id}/mcp-servers`
386+
#### `POST /api/v1/agents/{id}/mcp-servers:batchAdd`
394387

395-
替换该 Agent 的 `mcpServers` 直接映射。传入 `null` 会清除托管集合。响应与 `GET` 一样,返回未脱敏的 `desired`/`actual`
388+
接收 `{ "names": ["..."] }`,将 MCP catalog 中同名 server 的定义合并到该 Agent 托管的 MCP server。响应与 `GET` 一样。
396389

397-
#### `POST /api/v1/agents/{id}/mcp-servers:batchAdd`
390+
#### `POST /api/v1/agents/{id}/mcp-servers:batchDelete`
398391

399-
接收 `{ "names": ["..."] }`将 MCP catalog 中同名 server 的定义合并到该 Agent 的 `mcpServers` 映射。响应与 `GET` 一样,返回未脱敏的 `desired`/`actual`
392+
接收 `{ "names": ["..."] }`从该 Agent 托管的 MCP server 中移除同名 server。响应与 `GET` 一样。
400393

401394
### MCP server catalog
402395

docs/openclaw-mcp-restart-plan.zh.md

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -97,23 +97,16 @@ MCP servers 通道。
9797
配置页面使用下面的专用接口:
9898

9999
- `GET /api/v1/agents/{id}/mcp-servers`
100-
- `PUT /api/v1/agents/{id}/mcp-servers`
101100
- `POST /api/v1/agents/{id}/mcp-servers:batchAdd`
101+
- `POST /api/v1/agents/{id}/mcp-servers:batchDelete`
102102

103103
`GET` 返回:
104104

105105
```json
106106
{
107107
"agent_id": "u-alice",
108108
"runtime_kind": "openclaw_sandbox",
109-
"desired": {
110-
"context7": {
111-
"command": "uvx",
112-
"args": ["context7-mcp"],
113-
"env": { "CONTEXT7_API_KEY": "secret" }
114-
}
115-
},
116-
"actual": {
109+
"servers": {
117110
"context7": {
118111
"command": "uvx",
119112
"args": ["context7-mcp"],
@@ -123,16 +116,10 @@ MCP servers 通道。
123116
}
124117
```
125118

126-
非空的 `desired``actual` 都是直接 server 映射,且专用接口不脱敏,供有权限的
127-
配置页面回显和编辑 token。`desired` 是持久化的声明,保留 `null`(未托管)与
128-
`{}`(显式托管空集合)的区别;`actual` 是从 runtime 原生配置读回的结果。若 runtime
129-
展开了 workspace 占位符,二者在路径值上可以不同,删除和保存应以 `desired` 为准。
130-
如果原生配置暂时无法读取,接口仍会返回 `desired`,并将 `actual` 设为 `null`,同时
131-
在可选的 `actual_error` 中说明读取失败原因,避免配置页无法修复期望配置。
132-
133-
`PUT` 接收 Agent 的顶层 `mcpServers` 字段并整体替换集合;响应与 `GET`
134-
相同。`batchAdd` 接收 `{ "names": ["..."] }`,把 MCP catalog 中同名的
135-
server 定义合并到该 Agent 的集合后返回同一视图。
119+
`servers` 是直接 server 映射,且专用接口不脱敏,供有权限的配置页面回显和编辑 token。
120+
`batchAdd` 接收 `{ "names": ["..."] }`,把 MCP catalog 中同名的 server
121+
定义合并到该 Agent 的集合;`batchDelete` 用相同请求形态移除指定 server。两者都返回
122+
同一视图。
136123

137124
### MCP catalog
138125

@@ -191,7 +178,7 @@ env = { "CONTEXT7_API_KEY" = "secret" }
191178

192179
远程 server 的 `headers` 会转写为 Codex 的 `http_headers`。各 adapter 在
193180
写入原生配置前解析 `${workspace}``${workspaceDir}``{workspace}`
194-
`{workspaceDir}` 占位符;因此 `actual` 中可能显示已解析的运行时路径
181+
`{workspaceDir}` 占位符;这个运行时渲染过程不会修改持久化的 `mcpServers`
195182

196183
## 5. 保存、同步与失败处理
197184

@@ -203,18 +190,18 @@ env = { "CONTEXT7_API_KEY" = "secret" }
203190
4. 通过 `MCPServersRestartRequired` 判断是否需要重建 runtime。
204191
5. 无需重建时调用 `ReconcileMCPServers`;需要重建时由既有 lifecycle
205192
流程 provision 新 runtime 配置。
206-
6. 若同步或重建失败,保留已保存的期望状态并暴露既有 restart-required
193+
6. 若同步或重建失败,保留已保存的 MCP server 配置并暴露既有 restart-required
207194
状态,用户可以通过现有重建动作重试。
208195

209-
`MCPServersListController` 在读取实际原生文件时生成 `actual`,用于让配置页
210-
识别“期望状态已保存但 runtime 尚未同步”的情况。
196+
`MCPServersListController` 只在 Agent 尚未进入 CSGClaw 管理时读取 runtime
197+
原生配置,用它建立首次管理所需的 `servers` 集合;进入管理后,`servers` 直接
198+
来自持久化的 `Agent.MCPServers`,不会再额外返回一套 runtime 读取视图。
211199

212200
## 6. 前端行为与验证
213201

214202
前端创建和编辑 Agent 都以 `mcpServers` 作为草稿字段。通用 Agent 查询仅用于
215-
展示脱敏摘要;打开 MCP 编辑器时,前端获取专用原始视图并将 `desired` 合并进
216-
草稿。保存时只提交直接映射,避免 token 被脱敏占位值或 runtime 已解析的路径
217-
覆盖。
203+
展示脱敏摘要;打开 MCP 编辑器时,前端获取专用原始视图的 `servers` 作为草稿。
204+
保存时只提交直接映射,避免 token 被脱敏占位值覆盖。
218205

219206
验证覆盖应至少包括:
220207

@@ -223,5 +210,5 @@ env = { "CONTEXT7_API_KEY" = "secret" }
223210
- 通用 Agent 响应的 `env`/`headers` 脱敏,以及专用视图的原始回显。
224211
- 精确的 Agent 和 catalog 路由,包括 batchAdd 后缀。
225212
- OpenClaw、PicoClaw、Codex 的原生渲染和实际配置读取。
226-
- workspace 占位符的解析不会污染 `desired`
213+
- workspace 占位符的解析不会污染持久化的 `mcpServers`
227214
- MCP 编辑不会覆盖无关的 `runtime_options` 或 profile 字段。

internal/agent/mcp_servers.go

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@ import (
55
"fmt"
66
"strings"
77

8-
agentruntime "csgclaw/internal/runtime"
98
"csgclaw/internal/utils"
109
)
1110

1211
type MCPServersView struct {
1312
AgentID string `json:"agent_id"`
1413
RuntimeKind string `json:"runtime_kind"`
15-
Desired map[string]any `json:"desired"`
16-
Actual map[string]any `json:"actual"`
17-
ActualError string `json:"actual_error,omitempty"`
14+
Servers map[string]any `json:"servers"`
1815
}
1916

2017
// cloneMCPServers preserves the distinction between no managed MCP state and
@@ -44,27 +41,12 @@ func (s *Service) MCPServersView(ctx context.Context, id string) (MCPServersView
4441
view := MCPServersView{
4542
AgentID: got.ID,
4643
RuntimeKind: runtimeKind,
47-
Desired: cloneMCPServers(got.MCPServers),
4844
}
49-
if runtimeKind == "" {
50-
return view, nil
51-
}
52-
53-
rt, err := s.runtimeForKind(runtimeKind)
54-
if err != nil {
55-
view.ActualError = fmt.Sprintf("read runtime MCP servers: %v", err)
56-
return view, nil
57-
}
58-
lister, ok := rt.(agentruntime.MCPServersListController)
59-
if !ok {
60-
view.ActualError = fmt.Sprintf("runtime_kind %q does not expose MCP server state", runtimeKind)
61-
return view, nil
62-
}
63-
listed, err := lister.ListMCPServers(ctx, runtimeHandleForAgent(got), mcpServersSnapshotForAgent(got.MCPServers))
45+
servers, err := s.currentMCPServersForManagement(ctx, got)
6446
if err != nil {
65-
view.ActualError = fmt.Sprintf("read runtime MCP servers: %v", err)
47+
view.Servers = cloneMCPServers(got.MCPServers)
6648
return view, nil
6749
}
68-
view.Actual = cloneMCPServers(listed.Servers)
50+
view.Servers = cloneMCPServers(servers)
6951
return view, nil
7052
}

internal/agent/mcp_servers_view_test.go

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7-
"strings"
87
"testing"
98

109
agentruntime "csgclaw/internal/runtime"
@@ -56,9 +55,6 @@ func TestMCPServersViewPreservesNilAndExplicitEmptyMaps(t *testing.T) {
5655
if err != nil {
5756
t.Fatalf("MCPServersView() error = %v", err)
5857
}
59-
if view.ActualError != "" {
60-
t.Fatalf("ActualError = %q, want empty", view.ActualError)
61-
}
6258
data, err := json.Marshal(view)
6359
if err != nil {
6460
t.Fatalf("json.Marshal() error = %v", err)
@@ -67,17 +63,47 @@ func TestMCPServersViewPreservesNilAndExplicitEmptyMaps(t *testing.T) {
6763
if err := json.Unmarshal(data, &fields); err != nil {
6864
t.Fatalf("json.Unmarshal() error = %v", err)
6965
}
70-
if got := string(fields["desired"]); got != test.want {
71-
t.Fatalf("desired = %s, want %s", got, test.want)
72-
}
73-
if got := string(fields["actual"]); got != test.want {
74-
t.Fatalf("actual = %s, want %s", got, test.want)
66+
if got := string(fields["servers"]); got != test.want {
67+
t.Fatalf("servers = %s, want %s", got, test.want)
7568
}
7669
})
7770
}
7871
}
7972

80-
func TestMCPServersViewKeepsDesiredWhenRuntimeReadFails(t *testing.T) {
73+
func TestMCPServersViewReadsRuntimeServersBeforeFirstManagement(t *testing.T) {
74+
svc := &Service{
75+
agents: map[string]Agent{
76+
"u-mcp": {
77+
ID: "u-mcp",
78+
RuntimeKind: RuntimeKindCodex,
79+
},
80+
},
81+
runtimeRegistry: map[string]agentruntime.Runtime{
82+
RuntimeKindCodex: mcpServersViewTestRuntime{
83+
fakeAgentRuntime: fakeAgentRuntime{kind: RuntimeKindCodex},
84+
list: func(context.Context, agentruntime.Handle, agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) {
85+
return agentruntime.MCPServersSnapshot{Servers: map[string]any{
86+
"manual": map[string]any{"command": "uvx"},
87+
}}, nil
88+
},
89+
},
90+
},
91+
}
92+
93+
view, err := svc.MCPServersView(context.Background(), "u-mcp")
94+
if err != nil {
95+
t.Fatalf("MCPServersView() error = %v", err)
96+
}
97+
server, ok := view.Servers["manual"].(map[string]any)
98+
if !ok || server["command"] != "uvx" {
99+
t.Fatalf("Servers = %#v, want runtime server", view.Servers)
100+
}
101+
if got, ok := svc.Agent("u-mcp"); !ok || got.MCPServers != nil {
102+
t.Fatalf("Agent().MCPServers = %#v, want unmanaged state preserved on read", got.MCPServers)
103+
}
104+
}
105+
106+
func TestMCPServersViewKeepsPersistedServersWhenRuntimeReadFails(t *testing.T) {
81107
readErr := errors.New("native config is unreadable")
82108
desired := map[string]any{
83109
"context7": map[string]any{
@@ -107,15 +133,9 @@ func TestMCPServersViewKeepsDesiredWhenRuntimeReadFails(t *testing.T) {
107133
if err != nil {
108134
t.Fatalf("MCPServersView() error = %v, want desired state to remain readable", err)
109135
}
110-
if view.Actual != nil {
111-
t.Fatalf("Actual = %#v, want nil when the runtime read fails", view.Actual)
112-
}
113-
if !strings.Contains(view.ActualError, readErr.Error()) {
114-
t.Fatalf("ActualError = %q, want %q", view.ActualError, readErr)
115-
}
116-
server, ok := view.Desired["context7"].(map[string]any)
136+
server, ok := view.Servers["context7"].(map[string]any)
117137
if !ok || server["command"] != "uvx" {
118-
t.Fatalf("Desired = %#v, want raw desired server", view.Desired)
138+
t.Fatalf("Servers = %#v, want raw persisted server", view.Servers)
119139
}
120140
data, err := json.Marshal(view)
121141
if err != nil {
@@ -125,7 +145,7 @@ func TestMCPServersViewKeepsDesiredWhenRuntimeReadFails(t *testing.T) {
125145
if err := json.Unmarshal(data, &fields); err != nil {
126146
t.Fatalf("json.Unmarshal() error = %v", err)
127147
}
128-
if got := string(fields["actual"]); got != "null" {
129-
t.Fatalf("actual = %s, want null", got)
148+
if got := string(fields["servers"]); got == "null" {
149+
t.Fatalf("servers = %s, want persisted server map", got)
130150
}
131151
}

0 commit comments

Comments
 (0)