Skip to content

Keep subscribers active through empty StoreKit reads and empty web polls - #506

Open
jakemor wants to merge 4 commits into
developfrom
fix/subscription-status-anti-downgrade
Open

Keep subscribers active through empty StoreKit reads and empty web polls#506
jakemor wants to merge 4 commits into
developfrom
fix/subscription-status-anti-downgrade

Conversation

@jakemor

@jakemor jakemor commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

The problem

A paying Stripe subscriber flipped to INACTIVE ten times at cold launch in production, 0.8s after start. The mechanism:

  1. Every cold launch, the automatic StoreKit sync recomputes the status from device purchases. A web/Stripe subscriber has none, and StoreKit can also return nothing before it hydrates.
  2. The cached-web-entitlement merge normally rescues this. But one anomalous poll response can poison that cache: pollWebEntitlements is keyed on appUserId/deviceId alone, and a response with zero entitlements overwrote the cache unconditionally.
  3. With the cache empty, the empty device read set .inactive and persisted it. Recovery needed a network poll, so subscribers lost access exactly when the network was weak.

The fix

One principle, applied at the two write sites: nothing that is not an authoritative answer may downgrade a subscriber whose entitlement has not expired.

  • AutomaticPurchaseController.syncSubscriptionStatus: an empty device read keeps an .active status while one of its entitlements is within its expiry date. Entitlements with no expiry date do not hold the status, so a revoked lifetime purchase still deactivates.
  • WebEntitlementRedeemer.pollWebEntitlements: a response with zero entitlements no longer replaces cached web entitlements that are still within their expiry date. The fetch date is not saved, so the next poll retries without waiting out entitlementsMaxAge.

Tests

9 new tests, including a full reproduction of the production failure: status restored from disk, missing web cache, unreachable network, zero App Store purchases. As a negative control, I removed the guards and reran: the reproduction tests fail with the exact production symptoms, and the behavior-preservation tests (expiry, nil-expiry revocation, web-merge rescue) pass both ways.

Full suite: 928 tests in 96 suites pass.

Also includes a one-line build fix: develop fails to type-check DeviceHelper.swift:494 on Xcode 26.0.1 (mixed CGFloat/Double expression).

🌸 Shipped with Kanna — an open-source workspace for all your coding agents. Written by claude/fable.

jakemor and others added 2 commits August 16, 2026 00:04
Xcode 26.0.1 fails to type-check the mixed CGFloat/Double expression.
Convert to Double once so the operators resolve. Same result.

🌸 Shipped with Kanna — https://kanna.sh

Co-Authored-By: Kanna <noreply@kanna.sh>
Kanna-Agent: claude/fable
Two guards, one principle: nothing that is not an authoritative answer
may downgrade a subscriber whose entitlement has not expired.

1. AutomaticPurchaseController.syncSubscriptionStatus: an empty device
   read no longer sets .inactive while the current .active status holds
   an unexpired entitlement. StoreKit returns nothing at cold launch
   before it hydrates, and web/Stripe subscribers never have App Store
   purchases. Entitlements with no expiry date do not hold the status,
   so a revoked lifetime purchase still deactivates.

2. WebEntitlementRedeemer.pollWebEntitlements: a response with zero
   entitlements no longer replaces cached web entitlements that are
   still within their expiry date. The poll is keyed on appUserId and
   deviceId alone, so one anomalous response could poison the cache
   and make every later cold launch read the subscriber as inactive.

Production data showed a paying Stripe subscriber flip to INACTIVE ten
times at cold launch, 0.8s after start, before the network poll could
recover them. The new tests reproduce that flip and fail without the
guards.

🌸 Shipped with Kanna — https://kanna.sh

Co-Authored-By: Kanna <noreply@kanna.sh>
Kanna-Agent: claude/fable
@greptile-apps

greptile-apps Bot commented Aug 16, 2026

Copy link
Copy Markdown

PR author is not in the allowed authors list.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The AutomaticPurchaseController guard fires on any empty entitlement set, including the case where StoreKit authoritatively reports that every purchase is inactive. That makes a refunded or revoked App Store subscription keep .active status until its pre-refund expiry date.

Reviewed changes — full initial review of both commits on fix/subscription-status-anti-downgrade.

  • Anti-downgrade guard in AutomaticPurchaseController.syncSubscriptionStatus — when the derived entitlement set is empty, the status write is skipped entirely if the current status is .active and any of its entitlements has expiresAt in the future; nil-expiry entitlements are unprotected.
  • superwall: injection parameter — added to syncSubscriptionStatus as a test seam, defaulting to Superwall.shared, matching the pattern already used by WebEntitlementRedeemer and internallySetSubscriptionStatus.
  • Empty-poll guard in WebEntitlementRedeemer.pollWebEntitlements — a zero-entitlement getEntitlements response no longer overwrites LatestRedeemResponse when a cached entitlement is isActive and unexpired, and returns before LastWebEntitlementsFetchDate is armed.
  • DeviceHelper.fontScale arithmeticDouble(scaledValue) disambiguation for the Xcode 26.0.1 type-checker; numerically identical.
  • Tests — a new AutomaticPurchaseControllerTests suite (7 tests) plus 2 WebEntitlementRedeemerTests cases covering the empty-poll guard and its expired-entitlement counter-case.
  • Generated project filesSuperwallKit.xcodeproj bumped to objectVersion = 77 with the new test file wired in, and the shared scheme gains parallelizable = "NO".
  • CHANGELOG — new ## Unreleased### Fixes section describing both guards.

⚠️ The new tests depend on a scheme setting that xcodegen regenerates away

SuperwallKit.xcodeproj and its shared scheme are generated artifacts: scripts/pre-commit, scripts/build.sh and scripts/test.sh all run xcodegen before doing anything else. The hand-added parallelizable = "NO" in SuperwallKit.xcscheme (and the objectVersion = 77 / minimizedProjectReferenceProxies churn in the pbxproj) will not survive the next generation, so whatever isolation the new suites need has to be expressed in project.yml instead.

Technical details
# Serialized test execution is encoded in a generated file

## Affected sites
- `SuperwallKit.xcodeproj/xcshareddata/xcschemes/SuperwallKit.xcscheme:43-44``parallelizable = "NO"` added by hand; the scheme is emitted by `xcodegen` from the `SuperwallKit` target's `scheme:` block in `project.yml:12-16`.
- `SuperwallKit.xcodeproj/project.pbxproj``objectVersion = 77`, `preferredProjectObjectVersion`, `minimizedProjectReferenceProxies`, empty `packageProductDependencies`, removed `TargetAttributes`: all Xcode-written, all reverted by `xcodegen`. The new test file does not need a pbxproj entry — `sources: [Tests/]` picks it up.
- `scripts/pre-commit:2-3` (`xcodegen` then `git add SuperwallKit.xcodeproj`), `scripts/test.sh:16-21`, `scripts/build.sh:16-21`.
- `Tests/SuperwallKitTests/StoreKit/Purchase Controller/AutomaticPurchaseControllerTests.swift:20-29``@Suite(.serialized)` plus `init()` deleting `SubscriptionStatusKey` / `LatestRedeemResponse` / `LastWebEntitlementsFetchDate` from the shared on-disk storage. `.serialized` orders tests *within* a suite only; 19 suites already use it, and the two suites touched here now both mutate `LatestRedeemResponse` and `Superwall.subscriptionStatus`.

## Required outcome
- Whatever execution mode the new suites require holds after a plain `xcodegen` run, not just in the committed project. Either express it in `project.yml`, or remove the dependency by giving the new suites storage that no other suite can observe.
- The pbxproj/scheme diff either matches what `xcodegen` emits or is dropped from the PR.

## Open questions for the human
- Was `parallelizable = "NO"` needed to make the new suites pass, or was it incidental to opening the project in Xcode 26? If it was needed, disabling parallel execution for the whole test bundle to accommodate two suites is a notable CI cost — is per-suite storage isolation viable instead?

ℹ️ Nitpicks

  • WebEntitlementRedeemer.swift:936-943 returns before storage.save(Date(), forType: LastWebEntitlementsFetchDate.self) (line 959), so entitlementsMaxAge stays disarmed for the entire protected window, not just for one retry. While the backend keeps answering empty, every app foreground (willEnterForegroundNotificationhandleForegroundPolling → line 882) and every refreshConfiguration issues a fresh getEntitlements call, for up to the cached entitlement's remaining lifetime. Worth a bounded retry (e.g. arming the date with a shortened age) if that traffic matters.
  • The two guards express "unexpired" differently: WebEntitlementRedeemer.swift:933-935 requires $0.isActive && ($0.expiresAt ?? .distantPast) > Date(), while AutomaticPurchaseController.swift:45 drops the isActive half. Same idea, two predicates.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Refines the guard per review: an empty entitlement set is only a
non-answer when the purchases set is completely empty. Refunded and
expired transactions stay in the set as inactive (SK2 reads
Transaction.all; the SK1 receipt keeps cancelled purchases), so a
non-empty set with no active purchases is an authoritative answer and
downgrades immediately. This closes the window where a refunded App
Store subscription kept access until its pre-refund expiry date.

A device read also has no authority over entitlements from other
stores. An unexpired Stripe/web entitlement now holds the status even
when unrelated inactive App Store purchases exist.

This mirrors RevenueCat's model: a local StoreKit read never
overwrites cached state it has no authority over
(shouldComputeOfflineCustomerInfo requires a nil cache), and their
offline path reads currentEntitlements, which already excludes
revoked transactions.

Also adds the isActive check to the guard predicate so both guards
use the same definition of an unexpired entitlement.

🌸 Shipped with Kanna — https://kanna.sh

Co-Authored-By: Kanna <noreply@kanna.sh>
Kanna-Agent: claude/fable
@jakemor

jakemor commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the review in 606f7a7.

The refund finding (important): fixed, and verified against how RevenueCat handles this. The guard now distinguishes a void read from an authoritative one using the full purchases set:

  • Refunded and expired transactions stay in the set as inactive — SK2 reads Transaction.all (revoked → isActive: false via revocationDate), and the SK1 receipt keeps cancelled purchases with cancellationDate. A non-empty set with no active purchases is an authoritative answer and downgrades immediately.
  • A completely empty set (StoreKit before hydration, missing SK1 receipt, or a web-only subscriber) is a non-answer and cannot demote an unexpired .active status.
  • A device read also has no authority over non-App-Store entitlements, so an unexpired Stripe/web entitlement holds the status even when unrelated inactive App Store purchases exist. This mirrors RevenueCat's rule: their shouldComputeOfflineCustomerInfo only consults local StoreKit when no cached CustomerInfo exists at all, and their offline path reads currentEntitlements, which already excludes revoked transactions.

Two new tests cover the refund downgrade and the web-entitlement non-refutation. Full suite: 930 tests pass.

Predicate asymmetry (nitpick): fixed — both guards now require isActive plus an unexpired expiresAt.

Generated scheme/pbxproj (warning): parallelizable = "NO" was not hand-added — it is what the current xcodegen emits, and the repo's pre-commit hook regenerates and stages the project. The new suites do not depend on it; they use @Suite(.serialized) like the 19 existing suites, and the full run passes.

Unarmed fetch date (nitpick): deliberate for fast recovery and self-limiting (the window closes at the first non-empty response or at entitlement expiry), at the cost of one getEntitlements call per foreground while in the anomalous state. Happy to arm the date with a shortened age if that traffic matters — maintainer's call.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The narrowed guard treats a nil entitlement store as App-Store-refutable. Entitlement.store is documented as nil when no transaction unlocks the entitlement, and the decoder applies no default — so a web entitlement whose payload omits store still gets demoted by a device read, which is the exact population this PR set out to protect.

Reviewed changes — delta since the prior pullfrog review at 7a81787; the single commit 606f7a7 rewrites the AutomaticPurchaseController guard and extends its test suite.

  • Scoped the anti-downgrade guard by device-read authority — an empty entitlement set now only holds .active when purchases is entirely empty (a non-answer) or the held entitlement comes from a store other than .appStore; a non-empty, all-inactive purchases set downgrades immediately.
  • Aligned the two guards' "unexpired" predicateAutomaticPurchaseController now requires entitlement.isActive alongside the future expiresAt, matching WebEntitlementRedeemer.swift:933-935.
  • Rewrote the in-code rationale — the comment now states the authoritative-answer / non-answer split explicitly rather than claiming nil-expiry is the only unprotected revocation case.
  • Added two regression teststestInactivePurchases_appStoreStatus_becomesInactive (refunded App Store subscription with a future cached expiresAt must deactivate) and testInactivePurchases_webStatus_staysActive (inactive App Store transactions must not refute a Stripe entitlement), plus a shared entitlement(store:expiresAt:) factory that stripeEntitlement now delegates to.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment on lines +62 to +64
if purchases.isEmpty {
return true
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above says "purchases exist but none is active" is the authoritative case, but the code only checks purchases.isEmpty — it never checks that no purchase was active. entitlements is derived by mapping activePurchases through entitlementsInfo.byProductId (line 31), so an active subscription whose product is missing from the entitlement map (dropped by Config.swift:114-118, or a stale cached config via ConfigManager.swift:234-238) lands here with a non-empty purchases set and gets demoted.

Technical details
# Active purchases with no entitlement mapping are treated as authoritative

## Affected sites
- `Sources/SuperwallKit/StoreKit/Purchase Controller/AutomaticPurchaseController.swift:62-64``purchases.isEmpty` is the only non-answer test; `activePurchases` is computed at line 27 and then unused by the guard.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/ReceiptManager.swift:229-231``setEntitlementsFromConfig(mergedCustomerInfo.entitlementsByProductId)` immediately precedes the sync, so the map in effect is whatever config produced this run.
- `Sources/SuperwallKit/Models/Config/Config.swift:114-118` — undecodable products are dropped silently; `Sources/SuperwallKit/Config/ConfigManager.swift:234-238` serves a cached config that can predate a product.
- `SK2ReceiptManager.swift:100-131` / `SK1ReceiptManager.swift:65-71` build `purchases` unconditionally, decoupled from the entitlement map.

## Required outcome
- An empty entitlement set derived from a non-empty *active* purchase set is treated as a non-answer, not as "nothing is active", so the comment's stated invariant holds in code.
- Existing behavior for the refunded case is preserved: `purchases` non-empty with every member inactive still deactivates.

## Suggested approach
- Note this matches develop's behavior today, so it is a gap in the fix rather than a regression — worth closing while the surrounding logic is being written.
Suggested change
if purchases.isEmpty {
return true
}
if purchases.isEmpty || !activePurchases.isEmpty {
return true
}

Comment thread CHANGELOG.md

### Fixes

- Fixes subscribers being reported as `inactive` on cold launch when StoreKit returns no purchases before it finishes loading. This hit web and Stripe subscribers hardest, because they have no App Store purchases at all. The SDK now keeps an `active` subscription status while one of its entitlements is within its expiry date.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry describes the pre-606f7a7 behavior: the status is no longer held whenever an entitlement is within its expiry date, only when the device read is inconclusive (purchases empty) or the entitlement is not from the App Store. As written, a developer would expect a refunded App Store subscription to keep access until its original expiry, which the new guard deliberately does not do.

Per review: Entitlement.store decodes with no default, so a web or
manual grant whose payload omits store is nil — and the guard treated
nil as App-Store-refutable, demoting the exact population this PR
protects. Flip the predicate so nil holds the status.

Flipping alone would break SK1 refund enforcement: SK1ReceiptManager
built its receipt-derived entitlements without a store, so they were
nil too. Stamp .appStore on them, matching what EntitlementProcessor
already does on the SK2 path. After that, every device-derived active
entitlement is explicitly .appStore and the only nil-store actives are
grants from outside the App Store, which a device read cannot refute.

The transition is fail-open: caches written by older versions hold
nil-store SK1 entitlements, which the flipped guard protects until the
first sync rewrites them with .appStore. One side effect: equality
includes store, so SK1 users get a single active-to-active status
change event on first launch after upgrading.

🌸 Shipped with Kanna — https://kanna.sh

Co-Authored-By: Kanna <noreply@kanna.sh>
Kanna-Agent: claude/fable
@jakemor

jakemor commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the nil-store finding in 0ae3358.

Flipping the predicate alone would have traded one hole for another: SK1ReceiptManager built its receipt-derived entitlements without a store, so on StoreKit 1 every App-Store-backed active entitlement was cached with nil — and a nil-protected guard would have stopped SK1 refund enforcement. Two changes together close both:

  1. SK1ReceiptManager now stamps .appStore on receipt-derived entitlements, matching EntitlementProcessor on the SK2 path. The no-receipt fallback keeps nil — those placeholders genuinely have no transactions, and the guard only inspects active entitlements.
  2. The guard predicate is now purchases.isEmpty || entitlement.store != .appStore — nil is protected, as only non-App-Store grants can be nil once both receipt paths stamp explicitly.

The migration window is fail-open: caches written by older SDK versions hold nil-store SK1 entitlements, which the flipped guard protects until the first sync rewrites them. Known side effect: Entitlement equality includes store, so SK1 users get one active→active subscriptionStatusDidChange on first launch after upgrading.

New tests: nil-store entitlement survives inactive purchases; SK1 receipt-derived entitlements carry .appStore. Full suite: 932 tests pass.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No blocking issues in this delta — the nil-store fix is correct; two minor points inline.

Reviewed changes — delta since the prior pullfrog review at 606f7a7; the single commit 0ae3358 flips the guard's store test and stamps .appStore on StoreKit 1 receipt entitlements.

  • Protected nil-store entitlements from a device readif let store = entitlement.store, store != .appStore collapsed to entitlement.store != .appStore, so an entitlement whose payload omits store now holds the status instead of being demoted.
  • Stamped store: .appStore on SK1 receipt-derived entitlements — closes the hole the flipped predicate would otherwise open on StoreKit 1, where receipt entitlements previously carried no store.
  • Rewrote the guard's rationale comment — now states that a nil store means no App Store transaction unlocks the entitlement, and that both receipt managers stamp .appStore on device-derived entitlements.
  • Added two teststestInactivePurchases_nilStoreStatus_staysActive (an inactive App Store purchase must not refute a nil-store entitlement) and testSK1Entitlements_carryAppStoreStore (SK1-derived entitlements carry .appStore).

I traced whether the flipped predicate could shield a genuine App Store entitlement from a refund: it cannot. Every path that gives a device entitlement a real expiresAt also stamps .appStore (EntitlementProcessor.swift:271, and :409-423 carries it forward), the SK2 seed and the no-transaction fallback are always isActive: false, and SK1ReceiptManager never passes an expiresAt in either branch — so the isActive && expiresAt > Date() gate already excludes them. Worth noting that last point is what actually makes nil protection safe on StoreKit 1, not the new stamp.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment on lines +100 to +105
productIds: productIds,
// Receipt-derived entitlements are App Store entitlements. The
// anti-downgrade guard relies on this: a device read may only
// refute `.appStore` entitlements, and a nil store marks an
// entitlement as granted outside the App Store (web, manual).
store: .appStore

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop emits one entitlement per config entitlement id regardless of whether any receipt purchase unlocks it, so .appStore also lands on entitlements with isActive == false and no transaction at all. Entitlement.store is documented as nil when "there aren't any transactions that unlock this entitlement" (Entitlement.swift:75-78), and the StoreKit 2 path preserves that (EntitlementProcessor.swift:288-299 reinserts the raw entitlement for the no-transaction case), so the two receipt managers now disagree on a public field.

Technical details
# SK1 stamps `.appStore` on entitlements no transaction unlocks

## Affected sites
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/SK1ReceiptManager.swift:96-107` — the second pass iterates `entitlementProductIds`, which is built from every `serverEntitlementsByProductId` entry (`:82-87`), not from `activeProductIds`. `isActive` is computed per entitlement at `:93`, but `store` is set unconditionally.
- `Sources/SuperwallKit/StoreKit/Products/StoreProduct/Entitlement.swift:75-78` — documented contract for `store`.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift:288-299` — the SK2 equivalent keeps the raw (nil-store) entitlement when no transaction exists.
- `Sources/SuperwallKit/StoreKit/Products/StoreProduct/Entitlement.swift:268, 285``store` participates in `isEqual` and `hash`.
- `Sources/SuperwallKit/Superwall.swift:602-634``$customerInfo.removeDuplicates()` drives `customerInfoDidChange`, the `CustomerInfoDidChange` event, and the `LatestCustomerInfo` write. A `LatestCustomerInfo` blob written by an older build differs from the freshly derived one purely on `store`, so every StoreKit 1 install fires one spurious change on upgrade — and because inactive entitlements are stamped too, that covers all SK1 users, not just subscribers.

## Required outcome
- `store` reflects whether a transaction in the receipt actually unlocks the entitlement, matching the documented contract and the SK2 path.
- The anti-downgrade guard keeps its property that an active SK1-derived entitlement is `.appStore`.

## Suggested approach
- `purchases` is already in scope at `:74`. Derive `let purchasedProductIds = Set(purchases.map(\.id))` alongside `activeProductIds` and pass `store: productIds.contains(where: purchasedProductIds.contains) ? .appStore : nil`. `isActive ? .appStore : nil` is a cheaper approximation but mislabels an entitlement whose only purchase has lapsed.

let receiptManager = SK1ReceiptManager(receiptData: { MockReceiptData.newReceipt })
let entitlement = Entitlement(id: "pro")
let snapshot = await receiptManager.loadPurchases(
serverEntitlementsByProductId: ["com.nutcallalert.inapp.optimum": [entitlement]]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MockReceiptData.newReceipt contains CYCLEMAPS_PREMIUM (bundle net.zachariadis.cyclemaps), not com.nutcallalert.inapp.optimum — that id lives in noOriginalPurchaseDateCrashReceipt and legacyReceipt. Nothing in the receipt maps to pro, so the derived entitlement is isActive: false and the assertion never exercises the active, refund-enforcing entitlement the comment above says it protects; the receipt data is effectively inert here. Either key on a product the fixture actually contains, or drop the receipt and say plainly that the stamp is unconditional.

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.

1 participant