Replace the old dispatcher+queue IdentityManager with a Actor

[ianrumac]

• IdentityState: immutable data class with pure reducers (Updates) and async side-effecting actions (Actions) via IdentityContext

  • IdentityPersistenceInterceptor: diff-based storage writes on state change
  • SdkContext: cross-slice bridge so identity can call ConfigManager without a direct dependency
  • Reset flow gates identity readiness during cleanup, matching iOS
  • mergeAttributes aligned with iOS (no null filtering in nested collections)
  • Startup repair widened to handle empty stored attributes
  • Tracking/notify paths use enrichedAttributes consistently

  Includes pure reducer unit tests and SequentialActor integration tests
  covering concurrency, rapid identify/reset interleaving, and persistence.

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 replaces the old single-threaded-executor + runBlocking IdentityManager with a clean actor-model implementation: an immutable IdentityState data class, pure Updates reducers, async Actions, a SequentialActor backed by a Mutex, and a diff-based IdentityPersistenceInterceptor. The reset flow is now gated on identity readiness (matching iOS), and mergeAttributes drops null-filtering from nested collections for iOS parity.

Key changes:

• IdentityState + sealed Updates/Actions replace all mutable fields and ad-hoc coroutine launches in the old manager
• SequentialActor with re-entrant mutex serializes all identity transitions without blocking threads
• IdentityPersistenceInterceptor performs diff-based storage writes, eliminating redundant I/O
• SdkContext bridge decouples the identity slice from ConfigManager directly
• FullReset gates paywall presentation (Pending → Ready) around the cleanup window
• Solid test coverage: pure reducer unit tests + SequentialActor integration tests for concurrency and persistence

Issues found:

• IdentityManager.kt line 46: override val track: suspend (Trackable) -> Unit = { trackEvent(it as TrackableSuperwallEvent) } — the interface declares Trackable but the implementation hard-casts to TrackableSuperwallEvent. This will throw ClassCastException at runtime if the contract is ever exercised with a plain Trackable.
• Integration tests use Thread.sleep() inside runTest to wait for fire-and-forget effect actions; this creates timing-sensitive tests that can flake on slow CI.
• The comment in Identify's user-switch branch ("so storage.reset() doesn't wipe the new IDs") is factually incorrect — LocalStorage.reset() only clears confirmedAssignments/didTrackFirstSeen, not identity fields.

Confidence Score: 4/5

• Safe to merge after fixing the unsafe Trackable cast in IdentityManager; all other findings are non-blocking style improvements.
• The architectural refactor is well-executed — sequential serialization, pure reducers, diff-based persistence, and iOS-parity reset gating are all correct. One P1 issue remains: the it as TrackableSuperwallEvent cast in IdentityManager.track silently violates the Trackable contract and will throw at runtime if the abstraction is exercised as declared. All current callers pass InternalSuperwallEvent (a TrackableSuperwallEvent) so it works today, but it is a latent bug one refactor away from surfacing. The P2 findings (flaky sleep-based tests, misleading comment) are clean-up items that don't block merge.
• superwall/src/main/java/com/superwall/sdk/identity/IdentityManager.kt — the unsafe cast on line 46 needs to be resolved before merge.

Important Files Changed

Filename	Overview
superwall/src/main/java/com/superwall/sdk/identity/IdentityManagerActor.kt	New file defining IdentityState (immutable data class), pure Updates reducers, and async Actions. Well-structured; the misleading comment in Identify's user-switch branch about storage.reset() wiping identity fields is the only notable issue.
superwall/src/main/java/com/superwall/sdk/identity/IdentityManager.kt	Heavily refactored from ~340 lines to ~120 lines as a thin facade over the actor. Contains a P1 unsafe cast: override val track: suspend (Trackable) -> Unit = { trackEvent(it as TrackableSuperwallEvent) } — will throw ClassCastException if a non-TrackableSuperwallEvent Trackable is ever dispatched.
superwall/src/main/java/com/superwall/sdk/misc/primitives/ActorTypes.kt	SequentialActor serializes all actions via a Mutex with correct re-entrancy detection using CoroutineContext elements. Clean and correct implementation.
superwall/src/main/java/com/superwall/sdk/misc/primitives/StateActor.kt	Core actor with pluggable interceptor chains for updates and actions. Interceptor ordering is additive (each call prepends), which is correct. immediateUntil uses StateFlow.first after firing the action — safe since StateFlow.first checks the current value before suspending.
superwall/src/main/java/com/superwall/sdk/identity/IdentityPersistenceInterceptor.kt	Diff-based storage interceptor — only writes fields that actually changed. Correctly handles AppUserId null (delete vs write). Works safely under SequentialActor's mutex since no concurrent updates can occur between reading before and after.
superwall/src/main/java/com/superwall/sdk/Superwall.kt	Updated reset dispatch: public reset now delegates entirely to the identity actor (identityManager.reset()), while duringIdentify=true performs only the non-identity cleanup. Correctly gates paywall presentation during the reset window, matching iOS behavior.
superwall/src/main/java/com/superwall/sdk/identity/IdentityLogic.kt	Simplified mergeAttributes — removed null-filtering from nested List/Map values to align with iOS. Intentional behavior change; consumers must tolerate nulls in nested collections.
superwall/src/test/java/com/superwall/sdk/identity/IdentityActorIntegrationTest.kt	Good coverage of concurrency, rapid identify/reset interleaving, and persistence behavior. The use of Thread.sleep() inside runTest blocks for timing synchronization makes tests fragile on slow CI.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant IdentityManager
    participant SequentialActor
    participant Action
    participant PersistenceInterceptor
    participant Storage
    participant SdkContext

    Note over SequentialActor: Mutex serializes all actions

    Caller->>IdentityManager: configure(neverCalledStaticConfig)
    IdentityManager->>SequentialActor: effect(Actions.Configure)
    SequentialActor->>Action: executeAction (mutex acquired)
    Action->>PersistenceInterceptor: update(Updates.Configure)
    PersistenceInterceptor->>Storage: write only changed fields
    Action->>SdkContext: fetchAssignments() [if needed]
    Action->>PersistenceInterceptor: update(Updates.AssignmentsCompleted)
    Note over SequentialActor: phase → Ready

    Caller->>IdentityManager: identify(userId)
    IdentityManager->>SequentialActor: effect(Actions.Identify)
    SequentialActor->>Action: executeAction (waits for mutex)
    alt switching users
        Action->>Caller: completeReset() [storage/config/paywall cleanup]
        Action->>SequentialActor: immediate(Reset) [re-entrant, skips mutex]
    end
    Action->>PersistenceInterceptor: update(Updates.Identify)
    PersistenceInterceptor->>Storage: write appUserId, attributes
    Action->>SequentialActor: immediate(IdentityChanged) [re-entrant]
    SequentialActor->>Action: effect(ResolveSeed) [queued, fire-and-forget]
    SequentialActor->>Action: effect(FetchAssignments) [queued]

    Caller->>IdentityManager: reset()
    IdentityManager->>SequentialActor: effect(Actions.FullReset)
    SequentialActor->>Action: executeAction (waits for mutex)
    Action->>PersistenceInterceptor: update(Updates.Reset) [phase=Pending, new aliasId/seed]
    PersistenceInterceptor->>Storage: write aliasId, seed, clear appUserId
    Action->>Caller: completeReset() [storage/config/paywall cleanup]
    Action->>PersistenceInterceptor: update(Updates.ResetComplete)
    Note over SequentialActor: phase → Ready

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/identity/IdentityManager.kt
Line: 46

Comment:
**Unsafe cast violates `Trackable` contract**

The property is declared as `suspend (Trackable) -> Unit` in `IdentityContext`, but the implementation forcibly casts every `Trackable` to `TrackableSuperwallEvent`. All current callers in `IdentityManagerActor.kt` happen to pass `InternalSuperwallEvent` (which extends `TrackableSuperwallEvent`), so this works today — but if any future action dispatches a `Trackable` that isn't a `TrackableSuperwallEvent`, this will throw a `ClassCastException` at runtime with no compile-time warning.

The safest fix is to narrow the interface property type so the contract matches the implementation:

```suggestion
    override val track: suspend (TrackableSuperwallEvent) -> Unit = { trackEvent(it) }
```

Alternatively, update `IdentityContext.track` to `suspend (TrackableSuperwallEvent) -> Unit` consistently.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: superwall/src/test/java/com/superwall/sdk/identity/IdentityActorIntegrationTest.kt
Line: 181

Comment:
**`Thread.sleep` in `runTest` blocks creates flaky tests**

Several tests use `Thread.sleep(200)` / `Thread.sleep(500)` inside `runTest` to wait for fire-and-forget `effect` actions to process through the `SequentialActor`. This pattern is timing-dependent and can produce false positives on slow CI machines or under load (e.g. the 200 ms budget may not be enough for the full mutex queue to drain).

The same pattern appears at lines 222, 295, 309, 312, and 357.

A more deterministic approach would be to expose a test hook on `SequentialActor` (e.g. `suspend fun awaitIdle()` that waits until the internal queue is empty), or to make the integration-test actor configurable with `UnconfinedTestDispatcher` so `advanceUntilIdle()` from `kotlinx.coroutines.test` can drain all pending work without real-time sleeps.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/identity/IdentityManagerActor.kt
Line: 232-237

Comment:
**Misleading comment about `storage.reset()` wiping identity fields**

The comment says "reset other managers BEFORE updating state so storage.reset() doesn't wipe the new IDs". However, `LocalStorage.reset()` only clears `confirmedAssignments` and `didTrackFirstSeen` — it does **not** touch `AliasId`, `AppUserId`, `Seed`, or `UserAttributes`. The ordering concern described in the comment does not apply.

Contrast this with `FullReset`, which calls `update(Updates.Reset)` first (writing new IDs via the persistence interceptor) and then calls `completeReset()`. The inconsistent ordering between the two code paths is harmless in practice, but the misleading comment could cause future contributors to draw incorrect conclusions when modifying either flow.

Consider updating the comment to accurately describe *why* `completeReset()` is called before the state update (e.g. to ensure other managers are torn down before the new identity takes effect).

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Replace the old dispatcher+queue Identit..." | Re-trigger Greptile}

Greptile also left 3 inline comments on this PR.