fix(upgrade): Cover widget entry refresh and recorder edge cases

- WidgetConfigurationActivity refreshes the entitlement on resume: it is a second launcher entry point and can't rely on MainActivity reconciling. - The upgrade-return callback re-asks decideConfirm() instead of trusting the upgrade activity's result code, so RESULT_OK stays entitlement-gated. - RecorderModule stops the freshly started recorder when the header's diagnostics reads are cancelled, instead of leaking an untracked recording. - FOSS beta channel points at the GitHub releases page; the Play testing URL is signature-incompatible for FOSS builds. - Billing bug reports carry the contextual wrapper again, so the report is grouped by call site instead of the raw billing exception. Fixes review findings F1, F2, F3, F4, F5.
2026-09-14 18:26:11 -04:00 · 2026-07-29 14:05:26 +02:00
parent 3651bb3d55
commit e364a5b02c
6 changed files with 133 additions and 12 deletions
@@ -88,7 +88,7 @@ class UpgradeRepoFoss @Inject constructor(
    companion object {
        private const val STORE_SITE = "https://github.com/d4rken-org/capod/releases"
        private const val UPGRADE_SITE = "https://github.com/sponsors/d4rken"
-        private const val BETA_SITE = "https://play.google.com/apps/testing/eu.darken.capod"
+        private const val BETA_SITE = "https://github.com/d4rken-org/capod/releases"
        private val TAG = logTag("Upgrade", "Foss", "Repo")
    }
 }
@@ -381,7 +381,7 @@ class BillingManager @Inject constructor(
    private fun reportPermanentAckFailure(purchase: Purchase, error: Exception) {
        if (reportedAckFailures.add(purchase.purchaseToken)) {
            log(TAG, ERROR) { "Permanent ack failure for ${purchase.redacted()}:\n${error.asLog()}" }
-            Bugs.report(TAG, "Failed to acknowledge purchase", error)
+            Bugs.report(TAG, "Failed to acknowledge purchase", RuntimeException("Failed to acknowledge purchase", error))
        } else {
            log(TAG, WARN) { "Permanent ack failure (already reported) for ${purchase.redacted()}" }
        }
@@ -461,10 +461,10 @@ class BillingManager @Inject constructor(
            )
            when {
                e !is BillingException -> {
-                    Bugs.report(TAG, "State exception for $sku, U", e)
+                    Bugs.report(TAG, "State exception for $sku, U", RuntimeException("State exception for $sku, U", e))
                }
                e is BillingClientException && !e.result.responseCode.let { ignoredCodes.contains(it) } -> {
-                    Bugs.report(TAG, "Client exception for $sku", e)
+                    Bugs.report(TAG, "Client exception for $sku", RuntimeException("Client exception for $sku", e))
                }
            }

@@ -89,7 +89,16 @@ class RecorderModule @Inject constructor(
                        if (!isResume) {
                            val startTime = timeSource.currentTimeMillis()
                            writeTriggerFile(sessionDir, startTime)
-                            logRecordingHeader()
+                            // The recorder is already live but not yet committed to the state: a
+                            // cancellation escaping the header would abandon it where stopRecorder()
+                            // can't reach it.
+                            try {
+                                logRecordingHeader()
+                            } catch (e: CancellationException) {
+                                newRecorder.stop()
+                                this@RecorderModule.currentLogDir = null
+                                throw e
+                            }

                            this@RecorderModule.currentLogDir = sessionDir

@@ -100,7 +109,13 @@ class RecorderModule @Inject constructor(
                                persistedLogDir = null,
                            )
                        } else {
-                            logRecordingHeader()
+                            try {
+                                logRecordingHeader()
+                            } catch (e: CancellationException) {
+                                newRecorder.stop()
+                                this@RecorderModule.currentLogDir = null
+                                throw e
+                            }

                            this@RecorderModule.currentLogDir = sessionDir

@@ -23,6 +23,7 @@ import eu.darken.capod.common.debug.logging.log
 import eu.darken.capod.common.debug.logging.logTag
 import eu.darken.capod.common.theming.CapodTheme
 import eu.darken.capod.common.uix.Activity2
+import eu.darken.capod.common.upgrade.UpgradeRepo
 import eu.darken.capod.main.ui.MainActivity
 import eu.darken.capod.main.core.GeneralSettings
 import eu.darken.capod.main.core.currentThemeState
@@ -37,6 +38,7 @@ class WidgetConfigurationActivity : Activity2() {
    private val vm: WidgetConfigurationViewModel by viewModels()

    @Inject lateinit var generalSettings: GeneralSettings
+    @Inject lateinit var upgradeRepo: UpgradeRepo
    @ApplicationContext @Inject lateinit var appContext: Context

    private var widgetId: Int = AppWidgetManager.INVALID_APPWIDGET_ID
@@ -48,14 +50,21 @@ class WidgetConfigurationActivity : Activity2() {
            log(TAG) { "Upgrade flow canceled or incomplete (resultCode=${result.resultCode})" }
            return@registerForActivityResult
        }
+        // Same gate as the confirm tap: the return path re-asks the ViewModel instead of trusting
+        // the upgrade activity's result code, so RESULT_OK is only ever set for an entitled, valid
+        // configuration.
        lifecycleScope.launch {
-            val currentState = vm.state.first()
-            if (!currentState.canConfirm) {
-                log(TAG) { "Upgrade completed, but widget config is not valid, staying in config" }
-                return@launch
+            when (vm.decideConfirm()) {
+                WidgetConfigurationViewModel.ConfirmOutcome.Confirmed -> {
+                    log(TAG) { "Upgrade completed, auto-confirming widget selection" }
+                    confirmSelection(vm.state.first().isAncWidget)
+                }
+
+                WidgetConfigurationViewModel.ConfirmOutcome.UpgradeRequired,
+                WidgetConfigurationViewModel.ConfirmOutcome.Invalid -> {
+                    log(TAG) { "Upgrade returned, but confirming is not possible, staying in config" }
+                }
            }
-            log(TAG) { "Upgrade completed, auto-confirming widget selection" }
-            confirmSelection(currentState.isAncWidget)
        }
    }

@@ -137,6 +146,17 @@ class WidgetConfigurationActivity : Activity2() {
        }
    }

+    override fun onResume() {
+        super.onResume()
+        // Per-resume, unthrottled entitlement reconciliation. This activity is a second launcher
+        // entry point (the widget picker starts it directly), so it can't rely on MainActivity
+        // having reconciled: it needs its own. refresh() is bounded and swallows its own failures.
+        lifecycleScope.launch {
+            log(TAG) { "onResume(): refreshing upgrade info" }
+            upgradeRepo.refresh()
+        }
+    }
+
    private fun confirmSelection(isAncWidget: Boolean) {
        vm.confirmSelection()

@@ -3,14 +3,20 @@ package eu.darken.capod.common.debug.recording.core
 import androidx.test.core.app.ApplicationProvider
 import eu.darken.capod.common.InstallId
 import eu.darken.capod.common.SystemTimeSource
+import eu.darken.capod.common.debug.logging.FileLogger
+import eu.darken.capod.common.debug.logging.Logging
 import eu.darken.capod.common.upgrade.UpgradeDiagnostics
 import eu.darken.capod.main.core.CurriculumVitae
+import io.kotest.matchers.nulls.shouldBeNull
 import io.kotest.matchers.nulls.shouldNotBeNull
 import io.kotest.matchers.shouldBe
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.mockk
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -83,6 +89,60 @@ class RecorderModuleDiagnosticsTest : BaseTest() {
        module.stopRecorder().shouldNotBeNull()
    }

+    /**
+     * Cancellation is the one thing the header reads deliberately rethrow, so it is the one failure
+     * that can abort the state update. The recorder is already live at that point: it has to be
+     * stopped on the way out, or it keeps writing into a session the module no longer tracks.
+     *
+     * The start is launched, not awaited: an aborted update never flips isRecording, so
+     * startRecorder() stays suspended. The virtual-time delay is what lets the module's own
+     * background collectors run to completion.
+     */
+    @Test
+    fun `a cancelled pro-history read stops the recorder instead of leaking it`() = runTest {
+        val fileLoggersBefore = Logging.loggers.filterIsInstance<FileLogger>()
+        val cv = mockk<CurriculumVitae>()
+        coEvery { cv.proHistory() } throws CancellationException("scope died mid-read")
+        val diagnostics = mockk<UpgradeDiagnostics>()
+        coEvery { diagnostics.debugInfo() } returns "BillingCache(...)"
+
+        val module = buildModule(backgroundScope, cv, diagnostics)
+
+        backgroundScope.launch { module.startRecorder() }
+        delay(1_000)
+
+        coVerify { cv.proHistory() }
+        module.state.first().isRecording shouldBe false
+        module.currentLogDir.shouldBeNull()
+        // The recorder that was already writing when the header aborted got stopped.
+        Logging.loggers.filterIsInstance<FileLogger>() shouldBe fileLoggersBefore
+    }
+
+    @Test
+    fun `a cancelled upgrade-diagnostics read stops the recorder instead of leaking it`() = runTest {
+        val fileLoggersBefore = Logging.loggers.filterIsInstance<FileLogger>()
+        val cv = mockk<CurriculumVitae>()
+        coEvery { cv.proHistory() } returns CurriculumVitae.ProHistory(
+            lastState = null,
+            graceEngagedCount = 0,
+            graceEngagedLast = null,
+            proLostCount = 0,
+            proLostLast = null,
+        )
+        val diagnostics = mockk<UpgradeDiagnostics>()
+        coEvery { diagnostics.debugInfo() } throws CancellationException("scope died mid-read")
+
+        val module = buildModule(backgroundScope, cv, diagnostics)
+
+        backgroundScope.launch { module.startRecorder() }
+        delay(1_000)
+
+        coVerify { diagnostics.debugInfo() }
+        module.state.first().isRecording shouldBe false
+        module.currentLogDir.shouldBeNull()
+        Logging.loggers.filterIsInstance<FileLogger>() shouldBe fileLoggersBefore
+    }
+
    @Test
    fun `both reads failing still leaves a tracked recording`() = runTest {
        val cv = mockk<CurriculumVitae>()
@@ -149,6 +149,32 @@ class WidgetConfigurationViewModelTest : BaseTest() {
        outcome.await() shouldBe WidgetConfigurationViewModel.ConfirmOutcome.UpgradeRequired
    }

+    /**
+     * The activity's upgrade-return callback re-asks [WidgetConfigurationViewModel.decideConfirm]
+     * instead of trusting the upgrade activity's result code, so both return-path outcomes are the
+     * same decision the confirm tap makes.
+     */
+    @Test
+    fun `returning from the upgrade flow confirms once the entitlement arrived`() = runVmTest {
+        upgradeInfoFlow.value = info(isPro = false)
+        val vm = createViewModel()
+        vm.decideConfirm() shouldBe WidgetConfigurationViewModel.ConfirmOutcome.UpgradeRequired
+
+        upgradeInfoFlow.value = info(isPro = true)
+
+        vm.decideConfirm() shouldBe WidgetConfigurationViewModel.ConfirmOutcome.Confirmed
+    }
+
+    @Test
+    fun `returning from the upgrade flow still free stays in the configuration`() = runVmTest {
+        upgradeInfoFlow.value = info(isPro = false)
+        val vm = createViewModel()
+        vm.decideConfirm() shouldBe WidgetConfigurationViewModel.ConfirmOutcome.UpgradeRequired
+
+        // The upgrade activity returned, but no entitlement materialized: never RESULT_OK.
+        vm.decideConfirm() shouldBe WidgetConfigurationViewModel.ConfirmOutcome.UpgradeRequired
+    }
+
    @Test
    fun `an invalid widget id is never confirmable`() = runVmTest {
        upgradeInfoFlow.value = info(isPro = true)