From 372ef4adef46abdcace5d956b24b792359d5e1ff Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 6 May 2026 21:08:18 -0400 Subject: [PATCH] fix(mobile): unreachable UX cleanup + ws diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop the unreachable banner (Re-pair / Retry / Remove) from the worktree-list view; the same actions are reachable from the host action sheet, and the home screen surfaces the verdict text per host. - Header now shows just the status dot + Reconnect button (when applicable). The dot color reflects the verdict severity (red for unreachable / auth-failed, amber for warning / reconnecting) so it matches the home-screen label color. - Resume + Account-usage cards only render for currently-connected hosts. Showing stale cached data for a disconnected host implied live data; now both sections drop out and reappear when the host reconnects. - Drop the Resume skeleton — Account usage handles absence by simply not rendering, Resume now does the same. Less asymmetric UX. - rpc-client: inline rich diagnostic logs in ws.onclose / ws.onerror (constructToCloseMs, aliveMs, inboundIdleMs, eventKeys, eventStr) + connection-construction context (wsCount, msSinceLastConnected, msSinceLastClose, msSinceLastInbound) so we can distinguish the intermittent 'force-quit-fixes-it' failure modes (RN/OkHttp process-state poisoning vs router NAT wedge vs transient blip). Helpers are inlined per-handler, immune to hot-reload closure races. Co-authored-by: Orca --- mobile/app/h/[hostId]/index.tsx | 116 +++++++++------------------- mobile/app/index.tsx | 64 +++------------ mobile/src/components/StatusDot.tsx | 22 +++++- mobile/src/transport/rpc-client.ts | 106 ++++++++++++++++++++++++- 4 files changed, 171 insertions(+), 137 deletions(-) diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index b6670898b0e..ff45db42fa9 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -38,7 +38,6 @@ import { } from '../../../src/transport/client-context' import { classifyConnection, - unreachableHint, type ConnectionVerdict } from '../../../src/transport/connection-health' import type { RpcSuccess } from '../../../src/transport/types' @@ -691,42 +690,45 @@ export default function HostScreen() { router.back()}> - - - - {hostName || 'Host'} - - - {connState !== 'connected' && - (() => { - const verdict = classifyConnection({ - state: connState, - reconnectAttempts, - lastConnectedAt - }) - const isError = isErrorVerdict(verdict) - const showReconnectButton = - isError && - hostId && - verdict.kind !== 'auth-failed' && - verdict.kind !== 'unreachable' - return ( - - - {verdict.label} + {(() => { + const headerVerdict = classifyConnection({ + state: connState, + reconnectAttempts, + lastConnectedAt + }) + return ( + <> + + + + {hostName || 'Host'} - {showReconnectButton && ( - void forceReconnectHost(hostId!)} - hitSlop={8} - > - Reconnect - - )} - ) - })()} + {connState !== 'connected' && + (() => { + // Why: status label removed in favor of just the dot + + // Reconnect button — the home screen already surfaces the + // verdict text per host, and the dot color already + // signals severity here. Auth-failed routes through its + // dedicated banner so we still want to suppress the + // Reconnect button for that case. + const verdict = headerVerdict + const isError = isErrorVerdict(verdict) + const showReconnectButton = isError && hostId && verdict.kind !== 'auth-failed' + if (!showReconnectButton) return null + return ( + void forceReconnectHost(hostId!)} + hitSlop={8} + > + Reconnect + + ) + })()} + + ) + })()} {/* Filter/sort/group toolbar */} @@ -811,41 +813,6 @@ export default function HostScreen() { )} - {/* Unreachable banner: surfaces the "host moved / network changed" - case after enough hard-fail reconnects. We keep retrying in the - background, but the user gets an actionable affordance instead - of an indefinite "Reconnecting…". */} - {connState !== 'auth-failed' && - (() => { - const verdict = classifyConnection({ - state: connState, - reconnectAttempts, - lastConnectedAt - }) - if (verdict.kind !== 'unreachable') return null - return ( - - {unreachableHint(verdict.reason)} - - router.push('/pair-scan')}> - Re-pair - - {hostId && ( - void forceReconnectHost(hostId)} - > - Retry - - )} - setConfirmRemoveHost(true)}> - Remove - - - - ) - })()} - {/* Search bar */} {showSearch && ( @@ -1212,15 +1179,6 @@ const styles = StyleSheet.create({ fontWeight: '600', color: colors.textPrimary }, - statusText: { - color: colors.textSecondary, - fontSize: typography.metaSize - }, - statusRow: { - flexDirection: 'row', - alignItems: 'center', - gap: spacing.sm - }, reconnectButton: { paddingVertical: 4, paddingHorizontal: spacing.sm, diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index cd025325a79..d25d4b04bcb 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -446,12 +446,16 @@ export default function HomeScreen() { // tap target is still the same and a fresher snapshot from the live RPC // overwrites the card's contents in place when it lands. const resumeWorktree = useMemo(() => { - if (lastVisited && sortedHosts.some((h) => h.id === lastVisited.hostId)) { + // Why: only surface Resume for hosts that are currently connected. + // Showing a stale cached worktree for a disconnected host is + // misleading — the user would tap into a session route that can't + // load anything until the host reconnects. Once the host reconnects, + // the card reappears with fresh data. + if (lastVisited && hostStates[lastVisited.hostId] === 'connected') { const cached = getCachedWorktrees(lastVisited.hostId) as WorktreeSummary[] | null const match = cached?.find((w) => w.worktreeId === lastVisited.worktreeId) if (match) return { hostId: lastVisited.hostId, worktree: match } } - // Prefer a currently-connected host's data when we have it. for (const host of sortedHosts) { if (hostStates[host.id] !== 'connected') continue const info = worktreeInfo[host.id] @@ -459,39 +463,16 @@ export default function HomeScreen() { return { hostId: host.id, worktree: info.lastActiveWorktree } } } - // Fall back to whichever known host has cached data, regardless of - // current connection state. - for (const host of sortedHosts) { - const info = worktreeInfo[host.id] - if (info?.lastActiveWorktree) { - return { hostId: host.id, worktree: info.lastActiveWorktree } - } - } return null }, [sortedHosts, hostStates, worktreeInfo, lastVisited]) - const resumeLoading = useMemo( - () => - sortedHosts.some((host) => { - const state = hostStates[host.id] ?? 'connecting' - return ( - state === 'connecting' || - state === 'handshaking' || - state === 'reconnecting' || - (state === 'connected' && !worktreeInfo[host.id]) - ) - }), - [sortedHosts, hostStates, worktreeInfo] - ) - - // Why: only show the Account usage section for hosts that have at least - // one Claude or Codex account configured. Render whenever cached data - // exists, regardless of current connection state, so the cards don't - // disappear for ~1s on resume while the WebSocket reconnects. Streamed - // updates from the live RPC overwrite the snapshot in place when ready. + // Why: only show the Account usage section for hosts that are currently + // connected. Showing stale cached usage for a disconnected host implies + // live data; better to hide until the host reconnects and we can refresh. const accountsHosts = useMemo(() => { const items: Array<{ host: HostProfile; snapshot: AccountsSnapshot }> = [] for (const host of sortedHosts) { + if (hostStates[host.id] !== 'connected') continue const snap = accountsByHost[host.id] if (!snap) continue const hasClaude = snap.claude.accounts.length > 0 @@ -660,7 +641,7 @@ export default function HomeScreen() { {item.name} - + {verdict.label} {connected && info @@ -711,17 +692,6 @@ export default function HomeScreen() { - ) : hosts.length > 0 && resumeLoading ? ( - <> - Resume - - - - - - - - ) : null} {/* ─── Account usage ─── */} @@ -1198,18 +1168,6 @@ const styles = StyleSheet.create({ marginTop: 4 }, - /* ─── Skeleton ─── */ - skeletonBlock: { - backgroundColor: colors.bgRaised, - opacity: 0.5 - }, - skeletonLine: { - height: 12, - borderRadius: 4, - backgroundColor: colors.bgRaised, - opacity: 0.5 - }, - /* ─── Quick actions ─── */ quickActions: { flexDirection: 'row', diff --git a/mobile/src/components/StatusDot.tsx b/mobile/src/components/StatusDot.tsx index 86f36f45033..a29b5e95eba 100644 --- a/mobile/src/components/StatusDot.tsx +++ b/mobile/src/components/StatusDot.tsx @@ -1,6 +1,7 @@ import { View, StyleSheet } from 'react-native' import { colors } from '../theme/mobile-theme' import type { ConnectionState } from '../transport/types' +import type { ConnectionVerdict } from '../transport/connection-health' const stateColors: Record = { connected: colors.statusGreen, @@ -11,8 +12,25 @@ const stateColors: Record = { 'auth-failed': colors.statusRed } -export function StatusDot({ state }: { state: ConnectionState }) { - return +// Why: when caller passes a verdict, the dot color reflects the verdict's +// severity instead of the raw transport state. This avoids the "amber dot +// next to red 'Can't reach desktop' label" mismatch — the underlying +// transport is still 'reconnecting' (amber) but the user-visible meaning +// has escalated to error (red). +export function StatusDot({ + state, + verdict +}: { + state: ConnectionState + verdict?: ConnectionVerdict +}) { + const color = + verdict?.kind === 'unreachable' || verdict?.kind === 'auth-failed' + ? colors.statusRed + : verdict?.kind === 'warning' + ? colors.statusAmber + : (stateColors[state] ?? colors.textMuted) + return } const styles = StyleSheet.create({ diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 5fb7bf674de..0684ebd1918 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -88,6 +88,18 @@ export function connect( let activityProbeTimer: ReturnType | null = null let intentionallyClosed = false 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 + // need to see whether (a) the new attempts even differ from the old ones, + // (b) anything is happening at the OS / RN-bridge layer between attempts, + // and (c) what the timing pattern is (instant 1006 = port closed / route + // dead, slow 1006 = packet drop / timeout). These three timestamps + the + // ws-construction counter are the cheapest visibility into RN/OkHttp + // process-state poisoning hypotheses. + let lastInboundAt: number | null = null + let lastWsClosedAt: number | null = null + let wsConstructionCounter = 0 + let currentWsOpenedAt: number | null = null // Why: fresh ephemeral keypair per connection provides forward secrecy. // The shared key is derived from our ephemeral secret + server's static public key. @@ -160,13 +172,25 @@ export function connect( function openConnection() { if (intentionallyClosed) return + const now = Date.now() + wsConstructionCounter++ console.log('[net] openConnection', { attempt: reconnectAttempt, - endpoint: redactedEndpoint(endpoint) + endpoint: redactedEndpoint(endpoint), + // Why: process-poisoning diagnostic. If wsCount is high (e.g. >50) + // and every recent open fails with 1006, suspect RN/OkHttp internal + // pool corruption that only force-quit clears. Compare msSinceLast* + // values to the failure cadence: instant repeated fails with no + // inbound traffic between them = process-state stuck. + wsCount: wsConstructionCounter, + msSinceLastConnected: lastConnectedAt != null ? now - lastConnectedAt : null, + msSinceLastClose: lastWsClosedAt != null ? now - lastWsClosedAt : null, + msSinceLastInbound: lastInboundAt != null ? now - lastInboundAt : null }) setState('connecting') sharedKey = null + currentWsOpenedAt = now ws = new WebSocket(endpoint) const openingWs = ws @@ -215,6 +239,9 @@ export function connect( } ws.onmessage = (event) => { + // Why: track last-inbound for the openConnection diagnostic. Server + // pongs and stream events both bump this — anything from the wire. + lastInboundAt = Date.now() const raw = typeof event.data === 'string' ? event.data : String(event.data) // Why: during handshaking, e2ee_ready is plaintext because it precedes @@ -335,6 +362,45 @@ export function connect( ws.onclose = (event) => { const e = event as { code?: number; reason?: string; wasClean?: boolean } | undefined + const closeAt = Date.now() + // Why: time-since-construct distinguishes failure modes. Instant + // close (<300ms) = TCP RST / port closed / route unreachable / RN + // synchronous reject. Mid (300ms–3s) = DNS/connect attempt + reset. + // Slow (>3s) = TCP SYN timeout / packet loss / NAT wedge. If an + // entire reconnect burst is all instant, the problem is local + // process state or routing, not packet loss. + const constructToCloseMs = currentWsOpenedAt != null ? closeAt - currentWsOpenedAt : null + const aliveMs = + currentWsOpenedAt != null && state === 'connected' ? closeAt - currentWsOpenedAt : null + const inboundIdleMs = lastInboundAt != null ? closeAt - lastInboundAt : null + // Why: inline the diagnostic dump. Earlier hot-reload tripped + // `Property 'enumKeys' doesn't exist` because a stale closure + // captured a half-loaded module. Inlining keeps the handler's + // behavior fully decided at construction time. + let closeEventKeys: string[] = [] + let closeEventStr = '' + try { + closeEventKeys = event && typeof event === 'object' ? Object.keys(event as object) : [] + } catch { + closeEventKeys = [] + } + try { + const seen = new WeakSet() + closeEventStr = JSON.stringify( + event, + (_k, v) => { + if (typeof v === 'object' && v !== null) { + if (seen.has(v as object)) return '[circular]' + seen.add(v as object) + } + if (typeof v === 'function') return '[fn]' + return v + }, + 0 + ).slice(0, 500) + } catch { + closeEventStr = '[unstringifiable]' + } console.log('[net] ws.onclose', { code: e?.code, reason: e?.reason, @@ -342,8 +408,15 @@ export function connect( state, attempt: reconnectAttempt, intentionallyClosed, - endpoint: redactedEndpoint(endpoint) + endpoint: redactedEndpoint(endpoint), + constructToCloseMs, + aliveMs, + inboundIdleMs, + eventKeys: closeEventKeys, + eventStr: closeEventStr }) + lastWsClosedAt = closeAt + currentWsOpenedAt = null handleSocketClosed(openingWs) } @@ -352,10 +425,37 @@ export function connect( // onclose fires right after, but logging the error message gives us // the original cause that the close code alone can hide. const e = event as { message?: string } | undefined + // Why: inlined defensively — see ws.onclose comment. + let errEventKeys: string[] = [] + let errEventStr = '' + try { + errEventKeys = event && typeof event === 'object' ? Object.keys(event as object) : [] + } catch { + errEventKeys = [] + } + try { + const seen = new WeakSet() + errEventStr = JSON.stringify( + event, + (_k, v) => { + if (typeof v === 'object' && v !== null) { + if (seen.has(v as object)) return '[circular]' + seen.add(v as object) + } + if (typeof v === 'function') return '[fn]' + return v + }, + 0 + ).slice(0, 500) + } catch { + errEventStr = '[unstringifiable]' + } console.log('[net] ws.onerror', { message: e?.message, state, - attempt: reconnectAttempt + attempt: reconnectAttempt, + eventKeys: errEventKeys, + eventStr: errEventStr }) } }