diff --git a/docs/api.md b/docs/api.md index 93f0df1d..66981dce 100644 --- a/docs/api.md +++ b/docs/api.md @@ -377,27 +377,15 @@ Notes: #### `GET /api/v1/agents/{id}/mcp-servers` -Returns the agent's editable MCP server configuration. Unlike generic Agent -responses, this configuration-page endpoint returns raw values so a user can -review and edit existing tokens. `desired` and `actual` are each direct -server-name-to-server-config maps when non-null; neither is nested under -another MCP field. `desired` preserves the distinction between `null` (no -managed set) and `{}` (an explicitly managed empty set). If the native runtime -configuration cannot be read, the endpoint still returns `desired`, sets -`actual` to `null`, and includes an optional `actual_error` message. +Returns the agent's MCP server configuration as a direct raw +server-name-to-server-config map in `servers`. Unlike generic Agent responses, +this endpoint does not redact configured token values. ```json { "agent_id": "u-alice", "runtime_kind": "openclaw_sandbox", - "desired": { - "context7": { - "command": "uvx", - "args": ["context7-mcp"], - "env": { "CONTEXT7_API_KEY": "secret" } - } - }, - "actual": { + "servers": { "context7": { "command": "uvx", "args": ["context7-mcp"], @@ -407,17 +395,17 @@ configuration cannot be read, the endpoint still returns `desired`, sets } ``` -#### `PUT /api/v1/agents/{id}/mcp-servers` - -Replaces the agent's `mcpServers` direct map. Sending `null` clears the managed -set. The response has the same raw `desired`/`actual` shape as the `GET` -endpoint. - #### `POST /api/v1/agents/{id}/mcp-servers:batchAdd` Accepts `{ "names": ["..."] }` and merges the named server definitions from -the MCP catalog into the agent's `mcpServers` map. The response has the same raw -`desired`/`actual` shape as the `GET` endpoint. +the MCP catalog into the agent's managed MCP servers. The response has the same +shape as the `GET` endpoint. + +#### `POST /api/v1/agents/{id}/mcp-servers:batchDelete` + +Accepts `{ "names": ["..."] }` and removes the named servers from the +agent's managed MCP servers. The response has the same shape as the `GET` +endpoint. ### MCP server catalog diff --git a/docs/api.zh.md b/docs/api.zh.md index 70fda48a..d9d5e59e 100644 --- a/docs/api.zh.md +++ b/docs/api.zh.md @@ -367,20 +367,13 @@ OpenClaw、PicoClaw 和 Codex CLI agent 通过顶层 `mcpServers` 字段配置 M #### `GET /api/v1/agents/{id}/mcp-servers` -返回 Agent 可编辑的 MCP server 配置。与通用 Agent 响应不同，这个配置页面接口会返回原始值，以便用户查看和编辑已配置的 token。非空的 `desired` 与 `actual` 都是从 server 名称到 server 配置的直接映射，不再嵌套在其他 MCP 字段中。`desired` 会保留 `null`（未托管集合）与 `{}`（显式托管空集合）的区别；如果无法读取 runtime 原生配置，接口仍会返回 `desired`，将 `actual` 设为 `null`，并在可选 `actual_error` 中给出失败原因。 +返回 Agent 的 MCP server 配置。`servers` 是从 server 名称到 server 配置的直接原始映射。与通用 Agent 响应不同，这个接口不会脱敏已配置的 token。 ```json { "agent_id": "u-alice", "runtime_kind": "openclaw_sandbox", - "desired": { - "context7": { - "command": "uvx", - "args": ["context7-mcp"], - "env": { "CONTEXT7_API_KEY": "secret" } - } - }, - "actual": { + "servers": { "context7": { "command": "uvx", "args": ["context7-mcp"], @@ -390,13 +383,13 @@ OpenClaw、PicoClaw 和 Codex CLI agent 通过顶层 `mcpServers` 字段配置 M } ``` -#### `PUT /api/v1/agents/{id}/mcp-servers` +#### `POST /api/v1/agents/{id}/mcp-servers:batchAdd` -替换该 Agent 的 `mcpServers` 直接映射。传入 `null` 会清除托管集合。响应与 `GET` 一样，返回未脱敏的 `desired`/`actual`。 +接收 `{ "names": ["..."] }`，将 MCP catalog 中同名 server 的定义合并到该 Agent 托管的 MCP server。响应与 `GET` 一样。 -#### `POST /api/v1/agents/{id}/mcp-servers:batchAdd` +#### `POST /api/v1/agents/{id}/mcp-servers:batchDelete` -接收 `{ "names": ["..."] }`，将 MCP catalog 中同名 server 的定义合并到该 Agent 的 `mcpServers` 映射。响应与 `GET` 一样，返回未脱敏的 `desired`/`actual`。 +接收 `{ "names": ["..."] }`，从该 Agent 托管的 MCP server 中移除同名 server。响应与 `GET` 一样。 ### MCP server catalog diff --git a/docs/openclaw-mcp-restart-plan.zh.md b/docs/openclaw-mcp-restart-plan.zh.md index 09da90fa..a7c845db 100644 --- a/docs/openclaw-mcp-restart-plan.zh.md +++ b/docs/openclaw-mcp-restart-plan.zh.md @@ -97,8 +97,8 @@ MCP servers 通道。 配置页面使用下面的专用接口： - `GET /api/v1/agents/{id}/mcp-servers` -- `PUT /api/v1/agents/{id}/mcp-servers` - `POST /api/v1/agents/{id}/mcp-servers:batchAdd` +- `POST /api/v1/agents/{id}/mcp-servers:batchDelete` `GET` 返回： @@ -106,14 +106,7 @@ MCP servers 通道。 { "agent_id": "u-alice", "runtime_kind": "openclaw_sandbox", - "desired": { - "context7": { - "command": "uvx", - "args": ["context7-mcp"], - "env": { "CONTEXT7_API_KEY": "secret" } - } - }, - "actual": { + "servers": { "context7": { "command": "uvx", "args": ["context7-mcp"], @@ -123,16 +116,10 @@ MCP servers 通道。 } ``` -非空的 `desired` 和 `actual` 都是直接 server 映射，且专用接口不脱敏，供有权限的 -配置页面回显和编辑 token。`desired` 是持久化的声明，保留 `null`（未托管）与 -`{}`（显式托管空集合）的区别；`actual` 是从 runtime 原生配置读回的结果。若 runtime -展开了 workspace 占位符，二者在路径值上可以不同，删除和保存应以 `desired` 为准。 -如果原生配置暂时无法读取，接口仍会返回 `desired`，并将 `actual` 设为 `null`，同时 -在可选的 `actual_error` 中说明读取失败原因，避免配置页无法修复期望配置。 - -`PUT` 接收 Agent 的顶层 `mcpServers` 字段并整体替换集合；响应与 `GET` -相同。`batchAdd` 接收 `{ "names": ["..."] }`，把 MCP catalog 中同名的 -server 定义合并到该 Agent 的集合后返回同一视图。 +`servers` 是直接 server 映射，且专用接口不脱敏，供有权限的配置页面回显和编辑 token。 +`batchAdd` 接收 `{ "names": ["..."] }`，把 MCP catalog 中同名的 server +定义合并到该 Agent 的集合；`batchDelete` 用相同请求形态移除指定 server。两者都返回 +同一视图。 ### MCP catalog @@ -191,7 +178,7 @@ env = { "CONTEXT7_API_KEY" = "secret" } 远程 server 的 `headers` 会转写为 Codex 的 `http_headers`。各 adapter 在 写入原生配置前解析 `${workspace}`、`${workspaceDir}`、`{workspace}` 与 -`{workspaceDir}` 占位符；因此 `actual` 中可能显示已解析的运行时路径。 +`{workspaceDir}` 占位符；这个运行时渲染过程不会修改持久化的 `mcpServers`。 ## 5. 保存、同步与失败处理 @@ -203,18 +190,18 @@ env = { "CONTEXT7_API_KEY" = "secret" } 4. 通过 `MCPServersRestartRequired` 判断是否需要重建 runtime。 5. 无需重建时调用 `ReconcileMCPServers`；需要重建时由既有 lifecycle 流程 provision 新 runtime 配置。 -6. 若同步或重建失败，保留已保存的期望状态并暴露既有 restart-required +6. 若同步或重建失败，保留已保存的 MCP server 配置并暴露既有 restart-required 状态，用户可以通过现有重建动作重试。 -`MCPServersListController` 在读取实际原生文件时生成 `actual`，用于让配置页 -识别“期望状态已保存但 runtime 尚未同步”的情况。 +`MCPServersListController` 只在 Agent 尚未进入 CSGClaw 管理时读取 runtime +原生配置，用它建立首次管理所需的 `servers` 集合；进入管理后，`servers` 直接 +来自持久化的 `Agent.MCPServers`，不会再额外返回一套 runtime 读取视图。 ## 6. 前端行为与验证 前端创建和编辑 Agent 都以 `mcpServers` 作为草稿字段。通用 Agent 查询仅用于 -展示脱敏摘要；打开 MCP 编辑器时，前端获取专用原始视图并将 `desired` 合并进 -草稿。保存时只提交直接映射，避免 token 被脱敏占位值或 runtime 已解析的路径 -覆盖。 +展示脱敏摘要；打开 MCP 编辑器时，前端获取专用原始视图的 `servers` 作为草稿。 +保存时只提交直接映射，避免 token 被脱敏占位值覆盖。 验证覆盖应至少包括： @@ -223,5 +210,5 @@ env = { "CONTEXT7_API_KEY" = "secret" } - 通用 Agent 响应的 `env`/`headers` 脱敏，以及专用视图的原始回显。 - 精确的 Agent 和 catalog 路由，包括 batchAdd 后缀。 - OpenClaw、PicoClaw、Codex 的原生渲染和实际配置读取。 -- workspace 占位符的解析不会污染 `desired`。 +- workspace 占位符的解析不会污染持久化的 `mcpServers`。 - MCP 编辑不会覆盖无关的 `runtime_options` 或 profile 字段。 diff --git a/internal/agent/mcp_servers.go b/internal/agent/mcp_servers.go index c3837180..096e651f 100644 --- a/internal/agent/mcp_servers.go +++ b/internal/agent/mcp_servers.go @@ -5,16 +5,13 @@ import ( "fmt" "strings" - agentruntime "csgclaw/internal/runtime" "csgclaw/internal/utils" ) type MCPServersView struct { AgentID string `json:"agent_id"` RuntimeKind string `json:"runtime_kind"` - Desired map[string]any `json:"desired"` - Actual map[string]any `json:"actual"` - ActualError string `json:"actual_error,omitempty"` + Servers map[string]any `json:"servers"` } // cloneMCPServers preserves the distinction between no managed MCP state and @@ -44,27 +41,12 @@ func (s *Service) MCPServersView(ctx context.Context, id string) (MCPServersView view := MCPServersView{ AgentID: got.ID, RuntimeKind: runtimeKind, - Desired: cloneMCPServers(got.MCPServers), } - if runtimeKind == "" { - return view, nil - } - - rt, err := s.runtimeForKind(runtimeKind) - if err != nil { - view.ActualError = fmt.Sprintf("read runtime MCP servers: %v", err) - return view, nil - } - lister, ok := rt.(agentruntime.MCPServersListController) - if !ok { - view.ActualError = fmt.Sprintf("runtime_kind %q does not expose MCP server state", runtimeKind) - return view, nil - } - listed, err := lister.ListMCPServers(ctx, runtimeHandleForAgent(got), mcpServersSnapshotForAgent(got.MCPServers)) + servers, err := s.currentMCPServersForManagement(ctx, got) if err != nil { - view.ActualError = fmt.Sprintf("read runtime MCP servers: %v", err) + view.Servers = cloneMCPServers(got.MCPServers) return view, nil } - view.Actual = cloneMCPServers(listed.Servers) + view.Servers = cloneMCPServers(servers) return view, nil } diff --git a/internal/agent/mcp_servers_view_test.go b/internal/agent/mcp_servers_view_test.go index 59c16bbd..2b2ad22b 100644 --- a/internal/agent/mcp_servers_view_test.go +++ b/internal/agent/mcp_servers_view_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "strings" "testing" agentruntime "csgclaw/internal/runtime" @@ -56,9 +55,6 @@ func TestMCPServersViewPreservesNilAndExplicitEmptyMaps(t *testing.T) { if err != nil { t.Fatalf("MCPServersView() error = %v", err) } - if view.ActualError != "" { - t.Fatalf("ActualError = %q, want empty", view.ActualError) - } data, err := json.Marshal(view) if err != nil { t.Fatalf("json.Marshal() error = %v", err) @@ -67,17 +63,47 @@ func TestMCPServersViewPreservesNilAndExplicitEmptyMaps(t *testing.T) { if err := json.Unmarshal(data, &fields); err != nil { t.Fatalf("json.Unmarshal() error = %v", err) } - if got := string(fields["desired"]); got != test.want { - t.Fatalf("desired = %s, want %s", got, test.want) - } - if got := string(fields["actual"]); got != test.want { - t.Fatalf("actual = %s, want %s", got, test.want) + if got := string(fields["servers"]); got != test.want { + t.Fatalf("servers = %s, want %s", got, test.want) } }) } } -func TestMCPServersViewKeepsDesiredWhenRuntimeReadFails(t *testing.T) { +func TestMCPServersViewReadsRuntimeServersBeforeFirstManagement(t *testing.T) { + svc := &Service{ + agents: map[string]Agent{ + "u-mcp": { + ID: "u-mcp", + RuntimeKind: RuntimeKindCodex, + }, + }, + runtimeRegistry: map[string]agentruntime.Runtime{ + RuntimeKindCodex: mcpServersViewTestRuntime{ + fakeAgentRuntime: fakeAgentRuntime{kind: RuntimeKindCodex}, + list: func(context.Context, agentruntime.Handle, agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) { + return agentruntime.MCPServersSnapshot{Servers: map[string]any{ + "manual": map[string]any{"command": "uvx"}, + }}, nil + }, + }, + }, + } + + view, err := svc.MCPServersView(context.Background(), "u-mcp") + if err != nil { + t.Fatalf("MCPServersView() error = %v", err) + } + server, ok := view.Servers["manual"].(map[string]any) + if !ok || server["command"] != "uvx" { + t.Fatalf("Servers = %#v, want runtime server", view.Servers) + } + if got, ok := svc.Agent("u-mcp"); !ok || got.MCPServers != nil { + t.Fatalf("Agent().MCPServers = %#v, want unmanaged state preserved on read", got.MCPServers) + } +} + +func TestMCPServersViewKeepsPersistedServersWhenRuntimeReadFails(t *testing.T) { readErr := errors.New("native config is unreadable") desired := map[string]any{ "context7": map[string]any{ @@ -107,15 +133,9 @@ func TestMCPServersViewKeepsDesiredWhenRuntimeReadFails(t *testing.T) { if err != nil { t.Fatalf("MCPServersView() error = %v, want desired state to remain readable", err) } - if view.Actual != nil { - t.Fatalf("Actual = %#v, want nil when the runtime read fails", view.Actual) - } - if !strings.Contains(view.ActualError, readErr.Error()) { - t.Fatalf("ActualError = %q, want %q", view.ActualError, readErr) - } - server, ok := view.Desired["context7"].(map[string]any) + server, ok := view.Servers["context7"].(map[string]any) if !ok || server["command"] != "uvx" { - t.Fatalf("Desired = %#v, want raw desired server", view.Desired) + t.Fatalf("Servers = %#v, want raw persisted server", view.Servers) } data, err := json.Marshal(view) if err != nil { @@ -125,7 +145,7 @@ func TestMCPServersViewKeepsDesiredWhenRuntimeReadFails(t *testing.T) { if err := json.Unmarshal(data, &fields); err != nil { t.Fatalf("json.Unmarshal() error = %v", err) } - if got := string(fields["actual"]); got != "null" { - t.Fatalf("actual = %s, want null", got) + if got := string(fields["servers"]); got == "null" { + t.Fatalf("servers = %s, want persisted server map", got) } } diff --git a/internal/agent/service_profiles.go b/internal/agent/service_profiles.go index 082adc4b..a5b5e218 100644 --- a/internal/agent/service_profiles.go +++ b/internal/agent/service_profiles.go @@ -546,7 +546,7 @@ func (s *Service) AddMCPServersFromHub(ctx context.Context, id string, names []s if !ok { return Agent{}, fmt.Errorf("agent %q not found", id) } - currentServers, err := agentruntime.NormalizeMCPServers(current.MCPServers) + currentServers, err := s.currentMCPServersForManagement(ctx, current) if err != nil { return Agent{}, err } @@ -564,6 +564,80 @@ func (s *Service) AddMCPServersFromHub(ctx context.Context, id string, names []s }) } +func (s *Service) DeleteMCPServers(ctx context.Context, id string, names []string) (Agent, error) { + id = strings.TrimSpace(id) + if id == "" { + return Agent{}, fmt.Errorf("agent id is required") + } + if s == nil { + return Agent{}, fmt.Errorf("agent service is required") + } + names = normalizeMCPServerNames(names) + if len(names) == 0 { + return Agent{}, fmt.Errorf("mcp server names are required") + } + + s.mcpServersMu.Lock() + defer s.mcpServersMu.Unlock() + + s.mu.RLock() + current, _, ok := s.agentByIDLocked(id) + s.mu.RUnlock() + if !ok { + return Agent{}, fmt.Errorf("agent %q not found", id) + } + servers, err := s.currentMCPServersForManagement(ctx, current) + if err != nil { + return Agent{}, err + } + for _, name := range names { + if _, ok := servers[name]; !ok { + return Agent{}, fmt.Errorf("mcp server %q not found", name) + } + } + for _, name := range names { + delete(servers, name) + } + return s.update(ctx, id, UpdateRequest{ + MCPServers: &servers, + MCPServersSet: true, + FieldMask: []string{"mcpServers"}, + }) +} + +// currentMCPServersForManagement uses persisted MCPServers once an agent has +// entered CSGClaw management. For an unmanaged agent, it reads the runtime +// configuration once so the first management action can adopt every server. +func (s *Service) currentMCPServersForManagement(ctx context.Context, current Agent) (map[string]any, error) { + servers, err := agentruntime.NormalizeMCPServers(current.MCPServers) + if err != nil || servers != nil { + return servers, err + } + + runtimeKind := strings.TrimSpace(current.RuntimeKind) + if runtimeKind == "" { + return nil, nil + } + rt, err := s.runtimeForKind(runtimeKind) + if err != nil { + return nil, err + } + lister, ok := rt.(agentruntime.MCPServersListController) + if !ok { + return nil, nil + } + + listed, err := lister.ListMCPServers(ctx, runtimeHandleForAgent(current), agentruntime.MCPServersSnapshot{}) + if err != nil { + return nil, fmt.Errorf("read runtime mcpServers for agent %q: %w", current.ID, err) + } + servers, err = agentruntime.NormalizeMCPServers(listed.Servers) + if err != nil { + return nil, fmt.Errorf("normalize runtime mcpServers for agent %q: %w", current.ID, err) + } + return servers, nil +} + func normalizeMCPServerNames(values []string) []string { names := make([]string, 0, len(values)) seen := make(map[string]struct{}, len(values)) diff --git a/internal/agent/service_test.go b/internal/agent/service_test.go index 27436e66..003a8afc 100644 --- a/internal/agent/service_test.go +++ b/internal/agent/service_test.go @@ -150,6 +150,18 @@ type fakeAgentRuntime struct { streamLogs func(context.Context, agentruntime.Handle, agentruntime.LogOptions) error } +type fakeMCPServersListRuntime struct { + fakeAgentRuntime + list func(context.Context, agentruntime.Handle, agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) +} + +func (f fakeMCPServersListRuntime) ListMCPServers(ctx context.Context, h agentruntime.Handle, current agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) { + if f.list == nil { + return agentruntime.MCPServersSnapshot{}, nil + } + return f.list(ctx, h, current) +} + type fakeBareAgentRuntime struct { kind string } @@ -1648,6 +1660,151 @@ func TestUpdateCodexLocalWorkspaceDirMarksRunningRuntimeForRestart(t *testing.T) } } +func TestAddMCPServersFromHubImportsRuntimeServersOnce(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + listCalls := 0 + svc, err := NewService( + testModelConfig(), + config.ServerConfig{}, + "manager-image:test", + "", + WithRuntime(fakeMCPServersListRuntime{ + fakeAgentRuntime: fakeAgentRuntime{kind: RuntimeKindCodex}, + list: func(context.Context, agentruntime.Handle, agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) { + listCalls++ + return agentruntime.MCPServersSnapshot{Servers: map[string]any{ + "manual": map[string]any{"command": "manual-mcp"}, + }}, nil + }, + }), + ) + if err != nil { + t.Fatalf("NewService() error = %v", err) + } + svc.agents["u-dev"] = Agent{ + ID: "u-dev", + Name: "dev", + RuntimeID: "rt-u-dev", + RuntimeKind: RuntimeKindCodex, + Role: RoleWorker, + Status: string(agentruntime.StateStopped), + CreatedAt: time.Date(2026, 5, 18, 9, 0, 0, 0, time.UTC), + } + catalogServers := map[string]any{ + "context7": map[string]any{"command": "uvx", "args": []any{"context7-mcp"}}, + "remote": map[string]any{"url": "https://mcp.example.com/mcp"}, + } + + updated, err := svc.AddMCPServersFromHub(context.Background(), "u-dev", []string{"context7"}, catalogServers) + if err != nil { + t.Fatalf("first AddMCPServersFromHub() error = %v", err) + } + for _, name := range []string{"manual", "context7"} { + assertMCPServersHasServer(t, updated.MCPServers, name) + } + if listCalls != 1 { + t.Fatalf("ListMCPServers() calls = %d, want 1 for initial runtime import", listCalls) + } + + updated, err = svc.AddMCPServersFromHub(context.Background(), "u-dev", []string{"remote"}, catalogServers) + if err != nil { + t.Fatalf("second AddMCPServersFromHub() error = %v", err) + } + for _, name := range []string{"manual", "context7", "remote"} { + assertMCPServersHasServer(t, updated.MCPServers, name) + } + if listCalls != 1 { + t.Fatalf("ListMCPServers() calls = %d, want no second runtime import", listCalls) + } +} + +func TestDeleteMCPServersImportsRuntimeServersBeforeDeleting(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + listCalls := 0 + svc, err := NewService( + testModelConfig(), + config.ServerConfig{}, + "manager-image:test", + "", + WithRuntime(fakeMCPServersListRuntime{ + fakeAgentRuntime: fakeAgentRuntime{kind: RuntimeKindCodex}, + list: func(context.Context, agentruntime.Handle, agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) { + listCalls++ + return agentruntime.MCPServersSnapshot{Servers: map[string]any{ + "manual": map[string]any{"command": "manual-mcp"}, + "native": map[string]any{"command": "native-mcp"}, + }}, nil + }, + }), + ) + if err != nil { + t.Fatalf("NewService() error = %v", err) + } + svc.agents["u-dev"] = Agent{ + ID: "u-dev", + Name: "dev", + RuntimeID: "rt-u-dev", + RuntimeKind: RuntimeKindCodex, + Role: RoleWorker, + Status: string(agentruntime.StateStopped), + CreatedAt: time.Date(2026, 5, 18, 9, 0, 0, 0, time.UTC), + } + + updated, err := svc.DeleteMCPServers(context.Background(), "u-dev", []string{"manual"}) + if err != nil { + t.Fatalf("DeleteMCPServers() error = %v", err) + } + if _, exists := updated.MCPServers["manual"]; exists { + t.Fatalf("DeleteMCPServers() retained deleted server: %#v", updated.MCPServers) + } + assertMCPServersHasServer(t, updated.MCPServers, "native") + if listCalls != 1 { + t.Fatalf("ListMCPServers() calls = %d, want 1 for initial runtime import", listCalls) + } +} + +func TestAddMCPServersFromHubFailsWhenRuntimeMCPReadFails(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + readErr := errors.New("runtime config is unreadable") + svc, err := NewService( + testModelConfig(), + config.ServerConfig{}, + "manager-image:test", + "", + WithRuntime(fakeMCPServersListRuntime{ + fakeAgentRuntime: fakeAgentRuntime{kind: RuntimeKindCodex}, + list: func(context.Context, agentruntime.Handle, agentruntime.MCPServersSnapshot) (agentruntime.MCPServersSnapshot, error) { + return agentruntime.MCPServersSnapshot{}, readErr + }, + }), + ) + if err != nil { + t.Fatalf("NewService() error = %v", err) + } + svc.agents["u-dev"] = Agent{ + ID: "u-dev", + Name: "dev", + RuntimeID: "rt-u-dev", + RuntimeKind: RuntimeKindCodex, + Role: RoleWorker, + Status: string(agentruntime.StateStopped), + CreatedAt: time.Date(2026, 5, 18, 9, 0, 0, 0, time.UTC), + } + + _, err = svc.AddMCPServersFromHub(context.Background(), "u-dev", []string{"context7"}, map[string]any{ + "context7": map[string]any{"command": "uvx"}, + }) + if !errors.Is(err, readErr) { + t.Fatalf("AddMCPServersFromHub() error = %v, want runtime read failure", err) + } + if got, ok := svc.Agent("u-dev"); !ok || got.MCPServers != nil { + t.Fatalf("Agent().MCPServers = %#v, want no persisted MCP server changes", got.MCPServers) + } +} + func TestAddMCPServersSerializesConcurrentNameMerges(t *testing.T) { t.Setenv("HOME", t.TempDir()) diff --git a/internal/api/agent_mcp_servers.go b/internal/api/agent_mcp_servers.go index 6e671d71..d600e810 100644 --- a/internal/api/agent_mcp_servers.go +++ b/internal/api/agent_mcp_servers.go @@ -5,16 +5,36 @@ import ( "fmt" "net/http" "strings" - - "csgclaw/internal/agent" ) type batchAddAgentMCPServersRequest struct { Names []string `json:"names"` } +type batchDeleteAgentMCPServersRequest struct { + Names []string `json:"names"` +} + func (h *Handler) handleAgentMCPServersByID(w http.ResponseWriter, r *http.Request) { - h.handleAgentMCPServers(w, r, pathValue(r, "id")) + if h.svc == nil { + http.Error(w, "agent service is not configured", http.StatusServiceUnavailable) + return + } + id := strings.TrimSpace(pathValue(r, "id")) + if id == "" { + http.NotFound(w, r) + return + } + view, err := h.svc.MCPServersView(r.Context(), id) + if err != nil { + status := http.StatusBadRequest + if strings.Contains(strings.ToLower(err.Error()), "not found") { + status = http.StatusNotFound + } + http.Error(w, err.Error(), status) + return + } + writeJSON(w, http.StatusOK, view) } func (h *Handler) handleBatchAddAgentMCPServers(w http.ResponseWriter, r *http.Request) { @@ -70,61 +90,41 @@ func (h *Handler) handleBatchAddAgentMCPServers(w http.ResponseWriter, r *http.R writeJSON(w, http.StatusOK, view) } -func (h *Handler) handleAgentMCPServers(w http.ResponseWriter, r *http.Request, id string) { +func (h *Handler) handleBatchDeleteAgentMCPServers(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } if h.svc == nil { http.Error(w, "agent service is not configured", http.StatusServiceUnavailable) return } - id = strings.TrimSpace(id) + id := strings.TrimSpace(pathValue(r, "id")) if id == "" { http.NotFound(w, r) return } - switch r.Method { - case http.MethodGet: - view, err := h.svc.MCPServersView(r.Context(), id) - if err != nil { - status := http.StatusBadRequest - if strings.Contains(strings.ToLower(err.Error()), "not found") { - status = http.StatusNotFound - } - http.Error(w, err.Error(), status) - return - } - writeJSON(w, http.StatusOK, view) - case http.MethodPut: - cfg, set, err := decodeAgentMCPServersRequest(r) - if err != nil { - http.Error(w, fmt.Sprintf("decode request: %v", err), http.StatusBadRequest) - return - } - if !set { - http.Error(w, "mcpServers is required", http.StatusBadRequest) - return - } - updated, err := h.svc.Update(r.Context(), id, agent.UpdateRequest{ - MCPServers: cfg, - MCPServersSet: true, - FieldMask: []string{"mcpServers"}, - }) - if err != nil { - status := http.StatusBadRequest - if strings.Contains(strings.ToLower(err.Error()), "not found") { - status = http.StatusNotFound - } - http.Error(w, err.Error(), status) - return - } - h.publishUpdatedAgentUser(updated) - view, err := h.svc.MCPServersView(r.Context(), updated.ID) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - writeJSON(w, http.StatusOK, view) - default: - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + var req batchDeleteAgentMCPServersRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + http.Error(w, fmt.Sprintf("decode request: %v", err), http.StatusBadRequest) + return + } + if !hasMCPServerName(req.Names) { + http.Error(w, "names is required", http.StatusBadRequest) + return + } + updated, err := h.svc.DeleteMCPServers(r.Context(), id, req.Names) + if err != nil { + writeAgentMCPServersMutationError(w, err) + return } + h.publishUpdatedAgentUser(updated) + view, err := h.svc.MCPServersView(r.Context(), updated.ID) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + writeJSON(w, http.StatusOK, view) } func writeAgentMCPServersMutationError(w http.ResponseWriter, err error) { @@ -138,21 +138,11 @@ func writeAgentMCPServersMutationError(w http.ResponseWriter, err error) { http.Error(w, err.Error(), status) } -func decodeAgentMCPServersRequest(r *http.Request) (*map[string]any, bool, error) { - var raw map[string]json.RawMessage - if err := json.NewDecoder(r.Body).Decode(&raw); err != nil { - return nil, false, err - } - payload, ok := raw["mcpServers"] - if !ok { - return nil, false, nil - } - if string(payload) == "null" { - return nil, true, nil - } - var cfg map[string]any - if err := json.Unmarshal(payload, &cfg); err != nil { - return nil, false, err +func hasMCPServerName(names []string) bool { + for _, name := range names { + if strings.TrimSpace(name) != "" { + return true + } } - return &cfg, true, nil + return false } diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go index 91d52786..636aeb65 100644 --- a/internal/api/handler_test.go +++ b/internal/api/handler_test.go @@ -2374,21 +2374,14 @@ func TestAgentMCPServersDedicatedEndpointsUseDirectRawMaps(t *testing.T) { }, } - putBody, err := json.Marshal(map[string]any{"mcpServers": desired}) + updated, err := svc.Update(context.Background(), created.ID, agent.UpdateRequest{ + MCPServers: &desired, + MCPServersSet: true, + FieldMask: []string{"mcpServers"}, + }) if err != nil { - t.Fatalf("marshal MCP servers request: %v", err) - } - put := httptest.NewRequest(http.MethodPut, "/api/v1/agents/"+created.ID+"/mcp-servers", bytes.NewReader(putBody)) - putRec := httptest.NewRecorder() - srv.Routes().ServeHTTP(putRec, put) - if putRec.Code != http.StatusOK { - t.Fatalf("PUT status = %d, want %d; body=%s", putRec.Code, http.StatusOK, putRec.Body.String()) - } - var updated agent.MCPServersView - if err := json.NewDecoder(putRec.Body).Decode(&updated); err != nil { - t.Fatalf("decode PUT response: %v", err) + t.Fatalf("set MCP servers: %v", err) } - assertDedicatedMCPServersView(t, updated, "secret-env") get := httptest.NewRequest(http.MethodGet, "/api/v1/agents/"+created.ID+"/mcp-servers", nil) getRec := httptest.NewRecorder() @@ -2402,7 +2395,7 @@ func TestAgentMCPServersDedicatedEndpointsUseDirectRawMaps(t *testing.T) { } assertDedicatedMCPServersView(t, fetched, "secret-env") - saved, ok := svc.Agent(created.ID) + saved, ok := svc.Agent(updated.ID) if !ok { t.Fatalf("Agent(%q) not found", created.ID) } @@ -2427,23 +2420,20 @@ func TestAgentMCPServersDedicatedEndpointsUseDirectRawMaps(t *testing.T) { func assertDedicatedMCPServersView(t *testing.T, view agent.MCPServersView, wantSecret string) { t.Helper() - if _, wrapped := view.Desired["mcpServers"]; wrapped { - t.Fatalf("desired = %#v, want direct MCP server map", view.Desired) + if _, wrapped := view.Servers["mcpServers"]; wrapped { + t.Fatalf("servers = %#v, want direct MCP server map", view.Servers) } - context7 := mcpServerForTest(t, mcpServersForTest(t, view.Desired), "context7") + context7 := mcpServerForTest(t, mcpServersForTest(t, view.Servers), "context7") env, ok := context7["env"].(map[string]any) if !ok { - t.Fatalf("desired context7.env = %#v, want object", context7["env"]) + t.Fatalf("servers context7.env = %#v, want object", context7["env"]) } if got := env["CONTEXT7_API_KEY"]; got != wantSecret { - t.Fatalf("desired context7.env[CONTEXT7_API_KEY] = %#v, want raw secret %q", got, wantSecret) - } - if _, wrapped := view.Actual["mcpServers"]; wrapped { - t.Fatalf("actual = %#v, want direct MCP server map", view.Actual) + t.Fatalf("servers context7.env[CONTEXT7_API_KEY] = %#v, want raw secret %q", got, wantSecret) } } -func TestGetAgentMCPServersReturnsDesiredWhenRuntimeActualIsUnreadable(t *testing.T) { +func TestGetAgentMCPServersReturnsPersistedServersWhenRuntimeIsUnreadable(t *testing.T) { readErr := errors.New("native MCP config is unreadable") desired := map[string]any{ "context7": map[string]any{ @@ -2477,53 +2467,15 @@ func TestGetAgentMCPServersReturnsDesiredWhenRuntimeActualIsUnreadable(t *testin t.Fatalf("GET status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) } var response struct { - Desired map[string]any `json:"desired"` - Actual json.RawMessage `json:"actual"` - ActualError string `json:"actual_error"` + Servers map[string]any `json:"servers"` } if err := json.NewDecoder(rec.Body).Decode(&response); err != nil { t.Fatalf("decode response: %v", err) } - server := mcpServerForTest(t, response.Desired, "context7") + server := mcpServerForTest(t, response.Servers, "context7") env, ok := server["env"].(map[string]any) if !ok || env["CONTEXT7_API_KEY"] != "secret" { - t.Fatalf("desired = %#v, want raw persisted secret", response.Desired) - } - if got := string(response.Actual); got != "null" { - t.Fatalf("actual = %s, want null", got) - } - if !strings.Contains(response.ActualError, readErr.Error()) { - t.Fatalf("actual_error = %q, want %q", response.ActualError, readErr) - } -} - -func TestPutAgentMCPServersPreservesNullAndExplicitEmptyDesired(t *testing.T) { - srv, _, created := newAgentMCPManagementTestServer(t) - tests := []struct { - name string - body string - want string - }{ - {name: "clear unmanaged set", body: `{"mcpServers":null}`, want: "null"}, - {name: "explicitly manage empty set", body: `{"mcpServers":{}}`, want: "{}"}, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - req := httptest.NewRequest(http.MethodPut, "/api/v1/agents/"+created.ID+"/mcp-servers", strings.NewReader(test.body)) - rec := httptest.NewRecorder() - srv.Routes().ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("PUT status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) - } - var fields map[string]json.RawMessage - if err := json.NewDecoder(rec.Body).Decode(&fields); err != nil { - t.Fatalf("decode response: %v", err) - } - if got := string(fields["desired"]); got != test.want { - t.Fatalf("desired = %s, want %s", got, test.want) - } - }) + t.Fatalf("servers = %#v, want raw persisted secret", response.Servers) } } @@ -3505,7 +3457,7 @@ func TestHandleBatchAddAgentMCPServersAddsCatalogServersByName(t *testing.T) { if err := json.NewDecoder(rec.Body).Decode(&view); err != nil { t.Fatalf("decode response: %v", err) } - servers := mcpServersForTest(t, view.Desired) + servers := mcpServersForTest(t, view.Servers) context7 := mcpServerForTest(t, servers, "context7") if command, _ := context7["command"].(string); command != "uvx" { t.Fatalf("context7.command = %q, want uvx", command) @@ -3540,6 +3492,45 @@ func TestHandleBatchAddAgentMCPServersReturnsNotFoundWhenCatalogServerMissing(t } } +func TestHandleBatchDeleteAgentMCPServersUsesBackendManagedState(t *testing.T) { + srv, svc, created := newAgentMCPManagementTestServer(t) + initial := map[string]any{ + "context7": map[string]any{"command": "uvx", "args": []any{"context7-mcp"}}, + "native": map[string]any{"command": "npx", "args": []any{"native-mcp"}}, + } + if _, err := svc.Update(context.Background(), created.ID, agent.UpdateRequest{ + MCPServers: &initial, + MCPServersSet: true, + FieldMask: []string{"mcpServers"}, + }); err != nil { + t.Fatalf("Update() error = %v", err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/v1/agents/"+created.ID+"/mcp-servers:batchDelete", strings.NewReader(`{"names":["context7"]}`)) + rec := httptest.NewRecorder() + srv.Routes().ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + var view agent.MCPServersView + if err := json.NewDecoder(rec.Body).Decode(&view); err != nil { + t.Fatalf("decode response: %v", err) + } + if _, exists := view.Servers["context7"]; exists { + t.Fatalf("servers = %#v, retained deleted MCP server", view.Servers) + } + mcpServerForTest(t, view.Servers, "native") + + saved, ok := svc.Agent(created.ID) + if !ok { + t.Fatalf("Agent(%q) not found", created.ID) + } + if _, exists := saved.MCPServers["context7"]; exists { + t.Fatalf("saved MCPServers = %#v, retained deleted MCP server", saved.MCPServers) + } + mcpServerForTest(t, saved.MCPServers, "native") +} + func newAgentMCPManagementTestServer(t *testing.T) (*Handler, *agent.Service, agent.Agent) { t.Helper() t.Setenv("HOME", t.TempDir()) diff --git a/internal/api/router.go b/internal/api/router.go index 13149fc7..137783aa 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -38,8 +38,8 @@ func (h *Handler) registerCoreRoutes(router chi.Router) { r.Get("/skills/file", h.handleAgentSkillsFile) r.Delete("/skills/{name}", h.handleAgentSkillDelete) r.Get("/mcp-servers", h.handleAgentMCPServersByID) - r.Put("/mcp-servers", h.handleAgentMCPServersByID) r.Post("/mcp-servers:batchAdd", h.handleBatchAddAgentMCPServers) + r.Post("/mcp-servers:batchDelete", h.handleBatchDeleteAgentMCPServers) r.Route("/profile", func(r chi.Router) { r.Get("/", h.getAgentProfile) r.Put("/", h.updateAgentProfile) diff --git a/internal/runtime/codex/runtime_test.go b/internal/runtime/codex/runtime_test.go index f3f2713e..6c956dcf 100644 --- a/internal/runtime/codex/runtime_test.go +++ b/internal/runtime/codex/runtime_test.go @@ -1945,7 +1945,7 @@ func TestConfigureCodexHomeConfigReplacesExistingMCPServerTablesWhenManaged(t *t `manual-mcp`, } { if strings.Contains(config, unwanted) { - t.Fatalf("config should remove stale MCP server %q:\n%s", unwanted, config) + t.Fatalf("config should replace imported MCP server %q:\n%s", unwanted, config) } } for _, want := range []string{ @@ -1981,7 +1981,7 @@ func TestConfigureCodexHomeConfigClearsExistingMCPServerTablesWhenManagedEmpty(t `manual-mcp`, } { if strings.Contains(config, unwanted) { - t.Fatalf("config should remove stale MCP server %q:\n%s", unwanted, config) + t.Fatalf("config should clear imported MCP server %q:\n%s", unwanted, config) } } for _, want := range []string{ diff --git a/web/app/src/api/agents.ts b/web/app/src/api/agents.ts index cc6481d0..76307613 100644 --- a/web/app/src/api/agents.ts +++ b/web/app/src/api/agents.ts @@ -40,11 +40,9 @@ export type DeleteBotOptions = { }; export type AgentMCPServersView = { - actual?: JSONRecord | null; - actual_error?: string | null; agent_id?: string | null; - desired?: JSONRecord | null; runtime_kind?: string | null; + servers?: JSONRecord | null; }; export type AgentUpdatePayload = { @@ -186,17 +184,17 @@ export function fetchAgentMCPServers(agentID: string): Promise { - return put(`api/v1/agents/${encodeURIComponent(agentID)}/mcp-servers`, { mcpServers }); -} - export function batchAddAgentMCPServersRequest(agentID: string, serverNames: string[]): Promise { return post(`api/v1/agents/${encodeURIComponent(agentID)}/mcp-servers:batchAdd`, { names: serverNames }); } +export function batchDeleteAgentMCPServersRequest( + agentID: string, + serverNames: string[], +): Promise { + return post(`api/v1/agents/${encodeURIComponent(agentID)}/mcp-servers:batchDelete`, { names: serverNames }); +} + export function batchAddAgentSkillsRequest(agentID: string, skillNames: string[]): Promise { return post(`api/v1/agents/${encodeURIComponent(agentID)}/skills:batchAdd`, { names: skillNames }); } diff --git a/web/app/src/hooks/workspace/useAgentController.ts b/web/app/src/hooks/workspace/useAgentController.ts index 7c9187a0..6e714aa1 100644 --- a/web/app/src/hooks/workspace/useAgentController.ts +++ b/web/app/src/hooks/workspace/useAgentController.ts @@ -5,6 +5,7 @@ import { errorMessage } from "@/api/client"; import { loginCLIProxyProviderRequest } from "@/api/cliproxy"; import { batchAddAgentMCPServersRequest, + batchDeleteAgentMCPServersRequest, batchAddAgentSkillsRequest, createBotRequest, createManagerAgentRequest, @@ -23,7 +24,6 @@ import { patchNotificationBotRequest, runAgentActionRequest, startFeishuRegistrationRequest, - updateAgentMCPServersRequest, updateAgentRequest, } from "@/api/agents"; import type { AgentUpdatePayload, FeishuRegistration, FetchAgentsOptions } from "@/api/agents"; @@ -102,7 +102,7 @@ import type { RuntimeKind, } from "@/models/agents"; import { isDirectConversation, localIdentitiesMatch, upsertUserInData } from "@/models/conversations"; -import { mcpServersFromMap, mcpServersMap } from "@/models/mcp"; +import { mcpServersFromMap } from "@/models/mcp"; import { displayTeam } from "@/models/tasks"; import type { WorkspaceTeam } from "@/models/tasks"; import { @@ -650,15 +650,12 @@ export function useAgentController({ ? errorMessage(globalSkillsQuery.error, t("agentSkillsLoadFailed")) : ""; const agentMCPServers = useMemo(() => { - return mcpServersFromMap(agentMCPServersQuery.data?.actual ?? agentMCPServersQuery.data?.desired ?? {}); - }, [agentMCPServersQuery.data]); - const agentDesiredMCPServers = useMemo(() => { - return mcpServersFromMap(agentMCPServersQuery.data?.desired ?? {}); + return mcpServersFromMap(agentMCPServersQuery.data?.servers); }, [agentMCPServersQuery.data]); const agentMCPCandidates = useMemo(() => { - const currentNames = new Set(agentDesiredMCPServers.map((server) => server.name)); + const currentNames = new Set(agentMCPServers.map((server) => server.name)); return catalogMCPServers.filter((server) => server.name && !currentNames.has(server.name)); - }, [agentDesiredMCPServers, catalogMCPServers]); + }, [agentMCPServers, catalogMCPServers]); const activeConversation = useMemo( () => data?.rooms.find((item) => item.id === activeConversationId) ?? null, [data, activeConversationId], @@ -728,7 +725,7 @@ export function useAgentController({ const runtimeKind = normalizeRuntimeKind(agent?.runtime_kind || item?.runtime_kind || base.runtime_kind); return ensureNotifierPullSubscriptionDraft({ ...base, - mcpServers: cloneMCPServersForDraft(mcpServersView.desired), + mcpServers: cloneMCPServersForDraft(mcpServersView.servers), runtime_kind: runtimeKind || base.runtime_kind, bot_type: BOT_TYPE_NORMAL, }); @@ -2107,7 +2104,7 @@ export function useAgentController({ setAgentMCPDeleteError(""); try { const view = await batchAddAgentMCPServersRequest(agentDetailAgentID, names); - const mcpServers = cloneMCPServersForDraft(view.desired); + const mcpServers = cloneMCPServersForDraft(view.servers); setAgentPageDraft((current) => (current ? { ...current, mcpServers } : current)); setAgentPageSavedDraft((current) => (current ? { ...current, mcpServers } : current)); await Promise.all([ @@ -2135,18 +2132,14 @@ export function useAgentController({ if (!name) { return false; } - const servers = mcpServersMap(agentMCPServersQuery.data?.desired); - if (!Object.hasOwn(servers, name)) { - return false; - } - delete servers[name]; setAgentMCPDeleteBusy(true); setAgentMCPAddError(""); setAgentMCPDeleteError(""); try { - await updateAgentMCPServersRequest(agentDetailAgentID, servers); - setAgentPageDraft((current) => (current ? { ...current, mcpServers: { ...servers } } : current)); - setAgentPageSavedDraft((current) => (current ? { ...current, mcpServers: { ...servers } } : current)); + const view = await batchDeleteAgentMCPServersRequest(agentDetailAgentID, [name]); + const mcpServers = cloneMCPServersForDraft(view.servers); + setAgentPageDraft((current) => (current ? { ...current, mcpServers } : current)); + setAgentPageSavedDraft((current) => (current ? { ...current, mcpServers } : current)); await Promise.all([ queryClient.invalidateQueries({ queryKey: workspaceQueryKeys.agentMCPServers(agentDetailAgentID) }), queryClient.invalidateQueries({ queryKey: workspaceQueryKeys.agents() }), @@ -2159,7 +2152,7 @@ export function useAgentController({ setAgentMCPDeleteBusy(false); } }, - [agentMCPServersQuery.data?.desired, agentMCPDeleteBusy, queryClient, agentDetailAgentID, t], + [agentMCPDeleteBusy, queryClient, agentDetailAgentID, t], ); function directConversationForUser( diff --git a/web/app/tests/api/agents.test.ts b/web/app/tests/api/agents.test.ts index 0a6d6326..66a51d3c 100644 --- a/web/app/tests/api/agents.test.ts +++ b/web/app/tests/api/agents.test.ts @@ -2,11 +2,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { Mock } from "vitest"; import { batchAddAgentMCPServersRequest, + batchDeleteAgentMCPServersRequest, batchAddAgentSkillsRequest, deleteAgentSkillRequest, fetchAgentMCPServers, startFeishuRegistrationRequest, - updateAgentMCPServersRequest, } from "@/api/agents"; function mockFetch(): Mock { @@ -34,10 +34,10 @@ describe("agents API", () => { ); }); - it("uses the agent MCP servers paths and direct server map payloads", async () => { + it("uses the agent MCP servers paths", async () => { const fetchMock = vi.fn( async () => - new Response(JSON.stringify({ actual: {}, desired: {} }), { + new Response(JSON.stringify({ servers: {} }), { headers: { "content-type": "application/json" }, status: 200, }), @@ -45,21 +45,21 @@ describe("agents API", () => { vi.stubGlobal("fetch", fetchMock); await fetchAgentMCPServers("u-manager"); - await updateAgentMCPServersRequest("u-manager", { context7: { command: "npx" } }); await batchAddAgentMCPServersRequest("u-manager", ["context7"]); + await batchDeleteAgentMCPServersRequest("u-manager", ["context7"]); expect(fetchMock).toHaveBeenNthCalledWith(1, "api/v1/agents/u-manager/mcp-servers", expect.anything()); expect(fetchMock).toHaveBeenNthCalledWith( 2, - "api/v1/agents/u-manager/mcp-servers", + "api/v1/agents/u-manager/mcp-servers:batchAdd", expect.objectContaining({ - body: JSON.stringify({ mcpServers: { context7: { command: "npx" } } }), - method: "PUT", + body: JSON.stringify({ names: ["context7"] }), + method: "POST", }), ); expect(fetchMock).toHaveBeenNthCalledWith( 3, - "api/v1/agents/u-manager/mcp-servers:batchAdd", + "api/v1/agents/u-manager/mcp-servers:batchDelete", expect.objectContaining({ body: JSON.stringify({ names: ["context7"] }), method: "POST", diff --git a/web/app/tests/hooks/useAgentController.test.tsx b/web/app/tests/hooks/useAgentController.test.tsx index 0dc76547..ae5ee926 100644 --- a/web/app/tests/hooks/useAgentController.test.tsx +++ b/web/app/tests/hooks/useAgentController.test.tsx @@ -4,6 +4,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { UseQueryResult } from "@tanstack/react-query"; import { batchAddAgentMCPServersRequest, + batchDeleteAgentMCPServersRequest, batchAddAgentSkillsRequest, createBotRequest, createNotificationBotRequest, @@ -23,7 +24,6 @@ import { finalizeFeishuRegistrationRequest, runAgentActionRequest, startFeishuRegistrationRequest, - updateAgentMCPServersRequest, updateAgentRequest, } from "@/api/agents"; import { createUserRequest } from "@/api/im"; @@ -67,6 +67,7 @@ vi.mock("@/api/agents", async () => { return { ...actual, batchAddAgentMCPServersRequest: vi.fn(), + batchDeleteAgentMCPServersRequest: vi.fn(), batchAddAgentSkillsRequest: vi.fn(), createBotRequest: vi.fn(), createNotificationBotRequest: vi.fn(), @@ -86,7 +87,6 @@ vi.mock("@/api/agents", async () => { finalizeFeishuRegistrationRequest: vi.fn(), runAgentActionRequest: vi.fn(), startFeishuRegistrationRequest: vi.fn(), - updateAgentMCPServersRequest: vi.fn(), updateAgentRequest: vi.fn(), }; }); @@ -280,6 +280,7 @@ function useAgentControllerHarness( describe("useAgentController", () => { beforeEach(() => { vi.mocked(batchAddAgentMCPServersRequest).mockReset(); + vi.mocked(batchDeleteAgentMCPServersRequest).mockReset(); vi.mocked(fetchAgent).mockReset(); vi.mocked(fetchAgentProfile).mockReset(); vi.mocked(fetchAgentProfileDefaults).mockReset(); @@ -303,7 +304,6 @@ describe("useAgentController", () => { vi.mocked(fetchTeams).mockReset(); vi.mocked(runAgentActionRequest).mockReset(); vi.mocked(startFeishuRegistrationRequest).mockReset(); - vi.mocked(updateAgentMCPServersRequest).mockReset(); vi.mocked(updateAgentRequest).mockReset(); vi.mocked(patchCsgclawUserRequest).mockReset(); window.localStorage.removeItem(feishuRegistrationStorageKey); @@ -338,10 +338,9 @@ describe("useAgentController", () => { vi.mocked(deleteAgentSkillRequest).mockResolvedValue(undefined); vi.mocked(deleteBotRequest).mockResolvedValue(undefined); vi.mocked(fetchAgentMCPServers).mockResolvedValue({ - actual: null, agent_id: "u-manager", - desired: null, runtime_kind: "picoclaw_sandbox", + servers: null, }); vi.mocked(fetchAgentWorkspace).mockResolvedValue({ entries: [] }); vi.mocked(createUserRequest).mockResolvedValue({ id: "u-worker", name: "worker" }); @@ -388,21 +387,19 @@ describe("useAgentController", () => { participant_id: "dev", status: "configured", }); - vi.mocked(updateAgentMCPServersRequest).mockImplementation(async (agentID, mcpServers) => ({ - actual: mcpServers, - agent_id: agentID, - desired: mcpServers, - runtime_kind: "picoclaw_sandbox", - })); vi.mocked(batchAddAgentMCPServersRequest).mockImplementation(async (agentID, serverNames) => { const servers = Object.fromEntries(serverNames.map((name) => [name, { command: "uvx", args: [`${name}-mcp`] }])); return { - actual: servers, agent_id: agentID, - desired: servers, runtime_kind: "picoclaw_sandbox", + servers, }; }); + vi.mocked(batchDeleteAgentMCPServersRequest).mockResolvedValue({ + agent_id: "u-manager", + runtime_kind: "picoclaw_sandbox", + servers: {}, + }); vi.mocked(updateAgentRequest).mockResolvedValue(latestAgent); vi.mocked(patchCsgclawUserRequest).mockImplementation(async (userID, payload) => ({ avatar: payload.avatar || "", @@ -653,10 +650,9 @@ describe("useAgentController", () => { vi.mocked(fetchAgent).mockReset(); vi.mocked(fetchAgent).mockResolvedValue(workerAgent); vi.mocked(fetchAgentMCPServers).mockResolvedValue({ - actual: desiredMCPServers, agent_id: "u-worker", - desired: desiredMCPServers, runtime_kind: "picoclaw_sandbox", + servers: desiredMCPServers, }); vi.mocked(updateAgentRequest).mockResolvedValue({ ...workerAgent, name: "renamed worker" }); @@ -835,18 +831,21 @@ describe("useAgentController", () => { expect(result.current.agentViewProps.skills).toEqual([]); }); - it("deletes an MCP server from the desired agent MCP server map", async () => { + it("deletes an MCP server through the backend MCP management endpoint", async () => { vi.mocked(fetchAgentMCPServers).mockResolvedValueOnce({ - actual: { + agent_id: "u-manager", + runtime_kind: "codex", + servers: { catalog: { command: "npx", args: ["catalog-mcp"] }, manual: { command: "uvx", args: ["manual-mcp"] }, }, + }); + vi.mocked(batchDeleteAgentMCPServersRequest).mockResolvedValueOnce({ agent_id: "u-manager", - desired: { + runtime_kind: "codex", + servers: { catalog: { command: "npx", args: ["catalog-mcp"] }, - manual: { command: "uvx", args: ["manual-mcp"] }, }, - runtime_kind: "codex", }); const { result } = renderHook(() => useAgentControllerHarness().controller, { wrapper: createWrapper() }); @@ -859,20 +858,65 @@ describe("useAgentController", () => { await result.current.agentViewProps.onDeleteMCPServer?.("manual"); }); - expect(updateAgentMCPServersRequest).toHaveBeenCalledWith("u-manager", { - catalog: { command: "npx", args: ["catalog-mcp"] }, + expect(batchDeleteAgentMCPServersRequest).toHaveBeenCalledWith("u-manager", ["manual"]); + expect(result.current.agentViewProps.mcpDeleteError).toBe(""); + }); + + it("uses the single backend MCP server map for display, candidates, and deletion", async () => { + vi.mocked(fetchAgentMCPServers).mockResolvedValueOnce({ + agent_id: "u-manager", + runtime_kind: "codex", + servers: { + catalog: { command: "npx", args: ["catalog-mcp"] }, + manual: { command: "uvx", args: ["manual-mcp"] }, + native: { command: "npx", args: ["native-mcp"] }, + }, + }); + vi.mocked(batchDeleteAgentMCPServersRequest).mockResolvedValueOnce({ + agent_id: "u-manager", + runtime_kind: "codex", + servers: { + catalog: { command: "npx", args: ["catalog-mcp"] }, + native: { command: "npx", args: ["native-mcp"] }, + }, + }); + + const { result } = renderHook( + () => + useAgentControllerHarness({ + catalogMCPServers: [ + { name: "catalog", description: "Catalog", config: { command: "npx" } }, + { name: "manual", description: "Manual", config: { command: "uvx" } }, + { name: "native", description: "Native", config: { command: "npx" } }, + ], + }).controller, + { wrapper: createWrapper() }, + ); + + await waitFor(() => + expect(result.current.agentViewProps.mcpServers.map((server) => server.name)).toEqual([ + "catalog", + "manual", + "native", + ]), + ); + expect(result.current.agentViewProps.mcpCandidates).toEqual([]); + + await act(async () => { + await result.current.agentViewProps.onDeleteMCPServer?.({ name: "manual" }); }); + + expect(batchDeleteAgentMCPServersRequest).toHaveBeenCalledWith("u-manager", ["manual"]); expect(result.current.agentViewProps.mcpDeleteError).toBe(""); }); - it("filters catalog MCP candidates by desired servers while runtime state is stale", async () => { + it("filters catalog MCP candidates by the backend MCP server map", async () => { vi.mocked(fetchAgentMCPServers).mockResolvedValueOnce({ - actual: {}, agent_id: "u-manager", - desired: { + runtime_kind: "codex", + servers: { manual: { command: "uvx", env: { MCP_TOKEN: "secret" } }, }, - runtime_kind: "codex", }); const { result } = renderHook( @@ -886,19 +930,19 @@ describe("useAgentController", () => { { wrapper: createWrapper() }, ); - await waitFor(() => expect(result.current.agentViewProps.mcpServers).toEqual([])); + await waitFor(() => + expect(result.current.agentViewProps.mcpServers.map((server) => server.name)).toEqual(["manual"]), + ); expect(result.current.agentViewProps.mcpCandidates.map((server) => server.name)).toEqual(["context7"]); }); - it("falls back to desired MCP servers when the runtime state is unavailable", async () => { + it("renders the backend MCP server map when the runtime state is unavailable", async () => { vi.mocked(fetchAgentMCPServers).mockResolvedValueOnce({ - actual: null, - actual_error: "read runtime config: unavailable", agent_id: "u-manager", - desired: { + runtime_kind: "codex", + servers: { manual: { command: "uvx" }, }, - runtime_kind: "codex", }); const { result } = renderHook( @@ -917,21 +961,17 @@ describe("useAgentController", () => { it("installs MCP catalog servers through the agent MCP server endpoint", async () => { const installedView = { - actual: { - context7: { command: "uvx", args: ["context7-mcp"] }, - }, agent_id: "u-manager", - desired: { + runtime_kind: "picoclaw_sandbox", + servers: { context7: { command: "uvx", args: ["context7-mcp"] }, }, - runtime_kind: "picoclaw_sandbox", }; vi.mocked(fetchAgentMCPServers) .mockResolvedValueOnce({ - actual: null, agent_id: "u-manager", - desired: null, runtime_kind: "picoclaw_sandbox", + servers: null, }) .mockResolvedValue(installedView); vi.mocked(batchAddAgentMCPServersRequest).mockResolvedValueOnce(installedView); @@ -953,23 +993,25 @@ describe("useAgentController", () => { }); expect(batchAddAgentMCPServersRequest).toHaveBeenCalledWith("u-manager", ["context7"]); - expect(updateAgentMCPServersRequest).not.toHaveBeenCalled(); + expect(batchDeleteAgentMCPServersRequest).not.toHaveBeenCalled(); await waitFor(() => expect(result.current.agentViewProps.mcpServers.map((server) => server.name)).toEqual(["context7"]), ); expect(result.current.agentViewProps.mcpAddError).toBe(""); }); - it("submits an empty MCP server map when deleting the last desired MCP server", async () => { + it("asks the backend to delete the last MCP server", async () => { vi.mocked(fetchAgentMCPServers).mockResolvedValueOnce({ - actual: { - manual: { command: "uvx", args: ["manual-mcp"] }, - }, agent_id: "u-manager", - desired: { + runtime_kind: "codex", + servers: { manual: { command: "uvx", args: ["manual-mcp"] }, }, + }); + vi.mocked(batchDeleteAgentMCPServersRequest).mockResolvedValueOnce({ + agent_id: "u-manager", runtime_kind: "codex", + servers: {}, }); const { result } = renderHook(() => useAgentControllerHarness().controller, { wrapper: createWrapper() }); @@ -982,7 +1024,7 @@ describe("useAgentController", () => { await result.current.agentViewProps.onDeleteMCPServer?.({ name: "manual" }); }); - expect(updateAgentMCPServersRequest).toHaveBeenCalledWith("u-manager", {}); + expect(batchDeleteAgentMCPServersRequest).toHaveBeenCalledWith("u-manager", ["manual"]); expect(result.current.agentViewProps.mcpDeleteError).toBe(""); });