other(offerings): gate getOfferings delivery on ui_config and stale-cache paths#7181
Merged
Conversation
…he paths getOfferings (with remote config enabled) must not return until the paywall config data it depends on is queryable. Two gaps closed: Delivery already waited for the workflows topic and its prefetch blobs; it now also resolves the ui_config body, concurrently, so a paywall render right after getOfferings has its styling in hand without a further round trip. Both steps remain best-effort: either failing resolves nil rather than stranding delivery. And the stale-memory-cache path previously bypassed the gate entirely, returning before config readiness. Stale delivery now goes through the same gate, with the background refresh kicked first so the gate cannot delay it. The gate is a no-op once config has synced, so stale cache hits still return fast everywhere but the very first launch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
…ns the gate Review findings: the gated stale delivery captured the pre-gate snapshot, so a background refresh finishing during the wait still returned stale packages; delivery now re-reads the cache when the gate opens. The mock's topic hook stores every waiter (the stale test has two: gated delivery + background refresh) so completing it is deterministic. Also trims comment verbosity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
…ness gate Convergence-pass findings: the post-gate cache re-read could deliver an unrelated write (identity or locale change repopulating the single shared slot) to a request that started before it. The stale path's refresh now reports its result through its own completion, same-request by construction, and gated delivery prefers that over the captured snapshot; the shared slot is never read after the gate. Two pinning tests: a fresh-path delivery always returns its captured snapshot, and a stale-path delivery can return the captured snapshot or its own refresh result but never an unrelated slot write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0da3fc0. Configure here.
Bugbot follow-up on the stale-path delivery: the refresh completion writes its result on the main actor while the gated delivery read it from the gate task, so a refresh that finished first could still be invisible to the delivery. The read now happens inside the delivery's main-actor hop, where the write also lands, so main-queue ordering makes any earlier-enqueued refresh result visible. Delivery remains best-effort by design: the contract gates on config readiness, not on refresh completion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
facumenzella
commented
Jul 10, 2026
rickvdl
reviewed
Jul 10, 2026
rickvdl
left a comment
Member
There was a problem hiding this comment.
Nice work! I think this looks good! But would like an extra pair of eyes from @ajpallares before approving
Cut the causal-chain narration and the duplicated main-actor-ordering explanation across the two cached-delivery helpers down to the load-bearing constraint each. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
Per review: drop the opportunistic 'return the refreshed offerings if the background refresh beats the config gate' behavior. It was never required by the contract (which gates on config readiness, not snapshot freshness) and was the sole reason for the Atomic plumbing, the main-actor read-ordering, and the race findings that followed. Stale delivery now returns the snapshot captured for the request, matching the pre-existing stale-cache model; the background refresh still updates the cache for the next call. UiConfigProvider is stateless, so it's created in the gate instead of stored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
A stale-path background refresh passes a nil completion: it only updates the cache, with nothing to deliver. It still went through deliverWhenConfigReady, which awaits (and decodes) workflows + ui_config for a no-op dispatch. Skip the gate when there is no completion, so background refreshes don't pay the readiness wait or the ui_config decode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
The cached getOfferings diagnostic fired before deliverWhenConfigReady, so its recorded latency excluded the readiness wait this PR adds, making a gated cache hit look as fast as an ungated one. Move the tracking into the gated delivery so the metric reflects the true latency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
ajpallares
approved these changes
Jul 10, 2026
ajpallares
left a comment
Member
There was a problem hiding this comment.
I think it looks great! Just a couple of small comments, suggesting to add/improve tests
1 task
…elivery Review follow-ups from ajpallares on #7181: a test locking that a background refresh (nil completion) caches without entering the readiness gate; an assertion on the delivered offerings in the ui_config-failure path; and a test pinning that delivery still fires when both readiness branches resolve empty/failed (the non-throwing, always-awaited guarantee). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H
facumenzella
enabled auto-merge (squash)
July 10, 2026 09:04
This was referenced Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Checklist
purchases-androidand hybridsMotivation
With remote config on,
getOfferingsshouldn't return until the paywall config data it depends on is queryable: offerings, the workflows topic, andui_config(team-confirmed contract). Android already implements this; iOS was only waiting for the workflows topic, and skipped the gate entirely when serving a stale cache. Aligning also makes the iOS and Android launch measurements comparable: Android's numbers include theui_configround trips, iOS's structurally excluded them.Description
deliverWhenConfigReadynow resolves two things concurrently before delivering: the workflows topic + its prefetch blobs (as before), and theui_configbody via the existingUiConfigProvider. Both stay best-effort (each resolves nil on failure, so delivery can never be stranded). The stale-memory-cache path now goes through the same gate, with the background refresh kicked first so the gate can't delay it.Two behavior changes to consciously accept:
ui_config's blob fetches now happen insidegetOfferingsinstead of after it. That's the point — the paywall has its styling in hand whengetOfferingsreturns (parity withWorkflowManager.onPaywallConfigReadyon Android).On-simulator proof (before/after logs)
Cold launches of PaywallsTester (Debug,
ENABLE_REMOTE_CONFIG, live stress project5f07e7e3with 3 active topics / 20 referenced blobs), app container erased before each launch, SDK verbose logs captured vialog stream. TheGATE-PROOFmarkers wrap the app'sgetOfferingscall.BEFORE (
origin/main) —getOfferingsreturns without the ui_config blobs ever being fetched (a later paywall render pays those round trips):AFTER (this branch) — delivery completes the same millisecond the last ui_config blob lands; the app is render-ready when
getOfferingsreturns:Total cold
getOfferingsin these samples: 2.30s before vs 2.17s after (the four downloads are small and parallel; network variance dominates) — the contract is met at negligible cost for this payload, and the render-time fetches disappear.AI session context
AI Context
Metadata
facu/offerings-gate-ui-configGoal
Make
getOfferings(remote config on) return only when offerings, the workflows topic, andui_configare all queryable, matching Android's contract.Initial Prompt
While reconciling why Android's launch benchmark showed ~70% workflows overhead vs iOS's ~20%, the human asked "are you sure on iOS we're waiting for all the blobs to be downloaded, to actually return getOfferings?" — investigation showed iOS gates only on workflows-topic prefetch blobs, while Android additionally awaits the
ui_configbody. The human confirmed with the team: "on the SDK side we can't return getOfferings (when workflows is on) until uiconfig, offerings y workflows are ready", and instructed: check how Android does it and do the same.Important Follow-up Prompts
WorkflowManager.onPaywallConfigReady,OfferingsManagercall sites), not from memory.getOfferings(not prefetched during config sync), which is where Android's gate cost lives.Agent Contribution
awaitTopicAndPrefetchBlobsReady(.workflows)only; Android same PLUSuiConfigProvider.getUiConfig(), gating cached (incl. stale) and fresh deliveries.~/Developer/rc/plans/2026-07-09-ios-getofferings-paywall-config-gate.md), ran it through a Codex critique (1 finding applied: the mock needed a holdable blob-read hook; 2 accepted-as-is notes), implemented, and proved RED → GREEN.getOfferingsreturned on cold config launches (hundreds of samples), confirming the gap this PR closes.Human Decisions
getOfferingsmust not return until offerings + workflows + ui_config are ready.Key Implementation Decisions
deliverWhenConfigReadywith a concurrentasync letpair (workflows readiness,UiConfigProvider.getUiConfig()); both non-throwing and nil-on-failure, so Android's "best-effort, never strand delivery" semantics fall out of the types with no extra error handling.memoryCachedOfferingssnapshot, matching Android'svendCachedOfferingsAndMaybeRefresh(verified inoriginAndroid source: it delivers the capturedcachedOfferingsparam and does not hand the background refresh's result back to the caller). This replaced an earlier opportunistic "return the refresh result if it beats the gate" variant, deleting theAtomic<Offerings?>, the main-actor read-ordering, and the race class those attracted. Tradeoff (accepted, Android-identical): a stale snapshot may be returned even if the refresh already finished; the refresh still updates the cache for the next call.UiConfigProvideris created locally insidedeliverWhenConfigReady(it is stateless), not stored on the manager — one fewer property.Files / Symbols Touched
Sources/Purchasing/OfferingsManager.swift—deliverWhenConfigReady(widened to workflows + ui_config, provider created locally),offerings(appUserID:...)cached/stale delivery gated on the captured snapshot,handleOfferingsBackendResultskips the gate on nil-completion background refreshes.Tests/UnitTests/Purchasing/OfferingsManagerTests.swift— 3 new tests (ui_config hold, best-effort failure, concurrency of the two waits);testGetOfferingsFromStaleMemoryCacheDeliversImmediatelyrewritten to...GatesDeliveryAndStillRefreshes(the old test pinned the behavior this PR removes).Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift—MockRemoteConfigManagergains a holdable blob-read hook (shouldStoreBlobDataCompletion/completeStoredBlobReads()), mirroring the existing topic hook.Validation
OfferingsManagerTests: 45/45. FullUnitTestssuite: no failures introduced (the only failing class,BackendGetCustomerCenterConfigTestssnapshots, fails identically on pristineorigin/mainin this environment — machine-specific references).swift build(SPM): green. SwiftLint: clean (the stale-branch edit initially trippedfunction_body_length; resolved by collapsing the duplicated gated dispatch).Validation Gaps
last_blob_storedshould no longer exceedofferings).getUiConfig()has a tvOS variant; workflows aren't supported there).Review Focus
async letpair semantics: both children awaited, neither throws — any cancellation path that could stranddeliver()?fetchCurrent/uiPreviewMode path (currently ungated, pre-existing)?Risks / Reviewer Notes
getOfferingslatency increases by the ui_config fetch cost when its blobs are CDN-delivered; that is the intended contract, and khepri's inline-budget decisions control the size of the cost.getUiConfig()has no decoded cache, so the gate's decode work repeats at render; Android pays the same shape of cost (accepted parity).Note
Medium Risk
Changes launch-time getOfferings latency and first-launch stale-cache timing when remote config is enabled; behavior is intentional for Android parity but affects paywall readiness contracts.
Overview
Aligns iOS getOfferings (remote config on) with Android: callers don't get offerings until workflows (topic + prefetch blobs) and
ui_configare ready to query, not workflows alone.deliverWhenConfigReadynowasync let-awaits bothawaitTopicAndPrefetchBlobsReady(.workflows)andUiConfigProvider.getUiConfig()in parallel; both are best-effort (nil on failure, no stranded completions). All memory-cache deliveries—including stale hits—use this gate and return the snapshot captured at request time (background refresh still runs first for stale; it may finish before the gate but won't change what this call returns). Diagnostics latency for cache hits is recorded inside the gate. Background cache updates with a nil completion skip the gate and only write the cache.Tests add holdable
ui_configblob reads onMockRemoteConfigManager, multi-waiter topic completion, and coverage for ui_config wait, concurrent gates, stale gating, snapshot identity, and gate-skipped background refresh.Reviewed by Cursor Bugbot for commit e66bdf9. Bugbot is set up for automated code reviews on this repo. Configure here.