Merge branch 'main' into fix-applock-lifecycle

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

@@ -21,12 +21,12 @@ import org.libremail.data.local.entity.MessageEntity
  *
  * These queries *define* the device-only retention floor — the pruner deletes below it
  * ([MessageDao.syncedIdsBeyondCountInFolder] / [MessageDao.syncedIdsOlderThan]) and the backfiller
- * stops above it ([MessageDao.lowestSyncedUid] / [MessageDao.countSynced] /
- * [MessageDao.oldestSyncedTimestamp]) — so the whole "backfill and prune never fight over the same
- * rows" guarantee rests on their SQL. The [org.libremail.data.sync.MailPruner] /
- * [org.libremail.data.sync.MailBackfiller] unit tests mock the DAO, so the `ORDER BY … DESC LIMIT`
- * newest-N selection, the strict age cutoff, and the windowed reconcile that spares backfilled history
- * are exercised here against a real database instead.
+ * pages and stops around it ([MessageDao.lowestSyncedUid] / [MessageDao.countSynced]) — so the whole
+ * "backfill and prune never fight over the same rows" guarantee rests on their SQL. The
+ * [org.libremail.data.sync.MailPruner] / [org.libremail.data.sync.MailBackfiller] unit tests mock
+ * the DAO, so the `ORDER BY … DESC LIMIT` newest-N selection, the strict age cutoff, and the
+ * windowed reconcile that spares backfilled history are exercised here against a real database
+ * instead.
  */
 @RunWith(AndroidJUnit4::class)
 class MessageDaoRetentionTest {
@@ -153,6 +153,9 @@ class MessageDaoRetentionTest {
     /**
      * The backfiller's floor probes reflect only an account's synced rows in the given folder, and are
      * null/zero for a folder with nothing cached (so the backfiller then starts from `Long.MAX_VALUE`).
+     * The paging boundary additionally skips `uid <= 0` placeholder rows (#95): a row migrated before
+     * the `uid` column existed (backfilled to 0) or one whose UID the server failed to resolve (-1)
+     * must not collapse MIN(uid) to a bound the backfiller treats as "folder fully paged".
      */
     @Test
     fun floorProbesReflectOnlySyncedRowsInTheFolder() = runBlocking {
@@ -160,18 +163,22 @@ class MessageDaoRetentionTest {
             listOf(
                 message("a", uid = 30, timestampMillis = 300),
                 message("d", uid = 10, timestampMillis = 100),
+                message("legacy", uid = 0, timestampMillis = 40), // pre-uid-column migration row (#95)
+                message("unresolved", uid = -1, timestampMillis = 30), // UIDFolder.getUID failure (#95)
                 message("search", uid = 1, timestampMillis = 1, inInbox = false), // excluded
                 message("archive", uid = 5, timestampMillis = 50, folder = "Archive"), // different folder
             ),
         )
 
         assertEquals(10L, dao.lowestSyncedUid("acct", "INBOX"))
-        assertEquals(2, dao.countSynced("acct", "INBOX"))
-        assertEquals(100L, dao.oldestSyncedTimestamp("acct", "INBOX"))
+        assertEquals(4, dao.countSynced("acct", "INBOX"))
 
         assertNull(dao.lowestSyncedUid("acct", "Nonexistent"))
         assertEquals(0, dao.countSynced("acct", "Nonexistent"))
-        assertNull(dao.oldestSyncedTimestamp("acct", "Nonexistent"))
+
+        // A folder holding ONLY placeholder rows has no usable boundary: null (start from the top).
+        dao.insertNew(listOf(message("only-legacy", uid = 0, folder = "Imported")))
+        assertNull(dao.lowestSyncedUid("acct", "Imported"))
     }
 
     /** Backfill / prune enumerate their targets via [syncedFolders]: distinct synced folders, per account. */

app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt

@@ -53,7 +53,7 @@ class OnboardingFlowTest {
     @get:Rule
     val composeTestRule = createAndroidComposeRule<ComponentActivity>()
 
-    private fun string(resId: Int) = composeTestRule.activity.getString(resId)
+    private fun string(resId: Int, vararg args: Any) = composeTestRule.activity.getString(resId, *args)
 
     // Generous cap for the slow, animation-disabled CI matrix emulators; waitUntil returns as soon
     // as the text appears, so the happy path is unaffected.
@@ -206,6 +206,10 @@ class OnboardingFlowTest {
         // add" button sit below the fold of this scrolling screen, and a positional click on an
         // off-screen button is a silent no-op (which is why this passed only on API 37's taller AVD).
         waitForText(string(R.string.app_password_email))
+        // Gmail requires 2-Step Verification before app passwords, so its screen (and only its
+        // screen — see yahooSetup_hasNoTwoFactorHelpLink) links Google's setup article (issue #98).
+        composeTestRule.onNodeWithText(string(R.string.app_password_2fa_help))
+            .performScrollTo().assertIsDisplayed()
         composeTestRule.onNodeWithText(string(R.string.app_password_email))
             .performScrollTo().performTextInput("e2e@gmail.com")
         composeTestRule.onNodeWithText(string(R.string.app_password_field))
@@ -221,4 +225,18 @@ class OnboardingFlowTest {
         waitForText("E2E first message")
         composeTestRule.onNodeWithText("E2E first message").assertIsDisplayed()
     }
+
+    @Test
+    fun yahooSetup_hasNoTwoFactorHelpLink() {
+        setOnboardingContent(FakeAccountRepository(), FakeMailRepository())
+
+        composeTestRule.onNodeWithText(string(R.string.onboarding_add_account)).performClick()
+        waitForText("Yahoo Mail")
+        composeTestRule.onNodeWithText("Yahoo Mail").performClick()
+
+        // Yahoo's setup screen keeps its app-password link…
+        waitForText(string(R.string.app_password_open_page, "Yahoo Mail"))
+        // …but gains no 2-Step Verification link: that prerequisite is Gmail-specific (issue #98).
+        composeTestRule.onNodeWithText(string(R.string.app_password_2fa_help)).assertDoesNotExist()
+    }
 }

app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt

@@ -10,6 +10,7 @@ import androidx.lifecycle.SavedStateHandle
 import androidx.room.Room
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.work.WorkManager
 import kotlinx.coroutines.runBlocking
 import org.junit.Rule
 import org.junit.Test
@@ -27,6 +28,7 @@ import org.libremail.domain.model.ServerConfig
 import org.libremail.ui.FakeAccountRepository
 import org.libremail.ui.navigation.Routes
 import org.libremail.ui.theme.LibreMailTheme
+import javax.inject.Provider
 
 /**
  * End-to-end test for the per-account settings screen: editing the signature and toggling the
@@ -68,7 +70,7 @@ class AccountSettingsScreenTest {
             accountRepository = FakeAccountRepository(accounts = listOf(account)),
             accountSettingsRepository = repository,
             signatureRepository = SignatureRepository(db.signatureDao()),
-            syncScheduler = SyncScheduler(context),
+            syncScheduler = SyncScheduler(Provider { WorkManager.getInstance(context) }),
         )
         composeTestRule.setContent {
             LibreMailTheme(darkTheme = false, dynamicColor = false) {

app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt

@@ -8,6 +8,7 @@ import androidx.compose.ui.test.performClick
 import androidx.compose.ui.test.performScrollTo
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import androidx.work.WorkManager
 import kotlinx.coroutines.runBlocking
 import org.junit.Rule
 import org.junit.Test
@@ -25,6 +26,7 @@ import org.libremail.data.sync.SyncScheduler
 import org.libremail.push.BatteryOptimizationManager
 import org.libremail.ui.FakeAccountRepository
 import org.libremail.ui.theme.LibreMailTheme
+import javax.inject.Provider
 
 /**
  * End-to-end test for the top-level message-downloading setting: tapping a policy must round-trip
@@ -54,7 +56,7 @@ class SettingsScreenTest {
             appLockManager,
             keyStore,
             BatteryOptimizationManager(context),
-            SyncScheduler(context),
+            SyncScheduler(Provider { WorkManager.getInstance(context) }),
         )
 
         composeTestRule.setContent {

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

@@ -118,21 +118,25 @@ interface MessageDao {
     )
     suspend fun deleteSyncedInWindowNotIn(accountId: String, folder: String, minWindowUid: Long, keepIds: List<String>)
 
-    /** Lowest cached UID among an account's synced rows in [folder] — the backfill boundary. Null if none. */
-    @Query("SELECT MIN(uid) FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1")
+    /**
+     * Lowest cached *resolved* UID among an account's synced rows in [folder] — the backfill
+     * boundary. Placeholder rows with `uid <= 0` (a row migrated before the `uid` column existed, or
+     * a fetch where the server failed to resolve the UID) are excluded: letting one collapse
+     * MIN(uid) to `<= 0` would make the backfiller page below a bound `fetchOlderThan` treats as
+     * "nothing older", falsely marking the folder fully backfilled (#95, matching the
+     * `minWindowUid` guard in MailSyncer). Null when no resolved-UID row exists, in which case
+     * backfill starts over from the newest message.
+     */
+    @Query(
+        "SELECT MIN(uid) FROM messages WHERE accountId = :accountId AND folder = :folder " +
+            "AND inInbox = 1 AND uid > 0",
+    )
     suspend fun lowestSyncedUid(accountId: String, folder: String): Long?
 
     /** Number of an account's synced rows in [folder] (count-based retention floor / prune sizing). */
     @Query("SELECT COUNT(*) FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1")
     suspend fun countSynced(accountId: String, folder: String): Int
 
-    /** Oldest cached timestamp among an account's synced rows in [folder] (age-based retention floor). Null if none. */
-    @Query(
-        "SELECT MIN(timestampMillis) FROM messages " +
-            "WHERE accountId = :accountId AND folder = :folder AND inInbox = 1",
-    )
-    suspend fun oldestSyncedTimestamp(accountId: String, folder: String): Long?
-
     /** Distinct folders that have at least one synced row for [accountId] (backfill/prune targets). */
     @Query("SELECT DISTINCT folder FROM messages WHERE accountId = :accountId AND inInbox = 1")
     suspend fun syncedFolders(accountId: String): List<String>

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

@@ -54,12 +54,15 @@ class MailBackfiller @Inject constructor(
     private val mailRepository: MailRepository,
     private val maintenanceGate: MailMaintenanceGate,
 ) {
-    private data class FolderResult(val batches: Int, val complete: Boolean)
+    /** One folder's slice outcome: pages fetched, and whether an immediate follow-up slice has work to do. */
+    private data class FolderResult(val batches: Int, val moreWork: Boolean)
 
     /**
      * Runs one bounded slice of backfill across all accounts and their synced folders. Does at most
-     * [maxBatches] server pages total, persisting progress after each, then returns whether any
-     * folder still has history left to fetch (so the caller may schedule another run sooner).
+     * [maxBatches] server pages total, persisting progress after each, then returns whether an
+     * immediate follow-up slice has more work to do (so the caller may chain another run). A folder
+     * that stalled (see the unresolved-UID guard in [backfillFolder]) stays incomplete but does not
+     * count as more work — it is retried on a future scheduled run instead of spun on back-to-back.
      */
     suspend fun runBackfill(maxBatches: Int = DEFAULT_MAX_BATCHES): Boolean = maintenanceGate.mutex.withLock {
         var remaining = maxBatches
@@ -71,9 +74,9 @@ class MailBackfiller @Inject constructor(
                 if (remaining <= 0) return@withLock true
                 // Per-folder failures (e.g. a transient server error) must not abort the whole slice.
                 val result = runCatching { backfillFolder(account, params, folder, policy, remaining) }
-                    .getOrElse { FolderResult(batches = 0, complete = false) }
+                    .getOrElse { FolderResult(batches = 0, moreWork = true) }
                 remaining -= result.batches
-                if (!result.complete) moreWork = true
+                if (result.moreWork) moreWork = true
             }
         }
         moreWork
@@ -88,57 +91,88 @@ class MailBackfiller @Inject constructor(
     ): FolderResult {
         val progress = backfillProgressDao.get(account.id, folder)
         if (progress?.complete == true) {
-            return FolderResult(batches = 0, complete = true)
+            return FolderResult(batches = 0, moreWork = false)
         }
 
         // 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
+        // Both sources are guarded against unresolved-UID placeholders (#95): lowestSyncedUid excludes
+        // `uid <= 0` rows at the SQL level, and a stale persisted boundary `<= 0` is discarded rather
+        // than trusted — fetchOlderThan treats such a bound as "nothing older", which would falsely
+        // mark the folder fully backfilled.
+        var beforeUid = progress?.nextBeforeUid?.takeIf { it > 0L }
             ?: messageDao.lowestSyncedUid(account.id, folder)
             ?: Long.MAX_VALUE
         var batches = 0
+        var complete = false
+        var stalled = false
         while (batches < maxBatches) {
             currentCoroutineContext().ensureActive()
-            // 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)) {
-                markComplete(account.id, folder, beforeUid)
-                return FolderResult(batches, complete = true)
+            // Count floor (#13 precedence): once the folder holds as many messages as retention keeps,
+            // stop before fetching another page. Unlike the age floor below, this can be decided from
+            // the cache alone: the count pruner keeps the newest-N by UID — exactly the order paging
+            // descends in — so a Date/UID inversion cannot make it stop early.
+            if (reachedCountFloor(account.id, folder, policy)) {
+                complete = true
+                break
             }
             val fetched = imapClient.fetchOlderThan(params, folder, beforeUid, BACKFILL_BATCH_SIZE)
             batches++
-            if (fetched.isEmpty()) {
-                // Genuine end of the folder — mark complete so it is skipped on future runs.
-                markComplete(account.id, folder, beforeUid)
-                return FolderResult(batches, complete = true)
-            }
             val entities = fetched.map { it.toEntity(account.id, folder) }
+            // Stop at the genuine end of the folder, or at the age floor (#13): a page ENTIRELY older
+            // than the Date cutoff. The age decision is made from the page actually fetched, NOT from
+            // the oldest cached timestamp — paging is by UID (arrival order) while the floor cuts by
+            // the Date header, so a single high-UID message with an old Date (moved/imported mail)
+            // would drag the cached minimum below the cutoff and end paging while within-retention
+            // history is still unfetched (#94). An entirely-old page is pure prune-fodder, so it is
+            // not persisted either. Both cases mark the folder complete (rather than pausing): the
+            // pruner deleting aged-out rows must never re-open paging on the next run, forever
+            // re-downloading what was just pruned. A retention change resets progress
+            // (AccountRepository.resetBackfillProgress) so loosening still resumes paging.
+            if (fetched.isEmpty() || entirelyBeyondAgeFloor(entities, policy)) {
+                complete = true
+                break
+            }
             persistBatch(entities)
-            beforeUid = entities.minOf { it.uid }
+            // Derive the next boundary only from resolved UIDs (#95): a row whose UID the server
+            // failed to resolve (UIDFolder.getUID returns -1) must not collapse the boundary to <= 0,
+            // where fetchOlderThan reads "nothing older" and the folder would be FALSELY marked fully
+            // backfilled. If a whole page came back unresolved, stall the folder: not complete (so a
+            // later run retries once the server behaves) but claiming no more work either — an
+            // immediate follow-up slice would just spin on the same page.
+            val nextBeforeUid = entities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull()
+            if (nextBeforeUid == null) {
+                stalled = true
+                break
+            }
+            beforeUid = nextBeforeUid
             backfillProgressDao.upsert(BackfillProgressEntity(account.id, folder, beforeUid, complete = false))
             prefetchIfEnabled(entities.map { it.id })
             // Breathe between pages so a large mailbox doesn't hammer the server.
             delay(BACKFILL_BATCH_DELAY_MS)
         }
-        return FolderResult(batches, complete = false)
+        if (complete) markComplete(account.id, folder, beforeUid)
+        return FolderResult(batches, moreWork = !complete && !stalled)
     }
 
-    /** True once the folder already holds as much as the retention policy would keep (or more). */
-    private suspend fun reachedRetentionFloor(accountId: String, folder: String, policy: RetentionPolicy): Boolean {
-        if (policy.isUnlimited) return false
-        policy.countLimit?.let { limit ->
-            if (messageDao.countSynced(accountId, folder) >= limit) return true
-        }
-        policy.ageCutoffMillis(System.currentTimeMillis())?.let { cutoff ->
-            val oldest = messageDao.oldestSyncedTimestamp(accountId, folder)
-            if (oldest != null && oldest < cutoff) return true
-        }
-        return false
+    /** True once the folder already holds as many messages as the count retention keeps (or more). */
+    private suspend fun reachedCountFloor(accountId: String, folder: String, policy: RetentionPolicy): Boolean {
+        val limit = policy.countLimit ?: return false
+        return messageDao.countSynced(accountId, folder) >= limit
+    }
+
+    /**
+     * True when a fetched page sits entirely below the age retention floor — every message on it is
+     * older than the policy's Date cutoff. Deciding per page (rather than from the single oldest
+     * cached timestamp) makes the floor robust to Date/UID inversions (#94): one old-Dated high-UID
+     * message ends paging only if a whole page around it is old too. The trade is deliberate — a
+     * pathologically interleaved mailbox may over-fetch (the pruner reclaims the excess), but
+     * backfill never silently gaps within-retention history.
+     */
+    private fun entirelyBeyondAgeFloor(page: List<MessageEntity>, policy: RetentionPolicy): Boolean {
+        val cutoff = policy.ageCutoffMillis(System.currentTimeMillis()) ?: return false
+        return page.isNotEmpty() && page.all { it.timestampMillis < cutoff }
     }
 
     /** Inserts backfilled headers; never deletes. Uncancellable so a persisted boundary always has its rows. */

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

@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 package org.libremail.data.sync
 
-import android.content.Context
 import androidx.work.Constraints
 import androidx.work.ExistingPeriodicWorkPolicy
 import androidx.work.ExistingWorkPolicy
@@ -10,15 +9,20 @@ import androidx.work.OneTimeWorkRequestBuilder
 import androidx.work.OutOfQuotaPolicy
 import androidx.work.PeriodicWorkRequestBuilder
 import androidx.work.WorkManager
-import dagger.hilt.android.qualifiers.ApplicationContext
 import java.util.concurrent.TimeUnit
 import javax.inject.Inject
+import javax.inject.Provider
 import javax.inject.Singleton
 
 /** Schedules background mail sync, full-history backfill, and retention pruning via WorkManager. */
 @Singleton
-class SyncScheduler @Inject constructor(@ApplicationContext private val context: Context) {
-    private val workManager get() = WorkManager.getInstance(context)
+class SyncScheduler @Inject constructor(
+    // A Provider (not the WorkManager itself) so WorkManager.getInstance() is resolved lazily at
+    // schedule time — never during Hilt's Application field injection, which can run before the
+    // HiltWorkerFactory that on-demand WorkManager initialization needs is set.
+    private val workManagerProvider: Provider<WorkManager>,
+) {
+    private val workManager get() = workManagerProvider.get()
 
     private val networkConstraint = Constraints.Builder()
         .setRequiredNetworkType(NetworkType.CONNECTED)
@@ -41,7 +45,7 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context:
         val request = PeriodicWorkRequestBuilder<SyncWorker>(15, TimeUnit.MINUTES)
             .setConstraints(networkConstraint)
             .build()
-        workManager.enqueueUniquePeriodicWork(PERIODIC_WORK, ExistingPeriodicWorkPolicy.KEEP, request)
+        workManager.enqueueUniquePeriodicWork(PERIODIC_WORK, PERIODIC_POLICY, request)
     }
 
     /** One-shot sync, e.g. right after an account is added. */
@@ -55,13 +59,13 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context:
 
     /**
      * Periodic full-history backfill (issue #12). Each run pages a bounded slice and persists its
-     * boundary, so history fills in over successive runs; KEEP preserves an already-scheduled cadence.
+     * boundary, so history fills in over successive runs.
      */
     fun schedulePeriodicBackfill() {
         val request = PeriodicWorkRequestBuilder<BackfillWorker>(30, TimeUnit.MINUTES)
             .setConstraints(backfillConstraint)
             .build()
-        workManager.enqueueUniquePeriodicWork(PERIODIC_BACKFILL, ExistingPeriodicWorkPolicy.KEEP, request)
+        workManager.enqueueUniquePeriodicWork(PERIODIC_BACKFILL, PERIODIC_POLICY, request)
     }
 
     /** Kicks an immediate backfill slice (e.g. just after an account is added) without waiting for the cadence. */
@@ -79,7 +83,7 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context:
         val request = PeriodicWorkRequestBuilder<PruneWorker>(12, TimeUnit.HOURS)
             .setConstraints(pruneConstraint)
             .build()
-        workManager.enqueueUniquePeriodicWork(PERIODIC_PRUNE, ExistingPeriodicWorkPolicy.KEEP, request)
+        workManager.enqueueUniquePeriodicWork(PERIODIC_PRUNE, PERIODIC_POLICY, request)
     }
 
     /** Runs pruning promptly, e.g. right after the user tightens a retention limit. */
@@ -95,5 +99,13 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context:
         const val ONESHOT_BACKFILL = "libremail_oneshot_backfill"
         const val PERIODIC_PRUNE = "libremail_periodic_prune"
         const val ONESHOT_PRUNE = "libremail_oneshot_prune"
+
+        // UPDATE, not KEEP (issue #96). These periodic jobs are re-enqueued at every app start, so KEEP
+        // pinned an already-installed device to the interval/constraints from the version that first
+        // scheduled it — later tuning never reached upgraders. UPDATE re-applies the current spec while
+        // preserving the running period's progress: an unchanged spec is effectively a no-op, so this
+        // does NOT reset the schedule on launch the way REPLACE (cancel + re-enqueue) would. UPDATE is
+        // available since WorkManager 2.8.
+        val PERIODIC_POLICY = ExistingPeriodicWorkPolicy.UPDATE
     }
 }

app/src/main/kotlin/org/libremail/di/WorkManagerModule.kt

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.di
+
+import android.content.Context
+import androidx.work.WorkManager
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.android.qualifiers.ApplicationContext
+import dagger.hilt.components.SingletonComponent
+import javax.inject.Singleton
+
+/**
+ * Provides the process-wide [WorkManager] so schedulers can inject it (and unit tests can substitute
+ * a fake) instead of reaching for the [WorkManager.getInstance] static directly.
+ */
+@Module
+@InstallIn(SingletonComponent::class)
+object WorkManagerModule {
+
+    @Provides
+    @Singleton
+    fun provideWorkManager(@ApplicationContext context: Context): WorkManager = WorkManager.getInstance(context)
+}

app/src/main/kotlin/org/libremail/domain/model/Folder.kt

@@ -33,6 +33,25 @@ enum class FolderRole {
     ;
 
     companion object {
+        /**
+         * Single source of truth mapping a lowercase IMAP SPECIAL-USE attribute to the role it implies:
+         * RFC 6154's six attributes, plus Gmail's `\All` and RFC 8457's `\Important`. A `null` role
+         * marks an attribute that flags a folder as server-provisioned (not user-created) without
+         * implying one of the friendly [FolderRole]s. [roleOf] returns the first role-bearing entry the
+         * folder advertises; [isServerSpecial] treats every key as special-use. Insertion order sets
+         * [roleOf]'s precedence when a folder advertises more than one role-bearing attribute.
+         */
+        private val ATTRIBUTE_ROLES: Map<String, FolderRole?> = linkedMapOf(
+            "\\sent" to SENT,
+            "\\drafts" to DRAFTS,
+            "\\junk" to SPAM,
+            "\\trash" to TRASH,
+            "\\archive" to ARCHIVE,
+            "\\all" to null,
+            "\\flagged" to null,
+            "\\important" to null,
+        )
+
         /**
          * Classifies a folder from its name and IMAP SPECIAL-USE attributes (RFC 6154). Prefers the
          * server-advertised attribute; falls back to a case-insensitive name match because many
@@ -41,27 +60,14 @@ enum class FolderRole {
         fun roleOf(fullName: String, displayName: String, attributes: List<String>): FolderRole {
             if (fullName.equals("INBOX", ignoreCase = true)) return INBOX
             val attrs = attributes.map { it.lowercase() }
-            val byAttribute = when {
-                "\\sent" in attrs -> SENT
-                "\\drafts" in attrs -> DRAFTS
-                "\\junk" in attrs -> SPAM
-                "\\trash" in attrs -> TRASH
-                "\\archive" in attrs -> ARCHIVE
-                else -> null
+            val byAttribute = ATTRIBUTE_ROLES.entries.firstNotNullOfOrNull { (attribute, role) ->
+                role?.takeIf { attribute in attrs }
             }
             return byAttribute ?: roleFromDisplayName(displayName)
         }
 
-        /**
-         * The RFC 6154 SPECIAL-USE attributes (plus Gmail's `\All`) that mark a folder as one the
-         * server provisions itself, as opposed to a user-created folder.
-         */
-        private val SPECIAL_USE_ATTRIBUTES =
-            setOf("\\all", "\\archive", "\\drafts", "\\flagged", "\\junk", "\\sent", "\\trash")
-
         /** True when the server advertises any SPECIAL-USE attribute for the folder (RFC 6154). */
-        fun isServerSpecial(attributes: List<String>): Boolean =
-            attributes.any { it.lowercase() in SPECIAL_USE_ATTRIBUTES }
+        fun isServerSpecial(attributes: List<String>): Boolean = attributes.any { it.lowercase() in ATTRIBUTE_ROLES }
 
         /** Best-effort role from a folder's display name, for servers without SPECIAL-USE flags. */
         private fun roleFromDisplayName(displayName: String): FolderRole = when (displayName.lowercase().trim()) {

app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt

@@ -29,6 +29,12 @@ enum class MailProvider(
     val displayName: String,
     /** The page where the user creates an app password for this provider. */
     val appPasswordHelpUrl: String,
+    /**
+     * Setup instructions for the provider's two-factor prerequisite, or null when there isn't one.
+     * Only Gmail refuses to create app passwords until 2-Step Verification is on, so only Gmail
+     * links its setup article; Yahoo and iCloud gate nothing on it.
+     */
+    val twoFactorHelpUrl: String? = null,
     private val imapHost: String,
     private val smtpHost: String,
     private val smtpPort: Int,
@@ -38,6 +44,9 @@ enum class MailProvider(
         key = "gmail",
         displayName = "Gmail",
         appPasswordHelpUrl = "https://myaccount.google.com/apppasswords",
+        // Google's "Turn on 2-Step Verification" article — the app-passwords page above bounces
+        // accounts that haven't enabled it yet, so the setup screen offers this as a way out.
+        twoFactorHelpUrl = "https://support.google.com/accounts/answer/185839",
         imapHost = "imap.gmail.com",
         smtpHost = "smtp.gmail.com",
         // Google documents smtp.gmail.com:587 with STARTTLS as the standard submission endpoint.

app/src/main/kotlin/org/libremail/push/IdleService.kt

@@ -134,8 +134,9 @@ class IdleService : Service() {
         startAsForeground(mode)
         if (mode == PushMode.POLLING) {
             AppLog.i(TAG, "Battery low: pausing IMAP IDLE; mail arrives via 15-minute periodic sync")
-            // The periodic fallback is scheduled at every app start with KEEP, so this is normally a
-            // no-op — re-asserted here so the fallback provably exists whenever push is paused.
+            // The periodic fallback is already scheduled at every app start (UPDATE), so re-asserting it
+            // here is effectively a no-op — done anyway so the fallback provably exists whenever push is
+            // paused, without disturbing the running period.
             syncScheduler.schedulePeriodicSync()
         } else {
             AppLog.i(TAG, "Battery recovered: resuming IMAP IDLE push")

app/src/main/kotlin/org/libremail/ui/accountsetup/AppPasswordSetupScreen.kt

@@ -78,6 +78,12 @@ fun AppPasswordSetupScreen(
     val scope = rememberCoroutineScope()
     // Resolved up front so the failure handler (a non-composable lambda) can use it.
     val openFailedMessage = stringResource(R.string.app_password_open_failed)
+    // Shared by every outbound link on this screen: openUri throws if no browser/handler is
+    // installed, so surface that as an inline snackbar instead of crashing.
+    val openUrl: (String) -> Unit = { url ->
+        runCatching { uriHandler.openUri(url) }
+            .onFailure { scope.launch { snackbarHostState.showSnackbar(openFailedMessage) } }
+    }
 
     LaunchedEffect(form.status, form.addedAccountId) {
         if (form.status == SetupStatus.DONE) {
@@ -146,15 +152,23 @@ fun AppPasswordSetupScreen(
 
             Spacer(Modifier.height(12.dp))
             OutlinedButton(
-                onClick = {
-                    // openUri throws if no browser/handler is installed; surface it instead of crashing.
-                    runCatching { uriHandler.openUri(provider.appPasswordHelpUrl) }
-                        .onFailure { scope.launch { snackbarHostState.showSnackbar(openFailedMessage) } }
-                },
+                onClick = { openUrl(provider.appPasswordHelpUrl) },
                 modifier = Modifier.fillMaxWidth(),
             ) {
                 Text(stringResource(R.string.app_password_open_page, provider.displayName))
             }
+            // Only Gmail has a two-factor prerequisite (see MailProvider.twoFactorHelpUrl): its
+            // app-passwords page rejects accounts without 2-Step Verification, so give those users
+            // a way to set it up instead of a dead end.
+            provider.twoFactorHelpUrl?.let { twoFactorHelpUrl ->
+                Spacer(Modifier.height(8.dp))
+                OutlinedButton(
+                    onClick = { openUrl(twoFactorHelpUrl) },
+                    modifier = Modifier.fillMaxWidth(),
+                ) {
+                    Text(stringResource(R.string.app_password_2fa_help))
+                }
+            }
 
             Spacer(Modifier.height(20.dp))
             OutlinedTextField(

app/src/main/res/values/strings.xml

@@ -159,6 +159,7 @@
     <string name="app_password_what_is">An app password is a one-off password that lets an app sign in to your account without your main password or a two-factor code.</string>
     <string name="app_password_warning">Store this app password carefully — it grants full access to your email. LibreMail keeps it only on this device.</string>
     <string name="app_password_open_page">Create an app password for %1$s</string>
+    <string name="app_password_2fa_help">How to turn on 2-Step Verification</string>
     <string name="app_password_open_failed">Couldn\'t open your browser</string>
     <string name="app_password_email">Email address</string>
     <string name="app_password_field">App password</string>

app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt

@@ -42,11 +42,12 @@ class FolderMapperTest {
     }
 
     @Test
-    fun `the all-mail and starred attributes mark the folder special but drive no role of their own`() {
-        // Gmail's "All Mail" (\All) and "Starred" (\Flagged) are server special-use, yet roleOf maps
-        // neither to a role (the role still comes from the name — here neutral, so NORMAL). specialUse
-        // and role are independent axes; this documents the current wiring ahead of the #65 refactor.
-        listOf("\\All", "\\Flagged").forEach { attribute ->
+    fun `the all-mail, starred and important attributes mark the folder special but drive no role`() {
+        // Gmail's "All Mail" (\All), "Starred" (\Flagged) and "Important" (\Important, RFC 8457 — #62)
+        // are server special-use, yet roleOf maps none to a role (the role still comes from the name —
+        // here neutral, so NORMAL). specialUse and role are independent axes; the #65 unified table
+        // keeps them so.
+        listOf("\\All", "\\Flagged", "\\Important").forEach { attribute ->
             val entity = entityFor(listOf(attribute))
             assertTrue(entity.specialUse, "specialUse for $attribute")
             assertEquals(FolderRole.NORMAL.name, entity.role, "role for $attribute")

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

@@ -34,6 +34,7 @@ import org.libremail.domain.model.AccountSettings
 import org.libremail.domain.model.ImapConnectionParams
 import org.libremail.domain.model.MailSecurity
 import org.libremail.domain.repository.MailRepository
+import org.libremail.mail.FetchedMessage
 import org.libremail.mail.ImapClient
 import org.libremail.power.BatteryStatus
 import org.libremail.power.BatteryStatusProvider
@@ -41,6 +42,7 @@ import java.util.Date
 import java.util.Properties
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNotEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
 
@@ -221,11 +223,131 @@ class MailBackfillerTest {
         coVerify(atLeast = 1) { requireNotNull(lastMailRepository).prefetchMessage(any()) }
     }
 
-    /** Builds a backfiller wired to GreenMail with the in-memory fakes and the given account settings. */
+    /**
+     * Regression for #94: backfill pages by UID (arrival order) while the age floor cuts by the Date
+     * header. A single message with a HIGH UID but an OLD Date (mail moved/imported into the folder)
+     * used to drag the oldest cached timestamp below the cutoff and stop paging after zero pages,
+     * silently gapping every older-UID message whose Date is still within retention. The floor must
+     * instead be decided from the pages actually fetched.
+     */
+    @Test
+    fun `a high-UID old-Date message must not stop the age floor while history is unfetched`() = runTest {
+        val now = System.currentTimeMillis()
+        // UIDs 1..20: genuinely old (~8 months). UIDs 21..99: within retention, Dates ascending with
+        // UID. UID 100: a recent arrival whose Date header is 8 months old — the Date/UID inversion —
+        // which lands inside the seeded foreground window.
+        val old = (1..20).map { now - 8 * MONTH_MILLIS - it * DAY_MILLIS }
+        val recent = (1..79).map { now - (80 - it) * DAY_MILLIS }
+        val inverted = listOf(now - 8 * MONTH_MILLIS)
+        appendMessages(old + recent + inverted)
+        seedForegroundWindow()
+
+        val backfiller = backfiller(AccountSettings("acct", retentionMonths = 6))
+        var guard = 0
+        while (backfiller.runBackfill() && guard++ < 10) { /* drive to completion */ }
+
+        val cutoff = now - 6 * MONTH_MILLIS
+        assertEquals(
+            recent.size,
+            cached.count { it.timestampMillis >= cutoff },
+            "every within-retention message must be cached — no silent history gap",
+        )
+        assertEquals(true, progress["acct" to "INBOX"]?.complete, "paging still terminates at the floor")
+        assertNoDeletes()
+    }
+
+    /**
+     * The flip side of the page-based age floor (#94): a page ENTIRELY older than the cutoff ends
+     * paging — the folder is marked complete without caching that page, which is pure prune-fodder
+     * (persisting it would just make the pruner delete it again, the #12/#13 churn the sticky floor
+     * exists to prevent).
+     */
+    @Test
+    fun `an entirely-old page ends age paging without caching beyond the floor`() = runTest {
+        val now = System.currentTimeMillis()
+        // UIDs 1..50 are ~8 months old; UIDs 51..100 are within retention and fill the whole window.
+        val old = (1..50).map { now - 8 * MONTH_MILLIS - (51 - it) * DAY_MILLIS }
+        val recent = (1..50).map { now - (51 - it) * DAY_MILLIS }
+        appendMessages(old + recent)
+        seedForegroundWindow()
+
+        backfiller(AccountSettings("acct", retentionMonths = 6)).runBackfill()
+
+        assertEquals(true, progress["acct" to "INBOX"]?.complete, "one entirely-old page ends the folder")
+        assertEquals(0, totalOffered, "the beyond-the-floor page must not be cached")
+        val cutoff = now - 6 * MONTH_MILLIS
+        assertTrue(cached.none { it.timestampMillis < cutoff }, "nothing older than the cutoff is cached")
+        assertNoDeletes()
+    }
+
+    /**
+     * Regression for #95: a cached row with `uid = 0` — a row migrated before the `uid` column
+     * existed (MIGRATION_12_13 backfills a non-numeric id tail to 0) — used to collapse MIN(uid) to
+     * 0, and `fetchOlderThan` treats a bound `<= 1` as "nothing older": the folder was marked fully
+     * backfilled after caching NOTHING. The boundary must come from resolved (positive) UIDs only.
+     */
+    @Test
+    fun `a uid 0 placeholder row must not poison the backfill boundary`() = runTest {
+        appendMessages(TOTAL)
+        seedForegroundWindow()
+        // A pre-uid-column row, exactly as MIGRATION_12_13 leaves one whose id tail isn't numeric.
+        cached += cached.first().copy(id = "acct:INBOX:legacy", uid = 0L)
+
+        val backfiller = backfiller(AccountSettings("acct"))
+        var guard = 0
+        while (backfiller.runBackfill() && guard++ < 10) { /* drive to completion */ }
+
+        assertEquals(
+            TOTAL,
+            cached.mapTo(HashSet()) { it.uid }.count { it > 0L },
+            "the placeholder row must not stop backfill from paging the full history",
+        )
+        assertEquals(TOTAL - WINDOW, totalOffered, "each backfilled message fetched exactly once")
+        assertEquals(true, progress["acct" to "INBOX"]?.complete)
+        assertNoDeletes()
+    }
+
+    /**
+     * Regression for #95 (server variant): when every UID on a fetched page is unresolvable
+     * (UIDFolder.getUID returned -1), the boundary must not descend to -1 — the next fetch would come
+     * back empty and the folder would be FALSELY marked fully backfilled. The folder must instead
+     * stall: stay incomplete (retryable on a future run) without claiming immediate more-work (which
+     * would make the worker's slice-chaining loop spin on the same page).
+     */
+    @Test
+    fun `a page of unresolvable UIDs stalls the folder instead of falsely completing it`() = runTest {
+        // One cached window row makes INBOX a backfill target with boundary UID 60.
+        cached += fetchedMessage(uid = "60").toEntity("acct", "INBOX")
+
+        val imapClient = mockk<ImapClient>()
+        coEvery { imapClient.fetchOlderThan(any(), any(), any(), any()) } answers {
+            // The real client's contract: a collapsed boundary (<= 1) means "nothing older".
+            if (thirdArg<Long>() <= 1L) emptyList() else listOf(fetchedMessage(uid = "-1"))
+        }
+
+        val moreWork = backfiller(AccountSettings("acct"), imapClient = imapClient).runBackfill()
+
+        assertNotEquals(true, progress["acct" to "INBOX"]?.complete, "the folder must stay retryable")
+        assertFalse(moreWork, "a stalled folder must not spin the worker's slice-chaining loop")
+        coVerify(exactly = 0) { imapClient.fetchOlderThan(any(), any(), match { it <= 1L }, any()) }
+    }
+
+    private fun fetchedMessage(uid: String) = FetchedMessage(
+        uid = uid,
+        sender = "Sender",
+        senderEmail = "sender@example.org",
+        subject = "Message $uid",
+        timestampMillis = System.currentTimeMillis(),
+        isRead = false,
+        isFlagged = false,
+    )
+
+    /** Builds a backfiller wired to [imapClient] (GreenMail-backed by default) with the in-memory fakes. */
     private fun backfiller(
         accountSettings: AccountSettings,
         fetchPolicy: FetchPolicy = FetchPolicy.ON_DEMAND,
         battery: BatteryStatus = BatteryStatus(percent = 100, isCharging = false),
+        imapClient: ImapClient = client,
     ): MailBackfiller {
         val accountDao = mockk<AccountDao>()
         coEvery { accountDao.getAll() } returns listOf(accountEntity)
@@ -241,16 +363,13 @@ class MailBackfillerTest {
         }
         coEvery { messageDao.lowestSyncedUid("acct", any()) } answers {
             val folder = secondArg<String>()
-            cached.filter { it.inInbox && it.folder == folder }.minOfOrNull { it.uid }
+            // Mirrors the real query's `uid > 0` guard (#95): placeholder rows never drive the boundary.
+            cached.filter { it.inInbox && it.folder == folder && it.uid > 0L }.minOfOrNull { it.uid }
         }
         coEvery { messageDao.countSynced("acct", any()) } answers {
             val folder = secondArg<String>()
             cached.count { it.inInbox && it.folder == folder }
         }
-        coEvery { messageDao.oldestSyncedTimestamp("acct", any()) } answers {
-            val folder = secondArg<String>()
-            cached.filter { it.inInbox && it.folder == folder }.minOfOrNull { it.timestampMillis }
-        }
 
         val backfillProgressDao = mockk<BackfillProgressDao>(relaxed = true)
         coEvery { backfillProgressDao.get("acct", any()) } answers { progress["acct" to secondArg<String>()] }
@@ -277,7 +396,7 @@ class MailBackfillerTest {
             accountDao = accountDao,
             messageDao = messageDao,
             backfillProgressDao = backfillProgressDao,
-            imapClient = client,
+            imapClient = imapClient,
             connectionFactory = connectionFactory,
             settingsRepository = settingsRepository,
             accountSettingsRepository = accountSettingsRepository,

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

@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.data.sync
+
+import androidx.work.ExistingPeriodicWorkPolicy
+import androidx.work.ExistingWorkPolicy
+import androidx.work.OneTimeWorkRequest
+import androidx.work.PeriodicWorkRequest
+import androidx.work.WorkManager
+import io.mockk.mockk
+import io.mockk.verify
+import org.junit.Test
+import javax.inject.Provider
+
+/**
+ * The enqueue methods are thin wrappers over WorkManager, so these tests pin the one thing that carries
+ * behaviour: the existing-work policy each unique job is enqueued with.
+ */
+class SyncSchedulerTest {
+
+    private val workManager = mockk<WorkManager>(relaxed = true)
+    private val scheduler = SyncScheduler(Provider { workManager })
+
+    // Issue #96: periodic jobs must re-enqueue with UPDATE (not KEEP). At every app start KEEP would pin
+    // an already-installed device to the interval/constraints of whichever version first scheduled the
+    // job, so cadence/constraint tuning never reached upgraders; UPDATE re-applies the current spec.
+
+    @Test
+    fun `periodic sync is enqueued with UPDATE so upgrades re-apply its schedule`() {
+        scheduler.schedulePeriodicSync()
+
+        verify {
+            workManager.enqueueUniquePeriodicWork(
+                "libremail_periodic_sync",
+                ExistingPeriodicWorkPolicy.UPDATE,
+                any<PeriodicWorkRequest>(),
+            )
+        }
+    }
+
+    @Test
+    fun `periodic backfill is enqueued with UPDATE so upgrades re-apply its schedule`() {
+        scheduler.schedulePeriodicBackfill()
+
+        verify {
+            workManager.enqueueUniquePeriodicWork(
+                "libremail_periodic_backfill",
+                ExistingPeriodicWorkPolicy.UPDATE,
+                any<PeriodicWorkRequest>(),
+            )
+        }
+    }
+
+    @Test
+    fun `periodic prune is enqueued with UPDATE so upgrades re-apply its schedule`() {
+        scheduler.schedulePeriodicPrune()
+
+        verify {
+            workManager.enqueueUniquePeriodicWork(
+                "libremail_periodic_prune",
+                ExistingPeriodicWorkPolicy.UPDATE,
+                any<PeriodicWorkRequest>(),
+            )
+        }
+    }
+
+    // The one-shot kicks are a separate concern from #96 and keep their existing policies: syncNow and
+    // pruneNow REPLACE for a clean immediate attempt; backfillNow KEEPs an in-flight all-account run.
+
+    @Test
+    fun `syncNow replaces any pending one-shot sync`() {
+        scheduler.syncNow()
+
+        verify {
+            workManager.enqueueUniqueWork(
+                "libremail_oneshot_sync",
+                ExistingWorkPolicy.REPLACE,
+                any<OneTimeWorkRequest>(),
+            )
+        }
+    }
+
+    @Test
+    fun `backfillNow keeps an already-running backfill`() {
+        scheduler.backfillNow()
+
+        verify {
+            workManager.enqueueUniqueWork(
+                "libremail_oneshot_backfill",
+                ExistingWorkPolicy.KEEP,
+                any<OneTimeWorkRequest>(),
+            )
+        }
+    }
+
+    @Test
+    fun `pruneNow replaces any pending one-shot prune`() {
+        scheduler.pruneNow()
+
+        verify {
+            workManager.enqueueUniqueWork(
+                "libremail_oneshot_prune",
+                ExistingWorkPolicy.REPLACE,
+                any<OneTimeWorkRequest>(),
+            )
+        }
+    }
+}

app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt

@@ -37,9 +37,51 @@ class FolderRoleTest {
     fun `isServerSpecial is true only for special-use attributes`() {
         assertTrue(FolderRole.isServerSpecial(listOf("\\Junk")))
         assertTrue(FolderRole.isServerSpecial(listOf("\\All")))
+        // Issue #62: \Important (RFC 8457) is what Gmail advertises on [Gmail]/Important.
+        assertTrue(FolderRole.isServerSpecial(listOf("\\Important")))
         // Case-insensitive, and ignores non-special-use flags mixed in.
         assertTrue(FolderRole.isServerSpecial(listOf("\\HasNoChildren", "\\drafts")))
         assertFalse(FolderRole.isServerSpecial(emptyList()))
         assertFalse(FolderRole.isServerSpecial(listOf("\\HasNoChildren")))
     }
+
+    // Guards the #65 unified attribute-to-role table: roleOf and isServerSpecial read from one map.
+    // Role-bearing attributes drive a role AND mark the folder special; \All, \Flagged and \Important
+    // are server-special but role-less (their role comes from the display name — here "Neutral", so
+    // NORMAL). Pins that the refactor preserved every existing mapping and added \Important (#62).
+    @Test
+    fun `every special-use attribute maps to its role and is server-special`() {
+        val expectedRole = mapOf(
+            "\\Sent" to FolderRole.SENT,
+            "\\Drafts" to FolderRole.DRAFTS,
+            "\\Junk" to FolderRole.SPAM,
+            "\\Trash" to FolderRole.TRASH,
+            "\\Archive" to FolderRole.ARCHIVE,
+            "\\All" to FolderRole.NORMAL,
+            "\\Flagged" to FolderRole.NORMAL,
+            "\\Important" to FolderRole.NORMAL,
+        )
+        expectedRole.forEach { (attribute, role) ->
+            assertEquals(role, FolderRole.roleOf("X", "Neutral", listOf(attribute)), "role for $attribute")
+            assertTrue(FolderRole.isServerSpecial(listOf(attribute)), "special-use for $attribute")
+        }
+    }
+
+    // A folder advertising two role-bearing attributes resolves to the earlier table entry (Sent
+    // precedes Archive), whatever order the server listed them — preserving the old when-ladder's
+    // precedence now that it is table-driven.
+    @Test
+    fun `role precedence follows the table order, not the folder's attribute order`() {
+        assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Whatever", listOf("\\Archive", "\\Sent")))
+        assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Whatever", listOf("\\Sent", "\\Archive")))
+    }
+
+    // A role-less special-use attribute must not suppress the display-name fallback: \All on a folder
+    // named "Sent" still classifies as SENT, and Gmail's [Gmail]/Important (\Important, RFC 8457) is
+    // special-use yet keeps NORMAL because "Important" is not a friendly role.
+    @Test
+    fun `a role-less special-use attribute still allows the name fallback`() {
+        assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Sent", listOf("\\All")))
+        assertEquals(FolderRole.NORMAL, FolderRole.roleOf("[Gmail]/Important", "Important", listOf("\\Important")))
+    }
 }

app/src/test/kotlin/org/libremail/domain/model/MailProviderTest.kt

@@ -80,6 +80,21 @@ class MailProviderTest {
         }
     }
 
+    @Test
+    fun `only gmail links two-factor setup help, over https`() {
+        // Gmail's app-passwords page rejects accounts without 2-Step Verification, so its setup
+        // screen must offer the setup article as a way out (issue #98).
+        val gmailUrl = assertNotNull(
+            MailProvider.GMAIL.twoFactorHelpUrl,
+            "gmail must expose a 2-Step Verification help URL",
+        )
+        assertTrue(gmailUrl.startsWith("https://"), "gmail 2FA help URL must be https")
+
+        // Yahoo and iCloud gate nothing on two-factor, so they must not grow the extra link.
+        assertNull(MailProvider.YAHOO.twoFactorHelpUrl)
+        assertNull(MailProvider.ICLOUD.twoFactorHelpUrl)
+    }
+
     @Test
     fun `createAccount trims the email and derives a stable id and display name`() {
         val account = MailProvider.GMAIL.createAccount("  User@Gmail.com  ")