Skip to content

fix(entitlements): resolve each subscription group independently - #518

Open
yusuftor wants to merge 11 commits into
developfrom
feat/per-group-entitlement-resolution
Open

yusuftor wants to merge 11 commits into
developfrom
feat/per-group-entitlement-resolution

Conversation

@yusuftor

@yusuftor yusuftor commented Sep 7, 2026 •

Copy link
Copy Markdown
Collaborator

Description

An entitlement backed by more than one App Store subscription group was resolved from a single most-recently-purchased transaction. Transaction.subscriptionStatus only reports the state of that transaction's subscription group, so a refund in one group was applied to the whole entitlement and deactivated a paid, active subscription in another group. Whether it happened depended purely on which group held the newest transaction, and the scalar fields were spliced across groups: state and willRenew from one, expiresAt from another.

Real-world case this fixes:

  • Jul 18 — user starts a 7-day trial on a monthly product (group A).
  • Jul 25 — trial ends, billing fails, Apple puts it in billing retry.
  • Aug 2 — user buys a yearly product (group B), valid until Aug 2027. Same entitlement, different subscription group.
  • Aug 15 — Apple's retry on the group A sub goes through and charges.
  • Aug 16 — Apple refunds that charge.

The refunded Aug 15 renewal is the most recent transaction, so it won the resolution, came back revoked, and the SDK marked the whole entitlement inactive — even though the group B yearly was still paid for and active, and the payload listed it as active.

Changes

  • Transactions are split into independent grant sources — one per subscription group, plus one for a lifetime purchase — each resolved on its own.
  • The entitlement is active if any source grants it.
  • Scalar fields are taken from the source actually granting access (lifetime first, then the active source with the most time left), falling back to the most recent purchase only when nothing is active.
  • Each source's live status is authoritative in both directions for its own group, so a lapsed subscription in its grace period is reported active rather than judged inactive on its expiry date alone. Billing retry still defers to the dates, since it says nothing about whether the paid-for period has run out.
  • SubscriptionStatusProvider is keyed on EntitlementTransaction instead of StoreKit.Transaction so resolution can be tested without minting real StoreKit transactions, and statuses are cached per transaction so a group shared by several entitlements costs one lookup.

Testing

New PerGroupEntitlementResolutionTests suite (16 tests) covering the refund-in-one-group case, grace periods, billing retry, lifetime purchases, and scalar field selection. Full suite: 1010 tests in 101 suites passing on iPhone 17 Pro.

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs on iOS.
  • Demo project builds and runs on Mac Catalyst.
  • Demo project builds and runs on visionOS.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run swiftlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

🤖 Generated with Claude Code

RetriggerConfidence Score: 5/5

The PR appears safe to merge; both previous findings are resolved and no actionable new defect remains.

Summary

The PR resolves entitlement access independently for each App Store subscription group instead of allowing one group’s status to override every purchase granting the entitlement.

  • Partitions transactions into independent subscription-group and lifetime grant sources.
  • Selects entitlement scalar metadata from the source actually granting access while preserving purchase-recency semantics for device-level latest-subscription variables.
  • Reconciles StoreKit 2 purchase activity with every grant source, including subscriptions in billing grace periods.
  • Adds comprehensive per-group, refund, grace-period, lifetime, and scalar-selection tests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  T[StoreKit transactions] --> P[Partition by subscription group]
  T --> L[Lifetime purchase source]
  P --> S1[Resolve group A live status]
  P --> S2[Resolve group B live status]
  S1 --> A{Any source grants access?}
  S2 --> A
  L --> A
  A --> E[Build entitlement]
  E --> R[Select granting source for scalar fields]
  S1 --> G[Collect granting product IDs]
  S2 --> G
  G --> C[Correct StoreKit 2 purchase activity]
Loading

Reviews (3) · Last reviewed commit: "fix(entitlements): count every granting ..."

An entitlement backed by more than one subscription group was resolved
from a single most-recently-purchased transaction. Because
`Transaction.subscriptionStatus` reports the state of that transaction's
subscription group only, a refund in one group could be applied to the
whole entitlement and deactivate a paid, active subscription in another.
Whether it did depended purely on which group happened to hold the newest
transaction, and the scalar fields were spliced across groups: `state` and
`willRenew` from one, `expiresAt` from another.

Transactions are now split into independent grant sources — one per
subscription group, plus one for a lifetime purchase — each resolved on
its own. The entitlement is active if any source grants it, and its
scalar fields are taken from the source that is actually granting access
(lifetime, then the active source with the most time left), falling back
to the most recent purchase only when nothing is active.

Each source's live status is now authoritative in both directions for its
own group, so a lapsed subscription in its grace period is active rather
than being judged inactive on its expiry date alone. Billing retry still
defers to the dates, since it says nothing about whether the paid-for
period has run out.

`SubscriptionStatusProvider` is keyed on `EntitlementTransaction` instead
of `StoreKit.Transaction` so the resolution can be tested without minting
real StoreKit transactions, and statuses are cached per transaction so a
group shared by several entitlements costs one lookup.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated

@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 per-group split itself is sound and well tested, but making live status authoritative upward breaks an invariant that downstream code relies on — Entitlement.isActive == true with expiresAt in the past is now reachable, and Purchase.isActive / AutomaticPurchaseController still gate on a future expiry. That plausibly stops the CHANGELOG's grace-period fix from taking effect end to end. Three public Entitlement fields also changed meaning without their docs being updated.

Reviewed changes — full diff of the single commit 73ef1abf4 across EntitlementProcessor.swift, both test files, CHANGELOG.md and the generated project.pbxproj, plus the production caller (SK2ReceiptManager), the Entitlement model and AutomaticPurchaseController for downstream impact.

  • Grant sources replace single-transaction resolution — grantSources(for:) buckets an entitlement's transactions by "lifetime" / "group:<id>" / "product:<id>", resolves each from dates alone, and the entitlement is active if any source grants it.
  • Scalar fields now come from one chosen source — representativeSource(from:) prefers an active lifetime, then the active source with the furthest expiresAt, falling back to the most recent purchase when nothing is active; it supplies latestProductId, expiresAt, renewedAt, willRenew, state and offerType.
  • Live status is authoritative in both directions — .subscribed/.inGracePeriod force a source active, .revoked/.expired force it inactive, .inBillingRetryPeriod/nil defer to dates. Pre-PR the override only ever forced inactive.
  • SubscriptionStatusProvider collapsed to one method — resolveStatus(for: any EntitlementTransaction) async -> ResolvedSubscriptionStatus?, with results cached per representative transaction ID so a group shared by several entitlements costs one lookup.
  • Shared enrichment path — a private buildEntitlements(from:sourcesByEntitlement:…) is now used by both the date-only and live-status entry points, removing the previous build-then-patch two-pass structure.
  • New PerGroupEntitlementResolutionTests (16 tests) — the refund-across-groups, expired-group and grace-period cases genuinely fail against the pre-PR logic, so this is real coverage rather than restated behaviour.

⚠️ An active entitlement can now carry an expiry date in the past, and the paths that gate on that weren't updated

Before this PR, an App Store entitlement could only be isActive == true if some unrevoked transaction had expirationDate > now, so isActive implied expiresAt in the future (or nil for lifetime). The grace-period fix breaks that: the override sets isActive = true without recomputing expiresAt, which still holds the lapsed date. Two consumers were built on the old invariant and are untouched by this PR, so the entitlement reads active while the SDK's user-facing status does not.

Technical details
# Grace-period activation does not survive the internal purchase-controller path

## Affected sites
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift:439-451` —
  `.inGracePeriod` (and `.subscribed`) force `sources[index].isActive = true`; only
  `willRenew`, `state` and `offerType` are refreshed above, so `expiresAt` keeps the
  already-passed value that `grantSources` computed from the transaction dates.
- `Sources/SuperwallKit/StoreKit/Purchase Controller/AutomaticPurchaseController.swift:71-72` —
  `syncSubscriptionStatus(withPurchases:)` builds its entitlement set from
  `purchases.filter { $0.isActive }`, not from the processor's entitlements. A
  grace-period product yields no active purchase, so the set is empty and the code
  falls into the "non-answer" branch, whose hold requires
  `entitlement.isActive && (entitlement.expiresAt ?? .distantPast) > Date()`. The
  grace-period entitlement fails the expiry half, `holdsStatus` is false, and
  `internallySetSubscriptionStatus(to: .inactive)` runs.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/SK2ReceiptManager.swift:124`
  and `:173-186` — `Purchase.isActive` comes from `isAnyTransactionActive` (raw dates),
  and the correction block below only ever moves a purchase active → inactive, so it
  never picks up the newly-active grace-period entitlement.
- `Sources/SuperwallKit/StoreKit/Products/StoreProduct/Entitlement.swift:123-131` —
  `renewsAt` returns `expiresAt` whenever `isActive && willRenew == true`, so a
  grace-period entitlement now advertises a renewal date in the past.

## Required outcome
- After this change, `customerInfo.entitlements` and `Superwall.subscriptionStatus`
  must agree for a subscription in its billing grace period — the CHANGELOG bullet
  claims the user is no longer reported inactive, and today the automatic purchase
  controller still demotes them.
- Any consumer that treats `expiresAt` as "this entitlement is good until" (that
  includes the wire payload and audience filters, since `expiresAt` is always encoded)
  needs a defined answer for an active entitlement whose expiry has passed.

## Suggested approach (optional)
- Carry the grace-period end date onto the source when the override fires
  (`Product.SubscriptionInfo.RenewalInfo.gracePeriodExpirationDate` is available on the
  status the provider already fetches) and surface it as `expiresAt`, which repairs the
  invariant for every downstream gate at once rather than patching each one.

## Open questions for the human
- Is `Purchase.isActive` meant to track the corrected entitlement state, or is the
  divergence from `Entitlement.isActive` acceptable here?

ℹ️ Nitpicks

  • startsAt changed from transactions.last?.originalPurchaseDate to transactions.map(\.originalPurchaseDate).min() (EntitlementProcessor.swift:336). .min() is a genuine improvement over an iteration-order-dependent read and matches the field's doc better, but it now includes revoked transactions and consumables, neither of which ever unlocked the entitlement. No test asserts startsAt for a multi-transaction entitlement — testProcessRenewalTracking and testProcessTransactionsWithExtremeDates are both single-transaction, where .last and .min() coincide.
  • Coverage gaps in the new suite worth closing while the logic is fresh: no test asserts renewedAt, none asserts latestProductId for an entitlement with two active groups where the most-recent purchase is not the furthest-expiring one, and none pairs a revoked transaction with a .subscribed/.inGracePeriod status.
  • The comment at EntitlementProcessor.swift:401-404 says the cache means "each group costs one status lookup". It's keyed on the representative transaction ID, so two entitlements sharing a group but covering different product subsets within it still take two lookups. Harmless, but the comment overstates it.

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

Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated
Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated
yusuftor and others added 2 commits September 10, 2026 14:35
…xpiry

The `latestSubscription` device variables describe the newest subscription
on the device, so picking the source with the most time left on it could
report a yearly bought last year over a monthly bought yesterday. They now
follow purchase recency, and the winner is picked across every entitlement
rather than left to dictionary iteration order.

Three more things the per-group split left loose:

- A group reporting `.subscribed` could resurrect a source whose every
  transaction had been refunded. It now needs an unrevoked transaction.
- Forcing a lapsed source active left its expiry date in the past, so
  `Purchase.isActive` and `AutomaticPurchaseController` still read the
  entitlement as lapsed and demoted a subscriber in their grace period.
  The status now carries the date its access runs to — the end of the
  grace period, or the renewal date — and the purchase correction reads
  the granting product's entitlement in both directions.
- `startsAt` no longer counts refunded purchases or consumables, neither
  of which ever unlocked the entitlement.

Updates the `Entitlement` docs for the three fields whose meaning the
split changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ng one

`renewedAt` isn't a property of the current subscription period, so taking
it from the source describing the entitlement dropped a renewal that
happened in another group. It's now the latest renewal across every source.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@yusuftor

yusuftor commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator Author

Addressing the review findings

Pushed cfb87386b and 4be51bd36. All 1025 tests in 101 suites pass; scripts/lint.sh is clean apart from a pre-existing function_body_length violation in SK1ReceiptManager.swift.

The active-entitlement-with-a-past-expiry invariant

@pullfrog was right that the grace-period fix didn't survive the internal purchase-controller path, and that repairing the invariant beats patching each gate. Both halves are done:

  1. expiresAt moves with the grant. ResolvedSubscriptionStatus now carries activeUntil — gracePeriodExpirationDate when there is one, else renewalDate — and the .subscribed/.inGracePeriod override raises expiresAt to it. It only ever moves forward, so a live status can't shorten a period the user has already paid for. A grace-period entitlement now reports the end of the grace period, which renewsAt and the AutomaticPurchaseController expiry gate both read correctly.

  2. Purchase.isActive follows the corrected entitlement. The expiry fix alone wasn't enough: syncSubscriptionStatus(withPurchases:) builds its entitlement set from active purchases, and a grace-period product had none, so it still landed in the non-answer branch and failed the activeProductIds check. The correction block in SK2ReceiptManager is now bidirectional and extracted as correctPurchases(_:using:) so it's testable: all-inactive entitlements still deactivate a purchase, and a purchase that is an active entitlement's latestProductId is activated. Gating the upward direction on latestProductId is what stops a refunded subscription looking active because a different group unlocks the same entitlement — four tests cover it in SK2ReceiptManagerTests.

So to answer the open question directly: Purchase.isActive is meant to track the corrected entitlement state, and the divergence was a bug.

Nitpicks

All three taken:

  • startsAt now filters out revoked transactions and consumables before .min(), with a test.
  • Coverage gaps closed: renewedAt across groups, latestProductId where the most recent purchase isn't the furthest-expiring, and a revoked transaction paired with .subscribed.
  • The cache comment no longer claims one lookup per group — it's keyed on the transaction the group was resolved from.

Note for whoever runs the tests locally

Nothing to fix in this repo, but worth knowing: Xcode 27 can no longer build for iOS 13. Its SDK declares the floor itself — iPhoneSimulator.sdk/SDKSettings.plist has MinimumDeploymentTarget = 15.0 — so xcodebuild rejects the project before compiling anything:

error: The iOS Simulator deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 13.0,
but the range of supported deployment target versions is 15.0 to 27.0.x.

The SDK's 13.0 floor is deliberate and consistent across project.yml, Package.swift and the podspec, so I left it alone and passed IPHONEOS_DEPLOYMENT_TARGET=15.0 on the command line for the test run. Our Xcode workflows pin xcode-version: latest-stable, so they'll hit this whenever that flips to 27 — that's a decision about the SDK's support floor rather than anything to change here.

🤖 Generated with Claude Code

@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 prior round's concerns are addressed well — hasUnrevokedTransaction closes the resurrection hole, activeUntil repairs the active-but-expired invariant, and the two-directional correctPurchases carries the grace-period fix through AutomaticPurchaseController end to end. One gap remains: GrantSource.representative is still the newest transaction in the bucket including revoked ones, and the new upward correction turns that into a refunded purchase reported as active.

Reviewed changes — the delta since 73ef1abf4: cfb87386b across EntitlementProcessor.swift, SK2ReceiptManager.swift, Entitlement.swift, CHANGELOG.md and both test suites, plus the downstream consumers of Purchase.isActive (ReceiptManager.isSubscribed(to:), getActiveProductIds(), computeActiveSubscriptionGroupIds, AutomaticPurchaseController) and Apple's Product.SubscriptionInfo.RenewalInfo contract.

  • Guarded the upward live-status override — GrantSource.hasUnrevokedTransaction now gates the .subscribed/.inGracePeriod branch, so a group whose every transaction is revoked can no longer be resurrected by its group status.
  • Carried the real expiry onto an activated source — ResolvedSubscriptionStatus.activeUntil (grace-period end, else renewalInfo.renewalDate on iOS 17.2+) moves expiresAt forward when the live status forces a source active, so an active entitlement no longer advertises a date that has already passed.
  • Made correctPurchases two-directional and testable — extracted to a static on SK2ReceiptManager; a purchase is now forced active when it is the latestProductId of an active entitlement, which is what lets the grace-period fix reach AutomaticPurchaseController's .active(entitlements) branch.
  • Separated "latest subscription" from "granting source" — latestSubscriptionSource(from:) picks by purchase recency and the callback fires once after the loop, removing both the furthest-expiry mis-selection and the dictionary-iteration-order nondeterminism.
  • Narrowed startsAt — revoked transactions and consumables no longer date the start of an entitlement.
  • Rewrote the latestProductId / expiresAt / willRenew docs to describe granting-source semantics, and added CHANGELOG entries for the grace-period expiry and scalar-field coherence.
  • Ten new tests across PerGroupEntitlementResolutionTests (latest-subscription recency, revoked sources, three activeUntil cases, renewedAt/startsAt) and four in SK2ReceiptManagerTests for correctPurchases.

ℹ️ Nitpicks

  • Entitlement.renewedAt's doc (Entitlement.swift:90-96) was left behind while its three siblings were rewritten. It now also comes from the granting source alone — deliberately, per renewedAtComesFromTheGrantingGroup — so its nil-conditions list should say so too.
  • No test covers startsAt when every transaction is revoked, which now yields nil where the field previously always carried a date. Worth pinning if that's the intended reading of "there aren't any transactions that unlock this entitlement".

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

Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated
`GrantSource.representative` was the newest transaction in the bucket
including revoked ones, so a refund that landed after the purchase still
paying could name the refunded product as the one granting the
entitlement — and with the new upward purchase correction, that marked
the refunded purchase active. It now prefers the newest unrevoked
transaction, falling back to the whole bucket so a fully refunded source
can still report its last known state.

Also documents that `startsAt` skips refunds and consumables, and that
`renewedAt` counts a renewal in any group.

NOT VERIFIED: the local Xcode install is mid-update and its iOS SDK
plist is malformed, so neither xcodebuild nor swift build can run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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 invariant repair lands, and correctPurchases' new upward direction is what finally carries a grace-period grant through AutomaticPurchaseController — the previous review's main concern is genuinely closed end to end. Two things in the delta still need attention: GrantSource.representative is still picked without filtering revoked transactions, so the new upward correction can mark a refunded product's Purchase active, and the renewalDate half of the expiry fix is gated behind an availability check the API doesn't need.

Reviewed changes — the delta since pullfrog's last review (73ef1abf4), i.e. commits cfb87386b and 4be51bd36, read against the full PR diff for context plus every downstream consumer of Purchase.isActive (ReceiptManager.isSubscribed(to:), getActiveProductIds(), computeActiveSubscriptionGroupIds, AutomaticPurchaseController.syncSubscriptionStatus(withPurchases:)).

  • Repaired the active-with-past-expiry invariant — ResolvedSubscriptionStatus gained activeUntil (grace-period end, else renewalDate), and the .subscribed/.inGracePeriod override raises GrantSource.expiresAt to it under a strict > guard so a live status can only ever move the date forward.
  • Gated the upward override on an unrevoked transaction — GrantSource.hasUnrevokedTransaction is now required before .subscribed/.inGracePeriod can flip a source active, so a group status can't resurrect a fully-refunded source. revokedSourceDoesNotDescribeTheEntitlement also pins the expiresAt == nil → .distantFuture amplifier that used to let such a source outrank a paying one.
  • Made Purchase.isActive bidirectional — the correction block moved out of loadPurchases into a testable SK2ReceiptManager.correctPurchases(_:using:); a purchase that is an active entitlement's latestProductId is now forced active, gated on latestProductId so a different group's grant can't reactivate a refunded product. I traced all four readers: each collapses the set to product ids or entitlements before deciding, so the fan-out across a product's many renewal-period Purchase entries is harmless.
  • Reselected the latest-subscription source by purchase recency — latestSubscriptionSource(from:) picks the most recently bought non-lifetime source, and the callback moved out of the per-entitlement loop so it fires exactly once instead of last-dictionary-key-wins. This restores the pre-PR recency semantics, which also answers Greptile's inline finding.
  • Widened renewedAt, narrowed startsAt — renewedAt is now the max across all sources rather than the granting source's, and startsAt filters revoked transactions and consumables before .min().
  • Documented the granting-source semantics — latestProductId, expiresAt and willRenew doc comments rewritten, with two matching CHANGELOG bullets under 4.17.0 (which matches Constants.swift).

Verified against Apple's docs that activeUntil cannot overstate paid access: RenewalState documents .subscribed and .inGracePeriod as the two states "entitled to service", gracePeriodExpirationDate is nil outside a grace period, and renewalDate is "the expiration date of the most recent auto-renewable subscription purchase" rather than a speculative future date — so cancelled, expired and pending-change subscriptions all resolve to the boundary the customer already paid for.

ℹ️ Nitpicks

  • Entitlement.renewedAt's doc comment (Entitlement.swift:90-96) wasn't updated alongside the other three. Its semantics changed in this same delta — it is now the latest renewal in any subscription group, deliberately unlike latestProductId/expiresAt/willRenew, which the delta's new docs scope to the granting purchase. Worth one line saying so, since the four fields now read as if they all describe the same purchase.
  • The comment above for _ in 0..<10 in latestSubscriptionIsPickedAcrossEntitlements ("a last-one-wins pick would only sometimes land on the wrong one") overstates what the loop buys. Swift seeds Dictionary hashing once per process, so all ten iterations observe the same ordering — the loop doesn't sample orders, and against the old code the test would fail or pass consistently within a run depending on that process's seed. The test is still worth keeping; the comment just describes a guarantee it doesn't have.
  • A fully-refunded entitlement now reports startsAt == nil while latestProductId and state still name the refunded product, so the object simultaneously reads as "nothing ever unlocked this" and "this product did". Consistent with startsAt's doc as written, but the pairing is odd enough to be worth a deliberate decision.

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

Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated
Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated
yusuftor and others added 3 commits September 18, 2026 16:39
`getActiveUntil` gated `RenewalInfo.renewalDate` behind iOS 17.2, so on
iOS 15.0-17.1 it returned nil outside a grace period and the expiry
repair silently did nothing — leaving exactly the active entitlement with
a past `expiresAt` the method exists to prevent.

`renewalDate` is `@available(iOS 15.0)` and `@backDeployed(before: iOS
17.0)`, so the implementation ships in the app binary and works across
the enclosing type's whole availability range. The gate was copied from
`getOfferType`, where 17.2 is genuinely required because
`Transaction.offer` is. Verified against the iOS 27.0 SDK's StoreKit
swiftinterface.

Not unit tested: `getActiveUntil` takes a StoreKit-minted
`SubscriptionInfo.Status`, which the test suite can't construct. The
behaviour it feeds is covered through `MockSubscriptionStatusProvider`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`latestSubscriptionFollowsPurchaseRecency` left `group_1` `.expired`,
which forced its source inactive and left `group_2` as the only active
one — so the old expiry-ordered selection picked `group_2` too and the
test passed either way. It never created the recency-vs-time-left tension
its name describes.

`group_1` is now `.subscribed`, so it stays active with the furthest
expiry. Confirmed it fails against the previous selection: reverting the
callback to `representativeSource` breaks both `willRenew` and
`offerType` assertions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two conflicts, both from parallel additions rather than overlapping work:

- CHANGELOG.md: develop and this branch each appended to 4.17.0's Fixes
  list. Kept both sides.
- SuperwallKit.xcodeproj/project.pbxproj: generated, so regenerated with
  xcodegen from the merged project.yml rather than hand-resolved.

SK2ReceiptManager.swift auto-merged; no source conflicts.

Full suite green on the merged tree: 1115 tests in 111 suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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 critical issues — one minor suggestion inline.

All three threads from the previous review are genuinely closed: representative now prefers an unrevoked transaction, the renewalDate availability gate is gone, and the recency test was made to discriminate (I confirmed it fails against representativeSource selection and passes against latestSubscriptionSource). The remaining point is a narrow edge of the representative fix that affects reported metadata only, not access.

Reviewed changes — the delta since pullfrog's last review (4be51bd36): commits 10c64c1a8, 02a0164a4, b4ea3bcf8 and the develop merge e8e85fa83, read against the full PR diff plus every consumer of GrantSource.representative (latestProductId, latestPurchaseDate, willRenew, the resolveStatus call and its statusCache key, the subscriptions index match) and the SubscriptionTransaction readers downstream of them.

  • Preferred a still-granting transaction as the source's representative — grantSources(for:) now picks the newest unrevoked member, falling back to the whole bucket when every member is revoked, so latestProductId, expiresAt and willRenew describe the same purchase and a refunded product can no longer reach correctPurchases' isGranting branch. refundInsideAGroupDoesNotDescribeIt pins the monthly/yearly case and would fail against the previous selection.
  • Dropped the #available(iOS 17.2, …) gate on renewalInfo.renewalDate — renewalDate carries the same base availability as gracePeriodExpirationDate (already unguarded on the line above) plus @backDeployed(before: iOS 17.0, …), so this adds no platform constraint and extends the stale-expiry half of the fix down to iOS 15.
  • Documented startsAt's revoked/consumable exclusion and renewedAt's cross-group semantics — the two remaining doc fields now match the behaviour the earlier commits gave them, and startsAtIsNilWhenEverythingIsRefunded turns the nil reading into a deliberate, pinned decision.
  • Merged develop — base moved to 29413229c (DevServer and experiment-tracking work, unrelated). The CHANGELOG merged cleanly: all four entitlement bullets intact under 4.17.0, which still matches Constants.swift and the podspec, and the generated project.pbxproj picked up both sides' test-file registrations without losing SK2ReceiptManagerTests.swift.

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

Comment thread Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift Outdated
…a lapsed one

Two fixes from the same review round, both about which purchase speaks
for a subscription group.

`correctPurchases` marked a purchase active only when it was the
entitlement's `latestProductId`. With one entitlement unlocked by two
groups, a group in its billing grace period lost that contest to a group
with more time left, so its product stayed inactive and dropped out of
`isSubscribed(to:)` and the `activeProducts` device attribute even though
it was still paid for. The processor now reports the product of every
active grant source and `correctPurchases` keys off that, which is a more
direct statement of what the `latestProductId` restriction was reaching
for: a refunded group is never active, so it still can't ride on another
group unlocking the same entitlement. Not a regression — develop doesn't
correct purchases at all — but the delta from 4.16.3 is user-visible, so
it gets a changelog entry.

`GrantSource.representative` fell back to the whole bucket only when
every member was revoked, so a group holding an old lapsed transaction
plus a newer refunded one described itself by the old one. The fallback
now applies whenever the source grants nothing. The rule is that while a
source is granting, only a purchase that still counts may describe it;
once it grants nothing there is no such claim to protect and the most
recent purchase describes it. That keeps the earlier refund fix intact —
it only ever applied to active sources — and makes a refund
distinguishable from a plain lapse, which matters for targeting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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 critical issues — one minor suggestion inline.

The widened fallback closes the previous thread cleanly, and onGrantingProductIds is the right shape for the grace-period gap: it decouples "which product describes the entitlement" from "which products are paid for", which is exactly the distinction Greptile's P1 was about. I re-traced every consumer of the widened purchase set and found no eligibility or status regression — in particular intro-offer suppression reads SubscriptionTransaction.isActive, which this path never mutates. The one remaining point is that the representative selection keys off the pre-live-status active flag, which leaves a narrow window where the refunded product is the one named as granting.

Reviewed changes — the delta since pullfrog's last review (e8e85fa83): the single commit 7b47687a6 across EntitlementProcessor.swift, SK2ReceiptManager.swift, both test suites and CHANGELOG.md, read against the full PR diff plus every reader of SK2ReceiptManager.purchases (ReceiptManager.isSubscribed(to:) → TemplateLogic's per-product isSubscribed variable, getActiveProductIds() → DeviceHelper.activeProducts, computeActiveSubscriptionGroupIds, AutomaticPurchaseController.syncSubscriptionStatus(withPurchases:), ReceiptManager+Restore).

  • Widened the representative fallback to any non-granting source — candidates is now isActive && !unrevoked.isEmpty ? unrevoked : bucket, so a group that grants nothing describes itself by its most recent purchase whether or not an old unrevoked sibling survives. refundDescribesALapsedGroup pins it and fails against the previous unrevoked.isEmpty fallback. This makes Entitlement.latestProductId's documented nothing-active rule true as written, so the prior thread is genuinely closed and the author resolved it.
  • Reported every granting product, not just the describing one — a new onGrantingProductIds callback collects source.latestProductId for each active GrantSource after live-status correction, and correctPurchases' isGranting is now set membership instead of latestProductId == purchase.id. A group in its billing grace period alongside a longer-running group now reaches isSubscribed(to:) and activeProducts instead of being dropped.
  • Confirmed intro-offer eligibility is untouched by the widening — computeActiveSubscriptionGroupIds derives its suppression set from customerInfo.subscriptions.filter(\.isActive), and SubscriptionTransaction.isActive is set once in processTransactions and never mutated by the per-source correction (only willRenew / isInGracePeriod / isInBillingRetryPeriod are). isEligibleForIntroOffer is resolved live from StoreKit, and ReceiptManager+Restore builds its own Purchase set without calling correctPurchases.
  • Three new discriminating tests — gracePeriodGroupIsReportedAsGranting asserts grantingProductIds == ["monthly", "yearly"] while latestProductId == "yearly" (the old rule would have yielded only the yearly), refundDescribesALapsedGroup covers the widened fallback end to end, and gracePeriodGroupIsActiveAlongsideALongerGroup unit-tests the correctPurchases half.
  • Added the matching CHANGELOG bullet — isSubscribed(to:) and activeProducts no longer leave out a grace-period product, under the existing 4.17.0 section.

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

Widening the representative fallback in the previous commit introduced an
ordering bug. `candidates` keyed off the date-only `isActive` computed in
`grantSources`, but the live subscription status promotes a source later,
in `buildEntitlementsWithLiveSubscriptionData`. In the window where
`Transaction.all` hasn't caught up — an unrevoked lapsed transaction plus
a newer refunded one, with the group reporting `.subscribed` — the source
was resolved as inactive, so the refunded transaction named it, and then
the live status flipped it active. The refunded product reached
`Entitlement.latestProductId`, `grantingProductIds`, and from there
`isSubscribed(to:)` and `activeProducts`.

`GrantSource` now stores both candidates and derives `representative`
from `isActive`, so it follows the promotion instead of being fixed
before it. The ordering can't be got wrong because there is no longer an
order. `hasUnrevokedTransaction` is derived from the same stored value
rather than tracked separately.

The status lookup reads a new `statusTransaction` that never moves.
Resolving it from `representative` would have made the lookup depend on
the value that lookup decides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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 new issues — one test-coverage suggestion inline and two observations below.

ad376136c closes the previous review's thread properly, and I agree with the author that the structural fix beats both narrower options I suggested. Making representative derive from isActive rather than be fixed before it establishes an invariant I verified holds on every path: isActive == true ⇒ unrevokedRepresentative != nil ⇒ representative is unrevoked. Date-only isActive requires unrevoked.contains { … }, lifetime buckets drop revoked members at bucketing time, and live promotion is gated on hasUnrevokedTransaction — so a refunded product can no longer reach grantingProductIds or an active entitlement's latestProductId, with no ordering left to get wrong.

Reviewed changes — the delta since pullfrog's last review (7b47687a6): the single commit ad376136c across EntitlementProcessor.swift and PerGroupEntitlementResolutionTests.swift, read against the full PR diff plus every consumer of the retargeted write-back (CustomerInfo.subscriptions, ReceiptManager+Restore.deactivated(_:), CustomerInfo.activeSubscriptionProductIds, AddPaywallProducts.hasActiveIntroOffer, the persisted JSON) and every reader of GrantSource.representative.

  • Replaced the fixed representative with two stored candidates and a derived one — GrantSource now holds unrevokedRepresentative and lastKnownRepresentative, and representative picks between them off the live isActive, so it follows a promotion instead of being decided before it. hasUnrevokedTransaction falls out of the same value rather than being tracked separately.
  • Split the status lookup onto a stable statusTransaction — unrevokedRepresentative ?? lastKnownRepresentative, which never moves, so the resolveStatus call, the statusCache key and the subscriptions index match can't depend on the value that lookup goes on to change.
  • Retargeted the live-status write-back as a side effect — for a bucket that is date-inactive with a newer refunded member, willRenew / isInGracePeriod / isInBillingRetryPeriod now land on the newest unrevoked SubscriptionTransaction instead of the refunded one. I traced every reader and this is an improvement or a no-op in each; Entitlement's scalars are untouched because both candidates share a subscription group, so Transaction.subscriptionStatus answers identically either way.
  • Added promotedGroupIsNotNamedByARefund — a lapsed-but-unrevoked txn_a paired with a newer refunded txn_b under a .subscribed group status. Both assertions are exact and both would read "monthly_b" under 7b47687a6, so it discriminates.

ℹ️ The demotion direction of the now-movable representative is a silent behaviour change

representative moves both ways. When a source is active by its transaction dates but the live status says .revoked/.expired, isActive flips to false and the representative moves from the newest unrevoked transaction to lastKnownRepresentative, which may be refunded — so Entitlement.latestProductId names a different product than it did under 7b47687a6. This is harmless for access and consistent with latestProductId's documented "once nothing is active, the most recent purchase describes it" rule, but nothing in the suite would fail if it reverted. refundDescribesALapsedGroup looks like it covers this and doesn't: both of its transactions are already date-expired, so isActive was false before the live call ever happened.

Technical details
# The demoted-source representative movement is uncovered

## Affected sites
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/EntitlementProcessor.swift:266-272` —
  `representative` reads the mutable `isActive`, so `.revoked`/`.expired` at `:559-561`
  moves it as surely as `.subscribed`/`.inGracePeriod` at `:542-558` does.
- `Tests/SuperwallKitTests/StoreKit/Products/Receipt Manager/PerGroupEntitlementResolutionTests.swift:1108-1155` —
  `refundDescribesALapsedGroup` is the nearest existing test; `legacy` expires at
  `baseDate - 63_072_000` and `refunded` at `baseDate - 1_728_000`, so the date-only
  `isActive` at `EntitlementProcessor.swift:341` is already `false` and the `.revoked`
  status only reconfirms it. Old and new code agree on this fixture.

## Required outcome
- One test that pins which product describes a source whose transaction dates say active
  but whose live status demotes it, with `unrevokedRepresentative != lastKnownRepresentative`.
  Whichever answer is intended, it should be the one that fails if the `representative`
  guard or the demotion branch regresses.

## Suggested approach (optional)
- Mirror `promotedGroupIsNotNamedByARefund` with the polarity flipped: an unrevoked
  transaction with a *future* `expirationDate` plus a newer revoked one in the same group,
  under a `.revoked` group status. Assert `isActive == false`, `grantingProductIds.isEmpty`
  and the intended `latestProductId`.

## Open questions for the human
- Is naming the refunded product on a demoted source the intent here? It preserves the
  refund-versus-lapse distinction the commit message argues for, but it is a change from
  `7b47687a6`, which named the unrevoked one in this case.

ℹ️ Five public Entitlement fields changed meaning across this PR while the docs checklist stays unticked

This is the sixth review round and the PR looks close to merge, so it is worth raising once. latestProductId, expiresAt, willRenew, startsAt and renewedAt all have rewritten doc comments in this branch, and isSubscribed(to:) plus the activeProducts device attribute changed behaviour — all of which are visible in the wire payload and in audience filters. The in-source docs were updated with the code; "I have updated the SDK documentation as well as the online docs" in the PR checklist is still unticked, as are the UI tests and all three demo-project builds.

Technical details
# Public surface changed ahead of the docs

## Affected sites
- `Sources/SuperwallKit/StoreKit/Products/StoreProduct/Entitlement.swift:68-135` —
  `latestProductId`, `startsAt`, `renewedAt`, `expiresAt` and `willRenew` doc comments
  all rewritten to granting-source semantics.
- `CHANGELOG.md:26-30` — five bullets under `4.17.0`, including the two that describe
  observable changes to `isSubscribed(to:)` and `activeProducts`.

## Required outcome
- The online docs describing these fields should match the granting-source semantics
  before or alongside the release that ships them, since audience filters are authored
  against them.

## Open questions for the human
- Are the unticked checklist items (online docs, UI tests, the three demo-project builds)
  tracked elsewhere, or are they still outstanding for this PR?

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

`statusTransaction` exists so the status write-back targets the purchase
that still counts rather than a refund, but nothing asserted it: the two
tests where the candidates diverge only looked at the entitlement, never
at `subscriptions`.

`refundDescribesALapsedGroup` now pins it. That source stays inactive, so
`representative` is the refunded transaction while `statusTransaction` is
the one never refunded — the only place the two disagree. Confirmed it
discriminates: pointing the write-back back at `representative` fails
both new expectations.

`promotedGroupIsNotNamedByARefund` gets the same pair, which guards the
older fixed-at-construction representative rather than this choice —
once the live status flips `isActive` the two accessors agree there. Its
transactions are now on file as not renewing, so the status's
`willRenew: true` is observable at all; with the defaults both candidates
read `true` and the expectations would have held either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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 new issues found.

fa349a415 closes the last open thread, and the author's correction to my suggested assertions is right on both counts. promotedGroupIsNotNamedByARefund is not where representative and statusTransaction diverge — once .subscribed promotes the source both accessors resolve to txn_a; refundDescribesALapsedGroup is, because that source stays date-inactive so representative is the refunded txn_premium while statusTransaction is txn_legacy. I re-derived both fixtures against EntitlementProcessor.swift:569-575 and confirmed each pair is exact and discriminating: pointing the write-back back at representative flips both expectations in refundDescribesALapsedGroup, and restoring the pre-ad376136c fixed representative flips both in promotedGroupIsNotNamedByARefund. Setting both of that test's transactions to willRenew: false was necessary — with makeTransaction's true default the status's willRenew: true would have been unobservable on txn_a. Every one of the PR's 13 review threads is now resolved.

Reviewed changes — the delta since pullfrog's last review (ad376136c): the single test-only commit fa349a415 in PerGroupEntitlementResolutionTests.swift, traced against EntitlementProcessor.processTransactions (:204-219, which copies willRenew onto every SubscriptionTransaction including revoked ones) and the live-status write-back it pins.

  • Pinned the write-back target where the two candidates actually differ — refundDescribesALapsedGroup now asserts txn_legacy.willRenew == false (written from the .revoked status) and txn_premium.willRenew == true (untouched construction default), which is the only fixture in the suite where statusTransaction != representative.
  • Guarded the derived representative on the promotion path — promotedGroupIsNotNamedByARefund gained the matching pair plus willRenew: false on both fixture transactions, so the status's willRenew: true identifies which SubscriptionTransaction the flags landed on rather than reading true either way.
  • Kept the coverage claim honest — the commit message states explicitly that the second pair guards the older fixed-at-construction representative rather than the statusTransaction choice, instead of counting it as coverage it doesn't provide.

Pullfrog  | View workflow run | Using Claude Opus | 𝕏

…ive one

A group the live status resolved as refunded or lapsed could still leave its
purchase active when another group kept the entitlement active, because the
purchase fell back to its own transaction dates. Its products now count as
lapsed, so they drop out of activeProducts and isSubscribed(to:).

Also guard RenewalInfo.renewalDate behind compiler(>=5.9): it only exists in
the iOS 17 SDK, and the package still builds on Swift 5.5 toolchains.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@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 new issues in the logic — one CHANGELOG wording suggestion inline.

onLapsedProductIds closes the last direction that was still open: until now the correction could only ever force a purchase active, so a revoked group whose transaction carried no revocationDate kept isSubscribed(to:) and activeProducts reporting it alongside the group that was genuinely paying. I tried hard to find a case where the new rule deactivates a product the customer still holds and could not construct one — a date-inactive source can't contain a raw-active product, because GrantSource.isActive (EntitlementProcessor.swift:350) and Purchase.isActive (SK2ReceiptManager.isAnyTransactionActive, :271-283) read the same !isRevoked && expirationDate > now predicate over the same transactions, so the only bucket the rule can newly flip is one the .revoked/.expired branch demoted, which is the intent. The .subtracting(grantingProductIds) reconciliation also guarantees an active entitlement always retains at least one active Purchase, so AutomaticPurchaseController can't be pushed into its empty-set demotion branch.

One correction to an earlier round of mine, since it's load-bearing for eligibility: I previously recorded computeActiveSubscriptionGroupIds' productGroupIds half as inert for StoreKit 2. It isn't — it reads snapshot.purchases.filter(\.isActive), so free-trial suppression does see this narrowing. The net effect is still nil, for a different reason: the union's other half comes from customerInfo.subscriptions.filter(\.isActive), and SubscriptionTransaction.isActive is copied verbatim from the date-only Transaction.isActive and never mutated by the per-source correction, so in exactly the refund-without-a-revocationDate case this commit targets the group stays suppressed.

Reviewed changes — the delta since pullfrog's last review (fa349a415): the single commit 517de9ac3 across EntitlementProcessor.swift, SK2ReceiptManager.swift, both test suites and CHANGELOG.md, read against the full PR diff plus every consumer of the corrected purchase set (ReceiptManager.isSubscribed(to:), getActiveProductIds() → DeviceHelper.activeProducts, computeActiveSubscriptionGroupIds, AutomaticPurchaseController.syncSubscriptionStatus(withPurchases:)).

  • Turned a refunded group's products off beside an active one — GrantSource gained productIds (every product in the bucket, refunded or not); each non-granting source contributes them to a new onLapsedProductIds channel, fired once as lapsedProductIds.subtracting(grantingProductIds) so a product still granting through a narrower entitlement can't be caught by it.
  • Made correctPurchases settle on the lapsed set first — allRevokedOrExpired || lapsedProductIds.contains(purchase.id) short-circuits to inactive ahead of the isGranting branch, which is what lets a refunded group be turned off while a different group keeps the shared entitlement active. The doc comment's two bullets were re-cut to match which branch now owns the refund case.
  • Guarded renewalInfo.renewalDate behind #if compiler(>=5.9) — it only exists in the iOS 17 SDK, so this repairs the Xcode 14 build that 02a0164a4's availability-gate removal broke. I verified against Apple's docs that 5.9 is the right floor, that @backDeployed(before: iOS 17.0, …) means no runtime #available is needed on iOS 15/16, and that gracePeriodExpirationDate — unguarded on the line above — is an original iOS 15 property that correctly needs no equivalent gate.
  • Two discriminating tests — refundedGroupLapsesAlongsideAnActiveGroup asserts lapsedProductIds == ["yearly"], which reads empty without the change, and refundedGroupPurchaseIsInactiveAlongsideAnActiveGroup (renamed from otherGroupDoesNotReactivateARefundedPurchase) inverts its fixture so the refunded purchase now starts isActive: true — it proves the set deactivates rather than merely failing to reactivate.

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

Comment thread CHANGELOG.md
- Fixes an entitlement in a billing grace period reporting an expiry date that has already passed. `expiresAt` is now the date the grace period ends.
- An entitlement's `latestProductId`, `expiresAt` and `willRenew` now all describe the purchase currently unlocking it, so they no longer mix details from different subscriptions.
- Fixes `isSubscribed(to:)` and the `activeProducts` device attribute leaving out a product whose subscription is in its billing grace period.
- Fixes `isSubscribed(to:)` reporting a refunded subscription as active while another subscription still unlocks the same 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.

activeProducts changes here too, not just isSubscribed(to:) — both read the same corrected purchases set (ReceiptManager.swift:279-293), so a refunded product now drops out of the device attribute as well. The grace-period bullet two lines up names both surfaces, and activeProducts is the one authored against in audience filters.

Suggested change
- Fixes `isSubscribed(to:)` reporting a refunded subscription as active while another subscription still unlocks the same entitlement.
- Fixes `isSubscribed(to:)` and the `activeProducts` device attribute reporting a refunded subscription as active while another subscription still unlocks the same entitlement.

This branch has not been deployed

No deployments
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