Config Manager actor refactor

[ianrumac]

Changes in this pull request

Checklist

• All unit tests pass.
• All UI tests pass.
• Demo project builds and runs.
• I added/updated tests or detailed why my change isn't tested.
• I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
• I have run ktlint in the main directory and fixed any issues.
• I have updated the SDK documentation as well as the online docs.
• I have reviewed the contributing guide

Greptile Summary

This PR refactors ConfigManager into a SequentialActor-based system, moving all state-mutation logic into typed Actions/Updates sealed classes inside ConfigState. It also renames TestModeManager → TestMode, absorbs the activation UI flow into that class, fixes the thenIf boolean-guard bug, and ships a large ConfigManagerTest suite.

• P1: ConfigManager.reevaluateTestMode is a plain fun called from the IdentityManagerActor queue; it mutates TestMode.state concurrently with ConfigState.Actions.ApplyConfig (which runs on the config actor queue), creating an unsynchronized write-write race on the unguarded var state.

Confidence Score: 4/5

Mostly safe to merge; one concurrency issue in reevaluateTestMode should be addressed to prevent rare but real test-mode state corruption.

A single P1 data race on TestMode.state between two independent actor queues caps the score at 4. The rest of the changes are well-structured, well-tested, and address prior review feedback.

superwall/src/main/java/com/superwall/sdk/config/ConfigManager.kt (reevaluateTestMode) and superwall/src/main/java/com/superwall/sdk/store/testmode/TestMode.kt (unsynchronized state var)

Important Files Changed

Filename	Overview
superwall/src/main/java/com/superwall/sdk/config/ConfigManager.kt	Thin actor facade; reevaluateTestMode is a plain fun that mutates TestMode.state outside the actor queue, racing with ApplyConfig on the identity actor path.
superwall/src/main/java/com/superwall/sdk/config/models/ConfigState.kt	Core of the actor refactor — typed Actions/Updates seal all state mutations; contains redundant double-launch for tracking and the identityManager double-invoke in ApplyConfig.
superwall/src/main/java/com/superwall/sdk/config/ConfigContext.kt	New interface defining the actor execution context; clean and well-structured.
superwall/src/main/java/com/superwall/sdk/store/testmode/TestMode.kt	Renamed from TestModeManager; absorbed activation UI flow; state var is unsynchronized and accessible from multiple actor queues concurrently.
superwall/src/main/java/com/superwall/sdk/dependencies/DependencyContainer.kt	Wiring updated for actor and TestMode rename; contains a commented-out DebugInterceptor line that should be removed.
superwall/src/main/java/com/superwall/sdk/misc/Either.kt	thenIf boolean guard fixed — the boolean parameter is now properly consulted before executing the then lambda.
superwall/src/test/java/com/superwall/sdk/config/ConfigManagerTest.kt	Comprehensive new test suite covering fetch, retry, caching, test mode, preload, and refresh scenarios.
superwall/src/main/java/com/superwall/sdk/misc/primitives/StoreContext.kt	sideEffect suspend modifier removed — fire-and-forget semantics now correctly declared as a non-suspend fun.

Sequence Diagram

sequenceDiagram
    participant I as IdentityManagerActor
    participant S as SdkContext
    participant CM as ConfigManager
    participant A as SequentialActor (Config)
    participant TM as TestMode

    Note over I,TM: Config fetch path (inside actor)
    CM->>A: immediate(FetchConfig)
    A->>A: network.getConfig()
    A->>A: immediate(ApplyConfig)
    A->>TM: evaluateTestMode() [inside actor queue]
    TM-->>TM: state = Active/Inactive

    Note over I,TM: Identity change path (outside actor, concurrent)
    I->>S: reevaluateTestMode(appUserId, aliasId)
    S->>CM: reevaluateTestMode(...)
    CM->>TM: evaluateTestMode() [outside actor queue]
    TM-->>TM: state = Active/Inactive

    Note over A,TM: Race: both paths write TestMode.state concurrently

Loading

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
superwall/src/main/java/com/superwall/sdk/config/ConfigManager.kt:118-141
**Concurrent `TestMode.state` mutation bypasses the actor queue**

`reevaluateTestMode` is a plain `fun` (not an actor action), so it runs on whichever coroutine the caller uses — in practice the `IdentityManagerActor` queue (see `IdentityManagerActor.kt:317`). At the same time, `ConfigState.Actions.ApplyConfig` mutates the same `TestMode.state` var from inside the config actor queue. Because `TestMode.state` is an unsynchronized `var`, these two paths can race: one side can overwrite a just-evaluated state while the other is mid-check on `wasTestMode / isNowTestMode`, leading to stale or skipped activation / deactivation callbacks.

Wrapping `reevaluateTestMode` as a proper actor action (similar to `ApplyConfig`) would guarantee serial access to `TestMode.state` through the single FIFO queue the rest of the config logic already uses.

### Issue 2 of 4
superwall/src/main/java/com/superwall/sdk/dependencies/DependencyContainer.kt:449
**Commented-out debug code left in**

`// DebugInterceptor.install(configActor, name = "Config")` is a debugging artifact that should be removed before merging.

### Issue 3 of 4
superwall/src/main/java/com/superwall/sdk/config/models/ConfigState.kt:156-159
**Redundant double-launch for device-attributes tracking**

`track(...)` in `BaseContext` already does `scope.launch { tracker(event) }` internally, so wrapping it in another `scope.launch { track(...) }` creates a needless nested coroutine. The outer launch can be dropped.

```suggestion
            @Suppress("UNCHECKED_CAST")
            track(InternalSuperwallEvent.DeviceAttributes(attributes as HashMap<String, Any>))
```

### Issue 4 of 4
superwall/src/main/java/com/superwall/sdk/config/models/ConfigState.kt:308-312
**`identityManager` lambda invoked twice**

`identityManager?.invoke()` is called separately to read `appUserId` and `aliasId`. While the lambda is expected to return the same instance each time, invoking it twice is unnecessary. Capturing the result once avoids the duplicate call.

```suggestion
            val im = identityManager?.invoke()
            manager?.evaluateTestMode(
                config = config,
                bundleId = deviceHelper.bundleId,
                appUserId = im?.appUserId,
                aliasId = im?.aliasId,
                testModeBehavior = options.testModeBehavior,
            )
```

_{Reviews (2): Last reviewed commit: "Merge branch 'develop' into ir/refactor/..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[ianrumac]

@greptileai