From ffc0f1dcccf366fbef3f037e0c07a57e62d6a6d4 Mon Sep 17 00:00:00 2001 From: Achilleas90 Date: Mon, 15 Jun 2026 09:59:31 +0300 Subject: [PATCH] Harden CalDAV write-back with retries (#1193) Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> --- core/database.py | 44 ++++++ routes/calendar_routes.py | 94 +++++++++---- src/caldav_sync.py | 179 +++++++++++++++++++++++- src/caldav_writeback.py | 98 ++++++++++++- src/tool_implementations.py | 23 ++- tests/test_caldav_bidirectional_sync.py | 169 ++++++++++++++++++++++ tests/test_caldav_writeback.py | 10 +- tests/test_caldav_writeback_route.py | 14 +- tests/test_calendar_owner_scope.py | 1 + tests/test_null_owner_gates.py | 2 +- 10 files changed, 590 insertions(+), 44 deletions(-) create mode 100644 tests/test_caldav_bidirectional_sync.py diff --git a/core/database.py b/core/database.py index 6eec48d11..e4acc8d54 100644 --- a/core/database.py +++ b/core/database.py @@ -1602,6 +1602,7 @@ class CalendarCal(TimestampMixin, Base): # NULL for local calendars and for CalDAV calendars created before # multi-account support was added (treated as "use any configured account"). account_id = Column(String, nullable=True, index=True) + caldav_base_url = Column(String, nullable=True) events = relationship("CalendarEvent", back_populates="calendar", cascade="all, delete-orphan") @@ -1632,10 +1633,27 @@ class CalendarEvent(TimestampMixin, Base): # 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) + remote_href = Column(String, nullable=True) # CalDAV object URL for updates/deletes + remote_etag = Column(String, nullable=True) # Last seen CalDAV ETag, when available + caldav_sync_pending = Column(String, nullable=True) # create | update | delete retry marker calendar = relationship("CalendarCal", back_populates="events") +class CalendarDeletedEvent(TimestampMixin, Base): + """Hidden CalDAV delete tombstone retained until remote delete succeeds.""" + __tablename__ = "caldav_deleted_events" + + uid = Column(String, primary_key=True, index=True) + owner = Column(String, nullable=True, index=True) + calendar_id = Column(String, nullable=True, index=True) + remote_href = Column(String, nullable=True) + remote_etag = Column(String, nullable=True) + caldav_base_url = Column(String, nullable=True) + summary = Column(String, nullable=True) + last_error = Column(Text, nullable=True) + + class Integration(TimestampMixin, Base): """An external service connection (email, RSS, webhook, etc.).""" __tablename__ = "integrations" @@ -1767,6 +1785,7 @@ def init_db(): _migrate_add_calendar_is_utc() _migrate_add_calendar_origin() _migrate_add_calendar_account_id() + _migrate_add_caldav_sync_columns() _migrate_chat_messages_fts() _migrate_encrypt_email_passwords() _migrate_encrypt_signatures() @@ -2067,6 +2086,31 @@ def _migrate_add_calendar_account_id(): pass +def _migrate_add_caldav_sync_columns(): + """Add remote CalDAV metadata used for bidirectional sync.""" + import sqlite3 + db_path = DATABASE_URL.replace("sqlite:///", "") + if not os.path.exists(db_path): + return + try: + conn = sqlite3.connect(db_path) + ev_columns = [row[1] for row in conn.execute("PRAGMA table_info(calendar_events)").fetchall()] + if ev_columns and "remote_href" not in ev_columns: + conn.execute("ALTER TABLE calendar_events ADD COLUMN remote_href TEXT") + if ev_columns and "remote_etag" not in ev_columns: + conn.execute("ALTER TABLE calendar_events ADD COLUMN remote_etag TEXT") + if ev_columns and "caldav_sync_pending" not in ev_columns: + conn.execute("ALTER TABLE calendar_events ADD COLUMN caldav_sync_pending TEXT") + + cal_columns = [row[1] for row in conn.execute("PRAGMA table_info(calendars)").fetchall()] + if cal_columns and "caldav_base_url" not in cal_columns: + conn.execute("ALTER TABLE calendars ADD COLUMN caldav_base_url TEXT") + conn.commit() + conn.close() + except Exception as e: + logging.getLogger(__name__).warning(f"CalDAV sync metadata migration failed: {e}") + + def _migrate_add_calendar_metadata(): """Add importance/event_type/last_pinged columns to calendar_events table.""" import sqlite3 diff --git a/routes/calendar_routes.py b/routes/calendar_routes.py index 7b36df06a..87397e6fc 100644 --- a/routes/calendar_routes.py +++ b/routes/calendar_routes.py @@ -11,7 +11,7 @@ from pydantic import BaseModel from sqlalchemy import or_, and_ from dateutil.rrule import rrulestr -from core.database import SessionLocal, CalendarCal, CalendarEvent +from core.database import SessionLocal, CalendarCal, CalendarDeletedEvent, CalendarEvent from src.auth_helpers import require_user from src.upload_limits import read_upload_limited, ICS_MAX_BYTES @@ -126,6 +126,54 @@ def _resolve_base_uid(uid: str) -> str: raise ValueError("malformed compound UID: missing base before ::") return base + +async def _push_caldav_event_after_commit(owner: str, uid: str, action: str): + """Best-effort CalDAV write-through. Local writes stay authoritative if + the remote server is unreachable; pending flags let /sync retry later.""" + try: + result = {"ok": True} + if action == "create": + from src.caldav_sync import push_event_create + result = await push_event_create(owner, uid) + elif action == "update": + from src.caldav_sync import push_event_update + result = await push_event_update(owner, uid) + elif action == "delete": + from src.caldav_sync import push_event_delete + result = await push_event_delete(owner, uid) + if result and not result.get("ok") and not result.get("skipped"): + raise RuntimeError(result.get("error") or result) + except Exception as e: + logger.warning("CalDAV %s push failed for uid=%s: %s", action, uid, e) + if action in {"create", "update"}: + db = SessionLocal() + try: + ev = _get_or_404_event(db, uid, owner) + ev.caldav_sync_pending = action + db.commit() + except Exception: + db.rollback() + finally: + db.close() + + +def _record_caldav_delete_tombstone(db, ev: CalendarEvent, owner: str) -> None: + if not (ev.calendar and ev.calendar.source == "caldav"): + return + tombstone = db.query(CalendarDeletedEvent).filter( + CalendarDeletedEvent.uid == ev.uid, + CalendarDeletedEvent.owner == owner, + ).first() + if not tombstone: + tombstone = CalendarDeletedEvent(uid=ev.uid, owner=owner) + db.add(tombstone) + tombstone.calendar_id = ev.calendar_id + tombstone.remote_href = ev.remote_href + tombstone.remote_etag = ev.remote_etag + tombstone.caldav_base_url = getattr(ev.calendar, "caldav_base_url", None) + tombstone.summary = ev.summary or "" + tombstone.last_error = None + # ── Pydantic models ── class EventCreate(BaseModel): @@ -843,13 +891,13 @@ def setup_calendar_routes() -> APIRouter: return {"ok": False, "error": str(e)[:200]} @router.post("/sync") - async def sync_caldav_endpoint(request: Request): - """Pull events from the configured CalDAV server into local DB. + async def sync_caldav_endpoint(request: Request, direction: str = "pull"): + """Sync events with the configured CalDAV server. Returns counts + any per-calendar errors. Called by the frontend on calendar open and by the periodic scheduler loop.""" owner = _require_user(request) - from src.caldav_sync import sync_caldav - return await sync_caldav(owner) + from src.caldav_sync import sync_caldav_direction + return await sync_caldav_direction(owner, direction) @router.delete("/calendars/{cal_id}") @@ -1002,19 +1050,12 @@ def setup_calendar_routes() -> APIRouter: is_utc=_is_utc and not data.all_day, rrule=data.rrule or "", color=data.color or None, + caldav_sync_pending="create" if cal.source == "caldav" else None, ) db.add(ev) db.commit() if cal.source == "caldav": - # Push the new event to the remote so it appears on the user's - # other devices — the sync is otherwise pull-only (#800). - from src.caldav_writeback import writeback_event - await writeback_event(owner, cal.source, cal.id, { - "uid": uid, "summary": data.summary, "description": data.description, - "location": data.location, "dtstart": dtstart, "dtend": dtend, - "all_day": data.all_day, "is_utc": _is_utc and not data.all_day, - "rrule": data.rrule or "", - }) + await _push_caldav_event_after_commit(owner, uid, "create") return {"ok": True, "uid": uid} except HTTPException: raise @@ -1060,15 +1101,12 @@ def setup_calendar_routes() -> APIRouter: ev.rrule = data.rrule if data.color is not None: ev.color = data.color if data.color else None + is_caldav = ev.calendar and ev.calendar.source == "caldav" + if is_caldav: + ev.caldav_sync_pending = "update" db.commit() - cal = db.query(CalendarCal).filter(CalendarCal.id == ev.calendar_id).first() - if cal and cal.source == "caldav": - from src.caldav_writeback import writeback_event - await writeback_event(owner, cal.source, cal.id, { - "uid": ev.uid, "summary": ev.summary, "description": ev.description, - "location": ev.location, "dtstart": ev.dtstart, "dtend": ev.dtend, - "all_day": ev.all_day, "is_utc": ev.is_utc, "rrule": ev.rrule or "", - }) + if is_caldav: + await _push_caldav_event_after_commit(owner, base_uid, "update") return {"ok": True} except HTTPException: raise @@ -1089,15 +1127,13 @@ def setup_calendar_routes() -> APIRouter: db = SessionLocal() try: ev = _get_or_404_event(db, base_uid, owner) - # Capture what the remote push needs BEFORE the row is gone. - _cal = db.query(CalendarCal).filter(CalendarCal.id == ev.calendar_id).first() - _is_caldav = bool(_cal and _cal.source == "caldav") - _cal_id, _ev_uid = ev.calendar_id, ev.uid + is_caldav = ev.calendar and ev.calendar.source == "caldav" + if is_caldav: + _record_caldav_delete_tombstone(db, ev, owner) db.delete(ev) db.commit() - if _is_caldav: - from src.caldav_writeback import writeback_event - await writeback_event(owner, "caldav", _cal_id, {"uid": _ev_uid}, delete=True) + if is_caldav: + await _push_caldav_event_after_commit(owner, base_uid, "delete") return {"ok": True} except HTTPException: raise diff --git a/src/caldav_sync.py b/src/caldav_sync.py index e4afb89fd..4cf3c1e5a 100644 --- a/src/caldav_sync.py +++ b/src/caldav_sync.py @@ -128,6 +128,17 @@ def validate_caldav_url(raw_url: str) -> str: return urlunparse(parsed._replace(fragment="")).rstrip("/") +def _event_etag(obj) -> str: + """Best-effort ETag extraction from python-caldav resources.""" + try: + etag = getattr(obj, "etag", None) + if callable(etag): + etag = etag() + return str(etag or "") + except Exception: + return "" + + def _stable_cal_id(remote_url: str, owner: str = "", account_id: str = "") -> str: """Deterministic local id for a remote CalDAV calendar, scoped to owner and account so two users — or one user with two accounts — pointing at @@ -316,11 +327,12 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i color="#5b8abf", source="caldav", account_id=account_id or None, + caldav_base_url=remote_url, ) db.add(local_cal) db.commit() else: - # Refresh display name and stamp account_id if missing. + # Refresh display name and stamp CalDAV metadata if missing. changed = False if local_cal.name != display_name: local_cal.name = display_name @@ -328,6 +340,9 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i if account_id and not local_cal.account_id: local_cal.account_id = account_id changed = True + if local_cal.caldav_base_url != remote_url: + local_cal.caldav_base_url = remote_url + changed = True if changed: db.commit() result["calendars"] += 1 @@ -395,6 +410,9 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i existing = _find_existing_event(db, pending, uid_val, local_cal.id) if existing: + if existing.caldav_sync_pending in {"create", "update"}: + result["events"] += 1 + continue existing.calendar_id = local_cal.id existing.summary = summary existing.description = description @@ -405,6 +423,9 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i existing.is_utc = row_is_utc existing.rrule = rrule existing.origin = "caldav" + existing.remote_href = str(getattr(obj, "url", "") or "") or None + existing.remote_etag = _event_etag(obj) or None + existing.caldav_sync_pending = None else: new_ev = CalendarEvent( uid=uid_val, @@ -418,6 +439,8 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i is_utc=row_is_utc, rrule=rrule, origin="caldav", + remote_href=str(getattr(obj, "url", "") or "") or None, + remote_etag=_event_etag(obj) or None, ) db.add(new_ev) pending[uid_val] = new_ev @@ -442,6 +465,8 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i CalendarEvent.origin == "caldav", CalendarEvent.dtstart >= start, CalendarEvent.dtstart <= end, + CalendarEvent.remote_href.isnot(None), + CalendarEvent.caldav_sync_pending.is_(None), ~CalendarEvent.uid.in_(seen_uids) if seen_uids else CalendarEvent.uid.isnot(None), ).all() for ev in stale: @@ -458,6 +483,92 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i return result +def _event_payload(ev) -> dict: + return { + "uid": ev.uid, + "summary": ev.summary, + "description": ev.description, + "location": ev.location, + "dtstart": ev.dtstart, + "dtend": ev.dtend, + "all_day": ev.all_day, + "is_utc": ev.is_utc, + "rrule": ev.rrule or "", + } + + +def _load_event_for_writeback(owner: str, uid: str) -> tuple[str, str, dict] | None: + from core.database import CalendarCal, CalendarEvent, SessionLocal + + db = SessionLocal() + try: + ev = ( + db.query(CalendarEvent) + .join(CalendarCal) + .filter(CalendarEvent.uid == uid, CalendarCal.owner == owner) + .first() + ) + if not ev or not ev.calendar or ev.calendar.source != "caldav": + return None + return ev.calendar.source, ev.calendar.id, _event_payload(ev) + finally: + db.close() + + +def _load_delete_for_writeback(owner: str, uid: str) -> tuple[str, str, dict] | None: + from core.database import CalendarCal, CalendarDeletedEvent, CalendarEvent, SessionLocal + + db = SessionLocal() + try: + tombstone = db.query(CalendarDeletedEvent).filter( + CalendarDeletedEvent.uid == uid, + CalendarDeletedEvent.owner == owner, + ).first() + if tombstone: + return "caldav", tombstone.calendar_id, {"uid": uid} + + ev = ( + db.query(CalendarEvent) + .join(CalendarCal) + .filter(CalendarEvent.uid == uid, CalendarCal.owner == owner) + .first() + ) + if not ev or not ev.calendar or ev.calendar.source != "caldav": + return None + return ev.calendar.source, ev.calendar.id, {"uid": uid} + finally: + db.close() + + +def _pending_writeback_uids(owner: str) -> tuple[list[str], list[str]]: + from core.database import CalendarCal, CalendarDeletedEvent, CalendarEvent, SessionLocal + + db = SessionLocal() + try: + rows = ( + db.query(CalendarEvent.uid) + .join(CalendarCal) + .filter( + CalendarCal.owner == owner, + CalendarCal.source == "caldav", + CalendarEvent.status != "cancelled", + ( + (CalendarEvent.caldav_sync_pending.isnot(None)) + | (CalendarEvent.remote_href.is_(None)) + ), + ) + .all() + ) + delete_rows = ( + db.query(CalendarDeletedEvent.uid) + .filter(CalendarDeletedEvent.owner == owner) + .all() + ) + return [row[0] for row in rows], [row[0] for row in delete_rows] + finally: + db.close() + + def _load_caldav_accounts(owner: str) -> list: """Return the list of CalDAV accounts for *owner*, auto-migrating the legacy single-account ``caldav`` key to the new ``caldav_accounts`` list on first call. @@ -533,3 +644,69 @@ async def sync_caldav(owner: str) -> dict: for err in result.get("errors", []): totals["errors"].append(f"{label}: {err}") return totals + + +async def push_event_create(owner: str, uid: str) -> dict: + loaded = _load_event_for_writeback(owner, uid) + if not loaded: + return {"ok": True, "skipped": True} + source, calendar_id, payload = loaded + from src.caldav_writeback import writeback_event + return await writeback_event(owner, source, calendar_id, payload) + + +async def push_event_update(owner: str, uid: str) -> dict: + return await push_event_create(owner, uid) + + +async def push_event_delete(owner: str, uid: str) -> dict: + loaded = _load_delete_for_writeback(owner, uid) + if not loaded: + return {"ok": True, "skipped": True} + source, calendar_id, payload = loaded + from src.caldav_writeback import writeback_event + return await writeback_event(owner, source, calendar_id, payload, delete=True) + + +async def push_pending_events(owner: str) -> dict: + result = {"events": 0, "errors": []} + uids, delete_uids = _pending_writeback_uids(owner) + for event_uid in uids: + try: + out = await push_event_update(owner, event_uid) + if out.get("ok"): + result["events"] += 1 + elif not out.get("skipped"): + result["errors"].append(f"{event_uid}: {str(out.get('error') or out)[:160]}") + except Exception as e: + logger.warning("CalDAV pending push failed for uid=%s: %s", event_uid, e) + result["errors"].append(f"{event_uid}: {str(e)[:160]}") + for event_uid in delete_uids: + try: + out = await push_event_delete(owner, event_uid) + if out.get("ok"): + result["events"] += 1 + elif not out.get("skipped"): + result["errors"].append(f"{event_uid}: {str(out.get('error') or out)[:160]}") + except Exception as e: + logger.warning("CalDAV pending delete failed for uid=%s: %s", event_uid, e) + result["errors"].append(f"{event_uid}: {str(e)[:160]}") + return result + + +async def sync_caldav_direction(owner: str, direction: str = "pull") -> dict: + direction = (direction or "pull").strip().lower() + if direction == "pull": + return await sync_caldav(owner) + if direction == "push": + return await push_pending_events(owner) + if direction == "both": + pushed = await push_pending_events(owner) + pulled = await sync_caldav(owner) + return {"push": pushed, "pull": pulled} + return { + "calendars": 0, + "events": 0, + "deleted": 0, + "errors": [f"Unsupported CalDAV sync direction: {direction}"], + } diff --git a/src/caldav_writeback.py b/src/caldav_writeback.py index 0866e1467..ffb0021e3 100644 --- a/src/caldav_writeback.py +++ b/src/caldav_writeback.py @@ -89,6 +89,23 @@ def find_remote_calendar(calendars, local_cal_id: str, owner: str = "", account_ return None +def _resource_href(obj) -> str: + try: + return str(getattr(obj, "url", "") or "") + except Exception: + return "" + + +def _resource_etag(obj) -> str: + try: + etag = getattr(obj, "etag", None) + if callable(etag): + etag = etag() + return str(etag or "") + except Exception: + return "" + + def push_event(calendars, local_cal_id: str, ev: dict, *, delete: bool = False, owner: str = "", account_id: str = "") -> dict: """Create/update (or delete) ``ev`` on the matching remote calendar. @@ -105,6 +122,7 @@ def push_event(calendars, local_cal_id: str, ev: dict, *, delete: bool = False, remote = find_remote_calendar(calendars, local_cal_id, owner=owner, account_id=account_id) if remote is None: return {"ok": False, "error": "remote calendar not found"} + remote_url = str(getattr(remote, "url", "") or "") try: existing = remote.event_by_uid(uid) @@ -113,17 +131,34 @@ def push_event(calendars, local_cal_id: str, ev: dict, *, delete: bool = False, if delete: if existing is None: - return {"ok": True, "note": "already absent on remote"} + return {"ok": True, "note": "already absent on remote", "calendar_url": remote_url} existing.delete() - return {"ok": True} + return { + "ok": True, + "calendar_url": remote_url, + "remote_href": _resource_href(existing), + "remote_etag": _resource_etag(existing), + } ical = build_event_ical(ev) if existing is not None: existing.data = ical existing.save() - return {"ok": True, "updated": True} - remote.save_event(ical) - return {"ok": True, "created": True} + return { + "ok": True, + "updated": True, + "calendar_url": remote_url, + "remote_href": _resource_href(existing), + "remote_etag": _resource_etag(existing), + } + created = remote.save_event(ical) + return { + "ok": True, + "created": True, + "calendar_url": remote_url, + "remote_href": _resource_href(created), + "remote_etag": _resource_etag(created), + } def _discover_calendars(client): @@ -154,6 +189,54 @@ def _writeback_blocking(local_cal_id, ev, delete, url, username, password, owner=owner, account_id=account_id) +def _persist_writeback_result(owner: str, calendar_id: str, uid: str, result: dict, *, delete: bool) -> None: + from core.database import CalendarCal, CalendarDeletedEvent, CalendarEvent, SessionLocal + + if not uid or not isinstance(result, dict): + return + + db = SessionLocal() + try: + calendar = db.query(CalendarCal).filter( + CalendarCal.id == calendar_id, + CalendarCal.owner == owner, + ).first() + if calendar and result.get("calendar_url"): + calendar.caldav_base_url = result.get("calendar_url") + + if delete: + tombstone = db.query(CalendarDeletedEvent).filter( + CalendarDeletedEvent.uid == uid, + CalendarDeletedEvent.owner == owner, + ).first() + if result.get("ok"): + if tombstone: + db.delete(tombstone) + elif tombstone: + tombstone.last_error = str(result.get("error") or result)[:500] + db.commit() + return + + event = ( + db.query(CalendarEvent) + .join(CalendarCal) + .filter(CalendarEvent.uid == uid, CalendarCal.owner == owner) + .first() + ) + if event and result.get("ok"): + if result.get("remote_href"): + event.remote_href = result.get("remote_href") + if result.get("remote_etag"): + event.remote_etag = result.get("remote_etag") + event.caldav_sync_pending = None + db.commit() + except Exception: + db.rollback() + logger.exception("CalDAV write-back metadata persistence failed") + finally: + db.close() + + async def writeback_event(owner: str, calendar_source: str, calendar_id: str, ev: dict, *, delete: bool = False) -> dict: """Best-effort push of a local change to the remote CalDAV server. @@ -204,9 +287,12 @@ async def writeback_event(owner: str, calendar_source: str, calendar_id: str, result = await asyncio.to_thread( _writeback_blocking, calendar_id, ev, delete, url, user, pw, owner, acc_id ) + _persist_writeback_result(owner, calendar_id, (ev or {}).get("uid", ""), result, delete=delete) if not result.get("ok"): logger.warning("CalDAV write-back did not apply: %s", result.get("error") or result) return result except Exception as e: logger.exception("CalDAV write-back raised") - return {"ok": False, "error": str(e)[:200]} + result = {"ok": False, "error": str(e)[:200]} + _persist_writeback_result(owner, calendar_id, (ev or {}).get("uid", ""), result, delete=delete) + return result diff --git a/src/tool_implementations.py b/src/tool_implementations.py index 44aca917b..dee18c626 100644 --- a/src/tool_implementations.py +++ b/src/tool_implementations.py @@ -1445,7 +1445,15 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict: """Handle manage_calendar tool calls: list/create/update/delete calendar events (local SQLite).""" from datetime import datetime, timedelta from core.database import SessionLocal, CalendarCal, CalendarEvent, Note - from routes.calendar_routes import _ensure_default_calendar, _parse_dt, _parse_dt_pair, parse_due_for_user, _resolve_base_uid + from routes.calendar_routes import ( + _ensure_default_calendar, + _parse_dt, + _parse_dt_pair, + parse_due_for_user, + _resolve_base_uid, + _push_caldav_event_after_commit, + _record_caldav_delete_tombstone, + ) import uuid as _uuid try: @@ -1825,6 +1833,7 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict: rrule=args.get("rrule", "") or "", event_type=event_type, importance=importance, + caldav_sync_pending="create" if cal.source == "caldav" else None, ) db.add(ev) reminder_note_id = None @@ -1839,6 +1848,8 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict: dtstart_is_utc and not all_day, ) db.commit() + if cal.source == "caldav": + await _push_caldav_event_after_commit(owner, uid, "create") tag_blurb = f" [{event_type}]" if event_type else "" if minutes_before is None: reminder_blurb = "" @@ -1896,7 +1907,12 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict: ev.event_type = _tag or None if args.get("importance") is not None: ev.importance = args["importance"] + is_caldav = ev.calendar and ev.calendar.source == "caldav" + if is_caldav: + ev.caldav_sync_pending = "update" db.commit() + if is_caldav: + await _push_caldav_event_after_commit(owner, base_uid, "update") return {"response": f"Updated event {uid}", "exit_code": 0} elif action == "delete_event": @@ -1910,8 +1926,13 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict: ev = _event_query().filter(CalendarEvent.uid == base_uid).first() if not ev: return {"error": f"Event {uid} not found", "exit_code": 1} + is_caldav = ev.calendar and ev.calendar.source == "caldav" and ev.remote_href + if is_caldav: + _record_caldav_delete_tombstone(db, ev, owner) db.delete(ev) db.commit() + if is_caldav: + await _push_caldav_event_after_commit(owner, base_uid, "delete") return {"response": f"Deleted event {uid}", "exit_code": 0} else: diff --git a/tests/test_caldav_bidirectional_sync.py b/tests/test_caldav_bidirectional_sync.py new file mode 100644 index 000000000..f83dc450d --- /dev/null +++ b/tests/test_caldav_bidirectional_sync.py @@ -0,0 +1,169 @@ +"""Regression coverage for bidirectional CalDAV sync plumbing. + +These tests avoid a live CalDAV server. They pin the local invariants that keep +Odysseus-created CalDAV events from being pruned before they can be pushed. +""" + +from datetime import datetime +import importlib.util +from pathlib import Path +import sys + +from src.caldav_writeback import build_event_ical + + +def test_event_to_ical_serializes_core_fields_and_rrule(): + ical = build_event_ical({ + "uid": "evt-123", + "summary": "Planning", + "description": "Bring notes", + "location": "HQ", + "dtstart": datetime(2026, 6, 5, 9, 0), + "dtend": datetime(2026, 6, 5, 10, 0), + "all_day": False, + "is_utc": False, + "rrule": "FREQ=WEEKLY;COUNT=2", + }) + + assert "UID:evt-123" in ical + assert "SUMMARY:Planning" in ical + assert "DESCRIPTION:Bring notes" in ical + assert "LOCATION:HQ" in ical + assert "RRULE:FREQ=WEEKLY;COUNT=2" in ical + + +def test_caldav_pull_prune_skips_unsynced_or_pending_local_rows(): + source = Path("src/caldav_sync.py").read_text() + + assert 'existing.caldav_sync_pending in {"create", "update"}' in source + assert "CalendarEvent.remote_href.isnot(None)" in source + assert "CalendarEvent.caldav_sync_pending.is_(None)" in source + + +def test_http_calendar_writes_mark_pending_and_push_after_commit(): + source = Path("routes/calendar_routes.py").read_text() + + assert 'caldav_sync_pending="create" if cal.source == "caldav" else None' in source + assert 'ev.caldav_sync_pending = "update"' in source + assert 'await _push_caldav_event_after_commit(owner, uid, "create")' in source + assert 'await _push_caldav_event_after_commit(owner, base_uid, "update")' in source + assert 'await _push_caldav_event_after_commit(owner, base_uid, "delete")' in source + assert "_record_caldav_delete_tombstone(db, ev, owner)" in source + assert 'not result.get("ok")' in source + + +def test_agent_calendar_writes_share_caldav_push_path(): + source = Path("src/tool_implementations.py").read_text() + + assert "_push_caldav_event_after_commit" in source + assert 'caldav_sync_pending="create" if cal.source == "caldav" else None' in source + assert 'ev.caldav_sync_pending = "update"' in source + assert 'await _push_caldav_event_after_commit(owner, uid, "create")' in source + assert 'await _push_caldav_event_after_commit(owner, base_uid, "update")' in source + assert 'await _push_caldav_event_after_commit(owner, base_uid, "delete")' in source + assert "_record_caldav_delete_tombstone(db, ev, owner)" in source + + +def test_database_declares_and_migrates_caldav_remote_metadata(): + source = Path("core/database.py").read_text() + + for needle in [ + "class CalendarDeletedEvent", + "remote_href = Column(String, nullable=True)", + "remote_etag = Column(String, nullable=True)", + "caldav_sync_pending = Column(String, nullable=True)", + "caldav_base_url = Column(String, nullable=True)", + "ALTER TABLE calendar_events ADD COLUMN remote_href TEXT", + "ALTER TABLE calendar_events ADD COLUMN remote_etag TEXT", + "ALTER TABLE calendar_events ADD COLUMN caldav_sync_pending TEXT", + "ALTER TABLE calendars ADD COLUMN caldav_base_url TEXT", + "_migrate_add_caldav_sync_columns()", + ]: + assert needle in source + + +def test_failed_remote_delete_leaves_tombstone_and_later_retry_cleans_up(tmp_path, monkeypatch): + import src.caldav_writeback as writeback + + monkeypatch.setenv("DATABASE_URL", f"sqlite:///{tmp_path / 'calendar.db'}") + spec = importlib.util.spec_from_file_location("core.database", Path("core/database.py")) + dbmod = importlib.util.module_from_spec(spec) + monkeypatch.setitem(sys.modules, "core.database", dbmod) + spec.loader.exec_module(dbmod) + + CalendarCal = dbmod.CalendarCal + CalendarDeletedEvent = dbmod.CalendarDeletedEvent + CalendarEvent = dbmod.CalendarEvent + TestingSessionLocal = dbmod.SessionLocal + + session = TestingSessionLocal() + try: + cal = CalendarCal( + id="caldav-test", + owner="alice", + name="Remote", + source="caldav", + caldav_base_url="https://caldav.example/calendars/alice/main/", + ) + ev = CalendarEvent( + uid="evt-delete", + calendar_id=cal.id, + summary="Delete me", + dtstart=datetime(2026, 6, 5, 9, 0), + dtend=datetime(2026, 6, 5, 10, 0), + remote_href="https://caldav.example/calendars/alice/main/evt-delete.ics", + ) + session.add(cal) + session.add(ev) + session.commit() + + tombstone = CalendarDeletedEvent( + uid=ev.uid, + owner="alice", + calendar_id=ev.calendar_id, + remote_href=ev.remote_href, + remote_etag=ev.remote_etag, + caldav_base_url=cal.caldav_base_url, + summary=ev.summary, + ) + session.add(tombstone) + session.delete(ev) + session.commit() + + assert session.query(CalendarEvent).filter_by(uid="evt-delete").first() is None + tombstone = session.query(CalendarDeletedEvent).filter_by(uid="evt-delete").first() + assert tombstone is not None + assert tombstone.remote_href.endswith("evt-delete.ics") + finally: + session.close() + + writeback._persist_writeback_result( + "alice", + "caldav-test", + "evt-delete", + {"ok": False, "error": "temporary remote delete failure"}, + delete=True, + ) + + session = TestingSessionLocal() + try: + tombstone = session.query(CalendarDeletedEvent).filter_by(uid="evt-delete").first() + assert tombstone is not None + assert "temporary remote delete failure" in tombstone.last_error + finally: + session.close() + + writeback._persist_writeback_result( + "alice", + "caldav-test", + "evt-delete", + {"ok": True}, + delete=True, + ) + + session = TestingSessionLocal() + try: + assert session.query(CalendarDeletedEvent).filter_by(uid="evt-delete").first() is None + assert session.query(CalendarEvent).filter_by(uid="evt-delete").first() is None + finally: + session.close() diff --git a/tests/test_caldav_writeback.py b/tests/test_caldav_writeback.py index 7776e7541..fde2d1934 100644 --- a/tests/test_caldav_writeback.py +++ b/tests/test_caldav_writeback.py @@ -22,7 +22,9 @@ CAL_ID = _stable_cal_id(REMOTE_URL) class FakeEvent: - def __init__(self): + def __init__(self, url="https://p69-caldav.icloud.com/123/calendars/home/evt-1.ics"): + self.url = url + self.etag = '"abc123"' self.data = "OLD" self.saved = False self.deleted = False @@ -39,6 +41,7 @@ class FakeCalendar: self.url = url self._existing = existing self.saved_ical = None + self.created = FakeEvent(str(url).rstrip("/") + "/created.ics") def event_by_uid(self, uid): if self._existing is None: @@ -47,6 +50,7 @@ class FakeCalendar: def save_event(self, ical): self.saved_ical = ical + return self.created def _ev(**over): @@ -91,6 +95,8 @@ def test_push_create_calls_save_event(): res = push_event([cal], CAL_ID, _ev(), delete=False) assert res["ok"] and res.get("created") assert cal.saved_ical and "UID:evt-1" in cal.saved_ical + assert res["calendar_url"] == REMOTE_URL + assert res["remote_href"].endswith("/created.ics") def test_push_update_overwrites_existing(): @@ -100,6 +106,8 @@ def test_push_update_overwrites_existing(): assert res["ok"] and res.get("updated") assert existing.saved and "SUMMARY:Moved" in existing.data assert cal.saved_ical is None # used update path, not create + assert res["remote_href"].endswith("evt-1.ics") + assert res["remote_etag"] == '"abc123"' def test_push_delete_removes_existing(): diff --git a/tests/test_caldav_writeback_route.py b/tests/test_caldav_writeback_route.py index 8a5753a9d..a38703635 100644 --- a/tests/test_caldav_writeback_route.py +++ b/tests/test_caldav_writeback_route.py @@ -20,7 +20,7 @@ from sqlalchemy.pool import NullPool import core.database as cdb import routes.calendar_routes as croutes -import src.caldav_writeback as wb +import src.caldav_sync as csync from core.database import CalendarCal from routes.calendar_routes import EventCreate @@ -39,11 +39,16 @@ croutes.SessionLocal = _TS def calls(monkeypatch): recorded = [] - async def _fake_writeback(owner, source, cal_id, ev, *, delete=False): - recorded.append({"source": source, "cal_id": cal_id, "uid": ev.get("uid"), "delete": delete}) + async def _fake_create(owner, uid): + recorded.append({"uid": uid, "delete": False, "action": "create"}) return {"ok": True} - monkeypatch.setattr(wb, "writeback_event", _fake_writeback) + async def _fake_delete(owner, uid): + recorded.append({"uid": uid, "delete": True, "action": "delete"}) + return {"ok": True} + + monkeypatch.setattr(csync, "push_event_create", _fake_create) + monkeypatch.setattr(csync, "push_event_delete", _fake_delete) return recorded @@ -77,7 +82,6 @@ async def test_create_on_caldav_calendar_pushes_to_remote(calls): summary="Dentist", dtstart="2026-06-10T14:00:00Z", calendar_href=cal_id)) assert res["ok"] is True assert len(calls) == 1 - assert calls[0]["source"] == "caldav" and calls[0]["cal_id"] == cal_id assert calls[0]["delete"] is False diff --git a/tests/test_calendar_owner_scope.py b/tests/test_calendar_owner_scope.py index aa83d38cb..6006a4e1d 100644 --- a/tests/test_calendar_owner_scope.py +++ b/tests/test_calendar_owner_scope.py @@ -151,6 +151,7 @@ def _install_calendar_db_stub(monkeypatch): db = types.ModuleType("core.database") db.SessionLocal = MagicMock() db.CalendarCal = _CalendarCal + db.CalendarDeletedEvent = MagicMock() db.CalendarEvent = _CalendarEvent for name in [ "Base", diff --git a/tests/test_null_owner_gates.py b/tests/test_null_owner_gates.py index deada7e54..fee7e8fa0 100644 --- a/tests/test_null_owner_gates.py +++ b/tests/test_null_owner_gates.py @@ -28,7 +28,7 @@ from unittest.mock import MagicMock def _null_owner_stubs(monkeypatch): for _stub, _attrs in ( ("core.database", ( - "Base", "SessionLocal", "CalendarCal", "CalendarEvent", + "Base", "SessionLocal", "CalendarCal", "CalendarDeletedEvent", "CalendarEvent", "Document", "DocumentVersion", "Session", "ChatMessage", "GalleryImage", "GalleryAlbum", "Note", "ScheduledTask", "TaskRun", "ModelEndpoint", "Webhook",