From 260a4bd979228692d39f64a8e057881a52e44e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?sam=E7=9A=84=E7=94=B5=E8=84=91?= Date: Thu, 5 Mar 2026 05:47:14 +0800 Subject: [PATCH] fix(config): make state migration deterministic and transactional --- desloppify/base/config.py | 23 +++++++++++--- desloppify/tests/core/test_config.py | 46 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/desloppify/base/config.py b/desloppify/base/config.py index 6d5e1798..9311e144 100644 --- a/desloppify/base/config.py +++ b/desloppify/base/config.py @@ -377,10 +377,16 @@ def _migrate_single_state_file(config: dict, path: Path) -> bool: return False _merge_legacy_state_config(config, old_config) - _strip_config_from_state_file(path, state_data) return True +def _state_files_for_migration(state_dir: Path) -> list[Path]: + """Return state files in deterministic migration order.""" + scoped = sorted(state_dir.glob("state-*.json"), key=lambda p: p.name) + root = sorted(state_dir.glob("state.json"), key=lambda p: p.name) + return [*scoped, *root] + + def _migrate_from_state_files(config_path: Path) -> dict: """Migrate config keys from state-*.json files into config.json. @@ -393,12 +399,13 @@ def _migrate_from_state_files(config_path: Path) -> dict: if not state_dir.exists(): return config - state_files = list(state_dir.glob("state-*.json")) + list( - state_dir.glob("state.json") - ) + state_files = _state_files_for_migration(state_dir) migrated_any = False + migrated_files: list[Path] = [] for sf in state_files: - migrated_any = _migrate_single_state_file(config, sf) or migrated_any + if _migrate_single_state_file(config, sf): + migrated_any = True + migrated_files.append(sf) if migrated_any and config: try: @@ -407,6 +414,12 @@ def _migrate_from_state_files(config_path: Path) -> dict: log_best_effort_failure( logger, f"save migrated config to {config_path}", exc ) + else: + for sf in migrated_files: + state_data = _load_state_file_payload(sf) + if state_data is None: + continue + _strip_config_from_state_file(sf, state_data) return config diff --git a/desloppify/tests/core/test_config.py b/desloppify/tests/core/test_config.py index 0de9a5bf..0e65036c 100644 --- a/desloppify/tests/core/test_config.py +++ b/desloppify/tests/core/test_config.py @@ -4,6 +4,7 @@ import pytest +import desloppify.base.config as config_mod from desloppify.base.config import ( CONFIG_SCHEMA, _migrate_from_state_files, @@ -321,6 +322,27 @@ def test_merges_multiple_state_files(self, tmp_path): assert "ex1" in result.get("exclude", []) assert "ex2" in result.get("exclude", []) + def test_scalar_merge_order_is_deterministic(self, tmp_path): + state_dir = tmp_path + state_dir.mkdir(exist_ok=True) + + state_a = { + "version": 1, + "config": {"target_strict_score": 90}, + "issues": {}, + } + state_z = { + "version": 1, + "config": {"target_strict_score": 10}, + "issues": {}, + } + (state_dir / "state-z.json").write_text(json.dumps(state_z)) + (state_dir / "state-a.json").write_text(json.dumps(state_a)) + config_path = state_dir / "config.json" + + result = _migrate_from_state_files(config_path) + assert result.get("target_strict_score") == 90 + def test_no_state_files_returns_empty(self, tmp_path): config_path = tmp_path / "config.json" result = _migrate_from_state_files(config_path) @@ -378,3 +400,27 @@ def test_legacy_language_keys_from_state_are_not_auto_migrated(self, tmp_path): result = _migrate_from_state_files(config_path) assert "csharp_corroboration_min_signals" not in result assert "csharp_high_fanout_threshold" not in result + + def test_save_failure_does_not_strip_source_state_config( + self, tmp_path, monkeypatch + ): + state_dir = tmp_path + state_dir.mkdir(exist_ok=True) + state_data = { + "version": 1, + "config": {"ignore": ["smells::*::debug"]}, + "issues": {}, + } + state_file = state_dir / "state-python.json" + state_file.write_text(json.dumps(state_data)) + config_path = state_dir / "config.json" + + def _raise_save(*_args, **_kwargs): + raise OSError("disk full") + + monkeypatch.setattr(config_mod, "save_config", _raise_save) + + result = _migrate_from_state_files(config_path) + assert "smells::*::debug" in result.get("ignore", []) + updated_state = json.loads(state_file.read_text()) + assert "config" in updated_state