Merge main into perf-187-covering-index

Resolve DatabaseModule conflict from #320: main replaced the explicit .addMigrations(...) chain with .addMigrations(*ALL_MIGRATIONS) plus an introspectable ALL_MIGRATIONS list guarded by databaseModuleRegistersEveryDeclaredMigration (registered == declared). Add MIGRATION_19_20 to ALL_MIGRATIONS so the unified-inbox covering-index migration (cache schema v19->v20) is both registered on the Room builder and satisfies that safety-net test. Schema 20.json, the v20 @Database version, and DatabaseEncryptionTest's schema-version assertion (20) are unchanged.

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

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

@@ -13,6 +13,7 @@ import kotlinx.coroutines.runBlocking
 import org.json.JSONObject
 import org.junit.After
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
@@ -21,6 +22,8 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.libremail.data.local.entity.CredentialEntity
+import org.libremail.reporting.AppLog
+import org.libremail.reporting.RingLogBuffer
 
 /**
  * The one-time move performed by [AccountDataMigrator] (issue #111): copying accounts / credentials /
@@ -142,6 +145,26 @@ class AccountDataMigratorTest {
         }
     }
 
+    @Test
+    fun copyEmitsANonPiiAppLogBreadcrumbNamingOnlyTheMovedTables() = runBlocking<Unit> {
+        seedVersion14Cache()
+        val buffer = RingLogBuffer()
+        AppLog.install(buffer)
+
+        AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile)
+
+        val entry = buffer.snapshot()
+            .single { it.message.startsWith("moved account tables into the account database") }
+        assertEquals("the migration breadcrumb is a debug line", 'D', entry.level)
+        listOf("accounts", "credentials", "account_settings", "signatures").forEach { table ->
+            assertTrue("breadcrumb must name the moved table $table", entry.message.contains(table))
+        }
+        // The breadcrumb carries only table names — never the seeded email, secret, or passphrase.
+        assertFalse(entry.message.contains("ada@example.org"))
+        assertFalse(entry.message.contains("sealed-secret"))
+        assertFalse(entry.message.contains(passphrase))
+    }
+
     @Test
     fun reRunningTheCopyIsIdempotentAndKeepsLaterEdits() = runBlocking<Unit> {
         seedVersion14Cache()

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

@@ -17,6 +17,8 @@ import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.libremail.data.local.entity.MessageEntity
+import org.libremail.reporting.AppLog
+import org.libremail.reporting.RingLogBuffer
 import java.io.File
 
 /**
@@ -150,6 +152,42 @@ class DatabaseEncryptionTest {
         assertEquals("Room's schema version must survive the plaintext -> encrypted conversion", 20, version)
     }
 
+    @Test
+    fun conversionEmitsNonPiiAppLogBreadcrumbs() = runBlocking<Unit> {
+        val buffer = RingLogBuffer()
+        AppLog.install(buffer)
+
+        // The seeded row carries an email address so the PII assertions below are meaningful.
+        openPlaintext().apply {
+            messageDao().insertNew(listOf(message("acct:1")))
+            close()
+        }
+
+        DatabaseEncryption.ensureEncrypted(dbFile, passphrase)
+        val afterEncrypt = buffer.snapshot()
+        val converting = afterEncrypt.single { it.message.startsWith("converting local cache database") }
+        assertEquals("the start breadcrumb is informational", 'I', converting.level)
+        assertEquals("converting local cache database (targetEncrypted=true)", converting.message)
+        val convertedAfterEncrypt = afterEncrypt.single { it.message == "local cache database converted" }
+        assertEquals('D', convertedAfterEncrypt.level)
+
+        // Converting back to plaintext logs the same pair with the flag flipped.
+        buffer.clear()
+        DatabaseEncryption.ensurePlaintext(dbFile, passphrase)
+        val afterDecrypt = buffer.snapshot()
+        assertTrue(
+            afterDecrypt.any { it.message == "converting local cache database (targetEncrypted=false)" },
+        )
+        assertTrue(afterDecrypt.any { it.message == "local cache database converted" })
+
+        // Neither conversion's breadcrumbs may leak the passphrase, the on-disk path, or account PII.
+        (afterEncrypt + afterDecrypt).forEach { entry ->
+            assertFalse("must not leak the passphrase", entry.message.contains(passphrase))
+            assertFalse("must not leak the db file path", entry.message.contains(dbFile.absolutePath))
+            assertFalse("must not leak the seeded email", entry.message.contains("ada@example.org"))
+        }
+    }
+
     private fun openPlaintext(): LibreMailDatabase =
         Room.databaseBuilder(context, LibreMailDatabase::class.java, dbName).build()
 

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

@@ -166,6 +166,41 @@ class MessageDaoTest {
         )
     }
 
+    @Test
+    fun browsePagingBreaksTimestampTiesByAscendingIdForATotalPageOrder() = runBlocking {
+        // Bulk mail can share a timestamp (issue #311): without a unique tiebreaker, rows tied at a
+        // LIMIT/OFFSET page boundary can duplicate or skip. Insertion order is scrambled so the ORDER BY
+        // — not the storage order — must produce the result.
+        dao.insertNew(
+            listOf(
+                message("tie-c", timestampMillis = 1_000),
+                message("tie-a", timestampMillis = 1_000),
+                message("tie-b", timestampMillis = 1_000),
+                message("newer", timestampMillis = 2_000),
+            ),
+        )
+
+        // Newest timestamp first, then ties broken by ascending id — a deterministic total order.
+        val expected = listOf("newer", "tie-a", "tie-b", "tie-c")
+        assertEquals(expected, dao.pagingUnifiedFolderSummaries("INBOX").refreshIds())
+        assertEquals(expected, dao.pagingFolderSummaries("acct", "INBOX").refreshIds())
+    }
+
+    @Test
+    fun searchPagingBreaksTimestampTiesByAscendingIdForATotalPageOrder() = runBlocking {
+        dao.insertNew(
+            listOf(
+                message("hit-c", subject = "report", timestampMillis = 1_000),
+                message("hit-a", subject = "report", timestampMillis = 1_000),
+                message("hit-b", subject = "report", timestampMillis = 1_000),
+            ),
+        )
+
+        val expected = listOf("hit-a", "hit-b", "hit-c")
+        assertEquals(expected, dao.pagingUnifiedFolderSearchSummaries("INBOX", "%report%").refreshIds())
+        assertEquals(expected, dao.pagingFolderSearchSummaries("acct", "INBOX", "%report%").refreshIds())
+    }
+
     @Test
     fun getUnfetchedIdsReturnsOnlySyncedRowsMissingABody() = runBlocking {
         dao.insertNew(
@@ -232,6 +267,59 @@ class MessageDaoTest {
         assertEquals(true, row.inInbox)
     }
 
+    @Test
+    fun updateHeaderContentsRefreshesEveryRowInTheBatchAndLeavesFlagsAndBodiesUntouched() = runBlocking {
+        dao.insertNew(
+            listOf(
+                message("acct:1", isRead = true, isStarred = false, body = "cached-1", uid = 0),
+                message("acct:2", isRead = false, isStarred = true, body = "cached-2", uid = 0),
+            ),
+        )
+
+        // The sync path (issue #310) refreshes a whole recent window at once via this single-transaction
+        // batch. Only the six header fields of each passed entity are applied; the rest are ignored.
+        dao.updateHeaderContents(
+            listOf(
+                message(
+                    "acct:1",
+                    sender = "Charles",
+                    senderEmail = "charles@example.org",
+                    subject = "One",
+                    timestampMillis = 5_000L,
+                    uid = 42L,
+                ),
+                message(
+                    "acct:2",
+                    sender = "Grace",
+                    senderEmail = "grace@example.org",
+                    subject = "Two",
+                    timestampMillis = 6_000L,
+                    uid = 43L,
+                ),
+            ),
+        )
+
+        val one = requireNotNull(dao.getById("acct:1"))
+        assertEquals("Charles", one.sender)
+        assertEquals("charles@example.org", one.senderEmail)
+        assertEquals("One", one.subject)
+        assertEquals(5_000L, one.timestampMillis)
+        assertEquals(42L, one.uid)
+        // The casefold search columns track the refreshed headers (issue #232).
+        assertEquals("charles", one.senderFold)
+        assertEquals("one", one.subjectFold)
+        // Flags and the cached body are deliberately left untouched.
+        assertTrue(one.isRead)
+        assertEquals("cached-1", one.body)
+
+        val two = requireNotNull(dao.getById("acct:2"))
+        assertEquals("Grace", two.sender)
+        assertEquals("Two", two.subject)
+        assertEquals(43L, two.uid)
+        assertTrue(two.isStarred)
+        assertEquals("cached-2", two.body)
+    }
+
     @Test
     fun markSyncedPromotesSearchOnlyRowsIntoTheFolder() = runBlocking {
         dao.insertNew(

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

@@ -13,6 +13,7 @@ import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.libremail.di.DatabaseModule
 import java.lang.reflect.Modifier
 
 /**
@@ -356,6 +357,23 @@ class MigrationTest {
         )
     }
 
+    /**
+     * The replay tests discover migrations reflectively, but nothing there checks that [DatabaseModule]
+     * actually *registers* them. With no destructive fallback, a migration authored, schema-committed,
+     * and replay-tested but omitted from `addMigrations` still crash-loops every upgrading user at first
+     * database open (issue #312). Assert the builder's registered set ([DatabaseModule.ALL_MIGRATIONS])
+     * is exactly the reflectively-discovered set, so such an omission fails here instead.
+     */
+    @Test
+    fun databaseModuleRegistersEveryDeclaredMigration() {
+        assertEquals(
+            "DatabaseModule.ALL_MIGRATIONS must register every Migration in Migrations.kt " +
+                "(no destructive fallback, so a forgotten step crash-loops upgrades)",
+            allAppMigrations.map { it.startVersion to it.endVersion },
+            DatabaseModule.ALL_MIGRATIONS.sortedBy { it.startVersion }.map { it.startVersion to it.endVersion },
+        )
+    }
+
     /**
      * Creates a database at v7 (the oldest exported schema), fills it like a used install, then
      * replays every migration one step at a time — `runMigrationsAndValidate` diffs the migrated

app/src/androidTest/kotlin/org/libremail/ui/reader/ReaderScreenTest.kt

@@ -11,6 +11,7 @@ import androidx.compose.ui.test.performClick
 import androidx.lifecycle.SavedStateHandle
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import org.junit.Assert.assertEquals
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -18,6 +19,7 @@ import org.libremail.R
 import org.libremail.data.settings.SettingsRepository
 import org.libremail.domain.model.Attachment
 import org.libremail.domain.model.Message
+import org.libremail.domain.model.ReplyMode
 import org.libremail.ui.FakeMailRepository
 import org.libremail.ui.navigation.Routes
 import org.libremail.ui.theme.LibreMailTheme
@@ -44,8 +46,17 @@ class ReaderScreenTest {
     private fun attachment(partIndex: Int, filename: String) =
         Attachment(messageId, partIndex, filename, "application/pdf", 1_000L)
 
-    /** Renders [ReaderScreen] for the fixed [message] with the given [attachments] and awaits load. */
-    private fun renderReader(attachments: List<Attachment>, downloadedParts: Set<Int> = emptySet()) {
+    /** The draft id the reader last asked to open compose on (via OpenCompose), or null. */
+    private var openedDraftId: String? = null
+
+    /**
+     * Renders [ReaderScreen] for the fixed [message] with the given [attachments], awaits load, and
+     * returns the backing [FakeMailRepository] so a test can assert which reply drafts were built.
+     */
+    private fun renderReader(
+        attachments: List<Attachment> = emptyList(),
+        downloadedParts: Set<Int> = emptySet(),
+    ): FakeMailRepository {
         val context = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext
         val repo = FakeMailRepository(
             messages = listOf(message),
@@ -59,12 +70,14 @@ class ReaderScreenTest {
         )
         composeTestRule.setContent {
             LibreMailTheme(darkTheme = false, dynamicColor = false) {
-                ReaderScreen(onBack = {}, onReply = { _, _, _ -> }, viewModel = viewModel)
+                ReaderScreen(onBack = {}, onOpenCompose = { openedDraftId = it }, viewModel = viewModel)
             }
         }
+        // The Reply action appears once the message loads, regardless of whether it has attachments.
         composeTestRule.waitUntil(5_000) {
-            composeTestRule.onAllNodesWithText(attachments.first().filename).fetchSemanticsNodes().isNotEmpty()
+            composeTestRule.onAllNodesWithText(string(R.string.reader_reply)).fetchSemanticsNodes().isNotEmpty()
         }
+        return repo
     }
 
     @Test
@@ -111,4 +124,39 @@ class ReaderScreenTest {
         composeTestRule.onNodeWithText(seeMore(1)).assertIsDisplayed()
         composeTestRule.onNodeWithText("b.pdf").assertDoesNotExist()
     }
+
+    @Test
+    fun reader_reply_buildsQuotedReplyDraftAndOpensCompose() {
+        val repo = renderReader()
+
+        composeTestRule.onNodeWithText(string(R.string.reader_reply)).performClick()
+        composeTestRule.waitUntil(5_000) { openedDraftId != null }
+
+        // The reader routes through buildReplyDraft (quotes the original + bakes the signature),
+        // not a bare compose prefill (#303), and opens compose on the resulting draft.
+        assertEquals(listOf(messageId to ReplyMode.REPLY), repo.replyDrafts)
+        assertEquals("draft-$messageId", openedDraftId)
+    }
+
+    @Test
+    fun reader_forward_viaOverflow_buildsForwardDraft() {
+        val repo = renderReader()
+
+        composeTestRule.onNodeWithContentDescription(string(R.string.action_more)).performClick()
+        composeTestRule.onNodeWithText(string(R.string.action_forward)).performClick()
+        composeTestRule.waitUntil(5_000) { openedDraftId != null }
+
+        assertEquals(listOf(messageId to ReplyMode.FORWARD), repo.replyDrafts)
+    }
+
+    @Test
+    fun reader_replyAll_viaOverflow_buildsReplyAllDraft() {
+        val repo = renderReader()
+
+        composeTestRule.onNodeWithContentDescription(string(R.string.action_more)).performClick()
+        composeTestRule.onNodeWithText(string(R.string.action_reply_all)).performClick()
+        composeTestRule.waitUntil(5_000) { openedDraftId != null }
+
+        assertEquals(listOf(messageId to ReplyMode.REPLY_ALL), repo.replyDrafts)
+    }
 }

app/src/androidTest/kotlin/org/libremail/ui/reporting/StartupCrashPromptTest.kt

@@ -11,6 +11,8 @@ import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performClick
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import org.junit.After
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
@@ -57,7 +59,9 @@ class StartupCrashPromptTest {
 
     private fun string(resId: Int) = composeTestRule.activity.getString(resId)
 
-    private fun store() = ReportStore(dir)
+    // Unconfined scope runs ReportStore's initial scan inline so a reopened store (simulating a
+    // relaunch) is readable synchronously, as before the scan moved off-thread (#296).
+    private fun store() = ReportStore(dir, CoroutineScope(Dispatchers.Unconfined))
 
     private fun crash(id: String, createdAt: Long) = DebugReport(
         id = id,