From 618b5d3177047d41c96f81487e2c66a1925fc835 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sat, 13 Jun 2026 14:25:43 -0700 Subject: [PATCH] fix(mobile): retry handshake before latching auth-failed (#5200) (#5304) --- mobile/.oxlintrc.json | 2 +- mobile/app/h/[hostId]/index.tsx | 44 ++-------- mobile/src/components/AuthFailedBanner.tsx | 66 +++++++++++++++ mobile/src/transport/rpc-client.test.ts | 82 +++++++++++++++++++ mobile/src/transport/rpc-client.ts | 93 +++++++++++++++++----- 5 files changed, 228 insertions(+), 59 deletions(-) create mode 100644 mobile/src/components/AuthFailedBanner.tsx diff --git a/mobile/.oxlintrc.json b/mobile/.oxlintrc.json index 325faf024..58a74d102 100644 --- a/mobile/.oxlintrc.json +++ b/mobile/.oxlintrc.json @@ -64,7 +64,7 @@ { "files": ["src/transport/rpc-client.ts"], "rules": { - "max-lines": ["error", { "max": 1070, "skipBlankLines": true, "skipComments": true }] + "max-lines": ["error", { "max": 1074, "skipBlankLines": true, "skipComments": true }] } }, { diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index d64f94269..e271c64a2 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -52,6 +52,7 @@ import { ActionSheetContent } from '../../../src/components/ActionSheetModal' import { ConfirmModal } from '../../../src/components/ConfirmModal' import { BottomDrawer } from '../../../src/components/BottomDrawer' import { ProtocolBlockScreen } from '../../../src/components/ProtocolBlockScreen' +import { AuthFailedBanner } from '../../../src/components/AuthFailedBanner' import { getCachedWorktrees } from '../../../src/cache/worktree-cache' import { colors, radii, spacing, typography } from '../../../src/theme/mobile-theme' import { useResponsiveLayout } from '../../../src/layout/responsive-layout' @@ -998,19 +999,12 @@ export default function HostScreen() { {/* Auth failed banner */} {connState === 'auth-failed' && ( - - - Pairing rejected — re-pair from desktop or remove this host. - - - router.push('/pair-scan')}> - Re-pair - - setConfirmRemoveHost(true)}> - Remove - - - + hostId && void forceReconnectHost(hostId)} + onRepair={() => router.push('/pair-scan')} + onRemove={() => setConfirmRemoveHost(true)} + /> )} {/* Search bar */} @@ -1424,30 +1418,6 @@ const styles = StyleSheet.create({ fontSize: typography.metaSize, fontWeight: '600' }, - authBanner: { - backgroundColor: colors.bgPanel, - paddingVertical: spacing.sm, - paddingHorizontal: spacing.lg, - borderBottomWidth: 1, - borderBottomColor: colors.borderSubtle - }, - authBannerText: { - color: colors.statusRed, - fontSize: 13, - marginBottom: spacing.sm - }, - authActions: { - flexDirection: 'row', - gap: spacing.lg - }, - authAction: { - paddingVertical: spacing.xs - }, - authActionText: { - color: colors.accentBlue, - fontSize: 13, - fontWeight: '600' - }, toolbar: { flexDirection: 'row', alignItems: 'center', diff --git a/mobile/src/components/AuthFailedBanner.tsx b/mobile/src/components/AuthFailedBanner.tsx new file mode 100644 index 000000000..29be8b851 --- /dev/null +++ b/mobile/src/components/AuthFailedBanner.tsx @@ -0,0 +1,66 @@ +import { View, Text, Pressable, StyleSheet } from 'react-native' +import { colors, spacing } from '../theme/mobile-theme' + +// Why: auth-failed is no longer necessarily terminal (issue #5200) — a +// transient rejection can latch it even though the desktop still lists this +// device. Offer Retry (fresh client + handshake) ahead of the disruptive +// re-pair flow so the common transient case recovers without re-pairing. +export function AuthFailedBanner({ + canRetry, + onRetry, + onRepair, + onRemove +}: { + canRetry: boolean + onRetry: () => void + onRepair: () => void + onRemove: () => void +}) { + return ( + + + Authentication failed — try reconnecting first; if it keeps failing, re-pair from desktop. + + + {canRetry && ( + + Retry + + )} + + Re-pair + + + Remove + + + + ) +} + +const styles = StyleSheet.create({ + banner: { + backgroundColor: colors.bgPanel, + paddingVertical: spacing.sm, + paddingHorizontal: spacing.lg, + borderBottomWidth: 1, + borderBottomColor: colors.borderSubtle + }, + text: { + color: colors.statusRed, + fontSize: 13, + marginBottom: spacing.sm + }, + actions: { + flexDirection: 'row', + gap: spacing.lg + }, + action: { + paddingVertical: spacing.xs + }, + actionText: { + color: colors.accentBlue, + fontSize: 13, + fontWeight: '600' + } +}) diff --git a/mobile/src/transport/rpc-client.test.ts b/mobile/src/transport/rpc-client.test.ts index e1053a7e8..a45a018a6 100644 --- a/mobile/src/transport/rpc-client.test.ts +++ b/mobile/src/transport/rpc-client.test.ts @@ -712,6 +712,88 @@ describe('mobile rpc-client connection timeout', () => { }) }) + // Issue #5200: a single auth rejection used to latch 'auth-failed' + // permanently, forcing a needless re-pair even when the desktop still + // listed the device with a valid token. The client now retries the + // handshake a bounded number of times before declaring auth dead. + describe('auth rejection retry (issue #5200)', () => { + function authenticate(socket: MockWebSocket) { + socket.open() + socket.receive(JSON.stringify({ type: 'e2ee_ready' })) + socket.receive('encrypted:{"type":"e2ee_authenticated"}') + } + + it('retries the handshake on a transient e2ee_error instead of latching auth-failed', async () => { + const client = connect('ws://desktop.invalid', 'token', 'server-key') + const first = mockSockets[0]! + first.open() + first.receive(JSON.stringify({ type: 'e2ee_ready' })) + + // Transient rejection during handshake — must NOT latch auth-failed. + first.receive('encrypted:{"type":"e2ee_error","error":{"code":"unauthorized"}}') + expect(client.getState()).toBe('reconnecting') + + // A fresh socket gets a fresh handshake; this time it authenticates. + await vi.advanceTimersByTimeAsync(500) + authenticate(mockSockets[mockSockets.length - 1]!) + expect(client.getState()).toBe('connected') + + client.close() + }) + + it('latches auth-failed once the retry budget is exhausted', async () => { + const client = connect('ws://desktop.invalid', 'token', 'server-key') + + // Three consecutive handshake rejections (AUTH_RETRY_BUDGET = 3). + for (let i = 0; i < 3; i++) { + if (i > 0) { + await vi.advanceTimersByTimeAsync(500) + } + const socket = mockSockets[mockSockets.length - 1]! + socket.open() + socket.receive(JSON.stringify({ type: 'e2ee_ready' })) + socket.receive('encrypted:{"type":"e2ee_error","error":{"code":"unauthorized"}}') + } + + expect(client.getState()).toBe('auth-failed') + + client.close() + }) + + it('resets the budget after a successful connect between rejections', async () => { + const client = connect('ws://desktop.invalid', 'token', 'server-key') + + // Two rejections, then a clean connect resets the budget... + for (let i = 0; i < 2; i++) { + if (i > 0) { + await vi.advanceTimersByTimeAsync(500) + } + const socket = mockSockets[mockSockets.length - 1]! + socket.open() + socket.receive(JSON.stringify({ type: 'e2ee_ready' })) + socket.receive('encrypted:{"type":"e2ee_error","error":{"code":"unauthorized"}}') + } + await vi.advanceTimersByTimeAsync(500) + authenticate(mockSockets[mockSockets.length - 1]!) + expect(client.getState()).toBe('connected') + + // ...so a later mid-session rejection gets the full budget again + // rather than immediately latching auth-failed. + const live = mockSockets[mockSockets.length - 1]! + const request = client.sendRequest('status.get').catch(() => undefined) + // sendRequest awaits waitForConnected before sending — let it flush. + await Promise.resolve() + const id = sentRequest(live, 'status.get').id + live.receive( + `encrypted:${JSON.stringify({ id, ok: false, error: { code: 'unauthorized' } })}` + ) + await request + expect(client.getState()).toBe('reconnecting') + + client.close() + }) + }) + it('rejects requests waiting for reconnect after the retry cap', async () => { const client = connect('ws://desktop.invalid', 'token', 'server-key') const socket = mockSockets[0]! diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 04ea0daac..0179f8931 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -119,6 +119,15 @@ const RECONNECT_DELAYS = [500, 1000, 2000, 4000, 8000, 15_000, 30_000, 60_000] // drift the user sees "Reconnecting…" while the loop is silently // parked. const GIVE_UP_AFTER_ATTEMPTS = 12 +// Why: a single `unauthorized`/`e2ee_error` is not proof the pairing is dead. +// Issue #5200: a tablet showed "Auth failed" and forced a needless re-pair +// while the desktop still listed it as paired with a valid token — a transient +// rejection (mid-session resume race, a stale frame after background) latched +// the terminal auth-failed state permanently. Retry the full handshake this +// many times with a clean reconnect before declaring auth dead. A genuinely +// revoked token is rejected on every attempt and converges to auth-failed in +// seconds; a one-off glitch self-heals without the user re-pairing. +const AUTH_RETRY_BUDGET = 3 const REQUEST_TIMEOUT_MS = 30_000 const CONNECT_TIMEOUT_MS = 12_000 const HANDSHAKE_TIMEOUT_MS = 5_000 @@ -182,6 +191,11 @@ export function connect( let handshakeTimer: ReturnType | null = null let activityProbeTimer: ReturnType | null = null let intentionallyClosed = false + // Why: consecutive auth rejections since the last successful connect. We + // tolerate up to AUTH_RETRY_BUDGET (issue #5200) before latching auth-failed + // so a transient rejection doesn't force a needless re-pair. Reset to 0 on + // every 'connected'. + let authRejectionCount = 0 let lastConnectedAt: number | null = null // Why: diagnostic — when the rpc-client gets stuck in a state where every // openConnection fails with code 1006 and only a force-quit recovers, we @@ -247,6 +261,9 @@ export function connect( }) if (next === 'connected') { lastConnectedAt = Date.now() + // Why: a clean handshake proves the token is valid — clear the auth + // retry budget so a future isolated rejection gets the full budget again. + authRejectionCount = 0 for (const waiter of connectWaiters.splice(0)) { if (waiter.timeout) { clearTimeout(waiter.timeout) @@ -496,18 +513,11 @@ export function connect( } } else if (msg.type === 'e2ee_error' || (!msg.ok && msg.error?.code === 'unauthorized')) { console.log('[net] e2ee auth FAILED', { msgType: msg.type, error: msg.error }) - emitLog( - 'error', - 'Authentication rejected', - typeof msg.error?.message === 'string' ? msg.error.message : 'Unauthorized' - ) - intentionallyClosed = true - ws?.close() - ws = null - activeBrowserScreencastRequestId = null - pendingBrowserScreencastRequestId = null - setState('auth-failed') - rejectAllPending('Unauthorized — pairing may be revoked') + if (handshakeTimer) { + clearTimeout(handshakeTimer) + handshakeTimer = null + } + handleAuthRejection('Unauthorized — pairing may be revoked') } } catch { // Not JSON — ignore during handshake. @@ -549,16 +559,12 @@ export function connect( return } - // Why: auth failure is distinct from transient disconnect — retrying - // with a rejected token causes infinite reconnect churn. + // Why: a mid-session unauthorized may be a transient glitch, not a dead + // pairing (issue #5200). handleAuthRejection retries the handshake a few + // times before latching auth-failed, while still bounding churn via the + // budget so a genuinely revoked token doesn't reconnect forever. if (!response.ok && response.error.code === 'unauthorized') { - intentionallyClosed = true - ws?.close() - ws = null - activeBrowserScreencastRequestId = null - pendingBrowserScreencastRequestId = null - setState('auth-failed') - rejectAllPending('Unauthorized — pairing may be revoked') + handleAuthRejection('Unauthorized — pairing may be revoked') return } @@ -736,6 +742,51 @@ export function connect( scheduleReconnect() } + // Why: a token rejection (handshake e2ee_error/unauthorized or a mid-session + // unauthorized RPC) may be transient — issue #5200. Retry the full handshake + // up to AUTH_RETRY_BUDGET times before declaring auth dead, so a one-off + // glitch self-heals instead of forcing the user to re-pair. A genuinely + // revoked token fails every retry and latches auth-failed within seconds. + function handleAuthRejection(reason: string): void { + activeBrowserScreencastRequestId = null + pendingBrowserScreencastRequestId = null + authRejectionCount++ + if (authRejectionCount < AUTH_RETRY_BUDGET) { + console.log('[net] auth rejected — retrying handshake', { + attempt: authRejectionCount, + budget: AUTH_RETRY_BUDGET, + endpoint: redactedEndpoint(endpoint) + }) + emitLog( + 'warn', + 'Authentication rejected', + `Retrying (${authRejectionCount}/${AUTH_RETRY_BUDGET})` + ) + // Why: close the current socket but DON'T set intentionallyClosed — + // we want handleSocketClosed to route into the reconnect path so the + // token gets a fresh handshake. rejectAllPending unblocks in-flight RPCs. + const closing = ws + ws = null + sharedKey = null + rejectAllPending(reason) + if (closing) { + closing.close() + } + setState('reconnecting') + scheduleReconnect() + return + } + console.log('[net] auth rejected — budget exhausted, latching auth-failed', { + attempt: authRejectionCount, + endpoint: redactedEndpoint(endpoint) + }) + intentionallyClosed = true + ws?.close() + ws = null + setState('auth-failed') + rejectAllPending(reason) + } + function scheduleReconnect() { // Why: spinning reconnect forever drains battery and floods logs // when the host is genuinely unreachable (wrong IP, port closed,