feat: [SDK-5581] share startup flags latching across hosts by abdulraqeeb33 · Pull Request #29 · OneSignal/OneSignal-KMP-SDK

abdulraqeeb33 · 2026-10-21T17:11:55Z

Description

One line summary

Share startup flag snapshot and latching across hosts, so Android and iOS read APP_STARTUP flags the same way.

Details

StartupFlagsLatch reads a host-provided IStartupFlagsStore once per process and mirrors the APP_STARTUP subset of remote flags back to it, writing only when that subset changed. An unreadable store is rewritten rather than skipped.
Mirrors are serialized inside the latch, on a lock separate from snapshot reads. A supplier overload reads the latest keys under that lock, so racing mirrors leave the newest keys stored.
StartupFlags reports where values came from (STORE, MISSING, UNREADABLE, NO_STORE). Everything except STORE means off. isEnabled returns false for non APP_STARTUP flags.
FeatureManager.refreshAtStartup latches APP_STARTUP flags from that snapshot (even after an earlier refresh) and IMMEDIATE flags from remote keys. Startup flags that disagree with remote come back as DeferredFeatureActivation. Later refreshes never latch APP_STARTUP flags from remote keys.
Read errors other than cancellation are cached as UNREADABLE, so every reader in a process gets the same answer.
The ObjC header check covers the new public types.
Hosts only implement the store. Android usage: feat: [SDK-5581] latch APP_STARTUP flags from a small startup file OneSignal-Android-SDK#2808.

Testing

StartupFlagsTest (17 tests), including a remote turn-off reaching the next process, an unreadable store being rewritten, and cancellation not being cached.
./gradlew :kmp:testDebugUnitTest :kmp:iosSimulatorArm64Test spotlessCheck :kmp:verifyOneSignalKMPXCFramework passes.

- StartupFlagsLatch reads a host-provided IStartupFlagsStore once per process and mirrors the APP_STARTUP subset of remote flags back to it, writing only when it changed. - StartupFlags reports where the values came from (store, missing, unreadable, no store) and rejects non APP_STARTUP flags. - FeatureManager.refreshAtStartup latches APP_STARTUP flags from that snapshot, so isEnabled matches code that ran before init. Later refreshes never latch APP_STARTUP flags from remote keys. Co-authored-by: Cursor <cursoragent@cursor.com>

@throws

- Mark IStartupFlagsStore.read/write and StartupFlagsLatch.mirror @throws, so a Swift store can report an unreadable store or failed write instead of crashing, and Swift callers can catch mirror failures. - StartupFlags keys are canonical and empty unless the source is STORE, so isEnabled and refreshAtStartup can never report a flag on from a missing or unreadable store. - Point the FeatureManager class doc at refreshAtStartup, and note that store implementations must not call back into the latch. Co-authored-by: Cursor <cursoragent@cursor.com>

cursor

Multi-model review (Opus 5.5, GPT 5.6 Sol, Grok 4.7) on 355 lines of Kotlin (FeatureManager + StartupFlags + tests).

The intended snapshot → refreshAtStartup → refresh(false) → mirror path matches the tests. The state machine does not enforce that order, and store I/O is not failure-safe.

Act on

Any earlier refresh() latches APP_STARTUP from remote; a later refreshAtStartup only stores startupKeys and cannot apply the snapshot (3/3).
mirror lets a throwing read() skip write, so a corrupt store stays UNREADABLE and every later process stays all-off (Opus, verified).

Consider

snapshot runs the host lambda and store.read() under non-reentrant NSLock; a thrown provider also leaves snapshot null, so the next call can disagree (3/3).
mirror is an unsynchronized RMW against the same store snapshot reads (3/3).

Noted: first upgrade run is MISSING/all-off even if cache has flags (current catalog is absent=off). localOverrides can diverge FeatureManager from the pre-init snapshot. appStartupKeysIn / Collection<String> widen the Swift surface.

Dismissed: Kotlin/NSSet equality as a correctness bug (speculative; extra writes from raw vs canonical keys are self-healing).

_{Sent by Cursor Automation: PR Reviews}

cursor · 2026-10-21T20:17:12Z

+            startupKeys = startup.keys
+            refreshLocked(remoteKeys, true, localOverrides)
+        }
+
+    // Callers hold [lock]; PlatformLock is not reentrant on iOS.
+    private fun refreshLocked(
+        remoteKeys: List<String>,
+        applyAppStartupFlags: Boolean,
+        localOverrides: List<String>,
+    ): List<DeferredFeatureActivation> {
+        val overrideKeys = localOverrides.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) }
+        val enabledKeys = remoteKeys.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) } + overrideKeys
+        val startup = startupKeys

-            val nextStates = featureStates.toMutableMap()
-            val deferred = mutableListOf<DeferredFeatureActivation>()
-            for (feature in FeatureFlag.entries) {
-                val desired = feature.isEnabledIn(enabledKeys)
-                when (feature.activationMode) {
-                    FeatureActivationMode.IMMEDIATE -> nextStates[feature] = desired
-                    FeatureActivationMode.APP_STARTUP -> {
-                        // After the first refresh every catalog flag is present in
-                        // [featureStates], including those that resolved to false.
-                        val alreadyLatched = nextStates.containsKey(feature)
-                        val applyStartup =
-                            !alreadyLatched ||
-                                (applyAppStartupFlags && !hasCompletedFirstRefresh)
-                        if (applyStartup) {
-                            nextStates[feature] = desired
-                        } else {
-                            val latched = nextStates[feature] ?: false
-                            if (latched != desired) {
-                                deferred.add(
-                                    DeferredFeatureActivation(
-                                        key = feature.key,
-                                        desiredEnabled = desired,
-                                        latchedEnabled = latched,
-                                    ),
-                                )
-                            }
+        val nextStates = featureStates.toMutableMap()
+        val deferred = mutableListOf<DeferredFeatureActivation>()
+        for (feature in FeatureFlag.entries) {
+            val desired = feature.isEnabledIn(enabledKeys)
+            when (feature.activationMode) {
+                FeatureActivationMode.IMMEDIATE -> nextStates[feature] = desired
+                FeatureActivationMode.APP_STARTUP -> {
+                    // After the first refresh every catalog flag is present in
+                    // [featureStates], including those that resolved to false.
+                    val alreadyLatched = nextStates.containsKey(feature)
+                    val applyStartup =
+                        !alreadyLatched ||
+                            (applyAppStartupFlags && !hasCompletedFirstRefresh)
+                    if (applyStartup) {
+                        nextStates[feature] =
+                            if (startup != null) feature.key in startup || feature.key in overrideKeys else desired


Act on (3/3): the first refresh() latches every APP_STARTUP flag from remote via !alreadyLatched while startupKeys is still null. A later refreshAtStartup assigns startupKeys here, then refreshLocked sees hasCompletedFirstRefresh and a full map, so the snapshot is ignored.

That is the mismatch this PR is meant to prevent: pre-init StartupFlagsLatch.snapshot and later FeatureManager.isEnabled can disagree. refresh(true) after a real refreshAtStartup is already safe; the other order is not. Make the first refreshAtStartup the only APP_STARTUP writer, or fail if a refresh already ran.

cursor · 2026-10-21T20:17:13Z

+        remoteKeys: Collection<String>,
+    ): Boolean {
+        val wanted = appStartupKeysIn(remoteKeys)
+        if (store.read() == wanted) return false


Act on (Opus, verified) / consider race (3/3): store.read() is unguarded. If it throws, write(wanted) never runs, so every later process snapshots UNREADABLE and stays all-off. Treat a failed read as “changed” and still write.

The read/compare/write is also unlocked, so two mirrors or a snapshot overlapping a write can persist a stale set or cache UNREADABLE for the process.

cursor · 2026-10-21T20:17:13Z

+        lock.withLock {
+            snapshot ?: read(store()).also { snapshot = it }


Consider (3/3): store() and store.read() run under PlatformLock (iOS NSLock is not reentrant). Re-entry from the provider deadlocks on iOS and can be hidden by Android synchronized.

store() is also outside read()’s try/catch, and snapshot has no @Throws. A thrown provider leaves snapshot null, so the next call can return a different value and break “first snapshot wins, including an unreadable one.” Resolve the store / read outside the lock and map provider failure to UNREADABLE.

fadi-george · 2026-10-21T20:19:54Z

I would address these:

Blocking: StartupFlags.kt:71, mirror calls store.read() first, so if read throws, we never write. The store stays UNREADABLE and startup flags stay off for good. Treat a read failure as changed and write anyway.
Non-blocking: FeatureManager.kt:74, if refresh() runs before refreshAtStartup, the snapshot is silently ignored. Android is fine since init calls refreshAtStartup first, but maybe fail or log so other hosts don't trip on it.
Non-blocking: StartupFlags.kt:66, mirror's read/compare/write isn't under lock. Android covers this with its own lock, but moving it into the latch would protect every host.
Non-blocking: FeatureManager.kt:107, when the snapshot and remote keys disagree at startup, no DeferredFeatureActivation is returned, so the most common divergence never gets logged.
Nit: StartupFlags.kt:82, catch Exception instead of Throwable so CancellationException and Errors aren't cached as UNREADABLE.
Nit: kmp/build.gradle:346, add StartupFlags, StartupFlagsSource, StartupFlagsLatch and IStartupFlagsStore to the ObjC header check.
Nit: StartupFlags.kt:21, @throws makes every Swift isEnabled call need try. Maybe return false for non APP_STARTUP flags instead.

- Mirror writes when the stored file can't be read. - refreshAtStartup latches the snapshot even after an earlier refresh. - Mirror serializes under its own lock and takes a supplier for the latest keys. - Startup flags that disagree with remote come back as deferred activations. - Cancellation is rethrown instead of cached as UNREADABLE. - isEnabled returns false for IMMEDIATE flags instead of throwing. - Header check covers the startup flag types. Co-authored-by: Cursor <cursoragent@cursor.com>

abdulraqeeb33 · 2026-10-21T20:40:05Z

All seven addressed in 05c808f.

Mirror treats a read failure as changed and writes.
refreshAtStartup now applies the snapshot even after an earlier refresh, and returns the difference as deferred so it gets logged.
Mirror lock moved into the latch, separate from the snapshot lock. A supplier overload reads the latest keys under it.
Startup divergence is returned as DeferredFeatureActivation.
Partial: cancellation is rethrown. Other errors still cache as UNREADABLE, so every reader in the process gets the same answer.
Header check covers the four types.
isEnabled returns false for non APP_STARTUP flags, no @Throws.

JonGonOs

👍

abdulraqeeb33 requested a review from a team October 21, 2026 17:11

This was referenced Oct 21, 2026

feat: [SDK-5581] latch APP_STARTUP flags from a small startup file OneSignal/OneSignal-Android-SDK#2808

Merged

feat: [SDK-5581] gate dispatcher bootstrap behind sdk_dispatcher_bootstrap OneSignal/OneSignal-Android-SDK#2805

Merged

abdulraqeeb33 marked this pull request as ready for review October 21, 2026 19:57

cursor Bot reviewed Oct 21, 2026

View reviewed changes

abdulraqeeb33 requested a review from fadi-george October 21, 2026 20:55

fadi-george approved these changes Oct 21, 2026

View reviewed changes

JonGonOs approved these changes Oct 22, 2026

View reviewed changes

abdulraqeeb33 merged commit 35d538a into main Oct 22, 2026
3 checks passed

abdulraqeeb33 deleted the ar/startup-flags branch October 22, 2026 13:36

Navigation Menu

Repository navigation

feat: [SDK-5581] share startup flags latching across hosts - #29

feat: [SDK-5581] share startup flags latching across hosts#29
abdulraqeeb33 merged 3 commits into
mainfrom
ar/startup-flags

abdulraqeeb33 commented Oct 21, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Oct 21, 2026

Uh oh!

cursor Bot Oct 21, 2026

Uh oh!

cursor Bot Oct 21, 2026

Uh oh!

fadi-george commented Oct 21, 2026 •

edited

Loading

Uh oh!

abdulraqeeb33 commented Oct 21, 2026

Uh oh!

JonGonOs left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		lock.withLock {
		snapshot ?: read(store()).also { snapshot = it }

Conversation

abdulraqeeb33 commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

One line summary

Details

Testing

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

fadi-george commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abdulraqeeb33 commented Oct 21, 2026

Uh oh!

JonGonOs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abdulraqeeb33 commented Oct 21, 2026 •

edited

Loading

fadi-george commented Oct 21, 2026 •

edited

Loading