From c6f0ac4040cae54bdb6ec12e4a57b063ae1ff453 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 20 Jul 2026 03:18:35 -0700 Subject: [PATCH] refactor(comments): slim verbose comments in mobile (#9547) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapse multi-line explanatory comment blocks into single-line "why" statements per AGENTS.md ("Document the Why, Briefly"): drop restatements of the code and mechanism narration; keep the non-obvious reason, external refs, and directives. Comments-only — verified no code changed via a Babel/esbuild comment-strip token-equality gate against origin/main; typecheck and oxlint clean. Area: mobile. 11 files changed, 339 insertions(+), 1137 deletions(-). Co-authored-by: Orca --- mobile/app/h/[hostId]/index.tsx | 155 ++---- .../app/h/[hostId]/session/[worktreeId].tsx | 514 +++++------------- mobile/app/index.tsx | 110 +--- .../src/notifications/mobile-notifications.ts | 73 +-- .../use-mobile-pr-sidebar-controller.ts | 80 +-- .../MobileSourceControlPanel.tsx | 67 +-- .../src/terminal/terminal-viewport-refit.ts | 75 +-- mobile/src/terminal/terminal-webview-html.ts | 16 +- mobile/src/transport/client-context.tsx | 117 +--- mobile/src/transport/host-store.ts | 77 +-- mobile/src/transport/rpc-client.ts | 192 ++----- 11 files changed, 339 insertions(+), 1137 deletions(-) diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index 442066ca9..126744a98 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -108,14 +108,9 @@ function isErrorVerdict(v: ConnectionVerdict): boolean { const REPO_METADATA_REFRESH_MS = 60_000 type HostScreenProps = { - // Why: when true, this worktree list is rendered as the persistent tablet - // sidebar by the host layout rather than as its own routed screen. That - // swaps the back button for a hide-sidebar control, drives data fetching - // from a plain mount effect (the sidebar is never the "focused" route), and - // opens sessions into the detail pane instead of pushing a new full screen. + // When true, rendered as the persistent tablet sidebar by the host layout, not as its own routed screen. embedded?: boolean - // Route params aren't in scope when rendered from the layout, so the caller - // passes hostId/action explicitly; falls back to the local route params. + // Route params aren't in scope when rendered from the layout, so the caller passes these explicitly. hostId?: string action?: string onHideSidebar?: () => void @@ -133,16 +128,12 @@ export function HostScreen({ const router = useRouter() const pathname = usePathname() const insets = useSafeAreaInsets() - // Why: cap and center the worktree list on wide/tablet canvases; on phones - // isWideLayout is false so the list stays edge-to-edge as before. When - // embedded as the sidebar the list already lives in a narrow pane, so the - // cap is skipped (see the SectionList contentContainerStyle below). + // Why: cap and center the list on wide/tablet canvases; on phones isWideLayout is false so it stays edge-to-edge. const { isWideLayout, contentMaxWidth } = useResponsiveLayout() const [initialCache] = useState(() => hostId ? (getCachedWorktrees(hostId) as Worktree[] | null) : null ) - // Why: shared client per host owned by RpcClientProvider. See - // docs/mobile-shared-client-per-host.md. + // 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 lastConnectedAt = useLastConnectedAt(hostId) @@ -156,9 +147,7 @@ export function HostScreen({ const forceReconnectHost = useForceReconnect() const [worktrees, setWorktrees] = useState(initialCache ?? []) const [worktreesLoaded, setWorktreesLoaded] = useState(initialCache != null) - // Why: opening a worktree activates it on the host, but the active-row - // highlight otherwise waits for the next worktree.ps poll to reflect it. - // Track the locally-opened worktree so the highlight moves instantly. + // Why: track the locally-opened worktree so the active-row highlight moves instantly instead of waiting for the next poll. const [optimisticActiveWorktreeId, setOptimisticActiveWorktreeId] = useState(null) // One tick drives every visible agent row's relative timestamp. const now = useNow(30_000) @@ -180,9 +169,7 @@ export function HostScreen({ const [workspaceStatuses, setWorkspaceStatuses] = useState( DEFAULT_MOBILE_WORKSPACE_STATUSES ) - // displayName → repo id, populated from repo.list. The filter model keys on - // repo ids (desktop's PersistedUIState), but the section headers/rows key on - // displayName, so we bridge the two here. + // displayName → repo id: filters key on repo id, but section headers/rows key on displayName, so bridge the two. const [repoIdsByName, setRepoIdsByName] = useState>(new Map()) const [showSortPicker, setShowSortPicker] = useState(false) const [showGroupPicker, setShowGroupPicker] = useState(false) @@ -201,9 +188,7 @@ export function HostScreen({ }, [router]) const [pinnedIds, setPinnedIds] = useState>(new Set()) const [collapsedGroups, setCollapsedGroups] = useState>(new Set()) - // Why: snapshot of the synced view settings so the focus-effect ui.get merge - // and the optimistic ui.set writes read the latest values without forcing the - // callbacks to re-create on every state change. + // Why: ref so the ui.get merge and ui.set writes read the latest values without re-creating callbacks on every state change. const viewStateRef = useRef({ groupMode: 'repo', sortMode: 'recent', @@ -226,8 +211,7 @@ export function HostScreen({ } }, [groupMode, sortMode, filters, collapsedGroups, workspaceStatuses]) - // Apply a MobileViewState (e.g. from a desktop ui.get) onto the individual - // states and the snapshot ref in one shot. + // Apply a MobileViewState onto the individual states and the snapshot ref in one shot. const applyViewState = useCallback((next: MobileViewState) => { viewStateRef.current = next setGroupMode(next.groupMode) @@ -241,8 +225,7 @@ export function HostScreen({ }) }, []) - // Optimistically apply a partial change locally, then push the full mapped - // settings to the desktop's shared store via ui.set so both apps stay in sync. + // Apply the change locally, then push full settings to the desktop's shared store (ui.set) so both apps stay in sync. const persistViewSettings = useCallback( (patch: Partial) => { const next: MobileViewState = { ...viewStateRef.current, ...patch } @@ -275,8 +258,7 @@ export function HostScreen({ }, []) const resolvedRouteActionState = resolveHostRouteActionState(routeActionState, action) - // Why: `action=newWorktree` is a route-derived open edge. Resolve it before - // commit, but don't reopen after the user closes while the same URL remains. + // Why: resolve `action=newWorktree` before commit, but don't reopen after the user closes while the URL persists. if (resolvedRouteActionState !== routeActionState) { setRouteActionState(resolvedRouteActionState) } @@ -285,8 +267,7 @@ export function HostScreen({ setRouteActionState((current) => setHostRouteNewWorktreeVisible(current, visible)) }, []) - // Load persisted pins from the local cache. View settings are no longer - // stored locally — they sync from the desktop's shared store via ui.get. + // Load persisted pins from local cache; view settings are no longer local (they sync via ui.get). useEffect(() => { if (!hostId) { return @@ -304,9 +285,7 @@ export function HostScreen({ } }, [hostId]) - // Read the desktop's shared view settings (PersistedUIState) and merge them - // onto local state. Runs on connect and on screen focus so changes made on - // desktop appear on the phone. + // Merge the desktop's shared view settings (PersistedUIState) onto local state so desktop changes appear here. const syncViewSettingsFromDesktop = useCallback(async () => { if (!client || connState !== 'connected') { return @@ -328,9 +307,7 @@ export function HostScreen({ } }, [client, connState, hostId, applyViewState]) - // 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. + // Why: mirror client into a ref so imperative call sites read it without re-subscribing. useEffect(() => { clientRef.current = client }, [client]) @@ -342,9 +319,7 @@ export function HostScreen({ setRepoColorsByName(new Map()) setRepoIconsByName(new Map()) repoMetadataFetchedAtRef.current = 0 - // 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. + // Why: useState initializer runs only on first mount, so re-seed the cache when Expo Router reuses this screen for a new hostId. const freshCache = hostId ? (getCachedWorktrees(hostId) as Worktree[] | null) : null if (freshCache) { setWorktrees(freshCache) @@ -432,8 +407,7 @@ export function HostScreen({ if (!options.allowDuringModal && newWorktreeModalVisibleRef.current) { return } - // The embedded sidebar polls for the whole split-view session; keep slow - // remote hosts from stacking overlapping expensive list requests. + // Why: prevent slow remote hosts from stacking overlapping worktree.ps requests during polling. if (fetchWorktreesInFlightRef.current) { return } @@ -442,8 +416,7 @@ export function HostScreen({ const requestHostId = hostId try { - // Why: worktree.ps defaults to 200 and silently truncates; match the - // desktop's high cap so large hosts don't drop workspaces on mobile. + // Why: worktree.ps silently truncates at 200; use a high cap so large hosts don't drop workspaces. const response = await requestClient.sendRequest('worktree.ps', { limit: 10000 }) if (clientRef.current !== requestClient || hostId !== requestHostId) { return @@ -453,9 +426,7 @@ export function HostScreen({ } if (response.ok) { const result = (response as RpcSuccess).result as { worktrees: Worktree[] } - // Why: large hosts can return identical worktree.ps snapshots every - // poll. Preserving the existing array keeps SectionList/sort rebuilds - // off the JS tap path unless something actually changed. + // Why: reuse the existing array on identical snapshots to keep SectionList/sort rebuilds off the tap path. setWorktrees((current) => areWorktreeListsEqual(current, result.worktrees) ? current : result.worktrees ) @@ -463,25 +434,18 @@ export function HostScreen({ areWorktreeListsEqual(current, result.worktrees) ? current : result.worktrees ) setWorktreesLoaded(true) - // Why (#8498): the host detail screen seeds its list from the - // home-written cache, so a partial home fetch could poison it until a - // focus poll corrected it. Write the confirmed snapshot back through - // the same cache so a reconnect refetch (or a remount) can't serve a - // stale worktree list. + // Why (#8498): overwrite the home-written cache with the confirmed snapshot so a reconnect/remount can't serve a stale list. if (hostId) { setCachedWorktrees(hostId, result.worktrees) } - // Drop the optimistic active override once the host confirms it (the - // activate RPC has landed and worktree.ps now reports it active), so we - // stop overriding and respect any later desktop-driven change. + // Drop the optimistic active override once the host reports it active, so later desktop changes win. setOptimisticActiveWorktreeId((pending) => pending && result.worktrees.some((w) => w.worktreeId === pending && w.isActive) ? null : pending ) - // Clear optimistic sleep overrides once the server confirms the - // worktree is actually inactive (liveTerminalCount dropped to 0). + // Clear optimistic sleep overrides once the server confirms inactive (liveTerminalCount === 0). setSleptIds((prev) => { if (prev.size === 0) { return prev @@ -496,8 +460,7 @@ export function HostScreen({ return still.size === prev.size ? prev : still }) - // Sync local pin state from server so desktop-initiated pins/unpins - // are reflected without relying on stale AsyncStorage. + // Sync pin state from server so desktop-initiated pins reflect without relying on stale AsyncStorage. const serverPinned = new Set( result.worktrees.filter((w) => w.isPinned).map((w) => w.worktreeId) ) @@ -520,17 +483,10 @@ export function HostScreen({ [client, connState, hostId] ) - // Why: read desktop's protocol version from status.get on every connect - // and re-evaluate compatibility. If the desktop declares this mobile - // build too old (or vice versa via the local minimum), the host detail - // screen swaps to a hard-block screen instead of the worktree list. - // Today's compat constants are wide-open so this never blocks; the wire - // format is in place to flip a switch in a future release. + // Why: re-evaluate protocol compat on connect; today's constants are wide-open so this never blocks yet. useEffect(() => { if (connState !== 'connected' || !client) { - // Why: drop the prior host's capabilities while disconnected/switching so - // a capability-gated action (e.g. Agent Session History) can't linger for - // a host that doesn't support it. + // Why: drop capabilities while disconnected/switching so a capability-gated action can't linger for a new host. setHostCapabilities([]) return } @@ -556,8 +512,7 @@ export function HostScreen({ }) setCompatVerdict(verdict) if (verdict.kind === 'blocked') { - // Why: deterministic breadcrumb so support can confirm a block - // actually fired (vs a render bug). No PII — just version ints. + // Why: support breadcrumb to confirm a block fired vs a render bug; no PII, just version ints. console.warn('[protocol-compat] blocked', { reason: verdict.reason, desktopVersion: verdict.desktopVersion, @@ -566,8 +521,7 @@ export function HostScreen({ }) } } catch { - // Why: rare path — sendRequest can throw on transport tear-down. - // Treat as transient; verdict stays at previous value. + // Why: sendRequest can throw on transport tear-down; treat as transient, keep the prior verdict. } })() return () => { @@ -577,29 +531,22 @@ export function HostScreen({ useFocusEffect( useCallback(() => { - // Why: opening the host is a strong user signal — reset a backed-off or - // trickling reconnect loop (and probe a possibly half-open socket) - // immediately instead of waiting out its timer. Deps stay empty so this - // fires per focus transition, not per connection-state change; nudging - // on every reconnecting↔connecting flip would defeat the backoff. + // Why: focus nudges reconnect and probes a possibly half-open socket; empty deps fire per focus, not per state flip (which defeats backoff). clientRef.current?.notifyForeground() }, []) ) useFocusEffect( useCallback(() => { - // The embedded sidebar drives its own polling below; focus never fires - // for it since it isn't a routed screen. + // The embedded sidebar isn't a routed screen (focus never fires); it polls via the mount effect below. if (embedded || connState !== 'connected') { return } void fetchWorktrees() void fetchRepoMetadata() - // Pull desktop's shared view settings on focus so desktop-side changes - // show up here without a manual refresh. + // Pull desktop's shared view settings on focus so desktop changes show up without a manual refresh. void syncViewSettingsFromDesktop() - // Why: React Navigation keeps previous stack screens mounted; only - // poll the host list while this route is visible. + // Why: React Navigation keeps prior screens mounted; only poll while this route is visible. const interval = setInterval(() => { void fetchWorktrees() void fetchRepoMetadata() @@ -608,9 +555,7 @@ export function HostScreen({ }, [embedded, connState, fetchWorktrees, fetchRepoMetadata, syncViewSettingsFromDesktop]) ) - // Why: as the persistent tablet sidebar this list is never the focused - // route, so useFocusEffect won't fetch/poll. Mirror that behavior from a - // plain mount effect while connected instead. + // Why: the embedded sidebar is never the focused route, so useFocusEffect never polls; mirror it from a mount effect. useEffect(() => { if (!embedded || connState !== 'connected') { return @@ -625,10 +570,7 @@ export function HostScreen({ return () => clearInterval(interval) }, [embedded, connState, fetchWorktrees, fetchRepoMetadata, syncViewSettingsFromDesktop]) - // Why (#8498): reconnect refetch + manual pull-to-refresh, extracted to - // useWorktreeResync so this screen stays under its max-lines budget. The - // steady-state focus/embedded polls don't cover the transition INTO - // 'connected' after a background/sleep, which is when the cache is stalest. + // Why (#8498): steady-state polls miss the transition INTO 'connected' after background/sleep, when the cache is stalest. const { refreshing, onRefresh } = useWorktreeResync({ client, connState, @@ -720,8 +662,7 @@ export function HostScreen({ await removeHostAndCloseClient(hostId, closeHostClient) leaveHost() } catch { - // Why: metadata commit can fail while the host is still paired; keep the - // screen mounted and re-open confirm (ConfirmModal closes on confirm). + // Why: removal can fail while still paired; re-open confirm (ConfirmModal closes on confirm). setConfirmRemoveHost(true) Alert.alert('Could not remove host', 'Please try again.') } @@ -748,11 +689,9 @@ export function HostScreen({ const openWorktreeSession = useCallback( (item: Worktree) => { - // Highlight the row immediately; the next worktree.ps poll confirms it. setOptimisticActiveWorktreeId(item.worktreeId) if (client && connState === 'connected') { - // Why: opening a mobile session should hydrate host-owned tabs without - // pulling other paired clients, especially desktop, into this worktree. + // Why: notifyClients:false hydrates host tabs without pulling desktop into this worktree. void client .sendRequest('worktree.activate', { worktree: `id:${item.worktreeId}`, @@ -831,8 +770,7 @@ export function HostScreen({ const slept = sleptIds.has(w.worktreeId) ? { liveTerminalCount: 0, hasAttachedPty: false, status: 'inactive' as const } : null - // Force the just-opened worktree active (and the rest inactive) until the - // next poll confirms it, so the highlight doesn't lag the navigation. + // Force the just-opened worktree active until the next poll confirms it, so the highlight doesn't lag. const active = optimisticActiveWorktreeId !== null ? { isActive: w.worktreeId === optimisticActiveWorktreeId } @@ -912,12 +850,7 @@ export function HostScreen({ {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. + // Why: auth-failed has its own banner, so suppress the Reconnect button for that verdict. const verdict = headerVerdict const isError = isErrorVerdict(verdict) const showReconnectButton = isError && hostId && verdict.kind !== 'auth-failed' @@ -1166,8 +1099,7 @@ export function HostScreen({ onChangeText={setSearch} placeholder="Search worktrees…" autoFocus - // Why: new key each open remounts focus effect if the field stays mounted - // across rapid toggles; pairs with delayed focus so the keyboard appears. + // Why: new key per open remounts the focus effect across rapid toggles so the keyboard reappears. focusKey={showSearch} accessibilityLabel="Search worktrees" /> @@ -1206,13 +1138,10 @@ export function HostScreen({ keyboardShouldPersistTaps="handled" keyboardDismissMode="on-drag" onScrollToIndexFailed={onScrollToIndexFailed} - // Why: edge-to-edge — the list scrolls under the system nav bar - // while reserving insets.bottom keeps the last worktree row reachable - // above the Samsung 3-button nav / iOS home indicator. + // Why: edge-to-edge under the system nav bar; insets.bottom keeps the last row above it. contentContainerStyle={[ styles.list, - // Phone shows a floating "+" button bottom-right; reserve room so the - // last row stays tappable above it. Embedded sidebars keep the toolbar +. + // Reserve room so the last row stays tappable above the phone's floating "+" (embedded uses the toolbar +). { paddingBottom: (embedded ? spacing.lg : FAB_SIZE + spacing.xl) + insets.bottom }, isWideLayout && !embedded && { maxWidth: contentMaxWidth, width: '100%', alignSelf: 'center' } @@ -1252,8 +1181,7 @@ export function HostScreen({ ) }} ItemSeparatorComponent={ListSeparator} - // Why (#8498): manual pull-to-refresh forces a fresh worktree - // snapshot after a reconnect or whenever the cache looks stale. + // Why (#8498): manual pull-to-refresh forces a fresh snapshot after a stale-cache reconnect. refreshControl={ void keyboardLift: number }) { - // The editor lives in a WebView; native Keyboard events under-report its - // covered area, so prefer the inset measured inside the WebView when larger. + // Native Keyboard events under-report the WebView editor's covered area, so prefer the larger WebView-measured inset. const [webviewKeyboardInset, setWebviewKeyboardInset] = useState(0) const effectiveKeyboardLift = Math.max(keyboardLift, webviewKeyboardInset) if (!doc || doc.status === 'loading') { @@ -333,8 +332,7 @@ function MarkdownReader({ pointerEvents="box-none" style={[ styles.markdownFloatingBar, - // Why: the editor focus lives inside a WebView, so keep native - // Save/Discard controls lifted instead of resizing that surface. + // Why: editor focus lives in a WebView, so lift native Save/Discard controls instead of resizing it. { bottom: resolveMarkdownFloatingActionsBottom({ keyboardLift: effectiveKeyboardLift, @@ -423,8 +421,7 @@ function DiffLineRow({ const commentLine = line.newLineNumber const isCommenting = commentLine !== undefined && activeCommentLine === commentLine const canComment = commentLine !== undefined - // Why: review notes are anchored to the modified side, so the single mobile - // gutter should show the same line number the note will reference. + // Why: review notes anchor to the modified side, so show that line number in the single mobile gutter. const gutterLineNumber = line.newLineNumber ?? line.oldLineNumber ?? '' return ( @@ -647,8 +644,7 @@ function FileReader({ return undefined } - // Why: highlighting can create many nested Text nodes; defer it one tick so - // large files show immediately as plain text before colors are applied. + // Why: defer highlighting one tick so large files show as plain text immediately before colors are applied. const timer = setTimeout(() => { // file + html share the syntax-segment source view (html's "Source" toggle). if (doc.kind === 'file' || doc.kind === 'html') { @@ -822,8 +818,7 @@ export default function SessionScreen() { const isFolderWorkspaceRoute = worktreeId.startsWith('folder:') // Synthetic ids have no repo scope. const router = useRouter() const insets = useSafeAreaInsets() - // Why: shared client per host owned by RpcClientProvider. See - // docs/mobile-shared-client-per-host.md. + // Why: shared client per host owned by RpcClientProvider (docs/mobile-shared-client-per-host.md). const { client, state: connState } = useHostClient(hostId) const reconnectAttempts = useReconnectAttempt(hostId) const lastConnectedAt = useLastConnectedAt(hostId) @@ -834,8 +829,7 @@ export default function SessionScreen() { routeName: routeWorktreeName, worktreeId }) - // Master-detail host state (U5/KTD2): on wide layouts a tapped panel docks beside the - // session content; on narrow it stays null and the icons push full-screen routes. + // Master-detail state: wide layouts dock a tapped panel beside the session; narrow keeps it null and pushes full-screen routes. const { isWideLayout } = useResponsiveLayout() const [activePanel, setActivePanel] = useState(null) const [sessionContentRowWidth, setSessionContentRowWidth] = useState(0) @@ -844,17 +838,13 @@ export default function SessionScreen() { availableWidth: sessionContentRowWidth, dockWidth: HOST_DOCK_MIN_WIDTH }) - // Why: docking needs enough measured row width. If rotation/split-screen makes - // the session row too narrow while a panel is docked, clear activePanel so the - // icon state and live mounted panel do not survive into overlay/push mode. + // Why: if rotation/split-screen makes the docked row too narrow, clear activePanel so it doesn't survive into overlay/push mode. useEffect(() => { if (!canDockPanel && activePanel !== null) { setActivePanel(null) } }, [canDockPanel, activePanel]) - // Session-level GitHub remote probe gates the PR dock icon so non-GitHub - // providers do not open the hosted-review surface. Branch/head/status for the - // hub are loaded inside MobileSourceControlPanel — skip the unused identity RPCs. + // GitHub remote probe gates the PR dock icon so non-GitHub providers can't open the hosted-review surface; skip the unused identity RPCs. const { isGithubRepo: prIsGithubRepo, repoLoaded: prRepoContextLoaded } = useMobilePrBranchContext({ client, @@ -872,23 +862,15 @@ export default function SessionScreen() { const terminalsRef = useRef([]) const [sessionTabs, setSessionTabs] = useState([]) const sessionTabsRef = useRef([]) - // Why: subscription, 2s polling, and post-mutation refetch race to apply tab - // snapshots. Track the last applied (publicationEpoch, snapshotVersion) so a - // late-arriving older snapshot from the same publisher can't overwrite (and - // resurrect closed tabs in) a newer one. See session-tab-snapshot-gate. + // Why: track the last applied (epoch, version) so a late older snapshot can't overwrite a newer one and resurrect closed tabs (session-tab-snapshot-gate). const appliedSnapshotMarkerRef = useRef({ epoch: null, version: -1 }) - // Why: after an optimistic local close, suppress the tab until the publisher - // confirms its absence, so an in-flight snapshot generated before the close - // propagated (and thus newer by version) can't flash the tab back. Maps tab id - // to an expiry timestamp so a failed host-side close can't hide a tab forever. + // Why: after an optimistic close, suppress the tab (with expiry) until the publisher confirms, so an in-flight snapshot can't flash it back. const closedTabTombstonesRef = useRef>(new Map()) const [terminalsLoaded, setTerminalsLoaded] = useState(false) const [input, setInput] = useState('') - // Why: baseline terminal zoom, reloaded on focus so a Settings → Terminal change - // applies in place (the terminal panes stay mounted). + // Why: baseline terminal zoom reloaded on focus so a Settings → Terminal change applies in place (panes stay mounted). const [terminalTextScale, setTerminalTextScale] = useState(1) - // Why: local opt-in for keyboard autocomplete/autocorrect on the terminal - // command bar; reloaded on focus so a Settings → Terminal toggle takes effect on return. + // Why: terminal command-bar autocomplete opt-in, reloaded on focus so a Settings → Terminal toggle takes effect on return. const [autocompleteEnabled, setAutocompleteEnabled] = useState(false) const [terminalLinkOpenMode, setTerminalLinkOpenMode] = useState('orca-browser') @@ -904,8 +886,7 @@ export default function SessionScreen() { const [activeHandle, setActiveHandle] = useState(null) const [activeSessionTabId, setActiveSessionTabId] = useState(null) const activeSessionTabIdRef = useRef(null) - // Auto-scroll the tab strip so the active tab (synced from desktop on - // worktree entry) is revealed without a manual scroll. + // Auto-scroll the tab strip so the desktop-synced active tab is revealed without a manual scroll. const tabStripRef = useRef(null) const tabStripOffsetRef = useRef(0) const tabStripViewportWidthRef = useRef(0) @@ -920,9 +901,7 @@ export default function SessionScreen() { const [pendingDiffNotesDelivery, setPendingDiffNotesDelivery] = useState(null) const [creating, setCreating] = useState(false) - // Why: React state isn't a synchronous lock — a fast double-tap can fire two - // creates before `creating` re-renders. This ref blocks the second one in the - // same tick (server idempotency only dedupes identical clientMutationIds). + // Why: React state isn't a synchronous lock; this ref blocks a double-tap's second create in the same tick before `creating` re-renders. const creatingTerminalRef = useRef(false) const [creatingBrowser, setCreatingBrowser] = useState(false) const [creatingMarkdown, setCreatingMarkdown] = useState(false) @@ -966,14 +945,9 @@ export default function SessionScreen() { () => getVisibleTerminalAccessoryKeys(visibleBuiltInIds), [visibleBuiltInIds] ) - // Why: in Expo SDK 55 edge-to-edge mode the OS does NOT resize the window when - // the IME opens — the keyboard draws on top of the app. We track the keyboard - // height ourselves and translate the input/accessory area above the IME without - // changing the terminal frame height, so keyboard open/close does not resize - // the desktop PTY. + // Why: Expo SDK 55 edge-to-edge doesn't resize the window on IME open, so track keyboard height ourselves and lift the input without resizing the desktop PTY. const [keyboardHeight, setKeyboardHeight] = useState(0) - // Why: server-authoritative display mode per terminal. The runtime is the - // single source of truth — this state is populated from subscribe responses. + // Why: server-authoritative display mode per terminal, populated from subscribe responses. const [terminalModes, setTerminalModes] = useState>(new Map()) const [terminalKeyboardMetrics, setTerminalKeyboardMetrics] = useState< Map @@ -981,15 +955,13 @@ export default function SessionScreen() { const [selectModeActive, setSelectModeActive] = useState(false) const [canPaste, setCanPaste] = useState(false) const [showDictationSetup, setShowDictationSetup] = useState(false) - // 'hold' makes the mic press-and-hold; 'toggle' makes it tap-to-start/stop. - // Mirrors Settings ▸ Voice ▸ Dictation Mode so the button matches the setting. + // 'hold' = press-and-hold mic, 'toggle' = tap-to-start/stop; mirrors Settings ▸ Voice ▸ Dictation Mode. const [dictationMode, setDictationMode] = useState<'toggle' | 'hold'>('toggle') const [toastMessage, setToastMessage] = useState(null) const toastOpacityRef = useRef(new Animated.Value(0)) const toastHideTimerRef = useRef | null>(null) const toastSeqRef = useRef(0) - // Why: WebView pushes terminal modes (bracketed-paste, alt-screen) on every - // change so paste reads a synchronous snapshot — no round-trip required. + // Why: WebView pushes terminal modes on every change so paste reads a synchronous snapshot — no round-trip. const ptyModesRef = useRef>(new Map()) const terminalGestureInputBucketsRef = useRef>(new Map()) const terminalGestureInputQueuesRef = useRef>(new Map()) @@ -997,13 +969,11 @@ export default function SessionScreen() { const terminalCwdRef = useRef>(new Map()) const initialModesSeenRef = useRef>(new Set()) const deviceTokenRef = useRef(null) - // Why: state (not a ref) — the connection verdict needs a re-render once - // the endpoint loads so the Tailscale hint can appear. + // Why: state (not a ref) so the connection verdict re-renders when the endpoint loads and the Tailscale hint can appear. const [hostEndpoint, setHostEndpoint] = useState(null) const clientRef = useRef(null) const connStateRef = useRef(connState) - // Why: measured once from TerminalWebView on mount, then passed with every - // subscribe call so the server can auto-fit the PTY to phone dimensions. + // Why: measured once on mount, then passed with every subscribe so the server can auto-fit the PTY to phone dims. const viewportRef = useRef<{ cols: number; rows: number } | null>(null) const viewportMeasuredRef = useRef(false) const terminalRefs = useRef>(new Map()) @@ -1023,53 +993,31 @@ export default function SessionScreen() { const subscribingHandlesRef = useRef>(new Set()) const initializedHandlesRef = useRef>(new Set()) const terminalDiagnosticsRef = useRef(new MobileTerminalDiagnostics()) - // Why: WebViews load xterm.js from CDN asynchronously. Hidden WebViews - // (opacity:0) may have delayed JS execution on iOS. We must not subscribe - // until the WebView has fired web-ready, otherwise init() messages queue - // and may not render reliably. + // Why: don't subscribe until the WebView fires web-ready — iOS may defer JS in hidden WebViews and init() messages would queue unrendered. const webReadyHandlesRef = useRef>(new Set()) const activeHandleRef = useRef(null) const activeSessionTabTypeRef = useRef(null) const pendingActiveSessionTabIdRef = useRef(null) const pendingActiveTerminalHandleRef = useRef(null) - // Why: a browser tab opened from a terminal-tapped HTML must be focused as an - // Orca session tab (bridge auto-activate only flags the live webContents, not - // the app-level active tab). We remember the page id and, once its session tab - // syncs, activate it through the normal switchSessionTab path (which also makes - // switching back to the terminal work). A ref breaks the callback dep cycle. + // Why: remember the page id to activate its session tab once it syncs (bridge auto-activate flags only webContents, not the app-level active tab). const pendingBrowserFocusPageIdRef = useRef(null) const switchSessionTabRef = useRef<((tab: MobileSessionTab) => void) | null>(null) const pendingTerminalActivationAttemptRef = useRef(null) - // Why: handleTerminalOpenUrl is memoized on terminalLinkOpenMode, but - // handleCreateBrowser is a per-render closure that captures the live `client`. - // A terminal URL tap must run the CURRENT closure (the memoized one can hold a - // render where client was still null/connecting, silently no-opping the - // in-app-browser open). Route through a ref kept current every render. + // Why: route the terminal URL tap through a ref so it runs the current handleCreateBrowser closure (the memoized one may hold a null-client render). const handleCreateBrowserRef = useRef<((rawUrl?: string) => Promise) | null>(null) const initialEmptySessionAutoCreateRef = useRef(null) const markdownSaveSeqRef = useRef>(new Map()) const markdownSaveInFlightRef = useRef>(new Set()) const subscribeSeqRef = useRef>(new Map()) - // Why: post-RPC refresh timers capture this screen and must not survive - // route reuse or unmount. + // Why: post-RPC refresh timers capture this screen and must not survive route reuse or unmount. const delayedActionTimersRef = useRef>>(new Set()) - // Why: server-side layout state machine emits a monotonic seq on every - // applyLayout. Track the highest seq we've observed per handle and drop - // any scrollback/resized event with a strictly older seq — these are - // late-arriving events from a superseded layout (e.g. phone-fit dims - // landing after the user toggled to desktop). Drops below `>20`-window - // gap reset (treat as a fresh subscription, e.g. server restart). + // Why: highest applyLayout seq seen per handle; drop older scrollback/resized as stale, but a >20 gap resets (fresh subscription/server restart). const layoutSeqRef = useRef>(new Map()) const sendingRef = useRef(false) - // Why: tracks the pixel height of the terminal frame so measureFitDimensions - // can use the exact container height instead of relying on window.innerHeight, - // which can overstate the visible area due to layout timing. + // Why: exact terminal-frame height for measureFitDimensions; window.innerHeight can overstate the visible area. const terminalFrameHeightRef = useRef(0) - // Why: the terminal frame's width changes when EITHER sidebar is resized (the - // left worktree sidebar shrinks the detail pane; the right dock takes a slice of - // the row) without any window-dim change. Tracking the measured width lets the - // refit hook re-fit the PTY on those resizes — see terminal-viewport-refit.ts. + // Why: sidebar resizes change the terminal frame width without a window-dim change; track it so the refit hook re-fits (see terminal-viewport-refit.ts). const [terminalFrameWidth, setTerminalFrameWidth] = useState(0) const activeSessionTab = sessionTabs.find((tab) => tab.id === activeSessionTabId) ?? null const { @@ -1098,9 +1046,7 @@ export default function SessionScreen() { activeSessionTab?.type !== 'browser' const liveInputEnabled = activeHandle ? liveInputTerminalHandles.has(activeHandle) : false const [browserScreencastSupported, setBrowserScreencastSupported] = useState(null) - // Why: hosts without aiVault.v1 reject aiVault.listSessions, so the header - // entry stays hidden there (mirrors the gated host-list action) instead of - // opening a dead-end "update this host" panel. + // Why: hosts without aiVault.v1 reject listSessions, so hide the header entry instead of a dead-end "update this host" panel. const [agentSessionHistorySupported, setAgentSessionHistorySupported] = useState( null ) @@ -1109,8 +1055,7 @@ export default function SessionScreen() { // the capability probe resolves after the callbacks are created. const browserScreencastSupportedRef = useRef(browserScreencastSupported) browserScreencastSupportedRef.current = browserScreencastSupported - // Why: terminal gesture/input callbacks are intentionally stable and - // imperative; keep their refs current before commit instead of one effect later. + // Why: terminal gesture/input callbacks are stable/imperative, so keep their refs current before commit, not in a later effect. clientRef.current = client connStateRef.current = connState activeSessionTabTypeRef.current = activeSessionTab?.type ?? null @@ -1121,8 +1066,7 @@ export default function SessionScreen() { createWarningState, initialCreateWarning ) - // Why: Expo can reuse this screen for a new route. Reconcile before paint - // so a dismissed old creation warning never flashes for the next session. + // Why: Expo can reuse this screen for a new route; reconcile before paint so a dismissed old warning doesn't flash. if (reconciledCreateWarningState !== createWarningState) { setCreateWarningState(reconciledCreateWarningState) } @@ -1214,8 +1158,7 @@ export default function SessionScreen() { client, enabled: canSend, onTranscript: (text) => { - // Why: dictation belongs to the visible composer. Native chat consumes it - // locally; terminal mode retains the live-input routing and flush contract. + // Why: dictation belongs to the visible composer — native chat consumes it locally, terminal mode keeps live-input routing. if (showNativeChatRef.current) { nativeChatController.setChatComposerText((current) => appendBufferedDictation(current, text) @@ -1223,9 +1166,7 @@ export default function SessionScreen() { showToast('Dictation inserted') return } - // Live mode inserts the transcript straight into its originating PTY as - // text (no Return — the user sends it themselves), matching live keystroke - // semantics; buffered mode keeps appending to the command field. + // Live mode inserts the transcript into its PTY as text (no Return); buffered mode appends to the command field. const routeContext = dictationRouteContextRef.current dictationRouteContextRef.current = null const route = routeDictationTranscript( @@ -1254,8 +1195,7 @@ export default function SessionScreen() { }, onError: (err) => { dictationRouteContextRef.current = null - // Dictation isn't set up on the desktop yet → open the setup sheet so the - // user can download a model + enable it from here, instead of a dead-end toast. + // Dictation not set up on desktop → open the setup sheet instead of a dead-end toast. if (isDictationSetupRequiredError(err.message)) { setShowDictationSetup(true) return @@ -1325,8 +1265,7 @@ export default function SessionScreen() { } }, [client]) - // Re-read on focus so a Dictation Mode change made in Settings ▸ Voice is - // reflected when the user returns to the session. + // Re-read on focus so a Settings ▸ Voice dictation-mode change is reflected on return. useFocusEffect( useCallback(() => { void refreshDictationMode() @@ -1348,9 +1287,7 @@ export default function SessionScreen() { subscribingHandlesRef.current.delete(handle) terminalDiagnosticsRef.current.terminalUnsubscribed(handle) subscribeSeqRef.current.set(handle, (subscribeSeqRef.current.get(handle) ?? 0) + 1) - // Why: a fresh subscription will land on a new server-side state machine - // run (or the same one with a higher seq); reset the high-water mark so - // the first scrollback isn't accidentally dropped as stale. + // Why: reset the high-water mark so a fresh subscription's first scrollback isn't dropped as stale. layoutSeqRef.current.delete(handle) clearNativeChatInputLease(handle) }, @@ -1376,9 +1313,7 @@ export default function SessionScreen() { } }, [clearNativeChatInputLease]) - // Why: measures the phone viewport once from the first available TerminalWebView. - // The viewport dims are passed with every subscribe call so the server can - // auto-fit the PTY without a separate RPC round-trip. + // Why: measure the phone viewport once from the first TerminalWebView; dims ride every subscribe so the server auto-fits without a separate RPC. const measureViewportOnce = useCallback( async (handle: string) => { if (viewportMeasuredRef.current) { @@ -1418,8 +1353,7 @@ export default function SessionScreen() { activeHandleRef.current, handle ) - // Why: a native-chat-covered terminal subscribes as the input-floor lease - // without a mounted xterm webview, so only gate on the webview when NOT covered. + // Why: a native-chat-covered terminal has no mounted webview, so only gate on the webview when not covered. if (!covered) { if (!getTerminalRef(handle)) { logSkippedGate('no-webview-ref') @@ -1436,10 +1370,7 @@ export default function SessionScreen() { subscribeSeqRef.current.set(handle, seq) diagnostics.streamArmed(handle, seq, viewportRef.current) - // Why: server handles auto-fit on subscribe — no terminal.focus call needed. - // The viewport is embedded in the subscribe params so the server resizes - // the PTY before serializing scrollback. This eliminates the focus→safeFit - // race and the measure→resize→resubscribe pipeline. + // Why: viewport is embedded in the subscribe params so the server auto-fits before serializing scrollback (no focus→safeFit race). const unsub = subscribeMobileTerminalSafely( client, { @@ -1462,8 +1393,7 @@ export default function SessionScreen() { markNativeChatInputLeaseReady(handle) return } - // Why: retain the subscription as the mobile input-floor lease, but - // do not mutate covered xterm state; return-to-terminal resubscribes. + // Why: keep the subscription as the input-floor lease but don't mutate covered xterm state; return-to-terminal resubscribes. if ( nativeChatTerminalStream.isTerminalCoveredByNativeChat( showNativeChatRef.current, @@ -1473,15 +1403,7 @@ export default function SessionScreen() { ) { return } - // Why: stale-event filter. Server-side state machine bumps a - // monotonic seq on every applyLayout. Drop `resized` events - // whose seq is strictly older than what we've already observed - // for this handle — they're late-arriving from a superseded - // layout. `scrollback` is the response to a fresh subscribe, - // so it always resets the high-water mark regardless of seq - // (post-WS-reconnect or post-resubscribe the server may emit - // scrollback at a seq lower than what we'd seen pre-reconnect; - // dropping it would leave the user with a blank terminal). + // Why: drop `resized` events older than the seen seq (superseded layout); scrollback always resets the mark, else reconnect blanks the terminal. const eventSeq = typeof data.seq === 'number' ? data.seq : null if (eventSeq != null && data.type === 'resized') { const last = layoutSeqRef.current.get(handle) @@ -1517,11 +1439,7 @@ export default function SessionScreen() { : '' const oscLinks = isTerminalOscLinkRanges(data.oscLinks) ? data.oscLinks : undefined const ref = getTerminalRef(handle) - // Why: previously we set `initializedHandlesRef` even when the - // WebView wasn't mounted yet (ref=null). The init message went - // nowhere, but the flag stayed true, so any subsequent scrollback - // for THIS handle was silently dropped → blank terminal. Only - // mark initialized if init() actually reached the WebView. + // Why: only mark initialized once init() reaches the WebView, else later scrollback is dropped and the terminal stays blank. if (!ref) { console.log('[fit][session] scrollback DROPPED — no terminal ref', { handle: handle.slice(-8), @@ -1537,21 +1455,9 @@ export default function SessionScreen() { new Map(prev).set(handle, data.displayMode as MobileDisplayMode) ) } - // Why: belt-and-suspenders cold-start fit. The applyFitScale - // queued by init() runs after writes drain, but on cold start - // xterm's scrollWidth can still be transient when it commits. - // Re-fire after a short delay so it runs against a settled DOM. - // Mirrors the 'resized' handler below. + // Why: cold-start refit — init()'s fit can run against a transient scrollWidth, so re-fire against a settled DOM. scheduleDelayedAction(() => getTerminalRef(handle)?.resetZoom(), 200) - // Why: viewport measurement needs xterm to be initialized (cell - // dimensions come from the renderer). On the first subscribe the - // WebView hasn't loaded yet, so viewportRef is null and the server - // can't auto-fit. After the first init we can measure, then - // resubscribe so the server gets the viewport and phone-fits. - // If viewport was measured by a parallel path BUT the scrollback - // we just received came back at desktop dims, our subscribe - // beat the measure; the server still has a null viewport for - // this subscriber record — resubscribe so it gets stored. + // Why: first subscribe has no viewport (xterm not loaded yet), so measure after init and resubscribe so the server can phone-fit. const needsResubscribe = !viewportMeasuredRef.current || (viewportRef.current != null && @@ -1559,14 +1465,7 @@ export default function SessionScreen() { scrollbackRows !== viewportRef.current.rows)) if (needsResubscribe) { void (async () => { - // Why: wait for the WebView's init() rAF chain to fully - // run (term.open → renderService population → first - // paint) before measuring. Without this, the measure - // postMessage races ahead of init's async work and - // returns null (term not ready / cells size 0), the - // resubscribe never fires, and the server never gets - // phone dims. See log dump 2026-05-06 confirming the - // race + measure-result null pattern. + // Why: wait for init()'s rAF chain before measuring, else the measure races ahead and returns null (log dump 2026-05-06). await getTerminalRef(handle)?.awaitReady() if (subscribeSeqRef.current.get(handle) !== seq) { return @@ -1574,25 +1473,14 @@ export default function SessionScreen() { const dims = await getTerminalRef(handle)?.measureFitDimensions( terminalFrameHeightRef.current || undefined ) - // Why: re-check seq after the awaits — awaitReady (up to - // 3s) and measureFitDimensions can take hundreds of ms, - // during which a newer subscribe cycle may have armed - // its own subscription. Tearing it down here would reset - // the freshly-armed initialized flag and re-subscribe a - // stale generation. + // Why: re-check seq — the awaits may have let a newer subscribe cycle arm; tearing it down would resubscribe a stale generation. if (subscribeSeqRef.current.get(handle) !== seq) { return } if (!getTerminalRef(handle)) { return } - // Why: we just got `scrollback` with cols=80 (server's - // default fallback for null viewport). That means the - // server-side subscriber record was registered before we - // could send viewport. Even if `viewportMeasuredRef` - // raced ahead via a parallel `measureViewportOnce`, the - // server still has a null viewport for THIS subscriber - // record — we MUST resubscribe so the server stores it. + // Why: scrollback came back at cols=80 (server's null-viewport fallback), so this subscriber record has no viewport — resubscribe so the server stores it. if (dims) { diagnostics.streamResubscribing(handle, seq, dims) viewportRef.current = dims @@ -1607,11 +1495,7 @@ export default function SessionScreen() { updateTerminalCwdFromStreamEvent(handle, data, terminalCwdRef.current) } else if (data.type === 'data') { updateTerminalCwdFromStreamEvent(handle, data, terminalCwdRef.current) - // Why: log when data arrives but the WebView ref is missing - // — this is the most likely cause of "blank but input works": - // server stream is alive, sends flow, but writes are dropped - // because the WebView ref disappeared (unmount mid-flight) or - // the scrollback never landed (so xterm has no buffer). + // Why: missing ref is the likely cause of "blank but input works" — writes dropped after mid-flight unmount or scrollback never landed. const dataRef = getTerminalRef(handle) if (!dataRef) { console.log('[fit][session] data DROPPED — no terminal ref', { @@ -1630,12 +1514,7 @@ export default function SessionScreen() { dataRef.write(data.chunk as string) } else if (data.type === 'resized') { updateTerminalCwdFromStreamEvent(handle, data, terminalCwdRef.current) - // Why: inline resize event — the server changed the PTY dimensions - // (mode toggle, desktop restore, or a width reflow). When the server - // includes a fresh full-buffer snapshot (width reflow), reinitialize - // xterm at the new dims so the hard-wrapped scrollback rewraps; - // preserve the reader's scroll position across the replay. Otherwise - // resize xterm geometry and let the TUI's own redraw repaint. + // Server resize: reinit xterm on a full-buffer snapshot (width reflow rewraps scrollback), else just resize geometry. const cols = (data.cols as number) || 80 const rows = (data.rows as number) || 24 const serialized = typeof data.serialized === 'string' ? data.serialized : null @@ -1679,9 +1558,7 @@ export default function SessionScreen() { unsubscribe: unsubscribeTerminal }) - // Why: toggles between phone and desktop mode via server RPC. The server - // handles the actual resize and emits a 'resized' event on the existing - // subscription stream — no client-side state tracking needed. + // Why: server does the resize and emits 'resized' on the existing subscription — no client-side state tracking needed. const toggleInFlightRef = useRef>(new Set()) const toggleDisplayMode = useCallback( async (handle: string) => { @@ -1692,8 +1569,7 @@ export default function SessionScreen() { return } const current = terminalModes.get(handle) ?? 'auto' - // Why: 'phone' on the wire is an observation ("currently phone-fitted"), - // not a setting. The toggle only ever requests 'auto' or 'desktop'. + // Why: 'phone' is an observed state, not a setting; the toggle only requests 'auto' or 'desktop'. const next: 'auto' | 'desktop' = current === 'auto' || current === 'phone' ? 'desktop' : 'auto' toggleInFlightRef.current.add(handle) @@ -1701,14 +1577,11 @@ export default function SessionScreen() { await client.sendRequest('terminal.setDisplayMode', { terminal: handle, mode: next, - // Why: presence-lock take-floor signal — requesting 'auto' is the - // explicit "I want to drive at phone dims" gesture. + // Why: presence-lock take-floor — requesting 'auto' is the explicit "drive at phone dims" gesture. ...(deviceTokenRef.current ? { client: { id: deviceTokenRef.current, type: 'mobile' as const } } : {}), - // Why: late-bind viewport for terminals whose subscribe record - // was registered before measurement landed. Without this the - // server's stored viewport is null and auto toggles no-op. + // Why: late-bind viewport for terminals subscribed before measurement, or auto toggles no-op on a null stored viewport. ...(viewportRef.current && next === 'auto' ? { viewport: viewportRef.current } : {}) }) } catch { @@ -1744,19 +1617,14 @@ export default function SessionScreen() { if (result.terminals.length === 0 && !allowEmptyLoaded) { return } - // Why: protect against transient empty responses from the server - // during rapid tab switching or RPC timing. If we previously had - // terminals and the server now says 0, require a second consecutive - // empty to confirm. This prevents the UI from flashing empty during - // rapid interactions while still allowing genuine cleanup. + // Why: require two consecutive empties before trusting 0, so transient empty responses don't flash the UI empty. if (result.terminals.length === 0 && lastKnownTerminalCountRef.current > 0) { lastKnownTerminalCountRef.current = 0 return } const liveHandles = new Set(result.terminals.map((terminal) => terminal.handle)) - // Why: terminal.list is the lifetime signal; session-tab snapshots can lag - // mobile-created tabs and must not erase a user's buffered-mode opt-out. + // Why: terminal.list is the lifetime signal; lagging tab snapshots must not erase a user's buffered-mode opt-out. pruneTerminalHandlesFromLiveInput(liveHandles) defaultTerminalHandlesToLiveInput([...liveHandles]) for (const handle of Array.from(terminalUnsubsRef.current.keys())) { @@ -1776,11 +1644,7 @@ export default function SessionScreen() { } } lastKnownTerminalCountRef.current = result.terminals.length - // 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. + // Why: dedupe duplicate handles (rename/split race) to avoid a React duplicate-key throw; keep first for tab-strip order. const seen = new Set() const deduped = result.terminals.filter((t) => { if (seen.has(t.handle)) { @@ -1800,8 +1664,7 @@ export default function SessionScreen() { ) terminalsRef.current = mergedTerminals - // Session tabs are the UI authority. terminal.list only refreshes - // per-handle metadata for existing ready terminal surfaces. + // Session tabs are the UI authority; terminal.list only refreshes per-handle metadata for existing terminal surfaces. } } catch { // Failed to list terminals @@ -1823,8 +1686,7 @@ export default function SessionScreen() { const applySessionTabs = useCallback( (result: SessionTabsResult) => { const diagnostics = terminalDiagnosticsRef.current - // Reject out-of-order snapshots, then suppress just-closed tabs until the - // publisher confirms their absence. See session-tab-snapshot-gate. + // Reject stale snapshots; suppress just-closed tabs until the publisher confirms absence — see session-tab-snapshot-gate. if (!acceptSessionSnapshot(result, appliedSnapshotMarkerRef.current)) { return } @@ -1846,8 +1708,7 @@ export default function SessionScreen() { tab.type === 'markdown' && tab.id === tabId ) if (draftTab) { - // Why: save-only mobile edits live only on the phone until Save. If the - // desktop tab disappears, keep every local draft reachable for copy/discard. + // Why: mobile edits live on the phone until Save; if the desktop tab vanishes, keep drafts reachable for copy/discard. orphanedDraftTabs.push({ ...draftTab, isActive: tabId === activeSessionTabIdRef.current }) } } @@ -1855,8 +1716,7 @@ export default function SessionScreen() { nextTabs = [...orphanedDraftTabs, ...nextTabs] } sessionTabsRef.current = nextTabs - // Why: subscribe snapshots often repeat identical tab payloads. Avoid a - // render loop where the subscription effect tears down and replays itself. + // Why: subscribe snapshots often repeat identical payloads; skip re-set to avoid a subscription teardown/replay loop. setSessionTabs((prev) => (mobileSessionTabsEqual(prev, nextTabs) ? prev : nextTabs)) const terminalTabs = getTerminalRecordsFromSessionTabs(nextTabs) const terminalTabHandles = terminalTabs.map((terminal) => terminal.handle) @@ -1890,8 +1750,7 @@ export default function SessionScreen() { } else { const pendingTab = nextTabs.find((tab) => tab.id === pendingActiveSessionTabId) if (pendingTab) { - // Why: desktop tab snapshots can lag a mobile tap while activate RPC - // is in flight. Keep the locally selected tab to avoid snapping back. + // Why: desktop tab snapshots can lag a mobile tap mid-activate-RPC; keep the local selection to avoid snapping back. active = pendingTab selectionSource = 'pending-tab' } else { @@ -1917,9 +1776,7 @@ export default function SessionScreen() { selectionSource = 'pending-handle-local-ack' } } else if (pendingTerminalTab) { - // Why: desktop active flags can lag a mobile terminal tap. Key by - // terminal handle too, because fallback PTY tabs may not yet have a - // stable session tab id during new-worktree startup. + // Why: desktop active flags lag a mobile tap; key by handle too, as fallback PTY tabs lack a stable tab id at startup. active = pendingTerminalTab selectionSource = 'pending-handle-tab' } else if (pendingTerminalExists) { @@ -2006,10 +1863,7 @@ export default function SessionScreen() { if (!shouldReadMarkdownFromDiskAfterReadTabFailure(response as RpcFailure)) { throw new Error((response as RpcFailure).error.message) } - // Why: a headless host (no desktop renderer) can't serve the live editor - // document and fails markdown.readTab with renderer_unavailable. Fall back - // to the on-disk file so markdown still renders read-only, matching how - // other file types load via files.read. + // Why: a headless host fails markdown.readTab (renderer_unavailable); fall back to the on-disk file for read-only render. const fallback = await client.sendRequest('files.read', { worktree: `id:${worktreeId}`, relativePath: tab.relativePath @@ -2270,8 +2124,7 @@ export default function SessionScreen() { router.back() return } - // Why: Android back can arrive when this session is the root route; using - // replace avoids React Navigation's dev-only unhandled GO_BACK warning. + // Why: Android back can fire at the root route; replace avoids React Navigation's dev-only GO_BACK warning. router.replace(`/h/${hostId}`) }, [hostId, router]) @@ -2418,8 +2271,7 @@ export default function SessionScreen() { const result = (response as RpcSuccess).result as SessionTabsResult terminalDiagnosticsRef.current.tabsFetchSucceeded(result) applySessionTabs(result) - // Focus a just-opened browser tab once it appears in the snapshot, via the - // normal activate path so it sticks and the user can still switch away. + // Focus a just-opened browser tab when it appears, via the normal activate path so it sticks yet stays switchable. const pendingPageId = pendingBrowserFocusPageIdRef.current if (pendingPageId) { const browserTab = result.tabs.find( @@ -2500,10 +2352,7 @@ export default function SessionScreen() { } }, [client, connState]) - // 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. + // Why: read deviceToken from host record so code can pass client.id on subscribe/send for driver-state-machine identity. useEffect(() => { if (!hostId) { return @@ -2578,9 +2427,7 @@ export default function SessionScreen() { for (const terminalRef of terminalRefs.current.values()) { terminalRef.prepareForForegroundRecovery() } - // Why: iOS can resume a live WKWebView with a blank xterm backing store - // without firing web-ready/reconnect; invalidate the native readiness - // latch before replay so init waits for the document's pong. + // Why: iOS can resume a WKWebView with a blank xterm store and no web-ready; invalidate the latch so init waits for the pong. const outcome = recoverActiveTerminalAfterForeground({ activeHandleRef, terminalRefs, @@ -2597,10 +2444,7 @@ export default function SessionScreen() { } }, [scheduleDelayedAction, subscribeToTerminal, unsubscribeTerminal]) - // Why: resume usually lands mid-reconnect (the socket dies after ~60-80s of - // background), so the recovery above defers. Re-run it once the connection - // is back; otherwise a blanked WKWebView whose socket was merely probed (no - // stream replay) stays stale until a manual tab switch. + // Why: resume lands mid-reconnect (socket dies in bg); re-run recovery once connected or a blanked WKWebView stays stale. useEffect(() => { if (connState !== 'connected' || !pendingForegroundRecoveryRef.current) { return @@ -2620,9 +2464,7 @@ export default function SessionScreen() { }) }, [connState, scheduleDelayedAction, subscribeToTerminal, unsubscribeTerminal]) - // Why: viewport refits for layout changes outside the subscribe path - // (tab strip toggling, fold/unfold, rotation) live in a dedicated hook — - // see terminal-viewport-refit.ts for the full rationale. + // Why: non-subscribe layout refits (tab strip, fold, rotation) live in a dedicated hook — see terminal-viewport-refit.ts. const { notifyTerminalFrameHeight, notifyKeyboardVisibility } = useTerminalViewportRefit({ activeHandleRef, terminalRefs, @@ -2680,8 +2522,7 @@ export default function SessionScreen() { } }, []) - // Reveal the active tab whenever it changes (e.g. desktop's open tab synced on - // worktree entry). Defer one frame so freshly mounted tab layouts are recorded. + // Reveal the active tab on change; defer one frame so freshly mounted tab layouts are recorded. useEffect(() => { const id = requestAnimationFrame(() => scrollActiveTabIntoView(activeSessionTabId, true)) return () => cancelAnimationFrame(id) @@ -2711,9 +2552,7 @@ export default function SessionScreen() { }, [router]) useEffect(() => { - // Why: Expo can reuse this screen across worktrees. Reset pending - // keyboard listeners, snapshot floors, and tombstones so prior route state - // cannot open stale UI or reject the next worktree's first snapshot. + // Why: Expo reuses this screen across worktrees; reset route state so it can't open stale UI or reject the next snapshot. sessionTabActionSheetRequestSeqRef.current += 1 sessionTabActionSheetKeyboardHideSubRef.current?.remove() sessionTabActionSheetKeyboardHideSubRef.current = null @@ -2762,18 +2601,11 @@ export default function SessionScreen() { if (connState !== 'connected') { return } - // Why: the RPC client auto-resends terminal.subscribe on reconnect. - // Keep the current xterm visible while the binary snapshot hydrates, - // instead of clearing to a blank "Loading terminals" surface. + // Why: keep the current xterm visible while the reconnect snapshot hydrates, not a blank "Loading terminals" surface. if (initializedHandlesRef.current.size === 0) { setTerminalsLoaded(false) } - // Why: on reconnect the RPC client auto-resends terminal.subscribe and - // the server sends a fresh scrollback frame. The subscribe handler drops - // scrollback when initializedHandlesRef already contains the handle, so - // we'd keep stale pre-disconnect content (and lose any output emitted - // during the disconnect). Clear the flag so the fresh snapshot calls - // ref.init(...) and replaces the buffer. + // Why: clear the initialized flag so the reconnect scrollback replaces stale content instead of being dropped. initializedHandlesRef.current.clear() let disposed = false const timers: ReturnType[] = [] @@ -2790,8 +2622,7 @@ export default function SessionScreen() { } } if (client && created !== '1') { - // Why: mobile needs host-owned tabs hydrated for this route, but should - // not pull other paired clients, especially desktop, into this worktree. + // Why: hydrate host-owned tabs without pulling other paired clients (esp. desktop) into this worktree. void client .sendRequest('worktree.activate', { worktree: `id:${worktreeId}`, @@ -2882,8 +2713,7 @@ export default function SessionScreen() { } void fetchSessionTabs() void fetchTerminals() - // Why: the live tab subscription stays mounted for stream ownership, - // but the fallback list poll should stop while this route is hidden. + // Why: live subscription keeps stream ownership, but the fallback list poll should stop while this route is hidden. const interval = setInterval(() => { void fetchSessionTabs() void fetchTerminals() @@ -2892,8 +2722,7 @@ export default function SessionScreen() { }, [connState, fetchSessionTabs, fetchTerminals]) ) - // Why: pick up the Settings → Terminal text size when returning here — the - // terminal panes stay mounted, so they update in place. + // Why: pick up Settings → Terminal text size on return; panes stay mounted and update in place. useFocusEffect( useCallback(() => { let active = true @@ -2938,10 +2767,7 @@ export default function SessionScreen() { }, []) ) - // Why: unsubscribe the old terminal so the server restores its desktop dims - // (clearing the phone-fit banner), then subscribe the new terminal with the - // measured viewport so the server phone-fits it. Also call terminal.focus - // so the desktop renderer follows the mobile user's active terminal. + // Why: unsubscribe restores old dims (clears phone-fit banner); resubscribe phone-fits the new one; terminal.focus makes desktop follow. const switchTab = useCallback( (handle: string) => { triggerSelection() @@ -2971,8 +2797,7 @@ export default function SessionScreen() { if (client) { void focusMobileTerminal(client, handle).catch(() => {}) if (matchingTab) { - // Why: persist selection for headless hosts; the snapshot gate keeps - // this phone-local acknowledgement from impersonating desktop focus. + // Why: persist selection for headless hosts; snapshot gate stops this phone-local ack from impersonating desktop focus. void activateMobileSessionTab(client, { worktree: `id:${worktreeId}`, tabId: matchingTab.id, @@ -3052,20 +2877,15 @@ export default function SessionScreen() { if (cached?.status === 'ready' && cached.isDirty) { return } - // Why: desktop clean saves do not carry a reliable content version in the - // lightweight tab list. Re-read on revisit unless the phone has a draft. + // Why: tab list lacks a reliable version for desktop clean saves; re-read on revisit unless the phone has a draft. void readMarkdownTab(tab) }, [client, markdownDocs, readFileTab, readMarkdownTab, switchTab, unsubscribeTerminal, worktreeId] ) - // Keep the ref pointing at the latest switchSessionTab so fetchSessionTabs can - // activate a freshly-synced browser tab without a callback dependency cycle. + // Ref to latest switchSessionTab so fetchSessionTabs can activate a synced browser tab without a dependency cycle. switchSessionTabRef.current = switchSessionTab - // Why: just store the ref. Subscription is deferred to handleTerminalWebReady - // which fires after the WebView has loaded xterm.js and is ready to process - // init messages. This prevents the blank terminal race where init() was - // queued before the WebView loaded. + // Why: only store the ref; subscribe on web-ready to avoid the blank-terminal race (init queued before xterm.js loaded). const setTerminalWebViewRef = useCallback((handle: string, ref: TerminalWebViewHandle | null) => { terminalDiagnosticsRef.current.webViewRef(handle, ref != null) if (ref) { @@ -3093,12 +2913,7 @@ export default function SessionScreen() { handle === activeHandleRef.current ) if (wasAlreadyReady && initializedHandlesRef.current.has(handle)) { - // Why: the native WebView reloaded (Metro hot reload or Android - // process churn). The old xterm buffer is gone, so force a fresh - // scrollback snapshot. Only resubscribe if this is a reload — on - // first load the subscription is already running and pendingMessages - // will flush the queued init after this callback returns. - // (unsubscribeTerminal also clears layoutSeqRef for this handle.) + // Why: WebView reloaded (hot reload / Android churn); old xterm buffer is gone, so resubscribe for a fresh scrollback. unsubscribeTerminal(handle) initializedHandlesRef.current.delete(handle) if (handle === activeHandleRef.current) { @@ -3106,17 +2921,8 @@ export default function SessionScreen() { } return } - // Why: on first web-ready, the initial subscribeToTerminal call from - // fetchTerminals may have been skipped (reason=no-ref, WebView wasn't - // mounted yet). Now that the WebView is ready, subscribe if this is the - // active terminal and no subscription is running. Await measure before - // subscribe so the very first subscribe carries the viewport — without - // this, subscribe(viewport=null) lands on the server first and the - // post-scrollback measure path's resubscribe sees alreadyMeasured=true - // (because measureViewportOnce won the race) and silently skips. - // Why: a just-created tab can briefly lose activeHandleRef to a lagging - // session-tab snapshot; honor the pending marker so its one web-ready - // subscribe still fires (see handleCreateTerminal). + // Why: first subscribe may skip (no WebView ref); await measure so it carries the viewport, else it races measureViewportOnce and skips. + // Why: a just-created tab can lose activeHandleRef to a lagging snapshot; honor the pending marker so its web-ready subscribe still fires. const isIntendedActive = () => handle === activeHandleRef.current || handle === pendingActiveTerminalHandleRef.current if (isIntendedActive() && !terminalUnsubsRef.current.has(handle)) { @@ -3165,9 +2971,7 @@ export default function SessionScreen() { terminal: activeHandle, text, enter: true, - // Why: presence-lock take-floor signal. Identifies this phone as - // the active mobile actor so the runtime can resolve multi-mobile - // contention (most-recent-actor's viewport wins). + // Why: presence-lock take-floor; marks this phone active so multi-mobile contention resolves to the last actor. ...(deviceTokenRef.current ? { client: { id: deviceTokenRef.current, type: 'mobile' as const } } : {}) @@ -3286,8 +3090,7 @@ export default function SessionScreen() { const openSessionTabActionSheetAfterKeyboardDismiss = useCallback( (tab: MobileSessionTab) => { - // Why: live input can have a queued refocus; action sheets should open after - // the terminal keyboard is gone, not race it under the drawer. + // Why: live input may queue a refocus; open the action sheet after the keyboard is gone, not racing it under the drawer. sessionTabActionSheetRequestSeqRef.current += 1 const requestSeq = sessionTabActionSheetRequestSeqRef.current clearSessionTabActionSheetKeyboardListener() @@ -3345,9 +3148,7 @@ export default function SessionScreen() { [focusLiveInput] ) - // Tap on a file path in terminal output → resolve it on the host and open it - // as a file tab (mirrors desktop Cmd/Ctrl-click). Silent on a miss; the - // WebView only emits this when the tap landed on a detected path. + // Tap a terminal file path → resolve on host, open as file tab (mirrors desktop Cmd/Ctrl-click); silent on a miss. const handleFileTapActivationSeqRef = useRef(0) const handleFileTap = useCallback( (handle: string, pathText: string, line: number | null, column: number | null) => { @@ -3387,9 +3188,7 @@ export default function SessionScreen() { ) const handleOpenedFileDiffActivationSeqRef = useRef(0) - // Active tab captured at tap time (before the openDiff RPC). Capturing it when - // the diff finishes opening would misread a tab the user switched to mid-RPC - // as the tap-time tab, letting the retry steal focus back to the diff. + // Capture active tab at tap time; reading it after openDiff would misread a mid-RPC switch and let the retry steal focus. const fileOpenStartActiveTabIdRef = useRef(null) const handleFileOpenStart = useCallback(() => { fileOpenStartActiveTabIdRef.current = activeSessionTabIdRef.current @@ -3401,8 +3200,7 @@ export default function SessionScreen() { let activated = false const activateOpenedTab = async (): Promise => { - // Route matching through the shared helper so the deterministic repro - // test exercises the same logic production runs. + // Route matching through the shared helper so the repro test exercises the same logic production runs. const settled = await activateOpenedSourceControlDiffTab({ relativePath, activeTabIdAtTap, @@ -3469,8 +3267,7 @@ export default function SessionScreen() { current.tokens + elapsedSeconds * TERMINAL_GESTURE_INPUT_REFILL_PER_SECOND ) - // Why: tokens represent terminal control sequences, not WebView messages; - // one legitimate gesture message may batch up to 32 wheel/key reports. + // Why: tokens count terminal control sequences, not WebView messages; one gesture may batch up to 32 wheel/key reports. if (tokens < sequenceCount) { terminalGestureInputBucketsRef.current.set(handle, { tokens, lastRefillMs: now }) return false @@ -3556,12 +3353,7 @@ export default function SessionScreen() { if (!terminalGestureInputInFlightRef.current.has(handle)) { void flushTerminalGestureInput(handle) } else { - // Why: an RPC is in-flight and the new batch would overflow the - // pending-sequences cap. Appending preserves the already-queued - // bytes (which would otherwise be dropped) — the in-flight flush's - // finally block will pick up the merged queue. The cap is a soft - // guideline; brief overflow during in-flight is preferable to - // silently dropping user input. + // Why: cap is a soft guideline — append instead of dropping queued bytes; the in-flight flush picks up the merged queue. current.bytes += bytes current.sequenceCount += sequenceCount current.lastUpdatedMs = now @@ -3597,8 +3389,7 @@ export default function SessionScreen() { return } const modes = ptyModesRef.current.get(handle) - // Why: WebView gesture bytes can become PTY input here, so mouse-aware - // reports stay behind validation and SSH-safe rate limiting. + // Why: WebView gesture bytes can become PTY input, so gate mouse reports behind validation and SSH-safe rate limiting. if (!modes?.altScreen && !isGestureMouseTrackingMode(modes?.mouseTrackingMode)) { return } @@ -3641,18 +3432,10 @@ export default function SessionScreen() { } } - // Why: press-and-hold key repeat for keys flagged repeatable (arrows, - // backspace, forward-delete). Matches iOS keyboard cadence: instant first - // fire, then ~400ms before the second, then ~45ms between subsequent - // repeats. Non-repeatable keys (Tab, Esc, Ctrl-*) intentionally fire once - // because holding them is destructive or meaningless. + // Why: hold-to-repeat matches iOS cadence (400ms then 45ms); non-repeatable keys fire once (holding is destructive). const repeatTimeoutRef = useRef | null>(null) const repeatIntervalRef = useRef | null>(null) - // Why: hold the latest handleAccessoryKey in a ref so the repeat interval - // always invokes the current callback. Otherwise a held key keeps firing - // through the callback captured when the interval started, which can route - // bytes to a stale terminal/RPC client after a tab switch or reconnect - // mid-hold. + // Why: ref keeps repeat firing the current callback; else a mid-hold tab switch/reconnect routes bytes to a stale terminal. const handleAccessoryKeyRef = useRef(handleAccessoryKey) handleAccessoryKeyRef.current = handleAccessoryKey const stopAccessoryRepeat = useCallback(() => { @@ -3681,9 +3464,7 @@ export default function SessionScreen() { if (node !== null) { return } - // Why: terminal subscriptions and route-level timers must clear only on - // real route detach; client churn during mount can otherwise wipe xterm - // state mid-subscribe. + // Why: clear only on real route detach; client churn during mount would wipe xterm state mid-subscribe. toastSeqRef.current += 1 clearTerminalCache() clearToastHideTimer() @@ -3726,10 +3507,7 @@ export default function SessionScreen() { try { await Clipboard.setStringAsync(text) triggerSuccess() - // Why: Android 13+ shows its own system "Copied to clipboard" toast on - // every clipboard write, so our toast would be redundant; iOS shows - // nothing on copy (it only banners on paste), so the in-app toast is - // the only success signal there. + // Why: Android 13+ shows its own system copy toast; iOS shows none, so only iOS needs our in-app toast. if (Platform.OS === 'ios') { showToast('Copied') } @@ -3970,10 +3748,7 @@ export default function SessionScreen() { setCreating(true) setCreateError('') - // Why: idempotency key so a transport-level retry (reconnect replay) of this - // create resolves to the same terminal instead of spawning a duplicate. Kept - // compact (no worktree id) to stay under the schema's length cap; the ref - // guard above blocks concurrent taps synchronously. + // Why: idempotency key so a transport retry (reconnect replay) resolves to the same terminal, not a duplicate; kept compact (no worktree id) for the schema length cap. const clientMutationId = `mobile-create:${Date.now().toString(36)}-${Math.random() .toString(36) .slice(2, 10)}` @@ -3993,9 +3768,7 @@ export default function SessionScreen() { if (response.ok) { const result = (response as RpcSuccess).result as TerminalCreateResult const created = result.tab - // Why: unsubscribe the old active terminal so the server restores its - // desktop dims. Without this, the old terminal's mobile subscription - // stays alive and its restore timer is never set. + // Why: unsubscribe the old terminal so the server restores its desktop dims; otherwise its restore timer is never set. const prev = activeHandleRef.current if (prev) { unsubscribeTerminal(prev) @@ -4013,10 +3786,7 @@ export default function SessionScreen() { if (typeof created.terminal === 'string') { const createdHandle = created.terminal defaultTerminalHandlesToLiveInput([createdHandle]) - // Why: session-tab snapshots can lag the create RPC. Without the - // handle marker, applySessionTabs snaps activeHandleRef back to the - // previous terminal, and the new pane's web-ready subscribe (gated - // on the active handle) is skipped — blank tab until a manual switch. + // Why: snapshots lag the create RPC; without this marker applySessionTabs reverts the active handle, blanking the new pane. pendingActiveTerminalHandleRef.current = createdHandle activeHandleRef.current = createdHandle setActiveHandle(createdHandle) @@ -4079,8 +3849,7 @@ export default function SessionScreen() { showToast(options.successToast) } } else { - // Why: a prior pending handle must not outlive a create that returned - // no terminal; web-ready subscribe gates on this ref as active. + // Why: a prior pending handle must not outlive a create that returned no terminal; web-ready subscribe gates on this ref. pendingActiveTerminalHandleRef.current = null activeHandleRef.current = null setActiveHandle(null) @@ -4184,8 +3953,7 @@ export default function SessionScreen() { if (!client || creatingBrowser) { return false } - // Why: read via ref so a tap that fires before the capability probe resolves - // (or from a stale callback) still sees the live support value. + // Why: read via ref so a tap before the capability probe resolves (or a stale callback) still sees the live value. if (browserScreencastSupportedRef.current !== true) { showToast('Desktop update required for mobile browser streaming', 1600) return false @@ -4214,9 +3982,7 @@ export default function SessionScreen() { if (!response.ok) { throw new Error((response as RpcFailure).error.message) } - // Focus the new browser tab once it syncs (fetchSessionTabs activates it - // via the normal path). Refresh a few times since the desktop registers - // the tab asynchronously. + // Focus the new browser tab once it syncs; refresh a few times since the desktop registers the tab asynchronously. const created = (response as RpcSuccess).result as { browserPageId?: string } if (created.browserPageId) { pendingBrowserFocusPageIdRef.current = created.browserPageId @@ -4234,8 +4000,7 @@ export default function SessionScreen() { setCreatingBrowser(false) } } - // Keep the ref pointing at the latest handleCreateBrowser so a terminal URL - // tap (handleTerminalOpenUrl) always runs the current closure. + // Keep the ref at the latest handleCreateBrowser so a terminal URL tap always runs the current closure. handleCreateBrowserRef.current = handleCreateBrowser async function handleBrowserNavigationCommand( @@ -4345,9 +4110,7 @@ export default function SessionScreen() { clearTerminalLiveInputDefault(terminalHandle) } setSessionTabs((prev) => prev.filter((candidate) => candidate.id !== tab.id)) - // Why: tombstone the closed tab and rely on the subscription/poll - // snapshot (gated by snapshotVersion) instead of a blind 300ms refetch - // that re-applied whatever the host had — often the not-yet-closed list. + // Why: tombstone the closed tab and rely on the snapshot, not a blind refetch that often re-added the not-yet-closed tab. closedTabTombstonesRef.current.set(tab.id, Date.now() + 10_000) if (activeSessionTabId === tab.id) { activeSessionTabTypeRef.current = null @@ -4389,8 +4152,7 @@ export default function SessionScreen() { if (pendingTerminalActivationAttemptRef.current === activationKey) { return } - // Why: a hydrated headless/server-owned tab can already be active but still - // pending; activation is the RPC that materializes or focuses its PTY handle. + // Why: a server-owned tab can be active but still pending; activation is the RPC that materializes its PTY handle. pendingTerminalActivationAttemptRef.current = activationKey void activateMobileSessionTab(client, { worktree: `id:${worktreeId}`, @@ -4439,16 +4201,13 @@ export default function SessionScreen() { ) { return } - // Why: a sleeping/new workspace can hydrate with zero session tabs. Create - // the first terminal once on initial load instead of leaving mobile blank. + // Why: a sleeping/new workspace can hydrate with zero tabs; create the first terminal once so mobile isn't blank. initialEmptySessionAutoCreateRef.current = worktreeId setCreateError('') void handleCreateTerminal() }, [client, creating, creatingBrowser, creatingMarkdown, showEmptyState, worktreeId]) - // Why: the reconnect loop slows to a 90s trickle at its give-up cap; - // surface tap-to-retry once the verdict escalates so recovery doesn't - // wait out the trickle timer (issue #5049). + // Why: reconnect trickles to 90s at its give-up cap; surface tap-to-retry so recovery needn't wait it out (issue #5049). const connectionVerdict = classifyConnection({ state: connState, reconnectAttempts, @@ -4469,9 +4228,7 @@ export default function SessionScreen() { ? `${verdictDisplayLabel(connectionVerdict)} — tap to retry` : MOBILE_SESSION_STATUS_LABELS[connState] - // Why: keep safe-area padding in layout at all times, then visually translate - // the controls over the terminal when the keyboard appears. iOS keyboard - // height includes the home-indicator inset; Android IME height does not. + // Why: iOS keyboard height includes the home-indicator inset; Android IME height does not. const keyboardLift = keyboardHeight > 0 ? Platform.OS === 'ios' @@ -4493,8 +4250,7 @@ export default function SessionScreen() { const cursorBottom = (metrics.cursorY + 1) * rowHeight const dockTop = terminalFrameHeightRef.current - keyboardLift const margin = rowHeight - // Why: only move the terminal when the active cursor would sit under the - // raised input dock. Short shell output near the top should stay put. + // Why: only move the terminal when the cursor would sit under the raised input dock; short top output stays put. return Math.min(keyboardLift, Math.max(0, cursorBottom + margin - dockTop)) })() const toastAnimatedStyle = { @@ -4592,8 +4348,7 @@ export default function SessionScreen() { ] : [] - // Routes a header panel-icon tap through the pure dock-vs-push decision (U1): - // measured dock-capable rows toggle/swap, constrained rows push full-screen. + // Panel-icon taps route through the dock-vs-push decision (U1): dock-capable rows dock, constrained rows push. const handleSessionContentRowLayout = useCallback((event: LayoutChangeEvent) => { const width = Math.round(event.nativeEvent.layout.width) setSessionContentRowWidth((prev) => (prev === width ? prev : width)) @@ -4612,8 +4367,7 @@ export default function SessionScreen() { hostId, worktreeId, name: worktreeName || '', - // SC + PR both land on the source-control hub; post-diff-open dismissal - // keys off origin: 'session' (U2). Files keeps its own route without origin. + // SC + PR both land on the source-control hub with origin:'session' for post-diff-open dismissal (U2); Files opts out. ...(action.panel === 'sourceControl' || action.panel === 'pr' ? { origin: 'session' } : {}), // The PR panel routes into the hub's Pull Request segment via descriptor params. ...descriptor.params @@ -4695,10 +4449,7 @@ export default function SessionScreen() { {visibleTabs.length > 0 && ( - {/* Why: tab taps must register on the first press while the live - keyboard is open instead of being eaten by keyboard dismissal - (#5106); leaving a non-live tab still closes the keyboard - because the live input unmounts. */} + {/* Why: tab taps must register on first press with the keyboard open instead of being eaten by dismissal (#5106). */} ))} - {/* Why: pinned outside the scroll strip so the new-agent button - stays reachable no matter how far the tabs scroll. */} + {/* Why: pinned outside the scroll strip so the new-agent button stays reachable however far the tabs scroll. */} [ styles.newTerminalButton, @@ -4797,10 +4547,7 @@ export default function SessionScreen() { )} - {/* Content-row host (KTD2): the header/tab chrome stays a full-width sibling - above; on wide the post-chrome content shares this row with the docked panel. - There is no single terminal node, so the entire conditional block is the - flex-1 left child. On narrow the dock never renders and layout is unchanged. */} + {/* Content-row host (KTD2): on wide, content shares this row with the docked panel as the flex-1 left child. */} {createWarning ? ( @@ -4898,8 +4645,7 @@ export default function SessionScreen() { ) : activeBrowserTab ? ( - {/* Why: the pane owns imperative frame refs; browser tabs should - never render a stale frame while the old stream effect cleans up. */} + {/* Why: pane owns imperative frame refs; don't render a stale frame while the old stream effect cleans up. */} { terminalFrameHeightRef.current = e.nativeEvent.layout.height - // Why: notify height imperatively so dock settling re-fits the - // PTY without rerendering SessionScreen for layout callbacks. + // Why: notify height imperatively so dock settling re-fits the PTY without rerendering SessionScreen. const nextWidth = Math.round(e.nativeEvent.layout.width) const nextHeight = Math.round(e.nativeEvent.layout.height) setTerminalFrameWidth((prev) => (prev === nextWidth ? prev : nextWidth)) @@ -4945,8 +4690,7 @@ export default function SessionScreen() { terminalTheme={terminal.terminalTheme} textScale={terminalTextScale} onTextScaleChange={(scale) => { - // Why: pinch-to-zoom in the WebView reports a new preset; persist - // it so the size sticks across panes and app launches. + // Why: pinch-to-zoom reports a new preset; persist it so the size sticks across panes and launches. setTerminalTextScale(scale) void saveTerminalTextScale(scale) }} @@ -4985,9 +4729,7 @@ export default function SessionScreen() { )} - {/* Why: translate instead of resizing so keyboard open/close does not - trigger a server-side PTY viewport change. The dock hides in native - chat because that view supplies its own composer. */} + {/* Why: translate instead of resize so keyboard toggles don't trigger a server-side PTY viewport change. */} {!activeMarkdownTab && !activeFileTab && !activeBrowserTab && !showNativeChat && ( {/* Accessory keys */} - {/* Why: a fixed, always-visible escape hatch from the open - keyboard. Kept outside the horizontal ScrollView so it does - not scroll away, and out of the terminal-byte shortcut path so - it cannot be hidden by user shortcut customization (#5106). */} + {/* Why: fixed keyboard escape hatch; outside ScrollView + shortcut path so it can't scroll away or be hidden (#5106). */} {keyboardLift > 0 && ( [ @@ -5024,9 +4763,7 @@ export default function SessionScreen() { )} - {/* Why: with default tap handling the first tap on any accessory - key dismisses the open keyboard and is swallowed, so live - input lost its keyboard on every Esc/Tab press (#5106). */} + {/* Why: default tap handling makes the first accessory-key tap dismiss the keyboard and get swallowed (#5106). */} () let nextClientIdentity = 1 function clientKey(client: RpcClient): number { @@ -164,11 +161,7 @@ function fetchWorktreeInfo( ) => void, disposed: () => boolean ) { - // Why: only seed an empty zeroed entry when this host has no prior info - // at all (e.g., first ever load before any cache hydration). On a - // transient failure for a host that already has cached data, leave the - // cached entry alone so the Resume card and host-meta line don't - // momentarily flip to "0 worktrees" / disappear during reconnects. + // Why: only seed a zeroed entry when the host has no prior info; keep cached data on transient failure so counts don't flip to 0 during reconnects. const markLoadedIfMissing = () => { setInfo((prev) => { if (prev[hostId]) { @@ -187,8 +180,7 @@ function fetchWorktreeInfo( } client - // Why: worktree.ps defaults to 200 and silently truncates; request the full - // set so the host worktree count and active count are accurate. + // Why: worktree.ps defaults to 200 and silently truncates; request all so counts are accurate. .sendRequest('worktree.ps', { limit: 10000 }) .then((response) => { if (disposed()) { @@ -286,8 +278,7 @@ function fetchTaskProviders( }) } -// Why: repo names get a stable color derived from hashing, matching the -// host detail page's colored dots for visual consistency. +// Why: hash repo name to a stable color, matching the host detail page's dots. const REPO_COLORS = ['#8b5cf6', '#3b82f6', '#22c55e', '#f59e0b', '#ef4444', '#ec4899', '#06b6d4'] function repoColor(name: string): string { let hash = 0 @@ -300,8 +291,7 @@ function repoColor(name: string): string { export default function HomeScreen() { const router = useRouter() const insets = useSafeAreaInsets() - // Why: cap and center content on wide/tablet canvases so cards don't stretch - // edge-to-edge on iPad; on phones isWideLayout is false and layout is unchanged. + // Why: cap/center content on wide/tablet canvases so cards don't stretch edge-to-edge on iPad. const { isWideLayout, contentMaxWidth } = useResponsiveLayout() const [hosts, setHosts] = useState([]) const [actionTarget, setActionTarget] = useState(null) @@ -318,9 +308,7 @@ export default function HomeScreen() { ) const notificationOptInCheckedRef = useRef(false) - // 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. + // Why: shared clients from the per-host store, not N independent WebSockets. See docs/mobile-shared-client-per-host.md. const hostIds = useMemo(() => hosts.map((h) => h.id), [hosts]) const allClients = useAllHostClients(hostIds) const hostPaths = useMemo( @@ -330,27 +318,20 @@ export default function HomeScreen() { 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). + // Why: prime the cache with loaded HostProfiles to avoid a second serialized Keychain pass (multi-second connect latency) on cold start. useEffect(() => { if (hosts.length > 0) { primeHosts(hosts) } }, [hosts, primeHosts]) const allClientsRef = useRef>([]) - // Why: the focus callback stays stable to avoid refetching on every - // client-store render, but it still needs the latest host clients. + // Why: keep the focus callback stable (no refetch per render) while still exposing the latest host clients. allClientsRef.current = allClients.map((entry) => ({ hostId: entry.hostId, client: entry.client })) - // Why: hydrate the home page from a persisted snapshot on cold-start so - // Resume + Account-usage cards paint immediately with last-known data - // instead of flashing empty for ~1s while the WebSocket reconnects. - // Stream/list responses overwrite this seed in place when they arrive. + // Why: hydrate from a persisted snapshot on cold-start so Resume + Account cards paint immediately instead of flashing empty. const hydratedRef = useRef(false) useEffect(() => { if (hydratedRef.current) { @@ -367,8 +348,7 @@ export default function HomeScreen() { for (const [hostId, info] of Object.entries(snap.worktreeInfo)) { const wt = info.lastActiveWorktree if (wt) { - // Why: also seed the in-memory worktree cache so resumeWorktree's - // lastVisited fast-path can find the cached worktree object. + // Why: seed the in-memory cache so resumeWorktree's lastVisited fast-path finds the worktree object. setCachedWorktrees(hostId, [wt]) } } @@ -378,9 +358,7 @@ export default function HomeScreen() { } }, []) - // Why: persist the merged snapshot whenever either piece updates so the - // next cold-start has fresh seed data. The cache module debounces writes - // internally so a flurry of streamed updates doesn't hammer disk. + // Why: persist the merged snapshot on each update so the next cold-start has fresh seed data (cache debounces writes). useEffect(() => { if (Object.keys(worktreeInfo).length === 0 && Object.keys(accountsByHost).length === 0) { return @@ -436,8 +414,7 @@ export default function HomeScreen() { [hosts] ) - // Why: mirror per-host connection state into hostStates so existing - // render code (status dots, connecting indicators) keeps working. + // Why: mirror per-host connection state into hostStates so existing render code (status dots) keeps working. useEffect(() => { setHostAttempts((prev) => { const next: Record = { ...prev } @@ -473,11 +450,7 @@ export default function HomeScreen() { 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'. + // Why: reflect hosts that dropped from allClients, but only if already tracked — else the initial-acquire frame flips all to 'disconnected'. for (const host of hosts) { if (liveIds.has(host.id)) { continue @@ -506,11 +479,7 @@ export default function HomeScreen() { }) }, [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. + // Per-host notif/accounts subs + one-shot stats on 'connected'; re-runs per (hostId, client) pair, socket stays open so it's cheap. useEffect(() => { const cleanups: Array<() => void> = [] for (const entry of allClients) { @@ -563,12 +532,7 @@ export default function HomeScreen() { c() } } - // Why: depend on the host-id set AND each entry's client identity, so - // resubscriptions don't fire on every render that produces a new - // array reference, but DO fire when forceReconnect swaps the - // underlying client for a host (otherwise wireUp would keep firing - // on a closed client and never re-attach to the fresh one, leaving - // notifications/accounts subs broken until the user navigates). + // Why: key on host-id set + each client's identity so resubs fire when forceReconnect swaps a host's client, not on every render. // eslint-disable-next-line react-hooks/exhaustive-deps }, [ allClients @@ -577,22 +541,10 @@ export default function HomeScreen() { .join(',') ]) - // Why: prefer the worktree the user last opened on this device so the - // "Resume" card reflects their mobile session history, not just the - // desktop's most-recently-outputting worktree. - // Why: rendering used to be gated on hostStates === 'connected', which - // caused the Resume card to vanish for ~1s on every cold-start / - // resume-from-background while the WebSocket reconnected, even though we - // had perfectly good cached worktree data. Now the card stays visible as - // long as we have a cached lastActiveWorktree for any known host; the - // tap target is still the same and a fresher snapshot from the live RPC - // overwrites the card's contents in place when it lands. + // Why: prefer the worktree last opened on this device so Resume reflects mobile session history. + // Why: don't gate on 'connected' so the card doesn't flash empty for ~1s on cold-start; cached data holds until fresh RPC lands. const resumeWorktree = useMemo(() => { - // 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. + // Why: only surface Resume for connected hosts; a stale worktree taps into a route that can't load. if (lastVisited && hostStates[lastVisited.hostId] === 'connected') { const cached = getCachedWorktrees(lastVisited.hostId) as WorktreeSummary[] | null const match = cached?.find((w) => w.worktreeId === lastVisited.worktreeId) @@ -612,9 +564,7 @@ export default function HomeScreen() { return null }, [sortedHosts, hostStates, worktreeInfo, lastVisited]) - // 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. + // Why: only show Account usage for connected hosts; stale cached usage would imply live data. const accountsHosts = useMemo(() => { const items: Array<{ host: HostProfile; snapshot: AccountsSnapshot }> = [] for (const host of sortedHosts) { @@ -625,9 +575,7 @@ export default function HomeScreen() { if (!snap) { continue } - // Why: also show hosts whose only usage is the system-default login - // (no Orca-managed accounts but live rate-limit data for the active - // target), otherwise system-default users see no usage section at all. + // Why: also show hosts whose only usage is the system-default login, else those users see no usage section. if (hasRenderableUsage(snap, 'claude') || hasRenderableUsage(snap, 'codex')) { items.push({ host, snapshot: snap }) } @@ -716,8 +664,7 @@ export default function HomeScreen() { setConfirmRemove(null) setHosts(await loadHosts()) } catch { - // Why: ConfirmModal closes on confirm; re-open for retry and surface the - // failure instead of silently leaving the host listed. + // Why: ConfirmModal closes on confirm; re-open for retry so the failure isn't silent. setConfirmRemove(hostToRemove) Alert.alert('Could not remove host', 'Please try again.') } @@ -782,9 +729,7 @@ export default function HomeScreen() { h.id} - // Why: edge-to-edge — let the list scroll under the system nav bar - // but reserve insets.bottom so the last row stays reachable above - // the Samsung 3-button nav / iOS home indicator. + // Why: reserve insets.bottom so the last row stays reachable above the system nav bar / home indicator. contentContainerStyle={[ styles.list, { paddingBottom: spacing.xl + insets.bottom }, @@ -961,10 +906,7 @@ export default function HomeScreen() { ? snapshot.claude.accounts : snapshot.codex.accounts const limits = getActiveProviderRateLimits(snapshot, provider) - // Why: with no managed accounts, still render a - // "System default" row when the active target has - // live usage data; the row label already falls back - // to "System default" below. + // Why: with no managed accounts, still render the row when the active target has live usage data. if (accounts.length === 0 && !hasActiveProviderUsage(limits)) { return null } @@ -1027,11 +969,7 @@ export default function HomeScreen() { state === 'connecting' || state === 'handshaking' || state === 'reconnecting' - // Why: "Reconnect" implies "you were connected, try again". If - // the client has never reached 'connected' this session (cold - // start, unreachable host, or after Disconnect) the action is - // functionally a fresh Connect — using the right verb makes - // the affordance match what tapping it actually does. + // Why: label "Connect" (not "Reconnect") when never connected this session, so the verb matches the action. const hasEverConnected = (hostLastConnected[host.id] ?? null) != null const items: ActionSheetAction[] = [] items.push({ diff --git a/mobile/src/notifications/mobile-notifications.ts b/mobile/src/notifications/mobile-notifications.ts index d38663931..7e1444d9b 100644 --- a/mobile/src/notifications/mobile-notifications.ts +++ b/mobile/src/notifications/mobile-notifications.ts @@ -17,9 +17,7 @@ type NotificationEvent = { body: string worktreeId?: string notificationId?: string - // Mirrors the desktop-assigned MobileNotificationEvent.notificationSeq used - // for reconnect catch-up (#8129). Optional because older runtimes / non- - // replay events may omit it. + // Desktop-assigned seq for reconnect catch-up (#8129); optional since older runtimes may omit it. notificationSeq?: number } @@ -42,12 +40,7 @@ type ScheduledNotificationState = { const scheduledNotificationsByHostAndNotificationId = new Map() -// Why: notificationId embeds a per-completion timestamp (buildAgentNotificationId), -// so every agent-task-complete inserts a new, never-reused key. Entries are only -// removed when the desktop sends a matching dismiss — which a remote mobile user -// (not at the desktop) frequently never gets — so the map grew for the app's whole -// life. Bound it; a settled entry only retains a small identifier used for later -// programmatic dismissal, unnecessary for long-past completions. +// Why: keys never repeat and are only freed on desktop dismiss (which remote users often miss), so bound the map to stop unbounded growth. const MAX_SCHEDULED_NOTIFICATIONS = 256 let maxScheduledNotifications = MAX_SCHEDULED_NOTIFICATIONS @@ -55,8 +48,7 @@ function getStoredNotificationKey(hostId: string, notificationId: string): strin return `${encodeURIComponent(hostId)}:${encodeURIComponent(notificationId)}` } -// Evict the oldest SETTLED entries (never one mid-schedule) until within the cap. -// Map iteration is insertion order, so the first match is the oldest. +// Evict oldest settled entries (never mid-schedule); Map iteration is insertion order so the first match is oldest. function boundScheduledNotifications(): void { while (scheduledNotificationsByHostAndNotificationId.size > maxScheduledNotifications) { let evicted = false @@ -91,16 +83,13 @@ export async function getNotificationPermissionState(): Promise= 33) } } -// Why: permissions must be requested before scheduling any local notification. -// Read the OS state every time because users can change it in Settings while -// Orca remains alive in the background. +// Why: re-read OS state every call — users can change it in Settings while Orca is backgrounded. export async function ensureNotificationPermissions(): Promise { const existing = await getNotificationPermissionState() if (existing.granted) { @@ -225,8 +214,7 @@ async function dismissLocalNotification( return } if (state.pending) { - // Why: desktop can send dismiss while iOS/Android is still scheduling the - // matching local notification. Remember it so no stale banner survives. + // Why: dismiss can arrive while the OS is still scheduling; defer it so no stale banner survives. state.dismissAfterSchedule = true return } @@ -237,29 +225,15 @@ async function dismissLocalNotification( await Notifications.dismissNotificationAsync(state.identifier).catch(() => {}) } -// Why: each host connection gets its own notification subscription. When the -// connection drops, the unsubscribe function cleans up the streaming RPC. -// On reconnect the same subscribe stream is re-established by the RPC client; -// we use its `ready` event to trigger catch-up (#8129): fetch notifications -// dispatched while the socket was reaped, watermarked by the last seq we -// already delivered so the desktop never re-sends an already-pushed one. -// Returns an unsubscribe function. +// Per-connection subscription; a reconnect `ready` triggers watermarked catch-up (#8129) so already-pushed events aren't re-sent. export function subscribeToDesktopNotifications(client: RpcClient, hostId: string): () => void { configureNotificationChannel() let subscriptionId: string | null = null let disposed = false - // Highest seq delivered on the live stream or replay for this connection. - // Persisted per-host so a cold app start still resumes from the right cut. + // Highest seq delivered (live or replay) this connection; persisted per-host so cold start resumes from the right cut. let lastDeliveredSeq = 0 - // Why: per-connection dedup guard applied ONLY to the replay path - // (fetchMissed), never the live stream. The desktop already guarantees the - // replay cannot contain an event with seq <= lastDeliveredSeq (both live and - // replay advance the same watermark), so live + replay never overlap there. - // This set is defense-in-depth: if the desktop's bounded buffer evicted an - // old entry and a reconnect re-fetches across a boundary, an id delivered in - // the same connection isn't pushed twice. Bounded (RECENTLY_SEEN_CAP) so a - // long-lived session can't grow without limit. + // Why: defense-in-depth dedup for replayed events if the desktop's bounded buffer evicted across a reconnect boundary. const seenReplay = createSeenNotificationGuard() function deliverLive( @@ -270,10 +244,7 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin lastDeliveredSeq = event.notificationSeq void saveLastSeenSeq(hostId, lastDeliveredSeq) } - // Why (#8129 dedup): mark the event seen on EVERY delivery path (live AND - // replay) so a replay that re-includes an id already pushed live in this - // connection is dropped instead of double-pushed. fetchMissed also - // pre-checks seenReplay, but without this the live path never populated it. + // Why (#8129): mark seen on the live path too, so a later replay of an already-pushed id dedups instead of double-pushing. const key = seenKeyForEvent(event) if (key) { seenReplay.add(key) @@ -284,12 +255,7 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin return dismissLocalNotification(event as DismissNotificationEvent, hostId) } - // Why: on a reconnect `ready` the desktop has already dispatched whatever we - // missed; ask for it from our persisted watermark. Because the desktop cuts - // by seq > lastSeenSeq this is idempotent — we only ever get events we have - // not delivered before. The seenReplay guard is a second layer so a replay - // that somehow re-includes an id already delivered this connection is - // dropped instead of double-pushed. + // Why: desktop cuts by seq > lastSeenSeq, so re-fetching from the watermark is idempotent (seenReplay guards residual overlap). async function fetchMissed(): Promise { if (disposed) { return @@ -321,10 +287,7 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin } } - // Why: lazily seed the watermark from durable storage on first use so we - // don't block subscribe() on an AsyncStorage read. The first `ready` (cold - // open) does NOT need catch-up — the live stream starts fresh; only - // subsequent reconnect `ready` events fetch missed notifications. + // Why: seed the watermark lazily so subscribe() doesn't block on an AsyncStorage read. let watermarkLoaded = false void loadLastSeenSeq(hostId).then((seq) => { lastDeliveredSeq = Math.max(lastDeliveredSeq, seq) @@ -352,10 +315,7 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin unsubscribeStream() return } - // Why: first ready is the cold-open live stream — no catch-up needed. - // Every later ready is a reconnect; fetch what we missed from the - // watermark. Guard on watermarkLoaded so a fast reconnect doesn't - // fetch from a stale 0 watermark (which would re-push everything). + // Why: only reconnects fetch missed; watermarkLoaded guards against fetching from a stale 0 (which re-pushes everything). if (reconnectReadyCount > 1 && watermarkLoaded) { void fetchMissed() } @@ -379,12 +339,7 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin return () => { disposed = true - // Why: the client may already be closed when this cleanup runs (component - // unmount races with disconnect). sendRequest rejects immediately on a - // closed client — swallow it since server-side cleanup happens via - // connection-close anyway. - // Always drop the local stream first; readiness can race unmount and we - // must not retain the callback while waiting for a subscription id. + // Why: drop the local stream first — readiness can race unmount; don't hold the callback while a subscription id is pending. unsubscribeStream() if (subscriptionId) { unsubscribeServer(subscriptionId) diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller.ts b/mobile/src/session/use-mobile-pr-sidebar-controller.ts index 752b2ecb3..6c97b9e0b 100644 --- a/mobile/src/session/use-mobile-pr-sidebar-controller.ts +++ b/mobile/src/session/use-mobile-pr-sidebar-controller.ts @@ -24,21 +24,17 @@ type PrSidebarControllerInput = { client: RpcClient | null connState: ConnectionState worktreeId: string - // Head branch + SHA come from git.status (`branch`/`head`) via the review screen, - // not the branchCompare base ref nor worktree metadata (which carries no branch). + // branch/headSha come from git.status (not the branchCompare base ref nor worktree metadata, which carries no branch). branch: string | null headSha: string | null } -// Load options for the shared PR controller. The Source Control hub chip only needs -// phase 1 (PR + checks); phase 2 (comments/body) is heavy and should wait until the -// Pull Request segment is actually open. +// Load options: the hub chip needs only phase 1 (PR + checks); phase 2 (comments/body) is heavy and waits until the PR segment opens. export type PrSidebarLoadOptions = { includeDetails?: boolean } -// Identity is worktree + branch only. Head SHA advances on every commit and must not -// wipe a ready chip/sidebar to "loading" — soft-refresh uses the new head instead. +// Identity is worktree + branch only: headSha advances every commit and must not wipe a ready chip to "loading" (soft-refresh handles it). export function buildMobilePrSidebarIdentity(args: { worktreeId: string branch: string | null @@ -48,33 +44,23 @@ export function buildMobilePrSidebarIdentity(args: { export function useMobilePrSidebarController(input: PrSidebarControllerInput) { const { client, connState, worktreeId, branch, headSha } = input - // The dedicated PR icon is available whenever the repo has a GitHub remote — - // independent of whether the branch has an open PR (a no-PR branch shows an - // empty state rather than hiding the icon). + // PR icon shows for any GitHub remote, regardless of an open PR — a no-PR branch shows an empty state rather than hiding the icon. const [isGithubRepo, setIsGithubRepo] = useState(false) - // False until the probe resolves for this worktree. Consumers gate "unavailable - // for this provider" copy on it — isGithubRepo=false is meaningless mid-probe. + // False until the probe resolves — isGithubRepo=false is meaningless mid-probe, so consumers gate "unavailable" copy on this. const [repoProbeLoaded, setRepoProbeLoaded] = useState(false) const [state, setState] = useState({ kind: 'hidden' }) const [showPRSidebar, setShowPRSidebar] = useState(false) const loadSeqRef = useRef(0) - // Phase-2-only fetches use a separate sequence so they cannot cancel a concurrent - // phase-1 soft refresh (and vice versa) when chip bootstrap left details null. + // Separate seq for phase-2 fetches so they can't cancel a concurrent phase-1 soft refresh (and vice versa). const detailsSeqRef = useRef(0) - // The (seq, prNumber) of the phase-2 fetch currently in flight. The hub's - // fill-in effect fires as soon as phase 1 renders ready with null details — - // exactly when load()'s own phase 2 just started. Without this claim, every - // cold PR-segment open fetched the heavy details payload twice. + // (seq, prNumber) of the in-flight phase-2 fetch; without this claim every cold PR-segment open fetched the heavy details twice. const detailsInFlightRef = useRef<{ seq: number; prNumber: number } | null>(null) const stateIdentityRef = useRef(null) const stateRef = useRef(state) stateRef.current = state const headShaRef = useRef(headSha) - // Repo eligibility (a GitHub remote) is independent of the branch, so the probe - // must not require one: a detached HEAD / mid-rebase worktree (branch === null) - // would otherwise never set repoProbeLoaded, stranding the PR segment on a - // forever spinner instead of the "Current branch unavailable" state. + // Probe is branch-independent (repo eligibility is): requiring a branch would strand a detached-HEAD worktree on a forever spinner. const probeReady = client !== null && connState === 'connected' const identity = buildMobilePrSidebarIdentity({ worktreeId, branch }) @@ -91,9 +77,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { } }, [client]) - // Probe whether this is a GitHub repo to decide icon availability (GitHub-only). - // Worktree change must reset eligibility; a brief disconnect must not — otherwise - // the hub shows "unavailable for this provider" and hides the chip mid-session. + // Probe GitHub-repo eligibility for the icon; a worktree change resets it, a brief disconnect must not (else the chip hides mid-session). useEffect(() => { setIsGithubRepo(false) setRepoProbeLoaded(false) @@ -112,8 +96,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { } }) .catch(() => { - // Why: sendGithubPrRead already normalizes throws, but a cancelled - // unmount + any unexpected rejection must not surface as LogBox. + // Why: sendGithubPrRead normalizes throws, but a stray rejection on unmount must not surface as LogBox. if (!cancelled) { setIsGithubRepo(false) setRepoProbeLoaded(true) @@ -133,8 +116,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { return } if (stateIdentityRef.current !== null && stateIdentityRef.current !== identity) { - // Why: ready/loading data is scoped to branch. A branch switch must not let - // the open panel keep rendering the previous PR as "fresh." + // Why: data is scoped to branch; a branch switch must not keep rendering the previous PR as "fresh." loadSeqRef.current += 1 detailsSeqRef.current += 1 stateIdentityRef.current = null @@ -152,16 +134,10 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { } const seq = loadSeqRef.current + 1 loadSeqRef.current = seq - // In-flight phase-2 work is NOT invalidated here: this load only takes phase-2 - // ownership when its own phase 2 actually starts (the bump below), so a load - // superseded at the phase-1 guard can never orphan a detailsSeq and silently - // discard the only details fetch. A stale ensure applying mid-phase-1 is safe — - // its identity/number/kind guards only let matching details through. + // Don't invalidate in-flight phase 2 here: it's only claimed when this load's own phase 2 starts, so a superseded phase-1 load can't orphan the details fetch. const previousIdentity = stateIdentityRef.current stateIdentityRef.current = loadIdentity - // Soft refresh: same branch already showing ready/none stays visible while - // checks re-fetch (head advanced after commit). Hard loading only on first load - // or after a real identity wipe. + // Soft refresh: same-branch ready/none stays visible while checks re-fetch; hard "loading" only on first load or identity wipe. const keepVisible = previousIdentity === loadIdentity && (stateRef.current.kind === 'ready' || @@ -170,8 +146,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { if (!keepVisible) { setState({ kind: 'loading' }) } - // Phase 1: PR + checks (fast) — the worktree linkedPR read is parallelized with - // forBranch inside loadPrSidebarData so a closed/merged linked PR still resolves. + // Phase 1: PR + checks (fast); linkedPR read runs in parallel with forBranch so a closed/merged linked PR still resolves. const next = await loadPrSidebarData(deps, { worktreeId, branch, headSha }) if ( !shouldApplyResult(seq, loadSeqRef.current) || @@ -181,9 +156,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { } stateIdentityRef.current = loadIdentity - // Keep prior comments/body visible across phase 1 when the same PR is still open. - // loadPrSidebarData always returns details:null; without this, soft refresh and - // PR-tab refresh blank the comment tree until phase 2 finishes. + // Preserve prior details across phase 1 (loadPrSidebarData returns details:null) so soft/PR-tab refresh doesn't blank the comment tree. const priorDetails = next.kind === 'ready' && stateRef.current.kind === 'ready' && @@ -213,10 +186,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { if (detailsInFlightRef.current?.seq === detailsSeq) { detailsInFlightRef.current = null } - // Phase-2 ownership is encoded by detailsSeq + identity + PR number alone — - // deliberately NOT by loadSeq: a chip-only soft refresh bumps loadSeq without - // bumping detailsSeq, and must not discard the in-flight details it preserved - // (ensure dedupes against this claim, so nothing would re-fetch them). + // Ownership keyed on detailsSeq (not loadSeq): a chip-only soft refresh bumps loadSeq without detailsSeq and must not discard these details. if ( detailsSeq !== detailsSeqRef.current || stateIdentityRef.current !== loadIdentity || @@ -236,11 +206,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { [buildDeps, branch, headSha, identity, worktreeId] ) - // Phase-2 only — used when the hub opens the PR segment after a chip-only load. - // Uses detailsSeqRef (not loadSeqRef) so it cannot cancel a concurrent soft phase-1. - // Retries synthetic placeholders too: a failed phase-2 installs non-null empty - // details so Description/Comments leave the spinner, and without this ensure - // would never re-fetch on tab re-open. + // Phase-2-only fill-in; uses detailsSeqRef so it can't cancel a concurrent phase-1, and re-fetches non-null placeholders too. const ensurePrSidebarDetails = useCallback(async () => { const current = stateRef.current if (current.kind !== 'ready' || !prSidebarDetailsNeedFetch(current.data.details)) { @@ -252,8 +218,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { return } const prNumber = current.data.pr.number - // A live phase-2 fetch for this PR is already in flight (its claim still owns - // the latest details seq) — do not start a duplicate. + // Skip if a live phase-2 fetch for this PR already owns the latest details seq (dedupe). const inFlight = detailsInFlightRef.current if (inFlight && inFlight.prNumber === prNumber && inFlight.seq === detailsSeqRef.current) { return @@ -284,18 +249,13 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { }) }, [buildDeps, identity, worktreeId]) - // Soft-refresh checks when HEAD advances on the same branch (post-commit). - // Also restarts an in-flight phase-1 load so a mid-flight head advance is not - // applied with a stale SHA (headShaRef would otherwise advance with no reload). + // Soft-refresh on same-branch HEAD advance; restart in-flight load so the advance isn't applied with a stale SHA. useEffect(() => { if (headShaRef.current === headSha) { return } headShaRef.current = headSha - // Identity was just wiped (branch/worktree switch): stateRef still holds the - // pre-wipe state in this effect flush, which would start a load flavored for - // the OLD surface (e.g. heavy details on the Changes tab). Let the owning - // surface's hidden-state effects drive the first load for the new identity. + // Identity just wiped: stateRef still holds stale pre-wipe state, so let the surface's hidden-state effects drive the new load. if (stateIdentityRef.current === null) { return } diff --git a/mobile/src/source-control/MobileSourceControlPanel.tsx b/mobile/src/source-control/MobileSourceControlPanel.tsx index 25656aae2..9c3ebb270 100644 --- a/mobile/src/source-control/MobileSourceControlPanel.tsx +++ b/mobile/src/source-control/MobileSourceControlPanel.tsx @@ -46,16 +46,13 @@ export function MobileSourceControlPanel({ onOpenedFileDiff }: MobileSourceControlPanelProps) { const [activeTab, setActiveTab] = useState(initialTab) - // Track first visit so Changes/History keep scroll state across segment switches - // without paying the mount cost until the user actually opens them. PR unmounts - // when inactive (WebViews / comment tree) but its controller state stays for the chip. + // Track first visit so Changes/History stay mounted (keep scroll) after first open; PR still unmounts when inactive. const [visitedTabs, setVisitedTabs] = useState>( () => new Set([initialTab]) ) const [historyRefreshNonce, setHistoryRefreshNonce] = useState(0) - // Deep-link / push with a different `tab` param should adopt the new segment - // (expo-router can reuse the screen instance when only query params change). + // expo-router reuses the screen instance when only query params change, so adopt the new tab. useEffect(() => { setActiveTab(initialTab) setVisitedTabs((prev) => { @@ -119,13 +116,7 @@ export function MobileSourceControlPanel({ const ioBusy = busyAction !== null || openingPath !== null || openingBranchPath !== null const ready = screenState.kind === 'ready' - // One PR controller feeds both the branch-card chip and the Pull Request - // segment, so the chip's rollup can never disagree with the checks list it - // links to. Branch + head come from the already-loaded git.status — no second - // status read. The chip loads independently, so it never blocks the file list. - // Keep last-known identity across a transient status unload (disconnect / - // failed refresh) so the controller does not wipe ready → hidden → cold start. - // Head SHA matches the review path: status.head ?? branchCompare headOid. + // Keep last-known branch/head across a transient status unload so the PR controller isn't wiped ready → hidden → cold start. const lastPrBranchRef = useRef(null) const lastPrHeadRef = useRef(null) useEffect(() => { @@ -134,8 +125,7 @@ export function MobileSourceControlPanel({ }, [worktreeId]) const statusBranch = status?.branch ?? null const statusHead = status?.head ?? branchCompareResult?.summary.headOid ?? null - // Write last-known identity in an effect, not the render body: a discarded - // concurrent render must not leave the fallback holding a never-committed value. + // Write last-known identity in an effect, not render: a discarded concurrent render must not leave the fallback stale. useEffect(() => { if (statusBranch) { lastPrBranchRef.current = statusBranch @@ -163,8 +153,7 @@ export function MobileSourceControlPanel({ const ensurePrDetailsRef = useRef(ensurePrDetails) ensurePrDetailsRef.current = ensurePrDetails - // Chip bootstrap: phase-1 only (PR + checks). Full comment payload waits until - // the Pull Request segment is open — opening SC to stage must not pull details. + // Chip bootstrap: phase-1 only (PR + checks); full payload waits for the PR segment so opening SC to stage doesn't pull details. useEffect(() => { if (activeTab === 'pr') { return @@ -174,22 +163,14 @@ export function MobileSourceControlPanel({ } }, [activeTab, prBranch, isHostedRepo, prSidebarKind]) - // Number of the ready PR whose phase-2 details are still missing (null when - // none). Keyed by PR number — not a boolean — so a same-branch PR swap during a - // chip-only soft refresh re-arms phase 2 for the new PR instead of leaving its - // comments on a forever-spinner (a stale ensure bails on the number mismatch). - // Placeholder details (failed phase 2) also count as missing so reopening the - // PR tab retries instead of leaving empty Description/Comments forever. + // Keyed by PR number, not a boolean, so a same-branch PR swap re-arms phase 2 for the new PR. const prDetailsMissingFor = prController.prSidebarState.kind === 'ready' && prSidebarDetailsNeedFetch(prController.prSidebarState.data.details) ? prController.prSidebarState.data.pr.number : null - // PR segment: full load or phase-2 fill-in. Body only mounts while active. - // Guarded (and keyed) on prBranch so a branch that arrives while the segment is - // already open — e.g. mounted on a detached HEAD, then checkout — still loads; - // kind stays 'hidden' through that transition, so kind alone can't re-fire this. + // PR segment full load / phase-2 fill-in; key on prBranch so a branch arriving while open still loads (kind stays 'hidden'). useEffect(() => { if (activeTab !== 'pr' || !isHostedRepo || !prBranch) { return @@ -204,9 +185,7 @@ export function MobileSourceControlPanel({ }, [activeTab, isHostedRepo, prBranch, prSidebarKind, prDetailsMissingFor]) const prChip = useMemo(() => { - // No branch (detached HEAD / mid-rebase) never runs a PR load, so the shared - // state stays 'hidden' — which the chip would render as a forever spinner. - // Hide the chip instead; the Pull Request segment shows "branch unavailable". + // No branch (detached HEAD / mid-rebase) never loads a PR, so state stays 'hidden'; hide the chip or it spins forever. if (!isHostedRepo || !prBranch) { return null } @@ -217,9 +196,7 @@ export function MobileSourceControlPanel({ return buildMobilePrChipSummary(prController.prSidebarState, commentCount) }, [isHostedRepo, prBranch, prController.prSidebarState]) - // Design: refresh the active segment's body work, plus git.status for the shared - // branch card (counts/sync stay honest even while on History). Preserve ready - // status on a failed refresh so PR chip identity is not wiped to hidden. + // Refresh the active segment plus git.status (branch card stays honest on History); preserve ready on failure so the PR chip isn't wiped. const onRefresh = useCallback(() => { void loadStatus({ preserveReadyOnFailure: true }) if (activeTab === 'history') { @@ -237,11 +214,9 @@ export function MobileSourceControlPanel({ void refetchPr({ includeDetails: false }) }, [activeTab, isHostedRepo, loadStatus, refetchPr]) - // Embedded mode docks beside the terminal: close the dock instead of popping - // a route, and skip the full-screen safe-area chrome (the dock column owns it). + // Embedded mode docks beside the terminal: close the dock instead of popping a route; skip safe-area chrome (the dock column owns it). const onBack = embedded ? (onRequestClose ?? (() => router.back())) : () => router.back() - // Chromeless PR body has no panel header — surface open-on-web on the hub chrome - // while the Pull Request segment is active (same affordance as the old /pr route). + // Chromeless PR body has no header, so surface open-on-web on the hub chrome while the PR segment is active. const prWebUrl = activeTab === 'pr' && prController.prSidebarState.kind === 'ready' && @@ -277,10 +252,7 @@ export function MobileSourceControlPanel({ { - // Why: retrying the request is useless while the transport's - // reconnect loop is parked at its give-up cap — revive the - // connection instead (issue #5049). loadStatus re-runs via - // its connState effect once the new client connects. + // Why: a parked reconnect loop makes retry useless — revive the connection instead (issue #5049); loadStatus re-runs on reconnect. if (connState !== 'connected' && hostId) { void forceReconnect(hostId) return @@ -294,13 +266,10 @@ export function MobileSourceControlPanel({ ) : null - // History only needs the RPC client — do not block it behind git.status. - // Changes/PR need status (branch, file list, head SHA), so they stay gated. + // History only needs the RPC client, so it isn't gated on git.status; Changes/PR need status (branch, files, head). const showChanges = ready && (activeTab === 'changes' || visitedTabs.has('changes')) const showHistory = activeTab === 'history' || visitedTabs.has('history') - // Why: only mount the PR body while its segment is active. Keep-mounting retained - // Mermaid WebViews and re-rendered the comment tree on every commit keystroke. - // Controller + chip state still live for instant re-open without a full cold start. + // Only mount PR body while active: keep-mounting retained Mermaid WebViews and re-rendered the comment tree per keystroke. const showPrBody = ready && activeTab === 'pr' const conflictOperation = status?.conflictOperation ?? null // Git status always reports a conflictOperation enum; 'unknown' means none. @@ -319,10 +288,7 @@ export function MobileSourceControlPanel({ - {/* Branch card + PR chip are the Changes/Commits glance layer. On the PR - tab they duplicate the ready PR body (#, state, checks rollup, branch - trajectory), so hide the whole card there and let the PR panel own it — - unless a merge/rebase conflict is active, which only this card can abort. */} + {/* On the PR tab the branch card duplicates the ready PR body, so hide it there — unless a conflict is active, which only this card can abort. */} {ready && (activeTab !== 'pr' || hasActiveConflict) ? ( diff --git a/mobile/src/terminal/terminal-viewport-refit.ts b/mobile/src/terminal/terminal-viewport-refit.ts index e54c7c477..6194fabc6 100644 --- a/mobile/src/terminal/terminal-viewport-refit.ts +++ b/mobile/src/terminal/terminal-viewport-refit.ts @@ -28,14 +28,9 @@ type TerminalViewportRefitOptions = { initializedHandlesRef: RefObject> connState: ConnectionState tabStripVisible: boolean - // Why: terminal text size (font scale) — changing it changes the cell size, so - // the PTY must be re-fitted to a new column count and reflowed. + // Why: text size (font scale); changing it changes cell size, so the PTY must be re-fitted to a new column count. textScale: number - // Why: the terminal's measured frame width changes when a side panel docks/undocks - // or EITHER sidebar is drag-resized (the left worktree sidebar shrinks the detail - // pane; the right dock takes a slice of the row) — all without any window-dim or - // tab-strip change. Carries that measured width so those resizes re-fit the PTY; - // the 150ms debounce coalesces the stream of drag widths into one settle-time refit. + // Why: measured frame width; panel dock/undock or sidebar resize changes it with no window/tab change, so it re-fits the PTY. terminalFrameWidth: number unsubscribeTerminal: (handle: string) => void subscribeToTerminal: (handle: string) => void @@ -46,12 +41,7 @@ type TerminalViewportRefitNotifications = { notifyKeyboardVisibility: (visible: boolean) => void } -// Why: re-measure the phone viewport when layout-affecting state changes -// outside the subscribe path — the tab strip toggling visibility, and the -// window itself resizing (fold/unfold on foldables, orientation rotation, -// split-screen). Without the resize trigger, a PTY fitted on the folded -// cover screen stays at cover-screen cols after unfolding and the terminal -// renders in only part of the display (#4579's "cut in half" symptom). +// Why: re-measure on layout changes outside the subscribe path (tab strip, fold/rotate/resize), or a PTY renders "cut in half" (#4579). export function useTerminalViewportRefit( options: TerminalViewportRefitOptions ): TerminalViewportRefitNotifications { @@ -82,9 +72,7 @@ export function useTerminalViewportRefit( keyboardVisible: false, pending: false }) - // Why: marks the currently-armed timer as a height refit so its callback can - // re-check the keyboard at fire time. Non-height refits (width/rotation and - // the forced reconnect/foreground re-asserts) stay unguarded so they always run. + // Why: marks the armed timer as a height refit so its callback re-checks the keyboard; other refits always run unguarded. const heightOriginatedRefitRef = useRef(false) const scheduleViewportRefit = useCallback( (options?: { heightOriginated?: boolean }) => { @@ -94,9 +82,7 @@ export function useTerminalViewportRefit( heightOriginatedRefitRef.current = options?.heightOriginated ?? false refitTimerRef.current = setTimeout(() => { refitTimerRef.current = null - // Why: a height refit deferred at keyboard-close can fire after the keyboard - // reopened within the 150ms debounce; re-check and re-defer so we never - // reflow the PTY mid-keystroke. Scoped via the height-originated flag. + // Why: a height refit can fire after the keyboard reopened within the debounce; re-check so we never reflow the PTY mid-keystroke. if (heightOriginatedRefitRef.current) { heightOriginatedRefitRef.current = false const decision = reduceTerminalFrameHeightRefit(frameHeightRefitStateRef.current, { @@ -143,11 +129,7 @@ export function useTerminalViewportRefit( } 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. + // Why: prefer in-place updateViewport over resubscribe to keep the mobile subscriber record alive. See docs/mobile-presence-lock.md. const rpc = clientRef.current const deviceToken = deviceTokenRef.current if (rpc && deviceToken && updateViewportCapabilityRef.current !== 'unsupported') { @@ -165,11 +147,7 @@ export function useTerminalViewportRefit( if (isTerminalUpdateViewportUpdated(response)) { rpc.updateTerminalSubscriptionViewport(handle, dims) if (isTerminalUpdateViewportApplied(response)) { - // Why: updateViewport reflows the server PTY and re-streams only - // the visible screen, so the WebView's local xterm scrollback - // stays wrapped at the old width. Reflow it locally only when - // the server actually applied phone-fit; desktop mode records - // the viewport but leaves the PTY at desktop dims. + // Why: updateViewport re-streams only the visible screen, so local scrollback stays wrapped at the old width — reflow it locally. ref.reflow(dims.cols, dims.rows) } return @@ -205,14 +183,7 @@ export function useTerminalViewportRefit( scheduleViewportRefit() }, [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) re-runs against the new layout. - // Also schedule an explicit refit to cover the case where no new - // subscribe is happening. + // Why: the tab strip toggles at the 1↔2 terminal boundary (~40px area change), so the cached viewport goes stale. const prevTabStripVisibleRef = useRef(tabStripVisible) useEffect(() => { if (prevTabStripVisibleRef.current === tabStripVisible) { @@ -223,10 +194,7 @@ export function useTerminalViewportRefit( scheduleViewportRefit() }, [tabStripVisible, viewportMeasuredRef, scheduleViewportRefit]) - // Why: fold/unfold and rotation change the window dimensions without any - // subscribe or tab-strip transition. The PTY must be re-fitted to the new - // viewport or the terminal keeps the old grid (fit scale is capped at 1, - // so a grown window leaves the surface pinned to a fraction of the screen). + // Why: fold/unfold and rotation change window dims with no subscribe/tab change; refit or the grid stays stale (fit capped at 1). const { width: windowWidth, height: windowHeight } = useWindowDimensions() const prevWindowDimsRef = useRef({ width: windowWidth, height: windowHeight }) useEffect(() => { @@ -235,8 +203,7 @@ export function useTerminalViewportRefit( return } prevWindowDimsRef.current = { width: windowWidth, height: windowHeight } - // Why: adjustResize can change only window height while the IME is open; - // the frame-height notifier schedules one correction after it closes. + // Why: adjustResize can change only window height while the IME is open; the frame-height notifier corrects once it closes. if (prev.width === windowWidth && frameHeightRefitStateRef.current.keyboardVisible) { return } @@ -244,10 +211,7 @@ export function useTerminalViewportRefit( scheduleViewportRefit() }, [windowWidth, windowHeight, viewportMeasuredRef, scheduleViewportRefit]) - // Why: the text size changed, so the WebView is re-rendering at a new font/cell - // size. Re-measure and resize the PTY so the server reflows to the new column - // count. The refit's own 150ms debounce gives the WebView a frame to apply the - // new fontSize before we measure the resulting cell metrics. + // Why: on text-size change the refit's 150ms debounce lets the WebView apply the new fontSize before we re-measure cell metrics. const prevTextScaleRef = useRef(textScale) useEffect(() => { if (prevTextScaleRef.current === textScale) { @@ -258,10 +222,7 @@ export function useTerminalViewportRefit( scheduleViewportRefit() }, [textScale, viewportMeasuredRef, scheduleViewportRefit]) - // Why: the terminal's measured frame width changes when a panel docks/undocks or - // either sidebar is drag-resized — none of which touch the window dims or tab - // strip — so the cached viewport goes stale and the PTY keeps the pre-resize - // width. Mark un-measured and refit when the measured width changes. + // Why: panel dock/undock or sidebar resize changes frame width with no window/tab change, so the cached viewport goes stale. const prevFrameWidthRef = useRef(terminalFrameWidth) useEffect(() => { if (prevFrameWidthRef.current === terminalFrameWidth) { @@ -284,8 +245,7 @@ export function useTerminalViewportRefit( }, [viewportMeasuredRef, scheduleViewportRefit] ) - // Why: notify imperatively so layout churn does not rerender the full session; - // a height change during typing is coalesced into one refit after keyboard close. + // Why: notify imperatively so layout churn doesn't rerender the full session. const notifyTerminalFrameHeight = useCallback( (height: number) => notifyFrameHeightRefitEvent({ type: 'frame-height', height }), [notifyFrameHeightRefitEvent] @@ -310,8 +270,7 @@ export function useTerminalViewportRefit( if (!shouldRefit) { return } - // Why: the cached grid can match while the host PTY changed in background; - // reasserting equal dimensions is the convergence signal after iOS resume. + // Why: cached grid can match while the host PTY changed in background; reassert equal dims to converge after iOS resume. viewportMeasuredRef.current = false scheduleForcedViewportRefit() }) @@ -325,11 +284,9 @@ export function useTerminalViewportRefit( if (previous === 'connected' || connState !== 'connected') { return } - // Why: an in-place desktop upgrade may add updateViewport; reconnect is the - // narrow boundary where an old-host method_not_found cache becomes stale. + // Why: an in-place desktop upgrade may add updateViewport; reconnect is where the cached method_not_found goes stale. updateViewportCapabilityRef.current = 'unknown' - // Why: reconnect can restore a PTY whose host-side size changed while the - // socket was down, so equal cached dimensions still need reassertion. + // Why: reconnect can restore a PTY resized while the socket was down, so equal cached dims still need reassertion. viewportMeasuredRef.current = false scheduleForcedViewportRefit() }, [connState, viewportMeasuredRef, scheduleForcedViewportRefit]) diff --git a/mobile/src/terminal/terminal-webview-html.ts b/mobile/src/terminal/terminal-webview-html.ts index e9d188b74..a4d9c350c 100644 --- a/mobile/src/terminal/terminal-webview-html.ts +++ b/mobile/src/terminal/terminal-webview-html.ts @@ -1,5 +1,4 @@ -// xterm.js WebView document + default Tokyonight theme. Extracted from -// TerminalWebView.tsx to keep that file within the max-lines budget. +// xterm.js WebView document + default Tokyonight theme; extracted from TerminalWebView.tsx for the max-lines budget. import type { RuntimeMobileTerminalTheme } from '../../../src/shared/runtime-types' import { colors } from '../theme/mobile-theme' import { TERMINAL_TEXT_SCALES } from '../storage/preferences' @@ -38,15 +37,7 @@ const DEFAULT_TERMINAL_THEME: RuntimeMobileTerminalTheme['theme'] = { brightWhite: '#c0caf5' } -// Why: TUI apps (Claude Code / Ink) emit escape codes with absolute cursor -// positioning designed for the desktop's terminal dimensions (~150+ cols). -// We initialize xterm at the desktop's exact cols/rows so those escape codes -// render correctly, then use a measured CSS transform: scale() to fit the -// canvas into the phone viewport. The scale is computed after xterm opens -// by measuring the rendered surface width, not hardcoded, so it adapts to -// any terminal column count (80, 150, 200+). All touch gestures (scroll, -// pinch-to-zoom, pan) are handled by custom JS rather than native WebView -// behavior, so they work correctly with the CSS scale transform. +// Why: TUI escape codes assume the desktop's cols/rows, so init xterm at those dims and fit the phone via a measured CSS scale() instead of resizing. export const XTERM_HTML = ` @@ -1892,6 +1883,5 @@ ${TERMINAL_WEBGL_RECOVERY_JS} ` -// Why: WebView treats source identity as page identity on some platforms; keep -// parent/session re-renders from reloading xterm and forcing fresh snapshots. +// Why: some WebViews treat source identity as page identity; keep this stable so re-renders don't reload xterm. export const XTERM_WEBVIEW_SOURCE = { html: XTERM_HTML } diff --git a/mobile/src/transport/client-context.tsx b/mobile/src/transport/client-context.tsx index d9e07be40..80ec7f11c 100644 --- a/mobile/src/transport/client-context.tsx +++ b/mobile/src/transport/client-context.tsx @@ -1,14 +1,5 @@ -// 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. +// Single shared RpcClient per host, collapsing the old per-screen WebSocket connections. +// Design: docs/mobile-shared-client-per-host.md. import { createContext, useCallback, @@ -42,40 +33,28 @@ export type RpcClientContextValue = { closeHost: (hostId: string) => void getState: (hostId: string) => ConnectionState getReconnectAttempt: (hostId: string) => number - // Why: timestamp (ms epoch) of the last successful 'connected' state - // transition for this host, or null if never connected this session. - // Used by the UI to escalate "Reconnecting…" into a "host appears - // unreachable, re-pair?" prompt. + // Why: ms-epoch of the last 'connected' (null if never this session); UI escalates "Reconnecting…" into a re-pair prompt. getLastConnectedAt: (hostId: string) => number | null getActivePath: (hostId: string) => MobileConnectionPath 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). + // Why: lets the home screen feed already-loaded HostProfiles so we don't pay loadHosts() latency twice. 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. + // Why: entries in a ref so state changes don't re-render the whole tree; propagation goes through per-host listener Sets. 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. + // Pending opens keyed by hostId so two acquire() callers in the same render don't race the host lookup. const pendingOpensRef = useRef(new HostClientOpenRegistry()) - // 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. + // Why: cache of already-loaded HostProfiles so openEntry can skip a second loadHosts()/Keychain pass on cold start. const primedHostsRef = useRef>(new Map()) function notifyHostState(hostId: string, state: ConnectionState) { @@ -118,28 +97,20 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { const pendingOpen = pendingOpensRef.current.register(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. + // Why: prefer the primed cache so we don't serialize a second Keychain pass 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. + // Why: cold-start Keychain failure (iOS mid-unlock / Android Keystore race); surface 'disconnected' so the user can Reconnect. notifyHostState(hostId, 'disconnected') notifyAllHosts() return null } if (!host) { - // Why: returning silently leaves mounted screens on a permanent - // spinner (STA-1511) — surface 'disconnected' so they can render - // their waiting/retry affordance instead. + // Why: silent return leaves screens on a permanent spinner (STA-1511); surface 'disconnected' so they show a retry affordance. notifyHostState(hostId, 'disconnected') notifyAllHosts() return null @@ -160,9 +131,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { try { client = openHostLogicalClient(host, (entry) => connectionLogStore.append(hostId, entry)) } 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. + // Why: openHostLogicalClient can throw synchronously (bad public key / invalid URL); notify so the UI leaves 'connecting'. notifyHostState(hostId, 'disconnected') notifyAllHosts() return null @@ -191,12 +160,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { } }, []) - // 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. + // Synchronous get-or-open: returns an existing client immediately, else kicks off an async open and returns null this tick. const acquire = useCallback( (hostId: string, host?: HostProfile): RpcClient | null => { if (host) { @@ -207,9 +171,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { existing.refCount += 1 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. + // Trigger async open; returns null this tick — consumers re-call acquire() from an effect that re-runs on state changes. void openEntry(hostId).then((entry) => { if (!entry) { return @@ -227,14 +189,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { } }, []) - // Why: refcount dropping to 0 no longer triggers an idle-close. The - // app deliberately keeps live WebSockets open while the app itself is - // foregrounded — closing on transient navigation gaps was producing - // false 'disconnected' flashes when the user navigated home → host → - // back to home faster than React could re-acquire on the home side. - // Connections still close on: explicit user Disconnect, host removal, - // app backgrounding (OS-level socket suspension), and provider - // unmount (app shutdown). + // Why: no idle-close on refcount→0 — transient nav gaps flashed false 'disconnected', so keep sockets alive while foregrounded. const release = useCallback((hostId: string) => { const entry = storeRef.current.get(hostId) if (!entry) { @@ -246,10 +201,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { const forceReconnect = useCallback( async (hostId: string) => { const entry = storeRef.current.get(hostId) - // Why: preserve refcount across the swap. If the user reaches - // forceReconnect via the Disconnect → Reconnect path, the entry - // was already closed and refCount=0; fall back to active listener - // count as a proxy for "screens still watching this host." + // Why: preserve refcount across the swap; via Disconnect→Reconnect the entry is already gone, so fall back to listener count. const listenerCount = stateListenersRef.current.get(hostId)?.size ?? 0 const savedRefCount = entry?.refCount ?? Math.max(1, listenerCount) if (entry) { @@ -318,16 +270,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { } }, []) - // Close all clients on provider unmount (app shutdown). - // Why: deps must be empty so this cleanup ONLY runs on real unmount. - // Hot-reload re-evaluates this module, which makes closeEntry a new - // function identity. With [closeEntry] as deps, every Fast Refresh - // would tear down all open WebSockets, leaving screens holding closed - // clients and the user staring at a 'Reconnecting…' card. Reading - // storeRef.current and the locally-scoped closeEntry inside the - // cleanup is safe — the ref is stable across renders, and the - // function captured here will be the one defined in the same - // closure as this effect. + // Close all clients on provider unmount. Empty deps: [closeEntry] would let Fast Refresh tear down all live sockets. // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => { const store = storeRef.current @@ -339,9 +282,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) { } }, []) - // Why: nudge every live client when the OS signals the link may be back - // (foreground, network restored/switched) so sessions recover without an - // app restart (issue #5049). + // Why: nudge live clients when the OS signals the link may be back so sessions recover without a restart (issue #5049). useEffect(() => { return subscribeConnectionRevivalTriggers(() => { for (const entry of storeRef.current.values()) { @@ -392,9 +333,7 @@ export function useRpcClientContext(): RpcClientContextValue { 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. +// Primary hook for screens: acquires the shared client on mount, releases on unmount, re-renders on state change. export function useHostClient(hostId: string | undefined): { client: RpcClient | null state: ConnectionState @@ -419,17 +358,13 @@ export function useHostClient(hostId: string | undefined): { return } setState(next) - // Why: the client materialises after an async open, and forceReconnect - // swaps in a fresh client object. Re-read on every state change so a - // mounted screen never keeps driving a stale (closed) client. + // Why: async open and forceReconnect swap the client object; re-read each state change so screens never drive a stale one. const found = ctx.getAllClients().find((entry) => entry.hostId === hostId) if (found && found.client !== clientRef.current) { clientRef.current = found.client force((n) => n + 1) } else if (!found && clientRef.current) { - // Why: closeHost deletes the entry without a replacement; holding the - // closed client would let screens keep issuing requests that can never - // resolve (STA-1511). Null it so they render disconnected states. + // Why: closeHost deletes the entry with no replacement; null it so screens don't drive a dead client (STA-1511). clientRef.current = null force((n) => n + 1) } @@ -450,9 +385,7 @@ export function useHostClient(hostId: string | undefined): { 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. +// Why: refcounting prevents a double-open when a host-detail screen shares one of these hosts. export function useAllHostClients(hostIds: string[]): Array<{ hostId: string client: RpcClient @@ -510,9 +443,7 @@ export function useAllHostClients(hostIds: string[]): Array<{ }, [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. +// Why: host-store's removeHost() must close the live client but has no React-side handle; this hook bridges to it. export function useCloseHost(): (hostId: string) => void { const ctx = useRpcClientContext() return ctx.closeHost @@ -524,9 +455,7 @@ export function useForceReconnect(): (hostId: string) => Promise { 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. +// Why: primes already-loaded HostProfiles so the provider can skip a second loadHosts()/Keychain pass on cold start. export function usePrimeHosts(): (hosts: HostProfile[]) => void { const ctx = useRpcClientContext() return ctx.primeHosts diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index a404379e5..61ed6742d 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -23,16 +23,12 @@ import { deleteMobileRelayDirectUpgradeJournal } from './mobile-relay-direct-upg import { scheduleOrphanedMobileRelayCleanup } from './mobile-relay-orphan-cleanup' const STORAGE_KEY = 'orca:hosts' -// Why: SecureStore keys must match [A-Za-z0-9._-]; colons are rejected. -// Use dots as the separator so the key shape stays readable while -// satisfying the validator. +// Why: SecureStore keys must match [A-Za-z0-9._-] (colons rejected), so use dots as the separator. const TOKEN_KEY_PREFIX = 'orca.host-token.' const WEB_TOKEN_KEY_PREFIX = 'orca:web-host-token:' -// Why: WHEN_UNLOCKED_THIS_DEVICE_ONLY keeps the pairing token off -// iCloud Keychain and out of iCloud/iTunes backup restores onto a -// different physical device. Reads/writes are silent (no biometric -// prompt) since we don't request access control flags. +// Why: WHEN_UNLOCKED_THIS_DEVICE_ONLY keeps the pairing token off iCloud Keychain and out of backup restores onto another device. +// Reads/writes stay silent (no biometric prompt) because we don't request access control flags. const KEYCHAIN_OPTIONS: SecureStore.SecureStoreOptions = { keychainAccessible: SecureStore.WHEN_UNLOCKED_THIS_DEVICE_ONLY } @@ -46,8 +42,7 @@ function webTokenKey(hostId: string): string { } async function readDeviceToken(hostId: string): Promise { - // Why: Expo SecureStore has no working web backend; keep this fallback - // web-only so native builds still keep pairing tokens in the keychain. + // Why: Expo SecureStore has no working web backend; fall back to AsyncStorage only on web so native still uses the keychain. if (Platform.OS === 'web') { return AsyncStorage.getItem(webTokenKey(hostId)) } @@ -76,18 +71,10 @@ async function deleteHostCredentials(hostId: string): Promise { await deleteMobileRelayDirectUpgradeJournal(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). +// Why: Keychain reads are slow (50-200ms) and loadHosts() runs on every screen mount; cache per-hostId in memory, invalidate on save/remove. const tokenCache = new Map() let inflightLoad: Promise | null = null -// Why: rename / lastConnected / remove / save all RMW the same hosts JSON. -// Without a queue, concurrent writers re-read a stale snapshot and the last -// setItem wins — resurrecting a removed host or dropping a rename. +// Why: serialize RMW of the shared hosts JSON; without a queue concurrent writers drop writes (resurrect a removed host, drop a rename). let hostListMutation: Promise = Promise.resolve() function parseStoredHosts(raw: string | null): StoredHostProfile[] | null { @@ -100,9 +87,7 @@ function parseStoredHosts(raw: string | null): StoredHostProfile[] | null { return null } return parsed.flatMap((item) => { - // Why: pre-v0.0.3 records carry the deviceToken in AsyncStorage. - // Drop them silently — the three pre-launch users will re-pair on - // first run rather than carry a migration shim through the auth path. + // Why: pre-v0.0.3 records stored deviceToken in AsyncStorage; drop them (users re-pair) rather than carry a migration shim. if (item && typeof item === 'object' && 'deviceToken' in item) { return [] } @@ -115,11 +100,9 @@ function parseStoredHosts(raw: string | null): StoredHostProfile[] | null { } export async function loadHosts(): Promise { - // Why: writers hold the mutation chain across their full RMW; wait so a - // load right after rename/remove does not race a half-written list. + // Why: writers hold the mutation chain across their full RMW; wait so a load doesn't race a half-written list. await hostListMutation - // Why: deduplicate concurrent loadHosts() calls so multiple screens - // mounting simultaneously share one Keychain read pass. + // Why: deduplicate concurrent loadHosts() calls so simultaneously mounting screens share one Keychain read pass. if (inflightLoad) { return inflightLoad } @@ -152,16 +135,11 @@ async function doLoadHosts(): Promise { try { fetched = await readDeviceToken(stored.id) } catch { - // Why: a transient Keychain failure for one entry (e.g. - // errSecInteractionNotAllowed while the device is briefly locked, - // or a single corrupt record) must not blank the entire host list. - // Skip just this host — it'll reappear on the next load. + // Why: a transient Keychain failure for one entry (e.g. errSecInteractionNotAllowed while locked) must not blank the whole host list; skip it. continue } 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. + // Why: orphaned metadata with no matching keychain entry; skip rather than surface a half-broken host. continue } token = fetched @@ -187,8 +165,7 @@ export async function resolvePairingHostIdentity( publicKeyB64: string, newHostId: string ): Promise<{ id: string; name: string }> { - // Why: one durable read both preserves an existing identity and names a new host, - // avoiding duplicate cards and a second serial storage read before connecting. + // Why: one durable read both preserves an existing identity and names a new host, avoiding duplicate cards. await hostListMutation const hosts = await readStoredHostsForMutation() const match = hosts.find((host) => host.publicKeyB64 === publicKeyB64) @@ -201,8 +178,7 @@ async function readStoredHostsForMutation(): Promise { try { const parsed = parseStoredHosts(await AsyncStorage.getItem(STORAGE_KEY)) if (!parsed) { - // Why: refuse to RMW over unreadable payload — treating it as [] would - // wipe the durable host list on the next rename/remove/save. + // Why: refuse to RMW over unreadable payload — treating it as [] would wipe the durable host list on the next write. throw new Error('host list storage unreadable') } return parsed @@ -260,8 +236,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise } if (index >= 0) { updatedExistingHost = true - // Why: affected installs may already contain duplicate rows; an authoritative - // save is the safe point to collapse them to the preserved host id. + // Why: an authoritative save is the safe point to collapse pre-existing duplicate rows to the preserved host id. return hosts .filter(({ id }) => !duplicateHostIds.has(id)) .map((candidate) => (candidate.id === stored.id ? stored : candidate)) @@ -272,11 +247,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise } return [...hosts.filter(({ id }) => !duplicateHostIds.has(id)), stored] }) - // Why: write metadata BEFORE the keychain token so a crash between the two - // leaves orphaned metadata (which loadHosts skips and removeHost can clean - // up) rather than an orphaned keychain token with no metadata pointer — - // the latter would persist forever since removeHost only deletes by hostId - // from current metadata. + // Why: write metadata before the keychain token so a crash leaves recoverable orphaned metadata, not an orphaned token that persists forever. await writeDeviceToken(stored.id, validated.deviceToken) tokenCache.set(stored.id, validated.deviceToken) if (validated.endpoints) { @@ -293,8 +264,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise overlayRemovalIds.push(stored.id) } if (overlayRemovalIds.length > 0) { - // Why: reusing an id for direct-only re-pairing must not retain routing - // metadata from the host's previous transport state. + // Why: reusing an id for direct-only re-pairing must not retain routing metadata from the previous transport state. await removeMobileRelayHostOverlays(overlayRemovalIds) } for (const duplicateHostId of duplicateHostIds) { @@ -313,11 +283,9 @@ export async function removeHost(hostId: string): Promise { try { await removeMobileRelayHostOverlay(hostId) } catch { - // The missing legacy base is authoritative, so a retained overlay cannot - // resurrect this host and can be cleaned on a later explicit retry. + // Base removal is authoritative; a retained overlay can't resurrect the host and is cleaned on a later retry. } - // Why: await only the durable cleanup intent (AsyncStorage). Native keychain - // delete can reject or stall and must not freeze removeHost / the UI. + // Why: keychain delete can stall/reject; await only the durable cleanup intent so removeHost can't freeze the UI. try { await scheduleHostCredentialCleanup(hostId, deleteHostCredentials) } catch { @@ -333,10 +301,7 @@ export async function retryPendingHostCredentialCleanup(): Promise<{ return retryPendingHostCredentialCleanups(deleteHostCredentials) } -// Why: Edit host can change name and endpoint together; a single -// mutateStoredHosts pass keeps both fields committed atomically so a -// mid-save failure can never persist one change without the other, and a -// host removed mid-edit throws consistently instead of silently no-oping. +// Why: single mutation pass commits name + endpoint atomically so a mid-save failure can't persist one without the other. export async function updateHostNameAndEndpoint( hostId: string, updates: { name?: string; endpoint?: string } @@ -368,9 +333,7 @@ export async function updateLastConnected(hostId: string): Promise { return next }) } catch { - // Why: last-connected is a best-effort timestamp and callers fire it with - // `void`. Swallow unreadable-storage failures so they don't surface as an - // unhandled promise rejection. + // Why: best-effort timestamp fired with void; swallow so unreadable storage doesn't reject. } } diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 5420ba679..578e1a1ba 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -79,74 +79,36 @@ export type RpcClient = { viewport: { cols: number; rows: number } ) => 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. + // 0 means never failed (reset on successful open); the UI escalates "Reconnecting…" to "Can't connect" past a threshold. getReconnectAttempt: () => number - // Why: timestamp (ms epoch) of the last time we reached 'connected'. - // null = never connected since the client was created. Used by the UI - // to distinguish "host moved/never reachable" from "transient blip". + // Last 'connected' timestamp (ms epoch); null = never connected. Lets the UI tell "never reachable" from "transient blip". getLastConnectedAt: () => number | null onStateChange: (listener: (state: ConnectionState) => void) => () => void - // Why: app-resume hook. Android/iOS can kill the TCP path or park the - // reconnect loop while the app is backgrounded; callers invoke this on - // AppState 'active' so the session recovers without an app restart. + // Why: app-resume hook — iOS/Android can kill the TCP path while backgrounded; call on AppState 'active' to recover. notifyForeground: () => void close: () => void } -// Why: tiered backoff. The first four entries (500ms→4s) keep -// auto-recovery snappy for the common case — a brief Wi-Fi blip, -// laptop wake, or AP-isolation cycle. Beyond that we slow down -// (8s→60s) so a phone whose desktop is genuinely unreachable doesn't -// burn a TCP SYN every 4s indefinitely while still healing on its -// own when the network recovers. With 12 total attempts, the last -// four reuse the 60s cap (Math.min(idx, length-1)), so total elapsed -// time across all 12 attempts is ≈ 6 minutes before the give-up cap -// fires (0.5+1+2+4+8+15+30+60+60+60+60+60 ≈ 360s). +// Why: tiered backoff — fast early entries recover blips; the slow tail avoids burning a SYN every 4s on an unreachable desktop. const RECONNECT_DELAYS = [500, 1000, 2000, 4000, 8000, 15_000, 30_000, 60_000] -// Why: cap fast auto-retry once we're clearly unreachable for a long time. -// With the tiered backoff above this is ≈ 6 minutes of continuous -// failure before the UI surfaces the re-pair banner. The longer -// runway tolerates flaky AP-isolation routers and laptop sleep cycles -// that briefly drop the LAN path. MUST stay aligned with -// connection-health.ts UNREACHABLE_ATTEMPTS so the "unreachable" -// verdict matches the moment the loop slows to the trickle cadence. +// Why: ≈6 min of failure before the re-pair banner; MUST stay aligned with connection-health.ts UNREACHABLE_ATTEMPTS. const GIVE_UP_AFTER_ATTEMPTS = 12 -// Why: past the cap the loop must never park permanently. A wedged -// Tailscale/VPN tunnel produces no AppState or network-type transition -// (still Wi-Fi, still "online"), so no revival nudge ever fires — users -// had to toggle Tailscale off/on just to force one. A slow trickle dial -// self-heals once the tunnel recovers while staying cheap: one TCP -// attempt per 90s, foreground-only (iOS/Android suspend JS timers in -// the background). +// Why: never park past the cap — a wedged VPN fires no AppState/network nudge to revive it, so trickle-dial every 90s to self-heal. const TRICKLE_RECONNECT_DELAY_MS = 90_000 -// 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. +// Why: one unauthorized isn't proof the pairing is dead (issue #5200) — retry the handshake this many times before latching auth-failed. const AUTH_RETRY_BUDGET = 3 const REQUEST_TIMEOUT_MS = 30_000 const CONNECT_TIMEOUT_MS = 12_000 const HANDSHAKE_TIMEOUT_MS = 5_000 -// Why: RN's WebSocket implementation may not expose static readyState -// constants, but the protocol value for CONNECTING is stable across runtimes. +// Why: RN may not expose WebSocket.readyState constants, but the CONNECTING protocol value (0) is stable across runtimes. const WEBSOCKET_CONNECTING_STATE = 0 -// Why: RN auto-pongs WebSocket pings natively, so JS needs an app-level -// liveness probe to detect half-open sockets. Any inbound app traffic after -// a probe starts proves the link is alive; otherwise an unanswered probe -// force-closes the socket so the reconnect path can recover. +// Why: RN auto-pongs pings natively, so JS needs an app-level probe to detect half-open sockets. const ACTIVITY_PROBE_INTERVAL_MS = 20_000 export type ConnectOptions = { onStateChange?: (state: ConnectionState) => void - // Fires for every observable lifecycle event so the UI can render a - // detailed connection log. Useful when 'Connecting…' hangs forever - // (e.g. broken Tailscale route) and you need to see *where* it's stuck. + // Fires for every lifecycle event so the UI can show where 'Connecting…' is stuck (e.g. broken Tailscale route). onLog?: ConnectionLogSink } @@ -185,14 +147,10 @@ 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'. + // Consecutive auth rejections; tolerate up to AUTH_RETRY_BUDGET (issue #5200) before latching to avoid a needless re-pair. let authRejectionCount = 0 let lastConnectedAt: number | null = null - // Why: cheap diagnostics for RN/OkHttp process-state poisoning: do retry - // attempts differ, is anything inbound, and are closes instant or slow? + // Why: cheap diagnostics for RN/OkHttp process-state poisoning (retry cadence, inbound traffic, close timing). let lastInboundAt: number | null = null let inboundSequence = 0 let lastWsClosedAt: number | null = null @@ -200,7 +158,6 @@ export function connect( 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. let sharedKey: Uint8Array | null = null const serverPublicKey = publicKeyFromBase64(serverPublicKeyB64) @@ -218,9 +175,7 @@ export function connect( stateListeners.add(onStateChange) } - // Diagnostic: tracks how long we've been in the current state. Useful - // for spotting "stuck in connecting" or "stuck in reconnecting" cases - // in the logs. + // Diagnostic: dwell time in the current state, for spotting "stuck in connecting/reconnecting". let stateEnteredAt = Date.now() function rejectConnectWaiters(reason: string) { @@ -250,8 +205,7 @@ 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. + // Why: a clean handshake proves the token is valid — reset the auth retry budget. authRejectionCount = 0 for (const waiter of connectWaiters.splice(0)) { if (waiter.timeout) { @@ -269,8 +223,7 @@ export function connect( } } - // Why: don't dump device tokens / full URLs into log scrolls; truncate to - // the host:port so reconnect lifecycles are still readable. + // Why: keep device tokens / full URLs out of log scrolls — truncate to host:port. function redactedEndpoint(ep: string): string { try { const m = ep.match(/^wss?:\/\/([^/]+)/i) @@ -288,17 +241,13 @@ export function connect( return Promise.reject(new Error('Client closed')) } if (state === 'reconnecting' && reconnectAttempt >= GIVE_UP_AFTER_ATTEMPTS) { - // Why: past the retry cap the loop only trickles every 90s — callers - // must fail fast rather than hang on a host that's been unreachable - // for minutes. A trickle dial that succeeds flips state to 'connected' - // and later requests go through normally. + // Why: past the cap the loop only trickles every 90s — fail fast instead of hanging on a long-unreachable host. return Promise.reject(new Error('Connection retry limit reached')) } return new Promise((resolve, reject) => { const waiter: ConnectWaiter = { resolve, reject, timeout: null } if (timeoutMs !== undefined) { - // Why: explicit per-request timeouts must include offline/reconnect - // waiting, not only the RPC after the socket becomes connected. + // Why: per-request timeouts must cover offline/reconnect waiting, not just the RPC after connect. waiter.timeout = setTimeout( () => { const index = connectWaiters.indexOf(waiter) @@ -328,11 +277,7 @@ export function connect( console.log('[net] openConnection', { attempt: reconnectAttempt, 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. + // Why: diagnostic for RN/OkHttp pool corruption — high wsCount + repeated 1006 closes means process-state stuck. wsCount: wsConstructionCounter, msSinceLastConnected: lastConnectedAt != null ? now - lastConnectedAt : null, msSinceLastClose: lastWsClosedAt != null ? now - lastWsClosedAt : null, @@ -354,8 +299,7 @@ export function connect( if (ws === openingWs) { return false } - // Why: React Native can deliver callbacks from a timed-out socket after - // reconnect has swapped in a replacement; stale events must not mutate it. + // Why: RN can deliver callbacks from a timed-out socket after reconnect swapped in a replacement — ignore them. console.log('[net] stale ws event ignored', { eventName, state, @@ -364,9 +308,7 @@ export function connect( return true } - // Why: React Native can leave TCP/WebSocket opens pending indefinitely on - // flaky network handoffs. Force the existing onclose reconnect path if - // onopen never arrives, instead of leaving the UI stuck at "Connecting...". + // Why: RN can leave opens pending forever on flaky handoffs — force reconnect if onopen never arrives. connectTimer = setTimeout(() => { connectTimer = null if (ws === openingWs && openingWs.readyState === WEBSOCKET_CONNECTING_STATE) { @@ -396,9 +338,7 @@ export function connect( setState('handshaking') emitLog('success', 'WebSocket open', 'Starting E2EE handshake') - // Why: generate a fresh ephemeral keypair for each connection. - // This provides forward secrecy — compromising one session's key - // doesn't compromise past or future sessions. + // Why: fresh ephemeral keypair per connection provides forward secrecy. const ephemeral = generateKeyPair() const hello = JSON.stringify({ type: 'e2ee_hello', @@ -437,8 +377,7 @@ export function connect( lastInboundAt = Date.now() const raw = typeof rawData === 'string' ? rawData : null - // Why: during handshaking, e2ee_ready is plaintext because it precedes - // encrypted auth; e2ee_authenticated/e2ee_error are encrypted. + // Why: e2ee_ready is plaintext (precedes encrypted auth); e2ee_authenticated/e2ee_error are encrypted. if (state === 'handshaking') { if (raw === null) { return @@ -481,9 +420,7 @@ export function connect( removeStreamListener(id) continue } - // Why: setState('connected') notifies UI listeners synchronously; - // a listener may subscribe and send immediately before this - // reconnect replay loop resumes. + // Why: a UI listener notified synchronously by setState('connected') may already have sent this stream — skip it. if (stream.sent) { continue } @@ -515,8 +452,7 @@ export function connect( return } - // Why: guard against decrypt with an invalid key — sharedKey can be null - // after destroy() or if a message arrives during a reconnect race. + // Why: sharedKey can be null after destroy() or a reconnect race — don't decrypt with an invalid key. if (!sharedKey || sharedKey.length !== 32) { return } @@ -553,10 +489,7 @@ export function connect( } recordValidatedInboundTraffic() - // 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. + // Why: a mid-session unauthorized may be transient (issue #5200) — handleAuthRejection retries before latching auth-failed. if (!response.ok && response.error.code === 'unauthorized') { handleAuthRejection('Unauthorized — pairing may be revoked') return @@ -646,18 +579,12 @@ 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. + // Why: time-since-construct classifies the failure — instant close = RST/unreachable, slow = SYN timeout/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: statically imported (not closure-built) — an earlier hot-reload - // bug came from a stale closure capturing a half-loaded module. + // Why: statically imported — a hot-reload bug came from a stale closure capturing a half-loaded module. const closeEvent = describeSocketEvent(event) console.log('[net] ws.onclose', { code: e?.code, @@ -682,9 +609,7 @@ export function connect( if (ignoreStaleSocketEvent('error')) { return } - // Why: RN surfaces network errors here (DNS failure, TCP RST, etc). - // onclose fires right after, but logging the error message gives us - // the original cause that the close code alone can hide. + // Why: RN surfaces the original network error here — onclose follows but its close code alone hides the cause. const e = event as { message?: string } | undefined const errEvent = describeSocketEvent(event) console.log('[net] ws.onerror', { @@ -734,11 +659,7 @@ 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. + // Why: an auth rejection may be transient (issue #5200) — retry up to AUTH_RETRY_BUDGET times before latching auth-failed. function handleAuthRejection(reason: string): void { activeBrowserScreencastRequestId = null pendingBrowserScreencastRequestId = null @@ -754,9 +675,7 @@ export function connect( '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. + // Why: close without setting intentionallyClosed so handleSocketClosed routes to reconnect and retries the handshake. const closing = ws ws = null sharedKey = null @@ -782,19 +701,11 @@ export function connect( } function scheduleReconnect() { - // Why: spinning fast reconnects forever drains battery and floods logs - // when the host is genuinely unreachable (wrong IP, port closed, - // host moved). Past GIVE_UP_AFTER_ATTEMPTS the UI surfaces a - // "Can't reach desktop, re-pair?" banner and the loop drops to the - // 90s trickle cadence instead of parking — a permanently parked loop - // could only be revived by an AppState/network transition, which a - // wedged VPN tunnel never produces. + // Why: past the cap, trickle (never park) — a parked loop only revives on a network transition a wedged VPN never produces. const pastGiveUpCap = reconnectAttempt >= GIVE_UP_AFTER_ATTEMPTS let delay: number if (pastGiveUpCap) { - // Why: the counter holds at the cap — connection-health thresholds and - // the "Can't reach desktop" verdict key off attempts >= 12, and a - // successful open resets it to 0 anyway. + // Why: hold the counter at the cap — connection-health's "Can't reach desktop" verdict keys off attempts >= 12. delay = TRICKLE_RECONNECT_DELAY_MS rejectConnectWaiters('Connection retry limit reached') } else { @@ -824,10 +735,7 @@ export function connect( } } - // Why: app-level liveness probe — see ACTIVITY_PROBE_INTERVAL_MS comment - // at the top of the file. Fires while the channel is in 'connected' - // state, sends a tiny status.get, and force-closes the WS if the probe - // fails (which the existing onclose path then turns into a reconnect). + // Why: app-level liveness probe (see ACTIVITY_PROBE_INTERVAL_MS) — force-closes the WS on failure so onclose reconnects. function runActivityProbe() { if (state !== 'connected' || !ws) { return @@ -966,8 +874,7 @@ export function connect( removeStreamListener(id) return } - // Why: sent streams may still reply with `ready`; keep a tombstone so we - // can immediately unsubscribe. Queued streams never reached desktop. + // Why: a sent stream may still reply `ready`; keep the tombstone to unsubscribe it (queued streams never reached the desktop). if (!stream.sent) { removeStreamListener(id) } @@ -1013,11 +920,7 @@ export function connect( hasKey: !!sharedKey, state }) - // Why: if the state machine still thinks we're connected but the - // underlying WebSocket has flipped to CLOSING/CLOSED without onclose - // having fired (RN's WebSocket sometimes drops the event, or the - // server half-closed the stream), force a reconnect. Without this - // every send silently fails forever and the user sees a frozen UI. + // Why: RN can drop onclose, leaving state 'connected' over a dead socket; force reconnect or every send silently fails forever. if (state === 'connected' && ws && ws.readyState !== WebSocket.OPEN) { console.log('[net] sendEncrypted detected ws desync — forcing reconnect', { readyState: ws.readyState @@ -1125,9 +1028,7 @@ export function connect( if (pendingBrowserScreencastRequestId && pendingBrowserScreencastRequestId !== id) { disposeBrowserScreencastStream(pendingBrowserScreencastRequestId) } - // Why: browser screencast frames are connection-scoped and carry no - // stream id. Wait for the replacement stream's ready response before - // routing frames, so in-flight old-page pixels are dropped. + // Why: screencast frames carry no stream id, so route only after the new stream's ready to drop stale old-page pixels. pendingBrowserScreencastRequestId = id activeBrowserScreencastRequestId = null } @@ -1140,9 +1041,7 @@ export function connect( removeStreamListener(id) } } else { - // Stream is registered but the actual outbound subscribe will be - // sent (or re-sent) when the channel reaches 'connected'. Useful - // when terminals don't load — confirms the request is queued. + // Registered now; the outbound subscribe is (re-)sent once the channel reaches 'connected'. console.log('[net] subscribe queued — waiting for connected', { method, state }) } @@ -1157,12 +1056,7 @@ export function connect( return } if (stream?.method === 'terminal.subscribe') { - // Why: the runtime registers cleanup under the composite key - // `${terminal}:${clientId}` so two phones subscribing to the same - // terminal handle don't evict each other. Echo that composite key - // back on unsubscribe; also include `client.id` so the server can - // reconstruct it if a stale build emits a bare-handle id. See - // docs/mobile-presence-lock.md. + // Why: server keys cleanup by composite `${terminal}:${clientId}` so two phones don't evict each other. See docs/mobile-presence-lock.md. const unsubscribeParams = buildTerminalUnsubscribeParams(stream.params) if (unsubscribeParams) { sendEncrypted({ @@ -1211,20 +1105,14 @@ export function connect( return } if (state === 'connected') { - // Why: the OS can kill the TCP path while the app is backgrounded - // without delivering onclose, leaving a half-open socket that - // blackholes input. Probe now so death is detected in ≤8s instead - // of waiting out the 20s interval (issue #5049). + // Why: OS can kill the TCP path while backgrounded without onclose; probe now to detect the half-open socket in ≤8s (issue #5049). console.log('[net] foreground — probing live connection') startActivityProbe() runActivityProbe() return } if (state === 'reconnecting') { - // Why: while backgrounded the retry loop may be sitting on a 60s - // backoff or 90s trickle timer. Returning to the foreground is a - // strong user signal — restart with a fresh attempt budget - // immediately instead of waiting out the timer. + // Why: foreground is a strong user signal — restart immediately instead of waiting out a 60s/90s backoff timer. console.log('[net] foreground — restarting reconnect loop', { attempt: reconnectAttempt, hadTimer: !!reconnectTimer