fix: [SDK-4474] self-heal SDK-4388-stuck push subscriptions on session start (#2636)

sherwinski · fadi-george · AR Abdul Azeez · web-flow · commit 67cd220813a7 · 2026-05-15T17:54:55.000-04:00
Co-authored-by: Fadi George &lt;fadii925@gmail.com&gt;
Co-authored-by: AR Abdul Azeez &lt;abdul@onesignal.com&gt;
Co-authored-by: abdulraqeeb33 &lt;abdulraqeeb33@gmail.com&gt;
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt
@@ -13,12 +13,15 @@ import com.onesignal.core.internal.operations.Operation
 import com.onesignal.debug.LogLevel
 import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.user.internal.backend.IUserBackendService
+import com.onesignal.user.internal.backend.SubscriptionObject
 import com.onesignal.user.internal.backend.SubscriptionObjectType
 import com.onesignal.user.internal.builduser.IRebuildUserService
 import com.onesignal.user.internal.identity.IdentityModel
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.jwt.JwtTokenStore
 import com.onesignal.user.internal.operations.RefreshUserOperation
+import com.onesignal.user.internal.operations.UpdateSubscriptionOperation
+import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener
 import com.onesignal.user.internal.operations.impl.states.NewRecordsState
 import com.onesignal.user.internal.properties.PropertiesModel
 import com.onesignal.user.internal.properties.PropertiesModelStore
@@ -98,7 +101,13 @@ internal class RefreshUserOperationExecutor(
             // No longer hydrate timezone from remote, set locally
             propertiesModel.timezone = TimeUtils.getTimeZoneId()
 
+            // Retrieve the push subscription ID from the backend configuration model store. Used
+            // both for re-attaching the locally-cached push model below and for the push
+            // self-heal divergence check inside the loop.
+            val pushSubscriptionIdFromConfig = _configModelStore.model.pushSubscriptionId
+
             val subscriptionModels = mutableListOf<SubscriptionModel>()
+            var pushSelfHealOperationForStuckSubscription: UpdateSubscriptionOperation? = null
             for (subscription in response.subscriptions) {
                 val subscriptionModel = SubscriptionModel()
                 subscriptionModel.id = subscription.id!!
@@ -126,10 +135,21 @@ internal class RefreshUserOperationExecutor(
                 // so we don't want to cache these subscriptions from the backend.
                 if (subscriptionModel.type != SubscriptionType.PUSH) {
                     subscriptionModels.add(subscriptionModel)
+                } else if (subscription.id == pushSubscriptionIdFromConfig && pushSelfHealOperationForStuckSubscription == null) {
+                    // Self-heal for users stuck at "Never Subscribed". Older SDK builds dispatched
+                    // the merged create-subscription + update-subscription(SUBSCRIBED) batch as a
+                    // POST /subscriptions carrying the already-existing server-side id; the server
+                    // responded 200 {} (no-op) and the local op queue was emptied, leaving the
+                    // server stuck at enabled:false / notification_types:0. Once that user
+                    // upgrades to a fixed SDK build, nothing in the queue triggers the fix path.
+                    //
+                    // Detect the divergence here (every session start runs RefreshUser) and
+                    // re-assert local truth via PATCH. "Device is the source of truth" is the
+                    // existing policy for push; this just enforces it across the wire when the
+                    // server has drifted out of sync.
+                    pushSelfHealOperationForStuckSubscription = buildPushSelfHealOperationForStuckSubscription(op, subscription, pushSubscriptionIdFromConfig)
                 }
             }
-            // Retrieve the push subscription ID from the backend configuration model store
-            val pushSubscriptionIdFromConfig = _configModelStore.model.pushSubscriptionId
 
             if (pushSubscriptionIdFromConfig != null) {
                 // Retrieve the push subscription model from the model store
@@ -145,7 +165,10 @@ internal class RefreshUserOperationExecutor(
             _propertiesModelStore.replace(propertiesModel, ModelChangeTags.HYDRATE)
             _subscriptionsModelStore.replaceAll(subscriptionModels, ModelChangeTags.HYDRATE)
 
-            return ExecutionResponse(ExecutionResult.SUCCESS)
+            return ExecutionResponse(
+                ExecutionResult.SUCCESS,
+                operations = pushSelfHealOperationForStuckSubscription?.let { listOf<Operation>(it) },
+            )
         } catch (ex: BackendException) {
             val responseType = NetworkUtils.getResponseStatusType(ex.statusCode)
 
@@ -175,6 +198,50 @@ internal class RefreshUserOperationExecutor(
         }
     }
 
+    /**
+     * Push self-heal builder. Returns an [UpdateSubscriptionOperation] iff the cached push
+     * subscription model resolves to enabled-and-opted-in but the server response recorded
+     * the same id as disabled. Returns null otherwise (no divergence — no-op).
+     *
+     * Caller has already verified that [serverSubscription.id] matches the cached
+     * `pushSubscriptionId`, so this method only inspects the local model + server flags.
+     */
+    private fun buildPushSelfHealOperationForStuckSubscription(
+        op: RefreshUserOperation,
+        serverSubscription: SubscriptionObject,
+        pushSubscriptionId: String,
+    ): UpdateSubscriptionOperation? {
+        val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId)
+        if (cachedPushSubscriptionModel == null || cachedPushSubscriptionModel.type != SubscriptionType.PUSH) {
+            return null
+        }
+
+        val (localEnabled, localStatus) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(cachedPushSubscriptionModel)
+        val serverEnabled = (serverSubscription.enabled == true) && ((serverSubscription.notificationTypes ?: 0) > 0)
+        val divergent = localEnabled && !serverEnabled
+
+        return if (divergent) {
+            Logging.info(
+                "RefreshUserOperationExecutor: push subscription $pushSubscriptionId diverged from server " +
+                    "(server enabled=${serverSubscription.enabled} notificationTypes=${serverSubscription.notificationTypes}; " +
+                    "local opted-in and SUBSCRIBED). Enqueuing follow-up update-subscription op to re-assert local " +
+                    "truth via PATCH /subscriptions/{id}.",
+            )
+            UpdateSubscriptionOperation(
+                op.appId,
+                op.onesignalId,
+                _identityModelStore.model.externalId,
+                pushSubscriptionId,
+                cachedPushSubscriptionModel.type,
+                localEnabled,
+                cachedPushSubscriptionModel.address,
+                localStatus,
+            )
+        } else {
+            null
+        }
+    }
+
     companion object {
         const val REFRESH_USER = "refresh-user"
     }
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt
@@ -5,6 +5,8 @@ import com.onesignal.common.exceptions.BackendException
 import com.onesignal.common.modeling.ModelChangeTags
 import com.onesignal.core.internal.operations.ExecutionResult
 import com.onesignal.core.internal.operations.Operation
+import com.onesignal.debug.LogLevel
+import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.mocks.MockHelper
 import com.onesignal.user.internal.backend.CreateUserResponse
 import com.onesignal.user.internal.backend.IUserBackendService
@@ -18,6 +20,7 @@ import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getIdentit
 import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getJwtTokenStore
 import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState
 import com.onesignal.user.internal.operations.impl.executors.RefreshUserOperationExecutor
+import com.onesignal.user.internal.operations.impl.executors.SubscriptionOperationExecutor
 import com.onesignal.user.internal.properties.PropertiesModel
 import com.onesignal.user.internal.subscriptions.SubscriptionModel
 import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
@@ -61,8 +64,10 @@ class RefreshUserOperationExecutorTests : FunSpec({
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId, "aliasLabel1" to "aliasValue1"),
                 PropertiesObject(country = remoteCountry, language = remoteLanguage, timezoneId = remoteTimeZone, tags = remoteTags),
                 listOf(
-                    SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "on-backend-push-token"),
-                    SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken2"),
+                    // notificationTypes = 1 keeps server-side view healthy so the push
+                    // self-heal divergence check is a no-op for this happy-path test.
+                    SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, notificationTypes = 1, token = "on-backend-push-token"),
+                    SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, notificationTypes = 1, token = "pushToken2"),
                     SubscriptionObject(remoteSubscriptionId2, SubscriptionObjectType.EMAIL, token = "name@company.com"),
                 ),
             )
@@ -321,6 +326,172 @@ class RefreshUserOperationExecutorTests : FunSpec({
         }
     }
 
+    // Push self-heal divergence detection. Verifies that when the device-cached push
+    // subscription resolves to enabled-and-opted-in but the GET /users response returns the
+    // same subscription as disabled (the "Never Subscribed" stuck state),
+    // RefreshUserOperationExecutor emits a follow-up UpdateSubscriptionOperation to re-assert
+    // local truth via PATCH.
+    fun buildSelfHealHarness(
+        serverPushEnabled: Boolean,
+        serverNotificationTypes: Int?,
+        localOptedIn: Boolean,
+        localStatus: SubscriptionStatus,
+        localAddress: String,
+    ): Triple<RefreshUserOperationExecutor, SubscriptionModel, IUserBackendService> {
+        val mockUserBackendService = mockk<IUserBackendService>()
+        coEvery { mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } returns
+            CreateUserResponse(
+                mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
+                PropertiesObject(),
+                listOf(
+                    SubscriptionObject(
+                        existingSubscriptionId1,
+                        SubscriptionObjectType.ANDROID_PUSH,
+                        enabled = serverPushEnabled,
+                        notificationTypes = serverNotificationTypes,
+                        token = "on-backend-push-token",
+                    ),
+                ),
+            )
+
+        val mockIdentityModelStore = MockHelper.identityModelStore()
+        val mockIdentityModel = IdentityModel()
+        mockIdentityModel.onesignalId = remoteOneSignalId
+        every { mockIdentityModelStore.model } returns mockIdentityModel
+        every { mockIdentityModelStore.replace(any(), any()) } just runs
+
+        val mockPropertiesModelStore = MockHelper.propertiesModelStore()
+        val mockPropertiesModel = PropertiesModel()
+        mockPropertiesModel.onesignalId = remoteOneSignalId
+        every { mockPropertiesModelStore.model } returns mockPropertiesModel
+        every { mockPropertiesModelStore.replace(any(), any()) } just runs
+
+        val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
+        every { mockSubscriptionsModelStore.replaceAll(any(), any()) } just runs
+
+        val cachedPushSubscriptionModel = SubscriptionModel()
+        cachedPushSubscriptionModel.id = existingSubscriptionId1
+        cachedPushSubscriptionModel.type = SubscriptionType.PUSH
+        cachedPushSubscriptionModel.address = localAddress
+        cachedPushSubscriptionModel.status = localStatus
+        cachedPushSubscriptionModel.optedIn = localOptedIn
+        every { mockSubscriptionsModelStore.get(existingSubscriptionId1) } returns cachedPushSubscriptionModel
+
+        val mockConfigModelStore =
+            MockHelper.configModelStore {
+                it.pushSubscriptionId = existingSubscriptionId1
+            }
+
+        val executor =
+            RefreshUserOperationExecutor(
+                mockUserBackendService,
+                mockIdentityModelStore,
+                mockPropertiesModelStore,
+                mockSubscriptionsModelStore,
+                mockConfigModelStore,
+                mockk<IRebuildUserService>(),
+                getNewRecordState(),
+                getJwtTokenStore(), getIdentityVerificationService(),
+            )
+
+        return Triple(executor, cachedPushSubscriptionModel, mockUserBackendService)
+    }
+
+    test("push self-heal: enqueues follow-up update-subscription op when server is stuck-disabled but local is enabled") {
+        // Given: server view says push is disabled (the stuck state), local view says enabled
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = false,
+                serverNotificationTypes = 0,
+                localOptedIn = true,
+                localStatus = SubscriptionStatus.SUBSCRIBED,
+                localAddress = onDevicePushToken,
+            )
+
+        // Suppress logcat output for the self-heal WARN line so the unmocked android.util.Log
+        // in robolectric-free unit tests doesn't blow up. Restored in finally.
+        val originalLogLevel = Logging.logLevel
+        Logging.logLevel = LogLevel.NONE
+        try {
+            // When
+            val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null)))
+
+            // Then a single UpdateSubscriptionOperation is returned to the op repo
+            response.result shouldBe ExecutionResult.SUCCESS
+            response.operations?.count() shouldBe 1
+            val followup = response.operations!![0]
+            (followup is UpdateSubscriptionOperation) shouldBe true
+            followup as UpdateSubscriptionOperation
+            followup.name shouldBe SubscriptionOperationExecutor.UPDATE_SUBSCRIPTION
+            followup.appId shouldBe appId
+            followup.onesignalId shouldBe remoteOneSignalId
+            followup.subscriptionId shouldBe existingSubscriptionId1
+            followup.type shouldBe SubscriptionType.PUSH
+            followup.enabled shouldBe true
+            followup.address shouldBe onDevicePushToken
+            followup.status shouldBe SubscriptionStatus.SUBSCRIBED
+        } finally {
+            Logging.logLevel = originalLogLevel
+        }
+    }
+
+    test("push self-heal: does NOT enqueue follow-up op when server matches local (healthy push subscription)") {
+        // Given: server already enabled and notificationTypes=1, local also enabled
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = true,
+                serverNotificationTypes = 1,
+                localOptedIn = true,
+                localStatus = SubscriptionStatus.SUBSCRIBED,
+                localAddress = onDevicePushToken,
+            )
+
+        // When
+        val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null)))
+
+        // Then no follow-up ops emitted
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations shouldBe null
+    }
+
+    test("push self-heal: does NOT enqueue follow-up op when local is opted out (UNSUBSCRIBE is intentional)") {
+        // Given: server is disabled, but so is local (user explicitly opted out)
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = false,
+                serverNotificationTypes = -2,
+                localOptedIn = false,
+                localStatus = SubscriptionStatus.SUBSCRIBED,
+                localAddress = onDevicePushToken,
+            )
+
+        // When
+        val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null)))
+
+        // Then no follow-up — opt-out is the user's intent, not divergence
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations shouldBe null
+    }
+
+    test("push self-heal: does NOT enqueue follow-up op when local has NO_PERMISSION (OS-level disable is real)") {
+        // Given: server disabled, local also has no notification permission
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = false,
+                serverNotificationTypes = 0,
+                localOptedIn = true,
+                localStatus = SubscriptionStatus.NO_PERMISSION,
+                localAddress = onDevicePushToken,
+            )
+
+        // When
+        val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null)))
+
+        // Then no follow-up — OS denies notifications, server's view is correct
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations shouldBe null
+    }
+
     test("refresh user is retried when backend returns MISSING, but isInMissingRetryWindow") {
         // Given
         val mockUserBackendService = mockk<IUserBackendService>()
