fix: SDKE-846 - User Identity through selectPlacements #1146

alexs-mparticle · 2026-01-20T19:50:27Z

Background

A race condition prevented email/emailsha256 from being propagated to selectPlacements when:

An identify call was made during SDK initialization with email in userIdentities (asynchronous)
A selectPlacements call was made immediately after, before identify completed
Email was not explicitly passed in the selectPlacements attributes

selectPlacements only used attributes passed in the call and did not pull email/emailsha256 from current user identities, so selection calls could run without email even after identify was called.

What Has Changed

Automatically propagate email and emailsha256 from current user identities to enrichedAttributes in selectPlacements when not already present in the passed attributes
Refresh current user identities after identify completes to ensure latest values are available
Added test cases to verify email/emailsha256 propagation and race condition handling

Screenshots/Video

{Include any screenshots or video demonstrating the new feature or fix, if applicable}

Checklist

I have performed a self-review of my own code.
I have made corresponding changes to the documentation.
I have added tests that prove my fix is effective or that my feature works.
I have tested this locally.

Additional Notes

{Any additional information or context relevant to this PR}

Reference Issue (For employees only. Ignore if you are an outside contributor)

Closes https://go/j/SDKE-846

Copilot

Pull request overview

This PR fixes a race condition where email/emailsha256 identities were not properly propagated to selectPlacements calls when an asynchronous identify() call was made during SDK initialization. The fix ensures that current user identities are refreshed and automatically propagated to placement attributes when they're not explicitly provided.

Changes:

Added logic to refresh user identities and automatically propagate email/emailsha256 from current user identities to selectPlacements attributes when not already present
Added comprehensive test coverage for email/emailsha256 propagation, race condition handling, and ensuring explicit attributes are not overridden

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/roktManager.ts	Added user identity refresh logic and automatic email/emailsha256 propagation to enrichedAttributes in selectPlacements method
test/jest/roktManager.spec.ts	Added 4 new test cases covering email propagation, emailsha256 propagation, race condition scenarios, and explicit attribute preservation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/roktManager.ts

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/roktManager.ts

jaissica12

Small nit: suggestion that could simplify this a bit

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/roktManager.ts

test/jest/roktManager.spec.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/roktManager.ts

Copilot · 2026-01-21T18:55:45Z

test/jest/roktManager.spec.ts

+        it('should propagate email after identify completes when identify is called during init', async () => {
+            const kit: Partial<IRoktKit> = {
+                launcher: {
+                    selectPlacements: jest.fn(),
+                    hashAttributes: jest.fn(),
+                    use: jest.fn(),
+                },
+                selectPlacements: jest.fn().mockResolvedValue({}),
+                hashAttributes: jest.fn(),
+                setExtensionData: jest.fn(),
+            };
+
+            roktManager['placementAttributesMapping'] = [];
+            roktManager.kit = kit as IRoktKit;
+
+            let identifyCallback: (() => void) | null = null;
+            let identifyResolve: (() => void) | null = null;
+            const identifyPromise = new Promise<void>((resolve) => {
+                identifyResolve = resolve;
+            });
+
+            const mockIdentity = {
+                getCurrentUser: jest.fn()
+                    .mockReturnValueOnce({
+                        getUserIdentities: () => ({
+                            userIdentities: {}
+                        }),
+                        setUserAttributes: jest.fn()
+                    })
+                    .mockReturnValueOnce({
+                        getUserIdentities: () => ({
+                            userIdentities: {
+                                email: 'user@example.com'
+                            }
+                        }),
+                        setUserAttributes: jest.fn()
+                    }),
+                identify: jest.fn().mockImplementation((data, callback) => {
+                    identifyCallback = callback;
+                    setTimeout(() => {
+                        if (identifyCallback) {
+                            identifyCallback();
+                        }
+                        if (identifyResolve) {
+                            identifyResolve();
+                        }
+                    }, 10);
+                })
+            } as unknown as SDKIdentityApi;
+
+            roktManager['identityService'] = mockIdentity;
+
+            mockIdentity.identify({
+                userIdentities: {
+                    email: 'user@example.com'
+                }
+            }, () => {});
+
+            const options: IRoktSelectPlacementsOptions = {
+                attributes: {
+                    firstname: 'John'
+                }
+            };
+
+            const selectPlacementsPromise = roktManager.selectPlacements(options);
+
+            await identifyPromise;
+            await selectPlacementsPromise;
+
+            expect(kit.selectPlacements).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    attributes: expect.objectContaining({
+                        email: 'user@example.com',
+                        firstname: 'John'
+                    })
+                })
+            );
+        });


The test 'should propagate email after identify completes when identify is called during init' doesn't accurately test the race condition scenario described in the PR. The test calls identify directly (line 1699) and then selectPlacements (line 1711), but doesn't set up the store.identityCallInFlight flag. This means the wait logic in selectPlacements (lines 193-199) is never triggered. To properly test the race condition fix, the test should set up store.identityCallInFlight to true before calling selectPlacements, similar to the test on line 1929.

src/roktManager.ts

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/roktManager.ts

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-21T21:56:35Z

test/jest/roktManager.spec.ts

+            const startTime = Date.now();
+            const selectPlacementsPromise = roktManager.selectPlacements(options);
+
+            // Wait for both identity to complete and selectPlacements to finish
+            await Promise.all([identityCompletePromise, identifyPromise, selectPlacementsPromise]);
+            const elapsedTime = Date.now() - startTime;
+
+            // Verify that selectPlacements waited for the identity call to complete
+            expect(elapsedTime).toBeGreaterThanOrEqual(100);
+            expect(elapsedTime).toBeLessThan(500);
+


The timing assertions here are fragile and may cause test flakiness in CI environments or under load. The test expects completion between 100-500ms, but this assumes the setTimeout and polling intervals execute predictably. Consider either removing these timing assertions entirely (the functional behavior is already verified by checking the email was propagated), or using Jest's fake timers to control time explicitly.

Suggested change

const startTime = Date.now();

const selectPlacementsPromise = roktManager.selectPlacements(options);

// Wait for both identity to complete and selectPlacements to finish

await Promise.all([identityCompletePromise, identifyPromise, selectPlacementsPromise]);

const elapsedTime = Date.now() - startTime;

// Verify that selectPlacements waited for the identity call to complete

expect(elapsedTime).toBeGreaterThanOrEqual(100);

expect(elapsedTime).toBeLessThan(500);

const selectPlacementsPromise = roktManager.selectPlacements(options);

// Wait for both identity to complete and selectPlacements to finish

await Promise.all([identityCompletePromise, identifyPromise, selectPlacementsPromise]);

Copilot · 2026-01-21T21:56:36Z

test/jest/roktManager.spec.ts

+            const elapsedTime = Date.now() - startTime;
+            // Should have waited until identity call completed (allowing for polling interval variance)
+            // With 50ms polling interval, it should complete around 150-200ms
+            expect(elapsedTime).toBeGreaterThanOrEqual(100);
+            expect(elapsedTime).toBeLessThan(500); // Should complete well before max wait time


Similar to the previous test, these timing assertions (lines 2002-2006) are fragile and may cause test flakiness. The test already verifies the functional behavior (that selectPlacements waited for identity completion), so the timing assertions add risk without much value. Consider using Jest's fake timers or removing these timing checks.

Copilot · 2026-01-21T21:56:36Z

src/roktManager.ts

+                while (this.store?.identityCallInFlight && (Date.now() - startTime) < RoktManager.IDENTITY_CALL_MAX_WAIT_MS) {
+                    await new Promise(resolve => setTimeout(resolve, RoktManager.IDENTITY_CALL_POLL_INTERVAL_MS));
+                }


The polling implementation uses a busy-wait pattern that could be inefficient if the store check is expensive. While the 50ms interval mitigates this, consider whether the identityCallInFlight flag update triggers any observable event that could be used instead of polling. However, if polling is the only option, the current implementation with a reasonable interval and max timeout is acceptable.

Copilot · 2026-01-21T21:56:36Z

test/jest/roktManager.spec.ts

+        });
+
+        it('should wait briefly when identity call is in flight', async () => {
+            jest.setTimeout(10000); // Increase timeout for this test


Setting jest.setTimeout(10000) at the test level affects the timeout for this specific test, but this approach is deprecated in newer versions of Jest. Consider using the third parameter of the it() function instead: it('test name', async () => {...}, 10000). However, since the test includes timing assertions that wait for ~150ms, the default Jest timeout should be sufficient and this line may be unnecessary.

Suggested change

jest.setTimeout(10000); // Increase timeout for this test

@alexs-mparticle I believe we can listen to the bot here and clean up

sonarqubecloud · 2026-01-22T03:38:27Z

Quality Gate failed

Failed conditions
19.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

alexs-mparticle marked this pull request as ready for review January 20, 2026 20:16

alexs-mparticle requested review from Copilot, jaissica12 and rmi22186 January 20, 2026 20:16

Copilot started reviewing on behalf of alexs-mparticle January 20, 2026 20:16 View session

alexs-mparticle changed the base branch from master to development January 20, 2026 20:18

Copilot AI reviewed Jan 20, 2026

View reviewed changes

src/roktManager.ts Outdated Show resolved Hide resolved

src/roktManager.ts Outdated Show resolved Hide resolved

src/roktManager.ts Outdated Show resolved Hide resolved

src/roktManager.ts Outdated Show resolved Hide resolved

fix: SDKE-846 - User Identity through selectPlacements

0297078

alexs-mparticle force-pushed the fix/SDKE-846-identity-select-placements branch from 2790e95 to 0297078 Compare January 20, 2026 20:20

alexs-mparticle added 2 commits January 20, 2026 15:24

refactor: Update type for enrichedAttributes in RoktManager

6399236

Address PR Comments

db42d97

alexs-mparticle requested a review from Copilot January 20, 2026 20:30

Copilot started reviewing on behalf of alexs-mparticle January 20, 2026 20:30 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

src/roktManager.ts Outdated Show resolved Hide resolved

src/roktManager.ts Outdated Show resolved Hide resolved

jaissica12 reviewed Jan 20, 2026

View reviewed changes

src/roktManager.ts Outdated Show resolved Hide resolved

jaissica12 reviewed Jan 20, 2026

View reviewed changes

src/roktManager.ts Outdated Show resolved Hide resolved

jaissica12 requested changes Jan 20, 2026

View reviewed changes

alexs-mparticle added 2 commits January 21, 2026 10:06

Address PR Comments

5fc780c

Add tests cases for anonymous users and updates to filtered user

03921ca

alexs-mparticle requested review from Copilot and jaissica12 January 21, 2026 18:24

Copilot started reviewing on behalf of alexs-mparticle January 21, 2026 18:24 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

alexs-mparticle and others added 2 commits January 21, 2026 13:37

Update test/jest/roktManager.spec.ts

65e95e0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Address PR Comments

c508f99

alexs-mparticle requested a review from Copilot January 21, 2026 18:50

Copilot started reviewing on behalf of alexs-mparticle January 21, 2026 18:51 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

alexs-mparticle added 2 commits January 21, 2026 13:58

Address PR Comments

7490439

Address PR Comments

488c5cf

alexs-mparticle requested a review from Copilot January 21, 2026 19:43

Copilot started reviewing on behalf of alexs-mparticle January 21, 2026 19:44 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

src/roktManager.ts Outdated Show resolved Hide resolved

alexs-mparticle added 2 commits January 21, 2026 16:46

Address PR Comments

0deebed

Address PR Comments

5b15628

alexs-mparticle requested a review from Copilot January 21, 2026 21:52

Copilot started reviewing on behalf of alexs-mparticle January 21, 2026 21:53 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

jaissica12 approved these changes Jan 21, 2026

View reviewed changes

refactor: Send signal to Rokt Manager when identity completes

d4ed4b9

fix: SDKE-846 - User Identity through selectPlacements #1146

Are you sure you want to change the base?

fix: SDKE-846 - User Identity through selectPlacements #1146

Conversation

alexs-mparticle commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

What Has Changed

Screenshots/Video

Checklist

Additional Notes

Reference Issue (For employees only. Ignore if you are an outside contributor)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaissica12 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

jaissica12 Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

alexs-mparticle commented Jan 20, 2026 •

edited

Loading