mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 12:15:10 +00:00
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>
This commit is contained in:
1 parent
ae7f1065be
commit
fd3e29ac50
2 files changed
+68
-1
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
Reference in new issue
Block a user