Merge main into feat-239-purge-old-reports

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

@@ -25,6 +25,11 @@ import javax.inject.Singleton
  * Precedence with the #12 backfill is guaranteed two ways: backfill stops paging at the same
  * retention floor this pruner deletes below (their working sets are disjoint), and both jobs share
  * [MailMaintenanceGate] so they never run at once.
+ *
+ * Foreground sync ([MailSyncer]) is aligned the same way in BOTH retention modes so it never
+ * re-inserts what this pruner deletes: it caps its fetch window to the retention count AND drops
+ * anything older than the age cutoff before persisting (#193). The two limits are independent, so
+ * both bounds apply together.
  */
 @Singleton
 class MailPruner @Inject constructor(

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

@@ -87,11 +87,19 @@ class MailSyncer @Inject constructor(
     private suspend fun syncFolderHeaders(account: Account, folder: String, notify: Boolean): Result<Int> =
         runCatching {
             val params = connectionFactory.imapParamsFor(account)
+            val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id)
             // Never fetch more of the recent window than device-only retention (#13) would keep. Without
             // this, a count limit BELOW the window would make foreground sync re-download the same rows
             // the pruner just trimmed, on every sync — an endless re-download/re-prune fight.
-            val fetched = imapClient.fetchRecent(params, folder, recentWindowFor(account)) // cancellable network I/O
+            val window = policy.countLimit?.let { minOf(FETCH_LIMIT, it) } ?: FETCH_LIMIT
+            val fetched = imapClient.fetchRecent(params, folder, window) // cancellable network I/O
+            // Age-based retention (#193): drop anything older than the age cutoff before persisting. On a
+            // low-traffic mailbox the newest-N can extend PAST the cutoff, so without this a sync re-inserts
+            // rows the age pruner just deleted and the next prune deletes them again — a churn loop. Count/
+            // unlimited modes have a null cutoff and keep the full window, so their behavior is unchanged.
+            val cutoff = policy.ageCutoffMillis(System.currentTimeMillis())
             val entities = fetched.map { it.toEntity(account.id, folder) }
+                .let { mapped -> if (cutoff == null) mapped else mapped.filter { it.timestampMillis >= cutoff } }
 
             // Persist and notify atomically with respect to cancellation: an IDLE renewal that cancels
             // mid-sync must not drop a notification (the rows would then look "already seen" next time).
@@ -104,9 +112,11 @@ class MailSyncer @Inject constructor(
                     entities.filter { it.id !in existingIds && !it.isRead }
                 }
 
-                if (entities.isEmpty()) {
+                if (fetched.isEmpty()) {
                     // An empty recent window means the server folder itself is empty, so nothing (not
-                    // even backfilled history) should remain cached for it.
+                    // even backfilled history) should remain cached for it. Keyed on the raw fetch, not the
+                    // age-filtered set: a folder holding only mail older than the age cutoff is NOT empty on
+                    // the server, so its stale local rows are left to the pruner rather than wiped here.
                     messageDao.deleteSyncedByAccountFolder(account.id, folder)
                 } else {
                     val ids = entities.map { it.id }
@@ -147,16 +157,6 @@ class MailSyncer @Inject constructor(
             fetched.size
         }
 
-    /**
-     * The number of recent headers to fetch: the standard [FETCH_LIMIT], but capped by the account's
-     * effective device-only retention count so foreground sync never re-downloads rows the pruner
-     * would immediately trim. Age-only or unlimited retention leaves the full window in place.
-     */
-    private suspend fun recentWindowFor(account: Account): Int {
-        val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id)
-        return policy.countLimit?.let { minOf(FETCH_LIMIT, it) } ?: FETCH_LIMIT
-    }
-
     /**
      * Aggressively pre-caches each not-yet-fetched message's full content (body + attachments) per the
      * user's fetch policy, pausing at low battery regardless of policy — see

app/src/main/kotlin/org/libremail/reporting/DebugReport.kt

@@ -31,12 +31,19 @@ data class DebugReport(
     val userComment: String = "",
     /** Reply-to address the user supplied when submitting (see #159); required for online submit. */
     val userEmail: String = "",
+    /**
+     * Whether the startup crash prompt has already auto-offered this report (see #255). Internal
+     * bookkeeping only: persisted with the report but deliberately kept out of [toSubmissionPayload]
+     * so it never leaks into what the user reviews or submits. A missing flag (older stored reports)
+     * reads as `false` — not yet surfaced.
+     */
+    val surfaced: Boolean = false,
 ) {
     /** The exact text shown for review, copied, saved to a file, and POSTed on submit. */
     fun toSubmissionPayload(): String = toJson().toString(JSON_INDENT)
 
-    /** Compact form used for on-disk persistence. */
-    fun toStorageJson(): String = toJson().toString()
+    /** Compact form used for on-disk persistence; adds the internal [surfaced] bookkeeping flag. */
+    fun toStorageJson(): String = toJson().put("surfaced", surfaced).toString()
 
     private fun toJson(): JSONObject {
         val app = JSONObject()
@@ -92,6 +99,7 @@ data class DebugReport(
                 logs = logs,
                 userComment = json.optString("userComment", ""),
                 userEmail = json.optString("userEmail", ""),
+                surfaced = json.optBoolean("surfaced", false),
             )
         }
     }

app/src/main/kotlin/org/libremail/reporting/ReportStore.kt

@@ -27,6 +27,20 @@ class ReportStore(private val directory: File) {
 
     fun find(id: String): DebugReport? = _reports.value.firstOrNull { it.id == id }
 
+    /**
+     * Persistently marks a report as auto-surfaced so the startup crash prompt offers it at most once
+     * across launches (see #255). The report itself stays in the store (still listed under Problem
+     * Reports); only [delete] removes it. No-op if the report is missing or already surfaced.
+     */
+    fun markSurfaced(id: String) {
+        synchronized(lock) {
+            val report = _reports.value.firstOrNull { it.id == id } ?: return
+            if (report.surfaced) return
+            File(directory, fileName(id)).writeText(report.copy(surfaced = true).toStorageJson())
+            _reports.value = scan()
+        }
+    }
+
     fun delete(id: String) {
         synchronized(lock) {
             File(directory, fileName(id)).delete()

app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt

@@ -69,7 +69,6 @@ fun LibreMailApp(
     val start = startDestination ?: return
     val licenseAlreadyAccepted = licenseAccepted ?: return
     val navController = rememberNavController()
-    val pendingCrash by startupViewModel.pendingCrash.collectAsStateWithLifecycle()
 
     // A mailto:/share intent opens compose on top of the mailbox, pre-filled. Keyed on the request so
     // it fires once per intent (and again for a new intent delivered while the app is alive).
@@ -256,14 +255,28 @@ fun LibreMailApp(
     }
 
     // On launch, offer any saved crash report for review — never sent without the user's action.
+    StartupCrashPrompt(
+        viewModel = startupViewModel,
+        onReview = { reportId -> navController.navigate(Routes.reportReview(reportId)) },
+    )
+}
+
+/**
+ * Offers any pending crash report for review on launch (see #255). [StartupReportViewModel] gates this
+ * to a legitimate crash from the last 24h, shown at most once; this only renders its decision. "Review"
+ * and "Not now" both mark the report surfaced so it never re-nags; only "Discard" deletes it.
+ */
+@Composable
+internal fun StartupCrashPrompt(viewModel: StartupReportViewModel, onReview: (String) -> Unit) {
+    val pendingCrash by viewModel.pendingCrash.collectAsStateWithLifecycle()
     pendingCrash?.let { crash ->
         CrashReportDialog(
             onReview = {
-                startupViewModel.dismiss()
-                navController.navigate(Routes.reportReview(crash.id))
+                viewModel.dismiss(crash.id)
+                onReview(crash.id)
             },
-            onLater = startupViewModel::dismiss,
-            onDiscard = { startupViewModel.discard(crash.id) },
+            onLater = { viewModel.dismiss(crash.id) },
+            onDiscard = { viewModel.discard(crash.id) },
         )
     }
 }

app/src/main/kotlin/org/libremail/ui/reporting/StartupReportViewModel.kt

@@ -4,43 +4,58 @@ package org.libremail.ui.reporting
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
-import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.launch
 import org.libremail.reporting.ReportKind
 import org.libremail.reporting.ReportStore
 import javax.inject.Inject
 
-/** Surfaces a pending crash report (if any) so the app can offer it for review on launch. */
+/**
+ * Surfaces a pending crash report (if any) so the app can offer it for review on launch. The prompt is
+ * gated (see #255) so it fires at most once, only for a legitimate recent crash:
+ *
+ * - **First re-open only:** a report is auto-offered once, then persistently marked surfaced; it never
+ *   re-nags on later launches. It stays in the store (still listed under Problem Reports for manual
+ *   review); only [discard] deletes it.
+ * - **< 24h only:** older crashes are never auto-surfaced (they may still be reviewed manually).
+ * - **Legitimate crash only:** only `CrashReporter`'s uncaught-exception handler ever creates a
+ *   [ReportKind.CRASH] report, so an app update, a user-initiated close, or a force-stop create no
+ *   report and therefore never pop this prompt.
+ *
+ * @param now clock provider, injected so the age gate is unit-testable.
+ */
 @HiltViewModel
-class StartupReportViewModel @Inject constructor(private val store: ReportStore) : ViewModel() {
+class StartupReportViewModel(private val store: ReportStore, private val now: () -> Long) : ViewModel() {
 
-    private val dismissed = MutableStateFlow(false)
+    @Inject
+    constructor(store: ReportStore) : this(store, { System.currentTimeMillis() })
 
     val pendingCrash: StateFlow<ReportSummary?> =
-        combine(store.reports, dismissed) { reports, isDismissed ->
-            if (isDismissed) {
-                null
-            } else {
-                reports.firstOrNull { it.kind == ReportKind.CRASH }
-                    ?.let { ReportSummary(it.id, it.kind, it.createdAtMillis) }
-            }
+        store.reports.map { reports ->
+            reports.firstOrNull { report ->
+                report.kind == ReportKind.CRASH &&
+                    !report.surfaced &&
+                    report.createdAtMillis >= now() - CRASH_MAX_AGE_MS
+            }?.let { ReportSummary(it.id, it.kind, it.createdAtMillis) }
         }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(SUBSCRIBE_MS), null)
 
-    /** Hides the prompt for this launch; the report stays saved and is offered again next launch. */
-    fun dismiss() {
-        dismissed.value = true
+    /**
+     * "Not now" / "Review": persistently marks the crash surfaced so it is auto-offered at most once
+     * across launches. The report stays saved (still shown in Problem Reports); only [discard] deletes.
+     */
+    fun dismiss(id: String) {
+        viewModelScope.launch { store.markSurfaced(id) }
     }
 
     fun discard(id: String) {
-        dismissed.value = true
         viewModelScope.launch { store.delete(id) }
     }
 
     private companion object {
         const val SUBSCRIBE_MS = 5_000L
+        const val CRASH_MAX_AGE_MS = 24L * 60 * 60 * 1000
     }
 }