Upgrade: Invalidate a dead billing connection from refreshStrict too

refreshStrict() throws its incomplete-result error AFTER useConnection
already returned: refreshPurchases hands back a partial result instead of
throwing, so useConnection's dead-binder detection never sees it. A gate
that ran against a connection whose binder died mid-query (partialError
cause chain carrying SERVICE_DISCONNECTED / SERVICE_TIMEOUT) left that
connection installed, and every later purchase check kept talking to the
corpse until something else tore it down.

processReconciliation() already compensated for the refresh()/connect-loop
paths; the strict path had no such call. Extract that dead-connection block
into invalidateOnDeadConnection() and call it from both. The strict path
deliberately does NOT feed the episode clock — a gate the user aborted
mid-purchase is not a reconciliation outcome.

Fixes review finding F1.

Author: d4rken

app/src/gplay/java/eu/darken/amply/upgrade/core/billing/BillingManager.kt

@@ -432,17 +432,12 @@ open class BillingManager @Inject constructor(
         }
     }
 
-    // Everything a COMPLETED refresh owes the rest of the app, in one place: both the connect loop's
-    // initial refresh and manual refresh() calls run through it, so a Restore tap during an outage
-    // feeds the same bookkeeping the connect loop does. Only reached when refreshPurchases returned
-    // (it still throws when it found nothing AND a query failed — that path is the connect loop's /
-    // useConnection's).
-    private fun processReconciliation(refresh: BillingConnection.PurchaseRefresh) {
-        // A partial refresh no longer reaches useConnection's dead-binder detection (it returns
-        // instead of throwing), so the teardown that used to ride the throw path happens here. Cause
-        // chain, not the exception itself: the failure arrives user-friendly-mapped. Deliberately no
-        // holder CAS: the failing connection may already have been replaced, and the accepted cost of
-        // that rare race is one extra failed action while the loop reconnects.
+    // A partial refresh no longer reaches useConnection's dead-binder detection (it returns instead
+    // of throwing), so the teardown that used to ride the throw path happens here. Cause chain, not
+    // the exception itself: the failure arrives user-friendly-mapped. Deliberately no holder CAS:
+    // the failing connection may already have been replaced, and the accepted cost of that rare race
+    // is one extra failed action while the loop reconnects.
+    private fun invalidateOnDeadConnection(refresh: BillingConnection.PurchaseRefresh) {
         val clientError = refresh.partialError?.let {
             (it as? BillingClientException) ?: (it.cause as? BillingClientException)
         }
@@ -452,6 +447,15 @@ open class BillingManager @Inject constructor(
             }
             invalidations.trySend(Unit)
         }
+    }
+
+    // Everything a COMPLETED refresh owes the rest of the app, in one place: both the connect loop's
+    // initial refresh and manual refresh() calls run through it, so a Restore tap during an outage
+    // feeds the same bookkeeping the connect loop does. Only reached when refreshPurchases returned
+    // (it still throws when it found nothing AND a query failed — that path is the connect loop's /
+    // useConnection's).
+    private fun processReconciliation(refresh: BillingConnection.PurchaseRefresh) {
+        invalidateOnDeadConnection(refresh)
 
         if (!refresh.isComplete && !refresh.hasConfirmedProPurchase) {
             // A reconciliation that couldn't confirm the upgrade. Stamped with the refresh's COMMIT
@@ -529,6 +533,11 @@ open class BillingManager @Inject constructor(
     open suspend fun refreshStrict(): BillingData {
         log(TAG) { "refreshStrict()" }
         val fresh = useConnection { refreshPurchases() }
+        // A gate that hit a dying connection must still trigger the reconnect (the throw below
+        // bypasses useConnection's detection, which already returned), or every later purchase check
+        // keeps reusing the corpse. No-op on a complete refresh. The episode clock stays out of this
+        // path: an aborted gate is not a reconciliation outcome.
+        invalidateOnDeadConnection(fresh)
         if (!fresh.isComplete) {
             // partialError is set for every incomplete refresh; the fallback only exists so a future
             // incompleteness without a captured cause still fails closed instead of passing.

app/src/testGplay/java/eu/darken/amply/upgrade/core/billing/BillingManagerTest.kt

@@ -248,6 +248,44 @@ class BillingManagerTest {
         withTimeout(TIMEOUT_MS) { attempts.receive() }
     }
 
+    @Test fun `a strict gate failure on a dead connection still tears the connection down`(): Unit = runBlocking {
+        // The gate's throw happens AFTER useConnection already returned (the refresh hands back a
+        // partial result instead of throwing), so its dead-binder detection never sees it. Without
+        // the explicit invalidation the dying connection stays installed and every later purchase
+        // check keeps talking to the corpse.
+        val pending = TestPurchases.purchase(iapId, pending = true)
+        val attempts = Channel<Unit>(Channel.UNLIMITED)
+        val dying = client(iap = FakeBillingClient.Answer(purchases = listOf(pending)))
+        val subQueries = AtomicInteger(0)
+        // The binder dies BETWEEN the two refreshes: the connect loop's initial one is complete (so
+        // its own reconciliation invalidates nothing), and only the gate's own round-trip loses SUBS.
+        dying.beforeAnswer = { type ->
+            if (type == BillingClient.ProductType.SUBS && subQueries.incrementAndGet() == 2) {
+                dying.answers = dying.answers +
+                    (BillingClient.ProductType.SUBS to failing(BillingResponseCode.SERVICE_DISCONNECTED))
+            }
+        }
+        val manager = connectedManager(dying, client(), attempts = attempts)
+
+        // Connection 1 is established and its initial refresh committed.
+        withTimeout(TIMEOUT_MS) { manager.billingData.first() }
+        withTimeout(TIMEOUT_MS) { attempts.receive() }
+
+        withTimeout(TIMEOUT_MS) {
+            shouldThrow<GplayServiceUnavailableException> { manager.refreshStrict() }
+        }
+
+        // The dead connection was uninstalled and the loop reconnected, instead of the next caller
+        // being handed the same corpse.
+        withTimeout(TIMEOUT_MS) { attempts.receive() }
+        // That torn-down connection is a failed loop iteration and signals as one…
+        withTimeout(TIMEOUT_MS) { manager.connectionFailures.first() }
+        // …but the gate's own partial refresh must never reach the feed — a gate the user aborted
+        // mid-purchase is not a reconciliation outcome. This refresh confirmed nothing (only a
+        // payment in progress), so running it through the reconciliation path WOULD have signalled.
+        withTimeoutOrNull(QUIET_MS) { manager.connectionFailures.first() } shouldBe null
+    }
+
     // endregion
 
     // region pending payments