Skip to content

Commit fd3e29a

Browse files
Skip and warn on malformed project configs at startup instead of crashing the MCP server (#1424)
* 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>
1 parent ae7f106 commit fd3e29a

2 files changed

Lines changed: 68 additions & 1 deletion

File tree

src/serena/config/serena_config.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,18 @@ def get_value_or_default(field_name: str) -> Any:
869869
if path is None:
870870
continue
871871
num_migrations += 1
872-
project_config = ProjectConfig.load(path, serena_config=instance) # instance is sufficiently populated
872+
try:
873+
project_config = ProjectConfig.load(path, serena_config=instance) # instance is sufficiently populated
874+
except Exception as e:
875+
log.error(
876+
"Failed to load project configuration for %s: %s. "
877+
"This project will be skipped. Fix or delete its "
878+
".serena/project.yml (or remove it from "
879+
"serena_config.yml) to re-enable it.",
880+
path,
881+
e,
882+
)
883+
continue
873884
project = RegisteredProject(
874885
project_root=str(path),
875886
project_config=project_config,

test/serena/config/test_serena_config.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,62 @@ def test_configured_path_takes_precedence_when_exists(self):
499499
assert project.path_to_serena_data_folder() == str(custom_serena)
500500

501501

502+
class TestSerenaConfigFromConfigFileRobustness:
503+
"""Tests that ``SerenaConfig.from_config_file`` does not abort the whole
504+
loader when a single registered project has a broken ``project.yml``.
505+
"""
506+
507+
def setup_method(self):
508+
self.test_dir = Path(tempfile.mkdtemp())
509+
self.master_config_path = self.test_dir / "serena_config.yml"
510+
511+
def teardown_method(self):
512+
shutil.rmtree(self.test_dir)
513+
514+
def _make_project_dir(self, name: str, project_yml_body: str) -> Path:
515+
project_dir = self.test_dir / name
516+
(project_dir / SERENA_MANAGED_DIR_NAME).mkdir(parents=True)
517+
(project_dir / SERENA_MANAGED_DIR_NAME / "project.yml").write_text(project_yml_body)
518+
return project_dir
519+
520+
def _write_master_config(self, project_paths: list[Path]) -> None:
521+
body_lines = ["projects:"]
522+
for p in project_paths:
523+
body_lines.append(f" - {p}")
524+
self.master_config_path.write_text("\n".join(body_lines) + "\n")
525+
526+
def test_malformed_project_is_skipped_with_warning(self, caplog, monkeypatch):
527+
"""A malformed project.yml must not abort loading of the others."""
528+
good_project = self._make_project_dir(
529+
"good_project",
530+
'project_name: "good_project"\nlanguages: ["python"]\n',
531+
)
532+
# Invalid YAML: a stray colon at the start of a mapping value.
533+
bad_project = self._make_project_dir(
534+
"bad_project",
535+
": this is not : valid : yaml :\n",
536+
)
537+
self._write_master_config([good_project, bad_project])
538+
539+
# SerenaPaths is a process-wide singleton, so we cannot reliably
540+
# redirect it via SERENA_HOME after the fact. Instead, redirect the
541+
# config-file-path resolver directly.
542+
monkeypatch.setattr(
543+
SerenaConfig,
544+
"_determine_config_file_path",
545+
classmethod(lambda cls: str(self.master_config_path)),
546+
)
547+
548+
with caplog.at_level(logging.ERROR):
549+
config = SerenaConfig.from_config_file(generate_if_missing=False)
550+
551+
registered_roots = {Path(p.project_root).resolve() for p in config.projects}
552+
assert registered_roots == {good_project.resolve()}, f"Expected only the good project to be registered, got {registered_roots}"
553+
assert any("Failed to load project configuration" in msg and str(bad_project.resolve()) in msg for msg in caplog.messages), (
554+
f"Expected a warning naming {bad_project.resolve()}, got: {caplog.messages}"
555+
)
556+
557+
502558
class TestMemoriesManagerCustomPath:
503559
"""Tests for MemoriesManager with a custom serena data folder."""
504560

0 commit comments

Comments
 (0)