sickdays.gui: refuse a write while a background sync is in flight

sync() reads the pool, waits on the network, then writes its merged copy
back without a lock. A second write landing inside that window was
silently overwritten. Marking day A and then clearing day B within a
second or two lost the clear, so B stayed a sick day.

Every background sync now holds a module lock, and each write tries it
without blocking. A click that lands mid-sync shows "Syncing -- try again
in a moment." in the panel and calls on_saved with nothing. The phone's
picker avoids the same race by making the grid inert while it syncs.

The red-first test pins the lost write with a sync stub that reads, waits,
then writes back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PAhbNszs5JKdtnD4uBi3xp

sickdays/README.md

@@ -82,7 +82,7 @@ KeepPanel(parent, day, *, on_saved, on_cancel=None, paths=None).pack()
 
 Normal day: five switches, all off (off = stands down), "Mark sick"; sick
 day: its switches, "Save", "Clear sick day". Past/free/uncheckable days show
-why and offer no write; a refused write shows in the panel, never raised.
+why and offer no write; a refusal (or a click mid-sync) shows in the panel.
 `on_saved(SickDay | None)` fires after the write and sync start: the entry on
 Mark sick / a changed Save (`is_sick=True`) or Clear (`is_sick=False`), `None`
 on an unchanged Save (nothing written). The panel never destroys itself.

sickdays/sickdays/gui/_panel.py

@@ -39,6 +39,18 @@ if TYPE_CHECKING:
     from sickdays._paths import Paths
 
 
+#: Held by every background sync, and tried by every write. ``sync()`` reads
+#: the pool, waits on the network, then writes its merged copy back, so a
+#: write landing in between would be silently overwritten. A write that finds
+#: a sync running is refused ("try again") rather than lost.
+_SYNC_LOCK = threading.Lock()
+
+
+def _serialised_sync(paths: Paths) -> None:
+    with _SYNC_LOCK:
+        sync_quietly(paths=paths)
+
+
 def start_sync(paths: Paths) -> threading.Thread:
     """Push the write in the background so the phone sees it; never raises.
 
@@ -46,8 +58,8 @@ def start_sync(paths: Paths) -> threading.Thread:
     push half-way. The thread touches no widget.
     """
     thread = threading.Thread(
-        target=sync_quietly,
-        kwargs={"paths": paths},
+        target=_serialised_sync,
+        args=(paths,),
         name="sickdays-sync",
         daemon=False,
     )
@@ -66,8 +78,9 @@ class KeepPanel(tk.Frame):
     * "Save" with nothing changed -> ``None``: nothing written, no sync;
     * "Clear sick day" -> the cleared entry (``is_sick=False``).
 
-    A refused write (:class:`~sickdays.SickDayError`) is shown in the panel
-    and ``on_saved`` is not called. ``on_cancel``, when given, adds a
+    A refused write (:class:`~sickdays.SickDayError`, or a click while the
+    previous write's sync is still running) is shown in the panel and
+    ``on_saved`` is not called. ``on_cancel``, when given, adds a
     "Cancel" button that calls it. The panel never destroys itself; the
     callbacks decide what happens to it.
     """
@@ -171,11 +184,18 @@ class KeepPanel(tk.Frame):
         self._write(lambda: remove_sick_day(self._day, paths=self._paths))
 
     def _write(self, action: Callable[[], SickDay]) -> None:
+        # Non-blocking on purpose (no ``with``): waiting would freeze Tk for
+        # the whole network round trip.
+        if not _SYNC_LOCK.acquire(blocking=False):  # pylint: disable=consider-using-with
+            self._status.configure(text=_text.BUSY_NOTE, fg=tok.DANGER)
+            return
         try:
             entry = action()
         except SickDayError as exc:
             self._status.configure(text=str(exc), fg=tok.DANGER)
             return
+        finally:
+            _SYNC_LOCK.release()
         self.sync_thread = start_sync(self._paths)
         # Last: the callback may destroy this panel.
         self._on_saved(entry)

sickdays/sickdays/gui/_text.py

@@ -53,6 +53,7 @@ INTRO_DETAIL: Final = (
 )
 PAST_NOTE: Final = "This day has passed; it can no longer be changed."
 NONE_AHEAD: Final = "None ahead."
+BUSY_NOTE: Final = "Syncing \u2014 try again in a moment."
 WEEKDAY_INITIALS: Final = ("M", "T", "W", "T", "F", "S", "S")
 
 _WEEKDAYS: Final = ("Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun")

sickdays/sickdays/tests/test_gui_panel.py

@@ -4,16 +4,18 @@
 from __future__ import annotations
 
 from datetime import timedelta
+import threading
 import tkinter as tk
 from typing import TYPE_CHECKING
 
 import pytest
 
-from sickdays._api import add_sick_day, is_sick_day
+from sickdays._api import add_sick_day, is_sick_day, load
 from sickdays._day import today
 from sickdays._errors import ConflictCheckUnavailableError
 from sickdays._keep_api import kept_lockers
 from sickdays._paths import Paths
+from sickdays._store import write_pool
 from sickdays.gui import KeepPanel, _text
 from sickdays.gui import _tokens as tok
 
@@ -203,3 +205,41 @@ def test_explicit_paths_are_used_for_every_read_write_and_sync(
     assert not is_sick_day(FUTURE, log_path=other.log)
     assert synced == [other]
     assert not (tmp_path / "state" / "sick_days.json").exists()
+
+
+def test_a_write_during_a_sync_is_refused_not_lost(
+    tk_root: tk.Tk, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """sync() reads the pool, waits on the network, then writes its merged
+    copy back: a write landing in that window would be silently overwritten.
+    """
+    loaded = threading.Event()
+    release = threading.Event()
+    calls: list[int] = []
+
+    def slow_sync(*, paths: Paths | None = None) -> bool:
+        calls.append(1)
+        if len(calls) > 1 or paths is None:
+            return False
+        snapshot = load(paths.log)
+        loaded.set()
+        release.wait(timeout=5)
+        write_pool(paths.log, snapshot)
+        return True
+
+    monkeypatch.setattr("sickdays.gui._panel.sync_quietly", slow_sync)
+    saved: list[SickDay | None] = []
+    first = KeepPanel(tk_root, FUTURE, on_saved=saved.append)
+    _button(first, "mark").invoke()
+    assert loaded.wait(timeout=5)
+    later = FUTURE + timedelta(days=1)
+    second = KeepPanel(tk_root, later, on_saved=saved.append)
+    _button(second, "mark").invoke()
+    release.set()
+    _join(first)
+    if second.sync_thread is not None:
+        second.sync_thread.join(timeout=5)
+    for entry in saved:
+        assert entry is not None
+        assert is_sick_day(entry.day), f"{entry.day} was written, then lost"
+    assert _status(second) == _text.BUSY_NOTE