diff --git a/src/main/browser/browser-client-host-command-page.ts b/src/main/browser/browser-client-host-command-page.ts index 79c6a045097..58439e07ee5 100644 --- a/src/main/browser/browser-client-host-command-page.ts +++ b/src/main/browser/browser-client-host-command-page.ts @@ -34,6 +34,16 @@ export function selectCommandPage( } } if (command.pageHostGeneration < page.generation) { + const previous = page.records.get(command.commandSequence) + if ( + command.command.type === 'switchProfile' && + previous?.result?.status === 'completed' && + previous.event.pageHostGeneration === command.pageHostGeneration && + previous.event.commandId === command.commandId && + commandsMatch(previous.event, command) + ) { + return { page, commit: () => {} } + } throw new Error('browser_host_page_generation_stale') } if (command.pageHostGeneration === page.generation) { @@ -88,6 +98,12 @@ function commandsMatch( if (first.command.type !== second.command.type) { return false } + if (first.command.type === 'switchProfile' && second.command.type === 'switchProfile') { + return ( + first.command.nextPageHostGeneration === second.command.nextPageHostGeneration && + JSON.stringify(first.command.profile) === JSON.stringify(second.command.profile) + ) + } if (first.command.type === 'navigate' && second.command.type === 'navigate') { return first.command.url === second.command.url } @@ -121,6 +137,13 @@ export function assertNewPageCommand( page: PageState, command: BrowserClientHostCommandEvent ): void { + const active = [...page.records.values()].filter((record) => record.status !== 'settled') + if ( + active.some((record) => record.event.command.type === 'switchProfile') || + (command.command.type === 'switchProfile' && active.length > 0) + ) { + throw new Error('browser_host_profile_switch_pending') + } if (page.nextSequence === 1 && command.command.type === 'navigate') { throw new Error('browser_host_command_create_required') } @@ -169,6 +192,9 @@ export function recordBrowserClientPageCommandResult( command: BrowserClientHostCommandEvent['command'], result: BrowserClientHostCommandResult ): void { + if (command.type === 'switchProfile' && result.status === 'completed') { + page.generation = command.nextPageHostGeneration + } if (isBrowserClientPageBootstrapCommand(command)) { page.created = result.status === 'completed' page.createFailed = !page.created diff --git a/src/main/browser/browser-client-host-command-state.ts b/src/main/browser/browser-client-host-command-state.ts index a57b2054696..a1927264d36 100644 --- a/src/main/browser/browser-client-host-command-state.ts +++ b/src/main/browser/browser-client-host-command-state.ts @@ -124,6 +124,9 @@ export function createPageState(browserPageId: string, generation: number): Page function snapshotPageCommand( command: BrowserClientHostCommandEvent['command'] ): BrowserClientHostCommandEvent['command'] { + if (command.type === 'switchProfile') { + return Object.freeze({ ...command, profile: Object.freeze({ ...command.profile }) }) + } if (command.type === 'reclaimPage') { return Object.freeze({ ...command, diff --git a/src/main/browser/browser-client-network-route-registry.ts b/src/main/browser/browser-client-network-route-registry.ts index b0035694ac2..d7946e58ddd 100644 --- a/src/main/browser/browser-client-network-route-registry.ts +++ b/src/main/browser/browser-client-network-route-registry.ts @@ -1,7 +1,5 @@ -import type { - BrowserHostLeaseAuthority, - BrowserNetworkExecutionHost -} from '../../shared/browser-client-host-protocol' +import type { BrowserHostLeaseAuthority } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import type { BrowserClientPageNetworkRoute } from './browser-client-page-cleanup' import { assertBrowserClientNetworkRouteAddress, diff --git a/src/main/browser/browser-client-page-automation-runtime.test.ts b/src/main/browser/browser-client-page-automation-runtime.test.ts index 1f0b7f3f73c..aba8a28ff8a 100644 --- a/src/main/browser/browser-client-page-automation-runtime.test.ts +++ b/src/main/browser/browser-client-page-automation-runtime.test.ts @@ -68,6 +68,22 @@ describe('BrowserClientPageAutomationRuntime', () => { expect(fixture.unregisterGuest).toHaveBeenCalledWith('page-a') }) + it('retires an old automation session without stripping the guest needed by rollback', async () => { + const fixture = createFixture() + await fixture.runtime.execute(input(), new AbortController().signal) + await fixture.runtime.retire({ + browserPageId: 'page-a', + pageHostGeneration: 9, + registration, + keepGuestBinding: true + }) + expect(fixture.onTabClosed).toHaveBeenCalledWith(42) + expect(fixture.unregisterGuest).not.toHaveBeenCalled() + await expect(fixture.runtime.execute(input(), new AbortController().signal)).resolves.toEqual({ + clicked: true + }) + }) + it('rejects aborts and registration failures before RPC dispatch', async () => { const fixture = createFixture() const controller = new AbortController() diff --git a/src/main/browser/browser-client-page-automation-runtime.ts b/src/main/browser/browser-client-page-automation-runtime.ts index 3dd0943e525..827e0dc8537 100644 --- a/src/main/browser/browser-client-page-automation-runtime.ts +++ b/src/main/browser/browser-client-page-automation-runtime.ts @@ -16,7 +16,7 @@ type AutomationPageInput = { type AutomationPageRetirement = Pick< AutomationPageInput, 'browserPageId' | 'pageHostGeneration' | 'registration' -> +> & { keepGuestBinding?: boolean } type AutomationRuntimeDependencies = { browserManager: Pick< @@ -68,8 +68,9 @@ export class BrowserClientPageAutomationRuntime { await bridge.onTabClosed(current.webContentsId) } if ( + !input.keepGuestBinding && this.dependencies.browserManager.getGuestWebContentsId(input.browserPageId) === - current.webContentsId + current.webContentsId ) { this.dependencies.browserManager.unregisterGuest(input.browserPageId) } diff --git a/src/main/browser/browser-client-page-availability.ts b/src/main/browser/browser-client-page-availability.ts index 7a4a18510bd..f6f8140bdde 100644 --- a/src/main/browser/browser-client-page-availability.ts +++ b/src/main/browser/browser-client-page-availability.ts @@ -1,3 +1,4 @@ +import type { BrowserClientPageCommandExecutorDependencies } from './browser-client-page-command-executor-dependencies' import type { BrowserRoutePageGuestIdentity } from './browser-route-page-authority' import type { BrowserClientRetainedPage } from './browser-client-page-retained-state' @@ -22,3 +23,23 @@ export function markBrowserClientPageUnavailable( onUnavailable(registration.browserPageId, page.generation) return true } + +export function publishBrowserClientRetainedPage( + dependencies: BrowserClientPageCommandExecutorDependencies, + pages: Map, + page: BrowserClientRetainedPage +): void { + const previous = pages.get(page.inventory.browserPageId) + previous?.releaseAvailabilityWatch?.() + if (previous) { + previous.releaseAvailabilityWatch = undefined + } + page.releaseAvailabilityWatch = dependencies.routeWebContents.watchPageAvailability?.( + page.inventory.browserPageId, + (registration) => + markBrowserClientPageUnavailable(pages, registration, (browserPageId, generation) => + dependencies.onPageUnavailable?.(browserPageId, generation) + ) + ) + pages.set(page.inventory.browserPageId, page) +} diff --git a/src/main/browser/browser-client-page-command-executor-dependencies.ts b/src/main/browser/browser-client-page-command-executor-dependencies.ts index 4e647bc93da..227de48630a 100644 --- a/src/main/browser/browser-client-page-command-executor-dependencies.ts +++ b/src/main/browser/browser-client-page-command-executor-dependencies.ts @@ -47,6 +47,7 @@ export type BrowserClientPageCommandExecutorDependencies = BrowserClientPageAuth signal: AbortSignal ): Promise retireAutomation(input: { + keepGuestBinding?: boolean browserPageId: string pageHostGeneration: number registration: BrowserClientRetainedPage['registration'] diff --git a/src/main/browser/browser-client-page-command-executor-test-harness.ts b/src/main/browser/browser-client-page-command-executor-test-harness.ts index 56f9cd543e9..8e54733ef58 100644 --- a/src/main/browser/browser-client-page-command-executor-test-harness.ts +++ b/src/main/browser/browser-client-page-command-executor-test-harness.ts @@ -1,3 +1,4 @@ +import type { BrowserRoutePreparePageInput } from './browser-route-session-state' import type { BrowserClientHostCommandEvent } from '../../shared/browser-client-host-protocol' import { vi } from 'vitest' import { BrowserClientPageCommandExecutor } from './browser-client-page-command-executor' @@ -89,7 +90,7 @@ export function createHarness(options: { maxPages?: number } = {}) { }), selectRenderer: vi.fn(() => renderer), routeSessions: { - preparePage: vi.fn(async () => { + preparePage: vi.fn(async (_input: BrowserRoutePreparePageInput) => { order.push('prepare-page') return routeSession }) @@ -100,7 +101,7 @@ export function createHarness(options: { maxPages?: number } = {}) { bind: vi.fn(() => { order.push('bind-guest') }), - release: vi.fn(() => { + release: vi.fn((_registration: BrowserRoutePageGuestIdentity) => { order.push('release-guest') }) }, diff --git a/src/main/browser/browser-client-page-command-executor.ts b/src/main/browser/browser-client-page-command-executor.ts index ba6aad9703b..2d7c10940fb 100644 --- a/src/main/browser/browser-client-page-command-executor.ts +++ b/src/main/browser/browser-client-page-command-executor.ts @@ -1,3 +1,8 @@ +import { closeBrowserClientExecutorPages } from './browser-client-page-executor-close' +import { + BrowserClientProfileSwitches, + switchBrowserClientPageProfile +} from './browser-client-page-profile-switch' import { BROWSER_CLIENT_HOST_PAGE_INVENTORY_MAX_PAGES, type BrowserClientHostedPageInventory, @@ -20,7 +25,7 @@ import { BrowserClientPageNavigationFence } from './browser-client-page-navigati import { executeBrowserClientPageReconciliationCommand } from './browser-client-page-reconciliation' import { findBrowserClientPageByWebContentsId, - forgetBrowserClientRetainedPage, + retireBrowserClientRetainedPage, type BrowserClientRetainedPage } from './browser-client-page-retained-state' import { @@ -32,7 +37,7 @@ import { executeBrowserClientPageAutomationCommand, navigateBrowserClientPageCommand } from './browser-client-page-command-execution' -import { markBrowserClientPageUnavailable } from './browser-client-page-availability' +import { publishBrowserClientRetainedPage } from './browser-client-page-availability' import type { BrowserClientPageAuthorityIdentity, BrowserClientPageCommandExecutorDependencies @@ -43,6 +48,7 @@ export class BrowserClientPageCommandExecutor { private readonly pages = new Map() private readonly creatingPages = new Map() private readonly failedPages = new Map() + private readonly profileSwitches = new BrowserClientProfileSwitches() private readonly navigationFence = new BrowserClientPageNavigationFence() private closePromise: Promise | null = null private authorityConnectionIdentity: string @@ -68,13 +74,19 @@ export class BrowserClientPageCommandExecutor { signal: AbortSignal ): Promise { if (this.closed || this.authorityTransitioning || this.navigationFence.isFenced) { - return { status: 'failed', errorCode: 'browser_client_page_executor_closed' } + return { + status: 'failed', + errorCode: 'browser_client_page_executor_closed' + } } try { const value = await this.executeCommand(event, signal) return value === undefined ? { status: 'completed' } : { status: 'completed', value } } catch (error) { - return { status: 'failed', errorCode: browserClientPageCommandFailureCode(error, signal) } + return { + status: 'failed', + errorCode: browserClientPageCommandFailureCode(error, signal) + } } } @@ -83,6 +95,22 @@ export class BrowserClientPageCommandExecutor { signal: AbortSignal ): Promise { switch (event.command.type) { + case 'switchProfile': + return switchBrowserClientPageProfile({ + event, + signal, + switches: this.profileSwitches, + pages: this.pages, + dependencies: { + ...this.dependencies, + authorityConnectionIdentity: this.authorityConnectionIdentity, + legacyAuthorityConnectionIdentity: this.legacyAuthorityConnectionIdentity + }, + assertAvailable: () => + this.navigationFence.assertAvailable(this.closed || this.authorityTransitioning), + commit: (page) => publishBrowserClientRetainedPage(this.dependencies, this.pages, page), + cleanup: (page, releaseUploads) => this.cleanupPage(page, undefined, releaseUploads) + }) case 'createPage': return this.createPage(event, signal) case 'navigate': @@ -157,14 +185,14 @@ export class BrowserClientPageCommandExecutor { close(): Promise { this.closed = true return (this.closePromise ??= this.navigationFence.fenceBeforeCleanup( - this.pages.values(), + this.profileSwitches.withStagedPages(this.pages.values()), (claim) => this.dependencies.routeWebContents.revokeNavigation(claim), () => this.closePages() )) } fenceNavigation(): void { - this.navigationFence.fence(this.pages.values(), (claim) => + this.navigationFence.fence(this.profileSwitches.withStagedPages(this.pages.values()), (claim) => this.dependencies.routeWebContents.revokeNavigation(claim) ) } @@ -174,8 +202,9 @@ export class BrowserClientPageCommandExecutor { throw new Error('browser_client_page_authority_transition_unavailable') } this.authorityTransitioning = true - this.navigationFence.revoke(this.pages.values(), (claim) => - this.dependencies.routeWebContents.revokeNavigation(claim) + this.navigationFence.revoke( + this.profileSwitches.withStagedPages(this.pages.values()), + (claim) => this.dependencies.routeWebContents.revokeNavigation(claim) ) } @@ -188,24 +217,14 @@ export class BrowserClientPageCommandExecutor { this.authorityTransitioning = false } - async retirePage(browserPageId: string, pageHostGeneration: number): Promise { - const page = this.pages.get(browserPageId) - if (!page || page.generation !== pageHostGeneration) { - return false - } - page.retiring ??= this.cleanupPage(page) - try { - await page.retiring - } catch (error) { - forgetBrowserClientRetainedPage(this.pages, browserPageId, page) - this.failedPages.set( - browserPageId, - Object.freeze({ ...page.inventory, state: 'outcomeUnknown' }) - ) - throw error - } - forgetBrowserClientRetainedPage(this.pages, browserPageId, page) - return true + retirePage(browserPageId: string, pageHostGeneration: number): Promise { + return retireBrowserClientRetainedPage( + this.pages, + this.failedPages, + browserPageId, + pageHostGeneration, + (page) => this.cleanupPage(page) + ) } private async createPage( @@ -252,29 +271,24 @@ export class BrowserClientPageCommandExecutor { signal, () => this.navigationFence.assertAvailable(this.closed || this.authorityTransitioning), (page) => { - page.releaseAvailabilityWatch = this.dependencies.routeWebContents.watchPageAvailability?.( - event.browserPageId, - (registration) => - markBrowserClientPageUnavailable( - this.pages, - registration, - (browserPageId, generation) => - this.dependencies.onPageUnavailable?.(browserPageId, generation) - ) - ) - this.pages.set(event.browserPageId, page) + publishBrowserClientRetainedPage(this.dependencies, this.pages, page) } ) } // Why: the download relay only knows the guest WebContents, and staging is keyed by page identity. findPageByWebContentsId(webContentsId: number): BrowserClientHostedPageInventory | undefined { - return findBrowserClientPageByWebContentsId(this.pages.values(), webContentsId)?.inventory + // A staged profile-switch guest already owns its downloads while it loads the page. + return findBrowserClientPageByWebContentsId( + this.profileSwitches.withStagedPages(this.pages.values()), + webContentsId + )?.inventory } private async cleanupPage( page: BrowserClientRetainedPage, - previousRendererPage?: BrowserClientPageRendererIdentity + previousRendererPage?: BrowserClientPageRendererIdentity, + releaseUploads = true ): Promise { page.releaseAvailabilityWatch?.() page.releaseAvailabilityWatch = undefined @@ -285,34 +299,23 @@ export class BrowserClientPageCommandExecutor { routeWebContents: this.dependencies.routeWebContents, retireAutomation: this.dependencies.retireAutomation, releaseUploadStaging: async () => { - await this.dependencies.uploadStaging?.releasePage(page.inventory.browserPageId) + if (releaseUploads) { + await this.dependencies.uploadStaging?.releasePage(page.inventory.browserPageId) + } } }, previousRendererPage ) } - private async closePages(): Promise { - const failures: unknown[] = [ - ...(this.creatingPages.size > 0 - ? [new Error('browser_client_page_creation_still_running')] - : []), - ...(this.failedPages.size > 0 ? [new Error('browser_client_page_cleanup_unresolved')] : []) - ] - for (const [browserPageId, page] of this.pages) { - try { - page.retiring ??= this.cleanupPage(page) - await page.retiring - forgetBrowserClientRetainedPage(this.pages, browserPageId, page) - } catch (error) { - failures.push(error) - } - } - await this.dependencies.uploadStaging?.releaseAll().catch((error: unknown) => { - failures.push(error) + private closePages(): Promise { + return closeBrowserClientExecutorPages({ + profileSwitches: this.profileSwitches, + creatingPages: this.creatingPages, + failedPages: this.failedPages, + pages: this.pages, + cleanupPage: (page) => this.cleanupPage(page), + uploadStaging: this.dependencies.uploadStaging }) - if (failures.length > 0) { - throw new AggregateError(failures, 'Browser client page executor cleanup failed') - } } } diff --git a/src/main/browser/browser-client-page-creation.ts b/src/main/browser/browser-client-page-creation.ts index a4015ff10fa..38f0d06d49b 100644 --- a/src/main/browser/browser-client-page-creation.ts +++ b/src/main/browser/browser-client-page-creation.ts @@ -67,6 +67,7 @@ export async function createReservedBrowserClientPage( renderer = dependencies.selectRenderer() assertCurrentBrowserClientPageRenderer(renderer) routeSession = await dependencies.routeSessions.preparePage({ + ...(event.command.profile ? { profileMetadata: event.command.profile } : {}), identity: { orcaProfileId: dependencies.orcaProfileId, browserProfileId: event.command.browserProfileId, diff --git a/src/main/browser/browser-client-page-executor-close.ts b/src/main/browser/browser-client-page-executor-close.ts new file mode 100644 index 00000000000..9176ecf846b --- /dev/null +++ b/src/main/browser/browser-client-page-executor-close.ts @@ -0,0 +1,37 @@ +import type { BrowserClientRetainedPage } from './browser-client-page-retained-state' +import type { BrowserClientUploadStaging } from './browser-client-upload-staging' +import { forgetBrowserClientRetainedPage } from './browser-client-page-retained-state' +import type { BrowserClientProfileSwitches } from './browser-client-page-profile-switch' + +export async function closeBrowserClientExecutorPages(input: { + profileSwitches: Pick + creatingPages: ReadonlyMap + failedPages: ReadonlyMap + pages: Map + cleanupPage(page: BrowserClientRetainedPage): Promise + uploadStaging?: BrowserClientUploadStaging +}): Promise { + const retired = await Promise.allSettled(input.profileSwitches.retainedCleanups) + const failures: unknown[] = [ + ...retired.filter((result) => result.status === 'rejected').map((result) => result.reason), + ...(input.creatingPages.size > 0 || input.profileSwitches.stagedPages.size > 0 + ? [new Error('browser_client_page_creation_still_running')] + : []), + ...(input.failedPages.size > 0 ? [new Error('browser_client_page_cleanup_unresolved')] : []) + ] + for (const [browserPageId, page] of input.pages) { + try { + page.retiring ??= input.cleanupPage(page) + await page.retiring + forgetBrowserClientRetainedPage(input.pages, browserPageId, page) + } catch (error) { + failures.push(error) + } + } + await input.uploadStaging?.releaseAll().catch((error: unknown) => { + failures.push(error) + }) + if (failures.length > 0) { + throw new AggregateError(failures, 'Browser client page executor cleanup failed') + } +} diff --git a/src/main/browser/browser-client-page-guest-binding.ts b/src/main/browser/browser-client-page-guest-binding.ts index ce687bce63e..ab710993fd9 100644 --- a/src/main/browser/browser-client-page-guest-binding.ts +++ b/src/main/browser/browser-client-page-guest-binding.ts @@ -13,6 +13,7 @@ export type BrowserClientPageGuestBindingInput = { * the renderer at all. */ export type BrowserClientPageGuestBinding = { + bindReplacement?(input: BrowserClientPageGuestBindingInput): void bind(input: BrowserClientPageGuestBindingInput): void release(registration: BrowserRoutePageGuestIdentity): void } @@ -24,6 +25,24 @@ export function createBrowserClientPageGuestBinding( > ): BrowserClientPageGuestBinding { return { + bindReplacement: ({ registration, browserProfileId }) => { + const bound = browserManager.registerGuest( + { + browserPageId: registration.browserPageId, + sessionProfileId: browserProfileId, + webContentsId: registration.webContentsId, + rendererWebContentsId: registration.rendererWebContentsId + }, + { retainPreviousGuestPolicies: true } + ) + if ( + !bound || + browserManager.getGuestWebContentsId(registration.browserPageId) !== + registration.webContentsId + ) { + throw new Error('browser_client_page_guest_registration_failed') + } + }, bind: ({ registration, browserProfileId }) => { if ( browserManager.getGuestWebContentsId(registration.browserPageId) === diff --git a/src/main/browser/browser-client-page-profile-binding.test.ts b/src/main/browser/browser-client-page-profile-binding.test.ts new file mode 100644 index 00000000000..56f10481b6c --- /dev/null +++ b/src/main/browser/browser-client-page-profile-binding.test.ts @@ -0,0 +1,82 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const browserMocks = vi.hoisted(() => ({ + appGetPathMock: vi.fn(() => '/downloads'), + shellOpenExternalMock: vi.fn(), + browserWindowFromWebContentsMock: vi.fn(), + menuBuildFromTemplateMock: vi.fn(), + guestOffMock: vi.fn(), + guestOnMock: vi.fn(), + guestSetBackgroundThrottlingMock: vi.fn(), + guestSetWindowOpenHandlerMock: vi.fn(), + guestOpenDevToolsMock: vi.fn(), + webContentsFromIdMock: vi.fn(), + screenGetCursorScreenPointMock: vi.fn(() => ({ x: 0, y: 0 })), + openPopupWithOriginBarMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: browserMocks.appGetPathMock }, + BrowserWindow: { fromWebContents: browserMocks.browserWindowFromWebContentsMock }, + clipboard: { writeText: vi.fn() }, + shell: { openExternal: browserMocks.shellOpenExternalMock }, + Menu: { buildFromTemplate: browserMocks.menuBuildFromTemplateMock }, + screen: { getCursorScreenPoint: browserMocks.screenGetCursorScreenPointMock }, + webContents: { fromId: browserMocks.webContentsFromIdMock } +})) + +vi.mock('./popup-origin-bar-window', () => ({ + openPopupWithOriginBar: browserMocks.openPopupWithOriginBarMock +})) + +import { browserManager } from './browser-manager' +import { createBrowserClientPageGuestBinding } from './browser-client-page-guest-binding' +import { resetBrowserManagerMocks, resetBrowserManagerState } from './browser-manager-test-harness' + +const registration = (webContentsId: number, pageHostGeneration: number) => ({ + partition: 'persist:route', + browserPageId: 'page-a', + webContentsId, + pageHostGeneration, + rendererWebContentsId: 5001 +}) + +beforeEach(() => { + resetBrowserManagerMocks(browserMocks) + resetBrowserManagerState() +}) +afterEach(() => resetBrowserManagerState()) + +describe('staged guest binding', () => { + it('retains the old guest policy attachment so rollback can actually rebind it', () => { + const guests = [createGuest(41), createGuest(42)] + browserMocks.webContentsFromIdMock.mockImplementation( + (id: number) => guests.find((guest) => guest.id === id) ?? null + ) + for (const guest of guests) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Only the WebContents methods used by attachGuestPolicies are exercised by this fixture. + browserManager.attachGuestPolicies(guest as never) + } + const binding = createBrowserClientPageGuestBinding(browserManager) + binding.bind({ registration: registration(41, 1), browserProfileId: 'old-profile' }) + binding.bindReplacement!({ registration: registration(42, 2), browserProfileId: 'new-profile' }) + expect(browserManager.getGuestWebContentsId('page-a')).toBe(42) + binding.release(registration(42, 2)) + binding.bind({ registration: registration(41, 1), browserProfileId: 'old-profile' }) + expect(browserManager.getGuestWebContentsId('page-a')).toBe(41) + expect(browserManager.getSessionProfileIdForTab('page-a')).toBe('old-profile') + }) +}) + +function createGuest(id: number) { + return { + id, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: browserMocks.guestSetBackgroundThrottlingMock, + setWindowOpenHandler: browserMocks.guestSetWindowOpenHandlerMock, + on: browserMocks.guestOnMock, + off: browserMocks.guestOffMock, + openDevTools: browserMocks.guestOpenDevToolsMock + } +} diff --git a/src/main/browser/browser-client-page-profile-switch.test.ts b/src/main/browser/browser-client-page-profile-switch.test.ts new file mode 100644 index 00000000000..8417f446d80 --- /dev/null +++ b/src/main/browser/browser-client-page-profile-switch.test.ts @@ -0,0 +1,339 @@ +import { BrowserRoutePageAvailability } from './browser-route-page-availability' +import { describe, expect, it, vi } from 'vitest' +import { + createCommand, + createHarness, + createLifecycleClaim +} from './browser-client-page-command-executor-test-harness' +import type { BrowserClientHostCommandEvent } from '../../shared/browser-client-host-protocol' + +const switchCommand = (): BrowserClientHostCommandEvent => + createCommand('navigate', { + pageReconciliationProtocolVersion: 1, + command: { + type: 'switchProfile', + nextPageHostGeneration: 8, + profile: { + id: 'remote-isolated', + label: 'Remote profile', + scope: 'isolated' + } + } + }) + +async function fixture( + availability?: BrowserRoutePageAvailability, + oldGuestIsCurrent?: () => boolean +) { + const harness = createHarness() + if (oldGuestIsCurrent) { + harness.dependencies.routeWebContents.claimGuestLifecycle.mockImplementationOnce( + (registration) => createLifecycleClaim(registration, Promise.resolve(), oldGuestIsCurrent) + ) + } + if (availability) { + Object.assign(harness.dependencies.routeWebContents, { + watchPageAvailability: availability.watch + }) + } + const bindReplacement = vi.fn() + Object.assign(harness.dependencies.guestBinding, { bindReplacement }) + await harness.executor.handle(createCommand('createPage'), new AbortController().signal) + harness.dependencies.routeSessions.preparePage.mockResolvedValue({ + partition: `persist:orca-browser-v1-${'b'.repeat(64)}`, + release: vi.fn() + }) + harness.renderer.mountPage.mockResolvedValue({ webContentsId: 42 }) + harness.order.length = 0 + return { ...harness, bindReplacement } +} + +describe('staged client page profile switching', () => { + it('prepares a separate partition and guest before retiring the authenticated old guest', async () => { + const h = await fixture() + let retired = false + h.dependencies.routeWebContents.beginGuestRetirement.mockImplementation(() => { + retired = true + expect(h.bindReplacement).toHaveBeenCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ + webContentsId: 42, + pageHostGeneration: 8 + }) + }) + ) + return Promise.resolve() + }) + h.bindReplacement.mockImplementation(() => expect(retired).toBe(false)) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toEqual({ + status: 'completed' + }) + expect(h.executor.hasPage('page-a', 8)).toBe(true) + expect(h.executor.hasPage('page-a', 7)).toBe(false) + expect(h.executor.snapshotPageInventory()).toMatchObject([ + { browserProfileId: 'remote-isolated', pageHostGeneration: 8 } + ]) + expect(h.dependencies.routeSessions.preparePage).toHaveBeenLastCalledWith( + expect.objectContaining({ + identity: expect.objectContaining({ + browserProfileId: 'remote-isolated', + authorityConnectionIdentity: 'authority-record-a' + }), + profileMetadata: { + id: 'remote-isolated', + label: 'Remote profile', + scope: 'isolated' + } + }) + ) + }) + + it('preserves the old guest when target preparation fails', async () => { + const h = await fixture() + h.dependencies.routeSessions.preparePage.mockRejectedValueOnce( + new Error('target profile rejected') + ) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + expect(h.dependencies.routeWebContents.beginGuestRetirement).not.toHaveBeenCalled() + expect(h.bindReplacement).not.toHaveBeenCalled() + expect(h.executor.snapshotPageInventory()).toMatchObject([ + { browserProfileId: 'profile-a', state: 'active' } + ]) + }) + + it('retires only the staged guest and restores the old binding when binding fails', async () => { + const h = await fixture() + h.bindReplacement.mockImplementation(() => { + throw new Error('bind failed') + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + expect(h.dependencies.routeWebContents.beginGuestRetirement).toHaveBeenCalledTimes(1) + expect(h.dependencies.routeWebContents.beginGuestRetirement).toHaveBeenCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ webContentsId: 42 }) + }) + ) + expect(h.dependencies.guestBinding.bind).toHaveBeenLastCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ webContentsId: 41 }), + browserProfileId: 'profile-a' + }) + ) + }) + + it('restores the old guest when replacement navigation fails', async () => { + const h = await fixture() + await h.executor.handle( + createCommand('navigate', { + command: { type: 'navigate', url: 'https://example.com/authenticated' } + }), + new AbortController().signal + ) + h.dependencies.routeWebContents.navigateGuest.mockResolvedValueOnce(false) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + expect(h.executor.snapshotPageInventory()).toMatchObject([ + { + browserProfileId: 'profile-a', + currentUrl: 'https://example.com/authenticated', + state: 'active' + } + ]) + expect(h.dependencies.guestBinding.bind).toHaveBeenLastCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ webContentsId: 41 }) + }) + ) + // Rebinding before releasing the staged guest keeps the page's live downloads. + const binding = h.dependencies.guestBinding + const stagedRelease = binding.release.mock.calls.findIndex( + ([registration]) => registration.webContentsId === 42 + ) + expect(binding.bind.mock.invocationCallOrder.at(-1)).toBeLessThan( + binding.release.mock.invocationCallOrder[stagedRelease]! + ) + }) + + it('rejects a target partition collision without retiring the old guest', async () => { + const h = await fixture() + h.dependencies.routeSessions.preparePage.mockResolvedValueOnce(h.routeSession) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed', + errorCode: 'browser_client_page_profile_partition_conflict' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + expect(h.bindReplacement).not.toHaveBeenCalled() + expect(h.dependencies.routeWebContents.beginGuestRetirement).toHaveBeenCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ webContentsId: 42 }) + }) + ) + }) + + it('keeps the new availability watcher after retiring the old generation', async () => { + const availability = new BrowserRoutePageAvailability() + const h = await fixture(availability) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'completed' + }) + availability.report(h.dependencies.routeWebContents.claimGuestLifecycle.mock.calls[0]![0]) + expect(h.executor.snapshotPageInventory()).toMatchObject([ + { state: 'active', pageHostGeneration: 8 } + ]) + availability.report(h.dependencies.routeWebContents.claimGuestLifecycle.mock.calls.at(-1)![0]) + expect(h.executor.snapshotPageInventory()).toMatchObject([ + { state: 'outcomeUnknown', pageHostGeneration: 8 } + ]) + }) + + it('retains unresolved staged cleanup evidence without replacing the old page', async () => { + const h = await fixture() + h.bindReplacement.mockImplementation(() => { + throw new Error('bind failed') + }) + h.dependencies.routeWebContents.beginGuestRetirement.mockImplementationOnce(() => { + throw new Error('cleanup refused') + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + await expect(h.executor.close()).rejects.toThrow('Browser client page executor cleanup failed') + }) + + it('fences the staged guest when the connection is fenced during automation retirement', async () => { + const h = await fixture() + await h.executor.handle( + createCommand('navigate', { + command: { type: 'navigate', url: 'https://example.com/authenticated' } + }), + new AbortController().signal + ) + h.dependencies.routeWebContents.navigateGuest.mockClear() + h.dependencies.retireAutomation.mockImplementationOnce(async () => { + h.executor.fenceNavigation() + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed', + errorCode: 'browser_client_page_executor_closed' + }) + expect(h.dependencies.routeWebContents.revokeNavigation).toHaveBeenCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ webContentsId: 42 }) + }) + ) + expect(h.dependencies.routeWebContents.navigateGuest).not.toHaveBeenCalled() + expect(h.bindReplacement).not.toHaveBeenCalled() + expect(h.executor.hasPage('page-a', 7)).toBe(true) + }) + + it('keeps cleanup evidence when staging fails before handing over its guest', async () => { + const h = await fixture() + h.dependencies.routeWebContents.registerGuest.mockReturnValueOnce(false) + h.dependencies.routeWebContents.beginGuestRetirement.mockImplementationOnce(() => { + throw new Error('cleanup refused') + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed', + errorCode: 'browser_client_page_cleanup_failed' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + await expect(h.executor.close()).rejects.toThrow('Browser client page executor cleanup failed') + }) + + it('does not commit a replacement guest that died before the switch finished', async () => { + const h = await fixture() + let replacementAlive = true + h.dependencies.routeWebContents.claimGuestLifecycle.mockImplementationOnce((registration) => + createLifecycleClaim(registration, Promise.resolve(), () => replacementAlive) + ) + h.bindReplacement.mockImplementation(() => { + replacementAlive = false + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed', + errorCode: 'browser_client_page_generation_stale' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + expect(h.dependencies.routeWebContents.beginGuestRetirement).toHaveBeenCalledTimes(1) + expect(h.dependencies.routeWebContents.beginGuestRetirement).toHaveBeenCalledWith( + expect.objectContaining({ registration: expect.objectContaining({ webContentsId: 42 }) }) + ) + }) + + it('reports an old guest lost during a failed switch so recovery can run', async () => { + let oldAlive = true + const availability = new BrowserRoutePageAvailability() + const h = await fixture(availability, () => oldAlive) + const onPageUnavailable = vi.fn() + Object.assign(h.dependencies, { onPageUnavailable }) + h.dependencies.routeSessions.preparePage.mockImplementationOnce(async () => { + oldAlive = false + availability.report(h.dependencies.routeWebContents.claimGuestLifecycle.mock.calls[0]![0]) + throw new Error('target profile rejected') + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toMatchObject({ + status: 'failed' + }) + expect(onPageUnavailable).toHaveBeenCalledWith('page-a', 7) + expect(h.executor.snapshotPageInventory()).toMatchObject([ + { pageHostGeneration: 7, state: 'outcomeUnknown' } + ]) + expect(h.dependencies.guestBinding.bind).toHaveBeenCalledTimes(1) + }) + + it('lets the staged guest own downloads it starts while loading the page', async () => { + const h = await fixture() + await h.executor.handle( + createCommand('navigate', { + command: { type: 'navigate', url: 'https://example.com/report.csv' } + }), + new AbortController().signal + ) + let owner: { pageHostGeneration: number } | undefined + h.dependencies.routeWebContents.navigateGuest.mockImplementationOnce(async () => { + owner = h.executor.findPageByWebContentsId(42) + return true + }) + expect(await h.executor.handle(switchCommand(), new AbortController().signal)).toEqual({ + status: 'completed' + }) + expect(owner).toMatchObject({ browserPageId: 'page-a', pageHostGeneration: 8 }) + }) + + it('rejects stale generations without preparing or binding anything', async () => { + const h = await fixture() + const calls = h.dependencies.routeSessions.preparePage.mock.calls.length + expect( + await h.executor.handle( + { ...switchCommand(), pageHostGeneration: 6 }, + new AbortController().signal + ) + ).toMatchObject({ status: 'failed' }) + expect(h.dependencies.routeSessions.preparePage).toHaveBeenCalledTimes(calls) + expect(h.bindReplacement).not.toHaveBeenCalled() + }) + + it('rolls back an abort after successful target binding', async () => { + const h = await fixture() + const controller = new AbortController() + h.bindReplacement.mockImplementation(() => controller.abort()) + expect(await h.executor.handle(switchCommand(), controller.signal)).toMatchObject({ + status: 'failed' + }) + expect(h.executor.hasPage('page-a', 7)).toBe(true) + expect(h.dependencies.guestBinding.bind).toHaveBeenLastCalledWith( + expect.objectContaining({ + registration: expect.objectContaining({ webContentsId: 41 }) + }) + ) + }) +}) diff --git a/src/main/browser/browser-client-page-profile-switch.ts b/src/main/browser/browser-client-page-profile-switch.ts new file mode 100644 index 00000000000..01c75aed7e6 --- /dev/null +++ b/src/main/browser/browser-client-page-profile-switch.ts @@ -0,0 +1,182 @@ +import type { BrowserClientHostCommandEvent } from '../../shared/browser-client-host-protocol' +import { BrowserClientProfileMetadata } from '../../shared/browser-client-profile-switch-protocol' +import { sameBrowserClientPageAuthority } from './browser-client-host-command-authority' +import { + assertBrowserClientPageCommandNotAborted, + assertCurrentBrowserClientPageRenderer +} from './browser-client-page-command-admission' +import { + BrowserClientPageCommandError, + isBrowserClientPageCleanupFailure +} from './browser-client-page-command-failure' +import type { BrowserClientPageCommandExecutorDependencies } from './browser-client-page-command-executor-dependencies' +import { createReservedBrowserClientPage } from './browser-client-page-creation' +import type { BrowserClientRetainedPage } from './browser-client-page-retained-state' +import { updateBrowserClientPageInventoryCurrentUrl } from './browser-client-page-inventory' +import { markBrowserClientPageUnavailable } from './browser-client-page-availability' + +export async function switchBrowserClientPageProfile(input: { + event: BrowserClientHostCommandEvent + signal: AbortSignal + switches: BrowserClientProfileSwitches + pages: Map + dependencies: BrowserClientPageCommandExecutorDependencies + assertAvailable(): void + commit(page: BrowserClientRetainedPage): void + cleanup(page: BrowserClientRetainedPage, releaseUploads: boolean): Promise +}): Promise { + const { event, signal, dependencies, switches } = input + if (event.command.type !== 'switchProfile') { + throw new BrowserClientPageCommandError('browser_client_page_command_invalid') + } + const old = input.pages.get(event.browserPageId) + if ( + !old || + old.retiring || + old.reconciling || + !sameBrowserClientPageAuthority(old.inventory, event) || + event.command.nextPageHostGeneration <= old.generation + ) { + throw new BrowserClientPageCommandError('browser_client_page_generation_stale') + } + const profile = BrowserClientProfileMetadata.parse(event.command.profile) + const bindReplacement = dependencies.guestBinding.bindReplacement + if (!bindReplacement) { + throw new BrowserClientPageCommandError('browser_client_page_profile_switch_unsupported') + } + if (profile.id === old.inventory.browserProfileId) { + throw new BrowserClientPageCommandError('browser_client_page_profile_switch_redundant') + } + const create: BrowserClientHostCommandEvent = { + ...event, + pageHostGeneration: event.command.nextPageHostGeneration, + command: { + type: 'createPage', + browserProfileId: profile.id, + profile, + executionHostKey: old.inventory.executionHostKey, + ...(old.inventory.workspaceId ? { workspaceId: old.inventory.workspaceId } : {}) + } + } + let staged: BrowserClientRetainedPage | undefined + let bindingAttempted = false + old.reconciling = true + switches.stagedPages.set(event.browserPageId, null) + try { + await createReservedBrowserClientPage( + { + ...dependencies, + // A profile switch cannot change the execution route or supersede the old guest. + retireSupersededExecutionHostPages: async () => {}, + guestBinding: { bind: () => {}, release: () => {} } + }, + create, + signal, + input.assertAvailable, + (page) => { + staged = page + switches.stagedPages.set(event.browserPageId, page) + } + ) + if (!staged || staged.registration.partition === old.registration.partition) { + throw new BrowserClientPageCommandError('browser_client_page_profile_partition_conflict') + } + assertBrowserClientPageCommandNotAborted(signal) + input.assertAvailable() + assertCurrentBrowserClientPageRenderer(old.renderer) + await dependencies.retireAutomation({ + keepGuestBinding: true, + browserPageId: event.browserPageId, + pageHostGeneration: old.generation, + registration: old.registration + }) + // A fence that landed during that await must stop the staged guest before it navigates. + assertBrowserClientPageCommandNotAborted(signal) + input.assertAvailable() + assertCurrentBrowserClientPageRenderer(staged.renderer) + bindingAttempted = true + bindReplacement({ + registration: staged.registration, + browserProfileId: profile.id + }) + const url = old.inventory.currentUrl + if (url && url !== 'about:blank') { + if (!(await dependencies.routeWebContents.navigateGuest(staged.lifecycleClaim, url))) { + throw new BrowserClientPageCommandError('browser_client_page_navigation_failed') + } + staged.inventory = updateBrowserClientPageInventoryCurrentUrl(staged.inventory, url) + } + assertBrowserClientPageCommandNotAborted(signal) + input.assertAvailable() + assertCurrentBrowserClientPageRenderer(staged.renderer) + if ( + input.pages.get(event.browserPageId) !== old || + !old.lifecycleClaim.isCurrent() || + !staged.lifecycleClaim.isCurrent() + ) { + throw new BrowserClientPageCommandError('browser_client_page_generation_stale') + } + input.commit(staged) + } catch (error) { + const oldAlive = old.lifecycleClaim.isCurrent() + try { + if (bindingAttempted && oldAlive) { + // Rebind first: releasing the staged binding would otherwise cancel the page's downloads. + dependencies.guestBinding.bind({ + registration: old.registration, + browserProfileId: old.inventory.browserProfileId + }) + } + if (staged) { + const cleanup = input.cleanup(staged, false) + switches.retainedCleanups.add(cleanup) + await cleanup + switches.retainedCleanups.delete(cleanup) + } else if (isBrowserClientPageCleanupFailure(error)) { + // Staging failed before handing over its guest; keep the unreleased resources visible to close. + const unresolved = Promise.reject(error) + unresolved.catch(() => {}) + switches.retainedCleanups.add(unresolved) + } + } finally { + switches.stagedPages.delete(event.browserPageId) + old.reconciling = false + if (!old.lifecycleClaim.isCurrent()) { + // Its loss was reported once while reconciling suppressed it, so report it now. + markBrowserClientPageUnavailable(input.pages, old.registration, (id, generation) => + dependencies.onPageUnavailable?.(id, generation) + ) + } + } + throw error + } + switches.stagedPages.delete(event.browserPageId) + old.reconciling = false + await switches.retire(old, (page) => input.cleanup(page, true)) +} + +/** Profile-switch state the executor's fencing and close must also see. */ +export class BrowserClientProfileSwitches { + readonly retainedCleanups = new Set>() + /** In-flight switches by page; the staged guest once mounted. */ + readonly stagedPages = new Map() + + // Why: a staged guest holds a navigation grant and owns downloads before it replaces the page. + withStagedPages(pages: Iterable): BrowserClientRetainedPage[] { + return [...pages, ...[...this.stagedPages.values()].filter((page) => page !== null)] + } + + async retire( + page: BrowserClientRetainedPage, + cleanupPage: (page: BrowserClientRetainedPage) => Promise + ): Promise { + const cleanup = cleanupPage(page) + this.retainedCleanups.add(cleanup) + try { + await cleanup + this.retainedCleanups.delete(cleanup) + } catch (error) { + console.warn('[browser-client-page] Replaced guest cleanup remains unresolved', error) + } + } +} diff --git a/src/main/browser/browser-client-page-reconciliation-adapters.test.ts b/src/main/browser/browser-client-page-reconciliation-adapters.test.ts index afed5fee0dc..38c95298b08 100644 --- a/src/main/browser/browser-client-page-reconciliation-adapters.test.ts +++ b/src/main/browser/browser-client-page-reconciliation-adapters.test.ts @@ -46,7 +46,10 @@ function command( browserProfileId: 'profile-a', executionHostKey: 'execution-host-a' }, - closePage: { type: 'closePage' as const, targetAuthority: previousAuthority }, + closePage: { + type: 'closePage' as const, + targetAuthority: previousAuthority + }, restorePage: { type: 'restorePage' as const, browserProfileId: 'profile-a', @@ -244,6 +247,32 @@ describe('browser client page reconciliation adapters', () => { ]) }) + it('restores a host-owned profile with the metadata the host sent', async () => { + const { executor, dependencies } = createHarness() + const profile = { + id: 'remote-profile', + label: 'Remote profile', + scope: 'isolated' as const + } + const restore = command('restorePage', { + command: { + type: 'restorePage', + browserProfileId: profile.id, + profile, + executionHostKey: 'execution-host-a' + } + }) + await expect(executor.handle(restore, new AbortController().signal)).resolves.toEqual({ + status: 'completed' + }) + expect(dependencies.routeSessions.preparePage).toHaveBeenCalledWith( + expect.objectContaining({ + identity: expect.objectContaining({ browserProfileId: profile.id }), + profileMetadata: profile + }) + ) + }) + it('destroys a rekeyed guest instead of restoring authority after renderer failure', async () => { const { executor, renderer, routeWebContents } = createHarness() await executor.handle(command('createPage'), new AbortController().signal) diff --git a/src/main/browser/browser-client-page-reconciliation.ts b/src/main/browser/browser-client-page-reconciliation.ts index f1e643ab2ae..1fd253099fb 100644 --- a/src/main/browser/browser-client-page-reconciliation.ts +++ b/src/main/browser/browser-client-page-reconciliation.ts @@ -45,6 +45,7 @@ export function executeBrowserClientPageReconciliationCommand( return closeReconciledPage(context, event, signal) case 'restorePage': return restorePage(context, event, signal) + case 'switchProfile': case 'createPage': case 'navigate': case 'automation': @@ -84,7 +85,10 @@ async function reclaimPage( page.registration, page.registration.partition ) - const nextRegistration = { ...page.registration, pageHostGeneration: event.pageHostGeneration } + const nextRegistration = { + ...page.registration, + pageHostGeneration: event.pageHostGeneration + } const nextRendererPage = browserClientPageIdentity(nextRegistration, nextRegistration.partition) const rekeyGuestLifecycle = context.routeWebContents.rekeyGuestLifecycle const grantReconciledNavigation = context.routeWebContents.grantReconciledNavigation @@ -226,6 +230,9 @@ function reconciliationCreateEvent( command: { type: 'createPage', browserProfileId: event.command.browserProfileId, + ...(event.command.type === 'restorePage' && event.command.profile + ? { profile: event.command.profile } + : {}), executionHostKey: event.command.executionHostKey, ...(event.command.workspaceId ? { workspaceId: event.command.workspaceId } : {}) } diff --git a/src/main/browser/browser-client-page-retained-state.ts b/src/main/browser/browser-client-page-retained-state.ts index fa9c99b6a83..b960796419a 100644 --- a/src/main/browser/browser-client-page-retained-state.ts +++ b/src/main/browser/browser-client-page-retained-state.ts @@ -58,3 +58,26 @@ export function forgetBrowserClientRetainedPage( pages.delete(browserPageId) } } + +export async function retireBrowserClientRetainedPage( + pages: Map, + failedPages: Map, + browserPageId: string, + pageHostGeneration: number, + cleanupPage: (page: BrowserClientRetainedPage) => Promise +): Promise { + const page = pages.get(browserPageId) + if (!page || page.generation !== pageHostGeneration) { + return false + } + page.retiring ??= cleanupPage(page) + try { + await page.retiring + } catch (error) { + forgetBrowserClientRetainedPage(pages, browserPageId, page) + failedPages.set(browserPageId, Object.freeze({ ...page.inventory, state: 'outcomeUnknown' })) + throw error + } + forgetBrowserClientRetainedPage(pages, browserPageId, page) + return true +} diff --git a/src/main/browser/browser-client-route-cookie-import-partition-parity.test.ts b/src/main/browser/browser-client-route-cookie-import-partition-parity.test.ts index df4cb2ddc4d..abc576b68af 100644 --- a/src/main/browser/browser-client-route-cookie-import-partition-parity.test.ts +++ b/src/main/browser/browser-client-route-cookie-import-partition-parity.test.ts @@ -1,8 +1,6 @@ import { describe, expect, it, vi } from 'vitest' -import type { - BrowserHostLeaseAuthority, - BrowserNetworkExecutionHost -} from '../../shared/browser-client-host-protocol' +import type { BrowserHostLeaseAuthority } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' const { bindingStore, diff --git a/src/main/browser/browser-execution-host-storage-identity.ts b/src/main/browser/browser-execution-host-storage-identity.ts index 24d78ee03f6..845137d5b5f 100644 --- a/src/main/browser/browser-execution-host-storage-identity.ts +++ b/src/main/browser/browser-execution-host-storage-identity.ts @@ -1,4 +1,4 @@ -import type { BrowserNetworkExecutionHost } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' const STORAGE_IDENTITY_VERSION = 1 const STORAGE_IDENTITY_TAG = 'orca-browser-execution-host-storage' diff --git a/src/main/browser/browser-manager-registration.ts b/src/main/browser/browser-manager-registration.ts index f6f7b41bbb9..97895e8b179 100644 --- a/src/main/browser/browser-manager-registration.ts +++ b/src/main/browser/browser-manager-registration.ts @@ -5,15 +5,18 @@ import type { BrowserGuestRegistration } from './browser-manager-types' import { BrowserManagerGuestPolicy } from './browser-manager-guest-policy' export abstract class BrowserManagerRegistration extends BrowserManagerGuestPolicy { - registerGuest({ - browserPageId, - browserTabId: legacyBrowserTabId, - workspaceId, - worktreeId, - sessionProfileId, - webContentsId, - rendererWebContentsId - }: BrowserGuestRegistration): boolean { + registerGuest( + { + browserPageId, + browserTabId: legacyBrowserTabId, + workspaceId, + worktreeId, + sessionProfileId, + webContentsId, + rendererWebContentsId + }: BrowserGuestRegistration, + options?: { retainPreviousGuestPolicies?: boolean } + ): boolean { const browserTabId = browserPageId ?? legacyBrowserTabId // Why refuse rather than overwrite: the two halves of the registry must stay disjoint, or one // id resolves in both and the tool door silently prefers the document guest over the page. @@ -45,7 +48,12 @@ export abstract class BrowserManagerRegistration extends BrowserManagerGuestPoli const previousWebContentsId = this.webContentsIdByTabId.get(browserTabId) if (previousWebContentsId !== undefined && previousWebContentsId !== webContentsId) { - this.retireStaleGuestWebContents(previousWebContentsId) + if (options?.retainPreviousGuestPolicies) { + // A staged swap must be able to restore this still-live guest on failure. + this.tabIdByWebContentsId.delete(previousWebContentsId) + } else { + this.retireStaleGuestWebContents(previousWebContentsId) + } this.viewportPresetByTabId.delete(browserTabId) this.viewportScrollStateByTabId.delete(browserTabId) } diff --git a/src/main/browser/browser-network-execution-route.ts b/src/main/browser/browser-network-execution-route.ts index dce61416ef6..ca6a290e9a2 100644 --- a/src/main/browser/browser-network-execution-route.ts +++ b/src/main/browser/browser-network-execution-route.ts @@ -1,5 +1,5 @@ import { connect } from 'node:net' -import { BrowserNetworkExecutionHost } from '../../shared/browser-client-host-protocol' +import { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import type { BrowserNetworkTunnelOpen } from '../../shared/browser-network-tunnel-protocol' import type { BrowserNetworkTunnelSocket } from './browser-network-tunnel-stream-state' import { readNetworkProxySettings } from './browser-network-proxy-settings-source' diff --git a/src/main/browser/browser-route-partition-stability.test.ts b/src/main/browser/browser-route-partition-stability.test.ts index e98e721b785..cd39f3ba354 100644 --- a/src/main/browser/browser-route-partition-stability.test.ts +++ b/src/main/browser/browser-route-partition-stability.test.ts @@ -2,10 +2,8 @@ import { mkdtempSync, realpathSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' -import type { - BrowserHostLeaseAuthority, - BrowserNetworkExecutionHost -} from '../../shared/browser-client-host-protocol' +import type { BrowserHostLeaseAuthority } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import { BrowserClientNetworkRouteRegistry } from './browser-client-network-route-registry' import { browserNetworkExecutionHostKey } from './browser-network-execution-route' import { deriveBrowserRoutePartition } from './browser-route-identity' diff --git a/src/main/browser/browser-route-session-policy.ts b/src/main/browser/browser-route-session-policy.ts index 522ef57ba09..73c91538623 100644 --- a/src/main/browser/browser-route-session-policy.ts +++ b/src/main/browser/browser-route-session-policy.ts @@ -1,3 +1,4 @@ +import type { BrowserClientProfileMetadata } from '../../shared/browser-client-profile-switch-protocol' const PROXY_PROBE_URL = 'http://browser-route-probe.invalid/' export type BrowserRouteElectronSession = { @@ -15,6 +16,7 @@ export type BrowserRouteProxyEndpoint = Readonly<{ host: '127.0.0.1'; port: numb type BrowserRouteSessionPolicyDependencies = { getSession(partition: string): BrowserRouteElectronSession setupPolicies(input: { + profileMetadata?: BrowserClientProfileMetadata partition: string browserProfileId: string session: BrowserRouteElectronSession @@ -23,6 +25,7 @@ type BrowserRouteSessionPolicyDependencies = { } export async function prepareBrowserRouteSessionPolicy(input: { + profileMetadata?: BrowserClientProfileMetadata partition: string browserProfileId: string proxyEndpoint: BrowserRouteProxyEndpoint @@ -37,6 +40,7 @@ export async function prepareBrowserRouteSessionPolicy(input: { proxyBypassRules: '<-loopback>' }) await input.dependencies.setupPolicies({ + ...(input.profileMetadata ? { profileMetadata: input.profileMetadata } : {}), partition: input.partition, browserProfileId: input.browserProfileId, session diff --git a/src/main/browser/browser-route-session-registry-contract.ts b/src/main/browser/browser-route-session-registry-contract.ts index 365b04028c2..a37836deec1 100644 --- a/src/main/browser/browser-route-session-registry-contract.ts +++ b/src/main/browser/browser-route-session-registry-contract.ts @@ -1,3 +1,4 @@ +import type { BrowserClientProfileMetadata } from '../../shared/browser-client-profile-switch-protocol' import type { BrowserRoutePartitionIdentity, DerivedBrowserRoutePartition @@ -16,9 +17,10 @@ export type BrowserRoutePartitionBindingStore = { export type BrowserRouteSessionRegistryDependencies = { derivePartition?: (identity: BrowserRoutePartitionIdentity) => DerivedBrowserRoutePartition - validateProfile(browserProfileId: string): void + validateProfile(browserProfileId: string, metadata?: BrowserClientProfileMetadata): void getSession(partition: string): BrowserRouteElectronSession setupPolicies(input: { + profileMetadata?: BrowserClientProfileMetadata partition: string browserProfileId: string session: BrowserRouteElectronSession diff --git a/src/main/browser/browser-route-session-registry.test.ts b/src/main/browser/browser-route-session-registry.test.ts index 1e4a42ab9a8..4c83afa2dde 100644 --- a/src/main/browser/browser-route-session-registry.test.ts +++ b/src/main/browser/browser-route-session-registry.test.ts @@ -138,6 +138,45 @@ function deferred(): { } describe('BrowserRouteSessionRegistry', () => { + it('prepares authority-scoped remote metadata without needing a desktop profile record', async () => { + const { dependencies, registry } = createHarness() + const profileMetadata = { + id: identity.browserProfileId, + label: 'Remote profile', + scope: 'isolated' + } + await prepare(registry, { profileMetadata }) + expect(dependencies.validateProfile).toHaveBeenCalledWith( + identity.browserProfileId, + profileMetadata + ) + expect(dependencies.setupPolicies).toHaveBeenCalledWith( + expect.objectContaining({ + profileMetadata, + browserProfileId: identity.browserProfileId, + partition: 'persist:route-a' + }) + ) + }) + + it.each([ + { id: 'other-profile', label: 'Remote', scope: 'isolated' }, + { + id: identity.browserProfileId, + label: 'Remote', + scope: 'isolated', + partition: 'persist:local-default' + } + ])( + 'rejects inconsistent or arbitrary partition metadata before touching storage', + async (profileMetadata) => { + const { registry, dependencies } = createHarness() + await expect(prepare(registry, { profileMetadata })).rejects.toThrow() + expect(dependencies.getSession).not.toHaveBeenCalled() + expect(dependencies.bindingStore.set).not.toHaveBeenCalled() + } + ) + it('rekeys one prepared authority and makes the old release handle inert', async () => { const { dependencies, registry } = createHarness() const previousHandle = await prepare(registry) diff --git a/src/main/browser/browser-route-session-registry.ts b/src/main/browser/browser-route-session-registry.ts index 406e7e5b045..d5ce0185404 100644 --- a/src/main/browser/browser-route-session-registry.ts +++ b/src/main/browser/browser-route-session-registry.ts @@ -1,3 +1,4 @@ +import { BrowserClientProfileMetadata } from '../../shared/browser-client-profile-switch-protocol' import { deriveBrowserRoutePartition, type DerivedBrowserRoutePartition @@ -130,7 +131,13 @@ export class BrowserRouteSessionRegistry { input: BrowserRoutePreparePageInput, rendererFence: BrowserRouteRendererPrepareFence ): Promise { - this.dependencies.validateProfile(input.identity.browserProfileId) + const metadata = input.profileMetadata + ? BrowserClientProfileMetadata.parse(input.profileMetadata) + : undefined + if (metadata && metadata.id !== input.identity.browserProfileId) { + throw new Error('browser_route_partition_profile_unavailable') + } + this.dependencies.validateProfile(input.identity.browserProfileId, metadata) const derived = resolveBrowserRouteSessionPartition( this.dependencies, input, @@ -161,7 +168,8 @@ export class BrowserRouteSessionRegistry { const promise = this.preparePartition( derived, input.identity.browserProfileId, - input.proxyEndpoint + input.proxyEndpoint, + metadata ) const pendingState = { partition: derived.partition, @@ -238,9 +246,11 @@ export class BrowserRouteSessionRegistry { private async preparePartition( derived: DerivedBrowserRoutePartition, browserProfileId: string, - proxyEndpoint: ProxyEndpoint + proxyEndpoint: ProxyEndpoint, + profileMetadata?: BrowserClientProfileMetadata ): Promise { const session = await prepareBrowserRouteSessionPolicy({ + ...(profileMetadata ? { profileMetadata } : {}), partition: derived.partition, browserProfileId, proxyEndpoint, diff --git a/src/main/browser/browser-route-session-runtime.ts b/src/main/browser/browser-route-session-runtime.ts index 678a8f24da6..76f712fdb9d 100644 --- a/src/main/browser/browser-route-session-runtime.ts +++ b/src/main/browser/browser-route-session-runtime.ts @@ -1,3 +1,5 @@ +import { BrowserClientProfileMetadata } from '../../shared/browser-client-profile-switch-protocol' +import { installBrowserRoutePartitionPolicies } from './browser-session-route-policies' import { session, webContents } from 'electron' import { setBrowserClientRouteWebContentsProbe } from './browser-client-download-routing' import type { BrowserRoutePartitionBindingStore } from './browser-route-partition-binding-store' @@ -38,12 +40,23 @@ const bindingStore = { } export const browserRouteSessionRegistry = new BrowserRouteSessionRegistry({ - validateProfile: (browserProfileId) => { - browserSessionRegistry.requireRouteBrowserProfile(browserProfileId) + validateProfile: (browserProfileId, metadata) => { + if (metadata) { + if (BrowserClientProfileMetadata.parse(metadata).id !== browserProfileId) { + throw new Error('browser_route_partition_profile_unavailable') + } + } else { + browserSessionRegistry.requireRouteBrowserProfile(browserProfileId) + } }, getSession: (partition) => session.fromPartition(partition), - setupPolicies: ({ partition, browserProfileId }) => - browserSessionRegistry.setupRoutePartitionPolicies(partition, browserProfileId), + setupPolicies: ({ partition, browserProfileId, profileMetadata }) => + profileMetadata + ? installBrowserRoutePartitionPolicies( + { ...BrowserClientProfileMetadata.parse(profileMetadata), partition, source: null }, + partition + ) + : browserSessionRegistry.setupRoutePartitionPolicies(partition, browserProfileId), clearPolicies: ({ partition }) => { browserSessionRegistry.clearRoutePartitionPolicies(partition) }, diff --git a/src/main/browser/browser-route-session-state.ts b/src/main/browser/browser-route-session-state.ts index f2d129ce6b1..16dc7a438e5 100644 --- a/src/main/browser/browser-route-session-state.ts +++ b/src/main/browser/browser-route-session-state.ts @@ -1,3 +1,4 @@ +import type { BrowserClientProfileMetadata } from '../../shared/browser-client-profile-switch-protocol' import type { BrowserRoutePartitionIdentity } from './browser-route-identity' import type { BrowserRoutePreparedPageLedger } from './browser-route-prepared-page-ledger' import type { BrowserRoutePageAuthority } from './browser-route-page-authority' @@ -12,6 +13,7 @@ export type BrowserRouteSessionHandle = Readonly<{ }> export type BrowserRoutePreparePageInput = Readonly<{ + profileMetadata?: BrowserClientProfileMetadata identity: BrowserRoutePartitionIdentity /** Pre-migration identity, so an existing partition keeps serving this route. */ legacyIdentity?: BrowserRoutePartitionIdentity diff --git a/src/main/browser/paired-runtime-browser-client-host-runtime.ts b/src/main/browser/paired-runtime-browser-client-host-runtime.ts index 1101b2e73dc..0ba0ed271fb 100644 --- a/src/main/browser/paired-runtime-browser-client-host-runtime.ts +++ b/src/main/browser/paired-runtime-browser-client-host-runtime.ts @@ -1,3 +1,4 @@ +import { BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY } from '../../shared/browser-client-profile-switch-protocol' import { createHash } from 'node:crypto' import path from 'node:path' import { app } from 'electron' @@ -117,6 +118,7 @@ const browserClientHosts = browserHostClientId: getBrowserClientHostId(), hostCapabilities: [ 'webview', + BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY, BROWSER_CLIENT_AUTOMATION_HOST_CAPABILITY, BROWSER_CLIENT_FILE_CHANNEL_HOST_CAPABILITY ], diff --git a/src/main/browser/paired-runtime-browser-network-route.test.ts b/src/main/browser/paired-runtime-browser-network-route.test.ts index c54ab05a542..fe22a53c5f9 100644 --- a/src/main/browser/paired-runtime-browser-network-route.test.ts +++ b/src/main/browser/paired-runtime-browser-network-route.test.ts @@ -7,7 +7,7 @@ import { decodeBrowserNetworkTunnelFrame, encodeBrowserNetworkTunnelFrame } from '../../shared/browser-network-tunnel-protocol' -import type { BrowserNetworkExecutionHost } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import type { PairingOffer } from '../../shared/pairing' import type { RemoteRuntimeSubscription, diff --git a/src/main/browser/paired-runtime-browser-network-route.ts b/src/main/browser/paired-runtime-browser-network-route.ts index 27ddc943b51..9712f8711b7 100644 --- a/src/main/browser/paired-runtime-browser-network-route.ts +++ b/src/main/browser/paired-runtime-browser-network-route.ts @@ -1,7 +1,5 @@ -import type { - BrowserHostLeaseAuthority, - BrowserNetworkExecutionHost -} from '../../shared/browser-client-host-protocol' +import type { BrowserHostLeaseAuthority } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import type { BrowserNetworkTunnelOpen } from '../../shared/browser-network-tunnel-protocol' import type { PairedRuntimeConnection } from '../../shared/paired-runtime-connection' import type { RemoteRuntimeSubscriptionOptions } from '../../shared/remote-runtime-client' diff --git a/src/main/browser/paired-runtime-browser-network-transport.ts b/src/main/browser/paired-runtime-browser-network-transport.ts index 8559de59635..26ee1339d00 100644 --- a/src/main/browser/paired-runtime-browser-network-transport.ts +++ b/src/main/browser/paired-runtime-browser-network-transport.ts @@ -1,8 +1,8 @@ import { BrowserNetworkTunnelEvent, - type BrowserHostLeaseAuthority, - type BrowserNetworkExecutionHost + type BrowserHostLeaseAuthority } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import type { PairedRuntimeConnection } from '../../shared/paired-runtime-connection' import { subscribeRemoteRuntimeRequest, diff --git a/src/main/host/electron-browser-commands.ts b/src/main/host/electron-browser-commands.ts index 27ba3ae0c8a..0d8a2e9d13b 100644 --- a/src/main/host/electron-browser-commands.ts +++ b/src/main/host/electron-browser-commands.ts @@ -1,9 +1,15 @@ import { RuntimeBrowserCommands } from '../runtime/browser/browser-commands' import { setRuntimeBrowserCommandsFactory } from '../runtime/runtime-browser-commands-factory' +import { setBrowserHostClientProfileResolver } from '../runtime/browser-host/browser-host-client-profile-metadata' +import { browserSessionRegistry } from '../browser/browser-session-registry' /** The desktop factory. Importing this file is what pulls in the Chromium browser cluster. */ export function installElectronBrowserCommands(): void { setRuntimeBrowserCommandsFactory((host) => new RuntimeBrowserCommands(host), { clientHosting: true }) + setBrowserHostClientProfileResolver((browserProfileId) => { + const profile = browserSessionRegistry.getProfile(browserProfileId) + return profile ? { id: profile.id, label: profile.label, scope: profile.scope } : undefined + }) } diff --git a/src/main/runtime/browser-client-download-transfer-store.ts b/src/main/runtime/browser-client-download-transfer-store.ts index c6c63b322fe..47de09fc737 100644 --- a/src/main/runtime/browser-client-download-transfer-store.ts +++ b/src/main/runtime/browser-client-download-transfer-store.ts @@ -36,9 +36,10 @@ const stores = new WeakMap { - return stores.get(runtime)?.releasePage(browserPageId) ?? Promise.resolve() + return stores.get(runtime)?.releasePage(browserPageId, pageHostGeneration) ?? Promise.resolve() } export function getBrowserClientDownloadTransferStore( diff --git a/src/main/runtime/browser-client-download-transfers.test.ts b/src/main/runtime/browser-client-download-transfers.test.ts index 528140dc3bd..be45e5c8d21 100644 --- a/src/main/runtime/browser-client-download-transfers.test.ts +++ b/src/main/runtime/browser-client-download-transfers.test.ts @@ -217,6 +217,20 @@ describe('BrowserClientDownloadTransferStore', () => { expect(store.activeTransferCount()).toBe(1) }) + it('releases only the named generation of a page', async () => { + const { store } = createStore() + await store.accept({ ...base, contentBase64: 'AAA=', offset: 0, final: false }) + const replacement = { ...base, transferId: 'transfer-2', pageHostGeneration: 3 } + await store.accept({ ...replacement, contentBase64: 'AAA=', offset: 0, final: false }) + + await store.releasePage(base.browserPageId, base.pageHostGeneration) + + expect(store.activeTransferCount()).toBe(1) + await expect( + store.accept({ ...replacement, contentBase64: '', offset: 2, final: false }) + ).resolves.not.toThrow() + }) + it('rejects a transfer resumed under a replaced page generation', async () => { const { store } = createStore() await store.accept({ ...base, contentBase64: 'AAA=', offset: 0, final: false }) diff --git a/src/main/runtime/browser-client-download-transfers.ts b/src/main/runtime/browser-client-download-transfers.ts index 8341bacfe3f..3c12da70177 100644 --- a/src/main/runtime/browser-client-download-transfers.ts +++ b/src/main/runtime/browser-client-download-transfers.ts @@ -103,9 +103,12 @@ export class BrowserClientDownloadTransferStore { } // Why: page close and lease fencing must not leave half-written files in the remote workspace. - async releasePage(browserPageId: string): Promise { + /** Omitting the generation releases every generation of the page. */ + async releasePage(browserPageId: string, pageHostGeneration?: number): Promise { const owned = [...this.sessions.values()].filter( - (session) => session.browserPageId === browserPageId + (session) => + session.browserPageId === browserPageId && + (pageHostGeneration === undefined || session.pageHostGeneration === pageHostGeneration) ) for (const session of owned) { session.canceled = true diff --git a/src/main/runtime/browser-host/browser-host-client-page-connection.ts b/src/main/runtime/browser-host/browser-host-client-page-connection.ts index ac008bc8eab..1864e294da6 100644 --- a/src/main/runtime/browser-host/browser-host-client-page-connection.ts +++ b/src/main/runtime/browser-host/browser-host-client-page-connection.ts @@ -1,7 +1,11 @@ import type { BrowserHostLeaseState } from './browser-host-lease-records' import type { BrowserHostPagePlacementRegistry } from './browser-host-page-placement' -import type { RuntimeBrowserClientPlacement } from '../../../shared/runtime-browser-placement' +import { + sameRuntimeBrowserPlacement, + type RuntimeBrowserClientPlacement +} from '../../../shared/runtime-browser-placement' import { requireBrowserHostLeaseState } from './browser-host-lease-state-resolution' +import { pendingBrowserHostProfileSwitches } from './browser-host-client-profile-switch' export function requireBrowserClientPageConnection( input: { @@ -9,6 +13,8 @@ export function requireBrowserClientPageConnection( placement: RuntimeBrowserClientPlacement pairedDeviceId: string connectionId: string + /** File transfers only: a staged profile-switch guest downloads before its generation commits. */ + admitPendingProfileSwitch?: boolean }, dependencies: { authorityRuntimeId: string @@ -17,6 +23,22 @@ export function requireBrowserClientPageConnection( pagePlacements: BrowserHostPagePlacementRegistry } ): RuntimeBrowserClientPlacement { + const pending = input.admitPendingProfileSwitch + ? pendingBrowserHostProfileSwitches(dependencies.pagePlacements).get(input.browserPageId) + : undefined + const current = dependencies.pagePlacements.getPlacement(input.browserPageId) + if ( + pending && + current?.kind === 'client' && + sameRuntimeBrowserPlacement(pending, input.placement) + ) { + // Why: the replacement downloads while loading, before the host commits its generation. + requireBrowserClientPageConnection( + { ...input, placement: current, admitPendingProfileSwitch: false }, + dependencies + ) + return pending + } const state = requireBrowserHostLeaseState( dependencies.authorityEpoch, dependencies.leasesByClientId, diff --git a/src/main/runtime/browser-host/browser-host-client-page-creation.ts b/src/main/runtime/browser-host/browser-host-client-page-creation.ts index bc7125c937c..cf5d9d42610 100644 --- a/src/main/runtime/browser-host/browser-host-client-page-creation.ts +++ b/src/main/runtime/browser-host/browser-host-client-page-creation.ts @@ -1,3 +1,5 @@ +import { BROWSER_HOST_WEBVIEW_CAPABILITY } from './browser-host-capability-selection' +import { browserHostClientProfileFor } from './browser-host-client-profile-metadata' import type { BrowserClientHostCommandResult } from '../../../shared/browser-client-host-protocol' import { DEFAULT_CLIENT_PAGE_CREATION_TIMEOUT_MS, @@ -38,10 +40,10 @@ export async function createBrowserHostClientPage( } ): Promise { const timeoutMs = clientPageCreationTimeout(options.timeoutMs) - const lease = dependencies.selectLease( - options.browserHostClientId, - options.requiredCapabilities ?? [] - ) + const lease = dependencies.selectLease(options.browserHostClientId, [ + BROWSER_HOST_WEBVIEW_CAPABILITY, + ...(options.requiredCapabilities ?? []) + ]) if (lease.pairedDeviceId !== options.pairedDeviceId) { throw new Error('browser_host_lease_stale') } @@ -66,6 +68,7 @@ export async function createBrowserHostClientPage( const command = { type: 'createPage' as const, browserProfileId: options.browserProfileId, + ...browserHostClientProfileFor(lease, options.browserProfileId), executionHostKey: options.executionHostKey, ...(options.workspaceId ? { workspaceId: options.workspaceId } : {}) } @@ -110,7 +113,7 @@ export async function createBrowserHostClientPage( } } -function clientPageCreationTimeout(timeoutMs: number | undefined): number { +export function clientPageCreationTimeout(timeoutMs: number | undefined): number { const resolved = timeoutMs ?? DEFAULT_CLIENT_PAGE_CREATION_TIMEOUT_MS if ( !Number.isInteger(resolved) || @@ -122,7 +125,7 @@ function clientPageCreationTimeout(timeoutMs: number | undefined): number { return resolved } -async function waitForClientPageCreationResult( +export async function waitForClientPageCreationResult( result: Promise, timeoutMs: number ): Promise { @@ -144,7 +147,7 @@ async function waitForClientPageCreationResult( } } -function closeUncommittedClientPage( +export function closeUncommittedClientPage( state: BrowserHostLeaseState, browserPageId: string, placement: RuntimeBrowserClientPlacement diff --git a/src/main/runtime/browser-host/browser-host-client-profile-metadata.ts b/src/main/runtime/browser-host/browser-host-client-profile-metadata.ts new file mode 100644 index 00000000000..af9d269be0c --- /dev/null +++ b/src/main/runtime/browser-host/browser-host-client-profile-metadata.ts @@ -0,0 +1,35 @@ +import { + BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY, + BrowserClientProfileMetadata +} from '../../../shared/browser-client-profile-switch-protocol' + +type BrowserHostClientProfileResolver = (browserProfileId: string) => unknown + +let resolveProfile: BrowserHostClientProfileResolver | null = null + +/** Installed by the host that owns browser profiles; without one, pages carry only profile ids. */ +export function setBrowserHostClientProfileResolver( + resolver: BrowserHostClientProfileResolver | null +): void { + resolveProfile = resolver +} + +/** + * The host-owned profile a page command may carry to the desktop. + * + * Why: a desktop can only rebuild a page in a profile it was told about, so restores and + * recoveries after a switch need the same metadata the switch sent. Its own default needs none. + */ +export function browserHostClientProfileFor( + lease: { hostCapabilities: readonly string[] }, + browserProfileId: string +): { profile?: BrowserClientProfileMetadata } { + if ( + browserProfileId === 'default' || + !lease.hostCapabilities.includes(BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY) + ) { + return {} + } + const parsed = BrowserClientProfileMetadata.safeParse(resolveProfile?.(browserProfileId)) + return parsed.success && parsed.data.id === browserProfileId ? { profile: parsed.data } : {} +} diff --git a/src/main/runtime/browser-host/browser-host-client-profile-switch.test.ts b/src/main/runtime/browser-host/browser-host-client-profile-switch.test.ts new file mode 100644 index 00000000000..62b455da58e --- /dev/null +++ b/src/main/runtime/browser-host/browser-host-client-profile-switch.test.ts @@ -0,0 +1,325 @@ +import { DEFAULT_CLIENT_PAGE_CREATION_TIMEOUT_MS } from '../../../shared/browser-client-page-creation-timeouts' +import type { BrowserClientHostCommandEvent } from '../../../shared/browser-client-host-protocol' +import { describe, expect, it, vi } from 'vitest' +import { BrowserHostLeaseRegistry } from './browser-host-lease-registry' +import { BrowserClientHostCommandDispatcher } from '../../browser/browser-client-host-command-dispatcher' +import { createHarness } from '../../browser/browser-client-page-command-executor-test-harness' +import { + BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY, + BrowserClientProfileMetadata +} from '../../../shared/browser-client-profile-switch-protocol' +import { deriveBrowserRoutePartition } from '../../browser/browser-route-identity' +import { setBrowserHostClientProfileResolver } from './browser-host-client-profile-metadata' + +async function fixture(supported = true, onClientPageReleased?: (browserPageId: string) => void) { + const leases = new BrowserHostLeaseRegistry({ + authorityRuntimeId: 'runtime-a', + authorityEpoch: 'epoch-a', + onClientPageReleased + }) + const lease = leases.attach({ + browserHostClientId: 'client-a', + pairedDeviceId: 'device-a', + connectionId: 'connection-a', + hostCapabilities: ['webview', ...(supported ? [BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY] : [])], + pageCommandProtocolVersion: 1, + pageInventoryProtocolVersion: 1, + pageReconciliationProtocolVersion: 1, + pageInventory: [] + }).lease + const h = createHarness() + const bindReplacement = vi.fn() + Object.assign(h.dependencies.guestBinding, { bindReplacement }) + h.dependencies.routeSessions.preparePage.mockImplementation(async (input) => ({ + partition: deriveBrowserRoutePartition(input.identity).partition, + release: vi.fn() + })) + let guestId = 40 + h.renderer.mountPage.mockImplementation(async () => ({ + webContentsId: ++guestId + })) + const dispatcher = new BrowserClientHostCommandDispatcher({ + authority: { ...lease, pageCommandProtocolVersion: 1 }, + handler: (event, signal) => h.executor.handle(event, signal) + }) + const afterSettlement = vi.fn<(event: BrowserClientHostCommandEvent) => void>() + const delivered = vi.fn((event) => { + void dispatcher.dispatch(event).then((result) => { + leases.settleClientPageCommand( + { ...lease, connectionId: lease.connectionId }, + { ...event, result } + ) + afterSettlement(event) + }) + }) + leases.attachCommandDelivery(lease, delivered) + const placement = await leases.createClientPage({ + browserPageId: 'page-a', + browserHostClientId: lease.browserHostClientId, + pairedDeviceId: lease.pairedDeviceId, + browserProfileId: 'default', + executionHostKey: 'execution-host-a', + workspaceId: 'workspace-a' + }) + const authority = { + ...lease, + browserPageId: 'page-a', + pageHostGeneration: placement.pageHostGeneration + } + return { + leases, + h, + dispatcher, + delivered, + placement, + authority, + bindReplacement, + afterSettlement + } +} + +const isolated = { + id: 'remote-profile', + label: 'Remote-only profile', + scope: 'isolated' as const +} + +describe('negotiated profile switch over the command transport', () => { + it('adopts a replacement the client finished after the caller timed out', async () => { + const f = await fixture() + vi.useFakeTimers() + try { + let finishMount!: (value: { webContentsId: number }) => void + f.h.renderer.mountPage.mockImplementationOnce( + () => + new Promise((resolve) => { + finishMount = resolve + }) + ) + const lateCommit = vi.fn() + const rejected = expect( + f.leases.switchClientPageProfile(f.authority, isolated, lateCommit) + ).rejects.toThrow('browser_host_page_creation_timeout') + await vi.advanceTimersByTimeAsync(DEFAULT_CLIENT_PAGE_CREATION_TIMEOUT_MS + 1) + await rejected + expect( + f.delivered.mock.calls.filter(([event]) => event.command.type === 'closePage') + ).toHaveLength(0) + finishMount({ webContentsId: 99 }) + await vi.advanceTimersByTimeAsync(0) + const switchEvent = f.delivered.mock.calls + .map(([event]) => event) + .find((event) => event.command.type === 'switchProfile')! + if (switchEvent.command.type !== 'switchProfile') { + throw new Error('expected a switch command') + } + const next = switchEvent.command.nextPageHostGeneration + expect( + f.delivered.mock.calls.filter(([event]) => event.command.type === 'closePage') + ).toHaveLength(0) + expect(lateCommit).toHaveBeenCalledWith(expect.objectContaining({ pageHostGeneration: next })) + expect(f.leases.getPlacement('page-a')).toMatchObject({ pageHostGeneration: next }) + expect(f.h.executor.snapshotPageInventory()).toMatchObject([ + { browserProfileId: isolated.id, pageHostGeneration: next, state: 'active' } + ]) + } finally { + vi.useRealTimers() + } + }) + + it('switches both directions, preserves partition identity, and routes later commands to the new generation', async () => { + const f = await fixture() + f.afterSettlement.mockImplementation((event) => { + if (event.command.type === 'switchProfile') { + expect(() => + f.leases.issueClientPageCommand( + { ...f.authority, pageHostGeneration: event.pageHostGeneration }, + { type: 'navigate', url: 'https://example.com/' } + ) + ).toThrow('profile_switch_pending') + } + }) + const beforePartition = f.h.executor.snapshotPageInventory()[0]! + const next = await f.leases.switchClientPageProfile(f.authority, isolated) + expect(next.pageHostGeneration).toBeGreaterThan(f.placement.pageHostGeneration) + expect(f.h.executor.hasPage('page-a', next.pageHostGeneration)).toBe(true) + const switched = f.delivered.mock.calls.at(-1)![0] + const boundCount = f.bindReplacement.mock.calls.length + await expect(f.dispatcher.dispatch(switched)).resolves.toEqual({ + status: 'completed' + }) + expect(f.bindReplacement).toHaveBeenCalledTimes(boundCount) + expect( + f.leases.settleClientPageCommand( + { ...f.authority, connectionId: 'connection-a' }, + { ...switched, result: { status: 'completed' } } + ) + ).toBe(false) + expect(() => + f.leases.issueClientPageCommand(f.authority, { + type: 'navigate', + url: 'https://example.com/' + }) + ).toThrow('placement_stale') + + const navigate = f.leases.issueClientPageCommand( + { ...f.authority, pageHostGeneration: next.pageHostGeneration }, + { type: 'navigate', url: 'https://example.com/' } + ) + await expect(navigate.result).resolves.toEqual({ status: 'completed' }) + const back = await f.leases.switchClientPageProfile( + { ...f.authority, pageHostGeneration: next.pageHostGeneration }, + { id: 'default', label: 'Default', scope: 'default' } + ) + expect(f.h.executor.hasPage('page-a', back.pageHostGeneration)).toBe(true) + expect(f.h.dependencies.routeSessions.preparePage.mock.calls[0]![0].identity).toEqual( + f.h.dependencies.routeSessions.preparePage.mock.calls[2]![0].identity + ) + expect(beforePartition.browserProfileId).toBe('default') + expect(f.leases.getClientPageExecutionHostKey('page-a')).toBe('execution-host-a') + await f.dispatcher.close() + await f.h.executor.close() + }) + + it('rejects an old desktop before issuing a command or changing placement', async () => { + const f = await fixture(false) + const calls = f.delivered.mock.calls.length + await expect(f.leases.switchClientPageProfile(f.authority, isolated)).rejects.toThrow( + 'profile_switch_unsupported' + ) + expect(f.delivered).toHaveBeenCalledTimes(calls) + expect(f.leases.getPlacement('page-a')).toBe(f.placement) + expect(f.h.executor.hasPage('page-a', f.placement.pageHostGeneration)).toBe(true) + await f.dispatcher.close() + await f.h.executor.close() + }) + + it('rolls back failure and permits a later retry on the original generation', async () => { + const f = await fixture() + f.bindReplacement.mockImplementationOnce(() => { + throw new Error('binding refused') + }) + await expect(f.leases.switchClientPageProfile(f.authority, isolated)).rejects.toThrow() + expect(f.leases.getPlacement('page-a')).toBe(f.placement) + expect(f.h.executor.hasPage('page-a', f.placement.pageHostGeneration)).toBe(true) + const next = await f.leases.switchClientPageProfile(f.authority, isolated) + expect(f.h.executor.hasPage('page-a', next.pageHostGeneration)).toBe(true) + await f.dispatcher.close() + await f.h.executor.close() + }) + + it('serializes a staged replacement and rejects commands addressed to the old generation while it is pending', async () => { + const f = await fixture() + let release = () => {} + let started = () => {} + const gate = new Promise((resolve) => { + release = resolve + }) + const entered = new Promise((resolve) => { + started = resolve + }) + const prepare = f.h.dependencies.routeSessions.preparePage.getMockImplementation()! + f.h.dependencies.routeSessions.preparePage.mockImplementationOnce(async (input) => { + started() + await gate + return prepare(input) + }) + const switching = f.leases.switchClientPageProfile(f.authority, isolated) + await entered + expect(f.leases.getPlacement('page-a')).toBe(f.placement) + expect(f.h.executor.hasPage('page-a', f.placement.pageHostGeneration)).toBe(true) + expect(f.bindReplacement).not.toHaveBeenCalled() + expect(() => + f.leases.issueClientPageCommand(f.authority, { + type: 'navigate', + url: 'https://example.com/' + }) + ).toThrow('profile_switch_pending') + await expect(f.leases.switchClientPageProfile(f.authority, isolated)).rejects.toThrow( + 'reservation_pending' + ) + release() + const next = await switching + expect(f.h.executor.hasPage('page-a', next.pageHostGeneration)).toBe(true) + await f.dispatcher.close() + await f.h.executor.close() + }) + + it.each([ + [true, isolated], + [false, undefined] + ])( + 'recreates a host-owned profile page with its metadata (desktop supports switching: %s)', + async (supported, expected) => { + setBrowserHostClientProfileResolver((id) => (id === isolated.id ? isolated : undefined)) + try { + const f = await fixture(supported) + await f.leases.createClientPage({ + browserPageId: 'page-b', + browserHostClientId: f.authority.browserHostClientId, + pairedDeviceId: f.authority.pairedDeviceId, + browserProfileId: isolated.id, + executionHostKey: 'execution-host-a' + }) + const create = f.delivered.mock.calls.at(-1)![0] + expect(create.command).toMatchObject({ + type: 'createPage', + browserProfileId: isolated.id + }) + expect(create.command.profile).toEqual(expected) + expect( + f.h.dependencies.routeSessions.preparePage.mock.calls.at(-1)![0].profileMetadata + ).toEqual(expected) + await f.h.executor.close() + } finally { + setBrowserHostClientProfileResolver(null) + } + } + ) + + it('releases download transfers of the replaced generation once the switch commits', async () => { + const released = vi.fn() + const f = await fixture(true, released) + await f.leases.switchClientPageProfile(f.authority, isolated) + expect(released).toHaveBeenCalledWith('page-a', f.placement.pageHostGeneration) + await f.h.executor.close() + }) + + it('admits file transfers from the staged guest only while its switch is pending', async () => { + const f = await fixture() + const connection = (pageHostGeneration: number, admitPendingProfileSwitch?: boolean) => + f.leases.requireClientPageConnection({ + browserPageId: 'page-a', + placement: { ...f.placement, pageHostGeneration }, + pairedDeviceId: f.authority.pairedDeviceId, + connectionId: f.authority.connectionId, + admitPendingProfileSwitch + }) + let stagedGeneration = 0 + f.bindReplacement.mockImplementation(({ registration }) => { + stagedGeneration = registration.pageHostGeneration + expect(connection(stagedGeneration, true)).toMatchObject({ + pageHostGeneration: stagedGeneration + }) + expect(() => connection(stagedGeneration)).toThrow('browser_page_placement_stale') + }) + await f.leases.switchClientPageProfile(f.authority, isolated) + expect(f.bindReplacement).toHaveBeenCalledOnce() + expect(() => connection(f.placement.pageHostGeneration, true)).toThrow( + 'browser_page_placement_stale' + ) + expect(connection(stagedGeneration)).toMatchObject({ pageHostGeneration: stagedGeneration }) + await f.h.executor.close() + }) + + it('rejects arbitrary partition metadata instead of admitting a local cookie jar', () => { + expect(() => + BrowserClientProfileMetadata.parse({ + ...isolated, + partition: 'persist:local-default' + }) + ).toThrow() + expect(() => BrowserClientProfileMetadata.parse({ ...isolated, scope: 'unknown' })).toThrow() + expect(() => BrowserClientProfileMetadata.parse({ ...isolated, id: '' })).toThrow() + }) +}) diff --git a/src/main/runtime/browser-host/browser-host-client-profile-switch.ts b/src/main/runtime/browser-host/browser-host-client-profile-switch.ts new file mode 100644 index 00000000000..e002b06d98c --- /dev/null +++ b/src/main/runtime/browser-host/browser-host-client-profile-switch.ts @@ -0,0 +1,178 @@ +import { + BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY, + BrowserClientProfileMetadata +} from '../../../shared/browser-client-profile-switch-protocol' +import type { BrowserClientHostCommandResult } from '../../../shared/browser-client-host-protocol' +import type { BrowserHostLeaseRegistry } from './browser-host-lease-registry' +import type { RuntimeBrowserClientPlacement } from '../../../shared/runtime-browser-placement' +import type { BrowserHostLeaseState } from './browser-host-lease-records' +import type { + BrowserClientPageAuthority, + BrowserHostPagePlacementRegistry +} from './browser-host-page-placement' +import { + clientPageCreationTimeout, + waitForClientPageCreationResult, + closeUncommittedClientPage, + type BrowserClientPageExecutionHostGrant +} from './browser-host-client-page-creation' + +const pendingByPlacements = new WeakMap< + BrowserHostPagePlacementRegistry, + Map +>() + +/** Replacement placements of switches still waiting on the client, by page. */ +export function pendingBrowserHostProfileSwitches( + placements: BrowserHostPagePlacementRegistry +): Map { + let pending = pendingByPlacements.get(placements) + if (!pending) { + pending = new Map() + pendingByPlacements.set(placements, pending) + } + return pending +} + +export async function switchBrowserHostClientProfile( + authority: BrowserClientPageAuthority, + metadata: BrowserClientProfileMetadata, + input: { + timeoutMs?: number + registry: Pick + state: (clientId: string) => BrowserHostLeaseState + placements: BrowserHostPagePlacementRegistry + grants: Map + /** The client retired the old guest, so its transfers can never finish under the new generation. */ + onCommitted?: (browserPageId: string, replacedPageHostGeneration: number) => void + /** A switch the caller already saw time out, committed once the client finished it. */ + onLateCommit?: (placement: RuntimeBrowserClientPlacement) => void + } +) { + const timeoutMs = clientPageCreationTimeout(input.timeoutMs) + const profile = BrowserClientProfileMetadata.parse(metadata) + const expected = input.registry.requireClientPage(authority) + const lease = input.registry.select(expected.browserHostClientId, []) + if ( + !lease.hostCapabilities.includes(BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY) || + lease.browserHostGeneration !== expected.browserHostGeneration || + lease.pageReconciliationProtocolVersion !== 1 + ) { + throw new Error('browser_host_profile_switch_unsupported') + } + const state = input.state(expected.browserHostClientId) + const ledger = state.commandLedger + const grant = input.grants.get(authority.browserPageId) + if (!ledger || !grant || grant.placement !== expected) { + throw new Error('browser_page_placement_stale') + } + const reservation = input.placements.reserveNewClientPage(authority.browserPageId, expected) + let completed = false + let resultPending: Promise | undefined + const retirePlacement = () => { + if (input.placements.getPlacement(authority.browserPageId) === expected) { + input.placements.completePageRetirement( + input.placements.beginPageRetirement(authority.browserPageId, expected) + ) + } + } + // Why: a held grant blocks recreating the page once its placement is retired. + const releaseGrant = () => { + if (input.grants.get(authority.browserPageId) === grant) { + input.grants.delete(authority.browserPageId) + grant.release() + } + } + const closeSettledReplacement = () => { + try { + ledger.advancePageGeneration( + authority.browserPageId, + expected.pageHostGeneration, + reservation.placement.pageHostGeneration + ) + closeUncommittedClientPage(state, authority.browserPageId, reservation.placement) + } catch { + // A fenced lease or a newer reconciliation owns cleanup now. + } + } + const pending = pendingBrowserHostProfileSwitches(input.placements) + pending.set(authority.browserPageId, reservation.placement) + const forgetPending = () => { + if (pending.get(authority.browserPageId) === reservation.placement) { + pending.delete(authority.browserPageId) + } + } + const commit = () => { + input.registry.requireClientPage(authority) + const placement = input.placements.commitClientPageReplacement(reservation, expected) + ledger.advancePageGeneration( + authority.browserPageId, + expected.pageHostGeneration, + placement.pageHostGeneration + ) + input.grants.set(authority.browserPageId, { ...grant, placement }) + input.onCommitted?.(authority.browserPageId, expected.pageHostGeneration) + return placement + } + // The client retired the old guest once its switch completed, so that placement is gone. + const retireSwitchedPage = () => { + input.placements.cancelClientPageReservation(reservation) + retirePlacement() + closeSettledReplacement() + releaseGrant() + } + try { + const issued = ledger.issue({ + browserPageId: authority.browserPageId, + pageHostGeneration: expected.pageHostGeneration, + resultAdmission: 'reserved-page', + command: { + type: 'switchProfile', + profile, + nextPageHostGeneration: reservation.placement.pageHostGeneration + } + }) + resultPending = issued.result + const result = await waitForClientPageCreationResult(issued.result, timeoutMs) + resultPending = undefined + completed = result.status === 'completed' + if (result.status === 'failed') { + throw new Error(result.errorCode) + } + return commit() + } catch (error) { + if (resultPending) { + // Why: the client still decides; a late success is adopted, a late failure keeps the old page. + void resultPending + .then( + (result) => { + if (result.status !== 'completed') { + input.placements.cancelClientPageReservation(reservation) + return + } + try { + const placement = commit() + input.onLateCommit?.(placement) + } catch { + retireSwitchedPage() + } + }, + () => { + input.placements.cancelClientPageReservation(reservation) + retirePlacement() + releaseGrant() + } + ) + .finally(forgetPending) + } else if (completed) { + retireSwitchedPage() + } else { + input.placements.cancelClientPageReservation(reservation) + } + throw error + } finally { + if (!resultPending) { + forgetPending() + } + } +} diff --git a/src/main/runtime/browser-host/browser-host-command-ledger.ts b/src/main/runtime/browser-host/browser-host-command-ledger.ts index 7e6cc56c5ed..f9f34b7840b 100644 --- a/src/main/runtime/browser-host/browser-host-command-ledger.ts +++ b/src/main/runtime/browser-host/browser-host-command-ledger.ts @@ -113,9 +113,18 @@ export class BrowserHostCommandLedger { if (page.outstanding >= this.maxOutstandingCommandsPerPage) { throw new Error('browser_host_page_command_capacity') } + if (page.profileSwitchPending) { + throw new Error('browser_host_profile_switch_pending') + } + if (command.type === 'switchProfile' && page.outstanding !== 0) { + throw new Error('browser_host_profile_switch_pending') + } assertBrowserHostCommandOrder(page, command, input.resultAdmission ?? 'placed-page') admission.commit() recordBrowserHostCommandOrder(page, command) + if (command.type === 'switchProfile') { + page.profileSwitchPending = true + } const commandSequence = page.nextIssueSequence const event = Object.freeze({ type: 'command' as const, @@ -160,6 +169,16 @@ export class BrowserHostCommandLedger { assertBrowserHostCommandResultAuthority(this.authority, params) const page = this.pages.get(params.browserPageId) if (!page || page.generation !== params.pageHostGeneration) { + const previous = page?.records.get(params.commandSequence) + if ( + page && + previous?.event.command.type === 'switchProfile' && + previous.settled && + previous.event.pageHostGeneration === params.pageHostGeneration && + params.pageHostGeneration < page.generation + ) { + return replaySettledBrowserHostCommand(page, params) + } throw new Error('browser_host_command_result_page_stale') } if (params.commandSequence < page.nextSettlementSequence) { @@ -180,6 +199,9 @@ export class BrowserHostCommandLedger { this.outstandingCommands -= 1 page.settledSequences.push(params.commandSequence) this.resultCache.remember(page, record) + if (record.event.command.type === 'switchProfile' && result.status === 'failed') { + page.profileSwitchPending = false + } if (record.event.command.type === 'closePage' && result.status === 'failed') { page.terminalCommandIssued = false } else if (record.event.command.type === 'closePage' && !page.activeCapacityReleased) { @@ -189,6 +211,15 @@ export class BrowserHostCommandLedger { return true } + advancePageGeneration(browserPageId: string, previous: number, next: number): void { + const page = this.pages.get(browserPageId) + if (!page || page.generation !== previous || next <= previous || page.outstanding !== 0) { + throw new Error('browser_host_command_page_stale') + } + page.generation = next + page.profileSwitchPending = false + } + close(): void { if (this.closed) { return diff --git a/src/main/runtime/browser-host/browser-host-command-result-replay.ts b/src/main/runtime/browser-host/browser-host-command-result-replay.ts index 46d9390d8ec..ac9f5a93082 100644 --- a/src/main/runtime/browser-host/browser-host-command-result-replay.ts +++ b/src/main/runtime/browser-host/browser-host-command-result-replay.ts @@ -24,9 +24,17 @@ export function isBrowserHostUnplacedPageResult( const page = pages.get(params.browserPageId) const record = page?.records.get(params.commandSequence) return ( - page?.generation === params.pageHostGeneration && - record?.event.commandId === params.commandId && - record.resultAdmission !== 'placed-page' + (page?.generation === params.pageHostGeneration && + record?.event.commandId === params.commandId && + record.resultAdmission !== 'placed-page') || + Boolean( + page && + record?.event.command.type === 'switchProfile' && + record.settled && + record.event.pageHostGeneration === params.pageHostGeneration && + record.event.commandId === params.commandId && + params.pageHostGeneration < page.generation + ) ) } diff --git a/src/main/runtime/browser-host/browser-host-command-state.ts b/src/main/runtime/browser-host/browser-host-command-state.ts index 65d3cc519dc..9b95e18dadc 100644 --- a/src/main/runtime/browser-host/browser-host-command-state.ts +++ b/src/main/runtime/browser-host/browser-host-command-state.ts @@ -42,6 +42,7 @@ export type BrowserHostCommandPageState = { records: Map outstanding: number settledSequences: number[] + profileSwitchPending?: boolean terminalCommandIssued: boolean activeCapacityReleased: boolean } @@ -92,6 +93,18 @@ export function recordBrowserHostCommandOrder( export function snapshotBrowserHostPageCommand( command: BrowserClientHostCommandEvent['command'] ): BrowserClientHostCommandEvent['command'] { + if (command.type === 'switchProfile') { + return Object.freeze({ + ...command, + profile: Object.freeze({ ...command.profile }) + }) + } + if ((command.type === 'createPage' || command.type === 'restorePage') && command.profile) { + return Object.freeze({ + ...command, + profile: Object.freeze({ ...command.profile }) + }) + } if (command.type === 'reclaimPage') { return Object.freeze({ ...command, diff --git a/src/main/runtime/browser-host/browser-host-lease-registry-instance.ts b/src/main/runtime/browser-host/browser-host-lease-registry-instance.ts index ebfc3473887..2c68463e0e5 100644 --- a/src/main/runtime/browser-host/browser-host-lease-registry-instance.ts +++ b/src/main/runtime/browser-host/browser-host-lease-registry-instance.ts @@ -14,8 +14,12 @@ export function getBrowserHostLeaseRegistry( if (!registry) { registry = new BrowserHostLeaseRegistry({ authorityRuntimeId: runtime.getRuntimeId(), - onClientPageReleased: (browserPageId) => { - void releaseBrowserClientDownloadTransfersForPage(runtime, browserPageId).catch((error) => { + onClientPageReleased: (browserPageId, pageHostGeneration) => { + void releaseBrowserClientDownloadTransfersForPage( + runtime, + browserPageId, + pageHostGeneration + ).catch((error) => { console.warn('[browser-host-lease] download transfer cleanup failed:', error) }) }, diff --git a/src/main/runtime/browser-host/browser-host-lease-registry.ts b/src/main/runtime/browser-host/browser-host-lease-registry.ts index f50f7f9faa3..c2a56a926d1 100644 --- a/src/main/runtime/browser-host/browser-host-lease-registry.ts +++ b/src/main/runtime/browser-host/browser-host-lease-registry.ts @@ -1,3 +1,4 @@ +import { switchBrowserHostClientProfile } from './browser-host-client-profile-switch' import { randomUUID } from 'node:crypto' import type { BrowserClientHostCommandEvent, @@ -28,10 +29,7 @@ import type { import { requireLiveBrowserClientPage } from './browser-host-page-authority' import type { BrowserHostFenceReason } from './browser-host-lease-fence' import { dispatchBrowserHostLeaseFence } from './browser-host-lease-fence-dispatch' -import { - BROWSER_HOST_WEBVIEW_CAPABILITY, - selectBrowserHostLease -} from './browser-host-capability-selection' +import { selectBrowserHostLease } from './browser-host-capability-selection' import { createBrowserHostClientPage, type BrowserClientPageExecutionHostGrant, @@ -77,7 +75,8 @@ export class BrowserHostLeaseRegistry { * even when the client transport is already gone, so runtime-side per-page state (staged * download transfers) is not stranded. */ - onClientPageReleased?: (browserPageId: string) => void + /** A generation limits the release to that page generation's resources. */ + onClientPageReleased?: (browserPageId: string, pageHostGeneration?: number) => void /** * Called when a lease fence takes a client page's placement away. The page itself outlives * the host that placed it, so this hands the runtime its record back to retain rather than @@ -153,6 +152,8 @@ export class BrowserHostLeaseRegistry { placement: RuntimeBrowserClientPlacement pairedDeviceId: string connectionId: string + /** File transfers only: a staged profile-switch guest downloads before its generation commits. */ + admitPendingProfileSwitch?: boolean }): RuntimeBrowserClientPlacement { return requireBrowserClientPageConnection(input, { authorityRuntimeId: this.authorityRuntimeId, @@ -208,17 +209,28 @@ export class BrowserHostLeaseRegistry { options: BrowserHostClientPageCreateOptions ): Promise { return createBrowserHostClientPage(options, { - selectLease: (browserHostClientId, requiredCapabilities) => - this.select(browserHostClientId, [ - BROWSER_HOST_WEBVIEW_CAPABILITY, - ...requiredCapabilities - ]), + selectLease: (clientId, capabilities) => this.select(clientId, capabilities), requireLeaseState: (lease) => this.requireLeaseState(lease), pagePlacements: this.pagePlacements, executionHostGrants: this.clientPageExecutionHostGrants }) } + switchClientPageProfile( + authority: BrowserClientPageAuthority, + profile: Parameters[1], + onLateCommit?: (placement: RuntimeBrowserClientPlacement) => void + ): Promise { + return switchBrowserHostClientProfile(authority, profile, { + onLateCommit, + registry: this, + state: (clientId) => this.requireLeaseState(this.select(clientId, [])), + placements: this.pagePlacements, + grants: this.clientPageExecutionHostGrants, + onCommitted: this.options.onClientPageReleased + }) + } + requireClientPage(authority: BrowserClientPageAuthority): RuntimeBrowserClientPlacement { return requireLiveBrowserClientPage(this.pagePlacements, this.leasesByClientId, authority) } diff --git a/src/main/runtime/browser-host/browser-host-page-command-admission.ts b/src/main/runtime/browser-host/browser-host-page-command-admission.ts index 9b73c3b3549..19cdbaa8369 100644 --- a/src/main/runtime/browser-host/browser-host-page-command-admission.ts +++ b/src/main/runtime/browser-host/browser-host-page-command-admission.ts @@ -1,3 +1,4 @@ +import { BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY } from '../../../shared/browser-client-profile-switch-protocol' import type { BrowserClientHostCommandEvent } from '../../../shared/browser-client-host-protocol' import { BROWSER_CLIENT_AUTOMATION_HOST_CAPABILITY } from '../../../shared/browser-client-automation-protocol' import { BROWSER_CLIENT_FILE_CHANNEL_HOST_CAPABILITY } from '../../../shared/browser-client-file-channel-protocol' @@ -12,6 +13,12 @@ export function assertBrowserHostPageCommandAdmission( command: BrowserClientHostCommandEvent['command'], requireExecutionHost: (executionHostKey: string) => void ): void { + if ( + command.type === 'switchProfile' && + !lease.hostCapabilities.includes(BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY) + ) { + throw new Error('browser_host_profile_switch_unsupported') + } if ( command.type !== 'createPage' && command.type !== 'navigate' && diff --git a/src/main/runtime/browser-host/browser-host-page-placement.ts b/src/main/runtime/browser-host/browser-host-page-placement.ts index 2990c206dd7..ad00e6abcf2 100644 --- a/src/main/runtime/browser-host/browser-host-page-placement.ts +++ b/src/main/runtime/browser-host/browser-host-page-placement.ts @@ -138,6 +138,24 @@ export class BrowserHostPagePlacementRegistry { return reservation.placement } + commitClientPageReplacement( + reservation: BrowserClientPagePlacementReservation, + expected: RuntimeBrowserClientPlacement + ): RuntimeBrowserClientPlacement { + const state = this.placementsByPageId.get(reservation.browserPageId) + if ( + this.reservationsByPageId.get(reservation.browserPageId) !== reservation || + state?.placement !== expected || + state.retirement + ) { + throw new Error('browser_page_placement_stale') + } + this.reservationsByPageId.delete(reservation.browserPageId) + this.releaseReservationSlot(reservation) + this.placementsByPageId.set(reservation.browserPageId, { placement: reservation.placement }) + return reservation.placement + } + cancelClientPageReservation(reservation: BrowserClientPagePlacementReservation): boolean { if (this.reservationsByPageId.get(reservation.browserPageId) !== reservation) { return false diff --git a/src/main/runtime/browser-host/browser-host-page-reconciliation-actions.ts b/src/main/runtime/browser-host/browser-host-page-reconciliation-actions.ts index b97767f904e..3f9961e7465 100644 --- a/src/main/runtime/browser-host/browser-host-page-reconciliation-actions.ts +++ b/src/main/runtime/browser-host/browser-host-page-reconciliation-actions.ts @@ -4,6 +4,7 @@ import type { BrowserClientHostCommandResult } from '../../../shared/browser-client-host-protocol' import { assertBrowserHostPageCommandAdmission } from './browser-host-page-command-admission' +import { browserHostClientProfileFor } from './browser-host-client-profile-metadata' import type { BrowserHostLeaseState } from './browser-host-lease-records' import type { BrowserClientPagePlacementReservation, @@ -124,6 +125,7 @@ export class BrowserHostPageReconciliationActions { { type: 'restorePage', browserProfileId: intent.browserProfileId, + ...browserHostClientProfileFor(this.state.lease, intent.browserProfileId), executionHostKey: intent.executionHostKey, ...(url ? { url } : {}), ...(intent.workspaceId ? { workspaceId: intent.workspaceId } : {}) @@ -182,7 +184,9 @@ function pageAuthority(page: BrowserClientHostedPageInventory) { function assertNotAborted(signal: AbortSignal): void { if (signal.aborted) { - throw new Error('browser_host_page_reconciliation_aborted', { cause: signal.reason }) + throw new Error('browser_host_page_reconciliation_aborted', { + cause: signal.reason + }) } } @@ -193,7 +197,11 @@ async function waitForCommandResult( let removeAbort = (): void => {} const aborted = new Promise((_resolve, reject) => { const abort = (): void => - reject(new Error('browser_host_page_reconciliation_aborted', { cause: signal.reason })) + reject( + new Error('browser_host_page_reconciliation_aborted', { + cause: signal.reason + }) + ) if (signal.aborted) { abort() return diff --git a/src/main/runtime/browser-host/browser-host-profile-switch-recovery.test.ts b/src/main/runtime/browser-host/browser-host-profile-switch-recovery.test.ts new file mode 100644 index 00000000000..9c912519287 --- /dev/null +++ b/src/main/runtime/browser-host/browser-host-profile-switch-recovery.test.ts @@ -0,0 +1,173 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { switchBrowserHostClientProfile } from './browser-host-client-profile-switch' +import { BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY } from '../../../shared/browser-client-profile-switch-protocol' + +afterEach(() => vi.useRealTimers()) + +function recoveryFixture( + result: Promise<{ status: 'completed' } | { status: 'failed'; errorCode: string }> +) { + const expected = { + browserHostClientId: 'client', + browserHostGeneration: 1, + pageHostGeneration: 1 + } + const replacement = { ...expected, pageHostGeneration: 2 } + const ledger = { + issue: vi + .fn() + .mockReturnValueOnce({ result }) + .mockReturnValue({ result: Promise.resolve({ status: 'completed' }) }), + retirePage: vi.fn(), + advancePageGeneration: vi.fn() + } + const grant = { + placement: expected, + release: vi.fn(), + executionHostKey: 'host' + } + const placements = { + getPlacement: vi.fn().mockReturnValue(expected), + beginPageRetirement: vi.fn().mockReturnValue({}), + completePageRetirement: vi.fn(), + reserveNewClientPage: vi.fn().mockReturnValue({ placement: replacement }), + commitClientPageReplacement: vi.fn().mockImplementation(() => { + throw new Error('browser_page_placement_stale') + }), + cancelClientPageReservation: vi.fn() + } + const grants = new Map([['page', grant]]) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: switchBrowserHostClientProfile calls only the ledger, placement and lease members mocked here. + const input = { + timeoutMs: 10, + registry: { + requireClientPage: vi.fn().mockReturnValue(expected), + select: vi.fn().mockReturnValue({ + ...expected, + hostCapabilities: [BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY], + pageReconciliationProtocolVersion: 1 + }) + }, + state: () => ({ + commandLedger: ledger, + lease: { authorityRuntimeId: 'runtime', authorityEpoch: 'epoch' } + }), + placements, + grants + } as unknown as Parameters[2] + const authority = { + ...expected, + browserPageId: 'page', + authorityRuntimeId: 'runtime', + authorityEpoch: 'epoch' + } + return { input, authority, ledger, placements, grants, grant } +} + +describe('profile switch failure convergence', () => { + it('bounds a stalled result without racing a close against the pending switch', async () => { + vi.useFakeTimers() + const f = recoveryFixture(new Promise(() => {})) + const rejected = expect( + switchBrowserHostClientProfile( + f.authority, + { id: 'isolated', label: 'Isolated', scope: 'isolated' }, + f.input + ) + ).rejects.toThrow('browser_host_page_creation_timeout') + await vi.advanceTimersByTimeAsync(11) + await rejected + // The reservation and placement both wait for the client's verdict. + expect(f.placements.cancelClientPageReservation).not.toHaveBeenCalled() + expect(f.placements.completePageRetirement).not.toHaveBeenCalled() + expect(f.ledger.retirePage).not.toHaveBeenCalled() + expect(f.ledger.issue).toHaveBeenCalledTimes(1) + // The client may still be switching, so the page cannot be recreated under it yet. + expect(f.grants.has('page')).toBe(true) + expect(f.grant.release).not.toHaveBeenCalled() + }) + + it('orders late-success cleanup behind the original result and preserves its sequence', async () => { + vi.useFakeTimers() + let complete!: (result: { status: 'completed' }) => void + const f = recoveryFixture( + new Promise((resolve) => { + complete = resolve + }) + ) + const rejected = expect( + switchBrowserHostClientProfile( + f.authority, + { id: 'isolated', label: 'Isolated', scope: 'isolated' }, + f.input + ) + ).rejects.toThrow('browser_host_page_creation_timeout') + await vi.advanceTimersByTimeAsync(11) + await rejected + expect(f.ledger.issue).toHaveBeenCalledTimes(1) + expect(f.ledger.advancePageGeneration).not.toHaveBeenCalled() + complete({ status: 'completed' }) + await vi.advanceTimersByTimeAsync(0) + expect(f.ledger.advancePageGeneration).toHaveBeenCalledWith('page', 1, 2) + expect(f.ledger.issue).toHaveBeenLastCalledWith( + expect.objectContaining({ + pageHostGeneration: 2, + command: expect.objectContaining({ type: 'closePage' }) + }) + ) + expect(f.ledger.advancePageGeneration.mock.invocationCallOrder[0]).toBeLessThan( + f.ledger.issue.mock.invocationCallOrder[1] + ) + expect(f.placements.completePageRetirement).toHaveBeenCalledOnce() + expect(f.grants.has('page')).toBe(false) + expect(f.grant.release).toHaveBeenCalledOnce() + }) + + it('keeps the original placement when a timed-out switch later fails on the client', async () => { + vi.useFakeTimers() + let settle!: (result: { status: 'failed'; errorCode: string }) => void + const f = recoveryFixture( + new Promise((resolve) => { + settle = resolve + }) + ) + const rejected = expect( + switchBrowserHostClientProfile( + f.authority, + { id: 'isolated', label: 'Isolated', scope: 'isolated' }, + f.input + ) + ).rejects.toThrow('browser_host_page_creation_timeout') + await vi.advanceTimersByTimeAsync(11) + await rejected + settle({ + status: 'failed', + errorCode: 'browser_client_page_navigation_failed' + }) + await vi.advanceTimersByTimeAsync(0) + expect(f.ledger.issue).toHaveBeenCalledTimes(1) + expect(f.placements.completePageRetirement).not.toHaveBeenCalled() + expect(f.grants.has('page')).toBe(true) + expect(f.grant.release).not.toHaveBeenCalled() + }) + + it('closes the completed replacement when the host cannot commit it', async () => { + const f = recoveryFixture(Promise.resolve({ status: 'completed' })) + await expect( + switchBrowserHostClientProfile( + f.authority, + { id: 'isolated', label: 'Isolated', scope: 'isolated' }, + f.input + ) + ).rejects.toThrow('browser_page_placement_stale') + expect(f.ledger.advancePageGeneration).toHaveBeenCalledWith('page', 1, 2) + expect(f.ledger.issue).toHaveBeenLastCalledWith( + expect.objectContaining({ + pageHostGeneration: 2, + command: expect.objectContaining({ type: 'closePage' }) + }) + ) + expect(f.grants.has('page')).toBe(false) + expect(f.grant.release).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/runtime/browser/browser-command-contracts.ts b/src/main/runtime/browser/browser-command-contracts.ts index 65cd2d697cf..089613ae241 100644 --- a/src/main/runtime/browser/browser-command-contracts.ts +++ b/src/main/runtime/browser/browser-command-contracts.ts @@ -1,6 +1,6 @@ import type { AgentBrowserBridge } from '../../browser/agent-browser-bridge' import type { ExecutionHostId } from '../../../shared/execution-host' -import type { BrowserNetworkExecutionHost } from '../../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../../shared/browser-network-execution-host' import type { BrowserHostLeaseRegistry } from '../browser-host/browser-host-lease-registry' import type { RuntimeBrowserPageRegistry } from '../runtime-browser-page-registry' import type { BrowserWindow } from 'electron' diff --git a/src/main/runtime/browser/browser-profiles-client-placement.test.ts b/src/main/runtime/browser/browser-profiles-client-placement.test.ts new file mode 100644 index 00000000000..c15d4e2f347 --- /dev/null +++ b/src/main/runtime/browser/browser-profiles-client-placement.test.ts @@ -0,0 +1,196 @@ +import { BrowserHostLeaseRegistry } from '../browser-host/browser-host-lease-registry' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { RuntimeBrowserPageRegistry } from '../runtime-browser-page-registry' + +const mocks = vi.hoisted(() => ({ + getProfile: vi.fn(), + getDefaultProfile: vi.fn(), + getLocalProfile: vi.fn(), + on: vi.fn(), + removeListener: vi.fn(), + wait: vi.fn() +})) +vi.mock('electron', () => ({ + ipcMain: { on: mocks.on, removeListener: mocks.removeListener } +})) +vi.mock('../../browser/browser-session-registry', () => ({ + browserSessionRegistry: { + getProfile: mocks.getProfile, + getDefaultProfile: mocks.getDefaultProfile + } +})) +vi.mock('../../browser/browser-manager', () => ({ + browserManager: { getSessionProfileIdForTab: mocks.getLocalProfile } +})) +vi.mock('../../ipc/browser-tab-registration-wait', () => ({ + waitForTabRegistration: mocks.wait +})) +import { RuntimeBrowserProfiles } from './browser-profiles' +import { RuntimeBrowserTabCatalog } from './browser-tab-catalog' + +const profiles = [ + { scope: 'default', id: 'default', label: 'Default', partition: 'persist:local-default' }, + { scope: 'isolated', id: 'isolated', label: 'Isolated', partition: 'persist:local-isolated' } +] + +function fixture(currentProfile = 'isolated') { + const pages = new RuntimeBrowserPageRegistry() + pages.publishClientPage({ + browserPageId: 'remote-page', + workspaceId: 'workspace', + browserProfileId: currentProfile, + executionHostKey: 'remote-host', + placement: { + kind: 'client', + browserHostClientId: 'desktop', + browserHostGeneration: 1, + pageHostGeneration: 1 + }, + url: 'https://example.com/authenticated', + loading: false, + active: true + }) + const window = vi.fn(() => { + throw new Error('must not access renderer') + }) + const bridge = vi.fn(() => { + throw new Error('must not require local bridge') + }) + const leases = new BrowserHostLeaseRegistry({ authorityRuntimeId: 'runtime' }) + const switchProfile = vi + .spyOn(leases, 'switchClientPageProfile') + .mockRejectedValue(new Error('requires staged replacement support on both peers')) + const notify = vi.fn() + const localTarget = vi.fn(async () => { + throw new Error('must not resolve a server-local target') + }) + const runtime = new RuntimeBrowserProfiles( + { + getRuntimeBrowserPageRegistry: () => pages, + getAuthoritativeWindow: window, + getBrowserHostLeaseRegistry: () => leases, + notifyHeadlessBrowserSessionTabsChanged: notify + }, + { resolveBrowserCommandTarget: localTarget, requireAgentBrowserBridge: bridge }, + new RuntimeBrowserTabCatalog( + { + getAgentBrowserBridge: () => null, + // Folder selectors resolve here, never through the server-local worktree resolver. + resolveBrowserWorkspace: async (selector) => ({ + id: selector === 'id:folder:workspace' ? 'workspace' : selector + }), + getRuntimeBrowserPageRegistry: () => pages + }, + { requireAgentBrowserBridge: bridge } + ) + ) + return { pages, runtime, window, bridge, switchProfile, notify, localTarget } +} + +describe('client placement profile switch safety', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.getProfile.mockImplementation((id) => profiles.find((profile) => profile.id === id)) + mocks.getDefaultProfile.mockReturnValue(profiles[0]) + mocks.getLocalProfile.mockReturnValue('default') + }) + + it.each([ + ['isolated', 'default'], + ['default', 'isolated'] + ])('rejects %s to %s without mutating the old guest or metadata', async (current, next) => { + const { pages, runtime, window, bridge } = fixture(current) + const before = pages.getPage('remote-page') + await expect( + runtime.browserTabSetProfile({ page: 'remote-page', profileId: next }) + ).rejects.toThrow('requires staged replacement support on both peers') + expect(pages.getPage('remote-page')).toBe(before) + expect(window).not.toHaveBeenCalled() + expect(bridge).not.toHaveBeenCalled() + expect(mocks.getLocalProfile).not.toHaveBeenCalled() + expect(mocks.on).not.toHaveBeenCalled() + expect(mocks.wait).not.toHaveBeenCalled() + }) + + it('commits a proven replacement profile and generation before notifying session readers', async () => { + const { pages, runtime, switchProfile, notify } = fixture() + const previous = pages.getPage('remote-page')! + const replacement = { ...previous.placement, pageHostGeneration: 2 } + switchProfile.mockResolvedValue(replacement) + await expect( + runtime.browserTabSetProfile({ page: 'remote-page', profileId: 'default' }) + ).resolves.toMatchObject({ profileId: 'default' }) + expect(switchProfile).toHaveBeenCalledWith( + expect.objectContaining({ browserPageId: 'remote-page', pageHostGeneration: 1 }), + { id: 'default', label: 'Default', scope: 'default' }, + expect.any(Function) + ) + expect(pages.getPage('remote-page')).toMatchObject({ + browserProfileId: 'default', + placement: replacement, + metadataRevision: 0 + }) + expect(notify).toHaveBeenCalledWith('workspace') + }) + + it('uses authoritative profile identity for an actual no-op', async () => { + const { runtime, window } = fixture() + await expect( + runtime.browserTabSetProfile({ page: 'remote-page', profileId: 'isolated' }) + ).resolves.toEqual({ + browserPageId: 'remote-page', + profileId: 'isolated', + profileLabel: 'Isolated' + }) + expect(mocks.getLocalProfile).not.toHaveBeenCalled() + expect(window).not.toHaveBeenCalled() + }) + + it('resolves the active client page without a server-local bridge', async () => { + const { runtime, bridge } = fixture() + await expect( + runtime.browserTabSetProfile({ worktree: 'workspace', profileId: 'isolated' }) + ).resolves.toMatchObject({ browserPageId: 'remote-page', profileId: 'isolated' }) + expect(bridge).not.toHaveBeenCalled() + }) + + it('rejects an invalid profile without changing authoritative state', async () => { + const { pages, runtime, window } = fixture() + const before = pages.getPage('remote-page') + await expect( + runtime.browserTabSetProfile({ page: 'remote-page', profileId: 'missing' }) + ).rejects.toThrow('Browser profile missing was not found') + expect(pages.getPage('remote-page')).toBe(before) + expect(window).not.toHaveBeenCalled() + }) + + it('does not accept a page from a different explicit workspace', async () => { + const { runtime } = fixture() + await expect( + runtime.browserTabSetProfile({ + page: 'remote-page', + worktree: 'another-workspace', + profileId: 'isolated' + }) + ).rejects.toThrow('was not found in this worktree') + }) + + it('resolves a folder-workspace page without the server-local target path', async () => { + const { runtime, localTarget, window } = fixture() + await expect( + runtime.browserTabSetProfile({ worktree: 'id:folder:workspace', profileId: 'isolated' }) + ).resolves.toMatchObject({ browserPageId: 'remote-page', profileId: 'isolated' }) + expect(localTarget).not.toHaveBeenCalled() + expect(window).not.toHaveBeenCalled() + }) + + it('normalizes synthetic default before comparing the authoritative identity', async () => { + const canonical = { id: 'canonical-default', label: 'Default', partition: 'persist:canonical' } + mocks.getProfile.mockReturnValue(undefined) + mocks.getDefaultProfile.mockReturnValue(canonical) + const { runtime } = fixture('canonical-default') + await expect( + runtime.browserTabSetProfile({ page: 'remote-page', profileId: 'default' }) + ).resolves.toMatchObject({ profileId: 'canonical-default' }) + }) +}) diff --git a/src/main/runtime/browser/browser-profiles.ts b/src/main/runtime/browser/browser-profiles.ts index f586d259be9..053716f2584 100644 --- a/src/main/runtime/browser/browser-profiles.ts +++ b/src/main/runtime/browser/browser-profiles.ts @@ -27,15 +27,25 @@ import type { } from './browser-command-contracts' import type { RuntimeBrowserCommandTargets } from './browser-command-targets' import type { RuntimeBrowserTabCatalog } from './browser-tab-catalog' +import type { RuntimeBrowserClientPlacement } from '../../../shared/runtime-browser-placement' export class RuntimeBrowserProfiles { constructor( - private readonly host: Pick, + private readonly host: Pick< + RuntimeBrowserCommandHost, + | 'getAuthoritativeWindow' + | 'getRuntimeBrowserPageRegistry' + | 'getBrowserHostLeaseRegistry' + | 'notifyHeadlessBrowserSessionTabsChanged' + >, private readonly targets: Pick< RuntimeBrowserCommandTargets, 'requireAgentBrowserBridge' | 'resolveBrowserCommandTarget' >, - private readonly catalog: Pick + private readonly catalog: Pick< + RuntimeBrowserTabCatalog, + 'describeBrowserTab' | 'resolveClientHostedBrowserPage' + > ) {} async browserTabSetProfile( @@ -43,13 +53,6 @@ export class RuntimeBrowserProfiles { profileId: string } & BrowserCommandTargetParams ): Promise { - const target = await this.targets.resolveBrowserCommandTarget(params) - const browserPageId = - target.browserPageId ?? - this.targets.requireAgentBrowserBridge().getActivePageId(target.worktreeId) - if (!browserPageId) { - throw new BrowserError('browser_no_tab', 'No browser tab open in this worktree') - } // Why: 'default' is a synthetic id; fall back to the registry's default profile when not registered. const profile = browserSessionRegistry.getProfile(params.profileId) ?? @@ -61,6 +64,51 @@ export class RuntimeBrowserProfiles { ) } + // Why first: the server-local target path cannot resolve folder workspaces or remote guests. + const clientPage = await this.catalog.resolveClientHostedBrowserPage(params) + if (clientPage) { + const { browserPageId } = clientPage + if (clientPage.browserProfileId === profile.id) { + return { browserPageId, profileId: profile.id, profileLabel: profile.label } + } + const authority = this.host.getBrowserHostLeaseRegistry() + const publish = (placement: RuntimeBrowserClientPlacement): void => { + this.host + .getRuntimeBrowserPageRegistry() + .replaceClientPagePlacement( + browserPageId, + clientPage.placement, + placement, + undefined, + profile.id + ) + this.host.notifyHeadlessBrowserSessionTabsChanged?.(clientPage.workspaceId) + } + const placement = await authority.switchClientPageProfile( + { + authorityRuntimeId: authority.authorityRuntimeId, + authorityEpoch: authority.authorityEpoch, + browserPageId, + browserHostClientId: clientPage.placement.browserHostClientId, + browserHostGeneration: clientPage.placement.browserHostGeneration, + pageHostGeneration: clientPage.placement.pageHostGeneration + }, + { id: profile.id, label: profile.label, scope: profile.scope }, + // Why: a switch that finished after the caller's timeout still moved the page. + publish + ) + publish(placement) + return { browserPageId, profileId: profile.id, profileLabel: profile.label } + } + + const target = await this.targets.resolveBrowserCommandTarget(params) + const browserPageId = + target.browserPageId ?? + this.targets.requireAgentBrowserBridge().getActivePageId(target.worktreeId) + if (!browserPageId) { + throw new BrowserError('browser_no_tab', 'No browser tab open in this worktree') + } + // Why: short-circuit no-op switches so the renderer doesn't needlessly tear down and remount the webview. const currentProfileId = browserManager.getSessionProfileIdForTab(browserPageId) ?? 'default' if (currentProfileId === profile.id) { diff --git a/src/main/runtime/rpc/methods/browser-client-file-channel.ts b/src/main/runtime/rpc/methods/browser-client-file-channel.ts index c412b7abe7a..1d1a388963b 100644 --- a/src/main/runtime/rpc/methods/browser-client-file-channel.ts +++ b/src/main/runtime/rpc/methods/browser-client-file-channel.ts @@ -45,7 +45,8 @@ function requireFileChannelPage( browserPageId: params.browserPageId, placement, pairedDeviceId, - connectionId + connectionId, + admitPendingProfileSwitch: true }) if ( leases.requireLease({ diff --git a/src/main/runtime/runtime-browser-client-page-creation.ts b/src/main/runtime/runtime-browser-client-page-creation.ts index d2f65a7fa08..992cbda868c 100644 --- a/src/main/runtime/runtime-browser-client-page-creation.ts +++ b/src/main/runtime/runtime-browser-client-page-creation.ts @@ -1,4 +1,4 @@ -import type { BrowserNetworkExecutionHost } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import { BROWSER_CLIENT_AUTOMATION_HOST_CAPABILITY } from '../../shared/browser-client-automation-protocol' import { browserNetworkExecutionHostKey } from '../browser/browser-network-execution-route' import type { BrowserHostLeaseRegistry } from './browser-host/browser-host-lease-registry' diff --git a/src/main/runtime/runtime-browser-network-execution-host.ts b/src/main/runtime/runtime-browser-network-execution-host.ts index 5d053e2a9b9..a074c66f279 100644 --- a/src/main/runtime/runtime-browser-network-execution-host.ts +++ b/src/main/runtime/runtime-browser-network-execution-host.ts @@ -1,4 +1,4 @@ -import type { BrowserNetworkExecutionHost } from '../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../shared/browser-network-execution-host' import { parseExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' import type { SshConnectionState } from '../../shared/ssh-types' diff --git a/src/main/runtime/runtime-browser-page-registry.ts b/src/main/runtime/runtime-browser-page-registry.ts index f32f83dd105..ea30b874fa9 100644 --- a/src/main/runtime/runtime-browser-page-registry.ts +++ b/src/main/runtime/runtime-browser-page-registry.ts @@ -159,18 +159,23 @@ export class RuntimeBrowserPageRegistry { expected: RuntimeBrowserClientPlacement, placement: RuntimeBrowserClientPlacement, /** The route key the replacing host was actually placed under; keys do not survive a restart. */ - executionHostKey?: string + executionHostKey?: string, + browserProfileId?: string ): RuntimeBrowserClientPage { const current = this.requireExactPage(browserPageId, expected) if (executionHostKey !== undefined) { assertIdentity(executionHostKey) } + if (browserProfileId !== undefined) { + assertIdentity(browserProfileId) + } // Why the revision restarts: it is the replacing host's own counter, and a host that just took // this placement over counts from zero. Keeping the old high-water mark deafens the page to // that host until it catches up. const next = freezePage({ ...current, ...(executionHostKey === undefined ? {} : { executionHostKey }), + ...(browserProfileId === undefined ? {} : { browserProfileId }), metadataRevision: 0, placement: Object.freeze({ ...placement }) }) diff --git a/src/main/runtime/terminal/driver/terminal-driver-resources.ts b/src/main/runtime/terminal/driver/terminal-driver-resources.ts index 0fe3a33dac1..3aa1eb0cf72 100644 --- a/src/main/runtime/terminal/driver/terminal-driver-resources.ts +++ b/src/main/runtime/terminal/driver/terminal-driver-resources.ts @@ -4,7 +4,7 @@ import type { ResolvedWorktree } from '../../runtime-worktree-path-identity' import type { ExecutionHostId } from '../../../../shared/execution-host' import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' import type { RuntimeAccountController } from '../../runtime-account-controller' -import type { BrowserNetworkExecutionHost } from '../../../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../../../shared/browser-network-execution-host' import type { BrowserWindow } from 'electron' import type { BrowserSessionTabSelectionOptions } from '../../browser-tab-create-publication' import type { RuntimeStore } from '../../runtime-store-contract' diff --git a/src/main/runtime/terminal/launch/terminal-workspace-scope.ts b/src/main/runtime/terminal/launch/terminal-workspace-scope.ts index 894b3b44398..6e58de9edc3 100644 --- a/src/main/runtime/terminal/launch/terminal-workspace-scope.ts +++ b/src/main/runtime/terminal/launch/terminal-workspace-scope.ts @@ -5,7 +5,7 @@ import { isFloatingWorkspaceSelector } from '../../../../shared/floating-workspace-worktree' import { resolveFloatingTerminalCwd } from '../../../ipc/floating-workspace-directory' -import type { BrowserNetworkExecutionHost } from '../../../../shared/browser-client-host-protocol' +import type { BrowserNetworkExecutionHost } from '../../../../shared/browser-network-execution-host' import { LOCAL_EXECUTION_HOST_ID, getConnectionExecutionHostId, diff --git a/src/renderer/src/components/browser-pane/browser-client-page-profile-staging.test.ts b/src/renderer/src/components/browser-pane/browser-client-page-profile-staging.test.ts new file mode 100644 index 00000000000..0e1a83ac3e5 --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-client-page-profile-staging.test.ts @@ -0,0 +1,28 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it } from 'vitest' +import { + createRetainedHostFixture, + RETAINED_FIXTURE_PAGE +} from './browser-client-page-retained-host-fixture' + +afterEach(() => { + document.body.innerHTML = '' +}) + +describe('retained profile replacement staging', () => { + it('keeps the old visible guest alive while another partition mounts under the same page id', async () => { + const fixture = createRetainedHostFixture() + await fixture.mount() + const old = fixture.attach() + const oldWebview = old.webview + const next = { ...RETAINED_FIXTURE_PAGE, partition: 'persist:route-b', pageHostGeneration: 4 } + await fixture.mount(next) + expect(oldWebview.isConnected).toBe(true) + expect(fixture.registry.getMemoryProfile()).toMatchObject({ attachedPageCount: 2 }) + fixture.registry.retirePage(next) + expect(oldWebview.isConnected).toBe(true) + expect(() => old.nextMetadataRevision()).not.toThrow() + old.detach() + fixture.registry.dispose() + }) +}) diff --git a/src/shared/browser-client-host-protocol.ts b/src/shared/browser-client-host-protocol.ts index 934b468a3d2..cd2d735c7b8 100644 --- a/src/shared/browser-client-host-protocol.ts +++ b/src/shared/browser-client-host-protocol.ts @@ -1,4 +1,9 @@ +import { + BrowserClientProfileMetadata, + BrowserClientProfileSwitchCommand +} from './browser-client-profile-switch-protocol' import { z } from 'zod' +import { BrowserNetworkExecutionHost } from './browser-network-execution-host' import { BrowserClientAutomationCommand, BrowserClientAutomationResult @@ -195,34 +200,6 @@ export const BrowserClientHostLeaseAuthority = BrowserHostLeaseAuthority.extend( export type BrowserClientHostLeaseAuthority = z.infer -const BrowserNetworkNativeExecutionHost = z.object({ - kind: z.literal('native'), - runtimeId: Identity, - revision: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER) -}) - -const BrowserNetworkSshExecutionHost = z.object({ - kind: z.literal('ssh'), - targetId: Identity, - providerEpoch: Identity, - connectionGeneration: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER) -}) - -const BrowserNetworkWslExecutionHost = z.object({ - kind: z.literal('wsl'), - runtimeId: Identity, - revision: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER), - distro: Identity -}) - -export const BrowserNetworkExecutionHost = z.discriminatedUnion('kind', [ - BrowserNetworkNativeExecutionHost, - BrowserNetworkSshExecutionHost, - BrowserNetworkWslExecutionHost -]) - -export type BrowserNetworkExecutionHost = z.infer - const BrowserClientPageCommandAuthority = BrowserClientHostLeaseAuthority.extend({ pageCommandProtocolVersion: PageCommandProtocolVersion, browserPageId: Identity, @@ -234,6 +211,8 @@ const BrowserClientPageCommandAuthority = BrowserClientHostLeaseAuthority.extend const BrowserClientHostCreatePageCommand = z.object({ type: z.literal('createPage'), browserProfileId: Identity, + // Sent only to profile-switch-v1 desktops, for host-owned profiles the desktop does not know. + profile: BrowserClientProfileMetadata.optional(), executionHostKey: Identity, // The client never interprets this; it only echoes it back in the page inventory so a restarted // runtime can rebuild the workspace association it holds nowhere else. @@ -261,12 +240,14 @@ const BrowserClientHostClosePageCommand = z.object({ const BrowserClientHostRestorePageCommand = z.object({ type: z.literal('restorePage'), browserProfileId: Identity, + profile: BrowserClientProfileMetadata.optional(), executionHostKey: Identity, url: z.string().min(1).max(8192).optional(), workspaceId: Identity.optional() }) export const BrowserClientHostPageCommand = z.discriminatedUnion('type', [ + BrowserClientProfileSwitchCommand, BrowserClientHostCreatePageCommand, BrowserClientHostNavigateCommand, BrowserClientHostReclaimPageCommand, @@ -325,7 +306,9 @@ export const BrowserClientHostCommandResultParams = BrowserClientPageCommandAuth result: BrowserClientHostCommandResult }) -export const BrowserClientHostCommandResultAck = z.object({ accepted: z.boolean() }) +export const BrowserClientHostCommandResultAck = z.object({ + accepted: z.boolean() +}) export const BrowserClientHostEvent = z.union([ BrowserClientHostReady, diff --git a/src/shared/browser-client-profile-switch-protocol.ts b/src/shared/browser-client-profile-switch-protocol.ts new file mode 100644 index 00000000000..5722c54be7c --- /dev/null +++ b/src/shared/browser-client-profile-switch-protocol.ts @@ -0,0 +1,17 @@ +import { z } from 'zod' + +export const BROWSER_CLIENT_PROFILE_SWITCH_CAPABILITY = 'profile-switch-v1' as const + +// Only display metadata crosses the boundary; the desktop derives its own scoped partition. +export const BrowserClientProfileMetadata = z.strictObject({ + id: z.string().min(1).max(256), + label: z.string().min(1).max(256), + scope: z.enum(['default', 'isolated', 'imported']) +}) +export type BrowserClientProfileMetadata = z.infer + +export const BrowserClientProfileSwitchCommand = z.object({ + type: z.literal('switchProfile'), + profile: BrowserClientProfileMetadata, + nextPageHostGeneration: z.number().int().min(1).max(0xffff_ffff) +}) diff --git a/src/shared/browser-network-execution-host.ts b/src/shared/browser-network-execution-host.ts new file mode 100644 index 00000000000..3b6c97b5d4e --- /dev/null +++ b/src/shared/browser-network-execution-host.ts @@ -0,0 +1,31 @@ +import { z } from 'zod' + +const Identity = z.string().min(1).max(256) + +const BrowserNetworkNativeExecutionHost = z.object({ + kind: z.literal('native'), + runtimeId: Identity, + revision: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER) +}) + +const BrowserNetworkSshExecutionHost = z.object({ + kind: z.literal('ssh'), + targetId: Identity, + providerEpoch: Identity, + connectionGeneration: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER) +}) + +const BrowserNetworkWslExecutionHost = z.object({ + kind: z.literal('wsl'), + runtimeId: Identity, + revision: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER), + distro: Identity +}) + +export const BrowserNetworkExecutionHost = z.discriminatedUnion('kind', [ + BrowserNetworkNativeExecutionHost, + BrowserNetworkSshExecutionHost, + BrowserNetworkWslExecutionHost +]) + +export type BrowserNetworkExecutionHost = z.infer diff --git a/tests/e2e/paired-client-hosted-profile-cookies.spec.ts b/tests/e2e/paired-client-hosted-profile-cookies.spec.ts new file mode 100644 index 00000000000..46c63326fe3 --- /dev/null +++ b/tests/e2e/paired-client-hosted-profile-cookies.spec.ts @@ -0,0 +1,95 @@ +import { expect, test } from './helpers/orca-app' +import { RuntimeClient } from '../../src/cli/runtime/client' +import { launchHeadlessPairedRuntimeHost } from './helpers/headless-paired-runtime-host' +import { + launchPairedElectronClient, + type PairedElectronClient +} from './helpers/paired-electron-client' +import { + openClientHostedFixturePage, + selectPairedWorktreeGroup, + startClientHostedMarkerFixture, + waitForPairedWorktreeId, + waitForRenderedClientWebview +} from './helpers/client-hosted-browser-fixture' + +test('switches a paired client profile while preserving isolated native cookie jars', async ({ + testRepoPath +}, testInfo) => { + test.setTimeout(240_000) + const fixture = await startClientHostedMarkerFixture() + const host = await launchHeadlessPairedRuntimeHost() + const runtime = new RuntimeClient(host.userDataDir, 30_000) + let client: PairedElectronClient | null = null + try { + await runtime.call('repo.add', { path: testRepoPath, kind: 'git' }) + await runtime.call('terminal.create', { + worktree: `path:${testRepoPath}`, + title: 'Profile cookies' + }) + client = await launchPairedElectronClient(host.offer, testInfo, 'Profile cookies client') + const worktreeId = await waitForPairedWorktreeId(client.page, testRepoPath) + await selectPairedWorktreeGroup(client.page, client.environmentId, worktreeId) + const browser = await openClientHostedFixturePage(client, worktreeId, fixture.markerUrl) + await waitForRenderedClientWebview( + client.page, + { + urlPrefix: fixture.origin, + remotePageId: browser.remotePageId + }, + 'profile fixture did not render' + ) + const guestState = (cookieValue?: string) => + client!.app.evaluate( + async ({ webContents }, args) => { + const guest = webContents + .getAllWebContents() + .find( + (contents) => + contents.getType() === 'webview' && contents.getURL().startsWith(args.origin) + ) + if (!guest) { + return null + } + if (args.cookieValue) { + await guest.session.cookies.set({ + url: args.origin, + name: 'profile-probe', + value: args.cookieValue + }) + } + return { + id: guest.id, + cookies: (await guest.session.cookies.get({ url: args.origin })).map( + (cookie) => cookie.value + ) + } + }, + { origin: fixture.origin, cookieValue } + ) + const before = await guestState('default-cookie') + expect(before?.cookies).toContain('default-cookie') + const created = await runtime.call<{ profile: { id: string } }>('browser.profileCreate', { + label: 'Isolated profile cookie proof', + scope: 'isolated' + }) + const switchTo = (profileId: string) => + runtime.call('browser.tabSetProfile', { + worktree: `path:${testRepoPath}`, + page: browser.remotePageId, + profileId + }) + await switchTo(created.result.profile.id) + await expect.poll(guestState).toMatchObject({ cookies: [] }) + const isolated = await guestState('isolated-cookie') + expect(isolated?.id).not.toBe(before?.id) + await switchTo('default') + await expect.poll(guestState).toMatchObject({ cookies: ['default-cookie'] }) + await switchTo(created.result.profile.id) + await expect.poll(guestState).toMatchObject({ cookies: ['isolated-cookie'] }) + } finally { + await client?.dispose() + await host.dispose() + await fixture.close() + } +})