Fix tasks reverting after a client marks them done

Completing a task in a CalDAV client wrote through to the markdown
correctly, then the client silently un-completed it moments later.

`to_ics` stamped DTSTAMP and LAST-MODIFIED with the wall clock on every
serialization, and Radicale derives an item's ETag by hashing that
serialization. So an unchanged task minted a fresh ETag on every read, at
one-second granularity. A client that read the task, waited for the user
to tick the checkbox, then PUT with `If-Match: <the etag it holds>` always
got 412 Precondition Failed — and resolved that apparent conflict by
re-downloading the server copy, discarding the completion.

Persist a per-task `last_modified` in the sidecar instead, stamped only
when the task's content or status actually changes, and serve DTSTAMP,
LAST-MODIFIED and the Item's own last_modified from it. The ETag is now
stable while the task is, and changes exactly when the task does.

Schema goes to v2, migrated in place with ALTER TABLE: rebuilding the
database would regenerate every UID and replace clients' whole task list.

Also fixes a plain GET of an item returning 500 — Radicale asserts on
`Item.last_modified`, which was never passed. Only REPORT was covered by
tests, so nothing caught it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/mdcaldav/ical.py

@@ -63,7 +63,13 @@ def to_ics(
     last_modified: datetime | None = None,
     completed_at: datetime | None = None,
 ) -> str:
-    """Serialize one task as a standalone VCALENDAR containing a VTODO."""
+    """Serialize one task as a standalone VCALENDAR containing a VTODO.
+
+    `last_modified` must be stable for as long as the task is unchanged: it
+    lands in the output, and Radicale hashes the output to make the ETag. A
+    wall-clock default here would give every read a fresh ETag, and every
+    conditional PUT a 412.
+    """
     now = last_modified or datetime.now(timezone.utc)
     lines = [
         "BEGIN:VCALENDAR",

src/mdcaldav/index.py

@@ -23,7 +23,7 @@ from .globs import matches_any
 from .model import Status, Task, normalize_title
 from .parser import parse
 
-SCHEMA_VERSION = 1
+SCHEMA_VERSION = 2
 
 SCHEMA = """
 CREATE TABLE IF NOT EXISTS meta (schema_version INT, generation INT);
@@ -37,7 +37,8 @@ CREATE TABLE IF NOT EXISTS tasks (
   title_norm TEXT, content_hash TEXT,
   status TEXT, completed_at TEXT,
   parent_uid TEXT,
-  first_seen_gen INT, last_modified_gen INT, deleted_gen INT);
+  first_seen_gen INT, last_modified_gen INT, deleted_gen INT,
+  last_modified TEXT);
 CREATE INDEX IF NOT EXISTS tasks_lookup ON tasks(rel_path, heading_path, title_norm);
 CREATE INDEX IF NOT EXISTS tasks_sync ON tasks(last_modified_gen);
 CREATE INDEX IF NOT EXISTS tasks_deleted ON tasks(deleted_gen);
@@ -83,8 +84,20 @@ class Index:
                 "INSERT INTO meta (schema_version, generation) VALUES (?, 0)",
                 (SCHEMA_VERSION,),
             )
+        self._migrate()
         self.db.commit()
 
+    def _migrate(self) -> None:
+        """Add columns in place rather than rebuilding.
+
+        Dropping the database is safe but expensive: every UID is regenerated,
+        so clients see their whole task list replaced. Migrating keeps them.
+        """
+        columns = {r["name"] for r in self.db.execute("PRAGMA table_info(tasks)")}
+        if "last_modified" not in columns:
+            self.db.execute("ALTER TABLE tasks ADD COLUMN last_modified TEXT")
+        self.db.execute("UPDATE meta SET schema_version = ?", (SCHEMA_VERSION,))
+
     # ---- generations -----------------------------------------------------
 
     @property
@@ -328,7 +341,7 @@ class Index:
     def _persist(self, task: Task, rel: str, gen: int, *, is_new: bool) -> None:
         prior = self.db.execute(
             "SELECT status, completed_at, content_hash, last_modified_gen,"
-            " first_seen_gen FROM tasks WHERE uid = ?",
+            " first_seen_gen, last_modified FROM tasks WHERE uid = ?",
             (task.uid,),
         ).fetchone()
 
@@ -347,8 +360,18 @@ class Index:
         )
         last_gen = gen if changed else prior["last_modified_gen"]
 
+        # The ETag is a hash of the serialized VTODO, which carries this
+        # timestamp. Re-stamping an unchanged task would give it a new ETag on
+        # every read, and a client's conditional PUT would then always 412.
+        last_modified = None if changed else (prior["last_modified"] if prior else None)
+        last_modified = last_modified or _now()
+
         self.db.execute(
-            "INSERT OR REPLACE INTO tasks VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)",
+            "INSERT OR REPLACE INTO tasks ("
+            " uid, collection, rel_path, heading_path, group_path, sibling_index,"
+            " depth, title_norm, content_hash, status, completed_at, parent_uid,"
+            " first_seen_gen, last_modified_gen, deleted_gen, last_modified"
+            ") VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)",
             (
                 task.uid,
                 task.collection,
@@ -365,6 +388,7 @@ class Index:
                 prior["first_seen_gen"] if prior else gen,
                 last_gen,
                 None,
+                last_modified,
             ),
         )
 
@@ -391,6 +415,27 @@ class Index:
             return datetime.fromisoformat(row["completed_at"])
         return None
 
+    def last_modified(self, uid: str) -> datetime:
+        """When this task last actually changed — the basis of its ETag."""
+        row = self.db.execute(
+            "SELECT last_modified FROM tasks WHERE uid = ?", (uid,)
+        ).fetchone()
+        if row and row["last_modified"]:
+            return datetime.fromisoformat(row["last_modified"])
+        return datetime.now(timezone.utc)
+
+    def newest(self, collection: str | None = None) -> datetime:
+        """Newest task modification, for collection-level Last-Modified."""
+        sql = "SELECT MAX(last_modified) AS m FROM tasks WHERE deleted_gen IS NULL"
+        params: tuple = ()
+        if collection is not None:
+            sql += " AND collection = ?"
+            params = (collection,)
+        row = self.db.execute(sql, params).fetchone()
+        if row and row["m"]:
+            return datetime.fromisoformat(row["m"])
+        return datetime.now(timezone.utc)
+
     def collections(self) -> list[str]:
         rows = self.db.execute(
             "SELECT DISTINCT collection FROM tasks WHERE deleted_gen IS NULL"

src/mdcaldav/storage.py

@@ -75,16 +75,26 @@ class Collection(BaseCollection):
 
     @property
     def last_modified(self) -> str:
-        return _http_date(datetime.now(timezone.utc))
+        return _http_date(self._storage.index.newest(self._href))
 
     # ---- reading ---------------------------------------------------------
 
     def _item(self, uid: str) -> radicale_item.Item | None:
-        task = self._storage.index.tasks.get(uid)
+        index = self._storage.index
+        task = index.tasks.get(uid)
         if task is None or task.collection != self._href:
             return None
-        text = to_ics(task, completed_at=self._storage.index.completed_at(uid))
+        modified = index.last_modified(uid)
-        return radicale_item.Item(collection=self, href=f"{uid}.ics", text=text)
+        text = to_ics(
+            task, last_modified=modified, completed_at=index.completed_at(uid)
+        )
+        # Radicale asserts on `last_modified` when serving a plain GET.
+        return radicale_item.Item(
+            collection=self,
+            href=f"{uid}.ics",
+            text=text,
+            last_modified=_http_date(modified),
+        )
 
     def get_multi(
         self, hrefs: Iterable[str]
@@ -260,7 +270,7 @@ class Principal(BaseCollection):
 
     @property
     def last_modified(self) -> str:
-        return _http_date(datetime.now(timezone.utc))
+        return _http_date(self._storage.index.newest())
 
     def get_multi(self, hrefs):
         for href in hrefs:

tests/test_caldav_integration.py

@@ -8,6 +8,7 @@ from __future__ import annotations
 
 import threading
 from pathlib import Path
+from urllib.parse import urlparse
 from wsgiref.simple_server import WSGIRequestHandler, make_server
 
 import pytest
@@ -216,3 +217,102 @@ def test_external_vim_edit_becomes_visible(server):
         for t in daily.todos(include_completed=True)
     }
     assert "Order more compost" in summaries
+
+
+def _etag(url: str, href: str) -> str:
+    """The ETag a client would cache for one task."""
+    import re
+    import urllib.request
+
+    body = (
+        b'<?xml version="1.0"?><propfind xmlns="DAV:">'
+        b"<prop><getetag/></prop></propfind>"
+    )
+    request = urllib.request.Request(
+        url + href,
+        data=body,
+        method="PROPFIND",
+        headers={"Depth": "0", "Content-Type": "application/xml"},
+    )
+    payload = urllib.request.urlopen(request, timeout=10).read().decode()
+    match = re.search(r"<[^>]*getetag[^>]*>([^<]+)<", payload)
+    assert match, payload
+    return match.group(1)
+
+
+def test_etag_is_stable_while_the_task_is_unchanged(server):
+    """Regression: an unchanged task must keep its ETag across reads.
+
+    DTSTAMP/LAST-MODIFIED were stamped with the wall clock on every
+    serialization, and Radicale hashes the serialization to make the ETag. Every
+    read therefore minted a new ETag, so a client's conditional PUT always hit
+    412 and the client reverted its own edit — a task ticked off in the app
+    would silently un-tick itself moments later.
+    """
+    import time
+
+    url, _ = server
+    daily = next(c for c in _calendars(url) if c.url.path.rstrip("/").endswith("daily"))
+    todo = next(
+        t
+        for t in daily.todos()
+        if str(t.icalendar_component.get("SUMMARY")) == "Call the arborist"
+    )
+    href = urlparse(str(todo.url)).path
+
+    first = _etag(url, href)
+    time.sleep(1.1)  # cross a second boundary — the old bug's granularity
+    assert _etag(url, href) == first
+
+
+def test_conditional_completion_survives_a_delay(server):
+    """The end-to-end symptom: tick a task off, and have it stay ticked off."""
+    import time
+    import urllib.request
+
+    url, vault = server
+    daily = next(c for c in _calendars(url) if c.url.path.rstrip("/").endswith("daily"))
+    todo = next(
+        t
+        for t in daily.todos()
+        if str(t.icalendar_component.get("SUMMARY")) == "Call the arborist"
+    )
+    href = urlparse(str(todo.url)).path
+    uid = str(todo.icalendar_component.get("UID"))
+    held = _etag(url, href)
+
+    time.sleep(1.1)  # the user takes a moment before hitting the checkbox
+    payload = (
+        "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//test//EN\r\nBEGIN:VTODO\r\n"
+        f"UID:{uid}\r\nDTSTAMP:20260802T120000Z\r\nSUMMARY:Call the arborist\r\n"
+        "STATUS:COMPLETED\r\nPERCENT-COMPLETE:100\r\nEND:VTODO\r\nEND:VCALENDAR\r\n"
+    )
+    request = urllib.request.Request(
+        url + href,
+        data=payload.encode(),
+        method="PUT",
+        headers={"Content-Type": "text/calendar", "If-Match": held},
+    )
+    response = urllib.request.urlopen(request, timeout=10)
+    assert response.status in (200, 201, 204)
+
+    assert "- [x] [#B] Call the arborist" in (vault / "daily/2026-01-07.md").read_text()
+    assert _etag(url, href) != held  # the client must see the change
+
+
+def test_item_get_returns_the_task(server):
+    """Regression: Item was built without `last_modified`, so GET asserted (500)."""
+    import urllib.request
+
+    url, _ = server
+    daily = next(c for c in _calendars(url) if c.url.path.rstrip("/").endswith("daily"))
+    todo = next(
+        t
+        for t in daily.todos()
+        if str(t.icalendar_component.get("SUMMARY")) == "Call the arborist"
+    )
+    href = urlparse(str(todo.url)).path
+
+    response = urllib.request.urlopen(url + href, timeout=10)
+    assert response.status == 200
+    assert "SUMMARY:Call the arborist" in response.read().decode()