diff --git a/docs/renderer-memory-profile-2026-06-01.md b/docs/renderer-memory-profile-2026-06-01.md index cedca5a15..d047ee525 100644 --- a/docs/renderer-memory-profile-2026-06-01.md +++ b/docs/renderer-memory-profile-2026-06-01.md @@ -116,3 +116,16 @@ The follow-up changes the order: create the automation visibility lease first, then wait for paint while the pane is actually visible to automation. A renderer-side timeout releases the lease if paint never arrives, so a hung RAF does not pin an inactive browser pane indefinitely. + +## Follow-up: Browser Registration Readiness + +One remaining automation race was the wake path for parked or restored browser +tabs. Runtime browser commands asked the renderer to mount a hidden browser +pane, then waited a fixed 500 ms before reading the agent-browser tab registry. +On slow webview startup, that could still race `registerGuest` and make +agent-browser report no tab even though the tab was in the process of mounting. + +The follow-up extends the existing tab-registration wait from page-specific +creation to worktree/global wake flows. Runtime commands now wait for the +renderer's actual `browser:registerGuest` IPC before routing automation, with +the same timeout fallback used by tab creation. diff --git a/src/main/ipc/browser.test.ts b/src/main/ipc/browser.test.ts index 7ca96e305..60186ad8c 100644 --- a/src/main/ipc/browser.test.ts +++ b/src/main/ipc/browser.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- Why: browser IPC tests share one mocked trust-boundary handler registry plus registration waiters; splitting would duplicate setup and weaken coverage. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -6,6 +7,7 @@ const { registerGuestMock, unregisterGuestMock, getGuestWebContentsIdMock, + getWebContentsIdByTabIdMock, getWorktreeIdForTabMock, openDevToolsMock, setAnnotationViewportBridgeMock, @@ -20,6 +22,7 @@ const { registerGuestMock: vi.fn(), unregisterGuestMock: vi.fn(), getGuestWebContentsIdMock: vi.fn(), + getWebContentsIdByTabIdMock: vi.fn(() => new Map()), getWorktreeIdForTabMock: vi.fn(), openDevToolsMock: vi.fn().mockResolvedValue(true), setAnnotationViewportBridgeMock: vi.fn().mockResolvedValue(true), @@ -48,6 +51,7 @@ vi.mock('../browser/browser-manager', () => ({ registerGuest: registerGuestMock, unregisterGuest: unregisterGuestMock, getGuestWebContentsId: getGuestWebContentsIdMock, + getWebContentsIdByTabId: getWebContentsIdByTabIdMock, getWorktreeIdForTab: getWorktreeIdForTabMock, openDevTools: openDevToolsMock, setAnnotationViewportBridge: setAnnotationViewportBridgeMock, @@ -60,7 +64,9 @@ vi.mock('../browser/browser-manager', () => ({ import { registerBrowserHandlers, setAgentBrowserBridgeRef, - waitForTabRegistration + waitForAnyTabRegistration, + waitForTabRegistration, + waitForWorktreeTabRegistration } from './browser' describe('registerBrowserHandlers', () => { @@ -71,6 +77,8 @@ describe('registerBrowserHandlers', () => { registerGuestMock.mockReset() unregisterGuestMock.mockReset() getGuestWebContentsIdMock.mockReset() + getWebContentsIdByTabIdMock.mockReset() + getWebContentsIdByTabIdMock.mockReturnValue(new Map()) getWorktreeIdForTabMock.mockReset() openDevToolsMock.mockReset() setAnnotationViewportBridgeMock.mockReset() @@ -219,6 +227,71 @@ describe('registerBrowserHandlers', () => { } }) + it('resolves worktree and any-tab registration waiters when a guest registers', async () => { + vi.useFakeTimers() + try { + getWebContentsIdByTabIdMock.mockReturnValue(new Map()) + const worktreeWait = waitForWorktreeTabRegistration('worktree-1', 1000) + const anyWait = waitForAnyTabRegistration(1000) + const settled = Promise.allSettled([worktreeWait, anyWait]) + + registerBrowserHandlers() + + const registerHandler = handleMock.mock.calls.find( + ([channel]) => channel === 'browser:registerGuest' + )?.[1] as ( + event: { sender: Electron.WebContents }, + args: { + browserPageId: string + workspaceId: string + worktreeId: string + webContentsId: number + } + ) => boolean + + const result = registerHandler( + { + sender: { + id: 91, + isDestroyed: () => false, + getType: () => 'window', + getURL: () => 'file:///renderer/index.html' + } as Electron.WebContents + }, + { + browserPageId: 'page-worktree-1', + workspaceId: 'workspace-1', + worktreeId: 'worktree-1', + webContentsId: 123 + } + ) + + expect(result).toBe(true) + await vi.advanceTimersByTimeAsync(1001) + expect(await settled).toEqual([ + { status: 'fulfilled', value: undefined }, + { status: 'fulfilled', value: undefined } + ]) + } finally { + vi.useRealTimers() + } + }) + + it('resolves worktree registration waits immediately when a tab is already registered', async () => { + getWebContentsIdByTabIdMock.mockReturnValue(new Map([['page-1', 123]])) + getWorktreeIdForTabMock.mockReturnValue('worktree-1') + + await expect(waitForWorktreeTabRegistration('worktree-1', 1000)).resolves.toBeUndefined() + + expect(getWorktreeIdForTabMock).toHaveBeenCalledWith('page-1') + }) + + it('resolves any-tab registration waits immediately when a tab is already registered', async () => { + getWebContentsIdByTabIdMock.mockReturnValue(new Map([['page-1', 123]])) + + await expect(waitForAnyTabRegistration(1000)).resolves.toBeUndefined() + }) + it('validates annotation viewport bridge requests before syncing to the guest', async () => { registerBrowserHandlers() diff --git a/src/main/ipc/browser.ts b/src/main/ipc/browser.ts index 826a2eca8..1ea54b09e 100644 --- a/src/main/ipc/browser.ts +++ b/src/main/ipc/browser.ts @@ -42,17 +42,15 @@ let agentBrowserBridgeRef: AgentBrowserBridge | null = null // subsequent commands. Multiple commands can wait for the same page during // startup, so keep all one-shot resolvers keyed by browserPageId. const pendingTabRegistrations = new Map void>>() +const pendingWorktreeTabRegistrations = new Map void>>() +const pendingAnyTabRegistrations = new Set<() => void>() -export function waitForTabRegistration(browserPageId: string, timeoutMs = 8_000): Promise { - if (browserManager.getGuestWebContentsId(browserPageId) !== null) { - return Promise.resolve() - } +function waitForRegistrationSet( + registrationResolvers: Set<() => void>, + timeoutMs: number, + onEmpty: () => void +): Promise { return new Promise((resolve, reject) => { - let registrationResolvers = pendingTabRegistrations.get(browserPageId) - if (!registrationResolvers) { - registrationResolvers = new Set() - pendingTabRegistrations.set(browserPageId, registrationResolvers) - } const resolveRegistration = (): void => { clearTimeout(timer) resolve() @@ -60,7 +58,7 @@ export function waitForTabRegistration(browserPageId: string, timeoutMs = 8_000) const timer = setTimeout(() => { registrationResolvers.delete(resolveRegistration) if (registrationResolvers.size === 0) { - pendingTabRegistrations.delete(browserPageId) + onEmpty() } reject(new Error('Tab registration timed out')) }, timeoutMs) @@ -68,6 +66,65 @@ export function waitForTabRegistration(browserPageId: string, timeoutMs = 8_000) }) } +function resolvePendingRegistrations(registrationResolvers: Set<() => void> | undefined): void { + if (!registrationResolvers) { + return + } + for (const pendingResolve of registrationResolvers) { + pendingResolve() + } +} + +function hasRegisteredTabForWorktree(worktreeId: string): boolean { + for (const browserPageId of browserManager.getWebContentsIdByTabId().keys()) { + if (browserManager.getWorktreeIdForTab(browserPageId) === worktreeId) { + return true + } + } + return false +} + +export function waitForTabRegistration(browserPageId: string, timeoutMs = 8_000): Promise { + if (browserManager.getGuestWebContentsId(browserPageId) !== null) { + return Promise.resolve() + } + let registrationResolvers = pendingTabRegistrations.get(browserPageId) + if (!registrationResolvers) { + registrationResolvers = new Set() + pendingTabRegistrations.set(browserPageId, registrationResolvers) + } + return waitForRegistrationSet(registrationResolvers, timeoutMs, () => { + pendingTabRegistrations.delete(browserPageId) + }) +} + +export function waitForWorktreeTabRegistration( + worktreeId: string | undefined, + timeoutMs = 8_000 +): Promise { + if (!worktreeId) { + return waitForAnyTabRegistration(timeoutMs) + } + if (hasRegisteredTabForWorktree(worktreeId)) { + return Promise.resolve() + } + let registrationResolvers = pendingWorktreeTabRegistrations.get(worktreeId) + if (!registrationResolvers) { + registrationResolvers = new Set() + pendingWorktreeTabRegistrations.set(worktreeId, registrationResolvers) + } + return waitForRegistrationSet(registrationResolvers, timeoutMs, () => { + pendingWorktreeTabRegistrations.delete(worktreeId) + }) +} + +export function waitForAnyTabRegistration(timeoutMs = 8_000): Promise { + if (browserManager.getWebContentsIdByTabId().size > 0) { + return Promise.resolve() + } + return waitForRegistrationSet(pendingAnyTabRegistrations, timeoutMs, () => {}) +} + export function setTrustedBrowserRendererWebContentsId(webContentsId: number | null): void { trustedBrowserRendererWebContentsId = webContentsId } @@ -139,12 +196,14 @@ export function registerBrowserHandlers(): void { agentBrowserBridgeRef.onProcessSwap(args.browserPageId, args.webContentsId, previousWcId) } const pendingResolves = pendingTabRegistrations.get(args.browserPageId) - if (pendingResolves) { - pendingTabRegistrations.delete(args.browserPageId) - for (const pendingResolve of pendingResolves) { - pendingResolve() - } - } + pendingTabRegistrations.delete(args.browserPageId) + resolvePendingRegistrations(pendingResolves) + const pendingWorktreeResolves = pendingWorktreeTabRegistrations.get(args.worktreeId) + pendingWorktreeTabRegistrations.delete(args.worktreeId) + resolvePendingRegistrations(pendingWorktreeResolves) + const pendingAnyResolves = new Set(pendingAnyTabRegistrations) + pendingAnyTabRegistrations.clear() + resolvePendingRegistrations(pendingAnyResolves) return true } ) diff --git a/src/main/runtime/orca-runtime-browser.test.ts b/src/main/runtime/orca-runtime-browser.test.ts index 6a3de3816..59744dc79 100644 --- a/src/main/runtime/orca-runtime-browser.test.ts +++ b/src/main/runtime/orca-runtime-browser.test.ts @@ -1,11 +1,13 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentBrowserBridge } from '../browser/agent-browser-bridge' import type { RuntimeBrowserCommandHost } from './orca-runtime-browser' -const { webContentsFromIdMock, startBrowserScreencastMock } = vi.hoisted(() => ({ - webContentsFromIdMock: vi.fn(), - startBrowserScreencastMock: vi.fn() -})) +const { webContentsFromIdMock, startBrowserScreencastMock, waitForWorktreeTabRegistrationMock } = + vi.hoisted(() => ({ + webContentsFromIdMock: vi.fn(), + startBrowserScreencastMock: vi.fn(), + waitForWorktreeTabRegistrationMock: vi.fn() + })) vi.mock('electron', () => ({ ipcMain: { on: vi.fn(), removeListener: vi.fn() }, @@ -16,6 +18,11 @@ vi.mock('../browser/browser-screencast-stream', () => ({ startBrowserScreencast: startBrowserScreencastMock })) +vi.mock('../ipc/browser', () => ({ + waitForTabRegistration: vi.fn(), + waitForWorktreeTabRegistration: waitForWorktreeTabRegistrationMock +})) + function deferred() { let resolve!: (value: T) => void let reject!: (reason?: unknown) => void @@ -26,31 +33,83 @@ function deferred() { return { promise, resolve, reject } } -function createHost(): RuntimeBrowserCommandHost { - const bridge = { - getRegisteredTabs: vi.fn(() => new Map([['page-1', 100]])), - getActivePageId: vi.fn(() => 'page-1'), - tabList: vi.fn(() => ({ - tabs: [ - { - browserPageId: 'page-1', - index: 0, - url: 'about:blank', - title: 'Browser', - active: true - } - ] - })) - } as unknown as AgentBrowserBridge +function createHost(overrides: Partial = {}): RuntimeBrowserCommandHost { + const bridge = + overrides.getAgentBrowserBridge?.() ?? + ({ + getRegisteredTabs: vi.fn(() => new Map([['page-1', 100]])), + getActivePageId: vi.fn(() => 'page-1'), + tabList: vi.fn(() => ({ + tabs: [ + { + browserPageId: 'page-1', + index: 0, + url: 'about:blank', + title: 'Browser', + active: true + } + ] + })) + } as unknown as AgentBrowserBridge) return { - getAgentBrowserBridge: () => bridge, resolveWorktreeSelector: async (selector) => ({ id: selector.replace(/^id:/, '') }), getAuthoritativeWindow: vi.fn(), - getAvailableAuthoritativeWindow: vi.fn(() => null) + getAvailableAuthoritativeWindow: vi.fn(() => null), + ...overrides, + getAgentBrowserBridge: () => bridge } as unknown as RuntimeBrowserCommandHost } describe('RuntimeBrowserCommands browser screencast', () => { + beforeEach(() => { + webContentsFromIdMock.mockReset() + startBrowserScreencastMock.mockReset() + waitForWorktreeTabRegistrationMock.mockReset() + waitForWorktreeTabRegistrationMock.mockResolvedValue(undefined) + }) + + it('waits for explicit worktree browser registration after requesting a hidden mount', async () => { + const { RuntimeBrowserCommands } = await import('./orca-runtime-browser') + const send = vi.fn() + const bridge = { + getRegisteredTabs: vi.fn(() => new Map()), + tabList: vi.fn(() => ({ tabs: [] })) + } as unknown as AgentBrowserBridge + const commands = new RuntimeBrowserCommands( + createHost({ + getAgentBrowserBridge: () => bridge, + getAuthoritativeWindow: vi.fn(() => ({ webContents: { send } }) as never) + }) + ) + + await commands.browserTabList({ worktree: 'id:wt-1' }) + + expect(send).toHaveBeenCalledWith('browser:activateView', { worktreeId: 'wt-1' }) + expect(waitForWorktreeTabRegistrationMock).toHaveBeenCalledWith('wt-1') + expect(bridge.tabList).toHaveBeenCalledWith('wt-1') + }) + + it('waits for any browser registration after requesting a hidden mount without worktree scope', async () => { + const { RuntimeBrowserCommands } = await import('./orca-runtime-browser') + const send = vi.fn() + const bridge = { + getRegisteredTabs: vi.fn(() => new Map()), + tabList: vi.fn(() => ({ tabs: [] })) + } as unknown as AgentBrowserBridge + const commands = new RuntimeBrowserCommands( + createHost({ + getAgentBrowserBridge: () => bridge, + getAuthoritativeWindow: vi.fn(() => ({ webContents: { send } }) as never) + }) + ) + + await commands.browserTabList({}) + + expect(send).toHaveBeenCalledWith('browser:activateView', {}) + expect(waitForWorktreeTabRegistrationMock).toHaveBeenCalledWith(undefined) + expect(bridge.tabList).toHaveBeenCalledWith(undefined) + }) + it('lets a new same-page stream take over an active stale stream', async () => { const { RuntimeBrowserCommands } = await import('./orca-runtime-browser') webContentsFromIdMock.mockReturnValue({ isDestroyed: () => false }) diff --git a/src/main/runtime/orca-runtime-browser.ts b/src/main/runtime/orca-runtime-browser.ts index b6ef370e7..98973639d 100644 --- a/src/main/runtime/orca-runtime-browser.ts +++ b/src/main/runtime/orca-runtime-browser.ts @@ -62,7 +62,7 @@ import { importCookiesFromBrowser, selectBrowserProfile } from '../browser/browser-cookie-import' -import { waitForTabRegistration } from '../ipc/browser' +import { waitForTabRegistration, waitForWorktreeTabRegistration } from '../ipc/browser' export type BrowserCommandTargetParams = { worktree?: string @@ -171,9 +171,7 @@ export class RuntimeBrowserCommands { const bridge = this.host.getAgentBrowserBridge() if (bridge && bridge.getRegisteredTabs().size === 0) { try { - const win = this.host.getAuthoritativeWindow() - win.webContents.send('browser:activateView', {}) - await new Promise((resolve) => setTimeout(resolve, 500)) + await this.ensureBrowserWorktreeActive(undefined) } catch { // Window may not exist yet (e.g. during startup or in tests) } @@ -251,12 +249,13 @@ export class RuntimeBrowserCommands { // and registerGuest fires, but automation must not steal the user's visible // worktree/browser pane. Ask the renderer to background-mount the worktree and // acquire a hidden automation visibility lease instead of activating the UI. - private async ensureBrowserWorktreeActive(worktreeId: string): Promise { + private async ensureBrowserWorktreeActive(worktreeId: string | undefined): Promise { const win = this.host.getAuthoritativeWindow() - win.webContents.send('browser:activateView', { worktreeId }) - // Why: give the renderer time to mount the hidden paintable webview. - // The webview needs to attach and fire dom-ready before registerGuest runs. - await new Promise((resolve) => setTimeout(resolve, 500)) + win.webContents.send('browser:activateView', worktreeId ? { worktreeId } : {}) + // Why: parked/restored browser panes become operable only after the + // renderer's webview mounts and calls registerGuest. Waiting on that IPC is + // both faster and less flaky than sleeping for an arbitrary fixed delay. + await waitForWorktreeTabRegistration(worktreeId) } // Why: agent-browser drives navigation via CDP, which bypasses Electron's