fix(update): make app update check, discovery, and installer flow reliable

Automate background update checks on launch/foreground and share results across chat and settings. Disregard release candidates by default unless opted in, and handle installer cancellation and download failures cleanly.

app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateCache.kt

@@ -21,8 +21,9 @@ object AppUpdateCache {
      * Session-only dismissal of the chat banner. Snapshot-backed so the chat
      * screen recomposes when it flips; dies with the process.
      */
-    var dismissed by mutableStateOf(false)
-        private set
+    private var dismissedTags by mutableStateOf<Set<String>>(emptySet())
+
+    fun isDismissed(tag: String): Boolean = tag in dismissedTags
 
     /** Whether the full-screen / in-place update dialog should be visible. */
     var isDialogVisible by mutableStateOf(false)
@@ -32,8 +33,8 @@ object AppUpdateCache {
         _state.value = state
     }
 
-    fun dismiss() {
-        dismissed = true
+    fun dismiss(tag: String) {
+        dismissedTags = dismissedTags + tag
     }
 
     fun showDialog() {
@@ -46,7 +47,7 @@ object AppUpdateCache {
 
     /** Test hook: clear state and dismissal between tests. */
     internal fun reset() {
-        dismissed = false
+        dismissedTags = emptySet()
         isDialogVisible = false
         _state.value = AppUpdateState.Idle
     }

app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateChecker.kt

@@ -2,10 +2,15 @@ package com.m57.hermescontrol.data.update
 
 import com.m57.hermescontrol.data.remote.OkHttpProvider
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.withContext
 import kotlinx.serialization.Serializable
+import okhttp3.Call
+import okhttp3.Callback
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import okhttp3.OkHttpClient
 import okhttp3.Request
+import okhttp3.Response
 import java.io.File
 import java.io.IOException
 
@@ -79,17 +84,28 @@ fun selectLatestUpdate(
     includeReleaseCandidates: Boolean = false,
 ): UpdateInfo? =
     releases
-        .filter { !it.draft && it.apkAsset != null }
-        .filter {
-            if (!it.prerelease) {
-                true
-            } else {
-                includeReleaseCandidates && isReleaseCandidateVersion(it.tagName)
-            }
-        }.reduceOrNull { best, candidate ->
+        .filter(::hasUsableApk)
+        .filter { isEligibleRelease(it, includeReleaseCandidates) }
+        .reduceOrNull { best, candidate ->
             if (isNewerVersion(candidate.tagName, best.tagName)) candidate else best
         }
 
+private fun hasUsableApk(release: UpdateInfo): Boolean =
+    !release.draft && release.apkAsset?.browserDownloadUrl?.toHttpUrlOrNull()?.let {
+        it.scheme == "http" || it.scheme == "https"
+    } == true
+
+private fun isEligibleRelease(
+    release: UpdateInfo,
+    includeReleaseCandidates: Boolean,
+): Boolean {
+    val version = parseVersion(release.tagName) ?: return false
+    val isRc = version.prerelease.firstOrNull()?.let(RC_IDENTIFIER::matches) == true
+    val isStable = version.prerelease.isEmpty() && !release.prerelease
+    if (isStable) return true
+    return includeReleaseCandidates && isRc
+}
+
 private data class ParsedVersion(
     val core: List<Int>,
     val prerelease: List<String>,
@@ -161,17 +177,16 @@ open class AppUpdateChecker(
      */
     open suspend fun fetchLatestRelease(includeReleaseCandidates: Boolean = false): UpdateInfo? =
         withContext(Dispatchers.IO) {
-            if (includeReleaseCandidates) {
-                val body = get("$apiBaseUrl/releases?per_page=$RELEASE_LIST_PAGE_SIZE") ?: return@withContext null
-                selectLatestUpdate(parseReleaseList(body).orEmpty(), includeReleaseCandidates = true)
-            } else {
-                val body = get("$apiBaseUrl/releases/latest") ?: return@withContext null
-                parseUpdateInfo(body)
+            val latest = get("$apiBaseUrl/releases/latest")?.let(::parseRelease)
+            if (!includeReleaseCandidates && latest != null && selectLatestUpdate(listOf(latest)) != null) {
+                return@withContext latest
             }
+            val releases = fetchReleasePages() ?: return@withContext null
+            selectLatestUpdate(releases + listOfNotNull(latest), includeReleaseCandidates)
         }
 
-    /** GET [url] as JSON text, or null on any non-2xx response. */
-    private fun get(url: String): String? {
+    /** GET a GitHub URL. 404 means absent; other HTTP/network failures stay visible. */
+    private fun get(url: String): ReleasePage? {
         val request =
             Request
                 .Builder()
@@ -179,11 +194,45 @@ open class AppUpdateChecker(
                 .header("Accept", "application/vnd.github+json")
                 .build()
         client.newCall(request).execute().use { response ->
-            if (!response.isSuccessful) return null
-            return response.body.string().orEmpty()
+            if (response.code == HTTP_NOT_FOUND) return null
+            if (!response.isSuccessful) throw IOException("GitHub releases request failed (${response.code})")
+            return ReleasePage(response.body.string().orEmpty(), response.nextPageUrl())
         }
     }
 
+    private fun fetchReleasePages(): List<UpdateInfo>? {
+        var nextUrl: String? = "$apiBaseUrl/releases?per_page=$RELEASE_LIST_PAGE_SIZE&page=1"
+        val releases = mutableListOf<UpdateInfo>()
+        while (nextUrl != null) {
+            val page =
+                get(nextUrl)
+                    ?: return if (releases.isEmpty()) null else throw IOException("Incomplete GitHub releases list")
+            val parsed = parseReleaseList(page.body) ?: throw IOException("GitHub returned malformed release metadata")
+            releases += parsed
+            nextUrl = page.nextUrl
+        }
+        return releases
+    }
+
+    private fun parseRelease(page: ReleasePage): UpdateInfo =
+        parseUpdateInfo(page.body) ?: throw IOException("GitHub returned malformed release metadata")
+
+    private fun Response.nextPageUrl(): String? {
+        val link = header("Link") ?: return null
+        val url = NEXT_LINK.find(link)?.groupValues?.get(1) ?: return null
+        val next = url.toHttpUrlOrNull() ?: throw IOException("GitHub returned an invalid pagination URL")
+        val base = apiBaseUrl.toHttpUrlOrNull() ?: throw IOException("Invalid GitHub API base URL")
+        if (next.host != base.host || next.encodedPath != base.encodedPath.trimEnd('/') + "/releases") {
+            throw IOException("GitHub returned an untrusted pagination URL")
+        }
+        return next.toString()
+    }
+
+    private data class ReleasePage(
+        val body: String,
+        val nextUrl: String?,
+    )
+
     /**
      * Stream a release APK asset to [dest], reporting progress 0..1 via
      * [onProgress]. True on success; false (with the partial file deleted)
@@ -194,41 +243,103 @@ open class AppUpdateChecker(
         dest: File,
         onProgress: (Float) -> Unit,
     ): Boolean =
-        withContext(Dispatchers.IO) {
-            val request = Request.Builder().url(url).build()
-            try {
-                client.newCall(request).execute().use { response ->
-                    if (!response.isSuccessful) return@withContext false
-                    val body = response.body
-                    val total = body.contentLength()
-                    dest.outputStream().use { out ->
-                        val buffer = ByteArray(DEFAULT_BUFFER_SIZE)
-                        var read = 0L
-                        body.byteStream().use { input ->
-                            while (true) {
-                                val n = input.read(buffer)
-                                if (n == -1) break
-                                out.write(buffer, 0, n)
-                                read += n
-                                if (total > 0) {
-                                    onProgress((read.toFloat() / total.toFloat()).coerceIn(0f, 1f))
+        suspendCancellableCoroutine { continuation ->
+            val request =
+                try {
+                    val httpUrl =
+                        url.toHttpUrlOrNull()?.takeIf { it.scheme == "http" || it.scheme == "https" }
+                            ?: run {
+                                failDownload(continuation, dest)
+                                return@suspendCancellableCoroutine
+                            }
+                    Request.Builder().url(httpUrl).build()
+                } catch (_: IllegalArgumentException) {
+                    failDownload(continuation, dest)
+                    return@suspendCancellableCoroutine
+                }
+            val call =
+                try {
+                    client.newCall(request)
+                } catch (_: Exception) {
+                    failDownload(continuation, dest)
+                    return@suspendCancellableCoroutine
+                }
+            continuation.invokeOnCancellation {
+                call.cancel()
+                deletePartial(dest)
+            }
+            val callback =
+                object : Callback {
+                    override fun onFailure(
+                        call: Call,
+                        e: IOException,
+                    ) {
+                        failDownload(continuation, dest)
+                    }
+
+                    override fun onResponse(
+                        call: Call,
+                        response: Response,
+                    ) {
+                        var succeeded = false
+                        try {
+                            response.use {
+                                if (response.isSuccessful) {
+                                    val body = response.body
+                                    val total = body.contentLength()
+                                    dest.outputStream().use { out ->
+                                        val buffer = ByteArray(DEFAULT_BUFFER_SIZE)
+                                        var read = 0L
+                                        body.byteStream().use { input ->
+                                            while (true) {
+                                                val n = input.read(buffer)
+                                                if (n == -1) break
+                                                out.write(buffer, 0, n)
+                                                read += n
+                                                if (total > 0) {
+                                                    onProgress((read.toFloat() / total.toFloat()).coerceIn(0f, 1f))
+                                                }
+                                            }
+                                        }
+                                    }
+                                    succeeded = true
                                 }
                             }
+                        } catch (_: Exception) {
+                            // Includes response/body close failures; never report success before close.
+                        }
+                        if (succeeded) {
+                            if (continuation.isActive) continuation.resume(true) { _, _, _ -> }
+                        } else {
+                            failDownload(continuation, dest)
                         }
                     }
-                    true
                 }
-            } catch (e: Exception) {
-                dest.delete()
-                false
+            try {
+                call.enqueue(callback)
+            } catch (_: Exception) {
+                failDownload(continuation, dest)
             }
         }
 
+    private fun failDownload(
+        continuation: kotlinx.coroutines.CancellableContinuation<Boolean>,
+        dest: File,
+    ) {
+        deletePartial(dest)
+        if (continuation.isActive) continuation.resume(false) { _, _, _ -> }
+    }
+
+    private fun deletePartial(dest: File) {
+        runCatching { dest.delete() }
+    }
+
     private companion object {
         const val DEFAULT_BUFFER_SIZE = 8192
+        const val HTTP_NOT_FOUND = 404
 
-        /** How many recent releases the opt-in RC scan inspects. */
-        const val RELEASE_LIST_PAGE_SIZE = 20
+        const val RELEASE_LIST_PAGE_SIZE = 100
+        val NEXT_LINK = Regex("<([^>]+)>;\\s*rel=\"next\"")
     }
 }
 

app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateState.kt

@@ -40,6 +40,7 @@ sealed interface AppUpdateState {
 
     data class Error(
         val message: String,
+        val isCheckError: Boolean = false,
     ) : AppUpdateState
 }
 

app/src/main/java/com/m57/hermescontrol/data/update/UpdateNoticeManager.kt

@@ -6,23 +6,31 @@ import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.currentCoroutineContext
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
 import java.io.IOException
 
 /**
- * Startup update check (issue #890): one silent GitHub ping per installed
- * version, fired right after launch, so the chat screen can show an update
- * banner without the user ever opening the About tab.
+ * Shared release check (issue #890): checks after launch and on foreground
+ * return, with a short process-local debounce so the chat screen can show an
+ * update banner without the user ever opening the About tab.
  *
  * The result lands in [AppUpdateCache] (consumed by the chat banner and
  * adopted by the About tab) and the latest tag is persisted via
  * [AuthManager.setLastKnownLatestTag] so a dismissed banner can return on a
- * later launch. Failures leave the cache Idle — the About tab still has its
- * own manual retry path.
+ * later launch. Failed checks remain visible and can be retried from the About
+ * tab or dialog without persisting a false successful-check marker.
  */
 object UpdateNoticeManager {
     private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
+    private val checkLock = Any()
+    private var activeCheck: Job? = null
+    private var lastSuccessfulCheckAt: Long? = null
+    private var lastFailedCheckAt: Long? = null
+    private var channelGeneration = 0
 
     /**
      * Release-only feature (issue #890): the launch check and chat banner are
@@ -32,57 +40,153 @@ object UpdateNoticeManager {
     var enabled: Boolean = !BuildConfig.DEBUG
         internal set
 
-    /** Release check interval (24 hours). */
-    const val CHECK_INTERVAL_MS: Long = 24 * 60 * 60 * 1000L
+    /** Avoid duplicate foreground checks in one process; a cold launch always rechecks. */
+    const val CHECK_INTERVAL_MS: Long = 5 * 60 * 1000L
+    const val FAILURE_BACKOFF_MS: Long = 30 * 1000L
 
-    /** Run from Application.onCreate, after AuthManager.init. Checks at most once every 24h. */
+    /** Run after preferences are ready; failed checks never suppress the next foreground retry. */
     fun checkOnLaunch(
         checker: AppUpdateChecker = AppUpdateChecker(),
         currentVersion: String = BuildConfig.VERSION_NAME,
         ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
         now: Long = System.currentTimeMillis(),
+        force: Boolean = false,
     ) {
-        if (!enabled) return
-        val lastCheck = AuthManager.getLastUpdateCheckTimestamp()
-        val versionChanged = AuthManager.getUpdateCheckDoneForVersion() != currentVersion
-        if (!versionChanged && (now - lastCheck < CHECK_INTERVAL_MS)) return
-
-        // Captured once so the check and the notice path agree on the channel.
-        val includeReleaseCandidates = AuthManager.isCheckingReleaseCandidateUpdates()
-
-        scope.launch(ioDispatcher) {
-            AuthManager.setUpdateCheckDoneForVersion(currentVersion)
-            AuthManager.setLastUpdateCheckTimestamp(now)
-            val result =
-                try {
-                    checker.fetchLatestRelease(includeReleaseCandidates)
-                } catch (e: CancellationException) {
-                    throw e
-                } catch (e: IOException) {
-                    return@launch
-                } catch (e: Exception) {
-                    return@launch
-                }
-            val info = result ?: return@launch
-            // The user may have switched back to stable while this request was in
-            // flight — an RC result must never be published on the stable channel.
-            if (isReleaseCandidateVersion(info.tagName) && !AuthManager.isCheckingReleaseCandidateUpdates()) {
-                return@launch
+        if (!enabled && !force) return
+        synchronized(checkLock) {
+            if (activeCheck?.isActive == true) return
+            if (!force && lastSuccessfulCheckAt?.let { now - it < CHECK_INTERVAL_MS } == true) return
+            if (!force && lastFailedCheckAt?.let { now - it < FAILURE_BACKOFF_MS } == true) return
+            val includeReleaseCandidates = AuthManager.isCheckingReleaseCandidateUpdates()
+            val generation = channelGeneration
+            val previousState = AppUpdateCache.state.value
+            if (force ||
+                previousState !is AppUpdateState.UpdateAvailable
+            ) {
+                AppUpdateCache.update(AppUpdateState.Checking)
             }
-            val apk = info.apkAsset ?: return@launch
-            val state =
-                if (isNewerVersion(info.tagName, currentVersion)) {
-                    AppUpdateState.UpdateAvailable(
-                        latestTag = info.tagName,
-                        apkUrl = apk.browserDownloadUrl,
-                        sizeBytes = apk.size,
-                        releaseNotes = info.body,
-                    )
-                } else {
-                    AppUpdateState.UpToDate(latestTag = info.tagName)
+            activeCheck =
+                scope.launch(ioDispatcher) {
+                    val thisJob = currentCoroutineContext()[Job]
+                    var successful = false
+                    var failed = false
+                    try {
+                        val info = checker.fetchLatestRelease(includeReleaseCandidates)
+                        val apk = info?.apkAsset
+                        val state =
+                            if (info == null) {
+                                AppUpdateState.Error(NO_RELEASE_ERROR, isCheckError = true)
+                            } else if (apk == null) {
+                                AppUpdateState.Error(NO_APK_ERROR, isCheckError = true)
+                            } else if (isNewerVersion(info.tagName, currentVersion)) {
+                                AppUpdateState.UpdateAvailable(
+                                    latestTag = info.tagName,
+                                    apkUrl = apk.browserDownloadUrl,
+                                    sizeBytes = apk.size,
+                                    releaseNotes = info.body,
+                                )
+                            } else {
+                                AppUpdateState.UpToDate(latestTag = info.tagName)
+                            }
+                        var resultRejected = false
+                        synchronized(checkLock) {
+                            if (activeCheck !== thisJob || generation != channelGeneration) return@synchronized
+                            val currentChannel = AuthManager.isCheckingReleaseCandidateUpdates()
+                            if (includeReleaseCandidates != currentChannel ||
+                                (info != null && isReleaseCandidateVersion(info.tagName) && !currentChannel)
+                            ) {
+                                if (AppUpdateCache.state.value is AppUpdateState.Checking) {
+                                    AppUpdateCache.update(previousState)
+                                }
+                                resultRejected = true
+                                return@synchronized
+                            }
+                            AppUpdateCache.update(state)
+                            if (state is AppUpdateState.UpdateAvailable || state is AppUpdateState.UpToDate) {
+                                state.releaseTag()?.let { AuthManager.setLastKnownLatestTag(it) }
+                                AuthManager.setUpdateCheckDoneForVersion(currentVersion)
+                                AuthManager.setLastUpdateCheckTimestamp(System.currentTimeMillis())
+                                successful = true
+                            } else {
+                                failed = true
+                            }
+                        }
+                        if (resultRejected) return@launch
+                    } catch (e: CancellationException) {
+                        throw e
+                    } catch (e: IOException) {
+                        synchronized(checkLock) {
+                            if (activeCheck === thisJob && generation == channelGeneration) {
+                                failed = true
+                                if (AppUpdateCache.state.value is AppUpdateState.Checking) {
+                                    AppUpdateCache.update(AppUpdateState.Error(NETWORK_ERROR, isCheckError = true))
+                                }
+                            }
+                        }
+                    } catch (e: Exception) {
+                        synchronized(checkLock) {
+                            if (activeCheck === thisJob && generation == channelGeneration) {
+                                failed = true
+                                if (AppUpdateCache.state.value is AppUpdateState.Checking) {
+                                    AppUpdateCache.update(
+                                        AppUpdateState.Error(GENERIC_CHECK_ERROR, isCheckError = true),
+                                    )
+                                }
+                            }
+                        }
+                    } finally {
+                        synchronized(checkLock) {
+                            if (activeCheck === thisJob) {
+                                if (successful) lastSuccessfulCheckAt = now
+                                if (failed) lastFailedCheckAt = now
+                                activeCheck = null
+                            }
+                        }
+                    }
                 }
-            AppUpdateCache.update(state)
-            state.releaseTag()?.let { AuthManager.setLastKnownLatestTag(it) }
+        }
+    }
+
+    private const val NETWORK_ERROR = "Network error — check your connection"
+    private const val GENERIC_CHECK_ERROR = "Couldn't check for updates"
+    private const val NO_RELEASE_ERROR = "No release found yet"
+    private const val NO_APK_ERROR = "Release has no APK asset"
+
+    /** Explicit check action; shares in-flight work but bypasses the local debounce. */
+    fun checkNow(
+        checker: AppUpdateChecker,
+        currentVersion: String,
+        ioDispatcher: CoroutineDispatcher,
+    ) {
+        checkOnLaunch(checker, currentVersion, ioDispatcher, force = true)
+    }
+
+    /** Persist the channel choice and invalidate in-flight checks as one operation. */
+    fun channelChanged(enabled: Boolean) {
+        synchronized(checkLock) {
+            AuthManager.setCheckReleaseCandidateUpdates(enabled)
+            channelGeneration++
+            if (AppUpdateCache.state.value is AppUpdateState.Checking) {
+                AppUpdateCache.update(AppUpdateState.Idle)
+            }
+            activeCheck?.cancel()
+            activeCheck = null
+            lastSuccessfulCheckAt = null
+            lastFailedCheckAt = null
+        }
+    }
+
+    /** Test hook: reset process-local admission state between tests. */
+    internal fun resetForTests() {
+        synchronized(checkLock) {
+            activeCheck?.cancel()
+            activeCheck = null
+            if (AppUpdateCache.state.value is AppUpdateState.Checking) {
+                AppUpdateCache.update(AppUpdateState.Idle)
+            }
+            lastSuccessfulCheckAt = null
+            lastFailedCheckAt = null
+            channelGeneration++
         }
     }
 
@@ -94,12 +198,9 @@ object UpdateNoticeManager {
     fun noticeTag(currentVersion: String = BuildConfig.VERSION_NAME): String? {
         if (!enabled) return null
         val dismissed = AuthManager.getDismissedUpdateTag()
-        val candidateTag =
-            (AppUpdateCache.state.value as? AppUpdateState.UpdateAvailable)?.latestTag
-                ?: AuthManager.getLastKnownLatestTag()
-                ?: return null
+        val candidateTag = (AppUpdateCache.state.value as? AppUpdateState.UpdateAvailable)?.latestTag ?: return null
 
-        if (candidateTag == dismissed) return null
+        if (candidateTag == dismissed || AppUpdateCache.isDismissed(candidateTag)) return null
         if (isReleaseCandidateVersion(candidateTag) && !AuthManager.isCheckingReleaseCandidateUpdates()) {
             return null
         }