diff --git a/docs/mobile-shared-client-per-host.md b/docs/mobile-shared-client-per-host.md new file mode 100644 index 000000000..e9a8cee4d --- /dev/null +++ b/docs/mobile-shared-client-per-host.md @@ -0,0 +1,406 @@ +# Single Shared RPC Client per Host (Mobile) + +Design doc for collapsing the per-screen WebSocket connection model into a +single shared `RpcClient` per paired host, owned by a React context that +sits above the route tree. + +## Problem + +The mobile app today opens **one WebSocket per screen per host**: + +| Screen | Connections per host | +|---|---| +| Home (`app/index.tsx`) | 1 (with persistent `accounts.subscribe` stream) | +| Host detail (`app/h/[hostId]/index.tsx`) | +1 | +| Worktree session (`app/h/[hostId]/session/[worktreeId].tsx`) | +1 | +| Accounts (`app/h/[hostId]/accounts.tsx`) | +1 | +| Pair confirm (briefly) | +1 | + +A user actively browsing one host typically holds **3–4 simultaneous +sockets** to the desktop runtime. Each call to `connect()` runs its own +E2EE handshake, allocates an ephemeral keypair, and runs an independent +reconnect loop with exponential backoff. + +This causes three observable problems: + +1. **Stuck-connecting / reconnecting for minutes.** The desktop's + `MAX_WS_CONNECTIONS = 32` is shared across all clients. A user who + navigates rapidly accumulates stale sockets faster than they can be + reaped by TCP keepalive (which can take 60–300s on default systems + for half-open connections from a phone leaving Wi-Fi range or + backgrounding). Once the cap is hit, new sockets are rejected with WS + close code `1013 Maximum connections reached`. The mobile reconnect + loop does not recognize 1013 as terminal — it retries with backoff, + each retry also dropped, until enough stale sockets are reaped. + Result: a screen that should connect in <1s is stuck for 1–5 minutes. +2. **Tab create/delete hangs.** `client.sendRequest('terminal.create')` + awaits `waitForConnected()`. If the session-screen client is in + `connecting`/`reconnecting` state because its socket lost the cap + race, the await blocks until the 30s `REQUEST_TIMEOUT_MS` fires. + The user sees nothing happen. +3. **Triple cost on every cold-start.** Three E2EE handshakes, + three Curve25519 keypair generations, three subscription + re-registrations on every app launch. On low-end Android, this + visibly delays first paint by hundreds of milliseconds. + +The architecture also wastes server resources: each socket carries its +own E2EE channel, its own subscription set, its own driver-state-machine +client identity (cf. `docs/mobile-presence-lock.md`). The server already +has logic to reconcile multi-socket-per-token tear-down +(`hasOtherConnections` in `ws-transport.ts`) — that logic exists *because* +this design forced the question; with a single client per host, it becomes +unnecessary. + +## Today's transport ownership + +Five files independently call `connect(endpoint, deviceToken, publicKeyB64)`: + +``` +mobile/app/pair-confirm.tsx # one-shot during pairing +mobile/app/pair-scan.tsx # one-shot during pairing +mobile/app/index.tsx # N (one per paired host) +mobile/app/h/[hostId]/index.tsx # 1 (host detail) +mobile/app/h/[hostId]/session/[worktreeId].tsx # 1 (session) +mobile/app/h/[hostId]/accounts.tsx # 1 (accounts) +``` + +Each owns a `useRef` and calls `client.close()` from a +cleanup function. The home screen additionally maintains a +`clientsRef: Array<{ hostId, client }>` so its own usage of the per-host +client survives across navigation events. + +This pattern works for *correctness* — every cleanup eventually closes +its socket — but it breaks under three real-world conditions: + +1. **Rapid navigation.** Mounts spawn before unmounts complete; cleanup + `client.close()` runs after a new screen has already opened a fresh + socket to the same host. Two sockets briefly coexist for the same + token, multiplied across screens. +2. **Network drops.** A backgrounded/locked phone on a flaky network + leaves sockets half-open. The server doesn't get a FIN; cleanup + relies on TCP keepalive timing. Meanwhile the foreground app, on + resume, opens fresh sockets. The half-open ones eat the cap until + reaped. +3. **Hot reload during dev.** Each Metro hot reload fires a new render + tree without unmounting the old, so connections leak. + +## Goal + +> A paired host has at most **one active WebSocket** at any time, owned +> by a context provider above the route tree. All screens for that host +> share that client. The pair flows are the only places that create +> short-lived clients (and they explicitly close those after pairing +> completes). + +This is the architectural fix to the symptoms above. Combined with the +two recently-shipped hotfixes (token cache + stable `useEffect` +dependency on home screen), this completes the connection-lifecycle +work. + +## Design + +### Layered ownership + +``` +RootLayout () +└── routes + └── // mounts when route has hostId + ├── h/[hostId]/ // host detail + ├── h/[hostId]/session/[worktreeId]/ // session + └── h/[hostId]/accounts/ // accounts +``` + +Two providers, layered: + +1. **`RpcClientProvider` (root)** — owns one `RpcClient` per host, + keyed by `hostId`. Lifecycle: opens on first request for that + host's client, holds open until app shutdown OR until the host is + removed (`removeHost(hostId)` triggers explicit close). Reuses + existing `loadHosts()` cache from the recently-merged + `host-store.ts` work. +2. **`HostScopedClientGate` (per host)** — a thin route-layout + component placed at `app/h/_layout.tsx`. Reads `hostId` from + route params, requests the client for that host from the root + provider, exposes it via context to descendants, and renders a + loading state until the client reaches `connected`. Guarantees + every descendant screen sees the same client instance for that + host — no per-screen `connect()` calls. + +The home screen (`app/index.tsx`) lives outside `HostScopedClientGate` +since it spans all hosts; it consumes the root provider directly via +a multi-host hook (see API below). + +### API + +```ts +// New file: mobile/src/transport/client-context.tsx +type RpcClientContext = { + // Get-or-open. Returns the singleton client for hostId; opens it + // lazily on first call, reuses it for all subsequent callers. Never + // returns null (returns a placeholder client in 'connecting' state + // if open hasn't completed). + getClient: (hostId: string) => RpcClient + // Connection state for a given host (driven by client.onStateChange). + useHostState: (hostId: string) => ConnectionState + // Useful for the home screen which renders all hosts at once. + useAllClients: () => Array<{ hostId: string; client: RpcClient }> +} + +export const RpcClientProvider: React.FC<{ children: React.ReactNode }> + +export const useHostClient: (hostId: string) => { + client: RpcClient + state: ConnectionState +} +``` + +Internal store (single `useRef` in the provider): + +```ts +type StoreEntry = { + client: RpcClient + state: ConnectionState + refCount: number // number of active screens holding this client + closeTimer: NodeJS.Timeout | null +} +const store = useRef(new Map()) +``` + +### Lifecycle rules + +1. **Open on first read.** First `getClient(hostId)` call for a host + reads the host record (uses cached `loadHosts()`), then calls + `connect()` and stores the client. Subsequent calls return the + cached entry. +2. **Idle close timer.** When `refCount` drops to 0 (all screens for + that host unmounted), schedule a 30-second close timer. If a screen + for the same host mounts within 30s, cancel the timer. Otherwise, + close the client and remove from the store. + - Why 30s: covers fast tab-switching and back-navigation without + keeping idle sockets forever. Tunable based on observed behavior. +3. **Forced close on host removal.** `removeHost(hostId)` from + `host-store.ts` calls into the provider to close the client + immediately and delete the store entry. +4. **App backgrounded.** No special action — let TCP keepalive and + server-side reaping handle it. Reconnect happens on foreground. +5. **App foregrounded.** Trigger a `getState()` poll on every + non-closed entry; if any are in `disconnected` (TCP died while + backgrounded), the existing reconnect loop handles it. No new + client allocations. + +### Public surface for screens + +Each screen replaces: + +```ts +// Before +const [client, setClient] = useState(null) +const [connState, setConnState] = useState('disconnected') + +useEffect(() => { + let rpcClient: RpcClient | null = null + void (async () => { + const hosts = await loadHosts() + const host = hosts.find((h) => h.id === hostId) + if (!host) return + rpcClient = connect(host.endpoint, host.deviceToken, host.publicKeyB64, setConnState) + setClient(rpcClient) + })() + return () => { + rpcClient?.close() + } +}, [hostId]) +``` + +with: + +```ts +// After +const { client, state } = useHostClient(hostId) +``` + +Total LoC reduction across screens: ~150 lines. + +### Pair flow exception + +`app/pair-confirm.tsx` and `app/pair-scan.tsx` continue to call `connect()` +directly with **explicit `client.close()`** after the test request returns. +Reason: the host record doesn't yet exist in `loadHosts()` during pairing, +so the provider has nothing to look up. The pair flow's client is a +short-lived transient that delivers `getStatus()` once and then dies. + +After `saveHost()` succeeds, the user is navigated away; the next time +they enter `/h/[hostId]/...`, the provider opens a fresh client through +the normal path. + +### Streaming subscription handling + +The home screen's `accounts.subscribe` stream and the session screen's +terminal subscriptions remain owned by their respective screens — the +provider doesn't manage subscriptions, only the underlying transport. +Each screen's effect calls `client.subscribe(...)` and stores the +returned unsubscribe function. On unmount, the screen unsubscribes +(returns to the existing per-screen pattern, just over a shared +transport). The transport's `subscribe()` already correctly multiplexes +multiple listeners on one WebSocket via the `id` field. + +### State propagation + +`useHostState(hostId)` returns the live `ConnectionState`. The provider +maintains a per-host `useState` keyed by hostId; the `client.onStateChange` +listener is wired once at client creation and updates the corresponding +state slot. `useHostState` reads from this state via `useSyncExternalStore` +or a context selector — the choice is mostly preference; in this +codebase, given the small state shape, a simple `useContext + useMemo` +of the matching slot is fine. + +## Migration + +Step-by-step, each step independently shippable: + +1. **Add `RpcClientProvider` and `useHostClient`.** No callers yet. + Wire into `app/_layout.tsx`. Existing screens unchanged. +2. **Migrate session screen** (highest-risk, most-used). Replace + per-screen `connect()` with `useHostClient`. Test connection + behavior, terminal create/delete, scrollback hydration. +3. **Migrate host detail and accounts screens.** Same pattern. +4. **Migrate home screen.** Replace `clientsRef` with + `useAllClients()`. The home screen's per-host streaming + subscriptions move into a hook that runs per-host. +5. **Add `HostScopedClientGate`** at `app/h/_layout.tsx` to centralize + the gate and remove duplicated loading-state logic. +6. **Delete legacy code.** Remove the dead `connect()` import paths + from each screen. Codepoint reduction. +7. **Remove server-side `hasOtherConnections` complexity** in a + follow-up: with one socket per token, the multi-socket reconciliation + in `runtime-rpc.ts` `wsTransport.onConnectionClose` simplifies. + This is a desktop-side cleanup PR done after mobile rolls out. + +Each step is tested in isolation; rollback per step is trivial. + +## Risks + +### R1: Connection loss while screens are mounted + +**Risk.** Today, when a screen unmounts due to network loss, its +client closes and reopens on remount. Under the new design, a +network-loss-during-screen-mounted means the client lives but is in +`reconnecting` state. + +**Mitigation.** The existing `RpcClient` already handles this — its +internal reconnect loop runs invisibly. Screens already render based +on `connState === 'connected'`, so they stay in their `connecting` +UI until the loop succeeds. No regression. + +### R2: One bad host poisons the singleton + +**Risk.** If the client for one host is wedged in `reconnecting` due +to a desktop-side issue, all screens for that host inherit the wedged +state. Under the per-screen design, navigating to a different screen +gave a fresh client with a chance to connect cleanly. + +**Mitigation.** "Force reconnect" affordance: a button on the host +detail "Connection issues" UI calls +`provider.forceReconnect(hostId)` — close + reopen the client. Users +who hit a stuck state get a one-tap recovery without uninstalling. +Implemented as part of step 2. + +### R3: Idle close timer races + +**Risk.** A user navigates from session → home → back to session +within 35 seconds. The 30s idle timer fires between hops and closes +the client; the back-navigation has to wait for a fresh handshake. + +**Mitigation.** Cancel the timer at `getClient(hostId)` time, not at +mount time. As long as the consumer holds a reference to the client, +the timer is paused. Standard refcount pattern. + +### R4: Memory / state leak on rapid host removal + +**Risk.** User removes a host while a screen for it is mounted. +The screen's reference is now dangling. + +**Mitigation.** `removeHost(hostId)` triggers an explicit close + +delete from the store. Screens already handle `auth-failed` / +`disconnected` states (the client transitions to one of them on +forced close). Add a navigation-bounce in those states so the screen +returns to the host list. + +### R5: Pair-flow socket leaks through provider + +**Risk.** If pair-confirm crashes mid-handshake before its explicit +`close()`, the socket leaks. + +**Mitigation.** Independent of the provider — same risk exists today. +Add a try/finally + cleanup useEffect in pair-confirm. + +### R6: Provider re-initialization on hot reload (dev only) + +**Risk.** Metro hot reload re-runs `RpcClientProvider`, possibly +spawning new clients while old ones are still in the store. + +**Mitigation.** On provider mount, scan the store for entries whose +clients report `closed` state and prune. Acceptable dev-only friction. + +## Test Plan + +### Unit / hook tests + +- `useHostClient` returns the same client instance across multiple + consumers for the same hostId. +- `removeHost(hostId)` immediately closes the client (assert via + `client.getState()`). +- Idle close timer: zero refcount → 30s wait → client closed. +- Idle close timer cancellation: zero refcount → 15s wait → + consumer subscribes → no close. + +### Integration / manual + +- Create one host; navigate home → host detail → session → back ×10 + rapidly. Single socket on desktop (verify via desktop debug log). +- Background app for 5 min; foreground; verify reconnect uses same + client instance, no leak. +- Remove host while session screen mounted; screen bounces back to + home; client closed. +- Force-reconnect button on host detail; client closes and reopens + cleanly. +- Hot reload during development; no socket leak (verify desktop + active-connection count). + +### Regression + +- Terminal create/delete works during normal browse (Bug A from + initial reports — should never recur once cap pressure is removed). +- Scrollback hydration unchanged. +- Phone-fit / driver-lock state machine unchanged + (`docs/mobile-presence-lock.md` invariants hold). + +## Out of scope + +- **Desktop-side connection LRU eviction.** Useful as a defense-in-depth + but not needed once mobile self-limits to one socket per host. +- **Application-level ping/pong.** Worth adding but separate concern; + helps server reap dead sockets faster regardless of how many a single + client opens. +- **iOS share extension or system-wide deep-link integration.** Future + product work. + +## Effort estimate + +3–4 hours including tests and incremental migration. Each step +independently mergeable. + +## References + +- `docs/mobile-presence-lock.md` — driver-state-machine that depends + on per-client identity. Single-client-per-host simplifies but + doesn't break this contract. +- `docs/mobile-prefer-renderer-scrollback.md` — scrollback hydration + flow. Subscriptions remain per-screen; transport changes are + transparent. +- `mobile/src/transport/rpc-client.ts` — existing `connect()` + implementation; reconnect loop, E2EE handshake, subscription + multiplexing all preserved as-is. +- `src/main/runtime/rpc/ws-transport.ts` — server-side + `MAX_WS_CONNECTIONS = 32`, `hasOtherConnections` reconciliation + that becomes simpler post-migration. diff --git a/mobile/app/_layout.tsx b/mobile/app/_layout.tsx index bdae84320..5bfb54283 100644 --- a/mobile/app/_layout.tsx +++ b/mobile/app/_layout.tsx @@ -7,6 +7,7 @@ import * as Notifications from 'expo-notifications' import * as Linking from 'expo-linking' import { colors } from '../src/theme/mobile-theme' import { OrcaLogo } from '../src/components/OrcaLogo' +import { RpcClientProvider } from '../src/transport/client-context' // Why: keeps the native splash screen visible until the React tree is mounted // and ready to render. Without this the user sees a blank white/black frame @@ -79,33 +80,35 @@ export default function RootLayout() { }, []) return ( - - - - + + + + - - - - - - - - - + > + + }} + /> + + + + + + + + + + ) } diff --git a/mobile/app/h/[hostId]/accounts.tsx b/mobile/app/h/[hostId]/accounts.tsx index 9319b616e..24a555b7f 100644 --- a/mobile/app/h/[hostId]/accounts.tsx +++ b/mobile/app/h/[hostId]/accounts.tsx @@ -12,9 +12,10 @@ import { import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context' import { useLocalSearchParams, useRouter } from 'expo-router' import { ChevronLeft, Check, RefreshCw, User } from 'lucide-react-native' -import { connect, type RpcClient } from '../../../src/transport/rpc-client' +import type { RpcClient } from '../../../src/transport/rpc-client' import { loadHosts } from '../../../src/transport/host-store' -import type { ConnectionState, RpcSuccess } from '../../../src/transport/types' +import { useHostClient } from '../../../src/transport/client-context' +import type { RpcSuccess } from '../../../src/transport/types' import { colors, spacing, typography, radii } from '../../../src/theme/mobile-theme' import { ClaudeIcon, OpenAIIcon } from '../../../src/components/AgentIcons' import { @@ -30,8 +31,8 @@ export default function AccountsScreen() { const insets = useSafeAreaInsets() const { hostId } = useLocalSearchParams<{ hostId: string }>() - const [client, setClient] = useState(null) - const [connState, setConnState] = useState('connecting') + // Why: shared client per host. See docs/mobile-shared-client-per-host.md. + const { client, state: connState } = useHostClient(hostId) const [hostName, setHostName] = useState('') const [snapshot, setSnapshot] = useState(null) const [error, setError] = useState(null) @@ -39,34 +40,24 @@ export default function AccountsScreen() { const [busyAccountId, setBusyAccountId] = useState(null) const clientRef = useRef(null) - // Why: connect to the host's WebSocket on mount and tear down on - // unmount. Mirrors the connection lifecycle used in - // /h/[hostId]/index.tsx so reconnect/auth-failed states are handled - // identically. + useEffect(() => { + clientRef.current = client + }, [client]) + useEffect(() => { if (!hostId) return - let cancelled = false - let rpcClient: RpcClient | null = null - - void (async () => { - const hosts = await loadHosts() + let stale = false + void loadHosts().then((hosts) => { + if (stale) return const host = hosts.find((h) => h.id === hostId) if (!host) { - if (!cancelled) setError('Host not found') + setError('Host not found') return } - if (!cancelled) setHostName(host.name) - rpcClient = connect(host.endpoint, host.deviceToken, host.publicKeyB64, (state) => { - if (!cancelled) setConnState(state) - }) - clientRef.current = rpcClient - if (!cancelled) setClient(rpcClient) - })() - + setHostName(host.name) + }) return () => { - cancelled = true - if (rpcClient) rpcClient.close() - clientRef.current = null + stale = true } }, [hostId]) diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index e5c9c3182..7fd24db83 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -27,8 +27,14 @@ import { Check, UserCircle } from 'lucide-react-native' -import { connect, type RpcClient } from '../../../src/transport/rpc-client' +import type { RpcClient } from '../../../src/transport/rpc-client' import { loadHosts, updateLastConnected, removeHost } from '../../../src/transport/host-store' +import { + useHostClient, + useCloseHost, + useForceReconnect, + useReconnectAttempt +} from '../../../src/transport/client-context' import type { ConnectionState, RpcSuccess } from '../../../src/transport/types' import { triggerMediumImpact } from '../../../src/platform/haptics' import { StatusDot } from '../../../src/components/StatusDot' @@ -39,7 +45,7 @@ import { ActionSheetContent } from '../../../src/components/ActionSheetModal' import { ConfirmModal } from '../../../src/components/ConfirmModal' import { BottomDrawer } from '../../../src/components/BottomDrawer' import { getCachedWorktrees } from '../../../src/cache/worktree-cache' -import { colors, spacing, typography } from '../../../src/theme/mobile-theme' +import { colors, radii, spacing, typography } from '../../../src/theme/mobile-theme' import { loadPinnedIds, savePinnedIds, @@ -85,6 +91,22 @@ const STATUS_LABELS: Record = { 'auth-failed': 'Auth failed' } +// Why: same threshold as the home screen — kicks the label from +// "Reconnecting…" to "Can't connect" once the rpc-client has cycled enough +// times to indicate a real problem (wrong port, server down, network change). +const RECONNECT_FAILURE_THRESHOLD = 3 + +function getStatusDisplay( + state: ConnectionState, + attempts: number +): { label: string; isError: boolean } { + if (state === 'auth-failed') return { label: 'Auth failed', isError: true } + if (state === 'reconnecting' && attempts >= RECONNECT_FAILURE_THRESHOLD) { + return { label: "Can't connect", isError: true } + } + return { label: STATUS_LABELS[state], isError: false } +} + const SORT_OPTIONS: PickerOption[] = [ { value: 'smart', label: 'Smart', subtitle: 'Unread and active first' }, { value: 'name', label: 'Name', subtitle: 'Alphabetical by name' }, @@ -245,9 +267,13 @@ export default function HostScreen() { const [initialCache] = useState(() => hostId ? (getCachedWorktrees(hostId) as Worktree[] | null) : null ) - const [client, setClient] = useState(null) + // Why: shared client per host owned by RpcClientProvider. See + // docs/mobile-shared-client-per-host.md. + const { client, state: connState } = useHostClient(hostId) + const reconnectAttempts = useReconnectAttempt(hostId) const clientRef = useRef(null) - const [connState, setConnState] = useState('disconnected') + const closeHostClient = useCloseHost() + const forceReconnectHost = useForceReconnect() const [worktrees, setWorktrees] = useState(initialCache ?? []) const [worktreesLoaded, setWorktreesLoaded] = useState(initialCache != null) const [hostName, setHostName] = useState('') @@ -303,17 +329,16 @@ export default function HostScreen() { } }, [hostId]) + // Why: keep clientRef in sync so existing imperative call sites work + // unchanged. Also re-seed the cached worktree list on hostId change + // since the useState initializer only runs on first mount. + useEffect(() => { + clientRef.current = client + }, [client]) + useEffect(() => { - let disposed = false - let rpcClient: RpcClient | null = null - clientRef.current = null - setClient(null) - setConnState('connecting') setHostName('') setError('') - // Why: re-seed from the current host's cache on every hostId change. - // The useState initializer only runs on first mount, so if Expo Router - // reuses this screen with a different hostId, we must reset here. const freshCache = hostId ? (getCachedWorktrees(hostId) as Worktree[] | null) : null if (freshCache) { setWorktrees(freshCache) @@ -324,45 +349,20 @@ export default function HostScreen() { setWorktrees([]) setLastKnownWorktrees([]) } - - // Why: defer the RPC connection until after the navigation animation - // completes. Without this, connect() and loadHosts() block the JS - // thread during mount, delaying the screen transition by ~200-400ms. - // With cached worktrees the user sees content instantly; the live - // connection starts once the animation settles. - const rafId = requestAnimationFrame(() => { - if (disposed) return - void (async () => { - const hosts = await loadHosts() - const host = hosts.find((h) => h.id === hostId) - if (!host || disposed) { - if (!host && !disposed) setError('Host not found') - return - } - - rpcClient = connect(host.endpoint, host.deviceToken, host.publicKeyB64, (state) => { - if (!disposed) setConnState(state) - }) - if (disposed) { - rpcClient.close() - rpcClient = null - return - } - setHostName(host.name) - clientRef.current = rpcClient - setClient(rpcClient) - - await updateLastConnected(host.id) - })() - }) - - return () => { - disposed = true - cancelAnimationFrame(rafId) - rpcClient?.close() - if (clientRef.current === rpcClient) { - clientRef.current = null + if (!hostId) return + let stale = false + void loadHosts().then((hosts) => { + if (stale) return + const host = hosts.find((h) => h.id === hostId) + if (!host) { + setError('Host not found') + return } + setHostName(host.name) + void updateLastConnected(host.id) + }) + return () => { + stale = true } }, [hostId]) @@ -499,9 +499,14 @@ export default function HostScreen() { const handleRemoveHost = useCallback(async () => { if (!hostId) return + // Why: close the shared client first so its WebSocket is gone before + // the host record disappears; otherwise the next loadHosts() the + // provider does (e.g. on remount) wouldn't find this host but the + // socket would still be open, leaking state. + closeHostClient(hostId) await removeHost(hostId) router.back() - }, [hostId, router]) + }, [hostId, router, closeHostClient]) const openWorktreeSession = useCallback( (item: Worktree) => { @@ -645,9 +650,27 @@ export default function HostScreen() { {hostName || 'Host'} - {connState !== 'connected' && ( - {STATUS_LABELS[connState]} - )} + {connState !== 'connected' && + (() => { + const status = getStatusDisplay(connState, reconnectAttempts) + const showReconnectButton = status.isError && hostId && connState !== 'auth-failed' + return ( + + + {status.label} + + {showReconnectButton && ( + void forceReconnectHost(hostId!)} + hitSlop={8} + > + Reconnect + + )} + + ) + })()} {/* Filter/sort/group toolbar */} @@ -1102,6 +1125,24 @@ const styles = StyleSheet.create({ color: colors.textSecondary, fontSize: typography.metaSize }, + statusRow: { + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm + }, + reconnectButton: { + paddingVertical: 4, + paddingHorizontal: spacing.sm, + borderRadius: radii.button, + backgroundColor: colors.bgPanel, + borderWidth: 1, + borderColor: colors.borderSubtle + }, + reconnectButtonText: { + color: colors.textPrimary, + fontSize: typography.metaSize, + fontWeight: '600' + }, authBanner: { backgroundColor: colors.bgPanel, paddingVertical: spacing.sm, diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 97c560133..8c7b8f82a 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -15,8 +15,9 @@ import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context' import { useLocalSearchParams, useRouter } from 'expo-router' import AsyncStorage from '@react-native-async-storage/async-storage' import { ArrowUp, ChevronLeft, Monitor, Plus, Smartphone } from 'lucide-react-native' -import { connect, type RpcClient } from '../../../../src/transport/rpc-client' +import type { RpcClient } from '../../../../src/transport/rpc-client' import { loadHosts } from '../../../../src/transport/host-store' +import { useHostClient } from '../../../../src/transport/client-context' import type { ConnectionState, RpcSuccess } from '../../../../src/transport/types' import { triggerMediumImpact } from '../../../../src/platform/haptics' import { @@ -123,8 +124,9 @@ export default function SessionScreen() { }>() const router = useRouter() const insets = useSafeAreaInsets() - const [client, setClient] = useState(null) - const [connState, setConnState] = useState('disconnected') + // Why: shared client per host owned by RpcClientProvider. See + // docs/mobile-shared-client-per-host.md. + const { client, state: connState } = useHostClient(hostId) const [terminals, setTerminals] = useState([]) const [terminalsLoaded, setTerminalsLoaded] = useState(false) const [input, setInput] = useState('') @@ -397,7 +399,19 @@ export default function SessionScreen() { lastKnownTerminalCountRef.current = result.terminals.length const current = activeHandleRef.current - setTerminals(result.terminals) + // Why: defense-in-depth dedupe. If the server ever returns a list + // with the same handle twice (race during rename/split, or stale + // process tracking), React would throw 'two children with same + // key' on render. Keep the first occurrence — list order matters + // for the tab strip, and createParams puts new tabs at the end. + const seen = new Set() + const deduped = result.terminals.filter((t) => { + if (seen.has(t.handle)) return false + seen.add(t.handle) + return true + }) + + setTerminals(deduped) setTerminalsLoaded(true) if (!current || !result.terminals.some((t) => t.handle === current)) { @@ -419,34 +433,32 @@ export default function SessionScreen() { [client, worktreeId, subscribeToTerminal, unsubscribeTerminal] ) + // Why: keep clientRef in sync with the shared client from + // useHostClient() so the existing imperative call sites + // (clientRef.current.sendRequest...) keep working without churn. useEffect(() => { - let disposed = false - let rpcClient: RpcClient | null = null - - void (async () => { - const hosts = await loadHosts() - const host = hosts.find((h) => h.id === hostId) - if (!host || disposed) return - - deviceTokenRef.current = host.deviceToken - rpcClient = connect(host.endpoint, host.deviceToken, host.publicKeyB64, setConnState) - if (disposed) { - rpcClient.close() - return - } - setClient(rpcClient) - clientRef.current = rpcClient - })() - + clientRef.current = client return () => { - disposed = true clearTerminalCache() - rpcClient?.close() - if (clientRef.current === rpcClient) { - clientRef.current = null - } } - }, [clearTerminalCache, hostId]) + }, [client, clearTerminalCache]) + + // Why: deviceToken is read from host record so feature code can pass + // `client.id` on subscribe/send for driver-state-machine identity. + // The shared client itself stays alive across screens; we just need + // the token alongside the client. + useEffect(() => { + if (!hostId) return + let stale = false + void loadHosts().then((hosts) => { + if (stale) return + const host = hosts.find((h) => h.id === hostId) + if (host) deviceTokenRef.current = host.deviceToken + }) + return () => { + stale = true + } + }, [hostId]) useEffect(() => { void loadCustomKeys().then(setCustomKeys) @@ -456,71 +468,88 @@ export default function SessionScreen() { // doesn't resize the window) and refit xterm once the layout settles so the // terminal grid matches the new visible area. iOS exposes 'will' events that // animate in sync with the IME; Android only fires 'did' events reliably. - useEffect(() => { - let refitTimer: ReturnType | null = null - const scheduleRefit = () => { - if (refitTimer) clearTimeout(refitTimer) - refitTimer = setTimeout(() => { - const handle = activeHandleRef.current - if (!handle) return - const ref = terminalRefs.current.get(handle) - if (!ref) return - void (async () => { - const dims = await ref.measureFitDimensions(terminalFrameHeightRef.current || undefined) - if (!dims) return - const prev = viewportRef.current - if (prev && prev.cols === dims.cols && prev.rows === dims.rows) return - viewportRef.current = dims - viewportMeasuredRef.current = true - // Why: prefer the in-place viewport update RPC over the legacy - // unsubscribe → subscribe cycle. This keeps the server-side - // mobile subscriber record alive (no driver=idle blip on the - // desktop banner; no false phone-fit baseline capture on the - // re-subscribe). The 'resized' event from the server reinits - // the xterm at the new dims via the existing subscription - // stream. Falls back to the unsubscribe/subscribe path if the - // RPC isn't available (older host build) or no client is - // connected. See docs/mobile-presence-lock.md. - const rpc = clientRef.current - const deviceToken = deviceTokenRef.current - if (rpc && deviceToken) { - try { - const response = await rpc.sendRequest('terminal.updateViewport', { - terminal: handle, - client: { id: deviceToken, type: 'mobile' as const }, - viewport: dims - }) - if (response.ok) { - return - } - } catch { - // Fall through to legacy resubscribe. - } + // Also drives re-measurement when other layout-affecting state changes + // (e.g. tab strip toggling visibility when the terminal count crosses + // 0↔1 — without this, a freshly-created 2nd tab subscribes with a + // stale viewport that doesn't account for the now-visible tab strip, + // and the server phone-fits to dims a few rows too tall). + const refitTimerRef = useRef | null>(null) + const scheduleViewportRefit = useCallback(() => { + if (refitTimerRef.current) clearTimeout(refitTimerRef.current) + refitTimerRef.current = setTimeout(() => { + const handle = activeHandleRef.current + if (!handle) return + const ref = terminalRefs.current.get(handle) + if (!ref) return + void (async () => { + const dims = await ref.measureFitDimensions(terminalFrameHeightRef.current || undefined) + if (!dims) return + const prev = viewportRef.current + if (prev && prev.cols === dims.cols && prev.rows === dims.rows) return + viewportRef.current = dims + viewportMeasuredRef.current = true + // Why: prefer the in-place viewport update RPC over the legacy + // unsubscribe → subscribe cycle. This keeps the server-side + // mobile subscriber record alive (no driver=idle blip on the + // desktop banner; no false phone-fit baseline capture on the + // re-subscribe). See docs/mobile-presence-lock.md. + const rpc = clientRef.current + const deviceToken = deviceTokenRef.current + if (rpc && deviceToken) { + try { + const response = await rpc.sendRequest('terminal.updateViewport', { + terminal: handle, + client: { id: deviceToken, type: 'mobile' as const }, + viewport: dims + }) + if (response.ok) return + } catch { + // Fall through to legacy resubscribe. } - unsubscribeTerminal(handle) - initializedHandlesRef.current.delete(handle) - subscribeToTerminal(handle) - })() - }, 150) - } + } + unsubscribeTerminal(handle) + initializedHandlesRef.current.delete(handle) + subscribeToTerminal(handle) + })() + }, 150) + }, [subscribeToTerminal, unsubscribeTerminal]) + + useEffect(() => { const onShow = (e: KeyboardEvent) => { setKeyboardHeight(e.endCoordinates?.height ?? 0) - scheduleRefit() + scheduleViewportRefit() } const onHide = () => { setKeyboardHeight(0) - scheduleRefit() + scheduleViewportRefit() } const showEvent = Platform.OS === 'ios' ? 'keyboardWillShow' : 'keyboardDidShow' const hideEvent = Platform.OS === 'ios' ? 'keyboardWillHide' : 'keyboardDidHide' const showSub = Keyboard.addListener(showEvent, onShow) const hideSub = Keyboard.addListener(hideEvent, onHide) return () => { - if (refitTimer) clearTimeout(refitTimer) + if (refitTimerRef.current) clearTimeout(refitTimerRef.current) showSub.remove() hideSub.remove() } - }, [subscribeToTerminal, unsubscribeTerminal]) + }, [scheduleViewportRefit]) + + // Why: the tab strip is hidden when only one terminal exists and shown + // once a second is created. Crossing the 1↔2 boundary changes the + // visible terminal area by ~40px, so the cached viewport dims in + // viewportRef become stale. Mark the viewport as un-measured so the + // next subscribe path's self-correcting loop (init → measure → + // resubscribe-with-fresh-viewport, see the !viewportMeasuredRef branch + // above) re-runs against the new layout. Also schedule an explicit + // refit to cover the case where no new subscribe is happening. + const tabStripVisible = terminals.length > 1 + const prevTabStripVisibleRef = useRef(tabStripVisible) + useEffect(() => { + if (prevTabStripVisibleRef.current === tabStripVisible) return + prevTabStripVisibleRef.current = tabStripVisible + viewportMeasuredRef.current = false + scheduleViewportRefit() + }, [tabStripVisible, scheduleViewportRefit]) useEffect(() => { if (hostId && worktreeId) { @@ -747,10 +776,17 @@ export default function SessionScreen() { } activeHandleRef.current = created.handle setActiveHandle(created.handle) - setTerminals((prev) => [ - ...prev, - { handle: created.handle, title: created.title || 'Terminal', isActive: true } - ]) + setTerminals((prev) => { + // Why: guard against duplicates if a parallel fetchTerminals() + // already inserted this handle. Without this, React throws + // 'two children with the same key' when both the optimistic + // insert and a canonical refetch race during creation. + if (prev.some((t) => t.handle === created.handle)) return prev + return [ + ...prev, + { handle: created.handle, title: created.title || 'Terminal', isActive: true } + ] + }) subscribeToTerminal(created.handle) setTimeout(() => void fetchTerminals(), 500) } else { diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 596528168..933d2a546 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -11,7 +11,10 @@ import { GitPullRequest, ChevronRight, Terminal, - Plus + Plus, + RefreshCw, + PowerOff, + Edit3 } from 'lucide-react-native' import { ClaudeIcon, OpenAIIcon } from '../src/components/AgentIcons' import { @@ -22,13 +25,20 @@ import { } from '../src/components/AccountUsage' import AsyncStorage from '@react-native-async-storage/async-storage' import { loadHosts, removeHost, renameHost } from '../src/transport/host-store' -import { connect, type RpcClient } from '../src/transport/rpc-client' +import type { RpcClient } from '../src/transport/rpc-client' +import { + useAllHostClients, + useCloseHost, + useForceReconnect, + usePrimeHosts +} from '../src/transport/client-context' import { subscribeToDesktopNotifications } from '../src/notifications/mobile-notifications' import type { ConnectionState, HostProfile } from '../src/transport/types' import { triggerMediumImpact } from '../src/platform/haptics' import { OrcaLogo } from '../src/components/OrcaLogo' +import { StatusDot } from '../src/components/StatusDot' import { TextInputModal } from '../src/components/TextInputModal' -import { ActionSheetModal } from '../src/components/ActionSheetModal' +import { ActionSheetModal, type ActionSheetAction } from '../src/components/ActionSheetModal' import { ConfirmModal } from '../src/components/ConfirmModal' import { setCachedWorktrees, getCachedWorktrees } from '../src/cache/worktree-cache' import { loadHomeSnapshot, saveHomeSnapshot } from '../src/cache/home-snapshot-cache' @@ -52,6 +62,23 @@ const STATUS_LABELS: Record = { 'auth-failed': 'Auth failed' } +// Why: a few quick reconnects are normal (laptop wake, brief network blip). +// After this many failed attempts in a row, the user almost certainly has +// a real problem (wrong port, server down, network change), so escalate +// the label and color so it's obvious something's wrong. +const RECONNECT_FAILURE_THRESHOLD = 3 + +function getStatusDisplay( + state: ConnectionState, + attempts: number +): { label: string; isError: boolean } { + if (state === 'auth-failed') return { label: 'Auth failed', isError: true } + if (state === 'reconnecting' && attempts >= RECONNECT_FAILURE_THRESHOLD) { + return { label: "Can't connect", isError: true } + } + return { label: STATUS_LABELS[state], isError: false } +} + type StatsSummary = { totalAgentsSpawned: number totalPRsCreated: number @@ -198,13 +225,36 @@ export default function HomeScreen() { const [renameTarget, setRenameTarget] = useState(null) const [confirmRemove, setConfirmRemove] = useState(null) const [hostStates, setHostStates] = useState>({}) + const [hostAttempts, setHostAttempts] = useState>({}) const [stats, setStats] = useState(null) const [worktreeInfo, setWorktreeInfo] = useState>({}) const [accountsByHost, setAccountsByHost] = useState>({}) const [lastVisited, setLastVisited] = useState<{ hostId: string; worktreeId: string } | null>( null ) - const clientsRef = useRef>([]) + + // Why: read shared clients from the per-host store. Replaces the prior + // pattern of opening N independent WebSockets here. See + // docs/mobile-shared-client-per-host.md. + const hostIds = useMemo(() => hosts.map((h) => h.id), [hosts]) + const allClients = useAllHostClients(hostIds) + const closeHostClient = useCloseHost() + const forceReconnectHost = useForceReconnect() + const primeHosts = usePrimeHosts() + // Why: feed the loaded HostProfiles into the provider's prime cache as + // soon as we have them. This avoids a second Keychain pass inside + // openEntry on cold start (which serialised behind the first one and + // showed up as multi-second connect latency). + useEffect(() => { + if (hosts.length > 0) primeHosts(hosts) + }, [hosts, primeHosts]) + const allClientsRef = useRef>([]) + useEffect(() => { + allClientsRef.current = allClients.map((entry) => ({ + hostId: entry.hostId, + client: entry.client + })) + }, [allClients]) // Why: hydrate the home page from a persisted snapshot on cold-start so // Resume + Account-usage cards paint immediately with last-known data @@ -259,7 +309,7 @@ export default function HomeScreen() { setLastVisited(JSON.parse(raw)) } catch {} }) - for (const entry of clientsRef.current) { + for (const entry of allClientsRef.current) { if (entry.client.getState() === 'connected') { fetchStats(entry.client, setStats, () => stale) fetchWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => stale) @@ -277,51 +327,91 @@ export default function HomeScreen() { [hosts] ) + // Why: mirror per-host connection state into hostStates so existing + // render code (status dots, connecting indicators) keeps working. useEffect(() => { - let disposed = false - const notifCleanups: Array<() => void> = [] - const entries = hosts.flatMap((host) => { - if (!host.publicKeyB64 || !host.deviceToken) { - setHostStates((prev) => ({ ...prev, [host.id]: 'auth-failed' })) - return [] + setHostAttempts((prev) => { + const next: Record = { ...prev } + let changed = false + for (const entry of allClients) { + const a = entry.client.getReconnectAttempt() + if (next[entry.hostId] !== a) { + next[entry.hostId] = a + changed = true + } } - setHostStates((prev) => ({ - ...prev, - [host.id]: prev[host.id] ?? 'connecting' - })) - let client: ReturnType - try { - client = connect(host.endpoint, host.deviceToken, host.publicKeyB64, (state) => { - if (disposed) return - setHostStates((prev) => ({ ...prev, [host.id]: state })) - }) - } catch { - setHostStates((prev) => ({ ...prev, [host.id]: 'auth-failed' })) - return [] + return changed ? next : prev + }) + setHostStates((prev) => { + const next: Record = { ...prev } + let changed = false + const liveIds = new Set(allClients.map((e) => e.hostId)) + for (const entry of allClients) { + if (next[entry.hostId] !== entry.state) { + next[entry.hostId] = entry.state + changed = true + } } + // Why: when a paired host disappears from allClients (because the + // user tapped Disconnect, or the host record was invalid) the card + // must reflect that. We only force-update hosts whose state was + // already tracked — otherwise the initial-acquire frame (entry not + // yet materialised) would briefly flip every host to 'disconnected'. + for (const host of hosts) { + if (liveIds.has(host.id)) continue + if (!host.publicKeyB64 || !host.deviceToken) { + if (next[host.id] !== 'auth-failed') { + next[host.id] = 'auth-failed' + changed = true + } + continue + } + const prevState = next[host.id] + if (prevState && prevState !== 'disconnected' && prevState !== 'auth-failed') { + next[host.id] = 'disconnected' + changed = true + } + } + // Drop entries for hosts we no longer track at all. + for (const id of Object.keys(next)) { + if (!liveIds.has(id) && hosts.some((h) => h.id === id) === false) { + delete next[id] + changed = true + } + } + return changed ? next : prev + }) + }, [allClients, hosts]) + // Why: per-host streaming subscriptions (notifications + accounts) and + // one-shot stats fetches when each host transitions to 'connected'. + // Runs once per (hostId, client) pair and tears down when that pair + // changes. The provider keeps the underlying socket open across + // resubscription cycles so this is cheap. + useEffect(() => { + const cleanups: Array<() => void> = [] + for (const entry of allClients) { let unsubNotif: (() => void) | null = null let unsubAccounts: (() => void) | null = null let statsFetched = false - const unsubState = client.onStateChange((state) => { + const wireUp = (state: ConnectionState) => { if (state === 'connected') { if (!unsubNotif) { - unsubNotif = subscribeToDesktopNotifications(client) + unsubNotif = subscribeToDesktopNotifications(entry.client) } if (!unsubAccounts) { - unsubAccounts = client.subscribe('accounts.subscribe', null, (payload) => { - if (disposed || !payload || typeof payload !== 'object') return + unsubAccounts = entry.client.subscribe('accounts.subscribe', null, (payload) => { + if (!payload || typeof payload !== 'object') return const evt = payload as { type?: string; snapshot?: AccountsSnapshot } if ((evt.type === 'ready' || evt.type === 'snapshot') && evt.snapshot) { - const snap = evt.snapshot - setAccountsByHost((prev) => ({ ...prev, [host.id]: snap })) + setAccountsByHost((prev) => ({ ...prev, [entry.hostId]: evt.snapshot! })) } }) } if (!statsFetched) { statsFetched = true - fetchStats(client, setStats, () => disposed) - fetchWorktreeInfo(client, host.id, setWorktreeInfo, () => disposed) + fetchStats(entry.client, setStats, () => false) + fetchWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => false) } } else { if (unsubNotif) { @@ -333,25 +423,29 @@ export default function HomeScreen() { unsubAccounts = null } } - }) - notifCleanups.push(() => { + } + wireUp(entry.state) + const unsubState = entry.client.onStateChange(wireUp) + cleanups.push(() => { unsubState() unsubNotif?.() unsubAccounts?.() }) - - return [{ hostId: host.id, client }] - }) - - clientsRef.current = entries - - return () => { - disposed = true - clientsRef.current = [] - for (const cleanup of notifCleanups) cleanup() - for (const entry of entries) entry.client.close() } - }, [hosts]) + return () => { + for (const c of cleanups) c() + } + // Why: depend on the host-id set, not the whole allClients array, so + // resubscriptions don't fire on every render that produces a new + // array reference. Client identity is stable per hostId for the + // lifetime of the underlying transport. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + allClients + .map((e) => e.hostId) + .sort() + .join(',') + ]) // Why: prefer the worktree the user last opened on this device so the // "Resume" card reflects their mobile session history, not just the @@ -433,6 +527,9 @@ export default function HomeScreen() { async function handleRemove() { if (!confirmRemove) return try { + // Why: close the shared client first so the WebSocket is gone + // before the host record disappears from loadHosts(). + closeHostClient(confirmRemove.id) await removeHost(confirmRemove.id) setConfirmRemove(null) setHosts(await loadHosts()) @@ -538,8 +635,10 @@ export default function HomeScreen() { ItemSeparatorComponent={CardGap} renderItem={({ item }) => { const state = hostStates[item.id] ?? 'connecting' + const attempts = hostAttempts[item.id] ?? 0 const connected = state === 'connected' const info = worktreeInfo[item.id] + const status = getStatusDisplay(state, attempts) return ( [styles.hostCard, pressed && styles.hostCardPressed]} @@ -564,14 +663,11 @@ export default function HomeScreen() { {item.name} - - - {STATUS_LABELS[state]} + + + {status.label} {connected && info ? ` · ${info.totalWorktrees} worktree${info.totalWorktrees !== 1 ? 's' : ''}${info.activeCount > 0 ? ` · ${info.activeCount} active` : ''}` : ''} @@ -749,25 +845,52 @@ export default function HomeScreen() { visible={actionTarget != null} title={actionTarget?.name} message={actionTarget ? endpointLabel(actionTarget.endpoint) : undefined} - actions={[ - { - label: 'Rename', + actions={(() => { + const host = actionTarget + if (!host) return [] + const state = hostStates[host.id] ?? 'connecting' + const isLive = + state === 'connected' || + state === 'connecting' || + state === 'handshaking' || + state === 'reconnecting' + const items: ActionSheetAction[] = [] + items.push({ + label: 'Reconnect', + icon: RefreshCw, onPress: () => { - const host = actionTarget setActionTarget(null) - if (host) setRenameTarget(host) + void forceReconnectHost(host.id) } - }, - { + }) + if (isLive) { + items.push({ + label: 'Disconnect', + icon: PowerOff, + onPress: () => { + setActionTarget(null) + closeHostClient(host.id) + } + }) + } + items.push({ + label: 'Rename', + icon: Edit3, + onPress: () => { + setActionTarget(null) + setRenameTarget(host) + } + }) + items.push({ label: 'Remove', destructive: true, onPress: () => { - const host = actionTarget setActionTarget(null) - if (host) setConfirmRemove(host) + setConfirmRemove(host) } - } - ]} + }) + return items + })()} onClose={() => setActionTarget(null)} /> diff --git a/mobile/src/components/StatusDot.tsx b/mobile/src/components/StatusDot.tsx index 63777890d..86f36f450 100644 --- a/mobile/src/components/StatusDot.tsx +++ b/mobile/src/components/StatusDot.tsx @@ -7,7 +7,7 @@ const stateColors: Record = { connecting: colors.statusAmber, handshaking: colors.statusAmber, reconnecting: colors.statusAmber, - disconnected: colors.statusRed, + disconnected: colors.textMuted, 'auth-failed': colors.statusRed } diff --git a/mobile/src/transport/client-context.tsx b/mobile/src/transport/client-context.tsx new file mode 100644 index 000000000..bfa935396 --- /dev/null +++ b/mobile/src/transport/client-context.tsx @@ -0,0 +1,455 @@ +// Why: collapses the per-screen WebSocket connection model into a single +// shared RpcClient per host. Implements the design in +// docs/mobile-shared-client-per-host.md. +// +// Lifecycle rules: +// - First request for a host opens its client lazily. +// - Refcount tracks active subscribers; when it drops to zero we schedule +// a 30-second idle close timer. If a new subscriber arrives within that +// window we cancel and reuse the same client. +// - removeHost() forces an immediate close so re-pairing gets a fresh +// transport. +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useRef, + useState, + type ReactNode +} from 'react' +import { connect, type RpcClient } from './rpc-client' +import { loadHosts } from './host-store' +import type { ConnectionState, HostProfile } from './types' + +const IDLE_CLOSE_MS = 30_000 + +type StoreEntry = { + client: RpcClient + state: ConnectionState + refCount: number + idleTimer: ReturnType | null + unsubState: () => void +} + +type ContextValue = { + acquire: (hostId: string, host?: HostProfile) => RpcClient | null + release: (hostId: string) => void + forceReconnect: (hostId: string) => Promise + closeHost: (hostId: string) => void + getState: (hostId: string) => ConnectionState + getReconnectAttempt: (hostId: string) => number + subscribeHostState: (hostId: string, listener: (state: ConnectionState) => void) => () => void + getAllClients: () => Array<{ hostId: string; client: RpcClient }> + subscribeAllHosts: (listener: () => void) => () => void + // Why: lets the home screen feed already-loaded HostProfiles in so we + // don't pay loadHosts() latency twice (once in the focus-effect, again + // inside openEntry). + primeHosts: (hosts: HostProfile[]) => void +} + +const Ctx = createContext(null) + +export function RpcClientProvider({ children }: { children: ReactNode }) { + // Why: entries live in a ref so updates don't force re-renders of the + // entire tree on every connection state change. State propagation goes + // through per-host listener Sets instead. + const storeRef = useRef>(new Map()) + const stateListenersRef = useRef void>>>(new Map()) + const allHostsListenersRef = useRef void>>(new Set()) + + // Pending opens (avoid two acquire() callers in the same render racing the + // host lookup). Keyed by hostId, value is a sentinel resolved when the + // entry materialises. + const pendingOpensRef = useRef>>(new Map()) + + // Why: a fast-path cache of already-loaded HostProfiles. Screens that + // have run loadHosts() can call primeHosts() to populate this and skip + // the second loadHosts() inside openEntry. Without this we'd serialize + // two Keychain passes on cold start. + const primedHostsRef = useRef>(new Map()) + + function notifyHostState(hostId: string, state: ConnectionState) { + const set = stateListenersRef.current.get(hostId) + if (!set) return + for (const listener of set) listener(state) + } + + function notifyAllHosts() { + for (const listener of allHostsListenersRef.current) listener() + } + + const closeEntry = useCallback((hostId: string) => { + const entry = storeRef.current.get(hostId) + if (!entry) return + if (entry.idleTimer) clearTimeout(entry.idleTimer) + entry.unsubState() + entry.client.close() + storeRef.current.delete(hostId) + notifyHostState(hostId, 'disconnected') + notifyAllHosts() + }, []) + + const openEntry = useCallback(async (hostId: string): Promise => { + const existing = pendingOpensRef.current.get(hostId) + if (existing) { + await existing + return storeRef.current.get(hostId) ?? null + } + let resolve: () => void = () => {} + const promise = new Promise((res) => { + resolve = res + }) + pendingOpensRef.current.set(hostId, promise) + + try { + // Why: prefer the primed cache (populated by primeHosts when the + // screen already ran loadHosts) so we don't serialize a second + // Keychain pass behind the first one on cold start. + let host = primedHostsRef.current.get(hostId) + if (!host) { + try { + const hosts = await loadHosts() + host = hosts.find((h) => h.id === hostId) + } catch { + // Why: a Keychain failure on cold start (rare but observed — + // happens when iOS Keychain is mid-unlock or Android Keystore + // races the JS bridge). Surface it as 'disconnected' so the + // home card flips off the perma-spinner and the user can hit + // Reconnect from the action sheet to retry. + notifyHostState(hostId, 'disconnected') + notifyAllHosts() + return null + } + if (!host) return null + } + + // Re-check after any await — another acquire() may have completed. + const after = storeRef.current.get(hostId) + if (after) return after + + let client: RpcClient + try { + client = connect(host.endpoint, host.deviceToken, host.publicKeyB64) + } catch { + // Why: connect() can throw synchronously if the public key is + // malformed or the endpoint URL is invalid. Notify so the UI + // doesn't sit on a stale 'connecting' label forever. + notifyHostState(hostId, 'disconnected') + notifyAllHosts() + return null + } + const unsubState = client.onStateChange((state) => { + const cur = storeRef.current.get(hostId) + if (!cur) return + cur.state = state + notifyHostState(hostId, state) + }) + const entry: StoreEntry = { + client, + state: client.getState(), + refCount: 0, + idleTimer: null, + unsubState + } + storeRef.current.set(hostId, entry) + notifyHostState(hostId, entry.state) + notifyAllHosts() + return entry + } finally { + pendingOpensRef.current.delete(hostId) + resolve() + } + }, []) + + // Why: `acquire` is the synchronous get-or-open. If the entry already + // exists, return its client immediately and bump the refcount. If not, + // kick off an async open (the consumer will subscribe via + // `subscribeHostState` and re-read once 'connecting' fires). Optionally + // accepts the HostProfile so the caller can avoid an extra loadHosts() + // pass inside openEntry. + const acquire = useCallback( + (hostId: string, host?: HostProfile): RpcClient | null => { + if (host) primedHostsRef.current.set(hostId, host) + const existing = storeRef.current.get(hostId) + if (existing) { + existing.refCount += 1 + if (existing.idleTimer) { + clearTimeout(existing.idleTimer) + existing.idleTimer = null + } + return existing.client + } + // Trigger async open. The acquire-side will return null this tick and + // try again once the state listener fires; consumers are expected to + // call acquire() inside an effect that re-runs on state changes. + void openEntry(hostId).then((entry) => { + if (!entry) return + entry.refCount += 1 + }) + return null + }, + [openEntry] + ) + + const primeHosts = useCallback((hosts: HostProfile[]) => { + for (const host of hosts) primedHostsRef.current.set(host.id, host) + }, []) + + const release = useCallback( + (hostId: string) => { + const entry = storeRef.current.get(hostId) + if (!entry) return + entry.refCount = Math.max(0, entry.refCount - 1) + if (entry.refCount > 0) return + if (entry.idleTimer) clearTimeout(entry.idleTimer) + entry.idleTimer = setTimeout(() => { + // Why: only close if still idle when the timer fires. A late acquire + // would have cleared the timer. + const cur = storeRef.current.get(hostId) + if (!cur || cur.refCount > 0) return + closeEntry(hostId) + }, IDLE_CLOSE_MS) + }, + [closeEntry] + ) + + const forceReconnect = useCallback( + async (hostId: string) => { + const entry = storeRef.current.get(hostId) + // Why: if the entry was previously closed (e.g. user tapped + // Disconnect), refCount is lost. Fall back to the number of active + // state listeners as a proxy for "screens currently watching this + // host," so the freshly-opened entry doesn't trip the idle-close + // timer immediately. + const listenerCount = stateListenersRef.current.get(hostId)?.size ?? 0 + const savedRefCount = entry?.refCount ?? Math.max(1, listenerCount) + if (entry) { + if (entry.idleTimer) clearTimeout(entry.idleTimer) + entry.unsubState() + entry.client.close() + storeRef.current.delete(hostId) + } + const fresh = await openEntry(hostId) + if (fresh) fresh.refCount = savedRefCount + }, + [openEntry] + ) + + const getState = useCallback((hostId: string): ConnectionState => { + return storeRef.current.get(hostId)?.state ?? 'disconnected' + }, []) + + const getReconnectAttempt = useCallback((hostId: string): number => { + return storeRef.current.get(hostId)?.client.getReconnectAttempt() ?? 0 + }, []) + + const subscribeHostState = useCallback( + (hostId: string, listener: (state: ConnectionState) => void) => { + let set = stateListenersRef.current.get(hostId) + if (!set) { + set = new Set() + stateListenersRef.current.set(hostId, set) + } + set.add(listener) + return () => { + const s = stateListenersRef.current.get(hostId) + if (!s) return + s.delete(listener) + if (s.size === 0) stateListenersRef.current.delete(hostId) + } + }, + [] + ) + + const getAllClients = useCallback((): Array<{ hostId: string; client: RpcClient }> => { + const out: Array<{ hostId: string; client: RpcClient }> = [] + for (const [hostId, entry] of storeRef.current) { + out.push({ hostId, client: entry.client }) + } + return out + }, []) + + const subscribeAllHosts = useCallback((listener: () => void) => { + allHostsListenersRef.current.add(listener) + return () => { + allHostsListenersRef.current.delete(listener) + } + }, []) + + // Close all clients on provider unmount (app shutdown / hot reload). + useEffect(() => { + const store = storeRef.current + return () => { + for (const [hostId] of store) closeEntry(hostId) + } + }, [closeEntry]) + + const value = useMemo( + () => ({ + acquire, + release, + forceReconnect, + closeHost: closeEntry, + getState, + getReconnectAttempt, + subscribeHostState, + getAllClients, + subscribeAllHosts, + primeHosts + }), + [ + acquire, + release, + forceReconnect, + closeEntry, + getState, + getReconnectAttempt, + subscribeHostState, + getAllClients, + subscribeAllHosts, + primeHosts + ] + ) + + return {children} +} + +function useCtx(): ContextValue { + const ctx = useContext(Ctx) + if (!ctx) throw new Error('useHostClient must be used inside ') + return ctx +} + +// Why: the primary hook for screens. Acquires the shared client for a +// hostId on mount and releases on unmount. Re-renders when the host's +// connection state changes. +export function useHostClient(hostId: string | undefined): { + client: RpcClient | null + state: ConnectionState +} { + const ctx = useCtx() + const [, force] = useState(0) + const [state, setState] = useState(() => + hostId ? ctx.getState(hostId) : 'disconnected' + ) + const clientRef = useRef(null) + + useEffect(() => { + if (!hostId) { + clientRef.current = null + setState('disconnected') + return + } + let cancelled = false + // Subscribe before acquire so any state change during open is captured. + const unsub = ctx.subscribeHostState(hostId, (next) => { + if (cancelled) return + setState(next) + // Why: if the client was null at first acquire (async open), the + // first state change ('connecting'/'handshaking'/'connected') is our + // signal to re-read. + if (clientRef.current == null) { + const all = ctx.getAllClients() + const found = all.find((entry) => entry.hostId === hostId) + if (found) { + clientRef.current = found.client + force((n) => n + 1) + } + } + }) + const initial = ctx.acquire(hostId) + if (initial) { + clientRef.current = initial + setState(ctx.getState(hostId)) + } + return () => { + cancelled = true + unsub() + ctx.release(hostId) + clientRef.current = null + } + }, [ctx, hostId]) + + return { client: clientRef.current, state } +} + +// Why: home screen renders all paired hosts at once. Acquires each on +// mount, releases on unmount. The provider's refcounting ensures we +// don't double-open if a host-detail screen is also open. +export function useAllHostClients(hostIds: string[]): Array<{ + hostId: string + client: RpcClient + state: ConnectionState +}> { + const ctx = useCtx() + // Stable key so we don't tear down on every render of the array. + const key = useMemo(() => [...hostIds].sort().join(','), [hostIds]) + const [tick, setTick] = useState(0) + + useEffect(() => { + if (hostIds.length === 0) return + for (const id of hostIds) ctx.acquire(id) + const unsubs: Array<() => void> = [] + for (const id of hostIds) { + unsubs.push(ctx.subscribeHostState(id, () => setTick((n) => n + 1))) + } + unsubs.push(ctx.subscribeAllHosts(() => setTick((n) => n + 1))) + return () => { + for (const u of unsubs) u() + for (const id of hostIds) ctx.release(id) + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [key]) + + return useMemo(() => { + const out: Array<{ hostId: string; client: RpcClient; state: ConnectionState }> = [] + for (const id of hostIds) { + const all = ctx.getAllClients().find((entry) => entry.hostId === id) + if (all) { + out.push({ hostId: id, client: all.client, state: ctx.getState(id) }) + } + } + return out + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [key, tick]) +} + +// Why: removeHost() in host-store.ts must close the live client, but +// host-store has no React-side handle. Expose a hook that lets callers +// close a host after removal. +export function useCloseHost(): (hostId: string) => void { + const ctx = useCtx() + return ctx.closeHost +} + +// Why: future-proof "Connection issues — try again" affordance. +export function useForceReconnect(): (hostId: string) => Promise { + const ctx = useCtx() + return ctx.forceReconnect +} + +// Why: lets the home screen feed already-loaded HostProfiles in so the +// provider can skip its own loadHosts() pass when it eventually opens +// each host — collapses two serial Keychain reads on cold-start into one. +export function usePrimeHosts(): (hosts: HostProfile[]) => void { + const ctx = useCtx() + return ctx.primeHosts +} + +// Why: lets the home/host-detail UI escalate "Reconnecting…" to a more +// alarming "Can't connect" once the rpc-client has cycled enough times to +// indicate something's actually wrong (wrong port, server down, network +// loss). Reads through the context so it stays in sync with the live +// rpc-client instance even after forceReconnect swaps the underlying +// client. +export function useReconnectAttempt(hostId: string | undefined): number { + const ctx = useCtx() + const [, force] = useState(0) + useEffect(() => { + if (!hostId) return + return ctx.subscribeHostState(hostId, () => force((n) => n + 1)) + }, [ctx, hostId]) + return hostId ? ctx.getReconnectAttempt(hostId) : 0 +} diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index bf1ba5711..119e10d7c 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -25,7 +25,27 @@ function tokenKey(hostId: string): string { return `${TOKEN_KEY_PREFIX}${hostId}` } +// Why: SecureStore reads on Android Keystore can take 50-200ms each, and +// loadHosts() is called from every screen mount + every useFocusEffect. +// Stack with N hosts and you get N*200ms blocking every navigation, which +// triggers connection-churn cycles in the home-screen useEffect. Cache +// per-hostId in memory; invalidate only on save/remove. The cache lives +// for the JS-runtime lifetime, which matches AsyncStorage semantics +// (cleared on app uninstall, persisted across foreground/background). +const tokenCache = new Map() +let inflightLoad: Promise | null = null + export async function loadHosts(): Promise { + // Why: deduplicate concurrent loadHosts() calls so multiple screens + // mounting simultaneously share one Keychain read pass. + if (inflightLoad) return inflightLoad + inflightLoad = doLoadHosts().finally(() => { + inflightLoad = null + }) + return inflightLoad +} + +async function doLoadHosts(): Promise { const raw = await AsyncStorage.getItem(STORAGE_KEY) if (!raw) return [] let parsed: unknown @@ -48,12 +68,17 @@ export async function loadHosts(): Promise { const stored = StoredHostProfileSchema.safeParse(item) if (!stored.success) continue - const token = await SecureStore.getItemAsync(tokenKey(stored.data.id), KEYCHAIN_OPTIONS) + let token = tokenCache.get(stored.data.id) if (!token) { - // Why: orphaned metadata with no matching keychain entry — most - // likely a stale record from a development install. Skip it - // rather than surface a half-broken host. - continue + const fetched = await SecureStore.getItemAsync(tokenKey(stored.data.id), KEYCHAIN_OPTIONS) + if (!fetched) { + // Why: orphaned metadata with no matching keychain entry — most + // likely a stale record from a development install. Skip it + // rather than surface a half-broken host. + continue + } + token = fetched + tokenCache.set(stored.data.id, token) } out.push({ ...stored.data, deviceToken: token }) } @@ -100,6 +125,7 @@ export async function saveHost(host: HostProfile): Promise { } await SecureStore.setItemAsync(tokenKey(stored.id), validated.deviceToken, KEYCHAIN_OPTIONS) await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(hosts)) + tokenCache.set(stored.id, validated.deviceToken) } export async function removeHost(hostId: string): Promise { @@ -107,6 +133,7 @@ export async function removeHost(hostId: string): Promise { const filtered = hosts.filter((h) => h.id !== hostId) await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(filtered)) await SecureStore.deleteItemAsync(tokenKey(hostId), KEYCHAIN_OPTIONS) + tokenCache.delete(hostId) } export async function renameHost(hostId: string, newName: string): Promise { diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 29041bdb4..9bbeb5ee3 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -25,6 +25,9 @@ export type RpcClient = { sendRequest: (method: string, params?: unknown) => Promise subscribe: (method: string, params: unknown, onData: StreamingListener) => () => void getState: () => ConnectionState + // Why: UI escalates "Reconnecting…" to "Can't connect" once attempts cross + // a threshold. 0 means never failed; counter is reset on successful open. + getReconnectAttempt: () => number onStateChange: (listener: (state: ConnectionState) => void) => () => void close: () => void } @@ -32,6 +35,13 @@ export type RpcClient = { const RECONNECT_DELAYS = [1000, 2000, 4000, 8000, 16000] const REQUEST_TIMEOUT_MS = 30_000 const HANDSHAKE_TIMEOUT_MS = 5_000 +// Why: belt-and-suspenders against React Native WebSocket implementations +// that occasionally never fire onerror/onclose for an unreachable host +// (observed when waking the device with stale DNS). Without this safety +// net the UI sat on 'Connecting…' forever and only a Metro reload +// recovered. Five seconds is a generous upper bound — a healthy LAN WS +// typically connects in <100ms. +const CONNECT_TIMEOUT_MS = 5_000 export function connect( endpoint: string, @@ -45,6 +55,7 @@ export function connect( let reconnectAttempt = 0 let reconnectTimer: ReturnType | null = null let handshakeTimer: ReturnType | null = null + let connectTimer: ReturnType | null = null let intentionallyClosed = false // Why: fresh ephemeral keypair per connection provides forward secrecy. @@ -96,7 +107,24 @@ export function connect( ws = new WebSocket(endpoint) + connectTimer = setTimeout(() => { + connectTimer = null + // Why: WS still stuck before 'open' — force-close so onclose fires + // and reconnect logic kicks in (rather than sitting on 'connecting' + // until the user reloads Metro). Safe even if the WS is mid-open; + // close() will trigger the 'closing' → 'closed' transitions. + try { + ws?.close() + } catch { + // ignore — onclose will still wire up reconnect + } + }, CONNECT_TIMEOUT_MS) + ws.onopen = () => { + if (connectTimer) { + clearTimeout(connectTimer) + connectTimer = null + } reconnectAttempt = 0 setState('handshaking') @@ -239,6 +267,10 @@ export function connect( clearTimeout(handshakeTimer) handshakeTimer = null } + if (connectTimer) { + clearTimeout(connectTimer) + connectTimer = null + } if (intentionallyClosed) { setState('disconnected') rejectAllPending('Connection closed') @@ -360,6 +392,10 @@ export function connect( return state }, + getReconnectAttempt(): number { + return reconnectAttempt + }, + onStateChange(listener: (state: ConnectionState) => void): () => void { stateListeners.add(listener) return () => stateListeners.delete(listener) @@ -375,6 +411,10 @@ export function connect( clearTimeout(handshakeTimer) handshakeTimer = null } + if (connectTimer) { + clearTimeout(connectTimer) + connectTimer = null + } if (ws) { ws.close() ws = null diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 581bc2ee5..d8dd34275 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -137,10 +137,17 @@ export default function TerminalPane({ } requestAnimationFrame(fitAffectedPanes) // Why: belt-and-suspenders — if safeFit's fitAddon.fit() threw or - // was a no-op due to stale dimensions, this fallback uses the - // restored cols/rows from the runtime to force the resize. If - // safeFit already succeeded, the terminal is already at the right - // dims and this is a harmless no-op. + // was a no-op due to stale dimensions, fall back to a direct + // resize. ONLY fire if xterm is still parked at the prior + // mobile-fit dims, meaning safeFit failed to move it. Previously + // we also fired when xterm had moved to *any* size other than + // the captured baseline, which clobbered safeFit's correct + // DOM-measured fit when the desktop pane geometry had changed + // since mobile-fit started (e.g. user closed a split or resized + // the window while the phone was active). In that scenario the + // event.cols/rows is the stale baseline from the moment + // mobile-fit started, not the current pane geometry — applying + // it would shrink the terminal back to e.g. half-width. setTimeout(() => { for (const paneId of paneIds) { const pane = manager.getPanes().find((p) => p.id === paneId) @@ -148,13 +155,12 @@ export default function TerminalPane({ continue } safeFit(pane) - // Fallback: if terminal is still at mobile dims, force resize - // using the restored dimensions from the runtime notification. - if ( - event.cols > 0 && - event.rows > 0 && - (pane.terminal.cols !== event.cols || pane.terminal.rows !== event.rows) - ) { + const stuckAtMobile = + event.priorCols != null && + event.priorRows != null && + pane.terminal.cols === event.priorCols && + pane.terminal.rows === event.priorRows + if (stuckAtMobile && event.cols > 0 && event.rows > 0) { pane.terminal.resize(event.cols, event.rows) } } diff --git a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts index 66a4cf4cf..69dee7720 100644 --- a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts +++ b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts @@ -202,7 +202,9 @@ describe('onOverrideChange', () => { ptyId: 'pty-1', mode: 'mobile-fit', cols: 49, - rows: 20 + rows: 20, + priorCols: null, + priorRows: null }) unsub() @@ -218,7 +220,28 @@ describe('onOverrideChange', () => { ptyId: 'pty-1', mode: 'desktop-fit', cols: 120, - rows: 40 + rows: 40, + priorCols: null, + priorRows: null + }) + + unsub() + }) + + it('passes prior mobile-fit dims to desktop-fit listeners', () => { + setFitOverride('pty-1', 'mobile-fit', 49, 20) + const listener = vi.fn() + const unsub = onOverrideChange(listener) + + setFitOverride('pty-1', 'desktop-fit', 120, 40) + + expect(listener).toHaveBeenCalledWith({ + ptyId: 'pty-1', + mode: 'desktop-fit', + cols: 120, + rows: 40, + priorCols: 49, + priorRows: 20 }) unsub() diff --git a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts index 7e5a781a0..e48906962 100644 --- a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts +++ b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts @@ -23,6 +23,13 @@ type OverrideChangeEvent = { mode: 'mobile-fit' | 'desktop-fit' cols: number rows: number + // Why: the dimensions the PTY was at *before* this event fired. For a + // desktop-fit transition this is the prior mobile-fit cols/rows so + // listeners can check whether xterm is still stuck at phone dims and + // needs the safety-net resize, vs. already moved on (e.g. user resized + // the desktop pane while mobile was active). + priorCols: number | null + priorRows: number | null } type OverrideChangeListener = (event: OverrideChangeEvent) => void const changeListeners = new Set() @@ -44,12 +51,20 @@ export function setFitOverride( cols: number, rows: number ): void { + const prior = overridesByPtyId.get(ptyId) ?? null if (mode === 'mobile-fit') { overridesByPtyId.set(ptyId, { mode, cols, rows }) } else { overridesByPtyId.delete(ptyId) } - notifyChange({ ptyId, mode, cols, rows }) + notifyChange({ + ptyId, + mode, + cols, + rows, + priorCols: prior?.cols ?? null, + priorRows: prior?.rows ?? null + }) } export function getPaneIdsForPty(ptyId: string): number[] {