diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index eedf3dbda78..af4e9e82776 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -38,6 +38,16 @@ const SUPPRESSED_REACT_DOCTOR_DIAGNOSTICS = new Map([ new Set([ 'src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts' ]) + ], + [ + // The rule wants one named handle cleared by name. Both startup effects arm a variable number + // of refresh timers, every one of them through addTimer into `timers`, which their cleanups + // clear -- a shape the rule reports whether the handles live in an array, a Set, or a nested + // helper. The finding predates this list; it surfaced when the effect body changed. This map + // keys on file, not line, so the entry covers both effects in it; nothing else in the file + // arms a timer, so widening it further is the only alternative, not a narrower option. + 'react-doctor(effect-needs-cleanup)', + new Set(['mobile/src/session/use-mobile-session-startup.ts']) ] ]) diff --git a/mobile/src/components/HostProtocolGate.test.ts b/mobile/src/components/HostProtocolGate.test.ts index 44a2265ccb3..b34e4baacaf 100644 --- a/mobile/src/components/HostProtocolGate.test.ts +++ b/mobile/src/components/HostProtocolGate.test.ts @@ -44,6 +44,12 @@ function GateConsumer() { return createElement('GateStatus', null, hostCapabilities.join(',')) } +// Separate from GateStatus so the capability assertions keep their exact rendered shape. +function VerifiedConsumer() { + const { compatVerified } = useHostProtocolGates() + return createElement('GateVerified', null, compatVerified ? 'verified' : 'unverified') +} + // Counts mounts so a test can prove the routes were never torn down, which presence alone can't. const probeMounts = { count: 0 } function MountProbe() { @@ -57,7 +63,13 @@ function gateElement() { return createElement( HostProtocolGate, { hostId: 'host-1' }, - createElement('HostContent', null, createElement(GateConsumer), createElement(MountProbe)) + createElement( + 'HostContent', + null, + createElement(GateConsumer), + createElement(VerifiedConsumer), + createElement(MountProbe) + ) ) } @@ -154,13 +166,90 @@ describe('HostProtocolGate', () => { expect(client.sendRequest).toHaveBeenCalledOnce() }) + it('serves every descendant capability read from the one status.get it issues', async () => { + const client = clientWithStatus({ + protocolVersion: 5, + minCompatibleMobileVersion: 0, + capabilities: ['browser.screencast.v1', 'terminal.queryReplyInput.v1'] + }) + hostClient.current = { client, state: 'connected' } + renderer = await act(async () => { + const created = create( + createElement( + HostProtocolGate, + { hostId: 'host-1' }, + createElement(GateConsumer), + createElement(GateConsumer) + ) + ) + await Promise.resolve() + return created + }) + + // Why: the session route used to run its own retrying status.get on top of this one, so a + // cold open cost two round trips for the same answer. Consumers now read the gate's copy. + expect(client.sendRequest).toHaveBeenCalledOnce() + expect(client.sendRequest).toHaveBeenCalledWith('status.get') + const statuses = renderer.root.findAllByType('GateStatus') + expect(statuses).toHaveLength(2) + for (const status of statuses) { + expect(status.props.children).toBe('browser.screencast.v1,terminal.queryReplyInput.v1') + } + }) + + it('releases the cover on a failed status.get and upgrades when a retry lands', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + const sendRequest = vi + .fn() + .mockRejectedValueOnce(new Error('status.get timed out')) + .mockResolvedValue({ + ok: true, + result: { protocolVersion: 5, minCompatibleMobileVersion: 0, capabilities: ['late.v1'] } + }) + hostClient.current = { client: { sendRequest } as unknown as RpcClient, state: 'connected' } + renderer = await renderGate() + + // Why: a wedged status.get must never trap the routes behind the cover, so the first miss + // settles conservative gates immediately — no capabilities, but a usable UI. + let output = renderedText(renderer) + expect(output).toContain('HostContent') + expect(output).not.toContain('Checking host compatibility') + expect(output).toContain('"type":"GateStatus","props":{},"children":null') + + await act(async () => { + await vi.advanceTimersByTimeAsync(1_100) + }) + + // The probe kept retrying underneath, so the answer arrives without a remount. + expect(sendRequest).toHaveBeenCalledTimes(2) + expect(renderedText(renderer)).toContain('late.v1') + expect(probeMounts.count).toBe(1) + vi.useRealTimers() + }) + + it('blocks a desktop that omits protocolVersion, so a pending verdict is not a formality', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + // Why this case and not just an explicit old version: evaluateCompat reads a missing + // protocolVersion as 0, so the everyday shape of an old desktop is a blocking one. + hostClient.current = { + client: clientWithStatus({ capabilities: [] }), + state: 'connected' + } + renderer = await renderGate() + const output = renderedText(renderer) + expect(output).toContain('Update Orca on your computer') + expect(output).not.toContain('HostContent') + }) + it('renders the host UI while the host connection is still pending', async () => { hostClient.current = { client: null, state: 'connecting' } renderer = await renderGate() expect(renderedText(renderer)).toContain('HostContent') }) - it('does not mount host routes before a connected host passes the compatibility probe', async () => { + // Was: the routes were held back until status.get resolved, which serialised every route's + // own startup RPC behind this one round trip. They now mount immediately and are covered. + it('mounts host routes under the pending cover while status.get is still in flight', async () => { const client = { sendRequest: vi.fn().mockReturnValue(new Promise(() => {})) } as unknown as RpcClient @@ -168,9 +257,40 @@ describe('HostProtocolGate', () => { renderer = await renderGate() const output = renderedText(renderer) expect(output).toContain('Checking host compatibility') - expect(output).not.toContain('HostContent') - expect(probeMounts.count).toBe(0) + expect(output).toContain('HostContent') + expect(probeMounts.count).toBe(1) expect(client.sendRequest).toHaveBeenCalledOnce() + // Why: mounting early must not leak an unproven host's capabilities to the routes below; + // an empty join renders no children, so the consumer saw none. + expect(output).toContain('"type":"GateStatus","props":{},"children":null') + const overlay = renderer.root + .findAllByType('View') + .find((node) => node.props.accessibilityViewIsModal === true) + expect(overlay?.props.pointerEvents).toBe('auto') + }) + + it('unmounts the routes it mounted early when the verdict comes back blocked', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + let settle: ((response: unknown) => void) | null = null + const client = { + sendRequest: vi.fn().mockReturnValue( + new Promise((resolve) => { + settle = resolve + }) + ) + } as unknown as RpcClient + hostClient.current = { client, state: 'connected' } + renderer = await renderGate() + expect(renderedText(renderer)).toContain('HostContent') + + await act(async () => { + settle?.({ ok: true, result: { protocolVersion: 5, minCompatibleMobileVersion: 999 } }) + await Promise.resolve() + }) + + const output = renderedText(renderer) + expect(output).toContain('Update Orca Mobile') + expect(output).not.toContain('HostContent') }) it('overlays the pending spinner instead of unmounting routes mounted while connecting', async () => { @@ -259,4 +379,52 @@ describe('HostProtocolGate', () => { renderer = await renderGate() expect(renderedText(renderer)).toContain('HostContent') }) + + it('reports a rejected status.get as unverified, so failing open is not a passing verdict', async () => { + const sendRequest = vi + .fn() + .mockResolvedValue({ ok: false, error: { message: 'no such method' } }) + hostClient.current = { client: { sendRequest } as unknown as RpcClient, state: 'connected' } + renderer = await renderGate() + + // Navigation still works: the host said no, and that must not lock the user out of the route. + const output = renderedText(renderer) + expect(output).toContain('HostContent') + expect(output).not.toContain('Checking host compatibility') + // Why: `compatVerdict` is `ok` here purely as a fallback. Nothing about this host was proven, + // so callers that write to it read this flag instead of the verdict. + expect(output).toContain('["unverified"]') + }) + + it('reports a passing status reply as verified', async () => { + hostClient.current = { + client: clientWithStatus({ protocolVersion: 5, minCompatibleMobileVersion: 0 }), + state: 'connected' + } + renderer = await renderGate() + expect(renderedText(renderer)).toContain('["verified"]') + }) + + it('stays unverified through a failed status.get and flips once a retry answers', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + const sendRequest = vi + .fn() + .mockRejectedValueOnce(new Error('status.get timed out')) + .mockResolvedValue({ + ok: true, + result: { protocolVersion: 5, minCompatibleMobileVersion: 0 } + }) + hostClient.current = { client: { sendRequest } as unknown as RpcClient, state: 'connected' } + renderer = await renderGate() + + expect(renderedText(renderer)).toContain('["unverified"]') + + await act(async () => { + await vi.advanceTimersByTimeAsync(1_100) + }) + + // The retry landed, so the fallback is replaced by a real answer and writes are released. + expect(renderedText(renderer)).toContain('["verified"]') + vi.useRealTimers() + }) }) diff --git a/mobile/src/components/HostProtocolGate.tsx b/mobile/src/components/HostProtocolGate.tsx index 4d9c0c019f1..d69e4872784 100644 --- a/mobile/src/components/HostProtocolGate.tsx +++ b/mobile/src/components/HostProtocolGate.tsx @@ -22,45 +22,26 @@ export function useHostProtocolGates(): HostStatusGates { // Why: single choke point above every /h/[hostId] route so a blocked verdict replaces the // whole host UI (sidebar + detail stack) while the host list and other hosts stay usable. +// The routes mount as soon as the connection does, so their startup RPCs (session.tabs.list, +// terminal.list) fly alongside this status.get instead of queueing behind it; a blocked verdict +// then unmounts them and their answers are discarded. export function HostProtocolGate({ hostId, children }: Props) { const { client, state } = useHostClient(hostId) const gates = useHostStatusGates({ hostId, client, connState: state }) const { compatVerdict, statusPending } = gates const resolvedHostIdRef = useRef(null) - const mountedHostIdRef = useRef(null) const hostKey = hostId ?? null const resolvedNow = state === 'connected' && client !== null && !statusPending const blocked = compatVerdict.kind === 'blocked' const pending = statusPending && resolvedHostIdRef.current !== hostKey - const holdBack = pending && mountedHostIdRef.current !== hostKey - // Why: React can replay or discard a render, so the latches record committed - // outcomes only — a discarded children render must not count as mounted. + // Why: React can replay or discard a render, so the latch records committed outcomes only. useEffect(() => { if (resolvedNow) { resolvedHostIdRef.current = hostKey } - if (blocked) { - // Why: the block screen unmounts the routes, so a later pending window - // must not assume a live tree it can overlay. - mountedHostIdRef.current = null - } else if (!holdBack) { - mountedHostIdRef.current = hostKey - } }) - if (holdBack) { - // Why: nothing is mounted yet for this host, so hold the routes back entirely - // rather than letting them mount (and fire their connect RPCs) pre-verdict. - return ( - - - - ) - } if (blocked) { return } @@ -77,10 +58,11 @@ export function HostProtocolGate({ hostId, children }: Props) { {children} {pending ? ( - // Why: once the stack is mounted, unmounting it for a pending status.get destroys - // in-flight nested navigation, so cover it instead. Mount effects underneath still - // run — they wait for connState 'connected' and every capability-dependent call - // re-probes status.get itself, so nothing newer than the baseline fires here. + // Why: cover the stack rather than unmounting it — unmounting for a pending status.get + // destroys in-flight nested navigation, and holding it back would serialise every route's + // startup RPC behind this one. Mount effects underneath run pre-verdict by design; they + // read capabilities from this gate, which reports none until the verdict lands, so every + // capability-dependent surface stays closed rather than guessing. ({ start: vi.fn() })) -vi.mock('../transport/runtime-capability-probe', () => ({ +vi.mock('../transport/runtime-status-probe', () => ({ startRuntimeCapabilityProbe: probe.start })) diff --git a/mobile/src/components/codex-reset-credit-capability.ts b/mobile/src/components/codex-reset-credit-capability.ts index 1a32ef37873..129dd654ae0 100644 --- a/mobile/src/components/codex-reset-credit-capability.ts +++ b/mobile/src/components/codex-reset-credit-capability.ts @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react' import { CODEX_RESET_CREDIT_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' import type { RpcClient } from '../transport/rpc-client' -import { startRuntimeCapabilityProbe } from '../transport/runtime-capability-probe' +import { startRuntimeCapabilityProbe } from '../transport/runtime-status-probe' // Why: source the capability string from the shared contract so a host bump can never // silently drift from the mobile probe. diff --git a/mobile/src/session/MobileSessionActiveContent.tsx b/mobile/src/session/MobileSessionActiveContent.tsx index 019e83c6a99..571add41e0d 100644 --- a/mobile/src/session/MobileSessionActiveContent.tsx +++ b/mobile/src/session/MobileSessionActiveContent.tsx @@ -2,6 +2,8 @@ import { Animated, View, Text, Pressable, ActivityIndicator } from 'react-native import { saveTerminalTextScale } from '../storage/preferences' import { MobileBrowserPane } from '../browser/MobileBrowserPane' import { TerminalPaneView } from './TerminalPaneView' +import { TerminalEnginePrewarm } from './TerminalEnginePrewarm' +import { MOBILE_SESSION_TAB_BAR_HEIGHT } from './mobile-session-frame-styles' import { MobileNativeChatOverlay } from './MobileNativeChatOverlay' import { colors } from '../theme/mobile-theme' import { styles } from './mobile-session-styles' @@ -75,15 +77,30 @@ export function MobileSessionActiveContent({ isPendingTerminalRecoveryParked, retryPendingTerminalRecovery, showLoadingState, + measurePrewarmViewport, + visibleTabs, showEmptyState, keyboardLift, activeTerminalKeyboardLift, toastAnimatedStyle, createTabBusy } = controller + // Why the same list the header gates on: an unmounted tab bar gives the content row its band + // back, so the pre-warm would measure a taller box than the pane ever gets. Reading the header's + // own condition keeps the two from drifting when what counts as a visible tab changes. + const prewarmReservedTabBarHeight = visibleTabs.length > 0 ? 0 : MOBILE_SESSION_TAB_BAR_HEIGHT return showLoadingState ? ( - - + // Why: the engine boots inside the real terminal frame while the startup RPCs are still in + // flight, so the first pane inherits a warm WebView and a measured viewport (see prewarm). + + + + + ) : showEmptyState ? ( @@ -171,25 +188,35 @@ export function MobileSessionActiveContent({ )} ) : activePendingTerminalTab ? ( - - {!isPendingTerminalRecoveryParked && ( - - )} - - {isPendingTerminalRecoveryParked - ? 'Terminal is taking longer than expected' - : activePendingTerminalTab.title || 'Loading terminal'} - - {isPendingTerminalRecoveryParked && ( - [styles.createButton, pressed && styles.newTerminalButtonPressed]} - onPress={() => void retryPendingTerminalRecovery()} - > - Retry - - )} + + + {!isPendingTerminalRecoveryParked && ( + + )} + + {isPendingTerminalRecoveryParked + ? 'Terminal is taking longer than expected' + : activePendingTerminalTab.title || 'Loading terminal'} + + {isPendingTerminalRecoveryParked && ( + [ + styles.createButton, + pressed && styles.newTerminalButtonPressed + ]} + onPress={() => void retryPendingTerminalRecovery()} + > + Retry + + )} + + ) : ( ({ + init: vi.fn((_cols: number, _rows: number) => {}), + awaitReady: vi.fn(async () => {}), + measureFitDimensions: vi.fn(async (_containerHeight?: number) => ({ cols: 120, rows: 40 })), + onWebReady: null as (() => void) | null, + textScale: undefined as number | undefined +})) + +vi.mock('react-native', () => ({ + StyleSheet: { + create: (styles: T) => styles, + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 } + }, + View: 'View' +})) + +// Stands in for the real engine: records the ref the pre-warm pane holds and the ready callback +// it arms, so a test can drive web-ready and layout in either order. +vi.mock('../terminal/TerminalWebView', async () => { + const { forwardRef, useImperativeHandle } = await import('react') + return { + TerminalWebView: forwardRef< + TerminalWebViewHandle, + { onWebReady?: () => void; textScale?: number } + >(function MockTerminalWebView(props, ref) { + engine.onWebReady = props.onWebReady ?? null + engine.textScale = props.textScale + useImperativeHandle(ref, () => engine as unknown as TerminalWebViewHandle, []) + return createElement('MockTerminalWebView') + }) + } +}) + +import { TerminalEnginePrewarm } from './TerminalEnginePrewarm' + +const FRAME = { x: 0, y: 0, width: 390, height: 700 } + +const TEXT_SCALE = 1.25 + +function renderPrewarm(onEngineMeasured: (ref: TerminalWebViewHandle, height: number) => void): { + renderer: ReactTestRenderer + layout: (frame: { x: number; y: number; width: number; height: number }) => void + webReady: () => void +} { + let renderer: ReactTestRenderer | null = null + act(() => { + renderer = create( + createElement(TerminalEnginePrewarm, { + reservedTabBarHeight: 0, + textScale: TEXT_SCALE, + onEngineMeasured + }) + ) + }) + const created = renderer as unknown as ReactTestRenderer + return { + renderer: created, + layout: (frame) => + act(() => { + created.root.findAllByType('View')[0]?.props.onLayout({ nativeEvent: { layout: frame } }) + }), + webReady: () => + act(() => { + engine.onWebReady?.() + }) + } +} + +// The handoff now waits on the engine's ready promise, so tests have to let microtasks run. +async function flushReady(): Promise { + await act(async () => {}) +} + +afterEach(() => { + engine.measureFitDimensions.mockClear() + engine.init.mockClear() + engine.awaitReady.mockReset() + engine.awaitReady.mockResolvedValue(undefined) + engine.onWebReady = null + engine.textScale = undefined +}) + +describe('TerminalEnginePrewarm', () => { + it('boots the engine without waiting for a terminal to attach', () => { + const measured = vi.fn() + const { renderer } = renderPrewarm(measured) + // The engine mounts on the first render, so its bundle loads while the startup RPCs fly. + expect(renderer.root.findAllByType('MockTerminalWebView')).toHaveLength(1) + expect(measured).not.toHaveBeenCalled() + }) + + it('withholds the measurement until the pane has a real layout', async () => { + const measured = vi.fn() + const { webReady, layout } = renderPrewarm(measured) + + webReady() + // Why: this is the 80x24 trap — an unsized engine answers with xterm's default, and that + // number would ride the first subscribe to the host as the PTY size. + expect(measured).not.toHaveBeenCalled() + + layout({ ...FRAME, width: 0, height: 0 }) + expect(measured).not.toHaveBeenCalled() + + layout(FRAME) + await flushReady() + expect(measured).toHaveBeenCalledOnce() + expect(measured.mock.calls[0]?.[1]).toBe(FRAME.height) + }) + + it('withholds the measurement until the engine reports ready', async () => { + const measured = vi.fn() + const { layout, webReady } = renderPrewarm(measured) + + layout(FRAME) + expect(measured).not.toHaveBeenCalled() + + webReady() + await flushReady() + expect(measured).toHaveBeenCalledOnce() + }) + + it('measures once however many times layout and web-ready repeat', async () => { + const measured = vi.fn() + const { layout, webReady } = renderPrewarm(measured) + + layout(FRAME) + webReady() + webReady() + layout({ ...FRAME, height: 640 }) + layout(FRAME) + await flushReady() + + expect(measured).toHaveBeenCalledOnce() + }) + + it('opens the engine before handing it over, because web-ready alone builds no terminal', async () => { + const measured = vi.fn() + let releaseReady: (() => void) | null = null + engine.awaitReady.mockImplementation( + () => + new Promise((resolve) => { + releaseReady = resolve + }) + ) + const { layout, webReady } = renderPrewarm(measured) + + layout(FRAME) + webReady() + // Why: the WebView answers `measure` with null while it has no terminal, and the pane latches + // once, so handing the engine over before init would spend the one measurement on nothing. + expect(engine.init).toHaveBeenCalledOnce() + expect(measured).not.toHaveBeenCalled() + + releaseReady?.() + await flushReady() + expect(measured).toHaveBeenCalledOnce() + expect(measured.mock.calls[0]?.[0]).toBe(engine) + }) + + it('pre-warms at the text size the first pane will open with', () => { + renderPrewarm(vi.fn()) + // Cell size is what the frame gets divided by, so a default-sized engine would measure a + // different phone than the one the user is looking at. + expect(engine.textScale).toBe(TEXT_SCALE) + }) + + it('reports the frame the pane ended up with when a resize lands during engine start-up', async () => { + const measured = vi.fn() + let releaseReady: (() => void) | null = null + engine.awaitReady.mockImplementation( + () => + new Promise((resolve) => { + releaseReady = resolve + }) + ) + const { layout, webReady } = renderPrewarm(measured) + + layout(FRAME) + webReady() + expect(measured).not.toHaveBeenCalled() + + // A rotation or split-screen resize while the engine is still coming up. The latch has already + // fired, so this is the last chance to correct the height the one measurement is taken against. + const resized = { ...FRAME, width: 700, height: 360 } + layout(resized) + + releaseReady?.() + await flushReady() + + expect(measured).toHaveBeenCalledOnce() + expect(measured.mock.calls[0]?.[1]).toBe(resized.height) + }) + + it('drops the handoff when the pane unmounts before the engine is ready', async () => { + const measured = vi.fn() + let releaseReady: (() => void) | null = null + engine.awaitReady.mockImplementation( + () => + new Promise((resolve) => { + releaseReady = resolve + }) + ) + const { renderer, layout, webReady } = renderPrewarm(measured) + layout(FRAME) + webReady() + + act(() => { + renderer.unmount() + }) + releaseReady?.() + await flushReady() + + // The frame this measurement was taken against is gone, so it describes nothing. + expect(measured).not.toHaveBeenCalled() + }) + + it('is inert: no touches, no accessibility, and nothing sent to a terminal', async () => { + const measured = vi.fn() + const { renderer, layout, webReady } = renderPrewarm(measured) + layout(FRAME) + webReady() + await flushReady() + + const pane = renderer.root.findAllByType('View')[0] + expect(pane?.props.pointerEvents).toBe('none') + expect(pane?.props.accessibilityElementsHidden).toBe(true) + expect(pane?.props.importantForAccessibility).toBe('no-hide-descendants') + // The pane owns no handle, so it has no way to subscribe, send input, or resize a PTY. + // Opening the engine is WebView-local; the measurement itself is the caller's to take. + expect(engine.measureFitDimensions).not.toHaveBeenCalled() + expect(measured.mock.calls[0]?.[0]).toBe(engine) + }) +}) diff --git a/mobile/src/session/TerminalEnginePrewarm.tsx b/mobile/src/session/TerminalEnginePrewarm.tsx new file mode 100644 index 00000000000..a98585946e7 --- /dev/null +++ b/mobile/src/session/TerminalEnginePrewarm.tsx @@ -0,0 +1,111 @@ +import { useCallback, useRef } from 'react' +import { StyleSheet, View, type LayoutChangeEvent } from 'react-native' +import { TerminalWebView } from '../terminal/TerminalWebView' +import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' + +// Diagnostics label for the measurement this pane contributes; it is not a PTY handle. +export const TERMINAL_ENGINE_PREWARM_HANDLE = '(engine-prewarm)' + +// Why: the WebView builds no xterm until it is told to, and `measure` answers null while `term` +// is null, so the engine has to be opened before it can be asked anything. These are placeholder +// dimensions for an empty buffer nobody reads; the measurement derives its own cols and rows from +// the frame and the font's cell size, so nothing downstream inherits them. +const PREWARM_INIT_COLS = 80 +const PREWARM_INIT_ROWS = 24 + +type Props = { + // Height the tab bar will claim from the top of this frame once the session has a tab. The + // loading state has no visible tab, so the bar is not mounted yet and the box the pane will + // finally occupy is this much shorter. Reserving it keeps the measurement honest; measuring + // the taller box would latch too many rows and send them to the host as the PTY size. + reservedTabBarHeight: number + // Why: the first pane opens at the user's saved text size, and cell size is what the + // measurement divides the frame by. Pre-warming at a different size measures a different phone. + textScale: number + onEngineMeasured: (ref: TerminalWebViewHandle, frameHeight: number) => void +} + +// Why: a session still resolving its tabs already knows it is heading for a terminal, so load +// the xterm engine alongside the startup RPCs instead of after terminal.list returns. This pane +// owns no handle: it never subscribes, never sends input, and can never resize a PTY. Its only +// output is the viewport measurement the first real pane would otherwise pay a round trip for. +export function TerminalEnginePrewarm({ + reservedTabBarHeight, + textScale, + onEngineMeasured +}: Props) { + const engineRef = useRef(null) + const frameHeightRef = useRef(0) + const webReadyRef = useRef(false) + const measuredRef = useRef(false) + + // Idempotent by construction: both triggers funnel here and the latch fires once per mount. + const measureWhenSized = useCallback(() => { + const engine = engineRef.current + // Why: an unsized or unmounted WebView measures xterm's 80x24 default, and that number + // rides the first subscribe to the host. Only a laid-out engine is allowed to answer. + if (measuredRef.current || !webReadyRef.current || !engine || frameHeightRef.current <= 0) { + return + } + measuredRef.current = true + // `web-ready` only says the xterm bundle loaded. Opening the engine is what creates `term`, + // and `awaitReady` is what lets its cell dimensions exist before anything reads them. + engine.init(PREWARM_INIT_COLS, PREWARM_INIT_ROWS) + void engine.awaitReady().then(() => { + // React nulls the ref on unmount, so this proves the pane the frame belongs to is still up. + if (engineRef.current !== engine) { + return + } + // Why read the height here and not before the wait: a rotation or split-screen resize during + // engine start-up re-lays out this pane, and the latch above already refused the second + // handoff, so a height captured earlier would be the only one this pane ever reports. + onEngineMeasured(engine, frameHeightRef.current) + }) + }, [onEngineMeasured]) + + const handleLayout = useCallback( + (event: LayoutChangeEvent) => { + const { height, width } = event.nativeEvent.layout + if (width <= 0 || height <= 0) { + return + } + frameHeightRef.current = height + measureWhenSized() + }, + [measureWhenSized] + ) + + const handleWebReady = useCallback(() => { + webReadyRef.current = true + measureWhenSized() + }, [measureWhenSized]) + + return ( + + + + ) +} + +const styles = StyleSheet.create({ + prewarmPane: { + ...StyleSheet.absoluteFillObject, + opacity: 0 + }, + prewarmWebView: { + flex: 1 + } +}) diff --git a/mobile/src/session/mobile-session-frame-styles.ts b/mobile/src/session/mobile-session-frame-styles.ts index a02c14be014..8cdf550486c 100644 --- a/mobile/src/session/mobile-session-frame-styles.ts +++ b/mobile/src/session/mobile-session-frame-styles.ts @@ -2,6 +2,20 @@ import { StyleSheet } from 'react-native' import { colors, spacing, radii, typography } from '../theme/mobile-theme' +// Why one constant for the whole strip: the terminal frame is whatever the tab bar leaves behind, +// and the engine pre-warm has to reserve exactly that much before the bar exists. Every row child +// is pinned to this height so nothing can grow the bar without moving the reservation with it. +// +// The row deliberately has NO explicit height. React Native lays out border-box, so `height: 36` +// with a 1 px top border would render a 36 px row over a 35 px content area and squeeze children +// that are themselves 36 -- and it would leave this constant one pixel long, which is a whole row +// of drift once a frame sits near a row boundary. Left to size itself the row takes its tallest +// child and adds the border outside it, which is exactly the sum below. +export const MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT = 36 +export const MOBILE_SESSION_TAB_BAR_BORDER_WIDTH = 1 +export const MOBILE_SESSION_TAB_BAR_HEIGHT = + MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT + MOBILE_SESSION_TAB_BAR_BORDER_WIDTH + export const mobileSessionFrameStyles = StyleSheet.create({ container: { flex: 1, @@ -80,12 +94,12 @@ export const mobileSessionFrameStyles = StyleSheet.create({ tabBar: { flexDirection: 'row', alignItems: 'center', - borderTopWidth: 1, + borderTopWidth: MOBILE_SESSION_TAB_BAR_BORDER_WIDTH, borderTopColor: colors.borderSubtle }, tabScroll: { flex: 1, - maxHeight: 36 + maxHeight: MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT }, tabContent: { paddingLeft: spacing.sm, @@ -94,7 +108,7 @@ export const mobileSessionFrameStyles = StyleSheet.create({ tab: { width: 128, maxWidth: 128, - minHeight: 36, + minHeight: MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT, alignItems: 'center', justifyContent: 'center', paddingHorizontal: spacing.sm, @@ -123,7 +137,7 @@ export const mobileSessionFrameStyles = StyleSheet.create({ }, newTerminalButton: { width: 40, - height: 36, + height: MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT, alignItems: 'center', justifyContent: 'center', borderBottomWidth: 2, diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index bc951bfa206..1f69710cbf0 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -62,32 +62,32 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = '10071240ef9edafc2b9c8bed73be83dceaf7828e3b29f17dab55da020a7697a6' -const HEAD_HOOK_BINDING_SHA256 = '1dadb8c3dc0573ea20659ce7251629669e618dd0effaeac3a4536b29c2e865a1' +const HEAD_MAIN_HOOK_SHA256 = '32f0d40d90a76d381480b32f7e8a42b209fa6d6740def39e8691c8fc4dce1871' +const HEAD_HOOK_BINDING_SHA256 = '0f4fac965d009b93e7d0e128ddcbc650f1e83b7adb8c0e3c91d0710e3a8c8ccc' const HEAD_CALLBACK_IDENTITY_SHA256 = - '2a9e4825df007f6ef53b81aa5004991d6318eee7507b44d625c07e630be432eb' -const HEAD_CALLBACK_BODY_SHA256 = '22103ba85a86e3a3fcb80a7509c7a455d79863010cde3af02db6565b55e3ebe9' -const HEAD_EFFECT_SHA256 = 'd9ebfaabc1e79773cdada7ab370b20459ed972f1f8edce1652199f4d0391cd13' + 'e5df1043256bcb0b3813bf89161d91f5e65c00749fbb6d98176bca82e878d061' +const HEAD_CALLBACK_BODY_SHA256 = '6d9ed614ed139aef5cc911c33ea4220cc1fc5f888a1a564ef85e6910cc118bc3' +const HEAD_EFFECT_SHA256 = 'cf697133278832d33ecf9b87c1c2b1059091d238bad3ca6bed6032f8cf19ad7e' const HEAD_CONTENT_HOOK_SHA256 = '9c3b612fef3f370d66873aefdbe1d701f20cb64ded31fef5cc45fde6f8189581' const HEAD_NESTED_FUNCTION_SHA256 = - '536c72b233c813bb0cea164b090bdce5406ceb965bbc5b83c1f89b89b46f3821' + '0e553eb5ec7aeda8f8336b8da85ff87eb3657a21fa32d3c75c9cc32e36860244' const HEAD_NATIVE_REGISTRATION_SHA256 = 'cab85e4e4a3f43289ba93ddea9ccce57aea83e0bf14fd1620a965aad0c1cb49e' const HEAD_NATIVE_REMOVAL_SHA256 = '4c994574675a2a0f9c607b3ea89ab7a2ed5a83f7c72fa42342ddcb5f00fc3f4f' const HEAD_TIMER_CREATION_SHA256 = - '1a31b625e2174c3db77272249843196d2b6b06ab1e654a96d8f7858e3082e66b' -const HEAD_TIMER_CLEANUP_SHA256 = 'c73f1d1c2cc89642f3d727d6f3b6b81860a9d6f34234541a2065ec3d1a8cd116' + '36c3ccef371698e25cd2eb239df7a8dea6dcc674d9da43cc38cabfa3a8f64929' +const HEAD_TIMER_CLEANUP_SHA256 = '2f41ddc30d0e9c1b6d1d6b5e09d96d1b3facd3133acae1ff7436bb40e4ef39dc' const HEAD_RUNTIME_STRING_SHA256 = - '31951b0b83be01ebfa659c4b94df9ad7eaff6404df5338fbade89eb7473a3cb4' -const HEAD_HOST_JSX_SHA256 = '390405926b1695fa3a33686f0bc192b432f5468d8576499d7cafbb4922defbb5' -const HEAD_LEAF_JSX_SHA256 = '21dba981875e173f692590bf910d60964660c5f4cbb79f3a377c7e54f6a1f016' + 'f0e63142c8452bfd633eda1f42e73c718e3f4baf703d31d260e03b8048fd8527' +const HEAD_HOST_JSX_SHA256 = '37e6ad7ca6406a4d23ac85c347ca210235b434fd7c2578cffdfe58336221fbb4' +const HEAD_LEAF_JSX_SHA256 = '9e8faf5df0c6a792beb74c6608bce32ba872fd48becc0a4b6aea4b5a5bbbbeda' const HEAD_STYLE_REFERENCE_SHA256 = - '295a3501c2c6d7bea7c8bbf38b3f3534f01344cd7e1b91bb8e07c040821d596a' + '4a71a8620d825975375cdfe402424e612a987ba867042aa1701993ef9d0d6208' const HEAD_IDENTITY_FIELD_SHA256 = - '91146853930a34dd1f3d80e5c97fbacd7cf19fb93dd26fe8fc6f29169622f9d6' + 'a7444b7d0953edb34abc77180ba11d458b02081547b8499249571efd30ac0609' const HEAD_NAVIGATION_SHA256 = '9d96f5dad7de555d6553eac39c0fab00efad507470fd562cb9beaa32db16f512' -const HEAD_CAPABILITY_SHA256 = 'ca219f7909a091717110b823d5b94a20770ad3ae51894e0fa765e8628309392d' +const HEAD_CAPABILITY_SHA256 = '54c74cdb468d015c31517004e005187f6cff2ddb07e25fdb4a7060a2fac6b786' type Definition = { declaration: ts.FunctionDeclaration; sourceFile: ts.SourceFile } type HookFacts = { @@ -456,8 +456,10 @@ function readCompatibilityFacts(definitions: ReadonlyMap): { : '' const callText = canonical(node, sourceFile) if ( - ['startRuntimeCapabilityProbe', 'supportsMobileQuickCommands'].includes(callName) || - (callName === 'includes' && callText.includes('capabilities.includes')) + // hostCapabilities.* is included: the session route now reads the gate's shared status.get + // answer instead of running its own probe, and those reads still have to stay ratcheted. + ['useHostProtocolGates', 'supportsMobileQuickCommands'].includes(callName) || + (callName === 'includes' && /[cC]apabilities\.includes/.test(callText)) ) { capabilities.push(callText) } @@ -472,18 +474,18 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(266) + expect(main.hooks).toHaveLength(269) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) - expect(main.callbacks).toHaveLength(77) + expect(main.callbacks).toHaveLength(78) expect(hash(main.callbacks)).toBe(HEAD_CALLBACK_IDENTITY_SHA256) expect(hash(main.callbackBodies)).toBe(HEAD_CALLBACK_BODY_SHA256) - expect(main.effects).toHaveLength(24) + expect(main.effects).toHaveLength(25) expect(hash(main.effects)).toBe(HEAD_EFFECT_SHA256) expect(contentBindings).toHaveLength(14) expect(hash(contentBindings)).toBe(HEAD_CONTENT_HOOK_SHA256) const nestedFunctions = readNestedFunctions(definitions) - expect(nestedFunctions).toHaveLength(12) + expect(nestedFunctions).toHaveLength(13) expect(hash(nestedFunctions)).toBe(HEAD_NESTED_FUNCTION_SHA256) }) @@ -494,20 +496,23 @@ describe('mobile session route extraction parity', () => { expect(hash(native.registrations)).toBe(HEAD_NATIVE_REGISTRATION_SHA256) expect(native.removals).toHaveLength(9) expect(hash(native.removals)).toBe(HEAD_NATIVE_REMOVAL_SHA256) - expect(native.creations.filter((fact) => fact.startsWith('setTimeout'))).toHaveLength(7) + expect(native.creations.filter((fact) => fact.startsWith('setTimeout'))).toHaveLength(8) expect(native.creations.filter((fact) => fact.startsWith('setInterval'))).toHaveLength(1) expect( native.creations.filter((fact) => fact.startsWith('requestAnimationFrame')) ).toHaveLength(1) expect(hash(native.creations)).toBe(HEAD_TIMER_CREATION_SHA256) - expect(native.cleanups.filter((fact) => fact.startsWith('clearTimeout'))).toHaveLength(11) + expect(native.cleanups.filter((fact) => fact.startsWith('clearTimeout'))).toHaveLength(12) expect(native.cleanups.filter((fact) => fact.startsWith('clearInterval'))).toHaveLength(1) expect(native.cleanups.filter((fact) => fact.startsWith('cancelAnimationFrame'))).toHaveLength( 1 ) expect(hash(native.cleanups)).toBe(HEAD_TIMER_CLEANUP_SHA256) const compatibility = readCompatibilityFacts(definitions) - expect(compatibility.identityFields).toHaveLength(14) + // 13, not 14: both worktree.activate call sites now share one payload builder, so the + // literal `notifyClients: false` they used to repeat appears once. The guarantee itself is + // pinned in mobile-session-startup-source.test.ts, which requires exactly one call site. + expect(compatibility.identityFields).toHaveLength(13) expect(hash(compatibility.identityFields)).toBe(HEAD_IDENTITY_FIELD_SHA256) expect(compatibility.navigation).toHaveLength(6) expect(hash(compatibility.navigation)).toBe(HEAD_NAVIGATION_SHA256) @@ -517,14 +522,14 @@ describe('mobile session route extraction parity', () => { it('preserves runtime strings, styles, and the expanded JSX tree', () => { const strings = readRuntimeStrings() - expect(strings).toHaveLength(546) + expect(strings).toHaveLength(543) expect(hash(strings)).toBe(HEAD_RUNTIME_STRING_SHA256) const jsx = readJsxFacts(readDefinitions()) - expect(jsx.host).toHaveLength(124) + expect(jsx.host).toHaveLength(126) expect(hash(jsx.host)).toBe(HEAD_HOST_JSX_SHA256) - expect(jsx.leaf).toHaveLength(61) + expect(jsx.leaf).toHaveLength(63) expect(hash(jsx.leaf)).toBe(HEAD_LEAF_JSX_SHA256) - expect(jsx.styleReferences).toHaveLength(172) + expect(jsx.styleReferences).toHaveLength(174) expect(hash(jsx.styleReferences)).toBe(HEAD_STYLE_REFERENCE_SHA256) }) }) diff --git a/mobile/src/session/mobile-session-startup-parallelism.test.ts b/mobile/src/session/mobile-session-startup-parallelism.test.ts new file mode 100644 index 00000000000..5ef2c22a483 --- /dev/null +++ b/mobile/src/session/mobile-session-startup-parallelism.test.ts @@ -0,0 +1,279 @@ +import { createElement, type ReactElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useMobileSessionStartup } from './use-mobile-session-startup' +import type { MobileSessionKeyboardStateModel } from './use-mobile-session-keyboard-state' + +type Deferred = { promise: Promise; resolve: (value: T) => void; reject: (e: Error) => void } + +function defer(): Deferred { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +type StartupCall = { rpc: 'session.tabs.list' | 'terminal.list'; worktreeId: string } + +// One session's worth of scope: only the fields useMobileSessionStartup actually reads, plus +// the two reads under test wired to deferreds so a test controls exactly when they settle. +function makeScope(worktreeId: string, calls: StartupCall[], protocolVerified = true) { + const tabs = defer() + const terminals = defer() + const sendRequest = vi.fn().mockResolvedValue({ ok: true, result: {} }) + const scope = { + hostId: 'host-1', + worktreeId, + created: '0', + isFloatingWorkspaceRoute: false, + connState: 'connected', + client: { sendRequest }, + protocolVerified, + setTerminals: vi.fn(), + terminalsRef: { current: [] }, + setSessionTabs: vi.fn(), + appliedSnapshotMarkerRef: { current: { epoch: null, version: -1 } }, + closedTabTombstonesRef: { current: new Map() }, + setTerminalsLoaded: vi.fn(), + setActiveHandle: vi.fn(), + setActiveSessionTabId: vi.fn(), + setMarkdownDocs: vi.fn(), + setFileDocs: vi.fn(), + terminalGestureInputQueuesRef: { current: new Map() }, + terminalGestureInputInFlightRef: { current: new Set() }, + sessionTabActionSheetKeyboardHideSubRef: { current: null }, + sessionTabActionSheetRequestSeqRef: { current: 0 }, + initializedHandlesRef: { current: new Set() }, + terminalDiagnosticsRef: { current: { resetRoute: vi.fn() } }, + activeHandleRef: { current: null }, + activeSessionTabTypeRef: { current: null }, + pendingActiveSessionTabIdRef: { current: null }, + selectedSessionTabIdRef: { current: null }, + pendingActiveTerminalHandleRef: { current: null }, + pendingBrowserFocusPageIdRef: { current: null }, + pendingTerminalActivationAttemptRef: { current: null }, + initialSessionAutoCreateRef: { current: null }, + bufferedTerminalDraftState: { resetDrafts: vi.fn(), clearPendingRestorations: vi.fn() }, + clearPendingLiveInputCommit: vi.fn(), + clearDelayedActionTimers: vi.fn(), + showToast: vi.fn(), + clearTerminalCache: vi.fn(), + fetchTerminals: vi.fn(() => { + calls.push({ rpc: 'terminal.list', worktreeId }) + return terminals.promise + }), + ensureSessionTabs: vi.fn(() => { + calls.push({ rpc: 'session.tabs.list', worktreeId }) + return tabs.promise + }) + } + return { + scope: scope as unknown as MobileSessionKeyboardStateModel, + tabs, + terminals, + sendRequest, + activateCalls: () => + sendRequest.mock.calls.filter(([method]) => method === 'worktree.activate').length + } +} + +function StartupHarness({ + scope +}: { + scope: MobileSessionKeyboardStateModel +}): ReactElement | null { + useMobileSessionStartup(scope) + return null +} + +async function flush(): Promise { + await act(async () => { + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + }) +} + +describe('mobile session startup parallelism', () => { + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + vi.useRealTimers() + }) + + it('puts session.tabs.list and terminal.list on the wire together', async () => { + const calls: StartupCall[] = [] + const { scope } = makeScope('wt-1', calls) + + await act(async () => { + renderer = create(createElement(StartupHarness, { scope })) + await Promise.resolve() + }) + await flush() + + // Neither deferred has settled, so both requests are in flight at the same moment. Under the + // old chain the second call could not have been made until the first resolved. + expect(calls).toEqual([ + { rpc: 'session.tabs.list', worktreeId: 'wt-1' }, + { rpc: 'terminal.list', worktreeId: 'wt-1' } + ]) + }) + + it('isolates each read so one rejection cannot strand the follow-up refreshes', async () => { + const calls: StartupCall[] = [] + const { scope, tabs, terminals } = makeScope('wt-1', calls) + + await act(async () => { + renderer = create(createElement(StartupHarness, { scope })) + await Promise.resolve() + }) + await flush() + + await act(async () => { + tabs.reject(new Error('tabs rejected')) + terminals.reject(new Error('terminals rejected')) + await Promise.resolve() + }) + await flush() + + await act(async () => { + vi.advanceTimersByTime(1600) + await Promise.resolve() + }) + // The 750 ms and 1500 ms follow-up refreshes still armed despite both rejections; an + // unguarded await would have thrown out of the startup block and armed neither. + expect(calls.filter((call) => call.rpc === 'terminal.list')).toHaveLength(3) + }) + + it('drops results that land after the route moved to another session', async () => { + const calls: StartupCall[] = [] + const first = makeScope('wt-1', calls) + const second = makeScope('wt-2', calls) + + await act(async () => { + renderer = create(createElement(StartupHarness, { scope: first.scope })) + await Promise.resolve() + }) + await flush() + + await act(async () => { + renderer?.update(createElement(StartupHarness, { scope: second.scope })) + await Promise.resolve() + }) + await flush() + + // The first session's reads land only now, after its effect was torn down. + await act(async () => { + first.tabs.resolve(undefined) + first.terminals.resolve(true) + await Promise.resolve() + }) + await flush() + await act(async () => { + vi.advanceTimersByTime(1600) + await Promise.resolve() + }) + + // Why: a stale settlement must not schedule refreshes for a worktree the route has left. + expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) + }) + + it('withholds worktree.activate until the compatibility verdict lands', async () => { + const calls: StartupCall[] = [] + const pending = makeScope('wt-1', calls, false) + + await act(async () => { + renderer = create(createElement(StartupHarness, { scope: pending.scope })) + await Promise.resolve() + }) + await flush() + + // Why: a desktop that omits protocolVersion evaluates as version 0 and IS blocked, so the + // routes that now mount pre-verdict must not mutate a host the gate is about to refuse. + expect(pending.activateCalls()).toBe(0) + // The reads are not held back with it; that is the whole point of mounting early. + expect(calls).toHaveLength(2) + }) + + it('activates once the verdict lands without re-issuing the reads', async () => { + const calls: StartupCall[] = [] + const pending = makeScope('wt-1', calls, false) + + await act(async () => { + renderer = create(createElement(StartupHarness, { scope: pending.scope })) + await Promise.resolve() + }) + await flush() + expect(pending.activateCalls()).toBe(0) + + // Same session, verdict now proven: only the activation effect may re-run. + const verified = { + ...(pending.scope as unknown as Record), + protocolVerified: true + } as unknown as MobileSessionKeyboardStateModel + await act(async () => { + renderer?.update(createElement(StartupHarness, { scope: verified })) + await Promise.resolve() + }) + await flush() + + expect(pending.activateCalls()).toBe(1) + expect(pending.sendRequest).toHaveBeenCalledWith('worktree.activate', { + worktree: 'id:wt-1', + notifyClients: false, + navigation: 'caller' + }) + expect(calls).toHaveLength(2) + }) + + it('discards both parallel results when the session changes mid-flight', async () => { + const calls: StartupCall[] = [] + const first = makeScope('wt-1', calls) + const second = makeScope('wt-2', calls) + + await act(async () => { + renderer = create(createElement(StartupHarness, { scope: first.scope })) + await Promise.resolve() + }) + await flush() + expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) + + await act(async () => { + renderer?.update(createElement(StartupHarness, { scope: second.scope })) + await Promise.resolve() + }) + await flush() + + // Tabs land late first, then terminals, so each is separately proven inert. + await act(async () => { + first.tabs.resolve(undefined) + await Promise.resolve() + }) + await flush() + await act(async () => { + vi.advanceTimersByTime(1600) + await Promise.resolve() + }) + expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) + + await act(async () => { + first.terminals.resolve(true) + await Promise.resolve() + }) + await flush() + await act(async () => { + vi.advanceTimersByTime(1600) + await Promise.resolve() + }) + expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) + }) +}) diff --git a/mobile/src/session/mobile-session-startup-source.test.ts b/mobile/src/session/mobile-session-startup-source.test.ts index 83b2021b95e..7725d14a5fa 100644 --- a/mobile/src/session/mobile-session-startup-source.test.ts +++ b/mobile/src/session/mobile-session-startup-source.test.ts @@ -30,6 +30,10 @@ const autoCreateHookSource = readMobileSessionRouteSource( './use-initial-session-terminal-autocreate.ts' ) const foundationSource = readMobileSessionRouteSource('./use-mobile-session-foundation.ts') +const activeContentSource = readMobileSessionRouteSource('./MobileSessionActiveContent.tsx') +const subscriptionFoundationSource = readMobileSessionRouteSource( + './use-mobile-session-terminal-subscription-foundation.ts' +) const terminalRuntimeSource = readMobileSessionRouteSource( './use-mobile-session-terminal-runtime.ts' ) @@ -147,35 +151,72 @@ describe('mobile session startup', () => { ) }) - it('loads session tabs without waiting for desktop activation', () => { - const startupEffect = sliceBetween( + // Was: one effect that awaited tabs, then terminals, and fired worktree.activate alongside them. + // The reads are now concurrent and unblocked, while the activation moved to its own effect that + // waits for the compatibility verdict, because it writes host state. + it('loads session tabs and terminals concurrently, ahead of any desktop activation', () => { + const readEffect = sliceBetween( 'void (async () => {', 'return () => {\n disposed = true', startupSource ) - expect(startupEffect).toContain("void client\n .sendRequest('worktree.activate'") - expect(startupEffect).toContain("if (client && created !== '1' && !isFloatingWorkspaceRoute)") - expect(startupEffect).toContain("if (client && created === '1' && !isFloatingWorkspaceRoute)") - expect(startupEffect).toContain('notifyClients: false') - expect(startupEffect).toContain("navigation: 'caller'") - expect(startupEffect).not.toContain("await client\n .sendRequest('worktree.activate'") - expect(startupEffect.indexOf("sendRequest('worktree.activate'")).toBeLessThan( - startupEffect.indexOf('await ensureSessionTabs()') + expect(readEffect).toContain( + 'await Promise.all([\n ensureSessionTabs().catch(() => null),\n fetchTerminals({ allowEmptyLoaded: false }).catch(() => false)\n ])' ) - expect(startupEffect).toContain('headlessActivationNeedsHostRenderer(response.result)') - expect(startupEffect).toContain("showToast('Open Orca on the host to wake sleeping agents.'") + // The reads must not wait on the verdict; that is the point of mounting under the gate. + expect(readEffect).not.toContain('protocolVerified') + expect(readEffect).not.toContain('worktree.activate') + expect(startupSource).toContain('}, [connState, fetchTerminals, ensureSessionTabs])') }) - it('fails runtime capability gates closed before probing a replacement client', () => { + it('holds worktree.activate until the compatibility verdict lands', () => { + const activationEffect = sliceBetween( + "if (connState !== 'connected' || !client || !protocolVerified || isFloatingWorkspaceRoute) {", + 'return () => {\n disposed = true', + startupSource.slice(startupSource.indexOf('worktree.activate') - 2000) + ) + + // Why: a desktop that omits protocolVersion reads as version 0 and IS blocked, so mounting + // this route pre-verdict must not let it mutate a host the gate is about to refuse. + expect(activationEffect).toContain("sendRequest('worktree.activate'") + expect(activationEffect).toContain('notifyClients: false') + expect(activationEffect).toContain("navigation: 'caller'") + expect(activationEffect).toContain("if (created !== '1') {") + expect(activationEffect).toContain('headlessActivationNeedsHostRenderer(response.result)') + expect(activationEffect).toContain("showToast('Open Orca on the host to wake sleeping agents.'") + // The only worktree.activate calls in the route are the two this gated effect owns. + expect(startupSource.split("sendRequest('worktree.activate'")).toHaveLength(2) + expect(startupSource).toContain(' protocolVerified,\n showToast,\n worktreeId\n ])') + }) + + // Was: this route ran its own retrying status.get. The gate above every /h/ route already + // holds that answer, so the second request is gone and the gates read it instead. + it('fails runtime capability gates closed until the shared status.get is proven', () => { const capabilityEffect = sliceBetween( 'const hostQueryReplyInputSupportedRef = useRef(false)', 'return {\n consumeAcceptedSessionTabs', tabReconciliationSource ) - const probeStart = capabilityEffect.indexOf('startRuntimeCapabilityProbe(client,') - expect(probeStart).toBeGreaterThanOrEqual(0) + expect(tabReconciliationSource).not.toContain('startRuntimeCapabilityProbe') + expect(tabReconciliationSource).not.toContain('useHostProtocolGates') + // One read of the gate for the whole route, taken in the foundation and passed down. + expect(foundationSource).toContain( + 'const { compatVerdict, compatVerified, hostCapabilities, statusPending } = useHostProtocolGates()' + ) + // Settled is not passing, and passing-by-fallback is not answered. The write gate reads all + // three, so a host that never answered status.get cannot be mistaken for a verified one. + expect(foundationSource).toContain( + "const protocolVerified = !statusPending && compatVerified && compatVerdict.kind === 'ok'" + ) + expect(capabilityEffect).toContain( + "if (!client || connState !== 'connected' || !protocolVerified) {" + ) + const readStart = capabilityEffect.indexOf( + "setBrowserScreencastSupported(hostCapabilities.includes('browser.screencast.v1'))" + ) + expect(readStart).toBeGreaterThanOrEqual(0) for (const reset of [ 'setBrowserScreencastSupported(null)', 'setAgentSessionHistorySupported(null)', @@ -185,7 +226,7 @@ describe('mobile session startup', () => { ]) { const resetIndex = capabilityEffect.lastIndexOf(reset) expect(resetIndex).toBeGreaterThanOrEqual(0) - expect(resetIndex).toBeLessThan(probeStart) + expect(resetIndex).toBeLessThan(readStart) } }) @@ -290,4 +331,43 @@ describe('mobile session startup', () => { expect(source).toContain('onPendingTerminalRecoveryParked: setParkedPendingTerminalContext') expect(source).toContain('retryPendingTerminalRecovery()') }) + + it('boots the terminal engine while the startup reads are still in flight', () => { + // Why: the loading and pending-terminal states are exactly the window in which the startup + // RPCs are outstanding, so the engine loads there rather than after terminal.list answers. + const loadingBranch = sliceBetween( + 'return showLoadingState ? (', + ') : showEmptyState ? (', + activeContentSource + ) + const prewarmElement = + '' + expect(loadingBranch).toContain(prewarmElement) + expect(loadingBranch).toContain('') + + const pendingBranch = sliceBetween( + ') : activePendingTerminalTab ? (', + ') : (\n (') + expect(activeContentSource.indexOf(' (') + ) + }) + + it('refuses a pre-warm viewport measured before the frame had a height', () => { + const measure = sliceBetween( + 'const measurePrewarmViewport = useCallback(', + ' return {\n getTerminalRef', + subscriptionFoundationSource + ) + expect(measure).toContain('if (viewportMeasuredRef.current || frameHeight <= 0) {') + expect(measure).toContain('await engine.measureFitDimensions(frameHeight)') + // Why: the latch is re-checked after the await so a real pane that measured first wins. + expect(measure).toContain('if (dims && !viewportMeasuredRef.current) {') + }) }) diff --git a/mobile/src/session/terminal-prewarm-frame-geometry.test.ts b/mobile/src/session/terminal-prewarm-frame-geometry.test.ts new file mode 100644 index 00000000000..ca56f406add --- /dev/null +++ b/mobile/src/session/terminal-prewarm-frame-geometry.test.ts @@ -0,0 +1,181 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' +import { readMobileSessionRouteSource } from './mobile-session-route-source-family.test-support' + +type StyleLayer = { top?: number } + +// The applied top offset, read off the rendered pane rather than assumed. +function appliedTopOffset(style: unknown): number { + const layers = (Array.isArray(style) ? style : [style]) as (StyleLayer | null | undefined)[] + return layers.reduce( + (top, layer) => (typeof layer?.top === 'number' ? layer.top : top), + 0 + ) +} + +const engine = vi.hoisted(() => ({ + init: vi.fn((_cols: number, _rows: number) => {}), + awaitReady: vi.fn(async () => {}), + measureFitDimensions: vi.fn(async (_containerHeight?: number) => ({ cols: 100, rows: 40 })), + onWebReady: null as (() => void) | null +})) + +vi.mock('react-native', () => ({ + StyleSheet: { + create: (styles: T) => styles, + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 } + }, + View: 'View' +})) + +vi.mock('../terminal/TerminalWebView', async () => { + const { forwardRef, useImperativeHandle } = await import('react') + return { + TerminalWebView: forwardRef void }>( + function MockTerminalWebView(props, ref) { + engine.onWebReady = props.onWebReady ?? null + useImperativeHandle(ref, () => engine as unknown as TerminalWebViewHandle, []) + return createElement('MockTerminalWebView') + } + ) + } +}) + +import { TerminalEnginePrewarm } from './TerminalEnginePrewarm' +import { + MOBILE_SESSION_TAB_BAR_BORDER_WIDTH, + MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT, + MOBILE_SESSION_TAB_BAR_HEIGHT, + mobileSessionFrameStyles +} from './mobile-session-frame-styles' + +// The box the session content row occupies. Both states below live in it, so it is the one +// number the two frame heights are derived from. +const CONTENT_ROW_HEIGHT = 700 + +// The bar's rendered height, derived from the styles the header actually mounts rather than from +// the constant the pre-warm consumes — otherwise the comparison below would just restate itself. +// React Native sizes a row with no explicit height to its tallest child and puts the border +// outside that, so this is max(children) + border. +function renderedTabBarHeight(): number { + const row = mobileSessionFrameStyles.tabBar as { height?: number; borderTopWidth: number } + // An explicit height here would be border-box and would shrink the row below its children. + expect(row.height).toBeUndefined() + const tallestChild = Math.max( + mobileSessionFrameStyles.tabScroll.maxHeight, + mobileSessionFrameStyles.tab.minHeight, + mobileSessionFrameStyles.newTerminalButton.height, + mobileSessionFrameStyles.tabActionDivider.height + ) + return tallestChild + row.borderTopWidth +} + +// What the first real pane gets once its tab exists and the bar mounts above it. +function firstPaneFrameHeight(): number { + return CONTENT_ROW_HEIGHT - renderedTabBarHeight() +} +const headerSource = readMobileSessionRouteSource('./MobileSessionHeader.tsx') +const activeContentSource = readMobileSessionRouteSource('./MobileSessionActiveContent.tsx') + +// Reproduces React Native's absolute-fill layout: a box pinned to every edge of its parent with +// a top offset gets exactly that much less height. The offset is read off the component, never +// assumed, so a pre-warm that stopped reserving the bar would report the taller box here. +function measuredPrewarmHeight(reservedTabBarHeight: number): number { + let renderer: ReactTestRenderer | null = null + act(() => { + renderer = create( + createElement(TerminalEnginePrewarm, { reservedTabBarHeight, onEngineMeasured: () => {} }) + ) + }) + const created = renderer as unknown as ReactTestRenderer + const applied = appliedTopOffset(created.root.findAllByType('View')[0]?.props.style) + act(() => created.unmount()) + return CONTENT_ROW_HEIGHT - applied +} + +afterEach(() => { + engine.measureFitDimensions.mockClear() + engine.onWebReady = null +}) + +describe('terminal pre-warm frame geometry', () => { + it('states the height the bar actually renders at', () => { + // The constant is what the pre-warm reserves, so it has to equal what the header mounts. + // Deriving the latter from the styles catches the border-box trap: pinning an explicit + // height on the row would render it a pixel short of this sum and drift a whole row. + expect(renderedTabBarHeight()).toBe(MOBILE_SESSION_TAB_BAR_HEIGHT) + expect(MOBILE_SESSION_TAB_BAR_HEIGHT).toBe( + MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT + MOBILE_SESSION_TAB_BAR_BORDER_WIDTH + ) + expect(mobileSessionFrameStyles.tabBar.borderTopWidth).toBe(MOBILE_SESSION_TAB_BAR_BORDER_WIDTH) + // Every child is pinned to the content height, so nothing can grow the row unnoticed. + expect(mobileSessionFrameStyles.tabScroll.maxHeight).toBe(MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT) + expect(mobileSessionFrameStyles.tab.minHeight).toBe(MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT) + expect(mobileSessionFrameStyles.newTerminalButton.height).toBe( + MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT + ) + }) + + it('mounts the tab bar only once a tab is visible, which is what shortens the pane', () => { + expect(headerSource).toContain( + '{visibleTabs.length > 0 && (\n ' + ) + // So the reservation has to be the exact complement of that condition, read off the same list + // the header gates on rather than a proxy for it. + expect(activeContentSource).toContain( + 'const prewarmReservedTabBarHeight = visibleTabs.length > 0 ? 0 : MOBILE_SESSION_TAB_BAR_HEIGHT' + ) + }) + + it('measures the same frame height the first real pane will get', () => { + // Loading: no visible tab, so no tab bar, so the content row is all the pre-warm's to fill, + // minus whatever it reserves. Loaded: the first terminal produces a tab, the bar mounts, and + // the pane gets what is left. The right side is derived from the header's own styles. + expect(measuredPrewarmHeight(MOBILE_SESSION_TAB_BAR_HEIGHT)).toBe(firstPaneFrameHeight()) + }) + + it('would latch a taller frame than the pane if the bar were not reserved', () => { + // Guards the fix rather than the code: without the reservation the pre-warm measures the + // pre-tab-bar box, and every row of that difference is a row the host never had. + const unreserved = measuredPrewarmHeight(0) + expect(unreserved).toBe(CONTENT_ROW_HEIGHT) + expect(unreserved - firstPaneFrameHeight()).toBe(renderedTabBarHeight()) + }) + + it('hands the engine the reserved height, so no refit is owed after the first subscribe', async () => { + let measuredWith: number | null = null + let renderer: ReactTestRenderer | null = null + act(() => { + renderer = create( + createElement(TerminalEnginePrewarm, { + reservedTabBarHeight: MOBILE_SESSION_TAB_BAR_HEIGHT, + textScale: 1, + onEngineMeasured: (_ref: unknown, frameHeight: number) => { + measuredWith = frameHeight + } + }) + ) + }) + const created = renderer as unknown as ReactTestRenderer + const pane = created.root.findAllByType('View')[0] + const applied = appliedTopOffset(pane?.props.style) + act(() => { + pane?.props.onLayout({ + nativeEvent: { layout: { x: 0, y: 0, width: 390, height: CONTENT_ROW_HEIGHT - applied } } + }) + }) + act(() => { + engine.onWebReady?.() + }) + // The handoff waits on the engine's ready promise, so let those microtasks land. + await act(async () => {}) + + // The height the latched viewport is computed from equals the real pane's frame height, so + // the frame-height refit re-measures the same cols/rows and returns before it would send + // terminal.updateViewport (see the prev-dims guard in terminal-viewport-refit.ts). + expect(measuredWith).toBe(firstPaneFrameHeight()) + act(() => created.unmount()) + }) +}) diff --git a/mobile/src/session/terminal-prewarm-refit-debt.test.ts b/mobile/src/session/terminal-prewarm-refit-debt.test.ts new file mode 100644 index 00000000000..57a3567cffa --- /dev/null +++ b/mobile/src/session/terminal-prewarm-refit-debt.test.ts @@ -0,0 +1,147 @@ +import { createElement, useRef, type ReactElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' + +vi.mock('react-native', () => ({ + AppState: { currentState: 'active', addEventListener: () => ({ remove: () => {} }) }, + Platform: { OS: 'android' }, + StyleSheet: { + create: (styles: T) => styles, + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 }, + hairlineWidth: 1 + }, + useWindowDimensions: () => ({ width: 390, height: 844 }), + View: 'View' +})) + +import { useTerminalViewportRefit } from '../terminal/terminal-viewport-refit' +import { MOBILE_SESSION_TAB_BAR_HEIGHT } from './mobile-session-frame-styles' + +const CONTENT_ROW_HEIGHT = 700 +const CELL_HEIGHT = 17 +const HANDLE = 'term-1' +const REFIT_DEBOUNCE_MS = 150 + +// Stands in for the WebView's fit: the taller the box it is handed, the more rows it reports. +// This is what turns a frame that is one tab bar too tall into a row count the host never had. +function fitDimensions(containerHeight: number): { cols: number; rows: number } { + return { cols: 100, rows: Math.floor(containerHeight / CELL_HEIGHT) } +} + +type ColdOpenResult = { + updateViewportCalls: number + resubscribes: number + latchedRows: number +} + +// Replays a single-terminal cold open: the pre-warm measured `prewarmFrameHeight` and latched it, +// the first pane subscribed with those dims, and only then does the real frame report its layout. +async function runSingleTerminalColdOpen(prewarmFrameHeight: number): Promise { + const firstPaneFrameHeight = CONTENT_ROW_HEIGHT - MOBILE_SESSION_TAB_BAR_HEIGHT + const sendRequest = vi.fn(async () => ({ ok: true, result: { updated: true, applied: true } })) + const client = { + sendRequest, + updateTerminalSubscriptionViewport: vi.fn() + } as unknown as RpcClient + const engine = { + measureFitDimensions: vi.fn(async (containerHeight?: number) => + fitDimensions(containerHeight ?? 0) + ), + reflow: vi.fn() + } as unknown as TerminalWebViewHandle + const subscribeToTerminal = vi.fn() + const unsubscribeTerminal = vi.fn() + const viewport = { current: fitDimensions(prewarmFrameHeight) as { cols: number; rows: number } } + const viewportMeasured = { current: true } + // The real pane's frame, reported by its onLayout once the tab bar has mounted. + const frameHeight = { current: firstPaneFrameHeight } + + let notify: ((height: number) => void) | null = null + function RefitHarness(): ReactElement | null { + const terminalRefs = useRef(new Map([[HANDLE, engine]])) + const { notifyTerminalFrameHeight } = useTerminalViewportRefit({ + activeHandleRef: useRef(HANDLE), + terminalRefs, + terminalFrameHeightRef: frameHeight, + viewportRef: viewport, + viewportMeasuredRef: viewportMeasured, + nativeChatCoveredRef: useRef(false), + clientRef: useRef(client), + deviceTokenRef: useRef('device-1'), + initializedHandlesRef: useRef(new Set([HANDLE])), + connState: 'connected', + // One terminal, so the tab-strip corrector is not armed — this is the case that used to + // fall through to the frame-height reducer and pay for the mis-measurement. + tabStripVisible: false, + textScale: 1, + terminalFrameWidth: 390, + unsubscribeTerminal, + subscribeToTerminal + }) + notify = notifyTerminalFrameHeight + return null + } + + let renderer: ReactTestRenderer | null = null + await act(async () => { + renderer = create(createElement(RefitHarness)) + }) + await act(async () => { + notify?.(firstPaneFrameHeight) + }) + // Why drain microtasks between ticks and before unmount: the refit measures and sends inside an + // async block that bails once disposedRef flips, so tearing down early would fake a clean run. + await act(async () => { + vi.advanceTimersByTime(REFIT_DEBOUNCE_MS + 1) + for (let i = 0; i < 10; i += 1) { + await Promise.resolve() + } + }) + await act(async () => { + vi.advanceTimersByTime(REFIT_DEBOUNCE_MS + 1) + for (let i = 0; i < 10; i += 1) { + await Promise.resolve() + } + }) + act(() => (renderer as unknown as ReactTestRenderer).unmount()) + + return { + updateViewportCalls: sendRequest.mock.calls.filter( + ([method]) => method === 'terminal.updateViewport' + ).length, + resubscribes: subscribeToTerminal.mock.calls.length, + latchedRows: viewport.current.rows + } +} + +describe('terminal pre-warm refit debt', () => { + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('owes the host nothing after the first subscribe when the pre-warm reserved the tab bar', async () => { + const reserved = CONTENT_ROW_HEIGHT - MOBILE_SESSION_TAB_BAR_HEIGHT + const result = await runSingleTerminalColdOpen(reserved) + + expect(result.updateViewportCalls).toBe(0) + expect(result.resubscribes).toBe(0) + expect(result.latchedRows).toBe(fitDimensions(reserved).rows) + }) + + it('pays a terminal.updateViewport round trip if the pre-warm measured the pre-tab-bar box', async () => { + // Guards the fix, not the code: this is the frame the pre-warm saw before it reserved the bar. + const result = await runSingleTerminalColdOpen(CONTENT_ROW_HEIGHT) + + expect(result.updateViewportCalls).toBe(1) + // And the rows it had to correct are rows the host was told about and never had. + expect(fitDimensions(CONTENT_ROW_HEIGHT).rows).toBeGreaterThan( + fitDimensions(CONTENT_ROW_HEIGHT - MOBILE_SESSION_TAB_BAR_HEIGHT).rows + ) + }) +}) diff --git a/mobile/src/session/use-mobile-session-foundation.ts b/mobile/src/session/use-mobile-session-foundation.ts index fa2f9607bbc..6f9e0e849ca 100644 --- a/mobile/src/session/use-mobile-session-foundation.ts +++ b/mobile/src/session/use-mobile-session-foundation.ts @@ -14,6 +14,7 @@ import { isFloatingWorkspaceWorktreeId } from './floating-workspace' import { useLiveWorktreeName } from './use-live-worktree-name' import { useMissingWorktreeBounce } from './use-missing-worktree-bounce' import { hostRouteWithNotice } from '../host-route-notice' +import { useHostProtocolGates } from '../components/HostProtocolGate' export function useMobileSessionFoundation() { const { @@ -36,6 +37,14 @@ export function useMobileSessionFoundation() { const insets = useSafeAreaInsets() // Why: shared client per host owned by RpcClientProvider (docs/mobile-shared-client-per-host.md). const { client, clientId, state: connState } = useHostClient(hostId) + // Why: HostProtocolGate holds this connection's single status.get. Reading it here gives the + // whole route one source for host capabilities and for whether the compatibility verdict has + // landed — the routes now mount while it is still in flight, so "not yet known" is a real state. + const { compatVerdict, compatVerified, hostCapabilities, statusPending } = useHostProtocolGates() + // Why all three: a settled verdict is not necessarily a passing one, and a settled *passing* + // verdict is not necessarily an answered one — a host that cannot answer status.get fails open + // to `ok` so navigation still works. Writes read this flag, so they wait for a real reply. + const protocolVerified = !statusPending && compatVerified && compatVerdict.kind === 'ok' const reconnectAttempts = useReconnectAttempt(hostId) const lastConnectedAt = useLastConnectedAt(hostId) const forceReconnectHost = useForceReconnect() @@ -98,6 +107,8 @@ export function useMobileSessionFoundation() { client, clientId, connState, + hostCapabilities, + protocolVerified, reconnectAttempts, lastConnectedAt, forceReconnectHost, diff --git a/mobile/src/session/use-mobile-session-startup.ts b/mobile/src/session/use-mobile-session-startup.ts index f33d081f2cc..3c90433e12e 100644 --- a/mobile/src/session/use-mobile-session-startup.ts +++ b/mobile/src/session/use-mobile-session-startup.ts @@ -12,6 +12,7 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) isFloatingWorkspaceRoute, connState, client, + protocolVerified, setTerminals, terminalsRef, setSessionTabs, @@ -95,6 +96,8 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) worktreeId ]) + // Reads only. They carry no side effect on the host, so they do not wait on the compatibility + // verdict — that is the whole point of mounting this route while status.get is still in flight. // Every setTimeout goes through addTimer into `timers`, which the returned cleanup clears. // react-doctor-disable-next-line react-doctor/effect-needs-cleanup useEffect(() => { @@ -116,58 +119,81 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) timers.push(setTimeout(fn, ms)) } void (async () => { - const reportActivationOutcome = (response: RpcSuccess | null): void => { - if (!disposed && response && headlessActivationNeedsHostRenderer(response.result)) { - showToast('Open Orca on the host to wake sleeping agents.', 3000) - } - } - if (client && created !== '1' && !isFloatingWorkspaceRoute) { - // Why: hydrate host-owned tabs without pulling other paired clients (esp. desktop) into this worktree. - void client - .sendRequest('worktree.activate', { - worktree: `id:${worktreeId}`, - notifyClients: false, - navigation: 'caller' - }) - .then((response) => reportActivationOutcome(response.ok ? response : null)) - .catch(() => null) - } - if (disposed) { - return - } - await ensureSessionTabs().catch(() => null) - if (disposed) { - return - } - await fetchTerminals({ allowEmptyLoaded: false }) + // Why: session.tabs.list and terminal.list are independent reads, so issue both now and + // wait for the pair. Serialising them cost a full extra round trip before the first + // terminal could paint, which on a far relay cell is seconds, not milliseconds. Each + // call keeps its own catch so one rejection cannot strand the other's follow-up refreshes. + await Promise.all([ + ensureSessionTabs().catch(() => null), + fetchTerminals({ allowEmptyLoaded: false }).catch(() => false) + ]) if (disposed) { return } addTimer(() => void fetchTerminals({ allowEmptyLoaded: false }), 750) addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 1500) - if (client && created === '1' && !isFloatingWorkspaceRoute) { - addTimer(() => { - if (activeHandleRef.current) { + })() + return () => { + disposed = true + for (const t of timers) { + clearTimeout(t) + } + } + // Why no client/worktreeId here: both reads are useCallbacks that already list them, so a + // host or worktree change replaces their identity and re-runs this effect with them. + }, [connState, fetchTerminals, ensureSessionTabs]) + + // worktree.activate writes host state, so unlike the reads above it waits for the compatibility + // verdict. A missing protocolVersion reads as 0 and is blocked, so "pending" is not a formality: + // mounting early must not let this route mutate a host the gate is about to refuse. + // Every setTimeout goes through addTimer into `timers`, which the returned cleanup clears. + // react-doctor-disable-next-line react-doctor/effect-needs-cleanup + useEffect(() => { + if (connState !== 'connected' || !client || !protocolVerified || isFloatingWorkspaceRoute) { + return + } + let disposed = false + const timers: ReturnType[] = [] + function addTimer(fn: () => void, ms: number) { + if (disposed) { + return + } + timers.push(setTimeout(fn, ms)) + } + const activateWorktree = () => + client + .sendRequest('worktree.activate', { + worktree: `id:${worktreeId}`, + notifyClients: false, + navigation: 'caller' + }) + .catch(() => null) + const reportActivationOutcome = (response: RpcSuccess | null): void => { + if (!disposed && response && headlessActivationNeedsHostRenderer(response.result)) { + showToast('Open Orca on the host to wake sleeping agents.', 3000) + } + } + if (created !== '1') { + // Why: hydrate host-owned tabs without pulling other paired clients (esp. desktop) into this worktree. + void activateWorktree().then((response) => + reportActivationOutcome(response?.ok ? response : null) + ) + } else { + addTimer(() => { + if (activeHandleRef.current) { + return + } + void (async () => { + const activationResponse = await activateWorktree() + reportActivationOutcome(activationResponse?.ok ? activationResponse : null) + if (disposed) { return } - void (async () => { - const activationResponse = await client - .sendRequest('worktree.activate', { - worktree: `id:${worktreeId}`, - notifyClients: false, - navigation: 'caller' - }) - .catch(() => null) - reportActivationOutcome(activationResponse?.ok ? activationResponse : null) - if (disposed) { - return - } - await fetchTerminals({ allowEmptyLoaded: true }) - addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 750) - })() - }, 1800) - } - })() + await fetchTerminals({ allowEmptyLoaded: true }) + addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 750) + })() + }, 1800) + } return () => { disposed = true for (const t of timers) { @@ -179,8 +205,8 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) connState, created, fetchTerminals, - ensureSessionTabs, isFloatingWorkspaceRoute, + protocolVerified, showToast, worktreeId ]) diff --git a/mobile/src/session/use-mobile-session-tab-reconciliation.ts b/mobile/src/session/use-mobile-session-tab-reconciliation.ts index be4641dd297..22e57944a40 100644 --- a/mobile/src/session/use-mobile-session-tab-reconciliation.ts +++ b/mobile/src/session/use-mobile-session-tab-reconciliation.ts @@ -1,5 +1,4 @@ import { useEffect, useRef, useCallback, useMemo, useState } from 'react' -import { startRuntimeCapabilityProbe } from '../transport/runtime-capability-probe' import { supportsMobileQuickCommands } from '../terminal/quick-commands' import { MOBILE_AI_VAULT_CAPABILITY } from '../agent-history/agent-history-capability' import { TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' @@ -17,6 +16,8 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc worktreeId, client, connState, + hostCapabilities, + protocolVerified, sessionTabsRef, activeSessionTabIdRef, terminalsRef, @@ -144,8 +145,14 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc const hostQueryReplyInputSupportedRef = useRef(false) + // Why: the gate above every /h/ route already holds this connection's status.get answer (and + // retries it until one lands), so the route reads it through the foundation instead of issuing + // a second one. It reports no capabilities until the verdict is proven, which keeps the + // fail-closed reset below identical to the old pre-probe clear. useEffect(() => { - if (!client || connState !== 'connected') { + // Why: a client swap can keep the route connected while moving to an older + // host; clear the prior capability before exposing host-specific actions. + if (!client || connState !== 'connected' || !protocolVerified) { setBrowserScreencastSupported(null) setAgentSessionHistorySupported(null) setQuickCommandsSupported(null) @@ -153,26 +160,15 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc hostQueryReplyInputSupportedRef.current = false return } - // Why: a client swap can keep the route connected while moving to an older - // host; clear the prior capability before exposing host-specific actions. - setBrowserScreencastSupported(null) - setAgentSessionHistorySupported(null) - setQuickCommandsSupported(null) - setShowQuickCommands(false) - hostQueryReplyInputSupportedRef.current = false - // Why: the probe retries — a relay→direct cutover or request timeout rejects - // status.get without changing connState, which used to latch these hidden. - return startRuntimeCapabilityProbe(client, (capabilities) => { - setBrowserScreencastSupported(capabilities.includes('browser.screencast.v1')) - setAgentSessionHistorySupported(capabilities.includes(MOBILE_AI_VAULT_CAPABILITY)) - setQuickCommandsSupported(supportsMobileQuickCommands(capabilities)) - // Why: hosts without this capability strip inputKind from terminal.send, - // so a forwarded xterm reply would become floor-stealing shell input. - hostQueryReplyInputSupportedRef.current = capabilities.includes( - TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY - ) - }) - }, [client, connState]) + setBrowserScreencastSupported(hostCapabilities.includes('browser.screencast.v1')) + setAgentSessionHistorySupported(hostCapabilities.includes(MOBILE_AI_VAULT_CAPABILITY)) + setQuickCommandsSupported(supportsMobileQuickCommands(hostCapabilities)) + // Why: hosts without this capability strip inputKind from terminal.send, + // so a forwarded xterm reply would become floor-stealing shell input. + hostQueryReplyInputSupportedRef.current = hostCapabilities.includes( + TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY + ) + }, [client, connState, hostCapabilities, protocolVerified]) return { consumeAcceptedSessionTabs, hasSessionTabsRecoveryNeed, diff --git a/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts b/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts index 76d8229b288..6c833dfe088 100644 --- a/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts +++ b/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts @@ -1,4 +1,6 @@ import { useRef, useCallback } from 'react' +import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' +import { TERMINAL_ENGINE_PREWARM_HANDLE } from './TerminalEnginePrewarm' import type { MobileSessionNativeChatDictationModel } from './use-mobile-session-native-chat-dictation' export function useMobileSessionTerminalSubscriptionFoundation( @@ -101,12 +103,34 @@ export function useMobileSessionTerminalSubscriptionFoundation( }, [getTerminalRef] ) + // Why: the pre-warm engine occupies the frame the first pane will occupy, so let it satisfy + // the one-shot measurement. It has no handle, so it is passed its own ref instead of looking + // one up, and it must never latch a measurement taken before the frame has a real height. + const measurePrewarmViewport = useCallback( + async (engine: TerminalWebViewHandle, frameHeight: number) => { + if (viewportMeasuredRef.current || frameHeight <= 0) { + return + } + const dims = await engine.measureFitDimensions(frameHeight) + terminalDiagnosticsRef.current.viewportMeasured( + TERMINAL_ENGINE_PREWARM_HANDLE, + dims, + frameHeight + ) + if (dims && !viewportMeasuredRef.current) { + viewportRef.current = dims + viewportMeasuredRef.current = true + } + }, + [] + ) return { getTerminalRef, unsubscribeTerminal, unsubscribeTerminalRef, clearTerminalCache, - measureViewportOnce + measureViewportOnce, + measurePrewarmViewport } } diff --git a/mobile/src/transport/host-status-gates.ts b/mobile/src/transport/host-status-gates.ts index 91f0205a5c7..4f8bc8fcc52 100644 --- a/mobile/src/transport/host-status-gates.ts +++ b/mobile/src/transport/host-status-gates.ts @@ -1,6 +1,7 @@ import { useEffect, useState } from 'react' import type { RpcClient } from './rpc-client' -import type { ConnectionState, RpcSuccess } from './types' +import type { ConnectionState } from './types' +import { readRuntimeCapabilities, startRuntimeStatusProbe } from './runtime-status-probe' import { evaluateCompat, type CompatVerdict } from './protocol-compat' import type { DesktopStatus } from '../worktree/host-worktree-rpc-types' import { normalizeHostAppVersion, recordHostAppVersion } from './host-app-version-store' @@ -10,6 +11,10 @@ export type HostStatusGates = { floatingWorkspaceEnabled: boolean desktopAppVersion: string | null compatVerdict: CompatVerdict + // Why: `compatVerdict.kind === 'ok'` is not proof. A host that never answers status.get settles + // the same `ok` so navigation is not trapped, and that fallback must not read as a passing + // verdict. Only an evaluated status reply sets this, so writes to the host can gate on it. + compatVerified: boolean statusPending: boolean } @@ -21,8 +26,11 @@ type LoadedHostStatusGates = Omit & { const EMPTY_HOST_CAPABILITIES: string[] = [] -// Reads status.get on connect for capabilities, protocol-compat verdict, and the -// floating-workspace flag. Compat constants are wide-open today so this never blocks yet. +// The route tree's single status.get: it reads capabilities, the protocol-compat verdict, and +// the floating-workspace flag once per connection and publishes them through HostProtocolGate, +// so no descendant issues its own. The verdict really can block — evaluateCompat reads a missing +// protocolVersion as 0, below MIN_COMPATIBLE_DESKTOP_VERSION — so a pending verdict is a real +// state, not a formality, and anything that writes to the host must wait for it. export function useHostStatusGates(args: { hostId: string | undefined client: RpcClient | null @@ -39,30 +47,33 @@ export function useHostStatusGates(args: { setUnverified(true) return } - let cancelled = false const requestClient = client const settle = (gates: Omit) => { setLoaded({ hostId, client: requestClient, ...gates }) setUnverified(false) } - void (async () => { - try { - const response = await requestClient.sendRequest('status.get') - if (cancelled) { - return - } - if (!response.ok) { - settle({ - hostCapabilities: [], - floatingWorkspaceEnabled: false, - desktopAppVersion: null, - compatVerdict: { kind: 'ok' } - }) - return - } - const status = (response as RpcSuccess).result as DesktopStatus & { - capabilities?: string[] - } + // Why: a transient status failure must not trap navigation, so the first miss settles + // conservative gates and releases the pending overlay; the probe keeps retrying underneath + // so a cutover or timeout no longer latches capability-gated UI hidden until a remount. + // compatVerified stays false: this releases the UI, it proves nothing about the host. + let failedOpen = false + const failOpen = () => { + if (failedOpen) { + return + } + failedOpen = true + settle({ + hostCapabilities: [], + floatingWorkspaceEnabled: false, + desktopAppVersion: null, + compatVerdict: { kind: 'ok' }, + compatVerified: false + }) + } + return startRuntimeStatusProbe(requestClient, { + onUnavailable: failOpen, + onStatus: (result) => { + const status = result as DesktopStatus & { capabilities?: string[] } const verdict = evaluateCompat({ desktopProtocolVersion: status.protocolVersion, desktopMinCompatibleMobileVersion: status.minCompatibleMobileVersion @@ -72,10 +83,11 @@ export function useHostStatusGates(args: { void recordHostAppVersion(hostId, desktopAppVersion) } settle({ - hostCapabilities: status.capabilities ?? [], + hostCapabilities: [...readRuntimeCapabilities(result)], floatingWorkspaceEnabled: status.floatingWorkspaceEnabled === true, desktopAppVersion, - compatVerdict: verdict + compatVerdict: verdict, + compatVerified: true }) if (verdict.kind === 'blocked') { // Why: support breadcrumb to confirm a block fired vs a render bug; no PII, just version ints. @@ -86,21 +98,8 @@ export function useHostStatusGates(args: { requiredDesktopVersion: verdict.requiredDesktopVersion }) } - } catch { - // Why: a transient status failure must not trap navigation; conservative feature gates remain disabled. - if (!cancelled) { - settle({ - hostCapabilities: [], - floatingWorkspaceEnabled: false, - desktopAppVersion: null, - compatVerdict: { kind: 'ok' } - }) - } } - })() - return () => { - cancelled = true - } + }) }, [client, connState, hostId]) // Why: effects run after render, so key loaded gates by host and client to fail closed during route reuse. @@ -111,6 +110,7 @@ export function useHostStatusGates(args: { floatingWorkspaceEnabled: false, desktopAppVersion: null, compatVerdict: { kind: 'ok' }, + compatVerified: false, statusPending: connState === 'connected' && client !== null } } @@ -119,6 +119,7 @@ export function useHostStatusGates(args: { floatingWorkspaceEnabled: proven.floatingWorkspaceEnabled, desktopAppVersion: proven.desktopAppVersion, compatVerdict: proven.compatVerdict, + compatVerified: proven.compatVerified, // Why (F10): unchanged pending timing — the reconnect refetch is still "unknown", it just no // longer blanks the capabilities this same host already proved. statusPending: connState === 'connected' && unverified diff --git a/mobile/src/transport/runtime-capability-probe.test.ts b/mobile/src/transport/runtime-status-probe.test.ts similarity index 67% rename from mobile/src/transport/runtime-capability-probe.test.ts rename to mobile/src/transport/runtime-status-probe.test.ts index 2272c25610f..9b1de5c391e 100644 --- a/mobile/src/transport/runtime-capability-probe.test.ts +++ b/mobile/src/transport/runtime-status-probe.test.ts @@ -1,5 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { startRuntimeCapabilityProbe } from './runtime-capability-probe' +import { + readRuntimeCapabilities, + startRuntimeCapabilityProbe, + startRuntimeStatusProbe +} from './runtime-status-probe' import { LogicalClientCutoverError } from './stable-logical-rpc-client' import type { RpcClient } from './rpc-client' import type { RpcResponse } from './types' @@ -125,19 +129,46 @@ describe('startRuntimeCapabilityProbe', () => { cancel() }) - it('retries an ok:false response instead of settling', async () => { + // Was: an ok:false response was retried like a timeout. The probe now backs the gate that sits + // above every /h/ route, so polling a host that already answered would run for the life of the + // connection. A reply is an answer; only an unanswered request is retried. + it('settles once on an ok:false response rather than polling the host', async () => { const failure: RpcResponse = { ok: false, id: '1', error: { code: 'internal', message: 'nope' }, _meta: { runtimeId: 'r1' } } - const { client } = makeClient([failure, ok(['a.v1'])]) + const { client, calls } = makeClient([failure, ok(['a.v1'])]) const seen: (readonly string[])[] = [] - const cancel = startRuntimeCapabilityProbe(client, (capabilities) => seen.push(capabilities)) + const retrying: boolean[] = [] + const cancel = startRuntimeStatusProbe(client, { + onStatus: (status) => seen.push(readRuntimeCapabilities(status)), + onUnavailable: (isRetrying) => retrying.push(isRetrying) + }) await flushMicrotasks() + expect(retrying).toEqual([false]) expect(seen).toEqual([]) + + await vi.advanceTimersByTimeAsync(60_000) + expect(calls()).toBe(1) + expect(seen).toEqual([]) + cancel() + }) + + it('still retries a request the host never answered', async () => { + const { client, calls } = makeClient([new Error('timeout'), ok(['a.v1'])]) + const seen: (readonly string[])[] = [] + const retrying: boolean[] = [] + const cancel = startRuntimeStatusProbe(client, { + onStatus: (status) => seen.push(readRuntimeCapabilities(status)), + onUnavailable: (isRetrying) => retrying.push(isRetrying) + }) + await flushMicrotasks() + expect(retrying).toEqual([true]) + await vi.advanceTimersByTimeAsync(1_000) + expect(calls()).toBe(2) expect(seen).toEqual([['a.v1']]) cancel() }) @@ -182,4 +213,40 @@ describe('startRuntimeCapabilityProbe', () => { await flushMicrotasks() expect(seen).toEqual([]) }) + + it('reports the full status, not just capabilities', async () => { + const response: RpcResponse = { + ok: true, + id: '1', + result: { appVersion: '1.4.0', protocolVersion: 7, capabilities: ['a.v1'] }, + _meta: { runtimeId: 'r1' } + } + const { client } = makeClient([response]) + const seen: Record[] = [] + const cancel = startRuntimeStatusProbe(client, { onStatus: (status) => seen.push(status) }) + await flushMicrotasks() + expect(seen).toEqual([{ appVersion: '1.4.0', protocolVersion: 7, capabilities: ['a.v1'] }]) + cancel() + }) + + // Why: the gate needs to release its pending cover on the first miss rather than wait out the + // retries, so a wedged status.get cannot hold the whole host UI behind a spinner. + it('announces each failed attempt while the retry is still pending', async () => { + const { client, calls } = makeClient([new Error('boom'), ok(['a.v1'])]) + const misses: number[] = [] + const seen: Record[] = [] + const cancel = startRuntimeStatusProbe(client, { + onStatus: (status) => seen.push(status), + onUnavailable: () => misses.push(calls()) + }) + await flushMicrotasks() + expect(misses).toEqual([1]) + expect(seen).toEqual([]) + + await vi.advanceTimersByTimeAsync(1_000) + await flushMicrotasks() + expect(seen).toEqual([{ capabilities: ['a.v1'] }]) + expect(misses).toEqual([1]) + cancel() + }) }) diff --git a/mobile/src/transport/runtime-capability-probe.ts b/mobile/src/transport/runtime-status-probe.ts similarity index 51% rename from mobile/src/transport/runtime-capability-probe.ts rename to mobile/src/transport/runtime-status-probe.ts index ef636552863..03cec914871 100644 --- a/mobile/src/transport/runtime-capability-probe.ts +++ b/mobile/src/transport/runtime-status-probe.ts @@ -9,9 +9,20 @@ const CUTOVER_RETRY_DELAY_MS = 250 const FAILURE_RETRY_BASE_DELAY_MS = 1_000 const FAILURE_RETRY_MAX_DELAY_MS = 15_000 -export function startRuntimeCapabilityProbe( - client: RpcClient, - onCapabilities: (capabilities: readonly string[]) => void +export type RuntimeStatusProbeHandlers = { + onStatus: (status: Record) => void + // Fires once per attempt that produced no status. `retrying` is false when the host itself + // answered with an error: that is a definitive reply, so the probe stops rather than polling a + // host that has already said no. It is true when nothing reached us and a retry is armed, which + // lets a caller that must not stay blocked fail open on the first miss and be upgraded later. + onUnavailable?: (retrying: boolean) => void +} + +// Single status.get producer for a connected client: one request, retried until it +// lands. Callers share the answer instead of each issuing their own status.get. +export function startRuntimeStatusProbe( + client: Pick, + handlers: RuntimeStatusProbeHandlers ): () => void { let cancelled = false let retryTimer: ReturnType | null = null @@ -24,20 +35,15 @@ export function startRuntimeCapabilityProbe( return } if (!response.ok) { - scheduleRetry(false) + // Why not retry: the desktop replied. Re-asking every 15 s for the life of a connection + // from a probe mounted above every /h/ route buys nothing a reconnect would not. + handlers.onUnavailable?.(false) return } const result = (response as RpcSuccess).result - const rawCapabilities = - result && typeof result === 'object' - ? (result as { capabilities?: unknown }).capabilities - : null - const capabilities = - Array.isArray(rawCapabilities) && - rawCapabilities.every((value) => typeof value === 'string') - ? rawCapabilities - : [] - onCapabilities(capabilities) + handlers.onStatus( + result && typeof result === 'object' ? (result as Record) : {} + ) }, (error: unknown) => { if (cancelled) { @@ -55,6 +61,7 @@ export function startRuntimeCapabilityProbe( ? CUTOVER_RETRY_DELAY_MS : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) retryTimer = setTimeout(attempt, delay) + handlers.onUnavailable?.(true) } attempt() @@ -65,3 +72,17 @@ export function startRuntimeCapabilityProbe( } } } + +export function readRuntimeCapabilities(status: Record): readonly string[] { + const raw = status.capabilities + return Array.isArray(raw) && raw.every((value) => typeof value === 'string') ? raw : [] +} + +export function startRuntimeCapabilityProbe( + client: Pick, + onCapabilities: (capabilities: readonly string[]) => void +): () => void { + return startRuntimeStatusProbe(client, { + onStatus: (status) => onCapabilities(readRuntimeCapabilities(status)) + }) +} diff --git a/mobile/src/worktree/home-host-worktree-fetch.ts b/mobile/src/worktree/home-host-worktree-fetch.ts index 72b9e572ba1..4e9cfa3b2ab 100644 --- a/mobile/src/worktree/home-host-worktree-fetch.ts +++ b/mobile/src/worktree/home-host-worktree-fetch.ts @@ -13,7 +13,7 @@ import { WORKTREE_PS_FULL_LIMIT } from './worktree-catalog-snapshot-client' const ACTIVE_STATUSES = new Set(['working', 'active', 'permission']) // Why: a relay↔direct cutover rejects in-flight reads without ever leaving 'connected', so the // connect gate never re-arms. Re-issue on the replacement session; cap it so a migration loop -// can't spin. See runtime-capability-probe.ts for the same hazard on status.get. +// can't spin. See runtime-status-probe.ts for the same hazard on status.get. const CUTOVER_RETRY_LIMIT = 2 export type HostWorktreeInfoSetter = (