Add Privy integration for multi-wallet user identity clustering - #304
Add Privy integration for multi-wallet user identity clustering#304yosriady wants to merge 19 commits into
Conversation
…llets Make tagging every wallet linked to a Privy user under that user's DID a one-liner, so the 8-wallet case clusters into a single user end to end. - identifyPrivyUser(analytics, user, options?): new exported helper next to parsePrivyProperties. Loops the user's linked wallets and calls identify() for each, forwarding the previously-dropped per-wallet metadata (wallet_client, chain_type, is_embedded) alongside the shared profile properties, all tagged with userId: user.id. - Event attribution: only the active/connected wallet is promoted to the SDK's current address. identify() gains an optional setCurrentAddress flag (default true); non-active linked wallets are identified with it false so they no longer hijack which wallet later events attribute to. The active wallet is identified last; callers pass activeAddress (from useWallets()), otherwise the SDK falls back to embedded-first, attribute-last ordering. - Dedup: fold userId into the session identify key (address[:rdns][:userId]), so attaching a DID to a wallet that was already identified anonymously re-emits, while repeats with the same userId stay deduped. Backward compatible when userId is absent. - useIdentifyPrivyUser(user, options?): optional React binding that re-runs identifyPrivyUser when the Privy DID or linked-wallet set changes, covering login, linkWallet, and unlinkWallet. Exported from the React entry only so the core entry stays React-free. - Docs: new docs/PRIVY_INTEGRATION.md centered on the one-liner, with parsePrivyProperties kept documented for advanced use; README pointer added. - Tests: unit coverage for identifyPrivyUser ordering/metadata/attribution and userId-aware dedup, plus an end-to-end integration spec driving the real identify() (no hijack after a real connect; re-emit on DID attach). Verified against Privy docs: linked wallet addresses are available on the frontend via usePrivy() user.linkedAccounts; the active wallet comes from useWallets() (wallets[0]). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
…er defaults Follow-up to the identifyPrivyUser review. Tightens the attribution contract and the ergonomics; documents the deliberate scope boundaries. - setActive contract (was setCurrentAddress): the flag now gates the *whole* active identity — both currentAddress AND currentUserId/SESSION_USER_ID_KEY. Previously setCurrentAddress:false still let a non-active identify repoint the global user ID, so a caller could end up with the active wallet paired with a different, non-active user. Renamed to reflect it protects active attribution state, not just the address. - identifyPrivyUser: activeAddress now falls back to the SDK's existing currentAddress (e.g. from a prior wagmi connect) before the embedded-first heuristic, so callers that already track connects can drop the option. Exposed IFormoAnalytics.currentAddress (readonly) to support this. Walletless users are now logged instead of dropped silently. - Dedup storage: identified entries get their own cap (30, up from the shared 20), sized for many-wallet Privy users (up to ~2N entries across anon + DID) while staying under the ~4KB per-cookie limit; documented the FIFO-eviction consequence. - React hook + options: documented that options.properties is captured at first identify (session dedup means property-only changes don't re-emit) and that unlink is additive server-side (no unlink event). Extracted a shared isPrivyWalletAccount predicate so the hook's re-run key and parsePrivyProperties can't drift. - Tests: added integration coverage for the currentAddress attribution fallback and for setActive:false leaving the active (address, userId) pair untouched. Deferred (need a backend/event contract, out of scope for the one-liner): walletless userId-keyed identify, and an explicit wallet-unlink event. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
…by ordering Per review: revert the changes to the core identify() surface that the Privy helper doesn't strictly need, keeping the diff contained to the Privy module. - Removed the `setActive` parameter from identify() and the readonly `currentAddress` field from the IFormoAnalytics interface. The public identify() contract is now unchanged from main. - identifyPrivyUser now handles event attribution purely by ordering — it identifies the active/connected wallet last (embedded-first, then the active wallet, as a best-effort fallback), so identify()'s existing "last write wins" behavior lands attribution on the right wallet. This was the task's sanctioned approach and sidesteps the setActive contract concern entirely (there is no partial-attribution flag to get wrong). - Dropped the activeAddress -> currentAddress fallback (it required exposing currentAddress on the interface); callers pass activeAddress when they want precise attribution. - Reverted MAX_IDENTIFIED_ENTRIES back to the shared MAX_SESSION_ENTRIES (20); the dedup-cap bump was a general session-storage change, not required for the feature. Kept: the userId-in-session-dedup-key fix (required so attaching a DID to an already-identified wallet re-emits), the identifyPrivyUser/useIdentifyPrivyUser helpers, per-wallet metadata forwarding, and docs. Updated tests to assert ordering instead of the removed flag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
Replace the useIdentifyPrivyUser hook with a flag on identify() so a Privy app
needs only a single function: identify(user, { privy: true }).
- identify() gains an overload: identify(user, { privy: true, activeAddress?,
properties? }). When the { privy: true } flag is present, identify() treats
the first argument as a Privy user and delegates to identifyPrivyUser, which
expands user.linkedAccounts into one identify per wallet under the DID. The
Privy-specific logic stays in the privy module; core identify() only
dispatches. A normal identify({ address }, properties) is unaffected (the flag
lives in the options position, and an address-shaped first arg never matches).
- Removed the useIdentifyPrivyUser React hook and src/privy/react.ts. Apps call
formo.identify(user, { privy: true }) from their own effect (as the with-privy
example already does) instead of mounting a hook.
- identifyPrivyUser and parsePrivyProperties stay exported; the flag is sugar
over identifyPrivyUser.
- Docs rewritten around the single-call form; added integration tests that the
flag dispatches through the real identify() and that a normal identify with a
`privy` property is not misinterpreted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
…ptional
You can now call formo.identify(user, { privy: true }) with no address at all.
- When activeAddress is omitted, identifyPrivyUser now falls back to Privy's own
surfaced primary wallet (user.wallet) before the embedded-first heuristic, so
the caller doesn't need to derive or pass the active wallet.
- activeAddress stays as an optional override for callers who want to pin
attribution to a specific wallet (e.g. the live connected wallet from
useWallets()[0]?.address / wagmi account), which reflects the active wallet
more precisely than Privy's persisted user.wallet.
- Docs + example simplified to the no-argument form; added tests that user.wallet
is used as the default active wallet and that an explicit activeAddress
overrides it.
Note: Privy does not persist a live "active wallet" on the user object — that is
a runtime useWallets() concept the SDK can't see from outside React. user.wallet
is Privy's surfaced primary and a reasonable default; pass activeAddress when you
need the precise live wallet.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
|
@codex review |
![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: b3f8ad1b09
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| propertiesOrOptions && | ||
| (propertiesOrOptions as { privy?: unknown }).privy === true | ||
| ) { |
There was a problem hiding this comment.
Because IFormoEventProperties is Record<string, unknown>, a normal identify call can legitimately pass a property named privy. In that case, e.g. formo.identify({ address, userId }, { privy: true }), this guard takes the Privy branch, casts the address-shaped params to PrivyUser, and identifyPrivyUser finds no linkedAccounts, so the identify event is skipped entirely instead of recording the wallet/user. Please distinguish the overload by also validating that the first argument is a Privy user, or otherwise avoid using an un-namespaced property key as the dispatch signal.
Useful? React with 👍 / 👎.
| const activeAddress = ( | ||
| options.activeAddress ?? user.wallet?.address | ||
| )?.toLowerCase(); |
There was a problem hiding this comment.
When callers use the documented one-liner without activeAddress, this treats user.wallet as the active wallet and moves it to the final identify() call. In sessions where the SDK already tracks a connected external wallet but Privy's user.wallet is a different first/primary wallet (commonly the embedded wallet), the later identify overwrites currentAddress, so subsequent events are attributed to the wrong wallet unless every integration passes activeAddress. Prefer preserving the already-active SDK address when it is linked, or requiring/passing the connected wallet instead of defaulting to user.wallet.
Useful? React with 👍 / 👎.
| const hasActive = | ||
| !!activeAddress && | ||
| wallets.some((w) => w.address.toLowerCase() === activeAddress); |
There was a problem hiding this comment.
When a Privy user has Solana linked wallets and the caller passes an activeAddress that differs only by case (or links two Base58 addresses that collide after lowercasing), this case-insensitive match accepts the wrong wallet even though Solana addresses are case-sensitive; the later ordering can then move/drop the wrong entry and leave attribution on the wrong address. Compare EVM addresses case-insensitively only after detecting they are EVM, and keep Solana comparisons exact.
Useful? React with 👍 / 👎.
| return rdns ? `${address}:${rdns}` : address; | ||
| const parts = [address]; | ||
| if (rdns) parts.push(rdns); | ||
| if (userId) parts.push(userId); |
There was a problem hiding this comment.
With the new userId component included raw in the identified-wallet key, any app that uses an external userId containing a comma breaks session dedup: markWalletIdentified writes the key into a comma-joined cookie, and isWalletIdentified later splits on commas, so the exact key is never found and the same identify event re-emits on every call. Encode/escape the key components or store the list as JSON before adding arbitrary user IDs.
Useful? React with 👍 / 👎.
- identify() Privy dispatch: require the first argument to be Privy-user-shaped
(string `id`, no `address`), not just `{ privy: true }`. A normal identify
that happens to carry a property named `privy` no longer misdispatches and
drops the event. (Codex P2)
- identifyPrivyUser: prefer the SDK's already-active currentAddress (a connected
wallet) over Privy's user.wallet when resolving attribution, so the
multi-wallet identify loop doesn't overwrite the connected wallet. The flag
form passes this.currentAddress through as the default. (Codex P2)
- identifyPrivyUser: compare wallet addresses with chain-appropriate casing —
case-insensitive for EVM (0x hex), exact for Solana/Base58 — so a mis-cased
Solana address can't match the wrong wallet. (Codex P2)
- session dedup: percent-encode key components before the comma-joined cookie,
so an external userId containing a comma can't corrupt the key and defeat
dedup. Address/RDNS encode to themselves, so existing keys still match. (Codex P3)
Added tests for each; 673 passing, lint + build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed0006a3fe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await analytics.identify( | ||
| { address: wallet.address, userId: user.id }, | ||
| walletProperties | ||
| ); |
There was a problem hiding this comment.
When formo.identify(user, { privy: true }) runs while the SDK's current address is a connected wallet that is not in user.linkedAccounts (for example a wagmi wallet before it has been linked in Privy), the convenience path passes that address as activeAddress but activeWallet is undefined. This loop still calls normal identify() for every linked wallet, and each call updates currentAddress, so the unlinked connected wallet is overwritten by the last linked wallet and later events are attributed to the wrong address. Preserve/restore the original current address when no linked wallet matches the active address, or avoid setting current address for clustering-only identifies.
Useful? React with 👍 / 👎.
Follow-up to Codex review of the identify(user, { privy: true }) path. When the
SDK's current address is a connected wallet that is NOT in user.linkedAccounts
(e.g. a wagmi wallet not yet linked in Privy), the previous fix passed it as
activeAddress but no linked wallet matched, so the per-wallet loop still left
currentAddress on an arbitrary linked wallet.
- identifyPrivyUser now returns the address of the wallet it made active (the
one owning attribution), or undefined when it fell back to the heuristic
because no linked wallet matched the active address.
- The identify() Privy dispatch snapshots currentAddress before the sync and
restores it when identifyPrivyUser returns undefined, so a connected-but-
unlinked wallet keeps attribution while the linked wallets are still
identified for clustering. No public setActive flag reintroduced.
Added an integration test for the unlinked-connected-wallet case; 674 passing,
lint + build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbd5d3c9e0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await analytics.identify( | ||
| { address: wallet.address, userId: user.id }, | ||
| walletProperties | ||
| ); |
There was a problem hiding this comment.
When a Privy user's active wallet is Solana after an EVM wallet/chain was already current, this call only passes address/userId into identify(). identify() updates currentAddress but leaves currentChainId untouched, so subsequent tracking, excludeChains, and the active-wallet cookie can pair the Solana address with the stale EVM chain id and either suppress or lose attribution. Since wallet.chainType is known here, clear or update the chain state when making a Solana wallet the active one.
Useful? React with 👍 / 👎.
| if (!attributed && prevAddress && this.currentAddress !== prevAddress) { | ||
| this.currentAddress = prevAddress; | ||
| this.persistActiveWallet(); |
There was a problem hiding this comment.
When the connected wallet is not in user.linkedAccounts, the inner per-wallet identifies set currentUserId and the user-id cookie to the Privy DID before this fallback restores only currentAddress. The next track()/page() event is then sent with the unlinked connected wallet address plus the Privy DID, falsely associating that wallet with the Privy user; snapshot and restore the prior user id/cookie when falling back to the pre-sync address.
Useful? React with 👍 / 👎.
…tive state
Replaces the ordering + snapshot/restore approach (which kept spawning
attribution edge cases) with a structural fix, per review decision. Also owns
chain state for the active wallet.
Root cause: the per-wallet identify loop rewrote the SDK's active identity
(currentAddress / currentUserId / user-id cookie) for every wallet, so any
non-active wallet could win or corrupt attribution.
- identify() concrete impl gains an internal `setActive` flag (NOT on the public
IFormoAnalytics.identify overloads/interface). When false, the wallet↔user
link is still emitted and deduped for clustering, but currentAddress AND
currentUserId/cookie are left untouched.
- identifyPrivyUser emits clustering identifies with setActive:false and
promotes only the resolved active wallet with setActive:true. Ordering and the
dispatch snapshot/restore are gone; a connected wallet that isn't linked in
Privy is now preserved structurally (no wallet is promoted), which also fixes
the userId-restore gap (it's never repointed in the first place).
- Active-wallet resolution: an explicit/connected activeAddress is matched
strictly (no fallback), else user.wallet, else last-external heuristic.
- Chain state: identifyPrivyUser returns the active wallet's { address,
chainType }, and the identify(user,{privy:true}) dispatch clears currentChainId
when the active wallet's chain namespace no longer matches (e.g. a Solana
wallet while an EVM chain id was current), so events/cookie aren't paired with
a mismatched chain.
Tests reworked to assert setActive instead of loop order; added userId-preserve
and Solana-chain-clear coverage. Docs updated. 675 passing, lint + build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
… JSDoc
- Add docs/privy-identity-integration-plan.md: design/status doc covering the
clustering approach, the gaps closed, the attribution model (internal
setActive), Privy-docs validation, resolved review findings, and the deferred
Phase 2 items. Complements the usage-focused PRIVY_INTEGRATION.md.
- Fix IdentifyPrivyUserOptions.activeAddress JSDoc that still described the old
ordering ("identified last"); it now reflects the setActive promotion + strict
matching / preservation semantics.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c33237076
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await identify( | ||
| { | ||
| address: wallet.address, | ||
| userId: user.id, | ||
| setActive: wallet === activeWallet, | ||
| }, | ||
| walletProperties | ||
| ); |
There was a problem hiding this comment.
When consumers use the documented identifyPrivyUser(formo, user, { activeAddress }) entry point directly and select a Solana linked wallet after an EVM wallet is connected, these calls update currentAddress to the Solana address but leave the existing EVM currentChainId intact. The reconciliation exists only in the formo.identify(user, { privy: true }) dispatcher, despite the docs describing the two forms as equivalent, so subsequent events from the direct-helper flow can be emitted with a Solana address paired to an EVM chain ID.
Useful? React with 👍 / 👎.
| // still match (backward compatible). | ||
| const parts = [encodeURIComponent(address)]; | ||
| if (rdns) parts.push(encodeURIComponent(rdns)); | ||
| if (userId) parts.push(encodeURIComponent(userId)); |
There was a problem hiding this comment.
When an application supplies a userId but no rdns whose value equals a provider RDNS (for example, io.metamask), this creates the same address:io.metamask key as an anonymous identify with rdns: "io.metamask". If the provider identify occurs first, the later user-ID identify is deduped and its identity-link event is never emitted, defeating the new userId-aware re-emission behavior. Encode the presence of each optional component (or always retain an empty RDNS field) so these distinct tuples cannot collide.
Useful? React with 👍 / 👎.
| // Reconcile the chain id with the newly-active wallet's chain namespace | ||
| // so a Solana address isn't left paired with a stale EVM chain id. | ||
| if (active) this.syncPrivyActiveChain(active.chainType); |
There was a problem hiding this comment.
When tracking.excludeChains contains the currently connected EVM chain and this convenience form activates a linked Solana wallet, the helper emits its inner identify calls while currentChainId is still the excluded EVM value. Each inner trackEvent() therefore fails shouldTrack() and drops the wallet↔DID identify event; only after the loop does this line clear the stale chain ID. Reconcile the namespace before invoking the helper (or otherwise ensure those identify events are evaluated against the selected wallet) so the new Solana identity is not silently lost.
Useful? React with 👍 / 👎.
Addresses Codex review of 5c33237 (3 × P2). - Chain reconciliation moved into identifyPrivyUser, BEFORE the emit loop (previously in the dispatch, after). identify() runs each event through the tracking gate, which enforces excludeChains against the current chain id; if the stale chain was excluded, every clustering identify was dropped and the Privy identity silently lost. Reconciling first fixes that, and doing it in the helper means the direct identifyPrivyUser() entry point reconciles too (the two forms are now truly equivalent, as the docs claim). syncPrivyActiveChain is now an internal (non-public) method invoked via a structural cast. - Session dedup keys: when a userId is present, always emit the rdns slot (even empty), so a userId equal to a provider RDNS (e.g. "io.metamask") can't collide with an anonymous address:rdns key and get wrongly deduped. No-userId keys are unchanged (backward compatible). Added tests: identifies still emit when on an excluded chain but activating a Solana wallet; the direct helper reconciles chain; userId==rdns doesn't collide. Docs/JSDoc updated. 678 passing, lint + build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2581432b8e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await identify( | ||
| { | ||
| address: wallet.address, | ||
| userId: user.id, | ||
| setActive: wallet === activeWallet, | ||
| }, | ||
| walletProperties | ||
| ); |
There was a problem hiding this comment.
When a Privy user has more than the session store's 20-entry limit of linked wallets, this loop marks each wallet in sequence and evicts earlier (wallet, DID) keys. A later effect run then re-emits at least the evicted identities (and re-emits every wallet on each run once the set exceeds twice the limit), despite the documented per-session deduplication. Size the session identity store for the expanded linked-wallet set or avoid evicting keys generated by this synchronization.
Useful? React with 👍 / 👎.
Addresses Codex review of 2581432 (P2). The identified-wallet session store used a fixed 20-entry cap, so a Privy user with more than 20 linked wallets evicted earlier (wallet, DID) keys; a later sync then re-emitted the evicted identities, defeating the per-session dedup. Replace the fixed entry-count cap with a serialized-size bound: keep every identity that fits in the cookie and evict oldest only when the value would overflow (~3.5KB, safely under the ~4KB per-cookie limit — room for ~40 identities, beyond any realistic linked-wallet set). Chosen over a bigger magic cap (still bounded by cookie size) and over hashing keys (which would trade a harmless re-emit for a silent collision-drop of an identity link). The detected store keeps its small count cap. Added a test asserting 40 identities all remain recognized (past the old limit). 679 passing, lint + build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 594dc71de8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const activeWallet = resolveActiveWallet( | ||
| wallets, | ||
| options.activeAddress, | ||
| user.wallet?.address |
There was a problem hiding this comment.
When callers use the documented direct identifyPrivyUser(formo, user) form without activeAddress after Formo has already connected an external wallet, this passes undefined and resolveActiveWallet promotes user.wallet, overwriting attribution with (for example) an embedded wallet. The flag wrapper now passes this.currentAddress before reaching this helper, but the direct helper is documented as equivalent; preserve the existing Formo address here as well, or require callers to supply it.
Useful? React with 👍 / 👎.
Addresses Codex review of 594dc71 (P2). The direct identifyPrivyUser(formo, user) form (no activeAddress) fell back to user.wallet and overwrote attribution on an already-connected wallet, while the identify(user,{privy:true}) dispatch passed this.currentAddress first — so the two forms diverged despite being documented as equivalent. Move the currentAddress fallback INTO the helper: it reads the instance's currentAddress (via the existing internal cast) before user.wallet, so both entry points preserve a connected wallet identically. The dispatch is now a thin pass-through of opts.activeAddress. Unmatched connected wallets are still preserved (strict match, no promotion); stubs without currentAddress fall through to user.wallet as before. Added a direct-form preservation test. 680 passing, lint + build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aab64170c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (setActive !== false) { | ||
| this.currentAddress = validAddress; | ||
| this.persistActiveWallet(); | ||
| if (userId) { | ||
| this.currentUserId = userId; |
There was a problem hiding this comment.
For every linked wallet other than the resolved active one, setActive:false leaves currentUserId unchanged. trackEvent() passes that stale value into EventFactory.create(), which unconditionally overwrites the identify payload's user_id with its supplied current user ID (src/event/EventFactory.ts:804). Thus a fresh Privy sync sends user_id: null for all non-active wallets (or, worse, a previous user's ID), so only the active wallet is clustered under the Privy DID and the feature can corrupt existing identity associations. Pass the identify call's userId through to event creation without mutating persistent active state.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aab64170c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // clustering identifies from being silently dropped. Doing it here — rather | ||
| // than in the identify(user,{privy:true}) dispatch — means the direct | ||
| // identifyPrivyUser() entry point gets the same treatment. | ||
| target.syncPrivyActiveChain?.(activeWallet?.chainType); |
There was a problem hiding this comment.
When this helper is called on an excluded host/path (or after opt-out) with a linked wallet in the opposite namespace, this reconciliation runs before the inner identify() calls reach their suppression guard. It clears currentChainId even though none of the identifies may update the active wallet. In a SPA that then returns to an allowed route, the old EVM currentAddress remains but its excluded chain ID is gone, so shouldTrack() can no longer apply tracking.excludeChains and subsequent events for that wallet are collected. Guard reconciliation with the same suppression condition, or defer it until an active identify is actually accepted.
Useful? React with 👍 / 👎.
| if (setActive !== false) { | ||
| this.currentAddress = validAddress; | ||
| this.persistActiveWallet(); | ||
| if (userId) { | ||
| this.currentUserId = userId; | ||
| const domain = getIdentityCookieDomain(this.crossSubdomainCookies); | ||
| cookie().set(SESSION_USER_ID_KEY, userId, { | ||
| path: "/", | ||
| ...getIdentityCookieSecurity(), | ||
| ...(domain ? { domain } : {}), | ||
| }); | ||
| } |
There was a problem hiding this comment.
For every non-active linked wallet, setActive:false intentionally leaves currentUserId unchanged here, but trackEvent() passes that state to EventManager, and EventFactory.create() overwrites an identify event's payload userId with this passed state. Thus a normal multi-wallet sync emits only the active wallet under the Privy DID; the other wallet identifies have user_id: null (or, worse, a previous user's ID), so the advertised server-side wallet clustering fails or is misattributed. Preserve the payload DID when enqueueing these identify events rather than relying on active SDK state.
Useful? React with 👍 / 👎.
…n suppressed Two P1s from Codex review of 1aab641. 1. Clustering events lost the Privy DID. EventFactory.create() unconditionally overwrote each identify event's payload user_id with the active-session user id. Because setActive:false clustering identifies intentionally do NOT change currentUserId, every non-active wallet's identify was emitted with user_id: null (or a stale user), so only the active wallet clustered under the DID — silently breaking the whole feature. Fix: identify events keep their payload user_id (the per-wallet DID), falling back to the session user id only when the payload carries none. Other events are unchanged. This also preserves the WTo behavior (session currentUserId is still not repointed). 2. Chain reconciliation ran for suppressed visitors. identifyPrivyUser reconciles chain BEFORE the inner identifies reach their suppression guard, so on an excluded host/path (or after opt-out) it cleared currentChainId while no identify actually ran — leaving later events on an allowed route unable to apply excludeChains. Fix: gate the whole helper on isTrackingSuppressed (via the internal cast), covering both entry points. Added tests: EventFactory.create keeps the identify payload DID over/without the session user id; identifyPrivyUser emits nothing and does not reconcile chain when suppressed. 685 passing, lint + build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
Final review pass over the branch. No runtime bugs found; the issues were documentation left behind by the ordering -> setActive refactor, plus one dead export: - identify() @example and the IFormoAnalytics Privy-overload JSDoc still described the removed mechanisms ("identifies the active wallet last", "defaults to user.wallet") — both now state the real resolution order (explicit activeAddress -> connected wallet -> user.wallet) and the clustering-only nature of non-active identifies. - IdentifyPrivyUserOptions.activeAddress JSDoc was missing the connected-wallet fallback step. - PRIVY_INTEGRATION.md still claimed wallets are "identified in a deliberate order — the active wallet last". - isPrivyWalletAccount was exported solely for the deleted React hook; made module-private (it was never re-exported from the package entries, so no public API change). - Plan doc: resolved-findings list updated with the last four review rounds; test count refreshed (685). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVjnFuV1Bod4Vev9JYt2wq
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…-one-liner-6rovf1
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…-one-liner-6rovf1
…ware Closes gaps found while wiring the Privy example's account-linking UI. Parsing (src/privy): - Extract cross_app wallets. A cross_app account (e.g. Abstract Global Wallet) has no top-level address — its wallets live in embeddedWallets/ smartWallets, so they were dropped entirely and never clustered. - Parse phone, twitch, and custom_auth's customUserId, none of which were reaching the identify payload. - Deduplicate wallets by address (case-insensitive for EVM, exact for Solana), preferring a real wallet entry over a cross_app placeholder regardless of the order Privy lists accounts in. Dedup (src/session, src/FormoAnalytics): - Fold a fingerprint of the properties into the identify dedup key. Linking a social account leaves the wallets and the DID untouched, so every already-identified wallet used to dedupe and the new property never reached Formo until the session expired. - Writing a key supersedes that (address, rdns, userId) identity's previous entry instead of appending, so dedup means "same as this wallet's last identify". A profile that reverts (link then unlink) re-emits, and the cookie stays at one entry per wallet-user rather than growing per change. - The fingerprint is total: BigInt, functions, symbols, invalid Dates, NaN/Infinity, Map/Set and cycles canonicalize rather than throw; toJSON() is honored so wire-distinct values don't collide; undefined stays distinct from null; and a throwing getter degrades to pre-fingerprint dedup instead of losing the identify. This is about dedup only — BigInt/circular values still fail at the queue's native JSON.stringify, a pre-existing SDK-wide limitation that affects track() equally. - Auto-identify's pre-check now passes properties so it uses the same key as the identify it guards. Chain reconciliation: - Infer the EVM namespace from a 0x address when Privy omits chainType (smart_wallet and cross_app entries carry no chainType), and treat only "solana"/"ethereum" as known so an unrecognized namespace no longer clears a valid chain id. Account-set summary properties were considered and deliberately left out: they're derivable server-side, and being shared across wallets they'd make every link/unlink re-emit every wallet. 739 tests passing; lint, tsc, and full build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge from main left literal duplicate entries for `picomatch@4.0.5` and `fdir@6.5.0(picomatch@4.0.5)`, so every job that installs dependencies failed immediately with: ERR_PNPM_BROKEN_LOCKFILE ... is broken: duplicated mapping key (1609:3) This blocked build and audit, and cascaded into lint and test being skipped. Regenerated by pnpm; the diff is purely the removal of the ten duplicated lines, with no resolution or version changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc93e9813f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let newValue = identifiedWallets.join(","); | ||
| while ( | ||
| identifiedWallets.length > 1 && | ||
| newValue.length > MAX_IDENTIFIED_BYTES |
There was a problem hiding this comment.
When the dedup store contains user IDs with non-ASCII or reserved characters, this length check underestimates the value written to the browser: each component is already percent-encoded, and CookieStorage.set() encodes the complete value again at src/storage/built-in/cookie.ts:42. A newValue below 3,500 characters can therefore exceed the browser's roughly 4 KB cookie limit, causing the write to be rejected and subsequent identify calls to re-emit because their keys were never persisted. Apply the limit to the final encoded cookie size, including the cookie name/overhead, rather than newValue.length.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/session/index.ts">
<violation number="1" location="src/session/index.ts:228">
P2: The identified-wallet cookie is not actually bounded to 3500 bytes: the check measures the unencoded string and never handles one oversized entry, while `CookieStorage.set()` percent-encodes the value. Long or Unicode external IDs can therefore overflow the cookie and make the same wallet re-identify repeatedly; sizing the encoded value and defining an oversized-key strategy would keep dedup reliable.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| * per-cookie browser limit (leaving room for the cookie name and attributes); | ||
| * that comfortably holds ~40 identities, beyond any realistic linked-wallet set. | ||
| */ | ||
| const MAX_IDENTIFIED_BYTES = 3500; |
There was a problem hiding this comment.
P2: The identified-wallet cookie is not actually bounded to 3500 bytes: the check measures the unencoded string and never handles one oversized entry, while CookieStorage.set() percent-encodes the value. Long or Unicode external IDs can therefore overflow the cookie and make the same wallet re-identify repeatedly; sizing the encoded value and defining an oversized-key strategy would keep dedup reliable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/session/index.ts, line 228:
<comment>The identified-wallet cookie is not actually bounded to 3500 bytes: the check measures the unencoded string and never handles one oversized entry, while `CookieStorage.set()` percent-encodes the value. Long or Unicode external IDs can therefore overflow the cookie and make the same wallet re-identify repeatedly; sizing the encoded value and defining an oversized-key strategy would keep dedup reliable.</comment>
<file context>
@@ -54,23 +209,130 @@ export interface IFormoAnalyticsSession {
+ * per-cookie browser limit (leaving room for the cookie name and attributes);
+ * that comfortably holds ~40 identities, beyond any realistic linked-wallet set.
+ */
+const MAX_IDENTIFIED_BYTES = 3500;
+
export class FormoAnalyticsSession implements IFormoAnalyticsSession {
</file context>
Cookie budget (src/session):
- Measure the eviction budget on the ENCODED value. Key components are
percent-encoded when built and CookieStorage encodes the joined value
again, so `%3A` becomes `%253A` and each `,` becomes `%2C`. The raw-length
check understated what is written: 37 realistic DID-bearing keys measure
3500 raw but 3956 encoded, which with the cookie name and attributes
clears the ~4KB limit. The browser then rejects the write, nothing
persists, and every identify re-emits for the rest of the session — the
exact failure the store exists to prevent.
- Widen the properties fingerprint from one 32-bit FNV lane to two. A single
lane collides readily ({"x":"xefn1fnkq0"} and {"x":"filot3n704"} both hash
to 1mgjpo5), and a collision silently suppresses a changed profile for the
rest of the session.
- Mirror JSON.stringify's value semantics so the fingerprint tracks what is
actually sent: undefined/function/symbol properties are omitted (and
become null as array elements), NaN/Infinity become null, Map/Set send as
{}. Previously these fingerprinted differently from their wire form, so a
byte-identical payload could emit a second identify. null stays distinct
from omitted, and BigInt/cycles keep markers since JSON cannot represent
them at all.
Privy user parsing (src/privy):
- Normalize `createdAt`. PrivyUser is a structural interface: the React SDK
supplies a Date, but a user from the REST API or a JSON round-trip carries
an ISO string or epoch number, and `.getTime()` on those throws. Because
identify()'s outer catch swallows it, the whole one-liner would silently
emit nothing. Accepts Date/string/number, scales REST-style seconds, and
yields undefined rather than NaN for anything unparseable.
Docs (docs/PRIVY_INTEGRATION.md):
- Document the `currentAddress` step in the attribution order, which was
omitted — it is checked before `user.wallet` and matched strictly.
- Correct `identifyPrivyUser`'s return type; it resolves to the active
wallet, not void.
- Warn against looping public identify() over parsePrivyProperties' wallets:
public identify always promotes, so the last wallet takes attribution.
- Show a stable effect dependency, since dedup makes re-running safe but not
free, and drop the React-only usePrivy() call from the framework-agnostic
example.
Tests:
- Restore globalThis in the integration spec's teardown instead of deleting
it, which unbound it process-wide for every later test file.
744 tests passing; lint, tsc, and full build clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c046de5b3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return `[${value | ||
| .map((item) => stableStringify(item, seen) ?? "null") | ||
| .join(",")}]`; |
There was a problem hiding this comment.
When identify properties contain a sparse array, Array.prototype.map skips its holes, so a one-element sparse array fingerprints as [] even though JSON.stringify sends it as [null]. Consequently, changing a property from { tags: [] } to { tags: new Array(1) } is incorrectly deduplicated and the updated identify payload is never emitted during the session; iterate by array length so holes receive the same null representation as JSON serialization.
Useful? React with 👍 / 👎.
| while ( | ||
| identifiedWallets.length > 1 && | ||
| encodedCookieLength(newValue) > MAX_IDENTIFIED_ENCODED_BYTES |
There was a problem hiding this comment.
When a caller supplies a long or heavily percent-encoded external userId whose generated key alone exceeds the encoded cookie budget, the new size loop cannot run because identifiedWallets.length > 1 is false. The oversized value is still written and can be rejected by the browser, leaving no dedup marker and causing every identical identify() call to emit again; hash or otherwise bound arbitrary key components, or explicitly handle an oversized singleton.
Useful? React with 👍 / 👎.
| await identify( | ||
| { | ||
| address: wallet.address, | ||
| userId: user.id, | ||
| setActive: wallet === activeWallet, |
There was a problem hiding this comment.
When two Privy syncs overlap, as the documented un-awaited React effect can during an account switch or profile refresh, an older call can be suspended while identifying a non-active wallet, the newer call can promote the new user's wallet, and then the older call can resume and reach its active wallet here. That late promotion overwrites currentAddress and currentUserId with the previous user, so subsequent events are misattributed; serialize syncs, cancel stale generations, or perform the active promotion before the loop's first asynchronous yield.
Useful? React with 👍 / 👎.
| if (!wallet.fromCrossApp && deduped[existingIndex].fromCrossApp) { | ||
| deduped[existingIndex] = wallet; |
There was a problem hiding this comment.
When the same address appears first as a smart_wallet with little metadata and later as a regular wallet carrying walletClientType, chainType, or the correct embedded classification, this condition retains the first entry because replacement is limited to cross-app placeholders. The emitted identify then loses the richer metadata, and resolveActiveWallet() can make a different fallback choice based on the incorrect isEmbedded value; compare the available metadata for every duplicate rather than only checking fromCrossApp.
Useful? React with 👍 / 👎.
| return activeWallet | ||
| ? { address: activeWallet.address, chainType: activeWallet.chainType } | ||
| : undefined; |
There was a problem hiding this comment.
When an unsupported or malformed linked wallet is selected as active—for example a chainType: "bitcoin" entry permitted by these structural types—FormoAnalytics.identify() rejects its address, while every valid linked wallet was invoked with setActive: false. Nevertheless, this return reports the rejected wallet as owning attribution even though the prior identity remains active; validate or filter wallets before active resolution and only return a wallet that was successfully promoted.
Useful? React with 👍 / 👎.
| if (walletIsSolana !== currentIsSolana) { | ||
| this.currentChainId = undefined; | ||
| this.persistActiveWallet(); |
There was a problem hiding this comment.
When a Solana Privy wallet replaces an EVM wallet, clearing the chain ID here causes the subsequent active-wallet snapshot to contain the Solana address with no namespace information. On reload, loadActiveWallet() classifies a snapshot with no chain ID as EVM (isSolanaChainId(undefined) is false), stores the Solana address in _chainState.evm, and a later EVM disconnect or state synchronization can clear or replace the active Solana attribution; persist the resolved namespace separately when the exact chain ID is unknown.
Useful? React with 👍 / 👎.
| > // Re-runs on login, link, and unlink — not on every render. | ||
| > const identityKey = user | ||
| > ? `${user.id}:${user.linkedAccounts?.length ?? 0}` | ||
| > : null; |
There was a problem hiding this comment.
When Privy updates profile data without changing the number of linked accounts—for example an OAuth username/email refresh or a same-count account replacement—this recommended identityKey remains unchanged, so the effect does not call identify() and the new properties or wallet never reach Formo. This contradicts the preceding promise that the dependency changes with the profile; derive the key from the relevant linked-account identities and profile fields, or retain user as the dependency.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
3 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/session/index.ts">
<violation number="1" location="src/session/index.ts:100">
P2: Sparse arrays can be fingerprinted differently from their actual JSON payload, so dedup may emit a duplicate identify for values that serialize identically. The new array branch uses `.map(...)`, which skips holes; iterating by index (0..length-1) keeps JSON’s `null` behavior for missing elements.</violation>
</file>
<file name="docs/PRIVY_INTEGRATION.md">
<violation number="1" location="docs/PRIVY_INTEGRATION.md:67">
P2: The recommended `identityKey = `${user.id}:${user.linkedAccounts?.length ?? 0}`` only changes when the number of linked accounts changes. A profile update that doesn't change the account count (e.g. an OAuth email/username refresh) leaves this key unchanged, so the effect won't re-run and the updated properties never reach Formo — contradicting the surrounding text's claim that the key tracks profile changes. Consider deriving the key from relevant profile fields as well, not just account count.</violation>
<violation number="2" location="docs/PRIVY_INTEGRATION.md:74">
P2: Changing the connected wallet can leave attribution stale when this example is extended with `activeAddress`: the value is not included in the effect dependencies, and `wallets` is not defined in the shown snippet. The active address should be obtained explicitly (for example via `useWallets()`) and included as a scalar dependency alongside `identityKey`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| return `[${value | ||
| .map((item) => stableStringify(item, seen) ?? "null") | ||
| .join(",")}]`; |
There was a problem hiding this comment.
P2: Sparse arrays can be fingerprinted differently from their actual JSON payload, so dedup may emit a duplicate identify for values that serialize identically. The new array branch uses .map(...), which skips holes; iterating by index (0..length-1) keeps JSON’s null behavior for missing elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/session/index.ts, line 100:
<comment>Sparse arrays can be fingerprinted differently from their actual JSON payload, so dedup may emit a duplicate identify for values that serialize identically. The new array branch uses `.map(...)`, which skips holes; iterating by index (0..length-1) keeps JSON’s `null` behavior for missing elements.</comment>
<file context>
@@ -33,69 +33,84 @@ import type { IFormoEventProperties } from "../types/events";
- .map((item) => stableStringify(item, seen))
- .sort()
- .join(",")}}`;
+ return `[${value
+ .map((item) => stableStringify(item, seen) ?? "null")
+ .join(",")}]`;
</file context>
| return `[${value | |
| .map((item) => stableStringify(item, seen) ?? "null") | |
| .join(",")}]`; | |
| return `[${Array.from({ length: value.length }, (_, index) => | |
| stableStringify(value[index], seen) ?? "null" | |
| ).join(",")}]`; |
| > if (formo && authenticated && user) { | ||
| > formo.identify(user, { privy: true }); | ||
| > } | ||
| > }, [formo, authenticated, identityKey]); |
There was a problem hiding this comment.
P2: Changing the connected wallet can leave attribution stale when this example is extended with activeAddress: the value is not included in the effect dependencies, and wallets is not defined in the shown snippet. The active address should be obtained explicitly (for example via useWallets()) and included as a scalar dependency alongside identityKey.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/PRIVY_INTEGRATION.md, line 74:
<comment>Changing the connected wallet can leave attribution stale when this example is extended with `activeAddress`: the value is not included in the effect dependencies, and `wallets` is not defined in the shown snippet. The active address should be obtained explicitly (for example via `useWallets()`) and included as a scalar dependency alongside `identityKey`.</comment>
<file context>
@@ -54,9 +54,29 @@ That single `identify(user, { privy: true })` call identifies **every** wallet
+> if (formo && authenticated && user) {
+> formo.identify(user, { privy: true });
+> }
+> }, [formo, authenticated, identityKey]);
+> ```
+>
</file context>
| > ```ts | ||
| > // Re-runs on login, link, and unlink — not on every render. | ||
| > const identityKey = user | ||
| > ? `${user.id}:${user.linkedAccounts?.length ?? 0}` |
There was a problem hiding this comment.
P2: The recommended identityKey = ${user.id}:${user.linkedAccounts?.length ?? 0}`` only changes when the number of linked accounts changes. A profile update that doesn't change the account count (e.g. an OAuth email/username refresh) leaves this key unchanged, so the effect won't re-run and the updated properties never reach Formo — contradicting the surrounding text's claim that the key tracks profile changes. Consider deriving the key from relevant profile fields as well, not just account count.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/PRIVY_INTEGRATION.md, line 67:
<comment>The recommended `identityKey = `${user.id}:${user.linkedAccounts?.length ?? 0}`` only changes when the number of linked accounts changes. A profile update that doesn't change the account count (e.g. an OAuth email/username refresh) leaves this key unchanged, so the effect won't re-run and the updated properties never reach Formo — contradicting the surrounding text's claim that the key tracks profile changes. Consider deriving the key from relevant profile fields as well, not just account count.</comment>
<file context>
@@ -54,9 +54,29 @@ That single `identify(user, { privy: true })` call identifies **every** wallet
+> ```ts
+> // Re-runs on login, link, and unlink — not on every render.
+> const identityKey = user
+> ? `${user.id}:${user.linkedAccounts?.length ?? 0}`
+> : null;
+>
</file context>
2 participants
The problem
A Privy user is one account (a DID) with many linked wallets — an embedded wallet plus any external wallets they connect over time. Formo's analytics is address-keyed, so one person with 8 wallets becomes 8 Formo users. Retention, conversion, and user counts are all wrong, and no single journey is visible.
The approach
Tag every linked wallet with the same Privy
userId(the DID) viaidentify(). Because the wallets share auserId, existing server-side clustering merges them — noalias()API needed.Why one identify per wallet rather than one carrying a wallet array: the wallet profiler is address-keyed and needs an event per address to create a profile and bind it to the
anonymous_id.Public API
formo.identify(user, { privy: true, activeAddress?, properties? })identifyPrivyUser(analytics, user, options?){ address, chainType }.parsePrivyProperties(user)→{ properties, wallets }Exported from the package root and the React-free
./coreentry. There is no React hook — apps callidentify(user, { privy: true })from their own effect keyed on the Privyuser, which covers login,linkAccount, andunlinkAccount.Attribution — the core design
Every linked wallet is identified for clustering, but only one may own the SDK's current address (what later
track()/page()events attribute to).identify()impl carries an internalsetActiveflag — not on the publicIFormoAnalytics.identifyoverloads.setActive: falseemits and dedupes the wallet↔user link but does not touchcurrentAddress,currentUserId, or the user-id cookie.activeAddress(matched strictly) → the SDK's already-connected wallet →user.wallet→ last-external heuristic.activeAddresspromotes nothing, leaving the current address untouched.EventFactory.create()now keeps an identify event's payloaduser_idinstead of overwriting it with the active-session user id. Without this, everysetActive: falseevent was stripped of its DID — silently breaking clustering for exactly the wallets this feature exists to link.What each wallet sends
profilePropertiesare parsed fromlinkedAccounts:privyDid,privyCreatedAt,email,phone, socials (X, Twitch, Discord, GitHub, Farcaster, Google, …), andcustomUserId. Account-set summaries (wallet counts, type lists) are deliberately not sent — they're derivable server-side, and being shared across wallets they'd make every link/unlink re-emit every wallet.Wallets are
walletandsmart_walletaccounts plus across_appaccount'sembeddedWallets/smartWallets(e.g. Abstract Global Wallet), which carry addresses in arrays rather than a top-leveladdressand were previously dropped entirely. Deduplicated by address — case-insensitively for EVM, exactly for Solana — preferring a real wallet entry over across_appplaceholder regardless of account ordering.Dedup is profile-aware
Key:
(address, rdns, userId, hash(properties)).The properties fingerprint is what makes account linking work. Linking a social account leaves the wallets and the DID untouched, so without it every already-identified wallet deduped and the new property never reached Formo until the session expired.
Writing a key supersedes that identity's previous entry rather than appending, so dedup means "same as this wallet's last identify". A profile that reverts (link then unlink) re-emits, and the cookie stays at one entry per wallet-user instead of growing one per profile change.
The fingerprint is total:
BigInt, functions, symbols, invalidDates,NaN/Infinity,Map/Setand cycles canonicalize rather than throw;toJSON()is honored so wire-distinct values (a changedURL) don't collide;undefinedstays distinct fromnull; and a throwing getter or exoticProxydegrades to pre-fingerprint dedup rather than losing the identify. It runs after active state is mutated, so a throw would otherwise leave mutated identity with no emitted event.Key shapes are fixed by component count (1
address, 2address:rdns, 3+userId, 4+hash) so they can't collide, and shapes 1–2 are byte-identical to pre-userIdkeys — existing browser sessions still match.Note
This dedup change is global, not Privy-scoped: any
identify()passing changed properties now re-emits. That is deliberate — it's the correct semantics — but callers passing a volatile value (a timestamp, a random id) will emit one identify per call instead of one per session. Documented indocs/PRIVY_INTEGRATION.md.Chain reconciliation
identifyPrivyUserreconcilescurrentChainIdwith the active wallet's namespace before emitting, so identifies aren't dropped by anexcludeChainsgate on a stale chain id. The EVM namespace is inferred from a0xaddress when Privy omitschainType(smart_walletandcross_appentries carry none), and only"solana"/"ethereum"are treated as known — an unrecognized namespace leaves the chain id alone rather than guessing.Known limitation (pre-existing, out of scope)
BigIntand circular values inpropertiesstill fail at the event queue's nativeJSON.stringify, and because the wallet is dedup-marked before emitting, that identify is suppressed for the session. This affectstrack()equally and predates this branch; fixing it means changing payload semantics SDK-wide.Deferred to Phase 2 (needs ingest work)
identify()is address-keyed, so a Privy user with no linked wallet is a logged no-op. Needs auserId-keyed identify on ingest — which is also the real fix for copying user-level traits onto every wallet./userstab keyed onuser_id.Verification
739 tests passing (unit + real-
identify()integration +EventFactory.createuser-id resolution + session dedup covering link/unlink re-emit, profile reversion, and hostile property values). Lint,tsc, and full build clean.Reviewed across four adversarial Codex passes; all findings resolved except the pre-existing serialization limitation noted above.
Companion PR: getformo/examples#129 adds account linking to the
with-privyexample (draft — blocked until this ships as 1.34.0).Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.