fix(browser): reject a ref from another tab's snapshot after a tab switch (#36251)

Each page has its own agent-browser ref map, so an unpinned `click e2` after
`tab switch` hit the new tab's e2 and reported success.
This commit is contained in:
Kelvin Amoaba
2026-10-30 05:44:04 +00:00
committed by GitHub
parent 2e37fc4571
commit 2b6e63ee2f
13 changed files with 334 additions and 118 deletions
@@ -34,7 +34,17 @@ function withoutDaemonLifecycle(envelope: AgentBrowserEnvelope): AgentBrowserEnv
return { ...envelope, data: rest }
}
/** "Unknown ref" for a ref never issued; 0.39+ also says "Stale ref for role=…" once its element is gone. */
/** Every snapshot form (plain, --full, --delta) answers with `snapshot`; annotated screenshots with `annotations`. */
export function agentBrowserResultMintsRefs(data: unknown): boolean {
if (!data || typeof data !== 'object') {
return false
}
return 'snapshot' in data || ('annotations' in data && Array.isArray(data.annotations))
}
/** "Unknown ref" for a ref never issued; 0.39+ says "Stale ref for role=…" or "Could not locate element with role=…" once its element is gone. */
export function isAgentBrowserStaleRefError(message: string): boolean {
return /unknown ref|stale ref|ref not found|element not found: @e/i.test(message)
return /unknown ref|stale ref|ref not found|element not found: @e|could not locate element with role=/i.test(
message
)
}
@@ -210,11 +210,12 @@ describe('AgentBrowserBridge', () => {
await expect(bridge.click('@e2')).resolves.toEqual({ clicked: '@e2' })
})
it('reports a ref whose element is gone as stale', async () => {
// agent-browser 0.39.2+ wording when it cannot re-resolve a ref by role and name.
failWith(
'Stale ref for role=button name=Approve cannot be recovered: several elements now match. Run snapshot again and use a fresh ref.'
)
// agent-browser 0.39.2+ wording when it cannot re-resolve a ref by role and name.
it.each([
'Stale ref for role=button name=Approve cannot be recovered: several elements now match. Run snapshot again and use a fresh ref.',
'Could not locate element with role=link name=Back to basic'
])('reports a ref whose element is gone as stale: %s', async (message) => {
failWith(message)
await expect(bridge.click('@e2')).rejects.toMatchObject({ code: 'browser_stale_ref' })
})
@@ -34,9 +34,14 @@ export abstract class AgentBrowserBridgeCoreCommands extends AgentBrowserBridgeQ
worktreeId?: string,
browserPageId?: string
): Promise<BrowserClickResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['click', element])) as BrowserClickResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['click', element])) as BrowserClickResult
},
{ elements: [element] }
)
}
async dblclick(
@@ -44,9 +49,18 @@ export abstract class AgentBrowserBridgeCoreCommands extends AgentBrowserBridgeQ
worktreeId?: string,
browserPageId?: string
): Promise<BrowserClickResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['dblclick', element])) as BrowserClickResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: agent-browser's dblclick --json answers the BrowserClickResult shape.
return (await this.execAgentBrowser(sessionName, [
'dblclick',
element
])) as BrowserClickResult
},
{ elements: [element] }
)
}
async goto(url: string, worktreeId?: string, browserPageId?: string): Promise<BrowserGotoResult> {
@@ -162,7 +176,7 @@ export abstract class AgentBrowserBridgeCoreCommands extends AgentBrowserBridgeQ
await this.fillExplicitContentEditable(sessionName, element, value)
return { filled: element } as BrowserFillResult
},
{ requireScopedTarget: true }
{ requireScopedTarget: true, elements: [element] }
)
}
}
@@ -10,7 +10,11 @@ import {
pageUnavailableMessageForSession
} from './agent-browser-bridge-process'
import { translateResult } from './agent-browser-bridge-result'
import { AGENT_BROWSER_BATCH_ARGS, agentBrowserBatchInput } from './agent-browser-batch-command'
import {
AGENT_BROWSER_BATCH_ARGS,
agentBrowserBatchInput,
agentBrowserResultMintsRefs
} from './agent-browser-batch-command'
import { AgentBrowserBridgeTabs } from './agent-browser-bridge-tabs'
import { assertRemoteCliBrowserAccess } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope'
import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep'
@@ -114,6 +118,13 @@ export abstract class AgentBrowserBridgeExecution extends AgentBrowserBridgeTabs
)
}
if (agentBrowserResultMintsRefs(translated.result)) {
const browserPageId = this.resolveTabIdSafe(session.webContentsId)
if (browserPageId) {
this.recordRefPage(browserPageId)
}
}
if (needsInit) {
session.initialized = true
@@ -37,13 +37,18 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge
worktreeId?: string,
browserPageId?: string
): Promise<BrowserSelectResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, [
'select',
element,
value
])) as BrowserSelectResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return (await this.execAgentBrowser(sessionName, [
'select',
element,
value
])) as BrowserSelectResult
},
{ elements: [element] }
)
}
async scroll(
@@ -66,9 +71,14 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge
worktreeId?: string,
browserPageId?: string
): Promise<unknown> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return await this.execAgentBrowser(sessionName, ['scrollintoview', element])
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return await this.execAgentBrowser(sessionName, ['scrollintoview', element])
},
{ elements: [element] }
)
}
async get(
@@ -77,13 +87,18 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge
worktreeId?: string,
browserPageId?: string
): Promise<unknown> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
const args = ['get', what]
if (selector) {
args.push(selector)
}
return await this.execAgentBrowser(sessionName, args)
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
const args = ['get', what]
if (selector) {
args.push(selector)
}
return await this.execAgentBrowser(sessionName, args)
},
{ elements: [selector] }
)
}
async is(
@@ -92,9 +107,14 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge
worktreeId?: string,
browserPageId?: string
): Promise<unknown> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return await this.execAgentBrowser(sessionName, ['is', what, selector])
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return await this.execAgentBrowser(sessionName, ['is', what, selector])
},
{ elements: [selector] }
)
}
async keyboardInsertText(
@@ -28,9 +28,14 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserHoverResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['hover', element])) as BrowserHoverResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['hover', element])) as BrowserHoverResult
},
{ elements: [element] }
)
}
async drag(
@@ -39,9 +44,14 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserDragResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['drag', from, to])) as BrowserDragResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['drag', from, to])) as BrowserDragResult
},
{ elements: [from, to] }
)
}
async upload(
@@ -50,13 +60,18 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserUploadResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, [
'upload',
element,
...filePaths
])) as BrowserUploadResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return (await this.execAgentBrowser(sessionName, [
'upload',
element,
...filePaths
])) as BrowserUploadResult
},
{ elements: [element] }
)
}
async wait(
@@ -64,22 +79,27 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserWaitResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
const { args, conditionTimeoutMs, processTimeoutMs } = agentBrowserWaitArguments(
options ?? {}
)
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: agent-browser's wait --json answers the BrowserWaitResult shape.
return (await this.execAgentBrowser(sessionName, args, {
timeoutMs: processTimeoutMs,
timeoutError:
conditionTimeoutMs === undefined
? undefined
: new BrowserError(
'browser_timeout',
`Timed out waiting for browser condition after ${conditionTimeoutMs}ms.`
)
})) as BrowserWaitResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
const { args, conditionTimeoutMs, processTimeoutMs } = agentBrowserWaitArguments(
options ?? {}
)
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: agent-browser's wait --json answers the BrowserWaitResult shape.
return (await this.execAgentBrowser(sessionName, args, {
timeoutMs: processTimeoutMs,
timeoutError:
conditionTimeoutMs === undefined
? undefined
: new BrowserError(
'browser_timeout',
`Timed out waiting for browser condition after ${conditionTimeoutMs}ms.`
)
})) as BrowserWaitResult
},
{ elements: [options?.selector] }
)
}
async check(
@@ -88,10 +108,15 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserCheckResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
const args = checked ? ['check', element] : ['uncheck', element]
return (await this.execAgentBrowser(sessionName, args)) as BrowserCheckResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
const args = checked ? ['check', element] : ['uncheck', element]
return (await this.execAgentBrowser(sessionName, args)) as BrowserCheckResult
},
{ elements: [element] }
)
}
async focus(
@@ -99,9 +124,14 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserFocusResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['focus', element])) as BrowserFocusResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['focus', element])) as BrowserFocusResult
},
{ elements: [element] }
)
}
async clear(
@@ -122,7 +152,7 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
await this.fillExplicitContentEditable(sessionName, element, '')
return { cleared: element }
},
{ requireScopedTarget: true }
{ requireScopedTarget: true, elements: [element] }
)
}
@@ -131,13 +161,18 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserSelectAllResult> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
await this.execAgentBrowser(sessionName, ['focus', element])
return (await this.execAgentBrowser(sessionName, [
'press',
'Control+a'
])) as BrowserSelectAllResult
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
await this.execAgentBrowser(sessionName, ['focus', element])
return (await this.execAgentBrowser(sessionName, [
'press',
'Control+a'
])) as BrowserSelectAllResult
},
{ elements: [element] }
)
}
async keypress(
@@ -70,6 +70,9 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
browserPageId,
options.requireScopedTarget
)
if (!browserPageId && options.elements) {
this.assertRefsFromPage(pageId, options.elements)
}
const sessionName = `${ORCA_TAB_SESSION_PREFIX}${pageId}`
const agentCommand = this.browserManager.beginAgentCommand(pageId)
@@ -263,10 +263,39 @@ export abstract class AgentBrowserBridgeStateCommands extends AgentBrowserBridge
}
async exec(command: string, worktreeId?: string, browserPageId?: string): Promise<unknown> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
// Passthrough arguments must not override Orca's selected page or CDP proxy.
const args = stripAgentBrowserTargetArgs(parseShellArgs(command.trim()))
return await this.execAgentBrowser(sessionName, args, { cliArgv: true })
})
// Passthrough arguments must not override Orca's selected page or CDP proxy.
const args = stripAgentBrowserTargetArgs(parseShellArgs(command.trim()))
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => await this.execAgentBrowser(sessionName, args, { cliArgv: true }),
{ elements: passthroughElementArgs(args) }
)
}
}
// Commands whose first operand is an element, where agent-browser reads a bare `e2` as a ref.
const ELEMENT_FIRST_COMMANDS = new Set([
'click',
'dblclick',
'hover',
'focus',
'check',
'uncheck',
'fill',
'type',
'select',
'scrollintoview',
'upload',
'highlight',
'download',
'drag'
])
// Free text such as `keyboard type e2` is not a ref; explicit `@e2`/`ref=e2` are refs anywhere.
function passthroughElementArgs([command, ...operands]: string[]): string[] {
const explicit = operands.filter((arg) => arg.startsWith('@') || arg.startsWith('ref='))
return command && ELEMENT_FIRST_COMMANDS.has(command) && operands[0]
? [operands[0], ...explicit]
: explicit
}
@@ -15,6 +15,8 @@ export abstract class AgentBrowserBridgeState {
protected activeWebContentsId: number | null = null
// A person's tab pick waiting for the agent's tab to go idle; the undefined key is the global target.
protected readonly heldPersonSelections = new Map<string | undefined, number>()
// Each page has its own ref map; a worktree's unpinned refs belong to the page that last minted refs.
protected readonly refPageByWorktree = new Map<string | undefined, string>()
protected readonly sessions = new Map<string, SessionState>()
protected readonly commandQueues = new Map<string, QueuedCommand[]>()
protected readonly processingQueues = new Set<string>()
@@ -339,6 +339,48 @@ describe('AgentBrowserBridge', () => {
])
})
it('rejects an unpinned ref from another tab snapshot until the current tab is snapshotted', async () => {
const tabs = new Map([
['tab-a', 1],
['tab-b', 2]
])
const worktrees = new Map([
['tab-a', 'wt-1'],
['tab-b', 'wt-1']
])
const wc1 = mockWebContents(1)
const wc2 = mockWebContents(2)
webContentsFromIdMock.mockImplementation((id: number) => (id === 1 ? wc1 : wc2))
const b = new AgentBrowserBridge(mockBrowserManager(tabs, worktrees))
b.setActiveTab(1, 'wt-1')
const clickSessions: string[] = []
answerAgentBrowserCommands(execFileMock, stdinWrites, (command, cb, args) => {
if (command[0] === 'click') {
clickSessions.push(args[args.indexOf('--session') + 1])
}
const data = command[0] === 'snapshot' ? { snapshot: '' } : {}
cb(null, JSON.stringify({ success: true, data }), '')
})
await b.snapshot('wt-1')
await b.tabSwitch(undefined, 'wt-1', 'tab-b')
await expect(b.click('@e2', 'wt-1')).rejects.toMatchObject({
code: 'browser_stale_ref',
message: expect.stringContaining(
'Run snapshot to get refs for page tab-b, or pass --page tab-a'
)
})
await expect(b.exec('click e2', 'wt-1')).rejects.toMatchObject({ code: 'browser_stale_ref' })
await b.exec('keyboard type e2', 'wt-1')
expect(clickSessions).toEqual([])
await b.click('@e2', undefined, 'tab-a')
await b.exec('snapshot -i', 'wt-1')
await b.click('@e2', 'wt-1')
expect(clickSessions).toEqual(['orca-tab-tab-a', 'orca-tab-tab-b'])
})
it('queues tabSwitch behind in-flight commands on the current session', async () => {
const tabs = new Map([
['tab-a', 1],
@@ -53,6 +53,38 @@ export abstract class AgentBrowserBridgeTabs extends AgentBrowserBridgeState {
}
}
protected recordRefPage(browserPageId: string): void {
this.refPageByWorktree.set(
this.browserManager.getWorktreeIdForTab(browserPageId),
browserPageId
)
}
// Refs are numbered per page, so another page's ref would silently match an element here.
protected assertRefsFromPage(
browserPageId: string,
elements: readonly (string | undefined)[]
): void {
const ref = elements.find((element) => element !== undefined && isSnapshotRef(element))
if (!ref) {
return
}
const snapshotPageId = this.refPageByWorktree.get(
this.browserManager.getWorktreeIdForTab(browserPageId)
)
if (!snapshotPageId || snapshotPageId === browserPageId) {
return
}
// The latest snapshot may have been a pinned look elsewhere, so the snapshot hint comes first.
const pinnedRetry = this.browserManager.getWebContentsIdByTabId().has(snapshotPageId)
? `, or pass --page ${snapshotPageId} if ${ref} came from that page's snapshot`
: ''
throw new BrowserError(
'browser_stale_ref',
`The latest snapshot was of page ${snapshotPageId}, not the current tab (page ${browserPageId}), so ${ref} may point at the wrong element. Run snapshot to get refs for page ${browserPageId}${pinnedRetry}.`
)
}
private isAgentUsingTab(webContentsId: number): boolean {
const browserPageId = this.resolveTabIdSafe(webContentsId)
return browserPageId !== null && this.browserManager.isAgentUsingPage(browserPageId)
@@ -296,3 +328,8 @@ export abstract class AgentBrowserBridgeTabs extends AgentBrowserBridgeState {
function routingKeys(worktreeId: string | undefined): (string | undefined)[] {
return worktreeId ? [undefined, worktreeId] : [undefined]
}
// agent-browser reads `@e3`, `ref=e3` and bare `e3` as snapshot refs.
function isSnapshotRef(element: string): boolean {
return /^(?:@|ref=)?e\d+$/.test(element)
}
@@ -65,6 +65,8 @@ export type EnqueueTargetedCommandOptions = {
ensureSession?: boolean
// Text mutations must not fall back to a global tab from another worktree.
requireScopedTarget?: boolean
// Element arguments that may be snapshot refs.
elements?: readonly (string | undefined)[]
}
export type AgentBrowserBridgeOptions = {
@@ -113,45 +113,55 @@ export abstract class AgentBrowserBridgeUtilityCommands extends AgentBrowserBrid
throw new BrowserError('invalid_argument', `--path must be an absolute path: ${savePath}`)
}
const replyDeadline = Date.now() + AGENT_DOWNLOAD_REPLY_DEADLINE_MS
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName, target) => {
await mkdir(path.dirname(savePath), { recursive: true })
const claim = this.browserManager.claimNextDownload(target.webContentsId, savePath)
try {
await this.execAgentBrowser(sessionName, ['click', selector])
if (!(await withTimeout(claim.started, AGENT_DOWNLOAD_START_TIMEOUT_MS, false))) {
throw new BrowserError(
'browser_download_not_started',
`Clicking ${selector} did not start a download within ${AGENT_DOWNLOAD_START_TIMEOUT_MS / 1000}s.`
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName, target) => {
await mkdir(path.dirname(savePath), { recursive: true })
const claim = this.browserManager.claimNextDownload(target.webContentsId, savePath)
try {
await this.execAgentBrowser(sessionName, ['click', selector])
if (!(await withTimeout(claim.started, AGENT_DOWNLOAD_START_TIMEOUT_MS, false))) {
throw new BrowserError(
'browser_download_not_started',
`Clicking ${selector} did not start a download within ${AGENT_DOWNLOAD_START_TIMEOUT_MS / 1000}s.`
)
}
const finished = await withTimeout(
claim.finished,
Math.max(0, replyDeadline - Date.now()),
null
)
if (!finished) {
throw new BrowserError(
'browser_download_in_progress',
`The download is still in progress and will be saved to ${savePath}.`
)
}
if (finished.status !== 'completed') {
throw new BrowserError(
'browser_download_failed',
finished.error ?? `The download was ${finished.status}.`
)
}
return { path: savePath }
} finally {
claim.release()
}
const finished = await withTimeout(
claim.finished,
Math.max(0, replyDeadline - Date.now()),
null
)
if (!finished) {
throw new BrowserError(
'browser_download_in_progress',
`The download is still in progress and will be saved to ${savePath}.`
)
}
if (finished.status !== 'completed') {
throw new BrowserError(
'browser_download_failed',
finished.error ?? `The download was ${finished.status}.`
)
}
return { path: savePath }
} finally {
claim.release()
}
})
},
{ elements: [selector] }
)
}
async highlight(selector: string, worktreeId?: string, browserPageId?: string): Promise<unknown> {
return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => {
return await this.execAgentBrowser(sessionName, ['highlight', selector])
})
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
async (sessionName) => {
return await this.execAgentBrowser(sessionName, ['highlight', selector])
},
{ elements: [selector] }
)
}
async back(worktreeId?: string, browserPageId?: string): Promise<BrowserBackResult> {