From 759209cff3daa72dd9d47e57568e731d10573d63 Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Thu, 30 Jul 2026 20:55:47 -0700 Subject: [PATCH] fix(wrap/serena): stop creating serena_config.yml, unbricking Serena on fresh installs (#2676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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= 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. --- .serena/project.yml | 132 ++++++++----- headroom/cli/wrap.py | 239 ++++++++--------------- tests/conftest.py | 24 +++ tests/test_cli/test_serena_migrate.py | 1 - tests/test_cli/test_wrap_serena_boost.py | 109 +---------- tests/test_wrap_code_memory.py | 100 +++++++++- 6 files changed, 285 insertions(+), 320 deletions(-) diff --git a/.serena/project.yml b/.serena/project.yml index 0e9ac3ead..bc005c5fc 100644 --- a/.serena/project.yml +++ b/.serena/project.yml @@ -1,38 +1,5 @@ -# the name by which the project can be referenced within Serena -project_name: "feature-opencode-wrap" - - -# list of languages for which language servers are started; choose from: -# al angular ansible bash clojure -# cpp cpp_ccls crystal csharp csharp_omnisharp -# dart elixir elm erlang fortran -# fsharp go groovy haskell haxe -# hlsl html java json julia -# kotlin lean4 lua luau markdown -# matlab msl nix ocaml pascal -# perl php php_phpactor powershell python -# python_jedi python_ty r rego ruby -# ruby_solargraph rust scala scss solidity -# svelte swift systemverilog terraform toml -# typescript typescript_vts vue yaml zig -# (This list may be outdated. For the current list, see values of Language enum here: -# https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py -# For some languages, there are alternative language servers, e.g. csharp_omnisharp, ruby_solargraph.) -# Note: -# - For C, use cpp -# - For JavaScript, use typescript -# - For Angular projects, use angular (subsumes typescript+html; requires `npm install` in the project root) -# - For Svelte projects, use svelte (subsumes typescript/javascript for .svelte projects; requires npm) -# - For SCSS / Sass / plain CSS, use scss (some-sass-language-server handles all three) -# - For Free Pascal/Lazarus, use pascal -# Special requirements: -# Some languages require additional setup/installations. -# See here for details: https://oraios.github.io/serena/01-about/020_programming-languages.html#language-servers -# When using multiple languages, the first language server that supports a given file will be used for that file. -# The first language is the default language and the respective language server will be used as a fallback. -# Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored. -languages: -- typescript +# the name by which the project can be referenced within Serena/when chatting with the LLM. +project_name: "headroom" # the encoding used by text files in the project # For a list of possible encodings, see https://docs.python.org/3.11/library/codecs.html#standard-encodings @@ -55,23 +22,19 @@ ignore_all_files_in_gitignore: true # advanced configuration option allowing to configure language server-specific options. # Maps the language key to the options. -# Have a look at the docstring of the constructors of the LS implementations within solidlsp (e.g., for C# or PHP) to see which options are available. -# No documentation on options means no options are available. +# The settings are considered only if the project is trusted (see global configuration to define trusted projects). +# See https://oraios.github.io/serena/02-usage/050_configuration.html#language-server-specific-settings ls_specific_settings: {} -# list of additional workspace folder paths for cross-package reference support (e.g. in monorepos). -# Paths can be absolute or relative to the project root. -# Each folder is registered as an LSP workspace folder, enabling language servers to discover -# symbols and references across package boundaries. -# Currently supported for: TypeScript. -# Example: -# additional_workspace_folders: -# - ../sibling-package -# - ../shared-lib -additional_workspace_folders: [] - # list of additional paths to ignore in this project. # Same syntax as gitignore, so you can use * and **. +# Important: quote patterns that start with `*`, otherwise YAML treats them as aliases. +# Example: +# ignored_paths: +# - "examples/**" +# - ".worktrees/**" +# - "**/bin/**" +# - "**/obj/**" # Note: global ignored_paths from serena_config.yml are also applied additively. ignored_paths: [] @@ -131,3 +94,76 @@ read_only_memory_patterns: [] # Extends the list from the global configuration, merging the two lists. # Example: ["_archive/.*", "_episodes/.*"] ignored_memory_patterns: [] + +# list of additional workspace folder paths for cross-package reference support. +# Paths can be absolute or relative to the project root. +# Each folder is registered as an LSP workspace folder, enabling language servers to discover +# symbols and references across package boundaries, but these folders are not indexed by Serena, +# i.e. the respective symbols will not be found using Serena's symbol search tools. +# Example: +# additional_workspace_folders: +# - ../sibling-package +# - ../shared-lib +ls_additional_workspace_folders: [] + +# list of language servers to start when using the LSP backend; choose from: +# ada al angular ansible bash +# bsl clojure cpp cpp_ccls crystal +# csharp csharp_omnisharp cue dart elixir +# elm erlang fortran fsharp gdscript +# go groovy haskell haxe hlsl +# html java json julia kotlin +# latex lean4 lua luau markdown +# matlab msl nix ocaml pascal +# perl php php_phpactor php_phpantom powershell +# python python_basedpyright python_jedi python_pyrefly python_ty +# qml r rego ruby ruby_solargraph +# rust scala scss solidity svelte +# swift systemverilog terraform toml typescript +# typescript_vts vue yaml zig +# (This list may be outdated; generated with scripts/print_language_list.py; +# For the current list, see values of the LanguageServerId enum here: +# https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py) +# For some languages, there are several alternative language servers, e.g. csharp_omnisharp, ruby_solargraph.) +# Note: +# - For C, use cpp +# - For JavaScript, use typescript +# - For Angular projects, use angular (subsumes typescript+html; requires `npm install` in the project root) +# - For Svelte projects, use svelte (subsumes typescript/javascript for .svelte projects; requires npm) +# - For SCSS / Sass / plain CSS, use scss (some-sass-language-server handles all three) +# - For Free Pascal/Lazarus, use pascal +# Special requirements: +# Some language servers require additional setup/installations. +# See here for details: https://oraios.github.io/serena/01-about/020_programming-languages.html#language-servers +# When using multiple language servers, the first language server that supports a given file will be used for that file. +# The first language server is the default language and the respective language server will be used as a fallback. +# Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored. +language_servers: +- python +- rust +- typescript + +# list of workspace folder paths (LSP backend only). +# These folders will be used to build up Serena's symbol index. +# Paths must be within the project root and should thus be relative to the project root. +# Furthermore, the paths should not be filtered by ignore settings. +# Default setting: The entire project root folder (".") is considered. +# In (large) monorepos, this can be used to index only subfolders of the project root, e.g. +# ls_workspace_folders: +# - "./subproject1" +# - "./subproject2" +ls_workspace_folders: +- . + +# optional shell command to run before the language backend (LSP or JetBrains) is initialised. +# the command runs in the project root directory and is only executed if the project is trusted +# (see trusted_project_path_patterns in the global configuration). +# serena waits for the command to exit: a non-zero exit code is logged as an error but does not +# abort activation. a per-project timeout (activation_command_timeout, default 180s) is the safety +# backstop for non-terminating commands; on expiry the process is killed and activation continues. +# example: activation_command: "npx nx run-many -t build" +activation_command: + +# maximum time in seconds to wait for activation_command to complete before killing it (default 180s). +# must be a positive number. +activation_command_timeout: 180.0 diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index 9262e82aa..0afd8ec65 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -1607,38 +1607,79 @@ def _ensure_serena_dashboard_disabled(*, verbose: bool = False) -> None: """Disable Serena's browser dashboard auto-open in ``~/.serena/serena_config.yml``. Serena opens its web dashboard in a browser tab on launch by default - (``web_dashboard_open_on_launch: true``). Since Headroom now registers Serena - as the default code-memory MCP, flip that setting off so wrapped sessions - don't spawn a browser tab. The dashboard backend still runs and stays - reachable at http://localhost:24282/dashboard/. The setting lives in Serena's - own config (authoritative, unlike a startup flag); other keys and comments are - preserved via a targeted line edit rather than a YAML rewrite. + (``web_dashboard_open_on_launch: true``), so flip that off for users who run + Serena outside Headroom. The dashboard backend still runs and stays reachable + at http://localhost:24282/dashboard/. Other keys and comments are preserved + via a targeted line edit rather than a YAML rewrite. + + **Never creates the file.** Verified against Serena 1.6.2.dev0 + (``serena/config/serena_config.py``): Serena autogenerates its own complete + config only when the path does *not* exist (``if not + os.path.exists(config_file_path): cls._generate_config_file(...)``, ~line + 1033). Once any file exists it validates instead of filling gaps, and while + every other field falls back to a dataclass default via + ``get_value_or_default``, a missing ``projects`` key is fatal (~line 1064): + + SerenaConfigError: `projects` key not found in Serena configuration. + + So Headroom writing its own bootstrap file bricked Serena on every machine + without a pre-existing config — the MCP server died mid-handshake ("connection + closed: initialize response" on Codex, bare ``MCP error -32000`` on OpenCode) + and ``serena project index`` failed identically (#2674). Letting Serena + generate the file is immune to Serena adding required keys later; guessing the + schema is what caused the outage. + + Suppressing the popup does not need this 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. + + ``projects: []`` is still backfilled into an *existing* file, to repair + configs an affected Headroom version already wrote. """ import re cfg = Path.home() / ".serena" / "serena_config.yml" key = "web_dashboard_open_on_launch" + if not cfg.exists(): + # Let Serena bootstrap its own valid config; the MCP flag handles the popup. + if verbose: + click.echo(" Serena: no serena_config.yml yet — letting Serena generate it") + return try: - if cfg.exists(): - text = cfg.read_text(encoding="utf-8") - pattern = re.compile(rf"^(\s*){re.escape(key)}:\s*\S+\s*$", re.MULTILINE) - if pattern.search(text): - new = pattern.sub(rf"\g<1>{key}: false", text) - else: - new = text.rstrip("\n") + f"\n{key}: false\n" - if new != text: - cfg.write_text(new, encoding="utf-8") - if verbose: - click.echo(" Serena: disabled dashboard browser auto-open (serena_config.yml)") - else: - cfg.parent.mkdir(parents=True, exist_ok=True) - # Serena fills defaults for any keys we omit, so a single-key file is valid. - cfg.write_text(f"{key}: false\n", encoding="utf-8") - if verbose: - click.echo(" Serena: created serena_config.yml with dashboard auto-open off") + text = cfg.read_text(encoding="utf-8") + except OSError as e: + if verbose: + click.echo(f" Serena: could not read serena_config.yml ({e})") + return + + new = text + appended: list[str] = [] + + dashboard = re.compile(rf"^(\s*){re.escape(key)}:\s*\S+\s*$", re.MULTILINE) + if dashboard.search(new): + new = dashboard.sub(rf"\g<1>{key}: false", new) + else: + appended.append(f"{key}: false") + + # Repair a config left by an affected Headroom version (see #2674 above). + if not re.search(r"^\s*projects\s*:", new, re.MULTILINE): + appended.append("projects: []") + + if appended: + body = new.rstrip("\n") + new = (f"{body}\n" if body.strip() else "") + "\n".join(appended) + "\n" + + if new == text: + return + try: + cfg.write_text(new, encoding="utf-8") except OSError as e: if verbose: click.echo(f" Serena: could not update serena_config.yml ({e})") + return + if verbose: + click.echo(" Serena: updated serena_config.yml (dashboard auto-open off)") # Marker-fenced guidance steering the agent toward Serena's symbol tools. @@ -1668,73 +1709,6 @@ 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. @@ -1775,71 +1749,6 @@ def _inject_serena_instructions(file_path: Path, verbose: bool = False) -> bool: 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 _serena_project_skip_reason(root: Path) -> str | None: """Why Serena's per-project setup must not run for *root* (None = proceed). @@ -1963,17 +1872,25 @@ def _setup_serena_mcp( 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. + # the agent toward symbol-level tools, then warm the symbol cache. Both are + # best-effort and non-fatal — neither blocks the wrap. + # + # Headroom no longer writes ``.serena/project.yml`` language scoping. Serena + # determines the project's languages itself during + # ``ProjectConfig.autogenerate`` (``_determine_project_language_servers``), + # and it records them under ``language_servers`` — ``languages`` is a legacy + # name it migrates via ``RENAMED_FIELDS``. Our scoping therefore no-op'd on + # any Serena-generated project.yml (wrong key, block-style list) and only did + # anything when it created the file itself, which is the same partial-config + # trap as #2674 — and skipped the ``project.local.yml`` sidecar Serena writes + # alongside. Letting Serena own that file removes a hand-maintained ext→ + # language map that duplicated its detection. _inject_serena_instructions(_serena_instruction_file(registrar), verbose=verbose) skip_reason = _serena_project_skip_reason(Path.cwd()) if skip_reason is not None: if verbose: - click.echo(f" Serena: skipping language scope + pre-index ({skip_reason})") + click.echo(f" Serena: skipping pre-index ({skip_reason})") return - _scope_serena_languages(verbose=verbose) _index_serena_project(verbose=verbose) diff --git a/tests/conftest.py b/tests/conftest.py index a9ccee755..38de51499 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -30,6 +30,30 @@ def _scrub_developer_headroom_env(monkeypatch): monkeypatch.delenv("ANTHROPIC_CUSTOM_HEADERS", raising=False) +# The MCP install ledger defaults to ``~/.headroom/mcp_installs.json``, so any +# test that registers a server (directly or through `wrap`) writes into the +# developer's REAL ledger — observed adding a live `claude/serena` entry during a +# local run. Since the scrub above deletes HEADROOM_WORKSPACE_DIR, the default is +# always the real home. Redirect the ledger per-test instead: every writer +# (`record_install` / `clear_install` / `headroom_installed_matching`) resolves it +# through this module-global, so one patch covers them all. Patched here rather +# than pointing workspace_dir() at a tmp path, which would break the tests that +# assert the default workspace layout. +@pytest.fixture(autouse=True) +def _isolate_mcp_ledger(monkeypatch, tmp_path_factory): + # Same guard as _reset_copilot_routing_flag below: the macos/windows-native- + # wrapper CI jobs install only pytest and drive the installer shell scripts + # via subprocess, so headroom isn't importable and there is no ledger to + # redirect. Skip there instead of erroring at setup. + try: + from headroom.mcp_registry import ledger + except ModuleNotFoundError: + return + + ledger_file = tmp_path_factory.mktemp("mcp-ledger") / "mcp_installs.json" + monkeypatch.setattr(ledger, "ledger_path", lambda: ledger_file) + + # The Copilot "routed to Copilot" flag is a module-global ContextVar that # build_copilot_upstream_url() sets as a side effect. Unit tests that call that # builder directly (or otherwise run in the shared root context) would leave it diff --git a/tests/test_cli/test_serena_migrate.py b/tests/test_cli/test_serena_migrate.py index 947930778..7fe1172ef 100644 --- a/tests/test_cli/test_serena_migrate.py +++ b/tests/test_cli/test_serena_migrate.py @@ -87,7 +87,6 @@ def _isolate_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: # 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) diff --git a/tests/test_cli/test_wrap_serena_boost.py b/tests/test_cli/test_wrap_serena_boost.py index c5ca1d9c7..8c63c5782 100644 --- a/tests/test_cli/test_wrap_serena_boost.py +++ b/tests/test_cli/test_wrap_serena_boost.py @@ -23,8 +23,8 @@ 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``. + off by default (mirrors RTK). Tests that exercise the write path must opt in via + ``HEADROOM_SERENA_INSTRUCTIONS``. """ monkeypatch.setenv("HEADROOM_SERENA_INSTRUCTIONS", "1") @@ -94,111 +94,6 @@ def test_instruction_file_target_per_agent(tmp_path: Path, monkeypatch: pytest.M 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 # --------------------------------------------------------------------------- diff --git a/tests/test_wrap_code_memory.py b/tests/test_wrap_code_memory.py index 40cf9e2f6..9e2574681 100644 --- a/tests/test_wrap_code_memory.py +++ b/tests/test_wrap_code_memory.py @@ -12,6 +12,7 @@ import os from unittest.mock import patch import click +import pytest from click.testing import CliRunner from headroom.cli import wrap @@ -64,12 +65,105 @@ def test_serena_dashboard_disabled_flips_existing_config(tmp_path, monkeypatch) assert "web_dashboard: true" in text # other keys preserved -def test_serena_dashboard_disabled_creates_config(tmp_path, monkeypatch) -> None: +def test_serena_config_is_never_created_by_headroom(tmp_path, monkeypatch) -> None: + """Headroom must NOT pre-empt Serena's own config bootstrap (#2674). + + This is the exact invariant, and it is the reason the outage happened. + Verified against Serena 1.6.2.dev0 ``serena/config/serena_config.py``: Serena + autogenerates a complete config only when the path does not exist; once any + file is there it validates instead, and a missing ``projects`` key is fatal + (``SerenaConfigError``). Headroom used to write a one-key bootstrap file, + which killed Serena's MCP handshake on every fresh install. + + Asserting "we write nothing" is stronger than asserting which keys we write: + it stays correct even if Serena adds a new required key, whereas a test that + pins our own key list would go on passing while users broke again. + """ monkeypatch.setenv("HOME", str(tmp_path)) + wrap._ensure_serena_dashboard_disabled() + cfg = tmp_path / ".serena" / "serena_config.yml" - assert cfg.exists() - assert "web_dashboard_open_on_launch: false" in cfg.read_text() + assert not cfg.exists(), "Headroom created a config Serena would have generated itself" + + +def test_serena_dashboard_disabled_repairs_config_missing_projects(tmp_path, monkeypatch) -> None: + """Backfill ``projects`` into a config an older Headroom already wrote (#2674). + + Users who ran an affected version have the single-key file on disk, so simply + not creating new bad files would leave them broken forever. + """ + import yaml + + monkeypatch.setenv("HOME", str(tmp_path)) + cfg = tmp_path / ".serena" / "serena_config.yml" + cfg.parent.mkdir(parents=True) + cfg.write_text("web_dashboard_open_on_launch: false\n") + + wrap._ensure_serena_dashboard_disabled() + + parsed = yaml.safe_load(cfg.read_text()) + assert parsed["projects"] == [] + assert parsed["web_dashboard_open_on_launch"] is False + + +def test_serena_dashboard_disabled_preserves_registered_projects(tmp_path, monkeypatch) -> None: + """Never clobber Serena's real project registry — it is user data.""" + import yaml + + monkeypatch.setenv("HOME", str(tmp_path)) + cfg = tmp_path / ".serena" / "serena_config.yml" + cfg.parent.mkdir(parents=True) + cfg.write_text( + "# my serena config\nprojects:\n - /home/me/work/api\n - /home/me/work/web\n" + "web_dashboard_open_on_launch: true\n" + ) + + wrap._ensure_serena_dashboard_disabled() + + text = cfg.read_text() + parsed = yaml.safe_load(text) + assert parsed["projects"] == ["/home/me/work/api", "/home/me/work/web"] + assert parsed["web_dashboard_open_on_launch"] is False + assert "# my serena config" in text # comments preserved + assert text.count("projects:") == 1 # no duplicate key + + +def test_serena_dashboard_disabled_is_idempotent(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("HOME", str(tmp_path)) + cfg = tmp_path / ".serena" / "serena_config.yml" + cfg.parent.mkdir(parents=True) + cfg.write_text("projects: []\nweb_dashboard_open_on_launch: true\n") + wrap._ensure_serena_dashboard_disabled() + first = cfg.read_text() + wrap._ensure_serena_dashboard_disabled() + assert cfg.read_text() == first + + +def test_serena_config_required_keys_match_serena_source() -> None: + """Pin the assumption this fix rests on, against Serena's real source (#2674). + + Skipped unless a Serena checkout is present. When it is, this proves the claim + the fix depends on — that ``projects`` is the *only* hard-required key and that + Serena bootstraps only a missing file — rather than trusting a bug report. + Set ``SERENA_SRC`` to a Serena source tree to enable it. + """ + import os + import re + from pathlib import Path + + src = os.environ.get("SERENA_SRC", "") + if not src or not (Path(src) / "config" / "serena_config.py").is_file(): + pytest.skip("set SERENA_SRC to a Serena source tree to run this check") + + text = (Path(src) / "config" / "serena_config.py").read_text(encoding="utf-8") + # Serena bootstraps only when the file is absent — so we must not create one. + assert re.search(r"if not os\.path\.exists\(config_file_path\)", text) + # `projects` is the sole fatal omission; everything else has a default. + fatal = re.findall(r"raise SerenaConfigError\((.*?)\)", text, re.DOTALL) + projects_fatal = [f for f in fatal if "projects" in f] + assert projects_fatal, "Serena no longer rejects a missing `projects` key" + assert "get_value_or_default" in text, "Serena's default-filling path changed" def test_invalid_env_raises() -> None: