mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description Headroom repeatedly warns about user-managed Serena drift but has no scoped remediation command. Add a Claude-only read-only mcp reconcile command with explicit --adopt consent, using the canonical Serena spec and existing Claude registrar. Adoption validates every relevant ledger and Claude config root before mutation, writes only the Serena entry, and records ownership after the config write succeeds. Automatic wrap migration and ordinary install remain unchanged. Closes #3054 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (feature that would cause existing behavior to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Add Claude-only `headroom mcp reconcile`, read-only by default, with `--adopt` as its only mutation action. - Reuse the shared `CLAUDE_SERENA_CONTEXT` and canonical Claude Serena spec builder. - Fail closed on malformed or unreadable ledger/config state before adoption. - Preserve automatic wrap recovery, user-managed warnings, ordinary `mcp install --force`, unrelated Claude config, and corrupt-ledger tolerance outside explicit adoption. - Record Headroom ownership only after a successful registrar write. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_cli/test_mcp_reconcile.py tests/test_cli/test_serena_reconcile.py tests/test_mcp_registry/test_ledger.py`) - [x] Linting passes (`uv run ruff check .`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed through the file-backed Claude registrar ### Test Output ```text uv run pytest tests/test_cli/test_mcp_reconcile.py tests/test_mcp_registry/test_ledger.py tests/test_cli/test_serena_reconcile.py tests/test_mcp_registry/test_claude_registrar.py tests/test_mcp_registry/test_install.py -q 102 passed in 0.70s uv run ruff check headroom/mcp_registry/ledger.py headroom/cli/wrap.py tests/test_mcp_registry/test_ledger.py tests/test_cli/test_serena_reconcile.py tests/test_cli/test_mcp_reconcile.py All checks passed! uv run ruff format --check headroom/mcp_registry/ledger.py headroom/cli/wrap.py tests/test_mcp_registry/test_ledger.py tests/test_cli/test_serena_reconcile.py tests/test_cli/test_mcp_reconcile.py 5 files already formatted git diff --check ``` ## Real Behavior Proof - Environment: Windows, file-backed Claude configuration and isolated MCP ledger. - Exact command / steps: run the stale user-managed Serena fixture from `tests/fixtures/headroom-issue-3054.json`; run read-only reconcile; run `mcp reconcile --adopt`; rerun wrap and ordinary `mcp install --force`; exercise malformed JSON, non-dict `mcpServers`, null ledger agents, and unreadable-ledger adoption. - Observed result: read-only reconciliation leaves config and ledger bytes unchanged; adoption updates only Claude Serena and records ownership after a successful write; automatic wrap remains lenient; unsafe adoption inputs leave all files unchanged; ordinary install does not adopt Serena. - Not tested: live Claude CLI acceptance and Serena stdio handshake ## Runtime Rollout Safety - Rollout-managed feature(s): None; explicit `mcp reconcile --adopt` is the only mutation path. - Minimum rollout channel: Stable; no staged rollout mechanism exists for this command. - Stable/default behavior changed: No, read-only reconcile is the default and automatic wrap plus ordinary install remain unchanged. - Kill switch / disable path: Do not invoke `--adopt` or revert the release commit. - Unsafe override required: No. - Qualification impact: None. - Rollback path: Revert the release commit. ## 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 - [x] 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 - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The changelog is generated by the release pipeline. This change is limited to Claude Serena reconciliation and does not add a new persistent acknowledgement state or a multi-provider adoption route.
124 lines
4.9 KiB
Python
124 lines
4.9 KiB
Python
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
|
|
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
|
|
|
|
|
|
class _Registrar:
|
|
display_name = "Claude Code"
|
|
|
|
def __init__(self, current: ServerSpec | None, *, name: str = "claude"):
|
|
self.name = name
|
|
self.current = current
|
|
self.force_calls: list[bool] = []
|
|
|
|
def detect(self) -> bool:
|
|
return True
|
|
|
|
def get_server(self, name: str) -> ServerSpec | None:
|
|
return self.current if name == "serena" else None
|
|
|
|
def register_server(self, spec: ServerSpec, *, force: bool = False) -> RegisterResult:
|
|
self.force_calls.append(force)
|
|
if self.current == spec:
|
|
return RegisterResult(RegisterStatus.ALREADY, "matches")
|
|
if self.current is not None and not force:
|
|
return RegisterResult(RegisterStatus.MISMATCH, "different")
|
|
self.current = spec
|
|
return RegisterResult(RegisterStatus.REGISTERED, "updated")
|
|
|
|
|
|
def _quiet(monkeypatch):
|
|
monkeypatch.setattr(wrap_cli, "_ensure_serena_dashboard_disabled", lambda **kwargs: None)
|
|
monkeypatch.setattr(wrap_cli, "_inject_serena_instructions", lambda *args, **kwargs: None)
|
|
monkeypatch.setattr(wrap_cli, "_serena_project_skip_reason", lambda root: "test")
|
|
monkeypatch.setattr(wrap_cli, "_index_serena_project", lambda **kwargs: None)
|
|
monkeypatch.setattr(wrap_cli.shutil, "which", lambda name: "uvx" if name == "uvx" else None)
|
|
|
|
|
|
def test_automatic_wrap_migrates_owned_drift_and_recurs_to_noop(
|
|
monkeypatch, tmp_path: Path, capsys
|
|
):
|
|
_quiet(monkeypatch)
|
|
monkeypatch.setattr(
|
|
"headroom.mcp_registry.ledger.ledger_path", lambda: tmp_path / "ledger.json"
|
|
)
|
|
stale = ServerSpec("serena", "uvx", ("--from", "old"))
|
|
record_install("claude", stale)
|
|
registrar = _Registrar(stale)
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
assert registrar.current == build_serena_spec("claude-code")
|
|
assert registrar.force_calls == [False, True]
|
|
assert headroom_installed_matching("claude", registrar.current)
|
|
capsys.readouterr()
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
assert registrar.force_calls == [False, True, False]
|
|
|
|
|
|
def test_automatic_wrap_owned_drift_suggests_rerun_wrap(monkeypatch, tmp_path: Path, capsys):
|
|
_quiet(monkeypatch)
|
|
monkeypatch.setattr(
|
|
"headroom.mcp_registry.ledger.ledger_path", lambda: tmp_path / "ledger.json"
|
|
)
|
|
stale = ServerSpec("serena", "uvx", ("--from", "old"))
|
|
record_install("claude", stale)
|
|
|
|
class _FailedMigrationRegistrar(_Registrar):
|
|
def register_server(self, spec, *, force=False):
|
|
if force:
|
|
self.force_calls.append(force)
|
|
return RegisterResult(RegisterStatus.MISMATCH, "still different")
|
|
return super().register_server(spec, force=force)
|
|
|
|
wrap_cli._setup_serena_mcp(
|
|
_FailedMigrationRegistrar(stale), context="claude-code", verbose=True
|
|
)
|
|
|
|
output = capsys.readouterr().out
|
|
assert "run headroom wrap again" in output
|
|
assert "mcp reconcile --adopt" not in output
|
|
|
|
|
|
def test_automatic_wrap_preserves_user_managed_warning(monkeypatch, tmp_path: Path, capsys):
|
|
_quiet(monkeypatch)
|
|
monkeypatch.setattr(
|
|
"headroom.mcp_registry.ledger.ledger_path", lambda: tmp_path / "ledger.json"
|
|
)
|
|
user = ServerSpec("serena", "uvx", ("--from", "user"))
|
|
registrar = _Registrar(user)
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
assert registrar.current == user
|
|
assert registrar.force_calls == [False]
|
|
assert "existing config differs" in capsys.readouterr().out
|
|
|
|
|
|
def test_automatic_wrap_recovers_from_malformed_ledger(monkeypatch, tmp_path: Path):
|
|
_quiet(monkeypatch)
|
|
ledger = tmp_path / "ledger.json"
|
|
ledger.write_text("not json")
|
|
monkeypatch.setattr("headroom.mcp_registry.ledger.ledger_path", lambda: ledger)
|
|
registrar = _Registrar(None)
|
|
|
|
wrap_cli._setup_serena_mcp(registrar, context="claude-code", verbose=True)
|
|
|
|
current = registrar.get_server("serena")
|
|
assert current == build_serena_spec("claude-code")
|
|
assert headroom_installed_matching("claude", current)
|
|
|
|
|
|
def test_non_claude_wrap_keeps_usable_remediation_hint(monkeypatch, tmp_path: Path, capsys):
|
|
_quiet(monkeypatch)
|
|
monkeypatch.setattr(
|
|
"headroom.mcp_registry.ledger.ledger_path", lambda: tmp_path / "ledger.json"
|
|
)
|
|
registrar = _Registrar(ServerSpec("serena", "uvx", ("--from", "user")), name="codex")
|
|
|
|
wrap_cli._setup_serena_mcp(registrar, context="codex", verbose=True)
|
|
|
|
output = capsys.readouterr().out
|
|
assert "update or remove the existing serena MCP entry" in output
|
|
assert "mcp reconcile --adopt" not in output
|