From 3d142f8d2fbdd6d8a0baf0515f829deb1b278b8a Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 16 Sep 2026 23:38:00 -0700 Subject: [PATCH] fix(mobile): stop the LAN recovery button moving the host-policy radio The mint-failure "Use LAN" button called changeConnectionMode('local-only', {persist:false}), which moved the radio without persisting. Two consequences, both the wrong way round for #18211: - The radio read LAN while host policy was still Relay, so the control lied about what every already-paired phone was actually getting. - The equality guard (`nextMode === connectionMode`) then swallowed a later click on LAN, so the user could not persist LAN at all without toggling back through Relay first. Root cause was one piece of state doing two jobs. Split them: connectionMode stays host policy (what the radio shows, what persists), and a new mintModeOverride carries "mint the next QR over LAN" for the recovery path only. An explicit radio pick clears the override. Both symptoms are pinned: reintroducing the radio move fails these tests. --- .../src/components/mobile/MobilePage.test.tsx | 5 +- .../src/components/mobile/MobilePage.tsx | 26 ++++++---- .../components/settings/MobilePane.test.tsx | 11 +++- .../src/components/settings/MobilePane.tsx | 50 ++++++++----------- 4 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/renderer/src/components/mobile/MobilePage.test.tsx b/src/renderer/src/components/mobile/MobilePage.test.tsx index f982f95840c..8628cc99e7c 100644 --- a/src/renderer/src/components/mobile/MobilePage.test.tsx +++ b/src/renderer/src/components/mobile/MobilePage.test.tsx @@ -443,12 +443,15 @@ describe('MobilePage pairing connection mode', () => { connectionMode: 'local-only' }) await user.click(screen.getByRole('button', { name: 'Use LAN' })) - await waitFor(() => expect(screen.getByTestId('mode')).toHaveTextContent('local-only')) + await waitFor(() => expect(screen.getByTestId('pairing-qr')).toHaveTextContent('base64,local')) // Why: the persisted mode is host policy that withdraws Relay from every // paired phone; this button only promises a LAN QR (#18211). expect(mocks.storeState.updateSettings).not.toHaveBeenCalledWith( expect.objectContaining({ mobilePairingConnectionMode: 'local-only' }) ) + // The recovery moves the mint, not policy, so the radio must keep reading + // the host policy that is actually in force. + expect(screen.getByTestId('mode')).toHaveTextContent('automatic') }) it('switches to LAN while a Relay retry is still unresolved', async () => { diff --git a/src/renderer/src/components/mobile/MobilePage.tsx b/src/renderer/src/components/mobile/MobilePage.tsx index 4087e483efe..118cadf58c5 100644 --- a/src/renderer/src/components/mobile/MobilePage.tsx +++ b/src/renderer/src/components/mobile/MobilePage.tsx @@ -42,6 +42,9 @@ export default function MobilePage(): React.JSX.Element { const signedIn = useAppStore((state) => state.orcaProfileAuthStatus?.state === 'connected') const refreshAuthStatus = useAppStore((state) => state.fetchOrcaProfileAuthStatus) const [connectionMode, setConnectionMode] = useMobilePairingConnectionMode() + // Which path the next QR mints over, when that differs from host policy. The + // mint-failure LAN recovery sets it; the radio keeps showing host policy. + const [mintModeOverride, setMintModeOverride] = useState(null) const [networkInterfaces, setNetworkInterfaces] = useState([]) const pairingAddressChangeRef = useRef<(change: MobilePairingAddressChange) => void>(() => {}) const notifyPairingAddressChange = useCallback( @@ -84,7 +87,7 @@ export default function MobilePage(): React.JSX.Element { ) const { generatePairing } = useMobilePairingGeneration({ - connectionMode, + connectionMode: mintModeOverride ?? connectionMode, signedIn, selectedAddress, mountedRef, @@ -129,7 +132,7 @@ export default function MobilePage(): React.JSX.Element { }, [connectionMode, generatePairing, pairLoading, signedIn]) const handleConnectionModeChange = useCallback( - (nextMode: MobilePairingConnectionMode, options?: { persist?: boolean }): void => { + (nextMode: MobilePairingConnectionMode): void => { if (nextMode === connectionMode) { return } @@ -138,16 +141,21 @@ export default function MobilePage(): React.JSX.Element { // (below), which also covers cross-window preference syncs. setRelayMintFailure(null) setConnectionMode(nextMode) - // Why: the persisted setting is host policy — it withdraws Relay from - // every paired phone. A mint-failure recovery button only promises a - // LAN QR, so it must not persist. - if (options?.persist !== false) { - void updateSettings({ mobilePairingConnectionMode: nextMode }) - } + // An explicit policy pick supersedes a recovery mint. + setMintModeOverride(null) + void updateSettings({ mobilePairingConnectionMode: nextMode }) }, [connectionMode, updateSettings, setConnectionMode] ) + // Why not handleConnectionModeChange: the persisted setting is host policy and + // withdraws Relay from every paired phone. Recovery only promises a LAN QR, so + // it moves the mint and leaves policy — and the radio — alone. + const recoverWithLanMint = useCallback((): void => { + setMintModeOverride('local-only') + void generatePairing(false, undefined, 'local-only') + }, [generatePairing]) + const copyRelayDiagnostics = useCallback(async (): Promise => { if (relayMintFailure == null) { return @@ -350,7 +358,7 @@ export default function MobilePage(): React.JSX.Element { relayMintFailure={ connectionMode === 'automatic' && pairQrDataUrl == null ? relayMintFailure : null } - onUseLan={() => handleConnectionModeChange('local-only', { persist: false })} + onUseLan={recoverWithLanMint} onRetryRelay={() => void generatePairing(true)} onCopyRelayDiagnostics={() => void copyRelayDiagnostics()} platform={platform} diff --git a/src/renderer/src/components/settings/MobilePane.test.tsx b/src/renderer/src/components/settings/MobilePane.test.tsx index 0212f3c7e4c..4fa23a6d9f0 100644 --- a/src/renderer/src/components/settings/MobilePane.test.tsx +++ b/src/renderer/src/components/settings/MobilePane.test.tsx @@ -267,7 +267,6 @@ describe('MobilePane pairing connection mode', () => { connectionMode: 'local-only' }) await user.click(screen.getByRole('button', { name: 'Use LAN' })) - await waitFor(() => expect(screen.getByTestId('mode')).toHaveTextContent('local-only')) await waitFor(() => expect(screen.queryByTestId('relay-mint-failure-notice')).not.toBeInTheDocument() ) @@ -276,6 +275,14 @@ describe('MobilePane pairing connection mode', () => { expect(updateSettings).not.toHaveBeenCalledWith( expect.objectContaining({ mobilePairingConnectionMode: 'local-only' }) ) + // The recovery moves the mint, not policy, so the radio must keep reading + // the host policy that is actually in force. + expect(screen.getByTestId('mode')).toHaveTextContent('automatic') + // And because the radio never moved, the equality guard in the change + // handler no longer swallows a later click on LAN. + await user.click(screen.getByRole('button', { name: 'choose-local' })) + expect(updateSettings).toHaveBeenCalledWith({ mobilePairingConnectionMode: 'local-only' }) + expect(screen.getByTestId('mode')).toHaveTextContent('local-only') }) it('does not show mint failure after an honest Relay mint', async () => { @@ -408,7 +415,7 @@ describe('MobilePane pairing connection mode', () => { }) }) await waitFor(() => expect(screen.getByTestId('qr')).toHaveTextContent('base64,local')) - expect(screen.getByTestId('mode')).toHaveTextContent('local-only') + expect(screen.getByTestId('mode')).toHaveTextContent('automatic') }) it('restores a saved local-only preference without user interaction', () => { diff --git a/src/renderer/src/components/settings/MobilePane.tsx b/src/renderer/src/components/settings/MobilePane.tsx index 814f3beeaa1..8fb8d26ce2b 100644 --- a/src/renderer/src/components/settings/MobilePane.tsx +++ b/src/renderer/src/components/settings/MobilePane.tsx @@ -46,6 +46,9 @@ export function MobilePane(): React.JSX.Element { const signedIn = useAppStore((state) => state.orcaProfileAuthStatus?.state === 'connected') const settingsSearchQuery = useAppStore((state) => state.settingsSearchQuery) const [connectionMode, setConnectionMode] = useMobilePairingConnectionMode() + // Which path the next QR mints over, when that differs from host policy. The + // mint-failure LAN recovery sets it; the radio keeps showing host policy. + const [mintModeOverride, setMintModeOverride] = useState(null) const [rotateNextQr, setRotateNextQr] = useState(false) const codeCopiedResetTimerRef = useRef(null) const wasSignedInRef = useRef(signedIn) @@ -175,7 +178,7 @@ export function MobilePane(): React.JSX.Element { connectionModeOverride?: MobilePairingConnectionMode } = {} ) => { - const preferredMode = opts.connectionModeOverride ?? connectionMode + const preferredMode = opts.connectionModeOverride ?? mintModeOverride ?? connectionMode // Why: refuse signed-out Anywhere rather than degrading to a local-only QR // under the Relay label (canMint is the shared honesty gate). if (!canMintMobilePairingOffer({ connectionMode: preferredMode, signedIn })) { @@ -255,6 +258,7 @@ export function MobilePane(): React.JSX.Element { clearCodeCopiedResetTimer, connectionMode, loadDevices, + mintModeOverride, mountedRef, rotateNextQr, selectedAddress, @@ -263,7 +267,7 @@ export function MobilePane(): React.JSX.Element { ) const changeConnectionMode = useCallback( - (nextMode: MobilePairingConnectionMode, options?: { persist?: boolean }) => { + (nextMode: MobilePairingConnectionMode) => { if (nextMode === connectionMode) { return } @@ -271,37 +275,27 @@ export function MobilePane(): React.JSX.Element { // instead of snapping back to the default. handledModeRef.current = nextMode setConnectionMode(nextMode) - // Why: the persisted setting is host policy — it withdraws Relay from - // every paired phone. A mint-failure recovery button only promises a - // LAN QR, so it must not persist. - if (options?.persist !== false) { - void updateSettings({ mobilePairingConnectionMode: nextMode }) - } - // Why: after a Relay mint failure, LAN should mint immediately — including - // when the renderer has not chosen an address yet (main picks the default). - const shouldRecoverWithLan = relayMintFailure != null && nextMode === 'local-only' + // An explicit policy pick supersedes a recovery mint. + setMintModeOverride(null) + void updateSettings({ mobilePairingConnectionMode: nextMode }) // A displayed or in-flight code encodes the old connection policy. The // main process rotates on the mode mismatch, so don't arm a second rotate. invalidatePairing({ armRotate: false }) - // Why: switching to LAN after a Relay failure should mint immediately. - if ( - shouldRecoverWithLan && - canMintMobilePairingOffer({ connectionMode: nextMode, signedIn }) - ) { - void generateQR({ rotate: false, connectionModeOverride: 'local-only' }) - } }, - [ - connectionMode, - generateQR, - invalidatePairing, - relayMintFailure, - signedIn, - updateSettings, - setConnectionMode - ] + [connectionMode, invalidatePairing, updateSettings, setConnectionMode] ) + // Why not changeConnectionMode: the persisted setting is host policy and + // withdraws Relay from every paired phone. Recovery only promises a LAN QR, + // so it moves the mint and leaves policy — and the radio — alone. + const recoverWithLanMint = useCallback(() => { + setMintModeOverride('local-only') + invalidatePairing({ armRotate: false }) + if (canMintMobilePairingOffer({ connectionMode: 'local-only', signedIn })) { + void generateQR({ rotate: false, connectionModeOverride: 'local-only' }) + } + }, [generateQR, invalidatePairing, signedIn]) + const copyRelayDiagnostics = useCallback(async (): Promise => { if (relayMintFailure == null) { return @@ -401,7 +395,7 @@ export function MobilePane(): React.JSX.Element { {relayMintFailure != null && connectionMode === 'automatic' ? ( changeConnectionMode('local-only', { persist: false })} + onUseLan={recoverWithLanMint} onRetry={() => void generateQR({ rotate: true })} onCopyDiagnostics={() => void copyRelayDiagnostics()} busy={loading}