WIP

2026-09-26 16:02:43 +00:00 · 2026-09-23 14:41:04 -04:00
parent a77f87ea16
commit 24821ae633
7 changed files with 81 additions and 109 deletions
@@ -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 {
@@ -13,14 +13,9 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid
    browserPageId?: string
  ): Promise<BrowserScreenshotResult> {
    // 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<BrowserScreenshotResult> {
-    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 {
@@ -240,16 +240,21 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser

  async pdf(worktreeId?: string, browserPageId?: string): Promise<BrowserPdfResult> {
    // 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 }
+    )
  }
 }
@@ -49,12 +49,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
    worktreeId: string | undefined,
    execute: (sessionName: string) => Promise<T>
  ): Promise<T> {
-    return this.enqueueTargetedCommand(
-      worktreeId,
-      undefined,
-      async (sessionName) => execute(sessionName),
-      { ensureVisible: false }
-    )
+    return this.enqueueTargetedCommand(worktreeId, undefined, execute)
  }

  protected async enqueueTargetedCommand<T>(
@@ -101,7 +96,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
    execute: (sessionName: string, target: ResolvedBrowserCommandTarget) => Promise<T>,
    options: EnqueueTargetedCommandOptions
  ): Promise<T> {
-    if (options.ensureVisible === false) {
+    if (!options.needsPaint) {
      return execute(sessionName, target)
    }

@@ -257,10 +257,16 @@ export abstract class AgentBrowserBridgeStateCommands extends AgentBrowserBridge
  // ── Generic passthrough ──

  async exec(command: string, worktreeId?: string, browserPageId?: string): Promise<unknown> {
-    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 }
+    )
  }
 }
@@ -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
 }
@@ -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() {