Pro coverage for the two-value split, and a mock that can express an unverified plan by mpretty-cyro · Pull Request #131 · session-foundation/session-appium

mpretty-cyro · 2026-08-17T03:33:37Z

Draft — replaces #128, which was closed unintentionally. Rebased onto main now that #130 has merged.

What this is

Pro coverage for mobile and Desktop, plus the harness changes that make the interesting Pro states reachable at all.

The clients now answer two questions with two values:

ACCESS — "what may this device do right now" — the proof in config, validated on every read against expiry and the revocation list.
DISPLAY — "what state is the plan in" — the backend status, seeded from local config at launch when no response has arrived.

They are meant to disagree. A plan that has lapsed while the proof still has time left displays as expired while the features keep working; that overhang is intended behaviour.

The harness change worth reviewing first

proBackendStatus used to say what state the plan was in and grant access, which made one state unreachable by mock on every platform: an active plan with no usable proof. That is the state the message limit was wrong in — the composer offered the Pro limit, the message went out with no proof attached, and every recipient silently truncated it. Measured at 3000 characters sent, 2000 stored.

proProof is now the access half, with its own key on each client. A fixture meaning "this user is Pro" sets both.

New specs

the overhang — expired plan, valid proof: display says expired, Pro features still work. Found Android applying the standard message limit while its settings screen correctly said expired.
the silent refusal — active plan, no proof: a Pro action is refused and no upgrade prompt is shown, because the user cannot be sold a subscription they are already paying for.
the startup fetch gate on Desktop — a launch that cannot know it is expiring stays silent; the launch after the client learns its expiry warns. Against a real grant, which is what found two Desktop defects the mocked path could not.

Also here

A harness fix: runScriptAndLog judged success by stderr rather than exit code, so tools that report progress there — adb push does — were logged as failed on a passing run. Three false "cmd which failed" lines in a green run teach a reader to skip the string, which is how a real one gets past.

Scope

Everything here is test-side. The client behaviour it exercises is in the corresponding branches on the three client repos.

Pay, lose the install, restore, still Pro - the worst outcome for a subscriber and the one the suite did not guard. Covered on all three platforms. A real grant rather than a display mock, because the point is that Pro survives onto a client that has never seen it: the restored install derives the Pro master key from the recovery phrase and asks the backend, so this exercises that derivation end to end. A mock would only convince the client that already had it. The second device stands in for a reinstall rather than a linked device - nothing is carried across but the phrase, which is what a user has after losing their phone. The assertion is the stats section of the Pro settings screen, gated on an active plan everywhere, so its presence means the restored client fetched a real entitlement rather than rendering a Pro-looking screen. restoreFromSeed on Desktop gains an optional fallback name. Restoring an account created moments earlier prompts for a display name, since its profile has not reached the network yet, and without a fallback that is an error rather than something to type past. Mobile's restore has no such escape, so it asserts the name comes back where Desktop tolerates it missing - flagged rather than reconciled. Every Desktop window now carries a launch identity, not only those from the account builders, so a spec using sessionTestTwoWindows can restart one.

Covers the warning a non-renewing subscriber gets as their access approaches its end, and that dismissing it leaves them looking like a subscriber. `proLoadingState: 'success'` is load-bearing rather than incidental: every client arms this CTA only once a status fetch has been confirmed in the current process, so a mocked expiry alone reaches the state but not the gate. Android could not express that until its debug menu gained a force-success value, and its launch extra for the access expiry was never emitted. The CTA table carries no body for this variant because the copy interpolates the remaining time, which differs per platform fixture — hence the optional body in `checkCTAStrings` on both wrappers.

Both halves of the cold-launch status fetch, against a real grant: a launch that cannot know it is expiring must not warn, and the launch after the client learns its access expiry must. Nothing covered the decline before, so a client that warned off unconfirmed state would have gone unnoticed. `makeAccountPro` gains `durationSeconds` because the QA backend's compressed clock shortens proof expiry and not the account entitlement — a nominal 1M plan really does land thirty days out, which no expiry-window spec can use. It throws when the backend grants materially more than asked, since a route that ignored the override would otherwise quietly return the full plan length.

Every real-grant spec restarted the app and assumed the client would notice. It does not: each client declines the cold-launch status fetch for an account with no local expiry and no proof, which is exactly what a grant minted straight into the backend looks like. `Pinned conversation limit (Pro)` was asserting Pro behaviour against a non-Pro client on both platforms, and passing. - Replace the restart with `observeProGrant`, which restarts and then opens Pro settings — the restart makes the visit's fetch eligible rather than discovering anything, since both clients floor further fetches at 60s from the last attempt and only the first in a process is exempt. - Assert the stats header with `skipHealing`. Healing falls back to a fuzzy id match, which on the non-Pro version of that screen resolves to a neighbouring `pro-settings-*` element — so the wait passed while the client was NeverSubscribed and every later assertion ran against a non-Pro account. - Drop the equivalent restart on Desktop, whose `waitForProActive` already reaches Pro settings.

`Pro survives a restore from seed` reaches the same entitlement through the Pro settings screen, which refreshes status on arrival — so it passes whether or not the config-driven refresh works. These specs never open settings on the restored device. The composer's character countdown is the observable: it needs no navigation and no second party, and a client that believes it is not Pro caps at 2000 chars. Covers the proof half only. The client's Pro *status* can still be unknown while the proof is good — measured on a restored Android and Desktop client, whose settings row reads "Upgrade Session" while the Pro cap applies correctly. The row that would show it puts its id on a container with no text of its own, so that assertion is not expressible yet; noted so the gap is not mistaken for coverage.

`observeProGrant` proved a grant had landed by opening Pro settings and waiting for the stats header. That reads the active plan; the row on the parent settings list reads the fetched status, and the two disagree more often than expected — a client can hold a good proof, apply the Pro message cap, and still offer to sell you Pro on that row. Asserting it costs nothing, since the row is on screen on the way out. `assertProFromSettingsRow` is separated out for reuse as a cheap "is this client Pro" precondition. It reads the parent list deliberately: no client refreshes on opening it, so it observes what the client already believed instead of provoking the answer. Tapping the row is not passive, so it reads and leaves. Requires the new `pro-menu-item-title` id on both mobile clients — the existing `pro-menu-item` sits on the tap target, which carries no text of its own, so the row's state was not readable through it.

Status and proof answer different questions. The status is the plan's state, which only the backend knows; the proof is the entitlement that travels with the message and is what a recipient validates against. A message length is the second question, so it has to read the proof. Reading the status instead leaves the two ends permanently disagreeing: measured at 3000 characters sent and 2000 stored by the recipient, with nothing shown at either end. The sender's own copy keeps the full text, so the conversation holds two different messages. RED until the compose limit reads the proof. That change is in flight on all three clients as part of the access/display split; this spec is the check on it. The fixture needs no grant and no restore — a mocked active status supplies the status half while the config stays empty, which is the state exactly.

`proBackendStatus` said what state the plan was in AND granted access, so a client with an active plan and no usable proof was unreachable by mock on every platform. That is the state the message limit is wrong in: the composer offers the Pro limit and every recipient silently truncates, because no proof travels with the message. `proProof` is the access half. A fixture that wants an ordinary Pro user now sets both; the pair is only interesting when they disagree. `none` overrides a real proof rather than falling back, since a mock a real proof could outvote is useless on the devices most worth running it on. The app-side key differs per platform because the harness already translates per platform: mockCurrentUserSessionProProof, sessionProProof, and SESSION_PRO_MOCK_PROOF. The lowercasing for Desktop is load-bearing — it throws at flag-init on anything else, so an untranslated value stops the renderer starting.

…riber" The status mock no longer grants access, so a fixture claiming an active plan renders a subscriber whose features are all switched off. Anything meaning "this user is Pro" now says so twice, once per question. `iosActiveProContext` carries it for the mobile specs, so the per-spec changes are the Desktop ones plus the mobile Pro settings screen. The no-proof message limit spec gets the opposite pairing, stated explicitly: an active plan with `proProof: 'none'`. Until the mocks were split that state could not be reached, so the spec could not have measured what it claims to — it would have passed by offering the Pro limit, which is the behaviour it exists to catch.

The countdown renders through `Intl.NumberFormat`, so a four-figure remainder carries a group separator and the raw `-1000` never appears. The spec waited for it and timed out — in BOTH states, since before the fix the Pro limit left 7000 remaining, which is above the show buffer and renders nothing at all. So this could not have passed either way, and its red was being read as evidence that the client was still wrong. It was not: the composer applies the standard limit, lets the count go negative and leaves the refusal to the send gate.

…t the client Both failed against a client that was behaving correctly. The no-proof limit spec spread `iosActiveProContext`, which now grants BOTH halves of Pro state — so the fixture handed the spec the entitlement it exists to withhold. It then still failed on the countdown's text: iOS renders a four-figure remainder as `-1K`, so the literal never appears. Its presence is the assertion now, which is the actual question — the countdown only shows within 200 of the limit, so at this length it appears under the standard limit and not under the Pro one, on either platform and whatever each formats it as. The expired settings spec described a user who cannot exist: a lapsed plan with no proof and no access expiry. That is the case the startup fetch gate declines to fetch for, so the status was never confirmed and no CTA could arm. It passed before only because launch used to fetch unconditionally. A real lapsed subscriber has an expiry and it is in the past.

…push fail loudly `runScriptAndLog` judged success by stderr rather than by exit code, so any tool that reports progress there was logged as failed. `adb push` does exactly that — it prints its throughput to stderr and exits 0 — so a green run carried three "cmd which failed" lines for pushes that worked. That is worse than silence. Three false alarms in a passing run teach a reader to skip the string, which is how a real one gets past; it cost a wrong diagnosis before the stream was checked. The hardcoded exception for one tool's chatter was the same bug being patched one tool at a time. Success is now the exit code, which `exec` already throws on. Failure is still swallowed, which is right where a command is optional and wrong where a fixture depends on it. `runScriptOrThrow` covers the second case, and the media push takes it: the folder is cleared first, so a failed push leaves the device with no media and the spec fails later on a missing picker entry with nothing pointing back. The push is then verified by listing the file rather than by trusting the command, since exiting 0 and the file being there are different claims.

…ures A lapsed plan whose proof has time left is meant to display as expired and keep working. It is the one disagreement between the two Pro values that is a feature, and nothing exercised it — on any platform — because until the proof got its own mock the state could not be reached. Both halves are asserted in one launch deliberately. Each alone passes for a client that has collapsed the two values back together: reading only the proof shows Active and passes the feature check, reading only the plan shows expired and passes the display check. Only the pair fails, which is why this found Android applying the standard message limit while its settings screen correctly said expired — silent, and it takes a paid-for feature from someone entitled to it. Also splits `harvestAccountData` out of `newUser`. Creation and credential-reading are separate jobs and only the second one collides: it opens settings before any spec code runs, so a fixture arming an app-open CTA has the modal over the list while it hunts for a row. It now says so instead of polling for thirty seconds and naming a locator the spec never mentions. `newUser` still calls it by default.

…an upsell A plan reading active with no usable proof refuses a Pro action and says nothing. The gate is ACCESS — no proof, so the action cannot be allowed — while the prompt is DISPLAY, and offering to sell Pro to someone whose plan is active offers what they already pay for. The silence is the accepted trade for having no copy that explains a refusal without selling. That trade is only recorded in comments, and a comment cannot fail. Reinstating the upsell looks like an improvement at the call site and is the bug the split was made to remove, so this fails for it. Both halves are asserted: a prompt appearing is not the same as the action being refused, and an implementation that prompted and pinned anyway satisfies either alone. The expiry in the fixture is load-bearing — an active status with none inherits zero, which the client reads as expiring imminently and answers with the very CTA this asserts the absence of. Also lifts the Desktop pin helpers out of one spec so both consumers pin the same rooms the same way, rather than the second copy drifting from the first.

The gate's two halves were asserted on both mobile clients and on neither Desktop: a launch that cannot know it is expiring must stay silent, and the launch after the client learns its expiry must warn. Desktop could not express the second half until the expiry CTAs stopped being gated on being past app start, and could not pass it until they stopped being consumed by the Pro settings screen — the CTA fired there, cleared its own mark, and left the next launch with nothing to find. Both are now fixed, so the same user-visible rule is asserted on all three. A real grant rather than a mock, because the mocked path confirms a status directly and never consults the gate. That makes this the only Desktop spec whose CTA arrives through a real fetch, which is why it found both of those.

The union order is the repo's lint rule, and `STANDARD_MAX_CHARS` stopped being referenced when the mobile assertion moved from the countdown's text to its presence — the value it carried is now in the comment, where it still tells a reader why 3000 is the length chosen.

The overhang spec asserted the expired plan through the renew row alone. The row and the description are chosen by separate switches on the same status, so a client that fixed one and not the other offers to renew under a heading thanking the user for subscribing — and the expired copy is the only one of the three that tells someone in this window what to do about it. Bumps the localizer, which was eight commits behind and shipped older text for two of these tokens and lacked the third, so the copy asserted here is the copy the apps ship. None of the 142 tokens the suite already asserts change value across the bump.

Same reasoning as the expired case: the rows follow the plan's status while the description is chosen by a separate switch on that status, so asserting one says nothing about the other. The never-subscribed status is deliberately not covered here. Its copy is the only one of the three carrying markup, and it has three forms — a line break on iOS, `\n` on Android, and either a space or nothing from `tStripped` depending on whether it runs under Playwright. Label matching compares exactly after trimming, so none of them agree.

Completes the three hero states. Reached through the backend-error fixture because that is the only one landing on the settings screen as never-subscribed, and the two are independent: the description follows the plan's status while the banner follows the fetch, so an unreachable backend still renders the upgrade pitch. Label and text matching now collapse runs of whitespace. The same token arrives in three shapes — a rendered break on iOS, `\n` from Android's strings.xml, and a space from the localizer — and the difference is presentation rather than copy. Desktop needs the opening sentence instead: it renders the break as a `<br>`, which contributes no text, so the sentences run together there.

A window opened by the seeded template could not be restarted: only the onboarding templates assigned the multi and instance that bring an app back up on its own user-data directory, so any spec pairing a seeded template with `restartApp` threw instead of restarting. That combination is what a Pro spec needs — a real grant is only visible after the app comes back up — so the four Desktop message-length Pro cases could never have passed as written. Assigned positionally like the onboarding templates, from the same launch-order bookkeeping, and `MULTIS` moves to `open.ts` because the seeded opener cannot import from the templates that import it. Missing bookkeeping throws rather than assigning undefined, which would fail later and elsewhere.

Bilb · 2026-08-18T04:16:29Z

+   * omit it where the name coming back IS the assertion.
+   */
+  public async restoreFromSeed(recoveryPhrase: string, fallbackName?: string): Promise<void> {
+    await recoverFromSeed(this.page, recoveryPhrase, fallbackName ? { fallbackName } : undefined);


isn't recovering from seed without a found display name an error? It should be to me, hence why I had removed this not too long ago!
It most likely means that the qa-seeder didn't push to the network that the apps are looking for a connfig

Bilb · 2026-08-18T04:18:07Z

+ * Here rather than beside the test templates because both the onboarding and the seeded openers need
+ * it, and the seeded opener cannot import from the templates: they import it.
+ */
+export const MULTIS = ['A', 'B', 'C', 'D'];


not too sure why claude didn't see that the multis are entries from a few lines above
const multisAvailable = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';

Maybe just keep one, even if that's thew new one

Will update to use multisAvailable

Bilb · 2026-08-18T04:26:25Z

+test_Alice_1W_Bob_1W(
+  'No Pro proof means no Pro message limit',
+  async ({ alice, bob }) => {
+    await alice.createContactWith(bob);
+    await alice.openConversationWith(bob.userName);
+


as a general note, we sould need to call createContactWith expect in very limited cases. We can use the qa-seeder for that with test_Alice_1W_Bob_1W_friends

Bilb · 2026-08-18T04:27:36Z

+const PRO_MAX_CHARS = 10000;
+const COUNTDOWN_START_THRESHOLD = 200;


we should probably define (or reuse, if already defined) those constants in one place instead of per test.

We have the const STANDARD_MAX_CHARS = 2000; in another test

Ugh... sorry about all of these duplicate constants - I think I missed them as I was reviewing commit-by-commit while they were added rather than the full set at the end 🤦

Bilb · 2026-08-18T04:28:14Z

+test_Alice_1W_Bob_1W(
+  'Pro features survive the plan expiring',
+  async ({ alice, bob }) => {
+    await alice.createContactWith(bob);
+    await alice.openConversationWith(bob.userName);


same comment, use the _friends test setup so that the qa-seeder does it for us

Bilb · 2026-08-18T04:54:32Z

+    const names = await joinCommunities(alice, COMMUNITY_COUNT);
+
+    // The standard limit applies, because the limit is ACCESS.
+    for (const name of names.slice(0, STANDARD_PIN_LIMIT)) {
+      await pinConversation(alice, name);
+      await pinIconFor(alice, name).waitFor({ state: 'visible' });
+    }
+
+    const overLimit = names[STANDARD_PIN_LIMIT];
+    await pinConversation(alice, overLimit);


joinning communities is usually quite slow. Should we make the qa-seeder have a way to generate 10 contacts for a user, so we can use those to try to pin/unpinn?

Bilb · 2026-08-18T05:14:31Z

+  const { device } = await test.step(TestSteps.SETUP.NEW_USER, async () => {
+    const { device } = await openAppOnPlatformSingleDevice(platform, testInfo, {
+      ...iosActiveProContext(EXPIRING_SOON_DAYS),
+      proLoadingState: 'success',
+    });
+    await newUser(device, USERNAME.ALICE, { saveUserData: false });
+    return { device };
+  });


do you think we should get this part of sessionTest to auto create the user (via qa-seeder) and auto set the pro details it needs?

Bilb · 2026-08-18T05:38:34Z

+const PRO_MAX_CHARS = 10000;
+const COUNTDOWN_START_THRESHOLD = 200;


same comment

Bilb · 2026-08-18T05:42:40Z

+ */
+export async function harvestAccountData(
+  device: DeviceWrapper,
+  userName: UserNameType


I think this userName can be removed

Bilb · 2026-08-18T05:54:48Z

+  const { device } = await openAppOnPlatformSingleDevice(platform, testInfo, {
+    sessionProEnabled: 'true',
+    // The plan ended yesterday, and the backend has said so...
+    proBackendStatus: 'expired',
+    proLoadingState: 'success',
+    proAccessExpiry: String(Math.floor(Date.now() / 1000) - ONE_DAY_SECONDS),
+    // ...while the credential it was issued under is still good.
+    proProof: 'valid',
+  });
+
+  // `saveUserData: false` skips the settings visit `newUser` otherwise makes to read the recovery
+  // phrase. That visit happens before any spec code can run, so a fixture which arms an app-open CTA —
+  // as this one does — has the modal sitting over the settings list while it looks for a row.
+  await test.step(TestSteps.SETUP.NEW_USER, async () => {
+    return await newUser(device, USERNAME.ALICE, { saveUserData: false });
+  });


having a fixture setup with pro details could save us that issue.
i.e. if we had a alice1W_pro, where we can provide what are the pro details that we want for that user.

We would probably need a way to have pro details set before the app is started, and after it is started though

mpretty-cyro added 20 commits August 17, 2026 13:18

mpretty-cyro marked this pull request as ready for review August 18, 2026 02:12

Bilb reviewed Aug 18, 2026

View reviewed changes

Navigation Menu

Uh oh!

Pro coverage for the two-value split, and a mock that can express an unverified plan - #131

Pro coverage for the two-value split, and a mock that can express an unverified plan#131
mpretty-cyro wants to merge 20 commits into
mainfrom
feat/pro-restore-from-seed

mpretty-cyro commented Aug 17, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

mpretty-cyro Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

mpretty-cyro Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Bilb Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const PRO_MAX_CHARS = 10000;
		const COUNTDOWN_START_THRESHOLD = 200;

Uh oh!

Conversation

mpretty-cyro commented Aug 17, 2026

What this is

The harness change worth reviewing first

New specs

Also here

Scope

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants