From effd8ec5de9a70e79d1543015fbd0784b337a112 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 15:03:07 -0400 Subject: [PATCH] refactor(mobile): collapse the broker replay guard into one window Two classes and two Sets tracked the same thing: a page-minted id the broker has already honoured. Request and subscription ids share one window now. It stays a module rather than a broker field because the broker sits at its max-lines ceiling. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-web-broker-replay-guard.ts | 35 ------------------- .../mobile-web-broker-replay-window.test.ts | 34 ++++++++++++++++++ .../mobile-web-broker-replay-window.ts | 31 ++++++++++++++++ .../mobile-web-capability-broker.ts | 11 +++--- .../mobile-web-message-replay-window.test.ts | 33 ----------------- .../mobile-web-message-replay-window.ts | 31 ---------------- 6 files changed, 69 insertions(+), 106 deletions(-) delete mode 100644 mobile/src/mobile-web/mobile-web-broker-replay-guard.ts create mode 100644 mobile/src/mobile-web/mobile-web-broker-replay-window.test.ts create mode 100644 mobile/src/mobile-web/mobile-web-broker-replay-window.ts delete mode 100644 mobile/src/mobile-web/mobile-web-message-replay-window.test.ts delete mode 100644 mobile/src/mobile-web/mobile-web-message-replay-window.ts diff --git a/mobile/src/mobile-web/mobile-web-broker-replay-guard.ts b/mobile/src/mobile-web/mobile-web-broker-replay-guard.ts deleted file mode 100644 index 4199d99d372..00000000000 --- a/mobile/src/mobile-web/mobile-web-broker-replay-guard.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { - MOBILE_WEB_BRIDGE_MAX_PENDING_REQUESTS, - MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS -} from '../../../src/shared/mobile-web/bridge-contract' -import { MobileWebMessageReplayWindow } from './mobile-web-message-replay-window' - -export class MobileWebBrokerReplayGuard { - private readonly requests = new MobileWebMessageReplayWindow( - MOBILE_WEB_BRIDGE_MAX_PENDING_REQUESTS * 4 - ) - private readonly subscriptions = new MobileWebMessageReplayWindow( - MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS * 4 - ) - - acceptRequest(id: string, active: boolean): boolean { - if (active || this.requests.has(id)) { - return false - } - this.requests.remember(id) - return true - } - - acceptSubscription(id: string): boolean { - if (this.subscriptions.has(id)) { - return false - } - this.subscriptions.remember(id) - return true - } - - clear(): void { - this.requests.clear() - this.subscriptions.clear() - } -} diff --git a/mobile/src/mobile-web/mobile-web-broker-replay-window.test.ts b/mobile/src/mobile-web/mobile-web-broker-replay-window.test.ts new file mode 100644 index 00000000000..8ef134fe90c --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-broker-replay-window.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest' +import { MobileWebBrokerReplayWindow } from './mobile-web-broker-replay-window' + +describe('mobile web broker replay window', () => { + it('honours an id once and shares one window across request and subscription ids', () => { + const replay = new MobileWebBrokerReplayWindow() + + expect(replay.accept('request-1')).toBe(true) + expect(replay.accept('request-1')).toBe(false) + expect(replay.accept('subscription-1')).toBe(true) + expect(replay.accept('subscription-1')).toBe(false) + }) + + it('evicts the oldest id rather than exhausting a long-lived session', () => { + const replay = new MobileWebBrokerReplayWindow() + const ids = Array.from({ length: 5_000 }, (_, index) => `id-${index}`) + + for (const id of ids) { + expect(replay.accept(id)).toBe(true) + } + + expect(replay.accept(ids[0]!)).toBe(true) + expect(replay.accept(ids.at(-1)!)).toBe(false) + }) + + it('clears all authority on disposal', () => { + const replay = new MobileWebBrokerReplayWindow() + + replay.accept('request-1') + replay.clear() + + expect(replay.accept('request-1')).toBe(true) + }) +}) diff --git a/mobile/src/mobile-web/mobile-web-broker-replay-window.ts b/mobile/src/mobile-web/mobile-web-broker-replay-window.ts new file mode 100644 index 00000000000..c6f01dc1c25 --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-broker-replay-window.ts @@ -0,0 +1,31 @@ +import { + MOBILE_WEB_BRIDGE_MAX_PENDING_REQUESTS, + MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS +} from '../../../src/shared/mobile-web/bridge-contract' + +// One window covers request and subscription ids alike: both are page-minted and honoured once. +const REPLAY_WINDOW = + (MOBILE_WEB_BRIDGE_MAX_PENDING_REQUESTS + MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS) * 4 + +export class MobileWebBrokerReplayWindow { + private readonly ids = new Set() + + /** An id evicted from the window while still in flight is caught by the broker's pending map. */ + accept(id: string): boolean { + if (this.ids.has(id)) { + return false + } + this.ids.add(id) + if (this.ids.size > REPLAY_WINDOW) { + const oldest = this.ids.values().next().value + if (oldest !== undefined) { + this.ids.delete(oldest) + } + } + return true + } + + clear(): void { + this.ids.clear() + } +} diff --git a/mobile/src/mobile-web/mobile-web-capability-broker.ts b/mobile/src/mobile-web/mobile-web-capability-broker.ts index 605d3a9bba7..d59df52d709 100644 --- a/mobile/src/mobile-web/mobile-web-capability-broker.ts +++ b/mobile/src/mobile-web/mobile-web-capability-broker.ts @@ -20,7 +20,7 @@ import { executeMobileWebCapabilityRequest } from './mobile-web-capability-execu import { MobileWebCapabilityAuthorities } from './mobile-web-capability-authorities' import type { MobileWebCapabilityBrokerOptions } from './mobile-web-capability-broker-options' import { MobileWebBrokerMessageSender } from './mobile-web-broker-message-sender' -import { MobileWebBrokerReplayGuard } from './mobile-web-broker-replay-guard' +import { MobileWebBrokerReplayWindow } from './mobile-web-broker-replay-window' import { rememberMobileWebBrokerRoute } from './mobile-web-broker-route-memory' import { resolveMobileWebHostNavigationRoute } from './mobile-web-host-navigation-route' import { @@ -40,7 +40,7 @@ type PendingRequest = { operationKey: string; subscriptionId?: string; cancelled export class MobileWebCapabilityBroker { private readonly pending = new Map() - private readonly replay = new MobileWebBrokerReplayGuard() + private readonly replay = new MobileWebBrokerReplayWindow() private readonly subscriptions: MobileWebCapabilitySubscriptions private readonly terminalStreams: MobileWebTerminalStreams private readonly speechAuthority: MobileWebSpeechAuthority @@ -146,7 +146,7 @@ export class MobileWebCapabilityBroker { ) } private async handleRequest(request: PageRequest): Promise { - if (!this.replay.acceptRequest(request.requestId, this.pending.has(request.requestId))) { + if (this.pending.has(request.requestId) || !this.replay.accept(request.requestId)) { await this.messages.error(request.requestId, 'invalid_request', false) return } @@ -158,10 +158,7 @@ export class MobileWebCapabilityBroker { await this.messages.error(request.requestId, 'unsupported_capability', false) return } - if ( - request.mode === 'subscription' && - !this.replay.acceptSubscription(request.subscriptionId) - ) { + if (request.mode === 'subscription' && !this.replay.accept(request.subscriptionId)) { await this.messages.error(request.requestId, 'invalid_request', false) return } diff --git a/mobile/src/mobile-web/mobile-web-message-replay-window.test.ts b/mobile/src/mobile-web/mobile-web-message-replay-window.test.ts deleted file mode 100644 index 924379926f1..00000000000 --- a/mobile/src/mobile-web/mobile-web-message-replay-window.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { MobileWebMessageReplayWindow } from './mobile-web-message-replay-window' - -describe('mobile web message replay window', () => { - it('rejects recent IDs without permanently exhausting a long-lived session', () => { - const replay = new MobileWebMessageReplayWindow(3) - - replay.remember('a') - replay.remember('b') - replay.remember('c') - expect(replay.has('a')).toBe(true) - - replay.remember('d') - expect(replay.has('a')).toBe(false) - expect(replay.has('b')).toBe(true) - expect(replay.has('d')).toBe(true) - }) - - it('refreshes an existing ID and clears all authority on disposal', () => { - const replay = new MobileWebMessageReplayWindow(2) - - replay.remember('a') - replay.remember('b') - replay.remember('a') - replay.remember('c') - expect(replay.has('a')).toBe(true) - expect(replay.has('b')).toBe(false) - - replay.clear() - expect(replay.has('a')).toBe(false) - expect(replay.has('c')).toBe(false) - }) -}) diff --git a/mobile/src/mobile-web/mobile-web-message-replay-window.ts b/mobile/src/mobile-web/mobile-web-message-replay-window.ts deleted file mode 100644 index 89a4287859e..00000000000 --- a/mobile/src/mobile-web/mobile-web-message-replay-window.ts +++ /dev/null @@ -1,31 +0,0 @@ -export class MobileWebMessageReplayWindow { - private readonly ids = new Set() - - constructor(private readonly limit: number) { - if (!Number.isInteger(limit) || limit < 1) { - throw new Error('mobile_web_replay_window_invalid') - } - } - - has(id: string): boolean { - return this.ids.has(id) - } - - remember(id: string): void { - if (this.ids.delete(id)) { - this.ids.add(id) - return - } - this.ids.add(id) - if (this.ids.size > this.limit) { - const oldest = this.ids.values().next().value - if (oldest !== undefined) { - this.ids.delete(oldest) - } - } - } - - clear(): void { - this.ids.clear() - } -}