fix(sync): resolve code-review findings on fetch-all history + retention

Addresses the review of PR #46 (#12/#13):
- Age-retention backfill/prune loop: mark a folder complete at the
  retention floor and resume from the persisted nextBeforeUid low-water
  mark; loosening resumes via AccountRepository.resetBackfillProgress.
- Guard the windowed reconcile bound to the lowest positive UID so a
  getUID==-1 message can't collapse it and wipe backfilled history.
- Order count-based retention by uid DESC to match the fetch window,
  ending the re-fetch/re-prune churn for high-UID/old-Date messages.
- BackfillWorker chains slices while work remains.
- Extract shared effectiveRetention / isActiveNetworkUnmetered /
  attachmentCacheDir helpers; remove dead deleteSyncedNotIn/getForAccount;
  refresh only pre-existing rows in persistBatch; add composite index
  (accountId, folder, uid) with migration + regenerated 13.json.

Adds an age-floor prune regression test. Fast gate + androidTest compile
green on JDK 21.

Follow-ups filed for below-the-cut findings: #93, #94, #95, #96.

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

app/schemas/org.libremail.data.local.LibreMailDatabase/13.json

@@ -2,7 +2,7 @@
   "formatVersion": 1,
   "database": {
     "version": 13,
-    "identityHash": "c064a4da054e0f98b4688ea2d04ecaec",
+    "identityHash": "e4f7ef1e0d780324d6eec3efced768ae",
     "entities": [
       {
         "tableName": "accounts",
@@ -256,6 +256,17 @@
             ],
             "orders": [],
             "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_timestampMillis` ON `${TABLE_NAME}` (`timestampMillis`)"
+          },
+          {
+            "name": "index_messages_accountId_folder_uid",
+            "unique": false,
+            "columnNames": [
+              "accountId",
+              "folder",
+              "uid"
+            ],
+            "orders": [],
+            "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId_folder_uid` ON `${TABLE_NAME}` (`accountId`, `folder`, `uid`)"
           }
         ]
       },
@@ -654,7 +665,7 @@
     ],
     "setupQueries": [
       "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
-      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'c064a4da054e0f98b4688ea2d04ecaec')"
+      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'e4f7ef1e0d780324d6eec3efced768ae')"
     ]
   }
 }

app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt

@@ -169,8 +169,9 @@ class LibreMailDatabaseTest {
         assertEquals(setOf("acct:INBOX:1", "acct:INBOX:2"), messageDao.getSyncedIds("acct", "INBOX").toSet())
         assertEquals(listOf("acct:Archive:1"), messageDao.getSyncedIds("acct", "Archive"))
 
-        // Reconciling the inbox must not touch other folders' rows.
-        messageDao.deleteSyncedNotIn("acct", "INBOX", listOf("acct:INBOX:1"))
+        // Reconciling the inbox must not touch other folders' rows (windowed reconcile; whole-inbox
+        // window since these rows have uid 0).
+        messageDao.deleteSyncedInWindowNotIn("acct", "INBOX", minWindowUid = 0, keepIds = listOf("acct:INBOX:1"))
         assertEquals(
             setOf("acct:INBOX:1", "acct:Archive:1"),
             messageDao.observeSummaries().first().map { it.id }.toSet(),

app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt

@@ -68,28 +68,30 @@ class MessageDaoRetentionTest {
     )
 
     /**
-     * The count-based prune boundary keeps the newest [keep] by recency and returns the REST for
-     * deletion, breaking timestamp ties by the higher UID. This pins the ordering *direction* (a
-     * flipped `DESC` would keep the OLDEST rows — i.e. locally delete the user's most recent mail) and
-     * the tie-break, neither of which the mocked-DAO unit tests can catch.
+     * The count-based prune boundary keeps the newest [keep] by ARRIVAL (server UID) and returns the
+     * REST for deletion. Keeping by UID — not by the Date header — matches the newest-by-UID recent
+     * window foreground sync re-fetches, so a high-UID/old-Date message isn't re-downloaded every sync
+     * and re-pruned every cycle. This pins the ordering column (a Date-ordered keep would evict the
+     * high-UID/old-Date row) and its direction (a flipped DESC would keep the OLDEST arrivals).
      */
     @Test
-    fun syncedIdsBeyondCountInFolderKeepsNewestByTimestampThenUid() = runBlocking {
+    fun syncedIdsBeyondCountInFolderKeepsNewestByArrivalUid() = runBlocking {
         dao.insertNew(
             listOf(
-                message("A", uid = 30, timestampMillis = 300), // newest
-                message("B", uid = 25, timestampMillis = 200), // ties C on timestamp; higher uid => newer
-                message("C", uid = 20, timestampMillis = 200),
-                message("D", uid = 10, timestampMillis = 100), // oldest
+                message("recent-old-date", uid = 100, timestampMillis = 50), // newest arrival, oldest Date
+                message("A", uid = 30, timestampMillis = 300),
+                message("B", uid = 25, timestampMillis = 200),
+                message("D", uid = 10, timestampMillis = 100),
                 // Scoping decoys: a search-only row and another folder must never enter the ranking.
                 message("SR", uid = 99, timestampMillis = 999, inInbox = false),
                 message("AR", uid = 5, timestampMillis = 50, folder = "Archive"),
             ),
         )
 
-        // Keep the newest 2 (A, B); the rest are prunable. B is kept over C purely by the uid tie-break.
+        // Keep the newest 2 by UID (recent-old-date, A); the rest are prunable. A Date-ordered keep would
+        // wrongly evict recent-old-date (oldest Date) and keep B.
         assertEquals(
-            setOf("C", "D"),
+            setOf("B", "D"),
             dao.syncedIdsBeyondCountInFolder("acct", "INBOX", keep = 2).toSet(),
         )
         // Keeping at least as many as exist prunes nothing.

app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt

@@ -58,6 +58,8 @@ class FakeAccountRepository(
     override suspend fun deleteAccount(id: String) {
         accountsFlow.value = accountsFlow.value.filterNot { it.id == id }
     }
+
+    override suspend fun resetBackfillProgress(accountId: String?) = Unit
 }
 
 /**

app/src/main/kotlin/org/libremail/data/AttachmentCache.kt

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.data
+
+import java.io.File
+
+/**
+ * The per-message on-disk attachment cache directory, keyed by a filesystem-safe form of the message
+ * id. Shared by the writer ([org.libremail.data.repository.MailRepositoryImpl]) and the retention
+ * pruner ([org.libremail.data.sync.MailPruner]) so the two can never disagree on where a message's
+ * attachments live — a divergence would silently leak orphaned files that the pruner no longer finds.
+ */
+internal fun attachmentCacheDir(cacheDir: File, messageId: String): File {
+    val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_")
+    return File(cacheDir, "attachments/$safeId")
+}

app/src/main/kotlin/org/libremail/data/local/Migrations.kt

@@ -259,5 +259,11 @@ val MIGRATION_12_13 = object : Migration(12, 13) {
                 "`nextBeforeUid` INTEGER NOT NULL, `complete` INTEGER NOT NULL, " +
                 "PRIMARY KEY(`accountId`, `folder`))",
         )
+
+        // Index the folder-scoped UID probes the backfill/reconcile hot paths run on every page/sync.
+        db.execSQL(
+            "CREATE INDEX IF NOT EXISTS `index_messages_accountId_folder_uid` " +
+                "ON `messages` (`accountId`, `folder`, `uid`)",
+        )
     }
 }

app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt

@@ -12,12 +12,13 @@ interface BackfillProgressDao {
     @Query("SELECT * FROM backfill_progress WHERE accountId = :accountId AND folder = :folder LIMIT 1")
     suspend fun get(accountId: String, folder: String): BackfillProgressEntity?
 
-    @Query("SELECT * FROM backfill_progress WHERE accountId = :accountId")
-    suspend fun getForAccount(accountId: String): List<BackfillProgressEntity>
-
     @Insert(onConflict = OnConflictStrategy.REPLACE)
     suspend fun upsert(progress: BackfillProgressEntity)
 
     @Query("DELETE FROM backfill_progress WHERE accountId = :accountId")
     suspend fun deleteForAccount(accountId: String)
+
+    /** Clears all backfill progress (e.g. when the global retention default changes) so it re-evaluates. */
+    @Query("DELETE FROM backfill_progress")
+    suspend fun deleteAll()
 }

app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt

@@ -41,6 +41,10 @@ interface MessageDao {
     @Insert(onConflict = OnConflictStrategy.IGNORE)
     suspend fun insertNew(messages: List<MessageEntity>)
 
+    /** Of the given [ids], those that already have a row — lets a caller refresh only pre-existing rows. */
+    @Query("SELECT id FROM messages WHERE id IN (:ids)")
+    suspend fun existingIds(ids: List<String>): List<String>
+
     /**
      * Refreshes the display fields (and the materialized [MessageEntity.uid], keeping it fresh for
      * rows migrated before the column existed) from the server without touching the cached body, the
@@ -87,18 +91,12 @@ interface MessageDao {
     @Query("DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1")
     suspend fun deleteSyncedByAccountFolder(accountId: String, folder: String)
 
-    /** Drops synced rows in [folder] for an account that are no longer present on the server. */
-    @Query(
-        "DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " +
-            "AND id NOT IN (:keepIds)",
-    )
-    suspend fun deleteSyncedNotIn(accountId: String, folder: String, keepIds: List<String>)
-
     /**
      * Windowed deletion reconcile for full-history sync (issue #12): within [folder], delete synced
      * rows whose UID falls inside the freshly-fetched recent window (`uid >= minWindowUid`) but which
      * the server no longer returns ([keepIds]). Rows below the window — older history fetched by the
-     * background backfill — are deliberately left intact, unlike [deleteSyncedNotIn].
+     * background backfill — are deliberately left intact, unlike a whole-folder "not in the recent
+     * set" reconcile, which would wipe that backfilled history.
      */
     @Query(
         "DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " +
@@ -130,14 +128,16 @@ interface MessageDao {
     suspend fun syncedIdsOlderThan(accountId: String, cutoffMillis: Long): List<String>
 
     /**
-     * Ids of an account's synced rows in [folder] beyond the newest [keep] by recency (count-based
-     * prune candidates). Ties broken by UID so the boundary is deterministic.
+     * Ids of an account's synced rows in [folder] beyond the newest [keep] by ARRIVAL (server UID —
+     * count-based prune candidates). Ordering by UID (not by the Date header) matches the newest-by-UID
+     * recent window [org.libremail.mail.ImapClient.fetchRecent] keeps fresh, so a message with a high
+     * UID but an old Date isn't re-fetched by every sync and re-pruned by every cycle.
      */
     @Query(
         "SELECT id FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " +
             "AND id NOT IN (" +
             "SELECT id FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " +
-            "ORDER BY timestampMillis DESC, uid DESC LIMIT :keep)",
+            "ORDER BY uid DESC LIMIT :keep)",
     )
     suspend fun syncedIdsBeyondCountInFolder(accountId: String, folder: String, keep: Int): List<String>
 

app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt

@@ -8,7 +8,10 @@ import androidx.room.PrimaryKey
 
 @Entity(
     tableName = "messages",
-    indices = [Index("accountId"), Index("timestampMillis")],
+    // The (accountId, folder, uid) index serves the folder-scoped UID probes the backfill/reconcile
+    // hot paths run on every page/sync: MIN(uid) (lowestSyncedUid) and the uid >= window bound
+    // (deleteSyncedInWindowNotIn / syncedIdsBeyondCountInFolder).
+    indices = [Index("accountId"), Index("timestampMillis"), Index("accountId", "folder", "uid")],
 )
 data class MessageEntity(
     @PrimaryKey val id: String,

app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt

@@ -79,4 +79,9 @@ class AccountRepositoryImpl @Inject constructor(
         folderDao.deleteForAccount(id)
         backfillProgressDao.deleteForAccount(id)
     }
+
+    override suspend fun resetBackfillProgress(accountId: String?) {
+        if (accountId != null) backfillProgressDao.deleteForAccount(accountId) else backfillProgressDao.deleteAll()
+        syncScheduler.backfillNow()
+    }
 }

app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt

@@ -9,6 +9,7 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
 import org.libremail.data.ReplyBuilder
 import org.libremail.data.SignatureBlock
+import org.libremail.data.attachmentCacheDir
 import org.libremail.data.local.dao.AccountDao
 import org.libremail.data.local.dao.AttachmentDao
 import org.libremail.data.local.dao.DraftDao
@@ -357,9 +358,8 @@ class MailRepositoryImpl @Inject constructor(
      * and avoids filename collisions between messages.
      */
     private fun attachmentFile(messageId: String, partIndex: Int, filename: String): File {
-        val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_")
         val safeName = filename.substringAfterLast('/').substringAfterLast('\\').ifBlank { "attachment" }
-        return File(context.cacheDir, "attachments/$safeId/$partIndex/$safeName")
+        return File(attachmentCacheDir(context.cacheDir, messageId), "$partIndex/$safeName")
     }
 }
 

app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 package org.libremail.data.settings
 
+import kotlinx.coroutines.flow.first
 import java.time.Instant
 import java.time.ZoneOffset
 
@@ -49,3 +50,25 @@ data class RetentionPolicy(val count: Int, val months: Int) {
             )
     }
 }
+
+/**
+ * The effective retention policy for [accountId], resolving its per-account overrides against the
+ * global default. Read through the same [AccountSettingsRepository] / [SettingsRepository] every
+ * enforcement site uses — the foreground fetch window ([org.libremail.data.sync.MailSyncer]), the
+ * backfill floor ([org.libremail.data.sync.MailBackfiller]), and the pruner
+ * ([org.libremail.data.sync.MailPruner]) — so none of them can resolve a different floor, the
+ * divergence that would otherwise let backfill and prune fight over the same rows.
+ */
+internal suspend fun AccountSettingsRepository.effectiveRetention(
+    settings: SettingsRepository,
+    accountId: String,
+): RetentionPolicy {
+    val account = get(accountId)
+    val global = settings.settings.first()
+    return RetentionPolicy.resolve(
+        accountCount = account.retentionCount,
+        accountMonths = account.retentionMonths,
+        defaultCount = global.retentionCount,
+        defaultMonths = global.retentionMonths,
+    )
+}

app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt

@@ -7,6 +7,7 @@ import androidx.work.CoroutineWorker
 import androidx.work.WorkerParameters
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
+import kotlinx.coroutines.CancellationException
 
 /**
  * Runs one bounded slice of the full-history backfill (issue #12). Cancellable (WorkManager stops it
@@ -21,8 +22,13 @@ class BackfillWorker @AssistedInject constructor(
     private val backfiller: MailBackfiller,
 ) : CoroutineWorker(appContext, workerParams) {
 
-    override suspend fun doWork(): Result = runCatching { backfiller.runBackfill() }.fold(
+    override suspend fun doWork(): Result = runCatching {
+        // Chain bounded slices back-to-back while history remains, so a large mailbox isn't limited to
+        // one slice per periodic run. runBackfill() returns true while any folder still has pages left;
+        // isStopped lets WorkManager end a long run gracefully (the periodic schedule resumes it).
+        while (backfiller.runBackfill() && !isStopped) { /* page the next slice */ }
+    }.fold(
         onSuccess = { Result.success() },
-        onFailure = { Result.retry() },
+        onFailure = { error -> if (error is CancellationException) throw error else Result.retry() },
     )
 }

app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt

@@ -2,14 +2,11 @@
 package org.libremail.data.sync
 
 import android.content.Context
-import android.net.ConnectivityManager
-import android.net.NetworkCapabilities
 import dagger.hilt.android.qualifiers.ApplicationContext
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.ensureActive
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import org.libremail.data.local.dao.AccountDao
@@ -23,6 +20,7 @@ import org.libremail.data.settings.AccountSettingsRepository
 import org.libremail.data.settings.FetchPolicy
 import org.libremail.data.settings.RetentionPolicy
 import org.libremail.data.settings.SettingsRepository
+import org.libremail.data.settings.effectiveRetention
 import org.libremail.domain.model.Account
 import org.libremail.domain.model.ImapConnectionParams
 import org.libremail.domain.repository.MailRepository
@@ -67,7 +65,7 @@ class MailBackfiller @Inject constructor(
         var moreWork = false
         for (account in accountDao.getAll().map { it.toDomain() }) {
             val params = runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continue
-            val policy = effectivePolicy(account.id)
+            val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id)
             for (folder in messageDao.syncedFolders(account.id)) {
                 if (remaining <= 0) return@withLock true
                 // Per-folder failures (e.g. a transient server error) must not abort the whole slice.
@@ -87,23 +85,29 @@ class MailBackfiller @Inject constructor(
         policy: RetentionPolicy,
         maxBatches: Int,
     ): FolderResult {
-        if (backfillProgressDao.get(account.id, folder)?.complete == true) {
+        val progress = backfillProgressDao.get(account.id, folder)
+        if (progress?.complete == true) {
             return FolderResult(batches = 0, complete = true)
         }
 
-        // Always page strictly below the LOWEST currently-cached UID. Deriving the boundary from the
-        // cache (rather than a stored cursor) keeps backfill gap-free even after the pruner raised the
-        // floor, and lets a later loosening of retention resume filling automatically. The mutex in
-        // runBackfill keeps the pruner from moving this boundary mid-run.
-        var beforeUid = messageDao.lowestSyncedUid(account.id, folder) ?: Long.MAX_VALUE
+        // Resume from the persisted low-water mark so paging is monotonic: it never re-descends into a
+        // region an earlier run already reached, even after the pruner deletes rows above it. Falls back
+        // to the lowest currently-cached UID on the very first run, before any progress is persisted.
+        var beforeUid = progress?.nextBeforeUid
+            ?: messageDao.lowestSyncedUid(account.id, folder)
+            ?: Long.MAX_VALUE
         var batches = 0
         while (batches < maxBatches) {
             currentCoroutineContext().ensureActive()
-            // Retention floor (#13 precedence): pause — but do NOT mark complete — once the device-only
-            // limit is reached, so backfill and the pruner never contend for the same messages and a
-            // later loosening of the limit resumes paging from where it stopped.
+            // Retention floor (#13 precedence): once the folder holds everything retention keeps, mark it
+            // complete and stop. Marking complete (rather than pausing) is what keeps backfill and the
+            // pruner from fighting: otherwise the pruner deleting aged-out rows would raise the oldest
+            // cached timestamp back above the age cutoff and re-open paging on the next run, forever. A
+            // retention change resets progress (AccountRepository.resetBackfillProgress) so loosening
+            // still resumes paging.
             if (reachedRetentionFloor(account.id, folder, policy)) {
-                return FolderResult(batches, complete = false)
+                markComplete(account.id, folder, beforeUid)
+                return FolderResult(batches, complete = true)
             }
             val fetched = imapClient.fetchOlderThan(params, folder, beforeUid, BACKFILL_BATCH_SIZE)
             batches++
@@ -138,18 +142,25 @@ class MailBackfiller @Inject constructor(
 
     /** Inserts backfilled headers; never deletes. Uncancellable so a persisted boundary always has its rows. */
     private suspend fun persistBatch(entities: List<MessageEntity>) = withContext(NonCancellable) {
-        messageDao.insertNew(entities)
         val ids = entities.map { it.id }
-        messageDao.markSynced(ids)
-        entities.forEach {
-            messageDao.updateHeaderContent(
-                id = it.id,
-                sender = it.sender,
-                senderEmail = it.senderEmail,
-                subject = it.subject,
-                timestampMillis = it.timestampMillis,
-                uid = it.uid,
-            )
+        // insertNew (IGNORE) writes brand-new rows in full — headers, uid, and inInbox = 1 — so only rows
+        // that ALREADY existed (e.g. a former search-only row) need their membership/header refreshed.
+        // Limiting the updates to those avoids a redundant per-row UPDATE for every freshly-inserted row.
+        val preexisting = messageDao.existingIds(ids).toHashSet()
+        messageDao.insertNew(entities)
+        val toRefresh = entities.filter { it.id in preexisting }
+        if (toRefresh.isNotEmpty()) {
+            messageDao.markSynced(toRefresh.map { it.id })
+            toRefresh.forEach {
+                messageDao.updateHeaderContent(
+                    id = it.id,
+                    sender = it.sender,
+                    senderEmail = it.senderEmail,
+                    subject = it.subject,
+                    timestampMillis = it.timestampMillis,
+                    uid = it.uid,
+                )
+            }
         }
     }
 
@@ -165,7 +176,7 @@ class MailBackfiller @Inject constructor(
     private suspend fun prefetchIfEnabled(ids: List<String>) {
         val shouldPrefetch = when (settingsRepository.fetchPolicy()) {
             FetchPolicy.ALWAYS -> true
-            FetchPolicy.WIFI_ONLY -> isUnmetered()
+            FetchPolicy.WIFI_ONLY -> context.isActiveNetworkUnmetered()
             FetchPolicy.ON_DEMAND -> false
         }
         if (!shouldPrefetch) return
@@ -175,23 +186,6 @@ class MailBackfiller @Inject constructor(
         }
     }
 
-    private suspend fun effectivePolicy(accountId: String): RetentionPolicy {
-        val account = accountSettingsRepository.get(accountId)
-        val global = settingsRepository.settings.first()
-        return RetentionPolicy.resolve(
-            accountCount = account.retentionCount,
-            accountMonths = account.retentionMonths,
-            defaultCount = global.retentionCount,
-            defaultMonths = global.retentionMonths,
-        )
-    }
-
-    private fun isUnmetered(): Boolean {
-        val manager = context.getSystemService(ConnectivityManager::class.java) ?: return false
-        val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false
-        return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
-    }
-
     private companion object {
         /** Headers fetched per server page. */
         const val BACKFILL_BATCH_SIZE = 50

app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt

@@ -5,14 +5,14 @@ import android.content.Context
 import dagger.hilt.android.qualifiers.ApplicationContext
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.ensureActive
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.sync.withLock
+import org.libremail.data.attachmentCacheDir
 import org.libremail.data.local.dao.AccountDao
 import org.libremail.data.local.dao.MessageDao
 import org.libremail.data.settings.AccountSettingsRepository
 import org.libremail.data.settings.RetentionPolicy
 import org.libremail.data.settings.SettingsRepository
-import java.io.File
+import org.libremail.data.settings.effectiveRetention
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -37,17 +37,10 @@ class MailPruner @Inject constructor(
 ) {
     /** Prunes every account to its effective retention policy. Returns the number of messages removed. */
     suspend fun prune(nowMillis: Long = System.currentTimeMillis()): Int = maintenanceGate.mutex.withLock {
-        val global = settingsRepository.settings.first()
         var removed = 0
         for (account in accountDao.getAll()) {
             currentCoroutineContext().ensureActive()
-            val settings = accountSettingsRepository.get(account.id)
-            val policy = RetentionPolicy.resolve(
-                accountCount = settings.retentionCount,
-                accountMonths = settings.retentionMonths,
-                defaultCount = global.retentionCount,
-                defaultMonths = global.retentionMonths,
-            )
+            val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id)
             if (policy.isUnlimited) continue
             removed += pruneAccount(account.id, policy, nowMillis)
         }
@@ -80,8 +73,7 @@ class MailPruner @Inject constructor(
 
     /** Removes the per-message on-disk attachment cache (keyed the same way MailRepositoryImpl writes it). */
     private fun deleteCacheFiles(messageId: String) {
-        val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_")
-        runCatching { File(context.cacheDir, "attachments/$safeId").deleteRecursively() }
+        runCatching { attachmentCacheDir(context.cacheDir, messageId).deleteRecursively() }
     }
 
     private companion object {

app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt

@@ -2,13 +2,10 @@
 package org.libremail.data.sync
 
 import android.content.Context
-import android.net.ConnectivityManager
-import android.net.NetworkCapabilities
 import dagger.hilt.android.qualifiers.ApplicationContext
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.ensureActive
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
@@ -18,8 +15,8 @@ import org.libremail.data.local.toDomain
 import org.libremail.data.local.toEntity
 import org.libremail.data.settings.AccountSettingsRepository
 import org.libremail.data.settings.FetchPolicy
-import org.libremail.data.settings.RetentionPolicy
 import org.libremail.data.settings.SettingsRepository
+import org.libremail.data.settings.effectiveRetention
 import org.libremail.domain.model.Account
 import org.libremail.domain.repository.MailRepository
 import org.libremail.mail.ImapClient
@@ -43,7 +40,7 @@ class MailSyncer @Inject constructor(
     // Serializes all syncing: syncAll/syncAccount/syncFolder are invoked concurrently by the periodic
     // worker, pull-to-refresh, one-shot syncs, folder opens, and one IDLE watcher per account. Without
     // this, two runs can both compute the same message as "new" (double-notify) or let a stale
-    // deleteSyncedNotIn snapshot delete a row another run just inserted.
+    // deleteSyncedInWindowNotIn snapshot delete a row another run just inserted.
     private val syncMutex = Mutex()
 
     /** Syncs every account's inbox. Succeeds if at least one account synced (or there are none). */
@@ -128,9 +125,14 @@ class MailSyncer @Inject constructor(
                     }
                     // Reconcile server-side deletions ONLY within the fetched recent-UID window, so older
                     // history paged in by the background backfill (issue #12) survives each foreground sync
-                    // instead of being wiped by a whole-folder "not in the recent 50" delete.
-                    val minWindowUid = entities.minOf { it.uid }
-                    messageDao.deleteSyncedInWindowNotIn(account.id, folder, minWindowUid, ids)
+                    // instead of being wiped by a whole-folder "not in the recent 50" delete. Bound the
+                    // window by the lowest POSITIVE fetched UID: a message whose UID couldn't be resolved
+                    // (UIDFolder.getUID returns -1) must not collapse the bound to <= 0 and turn this into a
+                    // whole-folder delete that wipes the backfilled history below the window.
+                    val minWindowUid = entities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull()
+                    if (minWindowUid != null) {
+                        messageDao.deleteSyncedInWindowNotIn(account.id, folder, minWindowUid, ids)
+                    }
                 }
 
                 val shouldNotify = notify &&
@@ -150,14 +152,7 @@ class MailSyncer @Inject constructor(
      * would immediately trim. Age-only or unlimited retention leaves the full window in place.
      */
     private suspend fun recentWindowFor(account: Account): Int {
-        val accountSettings = accountSettingsRepository.get(account.id)
-        val global = settingsRepository.settings.first()
-        val policy = RetentionPolicy.resolve(
-            accountCount = accountSettings.retentionCount,
-            accountMonths = accountSettings.retentionMonths,
-            defaultCount = global.retentionCount,
-            defaultMonths = global.retentionMonths,
-        )
+        val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id)
         return policy.countLimit?.let { minOf(FETCH_LIMIT, it) } ?: FETCH_LIMIT
     }
 
@@ -169,7 +164,7 @@ class MailSyncer @Inject constructor(
     private suspend fun prefetchIfEnabled(account: Account, folder: String) {
         val shouldPrefetch = when (settingsRepository.fetchPolicy()) {
             FetchPolicy.ALWAYS -> true
-            FetchPolicy.WIFI_ONLY -> isUnmetered()
+            FetchPolicy.WIFI_ONLY -> context.isActiveNetworkUnmetered()
             FetchPolicy.ON_DEMAND -> false
         }
         if (!shouldPrefetch) return
@@ -179,13 +174,6 @@ class MailSyncer @Inject constructor(
         }
     }
 
-    /** True when the active network is unmetered (e.g. Wi-Fi), used by [FetchPolicy.WIFI_ONLY]. */
-    private fun isUnmetered(): Boolean {
-        val manager = context.getSystemService(ConnectivityManager::class.java) ?: return false
-        val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false
-        return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
-    }
-
     private companion object {
         const val INBOX = "INBOX"
 

app/src/main/kotlin/org/libremail/data/sync/NetworkStatus.kt

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.data.sync
+
+import android.content.Context
+import android.net.ConnectivityManager
+import android.net.NetworkCapabilities
+
+/**
+ * True when the device's active network is unmetered (e.g. Wi-Fi). Shared by [MailSyncer] and
+ * [MailBackfiller] so both background jobs agree on what `Wi-Fi only` prefetch means; a divergent
+ * copy would let one job download on cellular while the other doesn't.
+ */
+internal fun Context.isActiveNetworkUnmetered(): Boolean {
+    val manager = getSystemService(ConnectivityManager::class.java) ?: return false
+    val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false
+    return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
+}

app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt

@@ -19,4 +19,11 @@ interface AccountRepository {
     suspend fun addOutlookAccount(email: String, accessToken: String, authStateJson: String): Result<List<String>>
 
     suspend fun deleteAccount(id: String)
+
+    /**
+     * Discards full-history backfill progress so it re-evaluates against the current retention floor
+     * after a retention change: tightening re-hits the (tighter) floor cheaply, loosening resumes
+     * paging older history. [accountId] null clears every account (a global-default change).
+     */
+    suspend fun resetBackfillProgress(accountId: String?)
 }

app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt

@@ -66,6 +66,7 @@ class AccountSettingsViewModel @Inject constructor(
         viewModelScope.launch {
             accountSettingsRepository.setRetentionCount(accountId, value)
             syncScheduler.pruneNow()
+            accountRepository.resetBackfillProgress(accountId)
         }
     }
 
@@ -73,6 +74,7 @@ class AccountSettingsViewModel @Inject constructor(
         viewModelScope.launch {
             accountSettingsRepository.setRetentionMonths(accountId, value)
             syncScheduler.pruneNow()
+            accountRepository.resetBackfillProgress(accountId)
         }
     }
 

app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt

@@ -66,11 +66,13 @@ class SettingsViewModel @Inject constructor(
     fun setRetentionCount(value: Int) = update {
         settingsRepository.setRetentionCount(value)
         syncScheduler.pruneNow()
+        accountRepository.resetBackfillProgress(null)
     }
 
     fun setRetentionMonths(value: Int) = update {
         settingsRepository.setRetentionMonths(value)
         syncScheduler.pruneNow()
+        accountRepository.resetBackfillProgress(null)
     }
 
     private inline fun update(crossinline action: suspend () -> Unit) {

app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt

@@ -35,6 +35,7 @@ import org.libremail.domain.model.ImapConnectionParams
 import org.libremail.domain.model.MailSecurity
 import org.libremail.domain.repository.MailRepository
 import org.libremail.mail.ImapClient
+import java.util.Date
 import java.util.Properties
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -142,14 +143,52 @@ class MailBackfillerTest {
 
         assertTrue(afterFirst >= 60, "must fetch at least up to the retention floor")
         assertTrue(afterFirst < TOTAL, "must NOT page the entire 120-message history")
-        // Paused at the floor (NOT marked complete, so a later loosening could resume), and stable:
-        // running again fetches nothing more.
-        assertFalse(progress["acct" to "INBOX"]!!.complete)
+        // Marked complete at the floor so the pruner deleting aged-out rows can't re-open paging (a
+        // retention change resets progress to resume); stable — running again fetches nothing more.
+        assertTrue(progress["acct" to "INBOX"]!!.complete)
         backfiller.runBackfill()
         assertEquals(afterFirst, cached.size, "at the floor, further runs must not fetch more")
         assertNoDeletes()
     }
 
+    /**
+     * Regression for the #12/#13 AGE-retention contention: reaching the age floor marks the folder
+     * complete, so the pruner deleting aged-out rows — which raises the oldest cached timestamp back
+     * above the cutoff — can't re-open paging. Before the fix, the next run re-fetched exactly the rows
+     * the pruner had just deleted, an endless re-download/re-prune loop. (The count floor was already a
+     * fixpoint, so only an age-based case exercises this.)
+     */
+    @Test
+    fun `reaching the age floor is sticky across a prune, so backfill never re-fetches`() = runTest {
+        val now = System.currentTimeMillis()
+        // UID order follows append order: the first 60 are ~8 months old (beyond the 6-month cutoff),
+        // the last 60 are recent (within retention).
+        val old = (1..60).map { now - 8 * MONTH_MILLIS - it * DAY_MILLIS }
+        val recent = (1..60).map { now - it * DAY_MILLIS }
+        appendMessages(old + recent)
+        seedForegroundWindow()
+
+        val backfiller = backfiller(AccountSettings("acct", retentionMonths = 6))
+        backfiller.runBackfill()
+
+        assertEquals(
+            true,
+            progress["acct" to "INBOX"]!!.complete,
+            "backfill marks the folder complete at the age floor",
+        )
+        val offeredBeforePrune = totalOffered
+
+        // Simulate the pruner: drop every cached row older than the 6-month cutoff. This raises the
+        // oldest cached timestamp back above the cutoff — the state that used to re-open paging.
+        val cutoff = now - 6 * MONTH_MILLIS
+        cached.removeAll { it.timestampMillis < cutoff }
+
+        backfiller.runBackfill()
+
+        assertEquals(offeredBeforePrune, totalOffered, "a floored folder must not re-fetch after a prune")
+        assertNoDeletes()
+    }
+
     /** Builds a backfiller wired to GreenMail with the in-memory fakes and the given account settings. */
     private fun backfiller(accountSettings: AccountSettings): MailBackfiller {
         val accountDao = mockk<AccountDao>()
@@ -222,12 +261,17 @@ class MailBackfillerTest {
     private fun assertNoDeletes() {
         val dao = lastMessageDao ?: return
         coVerify(exactly = 0) { dao.deleteByIds(any()) }
-        coVerify(exactly = 0) { dao.deleteSyncedNotIn(any(), any(), any()) }
         coVerify(exactly = 0) { dao.deleteSyncedInWindowNotIn(any(), any(), any(), any()) }
         coVerify(exactly = 0) { dao.deleteSyncedByAccountFolder(any(), any()) }
     }
 
-    private fun appendMessages(count: Int) {
+    private fun appendMessages(count: Int) = appendMessages(List<Long?>(count) { null })
+
+    /**
+     * Appends messages to INBOX with the given per-message sent dates (null = server default, ~now).
+     * UID order follows list order, so earlier entries get lower UIDs.
+     */
+    private fun appendMessages(sentDates: List<Long?>) {
         val props = Properties().apply {
             put("mail.store.protocol", "imap")
             put("mail.imap.host", "127.0.0.1")
@@ -239,12 +283,13 @@ class MailBackfillerTest {
         try {
             val inbox = store.getFolder("INBOX")
             inbox.open(Folder.READ_WRITE)
-            val messages = (1..count).map { i ->
+            val messages = sentDates.mapIndexed { i, millis ->
                 MimeMessage(session).apply {
                     setFrom(InternetAddress("sender$i@example.org"))
                     setRecipient(Message.RecipientType.TO, InternetAddress("alice@example.org"))
                     subject = "Message $i"
                     setText("Body of message $i")
+                    if (millis != null) sentDate = Date(millis)
                 }
             }.toTypedArray()
             inbox.appendMessages(messages)
@@ -257,5 +302,7 @@ class MailBackfillerTest {
     private companion object {
         const val TOTAL = 120
         const val WINDOW = 50
+        private const val DAY_MILLIS = 24L * 60 * 60 * 1000
+        private const val MONTH_MILLIS = 30L * DAY_MILLIS
     }
 }