From 5b0bdb3ee2cf2ea338279dfb15b46d98cad45e85 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 17:23:28 -0400 Subject: [PATCH] fix(mobile): send worktree.activate only after the host compat verdict settles --- .../src/components/HostProtocolGate.test.ts | 171 ++++++++++++++++++ .../mobile-session-route-parity.test.ts | 24 +-- .../mobile-session-startup-source.test.ts | 50 +++-- .../session/use-mobile-session-foundation.ts | 4 + .../src/session/use-mobile-session-startup.ts | 112 +++++++----- 5 files changed, 297 insertions(+), 64 deletions(-) diff --git a/mobile/src/components/HostProtocolGate.test.ts b/mobile/src/components/HostProtocolGate.test.ts index 44a2265ccb3..b8b5ffb175e 100644 --- a/mobile/src/components/HostProtocolGate.test.ts +++ b/mobile/src/components/HostProtocolGate.test.ts @@ -2,6 +2,7 @@ import { createElement, useEffect } 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 { useMobileSessionStartup } from '../session/use-mobile-session-startup' import { HostProtocolGate, useHostProtocolGates } from './HostProtocolGate' const nativeTestState = vi.hoisted(() => ({ @@ -74,6 +75,95 @@ function renderedText(renderer: ReactTestRenderer): string { return JSON.stringify(renderer.toJSON()) } +// Why: mirrors the route's foundation -> startup plumbing (statusPending from the gate feeding the +// real startup hook) so the assertion lands on the shipped hook, not a restatement of it. +type StartupScope = Parameters[0] + +function createStartupScope() { + const noop = () => {} + return { + created: undefined as string | undefined, + activeHandleRef: { current: null as string | null }, + hostId: 'host-1', + worktreeId: 'wt-1', + isFloatingWorkspaceRoute: false, + setTerminals: noop, + terminalsRef: { current: [] }, + setSessionTabs: noop, + appliedSnapshotMarkerRef: { current: { epoch: null, version: -1 } }, + closedTabTombstonesRef: { current: new Map() }, + setTerminalsLoaded: noop, + setActiveHandle: noop, + setActiveSessionTabId: noop, + setMarkdownDocs: noop, + setFileDocs: noop, + terminalGestureInputQueuesRef: { current: new Map() }, + terminalGestureInputInFlightRef: { current: new Set() }, + sessionTabActionSheetKeyboardHideSubRef: { current: null }, + sessionTabActionSheetRequestSeqRef: { current: 0 }, + initializedHandlesRef: { current: new Set() }, + terminalDiagnosticsRef: { current: { resetRoute: noop } }, + activeSessionTabTypeRef: { current: null }, + pendingActiveSessionTabIdRef: { current: null }, + selectedSessionTabIdRef: { current: null }, + pendingActiveTerminalHandleRef: { current: null }, + pendingBrowserFocusPageIdRef: { current: null }, + pendingTerminalActivationAttemptRef: { current: null }, + initialSessionAutoCreateRef: { current: null }, + bufferedTerminalDraftState: { resetDrafts: noop, clearPendingRestorations: noop }, + clearPendingLiveInputCommit: noop, + clearDelayedActionTimers: noop, + showToast: noop, + clearTerminalCache: noop, + fetchTerminals: async () => true, + ensureSessionTabs: async () => {} + } +} + +let startupScope = createStartupScope() + +function StartupProbe() { + const { statusPending } = useHostProtocolGates() + useMobileSessionStartup({ + ...startupScope, + client: hostClient.current.client, + connState: hostClient.current.state, + statusPending + } as unknown as StartupScope) + return createElement('StartupProbe') +} + +// status.get stays unresolved until the test settles it; every other method lands on `activate`. +function clientWithDeferredStatus() { + let resolveStatus: (result: Record) => void = () => {} + const status = new Promise<{ ok: true; result: Record }>((resolve) => { + resolveStatus = (result) => resolve({ ok: true, result }) + }) + const activate = vi.fn().mockResolvedValue({ ok: true, result: {} }) + const client = { + sendRequest: (method: string, params?: unknown) => + method === 'status.get' ? status : activate(method, params) + } as unknown as RpcClient + return { activate, client, settle: (result: Record) => resolveStatus(result) } +} + +function startupGateElement() { + return createElement( + HostProtocolGate, + { hostId: 'host-1' }, + createElement('HostContent', null, createElement(StartupProbe)) + ) +} + +async function renderStartupGate(): Promise { + let created: ReactTestRenderer | null = null + await act(async () => { + created = create(startupGateElement()) + await Promise.resolve() + }) + return created as unknown as ReactTestRenderer +} + describe('HostProtocolGate', () => { let renderer: ReactTestRenderer | null = null @@ -81,11 +171,13 @@ describe('HostProtocolGate', () => { nativeTestState.openUrl.mockClear() nativeTestState.platform.OS = 'ios' probeMounts.count = 0 + startupScope = createStartupScope() }) afterEach(() => { act(() => renderer?.unmount()) renderer = null + vi.useRealTimers() vi.restoreAllMocks() }) @@ -259,4 +351,83 @@ describe('HostProtocolGate', () => { renderer = await renderGate() expect(renderedText(renderer)).toContain('HostContent') }) + // Reaches the connected-but-unverified window the way production does: the route mounts while the + // client is still connecting, so the gate overlays it instead of holding the tree back. + async function connectWithPendingStatus(client: RpcClient): Promise { + await act(async () => { + hostClient.current = { client, state: 'connected' } + renderer?.update(startupGateElement()) + await Promise.resolve() + }) + } + + async function settleVerdict(settle: (result: Record) => void): Promise { + await act(async () => { + settle({ protocolVersion: 3, minCompatibleMobileVersion: 0 }) + await Promise.resolve() + }) + } + + it('holds worktree.activate until the host compat verdict settles', async () => { + const { activate, client, settle } = clientWithDeferredStatus() + hostClient.current = { client: null, state: 'connecting' } + renderer = await renderStartupGate() + expect(activate).not.toHaveBeenCalled() + + await connectWithPendingStatus(client) + expect(renderedText(renderer)).toContain('Checking host compatibility') + // Why: activate writes host state (protocol-1 hosts ignore notifyClients: false), so an + // unknown verdict must not have produced one. + expect(activate).not.toHaveBeenCalled() + + await settleVerdict(settle) + expect(activate).toHaveBeenCalledTimes(1) + expect(activate).toHaveBeenCalledWith('worktree.activate', { + worktree: 'id:wt-1', + notifyClients: false, + navigation: 'caller' + }) + }) + + it('defers the created-workspace activate recovery past the pending verdict', async () => { + vi.useFakeTimers() + startupScope.created = '1' + const { activate, client, settle } = clientWithDeferredStatus() + hostClient.current = { client: null, state: 'connecting' } + renderer = await renderStartupGate() + await connectWithPendingStatus(client) + + await act(async () => { + await vi.advanceTimersByTimeAsync(5000) + }) + expect(activate).not.toHaveBeenCalled() + + await settleVerdict(settle) + await act(async () => { + await vi.advanceTimersByTimeAsync(1800) + }) + expect(activate).toHaveBeenCalledTimes(1) + expect(activate).toHaveBeenCalledWith('worktree.activate', { + worktree: 'id:wt-1', + notifyClients: false, + navigation: 'caller' + }) + }) + + it('skips the created-workspace activate recovery when a terminal is already active', async () => { + vi.useFakeTimers() + startupScope.created = '1' + const { activate, client, settle } = clientWithDeferredStatus() + hostClient.current = { client: null, state: 'connecting' } + renderer = await renderStartupGate() + await connectWithPendingStatus(client) + await settleVerdict(settle) + // The route reset clears this ref on mount, so a terminal can only claim it after the verdict. + startupScope.activeHandleRef.current = 'term-1' + + await act(async () => { + await vi.advanceTimersByTimeAsync(5000) + }) + expect(activate).not.toHaveBeenCalled() + }) }) diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index bc951bfa206..94c9f7fb310 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -62,24 +62,24 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = '10071240ef9edafc2b9c8bed73be83dceaf7828e3b29f17dab55da020a7697a6' -const HEAD_HOOK_BINDING_SHA256 = '1dadb8c3dc0573ea20659ce7251629669e618dd0effaeac3a4536b29c2e865a1' +const HEAD_MAIN_HOOK_SHA256 = 'c4e5825b6fe1ba9c64b5555fb3b0c6d37e7a72558cb5bd27df306b6649555891' +const HEAD_HOOK_BINDING_SHA256 = '969b8bc0193af0e817dcdb682fbed4cfd5a44c228b3e664860c4c94dbd426b4f' const HEAD_CALLBACK_IDENTITY_SHA256 = '2a9e4825df007f6ef53b81aa5004991d6318eee7507b44d625c07e630be432eb' const HEAD_CALLBACK_BODY_SHA256 = '22103ba85a86e3a3fcb80a7509c7a455d79863010cde3af02db6565b55e3ebe9' -const HEAD_EFFECT_SHA256 = 'd9ebfaabc1e79773cdada7ab370b20459ed972f1f8edce1652199f4d0391cd13' +const HEAD_EFFECT_SHA256 = '327c84aa19f531e2303cd8efa6cbe35304965131697c5081f09edba29fa3d645' 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' + '7b29db89d9b60acb732e80b2169b3315f32abe22de39e3b94c4196b352765dc3' const HEAD_HOST_JSX_SHA256 = '390405926b1695fa3a33686f0bc192b432f5468d8576499d7cafbb4922defbb5' const HEAD_LEAF_JSX_SHA256 = '21dba981875e173f692590bf910d60964660c5f4cbb79f3a377c7e54f6a1f016' const HEAD_STYLE_REFERENCE_SHA256 = @@ -472,18 +472,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(268) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) expect(main.callbacks).toHaveLength(77) 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,13 +494,13 @@ 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 diff --git a/mobile/src/session/mobile-session-startup-source.test.ts b/mobile/src/session/mobile-session-startup-source.test.ts index 83b2021b95e..4096ee2c3dc 100644 --- a/mobile/src/session/mobile-session-startup-source.test.ts +++ b/mobile/src/session/mobile-session-startup-source.test.ts @@ -148,23 +148,51 @@ describe('mobile session startup', () => { }) it('loads session tabs without waiting for desktop activation', () => { - const startupEffect = sliceBetween( + const readsEffect = 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(readsEffect).toContain('await ensureSessionTabs().catch(() => null)') + expect(readsEffect).toContain('await fetchTerminals({ allowEmptyLoaded: false })') + expect(readsEffect).toContain( + 'addTimer(() => void fetchTerminals({ allowEmptyLoaded: false }), 750)' ) - expect(startupEffect).toContain('headlessActivationNeedsHostRenderer(response.result)') - expect(startupEffect).toContain("showToast('Open Orca on the host to wake sleeping agents.'") + expect(readsEffect).toContain( + 'addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 1500)' + ) + // Why: hydration must not be sequenced behind a host write, so activation lives in its own effect. + expect(readsEffect).not.toContain('worktree.activate') + }) + + it('sends worktree.activate only after the host compat verdict settles', () => { + expect(foundationSource).toContain('const { statusPending } = useHostProtocolGates()') + expect(foundationSource).toContain( + "import { useHostProtocolGates } from '../components/HostProtocolGate'" + ) + expect(foundationSource).toContain('statusPending,') + expect(startupSource).toContain(' statusPending,\n') + + const activateEffect = sliceBetween( + 'if (statusPending || connState !== ', + 'return () => {\n disposed = true', + startupSource.slice(startupSource.indexOf('// Why: activate writes host state')) + ) + + expect(activateEffect).toContain( + "if (statusPending || connState !== 'connected' || !client || isFloatingWorkspaceRoute)" + ) + expect(activateEffect).toContain("void client\n .sendRequest('worktree.activate'") + expect(activateEffect).toContain("if (created !== '1')") + expect(activateEffect).toContain('notifyClients: false') + expect(activateEffect).toContain("navigation: 'caller'") + expect(activateEffect).not.toContain('await ensureSessionTabs()') + // The created-workspace recovery still yields to a terminal that claimed the route first. + expect(activateEffect).toContain('if (activeHandleRef.current)') + expect(activateEffect).toContain('}, 1800)') + expect(activateEffect).toContain('headlessActivationNeedsHostRenderer(response.result)') + expect(activateEffect).toContain("showToast('Open Orca on the host to wake sleeping agents.'") }) it('fails runtime capability gates closed before probing a replacement client', () => { diff --git a/mobile/src/session/use-mobile-session-foundation.ts b/mobile/src/session/use-mobile-session-foundation.ts index fa2f9607bbc..c742fcd56f7 100644 --- a/mobile/src/session/use-mobile-session-foundation.ts +++ b/mobile/src/session/use-mobile-session-foundation.ts @@ -3,6 +3,7 @@ import { useSafeAreaInsets } from 'react-native-safe-area-context' import { useLocalSearchParams, useRouter } from 'expo-router' import { HOST_DOCK_MIN_WIDTH } from '../storage/preferences' import { useHostClient, useForceReconnect } from '../transport/client-context' +import { useHostProtocolGates } from '../components/HostProtocolGate' import { useLastConnectedAt, useReconnectAttempt @@ -36,6 +37,8 @@ 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: every /h/ route renders under ; host-writing startup RPCs wait on its verdict. + const { statusPending } = useHostProtocolGates() const reconnectAttempts = useReconnectAttempt(hostId) const lastConnectedAt = useLastConnectedAt(hostId) const forceReconnectHost = useForceReconnect() @@ -98,6 +101,7 @@ export function useMobileSessionFoundation() { client, clientId, connState, + statusPending, 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..7bf630d2538 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, + statusPending, setTerminals, terminalsRef, setSessionTabs, @@ -116,25 +117,6 @@ 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 @@ -145,28 +127,6 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) } addTimer(() => void fetchTerminals({ allowEmptyLoaded: false }), 750) addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 1500) - if (client && created === '1' && !isFloatingWorkspaceRoute) { - addTimer(() => { - if (activeHandleRef.current) { - 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) - } })() return () => { disposed = true @@ -184,4 +144,74 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) showToast, worktreeId ]) + + // Why: activate writes host state, so it waits for the compat verdict to settle; a blocked + // verdict unmounts this route before the effect can run. + // Every setTimeout goes through addTimer into `timers`, which the returned cleanup clears. + // react-doctor-disable-next-line react-doctor/effect-needs-cleanup + useEffect(() => { + if (statusPending || connState !== 'connected' || !client || isFloatingWorkspaceRoute) { + return + } + let disposed = false + const timers: ReturnType[] = [] + function addTimer(fn: () => void, ms: number) { + if (disposed) { + return + } + timers.push(setTimeout(fn, ms)) + } + 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 client + .sendRequest('worktree.activate', { + worktree: `id:${worktreeId}`, + notifyClients: false, + navigation: 'caller' + }) + .then((response) => reportActivationOutcome(response.ok ? response : null)) + .catch(() => null) + } else { + addTimer(() => { + if (activeHandleRef.current) { + 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) + } + return () => { + disposed = true + for (const t of timers) { + clearTimeout(t) + } + } + }, [ + client, + connState, + created, + fetchTerminals, + isFloatingWorkspaceRoute, + showToast, + statusPending, + worktreeId + ]) }