From 7d8a829f3d41e52685cdc6dafb3a5ffdce10411f Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:31 -0400 Subject: [PATCH] refactor(mobile): run both connection log routes on one screen The native route inlined 245 lines of orchestration the hosted screen had already factored behind DiagnosticsDeviceOperations, so the two paths gathered, redacted, and submitted the same report differently. The screen now takes the device operations, a clipboard writer, and a host picker slot; the native route supplies the existing native operations and its host chips. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/connection-log.tsx | 246 +++--------------- mobile/host-web-app/connection-log.tsx | 9 +- .../connection-diagnostics-screen.test.tsx | 106 ++++++++ .../connection-diagnostics-screen.tsx | 78 +++--- 4 files changed, 197 insertions(+), 242 deletions(-) create mode 100644 mobile/src/diagnostics/connection-diagnostics-screen.test.tsx diff --git a/mobile/app/connection-log.tsx b/mobile/app/connection-log.tsx index 6301d6615fd..abf01c6bee0 100644 --- a/mobile/app/connection-log.tsx +++ b/mobile/app/connection-log.tsx @@ -1,39 +1,18 @@ -import { ConnectionDiagnosticsView } from '../src/diagnostics/connection-diagnostics-view' -import { useCallback, useEffect, useMemo, useState, useSyncExternalStore } from 'react' -import { View, Text, Pressable, Platform } from 'react-native' +import { useCallback, useEffect, useMemo, useState } from 'react' +import { View, Text, Pressable } from 'react-native' import { useLocalSearchParams, useRouter } from 'expo-router' import * as Clipboard from 'expo-clipboard' -import Constants from 'expo-constants' import { loadHosts } from '../src/transport/host-store' -import { connectionLogStore } from '../src/transport/persisted-connection-log-store' import { useHostClient, useRpcClientContext } from '../src/transport/client-context' -import { - useConnectionPathStatus, - useReconnectAttempt -} from '../src/transport/client-context-connection-metrics' -import { buildConnectionDiagnosticsReport } from '../src/diagnostics/connection-diagnostics-report' -import { mobileWebDiagnosticsStore } from '../src/mobile-web/mobile-web-diagnostics-store' -import { - diagnoseConnection, - getReportableConnectionIncidentId -} from '../src/diagnostics/connection-diagnostics-analysis' -import { submitConnectionDiagnostics } from '../src/diagnostics/connection-diagnostics-submission' -import { - readHydratedConnectionLog, - readConnectionDiagnosticsSnapshot, - resolveDiagnosticsHostId, - getDiagnosticsSubmissionState, - updateDiagnosticsSubmissionState, - type DiagnosticsSubmissionStates -} from '../src/diagnostics/connection-diagnostics-screen-data' import { useHostStatusGates } from '../src/transport/host-status-gates' -import { loadHostAppVersion } from '../src/transport/host-app-version-store' +import { ConnectionDiagnosticsScreen } from '../src/diagnostics/connection-diagnostics-screen' +import { createNativeDiagnosticsOperations } from '../src/diagnostics/native-diagnostics-operations' +import { + resolveDiagnosticsHostId, + type DiagnosticsHostSelection +} from '../src/diagnostics/connection-diagnostics-screen-data' import { connectionDiagnosticsScreenStyles as styles } from '../src/diagnostics/connection-diagnostics-screen-styles' -import type { ConnectionLogEntry, HostProfile } from '../src/transport/types' - -// Why: getSnapshot must be referentially stable when there's no data — -// a fresh [] per call would make useSyncExternalStore re-render forever. -const EMPTY_ENTRIES: readonly ConnectionLogEntry[] = [] +import type { HostProfile } from '../src/transport/types' // Why: reading the log is most needed while a host is failing, so this // screen also *acquires* the host client — opening it kicks a dial and the @@ -44,21 +23,14 @@ export default function ConnectionLogScreen() { const params = useLocalSearchParams<{ hostId?: string }>() const routeKey = useMemo(() => ({}), [params.hostId]) const [hosts, setHosts] = useState([]) - const [manualSelection, setManualSelection] = useState<{ - hostId: string - requestedHostId: string | undefined - routeKey: object - } | null>(null) - const [copiedHostId, setCopiedHostId] = useState(null) - const [submissionStates, setSubmissionStates] = useState({}) + const [manualSelection, setManualSelection] = useState(null) useEffect(() => { let stale = false void loadHosts().then((loaded) => { - if (stale) { - return + if (!stale) { + setHosts(loaded) } - setHosts(loaded) }) return () => { stale = true @@ -66,179 +38,45 @@ export default function ConnectionLogScreen() { }, []) const selectedId = resolveDiagnosticsHostId(hosts, params.hostId, manualSelection, routeKey) - const selected = hosts.find((h) => h.id === selectedId) ?? null + const selected = hosts.find((host) => host.id === selectedId) ?? null const { client, state } = useHostClient(selected?.id) - const { desktopAppVersion: liveDesktopAppVersion } = useHostStatusGates({ - hostId: selected?.id, - client, - connState: state - }) - const reconnectAttempts = useReconnectAttempt(selected?.id) - const { activePath, pendingPath } = useConnectionPathStatus(selected?.id) - - useEffect(() => { - if (selectedId) { - void readHydratedConnectionLog(connectionLogStore, selectedId) - } - }, [selectedId]) - - const subscribe = useCallback( - (listener: () => void) => - selectedId ? connectionLogStore.subscribe(selectedId, listener) : () => {}, - [selectedId] + // Why: refreshes the persisted desktop version the diagnostics report reads back. + useHostStatusGates({ hostId: selected?.id, client, connState: state }) + const device = useMemo( + () => (selected ? createNativeDiagnosticsOperations(selected, clientContext) : null), + [selected, clientContext] ) - const getSnapshot = useCallback( - () => (selectedId ? connectionLogStore.get(selectedId) : EMPTY_ENTRIES), - [selectedId] + const select = useCallback( + (hostId: string) => setManualSelection({ hostId, requestedHostId: params.hostId, routeKey }), + [params.hostId, routeKey] ) - const entries = useSyncExternalStore(subscribe, getSnapshot) - const subscribeMobileWeb = useCallback( - (listener: () => void) => mobileWebDiagnosticsStore.subscribe(listener), - [] - ) - const getMobileWebSnapshot = useCallback( - () => mobileWebDiagnosticsStore.get(selectedId), - [selectedId] - ) - const mobileWebDiagnostics = useSyncExternalStore(subscribeMobileWeb, getMobileWebSnapshot) - const diagnosis = selected - ? diagnoseConnection({ endpoint: selected.endpoint, state, activePath, pendingPath, entries }) - : null - const incidentId = selected - ? getReportableConnectionIncidentId({ - endpoint: selected.endpoint, - state, - activePath, - pendingPath, - entries - }) - : null - const submissionKey = selected && incidentId ? `${selected.id}:${incidentId}` : null - const submissionState = getDiagnosticsSubmissionState(submissionStates, submissionKey) - const copied = copiedHostId === selectedId - - const copyDiagnostics = useCallback(async () => { - if (!selected) { - return - } - const desktopAppVersion = liveDesktopAppVersion ?? (await loadHostAppVersion(selected.id)) - const snapshot = await readConnectionDiagnosticsSnapshot( - clientContext, - connectionLogStore, - selected.id - ) - const report = buildConnectionDiagnosticsReport({ - endpoint: selected.endpoint, - state: snapshot.state, - reconnectAttempts: snapshot.reconnectAttempts, - lastConnectedAt: snapshot.lastConnectedAt, - platform: `${Platform.OS} ${Platform.Version ?? ''}`.trim(), - appVersion: Constants.expoConfig?.version ?? 'unknown', - desktopAppVersion, - entries: snapshot.entries, - activePath: snapshot.activePath, - pendingPath: snapshot.pendingPath, - mobileWeb: mobileWebDiagnostics - }) - await Clipboard.setStringAsync(report) - setCopiedHostId(selected.id) - setTimeout(() => setCopiedHostId((hostId) => (hostId === selected.id ? null : hostId)), 2000) - }, [selected, liveDesktopAppVersion, clientContext, mobileWebDiagnostics]) - - const sendDiagnostics = useCallback(async () => { - if (!selected || !submissionKey || submissionState === 'sending') { - return - } - const startedKey = submissionKey - setSubmissionStates((states) => updateDiagnosticsSubmissionState(states, startedKey, 'sending')) - const appVersion = Constants.expoConfig?.version ?? 'unknown' - const platform = `${Platform.OS} ${Platform.Version ?? ''}`.trim() - const desktopAppVersion = liveDesktopAppVersion ?? (await loadHostAppVersion(selected.id)) - const snapshot = await readConnectionDiagnosticsSnapshot( - clientContext, - connectionLogStore, - selected.id - ) - const currentIncidentId = getReportableConnectionIncidentId({ - endpoint: selected.endpoint, - state: snapshot.state, - activePath: snapshot.activePath, - pendingPath: snapshot.pendingPath, - entries: snapshot.entries - }) - if (`${selected.id}:${currentIncidentId ?? ''}` !== startedKey) { - setSubmissionStates((states) => updateDiagnosticsSubmissionState(states, startedKey, null)) - return - } - const report = buildConnectionDiagnosticsReport({ - endpoint: selected.endpoint, - state: snapshot.state, - reconnectAttempts: snapshot.reconnectAttempts, - lastConnectedAt: snapshot.lastConnectedAt, - platform, - appVersion, - desktopAppVersion, - entries: snapshot.entries, - activePath: snapshot.activePath, - pendingPath: snapshot.pendingPath, - mobileWeb: mobileWebDiagnostics - }) - const result = await submitConnectionDiagnostics({ report, appVersion, platform }) - setSubmissionStates((states) => - updateDiagnosticsSubmissionState(states, startedKey, result.ok ? 'sent' : 'failed') - ) - }, [ - selected, - submissionKey, - submissionState, - liveDesktopAppVersion, - clientContext, - mobileWebDiagnostics - ]) return ( - Clipboard.setStringAsync(report)} onBack={() => router.back()} hostPicker={ - <> - {' '} - {hosts.length > 1 && ( - - {hosts.map((host) => ( - - setManualSelection({ - hostId: host.id, - requestedHostId: params.hostId, - routeKey - }) - } + hosts.length > 1 ? ( + + {hosts.map((host) => ( + select(host.id)} + > + - - {host.name} - - - ))} - - )} - + {host.name} + + + ))} + + ) : null } /> ) diff --git a/mobile/host-web-app/connection-log.tsx b/mobile/host-web-app/connection-log.tsx index eac1997842d..9645ac90baf 100644 --- a/mobile/host-web-app/connection-log.tsx +++ b/mobile/host-web-app/connection-log.tsx @@ -1,6 +1,6 @@ import { useRouter } from 'expo-router' import { useMobileWebNativeShell } from '../../src/mobile-web/src/native-shell-channel' -import { HostedConnectionDiagnosticsScreen } from '../src/diagnostics/hosted-connection-diagnostics-screen' +import { ConnectionDiagnosticsScreen } from '../src/diagnostics/connection-diagnostics-screen' export default function HostedConnectionLogRoute() { const router = useRouter() @@ -8,10 +8,13 @@ export default function HostedConnectionLogRoute() { if (!shell.client) { return null } + const client = shell.client return ( - client.native.clipboardWrite(report)} onBack={() => (router.canGoBack() ? router.back() : router.replace('/settings'))} /> ) diff --git a/mobile/src/diagnostics/connection-diagnostics-screen.test.tsx b/mobile/src/diagnostics/connection-diagnostics-screen.test.tsx new file mode 100644 index 00000000000..15ae88dc912 --- /dev/null +++ b/mobile/src/diagnostics/connection-diagnostics-screen.test.tsx @@ -0,0 +1,106 @@ +import { createElement, useEffect } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ConnectionDiagnosticsScreen } from './connection-diagnostics-screen' +import type { DiagnosticsDeviceOperations } from './diagnostics-device-operations' +import type { DiagnosticsSnapshot } from '../../../src/shared/mobile-web/diagnostics-device-contract' + +vi.mock('react-native', () => ({ + View: 'View', + Text: 'Text', + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (value: unknown) => value, hairlineWidth: 1 } +})) +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0 }) +})) +vi.mock('expo-router', () => ({ + useFocusEffect: (callback: () => void | (() => void)) => useEffect(callback, [callback]) +})) +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'Icon', + Copy: 'Icon', + Check: 'Icon', + Send: 'Icon' +})) +vi.mock('../components/ConnectionLog', () => ({ ConnectionLog: () => null })) + +const SNAPSHOT: DiagnosticsSnapshot = { + state: 'disconnected', + reconnectAttempts: 3, + lastConnectedAt: null, + activePath: 'lan', + pendingPath: null, + endpointIsTailscale: false, + platform: 'ios 18', + appVersion: '1.0.0', + desktopAppVersion: '2.0.0', + entries: [], + mobileWeb: {} +} + +let renderer: ReactTestRenderer | undefined +afterEach(() => { + act(() => renderer?.unmount()) + renderer = undefined +}) + +function device(overrides: Partial = {}) { + return { + snapshot: vi.fn().mockResolvedValue(SNAPSHOT), + probe: vi.fn().mockResolvedValue({ reachable: true }), + submit: vi.fn().mockResolvedValue({ ok: true }), + ...overrides + } as unknown as DiagnosticsDeviceOperations +} + +async function mount(props: Parameters[0]) { + await act(async () => { + renderer = create(createElement(ConnectionDiagnosticsScreen, props)) + }) +} + +function texts(): string[] { + return renderer!.root + .findAllByType('Text') + .flatMap((node) => node.children.filter((child): child is string => typeof child === 'string')) +} + +describe('shared connection diagnostics screen', () => { + it('reports either host through one device operations seam', async () => { + const write = vi.fn().mockResolvedValue(undefined) + const operations = device() + await mount({ + device: operations, + hostName: 'Desk', + writeClipboard: write, + onBack: () => {} + }) + + expect(texts()).toContain('disconnected') + expect(texts()).toContain(' · attempt 3') + expect(operations.snapshot).toHaveBeenCalled() + await act(async () => { + await renderer!.root.findAllByType('Pressable')[1]!.props.onPress() + }) + expect(write).toHaveBeenCalledWith(expect.stringContaining('2.0.0')) + }) + + it('surfaces a device failure instead of a permanent loading state', async () => { + await mount({ + device: device({ snapshot: vi.fn().mockRejectedValue(new Error('unreachable')) }), + hostName: 'Desk', + writeClipboard: vi.fn(), + onBack: () => {} + }) + + expect(texts().some((text) => text.includes('Could not load network diagnostics'))).toBe(true) + }) + + it('renders no host rather than loading when no host is selected', async () => { + await mount({ device: null, hostName: null, writeClipboard: vi.fn(), onBack: () => {} }) + + expect(texts()).not.toContain('Loading network diagnostics…') + }) +}) diff --git a/mobile/src/diagnostics/connection-diagnostics-screen.tsx b/mobile/src/diagnostics/connection-diagnostics-screen.tsx index 9c76244d7d6..e756a44e2ce 100644 --- a/mobile/src/diagnostics/connection-diagnostics-screen.tsx +++ b/mobile/src/diagnostics/connection-diagnostics-screen.tsx @@ -1,7 +1,6 @@ -import { useCallback, useState } from 'react' +import { useCallback, useState, type ReactNode } from 'react' import { Text } from 'react-native' import { useFocusEffect } from 'expo-router' -import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' import type { DiagnosticsSnapshot } from '../../../src/shared/mobile-web/diagnostics-device-contract' import type { ConnectionLogEntry } from '../transport/types' import type { MobileWebDiagnosticsSnapshot } from '../mobile-web/mobile-web-diagnostics-store' @@ -17,21 +16,40 @@ import { type DiagnosticsSubmissionStates } from './connection-diagnostics-screen-data' import { connectionDiagnosticsScreenStyles as styles } from './connection-diagnostics-screen-styles' +import type { DiagnosticsDeviceOperations } from './diagnostics-device-operations' -export function HostedConnectionDiagnosticsScreen({ - client, - onBack +function reportable(snapshot: DiagnosticsSnapshot) { + return { + ...snapshot, + entries: snapshot.entries as ConnectionLogEntry[], + mobileWeb: snapshot.mobileWeb as MobileWebDiagnosticsSnapshot + } +} + +// Why: reading the log matters most while a host is failing, so the screen +// re-polls the device instead of rendering a snapshot taken on mount. +export function ConnectionDiagnosticsScreen({ + device, + hostName, + writeClipboard, + onBack, + hostPicker }: { - client: MobileWebBridgeClient + device: DiagnosticsDeviceOperations | null + hostName: string | null + writeClipboard: (report: string) => Promise onBack: () => void + hostPicker?: ReactNode }) { - const device = client.native.diagnosticsDevice const [snapshot, setSnapshot] = useState(null) const [error, setError] = useState(null) const [copied, setCopied] = useState(false) const [submissions, setSubmissions] = useState({}) useFocusEffect( useCallback(() => { + if (!device) { + return + } let active = true let pending = false const refresh = async () => { @@ -61,44 +79,31 @@ export function HostedConnectionDiagnosticsScreen({ } }, [device]) ) - const data = snapshot - ? { - ...snapshot, - entries: snapshot.entries as ConnectionLogEntry[], - mobileWeb: snapshot.mobileWeb as MobileWebDiagnosticsSnapshot - } - : null + const data = snapshot ? reportable(snapshot) : null const diagnosis = data ? diagnoseConnection(data) : null const incident = data ? getReportableConnectionIncidentId(data) : null const submissionState = getDiagnosticsSubmissionState(submissions, incident) const copyDiagnostics = async () => { + if (!device) { + return + } try { - const fresh = await device.snapshot() - await client.native.clipboardWrite( - buildConnectionDiagnosticsReport({ - ...fresh, - entries: fresh.entries as ConnectionLogEntry[], - mobileWeb: fresh.mobileWeb as MobileWebDiagnosticsSnapshot - }) - ) + await writeClipboard(buildConnectionDiagnosticsReport(reportable(await device.snapshot()))) setCopied(true) + setTimeout(() => setCopied(false), 2000) } catch { setError('Could not copy the report. Try again.') } } const sendDiagnostics = async () => { - if (!incident || submissionState === 'sending') { + if (!device || !incident || submissionState === 'sending') { return } const started = incident setSubmissions((states) => updateDiagnosticsSubmissionState(states, started, 'sending')) try { const fresh = await device.snapshot() - const reportData = { - ...fresh, - entries: fresh.entries as ConnectionLogEntry[], - mobileWeb: fresh.mobileWeb as MobileWebDiagnosticsSnapshot - } + const reportData = reportable(fresh) if (getReportableConnectionIncidentId(reportData) !== started) { setSubmissions((states) => updateDiagnosticsSubmissionState(states, started, null)) return @@ -117,8 +122,8 @@ export function HostedConnectionDiagnosticsScreen({ } return ( - {error} - - ) : null + <> + {hostPicker} + {error ? ( + + {error} + + ) : null} + } /> )