Handle cross-tab resume failures

plans/current-review.md

@@ -75,7 +75,9 @@ This is not a lock race; it is a false positive in the deliberate conditional me
 
 **Severity: Low**
 
-At `index.tsx:740`, the cross-tab listener calls:
+**Status: Resolved.** The detached cross-tab resume now catches and logs failures with `safeMessage`; a focused provider test covers a rejected session factory.
+
+At `index.tsx:740`, the cross-tab listener originally called:
 
 ```ts
 void resumeSession(syncedAccount)
@@ -83,7 +85,7 @@ void resumeSession(syncedAccount)
 
 The pattern existed on `main`, where the resume factory was its main rejection source. This branch adds persistence rejection because `resumeSession` now awaits `store.dispatch`, which awaits `writeSession`. A storage failure can therefore become an unhandled promise rejection.
 
-**Minimal fix:** Attach `.catch(...)` and log the failure.
+**Resolution:** Attach `.catch(...)` and log the failure.
 
 ### B4 - Login-shaped methods reject after committing reducer state
 
@@ -154,12 +156,11 @@ Add a `refreshSession` test with a rejecting `writeSession` to pin the intended
 
 ## Remaining recommended order
 
-1. Fix B3 by catching and logging the cross-tab listener's `resumeSession` rejection.
+1. Decide and document the B4 persistence-failure policy for login-shaped methods.
-2. Decide and document the B4 persistence-failure policy for login-shaped methods.
+2. Add a `refreshSession` test with rejecting `writeSession`.
-3. Add a `refreshSession` test with rejecting `writeSession`.
 
 ## Verdict
 
 The core async design is sound. Serialization through `PasswordSession.#sessionPromise`, the per-bundle error channel, arm/kill lifecycle, reducer identity guards, and internally owned persistence lock compose correctly in the reviewed interleavings.
 
-The genuine state-corruption hole from B1 is now closed. B2 is accepted, documented, instrumented, and covered as a conditional-merge tradeoff. B3 and B4 remain lower-severity error-policy issues.
+The genuine state-corruption hole from B1 is now closed. B2 is accepted, documented, instrumented, and covered as a conditional-merge tradeoff. B3 is closed; B4 remains as the final lower-severity error-policy issue.

src/state/session/__tests__/provider-session-events-test.tsx

@@ -64,9 +64,16 @@ jest.mock('#/state/util', () => ({useCloseAllActiveElements: () => () => {}}))
 jest.mock('#/components/dialogs/Context', () => ({
   useGlobalDialogsControlContext: () => ({signinDialogControl: {open() {}}}),
 }))
+const mockAnalyticsLoggerError = jest.fn()
 jest.mock('#/analytics', () => ({
   AnalyticsContext: ({children}: {children: React.ReactNode}) => children,
-  useAnalyticsBase: () => ({metric() {}, logger: {debug() {}, error() {}}}),
+  useAnalyticsBase: () => ({
+    metric() {},
+    logger: {
+      debug() {},
+      error: (...args: unknown[]) => mockAnalyticsLoggerError(...args),
+    },
+  }),
   utils: {accountToSessionMetadata: () => ({}), useMeta: () => undefined},
 }))
 jest.mock('#/state/shell/onboarding', () => ({
@@ -145,6 +152,7 @@ import {
 import {type SessionApiContext} from '#/state/session/types'
 
 const DID = 'did:plc:example123'
+const OTHER_DID = 'did:plc:other456'
 const SERVICE = 'https://bsky.social/'
 
 function makeAccount(overrides: Partial<SessionAccount> = {}): SessionAccount {
@@ -299,6 +307,7 @@ beforeEach(() => {
   mockRebuild.mockClear()
   mockDisposeBundle.mockReset()
   mockEmitSessionDropped.mockReset()
+  mockAnalyticsLoggerError.mockReset()
 })
 
 /*
@@ -550,6 +559,29 @@ function emitSynced(session: Schema['session']) {
  * against the store having advanced underneath it.
  */
 describe('cross-tab sync', () => {
+  it('logs a failed resume for an account selected by another tab', async () => {
+    const account = makeAccount()
+    const bundle = makeBundle(account)
+    await renderLoggedIn(account, bundle)
+    const otherAccount = makeAccount({
+      did: OTHER_DID,
+      handle: 'bob.test',
+    })
+    const error = new Error('resume failed')
+    mockResume.mockRejectedValueOnce(error)
+
+    await act(async () => {
+      emitSynced({accounts: [otherAccount], currentAccount: otherAccount})
+      await Promise.resolve()
+      await Promise.resolve()
+    })
+
+    expect(mockAnalyticsLoggerError).toHaveBeenCalledWith(
+      'Failed to resume session from persisted update',
+      {safeMessage: error},
+    )
+  })
+
   it('short-circuits an update carrying the tokens the live session already has', async () => {
     const account = makeAccount()
     const bundle = makeBundle(account)

src/state/session/index.tsx

@@ -747,7 +747,11 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       if (syncedAccount && syncedAccount.refreshJwt) {
         if (syncedAccount.did !== state.currentBundleState.did) {
           // The leader refreshes before broadcasting, so followers receive fresh tokens.
-          void resumeSession(syncedAccount)
+          void resumeSession(syncedAccount).catch(error => {
+            ax.logger.error('Failed to resume session from persisted update', {
+              safeMessage: error,
+            })
+          })
         } else {
           /*
            * PasswordSession cannot be patched in place. Rebuild from the tokens
@@ -806,7 +810,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         }
       }
     })
-  }, [store, state, resumeSession, onSessionChange, cancelPendingTask])
+  }, [store, state, resumeSession, onSessionChange, cancelPendingTask, ax])
 
   const stateContext = useMemo(
     () => ({