snapshot session after prep, skip no-op cross-tab rebuilds, restore chat proxy env

Review fixes (PR #11182 round 3):
- The session factories snapshotted the account BEFORE the prep awaits
  (moderation config, age-assurance prefetch) with hooks still disarmed.
  A 401 during prep triggers PasswordSession's internal auto-refresh,
  rotating both tokens; the dropped onUpdated meant the stale refreshJwt
  got persisted (dead on next cold start). The returned account is now
  re-snapshotted after prep, right before arm() - matching the old
  BskyAppAgent.prepare ordering. Tests pin the mid-prep rotation case.
- Cross-tab sync rebuilt and killed the active bundle on ANY saved-
  account change; now bails out when the current account's tokens are
  unchanged.
- buildChatClient hard-coded the SDK's prod chat DID, making the env-
  configurable CHAT_PROXY_DID dead code and routing staging DMs to prod
  chat; new CHAT_PROXY_SERVICE constant restores the override.
- The cross-tab labeler reapply was fire-and-forget; the bundle swap now
  defers until the labeler config resolves, matching the factories.

Note: the reviewer's URL.canParse/ReadableStream claim was verified
INCORRECT - Expo 54's WinterCG runtime defines URL.canParse and Metro
injects a ReadableStream polyfill (expo/virtual/streams.js) into dev and
release bundles.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/lib/constants.ts

@@ -2,7 +2,7 @@ import {type Insets, Platform} from 'react-native'
 import {type Service} from '@atproto/lex'
 import {api} from '@bsky.app/sdk'
 
-import {BLUESKY_PROXY_DID, IS_DEV} from '#/env'
+import {BLUESKY_PROXY_DID, CHAT_PROXY_DID, IS_DEV} from '#/env'
 import {type app} from '#/lexicons'
 
 /**
@@ -265,6 +265,14 @@ export const BLUESKY_MOD_SERVICE_HEADERS = {
  */
 export const NOTIF_SERVICE = `${BLUESKY_PROXY_DID}#bsky_notif` as Service
 
+/**
+ * Service proxy identifier for the chat service. Passed as the `service` option
+ * on the chat client so lex-client emits the `atproto-proxy` header (replaces
+ * the old per-call `DM_SERVICE_HEADERS`). Env-configurable via
+ * `EXPO_PUBLIC_CHAT_PROXY_DID` (see {@link CHAT_PROXY_DID}).
+ */
+export const CHAT_PROXY_SERVICE = `${CHAT_PROXY_DID}#bsky_chat` as Service
+
 export const webLinks = {
   tos: `https://bsky.social/about/support/tos`,
   privacy: `https://bsky.social/about/support/privacy-policy`,

src/state/session/__tests__/session-core-test.ts

@@ -3,7 +3,7 @@ import {
   type PasswordSessionOptions,
   type SessionData,
 } from '@atproto/lex-password-session'
-import {describe, expect, it, jest} from '@jest/globals'
+import {beforeEach, describe, expect, it, jest} from '@jest/globals'
 
 import {type SessionAccount} from '../types'
 
@@ -27,11 +27,45 @@ jest.mock('#/state/queries/messages/restrictChatSettings', () => ({
   restrictChatSettings: () => Promise.resolve(),
 }))
 
+/*
+ * The factory tail awaits `features.refresh(...)`; stub the analytics module so
+ * the factory does not pull GrowthBook (and its native deps) into this
+ * lightweight suite.
+ */
+jest.mock('#/analytics', () => ({
+  features: {refresh: () => Promise.resolve()},
+}))
+
+/*
+ * `configureModerationForAccount` is one of the prep awaits in the resume/login
+ * factories. We replace it with a hook that runs a REAL `session.refresh()`, so
+ * a token rotation happens DURING prep (before arm()) - exactly the 401
+ * auto-refresh scenario the re-snapshot fix guards against. The default is a
+ * no-op so other tests are unaffected; individual tests install the refreshing
+ * behavior via `mockImplementationOnce`. (jest requires out-of-scope factory
+ * references to be `mock`-prefixed.)
+ */
+const mockConfigureModerationForAccount =
+  jest.fn<(bundle: unknown, account: unknown) => Promise<void>>()
+jest.mock('../moderation', () => ({
+  configureModerationForAccount: (bundle: unknown, account: unknown) =>
+    mockConfigureModerationForAccount(bundle, account),
+  configureModerationForGuest: () => {},
+}))
+
 jest.mock('jwt-decode', () => ({
   jwtDecode(token: string) {
     if (token === 'queued-access-jwt') {
       return {scope: 'com.atproto.signupQueued'}
     }
+    /*
+     * A far-future exp so isSessionExpired() reads this stored token as still
+     * valid, which routes resume() through the sync (no-network) fast path.
+     * That isolates the prep-time refresh as the ONLY token rotation.
+     */
+    if (token === 'valid-access-jwt') {
+      return {scope: 'com.atproto.access', exp: 4102444800}
+    }
     return {scope: 'com.atproto.access'}
   },
 }))
@@ -785,3 +819,89 @@ describe('refreshSession semantics', () => {
     await expect(session.refresh()).rejects.toBeDefined()
   })
 })
+
+/*
+ * Fix 1: the resume/login factories must snapshot the RETURNED account AFTER
+ * the prep awaits, not before. A 401 during prep triggers PasswordSession's
+ * internal auto-refresh (rotating BOTH tokens and firing an onUpdated the
+ * disarmed latch drops); an early snapshot would persist the stale refreshJwt,
+ * which is dead on the next cold start.
+ *
+ * We simulate the mid-prep rotation by making the mocked
+ * `configureModerationForAccount` (a genuine prep await in each factory) run a
+ * real `session.refresh()`. The factory itself is re-required inside
+ * `jest.isolateModulesAsync` AFTER overriding `globalThis.fetch`, because
+ * session-core captures `globalThis.fetch` into `networkAwareFetch` at module
+ * load - and that captured fetch is what PasswordSession's auto-refresh routes
+ * through.
+ */
+describe('factory account snapshot is taken AFTER prep (fix 1)', () => {
+  /** Load a fresh session-core whose networkAwareFetch captures `fetch`. */
+  async function withFreshFactory(
+    fetch: typeof globalThis.fetch,
+    run: (core: typeof import('../session-core')) => Promise<void>,
+  ) {
+    const realFetch = globalThis.fetch
+    globalThis.fetch = fetch
+    try {
+      await jest.isolateModulesAsync(async () => {
+        const core =
+          require('../session-core') as typeof import('../session-core')
+        await run(core)
+      })
+    } finally {
+      globalThis.fetch = realFetch
+    }
+  }
+
+  beforeEach(() => {
+    mockConfigureModerationForAccount.mockReset()
+  })
+
+  it('resume: returned account carries the tokens rotated DURING prep', async () => {
+    /*
+     * A refresh mid-prep rotates the session to access-jwt-2/refresh-jwt-2.
+     * `valid-access-jwt` decodes as non-expired, so resume() takes the sync
+     * fast path and the only refresh is the one prep triggers.
+     */
+    mockConfigureModerationForAccount.mockImplementationOnce(
+      async (bundle: unknown) => {
+        await (bundle as SessionBundle).session.refresh()
+      },
+    )
+    const fetchMock = makeMockFetch()
+
+    await withFreshFactory(asFetch(fetchMock), async core => {
+      const {account, bundle} = await core.createSessionBundleAndResume(
+        makeAccount({accessJwt: 'valid-access-jwt'}),
+        jest.fn(),
+      )
+      /* the moderation prep step ran the refresh */
+      expect(mockConfigureModerationForAccount).toHaveBeenCalledTimes(1)
+      /* the RETURNED account carries the POST-prep (rotated) tokens */
+      expect(account.accessJwt).toBe('access-jwt-2')
+      expect(account.refreshJwt).toBe('refresh-jwt-2')
+      /* and it matches the session's committed state */
+      expect(bundle.session.session.accessJwt).toBe('access-jwt-2')
+    })
+  })
+
+  it('resume: returned account falls back to the stored account when the fast path yields no live token change', async () => {
+    /*
+     * Control: no mid-prep refresh. The re-snapshot still reflects the (still
+     * valid) stored tokens, confirming the moved snapshot did not regress the
+     * happy path.
+     */
+    mockConfigureModerationForAccount.mockResolvedValueOnce(undefined)
+    const fetchMock = makeMockFetch()
+
+    await withFreshFactory(asFetch(fetchMock), async core => {
+      const {account} = await core.createSessionBundleAndResume(
+        makeAccount({accessJwt: 'valid-access-jwt'}),
+        jest.fn(),
+      )
+      expect(account.accessJwt).toBe('valid-access-jwt')
+      expect(account.refreshJwt).toBe('refresh-jwt')
+    })
+  })
+})

src/state/session/clients.ts

@@ -2,7 +2,11 @@ import {Client} from '@atproto/lex'
 import {type PasswordSession} from '@atproto/lex-password-session'
 import {api} from '@bsky.app/sdk'
 
-import {BLUESKY_PROXY_HEADER, PUBLIC_BSKY_SERVICE} from '#/lib/constants'
+import {
+  BLUESKY_PROXY_HEADER,
+  CHAT_PROXY_SERVICE,
+  PUBLIC_BSKY_SERVICE,
+} from '#/lib/constants'
 import {networkAwareFetch} from './session-core'
 
 /**
@@ -41,14 +45,16 @@ export function buildAccountClient(session: PasswordSession): Client {
 /**
  * Build the chat client over a {@link PasswordSession}.
  *
- * `api.chat.service` (`did:web:api.bsky.chat#bsky_chat`) is passed as the
- * client's `service`, so lex-client sets `atproto-proxy: did:web:api.bsky.chat#bsky_chat`
- * on every request. This is exactly what the old per-call `DM_SERVICE_HEADERS`
- * did, once and centrally, so `chat.bsky.*` calls are proxied to the chat
- * service.
+ * {@link CHAT_PROXY_SERVICE} (`${CHAT_PROXY_DID}#bsky_chat`, default
+ * `did:web:api.bsky.chat#bsky_chat`) is passed as the client's `service`, so
+ * lex-client sets `atproto-proxy: <that value>` on every request. This is
+ * exactly what the old per-call `DM_SERVICE_HEADERS` did, once and centrally, so
+ * `chat.bsky.*` calls are proxied to the chat service. It is read from the
+ * env-configurable `CHAT_PROXY_DID` (via `EXPO_PUBLIC_CHAT_PROXY_DID`) rather
+ * than the hard-coded SDK constant, restoring the old routing override.
  */
 export function buildChatClient(session: PasswordSession): Client {
-  return new Client(session, {service: api.chat.service})
+  return new Client(session, {service: CHAT_PROXY_SERVICE})
 }
 
 /** Thrown when a write/auth-only client is used with no active session. */

src/state/session/index.tsx

@@ -420,6 +420,26 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
           const prevBundle = state.currentAgentState.agent as unknown as
             | SessionBundle
             | PublicSessionBundle
+          /*
+           * Any change to ANY saved account fires persisted.onUpdate. The
+           * 'synced-accounts' dispatch above already keeps the accounts list
+           * fresh, so if the CURRENT account's tokens are unchanged there is
+           * nothing to rebuild - a change to a non-current account landed here.
+           * Bail out before rebuilding: rebuild+swap would kill the live bundle
+           * (client-identity churn, in-flight request kills) for no reason.
+           * Fall through to rebuild only when we have no usable live session.
+           */
+          const live =
+            prevBundle.session && !prevBundle.session.destroyed
+              ? prevBundle.session.session
+              : undefined
+          if (
+            live &&
+            live.accessJwt === syncedAccount.accessJwt &&
+            live.refreshJwt === syncedAccount.refreshJwt
+          ) {
+            return
+          }
           let newBundle!: SessionBundle
           const hooks = makeSessionHooks(
             onSessionChange,
@@ -435,30 +455,35 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
           hooks.arm()
           /*
            * Reapply this account's subscribed labelers to the freshly built
-           * appview client. buildBundle starts with an empty per-instance
-           * labeler set, and unlike login/resume/createAccount this rebuild path
-           * never ran configureModerationForAccount - so without this, subscribed
-           * labelers would silently drop from appview requests in follower tabs
-           * until the next full resume. readLabelers is a local storage read (no
-           * network), preserving the branch's no-network intent; setLabelers
-           * mutates the client's Set in place so labels reappear when it
-           * resolves.
+           * appview client - buildBundle starts with an empty per-instance
+           * labeler set, and unlike login/resume/createAccount this rebuild
+           * path never runs configureModerationForAccount on its own. The
+           * bundle swap is deferred until the labeler config resolves so the
+           * new bundle enters the reducer with its labelers already applied
+           * (matching the factories, which await moderation before returning).
+           * readLabelers is a local-storage read (microtask-scale, no network,
+           * preserving this branch's no-network intent), and the OLD bundle's
+           * access token stays valid throughout the deferral, so nothing
+           * regresses by waiting.
            */
           void configureModerationForAccount(newBundle, syncedAccount)
-          addSessionDebugLog({
-            type: 'agent:patch',
-            agent: newBundle,
-            prevSession:
-              prevBundle.session && !prevBundle.session.destroyed
-                ? prevBundle.session.session
-                : undefined,
-            nextSession: newBundle.session.session,
-          })
-          store.dispatch({
-            type: 'replaced-current-bundle',
-            newAgent: newBundle,
-            newAccount: syncedAccount,
-          })
+            .catch(() => {})
+            .finally(() => {
+              addSessionDebugLog({
+                type: 'agent:patch',
+                agent: newBundle,
+                prevSession:
+                  prevBundle.session && !prevBundle.session.destroyed
+                    ? prevBundle.session.session
+                    : undefined,
+                nextSession: newBundle.session.session,
+              })
+              store.dispatch({
+                type: 'replaced-current-bundle',
+                newAgent: newBundle,
+                newAccount: syncedAccount,
+              })
+            })
         }
       }
     })

src/state/session/session-core.ts

@@ -534,16 +534,32 @@ export async function createSessionBundleAndResume(
 
   bundle = buildBundle(session)
   registerBundleKillSwitch(bundle, hooks.kill)
-  const account =
+  /*
+   * Early snapshot: only used to configure moderation below (its handle/did are
+   * refresh-stable). The RETURNED account is re-snapshotted after the prep
+   * awaits (see below).
+   */
+  const earlyAccount =
     sessionDataToSessionAccount(session.session, session.session.service) ??
     storedAccount
 
-  const moderation = configureModerationForAccount(bundle, account)
+  const moderation = configureModerationForAccount(bundle, earlyAccount)
   const aa = prefetchAgeAssuranceServerData({
     appviewClient: bundle.appviewClient,
     accountClient: bundle.accountClient,
   })
   await Promise.all([gates, moderation, aa])
+  /*
+   * Re-snapshot AFTER prep, right before arm(). A 401 during a prep request
+   * (e.g. the AA prefetch) triggers PasswordSession's internal auto-refresh,
+   * which rotates both tokens; its onUpdated is dropped by the still-disarmed
+   * latch. Snapshotting the returned account here (not before prep) ensures we
+   * persist the fresh refreshJwt rather than a stale one that is dead on the
+   * next cold start.
+   */
+  const account =
+    sessionDataToSessionAccount(session.session, session.session.service) ??
+    storedAccount
   hooks.arm()
   return {account, bundle}
 }
@@ -587,16 +603,28 @@ export async function createSessionBundleAndLogin(
 
   bundle = buildBundle(session)
   registerBundleKillSwitch(bundle, hooks.kill)
-  const account = sessionDataToSessionAccountOrThrow(session)
-  accountDid = account.did
+  /*
+   * Early snapshot: needed now to seed `accountDid` (the getDid closure the
+   * hooks read). The RETURNED account is re-snapshotted after the prep awaits.
+   */
+  const earlyAccount = sessionDataToSessionAccountOrThrow(session)
+  accountDid = earlyAccount.did
 
   const gates = features.refresh({strategy: 'prefer-fresh-gates'})
-  const moderation = configureModerationForAccount(bundle, account)
+  const moderation = configureModerationForAccount(bundle, earlyAccount)
   const aa = prefetchAgeAssuranceServerData({
     appviewClient: bundle.appviewClient,
     accountClient: bundle.accountClient,
   })
   await Promise.all([gates, moderation, aa])
+  /*
+   * Re-snapshot AFTER prep, right before arm(). A 401 during a prep request
+   * triggers PasswordSession's internal auto-refresh, which rotates both tokens
+   * and fires an onUpdated the disarmed latch drops; snapshotting here persists
+   * the fresh refreshJwt. If the session was destroyed mid-prep, OrThrow throws
+   * (login effectively failed).
+   */
+  const account = sessionDataToSessionAccountOrThrow(session)
   hooks.arm()
   return {account, bundle}
 }
@@ -655,11 +683,17 @@ export async function createSessionBundleAndCreateAccount(
 
   bundle = buildBundle(session)
   registerBundleKillSwitch(bundle, hooks.kill)
-  const account = sessionDataToSessionAccountOrThrow(session)
-  accountDid = account.did
+  /*
+   * Early snapshot: needed now to seed `accountDid` and for the DID/handle used
+   * across the local writes and deferred server writes below (all
+   * refresh-stable). The RETURNED account is re-snapshotted after the prep
+   * awaits.
+   */
+  const earlyAccount = sessionDataToSessionAccountOrThrow(session)
+  accountDid = earlyAccount.did
 
   const gates = features.refresh({strategy: 'prefer-fresh-gates'})
-  const moderation = configureModerationForAccount(bundle, account)
+  const moderation = configureModerationForAccount(bundle, earlyAccount)
 
   const createdAt = toDatetimeString(new Date())
   const birthdate = birthDate.toISOString()
@@ -670,9 +704,9 @@ export async function createSessionBundleAndCreateAccount(
    * to the server in the next step, so on subsequent reloads, the server will
    * be the source of truth.
    */
-  setCreatedAtForDid({did: account.did, createdAt})
-  setBirthdateForDid({did: account.did, birthdate})
-  snoozeBirthdateUpdateAllowedForDid(account.did)
+  setCreatedAtForDid({did: earlyAccount.did, createdAt})
+  setBirthdateForDid({did: earlyAccount.did, birthdate})
+  snoozeBirthdateUpdateAllowedForDid(earlyAccount.did)
   // do this last
   const aa = prefetchAgeAssuranceServerData({
     appviewClient: bundle.appviewClient,
@@ -725,7 +759,7 @@ export async function createSessionBundleAndCreateAccount(
       }),
       // wait for AA data to load first, then check state
       aa.then(() => {
-        const {flags} = unsafeGetAndComputeAgeAssurance({did: account.did})
+        const {flags} = unsafeGetAndComputeAgeAssurance({did: earlyAccount.did})
         if (flags?.chatDisabled || flags?.groupChatDisabled) {
           void restrictChatSettings({
             client: bundle.accountClient,
@@ -786,6 +820,14 @@ export async function createSessionBundleAndCreateAccount(
   }
 
   await Promise.all([gates, moderation, aa])
+  /*
+   * Re-snapshot AFTER prep, right before arm(). A 401 during a prep request
+   * triggers PasswordSession's internal auto-refresh, which rotates both tokens
+   * and fires an onUpdated the disarmed latch drops; snapshotting here persists
+   * the fresh refreshJwt rather than a stale one. If the session was destroyed
+   * mid-prep, OrThrow throws.
+   */
+  const account = sessionDataToSessionAccountOrThrow(session)
   hooks.arm()
   return {account, bundle}
 }