Skip to content

fix: [SDK-4874] prevent stale UpdateSubscription leaving users Never Subscribed#1687

Merged
nan-li merged 1 commit into
mainfrom
nan/sdk-4874
Jul 20, 2026
Merged

fix: [SDK-4874] prevent stale UpdateSubscription leaving users Never Subscribed#1687
nan-li merged 1 commit into
mainfrom
nan/sdk-4874

Conversation

@nan-li

@nan-li nan-li commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Prevent a stale pre-permission UpdateSubscription (notification_types: -19) from overwriting a later permission grant and leaving users stuck Never Subscribed.

Details

Motivation

Rapid permission prompt acceptance can queue an UpdateSubscription while the push subscription is still in the “prompted but never answered” state (enabled: false, notification_types: -19). That request may stay waiting to be sent. When it later flushes with another UpdateSubscription (enabled = true) request, its payload can race with or overwrite the most recent update due to timing of requests accepted by the server, leaving the server subscription disabled / Never Subscribed even though the user accepted.

Fixes SDK-4874.

Scope

  • OSRequestUpdateSubscription rebuilds the PATCH body from the live subscription model in prepareForExecution (and drops the unused subscriptionObject).
  • OSSubscriptionOperationExecutor keeps at most one unsent UpdateSubscription per subscription model (last wins).
  • Adds regression tests in SubscriptionUpdateRaceTests.
  • Does not change public APIs, prompt UX, or skip sending -19 entirely. Does not serialize already in-flight PATCHes (follow-up if needed).

Testing

Unit testing

Added SubscriptionUpdateRaceTests:

  • Refresh in prepareForExecution picks up live enabled / notification_types after grant.
  • A pending pre-permission update + post-grant delta coalesces and sends only the subscribed payload.

Manual testing

Previously reproduced the race with demo app forced to enqueue conflicting UpdateSubscription requests; after the changes in this PR, the requests carry the current subscribed payload.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
    • Ran SubscriptionUpdateRaceTests locally (pass). Full CI pending on this PR.
  • I have personally tested this on my device, or explained why that is not possible
    • Unit coverage for the race; earlier DevApp verification of live refresh. Full CI / device pass still pending.

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

Made with Cursor

…ibed

- Coalesce unsent UpdateSubscription requests per subscription and rebuild the PATCH body from the live model before send
- Motivation: a pre-permission -19 snapshot meaning PROMPTED BUT NOT YET ANSWERED cannot overwrite a later grant as requests are sent async

Co-Authored-By: Cursor <cursoragent@cursor.com>
@nan-li
nan-li requested a review from a team July 20, 2026 16:26
@fadi-george

Copy link
Copy Markdown
Collaborator

Core approach looks good. Two concerns:

refreshParametersFromLiveModel() omits device_model, net_type, and test_type, so those updates are no longer sent.
Retryable and in-flight requests can bypass coalescing and still send stale data.
Also, fixed test sleeps may be flaky in CI.

@nan-li

nan-li commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Core approach looks good. Two concerns:
refreshParametersFromLiveModel() omits device_model, net_type, and test_type, so those updates are no longer sent.

"No longer sent" is not quite accurate, this is existing and it is not sent currently. And for subscription updates, those things don't really need updating, they are not really used nor change after an app is already installed.

Retryable and in-flight requests can bypass coalescing and still send stale data. Also, fixed test sleeps may be flaky in CI.

Called out in PR that it does not serialize already in-flight PATCHes (follow-up if needed). Omitted as out of scope for this PR, as it is possible but very hard to hit, in favor of a larger planned refactor of operation repo / executors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants