Pro: split access from display, and stop upselling an active plan by mpretty-cyro · Pull Request #2164 · session-foundation/session-android

mpretty-cyro · 2026-08-14T07:04:34Z

Draft — parked for review, not ready to merge.

Splits Pro into two values: ACCESS (the proof in config, validated on every read against expiry and the revocation list) and DISPLAY (the backend status, seeded from the proof at launch with no date). They are meant to disagree — a lapsed plan whose proof still has time left displays as expired while the features keep working.

Also here: upsell affordances read DISPLAY rather than inverted ACCESS, so a user whose plan reads active is not invited to buy Pro they already have; and a test mock for the proof, separate from the status mock, which is what makes an active-plan-with-no-proof client reachable at all.

Deliberately not done — held items and accepted trades are recorded in the code comments, including a refusal that is currently silent because no copy exists for "your plan is active but we cannot verify it yet".

Client-side counterpart to session-appium#128.

`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.

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.

`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.

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.

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.

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.

…forced 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.

…ng ACCESS `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.

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.

…roof drives ACCESS Replaces the additive shape from edfbbbc. 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.

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.

…DISPLAY 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.

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.

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.

"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.

…ve an Active plan no CTA 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.

mpretty-cyro added 16 commits August 14, 2026 12:50

Navigation Menu

Uh oh!

Pro: split access from display, and stop upselling an active plan - #2164

Pro: split access from display, and stop upselling an active plan#2164
mpretty-cyro wants to merge 16 commits into
session-foundation:devfrom
mpretty-cyro:fix/expiring-cta-confirmed-fetch

mpretty-cyro commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant