diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dd952a1..8fe8276e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm run start` now checks `~/.apm/runtimes/` before system PATH for runtime detection, preventing system stubs from shadowing APM-managed binaries (#651) - Propagate headers and environment variables through OpenCode MCP adapter with defensive copies to prevent mutation (#622) ### Changed diff --git a/src/apm_cli/core/script_runner.py b/src/apm_cli/core/script_runner.py index 87798c5c..21be5b83 100644 --- a/src/apm_cli/core/script_runner.py +++ b/src/apm_cli/core/script_runner.py @@ -847,7 +847,8 @@ def _create_minimal_config(self) -> None: def _detect_installed_runtime(self) -> str: """Detect installed runtime with priority order. - Priority: copilot > codex > error + Priority: APM-managed (~/.apm/runtimes/) > system PATH + Within each source: copilot > codex > error Returns: Name of detected runtime @@ -855,9 +856,17 @@ def _detect_installed_runtime(self) -> str: Raises: RuntimeError: If no compatible runtime is found """ - import shutil + runtime_dir = Path.home() / ".apm" / "runtimes" - # Priority order: copilot first (recommended), then codex + # Check APM-managed runtimes first (highest priority) + # Use shutil.which with path= to handle platform-specific extensions + # (e.g. .exe on Windows via PATHEXT) + for runtime_name in ["copilot", "codex"]: + found = shutil.which(runtime_name, path=str(runtime_dir)) + if found: + return runtime_name + + # Fall back to system PATH if shutil.which("copilot"): return "copilot" elif shutil.which("codex"): diff --git a/tests/unit/test_script_runner.py b/tests/unit/test_script_runner.py index 31c79b0a..58f7f411 100644 --- a/tests/unit/test_script_runner.py +++ b/tests/unit/test_script_runner.py @@ -864,3 +864,141 @@ def test_skips_resolution_on_non_windows(self, mock_sys, mock_which, mock_run): ) mock_which.assert_not_called() + + +class TestDetectInstalledRuntime: + """Test _detect_installed_runtime() APM-managed vs PATH priority (#605).""" + + def setup_method(self): + self.runner = ScriptRunner() + + @patch("apm_cli.core.script_runner.shutil.which") + @patch("apm_cli.core.script_runner.Path.home") + def test_apm_managed_copilot_preferred_over_path(self, mock_home, mock_which, tmp_path): + """APM-managed copilot in ~/.apm/runtimes/ takes priority over PATH.""" + runtime_dir = tmp_path / ".apm" / "runtimes" + runtime_dir.mkdir(parents=True) + (runtime_dir / "copilot").write_text("binary") + mock_home.return_value = tmp_path + + def which_side_effect(name, mode=None, path=None): + if path is not None: + # APM-dir search + return str(runtime_dir / name) if name == "copilot" else None + # System PATH — should never be reached + return "/usr/bin/codex" if name == "codex" else None + + mock_which.side_effect = which_side_effect + + result = self.runner._detect_installed_runtime() + + assert result == "copilot" + # Verify shutil.which was called with path= for the APM-dir check, + # and NOT called without path= (system PATH should be skipped) + for call in mock_which.call_args_list: + assert call.kwargs.get("path") or (len(call.args) > 2 and call.args[2] is not None), \ + "System PATH shutil.which should not be called when APM-managed runtime found" + + @patch("apm_cli.core.script_runner.shutil.which") + @patch("apm_cli.core.script_runner.Path.home") + def test_apm_managed_codex_when_no_copilot(self, mock_home, mock_which, tmp_path): + """APM-managed codex is returned when copilot is not in runtimes dir.""" + runtime_dir = tmp_path / ".apm" / "runtimes" + runtime_dir.mkdir(parents=True) + (runtime_dir / "codex").write_text("binary") + mock_home.return_value = tmp_path + + def which_side_effect(name, mode=None, path=None): + if path is not None: + return str(runtime_dir / name) if name == "codex" else None + return None + + mock_which.side_effect = which_side_effect + + result = self.runner._detect_installed_runtime() + + assert result == "codex" + + @patch("apm_cli.core.script_runner.shutil.which") + @patch("apm_cli.core.script_runner.Path.home") + def test_falls_back_to_path_when_no_apm_runtimes(self, mock_home, mock_which, tmp_path): + """Falls back to shutil.which when ~/.apm/runtimes/ has no binaries.""" + runtime_dir = tmp_path / ".apm" / "runtimes" + runtime_dir.mkdir(parents=True) + mock_home.return_value = tmp_path + + def which_side_effect(name, mode=None, path=None): + if path is not None: + return None # Nothing in APM dir + return "/usr/local/bin/copilot" if name == "copilot" else None + + mock_which.side_effect = which_side_effect + + result = self.runner._detect_installed_runtime() + + assert result == "copilot" + + @patch("apm_cli.core.script_runner.shutil.which", return_value=None) + @patch("apm_cli.core.script_runner.Path.home") + def test_raises_when_no_runtime_found(self, mock_home, mock_which, tmp_path): + """Raises RuntimeError when no runtime is found anywhere.""" + runtime_dir = tmp_path / ".apm" / "runtimes" + runtime_dir.mkdir(parents=True) + mock_home.return_value = tmp_path + + with pytest.raises(RuntimeError, match="No compatible runtime found"): + self.runner._detect_installed_runtime() + + @patch("apm_cli.core.script_runner.shutil.which") + @patch("apm_cli.core.script_runner.Path.home") + def test_apm_managed_ignores_directories(self, mock_home, mock_which, tmp_path): + """Directories named 'copilot' or 'codex' in runtimes dir should not match.""" + runtime_dir = tmp_path / ".apm" / "runtimes" + runtime_dir.mkdir(parents=True) + (runtime_dir / "copilot").mkdir() # directory, not a file + mock_home.return_value = tmp_path + + def which_side_effect(name, mode=None, path=None): + if path is not None: + return None # shutil.which won't match directories + return "/usr/bin/codex" if name == "codex" else None + + mock_which.side_effect = which_side_effect + + result = self.runner._detect_installed_runtime() + + assert result == "codex" + + @patch("apm_cli.core.script_runner.shutil.which", return_value=None) + @patch("apm_cli.core.script_runner.Path.home") + def test_nonexistent_runtime_dir_falls_to_path(self, mock_home, mock_which, tmp_path): + """When ~/.apm/runtimes/ doesn't exist, falls through to PATH check.""" + # Don't create the runtime_dir — it won't exist + mock_home.return_value = tmp_path + + with pytest.raises(RuntimeError, match="No compatible runtime found"): + self.runner._detect_installed_runtime() + + @patch("apm_cli.core.script_runner.shutil.which") + @patch("apm_cli.core.script_runner.Path.home") + def test_windows_exe_detected_in_apm_runtimes(self, mock_home, mock_which, tmp_path): + """On Windows, codex.exe in ~/.apm/runtimes/ is detected via shutil.which PATHEXT.""" + runtime_dir = tmp_path / ".apm" / "runtimes" + runtime_dir.mkdir(parents=True) + # Windows installs codex.exe, not codex + (runtime_dir / "codex.exe").write_text("binary") + mock_home.return_value = tmp_path + + def which_side_effect(name, mode=None, path=None): + if path is not None: + # Simulate Windows PATHEXT: shutil.which("codex", path=dir) finds codex.exe + if name == "codex": + return str(runtime_dir / "codex.exe") + return None + return None + + mock_which.side_effect = which_side_effect + + result = self.runner._detect_installed_runtime() + + assert result == "codex"