test(pro): what happens to Pro-gated state when Pro is revoked, and the CTA handling it needed - #160
Merged
Merged
Conversation
mpretty-cyro
marked this pull request as ready for review
August 27, 2026 06:28
Bilb
reviewed
Aug 27, 2026
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
force-pushed
the
feat/pro-pins-over-limit-after-lapse
branch
from
August 27, 2026 07:15
3b79553 to
7d36792
Compare
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.
Bilb
approved these changes
Aug 28, 2026
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.
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.
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:
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_avatarfreezes a recipient's copy,pro_unverifiable_proof_avatarfreezes onewhose 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 — twothirds 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.
assertPinOrderpartitions the before-order intopinned 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 twoaxes (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 genericpro-badge-icon, which the same widget class renders in several roles.DesktopWrapper.assertSettingsAvatarNotAnimated— the negative of the existing own-avatarassertion. 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.
dismissCTAtakes a mechanism, and confirms the dialog closed. It was a boolean picking betweentwo 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/unpinConversationsettle before returning. Every caller was waiting for theplus button by hand, and one of them had to tolerate a CTA instead, because a refused pin never
returns to the list.
readstatus gets its own bound. All three statuses shared 20s, sized for attachmentuploads.
sentandfailedare local decisions;readwaits on a receipt reaching the recipient andtravelling back, which on devnet has been seen to take anywhere from ~31s to over 60s.
Also here:
ConversationPinnedIcongains its iOS half, so the pin assertions run on both platformsrather 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 therevocation to reach the proof and asserts access is gone before asserting the refusal.
forceProRevocationRefreshis a precondition, not a convenience. The backend serves the productioncadence (
retry_in: 86400, inside libSession's [60s, 48h] clamp), so without it the client's nextrevocation 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.