Fix web redemption product details and trial handling

[ianrumac]

Changes in this pull request

• Web purchase redemption now exposes the full checkout product in didRedeemLink, tracks freeTrial_start for eligible trials, and schedules the active paywall's trial reminders when notification permission is granted.

Checklist

• All unit tests pass.
• All UI tests pass.
• Demo project builds and runs.
• 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 ktlint 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

[pullfrog]

Important

The redemption critical path now awaits the POST_NOTIFICATIONS system dialog through a non-cancellable, unbounded suspendCoroutine. If that callback never arrives (activity recreated while the dialog is up), didRedeemLink() and closePaywallIfExists() never run. Worth bounding before merge.

Reviewed changes — full initial review of the single commit on codex/fix-web-redemption-trials, covering the redemption reordering, the new public PaywallProduct model, the StoreProductType adapter, and the three new test files.

• Full checkout product on didRedeemLink — RedemptionResult.PaywallInfo gains a nullable product: PaywallProduct? carrying a 28-field snapshot of the web-checkout product variables; the primary constructor becomes @JvmOverloads constructor(...) to keep the pre-existing 6-arg Java constructor callable.
• RedemptionStoreProduct adapter — new internal class mapping PaywallProduct → StoreProductType, deliberately preserving the checkout snapshot's prices/periods instead of recomputing, with ISO-instant-then-LocalDate parsing for trialPeriodEndDate and a periodAlt override on top of StoreProductType.attributes.
• freeTrial_start + trial reminders for web redemptions — new WebPaywallRedeemer.handleTrialRedemption tracks the event and awaits a new Factory.scheduleTrialNotifications (default no-op), implemented in DependencyContainer by hopping to Dispatchers.Main.immediate and calling SuperwallPaywallActivity.attemptToScheduleNotifications. Side-effect failures are logged and swallowed by attemptTrialSideEffect.
• redeem() reordering and code-lookup fix — internallySetSubscriptionStatus moves ahead of the when (redemption) block so access is granted before trial work can block, and the previous it.codes.first { … } (which threw NoSuchElementException when the requested code was absent from the response) is replaced by a single codeResult with an Error fallback, reused for the restore branch, dismissal and delegate callback. Good fix.
• Permission-callback failures no longer escape into the framework — attemptToScheduleNotifications wraps NotificationScheduler.scheduleNotifications in try/catch and resumeWithExceptions instead of letting the throw propagate out of onRequestPermissionsResult.
• Tests — WebRedemptionTrialTest (13 cases incl. a strict access → trial → schedule → restore → close → callback ordering pin), RedemptionStoreProductTest, TrialNotificationPermissionTest (Robolectric), plus a web-redemption-trial.json fixture.

I confirmed the reordering of internallySetSubscriptionStatus is safe: nothing observing the subscription-status flow dismisses a presented paywall (store/Entitlements.kt:117-155, Superwall.kt:755-794; SubscriptionStatusDidChange.canImplicitlyTriggerPaywall is false, so Tracking.kt's dismiss branches are unreachable). I also refuted the concern that trialPeriodDays could be 0 for a week/month-only trial — RawStoreProduct.trialPeriodDays normalizes every unit into days.

⚠️ The new trial behavior has no test for the case that can strand the flow, and attribution is untested by construction

WebRedemptionTrialTest covers scheduling failure (coEvery { factory.scheduleTrialNotifications(any()) } throws …) and scheduling deferral-then-completion (CompletableDeferred completed inside the test), but never the case where the deferral simply never completes — which is the one that leaves the redemption stuck. A withTimeoutOrNull bound would be directly testable with runTest's virtual clock.

Separately, the fixture's checkout paywall is test_paywall (web-redemption-trial.json:22) while the mocked factory.getPaywallInfo() returns identifier = "active_paywall". That divergence looks deliberate, but no assertion pins which of the two ends up as paywall_identifier / experiment_id / variant_id on the emitted freeTrial_start, so the attribution decision is unpinned in either direction.

Technical details

# Missing coverage for the stranded-permission case and for trial-event attribution

## Affected sites
- `superwall/src/test/java/com/superwall/sdk/web/WebRedemptionTrialTest.kt:219-252` — the two `scheduleTrialNotifications` failure/deferral tests both terminate; neither models "callback never arrives".
- `superwall/src/test/java/com/superwall/sdk/web/WebRedemptionTrialTest.kt:121-139` — asserts `event.product.*` and three `getSuperwallParameters()` product keys, but no `paywall_identifier` / `experiment_id` / `variant_id` assertion.
- `superwall/src/test/resources/web-redemption-trial.json:22` (`test_paywall`) vs `WebRedemptionTrialTest.kt:55` (`active_paywall`).

## Required outcome
- A test that proves a never-resolving `scheduleTrialNotifications` still lets `didRedeemLink` and `closePaywallIfExists` run (this falls out naturally once the wait is bounded).
- An explicit assertion on which paywall identity the `freeTrial_start` event carries, so the choice between the on-screen paywall and `redemptionInfo.paywallInfo` is a decision the suite defends rather than an accident.

## Open questions for the human
- Should `freeTrial_start` from a web redemption be attributed to the paywall that was on screen at redemption time, or to the paywall recorded at web checkout (`redemptionInfo.paywallInfo.identifier` / `variantId` / `experimentId`)? In the usual web-to-app flow they coincide, but they need not.

ℹ️ Nitpicks

• RedemptionStoreProductTest.kt:75 is named product is retained without the deprecated identifier, but productIdentifier carries no @Deprecated annotation and neither the KDoc nor the CHANGELOG.md entry mentions a deprecation. Either annotate it or reword the test name.
• PaywallProduct's 28 fields carry no @SerialName annotations while every sibling field in PaywallInfo does. Functionally fine — the network decode path sets namingStrategy = null (DependencyContainer.kt:378-383) and DirectSuccessSerializer builds its own naming-strategy-free Json — but it's inconsistent with the surrounding style.
• WebRedemptionTrialTest calls WebRedemptionResponse.copy(codes = …) in several places; because allCodes is @Transient with a codes-derived default, copy carries the stale value forward rather than recomputing. Harmless here (storage.read(LatestRedemptionResponse) is stubbed to null) but a trap for future tests.

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

[ianrumac]

@pullfrog

[pullfrog]

Important

The redemption critical path awaits attemptToScheduleNotifications, which is a non-cancellable suspendCoroutine. If the hosting activity is recreated while the POST_NOTIFICATIONS dialog is up, didRedeemLink(), closePaywallIfExists() and startPolling() never run. Worth noting that wrapping the call in withTimeoutOrNull does not fix this — the timeout cannot resume a raw continuation, so the timeout itself hangs too.

Reviewed changes — full re-review of codex/fix-web-redemption-trials. The previously reviewed commit 1e34b38 is no longer reachable in the repository (the branch was rewritten), so no incremental delta was computable and the whole diff was re-read from scratch.

• Full checkout product on didRedeemLink — RedemptionResult.PaywallInfo gains a nullable product: PaywallProduct? holding a 28-field snapshot of the web-checkout product variables; the primary constructor becomes @JvmOverloads constructor(...) to keep the pre-existing 6-arg Java constructor callable.
• RedemptionStoreProduct adapter — new internal class mapping PaywallProduct → StoreProductType, deliberately preserving the checkout snapshot's prices and periods instead of recomputing them, with ISO-instant-then-LocalDate parsing for trialPeriodEndDate and a periodAlt override layered on StoreProductType.attributes.
• freeTrial_start + trial reminders for web redemptions — new WebPaywallRedeemer.handleTrialRedemption tracks the event and awaits a new Factory.scheduleTrialNotifications (default no-op), implemented in DependencyContainer by hopping to Dispatchers.Main.immediate and calling SuperwallPaywallActivity.attemptToScheduleNotifications. Failures are logged and swallowed by attemptTrialSideEffect, which correctly rethrows CancellationException.
• redeem() reordering and code-lookup fix — internallySetSubscriptionStatus moves ahead of the when (redemption) block so access is granted before trial work can block, and the old it.codes.first { … } (which threw NoSuchElementException when the requested code was absent) is replaced by a single codeResult with an Error fallback, reused for the restore branch, dismissal and delegate callback. Good fix.
• Permission-callback failures no longer escape into the framework — attemptToScheduleNotifications wraps NotificationScheduler.scheduleNotifications in try/catch and resumeWithExceptions rather than letting the throw propagate out of onRequestPermissionsResult.
• Tests — WebRedemptionTrialTest (13 cases including a strict access → trial → schedule → restore → close → callback ordering pin), RedemptionStoreProductTest, TrialNotificationPermissionTest (Robolectric), plus a web-redemption-trial.json fixture.

I re-confirmed two things so they don't get re-litigated: moving internallySetSubscriptionStatus earlier is safe (nothing observing the subscription-status flow dismisses a presented paywall, and SubscriptionStatusDidChange.canImplicitlyTriggerPaywall is false), and trialPeriodDays <= 0 is a sound "no trial" gate because RawStoreProduct.trialPeriodDays normalizes every unit into days. I also verified that the notification id scheme here matches the native path at DependencyContainer.kt:672 exactly, so the two flows dedupe against each other rather than double-scheduling. Wire compatibility of the new product field checks out in both directions: RedemptionResult.Success routes through DirectSuccessSerializer's own namingStrategy = null Json, and every relevant instance sets ignoreUnknownKeys = true.

⚠️ Trial eligibility and attribution are taken from the on-screen native paywall, not from the web checkout

handleTrialRedemption uses factory.getPaywallInfo() for both the eligibility gate and the event's paywall identity, while the trial being reported is the one described by redemptionInfo.paywallInfo. isFreeTrialAvailable is computed by PaywallLogic from the native Play products' introductory-offer eligibility, so a user who already consumed their Play trial has it false — and a genuine web trial then emits no freeTrial_start at all, even though product.trialPeriodDays says a trial is running.

Technical details

# Web trial reporting is gated and attributed by native-paywall state

## Affected sites
- `superwall/src/main/java/com/superwall/sdk/web/WebPaywallRedeemer.kt:333` — `if (!paywallInfo.isFreeTrialAvailable) return` gates a *web* trial on *native* product eligibility.
- `superwall/src/main/java/com/superwall/sdk/web/WebPaywallRedeemer.kt:336` — `FreeTrialStart(paywallInfo, …)` attributes the event to the on-screen paywall, so `paywall_identifier` / `experiment_id` / `variant_id` come from there rather than from `redemptionInfo.paywallInfo`.
- `superwall/src/main/java/com/superwall/sdk/paywall/request/PaywallLogic.kt:122-130` — origin of `isFreeTrialAvailable` (`hasFreeTrial` over the native products, subject to `isFreeTrialAvailableOverride`).
- `superwall/src/test/resources/web-redemption-trial.json:22` (`test_paywall`) vs `superwall/src/test/java/com/superwall/sdk/web/WebRedemptionTrialTest.kt:55` (`active_paywall`) — the divergence is set up but only the notification id (`active_paywall_TRIAL_STARTED`) is asserted; no assertion pins the event's paywall identity.

## Required outcome
- An explicit decision, defended by an assertion, on which paywall identity a web-redemption `freeTrial_start` carries.
- A gate whose meaning matches the thing being reported, so a real web trial is not dropped because the native products happen to be trial-ineligible.

## Open questions for the human
- Should `freeTrial_start` from a web redemption be attributed to the paywall on screen at redemption time, or to the paywall recorded at web checkout (`redemptionInfo.paywallInfo`)? They coincide in the usual flow but need not.
- Is the `isFreeTrialAvailable` check intended as "is this user trial-eligible" (in which case native eligibility is the wrong proxy for a web purchase) or as "does the on-screen paywall advertise a trial" (in which case it is deliberate and should be commented as such)?

ℹ️ The one failure mode that strands the flow is untested, and the reminder delay is pinned as correct

WebRedemptionTrialTest covers scheduling failure and scheduling deferral-then-completion, but never the deferral that simply never completes — the case that actually strands the redemption. That gap is not incidental: with attemptToScheduleNotifications built on a raw suspendCoroutine, such a test would hang the suite outright rather than fail, which is itself the clearest demonstration of the inline finding. Separately, the scheduling assertion expects reminder.copy(id = …) with delay unchanged, so it locks in the un-adjusted delay described in the other inline comment.

Technical details

# Missing coverage for the stranded-permission case; delay pass-through asserted as correct

## Affected sites
- `superwall/src/test/java/com/superwall/sdk/web/WebRedemptionTrialTest.kt:219-252` — the failure and deferral tests both terminate; neither models "callback never arrives".
- `superwall/src/test/java/com/superwall/sdk/web/WebRedemptionTrialTest.kt:127-129` — `coVerify { factory.scheduleTrialNotifications(listOf(reminder.copy(id = "active_paywall_TRIAL_STARTED"))) }` asserts `delay` is passed through untouched, so it would keep passing with the mis-timing bug present.

## Required outcome
- A test proving that a `scheduleTrialNotifications` which never resolves still lets `didRedeemLink` and `closePaywallIfExists` run. This becomes writable with `runTest`'s virtual clock only once the underlying suspension is genuinely cancellable.
- Once the delay is derived from the trial end date, an assertion on the resulting `delay` rather than on the config value.

ℹ️ Nitpicks

• RedemptionStoreProductTest.kt:75 is named product is retained without the deprecated identifier, but productIdentifier carries no @Deprecated annotation and neither the KDoc nor the CHANGELOG.md entry mentions a deprecation. Either annotate it or reword the test name.
• WebRedemptionTrialTest calls WebRedemptionResponse.copy(codes = …) in several places. Because allCodes is @Transient with a codes-derived default, copy carries the stale value forward instead of recomputing it. Harmless here (storage.read(LatestRedemptionResponse) is stubbed to null, so allCodes is never read back) but a trap for anyone extending these tests.

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

[pullfrog]

ℹ️ All eight prior review threads are addressed and resolved. One informational note inline, plus a CI observation below.

Reviewed changes — the delta since the prior pullfrog review (fb7b278) is the single commit e7d9289, which reworks the three findings from that review plus the idempotency and ABI threads from the first one.

• Bounded and made cancellable the notification-permission wait — attemptToScheduleNotifications becomes suspendCancellableCoroutine with invokeOnCancellation, releases a previous waiter before installing a new one, guards on continuation.isActive, and is released from onDestroy; the redemption side wraps it in withTimeoutOrNull(WEB_TRIAL_NOTIFICATION_TIMEOUT_MILLIS = 30s). The public 3-arg overload is retained and delegates with applySandboxScaling = true, so both pre-existing native callers (DependencyContainer.kt:685, Superwall.kt:1553) are unchanged apart from no longer leaking on activity destruction.
• Anchored web trial reminders to the checkout end date — new web/WebTrialReminder.kt back-derives trial start from trialPeriodEndDate − trialPeriodDays and returns null for reminders already due, reminders at/after trial end, non-offset or unparseable dates, and arithmetic overflow. The activity additionally subtracts the elapsed permission-dialog time, and NotificationScheduler skips its sandbox delay / 24 / 60 compression for these.
• Made freeTrial_start idempotent per code — a Mutex plus a persisted TrackedWebTrialCodes set, written only after a successful track, so a failed emit is retried rather than swallowed.
• Rewrote RedemptionResult.PaywallInfo to preserve the Kotlin JVM ABI — data class → plain @Serializable class with product as a body var … private set, a 7-param secondary constructor, two copy overloads, component1()–component7() and hand-written equals/hashCode/toString, so the original copy / copy$default / (…, int, DefaultConstructorMarker) descriptors stay callable from precompiled Kotlin. @SerialName was added to all 28 PaywallProduct fields.
• Documented the eligibility/attribution decision — handleTrialRedemption now states in code that the gate and the event's paywall identity deliberately come from the active paywall's presentation snapshot to match iOS, and the test asserts paywall_identifier / experiment_id / variant_id.
• Tests — WebTrialReminderTest (4 cases), five new TrialNotificationPermissionTest cases (delay subtraction, expiry during wait, cancellation, onDestroy release, sandbox bypass), six new WebRedemptionTrialTest cases, a reflection test pinning the old bytecode signatures, and testOptions { unitTests.isIncludeAndroidResources = true } for the Robolectric activity test.

I verified the parts that carry the most risk. The two copy overloads cannot be ambiguous (the 7-param one's trailing product has no default) and their copy$default bridges have different arities; Class.getConstructor does return the synthetic default-marker constructor; the bitmask literals 32 and 61 select exactly the parameters the assertions claim; kotlinx.serialization serializes class-body properties with backing fields and the generated $serializer is a nested type, so private set is reachable on decode; no call site in the repo destructures PaywallInfo or uses it as a map key. I also traced every resume path and found no double-resume — the two catch blocks are mutually exclusive because checkAndRequestNotificationPermissions swallows its own synchronous callback's exceptions, and the inner one returns before the trailing resume(Unit). Finally, bypassing sandbox scaling for web reminders is right rather than a QA regression: Play sandbox compresses native trial durations so the native reminder is compressed to match, while a Stripe test-mode trial runs its real duration and the delay is already anchored to an absolute instant.

ℹ️ Nothing in this delta has been compiled or tested by CI

The most recent PR Tests run on this branch was against 1e34b38a; ddb61c8d, fb7b278c and e7d9289 each have no unit-test check run at all. That matters more than usual here, because the delta's correctness rests on things only a compiler and a test runner can confirm: whether @Serializable accepts a body var with a private set, whether the hand-written copy overloads and the plugin-generated constructors coexist without a signature clash, and whether the new Robolectric cases run at all under the freshly added isIncludeAndroidResources flag.

Technical details

# No CI validation exists for the current head

## Affected sites
- `e7d92897`, `fb7b278c`, `ddb61c8d` — `GET /repos/superwall/Superwall-Android/commits/<sha>/check-runs` returns only the `pullfrog` run (empty for `ddb61c8d`).
- `.github/workflows/pr-tests.yml` — `on: pull_request: branches: [develop]`; the workflow's run list shows `1e34b38a` (success, 2026-09-07T14:37Z) as the last run for `codex/fix-web-redemption-trials`.
- `superwall/build.gradle.kts:88-90` — newly added `testOptions { unitTests.isIncludeAndroidResources = true }`, never exercised by CI.
- `superwall/src/main/java/com/superwall/sdk/models/internal/WebRedemption.kt:128-201` — hand-rolled ABI shims whose signature claims are only checkable at compile time.

## Required outcome
- A green `PR Tests` run against the commit that gets merged, not against a rewritten ancestor.

## Suggested approach
- Push an empty commit or re-run the workflow to confirm why the last two force-pushes did not trigger it, then confirm `:superwall:testDebugUnitTest` passes on `e7d9289`.

ℹ️ Nitpicks

• applySandboxScaling = false quietly carries a second, unrelated meaning: it is also what selects the "subtract the permission-dialog wait from each delay" branch at SuperwallPaywallActivity.kt:932-940. A name closer to the intent — absoluteDelays, say — would read truer at the DependencyContainer.kt:1326 call site, where the comment has to explain the coupling.
• WebRedemption.kt:149 — the secondary constructor's productIdentifier: String? = null default is unreachable positionally (the trailing product has no default) and no call site supplies it by name, so it only generates a $default bridge nobody calls.
• TrackedWebTrialCodes (CacheKeys.kt:325-329) is append-only — nothing prunes redeemed codes. Harmless at realistic code counts, but the set is persisted and unbounded.

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

[pullfrog]

Cancelling the wait also cancels the scheduling here. When withTimeoutOrNull fires at 30s the system dialog is typically still on screen and the activity is still alive, but this clears notificationPermissionCallback, so a grant the user taps at 31s makes onRequestPermissionsResult no-op and the trial reminder is never scheduled — silently, with the trial itself already tracked. onDestroy (line 830) already covers the case where the activity actually goes away, so leaving the callback installed here would let a late grant still schedule best-effort.

Technical details

# A permission grant arriving after the 30s bound is discarded without a trace

## Affected sites
- `superwall/src/main/java/com/superwall/sdk/paywall/view/SuperwallPaywallActivity.kt:960-964` — `invokeOnCancellation` nulls `notificationPermissionCallback` on timeout, not just on activity teardown.
- `superwall/src/main/java/com/superwall/sdk/paywall/view/SuperwallPaywallActivity.kt:1047` — `notificationPermissionCallback?.onPermissionResult(isGranted)` then silently no-ops.
- `superwall/src/main/java/com/superwall/sdk/web/WebPaywallRedeemer.kt:365-368` — the 30s `withTimeoutOrNull` that triggers it.
- `superwall/src/test/java/com/superwall/sdk/paywall/view/TrialNotificationPermissionTest.kt:110-128` — `cancelled permission wait ignores late and duplicate results` pins the current drop-on-cancel behaviour, so changing this is a deliberate decision, not an oversight.

## Required outcome
- A user who grants `POST_NOTIFICATIONS` at any point while the paywall activity is alive should get the trial reminder scheduled, even if the redemption stopped waiting for them.

## Suggested approach
- Drop the `invokeOnCancellation` clear and rely on `onDestroy` plus the replace-on-new-request path for cleanup; the callback already tolerates a completed continuation (`if (!continuation.isActive) return` at line 928 would need to move below the scheduling block instead of above it).

## Open questions for the human
- Is dropping the reminder the intended trade for bounding the wait, or is the bound only meant to unblock the redemption tail?