diff --git a/src/renderer/src/components/mobile/MobilePage.test.tsx b/src/renderer/src/components/mobile/MobilePage.test.tsx index 1df34602926..fdcd4ae4c64 100644 --- a/src/renderer/src/components/mobile/MobilePage.test.tsx +++ b/src/renderer/src/components/mobile/MobilePage.test.tsx @@ -596,13 +596,10 @@ describe('MobilePage pairing connection mode', () => { await waitFor(() => expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9') ) - await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) - await new Promise((resolve) => setTimeout(resolve, 20)) - // One Continue is one offer. Minting against the stale address and then // rotating to the settled one runs two overlapping mints through main, whose // rotate deletes the pending credential the first mint already returned. - expect(getPairingQR).toHaveBeenCalledTimes(1) + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) expect(getPairingQR).toHaveBeenCalledWith({ address: '10.0.0.9', connectionMode: 'automatic' @@ -624,13 +621,209 @@ describe('MobilePage pairing connection mode', () => { // lookup, so the auto-mint always runs before any address is known. await user.click(screen.getByRole('button', { name: 'Pair another device' })) + await waitFor(() => + expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.5') + ) await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) - await new Promise((resolve) => setTimeout(resolve, 20)) - expect(getPairingQR).toHaveBeenCalledTimes(1) expect(getPairingQR).toHaveBeenCalledWith({ address: '10.0.0.5', connectionMode: 'automatic' }) + + // Returning to the paired list and pairing again is a fresh visit: it must + // wait for its own lookup, not inherit the previous visit's answer. + await user.click(screen.getByRole('button', { name: 'Back' })) + await user.click(screen.getByRole('button', { name: 'Back' })) + await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('paired')) + getPairingQR.mockClear() + let resolveSecondLookup: ((value: Record) => void) | undefined + listNetworkInterfaces.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecondLookup = resolve + }) + ) + await user.click(screen.getByRole('button', { name: 'Pair another device' })) + expect(getPairingQR).not.toHaveBeenCalled() + + resolveSecondLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.9' }] }) + await waitFor(() => + expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9') + ) + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) + expect(getPairingQR).toHaveBeenCalledWith({ + address: '10.0.0.9', + connectionMode: 'automatic' + }) + }) + + it('waits for the newest lookup when the flow is re-entered mid-refresh', async () => { + const user = userEvent.setup() + let resolveFirstLookup: ((value: Record) => void) | undefined + let resolveSecondLookup: ((value: Record) => void) | undefined + listNetworkInterfaces + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirstLookup = resolve + }) + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecondLookup = resolve + }) + ) + render() + await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro')) + + // Enter, leave, and re-enter while the first lookup is still unanswered, so + // two lookups overlap and the older one is the first to settle. + await user.click(screen.getByRole('button', { name: 'Enter flow' })) + await user.click(screen.getByRole('button', { name: 'Back' })) + await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro')) + await user.click(screen.getByRole('button', { name: 'Enter flow' })) + await user.click(screen.getByRole('button', { name: 'Continue' })) + await waitFor(() => expect(listNetworkInterfaces).toHaveBeenCalledTimes(2)) + + // The superseded lookup answers first. It must neither move the picker nor + // release the mint — this visit's lookup has not answered yet. + resolveFirstLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] }) + await waitFor(() => expect(screen.getByTestId('pair-loading')).toHaveTextContent('true')) + expect(getPairingQR).not.toHaveBeenCalled() + expect(screen.getByTestId('selected-address')).toHaveTextContent('none') + + resolveSecondLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.9' }] }) + await waitFor(() => + expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9') + ) + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) + expect(getPairingQR).toHaveBeenCalledWith({ + address: '10.0.0.9', + connectionMode: 'automatic' + }) + }) + + it('does not re-block Step 2 when an abandoned visit’s lookup settles last', async () => { + const user = userEvent.setup() + let resolveAbandonedLookup: ((value: Record) => void) | undefined + let resolveCurrentLookup: ((value: Record) => void) | undefined + listNetworkInterfaces + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveAbandonedLookup = resolve + }) + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveCurrentLookup = resolve + }) + ) + render() + await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro')) + await user.click(screen.getByRole('button', { name: 'Enter flow' })) + await user.click(screen.getByRole('button', { name: 'Back' })) + await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro')) + await user.click(screen.getByRole('button', { name: 'Enter flow' })) + await user.click(screen.getByRole('button', { name: 'Continue' })) + await waitFor(() => expect(listNetworkInterfaces).toHaveBeenCalledTimes(2)) + + resolveCurrentLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.9' }] }) + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) + await waitFor(() => expect(screen.getByTestId('pair-loading')).toHaveTextContent('false')) + + // The abandoned visit answers last. Recording it as the settled visit would + // walk the marker backwards and leave Step 2 waiting on a lookup that is + // never coming, with its Generate action disabled. + resolveAbandonedLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] }) + await waitFor(() => + expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('false') + ) + expect(screen.getByTestId('pair-loading')).toHaveTextContent('false') + expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9') + expect(getPairingQR).toHaveBeenCalledTimes(1) + }) + + it('ignores an interface lookup that settles after a newer one', async () => { + listNetworkInterfaces.mockResolvedValue({ + interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] + }) + const user = userEvent.setup() + await openPairingStep() + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) + + // Two manual refreshes overlap; the older one answers last with a stale list. + let resolveStaleRefresh: ((value: Record) => void) | undefined + listNetworkInterfaces.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveStaleRefresh = resolve + }) + ) + await user.click(screen.getByRole('button', { name: 'Refresh addresses' })) + listNetworkInterfaces.mockResolvedValueOnce({ + interfaces: [{ name: 'Wi-Fi', address: '10.0.0.7' }] + }) + await user.click(screen.getByRole('button', { name: 'Refresh addresses' })) + await waitFor(() => + expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.7') + ) + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(2)) + expect(getPairingQR).toHaveBeenLastCalledWith({ + address: '10.0.0.7', + connectionMode: 'automatic', + rotate: true + }) + + resolveStaleRefresh?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.1' }] }) + await waitFor(() => + expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('false') + ) + + // The stale list must not reselect an address and rotate the live offer away. + expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.7') + expect(getPairingQR).toHaveBeenCalledTimes(2) + }) + + it('does not report the pairing step as busy during a manual address refresh', async () => { + listNetworkInterfaces.mockResolvedValue({ + interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] + }) + // Leave Step 2 with no QR on screen: that is the state where a refresh could + // be mistaken for a mint in progress. + getPairingQR.mockResolvedValueOnce({ + available: false, + reason: 'websocket_unavailable', + guidance: 'WebSocket transport is not running' + }) + const user = userEvent.setup() + await openPairingStep() + await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1)) + await waitFor(() => expect(screen.getByTestId('pair-loading')).toHaveTextContent('false')) + expect(screen.getByTestId('pairing-qr')).toHaveTextContent('none') + expect(screen.getByTestId('relay-failure')).toHaveTextContent('none') + + let resolveRefresh: ((value: Record) => void) | undefined + listNetworkInterfaces.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveRefresh = resolve + }) + ) + await user.click(screen.getByRole('button', { name: 'Refresh addresses' })) + await waitFor(() => + expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('true') + ) + + // Nothing is minting, so Step 2 must not claim it is — that would disable the + // Generate action while the user is only re-reading the interface list. + expect(screen.getByTestId('pair-loading')).toHaveTextContent('false') + resolveRefresh?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] }) + await waitFor(() => + expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('false') + ) }) it('keeps custom intent when the saved address is also discovered', async () => { diff --git a/src/renderer/src/components/mobile/MobilePage.tsx b/src/renderer/src/components/mobile/MobilePage.tsx index 79d3df74434..bd6e03e89d5 100644 --- a/src/renderer/src/components/mobile/MobilePage.tsx +++ b/src/renderer/src/components/mobile/MobilePage.tsx @@ -62,9 +62,13 @@ export default function MobilePage(): React.JSX.Element { const [refreshingNetworkInterfaces, setRefreshingNetworkInterfaces] = useState(false) const hasGeneratedRef = useRef(false) const pairingRequestIdRef = useRef(0) - // Why: written synchronously so the Step 2 auto-mint sees a lookup that started - // in the same commit, before `refreshingNetworkInterfaces` has been committed. - const networkInterfacesPendingRef = useRef(false) + // Why: each flow entry starts its own address lookup. Gating the Step 2 mint on + // "has this visit's lookup settled" is false until it answers, where "is a lookup + // running" cannot tell overlapping lookups apart and clears on the first to land. + const [pairingFlowVisit, setPairingFlowVisit] = useState(0) + const [addressedFlowVisit, setAddressedFlowVisit] = useState(-1) + const pairingAddressSettled = addressedFlowVisit === pairingFlowVisit + const networkInterfacesRequestIdRef = useRef(0) const mountedRef = useMountedRef() const closeMobilePage = useAppStore((s) => s.closeMobilePage) const showMobileButton = useAppStore((s) => s.settings?.showMobileButton !== false) @@ -191,25 +195,32 @@ export default function MobilePage(): React.JSX.Element { }) const loadNetworkInterfaces = useCallback(async () => { - networkInterfacesPendingRef.current = true + const requestId = ++networkInterfacesRequestIdRef.current + const visit = pairingFlowVisit if (mountedRef.current) { setRefreshingNetworkInterfaces(true) } try { const result = await window.api.mobile.listNetworkInterfaces() - if (mountedRef.current) { + // Why: a superseded lookup must not move the selection a newer one already + // resolved — that address change remints over the offer just advertised. + if (mountedRef.current && requestId === networkInterfacesRequestIdRef.current) { setNetworkInterfaces(result.interfaces) selectAddressAfterRefresh(result.interfaces) } } catch { // Network list is non-critical; the QR will still mint with default routing. } finally { - networkInterfacesPendingRef.current = false if (mountedRef.current) { - setRefreshingNetworkInterfaces(false) + // Why: max, not assignment — an older visit's lookup landing last must not + // un-settle the visit a newer one already answered. + setAddressedFlowVisit((settled) => Math.max(settled, visit)) + if (requestId === networkInterfacesRequestIdRef.current) { + setRefreshingNetworkInterfaces(false) + } } } - }, [mountedRef, selectAddressAfterRefresh]) + }, [mountedRef, pairingFlowVisit, selectAddressAfterRefresh]) useEffect(() => { if (stage !== 'flow') { @@ -268,19 +279,20 @@ export default function MobilePage(): React.JSX.Element { if (!canGenerate) { return } - // Why: entering Step 2 also starts the address lookup, and minting before it - // settles advertises an address the lookup is about to replace — the + // Why: entering Step 2 also starts this visit's address lookup, and minting + // before it settles advertises an address the lookup is about to replace — the // replacement then rotates away the credential this mint just created, so one // Continue runs two overlapping offers through main for one pending token. - if (refreshingNetworkInterfaces || networkInterfacesPendingRef.current) { + if (!pairingAddressSettled) { return } void generatePairing(false) - }, [stage, stepIdx, canGenerate, generatePairing, refreshingNetworkInterfaces]) + }, [stage, stepIdx, canGenerate, generatePairing, pairingAddressSettled]) // Why: entering the flow must mint a fresh pairing token — clear stale QR // state so we never flash an expired code from a previous session. const enterFlow = (): void => { + setPairingFlowVisit((visit) => visit + 1) hasGeneratedRef.current = false setPairQrDataUrl(null) setPairQrSize(null) @@ -293,6 +305,7 @@ export default function MobilePage(): React.JSX.Element { // Why: from the paired summary, "Pair another device" jumps straight to // Step 2 since the app is presumably already installed on the user's phone. const pairAnotherDevice = (): void => { + setPairingFlowVisit((visit) => visit + 1) hasGeneratedRef.current = false setPairQrDataUrl(null) setPairQrSize(null) @@ -325,15 +338,11 @@ export default function MobilePage(): React.JSX.Element { // Why: while the deferred first mint waits on the address, Step 2 would // otherwise read "Generate a pairing code to continue" — a prompt for work it - // is already about to do on the user's behalf. - const pairPreparing = - pairLoading || - (stage === 'flow' && - stepIdx === 1 && - canGenerate && - refreshingNetworkInterfaces && - pairQrDataUrl === null && - relayMintFailure === null) + // is already about to do on the user's behalf. Kept separate from pairLoading: + // that one feeds the invalidation hook's shouldRegenerate, so folding this into + // it would let a mode switch mint before the address settles. + const awaitingPairingAddress = + stage === 'flow' && stepIdx === 1 && canGenerate && !pairingAddressSettled return (