From f9b25ce4f8969ae76d95c6bb53cb84f81d8a847e Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 12:50:16 +1000 Subject: [PATCH 01/23] Pro: let a QA build force a successful status refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sessionProLoadingState=success` mapped to NORMAL, which only removes the override and defers to the real load state — and a process that has not confirmed a fetch reports Loading from launch, so `success` could never actually produce one. Adds a SUCCESS value that forces it, matching what iOS's equivalent mock already does. Reachable only from the debug menu and QaLaunchConfig, so it cannot be selected in a release build. It asserts a confirmed fetch that never happened, so it defeats anything gating on one — the KDoc says so, because a test whose subject is such a gate must not use it. --- .../securesms/debugmenu/DebugMenuViewModel.kt | 14 ++++++++++++++ .../thoughtcrime/securesms/pro/ProStatusManager.kt | 6 ++++++ .../thoughtcrime/securesms/qa/QaLaunchConfig.kt | 11 +++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/debugmenu/DebugMenuViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/debugmenu/DebugMenuViewModel.kt index 6cd4d63c24..13762dcca3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/debugmenu/DebugMenuViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/debugmenu/DebugMenuViewModel.kt @@ -126,6 +126,7 @@ class DebugMenuViewModel @AssistedInject constructor( DebugProPlanStatus.NORMAL, DebugProPlanStatus.LOADING, DebugProPlanStatus.ERROR, + DebugProPlanStatus.SUCCESS, ), selectedDebugProPlanStatus = textSecurePreferences.getDebugProPlanStatus() ?: DebugProPlanStatus.NORMAL, debugProPlans = subscriptionManagers.asSequence() @@ -685,9 +686,22 @@ class DebugMenuViewModel @AssistedInject constructor( } enum class DebugProPlanStatus(val label: String){ + /** No override — the real load state decides. Not the same as [SUCCESS]. */ NORMAL("Normal State"), LOADING("Always Loading"), ERROR("Always Erroring out"), + + /** + * Forces the refresh state to Success, which asserts that THIS PROCESS has had a fetch confirmed + * by the backend when it has not. That is a lie the app tells itself, so it is only reachable + * from the debug menu or `QaLaunchConfig` and cannot be selected in a shipping build. + * + * Distinct from [NORMAL] on purpose: NORMAL removes the override and defers to the real state, + * which on a cold start is `Init` and therefore Loading. Anything gating on a confirmed fetch — + * `HomeViewModel`'s expiring and expired CTAs — is *defeated* by this value, so do not use it in + * a test whose subject is one of those gates; it would pass whether the gate works or not. + */ + SUCCESS("Always Succeeding"), } sealed class Commands { diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index 31fa69702e..a308c7d99b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -125,6 +125,12 @@ class ProStatusManager @Inject constructor( val proDataRefreshState = when(debugProPlanStatus){ DebugMenuViewModel.DebugProPlanStatus.LOADING -> State.Loading DebugMenuViewModel.DebugProPlanStatus.ERROR -> State.Error(Exception()) + // QA override, debug/QA builds only. Asserts a confirmed fetch that has not happened, so + // it DEFEATS every consumer gating on one — see the enum's KDoc. It sits here, alongside + // the other overrides, rather than being folded into the real calculation below: the + // `when` on `proStatusState` stays exhaustive and keeps refusing to call `Init` or a + // persisted `Loaded` a success. + DebugMenuViewModel.DebugProPlanStatus.SUCCESS -> State.Success(Unit) else -> { // `Success` means THIS PROCESS has had a fetch confirmed by the backend, nothing // weaker: consumers gate on it to avoid acting on stale data, `HomeViewModel`'s diff --git a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt index 48a7accc0f..ecc668434f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt @@ -127,7 +127,14 @@ object QaLaunchConfig { /** * Load state of the Pro settings screen: `useActual` | `loading` | `error` | `success`. - * iOS's `mockCurrentUserSessionProLoadingState`. `success` maps to Android's `NORMAL`. + * iOS's `mockCurrentUserSessionProLoadingState`. + * + * `success` FORCES a successful refresh state, matching iOS's `.simulate(.success)`. It used to map to + * `NORMAL`, which only removed the override and deferred to the real state — and since a process that + * has not confirmed a fetch reports Loading from launch, `success` could not previously produce one. + * Note what it costs: a forced success asserts a confirmed fetch that never happened, so it defeats + * anything gating on one (`HomeViewModel`'s expiring/expired CTAs). Never use it in a test whose + * subject is one of those gates — see `DebugProPlanStatus.SUCCESS`. */ private const val EXTRA_PRO_LOADING_STATE = "sessionProLoadingState" @@ -487,7 +494,7 @@ object QaLaunchConfig { USE_ACTUAL -> null "loading" -> DebugMenuViewModel.DebugProPlanStatus.LOADING "error" -> DebugMenuViewModel.DebugProPlanStatus.ERROR - "success" -> DebugMenuViewModel.DebugProPlanStatus.NORMAL + "success" -> DebugMenuViewModel.DebugProPlanStatus.SUCCESS else -> { Log.e( TAG, From 3f6e4877c4b716380654e7fa0bc7fa3c841062ee Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 12:50:40 +1000 Subject: [PATCH 02/23] Pro: fetch on startup when a proof exists without an access expiry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The startup gate declined whenever there was no access expiry, treating that as never subscribed. A proof is entitlement while the expiry is only the payment horizon, and the two can come apart — a config that merged one without the other, or pruned history — leaving a client that knows it is Pro and nothing about when that ends. It now fetches in that state, matching iOS. No expiry AND no proof still declines, which is the minted-but-undiscovered grant: nothing local says the account is Pro, so there is nothing to warn about yet. `hasProof` is not defaulted because the failure mode of omitting it is a fetch that silently never happens. --- .../securesms/pro/ProStatusManager.kt | 53 +++++++++++++++--- .../securesms/pro/ProStartupGateTest.kt | 54 ++++++++++++------- 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index a308c7d99b..49e821d9cd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -426,6 +426,14 @@ class ProStatusManager @Inject constructor( * its own key — a routine refresh must not consume the gate's budget, and a startup fetch from * twenty hours ago must not satisfy the 60s floor. */ + /** The four config values [startupFetchReason] turns on, read together under one lock. */ + private data class StartupGateInputs( + val renewalDue: Instant?, + val autoRenewing: Boolean, + val grace: Duration, + val hasProof: Boolean, + ) + private fun startupGate(): Flow = flow { val now = snodeClock.currentTime() @@ -435,15 +443,24 @@ class ProStatusManager @Inject constructor( return@flow } - val (renewalDue, autoRenewing, grace) = configFactory.get().withUserConfigs { configs -> - Triple( - configs.userProfile.getProAccessExpiry()?.let(Instant::ofEpochSecond), - configs.userProfile.getProAutoRenewing(), - configs.userProfile.getProGracePeriod(), + // All four read under ONE config lock — the gate's inputs are cheap individually but the lock is + // not, so this deliberately does not read the proof in a second pass. + val inputs = configFactory.get().withUserConfigs { configs -> + StartupGateInputs( + renewalDue = configs.userProfile.getProAccessExpiry()?.let(Instant::ofEpochSecond), + autoRenewing = configs.userProfile.getProAutoRenewing(), + grace = configs.userProfile.getProGracePeriod(), + hasProof = configs.userProfile.getProConfig()?.proProof != null, ) } - val reason = startupFetchReason(renewalDue, autoRenewing, grace, now) + val reason = startupFetchReason( + renewalDue = inputs.renewalDue, + autoRenewing = inputs.autoRenewing, + grace = inputs.grace, + now = now, + hasProof = inputs.hasProof, + ) if (reason == null) { Log.d(DebugLogGroup.PRO_SUBSCRIPTION.label, "Startup gate: no CTA could fire, skipping the startup fetch") return@flow @@ -702,18 +719,38 @@ class ProStatusManager @Inject constructor( * | `auto_renewing && now >= renewalDue` | fetch — the renewal is overdue | * | `!auto_renewing && renewalDue` in the CTA window | fetch — the Expiring CTA may fire | * | `!auto_renewing && now >= renewalDue` | confirm before the Expired CTA | + * | no `renewalDue`, but a proof | fetch — entitled, horizon unknown | + * | no `renewalDue`, no proof | no fetch — never subscribed | * * No row tests `renewalDue + grace <= now`: a cold start does not need to know when coverage * ends, and testing it would double-count grace against the payment date the rows turn on. + * + * [hasProof] is deliberately not defaulted. It is the difference between a real subscriber who + * cannot be warned and an account we correctly leave alone, and the failure mode of forgetting it + * is a fetch that silently never happens — so every caller states it. */ internal fun startupFetchReason( renewalDue: Instant?, autoRenewing: Boolean, grace: Duration, now: Instant, + hasProof: Boolean, ): String? { - // No access expiry at all: never subscribed, so no CTA can fire and nothing to confirm. - if (renewalDue == null) return null + if (renewalDue == null) { + // A proof is entitlement; the access expiry is only the payment horizon, and the two can + // come apart — a config that merged one without the other, or pruned history. In that + // state the client positively knows it is Pro while knowing nothing about when that ends, + // so it is not the never-subscribed case this gate exists to protect and declining would + // leave a real subscriber unwarnable. Matches iOS's `expirySeconds > 0 || hasProof`. + // + // Both absent stays declined, which is the minted-but-undiscovered grant (no proof, no + // expiry): nothing local says the account is Pro, so there is nothing to warn about yet. + return if (hasProof) { + "a proof but no access expiry; entitled with no known horizon" + } else { + null + } + } val overdue = !now.isBefore(renewalDue) diff --git a/app/src/test/java/org/thoughtcrime/securesms/pro/ProStartupGateTest.kt b/app/src/test/java/org/thoughtcrime/securesms/pro/ProStartupGateTest.kt index 9c73e0a89a..124b42d976 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/pro/ProStartupGateTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/pro/ProStartupGateTest.kt @@ -31,49 +31,67 @@ class ProStartupGateTest { // --- never subscribed ----------------------------------------------------------------------- @Test - fun `no access expiry means no fetch`() { - // The population the gate exists for: no CTA can fire, so no fetch has a consumer. - assertNull(startupFetchReason(null, autoRenewing = false, grace = noGrace, now = now)) - assertNull(startupFetchReason(null, autoRenewing = true, grace = grace, now = now)) + fun `no access expiry and no proof means no fetch`() { + // The population the gate exists for: no CTA can fire, so no fetch has a consumer. This is also + // the minted-but-undiscovered grant — nothing local says the account is Pro — and it must keep + // declining, which is why the proof row below is a separate case rather than a relaxation. + assertNull(startupFetchReason(null, autoRenewing = false, grace = noGrace, now = now, hasProof = false)) + assertNull(startupFetchReason(null, autoRenewing = true, grace = grace, now = now, hasProof = false)) + } + + @Test + fun `a proof without an access expiry fetches`() { + // Entitlement without a horizon: the client knows it is Pro and nothing about when that ends, so + // the safe direction is to ask. Matches iOS's `expirySeconds > 0 || hasProof`. + assertNotNull(startupFetchReason(null, autoRenewing = false, grace = noGrace, now = now, hasProof = true)) + assertNotNull(startupFetchReason(null, autoRenewing = true, grace = grace, now = now, hasProof = true)) + } + + @Test + fun `a proof does not override the rows that turn on the expiry`() { + // The proof term is a fallback for a MISSING expiry, not an override of a present one. A holder + // comfortably mid-term still declines, or every Pro user would fetch on every cold start. + assertNull(startupFetchReason(inDays(20), autoRenewing = true, grace = grace, now = now, hasProof = true)) + assertNull(startupFetchReason(inDays(60), autoRenewing = false, grace = noGrace, now = now, hasProof = true)) } // --- auto-renewing -------------------------------------------------------------------------- @Test fun `auto-renewing and comfortably active does not fetch`() { - assertNull(startupFetchReason(inDays(20), autoRenewing = true, grace = grace, now = now)) + assertNull(startupFetchReason(inDays(20), autoRenewing = true, grace = grace, now = now, hasProof = false)) } @Test fun `auto-renewing and inside the grace window DOES fetch`() { // Renewal due 7 days ago, grace 14: still covered, charge not landed. The state this exists for. - assertNotNull(startupFetchReason(daysAgo(7), autoRenewing = true, grace = grace, now = now)) + assertNotNull(startupFetchReason(daysAgo(7), autoRenewing = true, grace = grace, now = now, hasProof = false)) } @Test fun `auto-renewing boundary - exactly at the renewal date fetches`() { - assertNotNull(startupFetchReason(now, autoRenewing = true, grace = grace, now = now)) + assertNotNull(startupFetchReason(now, autoRenewing = true, grace = grace, now = now, hasProof = false)) } @Test fun `auto-renewing boundary - one second before the renewal date does not fetch`() { // Negative control: without it, "inside grace fetches" also passes against a gate that always // fetches when auto-renewing. - assertNull(startupFetchReason(now.plusSeconds(1), autoRenewing = true, grace = grace, now = now)) + assertNull(startupFetchReason(now.plusSeconds(1), autoRenewing = true, grace = grace, now = now, hasProof = false)) } @Test fun `auto-renewing past coverage end still fetches`() { // Coverage ended 6 days ago: the renewal failed. Still fetches — this account is about to be // shown the Expired CTA, and config alone must never be the basis for that. - assertNotNull(startupFetchReason(daysAgo(20), autoRenewing = true, grace = grace, now = now)) + assertNotNull(startupFetchReason(daysAgo(20), autoRenewing = true, grace = grace, now = now, hasProof = false)) } @Test fun `auto-renewing and long dead does not fetch`() { // Coverage ended 46 days ago, past the CTA window. Unbounded, an account whose renewing flag // was never cleared fetches on every cold start forever. - assertNull(startupFetchReason(daysAgo(60), autoRenewing = true, grace = grace, now = now)) + assertNull(startupFetchReason(daysAgo(60), autoRenewing = true, grace = grace, now = now, hasProof = false)) } @Test @@ -81,32 +99,32 @@ class ProStartupGateTest { // Coverage ended 26 days ago, so the CTA can still fire. Discriminates the anchor: from the // payment date this reads as 40 days gone, past the window, and returns null. Only a grace // longer than the gap between the anchors tells them apart, hence the multi-day value. - assertNotNull(startupFetchReason(daysAgo(40), autoRenewing = true, grace = grace, now = now)) + assertNotNull(startupFetchReason(daysAgo(40), autoRenewing = true, grace = grace, now = now, hasProof = false)) } // --- not auto-renewing ---------------------------------------------------------------------- @Test fun `not auto-renewing and expiring inside the CTA window fetches`() { - assertNotNull(startupFetchReason(inDays(3), autoRenewing = false, grace = noGrace, now = now)) + assertNotNull(startupFetchReason(inDays(3), autoRenewing = false, grace = noGrace, now = now, hasProof = false)) } @Test fun `not auto-renewing boundary - just outside the 7 day window does not fetch`() { - assertNull(startupFetchReason(inDays(8), autoRenewing = false, grace = noGrace, now = now)) + assertNull(startupFetchReason(inDays(8), autoRenewing = false, grace = noGrace, now = now, hasProof = false)) } @Test fun `not auto-renewing and recently expired fetches to confirm before the Expired CTA`() { // Config can read expired while a renewal landed on another device and hasn't synced, so the // Expired CTA must never fire off config alone. - assertNotNull(startupFetchReason(daysAgo(5), autoRenewing = false, grace = noGrace, now = now)) + assertNotNull(startupFetchReason(daysAgo(5), autoRenewing = false, grace = noGrace, now = now, hasProof = false)) } @Test fun `not auto-renewing boundary - expired longer ago than the CTA window does not fetch`() { // Past 30 days the Expired CTA can no longer fire, so a confirming fetch has no consumer. - assertNull(startupFetchReason(daysAgo(31), autoRenewing = false, grace = noGrace, now = now)) + assertNull(startupFetchReason(daysAgo(31), autoRenewing = false, grace = noGrace, now = now, hasProof = false)) } @Test @@ -114,7 +132,7 @@ class ProStartupGateTest { // A prepaid or long non-renewing subscription: no CTA can fire. Unambiguous because the // proof-success path writes the renewing flag beside the expiry, so absent means not-renewing // rather than never-recorded. - assertNull(startupFetchReason(inDays(60), autoRenewing = false, grace = noGrace, now = now)) + assertNull(startupFetchReason(inDays(60), autoRenewing = false, grace = noGrace, now = now, hasProof = false)) } // --- grace's blast radius ------------------------------------------------------------------- @@ -124,7 +142,7 @@ class ProStartupGateTest { // Grace belongs to one row, the auto-renewing one. The guard against it being reintroduced // into the others: the wire sends grace = 0 when not auto-renewing, so anything keyed to a // non-zero grace on this path only ever fires on a fixture. - assertNull(startupFetchReason(daysAgo(31), autoRenewing = false, grace = grace, now = now)) - assertNotNull(startupFetchReason(inDays(3), autoRenewing = false, grace = grace, now = now)) + assertNull(startupFetchReason(daysAgo(31), autoRenewing = false, grace = grace, now = now, hasProof = false)) + assertNotNull(startupFetchReason(inDays(3), autoRenewing = false, grace = grace, now = now, hasProof = false)) } } From 7c911108b77a8a11f783128366639c6eb38c85e9 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 12:50:51 +1000 Subject: [PATCH 03/23] Pro: only refresh status when the synced access expiry actually changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `distinctUntilChanged` has no baseline for its first emission, so the first projection of config in a new process — config the app already had — was treated as a change and scheduled a get_pro_status fetch on every cold launch. That ran one second after the startup gate had declined and regardless of what it decided, so a never-subscribed account fetched on every start: exactly the traffic the gate exists to remove, invisible to it because it is a different trigger. iOS guards the same trigger the same way. A change arriving as the very first emission is swallowed, which is why this must not be paired with anything that also removes a restored account's only path to a status fetch. --- .../securesms/pro/ProStatusManager.kt | 24 ++++- .../pro/ProConfigChangeTriggerTest.kt | 101 ++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index 49e821d9cd..507178ea79 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -17,6 +17,7 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flow @@ -359,7 +360,7 @@ class ProStatusManager @Inject constructor( configs.userProfile.getProPrepaid() } } - .distinctUntilChanged() + .changesOnly() .map { "ProAccessExpiry/prepaid in config changes" }, proStatusRepository.get().loadState @@ -700,6 +701,27 @@ class ProStatusManager @Inject constructor( } companion object { + + /** + * Emits only GENUINE changes: distinct values, with the first one dropped. + * + * The drop is the point. `distinctUntilChanged` is per-collection and has no baseline for its + * first emission, so it always passes — and the first emission of a config projection in a new + * process is config the app *already had*, not a change. Without the drop, the + * access-expiry/prepaid trigger schedules a fetch on **every cold launch**, one second after the + * startup gate has just declined and regardless of what the gate decided. A never-subscribed + * account then makes a `get_pro_status` request on every start — exactly the traffic the gate + * exists to remove, invisible to it because it is a different trigger. + * + * Matches iOS's `hasProjectedUserConfig` guard (`SessionProManager.swift:550-563`), whose comment + * names the same failure. The first emission still establishes the baseline, so a value that + * changes afterwards — an `E` or prepaid marker synced from another device, which is the case this + * trigger exists for — is emitted normally. + * + * Consequence worth knowing: a change arriving as the very FIRST emission is swallowed. On a + * brand-new account that is nothing (no Pro to lose), and it is the same trade iOS makes. + */ + internal fun Flow.changesOnly(): Flow = distinctUntilChanged().drop(1) /** * How long after a wake instant to fetch. The backend judges against its own clock, so a wake * landing exactly on the boundary can read the pre-crossing state. diff --git a/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt new file mode 100644 index 0000000000..c706573d55 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt @@ -0,0 +1,101 @@ +package org.thoughtcrime.securesms.pro + +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test +import org.thoughtcrime.securesms.pro.ProStatusManager.Companion.changesOnly +import java.time.Instant + +/** + * Covers the guard on the access-expiry/prepaid config trigger. + * + * The bug it prevents: `distinctUntilChanged` is per-collection and has no baseline for its first + * emission, so a projection of config the app ALREADY had was treated as a change and scheduled a + * `get_pro_status` fetch on every cold launch — one second after the startup gate had declined, and + * invisible to that gate because it is a different trigger. A never-subscribed account fetched on every + * start. + * + * The two cases that matter are opposite risks, so both are asserted: the first projection must NOT fetch, + * and a genuine later change — an `E` or prepaid marker synced from another device, which is the case this + * trigger exists for — MUST still fetch. + */ +class ProConfigChangeTriggerTest { + + private val e1: Instant = Instant.parse("2026-08-15T20:23:28Z") + private val e2: Instant = Instant.parse("2026-09-15T20:23:28Z") + + /** The projected pair the real flow carries: (access expiry, prepaid marker). */ + private fun projection(expiry: Instant?, prepaid: Long?) = expiry to prepaid + + @Test + fun `the first projection does not fetch`() = runTest { + // A cold launch on an account whose expiry is already in config. Nothing changed, so nothing + // should be scheduled — this is the whole point of the guard. + val emitted = flowOf(projection(e1, null)).changesOnly().toList() + + assertEquals(emptyList>(), emitted) + } + + @Test + fun `a repeated projection does not fetch`() = runTest { + // Unrelated profile edits re-emit the config flow with the same two values. + val emitted = flowOf( + projection(e1, null), + projection(e1, null), + projection(e1, null), + ).changesOnly().toList() + + assertEquals(emptyList>(), emitted) + } + + @Test + fun `an expiry synced from another device after the first projection does fetch`() = runTest { + // The case the trigger exists for. The first emission establishes the baseline rather than being + // acted on, so the real change behind it still gets through. + val emitted = flowOf( + projection(e1, null), + projection(e2, null), + ).changesOnly().toList() + + assertEquals(listOf(projection(e2, null)), emitted) + } + + @Test + fun `a prepaid marker synced from another device does fetch`() = runTest { + // Prepaid moves independently of the expiry: another device purchased and the entitlement has to + // be pulled through even if that device goes offline before redeeming. + val emitted = flowOf( + projection(e1, null), + projection(e1, 1_760_000_000L), + ).changesOnly().toList() + + assertEquals(listOf(projection(e1, 1_760_000_000L)), emitted) + } + + @Test + fun `only the changes are emitted, not the noise between them`() = runTest { + val emitted = flowOf( + projection(e1, null), // first projection — baseline + projection(e1, null), // unrelated profile edit + projection(e2, null), // real change + projection(e2, null), // unrelated profile edit + projection(e2, 42L), // real change + ).changesOnly().toList() + + assertEquals(listOf(projection(e2, null), projection(e2, 42L)), emitted) + } + + @Test + fun `a change arriving as the very first emission is swallowed`() = runTest { + // Documenting the accepted cost rather than pretending it away: with no baseline there is no way + // to tell a change from a projection, so the first emission is always treated as a projection. + // Harmless on a brand-new account, which has no Pro to lose, and it is the same trade iOS makes + // with `hasProjectedUserConfig`. If a real transition is ever seen to land as the first emission + // on an EXISTING account, this test is the thing to revisit. + val emitted = flowOf(projection(e2, null)).changesOnly().toList() + + assertEquals(emptyList>(), emitted) + } +} From 7eec3e8c4645d817dba6d926c6dcb356aec5d429 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 12:51:04 +1000 Subject: [PATCH 04/23] Pro: don't warn about an expiry the backend has not confirmed The expiring CTA had no confirmed-fetch guard while the expired branch directly below it did, so someone who had already renewed could be told their access expires in two days on launch, before any fetch confirmed otherwise. Status is read from a cached response, and on a restored or foregrounded client that cache can predate the renewal. Applies the same condition the expired branch uses; iOS gates both variants above the equivalent switch. Cost: an offline subscriber who is genuinely expiring is no longer warned, since nothing can confirm it. That is the same trade the expired branch already made. --- .../java/org/thoughtcrime/securesms/home/HomeViewModel.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt index ec1594eed7..16c627a671 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt @@ -233,6 +233,14 @@ class HomeViewModel @Inject constructor( (prefs.hasSeenProExpiring() || prefs.hasSeenProExpired())){ prefs.clearProExpiryView() // reset expiry view if the user is active again } else if(subscription.type is ProStatus.Active.Expiring + // Same confirmed-fetch gate as the Expired branch below; both read the one + // `refreshState` predicate, so tightening or loosening it moves both CTAs. + // Success means THIS process has had a fetch confirmed, so a cold launch cannot + // warn off a stale local proof: `status` is inferred from that proof at launch, and + // nothing writes to config at renewal, so a single-device account that renewed + // while the app was closed reads as expiring until get_pro_status says otherwise. + // Consistent with the iOS fix, which gates both variants above the switch. + && subscription.refreshState is org.thoughtcrime.securesms.util.State.Success && !prefs.hasSeenProExpiring() ){ val validUntil = subscription.type.renewingAt From 131625bf8b3dc845d7ea1cb75232c66374e53f2a Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 12:51:14 +1000 Subject: [PATCH 05/23] Pro: don't refresh status when the settings list opens Opening the settings list fetched get_pro_status unconditionally, including for an account with no expiry and no proof, which has nothing to refresh. It was also ungated, so on a fresh process it consumed the one unfloored attempt before anything that needed it could. The row renders from cached state and the refresh that matters happens on entering the Pro screen, where the user is actually looking at Pro data. Do not merge this without the restored-account status refresh: it removes the fetch that was incidentally covering that case, so alone it leaves a restored subscriber seeing "Upgrade Session" with no path to correction. --- .../securesms/preferences/SettingsViewModel.kt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt index 984fe48edf..7994d802cd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt @@ -59,7 +59,6 @@ import org.thoughtcrime.securesms.database.RecipientRepository import org.thoughtcrime.securesms.dependencies.ConfigFactory import org.thoughtcrime.securesms.mms.MediaConstraints import org.thoughtcrime.securesms.pro.ProDataState -import org.thoughtcrime.securesms.pro.ProStatusRepository import org.thoughtcrime.securesms.pro.ProStatus import org.thoughtcrime.securesms.pro.ProStatusManager import org.thoughtcrime.securesms.pro.getDefaultSubscriptionStateData @@ -93,7 +92,6 @@ class SettingsViewModel @Inject constructor( private val inAppReviewManager: InAppReviewManager, private val avatarUploadManager: AvatarUploadManager, private val attachmentProcessor: AttachmentProcessor, - private val proStatusRepository: ProStatusRepository, private val donationManager: DonationManager, private val pathManager: PathManager, private val swarmApiExecutor: SwarmApiExecutor, @@ -165,10 +163,13 @@ class SettingsViewModel @Inject constructor( } } - // refreshes the pro status data - viewModelScope.launch { - proStatusRepository.requestRefresh() - } + // No status refresh here on purpose. Opening the settings LIST is not a reason to ask the + // backend: this screen renders the Pro row from cached state, and the refresh that matters + // happens on entering the PRO settings screen, where the user is actually looking at Pro data. + // + // Refreshing here fetched for every user who opened settings — including one with no expiry and + // no proof, who has nothing to refresh — and did so unconditionally, so on a fresh process it + // also consumed the one unfloored attempt before anything that needed it could. } private fun getVersionNumber(): CharSequence { From f476fa541cedcf8c9bff0dcc090494db2e6f744e Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 12:51:24 +1000 Subject: [PATCH 06/23] Add an accessibility id to the Pro settings row's title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row's existing id is on the tap target, which carries no text of its own, so the words that say which state the account is in — "Session Pro Beta" / "Upgrade Session" / "Renew Pro Beta" — were unreachable to a test. Adds `pro-menu-item-title`, agreed with iOS so one helper reads the row on both platforms without branching. ItemButton gains an optional `textQaTag` mirroring the `subtitleQaTag` it already had; it defaults to null, so the other 29 call sites are unchanged. --- .../securesms/preferences/SettingsScreen.kt | 3 +++ .../java/org/thoughtcrime/securesms/ui/Components.kt | 11 ++++++++++- content-descriptions/src/main/res/values/strings.xml | 4 ++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsScreen.kt index 79c5c05a40..9a73736ae8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsScreen.kt @@ -608,6 +608,9 @@ fun Buttons( ) }, modifier = Modifier.qaTag(R.string.qa_settings_item_pro), + // The row id above is the tap target and carries no text; this one is on the + // label, so a test can read WHICH of the three states the row is showing. + textQaTag = R.string.qa_settings_item_pro_title, colors = accentTextButtonColors() ) { activity?.push() diff --git a/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt b/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt index c4ea1f821b..152d8186ee 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt @@ -343,6 +343,13 @@ fun ItemButton( endIcon: @Composable (BoxScope.() -> Unit)? = null, subtitle: String? = null, @StringRes subtitleQaTag: Int? = null, + /** + * QA id for the button's own label. Off by default, like [subtitleQaTag] — most buttons are addressed + * by the id on the button itself. Pass one where a test needs to read the LABEL rather than find the + * button: the row's id sits on the tap target, which carries no text, so the words that say which + * state the item is in are only reachable through this. + */ + @StringRes textQaTag: Int? = null, enabled: Boolean = true, minHeight: Dp = LocalDimensions.current.minItemButtonHeight, textStyle: TextStyle = LocalType.current.h8, @@ -378,7 +385,9 @@ fun ItemButton( ) { Text( text, - Modifier.fillMaxWidth(), + Modifier + .fillMaxWidth() + .qaTag(textQaTag), style = textStyle ) diff --git a/content-descriptions/src/main/res/values/strings.xml b/content-descriptions/src/main/res/values/strings.xml index 6856e1861e..8242a0283b 100644 --- a/content-descriptions/src/main/res/values/strings.xml +++ b/content-descriptions/src/main/res/values/strings.xml @@ -256,6 +256,10 @@ donate-menu-item path-menu-item pro-menu-item + + pro-menu-item-title enjoy-session-positive-button From 28c77359e2ed5822b47d9fbd243077377af5787b Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:17:08 +1000 Subject: [PATCH 07/23] Pro: derive our own ACCESS from the proof, and read it where it is enforced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACCESS ("what may this device do") now comes from the proof for ourselves exactly as for anyone else, so it runs through the expiry and revocation filter on every resolve. Removes the `isSelf && response is Active` short-circuit in resolveProStatus: that response is DISPLAY, it is not revocation-filtered, and trusting it for access let a cached Active outlive a revocation we had already been told about. Adds one ACCESS function so a second opinion about our own Pro-ness cannot drift from the first, and routes the send path through it. Attaching a proof when sending is an access decision, and it previously honoured expiry but not revocation, so a revoked proof still went out on the wire. Splits reading it by what the reader does: rendering subscribes to an observed value recomputed at the existing change sources (config, revocation, proof expiry). InputbarViewModel's `by lazy` snapshot could not be demoted by a revocation landing while the screen was open; it is now a StateFlow over observeSelf(). Not a per-call read because onTextChanged runs per keystroke and getRecipientSync takes the config lock. enforcement calls the ACCESS function directly and unmemoized — the send path, the compose limit, the pinned-conversation gate. Those grant rather than draw, so they must not inherit a decision from render state. Also renames `changesOnly` to `dropFirstProjection` for one concept name across the three clients, and records in its KDoc that this guard's lifetime is the account session rather than the process — a restore re-arms it, which is the buggy side of that divergence and must not be ported to iOS to make the two match. --- .../sending_receiving/MessageSender.kt | 12 ++- .../securesms/InputbarViewModel.kt | 68 ++++++++++--- .../securesms/database/RecipientRepository.kt | 19 ++-- .../securesms/home/HomeViewModel.kt | 5 +- .../securesms/pro/ProStatusManager.kt | 97 ++++++++++++++++--- .../v2/ConversationViewModelTest.kt | 5 + .../pro/ProConfigChangeTriggerTest.kt | 14 +-- 7 files changed, 173 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt b/app/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt index 7d294e4b9c..958cfbc39f 100644 --- a/app/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt +++ b/app/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt @@ -52,6 +52,7 @@ import org.thoughtcrime.securesms.api.swarm.execute import org.thoughtcrime.securesms.auth.LoginStateRepository import org.thoughtcrime.securesms.database.RecipientRepository import org.thoughtcrime.securesms.dependencies.ManagerScope +import org.thoughtcrime.securesms.pro.ProStatusManager import org.thoughtcrime.securesms.pro.copyFromLibSession import org.thoughtcrime.securesms.service.ExpiringMessageManager import javax.inject.Inject @@ -78,6 +79,7 @@ class MessageSender @Inject constructor( @param:ManagerScope private val scope: CoroutineScope, private val loginStateRepository: LoginStateRepository, private val jobQueue: Provider, + private val proStatusManager: Provider, ) { // Error @@ -137,9 +139,13 @@ class MessageSender @Inject constructor( msg.toProto(builder, messageDataProvider) - // Attach pro proof - val proProof = configFactory.withUserConfigs { it.userProfile.getProConfig() }?.proProof - if (proProof != null && proProof.expirySeconds > snodeClock.currentTimeMillis() / 1000) { + // Attach pro proof. + // + // Routed through the one ACCESS function rather than checking expiry here: what we attach + // when sending IS an access decision, and it previously honoured expiry but NOT revocation, + // so a revoked proof we already knew about still went out on the wire. + val proProof = proStatusManager.get().currentUserProProofForAccess() + if (proProof != null) { builder.proMessageBuilder.proofBuilder.copyFromLibSession(proProof) } else { // If we don't have any valid pro proof, clear the pro message diff --git a/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt index 6df3180bda..fcaf05133e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt @@ -2,9 +2,13 @@ package org.thoughtcrime.securesms import android.content.Context import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope import org.session.libsession.utilities.Phrase import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import network.loki.messenger.R import org.session.libsession.utilities.StringSubstitutionConstants.LIMIT_KEY @@ -28,13 +32,43 @@ abstract class InputbarViewModel( private val _inputBarStateDialogsState = MutableStateFlow(InputBarDialogsState()) val inputBarStateDialogsState: StateFlow = _inputBarStateDialogsState - private val currentUser by lazy { recipientRepository.getSelf() } + /** + * ACCESS ("what may this device do") for the character limit — which gates SENDING, not just what + * the composer displays. + * + * Deliberately NOT `by lazy`. ACCESS is validated against proof expiry and the cached revocation + * list on every resolve, and a value captured once at first use is only ever validated once: a + * revocation landing while this screen is open could not demote us until the ViewModel was + * recreated. `observeSelf()`'s change sources include the revocation notification and a timer armed + * at the earliest proof expiry, so this stays current for the life of the screen. + * + * A `StateFlow` rather than a `getSelf()` call per use because [onTextChanged] runs on every + * keystroke and `getSelf()` is an uncached fetch that takes the config lock. + */ + private val isSelfPro: StateFlow = recipientRepository.observeSelf() + .map { it.isPro } + .stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + // Seeded synchronously so the very first keystroke is already correct rather than briefly + // reading as non-Pro. Guarded because `getSelf()` throws when not logged in. + initialValue = runCatching { recipientRepository.getSelf().isPro }.getOrDefault(false), + ) + + /** + * The composed length, kept so [validateMessageLength] can recompute against a FRESH access read + * instead of trusting the limit that was in force when the indicator was last drawn. + * + * A length is not an access decision, so caching it is safe; caching the limit is not. + */ + private var composedCodePointCount: Int = 0 fun onTextChanged(text: CharSequence) { - // check the character limit - val maxChars = proStatusManager.getCharacterLimit(currentUser.isPro) + // RENDERING: the observed value. This runs per keystroke, so it must not take the config lock. + val maxChars = proStatusManager.getCharacterLimit(isSelfPro.value) val message = text.toString() - val charsLeft = maxChars - message.codePointCount(0, message.length) + composedCodePointCount = message.codePointCount(0, message.length) + val charsLeft = maxChars - composedCodePointCount // update the char limit state based on characters left val charLimitState = if(charsLeft <= CHARACTER_LIMIT_THRESHOLD){ @@ -42,7 +76,7 @@ abstract class InputbarViewModel( count = charsLeft, countFormatted = NumberUtil.getFormattedNumber(charsLeft.toLong()), danger = charsLeft < 0, - showProBadge = !currentUser.isPro // only show the badge for non pro users + showProBadge = !isSelfPro.value // only show the badge for non pro users ) } else { null @@ -51,12 +85,22 @@ abstract class InputbarViewModel( _inputBarState.update { it.copy(charLimitState = charLimitState) } } + /** + * ENFORCEMENT, so it calls the ACCESS function directly and unmemoized rather than reading the + * observed value or the indicator's cached count. + * + * This gates SENDING, which makes it a grant rather than a draw: a proof that expired or was revoked + * since the indicator was last drawn must refuse here, and it cannot if the decision is inherited + * from render state. Recomputed from [composedCodePointCount] for the same reason — the stored + * `charLimitState.count` was computed against whatever limit was in force at the last keystroke. + */ fun validateMessageLength(): Boolean { - // the message is too long if we have a negative char left in the input state - val charsLeft = _inputBarState.value.charLimitState?.count ?: 0 + val hasProAccess = proStatusManager.currentUserHasProAccess() + val charsLeft = proStatusManager.getCharacterLimit(hasProAccess) - composedCodePointCount + return if(charsLeft < 0){ // the user is trying to send a message that is too long - we should display a dialog - if(currentUser.isPro){ + if(hasProAccess){ showMessageTooLongSendDialog() } else { showSessionProCTA() @@ -69,7 +113,7 @@ abstract class InputbarViewModel( } fun onCharLimitTapped(){ - if(currentUser.isPro){ + if(isSelfPro.value){ handleCharLimitTappedForProUser() } else { handleCharLimitTappedForRegularUser() @@ -103,7 +147,7 @@ abstract class InputbarViewModel( message = context.resources.getQuantityString( R.plurals.modalMessageCharacterDisplayDescription, charsLeft, // quantity for plural - proStatusManager.getCharacterLimit(currentUser.isPro), // 1st arg: total character limit + proStatusManager.getCharacterLimit(isSelfPro.value), // 1st arg: total character limit charsLeft, // 2nd arg: chars left ), positiveStyleDanger = false, @@ -121,7 +165,7 @@ abstract class InputbarViewModel( showSimpleDialog = SimpleDialogData( title = context.getString(R.string.modalMessageTooLongTitle), message = Phrase.from(context.getString(R.string.modalMessageCharacterTooLongDescription)) - .put(LIMIT_KEY, proStatusManager.getCharacterLimit(currentUser.isPro)) + .put(LIMIT_KEY, proStatusManager.getCharacterLimit(isSelfPro.value)) .format(), positiveStyleDanger = false, positiveText = context.getString(R.string.okay), @@ -137,7 +181,7 @@ abstract class InputbarViewModel( showSimpleDialog = SimpleDialogData( title = context.getString(R.string.modalMessageTooLongTitle), message = Phrase.from(context.getString(R.string.modalMessageTooLongDescription)) - .put(LIMIT_KEY, proStatusManager.getCharacterLimit(currentUser.isPro)) + .put(LIMIT_KEY, proStatusManager.getCharacterLimit(isSelfPro.value)) .format(), positiveStyleDanger = false, positiveText = context.getString(R.string.okay), diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt index 151a403aa0..41538c1385 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt @@ -502,15 +502,18 @@ class RecipientRepository @Inject constructor( it.isExpired(now) || proDatabase.isRevoked(it.revocationTag, snodeClock.get().currentTime()) } - // 2. Determine base Pro Data from valid proofs or ProStatusManager + // 2. Determine base Pro Data from valid proofs + // + // ACCESS ("what may this device do") comes from the PROOF, for ourselves exactly as for anyone + // else — so it runs through the expiry and revocation filter above on every resolve. There is + // deliberately no `isSelf` short-circuit on the cached `get_pro_status` response here: that + // response is DISPLAY ("what state is the plan in"), it is not revocation-filtered, and trusting + // it for access let a cached `Active` outlive a revocation we had already been told about. + // + // The two are MEANT to disagree — a proof that outlives an expired status still grants the + // feature, which is the deliberate overhang. Read `ProStatusManager.proDataState` when you want + // to describe the plan; read this when you want to know what is permitted. var proData = when { - // For ourselves, we "trust" ProStatusManager more than the ProProofs - recipient.isSelf && proStatusManager.get().proDataState.value.type is ProStatus.Active -> { - RecipientData.ProData( - showProBadge = proStatusManager.get().proDataState.value.showProBadge - ) - } - !proDataList.isNullOrEmpty() -> { RecipientData.ProData(showProBadge = proDataList.any { it.showProBadge }) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt index 16c627a671..9f02cca6a3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt @@ -357,8 +357,11 @@ class HomeViewModel @Inject constructor( fun setPinned(address: Address, pinned: Boolean) { // check the pin limit before continuing val totalPins = storage.getTotalPinned() + // ENFORCEMENT: the ACCESS function, called at the moment of the decision. Was a full + // `getSelf().isPro` resolve, which reaches the same answer but by a second route — one function + // means a revocation or expiry cannot be honoured here and missed on the send path. val maxPins = - proStatusManager.getPinnedConversationLimit(recipientRepository.getSelf().isPro) + proStatusManager.getPinnedConversationLimit(proStatusManager.currentUserHasProAccess()) if (pinned && totalPins >= maxPins) { // the user has reached the pin limit, show the CTA _dialogsState.update { diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index 507178ea79..c724406121 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -35,6 +35,7 @@ import network.loki.messenger.libsession_util.pro.BackendRequests import network.loki.messenger.libsession_util.pro.BackendRequests.PAYMENT_PROVIDER_APP_STORE import network.loki.messenger.libsession_util.pro.BackendRequests.PAYMENT_PROVIDER_GOOGLE_PLAY import network.loki.messenger.libsession_util.pro.ProConfig +import network.loki.messenger.libsession_util.pro.ProProof import network.loki.messenger.libsession_util.pro.ProResponseStatus import network.loki.messenger.libsession_util.protocol.ProFeature import network.loki.messenger.libsession_util.protocol.ProMessageFeature @@ -360,7 +361,7 @@ class ProStatusManager @Inject constructor( configs.userProfile.getProPrepaid() } } - .changesOnly() + .dropFirstProjection() .map { "ProAccessExpiry/prepaid in config changes" }, proStatusRepository.get().loadState @@ -568,6 +569,50 @@ class ProStatusManager @Inject constructor( } } + /** + * ACCESS: the Pro proof this device may currently act on, or `null` if there is none. + * + * This is the single place that answers "what may this device do", so that a second opinion about + * our own Pro-ness cannot drift from this one. It validates on EVERY call rather than caching: + * + * - expiry, against the network clock rather than the device clock, and + * - the cached revocation list, honouring each entry's effective timestamp. + * + * Do NOT answer this from `proDataState` / `get_pro_status`. That is DISPLAY ("what state is the + * plan in"), it is not revocation-filtered, and a cached `Active` response can outlive a revocation + * we have already been told about. The two are meant to disagree: a still-valid proof under an + * expired status keeps the features, which is the deliberate overhang. + */ + fun currentUserProProofForAccess(): ProProof? { + val proof = configFactory.get() + .withUserConfigs { it.userProfile.getProConfig() } + ?.proProof + ?: return null + + val now = snodeClock.currentTime() + if (proof.expirySeconds <= now.epochSecond) return null + if (proDatabase.isRevoked(proof.revocationTagHex, now)) return null + + return proof + } + + /** + * ACCESS as a boolean, for ENFORCEMENT sites that grant or refuse rather than draw. + * + * Call this UNMEMOIZED at the moment of the decision — the send path, the compose limit, the pinned + * conversation gate. Rendering does not use this: a render site subscribes to an observed value + * recomputed at the existing change sites (config change, revocation update, proof expiry), because + * some of them redraw per keystroke and this takes the config lock. + * + * Honours the QA force-grant deliberately. Without it, rendering (which resolves through + * `RecipientRepository`, where the override lives) and enforcement would disagree under a fixture: + * the composer would offer the Pro limit and sending would then refuse it. Note the force grants + * ACCESS but conjures no PROOF, so a forced client still attaches nothing when it sends — which is + * the truncation state, and is correct rather than a gap here. + */ + fun currentUserHasProAccess(): Boolean = + prefs.forceCurrentUserAsPro() || currentUserProProofForAccess() != null + /** * Logic to determine if we should animate the avatar for a user or freeze it on the first frame */ @@ -703,25 +748,45 @@ class ProStatusManager @Inject constructor( companion object { /** - * Emits only GENUINE changes: distinct values, with the first one dropped. + * Emits only GENUINE changes: distinct values, with the FIRST PROJECTION dropped. + * + * A relaunching subscriber loads its access expiry from a dump on every launch. Treating that + * first projection as a change would fetch on every cold launch and defeat the startup gate. + * + * (That sentence is deliberately identical on iOS — `isFirstProjection`, + * `SessionProManager.swift:550-563` — and on Desktop. The shared concept is FIRST PROJECTION; + * each client spells it to fit its language, and this one is an operator because there is a Flow + * to decorate.) + * + * The mechanics, since the sentence above does not give them: `distinctUntilChanged` is + * per-collection and has no baseline for its first emission, so it always passes. Without the + * drop, the access-expiry/prepaid trigger schedules a fetch one second after the startup gate has + * just declined and regardless of what the gate decided, so a never-subscribed account calls + * `get_pro_status` on every start — exactly the traffic the gate exists to remove, invisible to it + * because it is a different trigger. + * + * ⚠️ LIFETIME, AND THIS SIDE OF IT IS THE BUGGY ONE. This guard is scoped to the ACCOUNT SESSION, + * not the process: a restore or an account load re-arms it. The collector lives under + * [doWhileLoggedIn], which `AuthAwareComponentsHandler` drives with `collectLatest`, so a new + * `LoggedInState` tears it down and rebuilds it — and `drop(1)` starts again. * - * The drop is the point. `distinctUntilChanged` is per-collection and has no baseline for its - * first emission, so it always passes — and the first emission of a config projection in a new - * process is config the app *already had*, not a change. Without the drop, the - * access-expiry/prepaid trigger schedules a fetch on **every cold launch**, one second after the - * startup gate has just declined and regardless of what the gate decided. A never-subscribed - * account then makes a `get_pro_status` request on every start — exactly the traffic the gate - * exists to remove, invisible to it because it is a different trigger. + * So on a restore the post-restore projection is STRUCTURALLY the dropped one — always, not + * unluckily — and this guard cannot be the thing that discovers a restored subscriber. The + * config-change trigger is the only thing that fetches after a restore, so what a restored + * subscriber actually gets today is: no status until the next cold launch, or until they happen to + * open Pro settings, while the proof quietly grants the features. * - * Matches iOS's `hasProjectedUserConfig` guard (`SessionProManager.swift:550-563`), whose comment - * names the same failure. The first emission still establishes the baseline, so a value that - * changes afterwards — an `E` or prepaid marker synced from another device, which is the case this - * trigger exists for — is emitted normally. + * Do NOT "fix" that by removing the drop — that reinstates the every-cold-launch fetch. The fix + * under discussion is a separate trigger asking the SEMANTIC question ("is this config news we have + * never acted on") instead of the positional one, which is unruled and deliberately not built here. * - * Consequence worth knowing: a change arriving as the very FIRST emission is swallowed. On a - * brand-new account that is nothing (no Pro to lose), and it is the same trade iOS makes. + * iOS's equivalent flag is per-PROCESS and set eagerly at app setup, so it is already true by the + * time a restore lands and iOS therefore fetches. That is the CORRECT outcome, reached by + * construction order rather than by design — do not port this file's lifetime to iOS to make the + * two match, which would regress iOS onto the behaviour described above. Same concept, same + * comment, different lifetime, and only one of them is right. */ - internal fun Flow.changesOnly(): Flow = distinctUntilChanged().drop(1) + internal fun Flow.dropFirstProjection(): Flow = distinctUntilChanged().drop(1) /** * How long after a wake instant to fetch. The backend judges against its own clock, so a wake * landing exactly on the boundary can read the pre-crossing state. diff --git a/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt index 4429ac3165..a087d2f71e 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt @@ -128,6 +128,11 @@ class ConversationViewModelTest : BaseViewModelTest() { on { observeRecipient(recipient.address) } doAnswer { flowOf(recipient) } + // `InputbarViewModel` observes self for the character limit, which is an ACCESS decision + // and so must be read live rather than snapshotted. Stubbed here because the ViewModel + // subscribes during construction — an unstubbed `observeSelf()` returns a null Flow and + // every test in this class fails in ``. + on { observeSelf() } doAnswer { flowOf(recipient) } }, attachmentDownloadHandlerFactory = mock(), recipientSettingsDatabase = mock { diff --git a/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt index c706573d55..5252fff275 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/pro/ProConfigChangeTriggerTest.kt @@ -5,7 +5,7 @@ import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Test -import org.thoughtcrime.securesms.pro.ProStatusManager.Companion.changesOnly +import org.thoughtcrime.securesms.pro.ProStatusManager.Companion.dropFirstProjection import java.time.Instant /** @@ -33,7 +33,7 @@ class ProConfigChangeTriggerTest { fun `the first projection does not fetch`() = runTest { // A cold launch on an account whose expiry is already in config. Nothing changed, so nothing // should be scheduled — this is the whole point of the guard. - val emitted = flowOf(projection(e1, null)).changesOnly().toList() + val emitted = flowOf(projection(e1, null)).dropFirstProjection().toList() assertEquals(emptyList>(), emitted) } @@ -45,7 +45,7 @@ class ProConfigChangeTriggerTest { projection(e1, null), projection(e1, null), projection(e1, null), - ).changesOnly().toList() + ).dropFirstProjection().toList() assertEquals(emptyList>(), emitted) } @@ -57,7 +57,7 @@ class ProConfigChangeTriggerTest { val emitted = flowOf( projection(e1, null), projection(e2, null), - ).changesOnly().toList() + ).dropFirstProjection().toList() assertEquals(listOf(projection(e2, null)), emitted) } @@ -69,7 +69,7 @@ class ProConfigChangeTriggerTest { val emitted = flowOf( projection(e1, null), projection(e1, 1_760_000_000L), - ).changesOnly().toList() + ).dropFirstProjection().toList() assertEquals(listOf(projection(e1, 1_760_000_000L)), emitted) } @@ -82,7 +82,7 @@ class ProConfigChangeTriggerTest { projection(e2, null), // real change projection(e2, null), // unrelated profile edit projection(e2, 42L), // real change - ).changesOnly().toList() + ).dropFirstProjection().toList() assertEquals(listOf(projection(e2, null), projection(e2, 42L)), emitted) } @@ -94,7 +94,7 @@ class ProConfigChangeTriggerTest { // Harmless on a brand-new account, which has no Pro to lose, and it is the same trade iOS makes // with `hasProjectedUserConfig`. If a real transition is ever seen to land as the first emission // on an EXISTING account, this test is the thing to revisit. - val emitted = flowOf(projection(e2, null)).changesOnly().toList() + val emitted = flowOf(projection(e2, null)).dropFirstProjection().toList() assertEquals(emptyList>(), emitted) } From 35db96323c525b4e515e2e275f885dfd11b3df9a Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:17:22 +1000 Subject: [PATCH 08/23] QA: split the Pro status mock so DISPLAY can be mocked without granting ACCESS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `forceCurrentUserAsPro` doubled as the "use mocked state at all" gate for DISPLAY, so mocking a status necessarily also granted access and the state "get_pro_status says Active while no usable proof exists" could not be set up at all. iOS and Desktop can express it from their mocks, which made that edge case reachable on two clients out of three and unmeasurable on the third. DISPLAY now keys on the mocked subscription type; `forceCurrentUserAsPro` means only what it says, and is read by the ACCESS path alone. sessionProBackendStatus still writes both, so every existing spec that used it to make a client Pro behaves as before; sessionProForceAccess overrides the access half afterwards: sessionProBackendStatus=active sessionProForceAccess=false Withholding access conjures no expired proof — it means no proof, which is the state under test: the composer offers the standard limit and a send attaches nothing. Incidentally fixes the debug menu, where the subscription-status picker did nothing unless "force current user as Pro" happened to be on as well. Each control now does what its label says. The extra's NAME is provisional: the harness dictates keys identically across platforms and this one was coined here, so it should be renamed to whatever iOS/Desktop already spell "status without access" as. --- .../securesms/pro/ProStatusManager.kt | 17 ++++- .../securesms/qa/QaLaunchConfig.kt | 66 ++++++++++++++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index c724406121..17716ad271 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -158,7 +158,17 @@ class ProStatusManager @Inject constructor( } } - if(!forceCurrentUserAsPro){ + // Keyed on the DISPLAY mock being set, NOT on the access force-grant. + // + // These were one flag, so mocking a status necessarily also granted access, and the state + // "`get_pro_status` says Active while no usable proof exists" — the truncation case — could + // not be set up at all. iOS and Desktop can express it from their mocks; Android could not, + // which made an edge case reachable on two clients out of three. + // + // `forceCurrentUserAsPro` now means what it says: grant ACCESS. It is read by the ACCESS + // path (`RecipientRepository.resolveProStatus`'s debug override and + // [currentUserHasProAccess]) and deliberately has no say in DISPLAY. + if(debugSubscription == null){ Log.d(DebugLogGroup.PRO_DATA.label, "ProStatusManager: Getting REAL Pro data state") val nowMs = snodeClock.currentTimeMillis() @@ -176,7 +186,10 @@ class ProStatusManager @Inject constructor( }// debug data else { Log.d(DebugLogGroup.PRO_DATA.label, "ProStatusManager: Getting DEBUG Pro data state") - val subscriptionState = debugSubscription ?: DebugMenuViewModel.DebugSubscriptionStatus.AUTO_GOOGLE + // Non-null by the branch condition. The old `?: AUTO_GOOGLE` default existed because the + // branch was keyed on the force flag, so it could be entered with no status chosen; now + // that it is keyed on the status itself there is nothing to default to. + val subscriptionState = debugSubscription // SnodeClock, not Instant.now(), because every consumer of these instants reads // SnodeClock: the expiry label renders from `clock.currentTime()` diff --git a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt index ecc668434f..268b8adeaf 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt @@ -87,12 +87,24 @@ object QaLaunchConfig { * iOS uses on every mockable Pro feature; an ABSENT extra leaves the preferences untouched. * * Maps to TWO preferences, because Android splits the concerns iOS keeps in one key: - * `forceCurrentUserAsPro` is the "use mocked state at all" gate, and `DEBUG_SUBSCRIPTION_STATUS` - * picks which state. Collapsing them here is what keeps one `bothPlatformsIt` setup meaning the - * same thing on both platforms. + * `DEBUG_SUBSCRIPTION_STATUS` mocks the DISPLAY status, and `forceCurrentUserAsPro` grants ACCESS. + * Setting both from this one key is what keeps a single `bothPlatformsIt` setup meaning the same + * thing on both platforms. + * + * They are no longer WELDED, though, and the difference matters: `forceCurrentUserAsPro` used to + * double as the "use mocked state at all" gate for DISPLAY too, so a mocked status necessarily also + * granted access and `get_pro_status`-says-Active-with-no-usable-proof could not be set up at all. + * DISPLAY now keys on the subscription type alone. Use [EXTRA_PRO_FORCE_ACCESS] to vary the access + * half independently. */ private const val EXTRA_PRO_BACKEND_STATUS = "sessionProBackendStatus" + /** + * Grants/withholds mocked Pro ACCESS independently of [EXTRA_PRO_BACKEND_STATUS]'s DISPLAY mock. + * `true` | `false`; absent leaves whatever the status extra decided. See [applyProForceAccess]. + */ + private const val EXTRA_PRO_FORCE_ACCESS = "sessionProForceAccess" + /** * When the mocked Pro access expires, overriding the fixed offset the fixture selected by * [EXTRA_PRO_BACKEND_STATUS] carries. iOS's `mockCurrentUserAccessExpiryTimestamp`, which is an @@ -170,6 +182,8 @@ object QaLaunchConfig { applyServiceNetwork(intent, prefs) applyProBackend(intent, prefs) applyProBackendStatus(intent, prefs) + // After the status extra: it overrides the access half that one sets. + applyProForceAccess(intent, prefs) applyProAccessExpiry(intent, prefs) applyProLoadingState(intent, prefs) } catch (e: RuntimeException) { @@ -195,6 +209,7 @@ object QaLaunchConfig { EXTRA_PRO_BACKEND_URL, EXTRA_PRO_BACKEND_PUBKEY, EXTRA_PRO_BACKEND_STATUS, + EXTRA_PRO_FORCE_ACCESS, EXTRA_PRO_ACCESS_EXPIRY, EXTRA_PRO_LOADING_STATE, ) @@ -413,12 +428,57 @@ object QaLaunchConfig { // TextSecurePreferences.events, which is what ProStatusManager.proDataState collects. A generic // write would persist the value and emit nothing, so the mock would appear not to apply until // the next launch. + // + // Still writes BOTH, so every existing spec that uses this extra alone to "make the client Pro" + // behaves exactly as before. The two are no longer welded together though: DISPLAY now keys on + // the subscription type and ACCESS on the force flag, and [EXTRA_PRO_FORCE_ACCESS] can override + // the access half afterwards. That is what makes status-without-access expressible. prefs.setForceCurrentUserAsPro(mocked != null) prefs.setDebugSubscriptionType(mocked) Log.i(TAG, "Set mocked Pro state to '$raw' (debug subscription = ${mocked?.name ?: "off"})") return true } + /** + * Grants or withholds mocked Pro ACCESS independently of the mocked DISPLAY status. + * + * ⚠️ NAME IS PROVISIONAL — the harness dictates keys identically across platforms, and I cannot read + * the Appium repo from here to see how iOS/Desktop spell "status without access". If they already + * have a spelling, rename this constant to match; it is referenced in exactly three places. + * + * Applied AFTER [applyProBackendStatus] so it overrides the access half that extra sets. To reach + * the truncation state — `get_pro_status` says Active, no usable proof: + * + * sessionProBackendStatus=active sessionProForceAccess=false + * + * Absent leaves whatever [applyProBackendStatus] decided, so existing specs are unaffected. + * + * Withholding access does NOT conjure an expired proof — it means no proof at all, which is the + * state under test: the composer offers the standard limit, and a send attaches nothing. + */ + private fun applyProForceAccess(intent: Intent, prefs: TextSecurePreferences): Boolean { + if (!intent.hasExtra(EXTRA_PRO_FORCE_ACCESS)) { + return false + } + + val raw = intent.getStringExtra(EXTRA_PRO_FORCE_ACCESS).orEmpty().trim() + val grant: Boolean = when (raw.lowercase()) { + "true" -> true + "false" -> false + else -> { + Log.e( + TAG, + "Ignoring unknown '$EXTRA_PRO_FORCE_ACCESS' extra: '$raw'. Use true | false." + ) + return false + } + } + + prefs.setForceCurrentUserAsPro(grant) + Log.i(TAG, "Set mocked Pro ACCESS to $grant (DISPLAY status left as set)") + return true + } + /** Sets the mocked Pro access expiry. See [EXTRA_PRO_ACCESS_EXPIRY] for the accepted forms. */ private fun applyProAccessExpiry(intent: Intent, prefs: TextSecurePreferences): Boolean { if (!intent.hasExtra(EXTRA_PRO_ACCESS_EXPIRY)) { From a66a305abb0346cd559521f59b9d2ba14e662072 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:21:20 +1000 Subject: [PATCH 09/23] Pro: say payment-due, not paid-through, for the reported expiry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the cross-client terminology pin: E is the payment-due instant and coverage ends at E + G, so "paid-through" for E is the wording that let two clients mean different instants while sounding like they agreed. Rephrased rather than substituted. The sentence was drawing a real distinction — the backend deliberately does not follow Play's expiry extension — and a mechanical swap of the term would have dropped it, so the contrast is now stated against the extension explicitly. --- .../main/java/org/thoughtcrime/securesms/pro/ProStatus.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt index ccea0c1ae0..e70aa32a91 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt @@ -72,9 +72,9 @@ sealed interface ProStatus{ * [expiredAt] instead is short by exactly [gracePeriod], and empty once grace reaches the * window length. [gracePeriod] is coverage-past-expiry: the provider's dunning window plus the * backend's ~1h renewal-latency allowance. It is multi-day once a real dunning window is known - * (Apple states its retry window directly; for Play the backend keeps the reported expiry at - * the paid-through date and carries Play's expiry extension as the grace instead), and ~1h - * before then. + * (Apple states its retry window directly; for Play the backend does NOT follow Play's expiry + * extension — it keeps the reported expiry at the original payment-due instant and carries the + * extension as the grace instead), and ~1h before then. * * Derived here rather than at the consumer so a second reader cannot pick the other anchor. */ From c966b4c45c5b5e6ba5f9c4e98944f8e0ca271edf Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:26:53 +1000 Subject: [PATCH 10/23] =?UTF-8?q?QA:=20one=20lever=20per=20fact=20?= =?UTF-8?q?=E2=80=94=20proBackendStatus=20drives=20DISPLAY,=20sessionProPr?= =?UTF-8?q?oof=20drives=20ACCESS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the additive shape from edfbbbced5. proBackendStatus no longer grants access at all, so a spec wanting an ordinary Pro user sets both keys. Leaving a combined lever alongside two separate ones would have meant three keys describing two facts, with nothing to say which was authoritative. Key and values are the harness's: sessionProProof = valid | none | useActual, matching the harness field `proProof` (iOS mockCurrentUserSessionProProof, Desktop SESSION_PRO_MOCK_PROOF). The override is TRI-STATE rather than a boolean because `none` and `useActual` differ whenever a real proof exists — which it can, since the suite can point a client at a QA backend that mints them. `none` must deny such a proof and `useActual` must let it through; a boolean can only say one of those. Both directions are honoured on the render path as well as at enforcement, so a `none` fixture cannot show a Pro badge while the composer offers the standard limit. An unrecognised value is rejected and logged rather than treated as off: a silently ignored typo produces a passing test of the default state, which is worse than a failure. --- .../utilities/TextSecurePreferences.kt | 17 ++++ .../securesms/database/RecipientRepository.kt | 11 ++- .../securesms/pro/ProStatusManager.kt | 16 ++-- .../securesms/qa/QaLaunchConfig.kt | 78 +++++++++++-------- 4 files changed, 83 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt b/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt index 3c5615c098..52b3833862 100644 --- a/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt +++ b/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt @@ -219,6 +219,14 @@ interface TextSecurePreferences { fun setDebugMessageFeatures(features: Set) fun getDebugSubscriptionType(): DebugMenuViewModel.DebugSubscriptionStatus? + + /** + * Mocked Pro ACCESS override: `true` grants, `false` DENIES, `null` means no override so the real + * proof governs. Tri-state deliberately — `false` and `null` differ when a real proof exists, which + * it can on a QA backend that mints them. + */ + fun getDebugProAccessOverride(): Boolean? + fun setDebugProAccessOverride(granted: Boolean?) fun setDebugSubscriptionType(status: DebugMenuViewModel.DebugSubscriptionStatus?) fun getDebugProAccessExpiry(): Instant? fun setDebugProAccessExpiry(expiry: Instant?) @@ -386,6 +394,7 @@ interface TextSecurePreferences { const val DEBUG_PRO_MESSAGE_FEATURES = "debug_pro_message_features" const val DEBUG_PRO_PROFILE_FEATURES = "debug_pro_profile_features" const val DEBUG_SUBSCRIPTION_STATUS = "debug_subscription_status" + const val DEBUG_PRO_ACCESS_OVERRIDE = "debug_pro_access_override" const val DEBUG_PRO_ACCESS_EXPIRY = "debug_pro_access_expiry" const val DEBUG_PRO_PLAN_STATUS = "debug_pro_plan_status" const val DEBUG_FORCE_NO_BILLING = "debug_pro_has_billing" @@ -1247,6 +1256,14 @@ class AppTextSecurePreferences @Inject constructor( _events.tryEmit(TextSecurePreferences.DEBUG_SUBSCRIPTION_STATUS) } + override fun getDebugProAccessOverride(): Boolean? = + getStringPreference(TextSecurePreferences.DEBUG_PRO_ACCESS_OVERRIDE, null)?.toBooleanStrictOrNull() + + override fun setDebugProAccessOverride(granted: Boolean?) { + setStringPreference(TextSecurePreferences.DEBUG_PRO_ACCESS_OVERRIDE, granted?.toString()) + _events.tryEmit(TextSecurePreferences.DEBUG_PRO_ACCESS_OVERRIDE) + } + override fun getDebugProAccessExpiry(): Instant? { return getStringPreference(TextSecurePreferences.DEBUG_PRO_ACCESS_EXPIRY, null) ?.toLongOrNull() diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt index 41538c1385..e70df5d478 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt @@ -523,7 +523,16 @@ class RecipientRepository @Inject constructor( } // 3. Apply Debug Overrides - if (recipient.isSelf && proData == null && prefs.forceCurrentUserAsPro()) { + // + // The mocked-proof override is tri-state and BOTH directions are applied here, not just the + // grant: rendering and enforcement must never disagree about what a mocked run is entitled to, + // and `ProStatusManager.currentUserHasProAccess` honours `none` as a denial even against a real + // proof. If this only honoured the grant, a `proProof=none` fixture would show a Pro badge while + // the composer offered the standard limit. + val proAccessOverride = if (recipient.isSelf) prefs.getDebugProAccessOverride() else null + if (recipient.isSelf && proAccessOverride == false) { + proData = null + } else if (recipient.isSelf && proData == null && (proAccessOverride == true || prefs.forceCurrentUserAsPro())) { proData = RecipientData.ProData(showProBadge = true) } else if (!recipient.isSelf && (recipient.address is Address.Standard) diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index 17716ad271..4681bf97aa 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -617,14 +617,20 @@ class ProStatusManager @Inject constructor( * recomputed at the existing change sites (config change, revocation update, proof expiry), because * some of them redraw per keystroke and this takes the config lock. * - * Honours the QA force-grant deliberately. Without it, rendering (which resolves through + * Honours the QA overrides deliberately. Without them, rendering (which resolves through * `RecipientRepository`, where the override lives) and enforcement would disagree under a fixture: - * the composer would offer the Pro limit and sending would then refuse it. Note the force grants - * ACCESS but conjures no PROOF, so a forced client still attaches nothing when it sends — which is - * the truncation state, and is correct rather than a gap here. + * the composer would offer the Pro limit and sending would then refuse it. + * + * The proof mock is TRI-STATE and is checked first, because `none` and "no override" are different + * answers whenever a real proof exists — which it can on a QA backend that mints them. `false` must + * deny such a proof; absent must let it through. A boolean cannot say both. + * + * Note an override grants ACCESS but conjures no PROOF, so a mocked-Pro client still attaches nothing + * when it sends. That is the truncation state and it is correct rather than a gap here. */ fun currentUserHasProAccess(): Boolean = - prefs.forceCurrentUserAsPro() || currentUserProProofForAccess() != null + prefs.getDebugProAccessOverride() + ?: (prefs.forceCurrentUserAsPro() || currentUserProProofForAccess() != null) /** * Logic to determine if we should animate the avatar for a user or freeze it on the first frame diff --git a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt index 268b8adeaf..45d82d59f8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt @@ -94,16 +94,25 @@ object QaLaunchConfig { * They are no longer WELDED, though, and the difference matters: `forceCurrentUserAsPro` used to * double as the "use mocked state at all" gate for DISPLAY too, so a mocked status necessarily also * granted access and `get_pro_status`-says-Active-with-no-usable-proof could not be set up at all. - * DISPLAY now keys on the subscription type alone. Use [EXTRA_PRO_FORCE_ACCESS] to vary the access + * DISPLAY now keys on the subscription type alone. Use [EXTRA_PRO_PROOF] to vary the access * half independently. */ private const val EXTRA_PRO_BACKEND_STATUS = "sessionProBackendStatus" /** - * Grants/withholds mocked Pro ACCESS independently of [EXTRA_PRO_BACKEND_STATUS]'s DISPLAY mock. - * `true` | `false`; absent leaves whatever the status extra decided. See [applyProForceAccess]. + * Mocked Pro PROOF, i.e. ACCESS — the harness field `proProof`. + * + * `valid` grants access, `none` DENIES it regardless of any real proof, `useActual` (and an absent + * extra) applies no override so the real proof governs. iOS spells this + * `mockCurrentUserSessionProProof` and Desktop `SESSION_PRO_MOCK_PROOF`; the harness field name is + * what is identical across clients, the app-side literal follows each platform's own convention. + * + * DISPLAY and ACCESS are separate levers: [EXTRA_PRO_BACKEND_STATUS] says what the PLAN is and grants + * nothing, this says what the device MAY DO. A spec wanting an ordinary Pro user sets both, and the + * interesting fixture is the one that sets them to disagree — `proBackendStatus=active` with + * `proProof=none` is the truncation state, where the plan reads active and no usable proof exists. */ - private const val EXTRA_PRO_FORCE_ACCESS = "sessionProForceAccess" + private const val EXTRA_PRO_PROOF = "sessionProProof" /** * When the mocked Pro access expires, overriding the fixed offset the fixture selected by @@ -183,7 +192,7 @@ object QaLaunchConfig { applyProBackend(intent, prefs) applyProBackendStatus(intent, prefs) // After the status extra: it overrides the access half that one sets. - applyProForceAccess(intent, prefs) + applyProProof(intent, prefs) applyProAccessExpiry(intent, prefs) applyProLoadingState(intent, prefs) } catch (e: RuntimeException) { @@ -209,7 +218,7 @@ object QaLaunchConfig { EXTRA_PRO_BACKEND_URL, EXTRA_PRO_BACKEND_PUBKEY, EXTRA_PRO_BACKEND_STATUS, - EXTRA_PRO_FORCE_ACCESS, + EXTRA_PRO_PROOF, EXTRA_PRO_ACCESS_EXPIRY, EXTRA_PRO_LOADING_STATE, ) @@ -429,53 +438,56 @@ object QaLaunchConfig { // write would persist the value and emit nothing, so the mock would appear not to apply until // the next launch. // - // Still writes BOTH, so every existing spec that uses this extra alone to "make the client Pro" - // behaves exactly as before. The two are no longer welded together though: DISPLAY now keys on - // the subscription type and ACCESS on the force flag, and [EXTRA_PRO_FORCE_ACCESS] can override - // the access half afterwards. That is what makes status-without-access expressible. - prefs.setForceCurrentUserAsPro(mocked != null) + // DISPLAY ONLY. This deliberately no longer grants ACCESS: one lever per fact, so a spec that + // wants an ordinary Pro user sets this AND [EXTRA_PRO_PROOF]. Leaving a combined lever in place + // alongside the two separate ones would mean three keys describing two facts, and a later reader + // could not tell which was authoritative. prefs.setDebugSubscriptionType(mocked) - Log.i(TAG, "Set mocked Pro state to '$raw' (debug subscription = ${mocked?.name ?: "off"})") + Log.i(TAG, "Set mocked Pro DISPLAY status to '$raw' (debug subscription = ${mocked?.name ?: "off"}); grants no access") return true } /** - * Grants or withholds mocked Pro ACCESS independently of the mocked DISPLAY status. - * - * ⚠️ NAME IS PROVISIONAL — the harness dictates keys identically across platforms, and I cannot read - * the Appium repo from here to see how iOS/Desktop spell "status without access". If they already - * have a spelling, rename this constant to match; it is referenced in exactly three places. - * - * Applied AFTER [applyProBackendStatus] so it overrides the access half that extra sets. To reach - * the truncation state — `get_pro_status` says Active, no usable proof: + * Applies the mocked Pro PROOF, i.e. ACCESS. See [EXTRA_PRO_PROOF]. * - * sessionProBackendStatus=active sessionProForceAccess=false + * valid -> grant + * none -> DENY, even if a real proof exists + * useActual -> clear the override; the real proof governs * - * Absent leaves whatever [applyProBackendStatus] decided, so existing specs are unaffected. + * Tri-state rather than a boolean because `none` and `useActual` are different answers whenever a + * real proof exists — which it can, since the suite can point the client at a QA backend that mints + * them. Collapsing them would make `none` mean "don't force" rather than "deny". * - * Withholding access does NOT conjure an expired proof — it means no proof at all, which is the - * state under test: the composer offers the standard limit, and a send attaches nothing. + * An unrecognised value is REJECTED and logged rather than treated as off. A silently-ignored typo + * here produces a PASSING test of the default state, which is worse than a failure — the same + * reasoning as [warnOnUnrecognisedExtras]. */ - private fun applyProForceAccess(intent: Intent, prefs: TextSecurePreferences): Boolean { - if (!intent.hasExtra(EXTRA_PRO_FORCE_ACCESS)) { + private fun applyProProof(intent: Intent, prefs: TextSecurePreferences): Boolean { + if (!intent.hasExtra(EXTRA_PRO_PROOF)) { return false } - val raw = intent.getStringExtra(EXTRA_PRO_FORCE_ACCESS).orEmpty().trim() - val grant: Boolean = when (raw.lowercase()) { - "true" -> true - "false" -> false + val raw = intent.getStringExtra(EXTRA_PRO_PROOF).orEmpty().trim() + // null = clear the override. + val override: Boolean? = when (raw.lowercase()) { + "valid" -> true + "none" -> false + USE_ACTUAL -> null else -> { Log.e( TAG, - "Ignoring unknown '$EXTRA_PRO_FORCE_ACCESS' extra: '$raw'. Use true | false." + "Ignoring unknown '$EXTRA_PRO_PROOF' extra: '$raw'. Use valid | none | $USE_ACTUAL." ) return false } } - prefs.setForceCurrentUserAsPro(grant) - Log.i(TAG, "Set mocked Pro ACCESS to $grant (DISPLAY status left as set)") + prefs.setDebugProAccessOverride(override) + Log.i( + TAG, + "Set mocked Pro ACCESS to '$raw' " + + "(override = ${override?.toString() ?: "cleared, real proof governs"})" + ) return true } From 99b9480a77c390b151226c21dc7734f007ff34df Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:32:13 +1000 Subject: [PATCH 11/23] QA: an absent sessionProProof clears the override rather than leaving it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract says absent is identical to useActual, and the preference persists across launches, so returning early left the previous launch's override in place. A spec that set 'valid' would have made every later spec on the same device silently Pro — the leak Desktop needed PRO_ENV_KEYS to close. Android has no reset list, so absence has to do that work. Clears only the launch-extra override; the debug menu's own toggle is a separate preference and survives a relaunch as a developer would expect. --- .../org/thoughtcrime/securesms/qa/QaLaunchConfig.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt index 45d82d59f8..7249674b64 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt @@ -464,6 +464,16 @@ object QaLaunchConfig { */ private fun applyProProof(intent: Intent, prefs: TextSecurePreferences): Boolean { if (!intent.hasExtra(EXTRA_PRO_PROOF)) { + // ABSENT CLEARS, because the contract says absent is identical to `useActual` and this + // preference PERSISTS across launches. Returning early instead would leave the previous + // launch's override in place, so a spec that set `valid` would silently make every later + // spec on the same device Pro — the leak Desktop hit needing `PRO_ENV_KEYS`. Android has no + // reset list; absence has to do that work. + // + // Note this clears only the launch-extra override. The debug menu's own + // `forceCurrentUserAsPro` toggle is a separate preference and is deliberately untouched, so + // a developer's setting survives a relaunch as they would expect. + prefs.setDebugProAccessOverride(null) return false } From 5866bcedc12574cb82c24e29f2733de403ee66e5 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:35:39 +1000 Subject: [PATCH 12/23] Pro: the compose limit is ACCESS, but the dialog that explains it is DISPLAY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing the predicted behaviour for the truncation fixture surfaced that I had made both read ACCESS. Under proBackendStatus=active + proProof=none that refuses the send correctly and then offers to sell the user Pro — which they are already paying for. The limit stays on ACCESS. Which dialog explains the refusal now reads DISPLAY, so a subscriber holding no usable proof gets "message too long" and the upsell is reserved for users whose plan says they are not subscribed. Same split applied to onCharLimitTapped, which chooses between the same two explanations. --- .../thoughtcrime/securesms/InputbarViewModel.kt | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt index fcaf05133e..47896aced6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt @@ -99,8 +99,14 @@ abstract class InputbarViewModel( val charsLeft = proStatusManager.getCharacterLimit(hasProAccess) - composedCodePointCount return if(charsLeft < 0){ - // the user is trying to send a message that is too long - we should display a dialog - if(hasProAccess){ + // The LIMIT is an access question; which dialog explains it is a DISPLAY one, and the two + // deliberately read different values. + // + // A user whose plan reads Active but who holds no usable proof is over the standard limit — + // that is ACCESS, correctly refusing. But offering them "upgrade to Pro" is inviting them to + // buy something they are already paying for. They get "message too long" instead, and the + // upsell is reserved for users whose plan says they are not subscribed. + if(proStatusManager.proDataState.value.type is ProStatus.Active){ showMessageTooLongSendDialog() } else { showSessionProCTA() @@ -113,7 +119,10 @@ abstract class InputbarViewModel( } fun onCharLimitTapped(){ - if(isSelfPro.value){ + // Same split as [validateMessageLength]: this chooses which explanation to show, so it is DISPLAY. + // `handleCharLimitTappedForRegularUser` is the upsell, and a subscriber with no usable proof must + // not be upsold their own plan. + if(proStatusManager.proDataState.value.type is ProStatus.Active){ handleCharLimitTappedForProUser() } else { handleCharLimitTappedForRegularUser() From 18c68a8b3745cc3130073ea709b6a426ef1e44f3 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:41:39 +1000 Subject: [PATCH 13/23] Pro: the upsell affordances read DISPLAY, not inverted ACCESS Ruled: the composer badge is DISPLAY. Applied to both surfaces of that shape. The rule, which is what the comments state rather than the mechanism: the gate reads ACCESS, the thing that EXPLAINS the gate reads DISPLAY. A badge shown only to non-subscribers is an upsell, so it reads the plan's state; a subscriber whose proof has not arrived is already correctly held to the standard limit by ACCESS, and inviting them to buy what they pay for is a different question. InputbarViewModel the composer char-limit badge MessageDetailsViewModel a badge clickable only for non-subscribers Both were inverted ACCESS reads, so under proBackendStatus=active + proProof=none they upsold a subscriber. Commented against being restored to match the other badges, which are entitlement indicators and correctly read ACCESS. --- .../java/org/thoughtcrime/securesms/InputbarViewModel.kt | 9 ++++++++- .../securesms/conversation/v2/MessageDetailsViewModel.kt | 6 +++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt index 47896aced6..c1a8379d30 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt @@ -76,7 +76,14 @@ abstract class InputbarViewModel( count = charsLeft, countFormatted = NumberUtil.getFormattedNumber(charsLeft.toLong()), danger = charsLeft < 0, - showProBadge = !isSelfPro.value // only show the badge for non pro users + // THE RULE: the gate reads ACCESS, the thing that EXPLAINS the gate reads DISPLAY. + // + // This badge is an upsell, not an entitlement indicator, so it reads the plan's state. + // Do not "fix" it back to an inverted ACCESS read to match the other badges: a + // subscriber whose proof has not arrived is correctly held to the standard limit by + // ACCESS above, and inviting them to buy what they already pay for is a different + // question with a different answer. + showProBadge = proStatusManager.proDataState.value.type !is ProStatus.Active ) } else { null diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt index af518ac818..3dfa25be5d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt @@ -204,7 +204,11 @@ class MessageDetailsViewModel @AssistedInject constructor( thread = conversation, readOnly = isDeprecatedLegacyGroup, proFeatures = proStatusManager.getMessageProFeatures(messageRecord), - proBadgeClickable = !recipientRepository.getSelf().isPro // no badge click if the current user is pro + // A badge clickable only for non-subscribers is an upsell affordance, so it reads + // DISPLAY: the gate reads ACCESS, the thing that explains the gate reads DISPLAY. + // Was an inverted ACCESS read, which offered a subscriber holding no usable proof a + // route to buy the plan they already have. + proBadgeClickable = proStatusManager.proDataState.value.type !is ProStatus.Active ) } } From 947846952073ddf2b5e6145b7c1fb5650d875fdb Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:45:31 +1000 Subject: [PATCH 14/23] Pro: route the second pin gate through the ACCESS function too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pin limit is implemented in two ViewModels and I converted only HomeViewModel.setPinned. ConversationSettingsViewModel.pinConversation still went through recipientRepository.getSelf().isPro — a fresh read, so not stale, but a second route to "am I Pro", which is how one of two duplicated gates ends up honouring a revocation while the other does not. Behaviourally equivalent today, since the debug override is honoured on both paths. Committed for the single-function property rather than for a fix. --- .../v3/settings/ConversationSettingsViewModel.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt index af68a0d5fe..a1c79efeb6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt @@ -737,8 +737,12 @@ class ConversationSettingsViewModel @AssistedInject constructor( private fun pinConversation(){ // check the pin limit before continuing val totalPins = storage.getTotalPinned() + // ENFORCEMENT: the ACCESS function, called at the moment of the decision. The pin limit is + // implemented in TWO ViewModels (see HomeViewModel.setPinned) and both must go through the one + // ACCESS function — a second route to "am I Pro" is how one of them ends up honouring a + // revocation while the other does not. val maxPins = - proStatusManager.getPinnedConversationLimit(recipientRepository.getSelf().isPro) + proStatusManager.getPinnedConversationLimit(proStatusManager.currentUserHasProAccess()) if (totalPins >= maxPins) { // the user has reached the pin limit, show the CTA _dialogState.update { From 032256eb2dcc4346097db07ae1cb0c6a3c9c7b43 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 16:58:30 +1000 Subject: [PATCH 15/23] Pro: split ProStatus.Active so a dateless Active is expressible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Seed the status, no date" was not expressible: every Active variant required a non-null renewingAt plus duration and providerData, all owned by a get_pro_status response. A proof-derived seed therefore had to invent a date, and a sentinel is what the missing-date bugs are made of. Active entitlement only. `is Active` still matches, so every "are we Pro for display" check keeps working. Active.FromProof entitled, no plan detail. Cannot HOLD a date, so no reader can render one. Active.WithPlan renewingAt / duration / providerData and the two helpers that depend on them. Chosen over a nullable renewingAt because the absent date is then enforced by the compiler rather than by discipline — and ProSettingsViewModel floors a negative remaining duration to zero and renders "0 seconds" instead of "Expired", which is what discipline achieves here unaided. Compiler-guided: 51 sites, and each one answered "does this need plan detail?". The refund, cancel, choose-plan and manage screens narrow to WithPlan; the entitlement checks stay on Active. FromProof is not constructed yet, so this commit changes no behaviour — it only makes the seed possible. --- .../prosettings/ProSettingsHomeScreen.kt | 22 +++-- .../prosettings/ProSettingsViewModel.kt | 28 ++++--- .../prosettings/RefundInProgress.kt | 4 +- .../prosettings/RefundPlanNonOriginating.kt | 2 +- .../prosettings/RefundPlanScreen.kt | 2 +- .../chooseplan/ChoosePlanHomeScreen.kt | 2 +- .../chooseplan/ChoosePlanNonOriginating.kt | 2 +- .../thoughtcrime/securesms/pro/ProStatus.kt | 84 +++++++++++++------ 8 files changed, 95 insertions(+), 51 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt index 9f12471f56..50ecfc56b7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt @@ -261,7 +261,9 @@ fun ProSettingsHome( } // Pro Stats - if(subscriptionType is ProStatus.Active){ + // WithPlan: everything inside renders plan detail. A proof-seeded Active shows the + // header/refresh state above and no plan block, which is the "render absent" answer. + if(subscriptionType is ProStatus.Active.WithPlan){ Spacer(Modifier.height(LocalDimensions.current.spacing)) ProStats( data = data.proStats, @@ -270,11 +272,15 @@ fun ProSettingsHome( } // Pro account settings - if(subscriptionType is ProStatus.Active){ + // WithPlan, not Active: everything in this block renders plan detail a response owns. A + // proof-seeded Active shows the header and refresh state above and no plan block, which is the + // "render absent" answer rather than a formatted sentinel. + if(subscriptionType is ProStatus.Active.WithPlan){ Spacer(Modifier.height(LocalDimensions.current.smallSpacing)) ProSettings( showProBadge = data.proDataState.showProBadge, - proStatus = data.proDataState.type, + // the smart-cast value, not `data.proDataState.type` — the cast does not carry to it + proStatus = subscriptionType, subscriptionRefreshState = data.proDataState.refreshState, inSheet = inSheet, inGracePeriod = data.inGracePeriod, @@ -520,7 +526,7 @@ fun ProStatItem( fun ProSettings( modifier: Modifier = Modifier, showProBadge: Boolean, - proStatus: ProStatus.Active, + proStatus: ProStatus.Active.WithPlan, subscriptionRefreshState: State, inSheet: Boolean, expiry: CharSequence, @@ -857,6 +863,12 @@ fun ProManage( refundButton() } + // Entitled from a local proof, with no plan detail yet. Every action here needs plan + // detail a response has not supplied — there is no plan to cancel and no payment to + // refund — so nothing is offered. This screen fetches on arrival, so it is a transient + // state, and the refresh indicator elsewhere is what explains it. + ProStatus.Active.FromProof -> Unit + is ProStatus.NeverSubscribed -> { recoverButton() } @@ -931,7 +943,7 @@ fun ProSettingsFooter( ) { // Manage Pro - Expired has this in the header so exclude it here // We also don't want to show this while refund in process - val refunding = (proStatus as? ProStatus.Active)?.refundInProgress ?: false + val refunding = (proStatus as? ProStatus.Active.WithPlan)?.refundInProgress ?: false if(proStatus !is ProStatus.Expired && !refunding) { Spacer(Modifier.height(LocalDimensions.current.smallSpacing)) ProManage( diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt index 1b07ffab09..8479eb1b15 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt @@ -248,8 +248,10 @@ class ProSettingsViewModel @AssistedInject constructor( else -> "" }, + // WithPlan, not Active: a proof-seeded Active has no date, and the empty string + // is the "render absent" answer rather than a formatted sentinel. subscriptionExpiryDate = when(subType){ - is ProStatus.Active -> subType.renewingAtFormatted() + is ProStatus.Active.WithPlan -> subType.renewingAtFormatted() else -> "" }, ) @@ -294,7 +296,7 @@ class ProSettingsViewModel @AssistedInject constructor( // or the user is pro but non originating val noPriceNeeded = !hasBillingCapacity || (subType is ProStatus.Active && !hasValidSub) - || (subType is ProStatus.Active && subType.providerData.isFromAnotherPlatform()) + || (subType is ProStatus.Active.WithPlan && subType.providerData.isFromAnotherPlatform()) val plans = if(noPriceNeeded) emptyList() else { @@ -325,7 +327,7 @@ class ProSettingsViewModel @AssistedInject constructor( fun ensureCancelState(){ val sub = _proSettingsUIState.value.proDataState.type - if(sub !is ProStatus.Active) return + if(sub !is ProStatus.Active.WithPlan) return _cancelPlanState.update { State.Loading } viewModelScope.launch { @@ -345,7 +347,7 @@ class ProSettingsViewModel @AssistedInject constructor( fun ensureRefundState(){ val sub = _proSettingsUIState.value.proDataState.type - if(sub !is ProStatus.Active) return + if(sub !is ProStatus.Active.WithPlan) return _refundPlanState.update { State.Loading } @@ -446,13 +448,13 @@ class ProSettingsViewModel @AssistedInject constructor( // otherwise go to the "choose plan" screen else -> { // if we in the process of refunding on another platform, show that screen instead - if((_proSettingsUIState.value.proDataState.type as? ProStatus.Active)?.refundInProgress == true){ + if((_proSettingsUIState.value.proDataState.type as? ProStatus.Active.WithPlan)?.refundInProgress == true){ navigateTo(ProSettingsDestination.RefundInProgress) return } // otherwise handle the "Choose Plan" - val provider = (_proSettingsUIState.value.proDataState.type as? ProStatus.Active)?.providerData + val provider = (_proSettingsUIState.value.proDataState.type as? ProStatus.Active.WithPlan)?.providerData if(_proSettingsUIState.value.inGracePeriod){ _dialogState.update { it.copy( @@ -481,14 +483,14 @@ class ProSettingsViewModel @AssistedInject constructor( Commands.GoToRefund -> { val sub = _proSettingsUIState.value.proDataState.type - if(sub !is ProStatus.Active) return + if(sub !is ProStatus.Active.WithPlan) return navigateTo(ProSettingsDestination.RefundSubscription) } Commands.GoToCancel -> { val sub = _proSettingsUIState.value.proDataState.type - if(sub !is ProStatus.Active) return + if(sub !is ProStatus.Active.WithPlan) return navigateTo(ProSettingsDestination.CancelSubscription) } @@ -501,7 +503,7 @@ class ProSettingsViewModel @AssistedInject constructor( } Commands.OpenCancelSubscriptionPage -> { - val subUrl = (_proSettingsUIState.value.proDataState.type as? ProStatus.Active) + val subUrl = (_proSettingsUIState.value.proDataState.type as? ProStatus.Active.WithPlan) ?.providerData?.cancelSubscriptionUrl if(!subUrl.isNullOrEmpty()){ viewModelScope.launch { @@ -565,7 +567,7 @@ class ProSettingsViewModel @AssistedInject constructor( val currentSubscription = _proSettingsUIState.value.proDataState.type val selectedPlan = getSelectedPlan() ?: return - if(currentSubscription is ProStatus.Active){ + if(currentSubscription is ProStatus.Active.WithPlan){ val newSubscriptionExpiryString = currentSubscription.renewingAtFormatted() val currentSubscriptionDuration = DateUtils.getLocalisedProPlanLength( @@ -786,7 +788,7 @@ class ProSettingsViewModel @AssistedInject constructor( // by a fixed enum, so the unit is respected as transmitted. This is cosmetic and (count, unit) is // not a guaranteed-unique key, so we degrade gracefully: a SKU is "current" iff its period equals // the active plan's; if nothing matches (e.g. a "1y" plan vs a "12m" SKU), nothing is marked. - val activePeriod = (subType as? ProStatus.Active)?.duration + val activePeriod = (subType as? ProStatus.Active.WithPlan)?.duration // get prices from the subscription provider val prices = subscriptionCoordinator.getCurrentManager().getSubscriptionPrices() @@ -1019,12 +1021,12 @@ class ProSettingsViewModel @AssistedInject constructor( ) data class CancelPlanState( - val proStatus: ProStatus.Active, + val proStatus: ProStatus.Active.WithPlan, val hasValidSubscription: Boolean, // true is there is a current subscription AND the available subscription manager on this device has an account which matches the product id we got from libsession ) data class RefundPlanState( - val proStatus: ProStatus.Active, + val proStatus: ProStatus.Active.WithPlan, val isQuickRefund: Boolean, val quickRefundUrl: String? ) diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundInProgress.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundInProgress.kt index 0fabc74a9e..e72a30955f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundInProgress.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundInProgress.kt @@ -43,7 +43,7 @@ fun RefundInProgressScreen( onBack: () -> Unit, ) { val state by viewModel.proSettingsUIState.collectAsState() - val activePlan = state.proDataState.type as? ProStatus.Active ?: return + val activePlan = state.proDataState.type as? ProStatus.Active.WithPlan ?: return RefundInProgress( subscription = activePlan, @@ -55,7 +55,7 @@ fun RefundInProgressScreen( @OptIn(ExperimentalMaterial3Api::class, ExperimentalSharedTransitionApi::class) @Composable fun RefundInProgress( - subscription: ProStatus.Active, + subscription: ProStatus.Active.WithPlan, sendCommand: (ProSettingsViewModel.Commands) -> Unit, onBack: () -> Unit, ){ diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanNonOriginating.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanNonOriginating.kt index 3672f799c4..3ba83d4cec 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanNonOriginating.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanNonOriginating.kt @@ -23,7 +23,7 @@ import org.thoughtcrime.securesms.ui.theme.ThemeColors @OptIn(ExperimentalMaterial3Api::class, ExperimentalSharedTransitionApi::class) @Composable fun RefundPlanNonOriginating( - subscription: ProStatus.Active, + subscription: ProStatus.Active.WithPlan, sendCommand: (ProSettingsViewModel.Commands) -> Unit, onBack: () -> Unit, ){ diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanScreen.kt index 385bdfce00..99d2b001d3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/RefundPlanScreen.kt @@ -77,7 +77,7 @@ fun RefundPlanScreen( @OptIn(ExperimentalMaterial3Api::class, ExperimentalSharedTransitionApi::class) @Composable fun RefundPlan( - data: ProStatus.Active, + data: ProStatus.Active.WithPlan, isQuickRefund: Boolean, quickRefundUrl: String?, sendCommand: (ProSettingsViewModel.Commands) -> Unit, diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanHomeScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanHomeScreen.kt index 689edc28fb..6a6f47f076 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanHomeScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanHomeScreen.kt @@ -29,7 +29,7 @@ fun ChoosePlanHomeScreen( onBack = onBack ) { planData -> // Option 1. ACTIVE Pro subscription - if(planData.proStatus is ProStatus.Active) { + if(planData.proStatus is ProStatus.Active.WithPlan) { val subscription = planData.proStatus when { diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNonOriginating.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNonOriginating.kt index 325c977d4f..802339828c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNonOriginating.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNonOriginating.kt @@ -29,7 +29,7 @@ import org.thoughtcrime.securesms.util.DateUtils @OptIn(ExperimentalMaterial3Api::class, ExperimentalSharedTransitionApi::class) @Composable fun ChoosePlanNonOriginating( - subscription: ProStatus.Active, + subscription: ProStatus.Active.WithPlan, sendCommand: (ProSettingsViewModel.Commands) -> Unit, onBack: () -> Unit, ){ diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt index e70aa32a91..5fb5d95d97 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt @@ -11,11 +11,61 @@ sealed interface ProStatus{ data object NeverSubscribed: ProStatus sealed interface Active: ProStatus{ - val renewingAt: Instant // the payment/renewal-due date (E), as the backend sends it - val duration: ProPlanPeriod // the backend's raw (count, unit) — rendered generically, never bucketed - val providerData: PaymentProviderMetadata - val quickRefundExpiry: Instant? - val refundInProgress: Boolean + + /** + * Entitled, with NO plan detail known — the DISPLAY seed, derived from a local proof when no + * `get_pro_status` response has ever been persisted. + * + * Carries no date DELIBERATELY, and that is the point of it being its own type rather than a + * dated variant holding a sentinel. `E`, `G` and the provider are response-owned: only a response + * carries `latest_payment` and the values that must agree with it, and the proof's own expiry is a + * short clamped credential lifetime rather than the plan's payment-due date — on a compressed QA + * clock they differ by ~300s against ~30 days. Seeding a date from the proof would therefore show + * an account paid through a month as expiring in minutes. + * + * Because it cannot HOLD a date, no reader can render one. That is why this is a type and not a + * nullable field: `ProSettingsViewModel` already floors a negative remaining duration to zero and + * renders "0 seconds" instead of "Expired", which is what discipline alone achieves here. + * + * `is Active` still matches, so every "are we Pro for display purposes" check keeps working. Code + * that needs the plan's dates must narrow to [WithPlan], and the compiler will say so. + */ + data object FromProof: Active + + /** + * Active WITH plan detail, i.e. sourced from a `get_pro_status` response. + * + * Everything a response owns lives here rather than on [Active], so a reader that wants a date has + * to prove it has one. + */ + sealed interface WithPlan: Active { + val renewingAt: Instant // the payment/renewal-due date (E), as the backend sends it + val duration: ProPlanPeriod // the backend's raw (count, unit) — rendered generically, never bucketed + val providerData: PaymentProviderMetadata + val quickRefundExpiry: Instant? + val refundInProgress: Boolean + + /** + * Whether the store's own quick-refund window is still open, which decides between the + * <48h (#19/#22) and >48h (#20/#23) refund screens. + * + * [now] must come from [org.session.libsession.network.SnodeClock], as everywhere else in + * the Pro stack — `quickRefundExpiry` is a backend/store timestamp, so comparing it against + * the device clock lets clock skew flip the branch. + */ + fun isWithinQuickRefundWindow(now: Instant): Boolean { + return quickRefundExpiry?.isAfter(now) == true + } + + fun renewingAtFormatted(): String { + val pattern = if (BuildConfig.BUILD_TYPE != "release") + "MMMM d, yyyy, h:mm a" // non prod builds can show seconds for debugging purposes + else "MMMM d, yyyy" + return DateUtils.getLocaleFormattedDate( + renewingAt.toEpochMilli(), pattern + ) + } + } data class AutoRenewing( override val renewingAt: Instant, @@ -24,7 +74,7 @@ sealed interface ProStatus{ override val quickRefundExpiry: Instant?, override val refundInProgress: Boolean, val inGracePeriod: Boolean - ): Active + ): WithPlan data class Expiring( override val renewingAt: Instant, @@ -32,28 +82,8 @@ sealed interface ProStatus{ override val providerData: PaymentProviderMetadata, override val quickRefundExpiry: Instant?, override val refundInProgress: Boolean, - ): Active - - /** - * Whether the store's own quick-refund window is still open, which decides between the - * <48h (#19/#22) and >48h (#20/#23) refund screens. - * - * [now] must come from [org.session.libsession.network.SnodeClock], as everywhere else in - * the Pro stack — `quickRefundExpiry` is a backend/store timestamp, so comparing it against - * the device clock lets clock skew flip the branch. - */ - fun isWithinQuickRefundWindow(now: Instant): Boolean { - return quickRefundExpiry?.isAfter(now) == true - } + ): WithPlan - fun renewingAtFormatted(): String { - val pattern = if (BuildConfig.BUILD_TYPE != "release") - "MMMM d, yyyy, h:mm a" // non prod builds can show seconds for debugging purposes - else "MMMM d, yyyy" - return DateUtils.getLocaleFormattedDate( - renewingAt.toEpochMilli(), pattern - ) - } } data class Expired( From 3eb2a39d64ec94335dda36174cfc45d48d4ed81e Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 14 Aug 2026 17:01:34 +1000 Subject: [PATCH 16/23] Pro: seed DISPLAY from the proof, unfloor the expiry duration, and give an Active plan no CTA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of the ruled queue, all downstream of the type split. SEED — DISPLAY now falls back to the local proof when no get_pro_status response has ever been persisted, instead of answering NeverSubscribed. That was the "Upgrade Session" bug: a restored subscriber's row said they had never subscribed while their proof was granting them the features. A response still wins wherever one exists. Expiry-only, matching iOS's proProofIsActive, and commented as deliberate: revocation is an ACCESS concern and the two values are allowed to disagree. Two known gaps are named in the KDoc rather than closed — an access expiry with no proof, and an expired proof, both read NeverSubscribed, and both are contract questions for all three clients. UNFLOOR — ProSettingsViewModel floored the remaining duration at zero, which made getExpiryString's own "Expired" branch unreachable and rendered "0 seconds" for a past renewal date. That reads as a live countdown at the moment of lapse and gets chased as an expiry bug rather than a stale one. PIN CTA — an Active plan now gets no CTA at all. A subscriber whose proof has not arrived is refused by ACCESS, and selling them the plan they hold is worse than refusing silently. Applied to both ViewModels that implement the limit. The silence is an accepted trade: the copy that would explain a refusal without offering a purchase does not exist, and adding it is a translation round. --- .../settings/ConversationSettingsViewModel.kt | 3 ++ .../securesms/home/HomeViewModel.kt | 5 +++ .../prosettings/ProSettingsViewModel.kt | 10 +++-- .../securesms/pro/ProStatusManager.kt | 40 ++++++++++++++++++- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt index a1c79efeb6..ff0386f231 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt @@ -744,6 +744,9 @@ class ConversationSettingsViewModel @AssistedInject constructor( val maxPins = proStatusManager.getPinnedConversationLimit(proStatusManager.currentUserHasProAccess()) if (totalPins >= maxPins) { + // RULED: an Active plan gets no CTA — see HomeViewModel.setPinned for the reasoning. The + // pin limit lives in two ViewModels and both must apply it. + if (proStatusManager.proDataState.value.type is ProStatus.Active) return // the user has reached the pin limit, show the CTA _dialogState.update { it.copy( diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt index 9f02cca6a3..992a728171 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt @@ -363,6 +363,11 @@ class HomeViewModel @Inject constructor( val maxPins = proStatusManager.getPinnedConversationLimit(proStatusManager.currentUserHasProAccess()) if (pinned && totalPins >= maxPins) { + // RULED: an Active plan gets NO CTA. A subscriber whose proof has not arrived is refused + // the pin by ACCESS above, and offering to sell them the plan they already hold is worse + // than refusing silently. The silence is an accepted trade, not an oversight — the copy + // that would explain a refusal without offering a purchase does not exist. + if (proStatusManager.proDataState.value.type is ProStatus.Active) return // the user has reached the pin limit, show the CTA _dialogsState.update { it.copy( diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt index 8479eb1b15..7449e0d72f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsViewModel.kt @@ -230,9 +230,13 @@ class ProSettingsViewModel @AssistedInject constructor( } else { Phrase.from(context, R.string.proAutoRenewTime) .put( + // NOT floored at zero. `getExpiryString` already renders a + // negative remaining as "Expired", and the floor made that + // branch unreachable — a past renewal date rendered + // "0 seconds", which reads as a live countdown at the moment of + // lapse and gets chased as an expiry bug rather than a stale one. TIME_KEY, dateUtils.getExpiryString( remaining = Duration.between(now, subType.renewingAt) - .coerceAtLeast(Duration.ZERO) ) ) .format() @@ -241,9 +245,9 @@ class ProSettingsViewModel @AssistedInject constructor( is ProStatus.Active.Expiring -> Phrase.from(context, R.string.proExpiringTime) + // Not floored — see the AutoRenewing branch above. .put(TIME_KEY, dateUtils.getExpiryString( - remaining = Duration.between(now, subType.renewingAt) - .coerceAtLeast(Duration.ZERO))) + remaining = Duration.between(now, subType.renewingAt))) .format() else -> "" diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index 4681bf97aa..5d36b4867f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -179,7 +179,7 @@ class ProStatusManager @Inject constructor( ProDataState( type = proStatusState.lastUpdated?.let { (response, confirmedAt) -> response.toProStatus(nowMs, application, refundInProgress, confirmedAt) - } ?: ProStatus.NeverSubscribed, + } ?: seedDisplayStatusFromProof(), showProBadge = showProBadgePreference, refreshState = proDataRefreshState ) @@ -609,6 +609,44 @@ class ProStatusManager @Inject constructor( return proof } + /** + * DISPLAY seeded from the local proof, for when no `get_pro_status` response has ever been persisted. + * + * The bug this exists for: a restored subscriber's settings row read "Upgrade Session" while their + * proof was quietly granting them the features, because DISPLAY came from a response and nothing else. + * A response still WINS wherever one exists — this is a seed, not a second source of truth. + * + * ⚠️ EXPIRY-ONLY, AND THAT IS DELIBERATE. This does NOT use the revocation-aware ACCESS function, and + * must not be "fixed" to. Revocation is an ACCESS concern; the two values are allowed to disagree and + * this is one of the places they may. Matches iOS's `proProofIsActive`, which is expiry-only for the + * same reason. + * + * Returns [ProStatus.Active.FromProof] — no date — because `E`, `G` and the provider are response-owned + * and the proof's own expiry is a different quantity from the plan's payment-due date. + * + * ⚠️ TWO KNOWN GAPS, both reported and neither mine to close here: + * + * - An account holding an access expiry with NO proof reads `NeverSubscribed`, because this consults + * the proof alone. Reachable — the startup gate exists partly for that state, where a config merged + * one without the other. Adding `E` as a second seed source is a contract question for all three + * clients, not an Android fix. + * - An EXPIRED proof also reads `NeverSubscribed`, which is wrong for a subscriber whose proof simply + * has not renewed yet. `Expired` would be equally wrong and needs response-owned dates. Both + * available answers are wrong in different directions, so this takes the one that matches iOS. + */ + private fun seedDisplayStatusFromProof(): ProStatus { + val proof = configFactory.get() + .withUserConfigs { it.userProfile.getProConfig() } + ?.proProof + ?: return ProStatus.NeverSubscribed + + return if (proof.expirySeconds > snodeClock.currentTime().epochSecond) { + ProStatus.Active.FromProof + } else { + ProStatus.NeverSubscribed + } + } + /** * ACCESS as a boolean, for ENFORCEMENT sites that grant or refuse rather than draw. * From 8b9e080a82d646c178f1dcee3a5f0969bf25bd57 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 09:34:11 +1000 Subject: [PATCH 17/23] Pro: the access expiry decides the seeded status, the proof is the fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seeded display status consulted the proof alone, so an account whose config carried the access expiry before the credential read as never having subscribed — the "Upgrade Session" row in front of a paying subscriber. The access expiry is backend-derived plan state and answers the question display asks; the proof is evidence about this device's credential. The ordering is also what makes a valid proof under a past access expiry expressible: display expired, access granted until the proof lapses. A proof-first check collapses that into active. An expired proof now reads expired rather than never-subscribed. A lapsed credential still evidences a subscription that existed, and never-subscribed is a stronger claim than local state supports. Splits ProStatus.Expired the way Active was split, so a status derived from local state can be expressed without a date, and maps a response reporting EXPIRED with no expiry onto the dateless variant. The epoch stand-in it replaces put the Expired CTA's window decades in the past, so the CTA silently never fired. Status only: nothing seeded here carries a date, and the CTA that measures a window now requires the variant that has one. --- .../securesms/home/HomeViewModel.kt | 6 +- .../prosettings/ProSettingsHomeScreen.kt | 2 +- .../chooseplan/ChoosePlanNoBilling.kt | 6 +- .../securesms/pro/ProDataMapper.kt | 32 +++-- .../thoughtcrime/securesms/pro/ProStatus.kt | 79 ++++++------ .../securesms/pro/ProStatusManager.kt | 95 +++++++++----- .../pro/ProExpiredCoverageEndTest.kt | 2 +- .../pro/ProSeededDisplayStatusTest.kt | 116 ++++++++++++++++++ 8 files changed, 250 insertions(+), 88 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/pro/ProSeededDisplayStatusTest.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt index 992a728171..5f34ab5ac5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt @@ -256,7 +256,11 @@ class HomeViewModel @Inject constructor( } } } - else if(subscription.type is ProStatus.Expired + // WithPlan, because the window below is measured from a date only a response carries. + // An expired status derived from local state has no coverage-end instant, and there is + // no safe stand-in: a sentinel puts the window in the past, which suppresses the CTA + // without any sign that a date was missing. + else if(subscription.type is ProStatus.Expired.WithPlan // Only after a SUCCESSFUL get_pro_status request (the round-trip completed and the // backend answered — even if with "expired"/"not pro"), never off stale data from a // failed or in-flight fetch: on foreground the cached status can be pre-renewal, and a diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt index 50ecfc56b7..2f1320cb1c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt @@ -867,7 +867,7 @@ fun ProManage( // detail a response has not supplied — there is no plan to cancel and no payment to // refund — so nothing is offered. This screen fetches on arrival, so it is a transient // state, and the refresh indicator elsewhere is what explains it. - ProStatus.Active.FromProof -> Unit + ProStatus.Active.FromLocalState -> Unit is ProStatus.NeverSubscribed -> { recoverButton() diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNoBilling.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNoBilling.kt index a62ade813f..6b6959c2b4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNoBilling.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/chooseplan/ChoosePlanNoBilling.kt @@ -136,7 +136,7 @@ fun ChoosePlanNoBilling( ) // optional cell 3 - if(subscription is ProStatus.Expired) { + if(subscription is ProStatus.Expired.WithPlan) { add( NonOriginatingLinkCellData( title = Phrase.from(context.getText(R.string.onPlatformStoreWebsite)) @@ -157,13 +157,13 @@ fun ChoosePlanNoBilling( disabled = false, onBack = onBack, headerTitle = headerTitle, - buttonText = if(subscription is ProStatus.Expired) Phrase.from(context.getText(R.string.openPlatformWebsite)) + buttonText = if(subscription is ProStatus.Expired.WithPlan) Phrase.from(context.getText(R.string.openPlatformWebsite)) .put(PLATFORM_KEY, subscription.providerData.getPlatformDisplayName()) .format().toString() else null, dangerButton = false, onButtonClick = { - if(subscription is ProStatus.Expired) { + if(subscription is ProStatus.Expired.WithPlan) { sendCommand(ShowOpenUrlDialog(subscription.providerData.updateSubscriptionUrl)) } }, diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProDataMapper.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProDataMapper.kt index 39f458c2e4..cc2d960a3b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProDataMapper.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProDataMapper.kt @@ -96,18 +96,24 @@ fun GetProStatusResponse.toProStatus( } } - ProUserStatus.EXPIRED -> ProStatus.Expired( - // Both values come off the response, and neither may be read from config: they are only - // meaningful as a PAIR from one response. Not every status branch writes `G` to config, and - // the branches that clear `E` cascade `G` away with it, so a config read would pair this - // response's expiry with a grace period from a different one. - expiredAt = expiry ?: Instant.EPOCH, - gracePeriod = gracePeriod, - providerData = providerMetadata( - latestPayment?.paymentProvider ?: PAYMENT_PROVIDER_GOOGLE_PLAY, - context, - ), - ) + // A response that reports EXPIRED without an expiry describes the status but not the dates, so it + // maps to the dateless variant. The epoch is not a usable stand-in: the Expired CTA's window is + // measured from coverage end, and an epoch anchor puts that window decades in the past, so the CTA + // silently never fires and nothing indicates a date was missing. + ProUserStatus.EXPIRED -> expiry?.let { + ProStatus.Expired.WithPlan( + // Both values come off the response, and neither may be read from config: they are only + // meaningful as a PAIR from one response. Not every status branch writes `G` to config, and + // the branches that clear `E` cascade `G` away with it, so a config read would pair this + // response's expiry with a grace period from a different one. + expiredAt = it, + gracePeriod = gracePeriod, + providerData = providerMetadata( + latestPayment?.paymentProvider ?: PAYMENT_PROVIDER_GOOGLE_PLAY, + context, + ), + ) + } ?: ProStatus.Expired.FromLocalState // "never" + any unrecognized/future slug -> treat as not subscribed. else -> ProStatus.NeverSubscribed @@ -173,7 +179,7 @@ val previewAutoRenewingApple = ProStatus.Active.AutoRenewing( inGracePeriod = false ) -val previewExpiredApple = ProStatus.Expired( +val previewExpiredApple = ProStatus.Expired.WithPlan( expiredAt = Instant.now() - Duration.ofDays(14), // Zero grace, so expiredAt and coverage end coincide: the fixture means what it reads as. gracePeriod = Duration.ZERO, diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt index 5fb5d95d97..55a0f5850b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatus.kt @@ -13,24 +13,19 @@ sealed interface ProStatus{ sealed interface Active: ProStatus{ /** - * Entitled, with NO plan detail known — the DISPLAY seed, derived from a local proof when no - * `get_pro_status` response has ever been persisted. + * Entitled, with no plan detail known: derived from synced config before any response has + * settled the dates. * - * Carries no date DELIBERATELY, and that is the point of it being its own type rather than a - * dated variant holding a sentinel. `E`, `G` and the provider are response-owned: only a response - * carries `latest_payment` and the values that must agree with it, and the proof's own expiry is a - * short clamped credential lifetime rather than the plan's payment-due date — on a compressed QA - * clock they differ by ~300s against ~30 days. Seeding a date from the proof would therefore show - * an account paid through a month as expiring in minutes. + * Holds no date, and cannot, so nothing downstream can render one from it. The plan's dates and + * the provider metadata a rendered date must agree with are settled together by a + * `get_pro_status` response. The proof's own expiry is not a substitute: it is a clamped + * credential lifetime rather than the plan's payment-due date, and the two diverge by orders of + * magnitude under a compressed clock. * - * Because it cannot HOLD a date, no reader can render one. That is why this is a type and not a - * nullable field: `ProSettingsViewModel` already floors a negative remaining duration to zero and - * renders "0 seconds" instead of "Expired", which is what discipline alone achieves here. - * - * `is Active` still matches, so every "are we Pro for display purposes" check keeps working. Code - * that needs the plan's dates must narrow to [WithPlan], and the compiler will say so. + * `is Active` matches, so entitlement checks need no narrowing. Reading a date requires + * narrowing to [WithPlan]. */ - data object FromProof: Active + data object FromLocalState: Active /** * Active WITH plan detail, i.e. sourced from a `get_pro_status` response. @@ -86,29 +81,43 @@ sealed interface ProStatus{ } - data class Expired( - /** - * The payment-due date, as the backend sends it. This is the date to display; coverage ran a - * further [gracePeriod] past it. - */ - val expiredAt: Instant, - val gracePeriod: Duration, - val providerData: PaymentProviderMetadata - ): ProStatus { + sealed interface Expired: ProStatus { + /** - * When access actually ended, and the anchor for any window measuring how long ago that was. + * Expired with no plan detail: derived from local state before any response has settled the + * dates. * - * The backend only reports EXPIRED once coverage has ended, so a window measured from - * [expiredAt] instead is short by exactly [gracePeriod], and empty once grace reaches the - * window length. [gracePeriod] is coverage-past-expiry: the provider's dunning window plus the - * backend's ~1h renewal-latency allowance. It is multi-day once a real dunning window is known - * (Apple states its retry window directly; for Play the backend does NOT follow Play's expiry - * extension — it keeps the reported expiry at the original payment-due instant and carries the - * extension as the grace instead), and ~1h before then. - * - * Derived here rather than at the consumer so a second reader cannot pick the other anchor. + * Carries no date, so no window can be measured from it and no reader can render one. The dates + * a window needs are response-owned, and the alternative — an epoch sentinel — silently produces + * a window that has already elapsed, which reads as "no CTA is due" rather than as missing data. */ - val coverageEndedAt: Instant get() = expiredAt.plus(gracePeriod) + data object FromLocalState: Expired + + /** Expired with the payment dates a `get_pro_status` response carries. */ + data class WithPlan( + /** + * The payment-due date, as the backend sends it. This is the date to display; coverage ran a + * further [gracePeriod] past it. + */ + val expiredAt: Instant, + val gracePeriod: Duration, + val providerData: PaymentProviderMetadata + ): Expired { + /** + * When access actually ended, and the anchor for any window measuring how long ago that was. + * + * The backend only reports EXPIRED once coverage has ended, so a window measured from + * [expiredAt] instead is short by exactly [gracePeriod], and empty once grace reaches the + * window length. [gracePeriod] is coverage-past-expiry: the provider's dunning window plus + * the backend's ~1h renewal-latency allowance. It is multi-day once a real dunning window is + * known (Apple states its retry window directly; for Play the backend does NOT follow Play's + * expiry extension — it keeps the reported expiry at the original payment-due instant and + * carries the extension as the grace instead), and ~1h before then. + * + * Derived here rather than at the consumer so a second reader cannot pick the other anchor. + */ + val coverageEndedAt: Instant get() = expiredAt.plus(gracePeriod) + } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index 5d36b4867f..f339d20cc4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -179,7 +179,7 @@ class ProStatusManager @Inject constructor( ProDataState( type = proStatusState.lastUpdated?.let { (response, confirmedAt) -> response.toProStatus(nowMs, application, refundInProgress, confirmedAt) - } ?: seedDisplayStatusFromProof(), + } ?: seedDisplayStatusFromConfig(), showProBadge = showProBadgePreference, refreshState = proDataRefreshState ) @@ -259,19 +259,19 @@ class ProStatusManager @Inject constructor( refundInProgress = false ) - DebugMenuViewModel.DebugSubscriptionStatus.EXPIRED -> ProStatus.Expired( + DebugMenuViewModel.DebugSubscriptionStatus.EXPIRED -> ProStatus.Expired.WithPlan( expiredAt = now - Duration.ofDays(14), // Zero grace: these fixtures mean "coverage ended N days ago". gracePeriod = Duration.ZERO, providerData = providerMetadata(PAYMENT_PROVIDER_GOOGLE_PLAY, application) ) - DebugMenuViewModel.DebugSubscriptionStatus.EXPIRED_EARLIER -> ProStatus.Expired( + DebugMenuViewModel.DebugSubscriptionStatus.EXPIRED_EARLIER -> ProStatus.Expired.WithPlan( expiredAt = now - Duration.ofDays(60), // Zero grace: these fixtures mean "coverage ended N days ago". gracePeriod = Duration.ZERO, providerData = providerMetadata(PAYMENT_PROVIDER_GOOGLE_PLAY, application) ) - DebugMenuViewModel.DebugSubscriptionStatus.EXPIRED_APPLE -> ProStatus.Expired( + DebugMenuViewModel.DebugSubscriptionStatus.EXPIRED_APPLE -> ProStatus.Expired.WithPlan( expiredAt = now - Duration.ofDays(14), // Zero grace: these fixtures mean "coverage ended N days ago". gracePeriod = Duration.ZERO, @@ -303,7 +303,7 @@ class ProStatusManager @Inject constructor( expiry == null -> this this is ProStatus.Active.AutoRenewing -> copy(renewingAt = expiry) this is ProStatus.Active.Expiring -> copy(renewingAt = expiry) - this is ProStatus.Expired -> copy(expiredAt = expiry) + this is ProStatus.Expired.WithPlan -> copy(expiredAt = expiry) else -> this } @@ -610,41 +610,36 @@ class ProStatusManager @Inject constructor( } /** - * DISPLAY seeded from the local proof, for when no `get_pro_status` response has ever been persisted. + * DISPLAY derived from synced config, for when no `get_pro_status` response has ever been persisted. + * A response wins wherever one exists. * - * The bug this exists for: a restored subscriber's settings row read "Upgrade Session" while their - * proof was quietly granting them the features, because DISPLAY came from a response and nothing else. - * A response still WINS wherever one exists — this is a seed, not a second source of truth. + * The access expiry is consulted before the proof because it answers the question DISPLAY asks. `E` is + * backend-derived plan state that arrived by config sync, so its presence is evidence the ACCOUNT has + * a plan; the proof is evidence about THIS DEVICE's credential. A device restored from a config that + * carried `E` before the proof has a plan to describe, and consulting the proof first would describe + * it as never having subscribed. * - * ⚠️ EXPIRY-ONLY, AND THAT IS DELIBERATE. This does NOT use the revocation-aware ACCESS function, and - * must not be "fixed" to. Revocation is an ACCESS concern; the two values are allowed to disagree and - * this is one of the places they may. Matches iOS's `proProofIsActive`, which is expiry-only for the - * same reason. + * The ordering is also the only one that can express a valid proof under a past `E`: display expired, + * access still granted until the proof lapses. A proof-first check short-circuits to active and the + * state becomes unrepresentable. * - * Returns [ProStatus.Active.FromProof] — no date — because `E`, `G` and the provider are response-owned - * and the proof's own expiry is a different quantity from the plan's payment-due date. + * Status only. The variants returned here carry no dates, because `E`, `G` and the provider metadata + * that a rendered date must agree with are settled together by a response. * - * ⚠️ TWO KNOWN GAPS, both reported and neither mine to close here: - * - * - An account holding an access expiry with NO proof reads `NeverSubscribed`, because this consults - * the proof alone. Reachable — the startup gate exists partly for that state, where a config merged - * one without the other. Adding `E` as a second seed source is a contract question for all three - * clients, not an Android fix. - * - An EXPIRED proof also reads `NeverSubscribed`, which is wrong for a subscriber whose proof simply - * has not renewed yet. `Expired` would be equally wrong and needs response-owned dates. Both - * available answers are wrong in different directions, so this takes the one that matches iOS. + * The proof branch tests expiry alone. Revocation governs access rather than the state of the plan, so + * a revoked proof still describes a subscription that exists. */ - private fun seedDisplayStatusFromProof(): ProStatus { - val proof = configFactory.get() - .withUserConfigs { it.userProfile.getProConfig() } - ?.proProof - ?: return ProStatus.NeverSubscribed - - return if (proof.expirySeconds > snodeClock.currentTime().epochSecond) { - ProStatus.Active.FromProof - } else { - ProStatus.NeverSubscribed + private fun seedDisplayStatusFromConfig(): ProStatus { + val (accessExpiry, proofExpirySeconds) = configFactory.get().withUserConfigs { configs -> + configs.userProfile.getProAccessExpiry()?.let(Instant::ofEpochSecond) to + configs.userProfile.getProConfig()?.proProof?.expirySeconds } + + return seededDisplayStatus( + accessExpiry = accessExpiry, + proofExpiry = proofExpirySeconds?.let(Instant::ofEpochSecond), + now = snodeClock.currentTime(), + ) } /** @@ -850,6 +845,38 @@ class ProStatusManager @Inject constructor( */ private val WAKE_SLACK: Duration = Duration.ofSeconds(30) + /** + * The plan state implied by synced config alone, for display before any response has been + * persisted. Pure over plain values so it can be tested without a clock or config. + * + * The access expiry is consulted before the proof because the two answer different questions: + * `E` is backend-derived plan state that arrived by config sync, so its presence is evidence the + * ACCOUNT has a plan, while the proof is evidence about THIS DEVICE's credential. A device whose + * config carried `E` before the proof has a plan to describe. + * + * The ordering is also what makes a valid proof under a past `E` expressible — display expired, + * access still granted until the proof lapses. Consulting the proof first collapses that state + * into active. + * + * The proof branch tests expiry alone. Revocation governs access rather than the state of the + * plan, so a revoked proof still describes a subscription that exists. + */ + internal fun seededDisplayStatus( + accessExpiry: Instant?, + proofExpiry: Instant?, + now: Instant, + ): ProStatus = when { + accessExpiry != null -> + if (now.isBefore(accessExpiry)) ProStatus.Active.FromLocalState + else ProStatus.Expired.FromLocalState + + proofExpiry != null -> + if (now.isBefore(proofExpiry)) ProStatus.Active.FromLocalState + else ProStatus.Expired.FromLocalState + + else -> ProStatus.NeverSubscribed + } + /** * Whether a cold start should fetch `get_pro_status`, and why — or null to stay off the * network. Pure over plain values so it can be tested without a clock, a database or config. diff --git a/app/src/test/java/org/thoughtcrime/securesms/pro/ProExpiredCoverageEndTest.kt b/app/src/test/java/org/thoughtcrime/securesms/pro/ProExpiredCoverageEndTest.kt index 74c05002d3..0aaeccbcfa 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/pro/ProExpiredCoverageEndTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/pro/ProExpiredCoverageEndTest.kt @@ -24,7 +24,7 @@ class ProExpiredCoverageEndTest { private val ctaWindow: Duration = Duration.ofDays(30) private val paymentDue: Instant = Instant.parse("2026-08-01T00:00:00Z") - private fun expired(grace: Duration) = ProStatus.Expired( + private fun expired(grace: Duration) = ProStatus.Expired.WithPlan( expiredAt = paymentDue, gracePeriod = grace, providerData = previewAppleMetaData, diff --git a/app/src/test/java/org/thoughtcrime/securesms/pro/ProSeededDisplayStatusTest.kt b/app/src/test/java/org/thoughtcrime/securesms/pro/ProSeededDisplayStatusTest.kt new file mode 100644 index 0000000000..d3742f676c --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/pro/ProSeededDisplayStatusTest.kt @@ -0,0 +1,116 @@ +package org.thoughtcrime.securesms.pro + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.thoughtcrime.securesms.pro.ProStatusManager.Companion.seededDisplayStatus +import java.time.Duration +import java.time.Instant + +/** + * The plan state implied by synced config alone, as a pure function of (access expiry, proof expiry, now). + * + * Scope: the ordering and the four outcomes. Reading config and choosing to seed at all are the caller's. + * + * The ordering is the point of these tests. It is the same on iOS and Desktop, so a change here is a + * cross-client divergence rather than an Android preference, and the case that pins it is a valid proof + * under a past access expiry: proof-first collapses that into active and the state stops being + * expressible. + */ +class ProSeededDisplayStatusTest { + + private val now: Instant = Instant.parse("2026-08-17T00:00:00Z") + + private fun inDays(days: Long): Instant = now.plus(Duration.ofDays(days)) + private fun daysAgo(days: Long): Instant = now.minus(Duration.ofDays(days)) + + // --- the access expiry decides whenever it is present --------------------------------------- + + @Test + fun `a future access expiry is active`() { + assertEquals( + ProStatus.Active.FromLocalState, + seededDisplayStatus(accessExpiry = inDays(30), proofExpiry = null, now = now) + ) + } + + @Test + fun `a past access expiry is expired`() { + assertEquals( + ProStatus.Expired.FromLocalState, + seededDisplayStatus(accessExpiry = daysAgo(1), proofExpiry = null, now = now) + ) + } + + @Test + fun `an access expiry with no proof is not never-subscribed`() { + // The restored-device case: config carried the plan before the credential. Answering + // NeverSubscribed here is what put "Upgrade Session" in front of a paying subscriber. + val seeded = seededDisplayStatus(accessExpiry = inDays(30), proofExpiry = null, now = now) + assertEquals(ProStatus.Active.FromLocalState, seeded) + } + + // --- the overhang, which only this ordering can express ------------------------------------- + + @Test + fun `a valid proof under a past access expiry displays as expired`() { + // Display expired while access continues until the proof lapses. Consulting the proof first + // would answer active and the state would be unrepresentable. + assertEquals( + ProStatus.Expired.FromLocalState, + seededDisplayStatus(accessExpiry = daysAgo(1), proofExpiry = inDays(7), now = now) + ) + } + + @Test + fun `a future access expiry wins over an expired proof`() { + // The mirror: the plan is paid up while this device's credential has lapsed between renewals. + assertEquals( + ProStatus.Active.FromLocalState, + seededDisplayStatus(accessExpiry = inDays(30), proofExpiry = daysAgo(1), now = now) + ) + } + + // --- the proof is the fallback -------------------------------------------------------------- + + @Test + fun `a valid proof with no access expiry is active`() { + assertEquals( + ProStatus.Active.FromLocalState, + seededDisplayStatus(accessExpiry = null, proofExpiry = inDays(7), now = now) + ) + } + + @Test + fun `an expired proof with no access expiry is expired, not never-subscribed`() { + // A lapsed credential still evidences a subscription that existed. NeverSubscribed asserts the + // account never had one, which is a different and stronger claim than the local state supports. + assertEquals( + ProStatus.Expired.FromLocalState, + seededDisplayStatus(accessExpiry = null, proofExpiry = daysAgo(1), now = now) + ) + } + + // --- neither ------------------------------------------------------------------------------- + + @Test + fun `no access expiry and no proof is never subscribed`() { + assertEquals( + ProStatus.NeverSubscribed, + seededDisplayStatus(accessExpiry = null, proofExpiry = null, now = now) + ) + } + + @Test + fun `the boundary is exclusive on both branches`() { + // An expiry exactly at now is past, not future — the same comparison on both branches so they + // cannot disagree about the instant they share. + assertEquals( + ProStatus.Expired.FromLocalState, + seededDisplayStatus(accessExpiry = now, proofExpiry = null, now = now) + ) + assertEquals( + ProStatus.Expired.FromLocalState, + seededDisplayStatus(accessExpiry = null, proofExpiry = now, now = now) + ) + } +} From e04af3eccf9369dd66279c2fef765d43e494c39c Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 09:34:22 +1000 Subject: [PATCH 18/23] Pro: rewrite comments that recorded a conversation rather than the code These described who owned an open question, what had been reported, and what was under discussion. That register goes stale on its own: one of them asserted the expired-proof mapping "matches iOS" when iOS returns expired there, and another described a clamp that has since been removed. Rewritten to state why the code is as it is, and to cite what can be checked. The lifetime note keeps the fact that the guard is scoped to the account session and drops the claim about which client is wrong; the pin comments keep the reason the refusal is silent and drop the decision record. --- .../settings/ConversationSettingsViewModel.kt | 4 +-- .../securesms/home/HomeViewModel.kt | 8 ++--- .../securesms/pro/ProStatusManager.kt | 29 +++++++------------ 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt index ff0386f231..19536cc500 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v3/settings/ConversationSettingsViewModel.kt @@ -744,8 +744,8 @@ class ConversationSettingsViewModel @AssistedInject constructor( val maxPins = proStatusManager.getPinnedConversationLimit(proStatusManager.currentUserHasProAccess()) if (totalPins >= maxPins) { - // RULED: an Active plan gets no CTA — see HomeViewModel.setPinned for the reasoning. The - // pin limit lives in two ViewModels and both must apply it. + // No upsell when the plan already reads active — see HomeViewModel.setPinned. The pin limit + // is implemented in both ViewModels, so the condition has to exist in both. if (proStatusManager.proDataState.value.type is ProStatus.Active) return // the user has reached the pin limit, show the CTA _dialogState.update { diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt index 5f34ab5ac5..94b0aa3185 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt @@ -367,10 +367,10 @@ class HomeViewModel @Inject constructor( val maxPins = proStatusManager.getPinnedConversationLimit(proStatusManager.currentUserHasProAccess()) if (pinned && totalPins >= maxPins) { - // RULED: an Active plan gets NO CTA. A subscriber whose proof has not arrived is refused - // the pin by ACCESS above, and offering to sell them the plan they already hold is worse - // than refusing silently. The silence is an accepted trade, not an oversight — the copy - // that would explain a refusal without offering a purchase does not exist. + // No upsell when the plan already reads active. The pin was refused above on access, and + // a subscriber whose proof has not arrived would otherwise be offered the plan they hold. + // The refusal is therefore silent: the copy that would explain it without offering a + // purchase does not exist, and a silent refusal is the lesser of the two. if (proStatusManager.proDataState.value.type is ProStatus.Active) return // the user has reached the pin limit, show the CTA _dialogsState.update { diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index f339d20cc4..b24b9d78cc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -817,26 +817,19 @@ class ProStatusManager @Inject constructor( * `get_pro_status` on every start — exactly the traffic the gate exists to remove, invisible to it * because it is a different trigger. * - * ⚠️ LIFETIME, AND THIS SIDE OF IT IS THE BUGGY ONE. This guard is scoped to the ACCOUNT SESSION, - * not the process: a restore or an account load re-arms it. The collector lives under - * [doWhileLoggedIn], which `AuthAwareComponentsHandler` drives with `collectLatest`, so a new - * `LoggedInState` tears it down and rebuilds it — and `drop(1)` starts again. + * LIFETIME: this guard is scoped to the account session, not the process. The collector runs + * under [doWhileLoggedIn], which `AuthAwareComponentsHandler` drives with `collectLatest`, so a + * new `LoggedInState` tears it down and rebuilds it and the drop starts again. * - * So on a restore the post-restore projection is STRUCTURALLY the dropped one — always, not - * unluckily — and this guard cannot be the thing that discovers a restored subscriber. The - * config-change trigger is the only thing that fetches after a restore, so what a restored - * subscriber actually gets today is: no status until the next cold launch, or until they happen to - * open Pro settings, while the proof quietly grants the features. + * An account load therefore re-arms it, which makes the projection that follows a restore + * structurally the dropped one. This guard cannot be what discovers a restored subscriber, and + * removing the drop is not the way to make it one — that reinstates a fetch on every cold launch. + * Discovering a restored subscriber is a question about whether config is news, which position + * cannot answer. * - * Do NOT "fix" that by removing the drop — that reinstates the every-cold-launch fetch. The fix - * under discussion is a separate trigger asking the SEMANTIC question ("is this config news we have - * never acted on") instead of the positional one, which is unruled and deliberately not built here. - * - * iOS's equivalent flag is per-PROCESS and set eagerly at app setup, so it is already true by the - * time a restore lands and iOS therefore fetches. That is the CORRECT outcome, reached by - * construction order rather than by design — do not port this file's lifetime to iOS to make the - * two match, which would regress iOS onto the behaviour described above. Same concept, same - * comment, different lifetime, and only one of them is right. + * The lifetime is not the same on every client, so it is not safe to assume from the shared name: + * iOS's equivalent flag is per-process (`SessionProManager.swift:550-563`) and is already set by + * the time a restore lands, which is why a restore fetches there. */ internal fun Flow.dropFirstProjection(): Flow = distinctUntilChanged().drop(1) /** From f47838aaeb146285cb7f0ccaef45519c0d8b8955 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 09:41:41 +1000 Subject: [PATCH 19/23] Pro: name the functions the seed's proof rung deliberately does not call Comparing the proof's expiry directly looks arbitrary beside two revocation-aware helpers that appear to answer the same question, and the next reader is as likely to "fix" it into one of them as to leave it. Names both and says why neither belongs: revocation withdraws what the device may do and says nothing about whether the account is paying, and routing a display value through an access function would let an access mock move the menu row. Also states that both rungs compare against the single timestamp passed in. --- .../thoughtcrime/securesms/pro/ProStatusManager.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index b24b9d78cc..b93bdc06f1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -851,8 +851,15 @@ class ProStatusManager @Inject constructor( * access still granted until the proof lapses. Consulting the proof first collapses that state * into active. * - * The proof branch tests expiry alone. Revocation governs access rather than the state of the - * plan, so a revoked proof still describes a subscription that exists. + * The second rung compares the proof's expiry directly, rather than calling + * [currentUserProProofForAccess] or [currentUserHasProAccess]. Both of those are revocation-aware + * and one of them is mockable, and neither property belongs here: revocation withdraws what this + * device may do, while a revoked credential says nothing about whether the account is still + * paying. Routing this rung through an access function would also let an access mock change what + * the menu row says, which would make the two values separate in name only. + * + * Both rungs compare against the one [now] passed in. Reading a clock per branch can straddle + * them, pairing `E` at one instant with the proof at another — a combination no account is in. */ internal fun seededDisplayStatus( accessExpiry: Instant?, From f812a58687917aa73c10b676c0e6d90c7710e650 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 11:02:41 +1000 Subject: [PATCH 20/23] Pro: invalidate the cached self recipient when the proof override changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The composer read the standard limit during the overhang. Its limit comes from the observed self recipient, and step 3 of resolveProStatus reads the mocked proof override — but that preference was not among the flow's change sources, so a resolve cached before the write kept its answer and the grant never reached the render path. Enforcement was unaffected: it calls the access function per decision and reads the preference each time. A grant is what exposes the omission. Withholding access produces the same absent proData as no override at all, so the denying fixture passed whether or not the invalidation worked, and only granting during the overhang could tell them apart. Every render consumer of self shares this flow — the compose limit, the pinned badge, the animated avatar gate — so all of them were stale in the same state. --- .../thoughtcrime/securesms/database/RecipientRepository.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt index e70df5d478..2f63c5abb2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientRepository.kt @@ -229,8 +229,13 @@ class RecipientRepository @Inject constructor( changeSources = if (needFlow) { arrayListOf( configFactory.userConfigsChanged(onlyConfigTypes = EnumSet.of(UserConfigType.USER_PROFILE)), + // Every preference that step 3 of `resolveProStatus` reads has to be here, or a + // resolve cached before the write keeps its answer. A grant is what exposes an + // omission: withholding one produces the same absent proData as no override at + // all, so a missing invalidation looks like a correctly denied fixture. TextSecurePreferences.events.filter { it == TextSecurePreferences.SET_FORCE_CURRENT_USER_PRO + || it == TextSecurePreferences.DEBUG_PRO_ACCESS_OVERRIDE || it == TextSecurePreferences.DEBUG_SUBSCRIPTION_STATUS }, ) From 2087e7dd0c4f550c42f3732a30ee7586462464dc Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 11:24:24 +1000 Subject: [PATCH 21/23] QA: an absent sessionProProof leaves the override alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the clearing added in 7a5e7c2758. Absent and useActual should mean the same thing, but QaLaunchConfig.apply runs on every HomeActivity creation rather than once per test, and on a fresh install the launcher routes to onboarding and HomeActivity is created again afterwards with an intent carrying no QA extras. Clearing on that pass dropped ACCESS while the status and expiry mocks, which only write when present, survived — so the fixture half-applied and the client displayed the mocked plan while behaving as though it held no proof. That is the shape of the overhang failure: display expired, and a composer offering the standard limit because the grant had been wiped after it was set. All four Pro extras now behave the same way, and isolation between tests is the harness's as it already was for the other three: reinstall, or pass useActual. --- .../securesms/qa/QaLaunchConfig.kt | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt index 7249674b64..0476586b72 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt @@ -102,8 +102,9 @@ object QaLaunchConfig { /** * Mocked Pro PROOF, i.e. ACCESS — the harness field `proProof`. * - * `valid` grants access, `none` DENIES it regardless of any real proof, `useActual` (and an absent - * extra) applies no override so the real proof governs. iOS spells this + * `valid` grants access, `none` DENIES it regardless of any real proof, and `useActual` clears the + * override so the real proof governs. An ABSENT extra leaves the stored value untouched, matching + * every other Pro extra here — see [applyProProof] for why absent cannot mean `useActual`. iOS spells this * `mockCurrentUserSessionProProof` and Desktop `SESSION_PRO_MOCK_PROOF`; the harness field name is * what is identical across clients, the app-side literal follows each platform's own convention. * @@ -464,16 +465,17 @@ object QaLaunchConfig { */ private fun applyProProof(intent: Intent, prefs: TextSecurePreferences): Boolean { if (!intent.hasExtra(EXTRA_PRO_PROOF)) { - // ABSENT CLEARS, because the contract says absent is identical to `useActual` and this - // preference PERSISTS across launches. Returning early instead would leave the previous - // launch's override in place, so a spec that set `valid` would silently make every later - // spec on the same device Pro — the leak Desktop hit needing `PRO_ENV_KEYS`. Android has no - // reset list; absence has to do that work. + // Absent leaves the stored override alone, like every other Pro extra here. // - // Note this clears only the launch-extra override. The debug menu's own - // `forceCurrentUserAsPro` toggle is a separate preference and is deliberately untouched, so - // a developer's setting survives a relaunch as they would expect. - prefs.setDebugProAccessOverride(null) + // Clearing instead looks right — absent and `useActual` should mean the same thing — but + // [apply] runs on every HomeActivity creation, not once per test. On a fresh install the + // launcher routes to onboarding and HomeActivity is created a second time afterwards, with + // an intent that carries no QA extras. Clearing on that pass drops ACCESS while the status + // and expiry mocks, which only write when present, survive: a fixture then half-applies, + // and the client displays the mocked plan while behaving as though it holds no proof. + // + // Isolation between tests is therefore the harness's, exactly as it already is for + // `EXTRA_PRO_BACKEND_STATUS` and the rest — reinstall, or pass `useActual` to clear. return false } From 2211bc99ab7243123edca4d76e392d0b74f57f3a Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 12:51:33 +1000 Subject: [PATCH 22/23] Pro: evaluate the startup gate when the app becomes visible, not only at launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The expiring CTA arms seven days before the access expiry, while the wakes that survive backgrounding fire at the expiry and at coverage end — after that window has already opened. A subscriber who entered it while the app was merely backgrounded was therefore not warned until the process next started cold. The same gate at a second moment: no new predicate and no second trigger. Its persisted 24h interval is what keeps this cheap, since an evaluation inside that interval declines before it reads config, so this cannot become a fetch on every foreground. Reuses AppVisibilityManager rather than adding a lifecycle observer. Because that is a StateFlow, collection emits the current value and the launch evaluation runs the same code path as every later one — with the one difference that a login completing while backgrounded now defers its evaluation to the first foreground rather than running immediately, which is where the CTA it exists for can be seen. --- .../securesms/pro/ProStatusManager.kt | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt index b93bdc06f1..b78679c5d0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/pro/ProStatusManager.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull @@ -70,6 +71,7 @@ import org.thoughtcrime.securesms.pro.api.ServerApiRequest import org.thoughtcrime.securesms.pro.db.ProDatabase import org.thoughtcrime.securesms.pro.subscription.ProSubscriptionDuration import org.thoughtcrime.securesms.pro.subscription.SubscriptionManager +import org.thoughtcrime.securesms.util.AppVisibilityManager import org.thoughtcrime.securesms.util.State import org.thoughtcrime.securesms.util.castAwayType import java.time.Duration @@ -93,6 +95,7 @@ class ProStatusManager @Inject constructor( private val snodeClock: SnodeClock, private val proStatusRepository: Lazy, private val configFactory: Lazy, + private val appVisibilityManager: AppVisibilityManager, ) : AuthAwareComponent { val proDataState: StateFlow = loginState.flowWithLoggedInState { @@ -415,7 +418,21 @@ class ProStatusManager @Inject constructor( // (see manageProofRenewalScheduling) and nothing else. A status fetch on the proof's clock // couples the two loops, so a proof renewing early or late drags the status fetch with it. - startupGate() + // Evaluated when the app becomes visible, not only when this collector starts. The + // expiring CTA arms seven days before the access expiry, while the wakes that survive + // backgrounding fire AT the expiry and at coverage end — after that window has opened. A + // subscriber who enters it while the app is merely backgrounded would otherwise not be + // warned until the process next started cold. + // + // The same gate, at a second moment: no new predicate and no second trigger. Its persisted + // 24h interval is what keeps this cheap — an evaluation inside that interval declines + // before it reads config — so this cannot become a fetch on every foreground. + // + // `isAppVisible` is a StateFlow, so collection emits the current value and the launch + // evaluation is the same code path as every later one. + appVisibilityManager.isAppVisible + .filter { it } + .flatMapLatest { startupGate() } ).debounce(500.milliseconds) .collect { refreshReason -> Log.d( From cb59ca9b36cb48aec407f544a66577549152af02 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 17 Aug 2026 15:00:07 +1000 Subject: [PATCH 23/23] QA: identify the Pro settings hero description The three status messages share one slot and are told apart by their text, so the id goes on the node carrying the message. --- .../preferences/prosettings/ProSettingsHomeScreen.kt | 3 +++ content-descriptions/src/main/res/values/strings.xml | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt index 2f1320cb1c..58869c430d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/prosettings/ProSettingsHomeScreen.kt @@ -219,6 +219,9 @@ fun ProSettingsHome( } Text( + // One id for the slot: the three messages above are told apart by their text, so it + // belongs on the node that carries the message. + modifier = Modifier.qaTag(R.string.qa_pro_settings_description), text = Phrase.from(context.getText(headerText)) .format().toString(), style = LocalType.current.base, diff --git a/content-descriptions/src/main/res/values/strings.xml b/content-descriptions/src/main/res/values/strings.xml index 8242a0283b..f63792b783 100644 --- a/content-descriptions/src/main/res/values/strings.xml +++ b/content-descriptions/src/main/res/values/strings.xml @@ -373,6 +373,11 @@ message and must not gain a contentDescription — that would be a second source of truth for the same string, and the first thing to rot when the copy changes. Same string on iOS. --> pro-settings-status-banner + + pro-settings-description action-item-title action-item-subtitle action-item-icon