fix(mcp): register managed installs with a resolvable headroom command (#1386)

## Description

Managed Headroom installs can register the MCP server with a bare
`headroom mcp serve` command even when the active runtime lives in a
venv outside `PATH`. That leaves Claude and Codex with a registration
they cannot re-launch reliably, and Claude eventually fails with `Failed
to reconnect to headroom: ENOENT`.

This PR reuses the existing runtime command resolver when building the
shared Headroom MCP spec, so the generated registration follows the
active install instead of assuming `headroom` is globally discoverable.
It also updates the shared-builder and registrar tests so the proof rows
now flow through `build_headroom_spec()` and prove the same resolved
command contract on both the Claude CLI path and the Codex TOML path. A
follow-up CI fix keeps the Docker init E2E expectation aligned with that
same resolver-backed contract.

Closes #487

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- `headroom/mcp_registry/install.py`: build the Headroom MCP server spec
from the canonical runtime command resolver instead of hardcoding
`headroom mcp serve`
- `tests/test_mcp_registry/test_install.py`: cover the shared builder's
direct-binary and module-fallback command shapes
- `tests/test_mcp_registry/test_claude_registrar.py`: prove the Claude
CLI registration forwards the resolved command vector end to end
- `tests/test_mcp_registry/test_codex_registrar.py`: prove the Codex
registrar writes the same resolved command vector into TOML
- `e2e/init/run.py`: derive the Docker init E2E's expected Claude MCP
registration argv from `resolve_headroom_command()` so the CI harness
follows the same runtime contract
- `CHANGELOG.md`: note the managed-install MCP registration fix

## Testing

- [x] Unit tests pass (`uv run pytest
tests/test_mcp_registry/test_install.py -v`, `uv run pytest
tests/test_mcp_registry/test_claude_registrar.py -v`, `uv run pytest
tests/test_mcp_registry/test_codex_registrar.py -v`)
- [x] Linting passes (`uv run ruff check .`)
- [ ] Type checking passes (`uv run mypy headroom`) or explain N/A
truthfully
- [x] New tests added for new functionality when applicable
- [ ] Manual testing performed

### Test Output

```text
$ uv run pytest tests/test_mcp_registry/test_install.py -v
============================= 12 passed in 0.13s ==============================

$ uv run pytest tests/test_mcp_registry/test_claude_registrar.py -v
============================= 24 passed in 0.18s ==============================

$ uv run pytest tests/test_mcp_registry/test_codex_registrar.py -v
============================= 25 passed in 0.20s ==============================

$ uv run python -c "from e2e.init.run import _expected_headroom_mcp_call; print(_expected_headroom_mcp_call('http://127.0.0.1:9011'))"
['mcp', 'add', 'headroom', '-s', 'user', '-e', 'HEADROOM_PROXY_URL=http://127.0.0.1:9011', '--', '.../headroom', 'mcp', 'serve']

$ uv run ruff check e2e/init/run.py
All checks passed!

$ uv run ruff format e2e/init/run.py --check
1 file already formatted

$ uv run ruff check .
All checks passed!

$ uv run ruff format . --check
987 files already formatted
```

`uv run mypy headroom` was not run locally; this repo's focused local
gate for the touched Python registry path is the targeted pytest set
plus Ruff.

## Real Behavior Proof

- Environment: managed-install-safe MCP registration path, Python 3.11+,
no provider required
- Exact command / steps: run the focused MCP registry pytest files,
inspect the captured Claude CLI argv and rendered Codex TOML block, and
verify the Docker init E2E expectation derives its Claude MCP argv from
the same runtime helper
- Observed result: the persisted MCP registration uses a resolvable
command tied to the active Headroom runtime instead of bare `headroom`,
while `HEADROOM_PROXY_URL` handling stays unchanged
- Not tested: full live Claude reconnect against a real managed venv,
unless that is run during implementation

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

- Scoped to the MCP registration slice in `#487`. The RTK hook rewriting
thread from the same issue is intentionally out of scope here.
- `@erikpr1994` isolated the managed-install `ENOENT` failure mode in
the issue thread and narrowed it to the bare-command MCP registration
path.
- If existing owned registrations with the old bare-command contract
need an in-place upgrade path, that should be handled explicitly in the
final diff rather than left implicit.
This commit is contained in:
Rod Boev 2026-06-27 00:39:00 -04:00 committed by GitHub
parent adb793bee1
commit 22def93177
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 160 additions and 64 deletions

View file

@ -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}")

View file

@ -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,
)

View file

@ -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

View file

@ -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"},
}
}

View file

@ -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=()),

View file

@ -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"