diff --git a/src/apm_cli/integration/mcp_integrator.py b/src/apm_cli/integration/mcp_integrator.py index 577042dfe..209596b50 100644 --- a/src/apm_cli/integration/mcp_integrator.py +++ b/src/apm_cli/integration/mcp_integrator.py @@ -293,6 +293,45 @@ def get_server_names(mcp_deps: list) -> builtins.set: names.add(dep) return names + @staticmethod + def _check_self_defined_servers_needing_installation( + dep_names: list, + target_runtimes: list, + ) -> list: + """Return self-defined MCP servers missing from at least one runtime. + + Self-defined servers have no registry UUID, so installation checks use + the runtime config keys directly. Runtime config reads are cached per + runtime to avoid repeating the same client setup for every dependency. + """ + try: + from apm_cli.core.conflict_detector import MCPConflictDetector + from apm_cli.factory import ClientFactory + except ImportError: + return list(dep_names) + + runtime_existing = {} + runtime_failures = [] + for runtime in target_runtimes: + try: + client = ClientFactory.create_client(runtime) + detector = MCPConflictDetector(client) + runtime_existing[runtime] = detector.get_existing_server_configs() + except Exception: + runtime_failures.append(runtime) + + servers_needing_installation = [] + for dep_name in dep_names: + if runtime_failures: + servers_needing_installation.append(dep_name) + continue + for runtime in target_runtimes: + if dep_name not in runtime_existing.get(runtime, {}): + servers_needing_installation.append(dep_name) + break + + return servers_needing_installation + # ------------------------------------------------------------------ # Stale server cleanup # ------------------------------------------------------------------ @@ -887,7 +926,40 @@ def install( # --- Self-defined deps (registry: false) --- if self_defined_deps: + self_defined_names = [dep.name for dep in self_defined_deps] + self_defined_to_install = ( + MCPIntegrator._check_self_defined_servers_needing_installation( + self_defined_names, + target_runtimes, + ) + ) + already_configured_self_defined = [ + name + for name in self_defined_names + if name not in self_defined_to_install + ] + + if already_configured_self_defined: + if console: + for name in already_configured_self_defined: + console.print( + f"│ [green]✓[/green] {name} " + f"[dim](already configured)[/dim]" + ) + elif verbose: + for name in already_configured_self_defined: + _rich_info(f"{name} already configured, skipping") + else: + names_str = ", ".join(already_configured_self_defined) + _rich_success( + f"{len(already_configured_self_defined)} self-defined " + f"server(s) already configured, skipping: {names_str}" + ) + for dep in self_defined_deps: + if dep.name not in self_defined_to_install: + continue + synthetic_info = MCPIntegrator._build_self_defined_info(dep) self_defined_cache = {dep.name: synthetic_info} self_defined_env = dep.env or {} diff --git a/tests/unit/test_transitive_mcp.py b/tests/unit/test_transitive_mcp.py index 4d760c0f8..b3528a263 100644 --- a/tests/unit/test_transitive_mcp.py +++ b/tests/unit/test_transitive_mcp.py @@ -1,12 +1,11 @@ """Tests for transitive MCP dependency collection and deduplication.""" -from pathlib import Path -from unittest.mock import patch, MagicMock +from unittest.mock import MagicMock, patch import yaml -from apm_cli.models.apm_package import APMPackage, MCPDependency from apm_cli.integration.mcp_integrator import MCPIntegrator +from apm_cli.models.apm_package import APMPackage, MCPDependency # --------------------------------------------------------------------------- @@ -426,3 +425,197 @@ def test_mixed_registry_servers_show_already_configured_and_count_only_new( ) assert "ghcr.io/org/already" in printed_lines assert "already configured" in printed_lines + + +# --------------------------------------------------------------------------- +# _check_self_defined_servers_needing_installation +# --------------------------------------------------------------------------- +class TestCheckSelfDefinedServersNeeding: + + @patch("apm_cli.core.conflict_detector.MCPConflictDetector") + @patch("apm_cli.factory.ClientFactory") + def test_all_servers_need_installation_when_none_configured( + self, mock_factory_cls, mock_detector_cls + ): + """All servers need installation when config is empty.""" + mock_client = MagicMock() + mock_factory_cls.create_client.return_value = mock_client + mock_detector = MagicMock() + mock_detector.get_existing_server_configs.return_value = {} + mock_detector_cls.return_value = mock_detector + + result = MCPIntegrator._check_self_defined_servers_needing_installation( + ["atlassian", "zephyr"], ["copilot", "vscode"] + ) + assert sorted(result) == ["atlassian", "zephyr"] + + @patch("apm_cli.core.conflict_detector.MCPConflictDetector") + @patch("apm_cli.factory.ClientFactory") + def test_no_servers_need_installation_when_all_configured( + self, mock_factory_cls, mock_detector_cls + ): + """No servers need installation when all are present in all runtimes.""" + mock_client = MagicMock() + mock_factory_cls.create_client.return_value = mock_client + mock_detector = MagicMock() + mock_detector.get_existing_server_configs.return_value = { + "atlassian": {"type": "http"}, + "zephyr": {"type": "http"}, + } + mock_detector_cls.return_value = mock_detector + + result = MCPIntegrator._check_self_defined_servers_needing_installation( + ["atlassian", "zephyr"], ["copilot", "vscode"] + ) + assert result == [] + + @patch("apm_cli.core.conflict_detector.MCPConflictDetector") + @patch("apm_cli.factory.ClientFactory") + def test_server_needs_installation_when_missing_in_one_runtime( + self, mock_factory_cls, mock_detector_cls + ): + """Server needs install if missing from at least one target runtime.""" + mock_client = MagicMock() + mock_factory_cls.create_client.return_value = mock_client + + # First runtime has it, second does not + copilot_config = {"atlassian": {"type": "http"}} + vscode_config = {} + + mock_detector = MagicMock() + mock_detector.get_existing_server_configs.side_effect = [ + copilot_config, vscode_config, + ] + mock_detector_cls.return_value = mock_detector + + result = MCPIntegrator._check_self_defined_servers_needing_installation( + ["atlassian"], ["copilot", "vscode"] + ) + assert result == ["atlassian"] + + @patch("apm_cli.factory.ClientFactory") + def test_config_read_failure_assumes_needs_installation( + self, mock_factory_cls + ): + """If config read fails, assume server needs installation.""" + mock_factory_cls.create_client.side_effect = Exception("config error") + + result = MCPIntegrator._check_self_defined_servers_needing_installation( + ["atlassian"], ["copilot"] + ) + assert result == ["atlassian"] + + def test_empty_runtimes_returns_empty(self): + """With no target runtimes, no server is found missing.""" + result = MCPIntegrator._check_self_defined_servers_needing_installation( + ["a", "b"], [] + ) + # With no runtimes to check, no server is found missing → none need install + assert result == [] + + @patch("apm_cli.core.conflict_detector.MCPConflictDetector") + @patch("apm_cli.factory.ClientFactory") + def test_reads_each_runtime_config_once_for_multiple_servers( + self, mock_factory_cls, mock_detector_cls + ): + """Runtime config reads are cached instead of repeated per server.""" + mock_factory_cls.create_client.side_effect = [MagicMock(), MagicMock()] + + mock_copilot_detector = MagicMock() + mock_copilot_detector.get_existing_server_configs.return_value = {} + mock_vscode_detector = MagicMock() + mock_vscode_detector.get_existing_server_configs.return_value = {} + mock_detector_cls.side_effect = [mock_copilot_detector, mock_vscode_detector] + + result = MCPIntegrator._check_self_defined_servers_needing_installation( + ["atlassian", "zephyr"], ["copilot", "vscode"] + ) + + assert sorted(result) == ["atlassian", "zephyr"] + assert [ + call.args[0] for call in mock_factory_cls.create_client.call_args_list + ] == ["copilot", "vscode"] + assert mock_copilot_detector.get_existing_server_configs.call_count == 1 + assert mock_vscode_detector.get_existing_server_configs.call_count == 1 + + +# --------------------------------------------------------------------------- +# _install_mcp_dependencies – self-defined skip logic +# --------------------------------------------------------------------------- +class TestInstallSelfDefinedSkipLogic: + + @patch("apm_cli.integration.mcp_integrator._rich_success") + @patch("apm_cli.integration.mcp_integrator.MCPIntegrator._check_self_defined_servers_needing_installation") + @patch("apm_cli.integration.mcp_integrator.MCPIntegrator._install_for_runtime") + @patch("apm_cli.integration.mcp_integrator._get_console", return_value=None) + def test_already_configured_self_defined_servers_skipped( + self, _console, mock_install_runtime, mock_check, mock_rich_success + ): + """Self-defined servers already configured should not trigger _install_for_runtime.""" + mock_check.return_value = [] # none need installation + + dep = MCPDependency( + name="atlassian", transport="http", url="https://atlassian.example.com", + registry=False, + ) + count = MCPIntegrator.install([dep], runtime="vscode") + + assert count == 0 + mock_install_runtime.assert_not_called() + mock_rich_success.assert_called_once() + assert "already configured" in mock_rich_success.call_args.args[0] + + @patch("apm_cli.integration.mcp_integrator.MCPIntegrator._check_self_defined_servers_needing_installation") + @patch("apm_cli.integration.mcp_integrator.MCPIntegrator._install_for_runtime") + @patch("apm_cli.integration.mcp_integrator._get_console", return_value=None) + def test_new_self_defined_server_installed( + self, _console, mock_install_runtime, mock_check + ): + """Self-defined servers NOT already configured should be installed.""" + mock_check.return_value = ["atlassian"] + mock_install_runtime.return_value = True + + dep = MCPDependency( + name="atlassian", transport="http", url="https://atlassian.example.com", + registry=False, + ) + count = MCPIntegrator.install([dep], runtime="vscode") + + assert count == 1 + assert mock_install_runtime.call_count == 1 + + @patch("apm_cli.integration.mcp_integrator.MCPIntegrator._check_self_defined_servers_needing_installation") + @patch("apm_cli.integration.mcp_integrator.MCPIntegrator._install_for_runtime") + def test_mixed_self_defined_shows_already_configured( + self, mock_install_runtime, mock_check + ): + """Mix of new and existing self-defined servers: only new ones installed, existing shown as configured.""" + mock_check.return_value = ["new-srv"] + mock_install_runtime.return_value = True + mock_console = MagicMock() + + deps = [ + MCPDependency( + name="existing-srv", transport="http", + url="https://existing.example.com", registry=False, + ), + MCPDependency( + name="new-srv", transport="http", + url="https://new.example.com", registry=False, + ), + ] + + with patch( + "apm_cli.integration.mcp_integrator._get_console", + return_value=mock_console, + ): + count = MCPIntegrator.install(deps, runtime="vscode") + + assert count == 1 + assert mock_install_runtime.call_count == 1 + + printed_lines = "\n".join( + str(call.args[0]) for call in mock_console.print.call_args_list if call.args + ) + assert "existing-srv" in printed_lines + assert "already configured" in printed_lines