From ca7c7a281d810f0bd50491f0b732e7c95ee8c850 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 3 Sep 2026 22:52:21 -0400 Subject: [PATCH] fix(mobile): surface a dropped hosted tab subscription instead of spinning forever A rejected session snapshot (invalid shape, oversize, or a failed post) cancelled the shell-side subscription silently, the bridge client discarded the late error because the subscribe request had already resolved, and the page kept "Loading tabs" with no error and no retry. The shell now posts an error on the subscribe request id, the client routes it to the subscription's onError (which already falls back to polling), and the session screen shows Retry after two consecutive failures. Reuses the existing response opcode, so mixed versions degrade to today's behaviour. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-web-capability-subscriptions.ts | 4 + .../mobile-web-session-subscriptions.test.ts | 101 ++++++++++++++++++ .../mobile-web-session-subscriptions.ts | 25 ++++- .../session/MobileSessionActiveContent.tsx | 11 ++ .../mobile-session-route-parity.test.ts | 24 ++--- .../session/session-tabs-load-surface.test.ts | 87 +++++++++++++++ .../src/session/session-tabs-load-surface.ts | 35 ++++++ .../use-mobile-session-presentation.ts | 34 ++++-- .../use-mobile-session-tab-reconciliation.ts | 4 +- ...use-mobile-session-tabs-fetch-reporting.ts | 63 ++++++++--- .../src/mobile-web-bridge-client.test.ts | 35 ++++++ .../mobile-web-bridge-subscription-client.ts | 18 +++- 12 files changed, 404 insertions(+), 37 deletions(-) create mode 100644 mobile/src/mobile-web/mobile-web-session-subscriptions.test.ts create mode 100644 mobile/src/session/session-tabs-load-surface.test.ts create mode 100644 mobile/src/session/session-tabs-load-surface.ts diff --git a/mobile/src/mobile-web/mobile-web-capability-subscriptions.ts b/mobile/src/mobile-web/mobile-web-capability-subscriptions.ts index 533c24a84ca..835ca89b3d0 100644 --- a/mobile/src/mobile-web/mobile-web-capability-subscriptions.ts +++ b/mobile/src/mobile-web/mobile-web-capability-subscriptions.ts @@ -1,3 +1,4 @@ +import type { MobileWebBridgeErrorCode } from '../../../src/shared/mobile-web/bridge-contract' import { MobileWebAccountSubscriptions } from './mobile-web-account-subscriptions' import type { MobileWebBrowserAuthority } from './mobile-web-browser-authority' import { MobileWebBrowserStreams } from './mobile-web-browser-streams' @@ -26,6 +27,8 @@ export class MobileWebCapabilitySubscriptions { }) { const postEvent = (subscriptionId: string, sequence: number, event: unknown) => args.messages.event(subscriptionId, sequence, event) + const postError = (requestId: string, code: MobileWebBridgeErrorCode, retryable: boolean) => + args.messages.error(requestId, code, retryable) this.account = new MobileWebAccountSubscriptions({ isActive: args.isActive, postEvent }) this.browser = new MobileWebBrowserStreams({ isActive: args.isActive, @@ -41,6 +44,7 @@ export class MobileWebCapabilitySubscriptions { }) this.session = new MobileWebSessionSubscriptions({ isActive: args.isActive, + postError, browserAuthority: args.browserAuthority, nativeChatAuthority: args.nativeChatAuthority, postEvent diff --git a/mobile/src/mobile-web/mobile-web-session-subscriptions.test.ts b/mobile/src/mobile-web/mobile-web-session-subscriptions.test.ts new file mode 100644 index 00000000000..11c3c9acb4a --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-session-subscriptions.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import { AGENT_STATUS_ASSISTANT_MESSAGE_MAX_LENGTH } from '../../../src/shared/agent-status-types' +import { MobileWebBrowserAuthority } from './mobile-web-browser-authority' +import { MobileWebNativeChatAuthority } from './mobile-web-native-chat-authority' +import { mobileWebHostWorkspaceIdFromHost } from './mobile-web-workspace-authority' +import { MobileWebSessionSubscriptions } from './mobile-web-session-subscriptions' + +const HOST_WORKSPACE_ID = mobileWebHostWorkspaceIdFromHost('repo-1::/workspaces/one') + +function hostSnapshot(overrides: Record = {}): Record { + return { + worktree: HOST_WORKSPACE_ID, + publicationEpoch: 'renderer:1', + snapshotVersion: 1, + activeTabId: null, + activeTabType: null, + tabs: [], + ...overrides + } +} + +function harness(postEventImplementation?: () => Promise) { + const postEvent = vi.fn(postEventImplementation ?? (() => Promise.resolve())) + const postError = vi.fn(() => Promise.resolve()) + const subscriptions = new MobileWebSessionSubscriptions({ + isActive: () => true, + postEvent, + postError, + browserAuthority: new MobileWebBrowserAuthority((length) => new Uint8Array(length)), + nativeChatAuthority: new MobileWebNativeChatAuthority((length) => new Uint8Array(length)) + }) + let emit: (value: unknown) => void = () => {} + const unsubscribe = vi.fn() + const client = { + subscribe: vi.fn((_method: string, _params: unknown, onEvent: (value: unknown) => void) => { + emit = onEvent + return unsubscribe + }) + } as unknown as RpcClient + subscriptions.start({ + requestId: 'request-1', + subscriptionId: 'subscription-1', + pageWorkspaceId: 'workspace_0_page', + hostWorkspaceId: HOST_WORKSPACE_ID, + client + }) + return { subscriptions, postEvent, postError, unsubscribe, emit: (value: unknown) => emit(value) } +} + +describe('mobile web session subscriptions', () => { + it('reports a rejected host snapshot to the page instead of cancelling silently', () => { + const { postEvent, postError, unsubscribe, emit } = harness() + + emit(hostSnapshot({ worktree: 'repo-2::/workspaces/other' })) + + expect(postEvent).not.toHaveBeenCalled() + expect(unsubscribe).toHaveBeenCalledTimes(1) + expect(postError).toHaveBeenCalledWith('request-1', 'host_error', true) + }) + + it('reports an oversized snapshot to the page instead of cancelling silently', () => { + const { postEvent, postError, emit } = harness() + + emit( + hostSnapshot({ + tabs: Array.from({ length: 40 }, (_value, index) => ({ + type: 'terminal', + id: `tab-${index}`, + title: 'x'.repeat(240), + status: 'ready', + agentStatus: { + state: 'working', + lastAssistantMessage: 'y'.repeat(AGENT_STATUS_ASSISTANT_MESSAGE_MAX_LENGTH) + } + })) + }) + ) + + expect(postEvent).not.toHaveBeenCalled() + expect(postError).toHaveBeenCalledWith('request-1', 'too_large', false) + }) + + it('reports a failed event delivery to the page instead of cancelling silently', async () => { + const { postError, emit } = harness(() => Promise.reject(new Error('post failed'))) + + emit(hostSnapshot()) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(postError).toHaveBeenCalledWith('request-1', 'unavailable', true) + }) + + it('stays silent when the page or the broker retires the subscription', () => { + const { subscriptions, postError } = harness() + + subscriptions.cancel('subscription-1') + subscriptions.dispose() + + expect(postError).not.toHaveBeenCalled() + }) +}) diff --git a/mobile/src/mobile-web/mobile-web-session-subscriptions.ts b/mobile/src/mobile-web/mobile-web-session-subscriptions.ts index fd3250e8c78..a5e490be2fd 100644 --- a/mobile/src/mobile-web/mobile-web-session-subscriptions.ts +++ b/mobile/src/mobile-web/mobile-web-session-subscriptions.ts @@ -1,5 +1,8 @@ import type { MobileWebHostWorkspaceId } from './mobile-web-workspace-authority' -import { MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS } from '../../../src/shared/mobile-web/bridge-contract' +import { + MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS, + type MobileWebBridgeErrorCode +} from '../../../src/shared/mobile-web/bridge-contract' import { MOBILE_WEB_SESSION_EVENT_MAX_BYTES, type MobileWebSessionSnapshotResult @@ -29,6 +32,11 @@ export class MobileWebSessionSubscriptions { sequence: number, snapshot: MobileWebSessionSnapshotResult ) => Promise + postError: ( + requestId: string, + code: MobileWebBridgeErrorCode, + retryable: boolean + ) => Promise browserAuthority: MobileWebBrowserAuthority nativeChatAuthority: MobileWebNativeChatAuthority } @@ -139,11 +147,11 @@ export class MobileWebSessionSubscriptions { this.options.nativeChatAuthority ) } catch { - this.cancel(subscriptionId) + this.fail(subscriptionId, 'host_error', true) return } if (encodedByteLength(snapshot) > MOBILE_WEB_SESSION_EVENT_MAX_BYTES) { - this.cancel(subscriptionId) + this.fail(subscriptionId, 'too_large', false) return } const sequence = record.sequence @@ -159,9 +167,18 @@ export class MobileWebSessionSubscriptions { } }) .catch(() => { - this.cancel(subscriptionId) + this.fail(subscriptionId, 'unavailable', true) }) } + + // Why: the page waits on this stream to leave its loading state, so a shell-side + // drop it never hears about strands the session screen on the spinner forever. + private fail(subscriptionId: string, code: MobileWebBridgeErrorCode, retryable: boolean): void { + const requestId = this.cancel(subscriptionId) + if (requestId) { + void this.options.postError(requestId, code, retryable) + } + } } export class MobileWebSessionSubscriptionError extends Error { diff --git a/mobile/src/session/MobileSessionActiveContent.tsx b/mobile/src/session/MobileSessionActiveContent.tsx index 3c43eb40cce..33369cf2cea 100644 --- a/mobile/src/session/MobileSessionActiveContent.tsx +++ b/mobile/src/session/MobileSessionActiveContent.tsx @@ -73,6 +73,8 @@ export function MobileSessionActiveContent({ isPendingTerminalRecoveryParked, retryPendingTerminalRecovery, showLoadingState, + showTabsLoadError, + retryTabsLoad, showEmptyState, keyboardLift, activeTerminalKeyboardLift, @@ -86,6 +88,15 @@ export function MobileSessionActiveContent({ + ) : showTabsLoadError ? ( + + Tabs failed to load + + + Retry + + + ) : showEmptyState ? ( No tabs in this session diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index 7d61c1772d1..ce41801ddd0 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -66,11 +66,11 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = '4fae0d13d86c343b380969797051a376a101176cfaed2f945b4fb13e86b7fc25' -const HEAD_HOOK_BINDING_SHA256 = 'ccd2f55ded8d6f0a4bfed440d57302cc7bc6ea6d984ed0bba7b800b78fc48171' +const HEAD_MAIN_HOOK_SHA256 = 'eea8797cbbb19177db34d1d5db297824bc56ebaa71fb176ccd9a418b6e4908e7' +const HEAD_HOOK_BINDING_SHA256 = '73713d7f50e4bf9c59c4e287135f237369da7f18d160ff39fb0e6aacfa908c41' const HEAD_CALLBACK_IDENTITY_SHA256 = - '452c799e8bd87db34cb3176ee6640c6aa44836cd4e54abcaaf49a4c2f264b484' -const HEAD_CALLBACK_BODY_SHA256 = '383158ad94f45f982b1f175f8010ffea88c18f441ae5183b95a9169db8a52ed9' + '5abe27eb5145fe501574adae869e3765102d4eb8f0607224cfcc13eef722f9d0' +const HEAD_CALLBACK_BODY_SHA256 = 'f277bb218ff237c2717f8501cd9e9fc92223d36c459fca5898be4000590d64fe' const HEAD_EFFECT_SHA256 = '163e0de8969d8693c968e5c8f9b1ca366a2949c27f9f97b06882f1d7a87a4625' const HEAD_CONTENT_HOOK_SHA256 = 'd74431115b27c22dd38c29a510604554ca767cdd2585beaa73ec2e2dae0c5de4' const HEAD_NESTED_FUNCTION_SHA256 = @@ -83,11 +83,11 @@ const HEAD_TIMER_CREATION_SHA256 = '688342d48a1b4a46cdffbf0d8953bac245fb6d3c4fe1b5698a1ea6e1e1929bed' const HEAD_TIMER_CLEANUP_SHA256 = '8a45ae3c8a01a639a40ffaf3c0fc89a2e0b610623306818c86bad4ef9195b824' const HEAD_RUNTIME_STRING_SHA256 = - '70c0b2084ed16c423248e698dc1806dbca88b9aec9c043653409e7cc63191ff2' -const HEAD_HOST_JSX_SHA256 = '2911efcb57dbc9f6de1f2a7b3ed6ca4fa8a9735d48649fe062cb400df756e1ce' + '86029583841200cc9b7fea2cd764502ce149c537872fd0a73b78a43a1cca0277' +const HEAD_HOST_JSX_SHA256 = '37248c36ee013989fa8ae64cc6c0089427be5ba1f4730cc9c0c125e63274bb8e' const HEAD_LEAF_JSX_SHA256 = '7551bacf163f59c150cc8a9150c443df9804a882365f459053d3ab73ac557f42' const HEAD_STYLE_REFERENCE_SHA256 = - '3e4f57e5c8691d443187ffe306eae28506d5505276ea3de7a4f2f1df1cfa3885' + '618ed72381bff44f5b8a042544629ec2bd9834c3d4c3c3e6a71ab9f6673e6ac7' const HEAD_IDENTITY_FIELD_SHA256 = '2e5c63f41bf88bf07985d834319656306f0688469d212d586e29ec2fd77103ac' const HEAD_NAVIGATION_SHA256 = '12aba3574cb12b65e545f19e641e4ee07f90fa9d7ed98d24359a359d2c764aa4' @@ -476,10 +476,10 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(288) + expect(main.hooks).toHaveLength(289) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) - expect(main.callbacks).toHaveLength(84) + expect(main.callbacks).toHaveLength(85) expect(hash(main.callbacks)).toBe(HEAD_CALLBACK_IDENTITY_SHA256) expect(hash(main.callbackBodies)).toBe(HEAD_CALLBACK_BODY_SHA256) expect(main.effects).toHaveLength(24) @@ -521,14 +521,14 @@ describe('mobile session route extraction parity', () => { it('preserves runtime strings, styles, and the expanded JSX tree', () => { const strings = readRuntimeStrings() - expect(strings).toHaveLength(474) + expect(strings).toHaveLength(479) expect(hash(strings)).toBe(HEAD_RUNTIME_STRING_SHA256) const jsx = readJsxFacts(readDefinitions()) - expect(jsx.host).toHaveLength(126) + expect(jsx.host).toHaveLength(131) expect(hash(jsx.host)).toBe(HEAD_HOST_JSX_SHA256) expect(jsx.leaf).toHaveLength(63) expect(hash(jsx.leaf)).toBe(HEAD_LEAF_JSX_SHA256) - expect(jsx.styleReferences).toHaveLength(176) + expect(jsx.styleReferences).toHaveLength(181) expect(hash(jsx.styleReferences)).toBe(HEAD_STYLE_REFERENCE_SHA256) }) }) diff --git a/mobile/src/session/session-tabs-load-surface.test.ts b/mobile/src/session/session-tabs-load-surface.test.ts new file mode 100644 index 00000000000..24cea4164d5 --- /dev/null +++ b/mobile/src/session/session-tabs-load-surface.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from 'vitest' +import { + NO_SESSION_TABS_LOAD_FAILURE, + SESSION_TABS_LOAD_FAILURE_ATTEMPTS, + nextSessionTabsLoadFailure, + sessionTabsLoadSurface +} from './session-tabs-load-surface' + +function failedTimes(count: number) { + let failure = NO_SESSION_TABS_LOAD_FAILURE + for (let attempt = 0; attempt < count; attempt += 1) { + failure = nextSessionTabsLoadFailure(failure, 'host_error') + } + return failure +} + +describe('session tabs load surface', () => { + it('keeps the spinner while the first attempts are still in flight', () => { + expect( + sessionTabsLoadSurface({ + connected: true, + terminalsLoaded: false, + visibleTabCount: 0, + failure: failedTimes(SESSION_TABS_LOAD_FAILURE_ATTEMPTS - 1) + }) + ).toBe('loading') + }) + + it('replaces the spinner once loading has failed repeatedly', () => { + expect( + sessionTabsLoadSurface({ + connected: true, + terminalsLoaded: false, + visibleTabCount: 0, + failure: failedTimes(SESSION_TABS_LOAD_FAILURE_ATTEMPTS) + }) + ).toBe('error') + }) + + it('never covers a session that already has tabs or a landed snapshot', () => { + const failure = failedTimes(SESSION_TABS_LOAD_FAILURE_ATTEMPTS + 3) + expect( + sessionTabsLoadSurface({ + connected: true, + terminalsLoaded: true, + visibleTabCount: 0, + failure + }) + ).toBe('ready') + expect( + sessionTabsLoadSurface({ + connected: true, + terminalsLoaded: false, + visibleTabCount: 2, + failure + }) + ).toBe('ready') + expect( + sessionTabsLoadSurface({ + connected: false, + terminalsLoaded: false, + visibleTabCount: 0, + failure + }) + ).toBe('ready') + }) + + it('clears the failure run on the next success', () => { + const cleared = nextSessionTabsLoadFailure(failedTimes(5), null) + expect(cleared).toEqual(NO_SESSION_TABS_LOAD_FAILURE) + expect( + sessionTabsLoadSurface({ + connected: true, + terminalsLoaded: false, + visibleTabCount: 0, + failure: cleared + }) + ).toBe('loading') + }) + + it('reports the most recent failure code', () => { + expect(nextSessionTabsLoadFailure(failedTimes(2), 'too_large')).toEqual({ + attempts: 3, + code: 'too_large' + }) + }) +}) diff --git a/mobile/src/session/session-tabs-load-surface.ts b/mobile/src/session/session-tabs-load-surface.ts new file mode 100644 index 00000000000..d0ed8a18ab6 --- /dev/null +++ b/mobile/src/session/session-tabs-load-surface.ts @@ -0,0 +1,35 @@ +/** + * Whether the session screen is still waiting for its first tab snapshot or has + * given up on one. Repeated load failures used to be recorded in diagnostics only, + * so a workspace whose snapshot the host or the shell kept rejecting sat on the + * "Loading tabs" spinner with no error and no way to retry. + */ +export type SessionTabsLoadFailure = { + /** Consecutive failed loads since the last accepted snapshot. */ + attempts: number + code: string | null +} + +export const NO_SESSION_TABS_LOAD_FAILURE: SessionTabsLoadFailure = { attempts: 0, code: null } + +/** Polls run every 2s; two failures keeps one blip from flashing an error. */ +export const SESSION_TABS_LOAD_FAILURE_ATTEMPTS = 2 + +export function nextSessionTabsLoadFailure( + current: SessionTabsLoadFailure, + code: string | null +): SessionTabsLoadFailure { + return code === null ? NO_SESSION_TABS_LOAD_FAILURE : { attempts: current.attempts + 1, code } +} + +export function sessionTabsLoadSurface(args: { + connected: boolean + terminalsLoaded: boolean + visibleTabCount: number + failure: SessionTabsLoadFailure +}): 'loading' | 'error' | 'ready' { + if (!args.connected || args.terminalsLoaded || args.visibleTabCount > 0) { + return 'ready' + } + return args.failure.attempts >= SESSION_TABS_LOAD_FAILURE_ATTEMPTS ? 'error' : 'loading' +} diff --git a/mobile/src/session/use-mobile-session-presentation.ts b/mobile/src/session/use-mobile-session-presentation.ts index 32cfcf60944..7527763b895 100644 --- a/mobile/src/session/use-mobile-session-presentation.ts +++ b/mobile/src/session/use-mobile-session-presentation.ts @@ -1,8 +1,10 @@ +import { useCallback } from 'react' import { Platform } from 'react-native' import { classifyConnection, verdictDisplayLabel } from '../transport/connection-health' import { computeActiveTerminalKeyboardLift } from '../terminal/terminal-keyboard-avoidance-lift' import { useInitialSessionTerminalAutoCreate } from './use-initial-session-terminal-autocreate' import { MOBILE_SESSION_STATUS_LABELS } from './mobile-session-route-helpers' +import { sessionTabsLoadSurface } from './session-tabs-load-surface' import type { MobileSessionBulkCloseModel } from './use-mobile-session-bulk-close' export function useMobileSessionPresentation(scope: MobileSessionBulkCloseModel) { @@ -30,9 +32,23 @@ export function useMobileSessionPresentation(scope: MobileSessionBulkCloseModel) handleCreateTerminal, visibleTabs, sessionTabOperations, - setCreateError + setCreateError, + sessionTabsLoadFailure, + clearSessionTabsLoadFailure, + fetchSessionTabs } = scope - const showLoadingState = connState === 'connected' && !terminalsLoaded && visibleTabs.length === 0 + const tabsLoadSurface = sessionTabsLoadSurface({ + connected: connState === 'connected', + terminalsLoaded, + visibleTabCount: visibleTabs.length, + failure: sessionTabsLoadFailure + }) + const showLoadingState = tabsLoadSurface === 'loading' + const showTabsLoadError = tabsLoadSurface === 'error' + const retryTabsLoad = useCallback(() => { + clearSessionTabsLoadFailure() + void fetchSessionTabs() + }, [clearSessionTabsLoadFailure, fetchSessionTabs]) const showEmptyState = connState === 'connected' && terminalsLoaded && visibleTabs.length === 0 && !activeHandle @@ -66,11 +82,13 @@ export function useMobileSessionPresentation(scope: MobileSessionBulkCloseModel) const terminalSummary = connState === 'connected' - ? showLoadingState - ? 'Loading tabs' - : visibleTabs.length === 1 - ? '1 tab' - : `${visibleTabs.length} tabs` + ? showTabsLoadError + ? 'Tabs unavailable' + : showLoadingState + ? 'Loading tabs' + : visibleTabs.length === 1 + ? '1 tab' + : `${visibleTabs.length} tabs` : showConnectionRetry ? `${verdictDisplayLabel(connectionVerdict)} — tap to retry` : MOBILE_SESSION_STATUS_LABELS[connState] @@ -93,6 +111,8 @@ export function useMobileSessionPresentation(scope: MobileSessionBulkCloseModel) } return { showLoadingState, + showTabsLoadError, + retryTabsLoad, showEmptyState, connectionVerdict, showConnectionRetry, diff --git a/mobile/src/session/use-mobile-session-tab-reconciliation.ts b/mobile/src/session/use-mobile-session-tab-reconciliation.ts index 79a241a1b2b..bd592ee002f 100644 --- a/mobile/src/session/use-mobile-session-tab-reconciliation.ts +++ b/mobile/src/session/use-mobile-session-tab-reconciliation.ts @@ -117,7 +117,7 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc getPendingTerminalRecoveryContextKey, onPendingTerminalRecoveryParked: setParkedPendingTerminalContext, getApplicationRevision: getSessionTabsApplicationRevision, - ...sessionTabsFetchReporting + ...sessionTabsFetchReporting.reporting }) useEffect( @@ -168,6 +168,8 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc hasSessionTabsRecoveryNeed, getSessionTabsApplicationRevision, sessionTabsFetchReporting, + sessionTabsLoadFailure: sessionTabsFetchReporting.loadFailure, + clearSessionTabsLoadFailure: sessionTabsFetchReporting.clearLoadFailure, fetchSessionTabs, ensureSessionTabs, fetchPendingBrowserSessionTabs, diff --git a/mobile/src/session/use-mobile-session-tabs-fetch-reporting.ts b/mobile/src/session/use-mobile-session-tabs-fetch-reporting.ts index 13bbc7cb7a0..91c9a3ada9c 100644 --- a/mobile/src/session/use-mobile-session-tabs-fetch-reporting.ts +++ b/mobile/src/session/use-mobile-session-tabs-fetch-reporting.ts @@ -1,28 +1,67 @@ -import { useMemo, type MutableRefObject } from 'react' +import { useCallback, useMemo, useReducer, type MutableRefObject } from 'react' import type { MobileTerminalDiagnostics } from './mobile-terminal-diagnostics' +import { + NO_SESSION_TABS_LOAD_FAILURE, + nextSessionTabsLoadFailure, + type SessionTabsLoadFailure +} from './session-tabs-load-surface' type DiagnosticTabsSnapshot = Parameters[0] -/** Forwards session-tabs fetch outcomes to the screen's diagnostics recorder. - * Split out of the session route so the reconciliation wiring there stays a - * single call rather than five one-line callbacks. */ +type ScopedFailure = SessionTabsLoadFailure & { worktreeId: string } + +/** Forwards session-tabs fetch outcomes to the screen's diagnostics recorder and + * keeps the consecutive-failure run the screen needs to stop showing a spinner + * for a snapshot that never lands. Split out of the session route so the + * reconciliation wiring there stays a single call rather than five one-line + * callbacks. */ export function useMobileSessionTabsFetchReporting(args: { worktreeId: string diagnosticsRef: MutableRefObject }): { - onFetchStarted: () => void - onFetchSucceeded: (result: Result) => void - onFetchFailed: (code: string) => void - onFetchErrored: (error: unknown) => void + reporting: { + onFetchStarted: () => void + onFetchSucceeded: (result: Result) => void + onFetchFailed: (code: string) => void + onFetchErrored: (error: unknown) => void + } + loadFailure: SessionTabsLoadFailure + clearLoadFailure: () => void } { const { worktreeId, diagnosticsRef } = args - return useMemo( + // Why: the route is reused across workspaces, so a failure run from the previous + // one must not decide this one's surface. + const [scoped, recordOutcome] = useReducer( + (current: ScopedFailure, code: string | null): ScopedFailure => ({ + worktreeId, + ...nextSessionTabsLoadFailure( + current.worktreeId === worktreeId ? current : NO_SESSION_TABS_LOAD_FAILURE, + code + ) + }), + { worktreeId, ...NO_SESSION_TABS_LOAD_FAILURE } + ) + const reporting = useMemo( () => ({ onFetchStarted: () => diagnosticsRef.current.tabsFetchStarted(worktreeId), - onFetchSucceeded: (result: Result) => diagnosticsRef.current.tabsFetchSucceeded(result), - onFetchFailed: (code: string) => diagnosticsRef.current.tabsFetchFailed(code), - onFetchErrored: (error: unknown) => diagnosticsRef.current.tabsFetchErrored(error) + onFetchSucceeded: (result: Result) => { + diagnosticsRef.current.tabsFetchSucceeded(result) + recordOutcome(null) + }, + onFetchFailed: (code: string) => { + diagnosticsRef.current.tabsFetchFailed(code) + recordOutcome(code) + }, + onFetchErrored: (error: unknown) => { + diagnosticsRef.current.tabsFetchErrored(error) + recordOutcome('unavailable') + } }), [diagnosticsRef, worktreeId] ) + return { + reporting, + loadFailure: scoped.worktreeId === worktreeId ? scoped : NO_SESSION_TABS_LOAD_FAILURE, + clearLoadFailure: useCallback(() => recordOutcome(null), []) + } } diff --git a/src/mobile-web/src/mobile-web-bridge-client.test.ts b/src/mobile-web/src/mobile-web-bridge-client.test.ts index 2039a2cc519..1c292fa17cc 100644 --- a/src/mobile-web/src/mobile-web-bridge-client.test.ts +++ b/src/mobile-web/src/mobile-web-bridge-client.test.ts @@ -257,6 +257,41 @@ describe('mobile web bridge client', () => { }) }) + it('retires a live session subscription when the shell reports it dropped', async () => { + const messages: MobileWebBridgePageMessage[] = [] + const ids = ['Q'.repeat(22), 'S'.repeat(22)] + const onEvent = vi.fn() + const onError = vi.fn() + const client = new MobileWebBridgeClient({ + context: CONTEXT, + grants: [sessionSubscriptionGrant()], + postMessage: (message) => { + messages.push(message) + return true + }, + createRequestId: () => ids.shift() ?? 'Z'.repeat(22) + }) + const subscription = client.sessionSubscribe({ workspaceId: 'workspace-1' }, onEvent, onError) + client.receive(subscriptionResponse()) + await subscription.ready + + client.receive({ + version: MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, + type: 'response', + shellSessionId: CONTEXT.shellSessionId, + buildId: CONTEXT.buildId, + requestId: 'Q'.repeat(22), + status: 'error', + error: { code: 'host_error', retryable: true } + }) + + expect(onError).toHaveBeenCalledWith( + expect.objectContaining({ code: 'host_error', retryable: true }) + ) + client.receive(subscriptionEvent(0, 1)) + expect(onEvent).not.toHaveBeenCalled() + }) + it('retires a session subscription on a cross-workspace event', async () => { const messages: MobileWebBridgePageMessage[] = [] const ids = ['Q'.repeat(22), 'S'.repeat(22)] diff --git a/src/mobile-web/src/mobile-web-bridge-subscription-client.ts b/src/mobile-web/src/mobile-web-bridge-subscription-client.ts index c8c9fc2472b..d4676a27769 100644 --- a/src/mobile-web/src/mobile-web-bridge-subscription-client.ts +++ b/src/mobile-web/src/mobile-web-bridge-subscription-client.ts @@ -208,7 +208,10 @@ export class MobileWebBridgeSubscriptionClient { } const pending = this.pending.get(message.requestId) if (!pending) { - return false + // Why: the shell reports a dropped stream on the subscribe request id, which is + // no longer pending once `ready` resolved. Ignoring it strands the page waiting + // on events that will never arrive. + return message.status === 'error' && this.failByRequest(message.requestId, message.error) } clearTimeout(pending.timer) this.pending.delete(message.requestId) @@ -275,6 +278,19 @@ export class MobileWebBridgeSubscriptionClient { return true } + private failByRequest( + requestId: string, + error: { code: MobileWebBridgeClientError['code']; retryable: boolean } + ): boolean { + for (const [subscriptionId, subscription] of this.active) { + if (subscription.requestId === requestId) { + this.fail(subscriptionId, new MobileWebBridgeClientError(error.code, error.retryable)) + return true + } + } + return false + } + private unsubscribe(subscriptionId: string, expected: MobileWebActiveSubscription): void { if (this.active.get(subscriptionId) !== expected) { return