diff --git a/e2e/init/run.py b/e2e/init/run.py index b28c8315b..95166ed8b 100644 --- a/e2e/init/run.py +++ b/e2e/init/run.py @@ -43,6 +43,7 @@ from e2e._lib import ( # noqa: E402 run_cases, ) from headroom.cli import init as init_cli # noqa: E402 +from headroom.install.runtime import resolve_headroom_command # noqa: E402 # ----- helpers reused across cases -------------------------------------------- @@ -51,6 +52,30 @@ from headroom.cli import init as init_cli # noqa: E402 REPO_ROOT_IN_CONTAINER = Path("/workspace") +def _expected_headroom_mcp_calls(proxy_url: str) -> list[list[str]]: + # The harness restores PATH before assertions, but `headroom init` ran with a + # scrubbed PATH where the console-script entrypoint may be unavailable. + prefix = [ + "mcp", + "add", + "headroom", + "-s", + "user", + "-e", + f"HEADROOM_PROXY_URL={proxy_url}", + "--", + ] + variants = [ + [*prefix, *resolve_headroom_command(), "mcp", "serve"], + [*prefix, sys.executable, "-m", "headroom.cli", "mcp", "serve"], + ] + deduped: list[list[str]] = [] + for variant in variants: + if variant not in deduped: + deduped.append(variant) + return deduped + + def _read_jsonl(path: Path) -> list[dict[str, object]]: if not path.exists(): return [] @@ -111,24 +136,16 @@ def _verify_claude_local(ctx: CaseContext) -> None: # being dead pointers for users who never ran `headroom mcp install`). # The `-e HEADROOM_PROXY_URL=…` arg is only emitted when the proxy # port differs from the 8787 default; this case uses --port 9011. - expected = [ + expected_prefix = [ ["plugin", "marketplace", "add", str(REPO_ROOT_IN_CONTAINER)], ["plugin", "install", "headroom@headroom-marketplace", "--scope", "local"], - [ - "mcp", - "add", - "headroom", - "-s", - "user", - "-e", - "HEADROOM_PROXY_URL=http://127.0.0.1:9011", - "--", - "headroom", - "mcp", - "serve", - ], ] - if claude_calls != expected: + expected_mcp_calls = _expected_headroom_mcp_calls("http://127.0.0.1:9011") + if ( + len(claude_calls) != 3 + or claude_calls[:2] != expected_prefix + or claude_calls[2] not in expected_mcp_calls + ): raise AssertionError(f"Unexpected Claude install commands: {claude_calls}") diff --git a/headroom/mcp_registry/install.py b/headroom/mcp_registry/install.py index 89b1b7d03..49834e1b8 100644 --- a/headroom/mcp_registry/install.py +++ b/headroom/mcp_registry/install.py @@ -4,6 +4,8 @@ from __future__ import annotations from collections.abc import Iterable +from headroom.install.runtime import resolve_headroom_command + from .base import MCPRegistrar, RegisterResult, RegisterStatus, ServerSpec from .claude import ClaudeRegistrar from .codex import CodexRegistrar @@ -30,10 +32,11 @@ def build_headroom_spec(proxy_url: str = DEFAULT_PROXY_URL) -> ServerSpec: env: dict[str, str] = {} if proxy_url and proxy_url != DEFAULT_PROXY_URL: env["HEADROOM_PROXY_URL"] = proxy_url + command = resolve_headroom_command() return ServerSpec( name="headroom", - command="headroom", - args=("mcp", "serve"), + command=command[0], + args=(*command[1:], "mcp", "serve"), env=env, ) diff --git a/tests/test_cli/test_wrap_codex.py b/tests/test_cli/test_wrap_codex.py index 41ea3aa67..44cbe3c74 100644 --- a/tests/test_cli/test_wrap_codex.py +++ b/tests/test_cli/test_wrap_codex.py @@ -15,10 +15,12 @@ from pathlib import Path from unittest.mock import patch import pytest +import tomllib from click.testing import CliRunner from headroom.cli import wrap as wrap_mod from headroom.cli.main import main +from headroom.mcp_registry.install import build_headroom_spec def _set_test_home(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: @@ -1010,9 +1012,13 @@ def test_wrap_codex_prepare_only_updates_stale_mcp_proxy_url( assert result.exit_code == 0, result.output content = config_file.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + expected = build_headroom_spec() + headroom_mcp = parsed["mcp_servers"]["headroom"] assert "[mcp_servers.headroom]" in content - assert 'command = "headroom"' in content - assert 'args = ["mcp", "serve"]' in content + assert headroom_mcp["command"] == expected.command + assert headroom_mcp["args"] == list(expected.args) + assert "env" not in headroom_mcp or "HEADROOM_PROXY_URL" not in headroom_mcp["env"] assert "http://127.0.0.1:9000" not in content diff --git a/tests/test_mcp_registry/test_claude_registrar.py b/tests/test_mcp_registry/test_claude_registrar.py index ad44bcde9..12a1e2f68 100644 --- a/tests/test_mcp_registry/test_claude_registrar.py +++ b/tests/test_mcp_registry/test_claude_registrar.py @@ -11,6 +11,10 @@ import pytest from headroom.mcp_registry.base import RegisterStatus, ServerSpec from headroom.mcp_registry.claude import ClaudeRegistrar +from headroom.mcp_registry.install import build_headroom_spec + +_RESOLVED_COMMAND = ("/usr/bin/python", "-m", "headroom.cli") +_RESOLVED_ARGS = ("-m", "headroom.cli", "mcp", "serve") def _make_registrar( @@ -25,12 +29,20 @@ def _make_registrar( def _spec() -> ServerSpec: return ServerSpec( name="headroom", - command="headroom", - args=("mcp", "serve"), + command="/usr/bin/python", + args=("-m", "headroom.cli", "mcp", "serve"), env={}, ) +def _install_spec(monkeypatch: pytest.MonkeyPatch) -> ServerSpec: + monkeypatch.setattr( + "headroom.mcp_registry.install.resolve_headroom_command", + lambda: list(_RESOLVED_COMMAND), + ) + return build_headroom_spec() + + # ---------------------------------------------------------------------- # detect() # ---------------------------------------------------------------------- @@ -70,8 +82,8 @@ def test_get_server_reads_modern_config(tmp_path: Path) -> None: { "mcpServers": { "headroom": { - "command": "headroom", - "args": ["mcp", "serve"], + "command": _RESOLVED_COMMAND[0], + "args": list(_RESOLVED_ARGS), "env": {"HEADROOM_PROXY_URL": "http://127.0.0.1:9000"}, } } @@ -81,8 +93,8 @@ def test_get_server_reads_modern_config(tmp_path: Path) -> None: reg = _make_registrar(tmp_path, cli=None) got = reg.get_server("headroom") assert got is not None - assert got.command == "headroom" - assert got.args == ("mcp", "serve") + assert got.command == _RESOLVED_COMMAND[0] + assert got.args == _RESOLVED_ARGS assert got.env == {"HEADROOM_PROXY_URL": "http://127.0.0.1:9000"} @@ -90,13 +102,22 @@ def test_get_server_falls_back_to_legacy(tmp_path: Path) -> None: cfg = tmp_path / ".claude" / "mcp.json" cfg.parent.mkdir() cfg.write_text( - json.dumps({"mcpServers": {"headroom": {"command": "headroom", "args": ["mcp", "serve"]}}}) + json.dumps( + { + "mcpServers": { + "headroom": { + "command": _RESOLVED_COMMAND[0], + "args": list(_RESOLVED_ARGS), + } + } + } + ) ) reg = _make_registrar(tmp_path, cli=None) got = reg.get_server("headroom") assert got is not None - assert got.command == "headroom" - assert got.args == ("mcp", "serve") + assert got.command == _RESOLVED_COMMAND[0] + assert got.args == _RESOLVED_ARGS assert got.env == {} @@ -105,14 +126,23 @@ def test_get_server_reads_claude_config_dir( ) -> None: cfg = tmp_path / ".claude.json" cfg.write_text( - json.dumps({"mcpServers": {"headroom": {"command": "headroom", "args": ["mcp", "serve"]}}}) + json.dumps( + { + "mcpServers": { + "headroom": { + "command": _RESOLVED_COMMAND[0], + "args": list(_RESOLVED_ARGS), + } + } + } + ) ) monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(tmp_path)) reg = ClaudeRegistrar(claude_cli=None) got = reg.get_server("headroom") assert got is not None - assert got.command == "headroom" - assert got.args == ("mcp", "serve") + assert got.command == _RESOLVED_COMMAND[0] + assert got.args == _RESOLVED_ARGS # ---------------------------------------------------------------------- @@ -120,11 +150,13 @@ def test_get_server_reads_claude_config_dir( # ---------------------------------------------------------------------- -def test_register_via_cli_calls_claude_mcp_add(tmp_path: Path) -> None: +def test_register_via_cli_calls_claude_mcp_add( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: reg = _make_registrar(tmp_path, cli="/usr/local/bin/claude") fake_result = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") with patch("subprocess.run", return_value=fake_result) as run_mock: - result = reg.register_server(_spec()) + result = reg.register_server(_install_spec(monkeypatch)) assert result.status == RegisterStatus.REGISTERED cmds = [call.args[0] for call in run_mock.call_args_list] add_cmd = next(c for c in cmds if "add" in c) @@ -136,19 +168,18 @@ def test_register_via_cli_calls_claude_mcp_add(tmp_path: Path) -> None: "-s", "user", ] - assert add_cmd[-3:] == ["--", "headroom", "mcp"] or add_cmd[-4:] == [ + assert add_cmd[-(len(_RESOLVED_ARGS) + 2) :] == [ "--", - "headroom", - "mcp", - "serve", + _RESOLVED_COMMAND[0], + *_RESOLVED_ARGS, ] def test_register_via_cli_includes_env(tmp_path: Path) -> None: spec = ServerSpec( name="headroom", - command="headroom", - args=("mcp", "serve"), + command=_RESOLVED_COMMAND[0], + args=_RESOLVED_ARGS, env={"HEADROOM_PROXY_URL": "http://127.0.0.1:9000"}, ) reg = _make_registrar(tmp_path, cli="/usr/local/bin/claude") @@ -168,8 +199,8 @@ def test_register_writes_file_when_no_cli(tmp_path: Path) -> None: cfg = tmp_path / ".claude" / ".claude.json" data = json.loads(cfg.read_text()) assert "headroom" in data["mcpServers"] - assert data["mcpServers"]["headroom"]["command"] == "headroom" - assert data["mcpServers"]["headroom"]["args"] == ["mcp", "serve"] + assert data["mcpServers"]["headroom"]["command"] == _RESOLVED_COMMAND[0] + assert data["mcpServers"]["headroom"]["args"] == list(_RESOLVED_ARGS) def test_register_writes_to_legacy_when_only_legacy_exists(tmp_path: Path) -> None: @@ -194,7 +225,7 @@ def test_register_writes_to_claude_config_dir( assert result.status == RegisterStatus.REGISTERED cfg = tmp_path / ".claude.json" data = json.loads(cfg.read_text()) - assert data["mcpServers"]["headroom"]["command"] == "headroom" + assert data["mcpServers"]["headroom"]["command"] == _RESOLVED_COMMAND[0] assert not (tmp_path / ".claude" / ".claude.json").exists() @@ -207,7 +238,16 @@ def test_register_already_when_spec_matches(tmp_path: Path) -> None: cfg = tmp_path / ".claude" / ".claude.json" cfg.parent.mkdir() cfg.write_text( - json.dumps({"mcpServers": {"headroom": {"command": "headroom", "args": ["mcp", "serve"]}}}) + json.dumps( + { + "mcpServers": { + "headroom": { + "command": _RESOLVED_COMMAND[0], + "args": list(_RESOLVED_ARGS), + } + } + } + ) ) reg = _make_registrar(tmp_path, cli="/usr/local/bin/claude") with patch("subprocess.run") as run_mock: @@ -224,8 +264,8 @@ def test_register_mismatch_when_spec_differs_no_force(tmp_path: Path) -> None: { "mcpServers": { "headroom": { - "command": "headroom", - "args": ["mcp", "serve"], + "command": _RESOLVED_COMMAND[0], + "args": list(_RESOLVED_ARGS), "env": {"HEADROOM_PROXY_URL": "http://127.0.0.1:9999"}, } } @@ -305,7 +345,7 @@ def test_unregister_via_file_when_no_cli(tmp_path: Path) -> None: json.dumps( { "mcpServers": { - "headroom": {"command": "headroom", "args": ["mcp", "serve"]}, + "headroom": {"command": _RESOLVED_COMMAND[0], "args": list(_RESOLVED_ARGS)}, "other": {"command": "other"}, } } @@ -331,7 +371,7 @@ def test_unregister_removes_from_claude_config_dir( json.dumps( { "mcpServers": { - "headroom": {"command": "headroom", "args": ["mcp", "serve"]}, + "headroom": {"command": _RESOLVED_COMMAND[0], "args": list(_RESOLVED_ARGS)}, "other": {"command": "other"}, } } diff --git a/tests/test_mcp_registry/test_codex_registrar.py b/tests/test_mcp_registry/test_codex_registrar.py index 7032bf854..6a48b4d2b 100644 --- a/tests/test_mcp_registry/test_codex_registrar.py +++ b/tests/test_mcp_registry/test_codex_registrar.py @@ -9,6 +9,7 @@ import pytest from headroom.mcp_registry.base import RegisterStatus, ServerSpec from headroom.mcp_registry.codex import CodexRegistrar +from headroom.mcp_registry.install import build_headroom_spec if sys.version_info >= (3, 11): import tomllib @@ -16,6 +17,10 @@ else: # pragma: no cover import tomli as tomllib +_RESOLVED_COMMAND = ("/usr/bin/python", "-m", "headroom.cli") +_RESOLVED_ARGS = ("-m", "headroom.cli", "mcp", "serve") + + def _make_registrar(tmp_path: Path) -> CodexRegistrar: return CodexRegistrar(home_dir=tmp_path) @@ -23,12 +28,20 @@ def _make_registrar(tmp_path: Path) -> CodexRegistrar: def _spec(env: dict[str, str] | None = None) -> ServerSpec: return ServerSpec( name="headroom", - command="headroom", - args=("mcp", "serve"), + command=_RESOLVED_COMMAND[0], + args=_RESOLVED_ARGS, env=env or {}, ) +def _install_spec(monkeypatch: pytest.MonkeyPatch) -> ServerSpec: + monkeypatch.setattr( + "headroom.mcp_registry.install.resolve_headroom_command", + lambda: list(_RESOLVED_COMMAND), + ) + return build_headroom_spec() + + def _serena_spec() -> ServerSpec: return ServerSpec( name="serena", @@ -111,16 +124,16 @@ def test_get_server_returns_spec_when_table_present(tmp_path: Path) -> None: cfg.parent.mkdir() cfg.write_text( "[mcp_servers.headroom]\n" - 'command = "headroom"\n' - 'args = ["mcp", "serve"]\n' + f"command = {_RESOLVED_COMMAND[0]!r}\n" + f"args = {list(_RESOLVED_ARGS)!r}\n" "\n" "[mcp_servers.headroom.env]\n" 'HEADROOM_PROXY_URL = "http://127.0.0.1:9000"\n' ) got = _make_registrar(tmp_path).get_server("headroom") assert got is not None - assert got.command == "headroom" - assert got.args == ("mcp", "serve") + assert got.command == _RESOLVED_COMMAND[0] + assert got.args == _RESOLVED_ARGS assert got.env == {"HEADROOM_PROXY_URL": "http://127.0.0.1:9000"} @@ -136,9 +149,11 @@ def test_get_server_robust_to_unparseable_toml(tmp_path: Path) -> None: # ---------------------------------------------------------------------- -def test_register_creates_config_when_missing(tmp_path: Path) -> None: +def test_register_creates_config_when_missing( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: reg = _make_registrar(tmp_path) - result = reg.register_server(_spec()) + result = reg.register_server(_install_spec(monkeypatch)) assert result.status == RegisterStatus.REGISTERED cfg = _config_path(tmp_path) assert cfg.exists() @@ -146,8 +161,8 @@ def test_register_creates_config_when_missing(tmp_path: Path) -> None: assert "# --- Headroom MCP server ---" in text assert "[mcp_servers.headroom]" in text parsed = tomllib.loads(text) - assert parsed["mcp_servers"]["headroom"]["command"] == "headroom" - assert parsed["mcp_servers"]["headroom"]["args"] == ["mcp", "serve"] + assert parsed["mcp_servers"]["headroom"]["command"] == _RESOLVED_COMMAND[0] + assert parsed["mcp_servers"]["headroom"]["args"] == list(_RESOLVED_ARGS) def test_register_appends_to_existing_config_preserves_other_keys(tmp_path: Path) -> None: @@ -166,7 +181,7 @@ def test_register_appends_to_existing_config_preserves_other_keys(tmp_path: Path parsed = tomllib.loads(text) assert parsed["model"] == "gpt-4o" assert parsed["other_section"]["value"] == 42 - assert parsed["mcp_servers"]["headroom"]["command"] == "headroom" + assert parsed["mcp_servers"]["headroom"]["command"] == _RESOLVED_COMMAND[0] def test_register_includes_env_subtable(tmp_path: Path) -> None: @@ -193,7 +208,7 @@ def test_register_headroom_and_serena_coexist(tmp_path: Path) -> None: assert "# --- Headroom MCP server: serena ---" in text parsed = tomllib.loads(text) - assert parsed["mcp_servers"]["headroom"]["command"] == "headroom" + assert parsed["mcp_servers"]["headroom"]["command"] == _RESOLVED_COMMAND[0] assert parsed["mcp_servers"]["serena"]["command"] == "uvx" @@ -326,11 +341,11 @@ def test_unregister_preserves_user_managed_entry(tmp_path: Path) -> None: @pytest.mark.parametrize( "spec", [ - ServerSpec(name="headroom", command="headroom", args=("mcp", "serve")), + ServerSpec(name="headroom", command=_RESOLVED_COMMAND[0], args=_RESOLVED_ARGS), ServerSpec( name="headroom", - command="headroom", - args=("mcp", "serve"), + command=_RESOLVED_COMMAND[0], + args=_RESOLVED_ARGS, env={"HEADROOM_PROXY_URL": "http://127.0.0.1:9000"}, ), ServerSpec(name="headroom", command="/usr/bin/headroom", args=()), diff --git a/tests/test_mcp_registry/test_install.py b/tests/test_mcp_registry/test_install.py index af5cfcd43..674199992 100644 --- a/tests/test_mcp_registry/test_install.py +++ b/tests/test_mcp_registry/test_install.py @@ -51,10 +51,14 @@ class _FakeRegistrar(MCPRegistrar): # ---------------------------------------------------------------------- -def test_build_spec_default_proxy_no_env() -> None: +def test_build_spec_default_proxy_no_env(monkeypatch) -> None: + monkeypatch.setattr( + "headroom.mcp_registry.install.resolve_headroom_command", + lambda: ["/opt/headroom/bin/headroom"], + ) spec = build_headroom_spec() assert spec.name == "headroom" - assert spec.command == "headroom" + assert spec.command == "/opt/headroom/bin/headroom" assert spec.args == ("mcp", "serve") assert spec.env == {} @@ -69,6 +73,17 @@ def test_build_spec_default_url_omits_env() -> None: assert spec.env == {} +def test_build_spec_falls_back_to_python_module_when_no_binary(monkeypatch) -> None: + monkeypatch.setattr("headroom.install.runtime.shutil.which", lambda name: None) + monkeypatch.setattr("headroom.install.runtime.sys.executable", "/usr/bin/python") + + spec = build_headroom_spec() + + assert spec.command == "/usr/bin/python" + assert spec.args == ("-m", "headroom.cli", "mcp", "serve") + assert spec.env == {} + + def test_build_serena_spec_uses_agent_context() -> None: spec = build_serena_spec("codex") assert spec.name == "serena"