diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index f51933537..50f84b1e5 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -679,6 +679,49 @@ _rtk_option = click.option( ) +def _serena_instructions_opt_in() -> bool: + """Whether Serena instruction injection into the agent's hint file is enabled. + + Injecting "prefer Serena symbol tools" guidance rewrites the user's + ``CLAUDE.md``/``AGENTS.md``, so it is opt-in (off by default): turn it on + with ``--serena-instructions`` (which sets ``HEADROOM_SERENA_INSTRUCTIONS=1``) + or by exporting ``HEADROOM_SERENA_INSTRUCTIONS=1``. Serena's ``.serena/``-only + setup (language scoping, pre-indexing) stays on by default regardless. + """ + return os.environ.get("HEADROOM_SERENA_INSTRUCTIONS", "").strip().lower() in ( + "1", + "true", + "yes", + "on", + ) + + +def _serena_instructions_flag_callback(ctx: Any, param: Any, value: bool) -> bool: + """Click eager callback: ``--serena-instructions`` sets + HEADROOM_SERENA_INSTRUCTIONS so the central gate + (:func:`_serena_instructions_opt_in`) sees the opt-in without threading a + param through every wrap subcommand.""" + if value: + os.environ["HEADROOM_SERENA_INSTRUCTIONS"] = "1" + return value + + +# Shared opt-in flag for Serena instruction injection, applied to the wrap +# subcommands that set up Serena. ``expose_value=False`` so no subcommand +# signature changes; it works purely through HEADROOM_SERENA_INSTRUCTIONS. Same +# approach as _rtk_option above — set via the callback with NO ``envvar=`` so the +# settings_store drift guard doesn't flag it. +_serena_instructions_option = click.option( + "--serena-instructions", + is_flag=True, + default=False, + expose_value=False, + is_eager=True, + callback=_serena_instructions_flag_callback, + help="Inject 'prefer Serena symbol tools' guidance into the agent's hint file (opt-in; off by default).", +) + + # --- Code-memory MCP selection ------------------------------------------------ # The code-memory MCP is on by default (tokensave). Swap it with --code-memory # serena, or turn it off with --code-memory none. Selection flows through @@ -1533,6 +1576,244 @@ def _ensure_serena_dashboard_disabled(*, verbose: bool = False) -> None: click.echo(f" Serena: could not update serena_config.yml ({e})") +# Marker-fenced guidance steering the agent toward Serena's symbol tools. +# Injected only when Serena is the active code-memory engine. Mirrors the RTK +# instruction block (idempotent, marker-guarded). +_SERENA_MARKER = "" + +SERENA_INSTRUCTIONS_BLOCK = """\ + +# Serena — Symbol-First Code Navigation + +Serena's MCP tools expose this project's code as a symbol graph backed by a +language server. **Prefer these tools over reading whole files** — they return +only the code you need, cutting context usage sharply. Read a file end-to-end +only when a symbol view is insufficient (non-code files, or when you need the +surrounding glue). + +## Preferred workflow +- `get_symbols_overview()` — list a file's top-level symbols before opening it. +- `find_symbol()` — fetch a symbol's definition/body instead of reading the file. +- `find_referencing_symbols()` — find call sites / usages instead of grepping. +- `find_declaration()` — jump to where a symbol is defined. + +## Rule +Reach for a symbol tool first; fall back to reading a whole file only when the +symbol view does not answer the question. + +""" + +# Ext → Serena language key. Values match the ``Language`` enum in Serena's +# solidlsp ``ls_config`` (the same keys accepted by ``.serena/project.yml``'s +# ``languages`` list). Only real programming languages are mapped — data/markup +# formats (json/yaml/toml/md/html/css) are intentionally skipped so Serena does +# not spin up language servers that add no symbol-navigation value. +_EXT_TO_SERENA_LANGUAGE: dict[str, str] = { + ".py": "python", + ".pyi": "python", + ".ts": "typescript", + ".tsx": "typescript", + ".mts": "typescript", + ".cts": "typescript", + ".js": "typescript", + ".jsx": "typescript", + ".mjs": "typescript", + ".cjs": "typescript", + ".go": "go", + ".rs": "rust", + ".java": "java", + ".kt": "kotlin", + ".kts": "kotlin", + ".rb": "ruby", + ".erb": "ruby", + ".cs": "csharp", + ".cpp": "cpp", + ".cc": "cpp", + ".cxx": "cpp", + ".c++": "cpp", + ".hpp": "cpp", + ".hh": "cpp", + ".hxx": "cpp", + ".c": "cpp", + ".h": "cpp", + ".php": "php", + ".swift": "swift", + ".dart": "dart", + ".scala": "scala", + ".sbt": "scala", + ".sh": "bash", + ".bash": "bash", + ".lua": "lua", + ".r": "r", + ".pl": "perl", + ".pm": "perl", + ".ex": "elixir", + ".exs": "elixir", + ".clj": "clojure", + ".cljs": "clojure", + ".cljc": "clojure", + ".elm": "elm", + ".tf": "terraform", + ".tfvars": "terraform", + ".zig": "zig", + ".nix": "nix", + ".hs": "haskell", + ".jl": "julia", + ".sol": "solidity", + ".vue": "vue", + ".svelte": "svelte", +} + +# Directories never worth scanning for language detection (VCS, dependencies, +# build output, virtualenvs, caches). Pruned in-place during the walk. +_LANG_SCAN_IGNORE_DIRS = frozenset( + {".git", "node_modules", ".venv", "venv", "dist", "build", "__pycache__"} +) + + +def _serena_instruction_file(registrar: Any) -> Path: + """Resolve the project instruction file the agent reads for guidance. + + Claude Code reads ``CLAUDE.md``; Codex, Grok, and OpenCode read + ``AGENTS.md``. Both live at the project root, mirroring the RTK instruction + targets. + """ + name = getattr(registrar, "name", "") or "" + filename = "CLAUDE.md" if name == "claude" else "AGENTS.md" + return Path.cwd() / filename + + +def _inject_serena_instructions(file_path: Path, verbose: bool = False) -> bool: + """Steer the agent toward Serena's symbol tools over whole-file reads. + + Opt-in (off by default): mirrors :func:`_inject_rtk_instructions` and + early-returns unless ``--serena-instructions`` / ``HEADROOM_SERENA_INSTRUCTIONS`` + is set, so the user's hint file is left untouched by default. + + Idempotent — skips if the marker is already present. Appends to an existing + instruction file, or creates one. Returns True once the guidance is in place. + """ + if not _serena_instructions_opt_in(): + return False + if file_path.exists(): + existing = _read_text(file_path) + if _SERENA_MARKER in existing: + if verbose: + click.echo(f" Serena instructions already in {file_path.name}") + return True + _append_text(file_path, "\n\n" + SERENA_INSTRUCTIONS_BLOCK) + else: + file_path.parent.mkdir(parents=True, exist_ok=True) + _write_text(file_path, SERENA_INSTRUCTIONS_BLOCK) + + click.echo(f" Serena instructions injected into {file_path}") + return True + + +def _detect_repo_languages(root: Path) -> list[str]: + """Detect the Serena languages present under *root* by file extension. + + Returns the mapped Serena language keys ordered by file count (most common + first — Serena treats the first entry as the default/fallback language + server), with ties broken alphabetically for determinism. Dependency, + build, VCS, and cache directories are pruned from the walk. + """ + counts: dict[str, int] = {} + for _dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames if d not in _LANG_SCAN_IGNORE_DIRS] + for filename in filenames: + lang = _EXT_TO_SERENA_LANGUAGE.get(Path(filename).suffix.lower()) + if lang is not None: + counts[lang] = counts.get(lang, 0) + 1 + return [lang for lang, _ in sorted(counts.items(), key=lambda kv: (-kv[1], kv[0]))] + + +def _scope_serena_languages(*, verbose: bool = False) -> None: + """Pin the repo's languages into ``.serena/project.yml`` (best-effort). + + Scoping the LSP to the languages actually present keeps Serena from + starting unnecessary language servers. Runs before indexing so + ``serena project index`` respects the scope. Writes the ``languages`` key as + a YAML flow list (the format Serena's own project template uses) via a + targeted line edit — mirroring :func:`_ensure_serena_dashboard_disabled` — + and creates a minimal ``project.yml`` (``project_name`` + ``languages``, the + only fields Serena requires) when absent. An existing block-style or + otherwise unexpected ``languages`` entry is left untouched rather than risk + corrupting the file. Non-fatal on any I/O error. + """ + languages = _detect_repo_languages(Path.cwd()) + if not languages: + if verbose: + click.echo(" Serena: no recognized source languages detected — leaving scope unset") + return + + cfg = Path.cwd() / ".serena" / "project.yml" + value = "[" + ", ".join(f'"{lang}"' for lang in languages) + "]" + try: + if cfg.exists(): + text = _read_text(cfg) + # Match only a single-line flow list (the format we and Serena write). + pattern = re.compile(r"^(\s*)languages:\s*\[[^\]\n]*\]\s*$", re.MULTILINE) + if pattern.search(text): + new = pattern.sub(rf"\g<1>languages: {value}", text, count=1) + if new != text: + _write_text(cfg, new) + if verbose: + click.echo(f" Serena: scoped languages to {value} (project.yml)") + elif verbose: + click.echo( + " Serena: project.yml has a custom languages entry — leaving it untouched" + ) + else: + cfg.parent.mkdir(parents=True, exist_ok=True) + project_name = Path.cwd().name or "project" + _write_text(cfg, f'project_name: "{project_name}"\nlanguages: {value}\n') + if verbose: + click.echo(f" Serena: created project.yml scoped to {value}") + except OSError as e: + if verbose: + click.echo(f" Serena: could not scope languages ({e})") + + +def _index_serena_project(*, verbose: bool = False) -> None: + """Warm Serena's symbol cache for the current project (non-fatal). + + Runs ``serena project index`` (the same ``uvx --from git+…`` launch used to + start the MCP server) in the project directory so the first symbol query is + not paying for a cold index. Timeout-guarded and best-effort: Serena also + indexes lazily on demand, so a failure or timeout here never blocks the + wrap. Mirrors :func:`_index_tokensave_project`. + """ + if shutil.which("uvx") is None: + if verbose: + click.echo(" Serena: uvx not found — skipping pre-index") + return + try: + result = run( + [ + "uvx", + "--from", + "git+https://github.com/oraios/serena", + "serena", + "project", + "index", + ], + capture_output=True, + text=True, + timeout=300, + cwd=str(Path.cwd()), + ) + if result.returncode == 0: + click.echo(" Serena: project pre-indexed (symbol cache warmed)") + elif verbose: + click.echo(f" Serena: pre-index failed ({(result.stderr or '')[:100]})") + except subprocess.TimeoutExpired: + click.echo(" Serena: pre-index timed out (will index on demand)") + except Exception as e: + if verbose: + click.echo(f" Serena: pre-index skipped ({e})") + + def _setup_serena_mcp( registrar: Any, *, context: str, verbose: bool = False, force: bool = False ) -> None: @@ -1595,6 +1876,15 @@ def _setup_serena_mcp( if line is not None: click.echo(line) + # Serena is the active engine here (we passed the detect/uvx guards): steer + # the agent toward symbol-level tools, scope the LSP to the repo's + # languages, then warm the symbol cache. Scoping runs before indexing so + # ``serena project index`` respects the scope. Each step is best-effort and + # non-fatal — none of them block the wrap. + _inject_serena_instructions(_serena_instruction_file(registrar), verbose=verbose) + _scope_serena_languages(verbose=verbose) + _index_serena_project(verbose=verbose) + def _remove_headroom_installed_serena_mcp(registrar: Any) -> str: """Remove Serena MCP only if the ledger proves Headroom installed it.""" @@ -4559,6 +4849,7 @@ def wrap_selfheal(marker: str | None) -> None: @wrap.command(context_settings={"ignore_unknown_options": True}) @_rtk_option +@_serena_instructions_option @click.option( # no "-p" short alias here: claude's own -p/--print must fall through to CLAUDE_ARGS "--port", @@ -5636,6 +5927,7 @@ def _run_codex_wrap( @wrap.command(context_settings={"ignore_unknown_options": True}) @_rtk_option +@_serena_instructions_option @click.option( "--port", "-p", default=8787, type=click.IntRange(1, 65535), help="Proxy port (default: 8787)" ) @@ -6138,6 +6430,7 @@ def kimi( @wrap.command(context_settings={"ignore_unknown_options": True}) @_rtk_option +@_serena_instructions_option @click.option( "--port", "-p", default=8787, type=click.IntRange(1, 65535), help="Proxy port (default: 8787)" ) @@ -7329,6 +7622,7 @@ def openclaw( @wrap.command(context_settings={"ignore_unknown_options": True}) @_rtk_option +@_serena_instructions_option @click.option( "--port", "-p", default=8787, type=click.IntRange(1, 65535), help="Proxy port (default: 8787)" ) diff --git a/tests/test_cli/test_serena_migrate.py b/tests/test_cli/test_serena_migrate.py index 871ce2392..947930778 100644 --- a/tests/test_cli/test_serena_migrate.py +++ b/tests/test_cli/test_serena_migrate.py @@ -82,6 +82,13 @@ def _isolate_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: "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, "_scope_serena_languages", lambda *a, **k: None) + monkeypatch.setattr(wrap_cli, "_index_serena_project", lambda *a, **k: None) def test_rewrap_migrates_stale_headroom_serena( diff --git a/tests/test_cli/test_wrap_serena_boost.py b/tests/test_cli/test_wrap_serena_boost.py new file mode 100644 index 000000000..49dc7fe09 --- /dev/null +++ b/tests/test_cli/test_wrap_serena_boost.py @@ -0,0 +1,260 @@ +"""Serena "boost" wrap-time helpers: prefer-Serena instruction injection, +repo-language scoping of ``.serena/project.yml``, and symbol-cache pre-indexing. + +All Serena subprocess calls are mocked — these tests never invoke real ``uvx``. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path +from unittest.mock import Mock + +import pytest + +from headroom.cli import wrap as wrap_cli + +# --------------------------------------------------------------------------- +# _inject_serena_instructions +# --------------------------------------------------------------------------- + + +def _opt_in(monkeypatch: pytest.MonkeyPatch) -> None: + """Enable the opt-in gate so injection actually writes. + + Instruction injection rewrites the user's CLAUDE.md/AGENTS.md, so it is + off by default (mirrors RTK). Tests that exercise the write path must opt + in via ``HEADROOM_SERENA_INSTRUCTIONS``. + """ + monkeypatch.setenv("HEADROOM_SERENA_INSTRUCTIONS", "1") + + +def test_inject_creates_file_and_mentions_tools( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _opt_in(monkeypatch) + target = tmp_path / "AGENTS.md" + assert wrap_cli._inject_serena_instructions(target) is True + + content = target.read_text() + assert wrap_cli._SERENA_MARKER in content + # The whole point is steering the agent toward Serena's symbol tools. + for tool in ("get_symbols_overview", "find_symbol", "find_referencing_symbols"): + assert tool in content, f"{tool} missing from injected guidance" + + +def test_inject_is_idempotent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _opt_in(monkeypatch) + target = tmp_path / "AGENTS.md" + wrap_cli._inject_serena_instructions(target) + wrap_cli._inject_serena_instructions(target) # second call is a no-op + + content = target.read_text() + assert content.count(wrap_cli._SERENA_MARKER) == 1 + + +def test_inject_appends_to_existing_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _opt_in(monkeypatch) + target = tmp_path / "CLAUDE.md" + target.write_text("# Project notes\n\nkeep me\n") + wrap_cli._inject_serena_instructions(target) + + content = target.read_text() + assert "keep me" in content # existing content preserved + assert wrap_cli._SERENA_MARKER in content + + +def test_inject_off_by_default_writes_nothing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # Without opting in, injection is a no-op: returns False and never touches + # the user's hint file (the default, so the two OpenCode AGENTS.md tests pass). + monkeypatch.delenv("HEADROOM_SERENA_INSTRUCTIONS", raising=False) + + missing = tmp_path / "AGENTS.md" + assert wrap_cli._inject_serena_instructions(missing) is False + assert not missing.exists() # nothing created + + existing = tmp_path / "CLAUDE.md" + existing.write_text("# Project notes\n\nkeep me\n") + assert wrap_cli._inject_serena_instructions(existing) is False + assert existing.read_text() == "# Project notes\n\nkeep me\n" # untouched + assert wrap_cli._SERENA_MARKER not in existing.read_text() + + +def test_instruction_file_target_per_agent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + + class _Reg: + def __init__(self, name: str) -> None: + self.name = name + + assert wrap_cli._serena_instruction_file(_Reg("claude")).name == "CLAUDE.md" + assert wrap_cli._serena_instruction_file(_Reg("codex")).name == "AGENTS.md" + assert wrap_cli._serena_instruction_file(_Reg("grok")).name == "AGENTS.md" + + +# --------------------------------------------------------------------------- +# _detect_repo_languages +# --------------------------------------------------------------------------- + + +def test_detect_maps_extensions_to_serena_languages(tmp_path: Path) -> None: + (tmp_path / "app.py").write_text("print(1)\n") + (tmp_path / "web.ts").write_text("export const x = 1\n") + (tmp_path / "main.go").write_text("package main\n") + + assert set(wrap_cli._detect_repo_languages(tmp_path)) == {"python", "typescript", "go"} + + +def test_detect_ignores_deps_and_venv(tmp_path: Path) -> None: + (tmp_path / "app.py").write_text("print(1)\n") + # Languages that appear ONLY inside ignored dirs must not be reported. + (tmp_path / "node_modules").mkdir() + (tmp_path / "node_modules" / "dep.rs").write_text("fn main() {}\n") + (tmp_path / ".venv").mkdir() + (tmp_path / ".venv" / "lib.rb").write_text("puts 1\n") + + detected = set(wrap_cli._detect_repo_languages(tmp_path)) + assert detected == {"python"} + assert "rust" not in detected + assert "ruby" not in detected + + +def test_detect_orders_by_file_count(tmp_path: Path) -> None: + for i in range(3): + (tmp_path / f"m{i}.py").write_text("x = 1\n") + (tmp_path / "main.go").write_text("package main\n") + + ordered = wrap_cli._detect_repo_languages(tmp_path) + assert ordered[0] == "python" # most files → default/fallback language first + + +def test_detect_empty_when_no_source(tmp_path: Path) -> None: + (tmp_path / "README.md").write_text("# hi\n") # markup, not mapped + assert wrap_cli._detect_repo_languages(tmp_path) == [] + + +# --------------------------------------------------------------------------- +# _scope_serena_languages — pins languages into .serena/project.yml +# --------------------------------------------------------------------------- + + +def test_scope_creates_project_yml_when_absent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + (tmp_path / "app.py").write_text("print(1)\n") + + wrap_cli._scope_serena_languages() + + cfg = tmp_path / ".serena" / "project.yml" + assert cfg.exists() + text = cfg.read_text() + assert 'languages: ["python"]' in text + assert "project_name:" in text # required field written too + + +def test_scope_updates_existing_inline_languages( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + (tmp_path / "app.py").write_text("print(1)\n") + (tmp_path / "main.go").write_text("package main\n") + cfg = tmp_path / ".serena" / "project.yml" + cfg.parent.mkdir(parents=True) + cfg.write_text('project_name: "demo"\nlanguages: ["python"]\nencoding: "utf-8"\n') + + wrap_cli._scope_serena_languages() + + text = cfg.read_text() + # go + python (one file each → alphabetical tie-break), inline flow list. + assert 'languages: ["go", "python"]' in text + assert 'encoding: "utf-8"' in text # other keys preserved + + +def test_scope_leaves_block_style_untouched( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + (tmp_path / "app.py").write_text("print(1)\n") + cfg = tmp_path / ".serena" / "project.yml" + cfg.parent.mkdir(parents=True) + original = 'project_name: "demo"\nlanguages:\n- typescript\n' + cfg.write_text(original) + + wrap_cli._scope_serena_languages() + + # Block-style list is not something our single-line edit can safely touch, + # so it is left exactly as-is rather than corrupted. + assert cfg.read_text() == original + + +def test_scope_noop_when_no_languages(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + (tmp_path / "README.md").write_text("# hi\n") + + wrap_cli._scope_serena_languages() + + assert not (tmp_path / ".serena" / "project.yml").exists() + + +# --------------------------------------------------------------------------- +# _index_serena_project — best-effort, timeout-guarded pre-index +# --------------------------------------------------------------------------- + + +def _stub_uvx(monkeypatch: pytest.MonkeyPatch, present: bool = True) -> None: + monkeypatch.setattr( + wrap_cli.shutil, + "which", + lambda name, *a, **k: "/usr/bin/uvx" if (present and name == "uvx") else None, + ) + + +def test_preindex_runs_serena_in_cwd(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + _stub_uvx(monkeypatch) + mock_run = Mock( + return_value=subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + ) + monkeypatch.setattr(wrap_cli, "run", mock_run) + + wrap_cli._index_serena_project() + + mock_run.assert_called_once() + args, kwargs = mock_run.call_args + cmd = args[0] + assert cmd[0] == "uvx" + assert cmd[-3:] == ["serena", "project", "index"] + assert "git+https://github.com/oraios/serena" in cmd + assert kwargs["cwd"] == str(tmp_path) # invoked in the project cwd + assert "timeout" in kwargs # timeout-guarded + + +def test_preindex_skips_without_uvx(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_uvx(monkeypatch, present=False) + mock_run = Mock(side_effect=AssertionError("run must not be called without uvx")) + monkeypatch.setattr(wrap_cli, "run", mock_run) + + wrap_cli._index_serena_project() # no exception + + mock_run.assert_not_called() + + +def test_preindex_timeout_is_non_fatal(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_uvx(monkeypatch) + monkeypatch.setattr( + wrap_cli, + "run", + Mock(side_effect=subprocess.TimeoutExpired(cmd="serena", timeout=1)), + ) + # Must not propagate. + wrap_cli._index_serena_project(verbose=True) + + +def test_preindex_generic_error_is_non_fatal(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_uvx(monkeypatch) + monkeypatch.setattr(wrap_cli, "run", Mock(side_effect=RuntimeError("boom"))) + # Must not propagate. + wrap_cli._index_serena_project(verbose=True)