From e7e950a55e0b35cade8ae0b1f3e9fbfb05482a32 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 16:47:53 -0400 Subject: [PATCH 1/3] fix(mobile): tie the hosted page broker to the document lifecycle A hosted document is replaced without the shell session, its build or the view epoch moving: a native-route excursion deactivates the view to about:blank and reloads on return, the route error boundary reloads in place, and a re-attached view reloads its URL. The previous page's broker survived all three, and its subscription records kept holding every per-operation grant (one for workspace, account and source control), so the new document's subscribes were refused with rate_limited for the rest of the shell session and the host kept publishing to records nobody read. The shell owns the document lifecycle, so it now observes it: the native views report every document load start (Android through onPageStarted, iOS through didStartProvisionalNavigation, which is the only signal a page-initiated reload gives), and a load that displaces a document that finished loading bumps a page document epoch. The broker hook retires on that epoch exactly as it does on a view-epoch bump, before the incoming document is initialized. A first load and the duplicate load-start events for one navigation are not replacements and do not retire anything. Android also never destroyed its WebView: OnViewDestroys only dropped the registry entry, so every package swap, host switch and route exit leaked a renderer process, a JavaScript context, the message listener and both script handlers. Unmount now destroys it; deactivation still only parks it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/hybrid.tsx | 28 +- .../ExpoMobileWebShellModule.kt | 2 + .../mobilewebshell/MobileWebShellView.kt | 38 ++- .../ios/MobileWebShellView.swift | 8 + .../MobileWebHybridShellPresentation.tsx | 8 + .../hosted-webview-document-lifecycle.test.ts | 47 ++++ ...obile-native-shell-route-ownership.test.ts | 2 +- .../mobile-web-document-replacement.test.tsx | 264 ++++++++++++++++++ .../mobile-web-shell-load-failure.test.tsx | 1 + .../use-mobile-web-capability-broker.test.ts | 22 +- .../use-mobile-web-capability-broker.ts | 7 +- .../use-mobile-web-page-document.ts | 71 +++++ 12 files changed, 479 insertions(+), 19 deletions(-) create mode 100644 mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts create mode 100644 mobile/src/mobile-web/mobile-web-document-replacement.test.tsx create mode 100644 mobile/src/mobile-web/use-mobile-web-page-document.ts diff --git a/mobile/app/hybrid.tsx b/mobile/app/hybrid.tsx index 7f15ad67866..3101553ca63 100644 --- a/mobile/app/hybrid.tsx +++ b/mobile/app/hybrid.tsx @@ -13,6 +13,7 @@ import { useMobileWebCapabilityBroker, type MobileWebBrokerPageIdentity } from '../src/mobile-web/use-mobile-web-capability-broker' +import { useMobileWebPageDocument } from '../src/mobile-web/use-mobile-web-page-document' import { MOBILE_WEB_PRODUCTION_GRANTS } from '../src/mobile-web/mobile-web-production-grants' import { MobileWebHealthDeadline } from '../src/mobile-web/mobile-web-health-deadline' import { useMobileWebAlertSafePackageSession } from '../src/mobile-web/use-mobile-web-alert-safe-package-session' @@ -49,7 +50,6 @@ export default function HybridScreen() { const params = useLocalSearchParams<{ hostId?: string }>() const viewRef = useRef(null) const activeSessionIdRef = useRef(undefined) - const initializedSessionRef = useRef(undefined) const healthDeadlineRef = useRef(new MobileWebHealthDeadline(10_000)) const brokerRef = useRef(null) const postInitRef = useRef<() => Promise>(() => Promise.resolve()) @@ -60,7 +60,6 @@ export default function HybridScreen() { ) const { hosts, hostsLoading, hostLoadError, refreshHosts } = useMobileWebHostCatalog() const [selectedHostId, setSelectedHostId] = useState(params.hostId) - const [pageReadySessionId, setPageReadySessionId] = useState() const [brokerSessionId, setBrokerSessionId] = useState() const [hostedViewActive, setHostedViewActive] = useState(true) const selectHost = useCallback((hostId: string | undefined) => setSelectedHostId(hostId), []) @@ -140,14 +139,12 @@ export default function HybridScreen() { } }, [params.hostId]) - // A view-epoch bump replaces the document, so every page-scoped grant retires with it. - useEffect(() => { - initializedSessionRef.current = undefined - nativeRouteHandoffRef.current.clear() - setPageReadySessionId(undefined) - healthDeadlineRef.current.clear() - return () => healthDeadlineRef.current.clear() - }, [session?.sessionId, viewEpoch]) + const pageDocument = useMobileWebPageDocument({ + sessionId: session?.sessionId, + viewEpoch, + healthDeadlineRef, + routeHandoffRef: nativeRouteHandoffRef + }) const postToWeb = useCallback(async (message: MobileWebBridgeShellMessage) => { if (responseDropRef.current.shouldDrop(message)) { @@ -229,6 +226,7 @@ export default function HybridScreen() { sessionId: session?.sessionId, buildId: session?.buildId, viewEpoch, + documentEpoch: pageDocument.epoch, createBroker, onBrokerReady, onBrokerSessionChange: setBrokerSessionId @@ -243,7 +241,7 @@ export default function HybridScreen() { if (!current || activeSessionIdRef.current !== current.sessionId || !brokerRef.current) { return } - initializedSessionRef.current = current.sessionId + pageDocument.initializedSessionRef.current = current.sessionId healthDeadlineRef.current.arm(current.sessionId, (sessionId) => { if (activeSessionIdRef.current === sessionId) { void onHealthTimeout(sessionId) @@ -269,7 +267,7 @@ export default function HybridScreen() { useEffect(() => { brokerRef.current?.updateConnectionState(mobileWebBridgeConnectionState(state)) const current = session - if (!current || initializedSessionRef.current !== current.sessionId) { + if (!current || pageDocument.initializedSessionRef.current !== current.sessionId) { return } void postToWeb({ @@ -301,7 +299,7 @@ export default function HybridScreen() { if (parsed.value.type === 'ready') { // `ready` acknowledges init; echoing init here starves the health frame. if (activeSessionIdRef.current === current.sessionId) { - setPageReadySessionId(current.sessionId) + pageDocument.setReadySessionId(current.sessionId) } } else if (!backMessageHandled) { if (parsed.value.type === 'health') { @@ -356,7 +354,7 @@ export default function HybridScreen() { selectedHostId, connectionState: state, shellContext, - pageReadySessionId, + pageReadySessionId: pageDocument.readySessionId, brokerSessionId, getBroker, selectHost, @@ -394,7 +392,9 @@ export default function HybridScreen() { showWarning('That didn’t work. Try again.', 'recovery_action_failed') } onBridgeMessage={(message) => void handleBridgeMessage(message)} + onDocumentLoadStarted={pageDocument.onLoadStart} onPageLoaded={() => { + pageDocument.onLoaded() hardwareBackHandoff.resetPage() void postInit() }} diff --git a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt index 3a860cab257..9fde3ec34e1 100644 --- a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt +++ b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt @@ -94,6 +94,8 @@ class ExpoMobileWebShellModule : Module() { OnViewDestroys { view -> sessionViews.entries.removeAll { it.value === view } + // Deregister first: nothing may reach a WebView whose renderer process is being torn down. + view.destroy() } AsyncFunction("activateSessionView") { view: MobileWebShellView, sessionId: String -> diff --git a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt index f4e2f6b9162..3e8e4a3adb1 100644 --- a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt +++ b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt @@ -2,6 +2,7 @@ package expo.modules.mobilewebshell import android.annotation.SuppressLint import android.content.Context +import android.graphics.Bitmap import android.graphics.Color import android.net.Uri import android.view.View @@ -76,6 +77,7 @@ internal class MobileWebShellView( private var networkBlockerScriptHandler: ScriptHandler? = null private var debugProbeScriptHandler: ScriptHandler? = null private var bridgeMessageListenerAttached = false + private var destroyed = false private var webView: WebView init { @@ -125,6 +127,7 @@ internal class MobileWebShellView( } fun setSessionId(sessionId: String?) { + if (destroyed) return if (sessionId == null) { deactivateSessionView() return @@ -158,6 +161,7 @@ internal class MobileWebShellView( } fun deactivateSessionView() { + if (destroyed) return removeBridgeMessageListener() networkBlockerScriptHandler?.remove() networkBlockerScriptHandler = null @@ -176,6 +180,7 @@ internal class MobileWebShellView( require(message.toByteArray(Charsets.UTF_8).size <= MOBILE_WEB_MESSAGE_BYTE_LIMIT) { "mobile_web_bridge_message_too_large" } + if (destroyed) return webView.evaluateJavascript( """ (function(value){ @@ -196,14 +201,36 @@ internal class MobileWebShellView( if (activeSessionId == sessionId) postMessage(message) } + /** + * Detaching only parks the WebView — a deactivated view is reactivated in place — so the renderer + * process, the JavaScript context and both script handlers only go away here. Expo calls this once + * the view is unmounted, which is what a package swap, a host switch and a route exit all do. + */ + fun destroy() { + if (destroyed) return + destroyed = true + removeBridgeMessageListener() + networkBlockerScriptHandler?.remove() + networkBlockerScriptHandler = null + debugProbeScriptHandler?.remove() + debugProbeScriptHandler = null + activeSessionId = null + documentLoaded = false + webView.stopLoading() + webView.loadUrl("about:blank") + removeView(webView) + webView.destroy() + } + override fun onDetachedFromWindow() { removeBridgeMessageListener() - webView.stopLoading() + if (!destroyed) webView.stopLoading() super.onDetachedFromWindow() } override fun onAttachedToWindow() { super.onAttachedToWindow() + if (destroyed) return attachWebView() val sessionId = activeSessionId ?: return addBridgeMessageListener(sessionId) @@ -284,6 +311,15 @@ internal class MobileWebShellView( return !allowed } + override fun onPageStarted(view: WebView, url: String, favicon: Bitmap?) { + // The page can replace its own document (the route error boundary reloads on a failed chunk), + // and that is the only signal the shell gets. Every load start is reported so the shell can + // retire the outgoing page's grants before the new document initializes. + if (!isAllowedDocumentRequestUrl(Uri.parse(url))) return + documentLoaded = false + onLoadState(mapOf("state" to "loading")) + } + override fun onPageFinished(view: WebView, url: String) { if (isAllowedDocumentUrl(Uri.parse(url))) { documentLoaded = true diff --git a/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift b/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift index 077dc6721b9..2e0f9516098 100644 --- a/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift +++ b/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift @@ -566,6 +566,14 @@ final class MobileWebShellView: ExpoView, WKNavigationDelegate, WKUIDelegate, decisionHandler(allowed ? .allow : .cancel) } + func webView(_ webView: WKWebView, didStartProvisionalNavigation navigation: WKNavigation!) { + // The page can replace its own document (the route error boundary reloads on a failed chunk), + // and that is the only signal the shell gets. Every load start is reported so the shell can + // retire the outgoing page's grants before the new document initializes. + guard activeSessionId != nil else { return } + onLoadState(["state": "loading"]) + } + func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { guard isAllowedDocumentUrl(webView.url) else { return } onLoadState(["state": "loaded"]) diff --git a/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx b/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx index 97a22e6ca81..e575b189218 100644 --- a/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx +++ b/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx @@ -35,6 +35,7 @@ type MobileWebHybridShellPresentationProps = { onClearCache: () => void | Promise onRecoveryFailure: () => void onBridgeMessage: (message: string) => void + onDocumentLoadStarted: () => void onPageLoaded: () => void onLoadFailed: (reason: string | undefined) => void onNavigationBlocked: () => void @@ -57,6 +58,7 @@ export function MobileWebHybridShellPresentation({ onClearCache, onRecoveryFailure, onBridgeMessage, + onDocumentLoadStarted, onPageLoaded, onLoadFailed, onNavigationBlocked, @@ -141,6 +143,12 @@ export function MobileWebHybridShellPresentation({ sessionId={hostedViewActive ? session.sessionId : null} onBridgeMessage={(event) => onBridgeMessage(event.nativeEvent.data)} onLoadState={(event) => { + // A load only ever starts because the document is being replaced, and the outgoing + // page's grants have to retire before the incoming one initializes. + if (event.nativeEvent.state === 'loading') { + onDocumentLoadStarted() + return + } if (event.nativeEvent.state === 'loaded') { onPageLoaded() return diff --git a/mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts b/mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts new file mode 100644 index 00000000000..b08b8cd9c9a --- /dev/null +++ b/mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts @@ -0,0 +1,47 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const ANDROID_VIEW = source( + '../../packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt' +) +const ANDROID_MODULE = source( + '../../packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt' +) +const IOS_VIEW = source('../../packages/expo-mobile-web-shell/ios/MobileWebShellView.swift') + +describe('hosted WebView document lifecycle', () => { + // The shell owns the document lifecycle, but a page can replace its own document — the route + // error boundary reloads in place — and the load start is the only signal that reaches the shell. + it('reports a document load the page itself starts, on both platforms', () => { + expect(block(ANDROID_VIEW, 'override fun onPageStarted')).toContain( + 'onLoadState(mapOf("state" to "loading"))' + ) + expect( + block(IOS_VIEW, 'func webView(_ webView: WKWebView, didStartProvisionalNavigation') + ).toContain('onLoadState(["state": "loading"])') + }) + + // Deactivation parks the WebView for a reactivation in place, so only an unmount may destroy it. + it('destroys the Android WebView on unmount and never on deactivation', () => { + expect(block(ANDROID_MODULE, 'OnViewDestroys')).toContain('view.destroy()') + expect(block(ANDROID_VIEW, 'fun destroy()')).toContain('webView.destroy()') + expect(block(ANDROID_VIEW, 'fun deactivateSessionView()')).not.toContain('webView.destroy()') + expect(block(ANDROID_VIEW, 'override fun onDetachedFromWindow()')).not.toContain( + 'webView.destroy()' + ) + }) +}) + +function source(relativePath: string): string { + return readFileSync(fileURLToPath(new URL(relativePath, import.meta.url)), 'utf8') +} + +/** The declaration through the brace that closes it, so a match cannot leak into the next member. */ +function block(text: string, header: string): string { + const start = text.indexOf(header) + expect(start, `missing ${header}`).toBeGreaterThanOrEqual(0) + const indent = text.slice(0, start).split('\n').at(-1) ?? '' + const end = text.indexOf(`\n${indent}}`, start) + return text.slice(start, end === -1 ? undefined : end) +} diff --git a/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts b/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts index 89c45ab2690..9f8ea12e163 100644 --- a/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts +++ b/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts @@ -106,7 +106,7 @@ describe('mobile native shell route ownership', () => { const readyBranch = hybridShell.match( /if \(parsed\.value\.type === 'ready'\) \{([\s\S]*?)\} else if/ )?.[1] - expect(readyBranch).toContain('setPageReadySessionId(current.sessionId)') + expect(readyBranch).toContain('setReadySessionId(current.sessionId)') expect(readyBranch).not.toContain('postInit') }) diff --git a/mobile/src/mobile-web/mobile-web-document-replacement.test.tsx b/mobile/src/mobile-web/mobile-web-document-replacement.test.tsx new file mode 100644 index 00000000000..f29f293518f --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-document-replacement.test.tsx @@ -0,0 +1,264 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { createElement, useRef, type FunctionComponent } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, + type MobileWebBridgePageMessage, + type MobileWebBridgeShellMessage +} from '../../../src/shared/mobile-web/bridge-contract' +import type { RpcClient } from '../transport/rpc-client' +import { MobileWebCapabilityBroker } from './mobile-web-capability-broker' +import { MobileWebHealthDeadline } from './mobile-web-health-deadline' +import { MobileWebHybridShellPresentation } from './MobileWebHybridShellPresentation' +import { MobileWebNativeRouteHandoff } from './mobile-web-native-route-handoff' +import { useMobileWebCapabilityBroker } from './use-mobile-web-capability-broker' +import { useMobileWebPageDocument } from './use-mobile-web-page-document' + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + Pressable: 'Pressable', + StyleSheet: { create: (styles: Record) => styles }, + Text: 'Text', + View: 'View' +})) +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0, left: 0, right: 0 }) +})) +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'ChevronLeft', + MonitorSmartphone: 'MonitorSmartphone' +})) +vi.mock('@orca/expo-mobile-web-shell', () => ({ MobileWebShellView: 'MobileWebShellView' })) + +const noop = (): void => {} +const SESSION_ID = 'S'.repeat(43) +const BUILD_ID = 'a'.repeat(64) + +// A native-route excursion, the route error boundary's reload and a re-attach all replace the +// document while the shell session, its build and the view epoch stand still. The previous page's +// broker used to survive that, and its records held every per-operation grant (one for workspace), +// so the new document was refused with rate_limited for the life of the shell session. +describe('hosted document replacement', () => { + let renderer: ReactTestRenderer | null = null + let harness: ReturnType + + beforeEach(() => { + harness = createHarness() + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + it('re-subscribes after a native-route excursion reloads the page', async () => { + await mount() + await loadDocument() + await handle(subscribeRequest('A', 'Z')) + expect(harness.subscribe).toHaveBeenCalledOnce() + + // The excursion deactivates the view (about:blank) and reactivates it on return; the page is + // never unmounted, so the shell only ever sees the reload the reactivation starts. + await setHostedViewActive(false) + await setHostedViewActive(true) + await loadDocument() + await handle(subscribeRequest('B', 'Y')) + + expect(harness.unsubscribe).toHaveBeenCalledOnce() + expect(harness.subscribe).toHaveBeenCalledTimes(2) + expect(errorFor(harness.messages, 'B')).toEqual([]) + }) + + it('re-subscribes after the page reloads itself in place', async () => { + await mount() + await loadDocument() + await handle(subscribeRequest('A', 'Z')) + + // What the route error boundary's recovery button does: same view, same session, new document. + // The duplicate `loading` is what the shell really posts — once for the reload it starts and + // once for the navigation the page started — and the pair is still a single new page. + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loaded' }) + await handle(subscribeRequest('B', 'Y')) + + expect(harness.unsubscribe).toHaveBeenCalledOnce() + expect(harness.created).toBe(2) + expect(errorFor(harness.messages, 'B')).toEqual([]) + }) + + it('never retires the broker that is about to serve the first document', async () => { + await mount() + const live = harness.brokerRef.current + + // The shell posts `loading` for the first load too, and more than once for the same load. A + // page that has not loaded yet has nothing to retire, and retiring here would drop the grants + // out from under the document that is loading. + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loaded' }) + await handle(subscribeRequest('A', 'Z')) + + expect(harness.unsubscribe).not.toHaveBeenCalled() + expect(harness.created).toBe(1) + expect(harness.brokerRef.current).toBe(live) + expect(errorFor(harness.messages, 'A')).toEqual([]) + }) + + it('is the wiring the hosted screen uses', () => { + // The harness above composes the two seams by hand; this pins the screen to the same pair. + const source = readFileSync( + fileURLToPath(new URL('../../app/hybrid.tsx', import.meta.url)), + 'utf8' + ) + + expect(source).toContain('documentEpoch: pageDocument.epoch') + expect(source).toContain('onDocumentLoadStarted={pageDocument.onLoadStart}') + expect(source).toContain('pageDocument.onLoaded()') + }) + + async function mount(): Promise { + await act(async () => { + renderer = create(createElement(harness.Shell, { hostedViewActive: true })) + }) + } + + async function setHostedViewActive(hostedViewActive: boolean): Promise { + await act(async () => { + renderer?.update(createElement(harness.Shell, { hostedViewActive })) + }) + } + + async function loadDocument(): Promise { + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loaded' }) + } + + async function emitLoadState(nativeEvent: { state: string; reason?: string }): Promise { + const shell = renderer!.root.findByType('MobileWebShellView' as never) + await act(async () => { + shell.props.onLoadState({ nativeEvent }) + }) + } + + async function handle(message: MobileWebBridgePageMessage): Promise { + await act(async () => { + await harness.brokerRef.current?.handle(message) + }) + } +}) + +function createHarness() { + const messages: MobileWebBridgeShellMessage[] = [] + const unsubscribe = vi.fn() + const subscribe = vi.fn(() => unsubscribe) + const client = { + sendRequest: vi.fn(), + subscribe, + sendTerminalBinaryFrame: vi.fn(() => true) + } as unknown as RpcClient + const brokerRef: { current: MobileWebCapabilityBroker | null } = { current: null } + // Stable like hybrid.tsx's useCallback: a fresh identity per render would retire the broker on + // every render and hide whether the document epoch is doing the work. + const createBroker = (page: { sessionId: string; buildId: string }) => { + state.created += 1 + return new MobileWebCapabilityBroker({ + context: { shellSessionId: page.sessionId, buildId: page.buildId }, + getClient: () => client, + isConnected: () => true, + isActive: () => true, + postMessage: (message) => void messages.push(message), + nativeAuthority: { + clipboardAvailability: vi.fn(), + hapticFeedback: vi.fn(), + clipboardWrite: vi.fn(), + openExternal: vi.fn(), + terminalPreferences: vi.fn(), + terminalTextScaleUpdate: vi.fn() + }, + terminalClientId: 'device-token', + randomBytes: (length) => new Uint8Array(length).fill(1) + }) + } + const state = { + messages, + subscribe, + unsubscribe, + brokerRef, + created: 0, + Shell: undefined as unknown as FunctionComponent<{ hostedViewActive: boolean }> + } + state.Shell = ({ hostedViewActive }) => { + const healthDeadlineRef = useRef(new MobileWebHealthDeadline(10_000)) + const routeHandoffRef = useRef(new MobileWebNativeRouteHandoff()) + const pageDocument = useMobileWebPageDocument({ + sessionId: SESSION_ID, + viewEpoch: 0, + healthDeadlineRef, + routeHandoffRef + }) + useMobileWebCapabilityBroker({ + brokerRef, + sessionId: SESSION_ID, + buildId: BUILD_ID, + viewEpoch: 0, + documentEpoch: pageDocument.epoch, + createBroker, + onBrokerReady: noop, + onBrokerSessionChange: noop + }) + return createElement(MobileWebHybridShellPresentation, { + viewRef: { current: null }, + selectedHost: { id: 'host-1', name: 'Desk', publicKeyB64: 'k' } as never, + session: { sessionId: SESSION_ID, buildId: BUILD_ID } as never, + viewEpoch: 0, + packageLoading: false, + packageProgress: undefined, + packageWarning: undefined, + hostedViewActive, + onBack: noop, + onShowHosts: noop, + onRetryRecovery: noop, + onUsePrevious: noop, + onClearCache: noop, + onRecoveryFailure: noop, + onBridgeMessage: noop, + onDocumentLoadStarted: pageDocument.onLoadStart, + onPageLoaded: pageDocument.onLoaded, + onLoadFailed: noop, + onNavigationBlocked: noop, + onProcessTerminated: noop + }) + } + return state +} + +function subscribeRequest( + requestId: string, + subscriptionId: string +): Extract { + return { + version: MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, + shellSessionId: SESSION_ID, + buildId: BUILD_ID, + type: 'request', + mode: 'subscription', + requestId: requestId.repeat(22), + subscriptionId: subscriptionId.repeat(22), + capability: 'workspace', + operation: 'subscribe', + payload: {} + } +} + +function errorFor(messages: MobileWebBridgeShellMessage[], requestId: string) { + return messages.flatMap((message) => + message.type === 'response' && + message.requestId === requestId.repeat(22) && + message.status === 'error' + ? [message.error] + : [] + ) +} diff --git a/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx b/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx index ac8e8a93a5e..28d002478db 100644 --- a/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx +++ b/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx @@ -57,6 +57,7 @@ describe('hosted shell document load failures', () => { onClearCache: noop, onRecoveryFailure: noop, onBridgeMessage: noop, + onDocumentLoadStarted: noop, onPageLoaded: loaded, onLoadFailed: (reason) => failures.push(reason), onNavigationBlocked: noop, diff --git a/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts b/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts index 2891ffa5f6e..8e582fdde07 100644 --- a/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts +++ b/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts @@ -57,6 +57,20 @@ describe('useMobileWebCapabilityBroker', () => { expect(errorFor(harness.messages, 'B')).toEqual([]) }) + it('retires the previous page subscriptions when the document is replaced in place', async () => { + await mount(0) + await handle(subscribeRequest('A', 'Z')) + + await act(async () => { + renderer?.update(createElement(harness.Harness, { viewEpoch: 0, documentEpoch: 1 })) + }) + await handle(subscribeRequest('B', 'Y')) + + expect(harness.unsubscribe).toHaveBeenCalledOnce() + expect(harness.subscribe).toHaveBeenCalledTimes(2) + expect(errorFor(harness.messages, 'B')).toEqual([]) + }) + it('retires the broker on demand so an in-place reload cannot inherit it', async () => { await mount(0) await handle(subscribeRequest('A', 'Z')) @@ -99,7 +113,10 @@ function createHarness() { created: 0, brokerSessionId: undefined as string | undefined, retireBroker: undefined as (() => void) | undefined, - Harness: undefined as unknown as FunctionComponent<{ viewEpoch: number }> + Harness: undefined as unknown as FunctionComponent<{ + viewEpoch: number + documentEpoch?: number + }> } const createBroker = (page: MobileWebBrokerPageIdentity): MobileWebCapabilityBroker => { state.created += 1 @@ -124,12 +141,13 @@ function createHarness() { const onBrokerSessionChange = (sessionId: string | undefined): void => { state.brokerSessionId = sessionId } - state.Harness = ({ viewEpoch }) => { + state.Harness = ({ viewEpoch, documentEpoch = 0 }) => { const lifecycle = useMobileWebCapabilityBroker({ brokerRef, sessionId: CONTEXT.shellSessionId, buildId: CONTEXT.buildId, viewEpoch, + documentEpoch, createBroker, onBrokerReady, onBrokerSessionChange diff --git a/mobile/src/mobile-web/use-mobile-web-capability-broker.ts b/mobile/src/mobile-web/use-mobile-web-capability-broker.ts index 9b9b99398fa..248acfee0b4 100644 --- a/mobile/src/mobile-web/use-mobile-web-capability-broker.ts +++ b/mobile/src/mobile-web/use-mobile-web-capability-broker.ts @@ -5,12 +5,15 @@ export type MobileWebBrokerPageIdentity = { sessionId: string; buildId: string } // A view-epoch bump loads a fresh document over the same shell session, so the previous page's // broker (subscriptions, terminal streams, speech authority, replay window, rate limiter) has to -// be retired before the new document can post against it. +// be retired before the new document can post against it. A document epoch is the same boundary +// without a native remount: the shell replaces the document in place on a native-route return, an +// in-page reload and a re-attach, and each of those pages is just as new. export function useMobileWebCapabilityBroker({ brokerRef, sessionId, buildId, viewEpoch, + documentEpoch, createBroker, onBrokerReady, onBrokerSessionChange @@ -19,6 +22,7 @@ export function useMobileWebCapabilityBroker({ sessionId: string | undefined buildId: string | undefined viewEpoch: number + documentEpoch: number createBroker: (page: MobileWebBrokerPageIdentity) => MobileWebCapabilityBroker | null onBrokerReady: () => void onBrokerSessionChange: (sessionId: string | undefined) => void @@ -52,6 +56,7 @@ export function useMobileWebCapabilityBroker({ brokerRef, buildId, createBroker, + documentEpoch, onBrokerReady, onBrokerSessionChange, retireBroker, diff --git a/mobile/src/mobile-web/use-mobile-web-page-document.ts b/mobile/src/mobile-web/use-mobile-web-page-document.ts new file mode 100644 index 00000000000..a6ce4b548ef --- /dev/null +++ b/mobile/src/mobile-web/use-mobile-web-page-document.ts @@ -0,0 +1,71 @@ +import { useCallback, useEffect, useRef, useState, type MutableRefObject } from 'react' +import type { MobileWebHealthDeadline } from './mobile-web-health-deadline' +import type { MobileWebNativeRouteHandoff } from './mobile-web-native-route-handoff' + +/** + * The shell owns the document lifecycle, so it also owns the page-scoped state that dies with a + * document: what was initialized, what reported ready, and the health deadline armed for it. + * + * A document is replaced without the shell session, its build or the view epoch moving at all — a + * native-route excursion deactivates the view to about:blank and reloads on return, the route error + * boundary reloads in place, and a re-attached view reloads its URL. Each of those mints a new page + * with new subscription ids while the previous page's broker survives, and its records keep holding + * every per-operation grant (one for workspace, account and source control), so the new document's + * subscribes are refused with `rate_limited` for the life of the shell session. The document epoch + * is that boundary: it retires the outgoing page's broker before the incoming one initializes. + */ +export function useMobileWebPageDocument({ + sessionId, + viewEpoch, + healthDeadlineRef, + routeHandoffRef +}: { + sessionId: string | undefined + viewEpoch: number + healthDeadlineRef: MutableRefObject + routeHandoffRef: MutableRefObject +}): { + epoch: number + initializedSessionRef: MutableRefObject + readySessionId: string | undefined + setReadySessionId: (sessionId: string | undefined) => void + onLoadStart: () => void + onLoaded: () => void +} { + const initializedSessionRef = useRef(undefined) + const loadedRef = useRef(false) + const [epoch, setEpoch] = useState(0) + const [readySessionId, setReadySessionId] = useState() + + useEffect(() => { + initializedSessionRef.current = undefined + loadedRef.current = false + routeHandoffRef.current.clear() + setReadySessionId(undefined) + healthDeadlineRef.current.clear() + return () => healthDeadlineRef.current.clear() + }, [epoch, healthDeadlineRef, routeHandoffRef, sessionId, viewEpoch]) + + const onLoadStart = useCallback(() => { + // Only a load that displaces a document that finished loading is a replacement. The shell posts + // `loading` for the first load too, and more than once per load, and neither is a new page. + if (!loadedRef.current) { + return + } + loadedRef.current = false + setEpoch((current) => current + 1) + }, []) + + const onLoaded = useCallback(() => { + loadedRef.current = true + }, []) + + return { + epoch, + initializedSessionRef, + readySessionId, + setReadySessionId, + onLoadStart, + onLoaded + } +} From 0722a2b7d2e75509899f51847ad01d6377d87ff0 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 16:48:03 -0400 Subject: [PATCH 2/3] fix(mobile): close the terminal streams the shell retires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MobileWebTerminalStreams was the one subscription owner built without a postClosed, so five shell-side retirements were silent: the page kept a live terminal entry, its screen froze on the last frame it got, onError never fired and it never re-subscribed. The inactive-page branch of post() also retired without unsubscribing, and a retired record is unreachable from the registry, so nothing ever released the host PTY stream — a host switch leaves activeSessionId undefined for a window in which any frame from a busy terminal lands there. Retirement now goes through one path that drops the host stream first and posts the closure the page needs: not_found when the workspace binding is revoked, unavailable when bridge delivery fails, and cancelled/retryable from replaceClient, where the page and its grants survive but its terminals cannot. The lease streams share it. The inactive-page branch stays closure-free on purpose: the shell drops every frame for an inactive page. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-web-capability-broker.ts | 11 +- .../mobile-web-terminal-lease-streams.ts | 30 +++- .../mobile-web-terminal-stream-retirement.ts | 49 ++++++ .../mobile-web-terminal-streams.test.ts | 166 +++++++++++++++++- .../mobile-web/mobile-web-terminal-streams.ts | 47 ++--- 5 files changed, 266 insertions(+), 37 deletions(-) create mode 100644 mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts diff --git a/mobile/src/mobile-web/mobile-web-capability-broker.ts b/mobile/src/mobile-web/mobile-web-capability-broker.ts index e46d2f52563..30e225d7c17 100644 --- a/mobile/src/mobile-web/mobile-web-capability-broker.ts +++ b/mobile/src/mobile-web/mobile-web-capability-broker.ts @@ -14,6 +14,7 @@ import { MobileWebCommitMessageGeneration } from './mobile-web-commit-message-ge import { MobileWebCapabilitySubscriptions } from './mobile-web-capability-subscriptions' import { MOBILE_WEB_PRODUCTION_GRANT_INDEX } from './mobile-web-production-grants' import { MobileWebTerminalStreams } from './mobile-web-terminal-streams' +import { MOBILE_WEB_TERMINAL_CLIENT_CLOSURE } from './mobile-web-terminal-stream-retirement' import { MobileWebSpeechAuthority } from './mobile-web-speech-authority' import { executeMobileWebCapabilityRequest } from './mobile-web-capability-execution' import { MobileWebCapabilityAuthorities } from './mobile-web-capability-authorities' @@ -74,8 +75,8 @@ export class MobileWebCapabilityBroker { onFlowMetrics: options.onTerminalFlowMetrics, onResync: options.onTerminalResync, workspaceAuthority: this.authorities.workspace, - postEvent: (subscriptionId, sequence, event) => - this.messages.event(subscriptionId, sequence, event) + postEvent: this.messages.event.bind(this.messages), + postClosed: mobileWebSubscriptionClosedPoster(this.messages) }) } @@ -106,7 +107,8 @@ export class MobileWebCapabilityBroker { this.authorities.clear() this.commitMessageGeneration.replaceClient(client) this.subscriptions.dispose() - this.terminalStreams.dispose(null) + // The page and its grants survive a client swap, so its terminals can re-subscribe once told. + this.terminalStreams.dispose(null, MOBILE_WEB_TERMINAL_CLIENT_CLOSURE) this.speechAuthority.replaceClient() for (const [requestId, pending] of this.pending) { if (survivesCancellation(pending)) { @@ -242,8 +244,7 @@ export class MobileWebCapabilityBroker { sourceControlSubscriptions: this.subscriptions.sourceControl, sourceControlBranchCompare: this.authorities.sourceControlBranchCompare, speechAuthority: this.speechAuthority, - postSpeechEvent: (subscriptionId, sequence, event) => - this.messages.event(subscriptionId, sequence, event), + postSpeechEvent: this.messages.event.bind(this.messages), postSpeechClosed: mobileWebSubscriptionClosedPoster(this.messages), workspaceSubscriptions: this.subscriptions.workspace, terminalStreams: this.terminalStreams, diff --git a/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts b/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts index 4592b19b05f..723aa14e196 100644 --- a/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts +++ b/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts @@ -5,6 +5,14 @@ import { } from '../../../src/shared/mobile-web/terminal-stream-contract' import type { RpcClient } from '../transport/rpc-client' import { MobileWebBrokerError } from './mobile-web-broker-error' +import type { + MobileWebPostSubscriptionClosed, + MobileWebSubscriptionClosure +} from './mobile-web-subscription-closure' +import { + MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE, + MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE +} from './mobile-web-terminal-stream-retirement' import type { MobileWebWorkspaceAuthority } from './mobile-web-workspace-authority' type LeaseSubscribeRequest = Extract @@ -34,6 +42,7 @@ export class MobileWebTerminalLeaseStreams { sequence: number, event: MobileWebTerminalEvent ) => Promise + postClosed: MobileWebPostSubscriptionClosed } ) {} @@ -85,12 +94,12 @@ export class MobileWebTerminalLeaseStreams { return this.records.has(streamId) } - cancel(subscriptionId: string): string | null { + cancel(subscriptionId: string, closure?: MobileWebSubscriptionClosure): string | null { const record = this.records.get(subscriptionId) if (!record) { return null } - this.retire(record) + this.retire(record, closure) return record.requestId } @@ -102,9 +111,9 @@ export class MobileWebTerminalLeaseStreams { } } - dispose(): void { + dispose(closure?: MobileWebSubscriptionClosure): void { for (const subscriptionId of this.records.keys()) { - this.cancel(subscriptionId) + this.cancel(subscriptionId, closure) } } @@ -113,10 +122,14 @@ export class MobileWebTerminalLeaseStreams { } private receive(subscriptionId: string, record: LeaseRecord, value: unknown): void { - if (!this.isLive(subscriptionId, record) || !this.isAuthorized(record)) { + if (!this.isLive(subscriptionId, record)) { this.retire(record) return } + if (!this.isAuthorized(record)) { + this.retire(record, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) + return + } if (!isRecord(value) || typeof value.type !== 'string') { this.post( record, @@ -181,7 +194,7 @@ export class MobileWebTerminalLeaseStreams { this.retire(record) } }) - .catch(() => this.retire(record)) + .catch(() => this.retire(record, MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE)) } private isLive(subscriptionId: string, record: LeaseRecord): boolean { @@ -199,7 +212,7 @@ export class MobileWebTerminalLeaseStreams { } } - private retire(record: LeaseRecord): void { + private retire(record: LeaseRecord, closure?: MobileWebSubscriptionClosure): void { if (!record.active) { return } @@ -210,6 +223,9 @@ export class MobileWebTerminalLeaseStreams { } catch { // The authenticated host subscription is already gone. } + if (closure) { + this.options.postClosed(record.pageStreamId, closure) + } } } diff --git a/mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts b/mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts new file mode 100644 index 00000000000..39c54717dff --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts @@ -0,0 +1,49 @@ +import type { RpcClient } from '../transport/rpc-client' +import type { MobileWebTerminalStreamRecord } from './mobile-web-terminal-flow-control' +import type { + MobileWebPostSubscriptionClosed, + MobileWebSubscriptionClosure +} from './mobile-web-subscription-closure' +import { safeUnsubscribeMobileWebTerminal } from './mobile-web-terminal-host-transport' +import type { MobileWebTerminalStreamRegistry } from './mobile-web-terminal-stream-registry' + +/** The page cannot observe a shell-side retirement: its terminal keeps a live entry, freezes on the + * last frame it got and never re-subscribes. So every retirement the page did not ask for carries a + * closure, and every retirement drops the host stream first — a retired record is unreachable from + * the registry, so no later sweep can unsubscribe it. */ +export const MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE: MobileWebSubscriptionClosure = { + code: 'not_found', + retryable: false +} +export const MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE: MobileWebSubscriptionClosure = { + code: 'unavailable', + retryable: true +} +/** A client swap keeps the page and its grants, so re-subscribing on the new transport works. */ +export const MOBILE_WEB_TERMINAL_CLIENT_CLOSURE: MobileWebSubscriptionClosure = { + code: 'cancelled', + retryable: true +} + +export class MobileWebTerminalStreamRetirement { + constructor( + private readonly options: { + registry: MobileWebTerminalStreamRegistry + postClosed: MobileWebPostSubscriptionClosed + } + ) {} + + retire( + record: MobileWebTerminalStreamRecord, + client: RpcClient | null, + closure?: MobileWebSubscriptionClosure + ): void { + if (client) { + safeUnsubscribeMobileWebTerminal(client, record) + } + this.options.registry.retire(record) + if (closure) { + this.options.postClosed(record.subscriptionId, closure) + } + } +} diff --git a/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts b/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts index dbc13cd4232..43ede100bc7 100644 --- a/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts +++ b/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts @@ -10,7 +10,9 @@ import { TerminalStreamOpcode, type TerminalStreamFrame } from '../transport/terminal-stream-protocol' +import type { MobileWebSubscriptionClosure } from './mobile-web-subscription-closure' import { MobileWebTerminalStreams } from './mobile-web-terminal-streams' +import { MOBILE_WEB_TERMINAL_CLIENT_CLOSURE } from './mobile-web-terminal-stream-retirement' import { prepareMobileWebClipboardPaste, prepareMobileWebImageAttachment @@ -536,10 +538,159 @@ describe('MobileWebTerminalStreams', () => { expect(harness.streams.cancel(SUBSCRIPTION_ID, harness.client)).toBeNull() expect(harness.leaseUnsubscribe).toHaveBeenCalledOnce() }) + + // Every shell-side retirement below used to be silent: the page kept a live terminal entry with + // no error and no resubscribe, and some of them left the host PTY publishing into nothing. + it('releases the host stream when the page session stops being active', async () => { + let active = true + const harness = createHarness({ isActive: () => active }) + const hostStreamId = await subscribeHostStream(harness, 'request-inactive') + + active = false + harness.emitMultiplex({ type: 'subscribed', streamId: hostStreamId }) + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + // The shell drops every frame for an inactive page, so a closure would go nowhere. + expect(harness.closures).toEqual([]) + expect(harness.streams.countForOperation('terminal.subscribe')).toBe(0) + }) + + it('closes the page terminal when the workspace binding disappears under a host frame', async () => { + const harness = createHarness() + const hostStreamId = await subscribeHostStream(harness, 'request-frame-revoked') + harness.emitMultiplex({ type: 'subscribed', streamId: hostStreamId }) + harness.workspaceAuthority.synchronize([]) + + expect( + harness.emitFrame({ + opcode: TerminalStreamOpcode.Output, + streamId: hostStreamId, + seq: 0, + payload: encodeTerminalStreamText('hi') + }) + ).toBe(true) + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) + + it('closes the page terminal when a multiplex event sweeps an unauthorized record', async () => { + const harness = createHarness() + const hostStreamId = await subscribeHostStream(harness, 'request-sweep-revoked') + harness.workspaceAuthority.synchronize([]) + + harness.emitMultiplex({ type: 'ready' }) + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) + + it('closes the page terminal when its own request finds the binding revoked', async () => { + const harness = createHarness() + const hostStreamId = await subscribeHostStream(harness, 'request-revoked-closure') + harness.workspaceAuthority.synchronize([]) + + expect(() => + harness.streams.handle( + { operation: 'resize', streamId: SUBSCRIPTION_ID, viewport: { cols: 100, rows: 30 } }, + harness.client + ) + ).toThrow('not_found') + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) + + it('closes the page terminal when bridge delivery fails', async () => { + const harness = createHarness({ postEvent: () => Promise.reject(new Error('bridge gone')) }) + const hostStreamId = await subscribeHostStream(harness, 'request-delivery-failure') + + harness.emitMultiplex({ type: 'subscribed', streamId: hostStreamId }) + await settle() + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'unavailable', retryable: true } } + ]) + }) + + it('tells the page to re-subscribe when the transport is swapped underneath it', async () => { + const harness = createHarness() + await subscribeHostStream(harness, 'request-client-swap') + + // What MobileWebCapabilityBroker.replaceClient passes: the old transport can no longer carry an + // unsubscribe, so the page has to hear that its terminal is gone and ask again. + harness.streams.dispose(null, MOBILE_WEB_TERMINAL_CLIENT_CLOSURE) + + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: MOBILE_WEB_TERMINAL_CLIENT_CLOSURE } + ]) + expect(harness.multiplexUnsubscribe).toHaveBeenCalledOnce() + expect(harness.streams.countForOperation('terminal.subscribe')).toBe(0) + }) + + it('closes a lease stream whose workspace binding disappears', async () => { + const harness = createHarness() + await harness.streams.start({ + requestId: 'request-lease-revoked', + subscriptionId: SUBSCRIPTION_ID, + payload: { ...subscribePayload(), visible: false, leaseOnly: true }, + client: harness.client, + isRequestActive: () => true + }) + harness.workspaceAuthority.synchronize([]) + + harness.emitLease({ type: 'subscribed' }) + + expect(harness.leaseUnsubscribe).toHaveBeenCalledOnce() + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) }) -function createHarness() { +async function subscribeHostStream( + harness: ReturnType, + requestId: string +): Promise { + await harness.streams.start({ + requestId, + subscriptionId: SUBSCRIPTION_ID, + payload: subscribePayload(), + client: harness.client, + isRequestActive: () => true + }) + harness.emitMultiplex({ type: 'ready' }) + return decodeTerminalStreamJson>(harness.sentFrames.at(-1)!.payload)! + .streamId as number +} + +function createHarness( + overrides: { isActive?: () => boolean; postEvent?: () => Promise } = {} +) { const sentFrames: TerminalStreamFrame[] = [] + const closures: { subscriptionId: string; closure: MobileWebSubscriptionClosure }[] = [] const events: { sequence: number; event: MobileWebTerminalEvent }[] = [] const resyncReasons: string[] = [] const flowMetrics: { ackLagMs: number | undefined; outstandingBytes: number }[] = [] @@ -548,6 +699,7 @@ function createHarness() { let emitFrame = (_frame: TerminalStreamFrame): boolean => false let emitLease = (_result: unknown): void => {} const leaseUnsubscribe = vi.fn() + const multiplexUnsubscribe = vi.fn() const workspaceAuthority = new MobileWebWorkspaceAuthority(() => new Uint8Array(16).fill(9)) workspaceAuthority.synchronize([{ workspaceId: HOST_WORKSPACE_ID, repoId: '/secret/repo' }]) const client = { @@ -572,7 +724,7 @@ function createHarness() { } emitMultiplex = listener emitFrame = options?.onTerminalBinaryFrame ?? emitFrame - return vi.fn() + return multiplexUnsubscribe }), sendTerminalBinaryFrame: vi.fn((frame) => { sentFrames.push(frame) @@ -580,7 +732,7 @@ function createHarness() { }) } as unknown as RpcClient const streams = new MobileWebTerminalStreams({ - isActive: () => true, + isActive: overrides.isActive ?? (() => true), clientId: 'device-secret', now: () => nowMs, onFlowMetrics: (metrics) => flowMetrics.push(metrics), @@ -588,7 +740,9 @@ function createHarness() { workspaceAuthority, postEvent: async (_subscriptionId, sequence, event) => { events.push({ sequence, event }) - } + await overrides.postEvent?.() + }, + postClosed: (subscriptionId, closure) => closures.push({ subscriptionId, closure }) }) return { streams, @@ -605,7 +759,9 @@ function createHarness() { emitMultiplex: (result: unknown) => emitMultiplex(result), emitFrame: (frame: TerminalStreamFrame) => emitFrame(frame), emitLease: (result: unknown) => emitLease(result), - leaseUnsubscribe + leaseUnsubscribe, + multiplexUnsubscribe, + closures } } diff --git a/mobile/src/mobile-web/mobile-web-terminal-streams.ts b/mobile/src/mobile-web/mobile-web-terminal-streams.ts index 589b54388ec..1ad7f2027b6 100644 --- a/mobile/src/mobile-web/mobile-web-terminal-streams.ts +++ b/mobile/src/mobile-web/mobile-web-terminal-streams.ts @@ -9,6 +9,10 @@ import { type TerminalStreamFrame } from '../transport/terminal-stream-protocol' import { MobileWebBrokerError } from './mobile-web-broker-error' +import type { + MobileWebPostSubscriptionClosed, + MobileWebSubscriptionClosure +} from './mobile-web-subscription-closure' import { runMobileWebTerminalAction } from './mobile-web-terminal-actions' import { acknowledgeMobileWebTerminalOutput, @@ -16,13 +20,17 @@ import { type MobileWebTerminalStreamRecord } from './mobile-web-terminal-flow-control' import { - safeUnsubscribeMobileWebTerminal, sendMobileWebTerminalFrame, sendMobileWebTerminalSubscribe } from './mobile-web-terminal-host-transport' import { handleMobileWebTerminalMultiplexEvent } from './mobile-web-terminal-multiplex-events' import { resolveMobileWebTerminal } from './mobile-web-terminal-resolution' import { MobileWebTerminalStreamRegistry } from './mobile-web-terminal-stream-registry' +import { + MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE, + MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE, + MobileWebTerminalStreamRetirement +} from './mobile-web-terminal-stream-retirement' import type { MobileWebWorkspaceAuthority } from './mobile-web-workspace-authority' import { MobileWebTerminalLeaseStreams } from './mobile-web-terminal-lease-streams' import { sendMobileWebTerminalAckBytes } from './mobile-web-terminal-stream-control' @@ -35,6 +43,7 @@ import type { export class MobileWebTerminalStreams { private readonly registry: MobileWebTerminalStreamRegistry + private readonly retirement: MobileWebTerminalStreamRetirement private readonly leaseStreams: MobileWebTerminalLeaseStreams private disposeMultiplex: (() => void) | null = null private multiplexReady = false @@ -53,9 +62,14 @@ export class MobileWebTerminalStreams { sequence: number, event: MobileWebTerminalEvent ) => Promise + postClosed: MobileWebPostSubscriptionClosed } ) { this.registry = new MobileWebTerminalStreamRegistry(options.workspaceAuthority) + this.retirement = new MobileWebTerminalStreamRetirement({ + registry: this.registry, + postClosed: options.postClosed + }) this.leaseStreams = new MobileWebTerminalLeaseStreams(options) } @@ -124,8 +138,7 @@ export class MobileWebTerminalStreams { throw new MobileWebBrokerError('not_found') } if (!this.isAuthorized(record)) { - safeUnsubscribeMobileWebTerminal(client, record) - this.registry.retire(record) + this.retirement.retire(record, client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) throw new MobileWebBrokerError('not_found') } if ( @@ -165,10 +178,7 @@ export class MobileWebTerminalStreams { if (!record) { return null } - if (client) { - safeUnsubscribeMobileWebTerminal(client, record) - } - this.registry.retire(record) + this.retirement.retire(record, client) return record.requestId } @@ -185,12 +195,10 @@ export class MobileWebTerminalStreams { return operationKey === 'terminal.subscribe' ? this.registry.size + this.leaseStreams.size : 0 } - dispose(client: RpcClient | null): void { - this.leaseStreams.dispose() + dispose(client: RpcClient | null, closure?: MobileWebSubscriptionClosure): void { + this.leaseStreams.dispose(closure) for (const record of this.registry.records()) { - if (client) { - safeUnsubscribeMobileWebTerminal(client, record) - } + this.retirement.retire(record, client, closure) } this.registry.clear() this.disposeMultiplex?.() @@ -229,8 +237,7 @@ export class MobileWebTerminalStreams { return false } if (!this.isAuthorized(record)) { - safeUnsubscribeMobileWebTerminal(client, record) - this.registry.retire(record) + this.retirement.retire(record, client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) return true } handleMobileWebHostTerminalFrame(record, frame, this.flowContext(client)) @@ -266,26 +273,26 @@ export class MobileWebTerminalStreams { private post(record: MobileWebTerminalStreamRecord, event: MobileWebTerminalEvent): void { if (!this.options.isActive()) { - this.registry.retire(record) + // No closure: the shell drops every frame for an inactive page, so the host stream is all + // there is left to release. + this.retirement.retire(record, record.client) return } if (!this.isAuthorized(record)) { - safeUnsubscribeMobileWebTerminal(record.client, record) - this.registry.retire(record) + this.retirement.retire(record, record.client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) return } const sequence = record.bridgeSequence++ record.delivery = record.delivery .then(() => this.options.postEvent(record.subscriptionId, sequence, event)) .catch(() => { - safeUnsubscribeMobileWebTerminal(record.client, record) - this.registry.retire(record) + this.retirement.retire(record, record.client, MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE) }) } private retireUnauthorizedRecords(client: RpcClient): void { for (const record of this.registry.retireUnauthorized()) { - safeUnsubscribeMobileWebTerminal(client, record) + this.retirement.retire(record, client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) } } From 81f50aa8bf741d5bbc042a2f97c1f5a2214ad3c3 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 16:48:14 -0400 Subject: [PATCH 3/3] fix(mobile): order unpair cleanup and cap the retry loops it races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unpair deleted the host's package cache first, while the hybrid WebView was still mounted and still serving its document out of it, so every asset the page asked for next came back 403 — a failed dynamic import lands in the route error boundary. The metadata commit still goes first (a failure there must not strand a paired host), the client closes right after it, and the recoverable caches go last; the page's own removeHost leaves the route before any of it. A failed unpair now keeps the cache instead of having already deleted it. The capability probe retried a cutover failure at a fixed 250 ms with no cap, so a link that keeps cutting over polled status.get four times a second for as long as the screen was mounted; cutovers now get three prompt retries and then join the existing backoff ladder. The package refresh effect restarts from zero on every client swap and drops the staged bytes with it, so a flapping link re-downloaded the whole bundle back to back; a restart that follows an attempt which never reached a completed download now waits out a jittered ladder capped at 30 s, and a host change or a user-driven retry starts it over. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ...sted-connection-diagnostics-route.test.tsx | 17 ++++++ .../mobile-web-package-refresh-backoff.ts | 48 +++++++++++++++++ .../use-mobile-web-navigation-authority.ts | 4 ++ .../use-mobile-web-package-refresh.test.ts | 53 +++++++++++++++++++ .../use-mobile-web-package-refresh.ts | 18 ++++++- .../transport/host-removal-lifecycle.test.ts | 29 ++++++++++ .../src/transport/host-removal-lifecycle.ts | 10 ++-- .../runtime-capability-probe.test.ts | 20 +++++++ .../src/transport/runtime-capability-probe.ts | 11 ++-- 9 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 mobile/src/mobile-web/mobile-web-package-refresh-backoff.ts diff --git a/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx b/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx index 9c485fac89f..ac5bb7134df 100644 --- a/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx +++ b/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx @@ -7,6 +7,8 @@ import { handleMobileWebBrokerMessage } from './mobile-web-broker-message-handof import { MobileWebNativeRouteHandoff } from './mobile-web-native-route-handoff' import { MOBILE_WEB_PRODUCTION_NAVIGATION_GRANTS } from './mobile-web-production-navigation-grants' import { useMobileWebNavigationAuthority } from './use-mobile-web-navigation-authority' +import { leaveHostRoute } from '../host-route-exit' +import { removeHostAndCloseClient } from '../transport/host-removal-lifecycle' import type { MobileWebShellViewRef } from '@orca/expo-mobile-web-shell' import type { MobileWebNavigationAuthority } from './mobile-web-navigation-operations' @@ -73,6 +75,21 @@ describe('hosted network diagnostics route', () => { expect(deactivateSessionView).toHaveBeenCalledWith() }) + it('leaves the hosted route before the unpair deletes the package cache it serves', async () => { + const order: string[] = [] + vi.mocked(leaveHostRoute).mockImplementation(() => { + order.push('leave-route') + }) + vi.mocked(removeHostAndCloseClient).mockImplementation(async () => { + order.push('remove-host') + }) + const authority = renderNavigationAuthority(new MobileWebNativeRouteHandoff()) + + await authority.current?.removeHost() + + expect(order).toEqual(['leave-route', 'remove-host']) + }) + it('leaves the host picker exit on the page-local route path', () => { const routeHandoff = new MobileWebNativeRouteHandoff() const authority = renderNavigationAuthority(routeHandoff) diff --git a/mobile/src/mobile-web/mobile-web-package-refresh-backoff.ts b/mobile/src/mobile-web/mobile-web-package-refresh-backoff.ts new file mode 100644 index 00000000000..c14ffe44979 --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-package-refresh-backoff.ts @@ -0,0 +1,48 @@ +import { withReconnectJitter } from '../../../src/shared/reconnect-jitter' + +// Why: the refresh effect restarts from zero on every client swap and the staged bytes are dropped +// with it, so a flapping link re-downloads the whole bundle back to back on cellular data. The delay +// escalates per attempt that never reached a completed download and stops at a ceiling; a user-driven +// retry starts the ladder over. +const REFRESH_RETRY_BASE_DELAY_MS = 1_000 +const REFRESH_RETRY_MAX_DELAY_MS = 30_000 + +export function mobileWebPackageRefreshDelayMs( + unfinishedAttempts: number, + random: () => number = Math.random +): number { + if (unfinishedAttempts <= 0) { + return 0 + } + return withReconnectJitter( + Math.min( + REFRESH_RETRY_BASE_DELAY_MS * 2 ** (unfinishedAttempts - 1), + REFRESH_RETRY_MAX_DELAY_MS + ), + random + ) +} + +/** Resolves false when the wait was abandoned, so the caller drops the attempt instead of starting. */ +export function waitBeforeMobileWebPackageRefresh( + delayMs: number, + signal: AbortSignal +): Promise { + if (signal.aborted) { + return Promise.resolve(false) + } + if (delayMs <= 0) { + return Promise.resolve(true) + } + return new Promise((resolve) => { + const onAbort = (): void => { + clearTimeout(timer) + resolve(false) + } + const timer = setTimeout(() => { + signal.removeEventListener('abort', onAbort) + resolve(true) + }, delayMs) + signal.addEventListener('abort', onAbort, { once: true }) + }) +} diff --git a/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts b/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts index 4489ed95160..ff0e72af275 100644 --- a/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts +++ b/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts @@ -51,6 +51,10 @@ export function useMobileWebNavigationAuthority({ return forceReconnectHost(hostId) }, removeHost() { + // The hosted document is served out of the host's package cache that the unpair deletes, + // so leave the route first instead of letting the hosts-list change unmount the view later. + clearColdResumeRoute() + leaveHostRoute(router) return removeHostAndCloseClient(hostId, hostPublicKeyB64, closeHostClient) } } diff --git a/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts b/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts index cacb777ddee..61cd55480f5 100644 --- a/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts +++ b/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts @@ -99,4 +99,57 @@ describe('useMobileWebPackageRefresh', () => { expect(await reuse?.(BUILD_ID)).toBe(true) expect(native.openSession).not.toHaveBeenCalled() }) + + // A flapping link aborts the download and drops every staged byte, so an unpaced restart + // re-downloads the whole bundle over cellular data as fast as the link comes back. + it('paces a restart that follows an attempt which never finished downloading', async () => { + vi.useFakeTimers() + try { + downloadPackage.mockImplementation(() => new Promise(() => {})) + const props = { + client: { sendRequest: vi.fn() } as unknown as RpcClient, + host: HOST, + state: 'connected', + packageCapability: { status: 'supported', gzip: false }, + cachedBuildProbeRef: ref({ + hostEpoch: 1, + promise: Promise.resolve(null), + resolve: vi.fn() + }), + hostEpochRef: ref(1), + ownedSessionRef: ref(null), + rejectedBuildIdsRef: ref(new Set()), + refreshingHostEpochRef: ref(null), + publishSession: vi.fn(async () => true), + refreshEpoch: 0, + setPackageLoading: vi.fn(), + setPackageWarning: vi.fn(), + setPackageProgress: vi.fn() + } as const + function Harness({ swap }: { swap: number }): null { + useMobileWebPackageRefresh({ + ...props, + client: { sendRequest: vi.fn(), swap } as unknown as RpcClient + }) + return null + } + + await act(async () => { + renderer = create(createElement(Harness, { swap: 0 })) + }) + expect(downloadPackage).toHaveBeenCalledTimes(1) + + await act(async () => { + renderer?.update(createElement(Harness, { swap: 1 })) + }) + expect(downloadPackage).toHaveBeenCalledTimes(1) + + await act(async () => { + await vi.advanceTimersByTimeAsync(1_200) + }) + expect(downloadPackage).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) }) diff --git a/mobile/src/mobile-web/use-mobile-web-package-refresh.ts b/mobile/src/mobile-web/use-mobile-web-package-refresh.ts index 8c91dbe19e9..cc4097ab299 100644 --- a/mobile/src/mobile-web/use-mobile-web-package-refresh.ts +++ b/mobile/src/mobile-web/use-mobile-web-package-refresh.ts @@ -1,4 +1,4 @@ -import { useEffect, type MutableRefObject } from 'react' +import { useEffect, useRef, type MutableRefObject } from 'react' import ExpoMobileWebShell, { type MobileWebShellSession } from '@orca/expo-mobile-web-shell' import { MOBILE_WEB_BRIDGE_PROTOCOL_VERSION } from '../../../src/shared/mobile-web/bridge-contract' import { MOBILE_WEB_PACKAGE_MAX_RANGE_BYTES } from '../../../src/shared/mobile-web/package-rpc-contract' @@ -11,6 +11,10 @@ import { type MobileWebPackageDownloadProgress } from './mobile-web-package-downloader' import { mobileWebDiagnosticsStore } from './mobile-web-diagnostics-store' +import { + mobileWebPackageRefreshDelayMs, + waitBeforeMobileWebPackageRefresh +} from './mobile-web-package-refresh-backoff' import type { MobileWebCachedBuildProbe } from './mobile-web-cached-build-probe' import { mobileWebPackageRefreshWarning } from './mobile-web-package-refresh-warning' import type { MobileWebShellNotice } from './mobile-web-shell-notice' @@ -56,6 +60,7 @@ export function useMobileWebPackageRefresh(args: { setPackageWarning, setPackageProgress } = args + const retryRef = useRef({ hostId: '', refreshEpoch, attempts: 0 }) useEffect(() => { if (!host || !client || state !== 'connected' || packageCapability.status !== 'supported') { @@ -64,11 +69,20 @@ export function useMobileWebPackageRefresh(args: { const hostEpoch = hostEpochRef.current const cachedBuildProbe = cachedBuildProbeRef.current const controller = new AbortController() + // A user-driven retry and a host change both start the backoff ladder over. + if (retryRef.current.hostId !== host.id || retryRef.current.refreshEpoch !== refreshEpoch) { + retryRef.current = { hostId: host.id, refreshEpoch, attempts: 0 } + } + const retryDelayMs = mobileWebPackageRefreshDelayMs(retryRef.current.attempts) + retryRef.current.attempts += 1 refreshingHostEpochRef.current = hostEpoch setPackageLoading(true) setPackageWarning(undefined) setPackageProgress(undefined) void (async () => { + if (!(await waitBeforeMobileWebPackageRefresh(retryDelayMs, controller.signal))) { + return + } const refreshStartedAt = Date.now() try { const downloaded = await downloadMobileWebPackage( @@ -98,6 +112,8 @@ export function useMobileWebPackageRefresh(args: { } } ) + // The bundle is staged: whatever happens next costs no more download, so the ladder resets. + retryRef.current.attempts = 0 if (controller.signal.aborted || hostEpochRef.current !== hostEpoch) { return } diff --git a/mobile/src/transport/host-removal-lifecycle.test.ts b/mobile/src/transport/host-removal-lifecycle.test.ts index 984b044ac03..45f923af076 100644 --- a/mobile/src/transport/host-removal-lifecycle.test.ts +++ b/mobile/src/transport/host-removal-lifecycle.test.ts @@ -146,6 +146,35 @@ describe('host removal lifecycle', () => { expect(closeHostClient).toHaveBeenCalledWith('host-1') }) + it('deletes the package cache only after the client that served it is closed', async () => { + // The hosted WebView reads its document out of that cache: deleting it first turned every + // later asset request into a 403 under a still-mounted document. + const order: string[] = [] + removeHostMock.mockImplementation(async () => { + order.push('remove-metadata') + }) + removeMobileWebHostCacheMock.mockImplementation(async () => { + order.push('delete-cache') + }) + clearMobileWebColdResumeRouteForHostMock.mockImplementation(async () => { + order.push('clear-cold-route') + }) + + await removeHostAndCloseClient('host-1', 'public-key-1', () => order.push('close-client')) + + expect(order).toEqual(['remove-metadata', 'close-client', 'delete-cache', 'clear-cold-route']) + }) + + it('keeps the package cache when the unpair itself never commits', async () => { + removeHostMock.mockRejectedValue(new Error('storage unavailable')) + + await expect(removeHostAndCloseClient('host-1', 'public-key-1', vi.fn())).rejects.toThrow( + 'storage unavailable' + ) + + expect(removeMobileWebHostCacheMock).not.toHaveBeenCalled() + }) + it('forgets removed-host logs even when client teardown throws', async () => { removeHostMock.mockResolvedValue(undefined) const closeHostClient = vi.fn(() => { diff --git a/mobile/src/transport/host-removal-lifecycle.ts b/mobile/src/transport/host-removal-lifecycle.ts index 8eb91e5b1a8..50778b50908 100644 --- a/mobile/src/transport/host-removal-lifecycle.ts +++ b/mobile/src/transport/host-removal-lifecycle.ts @@ -12,10 +12,6 @@ export async function removeHostAndCloseClient( hostPublicKey: string, forgetHostClient: (hostId: string) => void ): Promise { - // Why: cache deletion is recoverable by redownload, so a hybrid-only failure here must - // never block the unpair itself — on a native build the cache may not even exist. - await removeMobileWebHostCache(hostPublicKey).catch(() => null) - await clearMobileWebColdResumeRouteForHost(hostId).catch(() => null) // Why: closing before the metadata commit can strand a still-paired host on // storage failure; closing immediately after success prevents socket leaks. await removeHost(hostId) @@ -26,5 +22,11 @@ export async function removeHostAndCloseClient( forgetHostNotificationSession(hostId) void clearWatermark(hostId) connectionLogStore.delete(hostId) + // Why last: the hybrid WebView serves its document out of this cache, so deleting it while one + // is still mounted 403s every asset the page asks for next. Cache deletion is recoverable by + // redownload, so a hybrid-only failure here must never block the unpair either — on a native + // build the cache may not even exist. + await removeMobileWebHostCache(hostPublicKey).catch(() => null) + await clearMobileWebColdResumeRouteForHost(hostId).catch(() => null) } } diff --git a/mobile/src/transport/runtime-capability-probe.test.ts b/mobile/src/transport/runtime-capability-probe.test.ts index 269702d0a61..58aa3452ae7 100644 --- a/mobile/src/transport/runtime-capability-probe.test.ts +++ b/mobile/src/transport/runtime-capability-probe.test.ts @@ -172,6 +172,26 @@ describe('startRuntimeCapabilityProbe', () => { cancel() }) + it('stops polling at cutover speed when the link keeps cutting over', async () => { + const outcomes: ProbeOutcome[] = Array.from( + { length: 6 }, + () => new LogicalClientCutoverError() + ) + outcomes.push(ok(['a.v1'])) + const { client, calls } = makeClient(outcomes) + const seen: (readonly string[])[] = [] + const cancel = startRuntimeCapabilityProbe(client, (capabilities) => seen.push(capabilities)) + await flushMicrotasks() + + // Three prompt retries, then the backoff ladder: an endless 4 Hz poll would be 40 calls here. + await vi.advanceTimersByTimeAsync(250 * 4) + expect(calls()).toBe(4) + await vi.advanceTimersByTimeAsync(1_000 + 2_000 + 4_000) + expect(seen).toEqual([['a.v1']]) + expect(calls()).toBe(7) + cancel() + }) + it('stops retrying and dropping results once cancelled', async () => { const { client, calls } = makeClient([new Error('timeout'), ok(['a.v1'])]) const seen: (readonly string[])[] = [] diff --git a/mobile/src/transport/runtime-capability-probe.ts b/mobile/src/transport/runtime-capability-probe.ts index 35cb32eda20..409c6ecb30b 100644 --- a/mobile/src/transport/runtime-capability-probe.ts +++ b/mobile/src/transport/runtime-capability-probe.ts @@ -6,6 +6,9 @@ import { isLogicalClientCutoverError } from './stable-logical-rpc-client' // status.get without ever changing connState, so a one-shot probe would latch // capability-gated UI hidden until the screen remounts; retry until one lands. const CUTOVER_RETRY_DELAY_MS = 250 +// Why a cap: a link that keeps forcing cutovers would otherwise poll status.get four times a second +// for as long as the screen is mounted. After the prompt attempts, cutovers join the backoff ladder. +const CUTOVER_FAST_RETRY_LIMIT = 3 const FAILURE_RETRY_BASE_DELAY_MS = 1_000 const FAILURE_RETRY_MAX_DELAY_MS = 15_000 @@ -43,6 +46,7 @@ export function startRuntimeCapabilityRead( let cancelled = false let retryTimer: ReturnType | null = null let failureRetries = 0 + let cutoverRetries = 0 function attempt(): void { void readCapabilities().then( @@ -64,9 +68,10 @@ export function startRuntimeCapabilityRead( function scheduleRetry(cutover: boolean): void { // Why: cutover means the replacement transport is already authenticated — // re-ask promptly; other failures back off so a wedged host isn't hammered. - const delay = cutover - ? CUTOVER_RETRY_DELAY_MS - : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) + const delay = + cutover && cutoverRetries++ < CUTOVER_FAST_RETRY_LIMIT + ? CUTOVER_RETRY_DELAY_MS + : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) retryTimer = setTimeout(attempt, delay) }