mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 08:02:35 +00:00
fix(browser): only pixel-capturing commands wait for the page to be drawn (#22528)
* WIP * WIP2 * fix(browser): only pixel-capturing commands wait for the page to be drawn Every targeted browser command used to take an automation-visibility lease, which waits for two desktop-window animation frames (capped at 2 s). When the desktop window is minimized or throttled, that wait always hits the cap, so each phone tap or agent command stalled for up to 2 s (STA-8024). Input, page JavaScript, layout and accessibility snapshots all work on a hidden page; only pixel capture needs it drawn. The lease is now opt-in via `needsPaint` on the two commands that can capture pixels through this path (`exec` passthrough and `pdf`); `ensureVisible` is removed. Screenshots keep managing their own lease. (Commits24821aeand07b4a09carry this change under WIP messages.) * refactor(browser): drop the session-recreate option only no-lease commands used Only exec and pdf take the automation-visibility lease now, and both create an agent-browser session, so the lease's re-registration path always recreates the session. Remove the `recreate` option from restartSessionForTarget and the options passthrough that fed it. Also fix the keypress test comment that still counted the lease's extra page lookup, and note that screenshots lease inside the screenshot lock. * fix(browser): resolve a command's page when it runs, not when it is queued Enqueue resolved the page's guest webContents and created its session up front. Before this PR every command re-resolved after its lease, which hid that; now a command queued behind exec/pdf, whose lease can re-register the page with a new guest webContents, would run against the old one. Enqueue now only picks the page (and so the queue). The webContents lookup and session setup move into the queued job and run when the command starts, after the lease for exec/pdf. The lease path's refresh collapses into that step. Resolving after the lease already sees a re-registration that happens during the lease, so the only thing left from the refresh is the case where the page's existing session is bound to an older guest: that session is restarted on the new one (keeping intercept routes) and the active-tab pointers move with it. That check now covers every session-backed command, not only leased ones. Admission is still checked at enqueue and again when the job starts. * refactor(browser): drop the session-restart reconcile no page can reach Every renderer re-registration of a page with a different guest goes through browser:registerGuest, which calls onProcessSwap in the same tick. That destroys the page's session and rejects its queued commands, so by the time a queued command runs, no session can still be bound to an older guest. The reconcile in the queue and restartSessionForTarget, its only caller, are removed along with the two tests that called it directly. A queued command now resolves the page's current guest, ensures its session, and runs. The re-registration tests now report the swap through onProcessSwap, as production does: commands queued behind the swapping lease are rejected with browser_tab_closed, the leased pdf runs on the new guest, and intercept routes survive the swap.
This commit is contained in:
@@ -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,14 +173,55 @@ 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
|
||||
)
|
||||
expect(createdProxyIds).toEqual([100, 200])
|
||||
// Why: the session is created when the command runs, after the lease, so the old guest never gets one.
|
||||
expect(createdProxyIds).toEqual([200])
|
||||
})
|
||||
|
||||
it('rejects commands queued behind a leased command whose page swaps guests', async () => {
|
||||
const tabs = new Map([['tab-1', 100]])
|
||||
const wc100 = mockWebContents(100)
|
||||
const wc200 = {
|
||||
...mockWebContents(200, 'https://example.com/reloaded', 'Reloaded'),
|
||||
printToPDF: vi.fn(async () => Buffer.from('pdf'))
|
||||
}
|
||||
webContentsFromIdMock.mockImplementation((id: number) =>
|
||||
id === 100 ? wc100 : id === 200 ? wc200 : null
|
||||
)
|
||||
|
||||
let releaseLease: (() => void) | null = null
|
||||
const acquireAutomationVisibility = vi.fn(
|
||||
() =>
|
||||
new Promise<() => void>((resolve) => {
|
||||
releaseLease = () => {
|
||||
// Why: mirrors browser:registerGuest, which reports the new guest to the bridge in the same tick.
|
||||
tabs.set('tab-1', 200)
|
||||
void b.onProcessSwap('tab-1', 200, 100)
|
||||
resolve(() => {})
|
||||
}
|
||||
})
|
||||
)
|
||||
const b = new AgentBrowserBridge(
|
||||
mockBrowserManager(tabs, undefined, { acquireAutomationVisibility })
|
||||
)
|
||||
b.setActiveTab(100)
|
||||
succeedWith(null)
|
||||
|
||||
const pdf = b.pdf(undefined, 'tab-1')
|
||||
await vi.waitFor(() => expect(releaseLease).not.toBeNull())
|
||||
const click = b.mouseClick(10, 20, 'left', undefined, 'tab-1')
|
||||
releaseLease!()
|
||||
|
||||
await expect(click).rejects.toMatchObject({ code: 'browser_tab_closed' })
|
||||
await expect(pdf).resolves.toEqual({ data: Buffer.from('pdf').toString('base64') })
|
||||
expect(wc200.printToPDF).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves intercept routes when automation visibility re-registers the webview', async () => {
|
||||
@@ -181,6 +242,7 @@ describe('AgentBrowserBridge', () => {
|
||||
const acquireAutomationVisibility = vi.fn(async () => {
|
||||
if (reregisterOnVisibility) {
|
||||
tabs.set('tab-1', 200)
|
||||
void b.onProcessSwap('tab-1', 200, 100)
|
||||
}
|
||||
return vi.fn()
|
||||
})
|
||||
@@ -201,7 +263,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 +274,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 }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -234,9 +234,7 @@ describe('AgentBrowserBridge keypress input', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: one keypress looks the page up three times — the queued target, the
|
||||
// automation-visibility refresh, then the dispatch guard. Serving the first N keeps the
|
||||
// later ones on the guard; the trailing assertions fail loudly if that count ever moves.
|
||||
// Why: one keypress looks the page up twice — the queued target, then the dispatch guard. Serving the first N keeps the later ones on the guard; the trailing assertions fail loudly if that count ever moves.
|
||||
function killPageAfterLookups(lookups: number): () => number {
|
||||
let remaining = lookups
|
||||
webContentsFromIdMock.mockImplementation((id: number) => {
|
||||
@@ -250,7 +248,7 @@ describe('AgentBrowserBridge keypress input', () => {
|
||||
}
|
||||
|
||||
it('rejects with tab not found when the page dies after its target is resolved', async () => {
|
||||
const remaining = killPageAfterLookups(2)
|
||||
const remaining = killPageAfterLookups(1)
|
||||
|
||||
await expect(bridge.keypress('a', undefined, 'tab-1')).rejects.toMatchObject({
|
||||
code: 'browser_tab_not_found'
|
||||
@@ -260,7 +258,7 @@ describe('AgentBrowserBridge keypress input', () => {
|
||||
})
|
||||
|
||||
it('rejects with tab not found when the page dies mid-dispatch', async () => {
|
||||
const remaining = killPageAfterLookups(3)
|
||||
const remaining = killPageAfterLookups(2)
|
||||
wc.debugger.sendCommand.mockRejectedValue(new Error('Inspected target navigated or closed'))
|
||||
|
||||
await expect(bridge.keypress('a', undefined, 'tab-1')).rejects.toMatchObject({
|
||||
|
||||
@@ -131,57 +131,6 @@ export abstract class AgentBrowserBridgeLifecycle extends AgentBrowserBridgeRawP
|
||||
}
|
||||
}
|
||||
|
||||
protected async restartSessionForTarget(
|
||||
sessionName: string,
|
||||
browserPageId: string,
|
||||
webContentsId: number,
|
||||
options: { recreate: boolean } = { recreate: true }
|
||||
): Promise<void> {
|
||||
const pendingCreation = this.pendingSessionCreation.get(sessionName)
|
||||
if (pendingCreation) {
|
||||
await pendingCreation.catch(() => {})
|
||||
}
|
||||
|
||||
const session = this.sessions.get(sessionName)
|
||||
if (session) {
|
||||
if (session.activeInterceptPatterns.length > 0) {
|
||||
this.pendingInterceptRestore.set(sessionName, [...session.activeInterceptPatterns])
|
||||
}
|
||||
this.sessions.delete(sessionName)
|
||||
this.pendingSessionCreation.delete(sessionName)
|
||||
if (session.activeProcess) {
|
||||
this.cancelledProcesses.add(session.activeProcess)
|
||||
try {
|
||||
session.activeProcess.kill()
|
||||
} catch {
|
||||
// Process may already be exiting.
|
||||
}
|
||||
session.activeProcess = null
|
||||
}
|
||||
|
||||
const destroy = (async (): Promise<void> => {
|
||||
try {
|
||||
await this.runAgentBrowserRaw(sessionName, ['--session', sessionName, 'close'], {
|
||||
timeoutMs: AGENT_BROWSER_CLEANUP_TIMEOUT_MS
|
||||
})
|
||||
} catch {
|
||||
// Session may already be dead.
|
||||
}
|
||||
await session.proxy.stop()
|
||||
})()
|
||||
this.pendingSessionDestruction.set(sessionName, destroy)
|
||||
try {
|
||||
await destroy
|
||||
} finally {
|
||||
this.pendingSessionDestruction.delete(sessionName)
|
||||
}
|
||||
}
|
||||
|
||||
if (options.recreate) {
|
||||
await this.ensureSession(sessionName, browserPageId, webContentsId)
|
||||
}
|
||||
}
|
||||
|
||||
protected async destroySession(
|
||||
sessionName: string,
|
||||
options: AgentBrowserCleanupOptions = { closeTimeoutMs: AGENT_BROWSER_CLEANUP_TIMEOUT_MS }
|
||||
|
||||
@@ -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>(
|
||||
@@ -64,13 +59,12 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
|
||||
options: EnqueueTargetedCommandOptions = {}
|
||||
): Promise<T> {
|
||||
this.assertCommandAdmission()
|
||||
const target = this.resolveCommandTarget(worktreeId, browserPageId, options.requireScopedTarget)
|
||||
const sessionName = `${ORCA_TAB_SESSION_PREFIX}${target.browserPageId}`
|
||||
|
||||
if (options.ensureSession !== false) {
|
||||
await this.ensureSession(sessionName, target.browserPageId, target.webContentsId)
|
||||
}
|
||||
this.assertCommandAdmission()
|
||||
const { browserPageId: pageId } = this.resolveCommandTarget(
|
||||
worktreeId,
|
||||
browserPageId,
|
||||
options.requireScopedTarget
|
||||
)
|
||||
const sessionName = `${ORCA_TAB_SESSION_PREFIX}${pageId}`
|
||||
|
||||
return new Promise<T>((resolve, reject) => {
|
||||
let queue = this.commandQueues.get(sessionName)
|
||||
@@ -79,14 +73,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
|
||||
this.commandQueues.set(sessionName, queue)
|
||||
}
|
||||
queue.push({
|
||||
execute: (() =>
|
||||
this.executeWithVisibleTarget(
|
||||
sessionName,
|
||||
worktreeId,
|
||||
target,
|
||||
execute,
|
||||
options
|
||||
)) as () => Promise<unknown>,
|
||||
execute: () => this.executeQueuedCommand(worktreeId, pageId, execute, options),
|
||||
resolve: resolve as (value: unknown) => void,
|
||||
reject
|
||||
})
|
||||
@@ -94,61 +81,32 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
|
||||
})
|
||||
}
|
||||
|
||||
protected async executeWithVisibleTarget<T>(
|
||||
sessionName: string,
|
||||
protected async executeQueuedCommand<T>(
|
||||
worktreeId: string | undefined,
|
||||
target: ResolvedBrowserCommandTarget,
|
||||
browserPageId: string,
|
||||
execute: (sessionName: string, target: ResolvedBrowserCommandTarget) => Promise<T>,
|
||||
options: EnqueueTargetedCommandOptions
|
||||
): Promise<T> {
|
||||
if (options.ensureVisible === false) {
|
||||
return execute(sessionName, target)
|
||||
}
|
||||
|
||||
this.assertCommandAdmission()
|
||||
const sessionName = `${ORCA_TAB_SESSION_PREFIX}${browserPageId}`
|
||||
// Why: inactive panes are display:none; the automation lease makes only this target paintable without selecting it.
|
||||
const restore = await this.browserManager.acquireAutomationVisibility(target.webContentsId)
|
||||
const restore = options.needsPaint
|
||||
? await this.browserManager.acquireAutomationVisibility(
|
||||
this.resolveCommandTarget(worktreeId, browserPageId).webContentsId
|
||||
)
|
||||
: undefined
|
||||
try {
|
||||
const visibleTarget = await this.refreshTargetAfterAutomationVisibility(
|
||||
sessionName,
|
||||
worktreeId,
|
||||
target,
|
||||
options
|
||||
)
|
||||
return await execute(sessionName, visibleTarget)
|
||||
// Why: the page's guest can change while queued; bind to the one current at execution.
|
||||
const target = this.resolveCommandTarget(worktreeId, browserPageId)
|
||||
if (options.ensureSession !== false) {
|
||||
await this.ensureSession(sessionName, browserPageId, target.webContentsId)
|
||||
}
|
||||
return await execute(sessionName, target)
|
||||
} finally {
|
||||
restore()
|
||||
restore?.()
|
||||
}
|
||||
}
|
||||
|
||||
protected async refreshTargetAfterAutomationVisibility(
|
||||
sessionName: string,
|
||||
worktreeId: string | undefined,
|
||||
target: ResolvedBrowserCommandTarget,
|
||||
options: EnqueueTargetedCommandOptions
|
||||
): Promise<ResolvedBrowserCommandTarget> {
|
||||
const visibleTarget = this.resolveCommandTarget(worktreeId, target.browserPageId)
|
||||
if (visibleTarget.webContentsId === target.webContentsId) {
|
||||
return visibleTarget
|
||||
}
|
||||
|
||||
if (this.activeWebContentsId === target.webContentsId) {
|
||||
this.activeWebContentsId = visibleTarget.webContentsId
|
||||
}
|
||||
if (worktreeId && this.activeWebContentsPerWorktree.get(worktreeId) === target.webContentsId) {
|
||||
this.activeWebContentsPerWorktree.set(worktreeId, visibleTarget.webContentsId)
|
||||
}
|
||||
|
||||
// Why: making a parked webview paintable can re-register the page with a new guest webContents; tear down the stale session.
|
||||
await this.restartSessionForTarget(
|
||||
sessionName,
|
||||
visibleTarget.browserPageId,
|
||||
visibleTarget.webContentsId,
|
||||
{ recreate: options.ensureSession !== false }
|
||||
)
|
||||
|
||||
return visibleTarget
|
||||
}
|
||||
|
||||
protected async processQueue(sessionName: string): Promise<void> {
|
||||
if (this.processingQueues.has(sessionName)) {
|
||||
return
|
||||
|
||||
@@ -194,29 +194,6 @@ describe('AgentBrowserBridge', () => {
|
||||
expect(closeCall?.[2]).toMatchObject({ timeout: 5_000 })
|
||||
})
|
||||
|
||||
it('uses the cleanup timeout when a target swap retires its session', async () => {
|
||||
succeedWith({ snapshot: 'initial' })
|
||||
await bridge.snapshot()
|
||||
execFileMock.mockClear()
|
||||
|
||||
succeedWith(null)
|
||||
await (
|
||||
bridge as unknown as {
|
||||
restartSessionForTarget: (
|
||||
sessionName: string,
|
||||
browserPageId: string,
|
||||
webContentsId: number,
|
||||
options: { recreate: boolean }
|
||||
) => Promise<void>
|
||||
}
|
||||
).restartSessionForTarget('orca-tab-tab-1', 'tab-1', 100, { recreate: false })
|
||||
|
||||
const closeCall = execFileMock.mock.calls.find((call: unknown[]) =>
|
||||
(call[1] as string[]).includes('close')
|
||||
)
|
||||
expect(closeCall?.[2]).toMatchObject({ timeout: 5_000 })
|
||||
})
|
||||
|
||||
it('waits for pending session destruction before recreating the same session', async () => {
|
||||
succeedWith({ snapshot: 'initial' })
|
||||
await bridge.snapshot()
|
||||
@@ -636,48 +613,6 @@ describe('AgentBrowserBridge', () => {
|
||||
expect(proxy.stop).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not recreate a session after shutdown observes its pending retirement', async () => {
|
||||
succeedWith({ snapshot: 'initial' })
|
||||
await bridge.snapshot()
|
||||
execFileMock.mockClear()
|
||||
|
||||
let releaseClose: (() => void) | null = null
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, args: string[], _opts: unknown, cb: ExecFileCallback) => {
|
||||
if (!args.includes('close')) {
|
||||
throw new Error(`unexpected agent-browser args ${args.join(' ')}`)
|
||||
}
|
||||
releaseClose = () => cb(null, JSON.stringify({ success: true, data: null }), '')
|
||||
return { kill: vi.fn() }
|
||||
}
|
||||
)
|
||||
|
||||
const restart = (
|
||||
bridge as unknown as {
|
||||
restartSessionForTarget: (
|
||||
sessionName: string,
|
||||
browserPageId: string,
|
||||
webContentsId: number
|
||||
) => Promise<void>
|
||||
}
|
||||
).restartSessionForTarget('orca-tab-tab-1', 'tab-1', 100)
|
||||
await vi.waitFor(() => expect(releaseClose).not.toBeNull())
|
||||
|
||||
const shutdown = bridge.destroyAllSessions()
|
||||
releaseClose!()
|
||||
|
||||
await expect(restart).rejects.toMatchObject({
|
||||
code: 'browser_owner_unavailable',
|
||||
message: 'Browser runtime is shutting down'
|
||||
})
|
||||
await shutdown
|
||||
|
||||
const sessions = (bridge as unknown as { sessions: Map<string, unknown> }).sessions
|
||||
expect(sessions.size).toBe(0)
|
||||
expect(CdpWsProxyMock.instances).toHaveLength(1)
|
||||
expect(execFileMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// Why: quit awaits destroyAllSessions inside a 20s barrier, so an unbounded close can hold the
|
||||
// window up for the whole deadline when the daemon is wedged (#16367).
|
||||
it('bounds every teardown close well inside the quit barrier', async () => {
|
||||
|
||||
@@ -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. Screenshots lease inside the screenshot lock instead.
|
||||
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() {
|
||||
|
||||
Reference in New Issue
Block a user