From 961b54aa837f3903bfcff7ff7fbf148c3305d540 Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Sun, 5 Jul 2026 03:53:22 -0700
Subject: [PATCH] fix: make fsync best-effort so only mkstemp/replace failures
 fall back

---
 app/state_store.py            | 25 +++++++++++++++----------
 app/tests/test_state_store.py | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/app/state_store.py b/app/state_store.py
index 5de4b24..11bb5bb 100644
--- a/app/state_store.py
+++ b/app/state_store.py
@@ -137,7 +137,7 @@ class AtomicJsonStore:
             with os.fdopen(fd, "w", encoding="utf-8") as f:
                 self._write_payload(payload, f)
                 f.flush()
-                os.fsync(f.fileno())
+                self._best_effort_fsync(f.fileno())
             os.replace(tmp_path, self.path)
             self._fsync_directory(parent)
         except Exception:
@@ -151,15 +151,20 @@ class AtomicJsonStore:
         with open(self.path, "w", encoding="utf-8") as f:
             self._write_payload(payload, f)
             f.flush()
-            try:
-                os.fsync(f.fileno())
-            except OSError as exc:
-                # Tolerate fsync being unsupported on the underlying filesystem
-                # (the same class of filesystem that forced this fallback), but
-                # let genuine storage failures such as ENOSPC/EIO surface rather
-                # than reporting a durable write that did not happen.
-                if exc.errno not in _ATOMIC_UNSUPPORTED_ERRNOS:
-                    raise
+            self._best_effort_fsync(f.fileno())
+
+    @staticmethod
+    def _best_effort_fsync(fileno: int) -> None:
+        # Tolerate fsync being unsupported on the underlying filesystem (for
+        # example a network mount that returns EINVAL/ENOSYS), but let genuine
+        # storage failures such as ENOSPC/EIO surface so a non-durable write is
+        # never reported as success. An unsupported fsync must not by itself
+        # abandon the atomic rename path.
+        try:
+            os.fsync(fileno)
+        except OSError as exc:
+            if exc.errno not in _ATOMIC_UNSUPPORTED_ERRNOS:
+                raise
 
     @staticmethod
     def _write_payload(payload: dict[str, Any], f: Any) -> None:
diff --git a/app/tests/test_state_store.py b/app/tests/test_state_store.py
index 089ee95..0189808 100644
--- a/app/tests/test_state_store.py
+++ b/app/tests/test_state_store.py
@@ -71,6 +71,25 @@ class StateStoreTests(unittest.TestCase):
             self.assertEqual(ctx.exception.errno, 13)
             self.assertFalse(os.path.exists(path))
 
+    def test_unsupported_fsync_keeps_atomic_path(self):
+        # fsync being unsupported (EINVAL/ENOSYS) must not by itself trigger the
+        # direct-write fallback; the atomic temp-file + rename path still runs.
+        import errno as _errno
+
+        with tempfile.TemporaryDirectory() as tmp:
+            path = os.path.join(tmp, "queue.json")
+            store = AtomicJsonStore(path, kind="persistent_queue:queue")
+
+            with patch(
+                "state_store.os.fsync",
+                side_effect=OSError(_errno.EINVAL, "Invalid argument"),
+            ):
+                with self.assertNoLogs("state_store", level="WARNING"):
+                    store.save({"items": [{"key": "a"}]})
+
+            self.assertEqual(store.load()["items"], [{"key": "a"}])
+            self.assertEqual([], [name for name in os.listdir(tmp) if name.endswith(".tmp")])
+
     def test_save_reraises_and_preserves_state_on_non_atomic_errno(self):
         # A storage failure such as ENOSPC is not an "atomic unavailable"
         # signal, so it must surface instead of falling back to a direct write