diff --git a/cloudsmith_cli/cli/commands/mcp.py b/cloudsmith_cli/cli/commands/mcp.py index 1cfd7e2b..9a3f8041 100644 --- a/cloudsmith_cli/cli/commands/mcp.py +++ b/cloudsmith_cli/cli/commands/mcp.py @@ -4,6 +4,7 @@ import os import shutil import sys +import tempfile from pathlib import Path import click @@ -16,6 +17,7 @@ SUPPORTED_MCP_CLIENTS = { "claude": "Claude Desktop", + "claude-code": "Claude Code", "cursor": "Cursor IDE", "vscode": "VS Code", "gemini-cli": "Gemini CLI", @@ -196,12 +198,17 @@ def configure(ctx, opts, client, is_global): # pylint: disable=unused-argument This command automatically adds the Cloudsmith MCP server configuration to the specified client's configuration file. Supported clients are: - Claude Desktop + - Claude Code - Cursor IDE - VS Code (GitHub Copilot) - Gemini CLI + For Claude Code, --global edits ~/.claude.json (user scope) and + --local writes ./.mcp.json (project scope, intended to be committed). + Examples:\n cloudsmith mcp configure --client claude\n + cloudsmith mcp configure --client claude-code\n cloudsmith mcp configure --client cursor --local\n cloudsmith mcp configure --client gemini-cli\n cloudsmith mcp configure # Auto-detect and configure all @@ -315,10 +322,16 @@ def _get_server_config(profile=None): def detect_available_clients(): """Detect which MCP clients are available on the system.""" available = [] + home = Path.home() for client in SUPPORTED_MCP_CLIENTS: config = get_config_path(client, is_global=True) - if config and config.parent.exists(): + if not config: + continue + # Parent-dir existence is the usual "app installed" marker, but a + # parent of $HOME (Claude Code) tells us nothing; require the file + # itself in that case. + if config.exists() or (config.parent.exists() and config.parent != home): available.append(client) return available @@ -344,6 +357,10 @@ def get_config_path(client_name, is_global=True): ), "linux": home / ".config" / "Claude" / "claude_desktop_config.json", }, + "claude-code": { + "global": home / ".claude.json", + "local": Path.cwd() / ".mcp.json", + }, "cursor": { "global": home / ".cursor" / "mcp.json", "local": Path.cwd() / ".cursor" / "mcp.json", @@ -369,8 +386,8 @@ def get_config_path(client_name, is_global=True): client_config = config_paths.get(client_name, {}) - # For Cursor and Gemini CLI, use global/local scope instead of platform - if client_name in ("cursor", "gemini-cli"): + # For scope-keyed (not platform-keyed) clients, look up by global/local. + if client_name in ("claude-code", "cursor", "gemini-cli"): scope = "global" if is_global else "local" return client_config.get(scope) @@ -385,47 +402,119 @@ def get_config_path(client_name, is_global=True): def configure_client(client_name, server_config, is_global=True, profile=None): """Configure a specific MCP client with the Cloudsmith server.""" - config_path = get_config_path(client_name, is_global) + server_name = f"cloudsmith-{profile}" if profile else "cloudsmith" + + if client_name == "claude-code": + return _configure_claude_code(server_name, server_config, is_global) + config_path = get_config_path(client_name, is_global) if not config_path: return False - # Ensure parent directory exists - config_path.parent.mkdir(parents=True, exist_ok=True) + key = "chat.mcp.servers" if client_name == "vscode" else "mcpServers" - # Read existing config or create new one - config = {} - if config_path.exists(): - with open(config_path) as f: - content = f.read() - try: - # Use json5 first as it handles both standard JSON and JSONC - # (comments, trailing commas) which VS Code settings.json may contain - config = json5.loads(content) - except (json.JSONDecodeError, ValueError) as e: - raise ValueError( - f"Cannot parse config file '{config_path}': {e}. " - f"Please fix the JSON syntax or remove the file to create a new one." - ) from e - - # Determine server name based on profile - server_name = f"cloudsmith-{profile}" if profile else "cloudsmith" + def mutate(config): + config.setdefault(key, {})[server_name] = server_config + + _safe_update_json(config_path, mutate) + return True - # Add Cloudsmith MCP server based on client format - if client_name in {"claude", "cursor", "gemini-cli"}: - if "mcpServers" not in config: - config["mcpServers"] = {} - config["mcpServers"][server_name] = server_config - elif client_name == "vscode": - # VS Code uses a different format in settings.json - if "chat.mcp.servers" not in config: - config["chat.mcp.servers"] = {} - config["chat.mcp.servers"][server_name] = server_config +def _configure_claude_code(server_name, server_config, is_global): + """Register the Cloudsmith MCP server with Claude Code. + + Why direct edit (not `claude mcp add-json`): avoids requiring the Claude + Code CLI on PATH. _safe_update_json handles the race with a running + Claude Code session writing to ~/.claude.json. + """ + path = get_config_path("claude-code", is_global=is_global) + if is_global and not path.exists(): + raise ValueError( + f"{path} not found. Launch Claude Code at least once, " + "then re-run this command." + ) - # Write updated config - with open(config_path, "w") as f: - # As we are using json5 to read, we write standard JSON back, which removes existing user JSON comments in the files (currently only present in VS Code settings.json). - json.dump(config, f, indent=2) + def mutate(config): + config.setdefault("mcpServers", {})[server_name] = server_config + _safe_update_json(path, mutate) return True + + +def _atomic_write_json(path: Path, data) -> None: + """Write JSON to ``path`` atomically via a tempfile + os.replace. + + Preserves the destination's existing file mode when present. + + Follows symlinks before writing so dotfile-managed configs (a symlinked + ~/.claude.json, settings.json, etc.) update through to the real file + rather than getting the link replaced. + """ + if path.is_symlink(): + path = Path(os.path.realpath(path)) + + path.parent.mkdir(parents=True, exist_ok=True) + existing_mode = path.stat().st_mode & 0o777 if path.exists() else None + + tmp = tempfile.NamedTemporaryFile( + mode="w", + dir=path.parent, + prefix=f".{path.name}.", + suffix=".tmp", + delete=False, + ) + tmp_path = Path(tmp.name) + try: + with tmp as f: + # json5 is used for reading; we write standard JSON, which drops + # any user comments (currently only relevant for VS Code's JSONC). + json.dump(data, f, indent=2) + f.flush() + os.fsync(f.fileno()) + if existing_mode is not None: + os.chmod(tmp_path, existing_mode) + os.replace(tmp_path, path) + except BaseException: + try: + tmp_path.unlink() + except FileNotFoundError: + pass + raise + + +def _safe_update_json(path: Path, mutate, *, max_retries: int = 3) -> None: + """Read JSON at ``path``, apply ``mutate(dict)``, atomic-write back. + + Retries on mtime change between read and replace -- guards against a + concurrent writer (e.g. a running Claude Code session updating + ~/.claude.json, or VS Code editing settings.json) clobbering deltas. + """ + for _ in range(max_retries): + if path.exists(): + mtime_before = path.stat().st_mtime_ns + with open(path) as f: + content = f.read() + try: + config = json5.loads(content) + except (json.JSONDecodeError, ValueError) as e: + raise ValueError( + f"Cannot parse config file '{path}': {e}. " + "Please fix the JSON syntax or remove the file to " + "create a new one." + ) from e + else: + mtime_before = None + config = {} + + mutate(config) + + if mtime_before is not None and path.stat().st_mtime_ns != mtime_before: + continue + + _atomic_write_json(path, config) + return + + raise ValueError( + f"Could not safely update {path}: another process keeps modifying " + "it. Close the consuming app and retry." + ) diff --git a/cloudsmith_cli/cli/tests/commands/test_mcp.py b/cloudsmith_cli/cli/tests/commands/test_mcp.py index c85256dc..c54a1e60 100644 --- a/cloudsmith_cli/cli/tests/commands/test_mcp.py +++ b/cloudsmith_cli/cli/tests/commands/test_mcp.py @@ -1,8 +1,21 @@ +import json +import os +import stat +from pathlib import Path from unittest.mock import patch import cloudsmith_api - -from ....cli.commands.mcp import list_groups, list_tools +import pytest + +from ....cli.commands.mcp import ( + _atomic_write_json, + _configure_claude_code, + _safe_update_json, + configure_client, + detect_available_clients, + list_groups, + list_tools, +) from ....core.mcp.data import OpenAPITool from ....core.mcp.server import DynamicMCPServer @@ -355,3 +368,214 @@ def test_server_respects_tool_filtering(self): assert len(server.tools) == 1 assert "repos_list" in server.tools assert "packages_list" not in server.tools + + +SERVER_CONFIG = {"command": "cloudsmith", "args": ["mcp", "start"]} + + +class TestMCPConfigureClaudeCode: + def test_user_scope_merges_into_existing_claude_json(self, tmp_path): + claude_json = tmp_path / ".claude.json" + existing = { + "numStartups": 42, + "mcpServers": {"other": {"command": "other-mcp"}}, + "projects": {"/some/path": {"lastSessionId": "abc"}}, + } + claude_json.write_text(json.dumps(existing, indent=2)) + + with patch("cloudsmith_cli.cli.commands.mcp.Path.home", return_value=tmp_path): + assert _configure_claude_code("cloudsmith", SERVER_CONFIG, is_global=True) + + result = json.loads(claude_json.read_text()) + assert result["mcpServers"]["other"] == {"command": "other-mcp"} + assert result["mcpServers"]["cloudsmith"] == SERVER_CONFIG + assert result["numStartups"] == 42 + assert result["projects"] == existing["projects"] + + def test_user_scope_errors_when_claude_json_missing(self, tmp_path): + with patch("cloudsmith_cli.cli.commands.mcp.Path.home", return_value=tmp_path): + with pytest.raises(ValueError, match="Launch Claude Code at least once"): + _configure_claude_code("cloudsmith", SERVER_CONFIG, is_global=True) + + def test_project_scope_writes_local_mcp_json(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + assert _configure_claude_code("cloudsmith", SERVER_CONFIG, is_global=False) + + local = tmp_path / ".mcp.json" + assert local.exists() + assert json.loads(local.read_text()) == { + "mcpServers": {"cloudsmith": SERVER_CONFIG} + } + + def test_project_scope_merges_into_existing_mcp_json(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + existing = {"mcpServers": {"other": {"command": "other-mcp"}}} + (tmp_path / ".mcp.json").write_text(json.dumps(existing)) + + _configure_claude_code("cloudsmith", SERVER_CONFIG, is_global=False) + + result = json.loads((tmp_path / ".mcp.json").read_text()) + assert result["mcpServers"]["other"] == {"command": "other-mcp"} + assert result["mcpServers"]["cloudsmith"] == SERVER_CONFIG + + def test_configure_client_routes_claude_code_with_profile(self, tmp_path): + (tmp_path / ".claude.json").write_text("{}") + with patch("cloudsmith_cli.cli.commands.mcp.Path.home", return_value=tmp_path): + configure_client( + "claude-code", SERVER_CONFIG, is_global=True, profile="staging" + ) + + result = json.loads((tmp_path / ".claude.json").read_text()) + assert "cloudsmith-staging" in result["mcpServers"] + + def test_detect_available_clients_respects_claude_json_presence(self, tmp_path): + # Real get_config_path for claude-code (resolves under the patched + # home); None for everything else so this host's real configs don't + # leak into the result. + import cloudsmith_cli.cli.commands.mcp as mcp_module + + real_get_config_path = mcp_module.get_config_path + + def selective(client, is_global=True): + if client == "claude-code": + return real_get_config_path(client, is_global=is_global) + return None + + with patch( + "cloudsmith_cli.cli.commands.mcp.Path.home", return_value=tmp_path + ), patch( + "cloudsmith_cli.cli.commands.mcp.get_config_path", side_effect=selective + ): + assert "claude-code" not in detect_available_clients() + + (tmp_path / ".claude.json").write_text("{}") + assert "claude-code" in detect_available_clients() + + +class TestSafeWriteHelpers: + def test_atomic_write_preserves_existing_file_mode(self, tmp_path): + target = tmp_path / "config.json" + target.write_text("{}") + os.chmod(target, 0o600) + + _atomic_write_json(target, {"k": "v"}) + + assert json.loads(target.read_text()) == {"k": "v"} + assert stat.S_IMODE(target.stat().st_mode) == 0o600 + + def test_atomic_write_creates_parent_directory(self, tmp_path): + target = tmp_path / "nested" / "deeper" / "config.json" + _atomic_write_json(target, {"k": "v"}) + assert json.loads(target.read_text()) == {"k": "v"} + + def test_atomic_write_cleans_up_tempfile_on_failure(self, tmp_path): + target = tmp_path / "config.json" + with patch( + "cloudsmith_cli.cli.commands.mcp.os.replace", + side_effect=OSError("boom"), + ): + with pytest.raises(OSError, match="boom"): + _atomic_write_json(target, {"k": "v"}) + + leftovers = [ + p for p in tmp_path.iterdir() if p.name.startswith(".config.json.") + ] + assert leftovers == [] + + def test_safe_update_creates_file_when_missing(self, tmp_path): + target = tmp_path / "config.json" + _safe_update_json(target, lambda c: c.setdefault("a", []).append(1)) + assert json.loads(target.read_text()) == {"a": [1]} + + def test_atomic_write_follows_symlinks(self, tmp_path): + real = tmp_path / "real" / "config.json" + real.parent.mkdir() + real.write_text(json.dumps({"existing": True})) + link = tmp_path / "config.json" + link.symlink_to(real) + + _atomic_write_json(link, {"k": "v"}) + + assert link.is_symlink() + assert link.readlink() == real + assert json.loads(real.read_text()) == {"k": "v"} + + def test_safe_update_retries_on_concurrent_write(self, tmp_path): + target = tmp_path / "config.json" + target.write_text(json.dumps({"counter": 0})) + + calls = {"n": 0} + + def mutate(config): + config["counter"] = config.get("counter", 0) + 1 + if calls["n"] == 0: + # Simulate a concurrent writer landing between our read and + # our pre-write mtime check. Bumping mtime explicitly so the + # change is visible even on filesystems with coarse mtime. + target.write_text(json.dumps({"counter": 99})) + before = target.stat() + os.utime( + target, ns=(before.st_atime_ns, before.st_mtime_ns + 1_000_000) + ) + calls["n"] += 1 + + _safe_update_json(target, mutate) + + # First attempt reads 0, racing writer lands 99, retry reads 99 and + # increments to 100. + assert json.loads(target.read_text())["counter"] == 100 + + def test_safe_update_raises_after_max_retries(self, tmp_path): + target = tmp_path / "config.json" + target.write_text("{}") + + original_stat = Path.stat + + def always_racing(self, *args, **kwargs): + result = original_stat(self, *args, **kwargs) + if str(self) == str(target): + # Bump the file on disk every time we observe it, so the + # mtime check always fails. + os.utime(target, ns=(result.st_atime_ns, result.st_mtime_ns + 1000)) + return result + + with patch.object(Path, "stat", always_racing): + with pytest.raises(ValueError, match="another process keeps modifying"): + _safe_update_json(target, lambda c: c, max_retries=2) + + def test_safe_update_raises_on_malformed_json(self, tmp_path): + target = tmp_path / "config.json" + target.write_text("{not valid json}") + + with pytest.raises(ValueError, match="Cannot parse config file"): + _safe_update_json(target, lambda c: c) + + +class TestConfigureClientSafeWrite: + """Other clients should now use the atomic safe-update path.""" + + def test_writes_atomically_for_claude_desktop(self, tmp_path): + config_path = tmp_path / "claude_desktop_config.json" + with patch( + "cloudsmith_cli.cli.commands.mcp.get_config_path", + return_value=config_path, + ): + configure_client("claude", SERVER_CONFIG, is_global=True) + + assert json.loads(config_path.read_text()) == { + "mcpServers": {"cloudsmith": SERVER_CONFIG} + } + + def test_writes_vscode_key_for_vscode_client(self, tmp_path): + config_path = tmp_path / "settings.json" + config_path.write_text(json.dumps({"editor.fontSize": 14})) + + with patch( + "cloudsmith_cli.cli.commands.mcp.get_config_path", + return_value=config_path, + ): + configure_client("vscode", SERVER_CONFIG, is_global=True) + + result = json.loads(config_path.read_text()) + assert result["editor.fontSize"] == 14 + assert result["chat.mcp.servers"]["cloudsmith"] == SERVER_CONFIG