feat: Add Workspace IDSync search on user identification#92
Open
rmi22186 wants to merge 7 commits intodevelopmentfrom
Open
feat: Add Workspace IDSync search on user identification#92rmi22186 wants to merge 7 commits intodevelopmentfrom
rmi22186 wants to merge 7 commits intodevelopmentfrom
Conversation
242f6c1 to
5bc5c16
Compare
When a kit setting `advertiserIdSyncApiKey` is provided, the kit calls
mParticle.Identity.searchAdvertiser(apiKey, { email }, callback) from
onUserIdentified. On a 200 response the kit sets the local user
attribute `userIdentifiedInAdvertiser = true` so downstream placement
selection can target users whose identity is matched in the advertiser
workspace.
The setting is a string (the advertiser workspace's API key) rather
than a boolean — this lets a single SDK send searches against an
arbitrary advertiser workspace without needing the SDK's own workspace
to be reconfigured.
Missing key, missing/invalid email, and a missing
mParticle.Identity.searchAdvertiser implementation are all silently
inert (no network call, no attribute set).
Pairs with the corresponding mParticle Web SDK change that exposes
mParticle.Identity.searchAdvertiser. Note: a Fastly CORS allow-list
update is required separately for the x-mp-key header on /v1/search.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts:
# src/Rokt-Kit.ts
5bc5c16 to
7439c4a
Compare
When `searchAdvertiser` returned 200 we were writing the flag into `this.userAttributes` directly. That races with `handleIdentityComplete`, which runs synchronously after `searchAdvertiser` inside the same `onUserIdentified` flow and reassigns `userAttributes` wholesale via `user.getAllUserAttributes()` — wiping the flag we just set. Subsequent identity events or `setUserAttribute` calls hit the same window. Move the flag to a dedicated public field on the kit (`userIdentifiedInAdvertiser`) so it survives any reassignment of the attribute map. `selectPlacements` merges it back into the placement attributes payload using the same `userIdentifiedInAdvertiser` key on the wire — no consumer-visible behaviour change. Tests updated: - Existing assertions migrated from `userAttributes.userIdentifiedInAdvertiser` to `forwarder.userIdentifiedInAdvertiser`. - New regression test `should preserve the flag when handleIdentityComplete reassigns userAttributes` provides a `getAllUserAttributes()` mock that omits the flag and asserts the kit-class flag survives, that the attribute map deliberately does not contain the flag, and that other user attributes still flow through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two bugs flagged in remote review, plus a follow-up efficiency: 1. Race condition between searchAdvertiser and selectPlacements (ultrareview bug_005). onUserIdentified fired searchAdvertiser synchronously and returned before the HTTP response landed. Partners doing the canonical `Identity.login(...).then(() => Rokt.selectPlacements(...))` flow read userIdentifiedInAdvertiser before it could be set, missing the flag for the most important placement call. Fix: searchAdvertiser now returns a Promise<void> that resolves when the SDK callback fires (or the short-circuit returns immediately). The latest in-flight Promise is stored on the kit. selectPlacements gates the existing dispatch logic on `Promise.race([inFlight, setTimeout(ADVERTISER_SEARCH_SELECT_TIMEOUT_MS)])` so the first placement call waits up to 1s for a real answer rather than racing the network. Stalled searches don't block placement rendering past the timeout. 2. Sticky userIdentifiedInAdvertiser flag (ultrareview bug_001). The flag was set true on a 200 but never reset, so logout sessions and short-circuit re-identifications carried the previous user's match forward — a privacy/correctness issue. Fix: reset the flag at every short-circuit path inside searchAdvertiser, and reset it explicitly in onLogoutComplete (which has no searchAdvertiser path of its own). 3. Cache by email so the same identity doesn't trigger redundant network calls. mParticle fires onUserIdentified on every identify/login/logout/modify, so the same email could trigger multiple /v1/search dispatches per session. Now searchAdvertiser caches the last successfully-dispatched email; if a subsequent identification arrives with the same email, we skip the network call and keep the existing flag. The cache is cleared on logout (so re-login dispatches fresh, avoiding stale answers across sessions) and on every short-circuit path. selectPlacements stays non-async (returns the existing RoktSelection | Promise<RoktSelection> | undefined union) because RoktSelection has an optional `then?` member which TS1058 rejects inside an async function's return type. The dispatch body moved to a private _dispatchPlacements helper so the wrapper can conditionally await the in-flight search. Tests: - should wait for an in-flight searchAdvertiser before selectPlacements builds attributes — race regression for bug_005, asserts the flag lands on the launcher's attributes payload after a deferred callback. - should reset userIdentifiedInAdvertiser on onLogoutComplete — bug_001 logout case. - should reset userIdentifiedInAdvertiser when re-identifying via a short-circuit path — bug_001 short-circuit case (new user with no email). - should not re-call Identity.searchAdvertiser when the same email re-identifies — verifies the email cache. - should re-call Identity.searchAdvertiser when the email changes — verifies cache invalidation on different email. - should re-call Identity.searchAdvertiser after logout even with the same email — verifies the logout cache clear. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two leaks in the email cache added in the previous IDSync fix: 1. searchAdvertiser set _advertiserLastSearchedEmail before calling the SDK. If the SDK threw synchronously, the email was cached as if it had been successfully dispatched — every subsequent identification with the same email short-circuited and the flag could never be re-evaluated. Clear the cache in the catch block so a failed dispatch does not poison future searches. 2. init reconfigures the kit (different api key, partner, session), but the advertiser search state was carried forward from the previous lifecycle. Reset userIdentifiedInAdvertiser, _advertiserSearchInFlight, and _advertiserLastSearchedEmail in init alongside the api-key assignment. The CI failure on PR #92 surfaced both: the "swallow errors" test poisoned the cache, then the "preserve the flag", "wait for in-flight", and reset/dedupe tests all read through that stale cache and saw the flag stuck at false. With this fix all 199 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "advertiser" terminology is Rokt-internal. The core mParticle SDK is renaming `Identity.searchAdvertiser` to `Identity.searchWorkspace` so the public surface reads naturally for any mParticle customer, not just Rokt — propagate the rename here. Kit changes (pre-release, no consumers yet): - `advertiserIdSyncApiKey` setting -> `workspaceIdSyncApiKey` - `userIdentifiedInAdvertiser` flag -> `userIdentifiedInWorkspace` - `USER_IDENTIFIED_IN_ADVERTISER_KEY` const -> `USER_IDENTIFIED_IN_WORKSPACE_KEY` - `AdvertiserIdSyncResult`/`AdvertiserIdSyncSearcher` types -> `WorkspaceIdSync*` - Private `searchAdvertiser` method + state fields renamed to `searchWorkspace`/`_workspace*` - Now calls `mp().Identity.searchWorkspace` (paired core-SDK rename). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The core SDK renamed the IDSync search primitive from `Identity.searchWorkspace` to `Identity.search` so it sits cleanly alongside `identify`/`login`/`logout`/`modify`/`aliasUsers`. Update the kit's call site, type declaration, private wrapper method, and test mocks in lockstep. The kit-facing surface (`workspaceIdSyncApiKey` setting, `userIdentifiedInWorkspace` flag, `WorkspaceIdSync*` types) is unchanged — those describe what the kit *does* with the primitive, not the primitive itself. Paired with the core SDK rename in mparticle-web-sdk. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alexs-mparticle
requested changes
Apr 30, 2026
| public launcher: RoktLauncher | null = null; | ||
| public filters: KitFilters = {}; | ||
| public userAttributes: Record<string, unknown> = {}; | ||
| // Flag set by the Workspace IDSync flow on a 200 response. Stored on the |
Collaborator
There was a problem hiding this comment.
Can we put a space between this comment and the previous line so it reads cleaner?
- Rename `_workspaceSearchInFlight` -> `_workspaceSearchInFlightPromise` so the field name signals its Promise type rather than reading like a boolean (r3169510786). - Add a blank line before the `WORKSPACE_SEARCH_SELECT_TIMEOUT_MS` comment block (r3169513331) and before the `_workspaceSearchInFlightPromise` comment block (r3169514112) so each comment reads as introducing the next declaration. - Add TODO breadcrumbs on `WorkspaceIdSyncResult` and `WorkspaceIdSyncSearcher` noting that they're structurally identical to `IIdentitySearchResult` / `SDKIdentityApi.search` from `@mparticle/web-sdk` and should be replaced with the imported versions once a core SDK release including those types is published. - Tighten the `selectPlacements` JSDoc to spell out the two reasons it stays non-async: the public union return is a superset of `RoktLauncher.selectPlacements`, and `RoktSelection`'s optional `then?` member trips TS1058 inside an async return position (r3169525266). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alexs-mparticle
approved these changes
Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
workspaceIdSyncApiKey(a workspace's mParticle API key). When set, the kit callsmParticle.Identity.search(apiKey, { email }, callback)fromonUserIdentifiedand, on a 200, sets a kit-local flaguserIdentifiedInWorkspace = truethat flows into the nextselectPlacementscall.getUserIdentities().userIdentities.emailmissing or non-string → no call;Identity.searchnot available on the SDK → graceful no-op; a throw is swallowed and logged; 404 or any non-200 leaves the flag unset.mParticle.Identity.search) — and depends on the Fastly/v1/searchCORS allowlist being updated to includex-mp-key. The origin server already returns 200/404 via curl; only the browser preflight is blocked today.npm run lint+npm run buildclean (IIFE + CJS + ESM + type defs);npm testcovers the 200 success path, 404 no-op, missing/empty api key, missing email identity, missing SDK method, and swallowed throws. Live browser smoke test still blocked on the Fastly allowlist update.🤖 Generated with Claude Code