Merge main into feat-71-strikethrough

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

@@ -9,8 +9,13 @@ import androidx.paging.PagingData
 import androidx.paging.map
 import dagger.hilt.android.qualifiers.ApplicationContext
 import jakarta.mail.Flags
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.launch
 import org.libremail.data.ReplyBuilder
 import org.libremail.data.SignatureBlock
 import org.libremail.data.Snippet
@@ -65,6 +70,12 @@ class MailRepositoryImpl @Inject constructor(
     private val signatureRepository: SignatureRepository,
 ) : MailRepository {
 
+    // Application-lifetime scope for fire-and-forget server pushes that must outlive the caller — e.g.
+    // openMessage() returning to the reader screen before the SEEN flag reaches the server (#148). Same
+    // pattern as LibreMailApplication.appScope / IdleService.scope: this class is @Singleton (bound to
+    // Hilt's SingletonComponent), so the scope's lifetime is the process's, not any one caller's coroutine.
+    private val backgroundScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+
     override fun observeFolderMessages(accountId: String, folder: String): Flow<List<Message>> =
         messageDao.observeFolderSummaries(accountId, folder).map { rows -> rows.map { it.toDomain() } }
 
@@ -114,13 +125,40 @@ class MailRepositoryImpl @Inject constructor(
                 attachmentDao.replaceForMessage(id, content.attachments.map { it.toEntity(id) })
                 messageDao.setRead(id, true)
             } else if (!entity.isRead) {
-                runCatching { imapClient.setFlag(params, entity.folder, uidOf(id), Flags.Flag.SEEN, true) }
+                // Optimistic, local-only: the reader can render as soon as this returns. The SEEN flag
+                // still needs to reach the server, but that IMAP round trip (connection + STORE) must not
+                // sit on this path (#148) — the body/attachments are already fully local, so nothing about
+                // rendering the screen needs it. Pushed on backgroundScope, which outlives this call.
                 messageDao.setRead(id, true)
+                pushSeenFlagInBackground(params, entity.folder, id)
             }
         }
         messageDao.getById(id)?.toDomain() ?: error("Message not found")
     }
 
+    /**
+     * Best-effort, fire-and-forget propagation of the SEEN flag to the server, off the message-open
+     * critical path (#148). Retries a few times with a short backoff, then gives up silently: local state
+     * is already correct (the caller set it before launching this), so a permanent failure here just means
+     * the server's copy stays "unread" until something else touches the flag — e.g. the message is opened
+     * from another client, or a future sync gains upward read-state reconciliation. Today's folder sync
+     * does NOT do that: it deliberately leaves cached read/star flags alone when refreshing headers from
+     * the server (see `MessageDao.updateHeaderContent`), so it protects an optimistic local flag from
+     * being clobbered by stale server state, but it does not re-drive a push that never reached the server
+     * either. This retry is in-memory only and does not survive process death mid-backoff.
+     */
+    private fun pushSeenFlagInBackground(params: ImapConnectionParams, folder: String, id: String) {
+        backgroundScope.launch {
+            var attempt = 0
+            while (true) {
+                attempt++
+                val result = runCatching { imapClient.setFlag(params, folder, uidOf(id), Flags.Flag.SEEN, true) }
+                if (result.isSuccess || attempt >= SEEN_FLAG_PUSH_MAX_ATTEMPTS) return@launch
+                delay(SEEN_FLAG_RETRY_BACKOFF_MS * attempt)
+            }
+        }
+    }
+
     override fun observeAttachments(messageId: String): Flow<List<Attachment>> =
         attachmentDao.observeForMessage(messageId).map { rows ->
             rows.map { it.toDomain() }
@@ -410,5 +448,11 @@ private const val SEARCH_LIMIT = 50
 /** Rows per page for the unified inbox (issue #124) — a page is a few screenfuls of message rows. */
 private const val MAILBOX_PAGE_SIZE = 40
 
+/** Attempts for the background best-effort SEEN-flag push before giving up silently (issue #148). */
+private const val SEEN_FLAG_PUSH_MAX_ATTEMPTS = 3
+
+/** Base backoff between SEEN-flag push retries, scaled by attempt number (2s, then 4s). */
+private const val SEEN_FLAG_RETRY_BACKOFF_MS = 2_000L
+
 /** Message id is "<accountId>:<uid>"; the uid is the trailing segment. */
 private fun uidOf(id: String): String = id.substringAfterLast(':')

app/src/test/kotlin/org/libremail/data/repository/MailRepositoryImplTest.kt

@@ -13,8 +13,12 @@ import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
 import io.mockk.slot
+import jakarta.mail.Flags
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.withTimeout
 import org.junit.Test
 import org.libremail.data.local.dao.AccountDao
 import org.libremail.data.local.dao.AttachmentDao
@@ -207,6 +211,62 @@ class MailRepositoryImplTest {
         assertEquals("Tom & Jerry say \"hi\"", snippet.captured)
     }
 
+    /**
+     * The #148 fix: when the body is already cached and the message is unread, openMessage() must not
+     * await the SEEN-flag IMAP round trip before returning. Uses `runBlocking` (not `runTest`) and a
+     * [CompletableDeferred] gate — same idiom as `MailMaintenanceGateTest` — because the behavior under
+     * test is genuine concurrency between the caller's coroutine and the repository's own background
+     * scope, not something a virtual-time test dispatcher can observe.
+     */
+    @Test
+    fun `openMessage returns without awaiting the background SEEN-flag push`() {
+        // Block body (not `= runBlocking { ... }`): the last statement below returns Boolean
+        // (CompletableDeferred.complete), and JUnit4 requires @Test methods to return void/Unit.
+        runBlocking {
+            val id = "acct:INBOX:40"
+            coEvery { messageDao.getById(id) } returns
+                messageEntity(id, "INBOX", bodyFetched = true) // isRead = false
+            coEvery { accountDao.getById("acct") } returns accountEntity()
+            coEvery { connectionFactory.imapParamsFor(any()) } returns imapParams()
+            coEvery { messageDao.setRead(id, true) } just Runs
+            val flagPushStarted = CompletableDeferred<Unit>()
+            val releaseFlagPush = CompletableDeferred<Unit>()
+            coEvery { imapClient.setFlag(any(), "INBOX", "40", Flags.Flag.SEEN, true) } coAnswers {
+                flagPushStarted.complete(Unit)
+                releaseFlagPush.await() // stays "in flight" until this test explicitly releases it
+            }
+
+            val result = repository.openMessage(id)
+
+            // openMessage already completed successfully even though the mocked setFlag call above is
+            // still parked on releaseFlagPush — proof it is not on the awaited path.
+            assertTrue(result.isSuccess)
+            coVerify { messageDao.setRead(id, true) } // the optimistic local write happens synchronously
+            // The push must still actually happen, just off this path — bounded wait, not indefinite.
+            withTimeout(FLAG_PUSH_AWAIT_TIMEOUT_MS) { flagPushStarted.await() }
+            releaseFlagPush.complete(Unit) // let the background coroutine finish cleanly before the test ends
+        }
+    }
+
+    /** Best-effort means retried, not "one attempt and silently give up" (#148 design point 2). */
+    @Test
+    fun `a failed SEEN-flag push is retried in the background`() = runBlocking {
+        val id = "acct:INBOX:41"
+        coEvery { messageDao.getById(id) } returns messageEntity(id, "INBOX", bodyFetched = true)
+        coEvery { accountDao.getById("acct") } returns accountEntity()
+        coEvery { connectionFactory.imapParamsFor(any()) } returns imapParams()
+        coEvery { messageDao.setRead(id, true) } just Runs
+        coEvery { imapClient.setFlag(any(), "INBOX", "41", Flags.Flag.SEEN, true) } throws RuntimeException("boom")
+
+        repository.openMessage(id)
+
+        // The first attempt fails immediately; observing a second proves this is a retry loop rather than
+        // a single best-effort attempt that gives up silently after one failure.
+        coVerify(timeout = FLAG_PUSH_RETRY_VERIFY_TIMEOUT_MS, atLeast = 2) {
+            imapClient.setFlag(any(), "INBOX", "41", Flags.Flag.SEEN, true)
+        }
+    }
+
     @Test
     fun `prefetchMessage leaves a plain-text body's literal angle brackets in the snippet`() = runTest {
         val cache = Files.createTempDirectory("attach").toFile()
@@ -656,3 +716,9 @@ class MailRepositoryImplTest {
             LoadResult.Page(data = rows, prevKey = null, nextKey = null)
     }
 }
+
+/** Bounded real-time wait for the background SEEN-flag push to have started (see `pushSeenFlagInBackground`). */
+private const val FLAG_PUSH_AWAIT_TIMEOUT_MS = 2_000L
+
+/** Bounded real-time wait for a second (retried) SEEN-flag push attempt to land. */
+private const val FLAG_PUSH_RETRY_VERIFY_TIMEOUT_MS = 5_000L