headroom/tests/test_cli/test_serena_disable.py
Tejas Chopra 919379a8a1
fix(serena): stop the Serena dashboard popup and make --no-serena actually disable Serena (#1003)
## 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.
2026-06-14 23:32:46 -07:00

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