fix(caldav): skip the prune when any object fails to parse (#3454)

* fix(caldav): don't prune the whole window when no objects could be parsed

The post-sync prune deletes local origin=="caldav" rows in the window whose UID
the server didn't just return. With an empty seen_uids it falls back to
`uid.isnot(None)` — a match-all delete. That's right when the calendar is
genuinely empty, but when the server returns objects and every one fails to
parse (malformed iCal / an icalendar error), seen_uids is empty only because
nothing could be read, so the match-all branch silently deletes every local
event in the 90-day-back/365-day-forward window.

Track whether any object failed to parse and gate the prune with a small pure
helper `_should_prune_window(seen_uids, parse_failed)`: prune when something was
read, or when the calendar is genuinely empty (no objects, no parse errors), but
never when objects came back unreadable.

Adds tests/test_caldav_prune_parse_failure.py for the three cases.

* fix(caldav): skip the prune on any parse failure, not just total

Review follow-up (#3454): _should_prune_window returned True whenever seen_uids
was non-empty, so a partial parse failure (say 48 of 50 objects parse) still
pruned the 2 unreadable-but-still-upstream events, because their UIDs were absent
from seen_uids. Any parse failure makes seen_uids an incomplete view of the
server, so pruning against it is unsafe whether the failure is total or partial.

Skip the prune on any parse failure (return not parse_failed); only prune on a
clean read (a genuinely empty window is still safe to prune). Tradeoff: one
permanently-unparseable event pauses deletion mirroring until it is fixed, which
is the safe direction (false-keep beats false-delete).

Replace the now-incorrect "partial failure still prunes" assertion with a
partial-failure regression: one object parses, one fails, so the prune is
skipped and the unparsed event's local copy is not deleted.

---------

Co-authored-by: Kenny Van de Maele <kenny@kvandemaele.be>

src/caldav_sync.py

@@ -242,6 +242,20 @@ def _build_dav_client(url: str, username: str, password: str):
     return client
 
 
+def _should_prune_window(seen_uids: set, parse_failed: bool) -> bool:
+    """Whether the post-sync prune of vanished CalDAV events is safe to run.
+
+    The prune deletes local ``origin=="caldav"`` rows in the window whose UID the
+    server did not just return. Any parse failure (total or partial) makes
+    ``seen_uids`` an incomplete view of the server, so pruning against it can
+    delete events that still exist upstream but could not be read: a total
+    failure wipes the whole window, a partial failure deletes just the
+    unreadable ones. Only prune on a clean read. An empty ``seen_uids`` after a
+    clean read is a genuinely empty window, which is safe to prune.
+    """
+    return not parse_failed
+
+
 def _sync_blocking(owner: str, url: str, username: str, password: str, account_id: str = "") -> dict:
     """The actual sync — synchronous, intended to run in a threadpool.
     Returns counts: {calendars, events, deleted, errors}."""
@@ -328,6 +342,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i
                 # duplicate UIDs within the same batch are updated, not re-inserted
                 # (which would violate the UNIQUE constraint on commit).
                 pending: dict = {}
+                parse_failed = False
                 try:
                     objs = remote_cal.date_search(start=start, end=end, expand=False)
                 except Exception as e:
@@ -339,6 +354,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i
                         ical = iCal.from_ical(obj.data)
                     except Exception as e:
                         result["errors"].append(f"{display_name}: parse failed ({e})")
+                        parse_failed = True
                         continue
 
                     for comp in ical.walk():
@@ -415,17 +431,23 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i
                 # are prunable; locally-created events (agent / email triage / a
                 # UI event whose write-back failed) carry origin NULL and must
                 # never be deleted just because the server didn't return them.
-                stale = db.query(CalendarEvent).filter(
-                    CalendarEvent.calendar_id == local_cal.id,
-                    CalendarEvent.origin == "caldav",
-                    CalendarEvent.dtstart >= start,
-                    CalendarEvent.dtstart <= end,
-                    ~CalendarEvent.uid.in_(seen_uids) if seen_uids else CalendarEvent.uid.isnot(None),
-                ).all()
-                for ev in stale:
-                    db.delete(ev)
-                result["deleted"] += len(stale)
-                db.commit()
+                # Skip the prune on any parse failure: seen_uids is then an
+                # incomplete view of the server, so pruning against it would
+                # delete events that still exist upstream but could not be read
+                # (the empty-seen_uids case wipes the whole window; a partial
+                # failure deletes just the unreadable rows).
+                if _should_prune_window(seen_uids, parse_failed):
+                    stale = db.query(CalendarEvent).filter(
+                        CalendarEvent.calendar_id == local_cal.id,
+                        CalendarEvent.origin == "caldav",
+                        CalendarEvent.dtstart >= start,
+                        CalendarEvent.dtstart <= end,
+                        ~CalendarEvent.uid.in_(seen_uids) if seen_uids else CalendarEvent.uid.isnot(None),
+                    ).all()
+                    for ev in stale:
+                        db.delete(ev)
+                    result["deleted"] += len(stale)
+                    db.commit()
             except Exception as e:
                 logger.exception("CalDAV sync failed for one calendar")
                 result["errors"].append(str(e)[:200])

tests/test_caldav_prune_parse_failure.py

@@ -0,0 +1,37 @@
+"""CalDAV sync must not prune the window when it can't fully read the server.
+
+The prune deletes local caldav rows whose UID the server didn't return. `seen_uids`
+is built only from objects that parsed, so any parse failure (total or partial)
+makes it an incomplete view of the server:
+
+- total failure: `seen_uids` is empty and the prune falls back to `uid.isnot(None)`
+  (match-all), wiping every event in the window;
+- partial failure: the events that failed to parse are absent from `seen_uids`, so
+  `~uid.in_(seen_uids)` deletes those still-upstream events.
+
+`_should_prune_window` therefore only allows the prune on a clean read.
+"""
+from src.caldav_sync import _should_prune_window
+
+
+def test_prune_runs_on_clean_read():
+    # Clean read with events -> the normal ~uid.in_(seen) prune is safe.
+    assert _should_prune_window({"uid-a", "uid-b"}, parse_failed=False) is True
+
+
+def test_prune_runs_when_calendar_genuinely_empty():
+    # Clean read, no objects -> genuinely empty window -> safe to prune.
+    assert _should_prune_window(set(), parse_failed=False) is True
+
+
+def test_prune_skipped_when_all_objects_failed_to_parse():
+    # Every object failed -> empty seen_uids is "couldn't read", not "empty
+    # calendar" -> must NOT prune (would delete the whole window).
+    assert _should_prune_window(set(), parse_failed=True) is False
+
+
+def test_prune_skipped_on_partial_parse_failure():
+    # Some objects parsed and at least one failed: seen_uids is incomplete, so
+    # pruning would delete the unparsed-but-still-upstream events. Skipping the
+    # prune keeps the local copy of the unparsed event instead of deleting it.
+    assert _should_prune_window({"parsed-uid"}, parse_failed=True) is False