From c34931c3f544a2b2d4fe6a6afbb8db7f6d5cf4f3 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Wed, 9 Sep 2026 10:00:05 +0700 Subject: [PATCH] fix(cc-config): .claude.json sits BESIDE ~/.claude by default, not inside it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit routed the read through getClaudeHome(), which appends /.claude when CLAUDE_HOME is unset — pointing the dashboard at ~/.claude/.claude.json, a file the CLI never writes by default. Verified against the real binary: with CLAUDE_CONFIG_DIR unset the CLI writes $HOME/.claude.json; with it set, .claude.json sits flat inside that dir. Branch on CLAUDE_HOME accordingly. The test now drives a fake claude that writes where the real one does and asserts getClaudeJsonPath() names that exact file, under both branches — the previous test only restated the implementation. --- docs/API.md | 4 +- openapi.yaml | 4 ++ server/__tests__/cc-plugin-cli.test.js | 58 ++++++++++++++++++-------- server/lib/cc-discovery.js | 22 ++++++---- server/openapi-extra/cc-config.js | 6 +++ 5 files changed, 68 insertions(+), 26 deletions(-) diff --git a/docs/API.md b/docs/API.md index 65debfb2..33493fe6 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1605,9 +1605,9 @@ POST /api/cc-config/mcp/remove Body: { name, scope? } **`PATCH /api/cc-config/settings` (key merge).** `scope` is `"user"`, `"project"`, or `"project-local"` (`settings.local.json`). `set` maps dotted key paths to JSON values; `unset` is a list of dotted key paths to delete. The server re-reads the target file **unredacted**, applies only the named keys, backs it up under `/.cc-config-backups/settings/`, and writes atomically — so keys the UI never displayed (including secret-valued ones) survive byte-for-byte. Returns `{ path, backupPath, created, changed }`. Hooks are edited through this same route, as `set: { "hooks.PreToolUse": [{ matcher, hooks: [{ type, command, timeout? }] }] }` — i.e. `settings.json`'s own nested shape, not the flattened shape `GET /hooks` returns. Errors: `EBADSCOPE` (unknown scope), `EBADNAME` (a key segment outside `[A-Za-z0-9_$][A-Za-z0-9._$-]*`, or `__proto__`/`prototype`/`constructor`), `EREDACTED` (400 — the payload contains the `` placeholder, which would destroy a real secret), `EBADJSON` (409 — the file on disk is not valid JSON; it is left untouched rather than clobbered), `ETOOLARGE`. The project layers always resolve against the **server's** cwd; `?cwd=` is honoured on reads only, never on this write. -**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 }`. +**Plugins and marketplaces.** These five endpoints shell out to the `claude plugin …` CLI with an explicit argv array (never a shell string); mutating calls are serialized in-process. `CLAUDE_CONFIG_DIR` is pinned to `CLAUDE_HOME` only when the operator explicitly set `CLAUDE_HOME` — left unset (the default), the CLI's own default config dir already matches what `cc-discovery` reads, so the pin is left off rather than pointed at the wrong directory (see `runClaude()`'s comment in `cc-plugin-cli.js` for the incident this fixed). `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 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. +**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. `GET /mcp`'s server objects carry `removable: false` for rows sourced from `~/.claude/settings.json` (absent/true means removable) — no CLI scope addresses them, so the panel reads this flag rather than matching the `source` display string to decide whether to offer Remove. `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 d81eb39f..6b25f80f 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -2039,6 +2039,10 @@ components: type: string description: Where the definition was found, e.g. `~/.claude.json (top-level)`, `~/.claude.json (projects[])`, or `~/.claude/settings.json`. example: ~/.claude.json (top-level) + removable: + type: boolean + description: False when no `claude mcp` scope can address this entry (currently only ~/.claude/settings.json-sourced servers), so POST /mcp/remove has no scope that would find it. Absent/true means removable. + example: true kind: type: string enum: diff --git a/server/__tests__/cc-plugin-cli.test.js b/server/__tests__/cc-plugin-cli.test.js index dd6946d8..1d53bcc8 100644 --- a/server/__tests__/cc-plugin-cli.test.js +++ b/server/__tests__/cc-plugin-cli.test.js @@ -225,26 +225,50 @@ describe("cc-plugin-cli CLAUDE_CONFIG_DIR pinning", () => { } }); - // The two env tests above pin one half each. This pins the pair: the file - // the dashboard READS must be the file the CLI WRITES, under both settings. - // `.claude.json` was the only read path in cc-discovery not derived from - // getClaudeHome(), which is exactly how the two halves drifted apart. - it("reads the same .claude.json the CLI writes, with CLAUDE_HOME set or unset", () => { + // The two env tests above pin the env var each half of the pair depends + // on. This pins the pair itself end to end: the file getClaudeJsonPath() + // returns must be the file the CLI actually wrote — under BOTH an unset + // and a set CLAUDE_HOME. A fake `claude` that writes `.claude.json` + // exactly where the real binary does (verified against it: flat inside + // CLAUDE_CONFIG_DIR when set, beside — NOT inside — $HOME when unset) + // stands in for the real CLI; no real CLI or real $HOME/CLAUDE_HOME touched. + const WRITE_CLAUDE_JSON = 'echo "{}" > "${CLAUDE_CONFIG_DIR:-$HOME}/.claude.json"'; + + it("writes .claude.json exactly where getClaudeJsonPath() reads it, with CLAUDE_HOME unset", async () => { const cc = require("../lib/cc-discovery"); - const originalHome = process.env.CLAUDE_HOME; - - // CLI's write target = /.claude.json, and - // CLAUDE_CONFIG_DIR is either CLAUDE_HOME or the CLI's own default, - // which is the same directory getClaudeHome() returns. + const originalHome = process.env.HOME; + const originalClaudeHome = process.env.CLAUDE_HOME; + const fakeHome = fs.mkdtempSync(path.join(os.tmpdir(), "agenthub-fakehome-")); + delete process.env.CLAUDE_HOME; + process.env.HOME = fakeHome; + const restore = stubClaude(WRITE_CLAUDE_JSON); try { - delete process.env.CLAUDE_HOME; - assert.equal(cc.getClaudeJsonPath(), path.join(os.homedir(), ".claude", ".claude.json")); - - process.env.CLAUDE_HOME = "/tmp/some-other-claude-home"; - assert.equal(cc.getClaudeJsonPath(), "/tmp/some-other-claude-home/.claude.json"); + await cli.marketplaceUpdate({ name: "m" }); + assert.equal(cc.getClaudeJsonPath(), path.join(fakeHome, ".claude.json")); + assert.ok(fs.existsSync(cc.getClaudeJsonPath()), "the CLI's write landed where reads look"); } finally { - if (originalHome === undefined) delete process.env.CLAUDE_HOME; - else process.env.CLAUDE_HOME = originalHome; + restore(); + process.env.HOME = originalHome; + if (originalClaudeHome !== undefined) process.env.CLAUDE_HOME = originalClaudeHome; + fs.rmSync(fakeHome, { recursive: true, force: true }); + } + }); + + it("writes .claude.json exactly where getClaudeJsonPath() reads it, with CLAUDE_HOME set", async () => { + const cc = require("../lib/cc-discovery"); + const originalClaudeHome = process.env.CLAUDE_HOME; + const fakeClaudeHome = fs.mkdtempSync(path.join(os.tmpdir(), "agenthub-fakeclaudehome-")); + process.env.CLAUDE_HOME = fakeClaudeHome; + const restore = stubClaude(WRITE_CLAUDE_JSON); + try { + await cli.marketplaceUpdate({ name: "m" }); + assert.equal(cc.getClaudeJsonPath(), path.join(fakeClaudeHome, ".claude.json")); + assert.ok(fs.existsSync(cc.getClaudeJsonPath()), "the CLI's write landed where reads look"); + } finally { + restore(); + if (originalClaudeHome === undefined) delete process.env.CLAUDE_HOME; + else process.env.CLAUDE_HOME = originalClaudeHome; + fs.rmSync(fakeClaudeHome, { recursive: true, force: true }); } }); }); diff --git a/server/lib/cc-discovery.js b/server/lib/cc-discovery.js index 02b1cd41..b3598afc 100644 --- a/server/lib/cc-discovery.js +++ b/server/lib/cc-discovery.js @@ -29,13 +29,21 @@ function getProjectClaudeDir(cwd) { } function getClaudeJsonPath() { - // One root for everything: this file used to resolve from $HOME while the - // other 19 read paths derived from getClaudeHome(), and cc-plugin-cli.js's - // runClaude() pinned CLAUDE_CONFIG_DIR to getClaudeHome() — which is where - // the CLI puts .claude.json. The dashboard read one file and made the CLI - // write another, so MCP add/remove was inert. Keep this and runClaude's - // env in step; they are the same decision seen from two sides. - return path.join(getClaudeHome(), ".claude.json"); + // NOT path.join(getClaudeHome(), ".claude.json") — verified against the + // real `claude` binary, the two CLAUDE_HOME branches behave differently: + // - CLAUDE_HOME set: the CLI's CLAUDE_CONFIG_DIR (pinned to it by + // cc-plugin-cli.js's runClaude()) becomes a $HOME-substitute for both + // `.claude.json` AND the rest of the tree (projects/, settings.json, + // plugins/) — flat inside it, no nested ".claude". Same value + // getClaudeHome() returns in this branch, so the two formulas agree. + // - CLAUDE_HOME unset: runClaude() deliberately leaves CLAUDE_CONFIG_DIR + // unset too, and the CLI's OWN default writes `.claude.json` BESIDE + // `~/.claude`, not inside it — while getClaudeHome()'s unset branch + // appends "/.claude". Delegating to getClaudeHome() here would read + // `~/.claude/.claude.json`, a file the CLI never writes by default. + return process.env.CLAUDE_HOME + ? path.join(process.env.CLAUDE_HOME, ".claude.json") + : path.join(os.homedir(), ".claude.json"); } /** diff --git a/server/openapi-extra/cc-config.js b/server/openapi-extra/cc-config.js index b66a86f8..143e8b9f 100644 --- a/server/openapi-extra/cc-config.js +++ b/server/openapi-extra/cc-config.js @@ -327,6 +327,12 @@ const schemas = { "Where the definition was found, e.g. `~/.claude.json (top-level)`, `~/.claude.json (projects[])`, or `~/.claude/settings.json`.", example: "~/.claude.json (top-level)", }, + removable: { + type: "boolean", + description: + "False when no `claude mcp` scope can address this entry (currently only ~/.claude/settings.json-sourced servers), so POST /mcp/remove has no scope that would find it. Absent/true means removable.", + example: true, + }, kind: { type: "string", enum: ["http", "stdio", "unknown"],