Add repro for workflows forceRefresh retry failure#3555
Merged
vegaro merged 1 commit intoJun 8, 2026
Conversation
2 of 5 checks passed
vegaro
pushed a commit
that referenced
this pull request
Jun 8, 2026
## Summary Adds a regression test that demonstrates the failed `forceRefresh` retry path in PR #3541. The test covers this sequence: 1. A workflows list fetch succeeds and caches `default -> wf_old`. 2. A later `forceRefresh = true` fetch runs while that old list is still fresh, but the backend request fails. 3. The next normal `getWorkflowsList(..., forceRefresh = false)` should retry and update the map to `wf_new`. On the current implementation from #3541, step 3 is skipped because the old workflows-list timestamp remains fresh after the failed forced refresh. ## Validation This is a repro PR, so the added test is expected to fail until the implementation changes. ```bash rtk ./gradlew :purchases:testDefaultsBc8DebugUnitTest --tests "com.revenuecat.purchases.common.workflows.WorkflowManagerTest.getWorkflowsList retries after failed forceRefresh even when old list cache was fresh" ``` Failure observed on PR #3541 head: ```text Verification failed ... 2 matching calls found, but needs at least 3 and at most 3 calls ``` Before adding this repro, `WorkflowManagerTest` and `WorkflowsCacheTest` passed on PR #3541 head in the focused run. <details> <summary>AI session context</summary> # AI Context ## Metadata - PR: Not captured - Branch: `facundo/pr3541-force-refresh-retry-test` - Author / human owner: `facumenzella` - Agent(s): Codex - Session source: current conversation - Generated: 2026-06-08 - Context document version: 1 ## Goal Review PR #3541, verify whether a suspected workflows-list `forceRefresh` retry regression is real, then open a repro PR against PR #3541 so the original author can decide how to handle it. ## Initial Prompt Review `https://github.com/RevenueCat/purchases-android/pull/3541`. ## Important Follow-up Prompts - User asked to write a test and prove the suspected issue is broken. - User then asked to open a PR against PR #3541 so `vegaro` can decide. ## Agent Contribution - Reviewed PR #3541 diff against its stacked base branch. - Identified a behavioral difference: replacing `forceWorkflowsListCacheStale()` with call-scoped `forceRefresh` no longer clears the old workflows-list timestamp before the forced fetch. - Added a focused failing unit test that proves a failed forced refresh is not retried by the next non-forced list call while the old cache timestamp is still fresh. - Ran the focused Gradle test command and captured the failure. - Created and pushed branch `facundo/pr3541-force-refresh-retry-test`. ## Human Decisions - Human accepted the review finding as worth proving with a test. - Human decided to open a PR against PR #3541 rather than directly changing the implementation. ## Key Implementation Decisions - Decision: Add only a regression test, not a production fix. - Rationale: The purpose is to provide reproducible evidence and let PR #3541's author decide the implementation. - Rejected: Directly patching `WorkflowManager` in this PR. ## Files / Symbols Touched - `purchases/src/test/java/com/revenuecat/purchases/common/workflows/WorkflowManagerTest.kt` - Why: Add the smallest unit-level repro for the retry behavior. - Symbols: `WorkflowManagerTest`, `getWorkflowsList retries after failed forceRefresh even when old list cache was fresh` - Review relevance: Confirms whether a failed forced refresh should leave the list retryable even when the previous list timestamp was fresh. ## Dependencies / Config / Migrations - None captured. ## Validation - Commands run: - `rtk ./gradlew :purchases:testDefaultsBc8DebugUnitTest --tests "com.revenuecat.purchases.common.workflows.WorkflowManagerTest.getWorkflowsList retries after failed forceRefresh even when old list cache was fresh"`: failed as expected with only two `Backend.getWorkflows` calls observed where the test expected three. - `rtk ./gradlew :purchases:testDefaultsBc8DebugUnitTest --tests "com.revenuecat.purchases.common.offerings.OfferingsManagerTest" --tests "com.revenuecat.purchases.common.workflows.WorkflowManagerTest" --tests "com.revenuecat.purchases.common.workflows.WorkflowsCacheTest"`: `WorkflowManagerTest` and `WorkflowsCacheTest` passed before adding the repro; `OfferingsManagerTest` failed at class initialization because Robolectric selected Android SDK 36 with Java 17. - Manual verification: - Inspected the failing test output and confirmed the skipped third backend call. - CI: - Not captured. ## Validation Gaps - The repro PR intentionally contains a failing test against PR #3541's current implementation. - `OfferingsManagerTest` was not validated locally due to Java/Robolectric SDK mismatch. ## Review Focus - Should a failed forced workflows-list refresh keep the list stale/retryable for the next call? - If yes, decide whether to restore timestamp-clearing behavior, track failed forced refresh state, or use another retry signal. ## Risks / Reviewer Notes - Risk: The test encodes a desired retry behavior rather than current behavior. - Evidence: It fails on PR #3541 head with only two backend list calls. - Mitigation: Treat this PR as a repro and decision aid, not a final implementation. ## Non-goals / Out of Scope - Production fix for the retry behavior. - Broader refactor of workflows list caching. ## Omitted Context - Raw transcript, unrelated exploration, sensitive details, repetitive attempts, and chain-of-thought-style content were omitted. </details>
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.
Summary
Adds a regression test that demonstrates the failed
forceRefreshretry path in PR #3541.The test covers this sequence:
default -> wf_old.forceRefresh = truefetch runs while that old list is still fresh, but the backend request fails.getWorkflowsList(..., forceRefresh = false)should retry and update the map towf_new.On the current implementation from #3541, step 3 is skipped because the old workflows-list timestamp remains fresh after the failed forced refresh.
Failure Flow
sequenceDiagram participant App as App / Offerings participant WM as WorkflowManager participant Cache as WorkflowsCache participant Backend as Backend App->>WM: getWorkflowsList(forceRefresh = false) WM->>Backend: fetch workflows list Backend-->>WM: success: default -> wf_old WM->>Cache: cache list, timestamp = fresh App->>WM: getWorkflowsList(forceRefresh = true) WM->>Backend: force fetch workflows list Backend-->>WM: network error Note over WM,Cache: Old list stays timestamp-fresh App->>WM: getWorkflowsList(forceRefresh = false) WM->>Cache: isWorkflowsListCacheStale() == false WM-->>App: completes without backend retry Note over App,Cache: Still using default -> wf_oldValidation
Failure observed on PR #3541 head:
Before adding this repro,
WorkflowManagerTestandWorkflowsCacheTestpassed on PR #3541 head in the focused run.AI session context
AI Context
Metadata
facundo/pr3541-force-refresh-retry-testfacumenzellaGoal
Review PR #3541, verify whether a suspected workflows-list
forceRefreshretry regression is real, open a repro PR against PR #3541, and make the failure easier to inspect visually.Initial Prompt
Review
https://github.com/RevenueCat/purchases-android/pull/3541.Important Follow-up Prompts
forceStaleflag with aforceRefreshparam #3541 sovegarocan decide.Agent Contribution
forceStaleflag with aforceRefreshparam #3541 diff against its stacked base branch.forceWorkflowsListCacheStale()with call-scopedforceRefreshno longer clears the old workflows-list timestamp before the forced fetch.facundo/pr3541-force-refresh-retry-test.Human Decisions
forceStaleflag with aforceRefreshparam #3541 rather than directly changing the implementation.Key Implementation Decisions
forceStaleflag with aforceRefreshparam #3541's author decide the implementation.WorkflowManagerin this PR.Files / Symbols Touched
purchases/src/test/java/com/revenuecat/purchases/common/workflows/WorkflowManagerTest.ktWorkflowManagerTest,getWorkflowsList retries after failed forceRefresh even when old list cache was freshDependencies / Config / Migrations
Validation
rtk ./gradlew :purchases:testDefaultsBc8DebugUnitTest --tests "com.revenuecat.purchases.common.workflows.WorkflowManagerTest.getWorkflowsList retries after failed forceRefresh even when old list cache was fresh": failed as expected with only twoBackend.getWorkflowscalls observed where the test expected three.rtk ./gradlew :purchases:testDefaultsBc8DebugUnitTest --tests "com.revenuecat.purchases.common.offerings.OfferingsManagerTest" --tests "com.revenuecat.purchases.common.workflows.WorkflowManagerTest" --tests "com.revenuecat.purchases.common.workflows.WorkflowsCacheTest":WorkflowManagerTestandWorkflowsCacheTestpassed before adding the repro;OfferingsManagerTestfailed at class initialization because Robolectric selected Android SDK 36 with Java 17.Validation Gaps
forceStaleflag with aforceRefreshparam #3541's current implementation.OfferingsManagerTestwas not validated locally due to Java/Robolectric SDK mismatch.Review Focus
Risks / Reviewer Notes
forceStaleflag with aforceRefreshparam #3541 head with only two backend list calls.Non-goals / Out of Scope
Omitted Context