mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-10 14:27:00 -04:00
## Description Headroom installs the Serena MCP server by default during `headroom wrap`, and many users reported the Serena web dashboard browser tab popping up on every session — even when they never opted into Serena. This PR fixes two distinct root causes: Serena's dashboard auto-open, and `--no-serena` not actually disabling an already-installed Serena. ## 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 - `build_serena_spec()` now passes `--open-web-dashboard False` to `serena start-mcp-server`. This is Serena's startup override for `web_dashboard_open_on_launch` (`serena/mcp.py:317-318`), so it suppresses the browser popup regardless of the user's `~/.serena/serena_config.yml` — the correct fix is at the launch point, not a per-machine config edit. The dashboard backend still runs and stays reachable at `http://localhost:24282/dashboard/`; only the auto-open is disabled. Applies to both launch paths (wrap + strands bundle) since both go through `build_serena_spec()`. - New `_disable_serena_mcp()`: `--no-serena` now actively removes the Serena entry Headroom installed (ledger-verified) instead of merely skipping registration. Previously a prior default wrap persisted a `serena` entry and the agent kept launching it; the old `Skipping Serena MCP` message was misleading. A user-managed Serena (absent from the ledger) is reported and left untouched; an absent Serena prints the skip message. Wired into both the Claude and Codex wrap paths. - `unwrap_codex` now removes Headroom-installed Serena. Codex writes Serena as its own `[mcp_servers.serena]` table, separate from the provider block the config-restore handles, so a "cleaned" unwrap previously left it behind (`unwrap_claude` already removed it; Codex was the gap). - Tests: updated `build_serena_spec` arg assertion + added a no-popup-default test; new `test_serena_disable.py` covering removed-when-headroom-owned, preserved-when-user-managed, skip-when-absent, noop-when-undetected, and `unwrap_codex` removal. ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ python -m pytest tests/test_cli/test_serena_disable.py tests/test_cli/test_wrap_codex.py tests/test_cli/test_unwrap_claude.py tests/test_mcp_registry/ -q 134 passed $ python -m pytest tests/test_mcp_registry/test_install.py -q ... passed (build_serena_spec arg + no-popup-default assertions) $ ruff check headroom/cli/wrap.py headroom/mcp_registry/install.py tests/test_cli/test_serena_disable.py tests/test_mcp_registry/test_install.py All checks passed! $ ruff format --check headroom/cli/wrap.py headroom/mcp_registry/install.py ... already formatted $ mypy headroom/cli/wrap.py headroom/mcp_registry/install.py Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: macOS (darwin), Python 3.12 venv, Serena 1.5.4 cached via uvx, headroom on branch fix/serena-no-dashboard-popup - Exact command / steps: Traced Serena source — `serena/cli.py` exposes `--open-web-dashboard <bool>`; `serena/mcp.py:317-318` sets `config.web_dashboard_open_on_launch = open_web_dashboard`; `serena/agent.py:706` feeds that to `DashboardManager`, which calls `webbrowser.open()` (`serena/dashboard.py:831`). Verified click parses `--open-web-dashboard False` → `False` via a CliRunner probe. Ran the test suites above. - Observed result: With the flag injected, the value that gates the browser-open is forced to False at startup regardless of local config, so no tab opens; dashboard backend still serves on its port. `--no-serena` removes the previously-installed `serena` entry (unregister called, "Removed previously-installed Serena MCP" printed) and `unwrap codex` removes it too. All 134 targeted tests pass; ruff + mypy clean. - Not tested: A full end-to-end `headroom wrap claude` against a live Claude Code install with a real browser was not run; verification is via Serena source tracing + the click-parse probe + unit/integration tests over the registrar and wrap/unwrap paths. ## 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 - [x] I have commented my code, particularly in hard-to-understand areas - [ ] 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes Two unchecked checklist items are N/A: no user-facing docs reference the Serena dashboard behavior, and CHANGELOG is generated via release-please from the conventional commits. "Manual testing performed" is left unchecked deliberately — see `Real Behavior Proof` → `Not tested` for the exact boundary of what was and wasn't exercised against a live browser.
125 lines
4.5 KiB
Python
125 lines
4.5 KiB
Python
"""`--no-serena` must actively disable Serena, not merely skip adding it.
|
|
|
|
Serena is installed by default, so a prior `headroom wrap` persists a
|
|
`serena` MCP entry and the agent keeps launching it (dashboard popup and
|
|
all). These tests pin that a later `--no-serena` removes the entry Headroom
|
|
installed, leaves a user-managed entry alone, and that Codex unwrap also
|
|
removes Serena.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
from click.testing import CliRunner
|
|
|
|
from headroom.cli import wrap as wrap_cli
|
|
from headroom.cli.main import main
|
|
from headroom.mcp_registry import build_serena_spec
|
|
from headroom.mcp_registry.base import ServerSpec
|
|
from headroom.mcp_registry.ledger import record_install
|
|
|
|
|
|
@pytest.fixture
|
|
def runner() -> CliRunner:
|
|
return CliRunner()
|
|
|
|
|
|
class _FakeRegistrar:
|
|
"""Minimal registrar capturing unregister calls."""
|
|
|
|
def __init__(self, name: str, *, detected: bool = True, server: ServerSpec | None = None):
|
|
self.name = name
|
|
self.display_name = name.capitalize()
|
|
self._detected = detected
|
|
self._server = server
|
|
self.unregistered: list[str] = []
|
|
|
|
def detect(self) -> bool:
|
|
return self._detected
|
|
|
|
def get_server(self, server_name: str) -> ServerSpec | None:
|
|
return self._server if server_name == "serena" else None
|
|
|
|
def unregister_server(self, server_name: str) -> bool:
|
|
self.unregistered.append(server_name)
|
|
self._server = None
|
|
return True
|
|
|
|
|
|
def test_disable_removes_headroom_installed_serena(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
|
) -> None:
|
|
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / ".headroom"))
|
|
spec = build_serena_spec("claude-code")
|
|
record_install("claude", spec) # ledger now proves Headroom owns it
|
|
registrar = _FakeRegistrar("claude", server=spec)
|
|
|
|
wrap_cli._disable_serena_mcp(registrar, verbose=True)
|
|
|
|
assert registrar.unregistered == ["serena"]
|
|
assert "Removed previously-installed Serena MCP" in capsys.readouterr().out
|
|
|
|
|
|
def test_disable_preserves_user_managed_serena(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
|
) -> None:
|
|
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / ".headroom"))
|
|
# Present in the agent config but NOT in Headroom's ledger → user-owned.
|
|
user_spec = ServerSpec(name="serena", command="/usr/local/bin/custom-serena")
|
|
registrar = _FakeRegistrar("claude", server=user_spec)
|
|
|
|
wrap_cli._disable_serena_mcp(registrar, verbose=True)
|
|
|
|
assert registrar.unregistered == [] # never touch a user-managed entry
|
|
assert "user-managed" in capsys.readouterr().out
|
|
|
|
|
|
def test_disable_noop_when_serena_absent(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
|
) -> None:
|
|
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / ".headroom"))
|
|
registrar = _FakeRegistrar("claude", server=None)
|
|
|
|
wrap_cli._disable_serena_mcp(registrar, verbose=True)
|
|
|
|
assert registrar.unregistered == []
|
|
assert "Skipping Serena MCP (--no-serena)" in capsys.readouterr().out
|
|
|
|
|
|
def test_disable_noop_when_agent_not_detected(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / ".headroom"))
|
|
spec = build_serena_spec("claude-code")
|
|
record_install("claude", spec)
|
|
registrar = _FakeRegistrar("claude", detected=False, server=spec)
|
|
|
|
wrap_cli._disable_serena_mcp(registrar, verbose=True)
|
|
|
|
assert registrar.unregistered == [] # not detected → leave everything alone
|
|
|
|
|
|
def test_unwrap_codex_removes_headroom_installed_serena(
|
|
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / ".headroom"))
|
|
spec = build_serena_spec("codex")
|
|
record_install("codex", spec)
|
|
registrar = _FakeRegistrar("codex", server=spec)
|
|
|
|
with (
|
|
patch("headroom.mcp_registry.CodexRegistrar", return_value=registrar),
|
|
patch(
|
|
"headroom.cli.wrap._restore_codex_provider_config",
|
|
return_value=("noop", tmp_path / "config.toml"),
|
|
),
|
|
patch("headroom.cli.wrap._stop_local_proxy_for_unwrap"),
|
|
):
|
|
result = runner.invoke(main, ["unwrap", "codex"])
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert registrar.unregistered == ["serena"]
|
|
assert "Removed Headroom-installed Serena MCP server from Codex" in result.output
|