Skip to content

Pro: separate the status and proof refresh loops, and make the freshness floor real - #2155

Draft
mpretty-cyro wants to merge 3 commits into
session-foundation:devfrom
mpretty-cyro:feature/pro-status-refresh-unification
Draft

Pro: separate the status and proof refresh loops, and make the freshness floor real#2155
mpretty-cyro wants to merge 3 commits into
session-foundation:devfrom
mpretty-cyro:feature/pro-status-refresh-unification

Conversation

@mpretty-cyro

Copy link
Copy Markdown
Collaborator

Android's half of the cross-client Pro status-refresh unification. Desktop and iOS carry the same
spec; where a decision was cross-client it was routed rather than settled here.

🔴 Known flaw — do not merge without reading this

This branch ships code that encodes a premise we have proved false. It does so deliberately, so
that all three clients stay consistent until the correction lands on all three together — but a
reviewer must not merge it believing the grace handling is correct.

get_pro_status.expiry_ts is grace-inclusive. The backend folds the grace period into the stored
expiry before sending it (Session-Pro-Backend backend.py _lookup_user_expiry
users.expiry_atserver.py:317), and judges active against that same value (:322). Its own
test subtracts grace from the wire value to recover the store's paid-through date
(tests/test_google.py:556-560). So E is coverage end, not paid-through, and now ≥ E is
grace's exit, not its entry.

What that means for this branch:

  • inGracePeriod = now >= expiry (pre-existing, ProDataMapper) sits inside a branch requiring
    now <= expirythe grace indicator is effectively dead code.
  • renewingAt = expiry renders the renewal date one whole grace period late.
  • The new grace poll (New group implementation #4) and the grace-label debounce both key off the same instant, so both
    currently use the superseded reading.

Why it is not fixed here: it is a user-visible date, and Desktop and iOS encode the identical
premise — in three different wordings, which is why it read as corroboration rather than one mistake
copied three times. Fixing one client alone makes the three disagree about what "in grace" means.

What has been done instead: the contested instant is isolated behind renewalDueAt() /
coverageEndsAt() in ProSettingsViewModel, so the correction is two one-line bodies; and the
false premise is marked in place at its source in ProDataMapper, with the superseded text kept
verbatim so it stays greppable across the three clients.

This is tracked as F8, with the architect. Do not merge until it is ruled on.

The startup gate IS in this branch, so row 1 of the gate is affected too. The real grace window
(E − grace ≤ now < E) lies entirely inside row 1's now < E, the row that declines to fetch — so the
gate is blind to precisely the state this rework exists to surface. Built that way deliberately:
Desktop and iOS ship the identical row, and correcting it is a three-client change. Two tests pin the
blind spot and will fail when F8 lands, which is the intended signal, not a regression.

Magnitude, per provider — this is not uniformly cosmetic. Apple configures no grace, so the
backend's own ~1h stand-in is the whole of it and the renewal date is an hour out. Google's real
grace is an operator-configured base-plan setting measured in days
, fetched and stored at the moment
the user enters grace (providers/google_play/notifications.py:704-718). So on Android — a Google Play
client — the renewal date can be days wrong, on the screen whose purpose is that date, and it is
largest exactly when someone is looking at it.

⚠️ Merge dependencies — this PR cannot land alone

# Dependency State
1 session-android #2153 (feature/pro-backend-override-qa) — this branch is cut from its head 445b74cc89, which carries the proof-acquisition floor open
2 libsession-util #121 (JasonFork/pro-auto-renewing-config, head 8e5634b8) — adds the A / auto_renewing user-profile key open
3 A LibSession-Util-Android release exposing #121, plus the libsessionUtilAndroidVersion bump in gradle/libs.versions.toml not started

Merge order is 2 → 3 → 1 → this. Until 3 exists the auto_renewing read switch cannot be built,
which is why only the write side is in this PR (see Deliberately not included).

Not a dependency of this PR — conditional, and listed only so the link isn't lost

user_profile_has_pro_auto_renewing() — a libSession accessor answering "has A ever been
written?"
, built and tested on feature/pro-auto-renewing-tristate (off 8e5634b8), unpushed and
unreviewed.

This branch does not call it and compiles and behaves correctly without it. A is presence-only,
so a stored false and a never-written one are indistinguishable through the plain accessor — which
is all this branch uses. The new accessor becomes a dependency only if the architect rules that
the startup gate must bootstrap a fetch when A has never been written (tracked as F2). If that
ruling lands, this PR additionally needs the accessor plus an Android JNI mirror of it.

Note what it does and does not make possible: "has anyone ever written A?" becomes answerable, so
bootstrap-once-then-trust is implementable. "Is this user explicitly not auto-renewing?" remains
unanswerable — that would need A to store 0 rather than erase, i.e. a wire-encoding change.

Not the -pfs branch. JasonFork/pro-auto-renewing-config-pfs has an identical commit subject and
a different sha, and is not a descendant — it rebases the change onto the PFS track. gh pr view 121 --json headRefOid confirms 8e5634b8.

What this changes

Entitlement is the proof; display is the status. They no longer drive each other.

  • Deleted the proof − 1h → status fetch trigger (source Add "is_deleted" column to sms/mms #3 of manageProStatusRefreshScheduling).
    A status fetch keyed to proof timing meant a proof renewing early or late dragged the status fetch
    with it.
  • Removed the proof-success → status refresh. The worker already writes E from the proof
    response, and that write fires the config-change trigger on its own.
  • Proof scheduling moved out of FetchProStatusWorker into ProStatusManager.manageProofRenewalScheduling,
    which watches the three config inputs to libsession's pro_renewal_target — the stored proof, E,
    and the prepaid marker. Proof renewal no longer depends on a status fetch having happened.
  • forceimmediate, with a closed, documented caller list: manual/recover, the post-purchase
    poll, and the bounded grace poll. Every routine trigger now goes through the floor.
  • Pro settings freshness: refresh on entering the screen (floored), plus a bounded grace poll that
    sleeps until the renewal falls due, then polls once a minute until it lands, coverage ends, or the
    screen closes.
  • Grace-warning debounce: the in-settings "renewal unsuccessful" label now requires a completed
    fetch at or after the renewal became due. Previously a status snapshot predating the crossing could
    raise an alarm about an event it could not have observed.
  • Startup fetch gated on whether a home CTA could plausibly fire, computed from synced config, with
    a persisted 24h minimum in its own attempt-stamped key. Non-subscribers and comfortably-active users
    no longer fetch on every cold start.
  • A confirmed status now means confirmed by THIS process — see the fourth bug below.

Bugs found and fixed along the way

Four defects in this path had one cause: a state meaning "we have not confirmed anything" being
compared against an enum that does not enumerate it
, so it fell into whichever branch happened to
catch it. Twice it fell outside the compared cases and disabled a check; twice it fell inside a
success-shaped default and satisfied one.

  • The freshness floor never applied on a cold start. It asked whether the load state was Loading
    or Loaded; Init is neither, so the check was skipped on exactly the path it exists for. It now
    reads the persisted timestamp. Covered by ProStatusFreshnessFloorTest.
  • A proof renewal that came due while the app was dead never ran. ProProofGenerationWorker gated
    on the in-memory load state holding an ACTIVE status, but WorkManager persists its schedule across
    process death and loadState restarts at Init. It now asks libsession the same question the
    scheduler does.
  • The false-expired protection was resting on an accident. HomeViewModel's Expired-CTA gate
    (refreshState is State.Success) never enforced what its comment claims, because the mapping ended
    in a catch-all that swept up Init. What actually suppressed the CTA was the unconditional startup
    fetch driving a Loading transition — so gating startup would have removed it.
    ProStatusRepository.fetchedInThisProcess guarantees the first request of a process still reaches
    the network; its comment records that it is load-bearing and why.
  • A "confirmed" status could come from a previous process — this one was live in production. The
    same catch-all also swept up a Loaded restored from WorkManager's persisted work state, so at
    process start, before asking anyone anything, the refresh state read as Success against a cached
    response. A renewal that happened while the app was closed therefore splashed the Expired CTA off
    the stale cache on the next launch — reported in the wild by the architect. LoadState.Loaded now
    carries whether the fetch landed in this process, and the catch-all is gone: the mapping is
    exhaustive, so the next state added has to declare which it is. That last part is the durable fix —
    the else is what made two different non-successes indistinguishable.

The floor's timestamp is stamped on attempt, not success, and needed a new pro_state key:
pro_status_updated_at is written in the same statement as the response blob and is unreadable
without it, so a failed fetch had nothing to record and a failing network was never throttled.

Deliberately not included

  • The gate's !auto_renewing && now < E && outside the CTA window row — held (F2). It returns "no
    fetch", the spec's letter, but A is presence-only so that also covers "never written", which is
    where every existing subscriber lands on their first run. Whether it needs a bootstrap fetch is a
    decision, not an implementation; a test pins the current answer.
  • Reading auto_renewing from config (ProDataMapper). The write side IS included — every
    get_pro_status now persists it alongside E. The read switch is held because autoRenewing is
    the branch selector for the construction F8 rewrites, under the comment F8 invalidates: landing it
    first would produce a diff whose justification the next diff deletes.
  • The grace-boundary correction. get_pro_status.expiry_ts is grace-inclusive (the backend
    folds grace into the stored expiry before sending it; verified in Session-Pro-Backend and by its
    own test), so now ≥ E is grace's exit rather than its entry. All three clients currently encode the
    opposite, including in merged code. This PR keeps the existing (spec) reading and isolates the
    contested instant behind renewalDueAt() / coverageEndsAt() in ProSettingsViewModel, so the
    ruling is a two-line change. Do not fix this on one client alone — it is a user-visible date.
  • Soft/hard warning copy — new strings are Crowdin-owned and the wording is a product decision.

Testing

:app:testPlayDebugUnitTest207 tests, 0 failures, verified against result-file mtimes rather
than counts alone (TEST-*.xml persists across runs that never happened, so counts alone are not
evidence of a run).

ProStatusFreshnessFloorTest covers the floor decision as a pure function, including both exemptions
and their negative controls. Mutation-checked: breaking the boundary or the absent-timestamp answer
turns exactly the intended cases red.

Note for reviewers: this branch also retires a stale baseline — ConversationViewModelTest's 9
known failures on dev are fixed by 59060641d2, so the suite is fully green here.

Note: This is based on https://github.com/mpretty-cyro/libsession-util/pull/2153

Lets a QA Pro backend be targeted without rebuilding, matching the iOS
customProBackendUrl/customProBackendPubkey launch variables.

Both values are required together: a QA URL paired with the production signing
key reads every QA-signed proof as invalid and silently strips Pro content.
…ation

Reading a SessionProtocol constant runs System.loadLibrary("session_util"), so
doing it from the companion's initialiser made the class impossible to
initialise wherever the native library is absent -- every JVM unit test. Mockito
could not instrument it, and the nine tests constructing a ConversationViewModel
failed with NoClassDefFoundError.

The constants stay single-sourced from libsession; they are just read on first
use rather than on class load.
A successful generate force-refreshes get_pro_status, which asks libsession for a
renewal target, and `proofExpiry - PRO_RENEWAL_LEAD` is permanently in the past for
any proof living less than the 60-minute lead -- so the worker rescheduled itself
immediately and looped.

Mirrors iOS SessionProManager.reconcileProofRenewal and Desktop, constants included:
60s while covered, 15s * attempt capped at 900s while dark, and re-arming rather than
dropping the work, since `target <= now` is also the normal renewal-due signal.

The state is in-memory as it is on the other two platforms; a process restart costs
one extra request rather than a loop.
@mpretty-cyro mpretty-cyro self-assigned this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant