mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description
`_ensure_serena_dashboard_disabled()` wrote a one-key bootstrap config
(`web_dashboard_open_on_launch: false`) into
`~/.serena/serena_config.yml` when the file was absent, assuming Serena
fills in any key it omits.
Verified against **Serena 1.6.2.dev0**
(`serena/config/serena_config.py`), that holds for every field except
one. Serena autogenerates its own complete config **only when the path
does not exist**:
```python
if not os.path.exists(config_file_path):
cls._generate_config_file(config_file_path)
```
Once any file is present it validates instead. Every other field falls
back to a dataclass default via `get_value_or_default`, but a missing
`projects` key is fatal (~line 1064):
```
SerenaConfigError: `projects` key not found in Serena configuration.
```
So Headroom's own bootstrap file killed Serena on **every machine
without a pre-existing Serena config**. The MCP server exited during
handshake — surfacing as `connection closed: initialize response` on
Codex and a bare `MCP error -32000: Connection closed` on OpenCode
(#2674) — and `serena project index` failed identically.
Headroom now leaves that file to Serena. That is immune to Serena adding
required keys later; guessing the schema is what caused the outage. The
popup never needed the file anyway: `build_serena_spec` passes
`--open-web-dashboard False`, which Serena applies *after* loading the
config (`serena/mcp.py:361` — `config.web_dashboard_open_on_launch =
open_web_dashboard`), so the flag wins regardless of what is on disk.
An **existing** config is still edited in place — dashboard key flipped,
`projects: []` backfilled to repair machines an affected version already
wrote — preserving a populated `projects` list, other keys and comments.
Closes #2674
## 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
- [x] Code refactoring (no functional changes)
## Changes Made
- `_ensure_serena_dashboard_disabled()` never creates
`serena_config.yml`; it only edits an existing one, and backfills
`projects: []` there to repair already-broken machines.
- Dropped `_scope_serena_languages` + `_detect_repo_languages` +
`_EXT_TO_SERENA_LANGUAGE` (**−133 lines**). Dead weight: Serena
determines languages itself in `ProjectConfig.autogenerate`
(`_determine_project_language_servers`) and records them under
`language_servers` — `languages`, which Headroom wrote, is a legacy name
Serena migrates via `RENAMED_FIELDS`. Serena's generated file uses a
block-style list, so our single-line-flow regex never matched it: on any
Serena-generated `project.yml` the function was a **verified no-op**.
The only case where it acted was creating the file — the same
partial-config trap — which also skipped the `project.local.yml` sidecar
Serena writes alongside.
- **Test isolation:** the MCP install ledger defaults to
`~/.headroom/mcp_installs.json`, so any test registering a server wrote
into the developer's real ledger (observed adding a live `claude/serena`
entry during a local run). `conftest.py` now redirects it per-test.
- **Repo config:** `.serena/project.yml` carried a stale `project_name`
(`"feature-opencode-wrap"`) and listed only `typescript`, so Serena's
symbol index skipped 1331 Python and 194 Rust files for every
contributor.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed
### Test Output
```text
$ pytest tests/test_wrap_code_memory.py tests/test_cli/test_wrap_serena_boost.py \
tests/test_cli/test_serena_migrate.py tests/test_cli/test_serena_disable.py -q
35 passed, 1 skipped in 0.84s
$ SERENA_SRC=<serena checkout> pytest tests/test_wrap_code_memory.py -q
13 passed in 0.62s # the skipped test runs when a Serena source tree is available
$ ruff check headroom/ tests/ --exclude headroom/dashboard/templates
All checks passed!
$ mypy headroom/cli/wrap.py
Success: no issues found in 1 source file
```
New tests. The key one asserts the invariant rather than our own key
list, so it stays correct even if Serena adds a required key — a test
pinning `projects: []` would keep passing while users broke again:
- `test_serena_config_is_never_created_by_headroom` — Headroom must not
pre-empt Serena's bootstrap
- `test_serena_dashboard_disabled_repairs_config_missing_projects` —
heals a config an affected version wrote
- `test_serena_dashboard_disabled_preserves_registered_projects` — never
clobbers the real registry; comments kept, no duplicate key
- `test_serena_dashboard_disabled_is_idempotent`
- `test_serena_config_required_keys_match_serena_source` — reads
Serena's real source and pins the two facts this fix rests on
(bootstrap-only-when-absent, `projects` is the sole fatal omission).
Skipped unless `SERENA_SRC` is set; deliberately **not** named
`HEADROOM_*` because `conftest.py` scrubs that namespace, which would
make it silently always-skip.
## Real Behavior Proof
- **Environment:** macOS 15.4 (darwin 25.4.0), Python 3.12.6, Serena
1.6.2.dev0 via `uvx --from git+https://github.com/oraios/serena`, Codex
CLI 0.146.0.
- **Exact command / steps:** a probe doing a real JSON-RPC `initialize`
handshake against the exact command `headroom wrap` registers — i.e.
what Codex/OpenCode actually do — in a throwaway `HOME` per case. (A)
pre-seeded with the one-line config an affected version wrote; (B) no
config; (C) real `headroom wrap codex --prepare-only`, then handshake.
- **Observed result:**
```text
=== A. BROKEN: single-key config (Headroom 0.33.0) ===
MCP handshake: FAIL — no initialize response (exit=1). stderr tail:
File ".../serena/config/serena_config.py", line 1064, in from_config_file
raise SerenaConfigError("`projects` key not found in Serena configuration. ...")
serena.config.serena_config.SerenaConfigError: `projects` key not found ...
config after run: 1 lines, has 'projects': False
=== B. FIXED: no config, Serena bootstraps it ===
MCP handshake: PASS — initialize OK — serverInfo.name='Serena'
config after run: 213 lines, has 'projects': True
=== C. FULL FLOW: real `headroom wrap codex` then handshake ===
Serena: no serena_config.yml yet — letting Serena generate it
Serena MCP: registered (restart OpenAI Codex CLI if it was already running)
Serena: project pre-indexed (symbol cache warmed)
serena_config.yml: 213 lines, written by Serena (correct)
MCP handshake: PASS — initialize OK — serverInfo.name='Serena'
--- verdict ---
A (broken config) started: False <- expected False
B (fixed, no config) started: True <- expected True
C (after real wrap) started: True <- expected True
```
A second `wrap` in the same HOME flips the dashboard without damage:
`true` → `false`, `projects` intact, all 153 comment lines intact. The
writer was isolated against a pristine 213-line Serena config: **delta 0
newlines**.
- **Not tested:** Windows and Linux (macOS only); Serena versions other
than 1.6.2.dev0; the JetBrains language backend. The probe needs network
+ `uvx` (~2 min) so it is a manual verification tool, not wired into CI.
## 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
- [x] I did **not** edit `CHANGELOG.md` — it is generated by
release-please from my Conventional Commit PR title (a CI guard enforces
this)
## Additional Notes
- **Docs:** N/A — no user-facing docs described the `serena_config.yml`
bootstrap or the language scoping.
- Also fixes the OpenCode report (#2674). The Codex-side report of the
same root cause quotes the `SerenaConfigError` verbatim; OpenCode only
surfaces the generic `-32000`, which is why it read as two different
bugs.
- Users already broken by an affected version are repaired automatically
on their next `headroom wrap` — no manual `serena_config.yml` edit
needed.
- A stacked PR removing the rtk/lean-ctx CLI context tools is based on
this branch; this one is deliberately small so it can land first.
151 lines
6 KiB
Python
151 lines
6 KiB
Python
"""Re-wrap must migrate a stale Headroom-installed Serena entry.
|
|
|
|
The dashboard-popup fix (#1003) added ``--open-web-dashboard False`` to the
|
|
Serena spec, but ``register_server`` refuses to overwrite a differing entry
|
|
without ``force``. So an already-wrapped user whose ``serena`` entry predates
|
|
the flag would keep the old spec — and the popup — on every re-wrap.
|
|
|
|
``_setup_serena_mcp`` closes that gap: when the ledger proves Headroom
|
|
installed the entry currently on disk, it force-updates to the current spec.
|
|
A user-managed Serena (absent from the ledger) is left untouched and the
|
|
mismatch is reported exactly as before.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import shutil
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from headroom.cli import wrap as wrap_cli
|
|
from headroom.mcp_registry import build_serena_spec
|
|
from headroom.mcp_registry.base import RegisterResult, RegisterStatus, ServerSpec
|
|
from headroom.mcp_registry.ledger import headroom_installed_matching, record_install
|
|
|
|
# The Serena spec Headroom wrote before the dashboard flag existed.
|
|
_STALE_SERENA_SPEC = ServerSpec(
|
|
name="serena",
|
|
command="uvx",
|
|
args=(
|
|
"--from",
|
|
"git+https://github.com/oraios/serena",
|
|
"serena",
|
|
"start-mcp-server",
|
|
"--project-from-cwd",
|
|
"--context",
|
|
"claude-code",
|
|
),
|
|
)
|
|
|
|
|
|
def _equivalent(a: ServerSpec, b: ServerSpec) -> bool:
|
|
return (a.command, tuple(a.args), dict(a.env)) == (b.command, tuple(b.args), dict(b.env))
|
|
|
|
|
|
class _FakeRegistrar:
|
|
"""Registrar mirroring real ``register_server`` overwrite semantics."""
|
|
|
|
def __init__(self, name: str, *, server: ServerSpec | None = None):
|
|
self.name = name
|
|
self.display_name = name.capitalize()
|
|
self._server = server
|
|
self.force_calls: list[bool] = []
|
|
|
|
def detect(self) -> bool:
|
|
return True
|
|
|
|
def get_server(self, server_name: str) -> ServerSpec | None:
|
|
return self._server if server_name == "serena" else None
|
|
|
|
def register_server(self, spec: ServerSpec, *, force: bool = False) -> RegisterResult:
|
|
self.force_calls.append(force)
|
|
if self._server is not None:
|
|
if _equivalent(self._server, spec):
|
|
return RegisterResult(RegisterStatus.ALREADY, "matches current configuration")
|
|
if not force:
|
|
return RegisterResult(RegisterStatus.MISMATCH, "args differ")
|
|
self._server = spec
|
|
return RegisterResult(RegisterStatus.REGISTERED, "registered")
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _isolate_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
|
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / ".headroom"))
|
|
# These tests drive ``_setup_serena_mcp`` with a fake registrar, so the real
|
|
# PATH is irrelevant — but the function bails early when ``uvx`` is absent.
|
|
# CI test shards run on runners without uvx, which would skip every code
|
|
# path under test. Stub uvx discovery so behaviour is PATH-independent.
|
|
real_which = shutil.which
|
|
monkeypatch.setattr(
|
|
wrap_cli.shutil,
|
|
"which",
|
|
lambda name, *a, **k: "/usr/bin/uvx" if name == "uvx" else real_which(name, *a, **k),
|
|
)
|
|
# ``_setup_serena_mcp`` now also injects guidance, scopes languages, and
|
|
# pre-indexes once registration succeeds. Those touch the real cwd / run
|
|
# real ``uvx`` — neutralise them so these registration-focused tests stay
|
|
# hermetic (covered directly in test_wrap_serena_boost.py).
|
|
monkeypatch.setattr(wrap_cli, "_inject_serena_instructions", lambda *a, **k: True)
|
|
monkeypatch.setattr(wrap_cli, "_index_serena_project", lambda *a, **k: None)
|
|
|
|
|
|
def test_rewrap_migrates_stale_headroom_serena(
|
|
capsys: pytest.CaptureFixture[str],
|
|
) -> None:
|
|
# Ledger proves Headroom installed the stale entry that's on disk.
|
|
record_install("claude", _STALE_SERENA_SPEC)
|
|
registrar = _FakeRegistrar("claude", server=_STALE_SERENA_SPEC)
|
|
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
|
|
fresh = build_serena_spec("claude-code")
|
|
assert _equivalent(registrar.get_server("serena"), fresh) # entry replaced
|
|
assert "--open-web-dashboard" in registrar.get_server("serena").args
|
|
assert registrar.force_calls == [False, True] # tried gentle, then forced
|
|
out = capsys.readouterr().out
|
|
assert "migrated previously-installed entry" in out
|
|
# Ledger now tracks the new spec, so a subsequent re-wrap is a no-op match.
|
|
assert headroom_installed_matching("claude", fresh)
|
|
|
|
|
|
def test_rewrap_leaves_user_managed_serena(
|
|
capsys: pytest.CaptureFixture[str],
|
|
) -> None:
|
|
# Differs from the current spec but is NOT in Headroom's ledger.
|
|
user_spec = ServerSpec(name="serena", command="/usr/local/bin/custom-serena")
|
|
registrar = _FakeRegistrar("claude", server=user_spec)
|
|
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
|
|
assert registrar.get_server("serena") is user_spec # never overwritten
|
|
assert registrar.force_calls == [False] # no forced retry
|
|
out = capsys.readouterr().out
|
|
assert "existing config differs" in out
|
|
assert "migrated" not in out
|
|
|
|
|
|
def test_rewrap_fresh_install_records_dashboard_off_spec(
|
|
capsys: pytest.CaptureFixture[str],
|
|
) -> None:
|
|
registrar = _FakeRegistrar("claude", server=None)
|
|
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
|
|
entry = registrar.get_server("serena")
|
|
assert entry is not None
|
|
assert ("--open-web-dashboard", "False") == tuple(entry.args[-2:])
|
|
assert registrar.force_calls == [False] # no entry → no forced retry needed
|
|
assert headroom_installed_matching("claude", entry)
|
|
|
|
|
|
def test_rewrap_already_current_is_noop(
|
|
capsys: pytest.CaptureFixture[str],
|
|
) -> None:
|
|
current = build_serena_spec("claude-code")
|
|
registrar = _FakeRegistrar("claude", server=current)
|
|
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
|
|
assert registrar.force_calls == [False] # ALREADY → no migration, no force
|
|
assert "migrated" not in capsys.readouterr().out
|