address review feedback across the stack

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

src/lib/lexClient.ts

@@ -40,7 +40,12 @@ export function createLexClient(
  * Requests use PLAIN `fetch`, not `networkAwareFetch`: the host is untrusted
  * input, and a typo'd or dead service must not be reported as the app losing
  * network reachability.
+ *
+ * `appLabelers: null` suppresses the global `Client.appLabelers` static: these
+ * are `com.atproto.server` calls to a host the user typed, which have no use
+ * for moderation labels, and the header would disclose the app's configured
+ * moderation authorities to an arbitrary third-party server.
  */
 export function createServiceClient(service: string): Client {
-  return createLexClient({service})
+  return createLexClient({service}, {appLabelers: null})
 }

src/screens/Login/LoginForm.tsx

@@ -169,7 +169,9 @@ export const LoginForm = ({
           )
         } else {
           logger.warn('Failed to login', {error: errMsg})
-          setError(cleanError(errMsg))
+          /* the error object, not its stringification: cleanError only
+           * extracts the clean server message from a live LexError */
+          setError(cleanError(err))
         }
       }
     }

src/screens/Signup/state.ts

@@ -354,7 +354,6 @@ export function useSubmitSignup() {
         onboardingDispatch({type: 'start'})
       } catch (err) {
         const e = err as Error
-        let errMsg = e.toString()
         if (
           matchXrpcError(e, com.atproto.server.createAccount) ===
           'InvalidInviteCode'
@@ -368,7 +367,9 @@ export function useSubmitSignup() {
           return
         }
 
-        const error = cleanError(errMsg)
+        /* the error object, not its stringification: cleanError only extracts
+         * the clean server message from a live LexError */
+        const error = cleanError(e)
         const isHandleError = error.toLowerCase().includes('handle')
 
         dispatch({type: 'setIsLoading', value: false})

src/state/queries/profile.ts

@@ -206,21 +206,21 @@ export function useProfileUpdateMutation() {
         appviewClient,
         profile.did,
         checkCommitted ||
-          (profile => {
+          (fresh => {
             if (typeof newUserAvatar !== 'undefined') {
-              if (newUserAvatar === null && profile.avatar) {
+              if (newUserAvatar === null && fresh.avatar) {
                 // url hasn't cleared yet
                 return false
-              } else if (profile.avatar === profile.avatar) {
+              } else if (fresh.avatar === profile.avatar) {
                 // url hasn't changed yet
                 return false
               }
             }
             if (typeof newUserBanner !== 'undefined') {
-              if (newUserBanner === null && profile.banner) {
+              if (newUserBanner === null && fresh.banner) {
                 // url hasn't cleared yet
                 return false
-              } else if (profile.banner === profile.banner) {
+              } else if (fresh.banner === profile.banner) {
                 // url hasn't changed yet
                 return false
               }
@@ -229,8 +229,8 @@ export function useProfileUpdateMutation() {
               return true
             }
             return (
-              profile.displayName === updates.displayName &&
+              fresh.displayName === updates.displayName &&
-              profile.description === updates.description
+              fresh.description === updates.description
             )
           }),
       )

src/state/session/index.tsx

@@ -157,7 +157,19 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       sessionEvent: AtpSessionEvent,
       sessionData?: SessionData,
     ) => {
-      if (sessionEvent === 'update' && sessionData) {
+      /*
+       * Only the live bundle may reset the expiry-rescue bookkeeping: a stale
+       * bundle's late update would otherwise clear the failed-generation set
+       * that bounds the rescue loop. (Its dispatch below is separately dropped
+       * by the reducer's identity guard.)
+       */
+      if (
+        sessionEvent === 'update' &&
+        sessionData &&
+        (store.getState().currentBundleState.bundle as unknown as
+          | SessionBundle
+          | PublicSessionBundle) === bundle
+      ) {
         failedExpiryTokensRef.current.get(accountDid)?.clear()
       }
 
@@ -505,6 +517,18 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
     if (after === before) {
       throw new Error('Failed to refresh session')
     }
+    /*
+     * The user may have logged out or switched accounts while the refresh was
+     * in flight. Reporting success then would run the caller's success path
+     * (dialogs closing, success toasts, SignupQueued advancing) against an
+     * account that is no longer active, so a stale bundle rejects instead.
+     */
+    if (
+      (store.getState().currentBundleState.bundle as unknown) !==
+      (bundle as unknown)
+    ) {
+      throw new Error('The session changed while it was being refreshed')
+    }
     /*
      * The session's `onUpdated` hook dispatches the new tokens into the store,
      * but that lands a render away; this snapshot exposes them immediately.

src/state/session/session-core.ts

@@ -95,7 +95,14 @@ export function buildBundle(
   session: PasswordSession,
   storedPdsUrl?: string,
 ): SessionBundle {
-  const agent = storedPdsUrl
+  /*
+   * The stored url is persisted data and may be malformed (legacy writes,
+   * corruption). `routeSessionToPds` feeds it to `new URL()` on every request,
+   * so an invalid value would throw from every client call; discard it here
+   * and let the session route against its own service instead.
+   */
+  const agent =
+    storedPdsUrl && URL.canParse(storedPdsUrl)
       ? routeSessionToPds(session, storedPdsUrl)
       : session
   return {

src/state/session/session-data.ts

@@ -1,4 +1,3 @@
-import {getPdsEndpoint, isValidDidDoc} from '@atproto/common-web'
 import {type SessionData} from '@atproto/lex-password-session'
 import {jwtDecode} from 'jwt-decode'
 
@@ -7,6 +6,42 @@ import {isJwtExpired} from '#/lib/jwt'
 import {hasProp} from '#/lib/type-guards'
 import {type SessionAccount} from './types'
 
+/**
+ * The PDS endpoint declared by a DID document, or `undefined`.
+ *
+ * This deliberately mirrors `extractPdsUrl` in `@atproto/lex-password-session`,
+ * the predicate `PasswordSession` uses to route its own requests: the first
+ * service entry whose `id` ends with `#atproto_pds`, taking its
+ * `serviceEndpoint` if it parses as a URL. A stricter predicate here (schema
+ * validation, `type` checks) would let the PERSISTED `pdsUrl` disagree with
+ * the host the live session actually routes to, so the next cold start would
+ * seed the wrong endpoint.
+ */
+function extractPdsUrl(didDoc: SessionData['didDoc']): string | undefined {
+  const services = prop(didDoc, 'service')
+  if (!Array.isArray(services)) {
+    return undefined
+  }
+  const pds = services.find(service => {
+    const id = prop(service, 'id')
+    return typeof id === 'string' && id.endsWith('#atproto_pds')
+  })
+  const endpoint = prop(pds, 'serviceEndpoint')
+  return typeof endpoint === 'string' && URL.canParse(endpoint)
+    ? endpoint
+    : undefined
+}
+
+/**
+ * Read a property off an unknown value the way JS optional chaining would,
+ * without narrowing assumptions about the shape of a `LexMap`.
+ */
+function prop(value: unknown, key: string): unknown {
+  return typeof value === 'object' && value !== null
+    ? (value as Record<string, unknown>)[key]
+    : undefined
+}
+
 /** Whether an access token was issued for a queued (waitlisted) signup. */
 export function isSignupQueued(accessJwt: string | undefined) {
   if (accessJwt) {
@@ -40,11 +75,7 @@ export function sessionDataToSessionAccount(
     return undefined
   }
   const normalizedService = new URL(service).toString()
-  const didDocPdsUrl =
+  const pdsUrl = extractPdsUrl(session.didDoc) ?? storedPdsUrl
-    session.didDoc && isValidDidDoc(session.didDoc)
-      ? getPdsEndpoint(session.didDoc)
-      : undefined
-  const pdsUrl = didDocPdsUrl ?? storedPdsUrl
   return {
     service: normalizedService,
     did: session.did,