From 8a4d116c13bb809ad3b8cc9071d28e8ed9f4f6df Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Thu, 13 Aug 2026 12:21:45 +0200 Subject: [PATCH] Fix: Parallel agents auto-registering projects could overwrite each other's changes to the global project list in `serena_config.yml` Fixes #1850 --- CHANGELOG.md | 4 ++ src/serena/config/serena_config.py | 18 +++--- test/serena/config/test_serena_config.py | 70 +++++++++++++----------- 3 files changed, 53 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c639d151..2f188595 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Status of the `main` branch. Changes prior to the next official version change will appear here. +* General: + - Fix: Parallel agents auto-registering projects could overwrite each other's changes to the global + project list in `serena_config.yml` + * Language Servers: - Fix: Scala cross-file queries waited a fixed 5s after the first file was opened, which on a cold Metals is long before its build import, indexing and compilation have finished; the first diff --git a/src/serena/config/serena_config.py b/src/serena/config/serena_config.py index 9a169962..4ba5c473 100644 --- a/src/serena/config/serena_config.py +++ b/src/serena/config/serena_config.py @@ -1296,18 +1296,20 @@ class SerenaConfig(SharedConfig, ModeSelectionDefinitionWithBaseModes): def _persist_projects(self) -> None: """ - Persists ONLY the registered-projects list, leaving every other setting at its on-disk value. - - Project (de)registration can happen while a session is running with transient runtime overrides applied - to this in-memory instance (e.g. ``start-mcp-server --language-backend`` / ``--log-level``). A full - :meth:`save` would write those overrides back to the global config, silently clobbering the user's - settings. Instead we re-load the persisted config, copy in the current project list, and save that — so - only ``projects`` is ever mutated on disk. + Persists the list of registered projects, merging it with the list currently found on disk + (parallel agent instances may have added or removed projects in the meantime). """ if self.config_file_path is None: return persisted = SerenaConfig.from_config_file() - persisted.projects = list(self.projects) + combined_projects = [] + handled_project_paths = set() + for p in persisted.projects + self.projects: + str_path = str(p.project_root) + if str_path not in handled_project_paths: + combined_projects.append(p) + handled_project_paths.add(str_path) + persisted.projects = combined_projects persisted._save() def _save(self) -> None: diff --git a/test/serena/config/test_serena_config.py b/test/serena/config/test_serena_config.py index c0b4cf20..cc04ab45 100644 --- a/test/serena/config/test_serena_config.py +++ b/test/serena/config/test_serena_config.py @@ -503,14 +503,24 @@ class TestProjectConfigYamlValidation: shutil.rmtree(project_dir) -class TestSerenaConfigFromConfigFileRobustness: - """Tests that ``SerenaConfig.from_config_file`` does not abort the whole - loader when a single registered project has a broken ``project.yml``. +class TestSerenaConfigLoadSave: + """ + Supports tests of loading/saving SerenaConfig, + allowing to set up a temporary directory with a master config and multiple project directories. """ - def setup_method(self): + @pytest.fixture(autouse=True) + def setup(self, monkeypatch): + """ + Sets up a temporary directory for testing, and monkeypatch SerenaConfig to use a test config file path. + """ self.test_dir = Path(tempfile.mkdtemp()) self.master_config_path = self.test_dir / "serena_config.yml" + monkeypatch.setattr( + SerenaConfig, + "_determine_config_file_path", + classmethod(lambda cls: str(self.master_config_path)), + ) def teardown_method(self): shutil.rmtree(self.test_dir) @@ -521,27 +531,19 @@ class TestSerenaConfigFromConfigFileRobustness: (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: + def _write_master_config(self, project_paths: list[Path | str]) -> 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_empty_projects_key_is_treated_as_empty_list(self, monkeypatch): + def test_empty_projects_key_is_treated_as_empty_list(self): """A bare ``projects:`` key should not abort config loading.""" self.master_config_path.write_text("projects:\n") - - monkeypatch.setattr( - SerenaConfig, - "_determine_config_file_path", - classmethod(lambda cls: str(self.master_config_path)), - ) - config = SerenaConfig.from_config_file(generate_if_missing=False) - assert config.projects == [] - def test_malformed_project_is_skipped_with_warning(self, caplog, monkeypatch): + def test_malformed_project_is_skipped_with_warning(self, caplog): """A malformed project.yml must not abort loading of the others.""" good_project = self._make_project_dir( "good_project", @@ -554,15 +556,6 @@ class TestSerenaConfigFromConfigFileRobustness: ) 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) @@ -572,7 +565,7 @@ class TestSerenaConfigFromConfigFileRobustness: f"Expected a warning naming {bad_project.resolve()}, got: {caplog.messages}" ) - def test_alias_like_ignored_path_error_is_logged_with_hint(self, caplog, monkeypatch): + def test_alias_like_ignored_path_error_is_logged_with_hint(self, caplog): good_project = self._make_project_dir( "good_project", 'project_name: "good_project"\nlanguages: ["python"]\n', @@ -583,12 +576,6 @@ class TestSerenaConfigFromConfigFileRobustness: ) self._write_master_config([good_project, bad_project]) - 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) @@ -596,6 +583,27 @@ class TestSerenaConfigFromConfigFileRobustness: assert registered_roots == {good_project.resolve()} assert any("must be quoted" in msg and str(bad_project.resolve()) in msg for msg in caplog.messages), caplog.messages + def test_parallel_agents_changing_project_lists(self): + """ + Tests that agents changing SerenaConfig by adding projects in parallel do not lose each other's changes. + """ + p1 = self._make_project_dir("project1", 'project_name: "project1"\nlanguages: ["python"]\n') + p2 = self._make_project_dir("project2", 'project_name: "project1"\nlanguages: ["python"]\n') + p3 = self._make_project_dir("project3", 'project_name: "project1"\nlanguages: ["python"]\n') + p4 = self._make_project_dir("project4", 'project_name: "project1"\nlanguages: ["python"]\n') + + self._write_master_config([p1, p2]) + + config1 = SerenaConfig.from_config_file(generate_if_missing=False) + config2 = SerenaConfig.from_config_file(generate_if_missing=False) + + config1.add_project_from_path(p3) + config2.add_project_from_path(p4) + + resulting_config = SerenaConfig.from_config_file(generate_if_missing=False) + + assert len(resulting_config.projects) == 4 + class TestGetRegisteredProjectWithDanglingProject: """A registered project whose root directory was deleted (e.g. a removed git