From cf5aa5c391a5fb69faec4e01a6e6d07638324964 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 18 Sep 2026 00:21:14 -0400 Subject: [PATCH] refactor(mobile): dial the host on tap in the dev bundle row, and name it Opening Troubleshoot in a dev build acquired a client at mount, which is what kicks a dial, on a screen that opened no connection before. The probe now acquires only once the row is tapped, and each request owns its AbortController so a re-run, an unmount or StrictMode's second mount abandons the previous fetch and stops its chunk reads instead of holding the host's read slots. The screen carries no host parameter and troubleshoots every paired host, so there is no host it is "on": the row still takes the first paired host but now names it in the result instead of implying it speaks for all of them. The label says whether it is still connecting or already fetching. There is no `__DEV__`-conditional `require` idiom in this repo to trim the row out of a release bundle with, which the route now records. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/troubleshoot.tsx | 6 +- .../mobile-web-bundle-probe-row.tsx | 38 ++-- .../use-mobile-web-bundle-probe.test.tsx | 199 ++++++++++++++++++ .../use-mobile-web-bundle-probe.ts | 52 +++-- 4 files changed, 260 insertions(+), 35 deletions(-) create mode 100644 mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx diff --git a/mobile/app/troubleshoot.tsx b/mobile/app/troubleshoot.tsx index a2d4e48c681..d64b238b577 100644 --- a/mobile/app/troubleshoot.tsx +++ b/mobile/app/troubleshoot.tsx @@ -3,7 +3,11 @@ import { MobileWebBundleProbeRow } from '../src/diagnostics/mobile-web-bundle-pr import { TroubleshootView } from '../src/diagnostics/troubleshoot-view' import { useTroubleshootDiagnostics } from '../src/diagnostics/use-troubleshoot-diagnostics' -// Same guard as push-token.ts: `__DEV__` is undefined outside the React Native runtime. +// Same guard as push-token.ts: `__DEV__` is undefined outside the React Native runtime. The import +// above is static, so a release bundle still carries the row's graph and evaluates its hoisted +// schemas at load; nothing mounts, no host is looked up and no request is made. This repo has no +// `__DEV__`-conditional `require` idiom to trim it with — every `require` in `mobile/src` is a Metro +// asset path — so introducing one is a change for the shell in Phase B, not for this row. const isDevelopmentBuild = typeof __DEV__ !== 'undefined' && __DEV__ export default function NativeTroubleshootRoute() { diff --git a/mobile/src/diagnostics/mobile-web-bundle-probe-row.tsx b/mobile/src/diagnostics/mobile-web-bundle-probe-row.tsx index 9dc588f4c16..06a1db2efcb 100644 --- a/mobile/src/diagnostics/mobile-web-bundle-probe-row.tsx +++ b/mobile/src/diagnostics/mobile-web-bundle-probe-row.tsx @@ -2,9 +2,7 @@ import { useEffect, useState } from 'react' import { View, Text, Pressable, ActivityIndicator } from 'react-native' import { Package } from 'lucide-react-native' import { loadHosts } from '../transport/host-store' -import { useHostClient } from '../transport/client-context' import { colors } from '../theme/mobile-theme' -import { selectDiagnosticsHostId } from './connection-diagnostics-screen-data' import { troubleshootScreenStyles as styles } from './troubleshoot-screen-styles' import { useMobileWebBundleProbe, @@ -12,6 +10,14 @@ import { } from './use-mobile-web-bundle-probe' import type { HostProfile } from '../transport/types' +/** Dialling the host is part of the tap, so the label says which half is still running. */ +function buttonLabel(state: MobileWebBundleProbeState, awaitingHost: boolean): string { + if (state.status !== 'running') { + return 'Fetch mobile web bundle' + } + return awaitingHost ? 'Connecting…' : 'Fetching bundle…' +} + /** One `label — detail` line in the same row shape the diagnostic checks use. */ function ProbeLine({ label, detail, failed }: { label: string; detail: string; failed?: boolean }) { return ( @@ -22,7 +28,7 @@ function ProbeLine({ label, detail, failed }: { label: string; detail: string; f ) } -function ProbeResult({ state }: { state: MobileWebBundleProbeState }) { +function ProbeResult({ state, hostName }: { state: MobileWebBundleProbeState; hostName: string }) { if (state.status === 'idle' || state.status === 'running') { return null } @@ -35,6 +41,8 @@ function ProbeResult({ state }: { state: MobileWebBundleProbeState }) { } return ( + + @@ -47,12 +55,15 @@ function ProbeResult({ state }: { state: MobileWebBundleProbeState }) { } /** - * Development-only: fetches the whole mobile web bundle from the paired desktop and reports what - * came back. Phase A ships no production path that renders a bundle, and this row is the only thing - * that exercises the operations end to end on a device. + * Development-only: fetches the whole mobile web bundle from a paired desktop and reports what came + * back. Phase A ships no production path that renders a bundle, and this row is the only thing that + * exercises the operations end to end on a device. * - * `app/troubleshoot.tsx` mounts it behind `__DEV__`, so a shipped build never runs the host lookup - * or acquires a client on this screen. + * `app/troubleshoot.tsx` mounts it behind `__DEV__`, so a shipped build never runs the host lookup. + * + * The screen carries no host parameter and troubleshoots every paired host, one reachability check + * each, so there is no host it is "on". The probe takes the first paired host and names it in the + * result rather than implying it speaks for all of them. */ export function MobileWebBundleProbeRow() { const [hosts, setHosts] = useState([]) @@ -67,9 +78,8 @@ export function MobileWebBundleProbeRow() { stale = true } }, []) - const hostId = selectDiagnosticsHostId(hosts, undefined, null) - const { client } = useHostClient(hostId ?? undefined) - const { state, run } = useMobileWebBundleProbe(client) + const host = hosts[0] ?? null + const { state, run, awaitingHost } = useMobileWebBundleProbe(host?.id ?? null) return ( @@ -88,11 +98,9 @@ export function MobileWebBundleProbeRow() { ) : ( )} - - {state.status === 'running' ? 'Fetching bundle…' : 'Fetch mobile web bundle'} - + {buttonLabel(state, awaitingHost)} - + ) } diff --git a/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx b/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx new file mode 100644 index 00000000000..e3df8d39fa2 --- /dev/null +++ b/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx @@ -0,0 +1,199 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { MobileWebBundleFetchResult } from '../transport/mobile-web-bundle-fetch' + +const push = vi.hoisted(() => ({ attach: vi.fn(), detach: vi.fn() })) +vi.mock('../notifications/push-registration', () => ({ attachPushRegistration: push.attach })) + +const connectMock = vi.hoisted(() => vi.fn()) +const loadHostsMock = vi.hoisted(() => vi.fn()) +const fetchMock = vi.hoisted(() => vi.fn()) + +vi.mock('../transport/rpc-client', () => ({ + connect: (...args: unknown[]) => connectMock(...args) +})) +vi.mock('../transport/host-logical-client', () => ({ + openHostLogicalClient: (...args: unknown[]) => connectMock(...args) +})) +vi.mock('../transport/host-store', () => ({ loadHosts: () => loadHostsMock() })) +vi.mock('../transport/connection-revival-triggers', () => ({ + subscribeConnectionRevivalTriggers: () => () => {} +})) +vi.mock('../transport/mobile-web-bundle-fetch', () => ({ + fetchMobileWebBundle: (...args: unknown[]) => fetchMock(...args) +})) + +import { RpcClientProvider } from '../transport/client-context' +import { + useMobileWebBundleProbe, + type MobileWebBundleProbeState +} from './use-mobile-web-bundle-probe' + +const HOST = { + id: 'host-1', + name: 'Host 1', + endpoint: 'ws://127.0.0.1:1', + deviceToken: 'token', + publicKeyB64: 'key', + lastConnected: 0 +} + +function fakeClient(): RpcClient { + return { + sendRequest: vi.fn(), + subscribe: vi.fn(() => () => {}), + updateTerminalSubscriptionViewport: vi.fn(), + getState: () => 'connected', + getReconnectAttempt: () => 0, + getLastConnectedAt: () => null, + onStateChange: () => () => {}, + notifyForeground: vi.fn(), + close: vi.fn() + } +} + +type ProbeHarness = { + readonly state: MobileWebBundleProbeState + readonly awaitingHost: boolean + run: () => Promise + unmount: () => Promise +} + +async function renderProbe(hostId: string | null): Promise { + let latest: ReturnType | null = null + let renderer: ReactTestRenderer | null = null + + function Probe(): null { + latest = useMobileWebBundleProbe(hostId) + return null + } + + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + }) + const mounted = renderer as ReactTestRenderer | null + const read = () => { + if (!latest) { + throw new Error('probe did not render') + } + return latest + } + return { + get state() { + return read().state + }, + get awaitingHost() { + return read().awaitingHost + }, + run: async () => { + await act(async () => { + read().run() + }) + }, + unmount: async () => { + await act(async () => { + mounted?.unmount() + }) + } + } +} + +function fetchedBundle(): MobileWebBundleFetchResult { + return { + manifest: { + schemaVersion: 1, + buildId: 'a'.repeat(64), + entrypoint: 'index.html', + totalBytes: 3, + assets: [ + { path: 'index.html', sha256: 'b'.repeat(64), byteLength: 3, contentType: 'text/html' } + ] + }, + assets: new Map([['index.html', new Uint8Array([1, 2, 3])]]), + totalBytes: 3, + elapsedMs: 12 + } +} + +beforeEach(() => { + push.attach.mockReset().mockReturnValue(push.detach) + push.detach.mockReset() + connectMock.mockReset().mockReturnValue(fakeClient()) + loadHostsMock.mockReset().mockResolvedValue([HOST]) + fetchMock.mockReset() +}) + +describe('useMobileWebBundleProbe', () => { + it('dials no host until the row is tapped', async () => { + const probe = await renderProbe(HOST.id) + + expect(connectMock).not.toHaveBeenCalled() + expect(fetchMock).not.toHaveBeenCalled() + expect(probe.state).toEqual({ status: 'idle' }) + + fetchMock.mockResolvedValue(fetchedBundle()) + await probe.run() + + expect(connectMock).toHaveBeenCalledTimes(1) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(probe.state).toEqual({ + status: 'done', + buildId: 'a'.repeat(64), + assetCount: 1, + totalBytes: 3, + elapsedMs: 12 + }) + expect(probe.awaitingHost).toBe(false) + }) + + it('reports the host code when the desktop refused', async () => { + fetchMock.mockRejectedValue(new Error('invalid_argument: mobile_web_bundle_unavailable')) + const probe = await renderProbe(HOST.id) + + await probe.run() + + expect(probe.state).toEqual({ status: 'failed', detail: 'mobile_web_bundle_unavailable' }) + }) + + it('reports a schema refusal, which carries no code, as its message', async () => { + fetchMock.mockRejectedValue( + new Error('invalid_argument: Invalid input: expected string, received number') + ) + const probe = await renderProbe(HOST.id) + + await probe.run() + + expect(probe.state).toEqual({ + status: 'failed', + detail: 'invalid_argument: Invalid input: expected string, received number' + }) + }) + + it('fails without dialling when no host is paired', async () => { + const probe = await renderProbe(null) + + await probe.run() + + expect(connectMock).not.toHaveBeenCalled() + expect(fetchMock).not.toHaveBeenCalled() + expect(probe.state).toEqual({ status: 'failed', detail: 'no paired host to fetch from' }) + }) + + it('aborts the run it started when the screen goes away', async () => { + let captured: AbortSignal | null = null + fetchMock.mockImplementation((args: { signal?: AbortSignal }) => { + captured = args.signal ?? null + return new Promise(() => {}) + }) + const probe = await renderProbe(HOST.id) + await probe.run() + + const signal = captured as AbortSignal | null + expect(signal?.aborted).toBe(false) + await probe.unmount() + + expect(signal?.aborted).toBe(true) + }) +}) diff --git a/mobile/src/diagnostics/use-mobile-web-bundle-probe.ts b/mobile/src/diagnostics/use-mobile-web-bundle-probe.ts index 4949a36aa12..9f780e06571 100644 --- a/mobile/src/diagnostics/use-mobile-web-bundle-probe.ts +++ b/mobile/src/diagnostics/use-mobile-web-bundle-probe.ts @@ -1,7 +1,7 @@ -import { useCallback, useRef, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' +import { useHostClient } from '../transport/client-context' import { fetchMobileWebBundle } from '../transport/mobile-web-bundle-fetch' import { readMobileWebBundleErrorCode } from '../transport/mobile-web-bundle-operations' -import type { RpcClient } from '../transport/rpc-client' export type MobileWebBundleProbeState = | { status: 'idle' } @@ -16,7 +16,7 @@ export type MobileWebBundleProbeState = | { status: 'failed'; detail: string } /** The host's own code when it refused, its message otherwise. A client-side integrity failure has - * no code and reads as the sentence it threw. */ + * no code, and so does a schema refusal the dispatcher raised before the handler ran. */ function describeFailure(error: unknown): string { const code = readMobileWebBundleErrorCode(error) if (code !== null) { @@ -28,30 +28,31 @@ function describeFailure(error: unknown): string { /** * Drives one bundle fetch from the troubleshooting screen. Dev-only: nothing in a shipped build * mounts this, and nothing here caches or renders what it downloads. + * + * The host is dialled on the first tap, not on mount: acquiring a client is what opens a connection, + * and opening Troubleshoot opened none before this row existed. Each request owns its + * `AbortController` so a re-run, an unmount, or StrictMode's second mount abandons the previous + * fetch instead of racing it — and, since the fetch checks that signal before every chunk, stops its + * reads rather than letting them hold the host's read slots. */ -export function useMobileWebBundleProbe(client: RpcClient | null): { +export function useMobileWebBundleProbe(hostId: string | null): { state: MobileWebBundleProbeState run: () => void + awaitingHost: boolean } { const [state, setState] = useState({ status: 'idle' }) - const runIdRef = useRef(0) - const abortRef = useRef(null) + const [request, setRequest] = useState<{ id: number } | null>(null) + const { client } = useHostClient(request !== null && hostId !== null ? hostId : undefined) - const run = useCallback(() => { - if (!client) { - setState({ status: 'failed', detail: 'no paired host is connected' }) + useEffect(() => { + if (request === null || client === null) { return } - // A second tap abandons the first run rather than racing it to the same state. - abortRef.current?.abort() + let abandoned = false const controller = new AbortController() - abortRef.current = controller - const runId = runIdRef.current + 1 - runIdRef.current = runId - setState({ status: 'running' }) fetchMobileWebBundle({ client, signal: controller.signal }).then( (fetched) => { - if (runIdRef.current !== runId) { + if (abandoned) { return } setState({ @@ -63,13 +64,26 @@ export function useMobileWebBundleProbe(client: RpcClient | null): { }) }, (error: unknown) => { - if (runIdRef.current !== runId) { + if (abandoned) { return } setState({ status: 'failed', detail: describeFailure(error) }) } ) - }, [client]) + return () => { + abandoned = true + controller.abort() + } + }, [client, request]) - return { state, run } + const run = useCallback(() => { + if (hostId === null) { + setState({ status: 'failed', detail: 'no paired host to fetch from' }) + return + } + setState({ status: 'running' }) + setRequest((previous) => ({ id: (previous?.id ?? 0) + 1 })) + }, [hostId]) + + return { state, run, awaitingHost: request !== null && client === null } }