Keep subscribers active through empty StoreKit reads and empty web polls - #506
Keep subscribers active through empty StoreKit reads and empty web polls#506jakemor wants to merge 4 commits into
Conversation
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
|
PR author is not in the allowed authors list. |
There was a problem hiding this comment.
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.activeand any of its entitlements hasexpiresAtin the future; nil-expiry entitlements are unprotected. superwall:injection parameter — added tosyncSubscriptionStatusas a test seam, defaulting toSuperwall.shared, matching the pattern already used byWebEntitlementRedeemerandinternallySetSubscriptionStatus.- Empty-poll guard in
WebEntitlementRedeemer.pollWebEntitlements— a zero-entitlementgetEntitlementsresponse no longer overwritesLatestRedeemResponsewhen a cached entitlement isisActiveand unexpired, and returns beforeLastWebEntitlementsFetchDateis armed. DeviceHelper.fontScalearithmetic —Double(scaledValue)disambiguation for the Xcode 26.0.1 type-checker; numerically identical.- Tests — a new
AutomaticPurchaseControllerTestssuite (7 tests) plus 2WebEntitlementRedeemerTestscases covering the empty-poll guard and its expired-entitlement counter-case. - Generated project files —
SuperwallKit.xcodeprojbumped toobjectVersion = 77with the new test file wired in, and the shared scheme gainsparallelizable = "NO". - CHANGELOG — new
## Unreleased→### Fixessection 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-943returns beforestorage.save(Date(), forType: LastWebEntitlementsFetchDate.self)(line 959), soentitlementsMaxAgestays disarmed for the entire protected window, not just for one retry. While the backend keeps answering empty, every app foreground (willEnterForegroundNotification→handleForegroundPolling→ line 882) and everyrefreshConfigurationissues a freshgetEntitlementscall, 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-935requires$0.isActive && ($0.expiresAt ?? .distantPast) > Date(), whileAutomaticPurchaseController.swift:45drops theisActivehalf. Same idea, two predicates.
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
|
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:
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 Generated scheme/pbxproj (warning): 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 |
There was a problem hiding this comment.
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
.activewhenpurchasesis entirely empty (a non-answer) or the held entitlement comes from a store other than.appStore; a non-empty, all-inactivepurchasesset downgrades immediately. - Aligned the two guards' "unexpired" predicate —
AutomaticPurchaseControllernow requiresentitlement.isActivealongside the futureexpiresAt, matchingWebEntitlementRedeemer.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 tests —
testInactivePurchases_appStoreStatus_becomesInactive(refunded App Store subscription with a future cachedexpiresAtmust deactivate) andtestInactivePurchases_webStatus_staysActive(inactive App Store transactions must not refute a Stripe entitlement), plus a sharedentitlement(store:expiresAt:)factory thatstripeEntitlementnow delegates to.
Claude Opus | 𝕏
| if purchases.isEmpty { | ||
| return true | ||
| } |
There was a problem hiding this comment.
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.| if purchases.isEmpty { | |
| return true | |
| } | |
| if purchases.isEmpty || !activePurchases.isEmpty { | |
| return true | |
| } |
|
|
||
| ### 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. |
There was a problem hiding this comment.
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
|
Addressed the nil-store finding in 0ae3358. Flipping the predicate alone would have traded one hole for another:
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: New tests: nil-store entitlement survives inactive purchases; SK1 receipt-derived entitlements carry |
There was a problem hiding this comment.
ℹ️ 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 read —
if let store = entitlement.store, store != .appStorecollapsed toentitlement.store != .appStore, so an entitlement whose payload omitsstorenow holds the status instead of being demoted. - Stamped
store: .appStoreon 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
.appStoreon device-derived entitlements. - Added two tests —
testInactivePurchases_nilStoreStatus_staysActive(an inactive App Store purchase must not refute a nil-store entitlement) andtestSK1Entitlements_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.
Claude Opus | 𝕏
| 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 |
There was a problem hiding this comment.
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]] |
There was a problem hiding this comment.
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.

The problem
A paying Stripe subscriber flipped to
INACTIVEten times at cold launch in production, 0.8s after start. The mechanism:pollWebEntitlementsis keyed onappUserId/deviceIdalone, and a response with zero entitlements overwrote the cache unconditionally..inactiveand 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.activestatus 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 outentitlementsMaxAge.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:494on Xcode 26.0.1 (mixed CGFloat/Double expression).🌸 Shipped with Kanna — an open-source workspace for all your coding agents. Written by
claude/fable.