From 89f00658d256b90895921d150c02eae446538bfb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:08:40 -0700 Subject: [PATCH] fix(mobile): bound home host auto-connect fanout (#11642) * fix(mobile): bound home host auto-connect fanout * fix(mobile): clarify bounded host connection state * fix(mobile): close unused settings host clients * fix(mobile): close released host clients * fix(mobile): sync host release policy in effect * fix(mobile): preserve focused host client ownership * test(mobile): cover manual Home client release * Fix React Doctor array type check --- mobile/app/index.tsx | 24 +- mobile/app/terminal-settings.tsx | 4 +- mobile/app/voice-settings.tsx | 15 +- mobile/src/transport/client-context.test.ts | 178 ++++ mobile/src/transport/client-context.tsx | 190 +++-- .../transport/home-host-auto-connect.test.ts | 56 ++ .../src/transport/home-host-auto-connect.ts | 24 + .../host-client-acquisition-count.ts | 9 + .../transport/host-client-context-state.ts | 72 ++ .../settings-host-client-connections.ts | 19 + .../settings-host-client-lifecycle.test.ts | 776 ++++++++++++++++++ mobile/src/transport/use-all-host-clients.ts | 142 +++- 12 files changed, 1372 insertions(+), 137 deletions(-) create mode 100644 mobile/src/transport/home-host-auto-connect.test.ts create mode 100644 mobile/src/transport/home-host-auto-connect.ts create mode 100644 mobile/src/transport/host-client-acquisition-count.ts create mode 100644 mobile/src/transport/host-client-context-state.ts create mode 100644 mobile/src/transport/settings-host-client-connections.ts create mode 100644 mobile/src/transport/settings-host-client-lifecycle.test.ts diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 1a4681565..70da22348 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -27,6 +27,10 @@ import { createHostConnectRefetchGate } from '../src/transport/host-connect-refe import { sendSingleFlightRequest } from '../src/transport/request-single-flight' import { useCloseHost, useForceReconnect, usePrimeHosts } from '../src/transport/client-context' import { useAllHostClients } from '../src/transport/use-all-host-clients' +import { + resolveHomeHostConnectionState, + selectHomeAutoConnectHostIds +} from '../src/transport/home-host-auto-connect' import { classifyConnection } from '../src/transport/connection-health' import { subscribeToDesktopNotifications } from '../src/notifications/mobile-notifications' import { @@ -247,7 +251,11 @@ export default function HomeScreen() { const hostIds = useMemo(() => hosts.map((h) => h.id), [hosts]) // Why: scoped to the paired hosts so an unpaired desktop's cached reply leaves the header total. const stats = useMemo(() => totalHomeStats(statsByHost, hostIds), [statsByHost, hostIds]) - const allClients = useAllHostClients(hostIds) + const autoConnectHostIds = useMemo(() => selectHomeAutoConnectHostIds(hosts), [hosts]) + const allClients = useAllHostClients(hostIds, { + autoConnectHostIds, + closeUnusedOnRelease: true + }) const hostPaths = useMemo( () => Object.fromEntries(allClients.map(({ hostId, path }) => [hostId, path])), [allClients] @@ -745,7 +753,11 @@ export default function HomeScreen() { } ItemSeparatorComponent={CardGap} renderItem={({ item }) => { - const state = hostStates[item.id] ?? 'connecting' + const state = resolveHomeHostConnectionState( + item.id, + hostStates[item.id], + autoConnectHostIds + ) const attempts = hostAttempts[item.id] ?? 0 const lastConnectedAt = hostLastConnected[item.id] ?? null const verdict = classifyConnection({ @@ -948,7 +960,13 @@ export default function HomeScreen() { message={actionTarget ? endpointLabel(actionTarget.endpoint) : undefined} actions={getHostListActionSheetActions({ host: actionTarget, - state: actionTarget ? (hostStates[actionTarget.id] ?? 'connecting') : 'disconnected', + state: actionTarget + ? resolveHomeHostConnectionState( + actionTarget.id, + hostStates[actionTarget.id], + autoConnectHostIds + ) + : 'disconnected', hasEverConnected: actionTarget ? (hostLastConnected[actionTarget.id] ?? null) != null : false, diff --git a/mobile/app/terminal-settings.tsx b/mobile/app/terminal-settings.tsx index e6aa691c7..e55819aa2 100644 --- a/mobile/app/terminal-settings.tsx +++ b/mobile/app/terminal-settings.tsx @@ -12,7 +12,7 @@ import { ChevronLeft, ChevronRight, Smartphone, Type } from 'lucide-react-native import { colors, radii, spacing, typography } from '../src/theme/mobile-theme' import { loadHosts } from '../src/transport/host-store' import type { HostProfile } from '../src/transport/types' -import { useAllHostClients } from '../src/transport/use-all-host-clients' +import { useFocusedSettingsHostClients } from '../src/transport/settings-host-client-connections' import type { RpcClient } from '../src/transport/rpc-client' import { PickerModal, type PickerOption } from '../src/components/PickerModal' import { TerminalShortcutSettings } from '../src/components/TerminalShortcutSettings' @@ -127,7 +127,7 @@ export default function TerminalSettingsScreen() { void loadHosts().then(setHosts) }, []) const hostIds = useMemo(() => hosts.map((h) => h.id), [hosts]) - const hostClients = useAllHostClients(hostIds) + const { clients: hostClients } = useFocusedSettingsHostClients(hostIds) const hostClientsById = useMemo( () => new Map(hostClients.map((entry) => [entry.hostId, entry.client])), [hostClients] diff --git a/mobile/app/voice-settings.tsx b/mobile/app/voice-settings.tsx index a09d964b3..8648a1d38 100644 --- a/mobile/app/voice-settings.tsx +++ b/mobile/app/voice-settings.tsx @@ -9,12 +9,12 @@ import { View } from 'react-native' import { useSafeAreaInsets } from 'react-native-safe-area-context' -import { useFocusEffect, useRouter } from 'expo-router' +import { useRouter } from 'expo-router' import { ChevronLeft, ChevronRight } from 'lucide-react-native' import { colors, radii, spacing, typography } from '../src/theme/mobile-theme' import { loadHosts } from '../src/transport/host-store' import type { HostProfile } from '../src/transport/types' -import { useAllHostClients } from '../src/transport/use-all-host-clients' +import { useFocusedSettingsHostClients } from '../src/transport/settings-host-client-connections' import type { RpcClient } from '../src/transport/rpc-client' import { BottomDrawer } from '../src/components/BottomDrawer' import { VoiceModelList } from '../src/components/VoiceModelList' @@ -47,7 +47,7 @@ export default function VoiceSettingsScreen(): React.JSX.Element { void loadHosts().then(setHosts) }, []) const hostIds = useMemo(() => hosts.map((h) => h.id), [hosts]) - const hostClients = useAllHostClients(hostIds) + const { clients: hostClients, focused: routeFocused } = useFocusedSettingsHostClients(hostIds) // Voice dictation runs on the paired desktop, so pick the first connected host. const client: RpcClient | null = useMemo( () => hostClients.find((entry) => entry.state === 'connected')?.client ?? null, @@ -59,15 +59,6 @@ export default function VoiceSettingsScreen(): React.JSX.Element { const [error, setError] = useState(null) const [busyAction, setBusyAction] = useState(null) const [modelDrawerOpen, setModelDrawerOpen] = useState(false) - const [routeFocused, setRouteFocused] = useState(false) - - useFocusEffect( - useCallback(() => { - setRouteFocused(true) - return () => setRouteFocused(false) - }, []) - ) - const refresh = useCallback(async (): Promise => { if (!client) { return false diff --git a/mobile/src/transport/client-context.test.ts b/mobile/src/transport/client-context.test.ts index e7c00008e..b08899f53 100644 --- a/mobile/src/transport/client-context.test.ts +++ b/mobile/src/transport/client-context.test.ts @@ -21,6 +21,8 @@ vi.mock('./connection-revival-triggers', () => ({ })) import { RpcClientProvider, useCloseHost, useForceReconnect, useHostClient } from './client-context' +import { useAllHostClients } from './use-all-host-clients' +import { selectHomeAutoConnectHostIds } from './home-host-auto-connect' type FakeClient = RpcClient & { emitState: (state: ConnectionState) => void @@ -386,3 +388,179 @@ describe('useHostClient', () => { expect(connectMock).not.toHaveBeenCalled() }) }) + +describe('useAllHostClients', () => { + it('only opens the requested startup subset', async () => { + const host2 = { ...HOST, id: 'host-2', name: 'Host 2' } + connectMock.mockReturnValue(makeFakeClient('connected')) + loadHostsMock.mockResolvedValue([HOST, host2]) + + let renderer: ReactTestRenderer | null = null + function Probe(): null { + useAllHostClients([HOST.id, host2.id], { autoConnectHostIds: [host2.id] }) + return null + } + + const restore = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + await Promise.resolve() + }) + expect(connectMock).toHaveBeenCalledOnce() + expect(connectMock).toHaveBeenCalledWith(host2, expect.any(Function)) + } finally { + restore() + act(() => renderer?.unmount()) + } + }) + + it('keeps startup connection fanout constant for a large saved-host list', async () => { + const hosts = Array.from({ length: 1_000 }, (_, index) => ({ + ...HOST, + id: `host-${index}`, + name: `Host ${index}`, + lastConnected: index + })) + const hostIds = hosts.map((host) => host.id) + const autoConnectHostIds = selectHomeAutoConnectHostIds(hosts) + connectMock.mockReturnValue(makeFakeClient('connected')) + loadHostsMock.mockResolvedValue(hosts) + + let renderer: ReactTestRenderer | null = null + function Probe(): null { + useAllHostClients(hostIds, { autoConnectHostIds }) + return null + } + + const restore = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + await Promise.resolve() + }) + expect(connectMock).toHaveBeenCalledTimes(3) + expect(connectMock.mock.calls.map(([host]) => host.id)).toEqual([ + 'host-999', + 'host-998', + 'host-997' + ]) + } finally { + restore() + act(() => renderer?.unmount()) + } + }) + + it('closes a demoted Home client when the recent-host set rotates', async () => { + const hosts = [ + { ...HOST, id: 'host-a', lastConnected: 4 }, + { ...HOST, id: 'host-b', lastConnected: 3 }, + { ...HOST, id: 'host-c', lastConnected: 2 }, + { ...HOST, id: 'host-d', lastConnected: 1 } + ] + const clients = new Map() + connectMock.mockImplementation((profile: typeof HOST) => { + const client = makeFakeClient('connected') + clients.set(profile.id, client) + return client + }) + loadHostsMock.mockResolvedValue(hosts) + + let activeHostIds: string[] = [] + let renderer: ReactTestRenderer | null = null + function Probe({ profiles }: { profiles: typeof hosts }): null { + const hostIds = profiles.map((host) => host.id) + activeHostIds = useAllHostClients(hostIds, { + autoConnectHostIds: selectHomeAutoConnectHostIds(profiles), + closeUnusedOnRelease: true + }).map(({ hostId }) => hostId) + return null + } + + const restore = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create( + createElement(RpcClientProvider, null, createElement(Probe, { profiles: hosts })) + ) + await Promise.resolve() + }) + expect(activeHostIds.sort()).toEqual(['host-a', 'host-b', 'host-c']) + + const rotatedHosts = hosts.map((host) => + host.id === 'host-d' ? { ...host, lastConnected: 5 } : host + ) + await act(async () => { + renderer?.update( + createElement(RpcClientProvider, null, createElement(Probe, { profiles: rotatedHosts })) + ) + await Promise.resolve() + }) + + expect(connectMock).toHaveBeenCalledTimes(4) + expect(activeHostIds.sort()).toEqual(['host-a', 'host-b', 'host-d']) + expect(clients.get('host-a')?.closeMock).not.toHaveBeenCalled() + expect(clients.get('host-b')?.closeMock).not.toHaveBeenCalled() + expect(clients.get('host-c')?.closeMock).toHaveBeenCalledOnce() + expect(clients.get('host-d')?.closeMock).not.toHaveBeenCalled() + } finally { + restore() + act(() => renderer?.unmount()) + } + }) + + it('retains connect-all behavior when no startup subset is provided', async () => { + const host2 = { ...HOST, id: 'host-2', name: 'Host 2' } + connectMock.mockReturnValue(makeFakeClient('connected')) + loadHostsMock.mockResolvedValue([HOST, host2]) + + let renderer: ReactTestRenderer | null = null + function Probe(): null { + useAllHostClients([HOST.id, host2.id]) + return null + } + + const restore = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + await Promise.resolve() + }) + expect(connectMock).toHaveBeenCalledTimes(2) + } finally { + restore() + act(() => renderer?.unmount()) + } + }) + + it('allows an excluded host to connect manually', async () => { + connectMock.mockReturnValue(makeFakeClient('connected')) + loadHostsMock.mockResolvedValue([HOST]) + + let reconnect: ((hostId: string) => Promise) | null = null + let renderer: ReactTestRenderer | null = null + function Probe(): null { + useAllHostClients([HOST.id], { autoConnectHostIds: [] }) + reconnect = useForceReconnect() + return null + } + + const restore = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + }) + expect(connectMock).not.toHaveBeenCalled() + if (!reconnect) { + throw new Error('reconnect harness did not initialize') + } + await act(async () => { + await reconnect?.(HOST.id) + }) + expect(connectMock).toHaveBeenCalledOnce() + } finally { + restore() + act(() => renderer?.unmount()) + } + }) +}) diff --git a/mobile/src/transport/client-context.tsx b/mobile/src/transport/client-context.tsx index 8c00a11b2..18ee0b778 100644 --- a/mobile/src/transport/client-context.tsx +++ b/mobile/src/transport/client-context.tsx @@ -14,9 +14,20 @@ import type { RpcClient } from './rpc-client' import { connectionLogStore } from './connection-log-buffer' import { subscribeConnectionRevivalTriggers } from './connection-revival-triggers' import { HostClientOpenRegistry } from './host-client-open-registry' +import { decrementPendingAcquisition } from './host-client-acquisition-count' +import { + clientActivePath, + listHostClients, + notifyAllHostListeners, + notifyHostStateListeners, + primeHostProfiles, + subscribeAllHostListener, + subscribeHostStateListener, + type CloseEntryOptions +} from './host-client-context-state' import { loadHosts } from './host-store' import { openHostLogicalClient } from './host-logical-client' -import type { MobileConnectionPath, StableLogicalRpcClient } from './stable-logical-rpc-client' +import type { MobileConnectionPath } from './stable-logical-rpc-client' import type { ConnectionState, HostProfile } from './types' type StoreEntry = { @@ -29,6 +40,8 @@ type StoreEntry = { export type RpcClientContextValue = { acquire: (hostId: string, host?: HostProfile) => RpcClient | null release: (hostId: string) => void + releaseAndCloseIfUnused: (hostId: string) => void + closeIfUnused: (hostId: string) => void forceReconnect: (hostId: string) => Promise closeHost: (hostId: string) => void getState: (hostId: string) => ConnectionState @@ -55,30 +68,27 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { // Pending opens keyed by hostId so two acquire() callers in the same render don't race the host lookup. const pendingOpensRef = useRef(new HostClientOpenRegistry()) + const pendingAcquisitionsRef = useRef>(new Map()) // Why: cache of already-loaded HostProfiles so openEntry can skip a second loadHosts()/Keychain pass on cold start. const primedHostsRef = useRef>(new Map()) - function notifyHostState(hostId: string, state: ConnectionState) { - const set = stateListenersRef.current.get(hostId) - if (!set) { - return - } - for (const listener of set) { - listener(state) - } - } + const notifyHostState = (hostId: string, state: ConnectionState) => + notifyHostStateListeners(stateListenersRef.current, hostId, state) + const notifyAllHosts = () => notifyAllHostListeners(allHostsListenersRef.current) - function notifyAllHosts() { - for (const listener of allHostsListenersRef.current) { - listener() - } - } - - const closeEntry = useCallback((hostId: string) => { - pendingOpensRef.current.cancel(hostId) - primedHostsRef.current.delete(hostId) + const closeEntry = useCallback((hostId: string, options: CloseEntryOptions) => { const entry = storeRef.current.get(hostId) + const acquisitionCount = entry?.refCount ?? pendingAcquisitionsRef.current.get(hostId) ?? 0 + pendingOpensRef.current.cancel(hostId) + if (options.preserveAcquisitions && acquisitionCount > 0) { + pendingAcquisitionsRef.current.set(hostId, acquisitionCount) + } else { + pendingAcquisitionsRef.current.delete(hostId) + } + if (options.forgetPrimedHost) { + primedHostsRef.current.delete(hostId) + } entry?.unsubState() storeRef.current.delete(hostId) entry?.client.close() @@ -86,6 +96,13 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { notifyAllHosts() }, []) + const closeHost = useCallback( + (hostId: string) => { + closeEntry(hostId, { forgetPrimedHost: true, preserveAcquisitions: true }) + }, + [closeEntry] + ) + const openEntry = useCallback(async (hostId: string): Promise => { const existing = pendingOpensRef.current.getActivePromise(hostId) if (existing) { @@ -151,9 +168,10 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { const entry: StoreEntry = { client, state: client.getState(), - refCount: 0, + refCount: pendingAcquisitionsRef.current.get(hostId) ?? 0, unsubState } + pendingAcquisitionsRef.current.delete(hostId) storeRef.current.set(hostId, entry) notifyHostState(hostId, entry.state) notifyAllHosts() @@ -175,50 +193,83 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { existing.refCount += 1 return existing.client } + pendingAcquisitionsRef.current.set( + hostId, + (pendingAcquisitionsRef.current.get(hostId) ?? 0) + 1 + ) // Trigger async open; returns null this tick — consumers re-call acquire() from an effect that re-runs on state changes. - void openEntry(hostId).then((entry) => { - if (!entry) { - return - } - entry.refCount += 1 - }) + void openEntry(hostId) return null }, [openEntry] ) - const primeHosts = useCallback((hosts: HostProfile[]) => { - for (const host of hosts) { - primedHostsRef.current.set(host.id, host) - } - }, []) + const primeHosts = useCallback( + (hosts: HostProfile[]) => primeHostProfiles(primedHostsRef.current, hosts), + [] + ) // Why: no idle-close on refcount→0 — transient nav gaps flashed false 'disconnected', so keep sockets alive while foregrounded. const release = useCallback((hostId: string) => { const entry = storeRef.current.get(hostId) - if (!entry) { + if (entry) { + entry.refCount = Math.max(0, entry.refCount - 1) return } - entry.refCount = Math.max(0, entry.refCount - 1) + decrementPendingAcquisition(pendingAcquisitionsRef.current, hostId) }, []) + const releaseAndCloseIfUnused = useCallback( + (hostId: string) => { + const entry = storeRef.current.get(hostId) + if (entry) { + entry.refCount = Math.max(0, entry.refCount - 1) + if (entry.refCount === 0) { + closeEntry(hostId, { forgetPrimedHost: false, preserveAcquisitions: false }) + } + return + } + if (decrementPendingAcquisition(pendingAcquisitionsRef.current, hostId) === 0) { + closeEntry(hostId, { forgetPrimedHost: false, preserveAcquisitions: false }) + } + }, + [closeEntry] + ) + + const closeIfUnused = useCallback( + (hostId: string) => { + const entry = storeRef.current.get(hostId) + const pendingCount = pendingAcquisitionsRef.current.get(hostId) + const hasPendingOpen = pendingOpensRef.current.getActivePromise(hostId) !== null + if (!entry && pendingCount === undefined && !hasPendingOpen) { + return + } + if ((entry?.refCount ?? pendingCount ?? 0) === 0) { + closeEntry(hostId, { forgetPrimedHost: false, preserveAcquisitions: false }) + } + }, + [closeEntry] + ) + const forceReconnect = useCallback( async (hostId: string) => { const entry = storeRef.current.get(hostId) - // Why: preserve refcount across the swap; via Disconnect→Reconnect the entry is already gone, so fall back to listener count. - const listenerCount = stateListenersRef.current.get(hostId)?.size ?? 0 - const savedRefCount = entry?.refCount ?? Math.max(1, listenerCount) + // Why: ownership survives explicit close/re-pair while observers never become synthetic owners. + const savedRefCount = entry?.refCount ?? pendingAcquisitionsRef.current.get(hostId) ?? 0 if (entry) { entry.unsubState() entry.client.close() storeRef.current.delete(hostId) } + if (savedRefCount > 0) { + pendingAcquisitionsRef.current.set( + hostId, + Math.max(savedRefCount, pendingAcquisitionsRef.current.get(hostId) ?? 0) + ) + } // Why: Retry must read amber for the whole reopen, not grey-then-amber. notifyHostState(hostId, 'connecting') - const fresh = await openEntry(hostId) - if (fresh) { - fresh.refCount = savedRefCount - } + await openEntry(hostId) }, [openEntry] ) @@ -252,41 +303,17 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { }, []) const subscribeHostState = useCallback( - (hostId: string, listener: (state: ConnectionState) => void) => { - let set = stateListenersRef.current.get(hostId) - if (!set) { - set = new Set() - stateListenersRef.current.set(hostId, set) - } - set.add(listener) - return () => { - const s = stateListenersRef.current.get(hostId) - if (!s) { - return - } - s.delete(listener) - if (s.size === 0) { - stateListenersRef.current.delete(hostId) - } - } - }, + (hostId: string, listener: (state: ConnectionState) => void) => + subscribeHostStateListener(stateListenersRef.current, hostId, listener), [] ) - const getAllClients = useCallback((): Array<{ hostId: string; client: RpcClient }> => { - const out: Array<{ hostId: string; client: RpcClient }> = [] - for (const [hostId, entry] of storeRef.current) { - out.push({ hostId, client: entry.client }) - } - return out - }, []) + const getAllClients = useCallback(() => listHostClients(storeRef.current), []) - const subscribeAllHosts = useCallback((listener: () => void) => { - allHostsListenersRef.current.add(listener) - return () => { - allHostsListenersRef.current.delete(listener) - } - }, []) + const subscribeAllHosts = useCallback( + (listener: () => void) => subscribeAllHostListener(allHostsListenersRef.current, listener), + [] + ) // Close all clients on provider unmount. Empty deps: [closeEntry] would let Fast Refresh tear down all live sockets. // eslint-disable-next-line react-hooks/exhaustive-deps @@ -294,8 +321,9 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { const store = storeRef.current return () => { pendingOpensRef.current.cancelAll() + pendingAcquisitionsRef.current.clear() for (const [hostId] of store) { - closeEntry(hostId) + closeEntry(hostId, { forgetPrimedHost: true, preserveAcquisitions: false }) } } }, []) @@ -313,8 +341,10 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { () => ({ acquire, release, + releaseAndCloseIfUnused, + closeIfUnused, forceReconnect, - closeHost: closeEntry, + closeHost, getState, getKnownState, getReconnectAttempt, @@ -328,8 +358,10 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { [ acquire, release, + releaseAndCloseIfUnused, + closeIfUnused, forceReconnect, - closeEntry, + closeHost, getState, getKnownState, getReconnectAttempt, @@ -437,13 +469,3 @@ export function usePrimeHosts(): (hosts: HostProfile[]) => void { const ctx = useRpcClientContext() return ctx.primeHosts } - -function clientActivePath(client: RpcClient | undefined): MobileConnectionPath { - const logical = client as Partial | undefined - if (typeof logical?.getActivePath !== 'function') { - return 'lan' - } - // Why: mid-migration the active path still names the session being replaced; the - // pending one is what the user is actually waiting on (F5). - return logical.getPendingPath?.() ?? logical.getActivePath() -} diff --git a/mobile/src/transport/home-host-auto-connect.test.ts b/mobile/src/transport/home-host-auto-connect.test.ts new file mode 100644 index 000000000..ecf049348 --- /dev/null +++ b/mobile/src/transport/home-host-auto-connect.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from 'vitest' +import type { HostProfile } from './types' +import { + HOME_AUTO_CONNECT_LIMIT, + resolveHomeHostConnectionState, + selectHomeAutoConnectHostIds +} from './home-host-auto-connect' + +function host(id: string, lastConnected: number, credentials = true): HostProfile { + return { + id, + name: id, + endpoint: `ws://${id}`, + deviceToken: credentials ? `token-${id}` : '', + publicKeyB64: credentials ? `key-${id}` : '', + lastConnected + } +} + +describe('home host auto-connect', () => { + it('limits startup connections to the most recently used credentialed hosts', () => { + const hosts = [ + host('old', 1), + host('newest', 5), + host('second', 4), + host('third', 3), + host('fourth', 2), + host('missing-credentials', 6, false) + ] + + expect(selectHomeAutoConnectHostIds(hosts)).toEqual(['newest', 'second', 'third']) + expect(selectHomeAutoConnectHostIds(hosts)).toHaveLength(HOME_AUTO_CONNECT_LIMIT) + }) + + it('does not mutate the host card order', () => { + const hosts = [host('old', 1), host('new', 2)] + + selectHomeAutoConnectHostIds(hosts) + + expect(hosts.map((item) => item.id)).toEqual(['old', 'new']) + }) + + it('only presents hosts in the startup subset as connecting before clients open', () => { + const autoConnectHostIds = ['recent'] + + expect(resolveHomeHostConnectionState('recent', undefined, autoConnectHostIds)).toBe( + 'connecting' + ) + expect(resolveHomeHostConnectionState('stale', undefined, autoConnectHostIds)).toBe( + 'disconnected' + ) + expect(resolveHomeHostConnectionState('stale', 'connected', autoConnectHostIds)).toBe( + 'connected' + ) + }) +}) diff --git a/mobile/src/transport/home-host-auto-connect.ts b/mobile/src/transport/home-host-auto-connect.ts new file mode 100644 index 000000000..995d653a0 --- /dev/null +++ b/mobile/src/transport/home-host-auto-connect.ts @@ -0,0 +1,24 @@ +import type { ConnectionState, HostProfile } from './types' + +export const HOME_AUTO_CONNECT_LIMIT = 3 + +export function selectHomeAutoConnectHostIds( + hosts: readonly HostProfile[], + limit = HOME_AUTO_CONNECT_LIMIT +): string[] { + return [...hosts] + .filter((host) => host.deviceToken.length > 0 && host.publicKeyB64.length > 0) + .sort( + (left, right) => right.lastConnected - left.lastConnected || left.id.localeCompare(right.id) + ) + .slice(0, Math.max(0, limit)) + .map((host) => host.id) +} + +export function resolveHomeHostConnectionState( + hostId: string, + state: ConnectionState | undefined, + autoConnectHostIds: readonly string[] +): ConnectionState { + return state ?? (autoConnectHostIds.includes(hostId) ? 'connecting' : 'disconnected') +} diff --git a/mobile/src/transport/host-client-acquisition-count.ts b/mobile/src/transport/host-client-acquisition-count.ts new file mode 100644 index 000000000..1a1123e5c --- /dev/null +++ b/mobile/src/transport/host-client-acquisition-count.ts @@ -0,0 +1,9 @@ +export function decrementPendingAcquisition(pending: Map, hostId: string): number { + const next = Math.max(0, (pending.get(hostId) ?? 0) - 1) + if (next === 0) { + pending.delete(hostId) + } else { + pending.set(hostId, next) + } + return next +} diff --git a/mobile/src/transport/host-client-context-state.ts b/mobile/src/transport/host-client-context-state.ts new file mode 100644 index 000000000..e0f7243ba --- /dev/null +++ b/mobile/src/transport/host-client-context-state.ts @@ -0,0 +1,72 @@ +import type { RpcClient } from './rpc-client' +import type { MobileConnectionPath, StableLogicalRpcClient } from './stable-logical-rpc-client' +import type { ConnectionState, HostProfile } from './types' + +export type CloseEntryOptions = { + forgetPrimedHost: boolean + preserveAcquisitions: boolean +} + +export function notifyHostStateListeners( + listeners: Map void>>, + hostId: string, + state: ConnectionState +): void { + for (const listener of listeners.get(hostId) ?? []) { + listener(state) + } +} + +export function notifyAllHostListeners(listeners: Set<() => void>): void { + for (const listener of listeners) { + listener() + } +} + +export function subscribeHostStateListener( + listeners: Map void>>, + hostId: string, + listener: (state: ConnectionState) => void +): () => void { + let hostListeners = listeners.get(hostId) + if (!hostListeners) { + hostListeners = new Set() + listeners.set(hostId, hostListeners) + } + hostListeners.add(listener) + return () => { + hostListeners.delete(listener) + if (hostListeners.size === 0) { + listeners.delete(hostId) + } + } +} + +export function subscribeAllHostListener( + listeners: Set<() => void>, + listener: () => void +): () => void { + listeners.add(listener) + return () => listeners.delete(listener) +} + +export function listHostClients( + entries: ReadonlyMap +): { hostId: string; client: RpcClient }[] { + return [...entries].map(([hostId, entry]) => ({ hostId, client: entry.client })) +} + +export function primeHostProfiles(cache: Map, hosts: HostProfile[]): void { + for (const host of hosts) { + cache.set(host.id, host) + } +} + +export function clientActivePath(client: RpcClient | undefined): MobileConnectionPath { + const logical = client as Partial | undefined + if (typeof logical?.getActivePath !== 'function') { + return 'lan' + } + // Why: during migration the pending path is what the user is waiting on. + return logical.getPendingPath?.() ?? logical.getActivePath() +} diff --git a/mobile/src/transport/settings-host-client-connections.ts b/mobile/src/transport/settings-host-client-connections.ts new file mode 100644 index 000000000..b2341471d --- /dev/null +++ b/mobile/src/transport/settings-host-client-connections.ts @@ -0,0 +1,19 @@ +import { useCallback, useState } from 'react' +import { useFocusEffect } from 'expo-router' +import { useAllHostClients } from './use-all-host-clients' + +export function useFocusedSettingsHostClients(hostIds: string[]) { + const [focused, setFocused] = useState(false) + + useFocusEffect( + useCallback(() => { + setFocused(true) + return () => setFocused(false) + }, []) + ) + + const clients = useAllHostClients(focused ? hostIds : [], { + closeUnusedOnRelease: true + }) + return { clients, focused } +} diff --git a/mobile/src/transport/settings-host-client-lifecycle.test.ts b/mobile/src/transport/settings-host-client-lifecycle.test.ts new file mode 100644 index 000000000..7a23c86ac --- /dev/null +++ b/mobile/src/transport/settings-host-client-lifecycle.test.ts @@ -0,0 +1,776 @@ +import { createElement, Fragment, useEffect } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { useAllHostClients } from './use-all-host-clients' +import { useFocusedSettingsHostClients } from './settings-host-client-connections' +import { + RpcClientProvider, + useHostClient, + usePrimeHosts, + useRpcClientContext, + type RpcClientContextValue +} from './client-context' +import { selectHomeAutoConnectHostIds } from './home-host-auto-connect' +import type { RpcClient } from './rpc-client' +import type { ConnectionState, HostProfile } from './types' + +const connectMock = vi.fn() +const loadHostsMock = vi.fn() +const routeFocus = vi.hoisted(() => ({ + effect: null as null | (() => void | (() => void)) +})) + +vi.mock('expo-router', () => ({ + useFocusEffect: (effect: () => void | (() => void)) => { + routeFocus.effect = effect + } +})) + +vi.mock('./host-logical-client', () => ({ + openHostLogicalClient: (...args: unknown[]) => connectMock(...args) +})) +vi.mock('./host-store', () => ({ + loadHosts: () => loadHostsMock() +})) +vi.mock('./connection-revival-triggers', () => ({ + subscribeConnectionRevivalTriggers: () => () => {} +})) + +type FakeClient = RpcClient & { + closeMock: ReturnType +} + +function makeFakeClient(initialState: ConnectionState): FakeClient { + const listeners = new Set<(state: ConnectionState) => void>() + const closeMock = vi.fn() + return { + sendRequest: vi.fn(), + subscribe: vi.fn(() => () => {}), + updateTerminalSubscriptionViewport: vi.fn(), + getState: () => initialState, + getReconnectAttempt: () => (initialState === 'reconnecting' ? 4 : 0), + getLastConnectedAt: () => null, + onStateChange: (listener) => { + listeners.add(listener) + return () => listeners.delete(listener) + }, + notifyForeground: vi.fn(), + close: closeMock, + closeMock + } as FakeClient +} + +function host(id: string, lastConnected: number, extra: Partial = {}): HostProfile { + return { + id, + name: id, + endpoint: `ws://${id}.internal:8787`, + deviceToken: `token-${id}`, + publicKeyB64: `key-${id}`, + lastConnected, + ...extra + } +} + +const HOSTS = [ + host('direct-recent', 50), + host('relay-recent', 40, { relayHostId: 'AbCdEf0123_-xyZ9' }), + host('ssh-provider-recent', 30), + host('folder-workspace-host', 20), + host('offline-host', 10) +] +const HOST_IDS = HOSTS.map((profile) => profile.id) +const HOME_HOST_IDS = selectHomeAutoConnectHostIds(HOSTS) + +let context: RpcClientContextValue | null = null + +function ContextProbe(): null { + context = useRpcClientContext() + return null +} + +function HomeProbe(): null { + useAllHostClients(HOST_IDS, { + autoConnectHostIds: HOME_HOST_IDS, + closeUnusedOnRelease: true + }) + const primeHosts = usePrimeHosts() + useEffect(() => { + primeHosts(HOSTS) + }, [primeHosts]) + return null +} + +function SettingsProbe(): null { + useAllHostClients(HOST_IDS, { + closeUnusedOnRelease: true + }) + return null +} + +function FocusedSettingsProbe(): null { + useFocusedSettingsHostClients(HOST_IDS) + return null +} + +function DynamicSettingsProbe({ hostIds }: { hostIds: string[] }): null { + useAllHostClients(hostIds, { closeUnusedOnRelease: true }) + return null +} + +function DetailProbe({ hostId }: { hostId: string }): null { + useHostClient(hostId) + return null +} + +type Screen = 'empty' | 'home' | 'settings' + +function TestApp({ + screen, + detailHostId +}: { + screen: Screen + detailHostId?: string +}): React.JSX.Element { + const screenProbe = + screen === 'home' + ? createElement(HomeProbe) + : screen === 'settings' + ? createElement(SettingsProbe) + : null + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + detailHostId ? createElement(DetailProbe, { hostId: detailHostId }) : null, + screenProbe + ) + ) +} + +function suppressRendererWarning(): () => void { + const originalConsoleError = console.error + const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { + if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { + return + } + originalConsoleError(...args) + }) + return () => spy.mockRestore() +} + +async function renderScreen(screen: Screen, detailHostId?: string): Promise { + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create(createElement(TestApp, { screen, detailHostId })) + await Promise.resolve() + await Promise.resolve() + }) + } finally { + restore() + } + if (!renderer) { + throw new Error('settings lifecycle harness did not render') + } + return renderer +} + +async function navigate( + renderer: ReactTestRenderer, + screen: Screen, + detailHostId?: string +): Promise { + await act(async () => { + renderer.update(createElement(TestApp, { screen, detailHostId })) + await Promise.resolve() + await Promise.resolve() + }) +} + +function activeHostIds(): string[] { + if (!context) { + throw new Error('client context was not captured') + } + return context + .getAllClients() + .map(({ hostId }) => hostId) + .sort() +} + +beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + context = null + routeFocus.effect = null + connectMock.mockReset() + loadHostsMock.mockReset() +}) + +describe('settings host client lifecycle', () => { + it('closes settings-only clients while a mounted Home owner keeps its bounded set', async () => { + const clients = new Map() + connectMock.mockImplementation((profile: HostProfile) => { + const client = makeFakeClient(profile.id === 'offline-host' ? 'reconnecting' : 'connected') + clients.set(profile.id, [...(clients.get(profile.id) ?? []), client]) + return client + }) + loadHostsMock.mockResolvedValue(HOSTS) + + function NavigationStack({ settingsVisible }: { settingsVisible: boolean }) { + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + createElement(HomeProbe), + settingsVisible ? createElement(SettingsProbe) : null + ) + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create(createElement(NavigationStack, { settingsVisible: false })) + await Promise.resolve() + await Promise.resolve() + }) + } finally { + restore() + } + if (!renderer) { + throw new Error('navigation stack harness did not render') + } + expect(activeHostIds()).toEqual([...HOME_HOST_IDS].sort()) + expect(connectMock).toHaveBeenCalledTimes(3) + + await act(async () => { + renderer?.update(createElement(NavigationStack, { settingsVisible: true })) + await Promise.resolve() + await Promise.resolve() + }) + expect(activeHostIds()).toEqual([...HOST_IDS].sort()) + expect(connectMock).toHaveBeenCalledTimes(HOSTS.length) + expect(loadHostsMock).toHaveBeenCalledTimes(HOME_HOST_IDS.length) + + await act(async () => { + renderer?.update(createElement(NavigationStack, { settingsVisible: false })) + }) + expect(activeHostIds()).toEqual([...HOME_HOST_IDS].sort()) + for (const hostId of HOME_HOST_IDS) { + expect(clients.get(hostId)).toHaveLength(1) + expect(clients.get(hostId)?.[0]?.closeMock).not.toHaveBeenCalled() + } + for (const hostId of ['folder-workspace-host', 'offline-host']) { + expect(clients.get(hostId)?.[0]?.closeMock).toHaveBeenCalledOnce() + } + + await act(async () => { + renderer?.update(createElement(NavigationStack, { settingsVisible: true })) + await Promise.resolve() + await Promise.resolve() + }) + expect(loadHostsMock).toHaveBeenCalledTimes(HOME_HOST_IDS.length) + await act(async () => { + renderer?.update(createElement(NavigationStack, { settingsVisible: false })) + }) + expect(activeHostIds()).toEqual([...HOME_HOST_IDS].sort()) + for (const hostId of HOME_HOST_IDS) { + expect(clients.get(hostId)).toHaveLength(1) + } + for (const hostId of ['folder-workspace-host', 'offline-host']) { + expect(clients.get(hostId)).toHaveLength(2) + expect(clients.get(hostId)?.every((client) => client.closeMock.mock.calls.length === 1)).toBe( + true + ) + } + + act(() => renderer.unmount()) + }) + + it('leaves no zero-reference clients after repeated settings navigation', async () => { + const clients = new Map() + connectMock.mockImplementation((profile: HostProfile) => { + const client = makeFakeClient('reconnecting') + clients.set(profile.id, [...(clients.get(profile.id) ?? []), client]) + return client + }) + loadHostsMock.mockResolvedValue(HOSTS) + + const renderer = await renderScreen('settings') + expect(activeHostIds()).toEqual([...HOST_IDS].sort()) + + await navigate(renderer, 'empty') + expect(activeHostIds()).toEqual([]) + for (const hostClients of clients.values()) { + expect(hostClients).toHaveLength(1) + expect(hostClients[0]?.closeMock).toHaveBeenCalledOnce() + } + + await navigate(renderer, 'settings') + await navigate(renderer, 'empty') + expect(activeHostIds()).toEqual([]) + expect(connectMock).toHaveBeenCalledTimes(HOSTS.length * 2) + for (const hostClients of clients.values()) { + expect(hostClients).toHaveLength(2) + expect(hostClients.every((client) => client.closeMock.mock.calls.length === 1)).toBe(true) + } + + act(() => renderer.unmount()) + }) + + it('reconciles host-list changes without restarting retained or shared clients', async () => { + const clients = new Map() + connectMock.mockImplementation((profile: HostProfile) => { + const client = makeFakeClient('reconnecting') + clients.set(profile.id, client) + return client + }) + loadHostsMock.mockResolvedValue(HOSTS) + + const retainedHostId = 'direct-recent' + const sharedHostId = 'relay-recent' + const removedHostId = 'folder-workspace-host' + const addedHostId = 'offline-host' + function HostListApp({ settingsHostIds }: { settingsHostIds: string[] }) { + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + createElement(DetailProbe, { hostId: sharedHostId }), + createElement(DynamicSettingsProbe, { hostIds: settingsHostIds }) + ) + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(HostListApp, { + settingsHostIds: [retainedHostId, sharedHostId, removedHostId] + }) + ) + await Promise.resolve() + await Promise.resolve() + }) + } finally { + restore() + } + if (!renderer) { + throw new Error('host-list lifecycle harness did not render') + } + + await act(async () => { + renderer?.update( + createElement(HostListApp, { + settingsHostIds: [retainedHostId, addedHostId] + }) + ) + await Promise.resolve() + await Promise.resolve() + }) + + expect(connectMock).toHaveBeenCalledTimes(4) + expect(activeHostIds()).toEqual([addedHostId, retainedHostId, sharedHostId].sort()) + expect(clients.get(retainedHostId)?.closeMock).not.toHaveBeenCalled() + expect(clients.get(sharedHostId)?.closeMock).not.toHaveBeenCalled() + expect(clients.get(removedHostId)?.closeMock).toHaveBeenCalledOnce() + expect(clients.get(addedHostId)?.closeMock).not.toHaveBeenCalled() + + act(() => renderer.unmount()) + expect(clients.get(retainedHostId)?.closeMock).toHaveBeenCalledOnce() + expect(clients.get(sharedHostId)?.closeMock).toHaveBeenCalledOnce() + expect(clients.get(addedHostId)?.closeMock).toHaveBeenCalledOnce() + }) + + it('does not close a settings client still held by an active consumer', async () => { + const clients = new Map() + connectMock.mockImplementation((profile: HostProfile) => { + const client = makeFakeClient('connected') + clients.set(profile.id, client) + return client + }) + loadHostsMock.mockResolvedValue(HOSTS) + + const detailHostId = 'folder-workspace-host' + const renderer = await renderScreen('settings', detailHostId) + await navigate(renderer, 'home', detailHostId) + + expect(activeHostIds()).toEqual([...HOME_HOST_IDS, detailHostId].sort()) + expect(clients.get(detailHostId)?.closeMock).not.toHaveBeenCalled() + + act(() => renderer.unmount()) + expect(clients.get(detailHostId)?.closeMock).toHaveBeenCalledOnce() + }) + + it('keeps a reconnect alive when another consumer remains after settings leaves', async () => { + const retryHost = host('ssh-retry-host', 1) + let resolveInitial: ((hosts: HostProfile[]) => void) | null = null + let resolveRetry: ((hosts: HostProfile[]) => void) | null = null + const initialLookup = new Promise((resolve) => { + resolveInitial = resolve + }) + const retryLookup = new Promise((resolve) => { + resolveRetry = resolve + }) + loadHostsMock.mockReturnValueOnce(initialLookup).mockReturnValueOnce(retryLookup) + const initialClient = makeFakeClient('connected') + const retryClient = makeFakeClient('reconnecting') + connectMock.mockReturnValueOnce(initialClient).mockReturnValueOnce(retryClient) + + function RetrySettingsProbe(): null { + useAllHostClients([retryHost.id], { + closeUnusedOnRelease: true + }) + return null + } + function RetryApp({ settingsVisible }: { settingsVisible: boolean }): React.JSX.Element { + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + createElement(DetailProbe, { hostId: retryHost.id }), + settingsVisible ? createElement(RetrySettingsProbe) : null + ) + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + act(() => { + renderer = create(createElement(RetryApp, { settingsVisible: true })) + }) + } finally { + restore() + } + if (!renderer || !resolveInitial || !resolveRetry) { + throw new Error('retry lifecycle harness did not initialize') + } + await act(async () => { + resolveInitial?.([retryHost]) + await initialLookup + }) + if (!context) { + throw new Error('client context was not captured') + } + + const reconnect = context.forceReconnect(retryHost.id) + expect(initialClient.closeMock).toHaveBeenCalledOnce() + act(() => renderer?.update(createElement(RetryApp, { settingsVisible: false }))) + await act(async () => { + resolveRetry?.([retryHost]) + await retryLookup + await reconnect + }) + + expect(activeHostIds()).toEqual([retryHost.id]) + expect(retryClient.closeMock).not.toHaveBeenCalled() + act(() => renderer?.unmount()) + expect(retryClient.closeMock).toHaveBeenCalledOnce() + }) + + it('cancels a released open without cancelling a rapid replacement acquisition', async () => { + const settingsOnlyHost = host('settings-only-offline', 0, { + deviceToken: '', + publicKeyB64: '' + }) + let resolveFirst: ((hosts: HostProfile[]) => void) | null = null + let resolveSecond: ((hosts: HostProfile[]) => void) | null = null + const firstLookup = new Promise((resolve) => { + resolveFirst = resolve + }) + const secondLookup = new Promise((resolve) => { + resolveSecond = resolve + }) + loadHostsMock.mockReturnValueOnce(firstLookup).mockReturnValueOnce(secondLookup) + const client = makeFakeClient('reconnecting') + connectMock.mockReturnValue(client) + + function PendingSettingsProbe(): null { + useAllHostClients([settingsOnlyHost.id], { + closeUnusedOnRelease: true + }) + return null + } + function PendingApp({ visible }: { visible: boolean }): React.JSX.Element { + return createElement( + RpcClientProvider, + null, + visible ? createElement(PendingSettingsProbe) : null + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + act(() => { + renderer = create(createElement(PendingApp, { visible: true })) + }) + } finally { + restore() + } + if (!renderer || !resolveFirst || !resolveSecond) { + throw new Error('pending settings harness did not initialize') + } + + act(() => renderer?.update(createElement(PendingApp, { visible: false }))) + act(() => renderer?.update(createElement(PendingApp, { visible: true }))) + await act(async () => { + resolveFirst?.([settingsOnlyHost]) + await firstLookup + }) + expect(connectMock).not.toHaveBeenCalled() + + await act(async () => { + resolveSecond?.([settingsOnlyHost]) + await secondLookup + }) + expect(connectMock).toHaveBeenCalledOnce() + + act(() => renderer?.update(createElement(PendingApp, { visible: false }))) + expect(client.closeMock).toHaveBeenCalledOnce() + act(() => renderer?.unmount()) + }) + + it('preserves Home ownership when re-pairing replaces a selected host client', async () => { + const clients = new Map() + connectMock.mockImplementation((profile: HostProfile) => { + const client = makeFakeClient('connected') + clients.set(profile.id, [...(clients.get(profile.id) ?? []), client]) + return client + }) + loadHostsMock.mockResolvedValue(HOSTS) + const replacedHostId = HOME_HOST_IDS[0]! + + function ReplacementApp({ + detailVisible, + settingsVisible + }: { + detailVisible: boolean + settingsVisible: boolean + }): React.JSX.Element { + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + createElement(HomeProbe), + detailVisible ? createElement(DetailProbe, { hostId: replacedHostId }) : null, + settingsVisible ? createElement(SettingsProbe) : null + ) + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(ReplacementApp, { detailVisible: false, settingsVisible: false }) + ) + await Promise.resolve() + }) + } finally { + restore() + } + if (!renderer || !context) { + throw new Error('replacement lifecycle harness did not initialize') + } + + const originalClient = clients.get(replacedHostId)?.[0] + act(() => context?.closeHost(replacedHostId)) + expect(originalClient?.closeMock).toHaveBeenCalledOnce() + + await act(async () => { + renderer?.update( + createElement(ReplacementApp, { detailVisible: true, settingsVisible: false }) + ) + await Promise.resolve() + await Promise.resolve() + }) + const replacementClient = clients.get(replacedHostId)?.[1] + expect(replacementClient).toBeDefined() + + await act(async () => { + renderer?.update( + createElement(ReplacementApp, { detailVisible: false, settingsVisible: true }) + ) + await Promise.resolve() + await Promise.resolve() + }) + act(() => + renderer?.update( + createElement(ReplacementApp, { detailVisible: false, settingsVisible: false }) + ) + ) + + expect(activeHostIds()).toEqual([...HOME_HOST_IDS].sort()) + expect(replacementClient?.closeMock).not.toHaveBeenCalled() + act(() => renderer?.unmount()) + expect(replacementClient?.closeMock).toHaveBeenCalledOnce() + }) + + it('releases a manual host after Home demotes or stops tracking it', async () => { + const manualHost = host('manual-relay-host', 1, { relayHostId: 'AbCdEf0123_-xyZ9' }) + const clients: FakeClient[] = [] + connectMock.mockImplementation(() => { + const client = makeFakeClient('reconnecting') + clients.push(client) + return client + }) + loadHostsMock.mockResolvedValue([manualHost]) + + function ManualHomeProbe({ selected }: { selected: boolean }): null { + useAllHostClients([manualHost.id], { + autoConnectHostIds: selected ? [manualHost.id] : [], + closeUnusedOnRelease: true + }) + return null + } + function ManualApp({ + selected, + homeVisible = true + }: { + selected: boolean + homeVisible?: boolean + }) { + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + homeVisible ? createElement(ManualHomeProbe, { selected }) : null + ) + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + act(() => { + renderer = create(createElement(ManualApp, { selected: false })) + }) + } finally { + restore() + } + if (!renderer || !context) { + throw new Error('manual connection harness did not initialize') + } + await act(async () => { + await context?.forceReconnect(manualHost.id) + }) + expect(activeHostIds()).toEqual([manualHost.id]) + + await act(async () => { + renderer?.update(createElement(ManualApp, { selected: true })) + await Promise.resolve() + }) + expect(activeHostIds()).toEqual([manualHost.id]) + + await act(async () => { + renderer?.update(createElement(ManualApp, { selected: false })) + await Promise.resolve() + }) + + expect(activeHostIds()).toEqual([]) + expect(clients[0]?.closeMock).toHaveBeenCalledOnce() + + await act(async () => { + await context?.forceReconnect(manualHost.id) + }) + expect(activeHostIds()).toEqual([manualHost.id]) + expect(clients).toHaveLength(2) + + act(() => renderer?.update(createElement(ManualApp, { selected: false, homeVisible: false }))) + expect(activeHostIds()).toEqual([]) + expect(clients[1]?.closeMock).toHaveBeenCalledOnce() + + act(() => renderer?.unmount()) + expect(clients.every((client) => client.closeMock.mock.calls.length === 1)).toBe(true) + }) + + it('releases settings-only clients on blur while shared owners stay mounted', async () => { + const clients = new Map() + connectMock.mockImplementation((profile: HostProfile) => { + const client = makeFakeClient('reconnecting') + clients.set(profile.id, client) + return client + }) + loadHostsMock.mockResolvedValue(HOSTS) + const detailHostId = 'folder-workspace-host' + + function FocusStack(): React.JSX.Element { + return createElement( + RpcClientProvider, + null, + createElement( + Fragment, + null, + createElement(ContextProbe), + createElement(HomeProbe), + createElement(DetailProbe, { hostId: detailHostId }), + createElement(FocusedSettingsProbe) + ) + ) + } + + let renderer: ReactTestRenderer | null = null + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create(createElement(FocusStack)) + await Promise.resolve() + await Promise.resolve() + }) + } finally { + restore() + } + if (!renderer || !routeFocus.effect) { + throw new Error('settings focus harness did not initialize') + } + expect(activeHostIds()).toEqual([...HOME_HOST_IDS, detailHostId].sort()) + + let blur: (() => void) | undefined + await act(async () => { + const cleanup = routeFocus.effect?.() + if (typeof cleanup === 'function') { + blur = cleanup + } + await Promise.resolve() + await Promise.resolve() + }) + expect(activeHostIds()).toEqual([...HOST_IDS].sort()) + + await act(async () => { + blur?.() + await Promise.resolve() + }) + expect(activeHostIds()).toEqual([...HOME_HOST_IDS, detailHostId].sort()) + for (const hostId of [...HOME_HOST_IDS, detailHostId]) { + expect(clients.get(hostId)?.closeMock).not.toHaveBeenCalled() + } + expect(clients.get('offline-host')?.closeMock).toHaveBeenCalledOnce() + + act(() => renderer?.unmount()) + }) +}) diff --git a/mobile/src/transport/use-all-host-clients.ts b/mobile/src/transport/use-all-host-clients.ts index a1e429b45..100091611 100644 --- a/mobile/src/transport/use-all-host-clients.ts +++ b/mobile/src/transport/use-all-host-clients.ts @@ -1,56 +1,126 @@ -import { useEffect, useMemo, useState } from 'react' +import { useEffect, useMemo, useRef, useState } from 'react' import type { RpcClient } from './rpc-client' import type { MobileConnectionPath } from './stable-logical-rpc-client' import type { ConnectionState } from './types' import { useRpcClientContext } from './client-context' -// Why: refcounting prevents a double-open when a host-detail screen shares one of these hosts. -export function useAllHostClients(hostIds: string[]) { +type UseAllHostClientsOptions = { + autoConnectHostIds?: readonly string[] + closeUnusedOnRelease?: boolean +} + +export function useAllHostClients(hostIds: string[], options?: UseAllHostClientsOptions) { const ctx = useRpcClientContext() - // Stable key so we don't tear down on every render of the array. - const key = useMemo(() => [...hostIds].sort().join(','), [hostIds]) + const autoConnectHostIds = options?.autoConnectHostIds ?? hostIds + const closeUnusedOnRelease = options?.closeUnusedOnRelease ?? false + const key = useMemo( + () => + [ + [...hostIds].sort().join(','), + [...autoConnectHostIds].sort().join(','), + closeUnusedOnRelease ? 'close' : 'keep' + ].join('|'), + [autoConnectHostIds, closeUnusedOnRelease, hostIds] + ) const [tick, setTick] = useState(0) + const acquiredHostIdsRef = useRef>(new Set()) + const hostUnsubscribesRef = useRef void>>(new Map()) + const closeUnusedRef = useRef(closeUnusedOnRelease) useEffect(() => { - if (hostIds.length === 0) { - return - } - for (const id of hostIds) { - ctx.acquire(id) - } - const unsubs: (() => void)[] = [] - for (const id of hostIds) { - unsubs.push(ctx.subscribeHostState(id, () => setTick((n) => n + 1))) - } - unsubs.push(ctx.subscribeAllHosts(() => setTick((n) => n + 1))) + closeUnusedRef.current = closeUnusedOnRelease + }, [closeUnusedOnRelease]) + + useEffect(() => { + const unsubscribeAllHosts = ctx.subscribeAllHosts(() => setTick((value) => value + 1)) return () => { - for (const u of unsubs) { - u() + unsubscribeAllHosts() + const trackedHostIds = [...hostUnsubscribesRef.current.keys()] + const acquiredHostIds = new Set(acquiredHostIdsRef.current) + for (const unsubscribe of hostUnsubscribesRef.current.values()) { + unsubscribe() } - for (const id of hostIds) { - ctx.release(id) + hostUnsubscribesRef.current.clear() + for (const id of acquiredHostIds) { + if (closeUnusedRef.current) { + ctx.releaseAndCloseIfUnused(id) + } else { + ctx.release(id) + } + } + if (closeUnusedRef.current) { + for (const id of trackedHostIds) { + if (!acquiredHostIds.has(id)) { + ctx.closeIfUnused(id) + } + } + } + acquiredHostIdsRef.current.clear() + } + }, [ctx]) + + useEffect(() => { + const trackedHostIds = new Set(hostIds) + const nextAcquiredHostIds = new Set(autoConnectHostIds.filter((id) => trackedHostIds.has(id))) + const removedTrackedHostIds: string[] = [] + + for (const [id, unsubscribe] of hostUnsubscribesRef.current) { + if (!trackedHostIds.has(id)) { + unsubscribe() + hostUnsubscribesRef.current.delete(id) + removedTrackedHostIds.push(id) } } - }, [key]) + for (const id of trackedHostIds) { + if (!hostUnsubscribesRef.current.has(id)) { + hostUnsubscribesRef.current.set( + id, + ctx.subscribeHostState(id, () => setTick((value) => value + 1)) + ) + } + } + + for (const id of acquiredHostIdsRef.current) { + if (!nextAcquiredHostIds.has(id)) { + if (closeUnusedOnRelease) { + ctx.releaseAndCloseIfUnused(id) + } else { + ctx.release(id) + } + } + } + for (const id of nextAcquiredHostIds) { + if (!acquiredHostIdsRef.current.has(id)) { + ctx.acquire(id) + } + } + if (closeUnusedOnRelease) { + for (const id of removedTrackedHostIds) { + ctx.closeIfUnused(id) + } + for (const id of trackedHostIds) { + if (!nextAcquiredHostIds.has(id)) { + ctx.closeIfUnused(id) + } + } + } + acquiredHostIdsRef.current = nextAcquiredHostIds + }, [ctx, key]) return useMemo(() => { - const out: { + const clientsByHostId = new Map( + ctx.getAllClients().map((entry) => [entry.hostId, entry.client]) + ) + return hostIds.flatMap<{ hostId: string client: RpcClient state: ConnectionState path: MobileConnectionPath - }[] = [] - for (const id of hostIds) { - const all = ctx.getAllClients().find((entry) => entry.hostId === id) - if (all) { - out.push({ - hostId: id, - client: all.client, - state: ctx.getState(id), - path: ctx.getActivePath(id) - }) - } - } - return out - }, [key, tick]) + }>((hostId) => { + const client = clientsByHostId.get(hostId) + return client + ? [{ hostId, client, state: ctx.getState(hostId), path: ctx.getActivePath(hostId) }] + : [] + }) + }, [ctx, hostIds, tick]) }