feat(rules): save atomically, preserving unknown fields

Write to a temp file and rename, so the hook can never read a partial
file. Fields this version does not understand round-trip untouched,
which is what makes the format belong to neither tool.

2 files changed, 101 insertions, 0 deletions

diff --git a/mailrules.py b/mailrules.py
index 7df1b42..8c65708 100755
--- a/mailrules.py
+++ b/mailrules.py
@@ -31,6 +31,7 @@ runs on every sync, and mailctl has no dependencies to inherit.
 import json
 import os
 import re
+import tempfile
 from dataclasses import dataclass, field
 from pathlib import Path
 
@@ -125,6 +126,59 @@ def load(path=None):
     return store
 
 
+def save(store, path=None):
+    """Write the store atomically: a temp file in the same directory, then
+    rename. Rename within a filesystem is atomic, so a concurrent reader sees
+    either the old file or the new one and never a partial write.
+
+    There is no locking. Last writer wins on a true collision, which is
+    accepted for a single-user setup; the failure that would actually hurt is
+    a truncated read by the hook, and rename eliminates it.
+    """
+    path = Path(path) if path else default_path()
+    path.parent.mkdir(parents=True, exist_ok=True)
+
+    payload = dict(store.unknown)
+    payload["version"] = FORMAT_VERSION
+    payload["rules"] = [_rule_to_dict(r) for r in store.rules]
+
+    # delete=False plus an explicit replace: NamedTemporaryFile would unlink
+    # the file on close, and the rename is the whole point.
+    handle = tempfile.NamedTemporaryFile(
+        mode="w", dir=path.parent, prefix=".rules-", suffix=".tmp",
+        delete=False)
+    try:
+        with handle:
+            json.dump(payload, handle, indent=2, ensure_ascii=False)
+            handle.write("\n")
+            handle.flush()
+            os.fsync(handle.fileno())
+        os.replace(handle.name, path)
+    except BaseException:
+        # A failed write must not leave the temp file beside the real one.
+        try:
+            os.unlink(handle.name)
+        except OSError:
+            pass
+        raise
+
+
+def _rule_to_dict(rule):
+    """Known fields first in a stable order, then anything this version did
+    not understand. Stable ordering keeps a diff of this file readable."""
+    out = {
+        "id": rule.id,
+        "stage": rule.stage,
+        "enabled": rule.enabled,
+        "add": list(rule.add),
+        "remove": list(rule.remove),
+        "query": rule.query,
+        "note": rule.note,
+    }
+    out.update(rule.unknown)
+    return out
+
+
 def ordered(rules):
     """Enabled rules in execution order: by stage ascending, ties by position.
 
diff --git a/test_mailrules.py b/test_mailrules.py
index e08f490..a3faa5e 100755
--- a/test_mailrules.py
+++ b/test_mailrules.py
@@ -190,6 +190,53 @@ def test_ordered_skips_disabled_rules():
         assert [r.id for r in store.rules] == ["on", "off"]
 
 
+def test_save_round_trips_unknown_fields():
+    """The neutrality guarantee. If this tool strips a field qtmaildir
+    added, the file is this tool's file that qtmaildir may read."""
+    with tempfile.TemporaryDirectory() as tmp:
+        path = write_rules(tmp, {
+            "version": 1,
+            "future_top_level": {"set_by": "another tool"},
+            "rules": [{
+                "id": "keeper",
+                "add": ["x"],
+                "query": "from:a@example.com",
+                "future_field": [1, 2, 3],
+            }],
+        })
+        store = mailrules.load(path)
+        assert store.rules[0].unknown == {"future_field": [1, 2, 3]}
+
+        mailrules.save(store, path)
+
+        raw = json.loads(path.read_text())
+        assert raw["future_top_level"] == {"set_by": "another tool"}
+        assert raw["rules"][0]["future_field"] == [1, 2, 3]
+        assert raw["rules"][0]["id"] == "keeper"
+        assert raw["version"] == 1
+
+
+def test_save_is_atomic():
+    """A reader must never see a half-written file: the hook runs every ten
+    minutes and a truncated read would be a failed sync."""
+    with tempfile.TemporaryDirectory() as tmp:
+        path = Path(tmp) / "rules.json"
+        store = mailrules.Store(rules=[
+            mailrules.Rule(id="a", query="from:a@example.com", add=["x"])])
+        mailrules.save(store, path)
+        # The temp file the write went through must not be left behind.
+        assert [p.name for p in Path(tmp).iterdir()] == ["rules.json"]
+        assert json.loads(path.read_text())["rules"][0]["id"] == "a"
+
+
+def test_save_creates_the_directory():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = Path(tmp) / "nested" / "rules.json"
+        mailrules.save(mailrules.Store(), path)
+        assert path.exists()
+        assert json.loads(path.read_text()) == {"version": 1, "rules": []}
+
+
 def run_all():
     for name, fn in sorted(globals().items()):
         if name.startswith("test_") and callable(fn):