diff --git a/src/mcp/cli/claude.py b/src/mcp/cli/claude.py index 071b4b6fb..93bf218fb 100644 --- a/src/mcp/cli/claude.py +++ b/src/mcp/cli/claude.py @@ -33,7 +33,7 @@ def get_claude_config_path() -> Path | None: # pragma: no cover def get_uv_path() -> str: """Get the full path to the uv executable.""" uv_path = shutil.which("uv") - if not uv_path: # pragma: no cover + if not uv_path: logger.error( "uv executable not found in PATH, falling back to 'uv'. Please ensure uv is installed and in your PATH" ) @@ -65,7 +65,7 @@ def update_claude_config( """ config_dir = get_claude_config_path() uv_path = get_uv_path() - if not config_dir: # pragma: no cover + if not config_dir: raise RuntimeError( "Claude Desktop config directory not found. Please ensure Claude Desktop" " is installed and has been run at least once to initialize its config." @@ -90,7 +90,7 @@ def update_claude_config( config["mcpServers"] = {} # Always preserve existing env vars and merge with new ones - if server_name in config["mcpServers"] and "env" in config["mcpServers"][server_name]: # pragma: no cover + if server_name in config["mcpServers"] and "env" in config["mcpServers"][server_name]: existing_env = config["mcpServers"][server_name]["env"] if env_vars: # New vars take precedence over existing ones @@ -103,22 +103,26 @@ def update_claude_config( # Collect all packages in a set to deduplicate packages = {MCP_PACKAGE} - if with_packages: # pragma: no cover + if with_packages: packages.update(pkg for pkg in with_packages if pkg) # Add all packages with --with for pkg in sorted(packages): args.extend(["--with", pkg]) - if with_editable: # pragma: no cover + if with_editable: args.extend(["--with-editable", str(with_editable)]) # Convert file path to absolute before adding to command # Split off any :object suffix first - if ":" in file_spec: + # First check if we have a Windows path (e.g., C:\...) + has_windows_drive = len(file_spec) > 1 and file_spec[1] == ":" + + # Split on the last colon, but only if it's not part of the Windows drive letter + if ":" in (file_spec[2:] if has_windows_drive else file_spec): file_path, server_object = file_spec.rsplit(":", 1) file_spec = f"{Path(file_path).resolve()}:{server_object}" - else: # pragma: no cover + else: file_spec = str(Path(file_spec).resolve()) # Add mcp run command @@ -127,7 +131,7 @@ def update_claude_config( server_config: dict[str, Any] = {"command": uv_path, "args": args} # Add environment variables if specified - if env_vars: # pragma: no cover + if env_vars: server_config["env"] = env_vars config["mcpServers"][server_name] = server_config diff --git a/tests/cli/test_claude.py b/tests/cli/test_claude.py new file mode 100644 index 000000000..73d4f0eb5 --- /dev/null +++ b/tests/cli/test_claude.py @@ -0,0 +1,146 @@ +"""Tests for mcp.cli.claude — Claude Desktop config file generation.""" + +import json +from pathlib import Path +from typing import Any + +import pytest + +from mcp.cli.claude import get_uv_path, update_claude_config + + +@pytest.fixture +def config_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Temp Claude config dir with get_claude_config_path and get_uv_path mocked.""" + claude_dir = tmp_path / "Claude" + claude_dir.mkdir() + monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: claude_dir) + monkeypatch.setattr("mcp.cli.claude.get_uv_path", lambda: "/fake/bin/uv") + return claude_dir + + +def _read_server(config_dir: Path, name: str) -> dict[str, Any]: + config = json.loads((config_dir / "claude_desktop_config.json").read_text()) + return config["mcpServers"][name] + + +def test_generates_uv_run_command(config_dir: Path): + """Should write a uv run command that invokes mcp run on the resolved file spec.""" + assert update_claude_config(file_spec="server.py:app", server_name="my_server") + + resolved = Path("server.py").resolve() + assert _read_server(config_dir, "my_server") == { + "command": "/fake/bin/uv", + "args": ["run", "--frozen", "--with", "mcp[cli]", "mcp", "run", f"{resolved}:app"], + } + + +def test_file_spec_without_object_suffix(config_dir: Path): + """File specs without :object should still resolve to an absolute path.""" + assert update_claude_config(file_spec="server.py", server_name="s") + + assert _read_server(config_dir, "s")["args"][-1] == str(Path("server.py").resolve()) + + +def test_with_packages_sorted_and_deduplicated(config_dir: Path): + """Extra packages should appear as --with flags, sorted and deduplicated with mcp[cli].""" + assert update_claude_config(file_spec="s.py:app", server_name="s", with_packages=["zebra", "aardvark", "zebra"]) + + args = _read_server(config_dir, "s")["args"] + assert args[:8] == ["run", "--frozen", "--with", "aardvark", "--with", "mcp[cli]", "--with", "zebra"] + + +def test_with_editable_adds_flag(config_dir: Path, tmp_path: Path): + """with_editable should add --with-editable after the --with flags.""" + editable = tmp_path / "project" + assert update_claude_config(file_spec="s.py:app", server_name="s", with_editable=editable) + + args = _read_server(config_dir, "s")["args"] + assert args[4:6] == ["--with-editable", str(editable)] + + +def test_env_vars_written(config_dir: Path): + """env_vars should be written under the server's env key.""" + assert update_claude_config(file_spec="s.py:app", server_name="s", env_vars={"KEY": "val"}) + + assert _read_server(config_dir, "s")["env"] == {"KEY": "val"} + + +def test_existing_env_vars_merged_new_wins(config_dir: Path): + """Re-installing should merge env vars, with new values overriding existing ones.""" + (config_dir / "claude_desktop_config.json").write_text( + json.dumps({"mcpServers": {"s": {"env": {"OLD": "keep", "KEY": "old"}}}}) + ) + + assert update_claude_config(file_spec="s.py:app", server_name="s", env_vars={"KEY": "new"}) + + assert _read_server(config_dir, "s")["env"] == {"OLD": "keep", "KEY": "new"} + + +def test_existing_env_vars_preserved_without_new(config_dir: Path): + """Re-installing without env_vars should keep the existing env block intact.""" + (config_dir / "claude_desktop_config.json").write_text(json.dumps({"mcpServers": {"s": {"env": {"KEEP": "me"}}}})) + + assert update_claude_config(file_spec="s.py:app", server_name="s") + + assert _read_server(config_dir, "s")["env"] == {"KEEP": "me"} + + +def test_other_servers_preserved(config_dir: Path): + """Installing a new server should not clobber existing mcpServers entries.""" + (config_dir / "claude_desktop_config.json").write_text(json.dumps({"mcpServers": {"other": {"command": "x"}}})) + + assert update_claude_config(file_spec="s.py:app", server_name="s") + + config = json.loads((config_dir / "claude_desktop_config.json").read_text()) + assert set(config["mcpServers"]) == {"other", "s"} + assert config["mcpServers"]["other"] == {"command": "x"} + + +def test_raises_when_config_dir_missing(monkeypatch: pytest.MonkeyPatch): + """Should raise RuntimeError when Claude Desktop config dir can't be found.""" + monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: None) + monkeypatch.setattr("mcp.cli.claude.get_uv_path", lambda: "/fake/bin/uv") + + with pytest.raises(RuntimeError, match="Claude Desktop config directory not found"): + update_claude_config(file_spec="s.py:app", server_name="s") + + +@pytest.mark.parametrize("which_result, expected", [("/usr/local/bin/uv", "/usr/local/bin/uv"), (None, "uv")]) +def test_get_uv_path(monkeypatch: pytest.MonkeyPatch, which_result: str | None, expected: str): + """Should return shutil.which's result, or fall back to bare 'uv' when not on PATH.""" + + def fake_which(cmd: str) -> str | None: + return which_result + + monkeypatch.setattr("shutil.which", fake_which) + assert get_uv_path() == expected + + +@pytest.mark.parametrize( + "file_spec, expected_last_arg", + [ + ("C:\\Users\\server.py", "C:\\Users\\server.py"), + ("C:\\Users\\server.py:app", "C:\\Users\\server.py:app"), + ], +) +def test_windows_drive_letter_not_split( + config_dir: Path, monkeypatch: pytest.MonkeyPatch, file_spec: str, expected_last_arg: str +): + """Drive-letter paths like 'C:\\server.py' must not be split on the drive colon. + + Before the fix, a bare 'C:\\path\\server.py' would hit rsplit(":", 1) and yield + ("C", "\\path\\server.py"), calling resolve() on Path("C") instead of the full path. + """ + seen: list[str] = [] + + def fake_resolve(self: Path) -> Path: + seen.append(str(self)) + return self + + monkeypatch.setattr(Path, "resolve", fake_resolve) + + assert update_claude_config(file_spec=file_spec, server_name="s") + + assert seen == ["C:\\Users\\server.py"] + assert _read_server(config_dir, "s")["args"][-1] == expected_last_arg diff --git a/tests/client/test_config.py b/tests/client/test_config.py deleted file mode 100644 index d1a0576ff..000000000 --- a/tests/client/test_config.py +++ /dev/null @@ -1,75 +0,0 @@ -import json -import subprocess -from pathlib import Path -from unittest.mock import patch - -import pytest - -from mcp.cli.claude import update_claude_config - - -@pytest.fixture -def temp_config_dir(tmp_path: Path): - """Create a temporary Claude config directory.""" - config_dir = tmp_path / "Claude" - config_dir.mkdir() - return config_dir - - -@pytest.fixture -def mock_config_path(temp_config_dir: Path): - """Mock get_claude_config_path to return our temporary directory.""" - with patch("mcp.cli.claude.get_claude_config_path", return_value=temp_config_dir): - yield temp_config_dir - - -def test_command_execution(mock_config_path: Path): - """Test that the generated command can actually be executed.""" - # Setup - server_name = "test_server" - file_spec = "test_server.py:app" - - # Update config - success = update_claude_config(file_spec=file_spec, server_name=server_name) - assert success - - # Read the generated config - config_file = mock_config_path / "claude_desktop_config.json" - config = json.loads(config_file.read_text()) - - # Get the command and args - server_config = config["mcpServers"][server_name] - command = server_config["command"] - args = server_config["args"] - - test_args = [command] + args + ["--help"] - - result = subprocess.run(test_args, capture_output=True, text=True, timeout=20, check=False) - - assert result.returncode == 0 - assert "usage" in result.stdout.lower() - - -def test_absolute_uv_path(mock_config_path: Path): - """Test that the absolute path to uv is used when available.""" - # Mock the shutil.which function to return a fake path - mock_uv_path = "/usr/local/bin/uv" - - with patch("mcp.cli.claude.get_uv_path", return_value=mock_uv_path): - # Setup - server_name = "test_server" - file_spec = "test_server.py:app" - - # Update config - success = update_claude_config(file_spec=file_spec, server_name=server_name) - assert success - - # Read the generated config - config_file = mock_config_path / "claude_desktop_config.json" - config = json.loads(config_file.read_text()) - - # Verify the command is the absolute path - server_config = config["mcpServers"][server_name] - command = server_config["command"] - - assert command == mock_uv_path