Repository navigation
Conversation
…nd handle the superwall/return link
|
This PR does not match any of the 1 configured review trigger rule. |
Maple review🔴 Confidence 2/10 · do not merge Adds browser checkout teleport, native return-screen handling, and purchase-style web redemption. The purchase and app-lifecycle paths have confirmed defects that need fixes before merging.
Findings🟠 Warning · F1 · Interrupted grace period leaves
|
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| Teleport return-link routing | entrypoint | yes | Tracks TeleportReturn through the SDK's existing analytics pipeline. |
| Teleport browser transition and return cover | background | yes | TeleportOpen analytics and Logger lifecycle messages follow existing SDK conventions. |
| CheckoutStatusCheck status requests | outbound | no | Raw URLSession requests have no spans; lifecycle logging surrounds the watcher, following the SDK's existing non-OTel convention. |
| Web purchase completion | operation | yes | Retains redemption and free-trial analytics and sends transaction_complete to the paywall. |
61aff7f · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
| for request: PaywallRequest | ||
| ) async -> Paywall { | ||
| var paywall = paywall | ||
| paywall.presentationId = UUID().uuidString |
There was a problem hiding this comment.
Warning
updatePaywall replaces the ID already used by loading events
F5 · Warning · correctness
On an uncached request, getRawPaywall assigns a presentation ID before tracking response loading, and product loading uses that paywall. The fresh-request path then calls updatePaywall and replaces the ID, so loading and presentation events from the same lifecycle no longer correlate.
Preserve the ID for the originating fresh request and allocate a new ID only for reused paywalls; test loading-to-presentation ID consistency.
🤖 Prompt to fix with an AI agent
In `Sources/SuperwallKit/Paywall/Request/PaywallRequestManager.swift:114`: `updatePaywall` replaces the ID already used by loading events.
On an uncached request, `getRawPaywall` assigns a presentation ID before tracking response loading, and product loading uses that paywall. The fresh-request path then calls `updatePaywall` and replaces the ID, so loading and presentation events from the same lifecycle no longer correlate.
Suggested fix: Preserve the ID for the originating fresh request and allocate a new ID only for reused paywalls; test loading-to-presentation ID consistency.
Verify the problem exists at that location before changing it, and keep the fix to those lines.
There was a problem hiding this comment.
won't fix
The premise doesn't hold: presentationId is assigned in exactly one place, updatePaywall. getRawPaywall never sets one, so the response and product loading events for a fresh request carry no presentation ID rather than a different one. The ID is allocated once per request, after loading, so every event from presentation onwards shares it, which is the intent. Reused cached paywalls get a fresh ID per request for the same reason.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
A paywall view controller that is reused can report a purchase from an earlier presentation as .purchased when the user simply closes it. There is also a background-task contract issue in TeleportReturnCover. Details are inline.
Reviewed changes
I reviewed the full teleport feature: return links, the waiting-screen cover, web purchase completion, redemption polling, and the environment script.
- Return links:
scheme://superwall/returnand theapp-linkuniversal links skipdeepLink_openand trackteleport_returnwith areason. They also notify the paywall through.superwallReturnLinkOpened. TeleportReturnCover: before opening the browser, it puts a still of the paywall in front, then has the page draw the waiting screen. In the background it checks the checkout and covers the waiting screen for the app switcher snapshot. On return it callshideCheckoutWaitingScreen({ reason }).- Web purchase as purchase:
stripe_checkout_completewithshould_dismisssendstransaction_completeto the paywall. It dismisses as.purchasedonly when bothshouldDismissandautomaticallyDismissare true. - Decline after purchase → purchased:
completedPurchaseProductplusresolvedDismissalturn a.declinedclose that follows an App Store or web purchase into.purchased. - Redemption polling fix: a checkout that completes while another poll is running now waits and then polls its own checkout. A stale poll stops once a newer checkout replaces the pending state.
- Attribution and environment:
presentationIdis now generated for each request, andpaywall_opencarries the presentation payload.SubscriptionsApiEnvironmentScriptpoints paywall.js at staging or local for non-production SDKs.
ℹ️ No changelog entry for the redemption spinner fix
The PR description lists a customer-visible bug fix: the loading spinner stayed up when a checkout completed while an older pending checkout was still being polled. The ### Fixes section of CHANGELOG.md doesn't mention it. CLAUDE.md asks for bug fixes to go in the changelog.
claude-opus-5.5 ｜ 𝕏
Maple review🔴 Confidence 3/10 · risky as written Adds teleport return handling and completes web checkouts as purchases. This head fixes F1–F4, but F5 still breaks presentation-ID correlation; the return-cover lifecycle fixes also lack regression coverage. Address those before merging.
Findings🟠 Warning · F6 ·
|
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| Teleport return-link handling | SDK entrypoint | yes | DeepLinkRouter tracks TeleportReturn with paywall information and return reason using existing SDK analytics. |
| Web checkout purchase completion and recovery | SDK purchase workflow | yes | Uses existing redemption tracking, FreeTrialStart tracking, and transactionComplete webview messaging. |
| Checkout background status checks | background polling | no | TeleportReturnCover uses existing Logger.debug conventions; CheckoutStatusCheck has no per-request or per-watch spans. |
a661d41 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
There was a problem hiding this comment.
ℹ️ The earlier findings are fixed. One minor note is inline.
Reviewed changes
This review covers the fix commit a661d41 and the develop merge that landed after the previous Pullfrog review.
- Purchase scoped to its presentation:
viewDidDisappearclearscompletedPurchaseProduct. A view controller that is reused, whether throughgetPaywallor the cache, no longer reports an older purchase as.purchased. - Synchronous expiration handler:
TeleportReturnCovernow covers the screen and ends the background task insideMainActor.assumeIsolated. Before, it hopped to aTask. - Reveal and return-link interplay:
uncover()cancels a reveal fade that is still running and sets the cover back to full opacity. The fade's completion checksfinishedanduncovering.revealingis cleared after the grace period, so a reveal that gets interrupted can run again on the next activation. - Purchase skips restore validation:
processEntitlementsonly checks entitlements against every product on the paywall when the completion is.restore. So when a paywall's tiers carry different entitlements, buying one tier no longer shows the restore failed alert. - Recovery keeps the completion mode:
PendingStripeCheckoutPollStatenow storesStripeCheckoutCompletion, so recovery on foreground or paywall open finishes a purchase as a purchase. State saved by older SDK versions still decodes as.restore. - Duplicate checkout reports:
handleStripeCheckoutCompleteexits early when the same checkout context has already been redeemed, and it stops waiting once an in-flight poll redeems it. - Changelog: added entries for the spinner fix and the tiered restore failure fix. Also extracted
DeepLinkRouter.handleReturnLink, a refactor with no behavior change.
claude-opus-5.5 ｜ 𝕏
Maple review🔴 Confidence 4/10 · risky as written The latest commit restores the Basic example’s signing settings to the base configuration and introduces no new defects. Teleport still needs attention: F5 and F6 remain open at this head. Still open from earlier reviews
What was checked
|
Maple review🟡 Confidence 5/10 · needs attention Adds test seams and lifecycle tests for
Still open from earlier reviews
What was checked
|
There was a problem hiding this comment.
ℹ️ No production issues. The new
TeleportReturnCovertests share the globalNotificationCenter, so they can flake. Details are inline.
Reviewed changes
This review covers d67aff8, the only commit with code since the previous Pullfrog review. (52d39f8 only reverted the Basic example's signing team.)
- Injectable timing and environment:
TeleportReturnCovermoves its constants into aTimingstruct and itsUIApplication, snapshot and network calls into anEnvironmentstruct. Both default to the previous behavior, so production is unchanged. - Test hooks: added
isCoveringandcoverAlpha, read-only accessors for tests. - New
TeleportReturnCoverTests: drives the cover with lifecycle notifications. It checks that an interrupted grace period still allows a later reveal, and that a return link arriving mid-fade restores full opacity until the page removes the waiting screen.
claude-opus-5.5 ｜ 𝕏
Maple review🟡 Confidence 6/10 · needs attention Teleport adds browser checkout completion as a native purchase. The latest revision isolates lifecycle tests with per-fixture notification centers and addresses F6; F5's loading-to-presentation ID mismatch remains unresolved, so the pull request still needs correction.
Still open from earlier reviews
Fixed since the last review
What was checked
|
There was a problem hiding this comment.
✅ No new issues found. All three earlier test findings are fixed.
Reviewed changes
This review covers 791ae62, the only commit since the previous Pullfrog review.
- Injectable notification center: added
TeleportReturnCover.Environment.notificationCenter, which defaults to.default. All four observers now register on it, so production behavior is unchanged. - Isolated test fixtures: each
Fixturecreates its ownNotificationCenter(), andpost(_:to:userInfo:)sends to that fixture's center. Tests running in parallel in this suite and inDeepLinkRouterTestscan no longer trigger each other's covers. - Test cleanup: removed the tautological
testReturnLinkWithoutCoverand the unusedFixture.applicationState.
claude-opus-5.5 ｜ 𝕏
This branch has not been deployed
1 participant
Summary
Adds the SDK side of teleport, which lets a native paywall send the purchase to a Superwall checkout page in the browser, and completes that web purchase like an App Store purchase.
scheme://superwall/returnand the…/app-link/superwall/returnuniversal link bring the user back without being tracked asdeepLink_open, so the paywall isn't dismissed. They're tracked asteleport_returninstead, with areason(purchased/closed) taken from the link.TeleportReturnCovercovers the paywall's waiting screen with a native picture of the paywall, so the waiting screen never shows in the app switcher snapshot or on return. It checks the checkout in the background (teleport_watch_start/teleport_watch_end) and tells the page when the user is back throughhideCheckoutWaitingScreen({ reason }).stripe_checkout_completewithshould_dismissfinishes the redemption as a purchase.transaction_completefires, free-trial start is tracked, and the paywall dismisses with.purchased. Withoutshould_dismissit keeps the restore behaviour.paywall_opencarries apresentation_id.teleport_openis tracked when the checkout page opens.SubscriptionsApiEnvironmentScripttells the paywall to use the staging or local subscriptions-api when the SDK runs with.developeror.local. Production is unchanged.Teleport is gated to SDK 4.18.0+ in paywall.js. Older SDKs keep the existing external web checkout.
Checklist
CustomerCenterSheetOwnershipTestsandProductsFetcherSK2Tests, fail the same way ondevelop.)CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.swiftlintin the main directory and fixed any issues.🤖 Generated with Claude Code