From fd3e29ac50e2c9e88ccf5d7f10dc4518c666afa1 Mon Sep 17 00:00:00 2001 From: Rabbit_Slayer <43283626+leavesprior@users.noreply.github.com> Date: Mon, 27 Apr 2026 03:39:34 -0700 Subject: [PATCH] Skip and warn on malformed project configs at startup instead of crashing the MCP server (#1424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Skip and warn on malformed project configs at startup `SerenaConfig.from_config_file` iterates the `projects:` list in `serena_config.yml` and calls `ProjectConfig.load(path)` with no exception handling. A single malformed `.serena/project.yml` (missing required key, bad language string, YAML syntax error, etc.) raises out of the master loader and aborts MCP server startup before any client handshake or tool registration. Other projects in the list — even ones that would load cleanly — become unreachable. This matches the failure mode behind #997: the schema-migration shim closed the *cause*, but the surrounding loader still has no isolation, so the next class of `project.yml` malformation will take the server down again. Wrap the `ProjectConfig.load(path)` call in the master loop with a narrow try/except that logs a warning and continues, matching the existing skip-with-warning pattern used a few lines above for inaccessible paths and missing project.yml files. One bad project becomes one skipped project, not a server-down event. A new test under `TestSerenaConfigFromConfigFileRobustness` registers two projects (one healthy, one with invalid YAML), asserts that `from_config_file` returns successfully, that only the healthy project ends up registered, and that a warning naming the bad path is logged. --------- Co-authored-by: Michael Panchenko <35432522+MischaPanch@users.noreply.github.com> --- src/serena/config/serena_config.py | 13 +++++- test/serena/config/test_serena_config.py | 56 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/serena/config/serena_config.py b/src/serena/config/serena_config.py index 5c8e4e10..329e0e86 100644 --- a/src/serena/config/serena_config.py +++ b/src/serena/config/serena_config.py @@ -869,7 +869,18 @@ class SerenaConfig(SharedConfig): if path is None: continue num_migrations += 1 - project_config = ProjectConfig.load(path, serena_config=instance) # instance is sufficiently populated + try: + project_config = ProjectConfig.load(path, serena_config=instance) # instance is sufficiently populated + except Exception as e: + log.error( + "Failed to load project configuration for %s: %s. " + "This project will be skipped. Fix or delete its " + ".serena/project.yml (or remove it from " + "serena_config.yml) to re-enable it.", + path, + e, + ) + continue project = RegisteredProject( project_root=str(path), project_config=project_config, diff --git a/test/serena/config/test_serena_config.py b/test/serena/config/test_serena_config.py index 3f2e7ac3..75c1e143 100644 --- a/test/serena/config/test_serena_config.py +++ b/test/serena/config/test_serena_config.py @@ -499,6 +499,62 @@ class TestProjectSerenaDataFolder: assert project.path_to_serena_data_folder() == str(custom_serena) +class TestSerenaConfigFromConfigFileRobustness: + """Tests that ``SerenaConfig.from_config_file`` does not abort the whole + loader when a single registered project has a broken ``project.yml``. + """ + + def setup_method(self): + self.test_dir = Path(tempfile.mkdtemp()) + self.master_config_path = self.test_dir / "serena_config.yml" + + def teardown_method(self): + shutil.rmtree(self.test_dir) + + def _make_project_dir(self, name: str, project_yml_body: str) -> Path: + project_dir = self.test_dir / name + (project_dir / SERENA_MANAGED_DIR_NAME).mkdir(parents=True) + (project_dir / SERENA_MANAGED_DIR_NAME / "project.yml").write_text(project_yml_body) + return project_dir + + def _write_master_config(self, project_paths: list[Path]) -> None: + body_lines = ["projects:"] + for p in project_paths: + body_lines.append(f" - {p}") + self.master_config_path.write_text("\n".join(body_lines) + "\n") + + def test_malformed_project_is_skipped_with_warning(self, caplog, monkeypatch): + """A malformed project.yml must not abort loading of the others.""" + good_project = self._make_project_dir( + "good_project", + 'project_name: "good_project"\nlanguages: ["python"]\n', + ) + # Invalid YAML: a stray colon at the start of a mapping value. + bad_project = self._make_project_dir( + "bad_project", + ": this is not : valid : yaml :\n", + ) + self._write_master_config([good_project, bad_project]) + + # SerenaPaths is a process-wide singleton, so we cannot reliably + # redirect it via SERENA_HOME after the fact. Instead, redirect the + # config-file-path resolver directly. + monkeypatch.setattr( + SerenaConfig, + "_determine_config_file_path", + classmethod(lambda cls: str(self.master_config_path)), + ) + + with caplog.at_level(logging.ERROR): + config = SerenaConfig.from_config_file(generate_if_missing=False) + + registered_roots = {Path(p.project_root).resolve() for p in config.projects} + assert registered_roots == {good_project.resolve()}, f"Expected only the good project to be registered, got {registered_roots}" + assert any("Failed to load project configuration" in msg and str(bad_project.resolve()) in msg for msg in caplog.messages), ( + f"Expected a warning naming {bad_project.resolve()}, got: {caplog.messages}" + ) + + class TestMemoriesManagerCustomPath: """Tests for MemoriesManager with a custom serena data folder."""