Pro glue: expose auto_renewing (key A) and grace period (key G) - #68
Open
mpretty-cyro wants to merge 5 commits into
Open
Pro glue: expose auto_renewing (key A) and grace period (key G)#68mpretty-cyro wants to merge 5 commits into
mpretty-cyro wants to merge 5 commits into
Conversation
Mirrors the getProPrepaid/setProPrepaid pattern for libsession's user_profile auto-renewing and grace-period accessors, added by core #121. A is presence-only: set_pro_auto_renewing uses set_nonzero_int, so writing false erases the key and the getter cannot distinguish "not auto-renewing" from "never written". G is deliberately not optional, because the backend sends 0 when the subscription isn't auto-renewing and E - 0 == E, so an absent key and a stored zero describe the same account. Both caveats are documented on the declarations rather than at a call site, since the next reader of the glue meets them there. G is milliseconds in the JS domain, converted to core's whole seconds here. That follows the other Pro accessors and the gracePeriodDurationMs field callers get from get_pro_status, rather than setNoteToSelfExpiry's seconds-through shape. E and G are only jointly meaningful when written from the same get_pro_status response, and a client clearing E must clear G or a later E pairs with a stale grace. Noted in the type declaration because no signature can enforce it. Not compiled against core: #121 is unmerged and the submodule is left at its recorded commit.
parse_pro_proof gained account_auto_renewing and account_grace_period in libsession core, so a proof fetch can refresh config keys A and G alongside E rather than leaving them to desync from a get_pro_status fetch. Both surface as `boolean | null` / `number | null` rather than as the C struct's value + has_ pair. That is deliberate: null means the backend did not say, which is NOT false or zero, and both config keys are stored presence-only so writing a collapsed value ERASES them. Against a backend predating either field, a caller using `?? false` would wipe on every proof fetch what a get_pro_status fetch had learned. Nullable in the type forces the caller to branch; a has_ flag alongside a non-null value would not. Grace is milliseconds in the JS domain, matching getProGracePeriod and the gracePeriodDurationMs field on the get_pro_status response; core stores seconds. Not compiled against core: the parse lands on an unmerged libsession branch and the submodule is left at its recorded commit.
libsession made account_auto_renewing and account_grace_period_duration mandatory on a successful proof parse — a response missing either is a parse error rather than a defaulted value — so the nullable shape described a state that cannot occur. Both are now plain boolean / number. The erasure hazard they were nullable to prevent has moved rather than gone: core leaves them at their struct defaults (false / 0s) on a failure outcome, where those are indistinguishable from a backend that really said "not renewing, no grace". Since both config keys are presence-only, writing that false or zero would erase what a get_pro_status fetch had learned. The declarations now say to read them only inside the success branch, which is the guarantee core actually gives. accountExpiryMs stays nullable: it is still std::optional in core, absent on the not_subscribed and revoked outcomes.
mpretty-cyro
marked this pull request as ready for review
August 10, 2026 06:56
The grace-period accessors and the proof response's `accountExpiryMs` were documented against an inverted model: that the backend folded grace into the stored expiry, making `E` the end of coverage and `E - G` the paid-through instant. It is the other way round — `E` is the true paid-through expiry and the backend serves until `E + G`. Comments only; no signature or behaviour changes. Four sites: both grace declarations (the .d.ts and the C++ header), the proof response's expiry field, and the flooring rationale in setProGracePeriod, where truncation is still the conservative direction but for the opposite reason. Also names which `grace_period_duration` this key holds. `get_pro_status` carries two fields of that name — the account-level one at the response root and a per-payment one — and only the root value is gated on auto-renewal, so a caller reaching for the wrong one would place coverage weeks late for an account that cancelled mid-retry.
The two new accessor docs ran to 8 and 24 lines against a 7-line longest in the rest of the file. Trimmed to 6 and 10 with every API fact kept: presence-only erase semantics, the units and the flooring, coverage as E + G, and the root-vs-latest_payment distinction. What went was the argument around them and the siren markers.
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
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 glue: expose
auto_renewingand the grace period, in config and on the proof responseAdds the
libsession_util_nodejsbindings Session Desktop's Pro status-refresh work needs. No behaviourchange to anything existing — four new methods and two new response fields, nothing altered.
Three commits. The first exposes the synced user-profile config keys:
getProAutoRenewing()/setProAutoRenewing(bool)user_profile_{get,set}_pro_auto_renewingbooleangetProGracePeriod()/setProGracePeriod(graceMs)user_profile_{get,set}_pro_grace_periodnumber(ms)Gholds the account-levelgrace_period_durationfromget_pro_status— the response carries aper-payment field of the same name, and only the account-level one is gated on auto-renewal.
Eis theaccount's true paid-through expiry and the backend serves until
E + G, so[E, E + G)isexpired-but-still-served. Both facts are stated at the declaration, since a caller with the wrong one
produces a plausible number that is weeks off.
Mirrors the existing
getProPrepaid/setProPrepaidpair — implementation plusInstanceMethodregistration in
src/user_config.cpp, declarations ininclude/user_config.hpp, and the threedeclaration sites in
types/user/userconfig.d.ts(interface,UserConfigWrapperNode,MakeActionCallunion).
The second surfaces two new fields on the
generate_pro_proofresponse, so a proof fetch can refreshthose config keys alongside the access expiry rather than leaving them to desync:
accountAutoRenewingGenerateProProofResponse::account_auto_renewingbooleanaccountGracePeriodMsGenerateProProofResponse::account_grace_periodnumber(ms)🔴 Both are non-null, and the hazard a nullable shape would have flagged is real — it just isn't
nullability. Core requires them
on a successful parse — a response missing either is a parse error, not a defaulted value — so a nullable
shape would describe a state that cannot occur. But on a failure outcome core never fills them and its
struct defaults (
false/0) come through, indistinguishable from a backend that really said "notrenewing, no grace". Both config keys are stored presence-only, so writing either would erase what a
get_pro_statusfetch had learned.So the protection is scope, not nullability: these two are meaningful only where
status === 'ok', andboth declarations say so. A reviewer should treat any read of them outside a success branch as a defect.
accountExpiryMsstays nullable — it is stillstd::optionalin core, absent onnot_subscribedandrevoked.A wrapper reviewer won't have this in mind, so stating it plainly. #121 is not sufficient: it introduces
config key
Aand has nopro_grace_periodsymbols at all, sosetProGracePeriod/getProGracePeriodherewould not resolve. #124 completes #121 and carries all four commits this binds:
8e5634b8(#121's contribution)user_profile_{get,set}_pro_auto_renewing269f8b88user_profile_{get,set}_pro_grace_periodf197a0bdparse_pro_proofrequiringaccount_auto_renewing+account_grace_period_durationb066ba27(#124's tip)Ealso erasesAThey are a chain, so #124's tip is the single thing to depend on.
b066ba27binds no new symbol and islisted because the presence-only caveat below refers to it: it is a correctness dependency for consumers
rather than a compile dependency here.
This PR cannot build until #124 is merged and the submodule pin moves.
What is verified, and what is not
Stated explicitly because "it compiled" is not available here:
InstanceMethodregistrations exactly.That pairing is unchecked at compile time on this platform — napi dispatch is by string — so it's worth
saying it was checked rather than assumed. The core signatures were read from
include/session/config/user_profile.hppat269f8b88, not taken from a description.(wrapper as any)[action]with no registry orallow-list, so there's no third site to keep in sync; the two string literals are the whole surface. A
missing or misnamed binding throws a named error on first call rather than failing silently.
recorded commit, because neither core change is merged. So there is no artefact to inspect for symbols,
and CI is the first real build. Two of three verification legs, by construction rather than by omission.
Two deliberate choices a reviewer may want to challenge
Gis milliseconds on the JS side, converted to core's whole seconds here. The wrapper has twoconventions in tension: timestamps are ms-in-JS with C++ converting (
setProAccessExpiry,setProPrepaid,setRefundRequested), while the one existing duration accessor,setNoteToSelfExpiry, takes seconds straight through.Gis a duration in the Pro family. Chose msbecause callers hold
gracePeriodDurationMsfrom theget_pro_statusresponse, and it keeps theconversion in one place instead of at every call site. The conversion floors, so a derived coverage end
(
E + G) never claims coverage the backend didn't grant. Easy to flip if seconds is preferred.Gis not optional, unlikeA. The backend sends0whenever the subscription isn't auto-renewing,and
E + 0 == E, so "no grace stored" and "a grace of zero" describe the same account. There's nothingfor a presence check to disambiguate, and adding one by symmetry with
Awould copy a trap rather than apattern.
Caveats documented at the declarations, not at call sites
The next reader of this glue won't have the design thread, so both live in the type declaration:
Ais presence-only. Core writes it withset_nonzero_int, sosetProAutoRenewing(false)erasesthe key — present iff its value is 1. A caller must therefore not key a change check on whether the key
is present. For "absent means not auto-renewing" to be sound, every writer of
Emust also writeA—a client that writes
Efrom a path carrying no renewing flag leaves the pair inconsistent. The clearside is handled in core as of
b066ba27, so only the write side is the client's responsibility.E,GandAare only jointly meaningful when written from the same response. They're theninternally consistent whatever grace was in force. No signature can enforce that, so it's stated.
Clearing is handled below core — as of
269f8b88clearingEerasesG, and as ofb066ba27iterases
Atoo — so a client need only take care to write them together.