Merge pull request 'fix(android): don't flip offline on WAN-validation flicker — trust /healthz' (#85) from dev into main

android/app/src/main/java/com/fabledsword/minstrel/connectivity/ConnectivityObserver.kt

@@ -14,11 +14,23 @@ import javax.inject.Inject
 import javax.inject.Singleton
 
 /**
- * Single source of truth for the device's "is the internet usable
- * right now" signal — wraps [ConnectivityManager] and exposes a hot
- * cold-startable Flow that emits `false` while the active network
- * lacks INTERNET + VALIDATED capabilities (airplane mode, no carrier,
- * captive portal, etc.) and `true` once a usable network appears.
+ * Single source of truth for "does the device have a network link at
+ * all" — wraps [ConnectivityManager] and exposes a hot cold-startable
+ * Flow that emits `false` only when there is no active INTERNET-capable
+ * network (airplane mode, no carrier/Wi-Fi) and `true` once any network
+ * link appears.
+ *
+ * Deliberately does NOT require `NET_CAPABILITY_VALIDATED`. VALIDATED
+ * tracks whether Android reached its own WAN internet-validation probe
+ * (Google's `generate_204`) — which is the wrong question for a
+ * self-hosted server that is usually on the LAN. A transient WAN/DNS
+ * blip (or Android's periodic re-validation) momentarily drops VALIDATED
+ * while the Minstrel box stays perfectly reachable; gating on it flipped
+ * the app to Offline with no debounce and fast-failed in-flight playback
+ * via [com.fabledsword.minstrel.player.OfflineGatedDataSource]. The
+ * authority on whether *Minstrel* is reachable is the `/healthz` poll
+ * ([com.fabledsword.minstrel.update.data.VersionCheckController], which
+ * has its own failure hysteresis), not this coarse device-link signal.
  *
  * Used by the shell-level ConnectionErrorBanner; downstream
  * repositories can also collect this to gate retry loops.
@@ -35,36 +47,36 @@ class ConnectivityObserver @Inject constructor(
             .build()
         val callback = object : ConnectivityManager.NetworkCallback() {
             override fun onAvailable(network: Network) {
-                trySend(hasUsableInternet())
+                trySend(hasActiveNetwork())
             }
             override fun onLost(network: Network) {
-                trySend(hasUsableInternet())
+                trySend(hasActiveNetwork())
             }
             override fun onCapabilitiesChanged(
                 network: Network,
                 capabilities: NetworkCapabilities,
             ) {
+                // INTERNET only -- NOT VALIDATED. A WAN/validation flicker
+                // must not read as "device offline" when the LAN (and the
+                // Minstrel server on it) is still reachable. /healthz is the
+                // authority on server reachability.
                 trySend(
                     capabilities.hasCapability(
                         NetworkCapabilities.NET_CAPABILITY_INTERNET,
-                    ) &&
-                        capabilities.hasCapability(
-                            NetworkCapabilities.NET_CAPABILITY_VALIDATED,
-                        ),
+                    ),
                 )
             }
         }
         cm.registerNetworkCallback(request, callback)
         // Seed the initial value so the banner doesn't flash before the
         // first capability callback fires.
-        trySend(hasUsableInternet())
+        trySend(hasActiveNetwork())
         awaitClose { cm.unregisterNetworkCallback(callback) }
     }.distinctUntilChanged()
 
-    private fun hasUsableInternet(): Boolean {
+    private fun hasActiveNetwork(): Boolean {
         val caps = cm.activeNetwork?.let { cm.getNetworkCapabilities(it) }
         return caps != null &&
-            caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) &&
-            caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
+            caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
     }
 }

android/app/src/main/java/com/fabledsword/minstrel/connectivity/ServerHealthController.kt

@@ -7,7 +7,10 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.stateIn
+import timber.log.Timber
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -19,11 +22,14 @@ import javax.inject.Singleton
  * Composed from two existing signals -- this controller doesn't poll its own
  * endpoint:
  *
- *  - [ConnectivityObserver.online] -- system-level NetworkCallback with the
- *    INTERNET + VALIDATED capability check (captive portals fail this).
+ *  - [ConnectivityObserver.online] -- system-level NetworkCallback that
+ *    answers only "is there an active INTERNET-capable network link" (NOT
+ *    VALIDATED -- a WAN-validation flicker must not read as offline when
+ *    the LAN server is reachable).
  *  - [VersionCheckController.reachable] -- did the last `/healthz` poll
- *    succeed. Distinguishes "device has network but our server is down" from
- *    "no network at all," which the connectivity-only signal can't.
+ *    succeed. This is the authority on whether *Minstrel* is reachable;
+ *    it has its own failure hysteresis. Distinguishes "device has a link
+ *    but our server is down" from "no network at all."
  *
  * `version too old` is intentionally *not* folded in here -- it's a separate
  * UX (the VersionTooOldBanner) and conflating it with offline would mask the
@@ -46,11 +52,17 @@ class ServerHealthController @Inject constructor(
             !serverReachable -> ServerHealth.ServerDown
             else -> ServerHealth.Healthy
         }
-    }.stateIn(
-        scope = scope,
-        started = SharingStarted.Eagerly,
-        initialValue = ServerHealth.Healthy,
-    )
+    }
+        // Transition log -- the signal had no instrumentation, which is why a
+        // false-offline (WAN flicker flipping playback to "Source error") was
+        // hard to diagnose from logcat. WARN-tier so ReleaseTree surfaces it.
+        .distinctUntilChanged()
+        .onEach { Timber.w("ServerHealth -> %s", it) }
+        .stateIn(
+            scope = scope,
+            started = SharingStarted.Eagerly,
+            initialValue = ServerHealth.Healthy,
+        )
 }
 
 /**