fix: pass scope-resolved lockfile path to MCPIntegrator.update_lockfile at --global scope

src/apm_cli/commands/install.py

@@ -1718,7 +1718,7 @@ def _install_apm_packages(ctx, outcome):
             )
 
         # Persist the new MCP server set and configs in the lockfile
-        MCPIntegrator.update_lockfile(new_mcp_servers, mcp_configs=new_mcp_configs)
+        MCPIntegrator.update_lockfile(new_mcp_servers, _lock_path, mcp_configs=new_mcp_configs)
     elif should_install_mcp and not mcp_deps:
         # No MCP deps at all -- remove any old APM-managed servers
         if old_mcp_servers:
@@ -1730,12 +1730,12 @@ def _install_apm_packages(ctx, outcome):
                 user_scope=(ctx.scope is InstallScope.USER),
                 scope=ctx.scope,
             )
-            MCPIntegrator.update_lockfile(builtins.set(), mcp_configs={})
+            MCPIntegrator.update_lockfile(builtins.set(), _lock_path, mcp_configs={})
         logger.verbose_detail("No MCP dependencies found in apm.yml")
     elif not should_install_mcp and old_mcp_servers:
         # --only=apm: APM install regenerated the lockfile and dropped
         # mcp_servers.  Restore the previous set so it is not lost.
-        MCPIntegrator.update_lockfile(old_mcp_servers, mcp_configs=old_mcp_configs)
+        MCPIntegrator.update_lockfile(old_mcp_servers, _lock_path, mcp_configs=old_mcp_configs)
 
     # Local .apm/ content integration is now handled inside the
     # install pipeline (phases/integrate.py + phases/post_deps_local.py,

src/apm_cli/install/mcp/command.py

@@ -116,7 +116,8 @@ def run_mcp_install(
             logger.verbose_detail(f"Registry: {registry_url}")
         with registry_env_override(registry_url):
             try:
-                _existing_lock = LockFile.read(get_lockfile_path(apm_dir))
+                _mcp_lock_path = get_lockfile_path(apm_dir)
+                _existing_lock = LockFile.read(_mcp_lock_path)
                 old_servers = set(_existing_lock.mcp_servers) if _existing_lock else set()
                 old_configs = dict(_existing_lock.mcp_configs) if _existing_lock else {}
                 MCPIntegrator.install(
@@ -132,7 +133,9 @@ def run_mcp_install(
                 merged_names = old_servers | new_names
                 merged_configs = dict(old_configs)
                 merged_configs.update(new_configs)
-                MCPIntegrator.update_lockfile(merged_names, mcp_configs=merged_configs)
+                MCPIntegrator.update_lockfile(
+                    merged_names, _mcp_lock_path, mcp_configs=merged_configs
+                )
             except Exception as exc:
                 # Keep the raw exception (which may contain internal paths,
                 # credentials, or stack-trace fragments) at verbose level

tests/integration/test_global_mcp_lockfile_e2e.py

@@ -0,0 +1,262 @@
+"""Integration tests: --global MCP install writes lockfile to ~/.apm/ (#794).
+
+Regression suite for #794.  Verifies that ``MCPIntegrator.update_lockfile``
+receives the scope-resolved lockfile path so MCP server entries are persisted
+in the user-scope lockfile (``~/.apm/apm.lock.yaml``) rather than the
+project-local lockfile in ``Path.cwd()``.
+
+Uses the CliRunner (in-process) pattern from ``test_selective_install_mcp.py``
+with ``Path.home()`` overridden to an isolated temporary directory.
+"""
+
+import os
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+import yaml
+from click.testing import CliRunner
+
+from apm_cli.deps.lockfile import LockedDependency, LockFile
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _write_apm_yml(
+    path: Path,
+    *,
+    name: str = "test-project",
+    deps: list | None = None,
+    mcp: list | None = None,
+):
+    """Write a minimal apm.yml."""
+    data: dict = {"name": name, "version": "1.0.0", "dependencies": {}}
+    if deps:
+        data["dependencies"]["apm"] = deps
+    if mcp:
+        data["dependencies"]["mcp"] = mcp
+    path.parent.mkdir(parents=True, exist_ok=True)
+    path.write_text(yaml.dump(data, default_flow_style=False), encoding="utf-8")
+
+
+def _make_pkg(
+    apm_modules: Path,
+    repo_url: str,
+    *,
+    name: str | None = None,
+    mcp: list | None = None,
+    apm_deps: list | None = None,
+):
+    """Create a package directory with apm.yml under *apm_modules*."""
+    pkg_dir = apm_modules / repo_url
+    pkg_dir.mkdir(parents=True, exist_ok=True)
+    pkg_name = name or repo_url.split("/")[-1]
+    _write_apm_yml(
+        pkg_dir / "apm.yml",
+        name=pkg_name,
+        deps=apm_deps,
+        mcp=mcp,
+    )
+
+
+def _seed_lockfile(
+    path: Path,
+    locked_deps: list | None = None,
+    mcp_servers: list | None = None,
+):
+    """Write a lockfile pre-populated with given dependencies."""
+    lf = LockFile()
+    for dep in locked_deps or []:
+        lf.add_dependency(dep)
+    if mcp_servers:
+        lf.mcp_servers = mcp_servers
+    lf.write(path)
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture()
+def global_env(tmp_path):
+    """Set up an isolated global + project environment.
+
+    Returns ``(fake_home, work_dir, runner)`` where:
+
+    * ``fake_home/.apm/`` is the global APM directory
+    * ``work_dir`` is a separate CWD (to detect lockfile leakage)
+    """
+    fake_home = tmp_path / "fakehome"
+    fake_home.mkdir()
+
+    work_dir = tmp_path / "workdir"
+    work_dir.mkdir()
+
+    # Create ~/.apm/ structure
+    apm_dir = fake_home / ".apm"
+    apm_dir.mkdir(parents=True)
+    (apm_dir / "apm_modules").mkdir()
+
+    return fake_home, work_dir, CliRunner()
+
+
+# ---------------------------------------------------------------------------
+# Tests
+# ---------------------------------------------------------------------------
+
+
+class TestGlobalMCPLockfilePlacement:
+    """Regression for #794: MCP lockfile entries at --global scope."""
+
+    @patch("apm_cli.commands._helpers.check_for_updates", return_value=None)
+    @patch("apm_cli.commands.install._validate_package_exists", return_value=True)
+    @patch("apm_cli.integration.mcp_integrator.MCPIntegrator.install", return_value=0)
+    @patch("apm_cli.deps.github_downloader.GitHubPackageDownloader")
+    def test_global_install_writes_mcp_servers_to_global_lockfile(
+        self,
+        mock_dl_cls,
+        mock_mcp_install,
+        mock_validate,
+        mock_updates,
+        global_env,
+    ):
+        """MCP server entries must land in ~/.apm/apm.lock.yaml, not cwd."""
+        fake_home, work_dir, runner = global_env
+        apm_dir = fake_home / ".apm"
+        apm_modules = apm_dir / "apm_modules"
+
+        # Root manifest with a dependency that transitively declares MCP
+        _write_apm_yml(
+            apm_dir / "apm.yml",
+            name="global-project",
+            deps=["acme/squad-alpha"],
+        )
+
+        # squad-alpha depends on infra-cloud
+        _make_pkg(apm_modules, "acme/squad-alpha", apm_deps=["acme/infra-cloud"])
+
+        # infra-cloud declares MCP servers
+        _make_pkg(
+            apm_modules,
+            "acme/infra-cloud",
+            mcp=[
+                "ghcr.io/acme/mcp-alpha",
+                "ghcr.io/acme/mcp-beta",
+            ],
+        )
+
+        # Pre-seed the global lockfile (must exist for update_lockfile to write)
+        _seed_lockfile(
+            apm_dir / "apm.lock.yaml",
+            [
+                LockedDependency(
+                    repo_url="acme/squad-alpha",
+                    depth=1,
+                    resolved_by=None,
+                    resolved_commit="cached",
+                ),
+                LockedDependency(
+                    repo_url="acme/infra-cloud",
+                    depth=2,
+                    resolved_by="acme/squad-alpha",
+                    resolved_commit="cached",
+                ),
+            ],
+        )
+
+        from apm_cli.cli import cli
+
+        orig_cwd = os.getcwd()
+        try:
+            os.chdir(work_dir)
+            with patch.object(Path, "home", return_value=fake_home):
+                result = runner.invoke(
+                    cli,
+                    ["install", "--global", "--trust-transitive-mcp"],
+                )
+        finally:
+            os.chdir(orig_cwd)
+
+        assert result.exit_code == 0, f"CLI failed (exit {result.exit_code}):\n{result.output}"
+
+        # POSITIVE: global lockfile contains MCP server entries
+        global_lock = LockFile.read(apm_dir / "apm.lock.yaml")
+        assert global_lock is not None, "Global lockfile missing after install"
+        assert "ghcr.io/acme/mcp-alpha" in global_lock.mcp_servers
+        assert "ghcr.io/acme/mcp-beta" in global_lock.mcp_servers
+
+        # NEGATIVE: no lockfile leaked into the working directory
+        assert not (work_dir / "apm.lock.yaml").exists(), "Lockfile leaked into working directory"
+        assert not (work_dir / "apm.lock").exists(), "Legacy lockfile leaked into working directory"
+
+    @patch("apm_cli.commands._helpers.check_for_updates", return_value=None)
+    @patch("apm_cli.commands.install._validate_package_exists", return_value=True)
+    @patch("apm_cli.integration.mcp_integrator.MCPIntegrator.install", return_value=0)
+    @patch("apm_cli.integration.mcp_integrator.MCPIntegrator.remove_stale")
+    @patch("apm_cli.deps.github_downloader.GitHubPackageDownloader")
+    def test_global_install_no_mcp_clears_servers_in_global_lockfile(
+        self,
+        mock_dl_cls,
+        mock_remove_stale,
+        mock_mcp_install,
+        mock_validate,
+        mock_updates,
+        global_env,
+    ):
+        """When the manifest has no MCP deps, global lockfile mcp_servers is cleared."""
+        fake_home, work_dir, runner = global_env
+        apm_dir = fake_home / ".apm"
+        apm_modules = apm_dir / "apm_modules"
+
+        # Root manifest with NO MCP dependencies
+        _write_apm_yml(
+            apm_dir / "apm.yml",
+            name="global-project",
+            deps=["acme/plain-pkg"],
+        )
+
+        # plain-pkg has no MCP
+        _make_pkg(apm_modules, "acme/plain-pkg")
+
+        # Pre-seed global lockfile WITH stale mcp_servers from a prior install
+        _seed_lockfile(
+            apm_dir / "apm.lock.yaml",
+            [
+                LockedDependency(
+                    repo_url="acme/plain-pkg",
+                    depth=1,
+                    resolved_by=None,
+                    resolved_commit="cached",
+                ),
+            ],
+            mcp_servers=["ghcr.io/acme/old-server"],
+        )
+
+        from apm_cli.cli import cli
+
+        orig_cwd = os.getcwd()
+        try:
+            os.chdir(work_dir)
+            with patch.object(Path, "home", return_value=fake_home):
+                result = runner.invoke(
+                    cli,
+                    ["install", "--global"],
+                )
+        finally:
+            os.chdir(orig_cwd)
+
+        assert result.exit_code == 0, f"CLI failed (exit {result.exit_code}):\n{result.output}"
+
+        # POSITIVE: global lockfile mcp_servers is cleared
+        global_lock = LockFile.read(apm_dir / "apm.lock.yaml")
+        assert global_lock is not None, "Global lockfile missing after install"
+        assert global_lock.mcp_servers == [], (
+            f"Expected empty mcp_servers, got: {global_lock.mcp_servers}"
+        )
+
+        # NEGATIVE: no lockfile leaked into the working directory
+        assert not (work_dir / "apm.lock.yaml").exists(), "Lockfile leaked into working directory"
+        assert not (work_dir / "apm.lock").exists(), "Legacy lockfile leaked into working directory"

tests/unit/test_global_mcp_scope.py

@@ -344,5 +344,61 @@ def side_effect(rt):
                     self.assertIsInstance(result, int)
 
 
+# ---------------------------------------------------------------------------
+# 5. Regression: update_lockfile receives scope-resolved path (#794)
+# ---------------------------------------------------------------------------
+
+
+class TestUpdateLockfileGlobalScope(unittest.TestCase):
+    """Regression tests for #794: update_lockfile must receive scope-resolved path."""
+
+    def test_install_module_passes_lock_path_to_update_lockfile(self):
+        """All update_lockfile calls in install.py pass _lock_path positionally."""
+        import ast
+        from pathlib import Path
+
+        install_src = Path(__file__).resolve().parent.parent.parent / (
+            "src/apm_cli/commands/install.py"
+        )
+        tree = ast.parse(install_src.read_text(encoding="utf-8"))
+
+        for node in ast.walk(tree):
+            if (
+                isinstance(node, ast.Call)
+                and isinstance(node.func, ast.Attribute)
+                and node.func.attr == "update_lockfile"
+            ):
+                # Must have at least 2 positional args (server_names, lock_path)
+                self.assertGreaterEqual(
+                    len(node.args),
+                    2,
+                    f"update_lockfile call at line {node.lineno} is missing "
+                    f"the lock_path positional argument (regression #794)",
+                )
+
+    def test_mcp_command_passes_lock_path_to_update_lockfile(self):
+        """update_lockfile call in mcp/command.py passes lock_path positionally."""
+        import ast
+        from pathlib import Path
+
+        cmd_src = Path(__file__).resolve().parent.parent.parent / (
+            "src/apm_cli/install/mcp/command.py"
+        )
+        tree = ast.parse(cmd_src.read_text(encoding="utf-8"))
+
+        for node in ast.walk(tree):
+            if (
+                isinstance(node, ast.Call)
+                and isinstance(node.func, ast.Attribute)
+                and node.func.attr == "update_lockfile"
+            ):
+                self.assertGreaterEqual(
+                    len(node.args),
+                    2,
+                    f"update_lockfile call at line {node.lineno} is missing "
+                    f"the lock_path positional argument (regression #794)",
+                )
+
+
 if __name__ == "__main__":
     unittest.main()