mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 20:15:20 +00:00
Fix: Parallel agents auto-registering projects could overwrite each other's changes
to the global project list in `serena_config.yml` Fixes #1850
This commit is contained in:
1 parent
a9f99c679b
commit
8a4d116c13
3 files changed
+53
-39
No files matched your search
@@ -2,6 +2,10 @@
|
|||||||
|
|
||||||
Status of the `main` branch. Changes prior to the next official version change will appear here.
|
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:
|
* Language Servers:
|
||||||
- Fix: Scala cross-file queries waited a fixed 5s after the first file was opened, which on a cold
|
- 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
|
Metals is long before its build import, indexing and compilation have finished; the first
|
||||||
|
|||||||
@@ -1296,18 +1296,20 @@ class SerenaConfig(SharedConfig, ModeSelectionDefinitionWithBaseModes):
|
|||||||
|
|
||||||
def _persist_projects(self) -> None:
|
def _persist_projects(self) -> None:
|
||||||
"""
|
"""
|
||||||
Persists ONLY the registered-projects list, leaving every other setting at its on-disk value.
|
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).
|
||||||
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.
|
|
||||||
"""
|
"""
|
||||||
if self.config_file_path is None:
|
if self.config_file_path is None:
|
||||||
return
|
return
|
||||||
persisted = SerenaConfig.from_config_file()
|
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()
|
persisted._save()
|
||||||
|
|
||||||
def _save(self) -> None:
|
def _save(self) -> None:
|
||||||
|
|||||||
@@ -503,14 +503,24 @@ class TestProjectConfigYamlValidation:
|
|||||||
shutil.rmtree(project_dir)
|
shutil.rmtree(project_dir)
|
||||||
|
|
||||||
|
|
||||||
class TestSerenaConfigFromConfigFileRobustness:
|
class TestSerenaConfigLoadSave:
|
||||||
"""Tests that ``SerenaConfig.from_config_file`` does not abort the whole
|
"""
|
||||||
loader when a single registered project has a broken ``project.yml``.
|
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.test_dir = Path(tempfile.mkdtemp())
|
||||||
self.master_config_path = self.test_dir / "serena_config.yml"
|
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):
|
def teardown_method(self):
|
||||||
shutil.rmtree(self.test_dir)
|
shutil.rmtree(self.test_dir)
|
||||||
@@ -521,27 +531,19 @@ class TestSerenaConfigFromConfigFileRobustness:
|
|||||||
(project_dir / SERENA_MANAGED_DIR_NAME / "project.yml").write_text(project_yml_body)
|
(project_dir / SERENA_MANAGED_DIR_NAME / "project.yml").write_text(project_yml_body)
|
||||||
return project_dir
|
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:"]
|
body_lines = ["projects:"]
|
||||||
for p in project_paths:
|
for p in project_paths:
|
||||||
body_lines.append(f" - {p}")
|
body_lines.append(f" - {p}")
|
||||||
self.master_config_path.write_text("\n".join(body_lines) + "\n")
|
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."""
|
"""A bare ``projects:`` key should not abort config loading."""
|
||||||
self.master_config_path.write_text("projects:\n")
|
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)
|
config = SerenaConfig.from_config_file(generate_if_missing=False)
|
||||||
|
|
||||||
assert config.projects == []
|
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."""
|
"""A malformed project.yml must not abort loading of the others."""
|
||||||
good_project = self._make_project_dir(
|
good_project = self._make_project_dir(
|
||||||
"good_project",
|
"good_project",
|
||||||
@@ -554,15 +556,6 @@ class TestSerenaConfigFromConfigFileRobustness:
|
|||||||
)
|
)
|
||||||
self._write_master_config([good_project, bad_project])
|
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):
|
with caplog.at_level(logging.ERROR):
|
||||||
config = SerenaConfig.from_config_file(generate_if_missing=False)
|
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}"
|
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 = self._make_project_dir(
|
||||||
"good_project",
|
"good_project",
|
||||||
'project_name: "good_project"\nlanguages: ["python"]\n',
|
'project_name: "good_project"\nlanguages: ["python"]\n',
|
||||||
@@ -583,12 +576,6 @@ class TestSerenaConfigFromConfigFileRobustness:
|
|||||||
)
|
)
|
||||||
self._write_master_config([good_project, bad_project])
|
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):
|
with caplog.at_level(logging.ERROR):
|
||||||
config = SerenaConfig.from_config_file(generate_if_missing=False)
|
config = SerenaConfig.from_config_file(generate_if_missing=False)
|
||||||
|
|
||||||
@@ -596,6 +583,27 @@ class TestSerenaConfigFromConfigFileRobustness:
|
|||||||
assert registered_roots == {good_project.resolve()}
|
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
|
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:
|
class TestGetRegisteredProjectWithDanglingProject:
|
||||||
"""A registered project whose root directory was deleted (e.g. a removed git
|
"""A registered project whose root directory was deleted (e.g. a removed git
|
||||||
|
|||||||
Reference in new issue
Block a user