fix: make session history save atomic and surface corrupt files

gcode/history.py

@@ -4,8 +4,11 @@
 ``messages_from_dict`` so tool-call <-> tool-message pairing survives a reload.
 """
 
+import contextlib
 import json
 import os
+import sys
+import tempfile
 
 from langchain_core.messages import messages_from_dict, messages_to_dict
 
@@ -20,26 +23,55 @@ def _path(session: str) -> str:
 
 
 def load(session: str = DEFAULT_SESSION):
-    """Load persisted messages for a session, or None if none exist."""
+    """Load persisted messages for a session, or None if none exist.
+
+    A missing file returns None silently. A file that exists but cannot be
+    parsed is reported to stderr (naming the path) instead of being silently
+    treated as "no history", so a corrupt session file cannot masquerade as an
+    empty conversation.
+    """
     path = _path(session)
     if not os.path.isfile(path):
         return None
     try:
         with open(path, encoding="utf-8") as f:
             data = json.load(f)
         return messages_from_dict(data)
-    except Exception:
+    except Exception as exc:  # noqa: BLE001 — surface corrupt/malformed history
+        print(
+            f"Warning: could not read session history from {path} ({exc}). "
+            "The file may be corrupt — inspect or remove it to start a fresh session.",
+            file=sys.stderr,
+        )
         return None
 
 
 def save(session: str, messages) -> None:
-    """Persist messages to disk (includes the leading system message)."""
+    """Persist messages to disk atomically (includes the leading system message).
+
+    Writes to a temporary file in the same directory and ``os.replace``s it
+    into place, so an interrupted or failed save can never leave a partial
+    session file behind. Failures are reported to stderr instead of being
+    silently swallowed — a broken save should not look like "no history".
+    """
     path = _path(session)
     try:
-        with open(path, "w", encoding="utf-8") as f:
-            json.dump(messages_to_dict(messages), f, indent=2)
-    except Exception:  # noqa: S110  # nosec B110 — persistence is best-effort; never crash the REPL on disk errors
-        pass
+        data = json.dumps(messages_to_dict(messages), indent=2)
+        fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path), suffix=".tmp")
+        try:
+            with os.fdopen(fd, "w", encoding="utf-8") as f:
+                f.write(data)
+            os.replace(tmp_path, path)
+        except Exception:
+            # Best-effort cleanup so a failed write leaves no stray temp file.
+            with contextlib.suppress(OSError):
+                os.unlink(tmp_path)
+            raise
+    except Exception as exc:  # noqa: BLE001 — persistence must never crash the REPL
+        print(
+            f"Warning: could not save session history to {path} ({exc}).",
+            file=sys.stderr,
+        )
 
 
 def clear(session: str = DEFAULT_SESSION) -> None:

tests/test_history.py

@@ -85,6 +85,28 @@ def test_load_corrupt_json(tmp_path, monkeypatch):
     assert result is None
 
 
+def test_load_corrupt_json_warns_with_path(tmp_path, monkeypatch, capsys):
+    """A corrupt session file must warn and name the path, not silently vanish."""
+    monkeypatch.setattr(history, "BASE_DIR", str(tmp_path))
+    filepath = history._path("corrupt_warn_session")
+    with open(filepath, "w", encoding="utf-8") as f:
+        f.write("{ invalid json content ...")
+
+    result = history.load("corrupt_warn_session")
+    assert result is None
+    err = capsys.readouterr().err
+    assert filepath in err
+    assert "corrupt" in err
+
+
+def test_load_missing_file_no_warning(tmp_path, monkeypatch, capsys):
+    """A missing session file stays silent (no history yet is not an error)."""
+    monkeypatch.setattr(history, "BASE_DIR", str(tmp_path))
+    result = history.load("never_saved_session")
+    assert result is None
+    assert capsys.readouterr().err == ""
+
+
 def test_load_invalid_message_dict(tmp_path, monkeypatch):
     monkeypatch.setattr(history, "BASE_DIR", str(tmp_path))
     filepath = history._path("invalid_msg_session")
@@ -118,9 +140,18 @@ def test_clear_default_session(tmp_path, monkeypatch):
     assert not os.path.isfile(history._path(history.DEFAULT_SESSION))
 
 
-def test_save_write_failure_resilience(tmp_path, monkeypatch):
+def test_save_write_failure_resilience(tmp_path, monkeypatch, capsys):
+    """A failed save must warn, never raise, and leave no partial session file."""
     monkeypatch.setattr(history, "BASE_DIR", str(tmp_path))
     messages = [HumanMessage(content="Test")]
-    with patch("builtins.open", side_effect=OSError("Disk full or permission denied")):
+    with patch("gcode.history.os.replace", side_effect=OSError("Disk full or permission denied")):
         # Should not raise exception
         history.save("fail_session", messages)
+
+    path = history._path("fail_session")
+    assert not os.path.isfile(path)  # no partial file at the real path
+    leftovers = [name for name in os.listdir(tmp_path) if name.endswith(".tmp")]
+    assert leftovers == []  # temp file cleaned up
+    err = capsys.readouterr().err
+    assert "could not save session history" in err
+    assert path in err