Pro: unify the get_pro_status refresh discipline (Desktop) - #1976
Merged
jagerman merged 26 commits intoAug 13, 2026
Conversation
mpretty-cyro
marked this pull request as ready for review
August 10, 2026 06:56
Session Pro is launching, so the proAvailable feature flag is permanently on.
Rather than leave a dead always-true flag wrapping every call site, remove it
entirely: delete the flag (from defaultFeatureFlags and the flag type union)
and collapse every getFeatureFlag('proAvailable') / getIsProAvailableMemo()
consumer to unconditional Pro-available behaviour, dropping the dead pre-launch
branches and now-unused imports. Dev-only flags (useTestProBackend,
proGroupsAvailable, mock*) and the debug menu itself stay gated.
Desktop's half of the cross-client refresh unification. Entitlement stays the proof; get_pro_status becomes display + account-expiry awareness only. Depends on libsession session-foundation#121 (auto_renewing, config key A) plus a libsession-util-nodejs release carrying the matching glue. The pin here has neither, so the auto_renewing lines do not yet typecheck. - Add a 60s status floor. Desktop had none, only an isFetching single-flight, so every trigger fetched. Drop-on-fresh rather than re-arming, or the routine triggers become a self-sustaining once/60s poll during grace. The timestamp is persisted, because a per-run one resets on the cold start it exists to cover. - Add `immediate` to bypass it, reserved to manual/recover, the bounded #4 poll and dev paths. Named for misuse-resistance: `force` invited every routine trigger to pass it, which is how Android's floor became dead code. - Exempt a process with no confirmed status of its own. The persisted floor is the only refuser that outlives the process, so without this it can decline the first fetch of a run that has no status at all: a rate limit acting as a mutex. - Gate the startup fetch on whether a home CTA could fire, at most once/24h, from synced config. Non-Pro users never fetch. Deliberately not `E + grace <= now`: grace is not knowable until you are in it, and session-foundation#121 adds auto_renewing only. - Add the single wake past the account horizon. The while-open poll is screen-scoped, so with the Pro page closed nothing re-checked between E and the proof loop's own wake ~1h before proof expiry. - Write auto_renewing into synced config on each fetch, unconditionally. libsession de-dupes, and a presence-based guard would be wrong since writing false erases the key. - Release the single-flight when the thunk rejects. isFetching was cleared only by the result landing via `fulfilled`, and post-processing runs outside the internal try/catch, so a throw left it set and silently disabled every later trigger. A latent defect whose surface this branch widens by one awaited call. - Name the floor, interval and CTA-window constants as a cross-client contract, and isolate the contested renewal instant behind `renewalDueAtMs`.
…ctly The config-change status trigger compared only the access expiry (E), so a prepaid marker (I) synced from another device's purchase produced no refresh. Watch both, as Android always has. Deliberately not watching auto_renewing: a change there re-derives the display from the value just received, so fetching to confirm it would be a pointless round trip. Call reconcileProProof() directly on an E change rather than relying on it as a side effect of the status refresh. It previously arrived that way, because every completed fetch ends with a reconcile, but a floored fetch never reaches that callback, so introducing the floor would have silently dropped the nudge on exactly the path that needs it: several config pushes arriving together. The edge is load-bearing, since with a lapsed account and an expired proof the renewal target returns nothing and the loop sits dormant with no wake of its own.
…CTAs The Expiring/Expired CTAs were gated on `fromAppStart`, which worked only because startup fetched unconditionally: every non-startup caller was therefore guaranteed that a fetch had completed or was in flight. Gating the startup fetch removes that guarantee and leaves two callers, opening a conversation and closing the settings modal, able to display a CTA from a flag a previous run persisted with nothing confirming it this session. That is the false-expired case the guard exists to prevent. Gate on a real predicate instead, computed inside the function rather than passed in, so a new call site cannot inherit an answer by copying a neighbour. Keep `fromAppStart` where it *enables* the donate CTA: that is a different question, and collapsing the two would have silently disabled it at startup.
- Trigger #4's while-open grace poll is exempt from the floor, and its interval is named. GRACE_POLL_INTERVAL_MS equals STATUS_FLOOR_MS, which is why it must bypass rather than be silently halved; the two constants sat in different files, so the equality is now stated where it matters. Its `!autoRenew` guard and the exemption are a pair: the guard is what bounds the poll, and being unfloored is what makes an unbounded version expensive. - The settings modal's on-open refresh drops its own once-a-minute throttle. That read the per-run `details.lastFetchedMs`, which is 0 again after every restart, so relaunching defeated it. The persisted floor is the same 60s and survives. - The two debug refresh buttons pass `immediate`, so a developer path is not silently floored for 60s while you are watching a backend change.
`expiry_ts` is coverage end, not the date the renewal falls due: the backend folds the grace period into the stored expiry for auto-renewing subscriptions and judges `user_status` against that same value. So the renewal is due at `E - G`, and the grace window — overdue but still covered — is `E - G <= now < E`. - The displayed renewal date subtracts grace. Unconditional, no provider branching: the wire sends `grace = 0` when the subscription isn't auto-renewing, so it is `E - 0 == E` for those accounts. - The in-settings grace indicator was keyed on `now >= E` inside a branch that requires `now <= E`, satisfiable only at a single instant — so it was unreachable and nothing surfaced an overdue renewal at all. It now keys on the renewal date, with an upper bound at coverage end so it stops rather than bleeding into genuine expiry. - The warning's debounce moves with it: it requires a fetch that COMPLETED at or after the renewal date. Keyed on `E` it would have fired a grace period late, which is harder to notice than not firing. Both values here are milliseconds, from the response's own units. Core stores the grace period in seconds and the wrapper converts, so config-sourced grace is on the other side of that boundary.
Every path that writes the access expiry now writes the auto-renewing flag and the grace period alongside it, from the same response. Previously the proof-success path wrote the expiry alone, with two consequences: an account whose expiry came only from a proof read back as terminal while it was in fact renewing (the flag is stored presence-only, so unwritten and false are the same bit), and `E - G` could pair a fresh expiry with a grace learned in a different billing period. The startup gate's comfortably-active row now compares against the renewal date rather than coverage end, so it no longer declines to fetch for the whole grace window, and the single wake past the horizon moves with it. All three proof-path writes are conditional on the field being present, and that is load-bearing rather than defensive: the two new fields are advisory optionals where absent means the backend did not say, NOT false or zero. Both config keys are presence-only, so a collapsed `?? false` / `?? 0` would erase them — against a backend predating either field, every proof fetch would wipe a value a get_pro_status fetch had correctly learned. That is worse than not writing at all. The wrapper surfaces both as nullable rather than as a value plus a has_ flag so the collapse is a type error rather than a convention. Depends on the libsession branch carrying the grace config key and the proof-response parse, plus a libsession-util-nodejs release exposing both; the pin here has neither.
The single wake past the horizon becomes two instants, `(E - G) + 30s` and `E + 30s`, because two distinct transitions matter and neither implies the other: whether the charge succeeded when it fell due, and whether grace ran out without a recovery. They collapse to one when the grace period is zero, which is every non-auto-renewing account. The second is not redundant with the proof loop. That loop does wake near coverage end by construction — the backend issues the proof expiry about an hour past the account expiry, and the renewal target is about an hour before proof expiry — and a proof outcome now writes the account fields, which fires the config-change trigger. But that chain is gated on the expiry actually changing, so it covers a renewal that succeeded and not one that failed, which is the case the grace warning exists for. Both instants are re-derived from scratch on every call, so a renewal that advances the expiry cannot leave a wake armed against the old horizon.
Both wake instants emit through the floored fetch path, and they sit one grace period apart. So when the grace period is shorter than the 60s status floor the second wake fires and its fetch is dropped — the timer runs, nothing happens, and no log line distinguishes that from a wake that was never scheduled. Production grace is at least an hour, so this is a test-environment property rather than a wake defect: the Google test provider sets grace to about ten seconds, and the backend deliberately scales its whole proof and renewal clock for compressed runs while this client-side constant does not participate in that compression. The second wake is simply where the mismatch first became visible. Documented at the floor constant, for whoever tunes or removes it, and again in the wake's own doc comment, for whoever debugs the missing fetch — the two readers arrive from opposite directions and neither has reason to open the other's file. Both notes name an env-var override of the floor as the sanctioned escape hatch, owned by the Pro UI-test work and deliberately not built here, so the gap is not mistaken for an oversight or "fixed" in the client.
…ll guards libsession made account_auto_renewing and account_grace_period_duration mandatory on a successful proof parse, so the null checks around those two writes tested a state that cannot occur. The erasure hazard they guarded has moved rather than gone. Core leaves both at their struct defaults on a failure outcome, where false and zero are indistinguishable from a backend that really said "not renewing, no grace" — and both config keys are presence-only, so writing either would erase what a get_pro_status fetch had learned. What protects that now is placement: the writes sit inside the success branch, which is where the fields are guaranteed and the only place they mean anything. The comment says so, since a null check no longer does. The access-expiry guard stays: that field is still optional in core, absent on the not_subscribed and revoked outcomes.
libsession erases the auto-renewing flag and the grace period alongside the access expiry whenever the expiry is cleared, so a renewing flag with no expiry beside it is not a reachable state. Every caller that clears the expiry is handling an account with no entitlement, and none of those is auto-renewing. Recorded at the only reader because the startup gate also happens to check the expiry first, and that ordering could be mistaken for what makes the flag safe. It isn't, any more: the write side maintains the invariant, so a future second reader doesn't inherit an obligation it has no way to know about.
No behaviour change. The ternary and the push form produce identical arrays: the single-instant branch only runs when the two values are equal, so which of them it named made no difference. Worth changing anyway because it was the only place an identical behaviour looked different across the three clients, and a reviewer arriving from another platform had to trace both paths to prove equivalence rather than reading it.
`expiry_ts` is the account's true paid-through expiry — what the user has paid for — and the backend keeps serving for `grace_period_duration` past it, judging active/expired against that later instant. The previous model had this inverted: it treated `E` as coverage end and derived the renewal date as `E - G`. Every consequence of the sign flips with it: display date `E - G` -> `E`, with no arithmetic at all coverage end `E` -> `E + G` grace window `[E-G, E)` -> `[E, E + G)` wake instants `(E-G)+30s`, `E+30s` -> `E+30s`, `(E+G)+30s` grace debounce `lastFetch >= E-G` -> `lastFetch >= E` The startup gate loses its grace read entirely rather than flipping it: coverage ends at `E + G`, but grace and post-coverage both resolve to "fetch", so `E` is the only instant that changes the answer. The display now reads the same value the rest of the selector uses, so a mocked expiry drives the shown date instead of being bypassed. Comments are rewritten rather than patched around the arithmetic — the old ones explained the inverted model at length, and a confidently-worded wrong explanation is worse than none.
The `get_pro_status` success path wrote `E` and `A` but never `G`, so the only writer of the grace key was the proof path. Nothing else populates it, which left every config-only consumer computing `E + 0` and made the wake's coverage-end instant coincide with its expiry instant — the second wake silently collapsing into the first. All three keys are now written from the same response, which is the only way they are jointly meaningful: `E + G` is coverage end, so a fresh `E` beside a grace left over from an earlier subscription state describes an instant that was never true. Reads the ACCOUNT-level `gracePeriodDurationMs` at the response root, not `latestPayment`'s field of the same name. The payment-level one is a single store's raw declaration and is not gated on auto-renewal — a subscriber who cancels mid-retry keeps a nonzero value in it, and using it would place coverage weeks past the truth.
Applies the house comment rule to this branch's own comments: why,
complexity and intent only, never a description of the change that
introduced them.
Six comments asserted what another client or repo currently does
("which is how Android's 60s floor ended up dead", "matching iOS").
Those cannot detect their own staleness from here — this repo's CI
never builds that file — so each keeps its hazard and drops the
attribution. The cross-client constants keep the fact that they ARE a
contract, which is the part a reader can act on.
The rest were narration: "this path used to write E alone", "replaces
the unconditional dispatch", "was tried". Each restated as the
invariant that makes the current code correct, so it still reads as
true to someone who never saw the diff — and in two places that turned
out to be the stronger statement, naming what a future writer would
break rather than what a past one did.
One correction of substance rather than style: the note on reading `A`
implied the gate's `E`-first ordering upholds the "A cannot outlive E"
invariant. It doesn't — core's clear cascade does. A reader could have
added a dependency on that ordering.
The note on reading `auto_renewing` asserted that libsession erases `A` alongside `E`. That is a claim about an unmerged branch in another repository: nothing here builds that file, so if the cascade lands in a different shape — or not at all — the comment goes quietly wrong and this repo has no way to notice. Restated as the invariant this file actually depends on: `A` is presence-only, so it is meaningful only beside the `E` it describes, and the one caller establishes `E` first. That is verifiable in this repo, stays true whether or not core adds the cascade, and tells a new caller what it has to do.
…ger fire Both rows of the startup gate were bounded below and unbounded above, so an account that lapsed years ago passed the gate on every cold start — one fetch per 24h, forever, to decide whether to raise a CTA whose own window closed long before. Bounded at `E + G + 30d`, the last instant an Expired CTA can be raised, matching iOS and Android. The constant was already here and already used by handleExpiryCTAs; only the gate never picked it up. Measured from coverage end rather than from `E`: an account is not lapsed until `E + G`, so measuring elapsed time from `E` would cut a renewing account's window short by exactly its grace period. This is the one place the gate needs the LENGTH of grace — every lower bound needs only "has the paid term ended", which is why they still read `E` alone.
Expired CTA fires while E + G <= now < E + G + 30d
Expiring CTA fires while E - 7d <= now < E
The displayed date stays E.
The Expired window was measured from `E`, but the backend reports
Expired only once coverage has ended at `E + G`, so the window a user
actually got was `[E + G, E + 30d)` — `30d - G` long, and empty when `G`
reaches 30 days, which a configured store grace period can. An account
with 16 days of store grace got 14 days of CTA instead of 30, and the
accounts affected are exactly the auto-renewing ones in dunning, which
are the ones most likely to recover.
The deadline now also equals the cold-start gate's upper bound, so the
gate no longer outlives the CTA it exists to serve.
Grace comes from the same response as the expiry rather than from
config: the other arguments here already do, and `E` and `G` are only
jointly meaningful read together — the Never/Expired paths clear config
and the unknown-status path writes nothing, so a config read here would
pair this response's `E` with an older `G`.
The Expiring window is untouched: it is a statement about the payment
date, not about coverage.
The backend forces `user_pro_status = Expired` on revocation, after the expiry and grace values are computed, so a refunded or charged-back account reports Expired while its coverage end is still in the future. The Expired CTA required `E < now`, so for those accounts it never appeared: the date chain saw an account inside its active or expiring window and treated it as one. Now keyed on the reported status, bounded above by the same deadline as before, and placed ahead of the date chain because that is the only position from which it can see a revoked account. The chain's own Expired branch is removed rather than left in place: it required Expired within that same deadline, which now returns before reaching it, so it could no longer act. For every other status the chain runs exactly as it did — the branch boundaries are untouched, and the grace window still enters the expiring branch and falls through on its status test.
Second comment pass. The first one removed narration of the change; this removes narration of the argument — how a conclusion was reached, what was believed before it, and emphasis added to make a line noticed. Most of the reduction is de-duplication rather than deletion. Three invariants were each stated twice at full length, and now have one owner with a one-clause reference at the other site: the compressed-backend floor trap STATUS_FLOOR_MS (was also on the wake) per-run vs persisted timestamps lastFetchedMs (was also on the floor exemption) `A` is presence-only the reader's doc (was also in its body) Also dropped: spec trigger numbers, which resolve against a document no reader of this file has, and the siren markers — a line that needs one to be noticed is in the wrong place. Kept at whatever length they need: the E/G model where it is the contract, the gate's decision table, ordering requirements, and the encoding traps (presence-only keys, root vs latest_payment grace, the ms/seconds boundary). 354 comment lines to 304 in the diff; 63.5% to 56.4%.
…ace reads Give useKeepProStatusFresh (trigger #4) an explicit E+G lifetime bound so it stops when the grace window closes, matching Android/iOS, rather than leaning on userHasExpiredPro flipping. Expose coverageEndMs from the pro-status selector to feed it. Guard the two getProGracePeriod() config reads (wake + startup gate) with ?? 0, so the E+G arithmetic can't go NaN once the number|null glue lands.
mpretty-cyro
force-pushed
the
feature/pro-status-refresh
branch
from
August 11, 2026 23:26
153325f to
4e4f046
Compare
`subscription_expired` re-set `E` from the failure response while its two sibling outcomes, `not_subscribed` and `revoked`, both clear it. All three mean the account is not entitled, so the odd one out was the re-set. It also left `A` and `G` untouched, and that combination is a live bug: a lapse typically leaves `E` unchanged while the grace and the renewing flag do change, so config ended up holding a fresh expiry beside a grace the backend had stopped honouring. The Pro screen then showed "renewal unsuccessful, retrying soon" for an account already reported Expired, and both the coverage-end wake and the cold-start gate's upper bound were stretched by the stale grace. Clearing `E` erases `A` and `G` with it, so the end state is absent keys rather than stored zeroes. The cold-start gate gains the condition that makes this safe. Clearing `E` removes every horizon the gate reasons from, and a latched Expired CTA cannot be shown until a fetch confirms the status in the current process — so without a fetch the flag would sit set and unshowable forever. A pending flag is therefore itself a reason to fetch. The interval cap moves above that row, since it is keyed on stored state rather than on a horizon and would otherwise fetch on every launch. tsc 66 -> 64: the two `accountExpiryMs` references this deletes; no other error moved.
Clearing the expiry on a lapse notifies nothing. The config watch that would refresh our status runs on incoming merges, so a write made on this device reaches no observer, and the cold-start gate has no horizon left to decide from once the expiry is gone. That left one path with no way to surface the lapse: when a proof outcome is the first thing to see it — the proof loop wakes on its own cadence, and the gate may be inside its interval and not have fetched — the Expired CTA is never raised, because it is a status fetch that raises it. Every later launch then has neither an expiry nor a raised CTA to act on. The refresh goes through the floored path, so when a status fetch has just run this is dropped, which is correct: that fetch already had the chance to raise it. Where no fetch has run, this is the only edge from "entitlement just ended" to "find out what to show". tsc unchanged at 64.
v0.6.19 predates the Pro config accessors and the proof-response account fields this branch calls, so it could not typecheck against its own declared pin. v0.7.0 carries them. tsc 64 -> 1. The 63 cleared are every reference to `get`/`setProAutoRenewing`, `get`/`setProGracePeriod`, the two proof-response fields and `getProPrepaid` — the last of which `dev` already required, so the base did not typecheck at its own pin either. The one that remains is not this branch's: the group-wrapper actions are missing `pushForRecovery` and `activeHashesByConfig`, which these types declare and only the config-recovery work implements. The version appears twice in the URL — the tag path and the filename — and a mismatch fails at install rather than at build.
`MetaGroupWrapperActions` is typed as `MetaGroupWrapperActionsCalls`, so the object literal has to name every method the type does. v0.7.0 added `pushForRecovery` and `activeHashesByConfig`, leaving the literal two short and the file failing to compile. Both are plain forwards in the same shape as their neighbours, each placed beside the sibling it extends. Neither takes arguments beyond the group key, so nothing here interprets a payload. Nothing calls either one, and the worker dispatches by string onto the wrapper, so a future caller resolves to the native method with no further plumbing. tsc 1 -> 0. First revision of this branch to typecheck clean; `dev` is at 2 against the same types.
The pro-config round-trip test set `version: 132` and expected it back. The stored credential is a single opaque bt-encoded value carrying only `e`, `g`, `r` and `s` — no version is written, and load forces v0 — so the test was asserting a round-trip the format refuses. A version cannot describe itself across a per-key merge, which is why a future format takes a new config key rather than an in-dict marker. The input stays deliberately non-zero so the read-back assertion shows the format's own value rather than an echo of the setter's. Suite: 920 passing, none failing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pro: unify the
get_pro_statusrefresh discipline (Desktop)Desktop's half of the cross-client Pro status-refresh unification. Android and iOS carry the matching
changes; the shared constants are named identically in all three so nobody tunes one platform alone.
Split of concerns this enforces: entitlement is the proof (Loop 1, libsession-timed, unchanged
here).
get_pro_statusis display + account-expiry awareness only (Loop 2). Status never drivesproof timing; the proof loop never depends on a status fetch.
Merge dependencies — one left
JasonFork/pro-status-sync—e5c14a852dev(#1975)JasonFork/remove-pro-gating—bcc9c3373, removes pre-launch Pro gatingAandG, the proof-response fields, the clear cascade(2) is the one that matters for merging. It is a single commit off
dev(8eee47876) by anotherauthor, replayed into this branch by the rebase, and it is the only commit in this PR's diff not authored
here. Everything below libsession is in place:
#124is ondev, the bindings are released as v0.7.0, andthis branch pins it.
Typecheck: 0 errors. This branch pins
libsession_util_nodejsv0.7.0, which carries the configaccessors and proof-response fields it calls. Measured with the real dependency resolved:
dev, typechecked against v0.7.0's typesThe branch is two errors better than its base, and both differences are wiring that v0.7.0's types
require: the userconfig worker interface doesn't declare the four Pro accessors, and
MetaGroupWrapperActionsis missing
pushForRecovery/activeHashesByConfig. This branch adds all six as plain forwards. Nothingcalls the two MetaGroup ones, and the worker dispatches by string onto the wrapper, so they need no further
plumbing.
devhas never typecheckedat its own declared pin: v0.6.19 is missing
getProPrepaid,getProRenewalTarget,deriveProRotatingKey,getRefundRequestedandMESSAGE_CHARACTER_LIMIT_PRO, all referenced by existingdevcode. Those failureswere the pin, and they are what the 42-error baseline in earlier revisions of this description was counting.
v0.7.0 is the first pin under which either side compiles.
Tests: 920 passing, none failing — and worth stating plainly, because the history looks otherwise. The
suite could not run before this branch: mocha runs compiled JS from
app/,build-ts-prodistsc && build-compile, andtscwas 42 ondevat its own pin. Taking it to 0 made the first run possible,which surfaced one pre-existing failure — a pro-config round-trip test asserting that a proof
versionsurvives storage, which the format refuses by design (the credential is a single opaque value carrying only
e,g,r,s; load forces v0). That test has been wrong since 2026-07-28 and nothing could see it. It iscorrected here to assert what the format guarantees. This branch did not break it; it made it runnable.
the
mock*feature flags intercept the selector, not the fetch path, so the floor, the startup gate, thewakes and the
immediaterule have no test exercising them. An injection point is buildable — the fetchfunnels through a single
ProBackendAPI.getProStatuscall site — and is not built here.The unified trigger set, as implemented
E/IchangedItoo now (wasEonly); floored; + proof reconcileimmediateE+30sand(E+G)+30s, collapsing to one when grace is zeroChanges
Status-refresh floor — new on Desktop.
refreshGetProStatusFromProBackendpreviously had only anisFetchingsingle-flight guard, so there was no minimum interval at all. Adds a 60s floor,drop-on-fresh rather than re-arming (a re-arming floor becomes a self-sustaining once/60s poll
during grace, when
Eis static). The timestamp is persisted inStorage, not redux — a per-run valueresets on every launch, which is the exact path the floor has to cover.
Two exemptions, both load-bearing:
immediate— bypasses the floor and nothing else (not single-flight, not the loading state). Namedimmediaterather thanforcebecauseforceinvited every routine trigger to pass it, which ishow Android's floor became dead code. Reserved to manual/recover, the bounded on-enter/while-open grace poll, and dev paths.
load state is
Init" exemption, which stops being true once the timestamp is persisted. Without it,relaunching within 60s of a previous fetch drops the fetch and nothing ever resolves the initial
loading state — a permanent spinner on the Pro screen and no CTA, since both gate on a confirmed
fetch.
Startup gate. Cold start now fetches only when a home CTA could plausibly fire, computed from synced
config, capped at once/24h. Non-Pro users (no
E) never fetch. Uses network time so clock skew can't flipa near-boundary decision.
expiry_tsis the true paid-through expiry, and coverage runs toE + G. The backend reports theaccount's own expiry but judges
user_statusagainst the later instant it stops serving, so[E, E + G)is expired-but-still-served. Two consequences:E, with no arithmetic — it is what the user paid for, and the honest thing toshow;
+ G: the grace indicator's upper bound, and the second scheduled wake.The gate is the interesting case: it reads
Eand deliberately notG. Coverage ends atE + G, butgrace and post-coverage both resolve to "fetch" — one to surface "renewal unsuccessful", the other to
surface Expired — so
Eis the only instant that changes the answer, and testing the later boundary woulddelay the first fetch without changing an outcome.
No provider branching, and none should be added: the root
grace_period_durationis 0 whenever thesubscription isn't auto-renewing, so this is
E + 0 == Efor those accounts. Stores differ in whether theystate grace separately or fold it into their own expiry, and a client treats what it receives as correct — a
store that folds it in simply sends a later
Eand a window that never opens.The grace value is the account-level one at the response root, not
latestPayment's field of the samename. The payment-level field is one store's raw declaration and is not gated on auto-renewal: a
subscriber who cancels mid-retry keeps a nonzero value in it, so reading it would place coverage weeks past
the truth.
Units: the response's
gracePeriodDurationMsis already milliseconds; core storesGin seconds and thenodejs wrapper converts, so config-sourced and response-sourced grace are on opposite sides of that
boundary. Both additions here are ms-on-ms.
Config-change trigger watches
E+I. A prepaid marker synced from another device's purchasepreviously produced no refresh. Does not watch
A: anauto_renewingchange re-derives the displayfrom the value just received, so fetching to confirm it would be a pointless round trip.
The
E-changed proof nudge is now explicit. It previously arrived as a side effect — theconfig-change handler dispatched a status refresh, and every completed fetch ends by calling
reconcileProProof(). A floored fetch never reaches that callback, so introducing the floor would havesilently dropped the nudge on precisely the path that needs it (several config pushes at once). Now
called directly from the config-change handler, gated on the
Ecomparison. This edge is load-bearing:with a lapsed account and an expired proof,
pro_renewal_targetreturns nothing and the proof loopsits dormant with no wake, and an advancing
Eis the only thing that can restart it.auto_renewingwritten to and read from synced config, from every path that writesE. Writtenunconditionally — libsession de-dupes a no-change write, and a presence-based guard would be actively
wrong since
Ais presence-only (writingfalseerases the key). Two writers: theget_pro_statusfetch,and the proof-success path from the response's
account_auto_renewing. The second is what makes "Aabsent means not auto-renewing" sound, and therefore what the gate's
!auto_renewingrows rest on — seethe merge-blocker note below, because one link of that chain is not in place yet. Read through a single
accessor, the only place in the client that touches
A.Home Pro CTAs now require a status confirmed by this process.
handleTriggeredCTAsgated theExpiring/Expired CTAs on
fromAppStart, which worked only because startup always fetched — everynon-startup caller was therefore guaranteed a fetch had happened or was in flight. Gating the startup
fetch removed that guarantee and left two callers (opening a conversation, closing the settings modal)
able to display a CTA from a flag a previous run persisted, with nothing confirming it this session —
the false-expired case the guard exists for. Now gated on a real predicate
(
details.lastFetchedMs, per-run and stamped on completion), computed inside the function so a newcall site cannot inherit an answer by copying a neighbour.
fromAppStartis retained where itenables the donate CTA: that is a different question and the two must not be collapsed.
The status fetch's single-flight now releases on failure.
isFetchingwas set by a dispatched actionbut cleared only by the result object landing via
fulfilled, and the thunk's internal try/catch coversthe network getter only — its
contextHandler/callbackrun outside it. So a throw in post-processingrejected the thunk with
isFetchingleft true for the rest of the process, silently early-returningevery later status trigger and thereby disabling the floor, the startup gate and the
E+30swake atonce.
isLoadingstuck the same way, which on a first-ever fetch is a permanent spinner on the Proscreen. The
rejectedcase now resets both, without touchingdataorlastFetchedMs— a failed fetchmust neither discard the last known-good status nor look like a confirmation.
This is a latent defect whose surface this branch widens by one call, not a regression it introduces.
Three awaited calls (
setProAccessExpiry,handleClearProProof,handleExpiryCTAs) were already in thatcallback, so the reject path was reachable before; this branch adds a fourth of the same kind
(
writeProAutoRenewingToConfig). Fixed here because a stuck single-flight nullifies the refreshdiscipline this PR exists to add, rather than merely coexisting with it.
Constants named as a cross-client contract (
STATUS_FLOOR_MS,STATUS_STARTUP_MIN_INTERVAL_MS,USER_EXPIRY_WAKE_DELAY_MS,PRO_EXPIRING_CTA_WINDOW_MS,PRO_EXPIRED_CTA_WINDOW_MS,GRACE_POLL_INTERVAL_MS,GRACE_POLL_CROSSING_SLACK_MS), replacing inline literals.GRACE_POLL_INTERVAL_MS === STATUS_FLOOR_MSis called out in a comment: that equality is why theon-enter/while-open grace poll must bypass the floor rather than be silently halved by it.
The Expired-CTA window is anchored at coverage end, not at
E. The backend reports Expired only oncecoverage has ended at
E + G, so measuring the 30 days fromEgave a window of[E + G, E + 30d)— `30dlong, and empty onceGreaches 30 days, which a configured store grace period can. The accounts it shortened were auto-renewing ones in dunning: the ones most likely to recover. Grace comes from the same response as the expiry, because this runs on every branch of the status switch and only the Active branch writesG` to config.The cold-start gate is bounded above at that same instant,⚠️ The two deadlines are now equal by construction; moving
E + G + 30d. Previously bounded only below,so an account that lapsed years ago passed the gate on every launch — one fetch per 24h, forever, to decide
whether to raise a CTA whose window had closed.
one requires moving the other.
A lapsed subscription clears the expiry rather than refreshing it.
subscription_expiredre-setEfromthe failure response while
not_subscribedandrevokedboth cleared it; all three mean the account is notentitled. Re-setting was also a live defect: a lapse typically leaves
Eunchanged while grace and therenewing flag change, so config held a fresh expiry beside a grace the backend had stopped honouring — the Pro
screen showed "renewal unsuccessful, retrying soon" for an account already reported Expired, and both the
coverage-end wake and the gate's upper bound were stretched by the stale grace. Clearing
EerasesAandGwith it, so the end state is absent keys rather than stored zeroes.Two edges make that safe, and both are load-bearing. Clearing
Eremoves every horizon the cold-startgate reasons from, and the Expired CTA is latched — written by a status fetch, displayed later, and only
once a fetch has confirmed the status in the current process. So:
sit set and unshowable forever. The 24h interval cap sits above that row, since it is keyed on stored state
rather than on a horizon and would otherwise fetch on every launch;
a local write reaches no observer. Without it, a proof outcome that is the first to see the lapse raises no
CTA at all. It goes through the floored path, so a fetch that just ran drops it — correct, since that fetch
already had the chance to raise it.
Unchanged deliberately: the proof acquisition floor (covered 60s / dark 15s→15min) and
useKeepProStatusFresh. Desktop is the reference implementation for both. Note the proof loopre-arms where the status floor drops — opposite by design, because a throttled proof acquisition
must still eventually happen. They should not be unified.
Do not merge without reading this — the advisory optionals must not be collapsed
A(auto-renewing) andG(grace) are written to config from both paths that writeE: theget_pro_statusfetch, and the proof-success path from the response'saccount_auto_renewing/account_grace_period_duration. The second is what makes "Aabsent means not auto-renewing" sound —without it, an account whose expiry arrived only from a proof reads back as terminal while it is renewing —
and what stops
E + Gpairing a fresh expiry with a grace from a different billing period.🔴 The two new fields are required on a successful parse, and what protects them is scope rather than a
null check. Core leaves both at their struct defaults on a failure outcome, where
falseand0areindistinguishable from a backend that really said "not renewing, no grace" — and both config keys are stored
presence-only, so writing either would erase what a
get_pro_statusfetch had learned. The writestherefore sit inside the success branch, which is the only place the fields are guaranteed and the only
place they mean anything. The access expiry keeps its own guard: that one is still optional in core, absent
on
not_subscribedandrevoked.A reviewer should treat any read of these two fields outside a success branch as a defect.
Chain state: the backend sends both fields; libsession core parses them (merged, on
dev); the nodejswrapper exposes them (dependency 4, second commit); this branch reads them. Nothing here is reachable until
the pin moves — see the typecheck note above, where four of the errors are exactly these two fields.
Open decisions
None. Every question this branch was built around has been ruled: where the grace period sits relative
to the reported expiry (
[E, E + G), implemented above), whether an unwrittenauto_renewingshould bootstrap a fetch (resolved at thesource — the proof response now carries the flag), and the grace-warning copy (one message covers the whole
window; the shipped strings already say "retrying soon" and "will remain active", so no copy work was
needed).
The debounce mechanism behind that warning is unchanged and is described under Changes: it requires a fetch
that completed at or after the expiry, so it cannot fire off a snapshot predating a renewal that may since
have landed.