fix: skip already-configured self-defined MCP servers on install

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 {}

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