From c73f136abbfe4ca9d08f3d07e364562460069f3a Mon Sep 17 00:00:00 2001 From: jepegit Date: Sat, 5 Sep 2026 21:40:45 +0200 Subject: [PATCH] Register with Cursor and VS Code too, and document all of it (#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Desktop was not a decision — it was the one client the design work happened to target, and nothing about the server is specific to it. #1 asked why, which is a fair question with no good answer. cellpy mcp install --client cursor cellpy mcp install --client vscode cellpy mcp install --list-clients ## The part that fails silently Clients differ in two ways. The file location is the obvious one. The other is the top-level key: **VS Code reads `servers`, everyone else reads `mcpServers`**. Write the wrong one and the file parses, saves, and does nothing — no error anywhere, and the user is left believing they registered. So the key is per-client data rather than a constant, and a test asserts each client gets its own. ## Claude Code is deliberately not written to Its servers live in `~/.claude.json`, next to the sign-in session and per-project trust decisions, or in a project-scoped `.mcp.json` whose location depends on which project was meant. `claude mcp add` exists and handles scopes. Merging into a file that important, to save someone one command, is a bad trade. So `config_path("claude-code")` refuses — and the refusal carries the command, filled in with this machine's interpreter, because a refusal that does not say what to do instead just moves the problem. ## Also - `status` now reports which clients it can see cellpy registered with, rather than naming one client's config path while supporting three. - README gains a "Registering with your client" section: the table of paths and keys, the `claude mcp add` line, a by-hand JSON block, and the reason to use the full interpreter path — a desktop client activates no virtualenv and inherits no shell PATH, which is the most common reason a server shows up as failed. Paths verified against the current VS Code and Cursor documentation rather than from memory. No cellpy change needed: `cellpy mcp install --client` was already passed straight through. Closes #1. Co-Authored-By: Claude Opus 5 --- README.md | 62 +++++++++++++++ src/cellpy_mcp/__init__.py | 33 ++++++-- src/cellpy_mcp/__main__.py | 20 ++++- src/cellpy_mcp/clients.py | 152 ++++++++++++++++++++++++++++++------- tests/test_clients.py | 87 +++++++++++++++++++++ 5 files changed, 321 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 3ab59ef..58aac9b 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,68 @@ Batch templating: Prompts: `analyse_cell`, `start_batch_project`, `explain_call`. +## Registering with your client + +`cellpy mcp install` writes the block for you. Pass `--client` for anything +other than Claude Desktop: + +```bash +cellpy mcp install # Claude Desktop (default) +cellpy mcp install --client cursor +cellpy mcp install --client vscode +cellpy mcp install --list-clients # where each one keeps its config +``` + +Add `--dry-run` to see the target first. Restart the client afterwards — none +of them re-read the file while running. + +| Client | File it writes | Key | +|---|---|---| +| Claude Desktop | `%APPDATA%/Claude/claude_desktop_config.json` · `~/Library/Application Support/Claude/…` · `~/.config/Claude/…` | `mcpServers` | +| Cursor | `~/.cursor/mcp.json` — global; a project's `.cursor/mcp.json` wins over it | `mcpServers` | +| VS Code | `%APPDATA%/Code/User/mcp.json` · `~/Library/Application Support/Code/User/mcp.json` · `~/.config/Code/User/mcp.json` | `servers` | + +**VS Code names the key `servers`, not `mcpServers`.** If you edit that file by +hand, this is the mistake to avoid: the wrong key parses, saves, and does +nothing at all. + +### Claude Code + +Claude Code is not registered by editing a file — its servers live in +`~/.claude.json` alongside your sign-in session and per-project trust +decisions, or in a project-scoped `.mcp.json`. It has a command that handles +scopes properly, so use that: + +```bash +claude mcp add cellpy --env CELLPY_MCP_ROOT=/path/to/cells -- python -m cellpy_mcp +``` + +`cellpy mcp install --list-clients` prints that line filled in with the +interpreter and the roots for your machine. + +### By hand + +Any client that speaks stdio can run this; it is an ordinary MCP server. The +block is the same everywhere except the top-level key: + +```json +{ + "mcpServers": { + "cellpy": { + "command": "/path/to/python", + "args": ["-m", "cellpy_mcp"], + "env": { "CELLPY_MCP_ROOT": "/path/to/cells" } + } + } +} +``` + +Use the **full path to the interpreter that has cellpy-mcp installed**, not a +bare `python`: a desktop client activates no virtualenv and inherits no shell +PATH, which is the most common reason a server shows up as failed. + +`cellpy mcp status` says which clients it can see cellpy registered with. + ## Where it may read and write Everything is confined to a set of roots, and both reads and writes are checked diff --git a/src/cellpy_mcp/__init__.py b/src/cellpy_mcp/__init__.py index 1041db7..2d0919d 100644 --- a/src/cellpy_mcp/__init__.py +++ b/src/cellpy_mcp/__init__.py @@ -59,19 +59,40 @@ def install( def describe() -> dict: """What `cellpy mcp status` reports beyond the two version numbers.""" - from .clients import config_path + from .clients import CLIENTS, config_path from .sandbox import Sandbox sandbox = Sandbox.from_environment() described = {"roots": ", ".join(str(root) for root in sandbox.roots)} - try: - target = config_path() - except ValueError: # pragma: no cover - only if CLIENTS shrinks - return described - described["client config"] = f"{target}{'' if target.exists() else ' (not present)'}" + + # Every client this can write to, and whether cellpy is already in it — + # "which client did I register with?" is the question `status` exists for, + # and answering it for one client while supporting three would mislead. + registered = [] + for name in sorted(CLIENTS): + try: + target = config_path(name) + except ValueError: # pragma: no cover - only if CLIENTS changes shape + continue + if _has_cellpy(target, CLIENTS[name].key): + registered.append(name) + described["registered with"] = ", ".join(registered) if registered else "nothing yet" return described +def _has_cellpy(target, key: str) -> bool: + """True when `target` already names a `cellpy` server under `key`.""" + import json + + try: + config = json.loads(target.read_text(encoding="utf-8") or "{}") + except (OSError, json.JSONDecodeError): + # Unreadable or malformed is not this function's problem to report; + # `install` says so properly when it is asked to write. + return False + return isinstance(config, dict) and "cellpy" in (config.get(key) or {}) + + def __getattr__(name: str): # `Sandbox` is re-exported for callers who want to build a server with an # explicit set of roots, but importing it eagerly would drag cellpy's diff --git a/src/cellpy_mcp/__main__.py b/src/cellpy_mcp/__main__.py index 6ef41cc..f7e60fb 100644 --- a/src/cellpy_mcp/__main__.py +++ b/src/cellpy_mcp/__main__.py @@ -23,7 +23,15 @@ def main(argv: list[str] | None = None) -> int: install = subcommands.add_parser("install", help="register with a chat client") install.add_argument("--root", help="a directory the server may read and write") - install.add_argument("--client", help="which chat client to register with") + install.add_argument( + "--client", + help="which client to register with (default: claude-desktop)", + ) + install.add_argument( + "--list-clients", + action="store_true", + help="list the clients this knows about and where each keeps its config", + ) install.add_argument( "--dry-run", action="store_true", help="print the target instead of writing" ) @@ -41,6 +49,16 @@ def main(argv: list[str] | None = None) -> int: return 0 if args.command == "install": + if args.list_clients: + from .clients import CLIENTS, MANUAL, command_for, config_path + + for name, spec in sorted(CLIENTS.items()): + print(f"{name:<16} {config_path(name)}") + if spec.note: + print(f"{'':<16} ({spec.note})") + for name in sorted(MANUAL): + print(f"{name:<16} run: {command_for(name)}") + return 0 try: target = do_install(root=args.root, client=args.client, dry_run=args.dry_run) except ValueError as exc: diff --git a/src/cellpy_mcp/clients.py b/src/cellpy_mcp/clients.py index 883bdc6..8543e66 100644 --- a/src/cellpy_mcp/clients.py +++ b/src/cellpy_mcp/clients.py @@ -1,4 +1,4 @@ -"""Registering the server with a chat client. +"""Registering the server with an MCP client. Under stdio there is nothing to host: the client spawns the server as a subprocess and talks to it over stdin/stdout. That is what makes this usable @@ -8,42 +8,128 @@ So: write the block for them. Carefully, because it is someone else's configuration file and it has their other servers in it. + +**Clients differ in two ways that matter**, and both fail silently if you get +them wrong. The file location is the obvious one. The less obvious one is the +top-level key: VS Code reads `servers`, everyone else reads `mcpServers`. Write +the wrong key into VS Code's file and it parses, saves, and does nothing — +which is why `KEY` is per-client data here rather than a constant. + +**Claude Code is deliberately not written to.** Its servers live in +`~/.claude.json` alongside the sign-in session and per-project trust decisions, +or in a project-scoped `.mcp.json` whose location depends on which project you +meant. `claude mcp add` exists, handles scopes, and is the supported path; +`command_for` returns it so callers can show it rather than guess at a file +that important. """ from __future__ import annotations import json import os +import shlex import sys +from dataclasses import dataclass from pathlib import Path -__all__ = ["CLIENTS", "config_path", "install"] +__all__ = ["CLIENTS", "DEFAULT_CLIENT", "command_for", "config_path", "install", "server_entry"] + + +@dataclass(frozen=True) +class Client: + """How one client wants to be told about a server.""" + + name: str + label: str + #: Top-level key holding the server map. VS Code is the odd one out. + key: str + #: `(windows, macos, linux)` paths, relative to the right base directory. + windows: tuple[str, ...] + macos: tuple[str, ...] + linux: tuple[str, ...] + note: str = "" + + +_CLIENTS = ( + Client( + name="claude-desktop", + label="Claude Desktop", + key="mcpServers", + windows=("%APPDATA%", "Claude", "claude_desktop_config.json"), + macos=("~", "Library", "Application Support", "Claude", "claude_desktop_config.json"), + linux=("~", ".config", "Claude", "claude_desktop_config.json"), + ), + Client( + name="cursor", + label="Cursor", + key="mcpServers", + # Cursor also reads a project-scoped `.cursor/mcp.json`, which takes + # priority. This writes the global one, since "my cells" is not a + # property of whichever repository happens to be open. + windows=("~", ".cursor", "mcp.json"), + macos=("~", ".cursor", "mcp.json"), + linux=("~", ".cursor", "mcp.json"), + note="global; a project's .cursor/mcp.json overrides it", + ), + Client( + name="vscode", + label="VS Code", + key="servers", + windows=("%APPDATA%", "Code", "User", "mcp.json"), + macos=("~", "Library", "Application Support", "Code", "User", "mcp.json"), + linux=("~", ".config", "Code", "User", "mcp.json"), + note="user profile; VS Code names the key 'servers', not 'mcpServers'", + ), +) + +CLIENTS = {client.name: client for client in _CLIENTS} +DEFAULT_CLIENT = "claude-desktop" -#: Clients we know where to find. The value is the per-platform location of the -#: file holding the `mcpServers` map. -CLIENTS = ("claude-desktop",) +#: Clients we know about but will not write to, and what to do instead. +MANUAL = { + "claude-code": ( + "Claude Code keeps MCP servers in ~/.claude.json, next to your sign-in " + "session and per-project trust decisions, or in a project-scoped " + ".mcp.json. Use its own command instead, which handles scopes:" + ) +} -DEFAULT_CLIENT = "claude-desktop" + +def _resolve(parts: tuple[str, ...]) -> Path: + """Turn a path template into a real path. + + `%APPDATA%` is expanded from the environment with a documented fallback, + rather than assumed: a roaming profile can put it somewhere else, and this + writes a file. + """ + first, *rest = parts + if first == "%APPDATA%": + base = Path(os.environ.get("APPDATA") or Path.home() / "AppData" / "Roaming") + elif first == "~": + base = Path.home() + else: + base = Path(first) + return base.joinpath(*rest).expanduser() -def config_path(client: str = DEFAULT_CLIENT) -> Path: +def config_path(client: str | None = None) -> Path: """Where `client` keeps its MCP server list on this platform.""" - if client != "claude-desktop": - known = ", ".join(CLIENTS) - raise ValueError(f"Unknown client {client!r}. Known clients: {known}.") + name = client or DEFAULT_CLIENT + if name in MANUAL: + raise ValueError( + f"{name} is not registered by editing a file — run this instead:\n" + f" {command_for(name)}" + ) + if name not in CLIENTS: + known = ", ".join(sorted([*CLIENTS, *MANUAL])) + raise ValueError(f"Unknown client {name!r}. Known clients: {known}.") + spec = CLIENTS[name] if sys.platform == "win32": - base = Path(os.environ.get("APPDATA") or Path.home() / "AppData/Roaming") - return base / "Claude" / "claude_desktop_config.json" + return _resolve(spec.windows) if sys.platform == "darwin": - return ( - Path.home() - / "Library" - / "Application Support" - / "Claude" - / "claude_desktop_config.json" - ) - return Path.home() / ".config" / "Claude" / "claude_desktop_config.json" + return _resolve(spec.macos) + return _resolve(spec.linux) def server_entry(roots: list[Path]) -> dict: @@ -61,6 +147,18 @@ def server_entry(roots: list[Path]) -> dict: } +def command_for(client: str, roots: list[Path] | None = None) -> str: + """The command to run for a client this cannot register by editing a file.""" + if client != "claude-code": + raise ValueError(f"No command form for {client!r}.") + root = os.pathsep.join(str(r) for r in (roots or [])) + parts = ["claude", "mcp", "add", "cellpy"] + if root: + parts += ["--env", f"CELLPY_MCP_ROOT={root}"] + parts += ["--", sys.executable, "-m", "cellpy_mcp"] + return " ".join(shlex.quote(part) if " " in part else part for part in parts) + + def install( roots: list[Path], client: str | None = None, @@ -71,15 +169,17 @@ def install( Returns the path written (or that would be written). Raises `ValueError` with something worth reading when it cannot. - Merges rather than writes: the file holds the caller's other servers, and - a tool that replaces it to add one line is a tool that loses their work. + Merges rather than writes: the file holds the caller's other servers, and a + tool that replaces it to add one line is a tool that loses their work. """ - target = config_path(client or DEFAULT_CLIENT) + name = client or DEFAULT_CLIENT + target = config_path(name) + key = CLIENTS[name].key config: dict = {} if target.exists(): try: - existing = json.loads(target.read_text(encoding="utf-8")) + existing = json.loads(target.read_text(encoding="utf-8") or "{}") except json.JSONDecodeError as exc: # Refusing is the whole point. Overwriting an unparseable config # would throw away a file we cannot even read to report. @@ -91,9 +191,9 @@ def install( raise ValueError(f"{target} does not contain a JSON object.") config = existing - servers = config.setdefault("mcpServers", {}) + servers = config.setdefault(key, {}) if not isinstance(servers, dict): - raise ValueError(f"{target} has an 'mcpServers' that is not an object.") + raise ValueError(f"{target} has a {key!r} that is not an object.") servers["cellpy"] = server_entry(roots) if dry_run: diff --git a/tests/test_clients.py b/tests/test_clients.py index b31a97b..8507088 100644 --- a/tests/test_clients.py +++ b/tests/test_clients.py @@ -97,3 +97,90 @@ def test_several_roots_travel_as_one_environment_variable(tmp_path): entry = clients.server_entry([tmp_path / "a", tmp_path / "b"]) assert entry["env"]["CELLPY_MCP_ROOT"].count(os.pathsep) == 1 + + +# -- more than one client (#1) --------------------------------------------------- + + +@pytest.mark.parametrize("client", sorted(clients.CLIENTS)) +def test_every_client_resolves_to_a_plausible_path(client): + """A path that is relative, or lands on `/`, would write somewhere strange.""" + path = clients.config_path(client) + assert path.is_absolute() + assert path.suffix == ".json" + assert path.parent != path.anchor + + +def test_vscode_uses_its_own_key(tmp_path, monkeypatch): + """The trap that fails silently. + + VS Code reads `servers`; everyone else reads `mcpServers`. Writing the + wrong one produces a file that parses, saves, and does nothing — no error + anywhere, and the user is left believing they registered. + """ + target = tmp_path / "vscode" / "mcp.json" + monkeypatch.setattr(clients, "config_path", lambda client=None: target) + + clients.install([tmp_path], client="vscode") + config = json.loads(target.read_text(encoding="utf-8")) + + assert "servers" in config + assert "cellpy" in config["servers"] + assert "mcpServers" not in config + + +def test_the_others_use_mcpservers(tmp_path, monkeypatch): + for client in ("claude-desktop", "cursor"): + target = tmp_path / client / "config.json" + monkeypatch.setattr(clients, "config_path", lambda client=None, t=target: t) + clients.install([tmp_path], client=client) + config = json.loads(target.read_text(encoding="utf-8")) + assert "cellpy" in config["mcpServers"], client + assert "servers" not in config, client + + +def test_an_existing_vscode_file_keeps_its_other_servers(tmp_path, monkeypatch): + target = tmp_path / "mcp.json" + target.write_text( + json.dumps({"servers": {"playwright": {"command": "npx"}}, "inputs": []}), + encoding="utf-8", + ) + monkeypatch.setattr(clients, "config_path", lambda client=None: target) + + clients.install([tmp_path], client="vscode") + config = json.loads(target.read_text(encoding="utf-8")) + + assert config["servers"]["playwright"] == {"command": "npx"} + assert config["inputs"] == [] + assert "cellpy" in config["servers"] + + +def test_claude_code_is_not_written_to_but_is_answered(tmp_path): + """Its servers share a file with the sign-in session and trust decisions. + + Refusing to edit that is the point — but a refusal that does not say what + to do instead just moves the problem, so the message carries the command. + """ + with pytest.raises(ValueError) as raised: + clients.config_path("claude-code") + + message = str(raised.value) + assert "claude mcp add" in message + + command = clients.command_for("claude-code", [tmp_path]) + assert command.startswith("claude mcp add cellpy") + assert "CELLPY_MCP_ROOT=" in command + assert "-m cellpy_mcp" in command + + +def test_installing_into_one_client_does_not_touch_another(tmp_path, monkeypatch): + """Each client has its own file; registering with Cursor is not a VS Code edit.""" + cursor = tmp_path / "cursor.json" + vscode = tmp_path / "vscode.json" + paths = {"cursor": cursor, "vscode": vscode} + monkeypatch.setattr(clients, "config_path", lambda client=None: paths[client]) + + clients.install([tmp_path], client="cursor") + + assert cursor.exists() + assert not vscode.exists()