headroom/tests/test_cli/test_serena_migrate.py

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

152 lines
6 KiB
Python
Raw Permalink Normal View History

fix(serena): migrate stale Headroom-installed Serena entry on re-wrap (#1008) ## Description #1003 added `--open-web-dashboard False` to the Serena spec to stop the dashboard browser tab popping up on every session — but the flag only reaches **fresh** registrations. `register_server` returns `MISMATCH` and refuses to overwrite a differing entry unless `force=True`, and the Claude wrap path calls `_setup_serena_mcp` **without** force (unlike the Codex path, which passes `force=True`). So anyone wrapped before #1003 has a `serena` entry whose args lack the flag. Every re-wrap detects the mismatch, prints `existing config differs … To update: remove the existing serena MCP entry, then rerun`, and gives up — the stale spec, and the popup, persist forever. The fix never reaches already-wrapped users, which is most of them. This completes #1003 by migrating those stale entries in place. Related to #1003 ## 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 - `_setup_serena_mcp` now migrates a stale entry: on `MISMATCH` (and when not already forced), it force-updates to the current spec **only when the ledger proves Headroom installed the entry currently on disk** (`headroom_installed_matching`). Prints `Serena MCP: migrated previously-installed entry to current spec`. - A user-managed Serena (absent from the ledger) is left untouched and the mismatch is reported exactly as before — the same ownership check `--no-serena` / `_disable_serena_mcp` already use, so a hand-rolled Serena is never clobbered. - No call-site change: migration is self-contained and gated on ledger ownership, not on the `force` param, so the Codex path keeps hard-overwriting as before. - New `tests/test_cli/test_serena_migrate.py`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ python -m pytest tests/test_cli/test_serena_migrate.py tests/test_cli/test_serena_disable.py tests/test_mcp_registry/ -q ============================== 89 passed in 4.26s ============================== $ ruff check headroom/cli/wrap.py tests/test_cli/test_serena_migrate.py All checks passed! ``` ## Real Behavior Proof - Environment: Fedora 44, Python 3.14.5, headroom working tree at this branch (off `upstream/main`), real `ClaudeRegistrar` (cli=None → file-backed), isolated `$HOME` + ledger via `tempfile` and `HEADROOM_WORKSPACE_DIR`. - Exact command / steps: wrote a pre-#1003 `serena` entry (no flag) into a throwaway `.claude/.claude.json`, recorded it in the ledger as Headroom-owned, then ran `_setup_serena_mcp(ClaudeRegistrar(claude_cli=None, home_dir=tmp), context="claude-code")`. Repeated with a `custom-serena` entry absent from the ledger. - Observed result: Headroom-owned entry rewritten on disk to end with `--open-web-dashboard False` (`migrated previously-installed entry` printed); user-managed `custom-serena` entry left byte-for-byte unchanged with the mismatch reported; fresh-install path writes the dashboard-off spec. Discovered originally on a live machine whose `~/.claude.json` kept the popup across re-wraps until the entry was hand-fixed — this PR removes the need for that. - Not tested: did not launch the Claude CLI end-to-end (the dashboard auto-open is Serena's documented response to `web_dashboard_open_on_launch=False`, traced in #1003); `mypy` not run. ## 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 - CHANGELOG / version: left to release-please (the repo's `fix:`-driven release PR aggregator), so no manual CHANGELOG edit. - Docs unchanged: behavior is internal to `headroom wrap`; the user-visible outcome (no dashboard popup) matches #1003's documented intent. - `mypy` not run locally (heavy dev extra pulls a compiled dep in this environment); happy to add the result if CI doesn't cover it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 16:17:13 -04:00
"""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),
)
feat(wrap): boost Serena — symbol-first guidance, wrap-time pre-index, repo-language scoping (#2425) When Serena is the active code-memory engine, `headroom wrap` now does three things (all best-effort, timeout-guarded, non-fatal, and fully inert when Serena/uvx are absent — mirroring the existing RTK/tokensave patterns): 1. **Symbol-first guidance** — injects a marker-guarded, idempotent block into the agent's hint file (`CLAUDE.md` for Claude; `AGENTS.md` for Codex/Grok/OpenCode) steering it to prefer Serena's `get_symbols_overview` / `find_symbol` / `find_referencing_symbols` / `find_declaration` over whole-file reads. This is the highest-leverage change — Serena only saves tokens if the agent actually uses it. 2. **Repo-language scoping** — detects the languages present in the repo (extension scan, pruning `.git`/`node_modules`/`.venv`/etc.) and pins them into `.serena/project.yml`'s `languages` list, so Serena doesn't spin up superfluous language servers. Conservative: only rewrites a single-line flow list or creates a minimal `project.yml`; a custom/block-style entry is left untouched to avoid corrupting hand-authored config. 3. **Wrap-time pre-index** — runs `serena project index` so the first symbol query isn't cold. Order is inject → scope → index (scope before index so the pre-index respects the scope). No new env vars, no settings_store drift, no behavior change outside the Serena path. The `languages` key and extension→language mapping were verified from Serena's local source (`project.template.yml`, `ProjectConfig`, `solidlsp/ls_config.py`), not the web. ## Testing New `tests/test_cli/test_wrap_serena_boost.py` (16 tests: injection idempotency + content, language detection incl. ignore-dirs, mocked pre-index/project.yml write incl. failure/timeout no-op). Updated `test_serena_migrate.py`'s fixture to neutralize the new side-effecting calls. Offline: 46 passed; ruff 0.15.17 + mypy clean.
2026-07-19 14:44:37 -07:00
# ``_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)
fix(serena): migrate stale Headroom-installed Serena entry on re-wrap (#1008) ## Description #1003 added `--open-web-dashboard False` to the Serena spec to stop the dashboard browser tab popping up on every session — but the flag only reaches **fresh** registrations. `register_server` returns `MISMATCH` and refuses to overwrite a differing entry unless `force=True`, and the Claude wrap path calls `_setup_serena_mcp` **without** force (unlike the Codex path, which passes `force=True`). So anyone wrapped before #1003 has a `serena` entry whose args lack the flag. Every re-wrap detects the mismatch, prints `existing config differs … To update: remove the existing serena MCP entry, then rerun`, and gives up — the stale spec, and the popup, persist forever. The fix never reaches already-wrapped users, which is most of them. This completes #1003 by migrating those stale entries in place. Related to #1003 ## 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 - `_setup_serena_mcp` now migrates a stale entry: on `MISMATCH` (and when not already forced), it force-updates to the current spec **only when the ledger proves Headroom installed the entry currently on disk** (`headroom_installed_matching`). Prints `Serena MCP: migrated previously-installed entry to current spec`. - A user-managed Serena (absent from the ledger) is left untouched and the mismatch is reported exactly as before — the same ownership check `--no-serena` / `_disable_serena_mcp` already use, so a hand-rolled Serena is never clobbered. - No call-site change: migration is self-contained and gated on ledger ownership, not on the `force` param, so the Codex path keeps hard-overwriting as before. - New `tests/test_cli/test_serena_migrate.py`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ python -m pytest tests/test_cli/test_serena_migrate.py tests/test_cli/test_serena_disable.py tests/test_mcp_registry/ -q ============================== 89 passed in 4.26s ============================== $ ruff check headroom/cli/wrap.py tests/test_cli/test_serena_migrate.py All checks passed! ``` ## Real Behavior Proof - Environment: Fedora 44, Python 3.14.5, headroom working tree at this branch (off `upstream/main`), real `ClaudeRegistrar` (cli=None → file-backed), isolated `$HOME` + ledger via `tempfile` and `HEADROOM_WORKSPACE_DIR`. - Exact command / steps: wrote a pre-#1003 `serena` entry (no flag) into a throwaway `.claude/.claude.json`, recorded it in the ledger as Headroom-owned, then ran `_setup_serena_mcp(ClaudeRegistrar(claude_cli=None, home_dir=tmp), context="claude-code")`. Repeated with a `custom-serena` entry absent from the ledger. - Observed result: Headroom-owned entry rewritten on disk to end with `--open-web-dashboard False` (`migrated previously-installed entry` printed); user-managed `custom-serena` entry left byte-for-byte unchanged with the mismatch reported; fresh-install path writes the dashboard-off spec. Discovered originally on a live machine whose `~/.claude.json` kept the popup across re-wraps until the entry was hand-fixed — this PR removes the need for that. - Not tested: did not launch the Claude CLI end-to-end (the dashboard auto-open is Serena's documented response to `web_dashboard_open_on_launch=False`, traced in #1003); `mypy` not run. ## 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 - CHANGELOG / version: left to release-please (the repo's `fix:`-driven release PR aggregator), so no manual CHANGELOG edit. - Docs unchanged: behavior is internal to `headroom wrap`; the user-visible outcome (no dashboard popup) matches #1003's documented intent. - `mypy` not run locally (heavy dev extra pulls a compiled dep in this environment); happy to add the result if CI doesn't cover it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 16:17:13 -04:00
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