fix(router): keep the logged-in screens mounted through an account switch

CLAUDE.md

@@ -17,6 +17,7 @@
 - After editing `protocol/avdl/` or `protocol/bin/enabled-calls.json`: from `protocol/`, run `node ./bin/generate-ts.ts && cp ./js/rpc*.tsx ../shared/constants/rpc` and commit the regenerated `shared/constants/rpc/rpc-gen.tsx`.
 - Never hand-edit generated code (rpc-gen, protocol output, mocks, codegen'd files of any kind). Edit the source it's generated from and rerun the generator. CI regenerates and fails on any diff.
 - When updating `electron`: run `shared/desktop/extract-electron-shasums.sh <version>`.
+- Keep an open PR's description in step with its branch. Whenever new commits change what the PR does or how (a new fix, a changed approach, a removed piece, new tests or evidence), rewrite the affected sections with `gh pr edit --body-file`, and the title if the scope moved. It should read as a description of the current diff, not a changelog. Skip it for commits that don't change the story (lint, renames, test placeholders).
 - Never patch `react-native` itself (patch-package or node_modules edits): we use prebuilt RN core and don't compile its source, so native-side patches never take effect. Work around RN core bugs in app code.
 
 ## Working Directory
@@ -29,4 +30,4 @@ Repo root is `client/`. TS source lives in `shared/`. Always use absolute paths
 ## Validation
 After TS changes (from `shared/`): `yarn lint:all` (= `yarn lint` && `yarn lint:bailouts` && `yarn tsc`). Plain `yarn lint` is eslint only and does NOT catch react-compiler bailouts — no compiler rule is wired into `eslint.config.mjs`, so bailouts only surface via `lint:bailouts`. `lint:bailouts` also flags components the compiler cannot name (an `isMobile ? arrow : arrow` ternary is never compiled at all, so nothing in it is memoized — name both branches instead), and memo scopes keyed on the whole props object (a `props.x` read inside a callback, or a destructure below one, makes the compiler key on `props` itself, so the cache never hits — read every prop through one destructure at the top, above every callback). Repo baseline is 0 bailouts and 0 whole-props deps; keep it there. When debugging visually, skip until fix is confirmed. Never delete the ESLint cache.
 
-Before reporting any TS change complete: run `yarn lint:all` and get it clean. Do NOT run `/code-review` while iterating, building, testing, or debugging — only once the change is about to be pushed (commit for a PR, push, or open a PR). At that point, if the diff has real logic in it, run `/code-review high` against the diff and fix what it finds; if a finding is wrong, say why instead of applying it. Skip the review for trivial diffs (a config/JSON line, a codegen resync, a typo) and say you skipped it.
+Before reporting any TS change complete: run `yarn lint:all` and get it clean. Do NOT run `/code-review` while iterating, building, testing, or debugging — only once the change is about to be pushed (commit for a PR, push, or open a PR). At that point, first get both `yarn lint:all` and `yarn test:unit` passing — never review, push, or open a PR with either failing. Then, if the diff has real logic in it, run `/code-review high` against the diff and fix what it finds; if a finding is wrong, say why instead of applying it. Skip the review for trivial diffs (a config/JSON line, a codegen resync, a typo) and say you skipped it.

shared/chat/conversation/input-area/input-state.test.tsx

@@ -962,3 +962,85 @@ test('a commandStatus written while the provider is frozen is applied on thaw',
 
   expect(inputState?.commandStatus).toEqual(commandStatusInfo)
 })
+
+describe('a pending draft save', () => {
+  const typeThenWait = (switchAccount: boolean) => {
+    jest.useFakeTimers()
+    try {
+      const saveDraft = jest.spyOn(T.RPCChat, 'localUpdateUnsentTextRpcPromise').mockResolvedValue(undefined)
+      jest.spyOn(T.RPCChat, 'localUpdateTypingRpcPromise').mockResolvedValue(undefined)
+      renderComposer()
+      act(() => {
+        mockPlatformInputProps?.onChangeText('a')
+      })
+      // inside the 200ms throttle, so this save waits for its trailing edge
+      act(() => {
+        mockPlatformInputProps?.onChangeText('ab')
+      })
+      if (switchAccount) {
+        act(() => {
+          useCurrentUserState.getState().dispatch.setBootstrap({
+            deviceID: 'device-id-2',
+            deviceName: 'test-device-2',
+            uid: 'uid-2',
+            username: 'testuser-mac',
+          })
+        })
+      }
+      act(() => {
+        jest.advanceTimersByTime(250)
+      })
+      return saveDraft.mock.calls.map(c => c[0].text)
+    } finally {
+      jest.useRealTimers()
+    }
+  }
+
+  test('is saved for the account that typed it', () => {
+    expect(typeThenWait(false)).toContain('ab')
+  })
+
+  test('is not saved for the next account when a switch lands first', () => {
+    expect(typeThenWait(true)).not.toContain('ab')
+  })
+})
+
+describe('a draft typed just before leaving the conversation', () => {
+  const typeThenUnmount = (switchAccount: boolean) => {
+    jest.useFakeTimers()
+    try {
+      const saveDraft = jest.spyOn(T.RPCChat, 'localUpdateUnsentTextRpcPromise').mockResolvedValue(undefined)
+      jest.spyOn(T.RPCChat, 'localUpdateTypingRpcPromise').mockResolvedValue(undefined)
+      const {unmount} = renderComposer()
+      act(() => {
+        mockPlatformInputProps?.onChangeText('a')
+      })
+      // inside the 200ms throttle, so this save is still pending at unmount
+      act(() => {
+        mockPlatformInputProps?.onChangeText('ab')
+      })
+      if (switchAccount) {
+        act(() => {
+          useCurrentUserState.getState().dispatch.setBootstrap({
+            deviceID: 'device-id-2',
+            deviceName: 'test-device-2',
+            uid: 'uid-2',
+            username: 'testuser-mac',
+          })
+        })
+      }
+      unmount()
+      return saveDraft.mock.calls.map(c => c[0].text)
+    } finally {
+      jest.useRealTimers()
+    }
+  }
+
+  test('is saved when the composer unmounts', () => {
+    expect(typeThenUnmount(false)).toContain('ab')
+  })
+
+  test('is not saved for the next account when the unmount comes from a switch', () => {
+    expect(typeThenUnmount(true)).not.toContain('ab')
+  })
+})

shared/chat/conversation/input-area/normal/index.tsx

@@ -241,7 +241,13 @@ const ConnectedPlatformInput = function ConnectedPlatformInput() {
   // throttled draft-save path rather than from onChangeText, so the composer does not
   // re-render on every keystroke. The preview debounces another 500ms downstream anyway.
   const [previewText, setPreviewText] = React.useState('')
+  // The account this composer was mounted for. After an account switch the service saves drafts
+  // for the next account, so the unmount flush of a draft typed here must not save it there.
+  const [composerUid] = React.useState(() => useCurrentUserState.getState().uid)
   const updateDraftRaw = (text: string) => {
+    if (useCurrentUserState.getState().uid !== composerUid) {
+      return
+    }
     // Immediately update local meta.draft so switching back to this thread
     // before the async unbox completes won't re-inject the old stale draft.
     // Merges from the current meta (same inbox version), so force past gating.
@@ -259,13 +265,8 @@ const ConnectedPlatformInput = function ConnectedPlatformInput() {
     }
     C.ignorePromise(f())
   }
-  const updateDraft = C.useThrottledCallback(updateDraftRaw, 200, {trailing: true})
-  // Flush any pending draft save before cancel fires on unmount (hooks cleanup runs in reverse order)
-  React.useLayoutEffect(() => {
-    return () => {
-      updateDraft.flush()
-    }
-  }, [updateDraft])
+  // flushOnUnmount: leaving the conversation must still save what was typed in the last 200ms
+  const updateDraft = C.useThrottledCallback(updateDraftRaw, 200, {flushOnUnmount: true, trailing: true})
 
   const textValueRef = React.useRef('')
   const onChangeText = (text: string) => {

shared/chat/conversation/thread-context.test.tsx

@@ -246,6 +246,7 @@ const separatePlainThreadWrapper = ({children}: {children: React.ReactNode}) =>
 )
 
 beforeEach(() => {
+  useConfigState.setState({loggedIn: true})
   useCurrentUserState.getState().dispatch.setBootstrap({
     deviceID: 'device-id',
     deviceName: 'test-device',
@@ -844,6 +845,48 @@ test('active change marks read after an eligible mounted thread load', async ()
   })
 })
 
+test('a thread still on screen after an account switch does not mark read for the next account', async () => {
+  useConfigState.setState({loggedIn: true})
+  useShellState.getState().dispatch.setActive(false)
+  jest
+    .spyOn(Common, 'isUserActivelyLookingAtThisThread')
+    .mockImplementation(() => useShellState.getState().active)
+  const markAsRead = jest
+    .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise')
+    .mockResolvedValue({offline: false})
+  jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => {
+    p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({
+      thread: JSON.stringify({
+        messages: [makeValidTextUIMessage(T.Chat.numberToMessageID(603), 'loaded inactive')],
+        pagination: {last: true, next: '', num: 100, previous: ''},
+      }),
+    })
+    await Promise.resolve()
+    return {offline: false}
+  })
+  const {result} = renderHook(() => useConversationThreadLoadMoreMessages(), {wrapper})
+  act(() => {
+    result.current({reason: 'tab selected'})
+  })
+  await act(async () => {
+    await flushPromises()
+  })
+
+  act(() => {
+    useCurrentUserState.getState().dispatch.setBootstrap({
+      deviceID: 'device-id-2',
+      deviceName: 'test-device-2',
+      uid: 'uid-2',
+      username: 'testuser-mac',
+    })
+    useShellState.getState().dispatch.setActive(true)
+  })
+  await act(async () => {
+    await flushPromises()
+  })
+  expect(markAsRead).not.toHaveBeenCalled()
+})
+
 test('active change does not mark read after a centered thread load', async () => {
   useConfigState.setState({loggedIn: true})
   useShellState.getState().dispatch.setActive(false)

shared/chat/conversation/thread-context.tsx

@@ -403,6 +403,9 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) =>
   const lookingAtThread = active && appFocused && routeFocused
   const previousLookingAtThreadRef = React.useRef(lookingAtThread)
   const activeMarkReadEnabledRef = React.useRef(false)
+  // The account this thread was loaded for. Its screen outlives an account switch by a few renders,
+  // and a mark-read sent then would mark the next account's read position.
+  const [threadUid] = React.useState(() => useCurrentUserState.getState().uid)
   const markReadBlockedRef = React.useRef(false)
 
   const getSnapshot = React.useEffectEvent(() => threadStore.getState())
@@ -422,6 +425,10 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) =>
         logger.info('mark read bail on not logged in')
         return
       }
+      if (useCurrentUserState.getState().uid !== threadUid) {
+        logger.info('mark read bail on thread loaded for another account')
+        return
+      }
       if (!T.Chat.isValidConversationIDKey(id)) {
         logger.info('mark read bail on no selected conversation')
         return

shared/chat/conversation/thread-load-status-context.test.tsx

@@ -5,6 +5,7 @@ import type * as React from 'react'
 import * as T from '@/constants/types'
 import {notifyEngineActionListeners} from '@/engine/action-listener'
 import {resetAllStores} from '@/util/zustand'
+import {useConfigState} from '@/stores/config'
 import {useCurrentUserState} from '@/stores/current-user'
 import {
   ConversationThreadLoadStatusProvider,
@@ -25,6 +26,7 @@ const flushPromises = async () => {
 
 beforeEach(() => {
   jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockResolvedValue(undefined)
+  useConfigState.setState({loggedIn: true})
   useCurrentUserState.getState().dispatch.setBootstrap({
     deviceID: 'device-id',
     deviceName: 'test-device',

shared/chat/inbox/engine.test.tsx

@@ -4,6 +4,7 @@ import {resetAllStores} from '@/util/zustand'
 import {handleConvoEngineIncoming} from './engine'
 import {getInboxConversationMeta, getInboxConversationParticipants} from './metadata'
 import {useConfigState} from '@/stores/config'
+import {useCurrentUserState} from '@/stores/current-user'
 import {updateInboxTyping} from '@/chat/inbox/typing-state'
 
 jest.mock('@/chat/inbox/badge-state', () => ({
@@ -260,6 +261,12 @@ test('global message activity routing preserves returned global data', () => {
 
 test('read message activity without attached inbox item refreshes service-owned metadata', () => {
   useConfigState.setState({loggedIn: true})
+  useCurrentUserState.getState().dispatch.setBootstrap({
+    deviceID: 'device-id',
+    deviceName: 'test-device',
+    uid: 'uid',
+    username: 'alice',
+  })
   const unbox = jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockResolvedValue(undefined)
 
   expect(

shared/chat/inbox/metadata.test.tsx

@@ -330,7 +330,7 @@ test('setUserSwitching abandons further unbox until switch completes', async ()
   await flushPromises()
   expect(T.RPCChat.localRequestInboxUnboxRpcPromise).toHaveBeenCalledTimes(1)
 
-  useConfigState.getState().dispatch.setUserSwitching(true)
+  useConfigState.getState().dispatch.setUserSwitching(true, 'testuser')
   resolvers[0]?.()
   await flushPromises()
 

shared/chat/inbox/use-inbox-state.test.ts

@@ -12,6 +12,7 @@ type MockInboxLayoutState = {
 }
 
 let mockInboxLayoutState: MockInboxLayoutState
+let mockConfigState = {loggedIn: true, userSwitching: false}
 
 jest.mock('@/constants', () => {
   const React = require('react')
@@ -60,7 +61,7 @@ jest.mock('./metadata', () => ({
 }))
 
 jest.mock('@/stores/config', () => ({
-  useConfigState: <T>(selector: (state: {loggedIn: boolean}) => T) => selector({loggedIn: true}),
+  useConfigState: <T>(selector: (state: typeof mockConfigState) => T) => selector(mockConfigState),
 }))
 
 jest.mock('@/stores/current-user', () => ({
@@ -85,6 +86,7 @@ let mockSetInboxRetriedOnCurrentEmpty: jest.Mock
 beforeEach(() => {
   mockLoadInboxNumSmallRows = jest.fn()
   mockInboxRefresh = jest.fn()
+  mockConfigState = {loggedIn: true, userSwitching: false}
   mockSetInboxRetriedOnCurrentEmpty = jest.fn()
   mockInboxLayoutState = {
     dispatch: {
@@ -140,3 +142,18 @@ test('useInboxState updates inbox row count without persisting when persist is f
   expect(result.current.inboxNumSmallRows).toBe(7)
   expect(T.RPCGen.configGuiSetValueRpcPromise).not.toHaveBeenCalled()
 })
+
+// The inbox RPCs refuse to run while a switch is under way, so the first load has to wait for it.
+test('useInboxState loads the inbox once an account switch ends', () => {
+  mockInboxRefresh.mockReturnValue(Promise.resolve())
+  mockInboxLayoutState.hasLoaded = false
+  mockConfigState = {loggedIn: true, userSwitching: true}
+  const {rerender} = renderHook(() => useInboxState())
+  // what fired on mount ran into the switch and was refused
+  mockInboxRefresh.mockClear()
+
+  mockConfigState = {loggedIn: true, userSwitching: false}
+  rerender()
+
+  expect(mockInboxRefresh).toHaveBeenCalledWith('componentNeverLoaded')
+})

shared/chat/inbox/use-inbox-state.tsx

@@ -62,7 +62,9 @@ export function useInboxState(
   refreshInbox?: T.Chat.ChatRootInboxRefresh
 ) {
   const isFocused = useIsFocused()
-  const loggedIn = useConfigState(s => s.loggedIn)
+  // Matches isChatSessionReady, which gates the inbox RPCs: loads skipped while a switch runs have
+  // to fire again once it ends.
+  const sessionReady = useConfigState(s => s.loggedIn && !s.userSwitching)
   const username = useCurrentUserState(s => s.username)
   const loadInboxNumSmallRows = C.useRPC(T.RPCGen.configGuiGetValueRpcPromise)
 
@@ -130,14 +132,14 @@ export function useInboxState(
   })
 
   React.useEffect(() => {
-    const ready = loggedIn && !!username && (!isMobile || isFocused)
+    const ready = sessionReady && !!username && (!isMobile || isFocused)
     if (!ready || !refreshInbox || handledRefreshNonceRef.current === refreshInbox.nonce) {
       return
     }
     handledRefreshNonceRef.current = refreshInbox.nonce
     C.ignorePromise(inboxRefresh(refreshInbox.reason))
     C.Router2.setChatRootParams({refreshInbox: undefined})
-  }, [inboxRefresh, isFocused, loggedIn, refreshInbox, username])
+  }, [inboxRefresh, isFocused, sessionReady, refreshInbox, username])
 
   C.Router2.useSafeFocusEffect(
     React.useCallback(() => {
@@ -148,15 +150,15 @@ export function useInboxState(
   )
 
   React.useEffect(() => {
-    const ready = loggedIn && !!username
+    const ready = sessionReady && !!username
     const shouldRetry = !inboxHasLoaded && ready && (!isMobile || isFocused)
     if (shouldRetry) {
       C.ignorePromise(inboxRefresh('componentNeverLoaded'))
     }
-  }, [inboxHasLoaded, inboxRefresh, isFocused, loggedIn, username])
+  }, [inboxHasLoaded, inboxRefresh, isFocused, sessionReady, username])
 
   React.useEffect(() => {
-    const ready = loggedIn && !!username
+    const ready = sessionReady && !!username
     if (!ready) {
       return
     }
@@ -200,10 +202,10 @@ export function useInboxState(
         inboxNumSmallRowsLoadVersionRef.current++
       }
     }
-  }, [inboxNumSmallRowsLoaded, loadInboxNumSmallRows, loggedIn, username])
+  }, [inboxNumSmallRowsLoaded, loadInboxNumSmallRows, sessionReady, username])
 
   React.useEffect(() => {
-    const ready = loggedIn && !!username && (!isMobile || isFocused)
+    const ready = sessionReady && !!username && (!isMobile || isFocused)
     if (!ready || isSearching || !inboxHasLoaded || inboxRows.length > 0 || inboxRetriedOnCurrentEmpty) {
       return
     }
@@ -216,7 +218,7 @@ export function useInboxState(
     inboxRows.length,
     isFocused,
     isSearching,
-    loggedIn,
+    sessionReady,
     setRetriedOnCurrentEmpty,
     username,
   ])

shared/constants/init/index.tsx

@@ -14,7 +14,7 @@ import logger from '@/logger'
 import {getEngine} from '@/engine'
 import {afterKbfsDaemonRpcStatusChanged} from '@/fs/common/lifecycle'
 import {logState, setThreadInputCommandStatus} from '@/constants/router'
-import {initSharedSubscriptions, _onEngineIncoming, onEngineConnected as onSharedEngineConnected} from './shared'
+import {initSharedSubscriptions, _onEngineIncoming} from './shared'
 import {noConversationIDKey} from '../types/chat/common'
 import {dumpLogs, persistRoute} from '@/util/storeless-actions'
 
@@ -362,21 +362,6 @@ export const onEngineIncoming = (action: EngineGen.Actions) => {
           .dispatch.setOutOfDate({critical: true, message: upgradeMsg, outOfDate: true, updating: false})
         break
       }
-      case 'keybase.1.NotifySession.loggedOut': {
-        if (useConfigState.getState().userSwitching) {
-          logger.info('Resetting renderer engine for account switch logout')
-          getEngine().reset()
-        }
-        break
-      }
-      case 'keybase.1.NotifySession.loggedIn': {
-        if (useConfigState.getState().userSwitching) {
-          logger.info('Refreshing renderer session registration for account switch login')
-          getEngine().reset()
-          onSharedEngineConnected()
-        }
-        break
-      }
       default:
     }
   }