diff --git a/README.md b/README.md index c8925449..a56fdcdf 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,8 @@ mcpm share "npx -y @modelcontextprotocol/server-everything" --retry 3 ```bash mcpm config clear-cache # Clear MCPM's registry cache. Cache defaults to refresh every 1 hour. +mcpm config set # Set global MCPM configuration, currently only support node_executable +mcpm config get # Get global MCPM configuration mcpm inspector # Launch the MCPM Inspector UI to examine server configs ``` diff --git a/README.zh-CN.md b/README.zh-CN.md index 11f7a64d..ca23ca62 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -208,6 +208,8 @@ mcpm share "npx -y @modelcontextprotocol/server-everything" --retry 3 ```bash mcpm config clear-cache # 清除 MCPM 的注册表缓存。缓存默认每 1 小时刷新一次。 +mcpm config set # 设置 MCPM 的全局配置,目前仅支持 node_executable +mcpm config get # 获取 MCPM 的全局配置 mcpm inspector # 启动 MCPM 检查器 UI 以检查服务器配置 ``` diff --git a/src/mcpm/commands/config.py b/src/mcpm/commands/config.py index bf807817..4d407d76 100644 --- a/src/mcpm/commands/config.py +++ b/src/mcpm/commands/config.py @@ -4,7 +4,9 @@ import click from rich.console import Console +from rich.prompt import Prompt +from mcpm.utils.config import NODE_EXECUTABLES, ConfigManager from mcpm.utils.repository import RepositoryManager console = Console() @@ -21,6 +23,45 @@ def config(): pass +@config.command() +@click.help_option("-h", "--help") +def set(): + """Set MCPM configuration. + + Example: + + \b + mcpm config set + """ + set_key = Prompt.ask("Configuration key to set", choices=["node_executable"], default="node_executable") + node_executable = Prompt.ask( + "Select default node executable, it will be automatically applied when adding npx server with mcpm add", + choices=NODE_EXECUTABLES, + ) + config_manager = ConfigManager() + config_manager.set_config(set_key, node_executable) + console.print(f"[green]Default node executable set to:[/] {node_executable}") + + +@config.command() +@click.argument("name", required=True) +@click.help_option("-h", "--help") +def get(name): + """Get MCPM configuration. + + Example: + + \b + mcpm config get node_executable + """ + config_manager = ConfigManager() + current_config = config_manager.get_config() + if name not in current_config: + console.print(f"[red]Configuration '{name}' not set or not supported.[/]") + return + console.print(f"[green]{name}:[/] {current_config[name]}") + + @config.command() @click.help_option("-h", "--help") def clear_cache(): diff --git a/src/mcpm/commands/target_operations/common.py b/src/mcpm/commands/target_operations/common.py index d4d78f3c..eaa3f71a 100644 --- a/src/mcpm/commands/target_operations/common.py +++ b/src/mcpm/commands/target_operations/common.py @@ -1,9 +1,9 @@ from rich.console import Console from mcpm.clients.client_registry import ClientRegistry -from mcpm.core.schema import ServerConfig +from mcpm.core.schema import ServerConfig, STDIOServerConfig from mcpm.profile.profile_config import ProfileConfigManager -from mcpm.utils.config import ConfigManager +from mcpm.utils.config import NODE_EXECUTABLES, ConfigManager from mcpm.utils.display import print_active_scope, print_no_active_scope from mcpm.utils.scope import ScopeType, extract_from_scope, parse_server @@ -32,6 +32,22 @@ def determine_target(target: str) -> tuple[ScopeType | None, str | None, str | N return scope_type, scope, server_name +def _replace_node_executable(server_config: ServerConfig) -> ServerConfig: + if not isinstance(server_config, STDIOServerConfig): + return server_config + command = server_config.command.strip() + if command not in NODE_EXECUTABLES: + return server_config + config = ConfigManager().get_config() + config_node_executable = config.get("node_executable") + if not config_node_executable: + return server_config + if config_node_executable != command: + console.print(f"[bold cyan]Replace node executable {command} with {config_node_executable}[/]") + server_config.command = config_node_executable + return server_config + + def client_add_server(client: str, server_config: ServerConfig, force: bool = False) -> bool: client_manager = ClientRegistry.get_client_manager(client) if not client_manager: @@ -41,6 +57,7 @@ def client_add_server(client: str, server_config: ServerConfig, force: bool = Fa console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in {client}.") console.print("Use --force to override.") return False + server_config = _replace_node_executable(server_config) success = client_manager.add_server(server_config) return success @@ -73,6 +90,7 @@ def profile_add_server(profile: str, server_config: ServerConfig, force: bool = console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in {profile}.") console.print("Use --force to override.") return False + server_config = _replace_node_executable(server_config) success = profile_manager.set_profile(profile, server_config) return success diff --git a/src/mcpm/utils/config.py b/src/mcpm/utils/config.py index 0f57aaca..15e19b52 100644 --- a/src/mcpm/utils/config.py +++ b/src/mcpm/utils/config.py @@ -24,6 +24,8 @@ MCPM_AUTH_HEADER = "X-MCPM-SECRET" MCPM_PROFILE_HEADER = "X-MCPM-PROFILE" +NODE_EXECUTABLES = ["npx", "bunx", "pnpm dlx", "yarn dlx"] + class ConfigManager: """Manages MCP basic configuration @@ -35,7 +37,7 @@ class ConfigManager: def __init__(self, config_path: str = DEFAULT_CONFIG_FILE): self.config_path = config_path self.config_dir = os.path.dirname(config_path) - self._config = None + self._config = {} self._ensure_dirs() self._load_config() diff --git a/src/mcpm/utils/scope.py b/src/mcpm/utils/scope.py index 3bcf9fe0..7fd10625 100644 --- a/src/mcpm/utils/scope.py +++ b/src/mcpm/utils/scope.py @@ -1,16 +1,4 @@ -import sys - -if sys.version_info >= (3, 11): - from enum import StrEnum -else: - from enum import Enum - - class StrEnum(str, Enum): - """String enumeration for Python versions before 3.11.""" - - def __str__(self) -> str: - return self.value - +from enum import StrEnum CLIENT_PREFIX = "@" PROFILE_PREFIX = "%" diff --git a/tests/conftest.py b/tests/conftest.py index ab0ee9f3..32a5adfb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,12 +7,14 @@ import sys import tempfile from pathlib import Path +from unittest.mock import Mock import pytest # Add the src directory to the path for all tests sys.path.insert(0, str(Path(__file__).parent.parent)) +from mcpm.clients.client_registry import ClientRegistry from mcpm.clients.managers.claude_desktop import ClaudeDesktopManager from mcpm.clients.managers.windsurf import WindsurfManager from mcpm.utils.config import ConfigManager @@ -43,12 +45,21 @@ def temp_config_file(): @pytest.fixture -def config_manager(): +def config_manager(monkeypatch): """Create a ClientConfigManager with a temp config for testing""" with tempfile.TemporaryDirectory() as temp_dir: - config_path = os.path.join(temp_dir, "config.json") + tmp_config_path = os.path.join(temp_dir, "config.json") # Create ConfigManager with the temp path - config_mgr = ConfigManager(config_path=config_path) + + _original_init = ConfigManager.__init__ + + def _mock_init(self, config_path=tmp_config_path): + _original_init(self, config_path) + self.config_path = tmp_config_path + + monkeypatch.setattr(ConfigManager, "__init__", _mock_init) + + config_mgr = ConfigManager() # Create ClientConfigManager that will use this ConfigManager internally from mcpm.clients.client_config import ClientConfigManager @@ -59,9 +70,13 @@ def config_manager(): @pytest.fixture -def windsurf_manager(temp_config_file): +def windsurf_manager(temp_config_file, monkeypatch, config_manager): """Create a WindsurfManager instance using the temp config file""" - return WindsurfManager(config_path=temp_config_file) + windsurf_manager = WindsurfManager(config_path=temp_config_file) + monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) + monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) + monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) + return windsurf_manager @pytest.fixture diff --git a/tests/test_add.py b/tests/test_add.py index bdc3ae0d..4fc499e7 100644 --- a/tests/test_add.py +++ b/tests/test_add.py @@ -2,7 +2,6 @@ from click.testing import CliRunner -from mcpm.clients.client_registry import ClientRegistry from mcpm.commands.target_operations.add import add from mcpm.core.schema import RemoteServerConfig from mcpm.utils.config import ConfigManager @@ -11,9 +10,6 @@ def test_add_server(windsurf_manager, monkeypatch): """Test add server""" - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) monkeypatch.setattr( RepositoryManager, "_fetch_servers", @@ -53,9 +49,6 @@ def test_add_server(windsurf_manager, monkeypatch): def test_add_server_with_missing_arg(windsurf_manager, monkeypatch): """Test add server with a missing argument that should be replaced with empty string""" - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) monkeypatch.setattr( RepositoryManager, "_fetch_servers", @@ -117,10 +110,6 @@ def test_add_server_with_missing_arg(windsurf_manager, monkeypatch): def test_add_server_with_empty_args(windsurf_manager, monkeypatch): """Test add server with missing arguments that should be replaced with empty strings""" - monkeypatch.setattr(ClientRegistry, "get_active_client", Mock(return_value="windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) monkeypatch.setattr( RepositoryManager, "_fetch_servers", @@ -211,11 +200,6 @@ def test_add_sse_server_to_claude_desktop(claude_desktop_manager, monkeypatch): def test_add_profile_to_client(windsurf_manager, monkeypatch): profile_name = "work" - client_name = "windsurf" - - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@" + client_name)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) monkeypatch.setattr(ConfigManager, "get_router_config", Mock(return_value={"host": "localhost", "port": 8080})) # test cli @@ -226,3 +210,45 @@ def test_add_profile_to_client(windsurf_manager, monkeypatch): profile_server = windsurf_manager.get_server("work") assert profile_server is not None + + +def test_add_server_with_configured_npx(windsurf_manager, monkeypatch): + monkeypatch.setattr(ConfigManager, "get_config", Mock(return_value={"node_executable": "bunx"})) + monkeypatch.setattr( + RepositoryManager, + "_fetch_servers", + Mock( + return_value={ + "server-test": { + "installations": { + "npm": { + "type": "npm", + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-test", "--fmt", "${fmt}"], + "env": {"API_KEY": "${API_KEY}"}, + } + }, + "arguments": { + "fmt": {"type": "string", "description": "Output format", "required": True}, + "API_KEY": {"type": "string", "description": "API key", "required": True}, + }, + } + } + ), + ) + + # Mock Rich's progress display to prevent 'Only one live display may be active at once' error + with patch("rich.progress.Progress.__enter__", return_value=Mock()), \ + patch("rich.progress.Progress.__exit__"), \ + patch("prompt_toolkit.PromptSession.prompt", side_effect=["json", "test-api-key"]): + runner = CliRunner() + result = runner.invoke(add, ["server-test", "--force", "--alias", "test"]) + assert result.exit_code == 0 + + # Check that the server was added with alias + server = windsurf_manager.get_server("test") + assert server is not None + # Should use configured node executable + assert server.command == "bunx" + assert server.args == ["-y", "@modelcontextprotocol/server-test", "--fmt", "json"] + assert server.env["API_KEY"] == "test-api-key" diff --git a/tests/test_clients/test_windsurf.py b/tests/test_clients/test_windsurf.py index 91787127..11223975 100644 --- a/tests/test_clients/test_windsurf.py +++ b/tests/test_clients/test_windsurf.py @@ -14,7 +14,6 @@ from mcpm.clients.managers.windsurf import WindsurfManager from mcpm.core.schema import ServerConfig, STDIOServerConfig -from mcpm.utils.config import ConfigManager class TestBaseClientManagerViaWindsurf: @@ -43,21 +42,6 @@ def sample_server_config(self): env={"API_KEY": "sample-key"}, ) - @pytest.fixture - def config_manager(self): - """Create a ClientConfigManager with a temp config for testing""" - with tempfile.TemporaryDirectory() as temp_dir: - config_path = os.path.join(temp_dir, "config.json") - # Create ConfigManager with the temp path - config_mgr = ConfigManager(config_path=config_path) - # Create ClientConfigManager that will use this ConfigManager internally - from mcpm.clients.client_config import ClientConfigManager - - client_mgr = ClientConfigManager() - # Override its internal config_manager with our temp one - client_mgr.config_manager = config_mgr - yield client_mgr - def test_list_servers(self, windsurf_manager): """Test list_servers method from BaseClientManager""" # list_servers returns a list of server names diff --git a/tests/test_remove.py b/tests/test_remove.py index 7260483e..74d7f654 100644 --- a/tests/test_remove.py +++ b/tests/test_remove.py @@ -6,13 +6,8 @@ from mcpm.commands.target_operations.remove import remove -def test_remove_server_success(windsurf_manager, monkeypatch): +def test_remove_server_success(windsurf_manager): """Test successful server removal""" - # Setup mocks - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) # Mock server info mock_server = Mock() @@ -31,12 +26,17 @@ def test_remove_server_success(windsurf_manager, monkeypatch): windsurf_manager.remove_server.assert_called_once_with("server-test") -def test_remove_server_not_found(windsurf_manager, monkeypatch): +def test_remove_server_not_found(windsurf_manager): """Test removal of non-existent server""" - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) + # Mock server not found + windsurf_manager.get_server = Mock(return_value=None) + + # Run the command with force flag + runner = CliRunner() + result = runner.invoke(remove, ["server-test", "--force"]) + + assert result.exit_code == 0 # Command exits successfully but with error message + assert "Server 'server-test' not found in windsurf" in result.output # Mock server not found windsurf_manager.get_server = Mock(return_value=None) @@ -60,12 +60,8 @@ def test_remove_server_unsupported_client(monkeypatch): assert "Client 'unsupported' not found." in result.output -def test_remove_server_cancelled(windsurf_manager, monkeypatch): +def test_remove_server_cancelled(windsurf_manager): """Test removal when user cancels the confirmation""" - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) # Mock server info mock_server = Mock() @@ -85,12 +81,8 @@ def test_remove_server_cancelled(windsurf_manager, monkeypatch): windsurf_manager.remove_server.assert_not_called() -def test_remove_server_failure(windsurf_manager, monkeypatch): +def test_remove_server_failure(windsurf_manager): """Test removal when the removal operation fails""" - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) # Mock server info mock_server = Mock() diff --git a/tests/test_stash_pop.py b/tests/test_stash_pop.py index 0da5ebe5..87e7dcff 100644 --- a/tests/test_stash_pop.py +++ b/tests/test_stash_pop.py @@ -9,12 +9,6 @@ def test_stash_server_success(windsurf_manager, monkeypatch): """Test successful server stashing""" - # Setup mocks - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - # Mock server info mock_server = Mock() mock_server.command = "npx" @@ -49,10 +43,6 @@ def test_stash_server_success(windsurf_manager, monkeypatch): def test_stash_server_already_stashed(windsurf_manager, monkeypatch): """Test stashing an already stashed server""" - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) # Mock server info mock_server = Mock() @@ -74,11 +64,6 @@ def test_stash_server_already_stashed(windsurf_manager, monkeypatch): def test_stash_server_remove_failure(windsurf_manager, monkeypatch): """Test stashing when server removal fails""" - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - # Mock server info mock_server = Mock() mock_server.command = "npx" @@ -103,11 +88,6 @@ def test_stash_server_remove_failure(windsurf_manager, monkeypatch): def test_stash_server_not_found(windsurf_manager, monkeypatch): """Test stashing a non-existent server""" - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - # Mock server not found windsurf_manager.get_server = Mock(return_value=None) @@ -143,11 +123,6 @@ def test_stash_server_unsupported_client(monkeypatch): def test_pop_server_success(windsurf_manager, monkeypatch): """Test successful server restoration""" - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - # Mock server data server_data = { "command": "npx", @@ -178,11 +153,6 @@ def test_pop_server_success(windsurf_manager, monkeypatch): def test_pop_server_not_stashed(windsurf_manager, monkeypatch): """Test popping a non-stashed server""" - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) - # Mock client config manager mock_config_manager = Mock() mock_config_manager.is_server_stashed = Mock(return_value=False) @@ -198,10 +168,6 @@ def test_pop_server_not_stashed(windsurf_manager, monkeypatch): def test_pop_server_add_failure(windsurf_manager, monkeypatch): """Test popping when server addition fails""" - monkeypatch.setattr(ClientRegistry, "get_active_target", Mock(return_value="@windsurf")) - monkeypatch.setattr(ClientRegistry, "get_active_client_manager", Mock(return_value=windsurf_manager)) - monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "windsurf"})) - monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=windsurf_manager)) # Mock server data server_data = { @@ -226,7 +192,7 @@ def test_pop_server_add_failure(windsurf_manager, monkeypatch): result = runner.invoke(pop, ["server-test"]) assert result.exit_code == 0 - assert "Failed to restore 'server-test' for windsurf" in result.output + assert "Failed to restore 'server-test' for Windsurf" in result.output mock_config_manager.stash_server.assert_called_once_with("@windsurf", "server-test", server_data)