Skip to content

Add repro for workflows forceRefresh retry failure#3555

Merged
vegaro merged 1 commit into
cesar/wfl-280-replace-forceworkflowslistcachestale-with-afrom
facundo/pr3541-force-refresh-retry-test
Jun 8, 2026
Merged

Add repro for workflows forceRefresh retry failure#3555
vegaro merged 1 commit into
cesar/wfl-280-replace-forceworkflowslistcachestale-with-afrom
facundo/pr3541-force-refresh-retry-test

Conversation

@facumenzella

@facumenzella facumenzella commented Jun 8, 2026

Copy link
Copy Markdown
Member

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.

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_old
Loading

Validation

Failure observed on PR #3541 head:

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.

AI session context

AI Context

Metadata

Goal

Review PR #3541, verify whether a suspected workflows-list forceRefresh retry 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

Agent Contribution

  • Reviewed PR Replace workflows forceStale flag with a forceRefresh param #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.
  • Added a Mermaid sequence diagram to the PR description showing the stale timestamp failure flow.

Human Decisions

Key Implementation Decisions

  • Decision: Add only a regression test, not a production fix.
  • Decision: Use Mermaid markdown instead of a bitmap image.
    • Rationale: GitHub renders it inline, it is easy to edit in the PR body, and it avoids adding generated assets to the repo.
    • Rejected: Committing an image or external diagram file.

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

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

Non-goals / Out of Scope

  • Production fix for the retry behavior.
  • Broader refactor of workflows list caching.
  • Adding generated image assets or AI context files to the repository.

Omitted Context

  • Raw transcript, unrelated exploration, sensitive details, repetitive attempts, and chain-of-thought-style content were omitted.

@vegaro vegaro marked this pull request as ready for review June 8, 2026 12:23
@vegaro vegaro requested a review from a team as a code owner June 8, 2026 12:23
@vegaro vegaro merged this pull request into cesar/wfl-280-replace-forceworkflowslistcachestale-with-a Jun 8, 2026
2 of 5 checks passed
@vegaro vegaro deleted the facundo/pr3541-force-refresh-retry-test branch June 8, 2026 12:23
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants