From e357323051a54507302d3256ab980018a6f34e67 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Tue, 17 Mar 2026 00:24:06 +0100 Subject: [PATCH] Fix: Ensure that local overrides from project.local.yml are never written to project.yml Previously, only project.yml resave due to incomplete config accounted for this, but dynamic language changes triggered via the dashboard (which triggered a save to update the language list) did not. Since ProjectConfig can always contain local overrides, * we handle configuration updates by explicitly updating on disk only the keys known to have been changed * we explicitly represent the presence of local overrides, such that inadmissible save operations are detectable. --- src/serena/config/serena_config.py | 57 ++++++++++++++++++++---- src/serena/project.py | 15 ++++--- test/serena/config/test_serena_config.py | 4 +- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/serena/config/serena_config.py b/src/serena/config/serena_config.py index f81c9396..6b36fc5f 100644 --- a/src/serena/config/serena_config.py +++ b/src/serena/config/serena_config.py @@ -245,6 +245,10 @@ class ProjectConfig(SharedConfig): initial_prompt: str = "" encoding: str = DEFAULT_SOURCE_FILE_ENCODING + # internal fields which are not mapped to/from the configuration file (must start with "_") + _has_local_overrides: bool = field(default=False, repr=False, compare=False) + + # class-level constants SERENA_PROJECT_FILE = "project.yml" SERENA_LOCAL_PROJECT_FILE = "project.local.yml" FIELDS_WITHOUT_DEFAULTS = {"project_name", "languages"} @@ -339,7 +343,8 @@ class ProjectConfig(SharedConfig): save_yaml(project_yml_path, config_with_comments) project_local_yml_path = os.path.join(os.path.dirname(project_yml_path), cls.SERENA_LOCAL_PROJECT_FILE) shutil.copy(PROJECT_LOCAL_TEMPLATE_FILE, project_local_yml_path) - return cls._from_dict(config_with_comments) + + return cls._from_dict(config_with_comments, has_local_overrides=False) @classmethod def default_project_yml_path(cls, project_root: str | Path) -> str: @@ -378,6 +383,8 @@ class ProjectConfig(SharedConfig): if apply_defaults: for field_info in dataclasses.fields(cls): key = field_info.name + if key.startswith("_"): + continue if key in cls.FIELDS_WITHOUT_DEFAULTS: continue if key not in data: @@ -395,9 +402,13 @@ class ProjectConfig(SharedConfig): return data, was_complete @classmethod - def _from_dict(cls, data: dict[str, Any]) -> Self: + def _from_dict(cls, data: dict[str, Any], has_local_overrides: bool) -> Self: """ Create a ProjectConfig instance from a (full) configuration dictionary + + :param data: the configuration dictionary; must contain all required fields and use the same field names as + the ProjectConfig dataclass + :param has_local_overrides: whether this configuration contains overrides from a project.local.yml file """ lang_name_mapping = {"javascript": "typescript"} languages: list[Language] = [] @@ -448,6 +459,7 @@ class ProjectConfig(SharedConfig): base_modes=data["base_modes"], default_modes=data["default_modes"], symbol_info_budget=symbol_info_budget, + _has_local_overrides=has_local_overrides, ) def _to_yaml_dict(self) -> dict: @@ -455,9 +467,18 @@ class ProjectConfig(SharedConfig): :return: a yaml-serializable dictionary representation of this configuration """ d = dataclasses.asdict(self) + + # drop internal fields starting with underscore + keys = list(d.keys()) + for k in keys: + if k.startswith("_"): + del d[k] + + # map fields using non-primitive types to a YAML-compatible representation d["languages"] = [lang.value for lang in self.languages] d["language_backend"] = self.language_backend.value if self.language_backend is not None else None d["line_ending"] = self.line_ending.value if self.line_ending is not None else None + return d @classmethod @@ -488,12 +509,13 @@ class ProjectConfig(SharedConfig): # if the configuration was incomplete, re-save it to disk if not was_complete: - project_config = cls._from_dict(yaml_data) + project_config = cls._from_dict(yaml_data, has_local_overrides=False) log.info("Project configuration in %s was incomplete, re-saving with default values for missing fields", yaml_path) - project_config.save(str(yaml_path)) + project_config.save(str(yaml_path), keys=None) # apply overrides from project.local.yml, if present local_yaml_path = os.path.join(os.path.dirname(yaml_path), cls.SERENA_LOCAL_PROJECT_FILE) + has_local_overrides = False if os.path.exists(local_yaml_path): local_yaml_data, _ = cls._load_yaml_dict(local_yaml_path, apply_defaults=False) if local_yaml_data: @@ -503,24 +525,41 @@ class ProjectConfig(SharedConfig): list(local_yaml_data.keys()), ) yaml_data.update(local_yaml_data) + has_local_overrides = True # instantiate the ProjectConfig - project_config = cls._from_dict(yaml_data) + project_config = cls._from_dict(yaml_data, has_local_overrides=has_local_overrides) return project_config - def save(self, project_yml_path: str) -> None: + def save(self, project_yml_path: str, keys: list[str] | None) -> None: """ - Saves the project configuration to disk. + Saves the project configuration to disk, updating the specified keys. :param project_yml_path: the path to the project.yml file + :param keys: the list of keys to update in the file; if None, all keys are updated + (which is admissible only if no local overrides have been applied to the current configuration) """ config_path = project_yml_path log.info("Saving updated project configuration to %s", config_path) - # load original commented map and update it with current values + # load commented map from the original file config_with_comments, _ = self._load_yaml_dict(config_path, self.YAML_COMMENT_NORMALISATION) - config_with_comments.update(self._to_yaml_dict()) + + # update the requested keys from the current in-memory configuration + cur_dict = self._to_yaml_dict() + if keys is None: + if self._has_local_overrides: + raise ValueError( + "Cannot save the project configuration with all keys updated when there are overrides from project.local.yml. Must specify the keys to update explicitly." + ) + config_with_comments.update(cur_dict) + else: + for key in keys: + if key in cur_dict: + config_with_comments[key] = cur_dict[key] + else: + raise ValueError(f"Invalid configuration key '{key}' for project configuration.") # transfer missing comments from the template file template_config, _ = self._load_yaml_dict(PROJECT_TEMPLATE_FILE, self.YAML_COMMENT_NORMALISATION) diff --git a/src/serena/project.py b/src/serena/project.py index 00d8ba40..8971c8cb 100644 --- a/src/serena/project.py +++ b/src/serena/project.py @@ -334,11 +334,16 @@ class Project(ToStringMixin): project_config = ProjectConfig.load(project_root, serena_config=serena_config, autogenerate=autogenerate) return Project(project_root=str(project_root), project_config=project_config, serena_config=serena_config) - def save_config(self) -> None: + def save_config(self, keys: list[str]) -> None: """ - Saves the current project configuration to disk. + Saves the current project configuration (project.yml) to disk. + + :param keys: the keys of the project configuration that were changed and need to be updated on disk. + Specifying these keys is necessary, because we cannot just save the full configuration, since + it may be subject to local overrides (from project.local.yml), which must never be saved to + project.yml. """ - self.project_config.save(self.path_to_project_yml()) + self.project_config.save(self.path_to_project_yml(), keys=keys) def path_to_serena_data_folder(self) -> str: return self._serena_data_folder @@ -673,7 +678,7 @@ class Project(ToStringMixin): # update the project configuration self.project_config.languages.append(language) - self.save_config() + self.save_config(keys=["languages"]) def remove_language(self, language: Language) -> None: """ @@ -688,7 +693,7 @@ class Project(ToStringMixin): return # update the project configuration self.project_config.languages.remove(language) - self.save_config() + self.save_config(keys=["languages"]) # stop the language server (if the LS manager is active) if self.language_server_manager is None: diff --git a/test/serena/config/test_serena_config.py b/test/serena/config/test_serena_config.py index 1c09cceb..af5ee859 100644 --- a/test/serena/config/test_serena_config.py +++ b/test/serena/config/test_serena_config.py @@ -187,7 +187,7 @@ class TestProjectConfigLanguageBackend: data["project_name"] = "test" data["languages"] = ["python"] data["language_backend"] = "JetBrains" - config = ProjectConfig._from_dict(data) + config = ProjectConfig._from_dict(data, has_local_overrides=False) assert config.language_backend == LanguageBackend.JETBRAINS def test_language_backend_none_when_missing_from_dict(self): @@ -197,7 +197,7 @@ class TestProjectConfigLanguageBackend: data["project_name"] = "test" data["languages"] = ["python"] data.pop("language_backend", None) - config = ProjectConfig._from_dict(data) + config = ProjectConfig._from_dict(data, has_local_overrides=False) assert config.language_backend is None