fix: identify FullStory with freshest user metadata to prevent stale overwrite (new-account session unsearchable) - #96849
Conversation
…overwrite For a newly created account USER_METADATA arrives in two stages (accountID first, then accountID + email), so consentAndIdentify spawns two racing async chains. The email-less chain can resolve last (web: onReady init path resolves on a 1s setTimeout fallback) and re-identify the user without an email, making the session unsearchable in FullStory. Keep a module-level latestUserMetadata reference and read it at resolve time, so whichever chain resolves last identifies with the most complete metadata. Also stop mutating the Onyx-held metadata object by building a fresh one. Fixes: Expensify#93730 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Simulate the new-account flow where an email-less identification chain resolves after the chain that carries the email, and assert the final identify call still includes the email on both web and native. Also assert the Onyx-held metadata object is no longer mutated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ullstory-identify-race # Conflicts: # src/libs/Fullstory/index.native.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 226c20ac34
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| FS.identify(localMetadata); | ||
| // Identify with the freshest metadata rather than the value captured when this chain | ||
| // started, so an email-less chain that resolves late does not clobber the identity. | ||
| FS.identify({...latestUserMetadata, environment: envName}); |
There was a problem hiding this comment.
Because the FullStory eligibility check above still uses the metadata captured when this async chain started, this line can identify a different latestUserMetadata object that would fail the gate itself. If an eligible account starts initialization and the user then switches to an ineligible account (for example an expensify.com/support or non-production non-test user) while the old getEnvironment/onReady promise is pending, the stale callback can consent/restart FullStory and identify the ineligible latest account; use the same fresh metadata for shouldInitialize or drop callbacks whose accountID is no longer current.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in the latest commit. The eligibility gate now reads the same freshest latestUserMetadata snapshot that identify uses, so the gate and the identity can no longer diverge. On web, since identify runs behind a second async hop (onReady), I re-read and re-check eligibility inside onReady — if the current user switched to an ineligible account (support / non-production) while onReady was pending, FS is shut down instead of identifying it. Native gates and identifies from a single synchronous read, so it stays consistent by construction. Added a unit test covering the eligible→ineligible account-switch case.
| FS.identify(userMetadata, envName); | ||
| // Identify with the freshest metadata rather than the value captured when this chain | ||
| // started, so an email-less chain that resolves late does not clobber the identity. | ||
| FS.identify(latestUserMetadata, envName); |
There was a problem hiding this comment.
This identifies the module-level latest metadata even though the shouldInitialize decision was made with the older metadata captured by this callback. During an account switch from an eligible account to an ineligible one (such as an expensify.com account or a non-production non-test account) while the older environment lookup is still pending, this stale callback can call restart/consent, identify the ineligible latest account, and set native tracking enabled; use the same current metadata for the gate or ignore outdated callbacks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in the latest commit. The eligibility gate now reads the same freshest latestUserMetadata snapshot that identify uses, so the gate and the identity can no longer diverge. On web, since identify runs behind a second async hop (onReady), I re-read and re-check eligibility inside onReady — if the current user switched to an ineligible account (support / non-production) while onReady was pending, FS is shut down instead of identifying it. Native gates and identifies from a single synchronous read, so it stays consistent by construction. Added a unit test covering the eligible→ineligible account-switch case.
Codex review flagged that shouldInitialize was checked against the metadata captured when the async chain started, while identify used the module-level latestUserMetadata. On an account switch from an eligible to an ineligible account mid-flight, a stale chain could pass the gate yet identify the ineligible account. Gate and identify now read the same freshest snapshot; web re-checks eligibility inside onReady and shuts FS down if the current user became ineligible while it was pending. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Exercise the isInitialized()==true path so the SHUTDOWN call inside the onReady eligibility recheck is covered (fixes Codecov patch miss). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@daledah friendly bump on this one — it's been open since the 23rd and just waiting on your review. Let me know if anything needs clarifying on my end, happy to walk through the change. Thanks! |
|
🚧 MonilBhavsar has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Requested QA testing here: #93730 (comment) |
|
@dilshodmackbook-sketch please merge main |
…ullstory-identify-race
|
Done, merged main. |
|
@dilshodmackbook-sketch please resolve conflict |
Resolve Fullstory conflicts: combine the freshest-metadata race fix (latestUserMetadata re-read at resolve/onReady time) with main's new required session param on shouldInitialize/consentAndIdentify. Update FullstoryConsentAndIdentifyTest to pass the session argument. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@daledah Done |
4 participants
Explanation of Change
For a newly-created production account, the FullStory session is tagged with a
uidbut noemail, so searching by email in the FullStory dashboard returns "No matches found".The root cause is an out-of-order async identification race in
consentAndIdentify.USER_METADATAis backend-populated in two stages for a new account — first{accountID}, then{accountID, email}— and each update firesconsentAndIdentify, spawning two racing async chains:{accountID}, no email): FullStory is not yet initialized, soonReadyrunsinit()and resolves on the ~1ssetTimeoutfallback.{accountID, email}): FullStory is already initialized, soonReadyresolves immediately via theOBSERVEcallback.Because each chain captured its own metadata at start time and Chain A resolves last, its
FS.identifyre-identifies the user with the email-less metadata, clobbering the good identity set by Chain B. This is why the bug is specific to new accounts and reproduces on both web and native.This PR keeps a module-level
latestUserMetadatareference and reads it inside the deferredonReady/getEnvironmentcallback, so whichever chain resolves last identifies with the freshest, most complete metadata received so far. The same change is applied to bothsrc/libs/Fullstory/index.ts(web) andsrc/libs/Fullstory/index.native.ts(native). As a bonus, the identify path now builds a fresh object instead of mutating the Onyx-held metadata object.Fixed Issues
$ #93730
PROPOSAL: #93730 (comment)
Tests
Because the actual bug (session searchable by email in the FullStory dashboard) can only be observed on production/staging with FullStory + Applause dashboard access, the fix itself is verified locally by the unit test, and the app is separately checked for no regression.
1. Unit test (verifies the fix mechanism — runnable by anyone):
The web/native tests reproduce the two-stage ordering race (the email-less chain resolves last) and assert the final
identifycall still carriesproperties.email; the remaining tests cover the single-update case and that the Onyx-held metadata object is not mutated.2. No regression (manual — see Screenshots/Videos):
Offline tests
N/A — FullStory identification only runs while online, so there is no offline-specific behavior for this change.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-07-23.at.15.36.13.mov
Android: mWeb Chrome
93730-mweb-chrome.mp4
iOS: Native
Screen.Recording.2026-07-23.at.15.18.26.mov
iOS: mWeb Safari
93730-mweb-safari.mp4
MacOS: Chrome / Safari
93730-web-chrome.mp4