Fix Customer Center sheets, cancellation, refunds and StoreKit thread warnings

[DreamingInBinary]

Changes in this pull request

Fixes four Customer Center bugs found testing 4.18.0 on a device (one commit each), then the problems that reviews of those fixes turned up.

1. Sheets requested on a subscription's screen never appeared. Tapping Cancel subscription on the detail screen did nothing. Going back then showed the survey over the root, with no animation, and SwiftUI logged A sheet was presented but its presenter is not yet in the window. The sheet modifiers lived only on the root screen, and pushing the detail with NavigationLink takes the root out of the window. A screen NavigationLink pushes now carries its own sheet modifiers and presents them while it's on top, as screens pushed through UIKit already did. Every sheet on the detail screen was affected (survey, change plan, web management page), in every SwiftUI presentation, presentCustomerCenter() included.

2. "Publishing changes from background threads is not allowed." When manageSubscriptionsSheet and refundRequestSheet close, StoreKit writes their isPresented bindings from a background thread, and the setters published the view model there. Those writes are now applied on the main thread. SwiftUI's own writes, already on the main thread, still apply immediately.

3. Refund requests always failed. refundRequestSheet reads its transaction ID from the render before the one that presents it. We changed the ID and isPresented in the same render, so every request went out for transaction 0 and failed with "Something went wrong requesting a refund". The refund sheet now presents one update after its transaction has rendered.

4. After the cancellation survey, Apple's manage sheet opened on no subscription. manageSubscriptionsSheet(isPresented:subscriptionGroupID:) reads its group the same way, from the render before the one that presents it. So after the survey, Apple's sheet opened with an empty group. On a device nothing appeared to happen; in the StoreKit test environment the sheet said "You don't have any subscriptions" to a customer who had one. Both StoreKit sheets now wait until their parameter has been rendered.

Follow-ups from review

5. Sheets stay with the screen that asked for them. The first fix handed a pushed screen's claim on the sheets back on any onDisappear, which also fires when the screen is only covered: by a tab switch, a host push or presentation, or a deeper screen. The root then owned the sheets while out of the window, and an open survey or change-plan sheet was torn down. A pushed screen now keeps its claim until it leaves the stack. pushDepth is no longer a stored number: it's derived from per-screen claims (PushedSurfaces, shared with the UIKit navigator), so the order screens report in can't matter. That order is what could hand the depth back to the root after a split view swapped its detail column. A new file, CustomerCenterLifecycleProbe.swift, adds a hidden UIKit child to the root and to every NavigationLink-pushed screen. It tells a cover from a removal, since onDisappear fires for both.

A sheet also stays with the screen that was on top when it was requested. Before, a refund that landed while its screen was being popped was presented again by the root: two refund requests for one tap. If that screen leaves the stack first, its request is dropped along with what it set up, such as a survey's pending answer. A refund's product is kept, so an outcome StoreKit still delivers is reported.

6. A covered embedded CustomerCenterView no longer reports that it closed. This one predates this PR and is on develop. Place a CustomerCenterView in a NavigationStack inside a TabView, then switch tabs or push over it. The visibility count dropped to zero and delivered customerCenterDidDismiss() and the close event while the Customer Center was still in the stack. dismiss() latches, so the real close later went unreported. The probe now vetoes that, as CustomerCenterViewController already did in UIKit. This changes when the close is delivered: after a cover, customerCenterDidDismiss() waits until the Customer Center is actually popped, dismissed or taken down. A screen that knows it was removed rather than covered reports it through surfaceWasRemoved(), which lifts the veto so the close still arrives, even for a Customer Center removed from under a cover.

7. With no subscription group, Apple's manage sheet opens on the customer's subscriptions. From iOS 17 a manage request without a group, from cached customer info missing the group plus a failed product lookup, still went to StoreKit's group variant with an empty group, which says "You don't have any subscriptions". It now goes to the plain manage sheet, as on earlier versions. Both variants are applied with bindings that can't both be true, so the view tree doesn't change as the sheet presents.

Along the way: StoreKit's sheets and the gate holding them back take their parameters from one value. What a screen last rendered into them is kept per screen rather than on the view model, so recording it no longer re-renders the whole Customer Center twice per sheet. The two StoreKit bindings share one builder. The drill-down tests moved into CustomerCenterSheetOwnershipTests.

Testing

• The unit-test host has no window scene, and without one UIKit leaves a covered screen in the window, so bug 1 can't be reproduced there. It also doesn't forward a tab switch's appearance callbacks. The tests assert which screen owns the sheets, and cover screens with host pushes, which it does drive. Each new cover and removal test was checked against the old behaviour: put the release back on onDisappear, or remove the probe, and they fail.
• Everything was also verified in a throwaway simulator app (iPhone 17 Pro, iOS 27) with mocked purchases, across all four presentation styles: sheet, zoom-transition sheet, embedded in a host NavigationStack, and CustomerCenterViewController presented modally. The follow-ups were checked there too:
  • a cancelled back-swipe on the detail;
  • a full-screen cover presented and dismissed over the Customer Center, with the detail up and on the root;
  • the survey presenting from the detail afterwards;
  • customerCenterDidDismiss arriving exactly once, only when the Customer Center actually closed.
• For bug 3, StoreKit logged Presenting overlay for transactionID 0 before the change and the requested ID after it. A StoreKit-only repro showed the cause: setting the ID and presenting in the same update sends 0, and presenting an update later sends the ID.
• Bugs 3 and 4, and follow-up 7, were checked on iOS 27 against a real local StoreKit purchase made from a .storekit configuration. The purchase was made through SKTestSession in a UI test, which is the only process SKTestSession runs in. After the survey, Apple's manage sheet opens on the subscription, with its group and without one. For follow-up 7, the group was removed from both the subscription and its product, the path that leaves the request with no group. The refund sheet opens on the purchase (Presenting overlay for transactionID 3).
• Follow-up 7 was also checked on iOS 26.5 with mocked purchases and no group. SKTestSession purchases fail with notEntitled on that runtime, so there's no real transaction there, but the plain manage sheet presents Apple's manage flow. No iOS 17 or 18 runtime was available.
• New tests are in CustomerCenterSheetOwnershipTests and CustomerCenterViewModelTests. All 209 Customer Center tests (23 suites) pass locally.

There's no CHANGELOG entry because the Customer Center hasn't shipped yet; it's new in 4.18.0.

Checklist

• All unit tests pass. (The 209 Customer Center tests pass locally. CI passed on e73c662; the run for the latest push is in progress.)
• All UI tests pass. — N/A, this repo has no UI test target.
• Demo project builds and runs on iOS. — Not run; verified in a simulator app instead (see Testing).
• Demo project builds and runs on Mac Catalyst. — The framework builds for Catalyst in CI (build-maccatalyst); the demo app wasn't run.
• Demo project builds and runs on visionOS. — Not verified.
• I added/updated tests or detailed why my change isn't tested. (Bug 1's presentation failure can't be reproduced in the hostless test target; see Testing.)
• I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes. — Not needed; the Customer Center hasn't shipped.
• I have run swiftlint in the main directory and fixed any issues. (11 warnings, all already on develop; none added.)
• I have updated the SDK documentation as well as the online docs. — N/A, no API changes.
• I have reviewed the contributing guide

cc @yusuftor @jakemor @anglinb

🤖 Generated with Claude Code

[greptile-apps]

PR author is not in the allowed authors list.

[pullfrog]

ℹ️ No blocking issues. One lifecycle contract worth confirming on a device, and one follow-up to make sure doesn't get lost.

Reviewed changes — full review of all three commits at 62ff299, tracing the sheet-ownership, main-thread and refund-gate changes against CustomerCenterPushNavigator, CustomerCenterViewModel and the existing ownership tests.

• NavigationLink-pushed screens carry their own sheets — new CustomerCenterLinkedScreen modifier wraps the drill-down destination with .customerCenterSheets(surfaceDepth:), publishes customerCenterSurfaceDepth into the environment, and claims/releases viewModel.pushDepth on appear/disappear.
• CustomerCenterDrillDown takes the view model — needed to build the destination's sheet modifiers; only ManagementScreenView.swift:34 constructs one, and the UIKit-navigator branch ignores the new property.
• StoreKit dismissals hop to the main thread — both boolean sheet setters route through a new nonisolated static onMainThread(_:) that runs synchronously via MainActor.assumeIsolated when already on main and defers via Task { @MainActor in } otherwise.
• Refund sheet waits one update for its transaction — new renderedRefundTransactionId on the view model, a .task(id: refundTransactionId) that reports it, and an extra equality term in refundBinding's getter.
• Tests — new CustomerCenterDrillDownSheetTests driving the real CustomerCenterView through a window, plus four tests appended to CustomerCenterSheetOwnershipTests; project.pbxproj updated for the new file.

I checked the new assertions against the pre-fix behaviour and they all genuinely fail without the fix: pushDepth == 1 is 0 without CustomerCenterLinkedScreen, backgroundDismissalWriteLandsOnMainThread counts off-main objectWillChange publishes, and refundSheetWaitsForItsTransaction pins the gate closed before the report lands.

I also traced the refund gate for stuck states — same transaction twice, two transactions back to back, a non-refund sheet in between, and several surfaces writing renderedRefundTransactionId concurrently. All self-heal, because the private refundTransactionId falls back to 0 for any non-.refund sheet, which re-fires the .task and resets the gate. Separately, MainActor.assumeIsolated is available at this SDK's iOS 13 floor and back-deploys, and the synchronous main-thread fast path is the endorsed idiom for bridging a non-actor-aware callback.

No CHANGELOG entry is correct: 4.18.0 is staged on develop and the Customer Center itself is what that entry introduces.

ℹ️ The same staleness may affect manageSubscriptionsSheet, and nothing is tracking it

The PR body notes manageSubscriptionsSheet(isPresented:subscriptionGroupID:) may read a stale group ID exactly the way the refund sheet read a stale transaction, and leaves it unchanged because the simulator can't show that sheet. That's a reasonable call, but the Customer Center ships in 4.18.0 and there's no issue holding the loose end — with groupId ?? "" in ManageSubscriptionsSheet, a stale read lands the user on the full subscriptions list instead of the one they tapped.

Technical details

# Track the possible stale `subscriptionGroupID` on `manageSubscriptionsSheet`

## Affected sites
- `Sources/SuperwallKit/CustomerCenter/Views/CustomerCenterSheets.swift:210-214` — `manageSubscriptionsSheet(isPresented:subscriptionGroupID:)` takes `groupId ?? ""`, derived from `viewModel.sheet` in the same update that flips `isPresented`, which is the shape the refund fix just proved to be a problem for the sibling modifier.
- `Sources/SuperwallKit/CustomerCenter/Views/CustomerCenterSheets.swift:102-111` — the pattern that would apply, if it does turn out to be affected.

## Required outcome
- A device check on whether Apple's sheet opens on the tapped subscription or on the full list, and either a fix or a recorded "not affected" — not an unrecorded maybe.

## Open questions for the human
- Is this being tracked anywhere, or should a follow-up issue be opened before this merges?

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in 4a2bac5. Not approving only because the open thread on CustomerCenterNavigator.swift:111 is still outstanding.

Reviewed changes — delta since the previous review at 62ff299, i.e. commit 4a2bac5 "Open Apple's manage sheet on the subscription's group, not an empty one". This is the follow-up the last review asked about, and it lands cleanly.

• The one-render-late gate now covers both StoreKit sheets — renderedRefundTransactionId is replaced by a StoreKitSheetParameters value type (manageGroupId, refundTransactionId) recorded as a whole on renderedStoreKitSheetParameters, and isManagePresented picks up the same hasRenderedStoreKitSheetParameters term refundBinding already had.
• .task(id:) records the whole struct rather than a single transaction ID, so both parameters are reported one update after the modifier rendered them.
• Tests generalised — refundSheetWaitsForItsTransaction becomes storeKitSheetWaitsForItsParameter parameterised over both sheet kinds, sheetModifierRecordsTheRenderedParameters now drives a .refund → .manageSubscriptions transition, and a new manageSheetWithoutAGroupPresentsAtOnce pins the nil-group fast path.

The whole-struct equality is sound here: CustomerCenterSheet is a single case at a time, so manageGroupId and refundTransactionId are never both non-default — the struct is a collapsed tagged union and comparing it is equivalent to comparing the field that matters. I re-traced the gate for stuck states across a same-parameter repeat, two different parameters back to back, a non-StoreKit sheet in between, and a .refund → .manageSubscriptions transition with no nil between; all self-heal, because both private accessors fall back to the default for any other sheet value, which re-fires the .task and resets the record. Letting groupId == nil present with no extra render is right — StoreKit already has "", so there is nothing to wait for.

The device evidence added for this commit is a real step up: reproducing both bugs against an SKTestSession purchase from a .storekit configuration is about as close to a first-party confirmation as this undocumented behaviour allows.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Important

Two things worth settling before this merges: the new stacked manage-sheet modifiers are unverified in exactly the shape that could make them inert, and a dropped sheet request is now swallowed with nothing to clean it up.

Reviewed changes — the delta since the prior review at 4a2bac5: commits dd17969, 6f0869d and 5f4737e. This is a substantially bigger delta than the earlier two, and it replaces the ownership model the first review looked at rather than tweaking it.

• Replaced pushDepth with per-screen claims — PushedSurfaces holds a list of (id: UUID, depth: Int); claim(_:depth:) evicts same-id and depth >= entries then appends, release(_:) removes by id, and pushDepth is now derived and read-only. Both navigators claim and release by UUID, so the order screens report in can't strand the depth.
• Added a UIKit lifecycle probe — the new CustomerCenterLifecycleProbe is a hidden child controller that splits viewDidDisappear into covered / removed / dismantled, so a NavigationLink-pushed screen keeps its claim when it is merely covered. This is the fix for the prior review's open thread on CustomerCenterNavigator.swift:111, and it lands the way the UIKit navigator already worked.
• Fixed sheet ownership at the request — sheetOwnerDepth is stamped from pushDepth in sheet's didSet instead of tracking it, so a sheet no longer moves to whatever screen happens to be on top by the time it presents. CustomerCenterSheetOwnership.isTopmost is gone.
• Lifted a cover veto nothing else would clear — surfaceWasRemoved() plus the probe's onRemoved/onDismantled mean a Customer Center covered by the host and then popped past, or dismissed from a drill-down, still reports its close.
• Moved the StoreKit render record onto the surface — renderedStoreKitSheetParameters on the view model became a per-modifier StoreKitSheetRenderRecord @StateObject, so recording what was rendered re-renders one modifier rather than the whole Customer Center. Both boolean bindings now come from one storeKitSheetBinding(_:rendered:afterDismissal:).
• Routed a no-group manage request to the plain manage sheet — ManageSubscriptionsSheet.groupId became a non-optional String, and on iOS 17+ both StoreKit manage-sheet modifiers are applied, gated by a new Binding.only(when:).
• Consolidated the tests — CustomerCenterDrillDownSheetTests.swift was deleted and folded into CustomerCenterSheetOwnershipTests.swift, which now drives the real drill-down row through CustomerCenterView in both .default and .embedded navigation.

I traced the new state machine for stuck and double-presentation states across: a deeper screen coming and going under an owner, a screen replaced at its own depth, screens popped together in either order, a late release arriving after the replacement claimed, and the root's implicit depth-0 ownership. All behave as the tests claim. sheetOwnerDepth not being @Published is fine: both mutation sites write it before the @Published write that publishes, and updatePushedSurfaces' early return only skips genuine no-ops.

I also checked the probe's covered-vs-removed rule against the presentations the Customer Center actually supports — presented as a sheet, embedded in a host NavigationStack, pushed onto a host UINavigationController, covered by a full-screen presentation, popped out of a SwiftUI NavigationStack (which detaches before reporting), and inside a container being dismissed. Each is classified correctly and each is directly tested against the real CustomerCenterLifecycleProbeController rather than a restatement of the rule. The one case it gets wrong — a host swapping window.rootViewController out from under a presented Customer Center — is the same blind spot CustomerCenterViewController.isLeavingHierarchy already has, so it isn't something this PR introduced.

The new assertions are exact and all fail against the pre-fix behaviour: coveredDetailKeepsItsClaim fails the moment the claim goes back to being released on onDisappear, the probe tests assert an exact event list, and the PushedSurfaces tests are parameterised over both report orders rather than the convenient one.

ℹ️ The description no longer describes the PR

The body still says "Fixes four Customer Center bugs... There's one commit per bug", and its Testing section covers those four. Since then the ownership model has been rewritten around claims and a new UIKit lifecycle probe, surfaceWasRemoved() has changed when customerCenterDidDismiss() fires, and 5f4737e fixes a fifth bug that isn't mentioned at all. Three named reviewers are cc'd on a description that predates the largest commit in the PR.

Technical details

# Bring the PR description up to date with `dd17969`, `6f0869d` and `5f4737e`

## Affected sites
- The PR body — "Fixes four Customer Center bugs found testing 4.18.0 on a device. There's one commit per bug." There are now seven commits and five fixes.
- The body's Testing section — records device/simulator checks for bugs 1-4 only. `6f0869d`'s verification is recorded in a review reply rather than the description, and `5f4737e` has no recorded verification anywhere.

## Required outcome
- A reader of the description alone should know that `pushDepth` was replaced by `PushedSurfaces` claims, that `CustomerCenterLifecycleProbe` is a new file carrying covered-vs-removed classification for both SwiftUI paths, that `surfaceWasRemoved()` changes when the close callback is delivered, and that a no-group manage request now goes to a different StoreKit sheet.
- The Testing section should say how `5f4737e` was verified, on which OS version, and with what subscription-group data.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in 8c1d28c. Not approving only because the open thread on CustomerCenterSheets.swift:267-268 — whether both stacked manageSubscriptionsSheet modifiers are actually live on iOS 17+ — is still outstanding.

Reviewed changes — the delta since the prior review at 5f4737e: commit 8c1d28c "Drop a sheet request whose screen has left the stack". It's the fix for that review's first thread, plus one teardown-ordering correction it made necessary.

• A request whose screen has gone is now dropped, not stranded — updatePushedSurfaces calls a new abandonSheet() instead of nulling sheetOwnerDepth directly. A .survey goes through cancelSurvey(), which also clears pendingSurvey, pendingAction and pendingActionAfterSheetDismiss; anything else clears sheet.
• pendingRefundProductId is deliberately kept — a refund StoreKit may already be showing still has to report its outcome through refundRequestDidFinish, and perform's .refund overwrites it on the next request. The doc comment says so.
• The probe's dismantle report is deferred a runloop turn — dismantleUIViewController copies onDismantled off the controller and calls it inside DispatchQueue.main.async, because what it reports can now clear sheet and SwiftUI dismantles views mid-update.
• Tests — poppedScreensSheetStaysWithIt gained sheet == nil assertions, and two new tests pin the drop's two halves: abandonedSurveyRequestIsCleared (the survey and its pending answer both go) and abandonedRefundStillReportsItsOutcome (the product survives, so the outcome still reaches the host). makeLoadedViewModel takes an injectable StoreKitTransactionLookupMock.
• The empty-group claim is softened — the comment on ManageSubscriptionsSheet now says the "You don't have any subscriptions" behaviour is observed in the StoreKit test environment and undocumented by Apple, rather than stating it as fact.

I traced abandonSheet() for the case that would make it dangerous: updatePushedSurfaces is reached from claimPushedSurface as well as releasePushedSurface, and PushedSurfaces.claim evicts every entry at its own depth or deeper — so a claim can put another screen's claim into departed and abandon a sheet that is currently up. It doesn't fire: a same-id re-claim at the same depth produces an identical array and is caught by the guard surfaces != pushedSurfaces early return, and the only way a different id claims the owner's depth is a genuine replacement, because CustomerCenterLinkedScreen's @State claim survives an apply(customerInfo:) republish — PurchasePresentation.id is the product ID, transaction ID or "entitlement:<id>", never a fresh UUID. Worth knowing that's an implicit invariant rather than an enforced one; nothing in the suite would catch it changing.

The DispatchQueue.main.async is safe in each ordering I checked. dismantleUIViewController is itself @MainActor, so this is a runloop turn rather than an actor hop. The closure is copied off the controller before scheduling and captures viewModel strongly, so a simultaneous teardown can't drop it, and scheduleDismissalIfNothingIsVisible's Task then holds the view model for the debounce. A stale releasePushedSurface arriving after a newer screen has claimed hits the same no-op early return, and a surfaceWasRemoved() that lands after a new surface has appeared finds visibleSurfaceCount != 0 and schedules nothing.

Both new tests fail against the pre-fix behaviour — sheet is non-nil after the release in each — and abandonedRefundStillReportsItsOutcome asserts the exact product and status rather than merely that a callback fired. The "refund" path does resolve for the mocked subscription: the default config's .refund carries no window, the mock has no product so the price check is skipped, and latestTransactionID comes from the injected lookup.

ℹ️ Nitpicks

• The PR body describes the review follow-ups as "three more commits" and follow-up 5 as only the no-double-present half; 8c1d28c makes it four commits and adds the drop-the-request half, which the description doesn't mention.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

✅ No new issues found. Both remaining threads are resolved, and the last outstanding question — whether both stacked manageSubscriptionsSheet modifiers are live on iOS 17+ — now has device evidence behind it.

Reviewed changes — the delta since the prior review at e73c662: commit 60a214a "Pin that a screen keeps its sheet through a customer-info update". Test-only; it closes the last coverage gap the previous review flagged as an implicit, unenforced invariant.

• Pinned the claim-identity rule — reclaimKeepsTheSheetReplacementDropsIt asserts both halves of PushedSurfaces.claim: the same UUID re-claiming its own depth keeps sheet and sheetOwnerDepth, while a different UUID taking that depth runs abandonSheet() and clears sheet.
• Pinned that a republish doesn't cost a pushed screen its sheet — customerInfoUpdateKeepsTheDetailsSheet drives the real CustomerCenterView in a window, opens the detail through the real row, presents the survey, then republishes through CustomerInfoProviderMock.subject. It asserts pushedSurfaces.claims is unchanged and sheet survives.
• Split the loaded-view-model helper — a Self.subscription(willRenew:) factory plus makeLoadedViewModelAndInfo, which also hands back the provider mock so a test can publish; makeLoadedViewModel delegates to it.
• Swapped a run-loop spin for a suspending wait — backgroundDismissalWriteLandsOnMainThread now await waitUntils, since the setter's off-main path is Task { @MainActor in … } and a run loop spun inside a @MainActor test can't be relied on to drain it.

The republish test is not vacuous. #require(purchases.first?.statusLine != statusBefore) is a real gate: badge(for:) returns .active when willRenew is true and .cancelled when it isn't (PurchasePresentationBuilder.swift:231-238), so the status genuinely moves from "Renews on …" to "Expires on …" and the test fails loudly if the update never lands. And the assertion it guards is the right one — PurchasePresentation.id is sub.productId (PurchasePresentationBuilder.swift:187), so ForEach matches the existing element and updates the pushed destination in place rather than re-creating it, which is exactly why CustomerCenterLinkedScreen's @State claim must come through unchanged. Comparing the whole claims array is the precise expression of that. The 300 ms sleep afterwards is the unavoidable shape of a negative assertion; the worst case is a vacuous pass, not a flake.

I also re-checked the waitUntil swap against the production path it exercises: onMainThread enqueues a @MainActor task when off-main, and the confirmation(expectedCount: 0) still holds — the publish it observes lands on the main thread inside the suspended wait, so the off-main counter stays at zero while the sheet == nil assertion remains exact.

On the thread that blocked the previous two runs: the author's evidence covers the path that matters — an iOS 27 SKTestSession purchase from a .storekit configuration with the group removed from both the subscription and its product, which is the CustomerCenterPathResolver.swift:79 path that yields nil, plus iOS 26.5 with mocks. Apple's manage flow presented in both, so plainSheetIsPresented is not inert. Worth carrying forward that no iOS 17 or 18 runtime was available and that nothing in CI renders both modifiers together, so this behaviour is guarded by manual evidence only.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[linear-code]

SW-6054