From 24821ae6339644f1015aff6802ada4e47b707f7f Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 23 Sep 2026 14:41:04 -0400 Subject: [PATCH] WIP --- ...owser-bridge-automation-visibility.test.ts | 101 +++++++----------- .../agent-browser-bridge-capture-commands.ts | 32 ++---- ...ent-browser-bridge-interaction-commands.ts | 27 +++-- .../browser/agent-browser-bridge-queue.ts | 9 +- .../agent-browser-bridge-state-commands.ts | 16 ++- .../browser/agent-browser-bridge-types.ts | 3 +- .../browser/browser-manager-visibility.ts | 2 +- 7 files changed, 81 insertions(+), 109 deletions(-) diff --git a/src/main/browser/agent-browser-bridge-automation-visibility.test.ts b/src/main/browser/agent-browser-bridge-automation-visibility.test.ts index c6674bebdc2..35ff212a069 100644 --- a/src/main/browser/agent-browser-bridge-automation-visibility.test.ts +++ b/src/main/browser/agent-browser-bridge-automation-visibility.test.ts @@ -79,7 +79,23 @@ describe('AgentBrowserBridge', () => { bridge.setActiveTab(100) }) - it('acquires an automation visibility lease while running snapshot commands', async () => { + it('does not lease automation visibility for ordinary commands', async () => { + const acquireAutomationVisibility = vi.fn(async () => () => {}) + const b = new AgentBrowserBridge( + mockBrowserManager(undefined, undefined, { acquireAutomationVisibility }) + ) + b.setActiveTab(100) + webContentsFromIdMock.mockReturnValue(mockWebContents(100)) + + succeedWith({ snapshot: 'tree' }) + await b.snapshot() + await b.click('@e1') + await b.mouseClick(10, 20) + + expect(acquireAutomationVisibility).not.toHaveBeenCalled() + }) + + it('acquires an automation visibility lease while running exec commands', async () => { const lifecycleEvents: string[] = [] const restore = vi.fn(() => { lifecycleEvents.push('restore-100') @@ -96,17 +112,17 @@ describe('AgentBrowserBridge', () => { ) b.setActiveTab(100) - let releaseSnapshot: (() => void) | null = null + let releaseExec: (() => void) | null = null execFileMock.mockImplementation( (_bin: string, args: string[], _opts: unknown, cb: ExecFileCallback) => { if (args.includes('close')) { cb(null, JSON.stringify({ success: true, data: null }), '') return } - if (args.includes('snapshot')) { - lifecycleEvents.push('command-snapshot') - releaseSnapshot = () => { - cb(null, JSON.stringify({ success: true, data: { snapshot: 'tree' } }), '') + if (args.includes('screenshot')) { + lifecycleEvents.push('command-exec') + releaseExec = () => { + cb(null, JSON.stringify({ success: true, data: { ok: true } }), '') } return } @@ -114,24 +130,28 @@ describe('AgentBrowserBridge', () => { } ) - const snapshot = b.snapshot() + const exec = b.exec('screenshot') await vi.waitFor(() => { - expect(releaseSnapshot).not.toBeNull() + expect(releaseExec).not.toBeNull() }) - expect(lifecycleEvents).toEqual(['acquire-100', 'command-snapshot']) + expect(lifecycleEvents).toEqual(['acquire-100', 'command-exec']) expect(restore).not.toHaveBeenCalled() - releaseSnapshot!() + releaseExec!() - await expect(snapshot).resolves.toEqual({ browserPageId: 'tab-1', snapshot: 'tree' }) - expect(lifecycleEvents).toEqual(['acquire-100', 'command-snapshot', 'restore-100']) + await expect(exec).resolves.toEqual({ ok: true }) + expect(lifecycleEvents).toEqual(['acquire-100', 'command-exec', 'restore-100']) }) - it('re-resolves the page after automation visibility re-registers the webview', async () => { + it('re-resolves the page when a pdf lease re-registers the webview', async () => { const tabs = new Map([['tab-1', 100]]) const wc100 = mockWebContents(100) - const wc200 = mockWebContents(200, 'https://example.com/reloaded', 'Reloaded') + const printToPDF = vi.fn(async () => Buffer.from('pdf')) + const wc200 = { + ...mockWebContents(200, 'https://example.com/reloaded', 'Reloaded'), + printToPDF + } webContentsFromIdMock.mockImplementation((id: number) => { if (id === 100) { return wc100 @@ -153,10 +173,11 @@ describe('AgentBrowserBridge', () => { ) b.setActiveTab(100) - succeedWith({ snapshot: 'tree' }) - await expect(b.snapshot()).resolves.toEqual({ browserPageId: 'tab-1', snapshot: 'tree' }) + succeedWith(null) + await expect(b.pdf()).resolves.toEqual({ data: Buffer.from('pdf').toString('base64') }) expect(acquireAutomationVisibility).toHaveBeenCalledWith(100) + expect(printToPDF).toHaveBeenCalled() const createdProxyIds = CdpWsProxyMock.instances.map( (instance) => (instance as { _wc?: { id?: number } })._wc?.id ) @@ -201,7 +222,7 @@ describe('AgentBrowserBridge', () => { await b.interceptEnable(['https://old.example/**']) reregisterOnVisibility = true - await expect(b.snapshot()).resolves.toEqual({ browserPageId: 'tab-1', ok: true }) + await expect(b.exec('get title')).resolves.toEqual({ ok: true }) const routeCalls = commandCalls.filter( (args) => args.includes('network') && args.includes('route') @@ -212,52 +233,6 @@ describe('AgentBrowserBridge', () => { expect(routeCalls.at(-1)).toContain('9222') }) - it('clears stale sessions after direct CDP visibility re-registration', async () => { - const tabs = new Map([['tab-1', 100]]) - const wc100 = mockWebContents(100) - const wc200 = mockWebContents(200, 'https://example.com/reloaded', 'Reloaded') - wc200.debugger.sendCommand.mockResolvedValue({}) - webContentsFromIdMock.mockImplementation((id: number) => { - if (id === 100) { - return wc100 - } - if (id === 200) { - return wc200 - } - return null - }) - - let reregisterOnVisibility = false - const acquireAutomationVisibility = vi.fn(async () => { - if (reregisterOnVisibility) { - tabs.set('tab-1', 200) - } - return vi.fn() - }) - const b = new AgentBrowserBridge( - mockBrowserManager(tabs, undefined, { - acquireAutomationVisibility - }) - ) - b.setActiveTab(100) - - succeedWith({ snapshot: 'before' }) - await b.snapshot() - - reregisterOnVisibility = true - await expect(b.mouseClick(10, 20, 'right', undefined, 'tab-1')).resolves.toEqual({ - clicked: { x: 10, y: 20, button: 'right', adjusted: false, handled: false } - }) - - succeedWith({ snapshot: 'after' }) - await expect(b.snapshot()).resolves.toEqual({ browserPageId: 'tab-1', snapshot: 'after' }) - - const createdProxyIds = CdpWsProxyMock.instances.map( - (instance) => (instance as { _wc?: { id?: number } })._wc?.id - ) - expect(createdProxyIds).toEqual([100, 200]) - }) - it('serializes screenshot visibility prep across sessions', async () => { vi.useFakeTimers() try { diff --git a/src/main/browser/agent-browser-bridge-capture-commands.ts b/src/main/browser/agent-browser-bridge-capture-commands.ts index 5719bfbc7b6..088e1d62c54 100644 --- a/src/main/browser/agent-browser-bridge-capture-commands.ts +++ b/src/main/browser/agent-browser-bridge-capture-commands.ts @@ -13,14 +13,9 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid browserPageId?: string ): Promise { // Why: agent-browser writes the screenshot to a temp file and returns its path; read it and return base64. - return this.enqueueTargetedCommand( - worktreeId, - browserPageId, - async (sessionName) => { - return this.captureScreenshotCommand(sessionName, ['screenshot'], 300, format) - }, - { ensureVisible: false } - ) + return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => { + return this.captureScreenshotCommand(sessionName, ['screenshot'], 300, format) + }) } async fullPageScreenshot( @@ -28,19 +23,14 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid worktreeId?: string, browserPageId?: string ): Promise { - return this.enqueueTargetedCommand( - worktreeId, - browserPageId, - async (sessionName, target) => { - return this.captureFullPageScreenshotCommand( - sessionName, - target.webContentsId, - 500, - format === 'jpeg' ? 'jpeg' : 'png' - ) - }, - { ensureVisible: false } - ) + return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName, target) => { + return this.captureFullPageScreenshotCommand( + sessionName, + target.webContentsId, + 500, + format === 'jpeg' ? 'jpeg' : 'png' + ) + }) } private readScreenshotFromResult(raw: unknown, format?: string): BrowserScreenshotResult { diff --git a/src/main/browser/agent-browser-bridge-interaction-commands.ts b/src/main/browser/agent-browser-bridge-interaction-commands.ts index 97fa3892ab9..a4c32f39f67 100644 --- a/src/main/browser/agent-browser-bridge-interaction-commands.ts +++ b/src/main/browser/agent-browser-bridge-interaction-commands.ts @@ -240,16 +240,21 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser async pdf(worktreeId?: string, browserPageId?: string): Promise { // Why: agent-browser's CDP printToPDF hangs in Electron webviews — use the native webContents.printToPDF(). - return this.enqueueTargetedCommand(worktreeId, browserPageId, async (_sessionName, target) => { - const wc = this.getWebContents(target.webContentsId) - if (!wc) { - throw new BrowserError('browser_no_tab', 'Tab is no longer available') - } - const buffer = await wc.printToPDF({ - printBackground: true, - preferCSSPageSize: true - }) - return { data: buffer.toString('base64') } - }) + return this.enqueueTargetedCommand( + worktreeId, + browserPageId, + async (_sessionName, target) => { + const wc = this.getWebContents(target.webContentsId) + if (!wc) { + throw new BrowserError('browser_no_tab', 'Tab is no longer available') + } + const buffer = await wc.printToPDF({ + printBackground: true, + preferCSSPageSize: true + }) + return { data: buffer.toString('base64') } + }, + { needsPaint: true } + ) } } diff --git a/src/main/browser/agent-browser-bridge-queue.ts b/src/main/browser/agent-browser-bridge-queue.ts index 74cdb50c14a..c5fd2630c4e 100644 --- a/src/main/browser/agent-browser-bridge-queue.ts +++ b/src/main/browser/agent-browser-bridge-queue.ts @@ -49,12 +49,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown worktreeId: string | undefined, execute: (sessionName: string) => Promise ): Promise { - return this.enqueueTargetedCommand( - worktreeId, - undefined, - async (sessionName) => execute(sessionName), - { ensureVisible: false } - ) + return this.enqueueTargetedCommand(worktreeId, undefined, execute) } protected async enqueueTargetedCommand( @@ -101,7 +96,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown execute: (sessionName: string, target: ResolvedBrowserCommandTarget) => Promise, options: EnqueueTargetedCommandOptions ): Promise { - if (options.ensureVisible === false) { + if (!options.needsPaint) { return execute(sessionName, target) } diff --git a/src/main/browser/agent-browser-bridge-state-commands.ts b/src/main/browser/agent-browser-bridge-state-commands.ts index 4e03055da57..597b56223c5 100644 --- a/src/main/browser/agent-browser-bridge-state-commands.ts +++ b/src/main/browser/agent-browser-bridge-state-commands.ts @@ -257,10 +257,16 @@ export abstract class AgentBrowserBridgeStateCommands extends AgentBrowserBridge // ── Generic passthrough ── async exec(command: string, worktreeId?: string, browserPageId?: string): Promise { - return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => { - // Why: strip target/session flags from passthrough so a caller can't override Orca's selected page or CDP proxy. - const args = stripAgentBrowserTargetArgs(parseShellArgs(command.trim())) - return await this.execAgentBrowser(sessionName, args) - }) + return this.enqueueTargetedCommand( + worktreeId, + browserPageId, + async (sessionName) => { + // Why: strip target/session flags from passthrough so a caller can't override Orca's selected page or CDP proxy. + const args = stripAgentBrowserTargetArgs(parseShellArgs(command.trim())) + return await this.execAgentBrowser(sessionName, args) + }, + // Why: passthrough can run screenshot/record. + { needsPaint: true } + ) } } diff --git a/src/main/browser/agent-browser-bridge-types.ts b/src/main/browser/agent-browser-bridge-types.ts index a89c9fcbca8..5fa72557619 100644 --- a/src/main/browser/agent-browser-bridge-types.ts +++ b/src/main/browser/agent-browser-bridge-types.ts @@ -55,7 +55,8 @@ export type AgentBrowserExecOptions = { export type EnqueueTargetedCommandOptions = { ensureSession?: boolean - ensureVisible?: boolean + // Why: only pixel capture needs a drawn page; input, JS, layout and snapshots work on a display:none page. + needsPaint?: boolean // Why: text-mutating commands must never fall back to the global tab (may be a worktree the user is viewing). requireScopedTarget?: boolean } diff --git a/src/main/browser/browser-manager-visibility.ts b/src/main/browser/browser-manager-visibility.ts index 1da2a75638d..c3566cc0d48 100644 --- a/src/main/browser/browser-manager-visibility.ts +++ b/src/main/browser/browser-manager-visibility.ts @@ -229,7 +229,7 @@ export abstract class BrowserManagerVisibility extends BrowserManagerState { return () => {} } - // Why: agent commands need a paintable webview for lazy-loading sites without stealing the user's visible tab. + // Why: pixel-capturing agent commands need a drawn webview without stealing the user's visible tab. const acquirePromise = renderer .executeJavaScript( `(async function() {