From d58202d10ef9c6a57b5f96e38c24564500da08b1 Mon Sep 17 00:00:00 2001
From: Mazen Tamer Salah <78306991+mazen-salah@users.noreply.github.com>
Date: Mon, 8 Jun 2026 20:16:37 +0300
Subject: [PATCH] fix(presets): persist presets atomically to avoid corruption
 on crash (#2169)

PresetManager.save() used a plain open("w") + json.dump, which truncates
presets.json before writing the new content. A crash, power loss, or
serialization error mid-write leaves the file truncated/empty and every
saved preset is lost.

Route the write through core.atomic_io.atomic_write_json (tmp file +
os.replace), matching how the rest of the codebase persists JSON state.
The helper is imported lazily so this module stays free of the heavy core
package import graph at module load time.

Adds tests/test_preset_atomic_save.py covering the source contract, a
failed-write leaving the existing file intact, and a round trip.
---
 src/preset_manager.py            |  9 ++++---
 tests/test_preset_atomic_save.py | 43 ++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 tests/test_preset_atomic_save.py

diff --git a/src/preset_manager.py b/src/preset_manager.py
index 6364b8a9c..ae88a9432 100644
--- a/src/preset_manager.py
+++ b/src/preset_manager.py
@@ -115,9 +115,12 @@ Use precise language. Show causal relationships explicitly. Quantify uncertainty
     def save(self, presets: Dict[str, Any]) -> bool:
         """Save presets to file"""
         try:
-            os.makedirs(os.path.dirname(self.presets_file), exist_ok=True)
-            with open(self.presets_file, 'w', encoding="utf-8") as f:
-                json.dump(presets, f, indent=2)
+            # Atomic write (tmp file + os.replace) so a crash or serialization
+            # error mid-write can't truncate presets.json and lose every saved
+            # preset. Lazy import keeps this module free of the heavy core
+            # package import graph at load time.
+            from core.atomic_io import atomic_write_json
+            atomic_write_json(self.presets_file, presets, indent=2)
             self.presets = presets
             return True
         except Exception as e:
diff --git a/tests/test_preset_atomic_save.py b/tests/test_preset_atomic_save.py
new file mode 100644
index 000000000..8af1d4f52
--- /dev/null
+++ b/tests/test_preset_atomic_save.py
@@ -0,0 +1,43 @@
+"""Regression: PresetManager.save() must persist presets atomically.
+
+save() used a plain open("w") + json.dump, which truncates presets.json before
+writing the new content. A crash / power loss / serialization error mid-write
+leaves the file truncated or empty — the user loses every saved preset. The
+save now goes through core.atomic_io.atomic_write_json (tmp file + os.replace),
+which the rest of the codebase already uses for JSON state files.
+"""
+import inspect
+import json
+
+from src.preset_manager import PresetManager
+
+
+class _Unserializable:
+    """json.dump cannot serialize this — stands in for a mid-write failure."""
+
+
+def test_save_uses_atomic_write_json():
+    src = inspect.getsource(PresetManager.save)
+    assert "atomic_write_json" in src, "save() must persist via atomic_write_json"
+    assert "open(" not in src, "save() must not write presets.json with a plain open('w')"
+
+
+def test_failed_save_does_not_truncate_existing_file(tmp_path):
+    mgr = PresetManager(str(tmp_path))
+    assert mgr.save({"custom": {"name": "keep"}}) is True
+    before = (tmp_path / "presets.json").read_text(encoding="utf-8")
+
+    # A payload that cannot be serialized must not clobber the good file.
+    assert mgr.save({"custom": {"obj": _Unserializable()}}) is False
+
+    after = (tmp_path / "presets.json").read_text(encoding="utf-8")
+    assert after == before
+    assert json.loads(after) == {"custom": {"name": "keep"}}
+
+
+def test_save_round_trip(tmp_path):
+    mgr = PresetManager(str(tmp_path))
+    assert mgr.save({"custom": {"name": "X", "temperature": 0.5}}) is True
+
+    reloaded = PresetManager(str(tmp_path))
+    assert reloaded.presets["custom"]["name"] == "X"