diff --git a/mobile/app/hybrid.tsx b/mobile/app/hybrid.tsx index 51438dcf1d1..7953ab9e15b 100644 --- a/mobile/app/hybrid.tsx +++ b/mobile/app/hybrid.tsx @@ -8,12 +8,10 @@ import { type MobileWebBridgeShellMessage, type MobileWebResumeRoute } from '../../src/shared/mobile-web/bridge-contract' -import { - MOBILE_WEB_PRODUCTION_GRANTS, - MobileWebCapabilityBroker -} from '../src/mobile-web/mobile-web-capability-broker' +import { MobileWebCapabilityBroker } from '../src/mobile-web/mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from '../src/mobile-web/mobile-web-production-grants' import { MobileWebHealthDeadline } from '../src/mobile-web/mobile-web-health-deadline' -import { useMobileWebPackageSession } from '../src/mobile-web/use-mobile-web-package-session' +import { useMobileWebAlertSafePackageSession } from '../src/mobile-web/use-mobile-web-alert-safe-package-session' import { createMobileWebNativeCapabilityAuthority } from '../src/mobile-web/mobile-web-native-capability-authority' import { MobileWebHybridShellPresentation } from '../src/mobile-web/MobileWebHybridShellPresentation' import { useMobileWebNavigationIntentHandoff } from '../src/mobile-web/use-mobile-web-navigation-intent-handoff' @@ -85,7 +83,7 @@ export default function HybridScreen() { recoverPrevious, clearCache, showWarning - } = useMobileWebPackageSession({ client, host: selectedHost, state }) + } = useMobileWebAlertSafePackageSession({ client, host: selectedHost, state }) const bridgeRuntimeRef = useMobileWebBridgeRuntimeRef(client, state, session?.sessionId) const coldResumeRoute = useMobileWebColdResumeRoute({ hosts, diff --git a/mobile/src/mobile-web/mobile-web-back-navigation-adapter.test.ts b/mobile/src/mobile-web/mobile-web-back-navigation-adapter.test.ts index 4a42326d712..636c77c4671 100644 --- a/mobile/src/mobile-web/mobile-web-back-navigation-adapter.test.ts +++ b/mobile/src/mobile-web/mobile-web-back-navigation-adapter.test.ts @@ -117,6 +117,28 @@ describe('mobile web BackHandler adapter', () => { expect(harness.target.location.href).toBe('https://orca.test/files') expect(handler).not.toHaveBeenCalled() }) + + it('tracks rapid delayed Back traversals independently', () => { + vi.useFakeTimers() + const harness = navigationHarness(500) + const backHandler = backHandlerTarget() + installMobileWebBackNavigationAdapter(backHandler, harness.target) + harness.target.history.pushState({}, '', '/files') + harness.target.history.pushState({}, '', '/preview') + harness.target.history.pushState({}, '', '/detail') + const handler = vi.fn(() => { + harness.target.history.back() + return true + }) + backHandler.addEventListener('hardwareBackPress', handler) + + expect(dispatchMobileWebBackNavigation(harness.target)).toBe(true) + expect(dispatchMobileWebBackNavigation(harness.target)).toBe(true) + vi.advanceTimersByTime(500) + + expect(handler).toHaveBeenCalledTimes(2) + expect(harness.target.location.href).toBe('https://orca.test/files') + }) }) function backHandlerTarget() { diff --git a/mobile/src/mobile-web/mobile-web-back-navigation-adapter.ts b/mobile/src/mobile-web/mobile-web-back-navigation-adapter.ts index 9c5d0a9aaed..a34452bc93b 100644 --- a/mobile/src/mobile-web/mobile-web-back-navigation-adapter.ts +++ b/mobile/src/mobile-web/mobile-web-back-navigation-adapter.ts @@ -21,8 +21,14 @@ type MobileWebNavigationTarget = { const HISTORY_INDEX_KEY = '__orcaMobileWebBackIndex' const PROGRAMMATIC_TRAVERSAL_TIMEOUT_MS = 5_000 +const MAX_EXPECTED_TRAVERSALS = 32 const installedHistories = new WeakMap boolean>() +type ExpectedTraversal = { + targetIndex: number + timeout: ReturnType +} + export function installMobileWebBackNavigationAdapter( backHandler: MobileWebBackHandlerTarget = BackHandler, target: MobileWebNavigationTarget = window @@ -40,19 +46,27 @@ export function installMobileWebBackNavigationAdapter( const originalForward = history.forward.bind(history) let currentIndex = historyIndex(history.state) ?? 0 let maximumIndex = currentIndex - let expectedProgrammaticIndex: number | null = null - let programmaticReset: ReturnType | undefined + const expectedTraversals: ExpectedTraversal[] = [] let restoringFromIndex: number | null = null originalReplaceState(indexedHistoryState(history.state, currentIndex), '', undefined) const markProgrammatic = (targetIndex: number): void => { - expectedProgrammaticIndex = targetIndex - clearTimeout(programmaticReset) - programmaticReset = setTimeout(() => { - expectedProgrammaticIndex = null - }, PROGRAMMATIC_TRAVERSAL_TIMEOUT_MS) + if (expectedTraversals.length === MAX_EXPECTED_TRAVERSALS) { + clearTimeout(expectedTraversals.shift()?.timeout) + } + const expected: ExpectedTraversal = { + targetIndex, + timeout: setTimeout(() => { + const index = expectedTraversals.indexOf(expected) + if (index !== -1) { + expectedTraversals.splice(index, 1) + } + }, PROGRAMMATIC_TRAVERSAL_TIMEOUT_MS) + } + expectedTraversals.push(expected) } + const projectedIndex = (): number => expectedTraversals.at(-1)?.targetIndex ?? currentIndex history.pushState = (data, unused, url) => { currentIndex += 1 maximumIndex = currentIndex @@ -62,21 +76,24 @@ export function installMobileWebBackNavigationAdapter( originalReplaceState(indexedHistoryState(data, currentIndex), unused, url) } history.go = (delta = 0) => { - const targetIndex = Math.max(0, Math.min(maximumIndex, currentIndex + delta)) - if (targetIndex !== currentIndex) { + const fromIndex = projectedIndex() + const targetIndex = Math.max(0, Math.min(maximumIndex, fromIndex + delta)) + if (targetIndex !== fromIndex) { markProgrammatic(targetIndex) } originalGo(delta) } history.back = () => { - if (currentIndex > 0) { - markProgrammatic(currentIndex - 1) + const fromIndex = projectedIndex() + if (fromIndex > 0) { + markProgrammatic(fromIndex - 1) } originalBack() } history.forward = () => { - if (currentIndex < maximumIndex) { - markProgrammatic(currentIndex + 1) + const fromIndex = projectedIndex() + if (fromIndex < maximumIndex) { + markProgrammatic(fromIndex + 1) } originalForward() } @@ -114,9 +131,12 @@ export function installMobileWebBackNavigationAdapter( } const direction = Math.sign(nextIndex - currentIndex) - if (nextIndex === expectedProgrammaticIndex) { - expectedProgrammaticIndex = null - clearTimeout(programmaticReset) + const expectedIndex = expectedTraversals.findIndex( + (expected) => expected.targetIndex === nextIndex + ) + if (expectedIndex !== -1) { + const consumed = expectedTraversals.splice(0, expectedIndex + 1) + consumed.forEach((expected) => clearTimeout(expected.timeout)) currentIndex = nextIndex return } diff --git a/mobile/src/mobile-web/mobile-web-bridge-roundtrip.test.ts b/mobile/src/mobile-web/mobile-web-bridge-roundtrip.test.ts index 2b3d68a0489..ab046b681c6 100644 --- a/mobile/src/mobile-web/mobile-web-bridge-roundtrip.test.ts +++ b/mobile/src/mobile-web/mobile-web-bridge-roundtrip.test.ts @@ -4,7 +4,7 @@ import { parseMobileWebBridgeShellMessage } from '../../../src/shared/mobile-web/bridge-contract' import type { RpcClient } from '../transport/rpc-client' -import { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-production-grants' import { createMobileWebBridgeRoundtripFixture } from './mobile-web-bridge-roundtrip-fixture' const CONTEXT = { diff --git a/mobile/src/mobile-web/mobile-web-capability-broker.ts b/mobile/src/mobile-web/mobile-web-capability-broker.ts index d32b3cdf418..0bc47ce6008 100644 --- a/mobile/src/mobile-web/mobile-web-capability-broker.ts +++ b/mobile/src/mobile-web/mobile-web-capability-broker.ts @@ -34,7 +34,6 @@ import { type PageRequest = Extract type PendingRequest = { operationKey: string; subscriptionId?: string; cancelled: boolean } -export { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-production-grants' export class MobileWebCapabilityBroker { private readonly pending = new Map() private readonly replay = new MobileWebBrokerReplayGuard() @@ -218,6 +217,7 @@ export class MobileWebCapabilityBroker { await this.messages.error(request.requestId, code, isRetryableMobileWebBridgeError(code)) } } finally { + this.authorities.sourceControlBranchCompare.releaseClaim(request.requestId) if (this.pending.get(request.requestId) === pending) { this.pending.delete(request.requestId) } diff --git a/mobile/src/mobile-web/mobile-web-capability-execution.ts b/mobile/src/mobile-web/mobile-web-capability-execution.ts index cc5aa0f3dc9..81fc9d28c53 100644 --- a/mobile/src/mobile-web/mobile-web-capability-execution.ts +++ b/mobile/src/mobile-web/mobile-web-capability-execution.ts @@ -231,6 +231,7 @@ export async function executeMobileWebCapabilityRequest( client: args.connectedClient(), workspaceAuthority: args.workspaceAuthority, branchComparePager: args.sourceControlBranchCompare, + requestId: request.requestId, terminalClientId: args.terminalClientId }) } diff --git a/mobile/src/mobile-web/mobile-web-capability-schema-corpus.test.ts b/mobile/src/mobile-web/mobile-web-capability-schema-corpus.test.ts index fee89cbadf7..149a824ac12 100644 --- a/mobile/src/mobile-web/mobile-web-capability-schema-corpus.test.ts +++ b/mobile/src/mobile-web/mobile-web-capability-schema-corpus.test.ts @@ -5,10 +5,8 @@ import { type MobileWebBridgeShellMessage } from '../../../src/shared/mobile-web/bridge-contract' import type { RpcClient } from '../transport/rpc-client' -import { - MOBILE_WEB_PRODUCTION_GRANTS, - MobileWebCapabilityBroker -} from './mobile-web-capability-broker' +import { MobileWebCapabilityBroker } from './mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-production-grants' const CONTEXT = { shellSessionId: 'S'.repeat(43), diff --git a/mobile/src/mobile-web/mobile-web-markdown-roundtrip.test.ts b/mobile/src/mobile-web/mobile-web-markdown-roundtrip.test.ts index 88b4fa44bd1..d9b654a8777 100644 --- a/mobile/src/mobile-web/mobile-web-markdown-roundtrip.test.ts +++ b/mobile/src/mobile-web/mobile-web-markdown-roundtrip.test.ts @@ -5,10 +5,8 @@ import { } from '../../../src/shared/mobile-web/bridge-contract' import { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' import type { RpcClient } from '../transport/rpc-client' -import { - MOBILE_WEB_PRODUCTION_GRANTS, - MobileWebCapabilityBroker -} from './mobile-web-capability-broker' +import { MobileWebCapabilityBroker } from './mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-production-grants' const CONTEXT = { shellSessionId: 'S'.repeat(43), diff --git a/mobile/src/mobile-web/mobile-web-native-alert.test.ts b/mobile/src/mobile-web/mobile-web-native-alert.test.ts index 5aa6da6ca95..93d283f1571 100644 --- a/mobile/src/mobile-web/mobile-web-native-alert.test.ts +++ b/mobile/src/mobile-web/mobile-web-native-alert.test.ts @@ -1,6 +1,9 @@ import type { AlertButton, AlertOptions } from 'react-native' import { describe, expect, it, vi } from 'vitest' -import { presentMobileWebNativeAlert } from './mobile-web-native-alert' +import { + MobileWebNativeAlertLifecycle, + presentMobileWebNativeAlert +} from './mobile-web-native-alert' vi.mock('react-native', () => ({ Alert: { alert: vi.fn() } })) @@ -57,4 +60,37 @@ describe('mobile web native alert', () => { options?.onDismiss?.() await expect(result).resolves.toEqual({ kind: 'dismissed' }) }) + + it('serializes native alerts across broker lifecycles', async () => { + let buttons: AlertButton[] = [] + let options: AlertOptions | undefined + const target = { + alert: vi.fn((_title, _message, nextButtons, nextOptions) => { + buttons = nextButtons ?? [] + options = nextOptions + }) + } + const lifecycle = new MobileWebNativeAlertLifecycle() + const first = lifecycle.present({ title: 'First', buttons: [{ text: 'OK' }] }, target) + + await expect( + lifecycle.present({ title: 'Second', buttons: [{ text: 'OK' }] }, target) + ).rejects.toMatchObject({ code: 'rate_limited' }) + let idle = false + const waiting = lifecycle.waitForIdle().then(() => { + idle = true + }) + await Promise.resolve() + expect(idle).toBe(false) + + buttons[0]?.onPress?.() + options?.onDismiss?.() + await expect(first).resolves.toEqual({ kind: 'button', buttonIndex: 0 }) + await waiting + expect(idle).toBe(true) + + const third = lifecycle.present({ title: 'Third', buttons: [{ text: 'OK' }] }, target) + options?.onDismiss?.() + await expect(third).resolves.toEqual({ kind: 'dismissed' }) + }) }) diff --git a/mobile/src/mobile-web/mobile-web-native-alert.ts b/mobile/src/mobile-web/mobile-web-native-alert.ts index a617352caa3..00ee630f0bb 100644 --- a/mobile/src/mobile-web/mobile-web-native-alert.ts +++ b/mobile/src/mobile-web/mobile-web-native-alert.ts @@ -3,6 +3,7 @@ import type { MobileWebNativeAlertPayload, MobileWebNativeAlertResult } from '../../../src/shared/mobile-web/native-operation-contract' +import { MobileWebBrokerError } from './mobile-web-broker-error' type MobileWebNativeAlertTarget = Pick @@ -32,3 +33,34 @@ export function presentMobileWebNativeAlert( ) }) } + +export class MobileWebNativeAlertLifecycle { + private pending: Promise | null = null + + readonly present = ( + payload: MobileWebNativeAlertPayload, + target: MobileWebNativeAlertTarget = Alert + ): Promise => { + if (this.pending) { + return Promise.reject(new MobileWebBrokerError('rate_limited')) + } + const result = presentMobileWebNativeAlert(payload, target) + const pending = result.then( + () => undefined, + () => undefined + ) + this.pending = pending + void pending.then(() => { + if (this.pending === pending) { + this.pending = null + } + }) + return result + } + + readonly waitForIdle = async (): Promise => { + await this.pending + } +} + +export const mobileWebNativeAlertLifecycle = new MobileWebNativeAlertLifecycle() diff --git a/mobile/src/mobile-web/mobile-web-native-capability-authority.ts b/mobile/src/mobile-web/mobile-web-native-capability-authority.ts index 9725cf4e8ab..41b6d35599a 100644 --- a/mobile/src/mobile-web/mobile-web-native-capability-authority.ts +++ b/mobile/src/mobile-web/mobile-web-native-capability-authority.ts @@ -49,7 +49,7 @@ import { } from '../components/codex-reset-credit' import { readCodexResetCreditCapability } from '../components/codex-reset-credit-capability' import type { RpcClient } from '../transport/rpc-client' -import { presentMobileWebNativeAlert } from './mobile-web-native-alert' +import { mobileWebNativeAlertLifecycle } from './mobile-web-native-alert' type MobileWebNativeDraftScope = { hostIdentity: string @@ -98,10 +98,13 @@ export type MobileWebNativeCapabilityAuthority = { } export function createMobileWebNativeCapabilityAuthority( - draftScope: MobileWebNativeDraftScope + draftScope: MobileWebNativeDraftScope, + alert: NonNullable< + MobileWebNativeCapabilityAuthority['alert'] + > = mobileWebNativeAlertLifecycle.present ): MobileWebNativeCapabilityAuthority { return { - alert: presentMobileWebNativeAlert, + alert, hapticFeedback(kind) { if (kind === 'selection') { triggerSelection() diff --git a/mobile/src/mobile-web/mobile-web-native-chat-pending-roundtrip.test.ts b/mobile/src/mobile-web/mobile-web-native-chat-pending-roundtrip.test.ts index 3d0beec1bf7..94f72727764 100644 --- a/mobile/src/mobile-web/mobile-web-native-chat-pending-roundtrip.test.ts +++ b/mobile/src/mobile-web/mobile-web-native-chat-pending-roundtrip.test.ts @@ -5,10 +5,8 @@ import { } from '../../../src/shared/mobile-web/bridge-contract' import { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' import type { RpcClient } from '../transport/rpc-client' -import { - MOBILE_WEB_PRODUCTION_GRANTS, - MobileWebCapabilityBroker -} from './mobile-web-capability-broker' +import { MobileWebCapabilityBroker } from './mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-production-grants' const CONTEXT = { shellSessionId: 'S'.repeat(43), diff --git a/mobile/src/mobile-web/mobile-web-native-chat-subscription-roundtrip.test.ts b/mobile/src/mobile-web/mobile-web-native-chat-subscription-roundtrip.test.ts index 3bbb805f970..f64591b2008 100644 --- a/mobile/src/mobile-web/mobile-web-native-chat-subscription-roundtrip.test.ts +++ b/mobile/src/mobile-web/mobile-web-native-chat-subscription-roundtrip.test.ts @@ -5,10 +5,8 @@ import { } from '../../../src/shared/mobile-web/bridge-contract' import { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' import type { RpcClient } from '../transport/rpc-client' -import { - MOBILE_WEB_PRODUCTION_GRANTS, - MobileWebCapabilityBroker -} from './mobile-web-capability-broker' +import { MobileWebCapabilityBroker } from './mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from './mobile-web-production-grants' const CONTEXT = { shellSessionId: 'S'.repeat(43), diff --git a/mobile/src/mobile-web/mobile-web-package-session-state.ts b/mobile/src/mobile-web/mobile-web-package-session-state.ts new file mode 100644 index 00000000000..c17d583b471 --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-package-session-state.ts @@ -0,0 +1,15 @@ +import type { MobileWebShellSession } from '@orca/expo-mobile-web-shell' + +export type MobileWebPackageSession = { + session: MobileWebShellSession | null + viewEpoch: number + packageLoading: boolean + packageWarning: string | undefined + markHealthy: (sessionId: string) => Promise + handleHealthTimeout: (sessionId: string) => Promise + handleProcessTerminated: (sessionId: string) => Promise + retryPackage: () => void + recoverPrevious: () => Promise + clearCache: () => Promise + showWarning: (warning: string) => void +} diff --git a/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.test.ts b/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.test.ts index e732471cdad..ee87a912404 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.test.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.test.ts @@ -12,6 +12,21 @@ const OID_A = 'a'.repeat(40) const OID_B = 'b'.repeat(40) describe('mobile web source-control branch-compare pager', () => { + it('creates continuation identities without a Node global Buffer', async () => { + const originalBuffer = globalThis.Buffer + vi.stubGlobal('Buffer', undefined) + try { + const pager = new MobileWebSourceControlBranchComparePager() + const client = compareClient(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT + 1) + + await expect(pager.page(firstPayload(), client, workspaceAuthority)).resolves.toMatchObject({ + nextOffset: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT + }) + } finally { + vi.stubGlobal('Buffer', originalBuffer) + } + }) + it('uses one host snapshot across single-use continuation claims', async () => { const pager = new MobileWebSourceControlBranchComparePager() const client = compareClient(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT + 1) @@ -46,22 +61,61 @@ describe('mobile web source-control branch-compare pager', () => { }) expect(client.sendRequest).toHaveBeenCalledOnce() }) + + it('keeps two identical comparison sequences independently claimable', async () => { + const pager = new MobileWebSourceControlBranchComparePager() + const client = compareClient(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT + 1) + const [first, second] = await Promise.all([ + pager.page(firstPayload(), client, workspaceAuthority), + pager.page(firstPayload(), client, workspaceAuthority) + ]) + const firstContinuation = continuationPayload(first) + const secondContinuation = continuationPayload(second) + + expect(pager.claimContinuation(firstContinuation, 'request-a')).toBe(true) + expect(pager.claimContinuation(secondContinuation, 'request-b')).toBe(true) + await expect( + Promise.all([ + pager.page(firstContinuation, client, workspaceAuthority, 'request-a'), + pager.page(secondContinuation, client, workspaceAuthority, 'request-b') + ]) + ).resolves.toEqual([ + expect.objectContaining({ nextOffset: null }), + expect.objectContaining({ nextOffset: null }) + ]) + expect(client.sendRequest).toHaveBeenCalledTimes(2) + }) + + it('does not let one comparison identity revoke another', async () => { + const pager = new MobileWebSourceControlBranchComparePager() + const client = compareClient(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT + 1) + const first = await pager.page(firstPayload('main'), client, workspaceAuthority) + const second = await pager.page(firstPayload('release'), client, workspaceAuthority) + const firstContinuation = continuationPayload(first, 'main') + const secondContinuation = continuationPayload(second, 'release') + + expect(pager.claimContinuation(firstContinuation, 'request-a')).toBe(true) + expect(pager.claimContinuation(secondContinuation, 'request-b')).toBe(true) + }) }) -function firstPayload(): MobileWebSourceControlBranchComparePayload { - return { workspaceId: 'workspace-1', baseRef: 'main', offset: 0, limit: 128 } +function firstPayload(baseRef = 'main'): MobileWebSourceControlBranchComparePayload { + return { workspaceId: 'workspace-1', baseRef, offset: 0, limit: 128 } } -function continuationPayload(first: { - nextOffset: number | null - revision: string -}): MobileWebSourceControlBranchComparePayload { +function continuationPayload( + first: { + nextOffset: number | null + revision: string + }, + baseRef = 'main' +): MobileWebSourceControlBranchComparePayload { if (first.nextOffset === null) { throw new Error('Expected a continuation') } return { workspaceId: 'workspace-1', - baseRef: 'main', + baseRef, offset: first.nextOffset, limit: 128, expectedRevision: first.revision diff --git a/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.ts b/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.ts index 0b822ce30da..09728e27e03 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-branch-compare-pager.ts @@ -3,6 +3,8 @@ import { type MobileWebSourceControlBranchComparePayload, type MobileWebSourceControlBranchCompareResult } from '../../../src/shared/mobile-web/source-control-history-contract' +import { sha256 } from '@noble/hashes/sha256' +import { Buffer } from 'buffer/' import type { RpcClient } from '../transport/rpc-client' import { MobileWebBrokerError } from './mobile-web-broker-error' import { mobileWebEncodedByteLength } from './mobile-web-request-accounting' @@ -10,6 +12,8 @@ import { sanitizeMobileWebBranchCompare } from './mobile-web-source-control-hist import type { MobileWebWorkspaceAuthority } from './mobile-web-workspace-authority' const HOST_RESULT_MAX_BYTES = 8 * 1024 * 1024 +const MAX_RETAINED_CONTINUATIONS = 8 +const DIRECT_REQUEST_ID = 'direct' type Continuation = { workspaceId: string @@ -20,77 +24,83 @@ type Continuation = { } export class MobileWebSourceControlBranchComparePager { - private continuation: Continuation | null = null - private claimed: Continuation | null = null - private active = false + private readonly continuations: Continuation[] = [] + private readonly claimed = new Map() + private sequence = 0 claimRequestContinuation(request: { + requestId: string capability: string operation: string payload: unknown }): boolean { - this.claimed = null return ( request.capability === 'sourceControl' && request.operation === 'branchCompare' && - this.claimContinuation(request.payload) + this.claimContinuation(request.payload, request.requestId) ) } - claimContinuation(payloadValue: unknown): boolean { + claimContinuation(payloadValue: unknown, requestId = DIRECT_REQUEST_ID): boolean { const parsed = MobileWebSourceControlBranchComparePayloadSchema.safeParse(payloadValue) - const continuation = this.continuation - if (!parsed.success || !parsed.data.expectedRevision || !continuation) { + if (!parsed.success || !parsed.data.expectedRevision || this.claimed.has(requestId)) { return false } const payload = parsed.data - if ( - continuation.workspaceId !== payload.workspaceId || - continuation.baseRef !== payload.baseRef || - continuation.revision !== payload.expectedRevision || - continuation.nextOffset !== payload.offset - ) { + const index = this.continuations.findIndex((continuation) => + matchesContinuation(continuation, payload) + ) + if (index === -1) { return false } - this.continuation = null - this.claimed = continuation + const [continuation] = this.continuations.splice(index, 1) + if (!continuation) { + return false + } + this.claimed.set(requestId, continuation) return true } async page( payloadValue: unknown, client: RpcClient, - workspaceAuthority: MobileWebWorkspaceAuthority + workspaceAuthority: MobileWebWorkspaceAuthority, + requestId = DIRECT_REQUEST_ID ): Promise { - if (this.active) { - throw new MobileWebBrokerError('rate_limited') - } - this.active = true try { const payload = MobileWebSourceControlBranchComparePayloadSchema.parse(payloadValue) - const hostResult = payload.expectedRevision - ? this.consumeClaim(payload) + const continuation = payload.expectedRevision ? this.consumeClaim(payload, requestId) : null + const hostResult = continuation + ? continuation.hostResult : await this.begin(payload, client, workspaceAuthority) - const page = sanitizeMobileWebBranchCompare(hostResult, payload) + const sanitized = sanitizeMobileWebBranchCompare( + hostResult, + continuation ? { ...payload, expectedRevision: undefined } : payload + ) + const revision = continuation?.revision ?? this.createRevision(sanitized.revision, requestId) + const page = { ...sanitized, revision } if (page.nextOffset !== null) { - this.continuation = { + this.retain({ workspaceId: payload.workspaceId, baseRef: payload.baseRef, revision: page.revision, nextOffset: page.nextOffset, hostResult - } + }) } return page } finally { - this.active = false - this.claimed = null + this.claimed.delete(requestId) } } clear(): void { - this.continuation = null - this.claimed = null + this.continuations.length = 0 + this.claimed.clear() + } + + releaseClaim(requestId: string): void { + this.claimed.delete(requestId) } private async begin( @@ -98,7 +108,6 @@ export class MobileWebSourceControlBranchComparePager { client: RpcClient, workspaceAuthority: MobileWebWorkspaceAuthority ): Promise { - this.clear() if (payload.offset !== 0) { throw new MobileWebBrokerError('invalid_request') } @@ -116,8 +125,11 @@ export class MobileWebSourceControlBranchComparePager { return response.result } - private consumeClaim(payload: MobileWebSourceControlBranchComparePayload): unknown { - const continuation = this.claimed + private consumeClaim( + payload: MobileWebSourceControlBranchComparePayload, + requestId: string + ): Continuation { + const continuation = this.claimed.get(requestId) if (!continuation) { throw new MobileWebBrokerError('invalid_request') } @@ -131,6 +143,31 @@ export class MobileWebSourceControlBranchComparePager { ) { throw new MobileWebBrokerError('invalid_request') } - return continuation.hostResult + return continuation + } + + private retain(continuation: Continuation): void { + if (this.continuations.length >= MAX_RETAINED_CONTINUATIONS) { + this.continuations.shift() + } + this.continuations.push(continuation) + } + + private createRevision(contentRevision: string, requestId: string): string { + this.sequence += 1 + const content = new TextEncoder().encode(`${contentRevision}:${requestId}:${this.sequence}`) + return Buffer.from(sha256(content)).toString('hex') } } + +function matchesContinuation( + continuation: Continuation, + payload: MobileWebSourceControlBranchComparePayload +): boolean { + return ( + continuation.workspaceId === payload.workspaceId && + continuation.baseRef === payload.baseRef && + continuation.revision === payload.expectedRevision && + continuation.nextOffset === payload.offset + ) +} diff --git a/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts b/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts index c971752013c..6e6830b0792 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts @@ -41,6 +41,7 @@ export async function executeMobileWebSourceControlHistoryOperation(args: { client: RpcClient workspaceAuthority: MobileWebWorkspaceAuthority branchComparePager?: MobileWebSourceControlBranchComparePager + requestId?: string }): Promise { if (args.operation === 'branches') { const payload = MobileWebSourceControlBranchesPayloadSchema.parse(args.payload) @@ -70,7 +71,12 @@ export async function executeMobileWebSourceControlHistoryOperation(args: { if (!args.branchComparePager) { throw new MobileWebBrokerError('internal') } - return args.branchComparePager.page(args.payload, args.client, args.workspaceAuthority) + return args.branchComparePager.page( + args.payload, + args.client, + args.workspaceAuthority, + args.requestId + ) } const payload = MobileWebSourceControlCommitComparePayloadSchema.parse(args.payload) diff --git a/mobile/src/mobile-web/mobile-web-source-control-operations.ts b/mobile/src/mobile-web/mobile-web-source-control-operations.ts index 1f9bc797f79..5bdd7abe482 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-operations.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-operations.ts @@ -61,6 +61,7 @@ export async function executeMobileWebSourceControlOperation(args: { client: RpcClient workspaceAuthority: MobileWebWorkspaceAuthority branchComparePager?: MobileWebSourceControlBranchComparePager + requestId?: string terminalClientId?: string }): Promise< | MobileWebSourceControlStatusResult @@ -96,7 +97,8 @@ export async function executeMobileWebSourceControlOperation(args: { payload: args.payload, client: args.client, workspaceAuthority: args.workspaceAuthority, - branchComparePager: args.branchComparePager + branchComparePager: args.branchComparePager, + requestId: args.requestId }) } if (args.operation === 'status') { diff --git a/mobile/src/mobile-web/use-mobile-web-alert-safe-package-session.ts b/mobile/src/mobile-web/use-mobile-web-alert-safe-package-session.ts new file mode 100644 index 00000000000..56417daf5a2 --- /dev/null +++ b/mobile/src/mobile-web/use-mobile-web-alert-safe-package-session.ts @@ -0,0 +1,11 @@ +import { mobileWebNativeAlertLifecycle } from './mobile-web-native-alert' +import { useMobileWebPackageSession } from './use-mobile-web-package-session' + +export function useMobileWebAlertSafePackageSession( + args: Omit[0], 'beforeSessionReplacement'> +): ReturnType { + return useMobileWebPackageSession({ + ...args, + beforeSessionReplacement: mobileWebNativeAlertLifecycle.waitForIdle + }) +} diff --git a/mobile/src/mobile-web/use-mobile-web-package-session.test.ts b/mobile/src/mobile-web/use-mobile-web-package-session.test.ts index eb6f626b1e3..0a9d20d82f6 100644 --- a/mobile/src/mobile-web/use-mobile-web-package-session.test.ts +++ b/mobile/src/mobile-web/use-mobile-web-package-session.test.ts @@ -56,10 +56,12 @@ const SESSION_B = { describe('useMobileWebPackageSession', () => { let renderer: ReactTestRenderer | null = null let packageSession: MobileWebPackageSession | null = null + let beforeSessionReplacement: (() => Promise) | undefined beforeEach(() => { globalThis.IS_REACT_ACT_ENVIRONMENT = true packageSession = null + beforeSessionReplacement = undefined native.openSession.mockReset() native.recoverSession.mockReset() native.markSessionHealthy.mockReset().mockResolvedValue({ buildId: SESSION_A.buildId }) @@ -78,11 +80,18 @@ describe('useMobileWebPackageSession', () => { renderer = null }) - function Harness({ state }: { state: ConnectionState }): null { + function Harness({ + state, + host = HOST + }: { + state: ConnectionState + host?: HostProfile | null + }): null { packageSession = useMobileWebPackageSession({ client: state === 'connected' ? CLIENT : null, - host: HOST, - state + host: host ?? undefined, + state, + beforeSessionReplacement }) return null } @@ -313,6 +322,59 @@ describe('useMobileWebPackageSession', () => { expect(packageSession?.packageLoading).toBe(false) }) + it('keeps the current session published until replacement is safe', async () => { + const safe = deferred() + beforeSessionReplacement = () => safe.promise + native.openSession.mockImplementation((_host: string, buildId: string | null) => + Promise.resolve(buildId ? SESSION_B : SESSION_A) + ) + downloadPackage.mockResolvedValue({ commit: { buildId: SESSION_B.buildId } }) + + await mount('connected') + + expect(packageSession?.session).toEqual(SESSION_A) + expect(native.openSession).toHaveBeenCalledWith( + HOST.publicKeyB64, + SESSION_B.buildId, + MOBILE_WEB_BRIDGE_PROTOCOL_VERSION + ) + expect(native.closeSession).not.toHaveBeenCalledWith(SESSION_A.sessionId) + + await act(async () => { + safe.resolve() + await safe.promise + await flushPromises() + }) + + expect(packageSession?.session).toEqual(SESSION_B) + expect(native.closeSession).toHaveBeenCalledWith(SESSION_A.sessionId) + }) + + it('rejects a replacement whose host becomes stale while activation waits', async () => { + const safe = deferred() + beforeSessionReplacement = () => safe.promise + native.openSession.mockImplementation((_host: string, buildId: string | null) => + Promise.resolve(buildId ? SESSION_B : SESSION_A) + ) + downloadPackage.mockResolvedValue({ commit: { buildId: SESSION_B.buildId } }) + + await mount('connected') + expect(packageSession?.session).toEqual(SESSION_A) + + await act(async () => { + renderer?.update(createElement(Harness, { state: 'connected', host: null })) + await flushPromises() + }) + await act(async () => { + safe.resolve() + await safe.promise + await flushPromises() + }) + + expect(packageSession?.session).toBeNull() + expect(native.closeSession).toHaveBeenCalledWith(SESSION_B.sessionId) + }) + it('keeps the loading state while a first desktop refresh is active', async () => { const refresh = deferred<{ commit: { buildId: string } }>() native.openSession.mockRejectedValue(new Error('cache unavailable')) diff --git a/mobile/src/mobile-web/use-mobile-web-package-session.ts b/mobile/src/mobile-web/use-mobile-web-package-session.ts index 22b6c609aa5..e89a3ac001c 100644 --- a/mobile/src/mobile-web/use-mobile-web-package-session.ts +++ b/mobile/src/mobile-web/use-mobile-web-package-session.ts @@ -17,32 +17,23 @@ import { import { mobileWebPackageRefreshWarning } from './mobile-web-package-refresh-warning' import { useMobileWebPackageCapability } from './use-mobile-web-package-capability' import { useMobileWebPackageRecovery } from './use-mobile-web-package-recovery' +import type { MobileWebPackageSession } from './mobile-web-package-session-state' + +export type { MobileWebPackageSession } from './mobile-web-package-session-state' const MOBILE_WEB_PACKAGE_UPDATE_REQUIRED_WARNING = 'Update Orca on this desktop to use its workspace interface.' -export type MobileWebPackageSession = { - session: MobileWebShellSession | null - viewEpoch: number - packageLoading: boolean - packageWarning: string | undefined - markHealthy: (sessionId: string) => Promise - handleHealthTimeout: (sessionId: string) => Promise - handleProcessTerminated: (sessionId: string) => Promise - retryPackage: () => void - recoverPrevious: () => Promise - clearCache: () => Promise - showWarning: (warning: string) => void -} - export function useMobileWebPackageSession({ client, host, - state + state, + beforeSessionReplacement }: { client: RpcClient | null host: HostProfile | undefined state: ConnectionState + beforeSessionReplacement?: () => Promise }): MobileWebPackageSession { const hostEpochRef = useRef(0) const activeHostIdRef = useRef(null) @@ -79,6 +70,13 @@ export function useMobileWebPackageSession({ return false } const previous = ownedSessionRef.current + if (previous && previous.sessionId !== next.sessionId) { + await beforeSessionReplacement?.() + if (hostEpochRef.current !== hostEpoch) { + await ExpoMobileWebShell.closeSession(next.sessionId).catch(() => {}) + return false + } + } ownedSessionRef.current = next setSession(next) setViewEpoch(0) @@ -94,7 +92,7 @@ export function useMobileWebPackageSession({ } return true }, - [] + [beforeSessionReplacement] ) useEffect(() => { diff --git a/mobile/src/transport/mobile-relay-hosted-bridge.integration.test.ts b/mobile/src/transport/mobile-relay-hosted-bridge.integration.test.ts index 291a8f14642..a5eed705f84 100644 --- a/mobile/src/transport/mobile-relay-hosted-bridge.integration.test.ts +++ b/mobile/src/transport/mobile-relay-hosted-bridge.integration.test.ts @@ -6,10 +6,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import nacl from 'tweetnacl' import WebSocketClient, { WebSocketServer, type RawData, type WebSocket } from 'ws' import { connectMobileRelayRpcSession } from './mobile-relay-rpc-session' -import { - MOBILE_WEB_PRODUCTION_GRANTS, - MobileWebCapabilityBroker -} from '../mobile-web/mobile-web-capability-broker' +import { MobileWebCapabilityBroker } from '../mobile-web/mobile-web-capability-broker' +import { MOBILE_WEB_PRODUCTION_GRANTS } from '../mobile-web/mobile-web-production-grants' import { MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, parseMobileWebBridgePageMessage,