Skip to content

test(pro): what happens to Pro-gated state when Pro is revoked, and the CTA handling it needed - #160

Merged
Bilb merged 21 commits into
mainfrom
feat/pro-pins-over-limit-after-lapse
Aug 28, 2026
Merged

test(pro): what happens to Pro-gated state when Pro is revoked, and the CTA handling it needed#160
Bilb merged 21 commits into
mainfrom
feat/pro-pins-over-limit-after-lapse

Conversation

@mpretty-cyro

@mpretty-cyro mpretty-cyro commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Description

Four specs covering what happens to Pro-gated state when Pro goes away, plus the harness pieces they
needed. All green on every platform they apply to:

spec iOS Android Desktop
Pins over the standard limit survive Pro being revoked 1.5m 1.3m 18.3s
An animated display picture freezes once Pro is revoked 1.8m 1.0m 1.1m
The home screen shows a Pro badge once the account is Pro 38.0s 31.9s N/A

What each one claims

Pins survive revocation. The rule is asymmetric and only half of it is enforced anywhere: pins over
the standard limit are kept, and adding another is refused. A client that quietly unpinned the
excess would be destroying user state, and every other pin spec runs at or under the limit where the two
halves are indistinguishable. Both halves are asserted.

An animated picture freezes, for its owner. The neighbouring specs assert this from the far side —
pro_rotation_animated_avatar freezes a recipient's copy, pro_unverifiable_proof_avatar freezes one
whose proof cannot be verified. Neither covers the owner's own device, where the picture is a local file
rather than something fetched, so a client gating only the inbound path satisfies both and still animates
for the one person guaranteed to look at it. The picture being kept is the other half: deleting it
would also stop it animating.

The home badge. Absent for a standard account, present after a grant — asserted in both directions
with the same locator, because "not found" alone is satisfied by a misspelt id or a screen that never
loaded.

The pinned limit across a relaunch is no longer a spec of its own. It is a step on the existing
Pinned conversation limit (non Pro), which already pinned to the limit and asserted the refusal — two
thirds of what a standalone version had to repeat, and it could not shed that half, because the
in-session refusal is the control the relaunch assertion needs. As a step, that control runs immediately
before it.

It is regression cover for session-ios#762: a client that rebuilds its pinned count only when a pin
changes reads zero on a fresh launch and lets a standard user past the limit. iOS carried a count seeded
at zero; Android queries live at the moment of the decision and never had the bug, so the same step now
asserts the behaviour on the platform that was already correct.

That spec's selection is offset by one at the same time. assertPinOrder partitions the before-order into
pinned and unpinned and expects the pinned ones first, so for a prefix selection the expected order is the
before-order itself and the assertion cannot fail. Three order assertions that predate this change were
unfalsifiable until that offset.

Supporting changes

  • ctaConfigs.pinnedConversationsRenew — the clients pick the pinned-conversation CTA body from two
    axes (over the limit × previously subscribed), so it is four pieces of copy rather than one. This is
    the over-the-limit-and-previously-subscribed cell; without an entry for it a spec asserting the
    standard copy fails confusingly, and one asserting only "a CTA appeared" passes against the wrong
    dialog. All three clients resolve the same tokens for it.
  • HomeHeaderProBadge + the two ids it needs. Scoped rather than reusing the generic
    pro-badge-icon, which the same widget class renders in several roles.
  • DesktopWrapper.assertSettingsAvatarNotAnimated — the negative of the existing own-avatar
    assertion. It settles before sampling and requires every sample to match, and it fails on the
    generated placeholder rather than passing, so "the picture was removed" cannot read as "the picture is
    correctly frozen".

The harness changes these specs forced

Three of these were found by the specs above failing in ways that named the wrong thing, so they are here
rather than in a PR of their own.

  • dismissCTA takes a mechanism, and confirms the dialog closed. It was a boolean picking between
    two of the three ways out; the third (the "X", the only exit from a CTA with no cancel button) had no
    representation. It now waits on the heading it used to check for presence, so a dismissal is confirmed
    against the same element that defined the CTA as showing. A click returns before the modal finishes
    closing, and the step after it then lands on the overlay and fails naming whatever it was looking
    for — several steps from the cause.
  • pinConversation/unpinConversation settle before returning. Every caller was waiting for the
    plus button by hand, and one of them had to tolerate a CTA instead, because a refused pin never
    returns to the list.
  • The desktop read status gets its own bound. All three statuses shared 20s, sized for attachment
    uploads. sent and failed are local decisions; read waits on a receipt reaching the recipient and
    travelling back, which on devnet has been seen to take anywhere from ~31s to over 60s.

Also here: ConversationPinnedIcon gains its iOS half, so the pin assertions run on both platforms
rather than Android alone. #165 reached the same locator independently and the merge keeps its version.

Two things worth knowing for review

The gate reads ACCESS, not display status, which is why the revocation specs are shaped as they are.
Access outlives the plan — a lapsed subscriber holding a live proof keeps every Pro feature — so a spec
that only ends the plan is exercising the overhang (pro_overhang), not this. Each of these forces the
revocation to reach the proof and asserts access is gone before asserting the refusal.

forceProRevocationRefresh is a precondition, not a convenience. The backend serves the production
cadence (retry_in: 86400, inside libSession's [60s, 48h] clamp), so without it the client's next
revocation poll is a day away and the proof stays valid for the whole run.

Scope

Each spec covers the revoked case, not the grandfathered one. A user who never subscribed cannot
create either state through the UI — the app enforces the limits they would have to violate — so the
grandfathered variants need the seeder to write conversation priorities and display pictures directly.

Comment thread run/test/specs/mobile/pro_pins_over_limit_after_revocation.spec.ts
@Bilb
Bilb changed the base branch from fix/pin-order-assertion-is-vacuous to main August 27, 2026 07:14
The standalone spec duplicated two thirds of `Pinned conversation limit (non Pro)` — pinning to the
limit and being refused past it — and could not shed that half, since the in-session refusal is the
control the relaunch assertion needs. As a step on the existing spec the control already runs
immediately before it.

Its selection is offset by one at the same time. `assertPinOrder` partitions the before-order into
pinned and unpinned and expects the pinned ones first, so for a prefix selection the expected order is
the before-order itself and the assertion cannot fail; every order assertion in the spec depends on
that offset.
Two specs carried byte-identical local copies of it. `DeviceWrapper` is where the other CTA handling
lives, and `dismissAnyProCTA` there mirrors the desktop helper of the same name.

It is not the same thing as `dismissCTA`: both no-op when nothing is showing, but `dismissCTA` taps at
(150,150), which does not close these modals on iOS — the next tap lands on the scrim instead and the
failure surfaces somewhere unrelated. Three existing specs already work around that at their call sites.
Three mechanisms close a CTA and only one of them worked from `dismissCTA`. Its scrim tap does not
close the Pro modals on iOS — the next tap lands on the scrim rather than the control it aimed at — so
five call sites reached past it for the CTA's own Cancel button, three of them carrying a comment
explaining why. `via: CTADismissal` gives all three a name, and the reason for choosing between them is
stated once on the type rather than at each site.

The boolean it replaces meant `closeButton`, which is the only way out of a CTA that has no negative
button.

`dismissAnyProCTA`, added earlier on this branch, is folded in as `via: 'negativeButton'` — the same
thing with the probe on the heading rather than the negative button, which is what the other mechanisms
already used and what `checkCTA` matches on.
@Bilb
Bilb force-pushed the feat/pro-pins-over-limit-after-lapse branch from 3b79553 to 7d36792 Compare August 27, 2026 07:15
Clicking the swipe action or context-menu item returns before that menu closes, so whatever touches the
list next can land on the closing overlay. Five call sites waited for the plus button afterwards to cover
that, none of them said why, and the button has nothing to do with pinning — it is used because it is only
reachable once nothing is covering the list.

The wait belongs to the action rather than to each caller, so it moves into `pinConversation` and
`unpinConversation` with the reason stated once. `pro_stats_usage` gains it: that caller pins and reads a
counter with no settle at all today.
session-ios#767 tags the marker with `Pinned icon: <conversation name>`, so iOS can now assert that a row
is pinned rather than inferring it from list order. The locator threw `not yet implemented` before, and no
open branch had picked the id up.

Order is the weaker signal: `assertPinOrder` partitions the before-order and expects the pinned names
hoisted, so it says where rows sit rather than what any row claims about itself, and it cannot fail at all
for a prefix selection.

The name is in the identifier rather than the label because iOS put it there deliberately — a label is read
aloud, and the screen reader has already announced the name from the cell. Matching the composite id also
avoids the id-plus-text pairing that breaks whenever an id is added to an element.
`clickOnElementAll` returns before the modal has finished closing, so whatever runs next can land on the
overlay and fail looking for the screen behind it. That surfaces several steps from the dismissal and
reads as a missing control.

Measured on Android: the renew CTA was still on screen when the spec went looking for the home screen, and
the failure named the plus button — nothing in it pointed at the modal.

The wait is on the heading, which is the element the presence check at the top of this method already
reads, so a dismissal is confirmed against whatever defined the CTA as showing.
Four sites clicked the CTA's negative button directly and then read the conversation list. The click
returns before the modal closes, so the read can land on the overlay — measured on Android, where the
renew CTA was still on screen when the order assertion ran and the failure named the plus button rather
than the modal.

`dismissCTA` waits for the heading to go, so routing these through it makes the dismissal a fact rather
than an attempt.

Three sites outside the pin flows still click directly (`pro_overhang`, `pro_revocation_own_account`,
`message_length`). They carry the same latent race and are left for a change with its own verification —
these four are covered by the pin specs run here.
`pinConversation` waits for the conversation list to become interactive again, and a refused pin does not
return to the list — it raises a CTA. The plus button is then legitimately unreachable, so the wait ran to
its timeout and named the plus button, several steps from the refusal that caused it.

Measured on Android: three runs of `pro_pins_over_limit_after_revocation` failed at 112s with the renew CTA
on screen, raised by the very pin whose settle was blocking.

A CTA being up is a settled state too — the caller is about to assert it — so the settle returns rather
than waiting for a control the modal is covering.
`read` shared a 20s ceiling with `sent` and `failed`, and that ceiling was written for something else — its
comment reads "a gif on mainnet can take a long time to upload", so it was chosen for attachment uploads
and never revisited for receipts.

They are not the same kind of wait. `sent` and `failed` are local transitions the client decides, so a tight
bound reports a slow one quickly and should stay tight. `read` needs a receipt to reach the recipient, be
acted on, and travel back, so it is bounded by the network.

Measured: a `read` receipt on devnet took 31s. `Read status` was failing while asserting behaviour that
works — it passed at 90s, and 60s leaves headroom without letting a genuinely lost receipt hang.
…t one sample

60s was chosen from a single 31s measurement and then failed at 69s on the next run. The receipt cost is
not fixed - 31s once, over 60s another time - and 90s is the only value observed to pass.

The spread is the more interesting fact and is recorded in the comment: a 2x range points at a poll cycle
rather than network latency, which would make the correct bound derivable instead of observed.
#165 landed the iOS pinned-marker locator and a second pinned-conversation
CTA, both of which this branch had reached independently.

- `ConversationPinnedIcon` for iOS is the same selector on both sides; kept
  main's comment, and carried over the note that a composite id is matched
  rather than an id paired with text, which breaks once an id is added.
- The CTA table now holds three pinned-conversation entries, not two: at the
  limit, already over it, and over it having previously subscribed. They are
  three of the four cells the clients pick from, and none substitutes for
  another.
- In the pin loops, main's removal of the `.onAndroid()` gate stands (the iOS
  locator exists now) and so does this branch's removal of the explicit
  `PlusButton` wait, which moved inside `pinConversation`.
mpretty-cyro added a commit that referenced this pull request Aug 28, 2026
Brings in main through #160: the iOS pinned-marker locator, the third
pinned-conversation CTA, and the qa-seeder bump. Nothing here overlaps with
the picker work — the only shared file is `DeviceWrapper`, and the two
changes sit in different methods.
The house rule is why, complexity or intent only — no narrating the change, no
describing what it replaced, no recording what was measured.

- The read-status bound gave the run count and the previous ceiling. It now
  gives the range a receipt is known to take, which is what sizes it.
- Two comments said "deliberately (not) the first N", defending a choice. The
  property is that the pinned set must not be a prefix of the starting order.
- Two said "measured on Android". A behaviour the platform exhibits does not
  need to say it was observed.
- A desktop spec counted the sibling specs calling `revokeAccountPro` and
  called the transition untested — a count that rots, and a claim this spec
  falsifies on merge.
@mpretty-cyro mpretty-cyro changed the title test(pro): what happens to Pro-gated state when Pro is revoked test(pro): what happens to Pro-gated state when Pro is revoked, and the CTA handling it needed Aug 28, 2026
@Bilb
Bilb merged commit 0b1f402 into main Aug 28, 2026
1 check passed
mpretty-cyro added a commit that referenced this pull request Aug 28, 2026
The only conflict is `copy_file_to_simulator`, where both sides had fixed the
same broken fixture path. Main's version stands: it drops the `MEDIA_ROOT`
constant and reads `mediaFolder` at the call site, which is where the body
already reads it from — keeping the alias would leave it unused.
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.

2 participants