fix(profile-picture): WA Web parity — port PR WhiskeySockets#2614 + add id/invite/persona_id/common_gid (#510)

* fix(profile-picture): WA Web parity — nest tctoken + add id/invite/persona_id/common_gid

Port of upstream WhiskeySockets#2614 + adicional gaps identified by
comparing our `profilePictureUrl` against WA Web (live source via CDP) and
whatsmeow source code. Brings our `<iq xmlns="w:profile:picture">` stanza
to byte-for-byte parity with the official client behavior.

What changes:

1. New `LIDMappingStore.getKnownLIDForPN(pn)` (port of PR WhiskeySockets#2614, literal).
   Returns the LID for a PN **only if locally known** (cache or store) — never
   triggers USync. Opportunistic call sites (profile-picture, etc.) use this
   so the failure-to-resolve path does NOT fire a USync round-trip. USync-
   on-profile-picture is not a behavior WA Web or whatsmeow emit; doing it
   makes our traffic profile stand out and may serve as a ban signal.

2. New `buildTcTokenNode({ authState, jid, getLIDForPN })` helper in
   tc-token-utils.ts. Same expiry / opportunistic-cleanup semantics as the
   pre-existing `buildTcTokenFromJid`, but returns the single <tctoken>
   BinaryNode (or undefined) instead of pushing into a content array. Callers
   that need the token as a nested CHILD of another node use this; the older
   `buildTcTokenFromJid` is kept intact for `presenceSubscribe` which uses
   the sibling-array shape.

3. `profilePictureUrl` refactored to match WA Web's
   `WASmaxOutProfilePictureGetRequest` mixin composition:

   - tctoken is now nested as a CHILD of <picture> (was a sibling — verified
     wrong against WA Web JS source: mergeStanzas(<picture>, <tctoken>)
     places tctoken inside picture, NOT alongside it).
   - tctoken resolver swapped to getKnownLIDForPN (no USync, PR WhiskeySockets#2614).
   - New optional params accepted in `opts`:
       * existingId  → <picture id="..."> for 304-NotModified caching
                       (matches WA Web pictureId + whatsmeow ExistingID).
                       Saves a CDN re-fetch when the picture hasn't changed.
       * invite      → <picture invite="..."> for group-invite-link preview
                       (fetch a group's picture without joining). When set,
                       we skip the tctoken (the invite IS the authorization).
       * personaId   → <picture persona_id="..."> for Meta AI bot personas.
       * commonGid   → <picture common_gid="..."> required when the target's
                       privacy is "My contacts" and we share a group but are
                       not in their contacts — without it the server returns
                       401/403. Matches WA Web.

   - Same callers using the old signature work unchanged (all new params are
     in an optional `opts` object).

4. `presenceSubscribe` is intentionally NOT changed — it doesn't wrap the
   tctoken inside a `<picture>` (no nesting question), and a USync miss
   during presence-subscribe is far less common (we usually know the LID by
   the time we subscribe). Kept on `buildTcTokenFromJid` + `getLIDForPN`.

Empirical validation — triple-confirmed before applying:

  a) PR WhiskeySockets#2614 description (`fix: nest profile picture tctoken and avoid
     usync on lookup`) plus cubic + coderabbit summaries.
  b) whatsmeow's `Client.GetProfilePictureInfo` in user.go — pictureContent
     is the `Content` of the picture node (Go semantics: child).
  c) WA Web's live JS bundle, extracted via Chrome DevTools Protocol
     (`Debugger.getScriptSource`). The mixin chain in
     `WASmaxOutProfilePictureGetRequest` is:
       smax("picture", attrs)
         → optionalMerge(mergeAddRequestMixin, ...)
         → optionalMerge(mergeTCTokenMixin, ...)
         → optionalMerge(mergeAvatarMixin, ...)
     where `mergeStanzas(target, mixin)` places the mixin INSIDE target.
     The tctoken is undeniably a child of <picture>.

Tests:

  - Existing 72 tests in `tc-token.test.ts` continue to pass (no signature
    changes to `buildTcTokenFromJid`).
  - 6 new tests for `buildTcTokenNode` (valid token, missing, expired +
    cleanup, no-wipe-when-missing, error swallow, return shape).
  - Full TC token suite: 78/78 passing.

Out of scope (intentionally NOT touched):
  - Carousel, lists, buttons, polls, view-once, biz quality_control,
    useLegacyLock, the broader TC token issuance flow, LID↔PN batched, Phase
    9 multi-DB, lidDbMigrated:false, the cacheMetricsInterval memory-leak
    fix, schema migrations + statement cache + busy retry, presence-
    subscribe path (covered separately above).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(lid-mapping): wrap getKnownLIDForPN in checkDestroyed + trackOperation (PR #510 review)

PR #510 review caught a P2 contract violation: every other public method on
`LIDMappingStore` calls `this.checkDestroyed()` at entry and runs its body
inside `this.trackOperation(...)`. The new `getKnownLIDForPN` (added in this
PR as part of the WA Web profile-picture parity port) was missing both
guards.

Why this matters:

- Without `checkDestroyed()`: the method runs after `destroy()` has been
  called, when `this.keys` may already have been torn down. The first
  `await this.keys.get(...)` then races UAF on the underlying signal key
  store.
- Without `trackOperation()`: `destroy()` does not wait for this method's
  in-flight I/O before clearing the mapping cache. A concurrent call can
  observe a half-cleared cache (a `mappingCache.set(...)` against a Map
  that `destroy()` is about to `clear()`) and silently return stale data
  rather than failing fast.

Both threads (copilot + cubic) flagged the same issue, cubic with
confidence 9. The fix is identical to the suggestion both reviewers
provided: wrap the body in `this.trackOperation(async () => { … })`
preceded by `this.checkDestroyed()`.

Caller-side impact: none. `buildTcTokenNode` (the sole call site) has a
try/catch that swallows any throw — so the only effect a post-destroy
call has is "tctoken not attached" (same as the existing miss path), not
a crash.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(tc-token): extract resolveTcTokenForJid helper (PR #510 review)

PR #510 cubic review (Thread 3, P3) flagged that `buildTcTokenFromJid` and
`buildTcTokenNode` carried byte-for-byte identical retrieval / expiry /
opportunistic-cleanup pipelines — only the return shape differed (sibling
array vs single node). Extracted that pipeline into a private
`resolveTcTokenForJid(...)` helper so the two public builders become thin
adapters around a single source of truth.

Behavioral contract preserved (verified against the existing 78 tests):

  - Both public builders keep their exact public signatures.
  - `buildTcTokenFromJid` still mutates `baseContent` in place when a
    token exists, and still returns `baseContent.length > 0 ? baseContent
    : undefined` on every "no token" branch — same legacy behavior the
    presenceSubscribe call site relies on.
  - `buildTcTokenNode` still returns a single `BinaryNode` or `undefined` —
    same shape profilePictureUrl uses for nesting under `<picture>`.
  - The opportunistic expired-token wipe (with senderTimestamp
    preservation) runs exactly once, in the helper, with the same write
    shape as before. Missing-token entries are still NOT wiped.
  - Key-store errors are still swallowed inside the helper; both public
    builders fall back to their no-token branch on throw.

Why the two builders stay as separate public surfaces (instead of one):

  - `presenceSubscribe` builds a `<presence>` whose content is the legacy
    sibling-array shape; it cares about preserving `baseContent` when no
    token exists. Forcing it to the single-node shape would require every
    call site to special-case `undefined`.
  - `profilePictureUrl` (port of PR WhiskeySockets#2614) needs the token nested as a
    CHILD of `<picture>`. It MUST get a single node, not an array.

So the API split is meaningful; only the internal implementation was
duplicated. The helper kills the duplication without changing the surface.

Test impact: 0 changes to the test suite, 78/78 still pass. Same numbers
for the LID-mapping suite (15) and identity-change suite (23) that
indirectly exercise the same signal-store paths — 116/116 in the
combined run.

Out of scope (intentionally NOT touched):
  - Carousel, lists, buttons, polls, view-once, biz quality_control,
    useLegacyLock, the broader TC token issuance flow (only retrieval
    helpers refactored), LID↔PN batched, Phase 9 multi-DB,
    lidDbMigrated:false, the cacheMetricsInterval memory-leak fix, schema
    migrations + statement cache + busy retry.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: rsalcara

src/Signal/lid-mapping.ts

@@ -505,6 +505,53 @@ export class LIDMappingStore {
 		})
 	}
 
+	/**
+	 * Port of upstream PR #2614 (`fix: nest profile picture tctoken and avoid
+	 * usync on lookup`). Returns the LID for a PN ONLY if the mapping is
+	 * already known (memory cache or on-disk store). Never triggers a USync
+	 * lookup.
+	 *
+	 * Use this on hot paths where firing a USync just to opportunistically
+	 * attach metadata (e.g. profile-picture tctoken) is undesired — both
+	 * because the latency is wasted (the operation must still proceed if the
+	 * mapping is unknown) AND because USync-on-look-up is a behavioral
+	 * fingerprint WA Web / whatsmeow don't emit, so doing it makes our
+	 * traffic profile stand out and may serve as a ban signal.
+	 *
+	 * Thread safety: wrapped in `checkDestroyed()` + `trackOperation()` —
+	 * same contract every other public method on this store follows. Without
+	 * these, the async `keys.get()` could race with `destroy()` (UAF on the
+	 * key store) and a post-destroy call could silently return stale data.
+	 * (PR #510 review — addresses cubic / copilot P2.)
+	 */
+	async getKnownLIDForPN(pn: string): Promise<string | null> {
+		this.checkDestroyed()
+
+		return this.trackOperation(async () => {
+			if (!isAnyPnUser(pn)) return null
+
+			const decoded = jidDecode(pn)
+			if (!decoded) return null
+
+			const pnUser = decoded.user
+			let lidUser = this.mappingCache.get(`pn:${pnUser}`)
+			if (!lidUser) {
+				const stored = await this.keys.get('lid-mapping', [pnUser])
+				const storedLidUser = stored[pnUser]
+				if (typeof storedLidUser === 'string' && storedLidUser) {
+					lidUser = storedLidUser
+					this.mappingCache.set(`pn:${pnUser}`, lidUser)
+					this.mappingCache.set(`lid:${lidUser}`, pnUser)
+				}
+			}
+
+			if (!lidUser) return null
+
+			const pnDevice = decoded.device !== undefined ? decoded.device : 0
+			return `${lidUser}${pnDevice ? `:${pnDevice}` : ''}@${decoded.server === 'hosted' ? 'hosted.lid' : 'lid'}`
+		})
+	}
+
 	/**
 	 * Get LIDs for multiple PNs - Optimized batch operation
 	 *

src/Socket/chats.ts

@@ -55,7 +55,7 @@ import {
 } from '../Utils'
 import { makeKeyedMutex, makeMutex } from '../Utils/make-mutex'
 import processMessage from '../Utils/process-message'
-import { buildTcTokenFromJid } from '../Utils/tc-token-utils'
+import { buildTcTokenFromJid, buildTcTokenNode } from '../Utils/tc-token-utils'
 import {
 	type BinaryNode,
 	getBinaryNodeChild,
@@ -96,6 +96,14 @@ export const makeChatsSocket = (config: SocketConfig) => {
 	} = sock
 
 	const getLIDForPN = signalRepository.lidMapping.getLIDForPN.bind(signalRepository.lidMapping)
+	/**
+	 * Local-only LID resolver (port of upstream PR #2614). Use on
+	 * profile-picture / similar paths where we OPPORTUNISTICALLY attach
+	 * metadata: a USync miss should NOT bubble out as a network round-trip
+	 * because that traffic profile diverges from WA Web / whatsmeow and
+	 * smells like a custom client.
+	 */
+	const getKnownLIDForPN = signalRepository.lidMapping.getKnownLIDForPN.bind(signalRepository.lidMapping)
 
 	let privacySettings: { [_: string]: string } | undefined
 
@@ -784,43 +792,88 @@ export const makeChatsSocket = (config: SocketConfig) => {
 	)
 
 	/**
-	 * fetch the profile picture of a user/group
-	 * type = "preview" for a low res picture
-	 * type = "image for the high res picture"
+	 * Fetch the profile picture URL of a user/group.
+	 *
+	 * `type`:        "preview" for a low-res picture, "image" for the high-res picture.
+	 * `existingId`:  the `id` of the last known picture for this JID. If supplied AND the
+	 *                picture has not changed, the server responds with a `304`-style empty
+	 *                result (sentinel for "use cached URL") instead of returning a fresh URL.
+	 *                Saves a CDN re-fetch per refresh — matches WA Web `pictureId` and
+	 *                whatsmeow `ExistingID`.
+	 * `invite`:      group-invite code. Allows fetching a group's picture without joining,
+	 *                used by the "preview before accepting invite" flow. Mutually exclusive
+	 *                with `tctoken` (server doesn't require it for invite-code lookups).
+	 * `personaId`:   Meta-AI bot persona id. Required to fetch the picture of an AI persona.
+	 * `commonGid`:   a group jid both parties belong to. Required when the target's privacy
+	 *                is set to "My contacts" and we are NOT in their contacts but share a
+	 *                group — without it the server returns 401/403. Matches WA Web.
+	 *
+	 * Stanza shape (matches WA Web `WASmaxOutProfilePictureGetRequest` + whatsmeow):
+	 *
+	 *   <iq xmlns="w:profile:picture" type="get" target="{jid}" to="s.whatsapp.net">
+	 *     <picture type="..." query="url" [id] [invite] [persona_id] [common_gid]>
+	 *       [<tctoken>...</tctoken>]   ← nested CHILD (not sibling)
+	 *     </picture>
+	 *   </iq>
 	 */
-	const profilePictureUrl = async (jid: string, type: 'preview' | 'image' = 'preview', timeoutMs?: number) => {
-		const baseContent: BinaryNode[] = [{ tag: 'picture', attrs: { type, query: 'url' } }]
-
-		// WA Web only includes tctoken for user JIDs (not groups/newsletters)
-		// and never for own profile pic (Chat model for self has no tcToken).
-		// Including tctoken for own JID causes the server to never respond.
+	const profilePictureUrl = async (
+		jid: string,
+		type: 'preview' | 'image' = 'preview',
+		timeoutMs?: number,
+		opts?: {
+			existingId?: string
+			invite?: string
+			personaId?: string
+			commonGid?: string
+		}
+	) => {
 		const normalizedJid = jidNormalizedUser(jid)
 		const isUserJid = isAnyPnUser(normalizedJid) || isAnyLidUser(normalizedJid)
 		const me = authState.creds.me
 		const isSelf =
 			me && (normalizedJid === jidNormalizedUser(me.id) || (me.lid && normalizedJid === jidNormalizedUser(me.lid)))
-		let content: BinaryNode[] | undefined = baseContent
 
-		if (isUserJid && !isSelf) {
-			content = await buildTcTokenFromJid({
+		// Build the <picture> attrs — include only the fields the caller supplied,
+		// so unset ones map to DROP_ATTR (matching WA Web's OPTIONAL serializer behavior).
+		const pictureAttrs: { [k: string]: string } = { type, query: 'url' }
+		if (opts?.existingId) pictureAttrs.id = opts.existingId
+		if (opts?.invite) pictureAttrs.invite = opts.invite
+		if (opts?.personaId) pictureAttrs.persona_id = opts.personaId
+		if (opts?.commonGid) pictureAttrs.common_gid = opts.commonGid
+
+		const pictureNode: BinaryNode = { tag: 'picture', attrs: pictureAttrs }
+
+		// Attach tctoken (if known) as a CHILD of <picture>. Match WA Web
+		// (WASmaxOutProfilePictureTCTokenMixin) and whatsmeow (pictureContent).
+		// WA Web only includes tctoken for user JIDs (not groups/newsletters)
+		// and never for own profile pic — including it for self causes the
+		// server to never respond. Invite-code lookups also skip the token
+		// (the invite IS the authorization).
+		if (isUserJid && !isSelf && !opts?.invite) {
+			const tctokenNode = await buildTcTokenNode({
 				authState,
 				jid: normalizedJid,
-				baseContent,
-				getLIDForPN
+				// Port of upstream PR #2614: never fire USync from the profile-picture
+				// path. If the LID mapping is unknown we send the IQ without the
+				// tctoken and let the server tell us (vs. doing a USync round trip
+				// that fingerprints us as non-WA-Web).
+				getLIDForPN: getKnownLIDForPN
 			})
+			if (tctokenNode) {
+				pictureNode.content = [tctokenNode]
+			}
 		}
 
-		jid = normalizedJid
 		const result = await query(
 			{
 				tag: 'iq',
 				attrs: {
-					target: jid,
+					target: normalizedJid,
 					to: S_WHATSAPP_NET,
 					type: 'get',
 					xmlns: 'w:profile:picture'
 				},
-				content
+				content: [pictureNode]
 			},
 			timeoutMs
 		)

src/Utils/tc-token-utils.ts

@@ -101,22 +101,55 @@ type TcTokenParams = {
 	getLIDForPN?: (pn: string) => Promise<string | null>
 }
 
-export async function buildTcTokenFromJid({
+/**
+ * Resolved tctoken state for a JID, shared by the two public builders.
+ *
+ * - `buffer`: present iff a non-expired, non-empty token exists in store.
+ * - Absent `buffer`: caller produces a "no tctoken" result (the shape of
+ *   that result differs per builder — array vs single node).
+ *
+ * The helper is also responsible for the opportunistic expired-token wipe
+ * documented inline. Callers must NOT re-do that bookkeeping.
+ */
+type ResolvedTcToken = { buffer?: Buffer }
+
+/**
+ * Shared retrieval + expiry + opportunistic-cleanup pipeline used by both
+ * `buildTcTokenFromJid` (sibling-array shape, kept for legacy call sites)
+ * and `buildTcTokenNode` (single-node shape, used for nested tctoken in
+ * `<picture>`). Extracting this collapses what used to be two
+ * byte-for-byte identical critical sections so any future change to the
+ * expiry / cleanup semantics happens in one place.
+ *
+ * Returns `{}` (no buffer) on every "no usable token" outcome:
+ *   - store miss
+ *   - empty token
+ *   - expired token (also performs the cleanup write)
+ *   - key-store error (swallowed; callers fall back to base content)
+ *
+ * Notes on the cleanup write (preserved from the original implementation):
+ *   - Only fires when an EXPIRED non-empty token was found. Missing tokens
+ *     are NOT wiped because nothing exists to wipe.
+ *   - If the entry carried a `senderTimestamp`, we preserve it via a
+ *     placeholder `{ token: Buffer.alloc(0), senderTimestamp }` so the
+ *     fire-and-forget issuance dedupe in messages-send survives. Otherwise
+ *     we tombstone the entry with `null`.
+ *   - Matches the exact same shape messages-send writes for issuance
+ *     placeholders, so we never accidentally widen the wipe to clear a
+ *     legitimate placeholder.
+ */
+async function resolveTcTokenForJid({
 	authState,
 	jid,
-	baseContent = [],
 	getLIDForPN
-}: TcTokenParams): Promise<BinaryNode[] | undefined> {
+}: Pick<TcTokenParams, 'authState' | 'jid' | 'getLIDForPN'>): Promise<ResolvedTcToken> {
 	try {
 		const storageJid = getLIDForPN ? await resolveTcTokenJid(jid, getLIDForPN) : jid
 		const tcTokenData = await authState.keys.get('tctoken', [storageJid])
 		const entry = tcTokenData?.[storageJid]
 		const tcTokenBuffer = entry?.token
 
 		if (!tcTokenBuffer?.length || isTcTokenExpired(entry?.timestamp)) {
-			// Opportunistic cleanup: drop the expired token but preserve senderTimestamp so the
-			// fire-and-forget issuance dedupe survives — same shape used in messages-send, so this
-			// path no longer wipes that placeholder (only clears a real, non-empty expired token).
 			if (tcTokenBuffer?.length) {
 				const cleared =
 					entry?.senderTimestamp !== undefined
@@ -125,19 +158,61 @@ export async function buildTcTokenFromJid({
 				await authState.keys.set({ tctoken: { [storageJid]: cleared } })
 			}
 
-			return baseContent.length > 0 ? baseContent : undefined
+			return {}
 		}
 
-		baseContent.push({
-			tag: 'tctoken',
-			attrs: {},
-			content: tcTokenBuffer
-		})
+		return { buffer: tcTokenBuffer }
+	} catch {
+		return {}
+	}
+}
 
-		return baseContent
-	} catch (error) {
+/**
+ * Legacy sibling-array shape. Used by `presenceSubscribe` where the tctoken
+ * is the only content of a `<presence>` (so the "sibling" framing is moot —
+ * there's nothing to sibling against). Kept on the legacy
+ * `buildTcTokenFromJid` + `getLIDForPN` resolver pair for behavioral
+ * stability.
+ *
+ * Returns `baseContent` (mutated in place with the `<tctoken>` appended)
+ * when a token exists, or `baseContent | undefined` otherwise — same exact
+ * contract as before the helper extraction.
+ */
+export async function buildTcTokenFromJid({
+	authState,
+	jid,
+	baseContent = [],
+	getLIDForPN
+}: TcTokenParams): Promise<BinaryNode[] | undefined> {
+	const { buffer } = await resolveTcTokenForJid({ authState, jid, getLIDForPN })
+
+	if (!buffer) {
 		return baseContent.length > 0 ? baseContent : undefined
 	}
+
+	baseContent.push({ tag: 'tctoken', attrs: {}, content: buffer })
+	return baseContent
+}
+
+/**
+ * Build a standalone <tctoken> BinaryNode (no container, no sibling array).
+ *
+ * Use this when the caller needs the tctoken as a CHILD of another stanza node
+ * — e.g. nested inside <picture> for `w:profile:picture` queries (port of
+ * upstream PR #2614 / matches WA Web's `WASmaxOutProfilePictureTCTokenMixin`
+ * + whatsmeow's `pictureContent`).
+ *
+ * Returns the node when a valid (non-expired, non-empty) tctoken exists for
+ * the resolved storage JID, or `undefined` otherwise.
+ */
+export async function buildTcTokenNode({
+	authState,
+	jid,
+	getLIDForPN
+}: Omit<TcTokenParams, 'baseContent'>): Promise<BinaryNode | undefined> {
+	const { buffer } = await resolveTcTokenForJid({ authState, jid, getLIDForPN })
+
+	return buffer ? { tag: 'tctoken', attrs: {}, content: buffer } : undefined
 }
 
 type StoreTcTokensParams = {