feat(workflows): persist prefetched workflow detail on disk#6917
Merged
Conversation
8310741 to
2fd53e1
Compare
vegaro
reviewed
Jun 5, 2026
50489ad to
16be480
Compare
vegaro
reviewed
Jun 8, 2026
Member
Author
|
Ready for another pass whenever you can @vegaro |
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 51484d0. Configure here.
vegaro
approved these changes
Jun 8, 2026
…recovery Prefetched workflow detail was kept in memory only, so a cold start with the backend down could restore the offeringId -> workflowId map but couldn't render a single workflow. This persists each prefetched workflow's resolved WorkflowDataResult to a new DeviceCache region, mirroring how the workflows list is stored, and restores it into the in-memory cache on a list-fetch failure so a later getWorkflow is a cache hit with no failed network call. - Persist on the prefetch path only (persistDetail flag), after a successful fetch, so a persisted detail is always renderable offline. - Prune the on-disk detail store to the current list on each list write, and clear it on identity transitions alongside the list disk cache. - Restore details fresh so getWorkflow serves them offline; the list restores stale so it refetches once the backend is back. Port of RevenueCat/purchases-android#3537 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Move workflow-detail disk restore into WorkflowsCache (next to the list restore) so all disk-restore logic lives in one place. - Add a batched in-memory cache(workflows:) and persist prefetched details to disk in a single write at the end of prefetchWorkflows instead of one write per workflow. - Guard the disk write with a generation token bumped on clearCache, so an in-flight prefetch from a since-logged-out user can't write its details back after the store was cleared (cross-user leak). The clear + generation bump run under the same lock as the write. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address Bugbot findings on the detail-cache persistence: - Extend the generation guard to the in-memory write so an in-flight fetch for the previous user can't repopulate memory after an identity change (workflow detail is user-scoped). The in-memory clear now runs under the same lock as the generation bump. - restoreWorkflowDetailsFromDisk fills only ids missing from memory, so a stale disk snapshot can't clobber a fresher on-demand fetch, and holds the disk-read + memory-write under the lock so a clear can't interleave. - persistWorkflowDetailsToDisk filters the persisted map to the current list ids, keeping the on-disk store a subset of the latest list when a slower prefetch from an earlier list lands after a prune. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cache(workflow:workflowId:) and cache(workflows:) have no production callers since getWorkflow writes via the generation-guarded cache(workflow:workflowId:ifGeneration:) and restore inlines its own fill-gaps write. Remove both and seed the in-memory cache in tests through the guarded path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The restored details are stamped fresh so they serve offline, but the old comment implied the stale-list restore drives their refresh. It doesn't: once the backend is back, getWorkflow cache-hits the fresh details (no backend call) until their own foreground TTL expires. The stale list only drives the list/map refetch. Clarify both comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tity change If a workflows list request issued for the previous user finished after an identity change cleared the cache, the success path still cached that list and prefetched its details, persisting the prior user's targeted workflow details to the shared on-disk map for the new session. Capture the cache generation when the list request is issued and, on success, drop the response (no list cache, no prefetch) when the generation no longer matches, still firing onComplete so callers aren't blocked. Thread that issue-time generation into the prefetch disk persist instead of re-capturing it after the clear, so a clear landing mid-prefetch is also caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
51484d0 to
c06fba9
Compare
This was referenced Jun 8, 2026
facumenzella
added a commit
that referenced
this pull request
Jun 9, 2026
…n or logout (#6944) * fix(workflows): close the residual list-cache cross-user race #6917 dropped a late list fetch from the previous user on identity change, but a narrow window remained between the success-path generation re-check and the still-unguarded cache(workflowsList:) write. Fold the check into a generation-guarded cache(workflowsList:ifGeneration:) that does the check, the memory and disk writes, and the detail prune atomically under detailsLock, and put restoreWorkflowsListFromDisk and the list clear under the same lock. The guarded write reports whether it landed so the manager skips prefetching a dropped list's details. Scope is the workflows cache only; the offerings cache is intentionally left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(workflows): guard prefetch in-memory write with the list generation getWorkflow re-captured currentCacheGeneration() at its own call time. From the prefetch path that's too late: a clearCache() landing between caching the list and the prefetch issuing its getWorkflow makes that fresh capture read the already-bumped generation, so the in-memory write passes its own guard and the previous user's (user-scoped) detail repopulates the cleared cache. The disk write was already correct because it used the list's generation. Forward the list generation into getWorkflow's in-memory write (on-demand callers still capture fresh via the nil default), so both the disk and in-memory writes are guarded by the same token. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(workflows): drop dead prune wrapper, rename lock-free prune body Address review: the standalone pruneWorkflowDetails wrapper was dead (the guarded list write is the only pruner and already holds detailsLock), and the "...Locked" suffix on the body read as if it took the lock. Collapse to a single pruneWorkflowDetails that documents the lock precondition instead of encoding a misleading suffix in the name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Port of RevenueCat/purchases-android#3537.
What
#6882/#6883 persist the
/workflowslist to disk and restore theofferingId → workflowIdmap when the backend is down. But each prefetched workflow's detail was kept in memory only, so a cold start with the backend down could restore the map yet not render a single workflow. The list disk cache was half a resilience feature on its own.This persists each prefetched workflow's detail too, so a cold start with the backend down renders the workflows we told the SDK mattered.
Flow
flowchart TD Start([getWorkflowsList]) --> Stale{list cache stale?} Stale -->|no| Done([onComplete]) Stale -->|yes| Fetch[backend.getWorkflows] Fetch -->|success| CacheList[cache list: memory + disk] CacheList --> Prune[prune disk details<br/>not in latest list] CacheList --> Prefetch[prefetch each prefetch=true workflow] Prefetch --> GW["getWorkflow: capture cacheGeneration,<br/>resolve, cache in memory if gen still matches"] GW --> Accum[accumulate resolved results] Accum --> LastOne{last prefetch landed?} LastOne -->|yes| Persist["batch-persist details to disk<br/>if gen matches, filtered to current list ids"] Persist --> Done Fetch -->|failure| RestoreList["restore list from disk → stale<br/>(drives refetch when backend is back)"] RestoreList --> RestoreDetails["restore details from disk → fresh<br/>(fill only ids missing in memory)"] RestoreDetails --> Done2([onComplete]) RestoreDetails -.enables.-> Offline["later getWorkflow → in-memory hit,<br/>no backend round-trip"] Clear([clearCache on identity change]) ==> Bump["under detailsLock: bump cacheGeneration,<br/>clear in-memory + on-disk details"] classDef new fill:#e8f5e9,stroke:#43a047,color:#1b5e20; class CacheList,Prune,Accum,Persist,RestoreDetails,Offline,Bump newGreen = added/changed by this PR. Everything else is the existing list-cache flow from #6882/#6883.
Divergences from Android
The Android PR persists the envelope (
url/hash/enrolled_variants) and re-resolves it offline, re-reading the CDN file. On iOS the envelope never reachesWorkflowManager: the networking layer consumes the raw response and hands back a fully-resolvedWorkflowDataResult, for consistency with how every other endpoint works on iOS. So iOS persists the resolved result and restores it directly.Consequences of that fork:
use_cdnworkflows the compiled JSON is duplicated on disk (the prefetch set is small). Android avoids this; iOS can't cheaply.Where iOS is deliberately stricter than Android (driven by Bugbot + review on this PR):
cacheWorkflowsListintentionally leaves the identity-transition race unguarded (last-write-wins, self-heals on the next fetch, same as the offerings cache). iOS guards the detail writes with acacheGenerationtoken so an in-flight fetch for the previous user can't repopulate memory or disk afterclearCache(). Workflow detail is user-scoped (subscribers/{appUserID}/workflows/{id}), so this closes a real wrong-user window.Same as Android: prefetch-only persistence (on-demand stays memory-only, LRU cap is a follow-up), prune-to-list on each list write, clear-on-identity, details restored fresh, and
onCompletefiring exactly once.Known follow-up (not in this PR): a late list fetch from a previous user is now dropped on identity change (the generation is captured when the list request is issued), so the common race no longer caches the wrong user's list or persists their details. A narrow residual window remains between the success-path generation re-check and the still-unguarded
cache(workflowsList:)write; it only affects theofferingId → workflowIdmap (details stay fully guarded) and self-heals on the next offerings refresh / list TTL. Closing it fully means a generation-guarded list write underdetailsLock, done consistently with the offerings cache rather than bolted on here.AI session context
Metadata
facu/workflow-persist-detail-cache)facu/workflow-persist-detail-cache(base:main; originally stacked on feat(workflows): synchronously seed workflow paywall from warm cache #6905, which has since merged, so the branch was rebased ontomain)Goal
Port purchases-android#3537 (persist prefetched workflow detail for backend-down recovery) to iOS, then address Bugbot/review findings and keep the branch current with
main.Initial Prompt
"let's port purchases-android#3537 to iOS. Take #6905 into account when integrating that. Most likely useful to branch off #6905."
Important Follow-up Prompts
WorkflowDataResult) over strict Android parity (raw envelope + re-resolve).cache(workflows:), prefetch uses the batched path).mainafter feat(workflows): synchronously seed workflow paywall from warm cache #6905 merged.r3374267695(High: "Late fetch persists prior user details") → confirmed real with a RED test, then "keep it proportionate" (vs. the heavier lock-guarded list write): manager-level drop of a late list fetch + thread the issue-time generation into the prefetch persist.Agent Contribution
WorkflowManager,WorkflowsCache,DeviceCache,WorkflowDetailProcessor,WorkflowsAPI,GetWorkflowOperation) and identified the architectural fork: the envelope never reachesWorkflowManageron iOS.DeviceCacheworkflow-details region,WorkflowsCachedisk methods (persist/merge/prune/restore/clear), prefetch-path persistence inWorkflowManager, disk restore on list-fetch failure.CodabletoWorkflowDataResultwith a populated-AnyDecodableencode→decode round-trip test to prove lossless persistence.cacheGenerationguard to the in-memory write and moved the in-memory clear underdetailsLock(cross-user race); made restore fill-gaps-only; filtered the persisted map to the current list ids; closed the same race on the restore path by holding the disk-read + memory-write under the lock.main.clearCache()landing while the list request was in flight was already baked into that generation and the prior user's detail writes passed the guard. Moved the capture to list-issue time, added a success-path generation re-check that drops the response (no list cache, no prefetch) while still firingonComplete, and threaded the issue-time generation into the prefetch disk persist. Proven RED then GREEN with a new manager test (+ aMockWorkflowsAPIbefore-completion hook seam).Human Decisions
WorkflowsCache, batchedcache(workflows:), prefetch uses the batched path.Key Implementation Decisions
WorkflowDataResult, not the raw envelope. Rationale: iOS never surfaces the envelope to the manager; matches every existing cache region; self-contained offline render with no CDN dependency. Rejected: persist raw envelope + re-resolve (faithful to Android but forces signature churn through the networking layer).getWorkflowserve offline; the stale list drives a refetch once the backend is back.cacheGeneration+detailsLock) covers both memory and disk detail writes; bumped onclearCache()so a fetch issued for the previous user is dropped. Scope is the detail cache only (see the list follow-up note above).onComplete), and pass that issue-time generation into the prefetch persist. Rationale: the prior re-capture happened after any in-flight clear, so it could not detect it. De-scoped (per "keep it proportionate"): a fully airtight, generation-guardedcache(workflowsList:ifGeneration:)underdetailsLock(would also needpruneWorkflowDetailsde-nested to avoid a re-entrant deadlock and the list clear moved under the lock). The residual list-map TOCTOU is pre-existing and narrow; details are unaffected.Files / Symbols Touched
Sources/Networking/Responses/WorkflowsResponse.swift—WorkflowDataResult: Codable. Review: lossless round-trip ofAnyDecodable-backed fields.Sources/Caching/DeviceCache.swift—CacheKey.workflowDetails,cachedWorkflowDetails()/cache(workflowDetails:)/clearWorkflowDetailsCache().Sources/Caching/WorkflowsCache.swift—cache(workflows:),cache(workflow:workflowId:ifGeneration:),persistWorkflowDetailsToDisk(_:ifGeneration:),currentCacheGeneration(),restoreWorkflowDetailsFromDisk()(fill-gaps, under lock),pruneWorkflowDetails(toListIds:),cachedListWorkflowIds(),cacheGeneration/detailsLock, clear-on-identity. Review: lock scope, list-id filter on persist, fill-gaps restore.Sources/Purchasing/WorkflowManager.swift—getWorkflowcapturescurrentCacheGeneration()and writes via theifGeneration:path;getWorkflowsListnow captures the generation at request-issue time and drops the success path if it changed; prefetch accumulates results and batch-persists once the last lands (now takes the issue-timegenerationinstead of re-capturing); restore on list-fetch failure. Review: the success-path re-check + the threaded generation.Tests/UnitTests/Caching/WorkflowsCacheTests.swift,Tests/UnitTests/Caching/DeviceCacheTests.swift,Tests/UnitTests/Purchasing/WorkflowManagerTests.swift(+testGetWorkflowsListDropsListAndPrefetchWhenIdentityChangesWhileListFetchInFlight),Tests/UnitTests/Mocks/MockDeviceCache.swift,Tests/UnitTests/Mocks/MockWorkflowsAPI.swift(onGetWorkflowsBeforeCompletionhook).Dependencies / Config / Migrations
Package.resolvedtouched only locally by tooling and reverted; not in the diff.)Validation
swift build: success.swiftlinton changed files: no violations.xcodebuild test -scheme UnitTestsforWorkflowsCacheTests(41) +WorkflowManagerTests(36): 77 passing, 0 failures, on iPhone 16 (latest sim), after rebasing ontomain.WorkflowManagerTestsran RED (5 failures: wrong-user list cached, prefetch ran, detail persisted to disk + memory, offering map populated) before the fix, then GREEN (37/37, including the two existing mid-prefetch identity tests) after.swiftlintclean on the changed files.Validation Gaps
Review Focus
detailsLockscope: generation-check + memory/disk writes atomic againstclearCache(); no nested-lock deadlock (lock orderdetailsLock → cachedWorkflows).cachedWorkflow(forOfferingId:): after recovery the list is stale, so the sync seed bails to the async path.Risks / Reviewer Notes
use_cdnworkflows. Mitigation: pruned to the current list on each list write; cleared on identity changes; prefetch set is small.cache(workflowsList:)write. Evidence: those two statements are adjacent and synchronous (no await between them), so the window is a few instructions; details are unaffected (their writes re-check the generation underdetailsLock). Mitigation: self-heals on the next offerings refresh / list TTL. Tracked as the follow-up noted above. The common race (clear before the list completion) is now closed by the success-path drop.Non-goals / Out of Scope
cache(workflowsList:ifGeneration:)underdetailsLock(do it consistently with the offerings cache). The common identity-transition race is handled in this PR.Omitted Context
Note
Medium Risk
Touches paywall workflow caching, cross-user identity races, and persisted user-targeted workflow JSON; mitigated by generation guards, pruning, and broad unit tests, but wrong-user list cache remains unguarded as noted in the PR.
Overview
Adds disk persistence for prefetched workflow details so a cold start with the backend down can still render paywalls after the workflows list is restored from disk.
DeviceCachegains a non-user-scopedworkflowDetailslarge-item region (workflowId→WorkflowDataResult), cleared on identity changes like the workflows list.WorkflowsCachebatches prefetched details to disk after prefetch completes, restores them into memory (fresh TTL, fill-gaps only) when the list fetch fails, and prunes on-disk details when the list no longer includes those ids. AcacheGenerationtoken plusdetailsLockdrop in-flight memory/disk writes afterclearCache()so the previous user’s user-scoped workflow payloads cannot repopulate the cache mid login/logout.WorkflowManageraccumulates successful prefetch results into one disk write, skips persisting on-demandgetWorkflowfetches, drops stale list responses if identity changed mid-flight, and restores list + details from disk on list failure.WorkflowDataResultisCodablefor JSON persistence (with round-trip tests forAnyDecodablefields).Reviewed by Cursor Bugbot for commit c06fba9. Bugbot is set up for automated code reviews on this repo. Configure here.