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.
This commit is contained in:
Tejas Chopra 2026-06-14 23:32:46 -07:00 committed by GitHub
parent 01fdedc630
commit 919379a8a1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 209 additions and 5 deletions

View file

@ -648,6 +648,41 @@ def _remove_headroom_installed_serena_mcp(registrar: Any) -> str:
return "failed" return "failed"
def _disable_serena_mcp(registrar: Any, *, verbose: bool = False) -> None:
"""Make ``--no-serena`` actively disable Serena, not merely skip adding it.
Serena is registered by default, so a prior ``headroom wrap`` persists a
``serena`` entry into the agent's MCP config; the agent then keeps
launching Serena on startup. Just *skipping* registration on a later
``--no-serena`` run leaves that stale entry in place so the flag has to
remove the entry Headroom installed. A user-managed Serena (absent from
our ledger) is reported but left untouched.
"""
if not registrar.detect():
if verbose:
click.echo(f" Serena MCP: {registrar.display_name} not detected — skipping")
return
if registrar.get_server("serena") is None:
if verbose:
click.echo(" Skipping Serena MCP (--no-serena)")
return
status = _remove_headroom_installed_serena_mcp(registrar)
if status == "removed":
click.echo(" Removed previously-installed Serena MCP (--no-serena)")
click.echo(f" restart {registrar.display_name} if it was already running")
elif status == "not_headroom_owned":
click.echo(
" Serena MCP is present but user-managed — leaving it in place "
"(--no-serena only removes entries Headroom installed)"
)
else: # "failed"
click.echo(
" Serena MCP: removal failed — remove the 'serena' entry from your MCP config manually"
)
_CBM_MCP_SERVER_NAME = "codebase-memory-mcp" _CBM_MCP_SERVER_NAME = "codebase-memory-mcp"
@ -2769,8 +2804,10 @@ def claude(
from headroom.mcp_registry import ClaudeRegistrar from headroom.mcp_registry import ClaudeRegistrar
_setup_serena_mcp(ClaudeRegistrar(), context="claude-code", verbose=verbose) _setup_serena_mcp(ClaudeRegistrar(), context="claude-code", verbose=verbose)
elif verbose: else:
click.echo(" Skipping Serena MCP (--no-serena)") from headroom.mcp_registry import ClaudeRegistrar
_disable_serena_mcp(ClaudeRegistrar(), verbose=verbose)
if code_graph: if code_graph:
_setup_code_graph(verbose=verbose) _setup_code_graph(verbose=verbose)
@ -3264,8 +3301,10 @@ def codex(
from headroom.mcp_registry import CodexRegistrar from headroom.mcp_registry import CodexRegistrar
_setup_serena_mcp(CodexRegistrar(), context="codex", verbose=verbose, force=True) _setup_serena_mcp(CodexRegistrar(), context="codex", verbose=verbose, force=True)
elif verbose: else:
click.echo(" Skipping Serena MCP (--no-serena)") from headroom.mcp_registry import CodexRegistrar
_disable_serena_mcp(CodexRegistrar(), verbose=verbose)
# Setup memory MCP server for Codex (native tool integration) # Setup memory MCP server for Codex (native tool integration)
if memory: if memory:
@ -4397,6 +4436,21 @@ def unwrap_codex(port: int, no_stop_proxy: bool) -> None:
) )
click.echo(f" Nothing to undo: {config_file} has no Headroom wrap markers.") click.echo(f" Nothing to undo: {config_file} has no Headroom wrap markers.")
# Serena is written as its own [mcp_servers.serena] table with Headroom
# markers, separate from the provider block handled above — a "cleaned"
# restore leaves it behind. Remove it explicitly (only if we installed it),
# mirroring unwrap_claude. Runs after the restore so a backup-restore that
# already dropped Serena makes this a safe no-op.
from headroom.mcp_registry import CodexRegistrar
codex_registrar = CodexRegistrar()
if codex_registrar.detect():
serena_status = _remove_headroom_installed_serena_mcp(codex_registrar)
if serena_status == "removed":
click.echo(" Removed Headroom-installed Serena MCP server from Codex.")
elif serena_status == "failed":
click.echo(" Serena MCP server matched Headroom ledger but could not be removed.")
click.echo() click.echo()
click.echo("✓ Codex is no longer routed through the Headroom proxy.") click.echo("✓ Codex is no longer routed through the Headroom proxy.")
if not no_stop_proxy and status != "noop": if not no_stop_proxy and status != "noop":

View file

@ -38,7 +38,18 @@ def build_headroom_spec(proxy_url: str = DEFAULT_PROXY_URL) -> ServerSpec:
def build_serena_spec(context: str) -> ServerSpec: def build_serena_spec(context: str) -> ServerSpec:
"""Construct the canonical Serena MCP server spec for an agent context.""" """Construct the canonical Serena MCP server spec for an agent context.
``--open-web-dashboard False`` suppresses Serena's browser popup on
startup. Headroom installs Serena by default, so without this flag every
wrapped session opens the Serena dashboard tab even for users who never
opted into Serena or created a ``~/.serena/serena_config.yml``. The flag
overrides Serena's own config at startup (it sets
``web_dashboard_open_on_launch=False``), so it works regardless of the
user's local config. The dashboard backend still runs and remains
reachable at http://localhost:24282/dashboard/ for anyone who wants it
only the automatic browser-open is disabled.
"""
return ServerSpec( return ServerSpec(
name="serena", name="serena",
command="uvx", command="uvx",
@ -50,6 +61,8 @@ def build_serena_spec(context: str) -> ServerSpec:
"--project-from-cwd", "--project-from-cwd",
"--context", "--context",
context, context,
"--open-web-dashboard",
"False",
), ),
) )

View file

@ -0,0 +1,125 @@
"""`--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

View file

@ -81,10 +81,22 @@ def test_build_serena_spec_uses_agent_context() -> None:
"--project-from-cwd", "--project-from-cwd",
"--context", "--context",
"codex", "codex",
"--open-web-dashboard",
"False",
) )
assert spec.env == {} assert spec.env == {}
def test_build_serena_spec_disables_dashboard_popup_by_default() -> None:
# Headroom installs Serena by default; the dashboard browser tab must not
# auto-open. The flag overrides the user's serena_config.yml at startup,
# so this holds even when the user never created a Serena config.
for context in ("codex", "claude-code"):
spec = build_serena_spec(context)
idx = spec.args.index("--open-web-dashboard")
assert spec.args[idx + 1] == "False"
# ---------------------------------------------------------------------- # ----------------------------------------------------------------------
# install_everywhere # install_everywhere
# ---------------------------------------------------------------------- # ----------------------------------------------------------------------