fix(caldav): don't prune locally-created events on sync (#2706)
The CalDAV pull prunes events in the synced calendar+window whose UID the server didn't just return, to propagate upstream deletions. But CalendarEvent had no field distinguishing a server-pulled row from a locally-created one, so the prune also deleted events that were never on the server: events created by the agent / email triage (which never write back to the server) and UI events whose best-effort write-back failed. Result: silent, unrecoverable loss of the user's appointments (hard db.delete, no soft-delete). Add an 'origin' column to calendar_events (lightweight idempotent migration, mirroring _migrate_add_calendar_is_utc), set origin='caldav' on rows the sync inserts/updates, and gate the prune on origin == 'caldav'. Locally-created events carry origin NULL and are never pruned. On the first sync after the migration nothing is pruned (all rows NULL until re-marked), erring toward keeping data. Fixes #2704
This commit is contained in:
@@ -1485,6 +1485,10 @@ class CalendarEvent(TimestampMixin, Base):
|
||||
importance = Column(String, default="normal") # low | normal | high | critical
|
||||
event_type = Column(String, nullable=True) # work | personal | health | travel | meal | social | admin | other
|
||||
last_pinged = Column(DateTime, nullable=True) # last time the assistant pinged about this event
|
||||
# "caldav" = pulled from a CalDAV server (so the sync may prune it when it
|
||||
# vanishes upstream). NULL/local = created locally (agent, email triage, or
|
||||
# a UI event whose write-back failed) and must NOT be pruned by the sync.
|
||||
origin = Column(String, nullable=True, index=True)
|
||||
|
||||
calendar = relationship("CalendarCal", back_populates="events")
|
||||
|
||||
@@ -1617,6 +1621,7 @@ def init_db():
|
||||
_migrate_seed_email_account()
|
||||
_migrate_add_calendar_metadata()
|
||||
_migrate_add_calendar_is_utc()
|
||||
_migrate_add_calendar_origin()
|
||||
_migrate_encrypt_email_passwords()
|
||||
_migrate_encrypt_signatures()
|
||||
_migrate_encrypt_endpoint_keys()
|
||||
@@ -1759,6 +1764,28 @@ def _migrate_add_calendar_is_utc():
|
||||
logging.getLogger(__name__).warning(f"is_utc migration failed: {e}")
|
||||
|
||||
|
||||
def _migrate_add_calendar_origin():
|
||||
"""Add `origin` to calendar_events so the CalDAV sync can tell server-pulled
|
||||
rows (prunable when they vanish upstream) from locally-created ones (agent /
|
||||
email triage / failed write-back), which must never be pruned. Idempotent."""
|
||||
import sqlite3
|
||||
db_path = DATABASE_URL.replace("sqlite:///", "")
|
||||
if not os.path.exists(db_path):
|
||||
return
|
||||
try:
|
||||
conn = sqlite3.connect(db_path)
|
||||
cursor = conn.execute("PRAGMA table_info(calendar_events)")
|
||||
columns = [row[1] for row in cursor.fetchall()]
|
||||
if columns and "origin" not in columns:
|
||||
conn.execute("ALTER TABLE calendar_events ADD COLUMN origin TEXT")
|
||||
conn.execute("CREATE INDEX IF NOT EXISTS ix_calendar_events_origin ON calendar_events(origin)")
|
||||
conn.commit()
|
||||
logging.getLogger(__name__).info("Migrated: added 'origin' column to calendar_events")
|
||||
conn.close()
|
||||
except Exception as e:
|
||||
logging.getLogger(__name__).warning(f"calendar_events.origin migration failed: {e}")
|
||||
|
||||
|
||||
def _migrate_add_calendar_metadata():
|
||||
"""Add importance/event_type/last_pinged columns to calendar_events table."""
|
||||
import sqlite3
|
||||
|
||||
@@ -265,6 +265,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict:
|
||||
existing.all_day = all_day
|
||||
existing.is_utc = row_is_utc
|
||||
existing.rrule = rrule
|
||||
existing.origin = "caldav"
|
||||
else:
|
||||
new_ev = CalendarEvent(
|
||||
uid=uid_val,
|
||||
@@ -277,6 +278,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict:
|
||||
all_day=all_day,
|
||||
is_utc=row_is_utc,
|
||||
rrule=rrule,
|
||||
origin="caldav",
|
||||
)
|
||||
db.add(new_ev)
|
||||
pending[uid_val] = new_ev
|
||||
@@ -286,8 +288,13 @@ def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict:
|
||||
# Prune locally-cached CalDAV events that vanished
|
||||
# upstream (only within our sync window — events outside
|
||||
# the window aren't in `objs`, so we'd false-delete them).
|
||||
# Only rows we previously pulled from the server (origin=="caldav")
|
||||
# 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),
|
||||
|
||||
101
tests/test_caldav_sync_prune_local_events.py
Normal file
101
tests/test_caldav_sync_prune_local_events.py
Normal file
@@ -0,0 +1,101 @@
|
||||
"""CalDAV sync must not prune locally-created events (#2704).
|
||||
|
||||
The prune step in `_sync_blocking` deletes events in the synced calendar+window
|
||||
whose UID the server didn't just return, to propagate upstream deletions. But
|
||||
`CalendarEvent` had no way to distinguish a server-pulled row from a locally
|
||||
created one (agent / email triage / a UI event whose write-back failed), so it
|
||||
also deleted events that were never on the server — silent data loss.
|
||||
|
||||
The fix adds an `origin` column and gates the prune on `origin == "caldav"`.
|
||||
This test replicates the exact prune query against an in-memory DB (the prune is
|
||||
pure DB logic; `_sync_blocking` itself needs a live CalDAV client) and asserts a
|
||||
local-origin event survives while a server-origin one with a vanished UID does
|
||||
not.
|
||||
"""
|
||||
import tempfile
|
||||
from datetime import datetime, timedelta
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
from sqlalchemy.pool import NullPool
|
||||
|
||||
import core.database as cdb
|
||||
from core.database import CalendarEvent, CalendarCal
|
||||
|
||||
_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
|
||||
_ENGINE = create_engine(
|
||||
f"sqlite:///{_TMPDB.name}",
|
||||
connect_args={"check_same_thread": False},
|
||||
poolclass=NullPool,
|
||||
)
|
||||
cdb.Base.metadata.create_all(_ENGINE)
|
||||
_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False)
|
||||
|
||||
_NOW = datetime(2026, 6, 4, 12, 0)
|
||||
_START = _NOW - timedelta(days=90)
|
||||
_END = _NOW + timedelta(days=365)
|
||||
|
||||
|
||||
def _prune(db, calendar_id, seen_uids):
|
||||
"""The exact prune filter from src/caldav_sync.py (post-fix)."""
|
||||
stale = db.query(CalendarEvent).filter(
|
||||
CalendarEvent.calendar_id == calendar_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)
|
||||
db.commit()
|
||||
return len(stale)
|
||||
|
||||
|
||||
def _seed():
|
||||
db = _TS()
|
||||
try:
|
||||
db.query(CalendarEvent).delete()
|
||||
db.query(CalendarCal).delete()
|
||||
db.add(CalendarCal(id="cal1", owner="alice", name="Work", source="caldav"))
|
||||
# A server-synced event whose UID is NO LONGER returned (deleted upstream).
|
||||
db.add(CalendarEvent(
|
||||
uid="server-gone@svc", calendar_id="cal1", summary="Old server event",
|
||||
dtstart=_NOW + timedelta(days=1), dtend=_NOW + timedelta(days=1, hours=1),
|
||||
origin="caldav",
|
||||
))
|
||||
# A locally-created event (agent / triage / failed write-back) — origin NULL.
|
||||
db.add(CalendarEvent(
|
||||
uid="local-uuid", calendar_id="cal1", summary="Dentist",
|
||||
dtstart=_NOW + timedelta(days=2), dtend=_NOW + timedelta(days=2, hours=1),
|
||||
origin=None,
|
||||
))
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def test_local_event_survives_prune():
|
||||
_seed()
|
||||
db = _TS()
|
||||
try:
|
||||
# Server returned nothing (both UIDs absent from seen_uids).
|
||||
deleted = _prune(db, "cal1", seen_uids={"some-other-uid"})
|
||||
# Only the server-origin, now-vanished event is pruned.
|
||||
assert deleted == 1
|
||||
assert db.query(CalendarEvent).filter_by(uid="local-uuid").first() is not None
|
||||
assert db.query(CalendarEvent).filter_by(uid="server-gone@svc").first() is None
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def test_synced_event_still_returned_is_kept():
|
||||
_seed()
|
||||
db = _TS()
|
||||
try:
|
||||
# The server still returns the synced event → it must be kept.
|
||||
deleted = _prune(db, "cal1", seen_uids={"server-gone@svc"})
|
||||
assert deleted == 0
|
||||
assert db.query(CalendarEvent).filter_by(uid="server-gone@svc").first() is not None
|
||||
assert db.query(CalendarEvent).filter_by(uid="local-uuid").first() is not None
|
||||
finally:
|
||||
db.close()
|
||||
Reference in New Issue
Block a user