sync: drop a deleted occurrence once its PUT lands

markSynced ran over every row of a resource, tombstones included, and
only ever sets href, etag and is_dirty = 0. A deleted override was left
out of the PUT body by serialize, so the write already removed it from
the server — but the row stayed behind as is_deleted = 1 with is_dirty
cleared, which no phase can reach: the ETag we recorded from our own PUT
matches the server's, so nothing re-downloads it and apply's stale
sweep never runs. The unique index on (list_id, uid, recurrence_id)
then makes re-adding that occurrence throw for good, and the view
models swallow it.

stored() now deletes those rows instead. Overrides only: markDeleted
tombstones a series master without its overrides, so a deleted master
still arrives here, and dropping it would cascade the live overrides
away via master_id. A test pins that until markDeleted is fixed.

app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt

@@ -503,7 +503,22 @@ class CollectionSyncer(
 
         private fun stored(local: LocalResource, href: HttpUrl, eTag: String?) {
             val key = href.toString()
-            store.markSynced(local.rows.map { it.id }, key, eTag)
+            // ⚠️ Only the rows that were in the body. `serialize` writes
+            // `local.live`, so a tombstoned override was left out — the PUT *is*
+            // its deletion, and the row has no job left. Marking it synced
+            // instead strands `is_deleted = 1` behind a cleared `is_dirty`,
+            // where no phase can reach it: the ETag we just recorded matches the
+            // server's, so nothing re-downloads it and `apply`'s stale-override
+            // sweep never runs. The unique index on
+            // (list_id, uid, recurrence_id) then makes re-adding that occurrence
+            // throw for good.
+            //
+            // Overrides only, for now. A tombstoned *master* still reaches here
+            // because `markDeleted` does not tombstone a series' overrides, and
+            // deleting it would cascade the live ones away via `master_id`.
+            val (buried, kept) = local.rows.partition { it.isDeleted && it.recurrenceId != null }
+            if (buried.isNotEmpty()) store.deleteAll(buried.map { it.id })
+            store.markSynced(kept.map { it.id }, key, eTag)
             // Both, because a resource that failed as a create was counted under
             // its UID and is now counted under its href. Clearing one would leak
             // the other into the store forever.

app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt

@@ -24,7 +24,12 @@ interface SyncStore {
 
     fun deleteAll(taskIds: List<Long>)
 
-    /** Records href and ETag on a whole resource, clearing `is_dirty`. */
+    /**
+     * Records href and ETag on a resource's rows, clearing `is_dirty`.
+     *
+     * The caller excludes any row it left out of the body — that row is deleted,
+     * not marked synced.
+     */
     fun markSynced(taskIds: List<Long>, href: String?, eTag: String?)
 
     fun setParent(taskId: Long, parentId: Long?)

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskDao.kt

@@ -160,10 +160,13 @@ interface TaskDao {
     /**
      * Records where a resource lives and which version we hold.
      *
-     * Applied to every row of a resource at once, because a master and its
+     * Applied to a resource's rows at once, because a master and its
      * `RECURRENCE-ID` overrides share one href and one ETag — they are one file
-     * on the server. `is_dirty` is cleared **explicitly** rather than left to a
-     * default: a downstream write that leaves the flag set uploads what was just
+     * on the server. A tombstoned override is excluded by the caller: it was
+     * left out of the body, so the caller deletes the row instead.
+     *
+     * `is_dirty` is cleared **explicitly** rather than left to a default: a
+     * downstream write that leaves the flag set uploads what was just
      * downloaded, which is how a sync loop starts.
      */
     @Query("UPDATE tasks SET href = :href, etag = :etag, is_dirty = 0 WHERE id IN (:taskIds)")

app/src/test/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncerTest.kt

@@ -278,6 +278,66 @@ class CollectionSyncerTest {
         assertThat(store.rows).isEmpty()
     }
 
+    @Test fun `a deleted occurrence is dropped from the store once the PUT lands`() {
+        remote.put("one.ics", vtodo("a", "Weekly"))
+        store.rows += task(id = 1, uid = "a", title = "Weekly")
+            .copy(href = "http://server/dav/tasks/one.ics", etag = "e-one.ics", rrule = "FREQ=WEEKLY")
+        store.rows += task(id = 2, uid = "a", title = "Weekly")
+            .copy(
+                href = "http://server/dav/tasks/one.ics",
+                etag = "e-one.ics",
+                masterId = 1,
+                recurrenceId = NOW,
+                isDeleted = true,
+                isDirty = true,
+            )
+
+        sync()
+
+        // The occurrence left the body, so the PUT is its deletion. Marking the
+        // tombstone synced instead strands is_deleted = 1 with is_dirty = 0,
+        // which no phase can reach and which the unique index on
+        // (list_id, uid, recurrence_id) turns into a permanent failure to re-add.
+        assertThat(remote.log.any { it.startsWith("UPDATE one.ics") }).isTrue()
+        assertThat(remote.log.none { it.startsWith("DELETE") }).isTrue()
+        assertThat(store.rows.none { it.isDeleted }).isTrue()
+        assertThat(store.rows.map { it.id }).containsExactly(1L)
+        // FakeStore models neither the unique index on (list_id, uid,
+        // recurrence_id) nor the master_id cascade, so the permanent-failure-to-
+        // re-add consequence is reasoned about, not asserted here.
+        assertThat(store.rows.single().isDirty).isFalse()
+    }
+
+    @Test fun `a tombstoned master with a live override keeps its row`() {
+        remote.put("one.ics", vtodo("a", "Series"))
+        store.rows += task(id = 1, uid = "a", title = "Series").copy(
+            href = "http://server/dav/tasks/one.ics",
+            etag = "e-one.ics",
+            isDeleted = true,
+            isDirty = true,
+        )
+        store.rows += task(id = 2, uid = "a", title = "Series").copy(
+            href = "http://server/dav/tasks/one.ics",
+            etag = "e-one.ics",
+            masterId = 1,
+            recurrenceId = NOW,
+        )
+
+        sync()
+
+        // Pins today's behaviour, which is wrong and is not this fix's to correct:
+        // markDeleted tombstones only the master, so isDeleted (rows.all) is false,
+        // the resource goes to uploadPhase rather than deletePhase, and no DELETE
+        // is ever sent. Dropping the master row here instead would cascade the live
+        // override away via master_id and leave the server holding a resource
+        // nothing here has rows for — so only overrides are dropped for now.
+        assertThat(remote.log.none { it.startsWith("DELETE") }).isTrue()
+        assertThat(store.rows.map { it.id }).containsExactly(1L, 2L)
+        val master = store.rows.single { it.id == 1L }
+        assertThat(master.isDeleted).isTrue()
+        assertThat(master.isDirty).isFalse()
+    }
+
     @Test fun `the sweep does not remove what this run just created`() {
         store.rows += task(id = 1, uid = "a", title = "Fresh").copy(isDirty = true)