Keep embedded browser commands on the registered Orca target (#9633)
* fix(browser): keep embedded commands on registered target * fix(browser): cover internal helper ownership paths * test(browser): cover stale helper routing in Electron * fix(browser): preserve direct command result semantics --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
GitHub
parent
fea0004a01
commit
085fc6ad89
|
|
@ -67,7 +67,8 @@ import {
|
|||
// inside a try/catch. Override the private method to inject our mock.
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
;(AgentBrowserBridge.prototype as any).getWebContents = function (id: number) {
|
||||
return webContentsFromIdMock(id) ?? null
|
||||
const target = webContentsFromIdMock(id)
|
||||
return target && !target.isDestroyed() ? target : null
|
||||
}
|
||||
|
||||
function mockBrowserManager(
|
||||
|
|
@ -89,10 +90,14 @@ function mockBrowserManager(
|
|||
}
|
||||
|
||||
function mockWebContents(id: number, url = 'https://example.com', title = 'Example') {
|
||||
let currentUrl = url
|
||||
return {
|
||||
id,
|
||||
getURL: () => url,
|
||||
getURL: () => currentUrl,
|
||||
getTitle: () => title,
|
||||
loadURL: vi.fn(async (nextUrl: string) => {
|
||||
currentUrl = nextUrl
|
||||
}),
|
||||
isDestroyed: () => false,
|
||||
invalidate: vi.fn(),
|
||||
focus: vi.fn(),
|
||||
|
|
@ -308,9 +313,9 @@ describe('AgentBrowserBridge', () => {
|
|||
expect(args[args.indexOf('--session') + 1]).toBe('orca-tab-tab-1')
|
||||
})
|
||||
|
||||
// ── --cdp first-use only ──
|
||||
// ── Embedded CDP ownership ──
|
||||
|
||||
it('passes --cdp only on first command for a session', async () => {
|
||||
it('passes --cdp on every helper command family so a restarted daemon cannot launch Chrome', async () => {
|
||||
succeedWith({ snapshot: '...' })
|
||||
await bridge.snapshot()
|
||||
|
||||
|
|
@ -322,16 +327,24 @@ describe('AgentBrowserBridge', () => {
|
|||
const cdpIdx = (snapshotCall![1] as string[]).indexOf('--cdp')
|
||||
expect((snapshotCall![1] as string[])[cdpIdx + 1]).toBe('9222')
|
||||
|
||||
succeedWith({ clicked: '@e1' })
|
||||
await bridge.click('@e1')
|
||||
await bridge.mouseMove(10, 20)
|
||||
await bridge.setOffline('on')
|
||||
await bridge.consoleLog()
|
||||
await bridge.exec('get title')
|
||||
|
||||
const clickCall = execFileMock.mock.calls.find((c: unknown[]) =>
|
||||
(c[1] as string[]).includes('click')
|
||||
)
|
||||
expect(clickCall![1]).not.toContain('--cdp')
|
||||
for (const command of ['click', 'mouse', 'set', 'console', 'get']) {
|
||||
const call = execFileMock.mock.calls.find((candidate: unknown[]) =>
|
||||
(candidate[1] as string[]).includes(command)
|
||||
)
|
||||
expect(call).toBeDefined()
|
||||
const args = call![1] as string[]
|
||||
expect(args).toContain('--cdp')
|
||||
expect(args[args.indexOf('--cdp') + 1]).toBe('9222')
|
||||
}
|
||||
})
|
||||
|
||||
it('continues when stale agent-browser session close hangs during session creation', async () => {
|
||||
it('fails closed when stale agent-browser session ownership cannot be reset', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const closeKill = vi.fn()
|
||||
|
|
@ -349,17 +362,17 @@ describe('AgentBrowserBridge', () => {
|
|||
)
|
||||
|
||||
const promise = bridge.snapshot()
|
||||
let settled = false
|
||||
void promise.finally(() => {
|
||||
settled = true
|
||||
const rejection = expect(promise).rejects.toMatchObject({
|
||||
code: 'browser_owner_unavailable',
|
||||
message:
|
||||
'Could not reset stale helper session orca-tab-tab-1; retry after agent-browser exits'
|
||||
})
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3_000)
|
||||
await Promise.resolve()
|
||||
|
||||
expect(settled).toBe(true)
|
||||
await expect(promise).resolves.toEqual({ browserPageId: 'tab-1', snapshot: 'ready' })
|
||||
await rejection
|
||||
expect(closeKill).toHaveBeenCalled()
|
||||
expect(execFileMock.mock.calls.some((call) => call[1].includes('snapshot'))).toBe(false)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
|
|
@ -876,6 +889,8 @@ describe('AgentBrowserBridge', () => {
|
|||
)
|
||||
expect(routeCalls).toHaveLength(2)
|
||||
expect(routeCalls.at(-1)).toContain('https://old.example/**')
|
||||
expect(routeCalls.at(-1)).toContain('--cdp')
|
||||
expect(routeCalls.at(-1)).toContain('9222')
|
||||
})
|
||||
|
||||
it('clears stale sessions after direct CDP visibility re-registration', async () => {
|
||||
|
|
@ -1119,7 +1134,11 @@ describe('AgentBrowserBridge', () => {
|
|||
const killedError = Object.assign(new Error('timeout'), { killed: true })
|
||||
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, _args: string[], _opts: unknown, cb: Function) => {
|
||||
(_bin: string, args: string[], _opts: unknown, cb: Function) => {
|
||||
if (args.includes('close')) {
|
||||
cb(null, JSON.stringify({ success: true, data: null }), '')
|
||||
return
|
||||
}
|
||||
cb(killedError, '', '')
|
||||
}
|
||||
)
|
||||
|
|
@ -1588,13 +1607,282 @@ describe('AgentBrowserBridge', () => {
|
|||
|
||||
// ── goto command ──
|
||||
|
||||
it('passes url to goto command', async () => {
|
||||
succeedWith({ url: 'https://example.com', title: 'Example' })
|
||||
await bridge.goto('https://example.com')
|
||||
it('navigates the registered webContents without spawning agent-browser', async () => {
|
||||
const wc = mockWebContents(100, 'https://example.com/start', 'Example')
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
const args = execFileMock.mock.calls.at(-1)![1] as string[]
|
||||
expect(args).toContain('goto')
|
||||
expect(args).toContain('https://example.com')
|
||||
await expect(bridge.goto('https://example.com/next')).resolves.toEqual({
|
||||
url: 'https://example.com/next',
|
||||
title: 'Example'
|
||||
})
|
||||
|
||||
expect(wc.loadURL).toHaveBeenCalledWith('https://example.com/next')
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves scheme-less navigation semantics at the direct WebContents boundary', async () => {
|
||||
const wc = mockWebContents(100)
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.goto('example.com')).resolves.toEqual({
|
||||
url: 'https://example.com/',
|
||||
title: 'Example'
|
||||
})
|
||||
expect(wc.loadURL).toHaveBeenCalledWith('https://example.com/')
|
||||
})
|
||||
|
||||
it('rejects unsupported direct navigation URLs without falling back to agent-browser', async () => {
|
||||
const wc = mockWebContents(100)
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.goto('javascript:alert(1)')).rejects.toMatchObject({
|
||||
code: 'invalid_argument',
|
||||
message: 'Unsupported browser URL: javascript:alert(1)'
|
||||
})
|
||||
expect(wc.loadURL).not.toHaveBeenCalled()
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails closed and releases the command queue when direct navigation never settles', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const wc = mockWebContents(100)
|
||||
wc.loadURL.mockReturnValue(new Promise<void>(() => {}))
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
const navigation = bridge.goto('https://example.com/hangs')
|
||||
const rejection = expect(navigation).rejects.toMatchObject({
|
||||
code: 'browser_error',
|
||||
message: 'Failed to navigate browser page tab-1: Browser navigation timed out after 30000ms'
|
||||
})
|
||||
await vi.advanceTimersByTimeAsync(30_000)
|
||||
|
||||
await rejection
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
expect(
|
||||
(bridge as unknown as { commandQueues: Map<string, unknown[]> }).commandQueues.size
|
||||
).toBe(0)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('fails closed when direct navigation is aborted', async () => {
|
||||
const wc = mockWebContents(100, 'https://example.com/current', 'Current')
|
||||
wc.loadURL.mockRejectedValue(
|
||||
Object.assign(new Error('ERR_ABORTED (-3)'), { code: 'ERR_ABORTED' })
|
||||
)
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.goto('https://example.com/download')).rejects.toMatchObject({
|
||||
code: 'browser_error',
|
||||
message: 'Failed to navigate browser page tab-1: ERR_ABORTED (-3)'
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each([
|
||||
{
|
||||
command: 'goto',
|
||||
run: (b: AgentBrowserBridge) => b.goto('https://embedded.example/next'),
|
||||
helperArg: 'goto',
|
||||
directMethod: 'loadURL'
|
||||
},
|
||||
{
|
||||
command: 'evaluate',
|
||||
run: (b: AgentBrowserBridge) => b.evaluate('document.title'),
|
||||
helperArg: 'eval',
|
||||
directMethod: 'Runtime.evaluate'
|
||||
}
|
||||
])(
|
||||
'does not route $command to another browser when the helper session is stale',
|
||||
async ({ run, helperArg, directMethod }) => {
|
||||
const wc = mockWebContents(100, 'https://embedded.example/current', 'Embedded')
|
||||
wc.debugger.sendCommand.mockImplementation(async (_method: string, params?: unknown) => ({
|
||||
result: {
|
||||
value:
|
||||
(params as { expression?: string } | undefined)?.expression === 'location.origin'
|
||||
? 'https://embedded.example'
|
||||
: 'Embedded'
|
||||
}
|
||||
}))
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
const wrongOwnerCalls: string[][] = []
|
||||
let helperSessionIsStale = false
|
||||
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, args: string[], _opts: unknown, cb: Function) => {
|
||||
if (args.includes('close')) {
|
||||
cb(null, JSON.stringify({ success: true, data: null }), '')
|
||||
} else if (args.includes('snapshot')) {
|
||||
cb(null, JSON.stringify({ success: true, data: { snapshot: 'ready' } }), '')
|
||||
} else {
|
||||
if (helperSessionIsStale && !args.includes('--cdp')) {
|
||||
wrongOwnerCalls.push(args)
|
||||
}
|
||||
cb(
|
||||
null,
|
||||
JSON.stringify({
|
||||
success: true,
|
||||
data: { url: 'https://external.example', title: 'External', result: 'external' }
|
||||
}),
|
||||
''
|
||||
)
|
||||
}
|
||||
return { kill: vi.fn() }
|
||||
}
|
||||
)
|
||||
|
||||
await bridge.snapshot()
|
||||
helperSessionIsStale = true
|
||||
await run(bridge)
|
||||
|
||||
expect(wrongOwnerCalls).toEqual([])
|
||||
expect(
|
||||
execFileMock.mock.calls.some((call) => (call[1] as string[]).includes(helperArg))
|
||||
).toBe(false)
|
||||
if (directMethod === 'loadURL') {
|
||||
expect(wc.loadURL).toHaveBeenCalledWith('https://embedded.example/next')
|
||||
} else {
|
||||
expect(wc.debugger.sendCommand).toHaveBeenCalledWith(
|
||||
'Runtime.evaluate',
|
||||
expect.objectContaining({ expression: 'document.title' })
|
||||
)
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
it('returns direct evaluation value and full page URL semantics without spawning agent-browser', async () => {
|
||||
const wc = mockWebContents(100, 'https://example.com/path?query=1')
|
||||
wc.debugger.sendCommand.mockResolvedValue({ result: { value: 42 } })
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.evaluate('6 * 7')).resolves.toEqual({
|
||||
result: '42',
|
||||
origin: 'https://example.com/path?query=1'
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each([
|
||||
[{ answer: 42 }, '{"answer":42}'],
|
||||
[['a', 'b'], '["a","b"]']
|
||||
])('preserves structured direct evaluation values as JSON text', async (value, expected) => {
|
||||
const wc = mockWebContents(100)
|
||||
wc.debugger.sendCommand.mockResolvedValue({ result: { value } })
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.evaluate('structuredValue')).resolves.toMatchObject({ result: expected })
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('surfaces direct evaluation exceptions without falling back to agent-browser', async () => {
|
||||
const wc = mockWebContents(100)
|
||||
wc.debugger.sendCommand.mockResolvedValue({
|
||||
result: { type: 'object' },
|
||||
exceptionDetails: {
|
||||
text: 'Uncaught',
|
||||
exception: { description: 'ReferenceError: missingValue is not defined' }
|
||||
}
|
||||
})
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.evaluate('missingValue')).rejects.toMatchObject({
|
||||
code: 'browser_eval_error',
|
||||
message: 'ReferenceError: missingValue is not defined'
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails closed when the registered webContents debugger is stale', async () => {
|
||||
const wc = mockWebContents(100)
|
||||
wc.debugger.sendCommand.mockRejectedValue(new Error('Debugger is detached'))
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
|
||||
await expect(bridge.evaluate('document.title')).rejects.toMatchObject({
|
||||
code: 'browser_error',
|
||||
message: 'Failed to evaluate in browser page tab-1: Debugger is detached'
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['goto', (b: AgentBrowserBridge) => b.goto('https://example.com/next', undefined, 'tab-1')],
|
||||
['evaluate', (b: AgentBrowserBridge) => b.evaluate('document.title', undefined, 'tab-1')]
|
||||
])('fails closed when direct %s targets a destroyed webContents', async (_command, run) => {
|
||||
const wc = mockWebContents(100)
|
||||
wc.isDestroyed = () => true
|
||||
webContentsFromIdMock.mockReturnValue(wc)
|
||||
const unregisterGuest = vi.fn()
|
||||
const b = new AgentBrowserBridge(
|
||||
mockBrowserManager(new Map([['tab-1', 100]]), new Map(), { unregisterGuest })
|
||||
)
|
||||
b.setActiveTab(100)
|
||||
|
||||
await expect(run(b)).rejects.toMatchObject({
|
||||
code: 'browser_tab_not_found',
|
||||
message: 'Browser page tab-1 is no longer available'
|
||||
})
|
||||
expect(unregisterGuest).toHaveBeenCalledWith('tab-1')
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns navigation state from a replacement registered during load', async () => {
|
||||
const tabs = new Map([['tab-1', 100]])
|
||||
const oldWc = mockWebContents(100, 'https://example.com/start', 'Old')
|
||||
const replacementWc = mockWebContents(200, 'https://example.com/final', 'Replacement')
|
||||
oldWc.loadURL.mockImplementation(async () => {
|
||||
tabs.set('tab-1', 200)
|
||||
})
|
||||
webContentsFromIdMock.mockImplementation((id: number) =>
|
||||
id === 100 ? oldWc : id === 200 ? replacementWc : null
|
||||
)
|
||||
const b = new AgentBrowserBridge(mockBrowserManager(tabs))
|
||||
b.setActiveTab(100)
|
||||
|
||||
await expect(b.goto('https://example.com/next')).resolves.toEqual({
|
||||
url: 'https://example.com/final',
|
||||
title: 'Replacement'
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('routes direct commands to the replacement registration, not the stale session owner', async () => {
|
||||
const tabs = new Map([['tab-1', 100]])
|
||||
const oldWc = mockWebContents(100, 'https://old.example', 'Old')
|
||||
const replacementWc = mockWebContents(200, 'https://new.example', 'New')
|
||||
replacementWc.debugger.sendCommand.mockImplementation(
|
||||
async (_method: string, params?: unknown) => ({
|
||||
result: {
|
||||
value:
|
||||
(params as { expression?: string } | undefined)?.expression === 'location.origin'
|
||||
? 'https://new.example'
|
||||
: 'New'
|
||||
}
|
||||
})
|
||||
)
|
||||
webContentsFromIdMock.mockImplementation((id: number) =>
|
||||
id === 100 ? oldWc : id === 200 ? replacementWc : null
|
||||
)
|
||||
const b = new AgentBrowserBridge(mockBrowserManager(tabs))
|
||||
b.setActiveTab(100)
|
||||
|
||||
succeedWith({ snapshot: 'ready' })
|
||||
await b.snapshot()
|
||||
tabs.set('tab-1', 200)
|
||||
await b.onProcessSwap('tab-1', 200, 100)
|
||||
execFileMock.mockClear()
|
||||
|
||||
await expect(b.evaluate('document.title', undefined, 'tab-1')).resolves.toEqual({
|
||||
result: 'New',
|
||||
origin: 'https://new.example'
|
||||
})
|
||||
expect(replacementWc.debugger.sendCommand).toHaveBeenCalledWith(
|
||||
'Runtime.evaluate',
|
||||
expect.objectContaining({ expression: 'document.title' })
|
||||
)
|
||||
expect(oldWc.debugger.sendCommand).not.toHaveBeenCalled()
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects oversized browser clipboard writes before spawning agent-browser', async () => {
|
||||
|
|
@ -2121,7 +2409,11 @@ describe('AgentBrowserBridge', () => {
|
|||
|
||||
it('passes stderr through as error message on execFile failure', async () => {
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, _args: string[], _opts: unknown, cb: Function) => {
|
||||
(_bin: string, args: string[], _opts: unknown, cb: Function) => {
|
||||
if (args.includes('close')) {
|
||||
cb(null, JSON.stringify({ success: true, data: null }), '')
|
||||
return
|
||||
}
|
||||
cb(new Error('exit code 1'), '', 'daemon crashed: segfault')
|
||||
}
|
||||
)
|
||||
|
|
@ -2130,7 +2422,11 @@ describe('AgentBrowserBridge', () => {
|
|||
|
||||
it('falls back to error.message when stderr is empty', async () => {
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, _args: string[], _opts: unknown, cb: Function) => {
|
||||
(_bin: string, args: string[], _opts: unknown, cb: Function) => {
|
||||
if (args.includes('close')) {
|
||||
cb(null, JSON.stringify({ success: true, data: null }), '')
|
||||
return
|
||||
}
|
||||
cb(new Error('Command failed'), '', '')
|
||||
}
|
||||
)
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ import type {
|
|||
BrowserCookie
|
||||
} from '../../shared/runtime-types'
|
||||
import { assertClipboardTextWriteWithinLimitWithYield } from '../../shared/clipboard-text'
|
||||
import { normalizeBrowserNavigationUrl } from '../../shared/browser-url'
|
||||
import { iterateBrowserTextInsertionChunks } from './browser-text-insertion'
|
||||
|
||||
// Why: must exceed agent-browser's internal timeouts (goto 30s, wait 60s) so the bridge never kills a command before its own timeout fires.
|
||||
|
|
@ -55,6 +56,7 @@ const EXEC_TIMEOUT_MS = 90_000
|
|||
const CONSECUTIVE_TIMEOUT_LIMIT = 3
|
||||
const WAIT_PROCESS_TIMEOUT_GRACE_MS = 1_000
|
||||
const STALE_SESSION_CLOSE_TIMEOUT_MS = 3_000
|
||||
const EMBEDDED_NAVIGATION_TIMEOUT_MS = 30_000
|
||||
export const AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES = 8 * 1024
|
||||
export const AGENT_BROWSER_CLIPBOARD_WRITE_MAX_BYTES = AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES
|
||||
|
||||
|
|
@ -766,9 +768,53 @@ export class AgentBrowserBridge {
|
|||
}
|
||||
|
||||
async goto(url: string, worktreeId?: string, browserPageId?: string): Promise<BrowserGotoResult> {
|
||||
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
|
||||
return (await this.execAgentBrowser(sessionName, ['goto', url])) as BrowserGotoResult
|
||||
})
|
||||
return this.enqueueTargetedCommand(
|
||||
worktreeId,
|
||||
browserPageId,
|
||||
async (_sessionName, target) => {
|
||||
const wc = this.requireTargetWebContents(target)
|
||||
const navigationUrl = normalizeBrowserNavigationUrl(url)
|
||||
if (!navigationUrl) {
|
||||
throw new BrowserError('invalid_argument', `Unsupported browser URL: ${url}`)
|
||||
}
|
||||
let navigationTimeout: ReturnType<typeof setTimeout> | null = null
|
||||
try {
|
||||
await Promise.race([
|
||||
wc.loadURL(navigationUrl),
|
||||
new Promise<never>((_resolve, reject) => {
|
||||
navigationTimeout = setTimeout(
|
||||
() =>
|
||||
reject(
|
||||
new Error(
|
||||
`Browser navigation timed out after ${EMBEDDED_NAVIGATION_TIMEOUT_MS}ms`
|
||||
)
|
||||
),
|
||||
EMBEDDED_NAVIGATION_TIMEOUT_MS
|
||||
)
|
||||
navigationTimeout.unref?.()
|
||||
})
|
||||
])
|
||||
} catch (error) {
|
||||
if (!this.getWebContents(target.webContentsId)) {
|
||||
throw this.createPageUnavailableError(`orca-tab-${target.browserPageId}`)
|
||||
}
|
||||
throw new BrowserError(
|
||||
'browser_error',
|
||||
`Failed to navigate browser page ${target.browserPageId}: ${error instanceof Error ? error.message : String(error)}`
|
||||
)
|
||||
} finally {
|
||||
if (navigationTimeout) {
|
||||
clearTimeout(navigationTimeout)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: cross-process navigation can replace the guest while retaining the same authoritative page id.
|
||||
const navigatedTarget = this.resolveCommandTarget(worktreeId, target.browserPageId)
|
||||
const navigatedWebContents = this.requireTargetWebContents(navigatedTarget)
|
||||
return { url: navigatedWebContents.getURL(), title: navigatedWebContents.getTitle() }
|
||||
},
|
||||
{ ensureSession: false }
|
||||
)
|
||||
}
|
||||
|
||||
async fill(
|
||||
|
|
@ -1403,9 +1449,62 @@ export class AgentBrowserBridge {
|
|||
worktreeId?: string,
|
||||
browserPageId?: string
|
||||
): Promise<BrowserEvalResult> {
|
||||
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
|
||||
return (await this.execAgentBrowser(sessionName, ['eval', expression])) as BrowserEvalResult
|
||||
})
|
||||
return this.enqueueTargetedCommand(
|
||||
worktreeId,
|
||||
browserPageId,
|
||||
async (_sessionName, target) => {
|
||||
const wc = this.requireTargetWebContents(target)
|
||||
let releaseDebugger = (): void => {}
|
||||
try {
|
||||
releaseDebugger = acquireElectronDebugger(wc).release
|
||||
const { result, exceptionDetails } = (await wc.debugger.sendCommand('Runtime.evaluate', {
|
||||
expression,
|
||||
returnByValue: true,
|
||||
awaitPromise: true
|
||||
})) as {
|
||||
result: { value?: unknown; description?: string }
|
||||
exceptionDetails?: { text: string; exception?: { description?: string } }
|
||||
}
|
||||
if (exceptionDetails) {
|
||||
throw new BrowserError(
|
||||
'browser_eval_error',
|
||||
exceptionDetails.exception?.description ?? exceptionDetails.text
|
||||
)
|
||||
}
|
||||
|
||||
const currentTarget = this.resolveCommandTarget(worktreeId, target.browserPageId)
|
||||
if (currentTarget.webContentsId !== target.webContentsId) {
|
||||
throw new BrowserError(
|
||||
'browser_tab_changed',
|
||||
`Browser page ${target.browserPageId} changed while evaluating; retry the command`
|
||||
)
|
||||
}
|
||||
return {
|
||||
result:
|
||||
result.value !== undefined
|
||||
? typeof result.value === 'object' && result.value !== null
|
||||
? JSON.stringify(result.value)
|
||||
: String(result.value)
|
||||
: (result.description ?? ''),
|
||||
origin: wc.getURL()
|
||||
}
|
||||
} catch (error) {
|
||||
if (error instanceof BrowserError) {
|
||||
throw error
|
||||
}
|
||||
if (!this.getWebContents(target.webContentsId)) {
|
||||
throw this.createPageUnavailableError(`orca-tab-${target.browserPageId}`)
|
||||
}
|
||||
throw new BrowserError(
|
||||
'browser_error',
|
||||
`Failed to evaluate in browser page ${target.browserPageId}: ${error instanceof Error ? error.message : String(error)}`
|
||||
)
|
||||
} finally {
|
||||
releaseDebugger()
|
||||
}
|
||||
},
|
||||
{ ensureSession: false }
|
||||
)
|
||||
}
|
||||
|
||||
async hover(
|
||||
|
|
@ -2283,12 +2382,9 @@ export class AgentBrowserBridge {
|
|||
const managesInterceptRoutes =
|
||||
commandArgs[0] === 'network' && (commandArgs[1] === 'route' || commandArgs[1] === 'unroute')
|
||||
|
||||
// Why: --cdp is init-only; pass the port (not a ws:// URL) so agent-browser's /json discovery sees only the proxy's webview, not the host renderer page.
|
||||
const needsInit = !session.initialized
|
||||
if (needsInit) {
|
||||
const port = session.proxy.getPort()
|
||||
args.push('--cdp', String(port))
|
||||
}
|
||||
// Why: a restarted named daemon auto-launches Chrome unless every invocation reasserts Orca's CDP owner.
|
||||
args.push('--cdp', String(session.proxy.getPort()))
|
||||
|
||||
// Why: exec passthrough can produce a large argv; spreading into push risks V8 argument limits.
|
||||
for (const commandArg of commandArgs) {
|
||||
|
|
@ -2323,6 +2419,8 @@ export class AgentBrowserBridge {
|
|||
await this.runAgentBrowserRaw(sessionName, [
|
||||
'--session',
|
||||
sessionName,
|
||||
'--cdp',
|
||||
String(session.proxy.getPort()),
|
||||
'network',
|
||||
'route',
|
||||
urlPattern,
|
||||
|
|
@ -2368,23 +2466,32 @@ export class AgentBrowserBridge {
|
|||
}
|
||||
|
||||
private closeStaleAgentBrowserSession(sessionName: string): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
return new Promise((resolve, reject) => {
|
||||
let child: ReturnType<typeof execFile> | null = null
|
||||
let settled = false
|
||||
|
||||
const finish = (): void => {
|
||||
const finish = (error?: Error): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
clearTimeout(timeout)
|
||||
resolve()
|
||||
if (error) {
|
||||
reject(error)
|
||||
} else {
|
||||
resolve()
|
||||
}
|
||||
}
|
||||
|
||||
// Why: best-effort daemon cleanup — a wedged close must not block the real browser action.
|
||||
// Why: proceeding after an unverified close can reuse a daemon that owns an unrelated browser.
|
||||
const timeout = setTimeout(() => {
|
||||
child?.kill()
|
||||
finish()
|
||||
finish(
|
||||
new BrowserError(
|
||||
'browser_owner_unavailable',
|
||||
`Could not reset stale helper session ${sessionName}; retry after agent-browser exits`
|
||||
)
|
||||
)
|
||||
}, STALE_SESSION_CLOSE_TIMEOUT_MS)
|
||||
|
||||
try {
|
||||
|
|
@ -2392,10 +2499,23 @@ export class AgentBrowserBridge {
|
|||
this.agentBrowserBin,
|
||||
['--session', sessionName, 'close'],
|
||||
{ timeout: STALE_SESSION_CLOSE_TIMEOUT_MS },
|
||||
finish
|
||||
(error) =>
|
||||
finish(
|
||||
error
|
||||
? new BrowserError(
|
||||
'browser_owner_unavailable',
|
||||
`Could not reset stale helper session ${sessionName}: ${error.message}`
|
||||
)
|
||||
: undefined
|
||||
)
|
||||
)
|
||||
} catch (error) {
|
||||
finish(
|
||||
new BrowserError(
|
||||
'browser_owner_unavailable',
|
||||
`Could not reset stale helper session ${sessionName}: ${error instanceof Error ? error.message : String(error)}`
|
||||
)
|
||||
)
|
||||
} catch {
|
||||
finish()
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
@ -2525,10 +2645,19 @@ export class AgentBrowserBridge {
|
|||
return null
|
||||
}
|
||||
|
||||
private requireTargetWebContents(target: ResolvedBrowserCommandTarget): WebContents {
|
||||
const wc = this.getWebContents(target.webContentsId)
|
||||
if (!wc || wc.isDestroyed()) {
|
||||
throw this.createPageUnavailableError(`orca-tab-${target.browserPageId}`)
|
||||
}
|
||||
return wc
|
||||
}
|
||||
|
||||
private getWebContents(webContentsId: number): Electron.WebContents | null {
|
||||
try {
|
||||
const { webContents } = require('electron')
|
||||
return webContents.fromId(webContentsId) ?? null
|
||||
const target = webContents.fromId(webContentsId)
|
||||
return target && !target.isDestroyed() ? target : null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
|
|
|
|||
|
|
@ -299,7 +299,7 @@ export class RuntimeBrowserCommands {
|
|||
await waitForTabRegistration(browserPageId)
|
||||
}
|
||||
|
||||
// Why: CDP navigation bypasses Electron's webview events, so the renderer's did-navigate listeners never fire; push updates to keep the UI in sync.
|
||||
// Why: helper-driven clicks can bypass Electron navigation events; push authoritative URL/title updates after automation.
|
||||
private notifyRendererNavigation(browserPageId: string, url: string, title: string): void {
|
||||
try {
|
||||
const win = this.host.getAuthoritativeWindow()
|
||||
|
|
@ -1345,7 +1345,7 @@ export class RuntimeBrowserCommands {
|
|||
bridge.setActiveTab(wcId, worktreeId)
|
||||
}
|
||||
|
||||
// Why: the webview loads about:blank first, so navigate via the bridge to make agent-browser's CDP session track the real URL.
|
||||
// Why: the webview loads about:blank first; route navigation through the bridge so its registered owner remains authoritative.
|
||||
if (url && url !== 'about:blank') {
|
||||
try {
|
||||
const result = await bridge.goto(url, worktreeId, browserPageId)
|
||||
|
|
|
|||
|
|
@ -1059,6 +1059,8 @@ export type BrowserErrorCode =
|
|||
| 'browser_no_tab'
|
||||
| 'browser_tab_not_found'
|
||||
| 'browser_tab_closed'
|
||||
| 'browser_tab_changed'
|
||||
| 'browser_owner_unavailable'
|
||||
| 'browser_stale_ref'
|
||||
| 'browser_ref_not_found'
|
||||
| 'browser_navigation_failed'
|
||||
|
|
|
|||
|
|
@ -0,0 +1,240 @@
|
|||
import { execFile } from 'node:child_process'
|
||||
import { chmodSync, existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { createServer, type Server } from 'node:http'
|
||||
import type { AddressInfo } from 'node:net'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { promisify } from 'node:util'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { ensureTerminalVisible, getActiveWorktreeId, waitForActiveWorktree } from './helpers/store'
|
||||
|
||||
const execFileAsync = promisify(execFile)
|
||||
// Why: Unix-domain helper sockets can exceed platform path limits under macOS's long temp root.
|
||||
const shortTempRoot =
|
||||
process.platform === 'win32' ? os.tmpdir() : path.join(path.parse(os.tmpdir()).root, 'tmp')
|
||||
const helperSocketDir = mkdtempSync(path.join(shortTempRoot, 'ob-'))
|
||||
const blockedBrowserPath = path.join(
|
||||
helperSocketDir,
|
||||
process.platform === 'win32' ? 'blocked-browser.cmd' : 'blocked-browser'
|
||||
)
|
||||
const externalLaunchMarker = `${blockedBrowserPath}.attempted`
|
||||
|
||||
writeFileSync(
|
||||
blockedBrowserPath,
|
||||
process.platform === 'win32'
|
||||
? '@echo off\r\ntype nul > "%~f0.attempted"\r\nexit /b 97\r\n'
|
||||
: '#!/bin/sh\n: > "$0.attempted"\nexit 97\n'
|
||||
)
|
||||
if (process.platform !== 'win32') {
|
||||
chmodSync(blockedBrowserPath, 0o755)
|
||||
}
|
||||
|
||||
test.use({
|
||||
orcaAppExtraEnv: {
|
||||
AGENT_BROWSER_SOCKET_DIR: helperSocketDir,
|
||||
PATH: `${path.join(process.cwd(), 'node_modules', '.bin')}${path.delimiter}${process.env.PATH ?? ''}`
|
||||
}
|
||||
})
|
||||
|
||||
type CreatedBrowserTab = {
|
||||
id: string
|
||||
pageId: string
|
||||
}
|
||||
|
||||
type RuntimeResponse = {
|
||||
ok: boolean
|
||||
result?: unknown
|
||||
error?: { code?: string; message?: string }
|
||||
}
|
||||
|
||||
async function startOwnershipServer(): Promise<{
|
||||
sourceUrl: string
|
||||
destinationUrl: string
|
||||
close: () => Promise<void>
|
||||
}> {
|
||||
const server = createServer((request, response) => {
|
||||
const pathname = new URL(request.url ?? '/', 'http://127.0.0.1').pathname
|
||||
const destination = pathname === '/destination'
|
||||
response.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' })
|
||||
response.end(`<!doctype html>
|
||||
<html>
|
||||
<head><title>${destination ? 'Owned destination' : 'Owned source'}</title></head>
|
||||
<body><h1 id="marker">${destination ? 'destination-webview' : 'source-webview'}</h1></body>
|
||||
</html>`)
|
||||
})
|
||||
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
|
||||
const origin = `http://127.0.0.1:${(server.address() as AddressInfo).port}`
|
||||
return {
|
||||
sourceUrl: `${origin}/source`,
|
||||
destinationUrl: `${origin}/destination`,
|
||||
close: () => closeServer(server)
|
||||
}
|
||||
}
|
||||
|
||||
async function closeServer(server: Server): Promise<void> {
|
||||
await new Promise<void>((resolve, reject) =>
|
||||
server.close((error) => {
|
||||
if (error) {
|
||||
reject(error)
|
||||
return
|
||||
}
|
||||
resolve()
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
async function createBrowserTab(
|
||||
page: Page,
|
||||
worktreeId: string,
|
||||
url: string
|
||||
): Promise<CreatedBrowserTab> {
|
||||
const browserTab = await page.evaluate(
|
||||
({ targetWorktreeId, targetUrl }) => {
|
||||
const created = window.__store?.getState().createBrowserTab(targetWorktreeId, targetUrl, {
|
||||
title: 'Embedded owner smoke',
|
||||
activate: true
|
||||
})
|
||||
return created?.activePageId ? { id: created.id, pageId: created.activePageId } : null
|
||||
},
|
||||
{ targetWorktreeId: worktreeId, targetUrl: url }
|
||||
)
|
||||
if (!browserTab) {
|
||||
throw new Error('Failed to create the embedded browser page')
|
||||
}
|
||||
return browserTab
|
||||
}
|
||||
|
||||
async function callBrowserRuntime(
|
||||
page: Page,
|
||||
method: string,
|
||||
params: Record<string, unknown>
|
||||
): Promise<RuntimeResponse> {
|
||||
return (await page.evaluate(
|
||||
({ targetMethod, targetParams }) =>
|
||||
window.api.runtime.call({ method: targetMethod, params: targetParams }),
|
||||
{ targetMethod: method, targetParams: params }
|
||||
)) as RuntimeResponse
|
||||
}
|
||||
|
||||
async function readEmbeddedPage(
|
||||
page: Page,
|
||||
browserTabId: string
|
||||
): Promise<{ marker: string | null; title: string; url: string } | null> {
|
||||
return page.evaluate(async (targetBrowserTabId) => {
|
||||
const overlay = document.querySelector(`[data-browser-overlay-tab-id="${targetBrowserTabId}"]`)
|
||||
const webview = overlay?.querySelector('webview') as Electron.WebviewTag | null
|
||||
if (!webview) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
return (await webview.executeJavaScript(`({
|
||||
marker: document.querySelector('#marker')?.textContent ?? null,
|
||||
title: document.title,
|
||||
url: location.href
|
||||
})`)) as { marker: string | null; title: string; url: string }
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}, browserTabId)
|
||||
}
|
||||
|
||||
function agentBrowserBinary(): string {
|
||||
const suffix = process.platform === 'win32' ? '.exe' : ''
|
||||
return path.join(
|
||||
process.cwd(),
|
||||
'node_modules',
|
||||
'agent-browser',
|
||||
'bin',
|
||||
`agent-browser-${process.platform}-${process.arch}${suffix}`
|
||||
)
|
||||
}
|
||||
|
||||
async function stopHelperDaemon(sessionName: string): Promise<void> {
|
||||
await execFileAsync(agentBrowserBinary(), ['--session', sessionName, 'close', '--json'], {
|
||||
env: { ...process.env, AGENT_BROWSER_SOCKET_DIR: helperSocketDir },
|
||||
timeout: 10_000
|
||||
})
|
||||
await expect
|
||||
.poll(() => existsSync(path.join(helperSocketDir, `${sessionName}.pid`)), { timeout: 5_000 })
|
||||
.toBe(false)
|
||||
}
|
||||
|
||||
test('stale helper cannot take goto or eval away from the real embedded webview', async ({
|
||||
electronApp,
|
||||
orcaPage,
|
||||
registerPostElectronShutdownCleanup
|
||||
}) => {
|
||||
registerPostElectronShutdownCleanup(async () => {
|
||||
rmSync(helperSocketDir, { recursive: true, force: true })
|
||||
})
|
||||
const server = await startOwnershipServer()
|
||||
try {
|
||||
await waitForActiveWorktree(orcaPage)
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
const worktreeId = await getActiveWorktreeId(orcaPage)
|
||||
if (!worktreeId) {
|
||||
throw new Error('Expected an active worktree for the embedded browser smoke test')
|
||||
}
|
||||
const browserTab = await createBrowserTab(orcaPage, worktreeId, server.sourceUrl)
|
||||
|
||||
await expect
|
||||
.poll(() => readEmbeddedPage(orcaPage, browserTab.id), { timeout: 10_000 })
|
||||
.toMatchObject({ marker: 'source-webview', title: 'Owned source', url: server.sourceUrl })
|
||||
|
||||
const snapshot = await callBrowserRuntime(orcaPage, 'browser.snapshot', {
|
||||
page: browserTab.pageId
|
||||
})
|
||||
expect(snapshot, JSON.stringify(snapshot)).toMatchObject({ ok: true })
|
||||
|
||||
const sessionName = `orca-tab-${browserTab.pageId}`
|
||||
await expect
|
||||
.poll(() => existsSync(path.join(helperSocketDir, `${sessionName}.pid`)), {
|
||||
timeout: 5_000
|
||||
})
|
||||
.toBe(true)
|
||||
|
||||
// Why: if routing escapes the registered webview, this executable records the attempt without launching Chrome.
|
||||
await electronApp.evaluate((_electron, executablePath) => {
|
||||
process.env.AGENT_BROWSER_EXECUTABLE_PATH = executablePath
|
||||
}, blockedBrowserPath)
|
||||
await stopHelperDaemon(sessionName)
|
||||
|
||||
const navigation = await callBrowserRuntime(orcaPage, 'browser.goto', {
|
||||
page: browserTab.pageId,
|
||||
url: server.destinationUrl
|
||||
})
|
||||
expect(existsSync(externalLaunchMarker)).toBe(false)
|
||||
expect(navigation).toMatchObject({
|
||||
ok: true,
|
||||
result: { url: server.destinationUrl, title: 'Owned destination' }
|
||||
})
|
||||
await expect
|
||||
.poll(() => readEmbeddedPage(orcaPage, browserTab.id), { timeout: 10_000 })
|
||||
.toMatchObject({
|
||||
marker: 'destination-webview',
|
||||
title: 'Owned destination',
|
||||
url: server.destinationUrl
|
||||
})
|
||||
await expect(orcaPage.locator(`[data-tab-id="${browserTab.id}"]`)).toContainText(
|
||||
'Owned destination'
|
||||
)
|
||||
|
||||
const evaluation = await callBrowserRuntime(orcaPage, 'browser.eval', {
|
||||
page: browserTab.pageId,
|
||||
expression: 'document.querySelector("#marker")?.textContent'
|
||||
})
|
||||
expect(existsSync(externalLaunchMarker)).toBe(false)
|
||||
expect(evaluation).toMatchObject({
|
||||
ok: true,
|
||||
result: { result: 'destination-webview', origin: server.destinationUrl }
|
||||
})
|
||||
// Why: the stopped helper has no cleanup left; a PID after eval means a forbidden relaunch.
|
||||
expect(existsSync(path.join(helperSocketDir, `${sessionName}.pid`))).toBe(false)
|
||||
} finally {
|
||||
await electronApp.evaluate(() => {
|
||||
delete process.env.AGENT_BROWSER_EXECUTABLE_PATH
|
||||
})
|
||||
await server.close()
|
||||
}
|
||||
})
|
||||
Loading…
Reference in New Issue