diff --git a/client/src/i18n/locales/en/ccConfig.json b/client/src/i18n/locales/en/ccConfig.json index a65b6988..64315745 100644 --- a/client/src/i18n/locales/en/ccConfig.json +++ b/client/src/i18n/locales/en/ccConfig.json @@ -131,6 +131,8 @@ "projectScope": "Project-scoped servers", "name": "Name", "scope": "Scope", + "scopeUser": "User", + "scopeLocal": "This project", "command": "Command", "args": "Args", "env": "Env names", @@ -138,7 +140,8 @@ "headers": "Header names", "kind": "Kind", "source": "Source", - "noServers": "No MCP servers found." + "noServers": "No MCP servers found.", + "notRemovable": "Configured in settings.json — remove it there; no CLI scope addresses this entry." }, "hooks": { "noHooks": "No hooks configured.", @@ -277,10 +280,7 @@ "body": "MCP server config lives inside `~/.claude.json`, the same file the live CLI writes to constantly. Concurrent writes from the dashboard would corrupt project state, so add/remove flow goes through Claude Code itself.", "list": "List configured servers:", "listCmd": "claude mcp list", - "add": "Add a server:", - "addCmd": "claude mcp add [args…]", - "remove": "Remove this server:", - "removeButton": "Copy remove command" + "add": "Add a server:" }, "marketplaces": { "title": "Marketplaces are where plugins come from", @@ -326,7 +326,7 @@ "addMcpCommandPlaceholder": "npx -y @modelcontextprotocol/server-github", "addMcpUrlPlaceholder": "https://mcp.example.com", "addMcpButton": "Add", - "addMcpHelp": "Runs `claude mcp add`. Scope user/project only — a `local`-scoped server is the CLI's per-checkout layer and isn't offered here.", + "addMcpHelp": "Runs `claude mcp add`. \"This project\" scopes the server to the current checkout — the CLI's separate `project` scope (a shared `.mcp.json`) isn't offered here because this dashboard can't see servers stored there.", "mcpAdded": "Added MCP server {{name}}.", "mcpRemoved": "Removed MCP server {{name}}.", "hookEvent": "Event name (e.g. PreToolUse)", diff --git a/client/src/i18n/locales/vi/ccConfig.json b/client/src/i18n/locales/vi/ccConfig.json index 0f8344a1..3ab4dda1 100644 --- a/client/src/i18n/locales/vi/ccConfig.json +++ b/client/src/i18n/locales/vi/ccConfig.json @@ -126,6 +126,8 @@ "projectScope": "Máy chủ phạm vi dự án", "name": "Tên", "scope": "Phạm vi", + "scopeUser": "Người dùng", + "scopeLocal": "Dự án này", "command": "Lệnh", "args": "Tham số", "env": "Tên env", @@ -133,7 +135,8 @@ "headers": "Tên Header", "kind": "Loại", "source": "Nguồn", - "noServers": "Không tìm thấy máy chủ MCP." + "noServers": "Không tìm thấy máy chủ MCP.", + "notRemovable": "Được cấu hình trong settings.json — hãy xóa ở đó; không có phạm vi CLI nào áp dụng cho mục này." }, "hooks": { "noHooks": "Chưa cấu hình hook.", @@ -272,10 +275,7 @@ "body": "Cấu hình MCP server nằm trong `~/.claude.json`, đúng tệp mà CLI đang chạy ghi liên tục. Ghi đồng thời từ dashboard sẽ phá hỏng trạng thái dự án, nên flow add/remove đi qua chính Claude Code.", "list": "Liệt kê server đã cấu hình:", "listCmd": "claude mcp list", - "add": "Thêm server:", - "addCmd": "claude mcp add [args…]", - "remove": "Gỡ server này:", - "removeButton": "Sao chép lệnh gỡ" + "add": "Thêm server:" }, "marketplaces": { "title": "Marketplace là nơi plugin đến từ đó", @@ -321,7 +321,7 @@ "addMcpCommandPlaceholder": "npx -y @modelcontextprotocol/server-github", "addMcpUrlPlaceholder": "https://mcp.example.com", "addMcpButton": "Thêm", - "addMcpHelp": "Chạy lệnh `claude mcp add`. Chỉ có phạm vi user/project — phạm vi `local` của CLI (theo từng checkout) không được cung cấp ở đây.", + "addMcpHelp": "Chạy lệnh `claude mcp add`. \"Dự án này\" giới hạn server trong checkout hiện tại — phạm vi `project` riêng của CLI (một `.mcp.json` dùng chung) không được cung cấp ở đây vì dashboard không thể thấy các server lưu ở đó.", "mcpAdded": "Đã thêm máy chủ MCP {{name}}.", "mcpRemoved": "Đã xóa máy chủ MCP {{name}}.", "hookEvent": "Tên sự kiện (vd PreToolUse)", diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index 2cdaa073..a646eccc 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -2671,10 +2671,13 @@ export interface CcMcpResponse { projectScoped: CcMcpServer[]; } -/** Scope the `claude mcp` CLI accepts from this form. Deliberately not the - * three-value settings union — the CLI's own third scope (`local`) is a - * different axis and is not offered here. */ -export type CcMcpScope = "user" | "project"; +/** Scope the `claude mcp` CLI accepts from this form — `user` and `local` + * ONLY, matching what GET /mcp's `readMcpServers` can actually see: + * `user` is `~/.claude.json` top-level, `local` is its `projects[]` + * block (the panel's `projectScoped` bucket, despite the name). The CLI's + * `project` scope writes `.mcp.json`, which `readMcpServers` never reads — + * offering it here would add a server the dashboard can never show exists. */ +export type CcMcpScope = "user" | "local"; /** Body of POST /api/cc-config/mcp/add. Either `url` (http/sse) or * `command`+`args` (stdio) is required, never both. `env` is stdio-only; diff --git a/client/src/pages/CcConfig.tsx b/client/src/pages/CcConfig.tsx index 7d040201..f81f3067 100644 --- a/client/src/pages/CcConfig.tsx +++ b/client/src/pages/CcConfig.tsx @@ -1903,8 +1903,8 @@ function McpPanel({ aria-label={t("mcp.scope")} className="h-8 bg-surface-1 border border-border rounded px-2 text-[12px] text-fg-primary focus:outline-none focus:ring-1 focus:ring-accent/40" > - - + +
{(["stdio", "http"] as const).map((k) => ( @@ -1994,7 +1994,7 @@ function McpPanel({
@@ -2063,29 +2067,33 @@ function McpCard({ )}
- { - if (!confirmRemove) { - setConfirmRemove(true); - return; - } - setConfirmRemove(false); - void run( - busyId, - () => api.ccConfig.mcpRemove(server.name, scope), - t("manage.mcpRemoved", { name: server.name }) - ); - }} - > - {busy === busyId - ? t("manage.working") - : confirmRemove - ? t("manage.confirmRemove") - : t("manage.remove")} - + {removable ? ( + { + if (!confirmRemove) { + setConfirmRemove(true); + return; + } + setConfirmRemove(false); + void run( + busyId, + () => api.ccConfig.mcpRemove(server.name, scope), + t("manage.mcpRemoved", { name: server.name }) + ); + }} + > + {busy === busyId + ? t("manage.working") + : confirmRemove + ? t("manage.confirmRemove") + : t("manage.remove")} + + ) : ( +

{t("mcp.notRemovable")}

+ )}
); diff --git a/docs/API.md b/docs/API.md index bf36b53a..65debfb2 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1607,7 +1607,7 @@ POST /api/cc-config/mcp/remove Body: { name, scope? } **Plugins and marketplaces.** These five endpoints shell out to the `claude plugin …` CLI with an explicit argv array (never a shell string), pinned to the dashboard's `CLAUDE_HOME` via `CLAUDE_CONFIG_DIR`; mutating calls are serialized in-process. `source` must be `owner/repo`, an `http(s)://` URL, or an absolute path; `spec`/`key` must be `` or `@`; `scope` is `user` (default), `project` or `local`. Validation failures return 400 `EBADARG`; a missing `claude` binary returns 503 `ECLINOTFOUND`; a CLI timeout returns 504 `ETIMEOUT`; a non-zero CLI exit returns **422 `ECLIEXIT`** with the CLI's message plus `stdout`/`stderr`. Successful runs return `{ ok: true, stdout, stderr }` and broadcast `cc_config_changed`. `GET /plugins/available` proxies `claude plugin list --available --json` as `{ installed, available }`. -**MCP servers.** `POST /api/cc-config/mcp/add` and `POST /api/cc-config/mcp/remove` shell out to `claude mcp add`/`claude mcp remove` the same way the plugin routes shell out to `claude plugin …` — an argv array, never a shell string, serialized in-process. `scope` is `user` or `project` only (required on add, optional on remove — omitted, the CLI removes from whichever scope the server exists in); the settings union's third value `project-local` is a different axis and is not accepted here. `add` takes either `url` (+ optional `transport: "http"|"sse"`, `headers`) for a remote server, or `command` + `args` (+ optional `env`) for a stdio server — exactly one of the two forms. Validation failures return 400 `EBADARG`; a non-zero CLI exit returns 422 `ECLIEXIT`. `GET /api/cc-config/mcp` is unaffected — it still reads `~/.claude.json` directly; the client refetches it after either call succeeds. +**MCP servers.** `POST /api/cc-config/mcp/add` and `POST /api/cc-config/mcp/remove` shell out to `claude mcp add`/`claude mcp remove` the same way the plugin routes shell out to `claude plugin …` — an argv array, never a shell string, serialized on the SAME in-process queue `cc-plugin-cli.js` exports (`serialize`), so a plugin install and an MCP add can never spawn `claude` concurrently. `scope` is `user` or `local` only (required on add, optional on remove — omitted, the CLI removes from whichever scope the server exists in). This is the inverse of what it sounds like: `user` writes `~/.claude.json`'s top level, `local` writes its `projects[]` block — the exact source `GET /mcp`'s `projectScoped` bucket reads. The CLI's own `project` scope (a separate, shared `.mcp.json`) is refused with 400: `readMcpServers` never reads that file, so a server added there would report success and never appear in the list. `add` takes either `url` (+ optional `transport: "http"|"sse"`, `headers`) for a remote server, or `command` + `args` (+ optional `env`) for a stdio server — exactly one of the two forms. Validation failures return 400 `EBADARG`; a non-zero CLI exit returns 422 `ECLIEXIT`. `GET /api/cc-config/mcp` is unaffected — it still reads `~/.claude.json` directly; the client refetches it after either call succeeds. Rows sourced from `~/.claude/settings.json` have no CLI scope that addresses them, so the panel does not offer Remove on them. `PUT /api/cc-config/keybindings` edits `~/.claude/keybindings.json` from a structured list of context groups (`{ groups: [{ context, bindings: [{ key, action }] }] }`). The server backs the file up first (under `/cc-config-backups/keybindings/`), preserves any top-level metadata (`$schema`/`$docs`), and replaces only the `bindings` array; duplicate contexts or duplicate keys within a context return `EBADCONTENT`. `keybindings.json` is replaced wholesale here because the live CLI does not rewrite it mid-session; `settings.json`, which it does, is only ever key-merged (see `PATCH /settings` above). diff --git a/openapi.yaml b/openapi.yaml index 89d41bdf..d81eb39f 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -8016,7 +8016,7 @@ paths: tags: - CcConfig summary: Add an MCP server - description: 'Mutating. Runs `claude mcp add` — never a direct edit of ~/.claude.json, which the running CLI rewrites concurrently. Body carries either `url` (+ optional `transport: http|sse` and `headers`) for a remote server, or `command` + `args` (+ optional `env`) for a stdio server; exactly one of the two forms is required. `scope` is required and is `user`|`project` only — not the settings union''s third `project-local` value, which is a different axis. A non-zero CLI exit is returned as 422, not 500. Emits a `cc_config_changed` websocket event on success.' + description: 'Mutating. Runs `claude mcp add` — never a direct edit of ~/.claude.json, which the running CLI rewrites concurrently. Body carries either `url` (+ optional `transport: http|sse` and `headers`) for a remote server, or `command` + `args` (+ optional `env`) for a stdio server; exactly one of the two forms is required. `scope` is required and is `user`|`local` only: `user` writes ~/.claude.json''s top level, `local` writes its `projects[]` block (GET /mcp''s `projectScoped` bucket). The CLI''s own `project` scope (a shared `.mcp.json`) is deliberately not offered — `readMcpServers` never reads that file, so a server added there would report success and never appear in the list. A non-zero CLI exit is returned as 422, not 500. Emits a `cc_config_changed` websocket event on success.' operationId: ccConfigPostMcpAdd requestBody: required: true @@ -8035,7 +8035,7 @@ paths: type: string enum: - user - - project + - local example: user transport: type: string @@ -8083,7 +8083,7 @@ paths: tags: - CcConfig summary: Remove an MCP server - description: Mutating. Runs `claude mcp remove`. `scope` (`user`|`project`) narrows which config layer to remove from; omit it to let the CLI find the server wherever it exists. A non-zero CLI exit (e.g. the name does not exist) is returned as 422. Emits a `cc_config_changed` websocket event on success. + description: Mutating. Runs `claude mcp remove`. `scope` (`user`|`local`) narrows which config layer to remove from; omit it to let the CLI find the server wherever it exists. The CLI's own `project` scope is refused with 400 — the panel has no row sourced from `.mcp.json` to remove. A non-zero CLI exit (e.g. the name does not exist) is returned as 422. Emits a `cc_config_changed` websocket event on success. operationId: ccConfigPostMcpRemove requestBody: required: true @@ -8101,7 +8101,7 @@ paths: type: string enum: - user - - project + - local responses: '200': description: The CLI's stdout/stderr on success. diff --git a/server/__tests__/cc-config.test.js b/server/__tests__/cc-config.test.js index ca9064a7..5b58eeac 100644 --- a/server/__tests__/cc-config.test.js +++ b/server/__tests__/cc-config.test.js @@ -1192,7 +1192,7 @@ describe("/api/cc-config", () => { try { const { status, body } = await fetchJson("/api/cc-config/mcp/remove", { method: "POST", - body: { name: "docs", scope: "project" }, + body: { name: "docs", scope: "local" }, }); assert.equal(status, 200); assert.deepEqual(body.stdout.trim().split("\n"), [ @@ -1200,10 +1200,19 @@ describe("/api/cc-config", () => { "remove", "docs", "--scope", - "project", + "local", ]); } finally { restore(); } }); + + it("POST /mcp/remove rejects the CLI's project scope with 400 — .mcp.json isn't addressable here", async () => { + const { status, body } = await fetchJson("/api/cc-config/mcp/remove", { + method: "POST", + body: { name: "docs", scope: "project" }, + }); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADARG"); + }); }); diff --git a/server/__tests__/cc-mcp-cli.test.js b/server/__tests__/cc-mcp-cli.test.js index 19af82d0..6839d4f1 100644 --- a/server/__tests__/cc-mcp-cli.test.js +++ b/server/__tests__/cc-mcp-cli.test.js @@ -65,11 +65,18 @@ describe("cc-mcp-cli argument validation", () => { assert.equal(cli.assertMcpScope("user", { required: true }), "user"); }); - it("rejects the settings-only project-local scope and any other unknown scope", () => { - for (const bad of ["project-local", "local", "bogus"]) { + it("rejects the CLI's own `project` scope — readMcpServers never reads .mcp.json", () => { + // The Critical this pins: `--scope project` writes .mcp.json, which the + // dashboard's read path never parses, so a successful add would report + // an "added" toast for a server that never appears in the list. + for (const bad of ["project", "project-local", "bogus"]) { assert.throws(() => cli.assertMcpScope(bad), /scope must be one of/, `accepted ${bad}`); } }); + + it("accepts local — the CLI scope that actually backs the panel's projectScoped bucket", () => { + assert.equal(cli.assertMcpScope("local", { required: true }), "local"); + }); }); describe("cc-mcp-cli argv construction", () => { @@ -119,7 +126,7 @@ describe("cc-mcp-cli argv construction", () => { try { const result = await cli.mcpAdd({ name: "corridor", - scope: "project", + scope: "local", transport: "http", url: "https://app.corridor.dev/api/mcp", headers: { Authorization: "Bearer xyz" }, @@ -128,7 +135,7 @@ describe("cc-mcp-cli argv construction", () => { "mcp", "add", "--scope", - "project", + "local", "--transport", "http", "corridor", @@ -172,18 +179,45 @@ describe("cc-mcp-cli argv construction", () => { const restore = stubClaude(); try { assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs" })), ["mcp", "remove", "docs"]); - assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs", scope: "project" })), [ + assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs", scope: "user" })), [ "mcp", "remove", "docs", "--scope", - "project", + "user", ]); } finally { restore(); } }); + it("builds a local-scoped remove — the argv the CLI actually accepts for the panel's projectScoped rows", async () => { + // The other Critical this pins: the panel used to send scope: "project" + // for these rows, which the real CLI never matches (it stores them under + // "local"), so Remove failed every time. `argvOf` is the exact argv + // `execFile` spawns — there is no shell in between to paper over a wrong + // scope string. + const restore = stubClaude(); + try { + assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs", scope: "local" })), [ + "mcp", + "remove", + "docs", + "--scope", + "local", + ]); + } finally { + restore(); + } + }); + + it("rejects a remove scoped to the CLI's project (.mcp.json is not addressable here)", async () => { + await assert.rejects( + () => cli.mcpRemove({ name: "docs", scope: "project" }), + /scope must be one of/ + ); + }); + it("rejects url and command together, and neither", async () => { await assert.rejects( () => cli.mcpAdd({ name: "x", scope: "user", url: "https://a", command: "b" }), @@ -212,6 +246,48 @@ describe("cc-mcp-cli argv construction", () => { ); }); + it("allows an empty stdio arg — only type/length/control-char bounds are enforced", async () => { + // A trailing empty argv element is invisible to `.trim().split("\n")` + // (it collapses with the final newline), so assert with a marker arg + // after it rather than checking the last line directly. + const restore = stubClaude(); + try { + const result = await cli.mcpAdd({ + name: "x", + scope: "user", + command: "node", + args: ["", "marker"], + }); + assert.deepEqual(argvOf(result).slice(-3), ["node", "", "marker"]); + } finally { + restore(); + } + }); + + it("rejects an object env/header value as a 400 instead of writing [object Object]", async () => { + await assert.rejects( + () => + cli.mcpAdd({ + name: "x", + scope: "user", + command: "node", + env: { KEY: { nope: true } }, + }), + /KEY is required/ + ); + await assert.rejects( + () => + cli.mcpAdd({ + name: "x", + scope: "user", + transport: "http", + url: "https://a", + headers: { Authorization: { nope: true } }, + }), + /Authorization is required/ + ); + }); + it("never lets an args element reach a shell", async () => { // A shell-metacharacter payload is a legal argv element (many real flags // contain punctuation) — the guarantee is execFile's argv array, not a diff --git a/server/lib/cc-mcp-cli.js b/server/lib/cc-mcp-cli.js index 79484d39..b089d153 100644 Binary files a/server/lib/cc-mcp-cli.js and b/server/lib/cc-mcp-cli.js differ diff --git a/server/lib/cc-plugin-cli.js b/server/lib/cc-plugin-cli.js index f237da86..e462fe77 100644 --- a/server/lib/cc-plugin-cli.js +++ b/server/lib/cc-plugin-cli.js @@ -259,6 +259,10 @@ function cliMessage(res) { module.exports = { runClaude, cliMessage, + // Exported so cc-mcp-cli.js queues onto this SAME chokepoint — one CLI + // process against CLAUDE_CONFIG_DIR at a time, whether the click was a + // plugin install or an MCP add. + serialize, listAvailable, marketplaceAdd, marketplaceRemove, diff --git a/server/openapi-extra/cc-config.js b/server/openapi-extra/cc-config.js index 7f516f6e..b66a86f8 100644 --- a/server/openapi-extra/cc-config.js +++ b/server/openapi-extra/cc-config.js @@ -1597,7 +1597,7 @@ const paths = { tags: ["CcConfig"], summary: "Add an MCP server", description: - "Mutating. Runs `claude mcp add` — never a direct edit of ~/.claude.json, which the running CLI rewrites concurrently. Body carries either `url` (+ optional `transport: http|sse` and `headers`) for a remote server, or `command` + `args` (+ optional `env`) for a stdio server; exactly one of the two forms is required. `scope` is required and is `user`|`project` only — not the settings union's third `project-local` value, which is a different axis. A non-zero CLI exit is returned as 422, not 500. Emits a `cc_config_changed` websocket event on success.", + "Mutating. Runs `claude mcp add` — never a direct edit of ~/.claude.json, which the running CLI rewrites concurrently. Body carries either `url` (+ optional `transport: http|sse` and `headers`) for a remote server, or `command` + `args` (+ optional `env`) for a stdio server; exactly one of the two forms is required. `scope` is required and is `user`|`local` only: `user` writes ~/.claude.json's top level, `local` writes its `projects[]` block (GET /mcp's `projectScoped` bucket). The CLI's own `project` scope (a shared `.mcp.json`) is deliberately not offered — `readMcpServers` never reads that file, so a server added there would report success and never appear in the list. A non-zero CLI exit is returned as 422, not 500. Emits a `cc_config_changed` websocket event on success.", operationId: "ccConfigPostMcpAdd", requestBody: { required: true, @@ -1608,7 +1608,7 @@ const paths = { required: ["name", "scope"], properties: { name: { type: "string", example: "vercel" }, - scope: { type: "string", enum: ["user", "project"], example: "user" }, + scope: { type: "string", enum: ["user", "local"], example: "user" }, transport: { type: "string", enum: ["http", "sse"] }, url: { type: "string", example: "https://mcp.vercel.com" }, headers: { @@ -1652,7 +1652,7 @@ const paths = { tags: ["CcConfig"], summary: "Remove an MCP server", description: - "Mutating. Runs `claude mcp remove`. `scope` (`user`|`project`) narrows which config layer to remove from; omit it to let the CLI find the server wherever it exists. A non-zero CLI exit (e.g. the name does not exist) is returned as 422. Emits a `cc_config_changed` websocket event on success.", + "Mutating. Runs `claude mcp remove`. `scope` (`user`|`local`) narrows which config layer to remove from; omit it to let the CLI find the server wherever it exists. The CLI's own `project` scope is refused with 400 — the panel has no row sourced from `.mcp.json` to remove. A non-zero CLI exit (e.g. the name does not exist) is returned as 422. Emits a `cc_config_changed` websocket event on success.", operationId: "ccConfigPostMcpRemove", requestBody: { required: true, @@ -1663,7 +1663,7 @@ const paths = { required: ["name"], properties: { name: { type: "string", example: "vercel" }, - scope: { type: "string", enum: ["user", "project"] }, + scope: { type: "string", enum: ["user", "local"] }, }, }, },