fix(browser): stop agent clicks from opening the system file picker over your work (#41299)

* fix(browser): stop agent clicks from opening the system file picker

An agent click on a file input opened the macOS Open panel, which brought
Orca's window forward over the app the user was in. Page actions now catch
the picker the way upload does and tell the agent to use upload.

* fix(browser): enable Page, cancel the picker, and clean up outside host scope

* fix(browser): limit picker blocking to input delivery and bound its setup

* perf(browser): skip picker blocking for actions without user activation

* fix(browser): bound picker interception cleanup on a hung page

* fix(browser): bound the picker interception setup as a whole
This commit is contained in:
Kelvin Amoaba
2026-11-12 00:18:39 +00:00
committed by GitHub
parent 4509730131
commit ada69cba8c
11 changed files with 253 additions and 37 deletions
+1 -1
View File
@@ -64,7 +64,7 @@ ORCA exec --page <browserPageId> --command "get text @e3" --json
Browser rules:
- Actions (`click`, `fill`, `type`, `keypress`, `select`, `check`, `hover`, `scroll`, `goto`, `back`, `reload`, `mouse` and the rest) wait briefly for the page to settle and then report what they caused in `result.effects` (text mode prints it under the action): `url`/`title` when they changed, `newTabs` with their `browserPageId`, blocked or opened `popups`, `downloads` started, `consoleErrors`, a `passkeyRequest` still waiting, and `page`: `"unchanged"`, or the snapshot lines the action added or replaced, with their refs (`full: true` means the whole new page). `loading: true` means a navigation was still running: `wait --load load`, then `snapshot`. No `page` means the page could not be read: run `snapshot`. `settled: false` means the page was still changing when it was read. An action that opens a JavaScript dialog fails with `browser_dialog_opened` instead.
- Actions (`click`, `fill`, `type`, `keypress`, `select`, `check`, `hover`, `scroll`, `goto`, `back`, `reload`, `mouse` and the rest) wait briefly for the page to settle and then report what they caused in `result.effects` (text mode prints it under the action): `url`/`title` when they changed, `newTabs` with their `browserPageId`, blocked or opened `popups`, `downloads` started, `consoleErrors`, a `passkeyRequest` still waiting, and `page`: `"unchanged"`, or the snapshot lines the action added or replaced, with their refs (`full: true` means the whole new page). `loading: true` means a navigation was still running: `wait --load load`, then `snapshot`. No `page` means the page could not be read: run `snapshot`. `settled: false` means the page was still changing when it was read. An action that opens a JavaScript dialog fails with `browser_dialog_opened` instead. One that opens a file picker fails with `browser_file_chooser_opened`: Orca closes the picker so the system dialog never covers the user's screen; use `ORCA upload` on the file input or its button instead.
- Snapshot again only when `page.truncated` is true, after `wait`, after a tab switch, on a `browser_stale_ref`, or when a result has no `effects` (older Orca, or a server-hosted page).
- Pass `--expect-url <url>` with `--page` when acting on the wrong resource would matter, such as writing or sending in one conversation of a chat app. Use the exact absolute URL from `result.tab.url` in `ORCA tab current --json`, or `url` from a `snapshot --json` or `goto`. Path, query and fragment must all match, so a page that rewrites its URL while you work refuses until you read the URL again. If the page's URL differs, or a navigation to another URL is under way, the command fails with `browser_expected_url_mismatch`, and `error.data` holds `expectedUrl`, `actualUrl`, `browserPageId` and `mayHaveStarted`. `mayHaveStarted: false` means nothing ran; `true` means a step of a longer command (a typed run, a `select-all`) may have happened, so look at the page before retrying. Success adds `browserPageId` and `checkedUrl`, the URL last checked; the action itself may have navigated since, and `effects.url` says where. The check runs right before the action and again before each later step, but it is not a lock: a route change in the instant after a check can still receive the action. It compares the top-level page only, so it says nothing about an iframe or an app that shows different content at one URL; read the page for those. `exec` refuses the flag, because one raw batch can navigate and retarget. A runtime, client or desktop that cannot check the URL refuses with `incompatible_runtime` or `browser_expect_url_unsupported` rather than acting unguarded.
- Refs like `@e1` are assigned by `snapshot`, scoped to one tab, and invalidated by navigation or tab switch.
+1 -1
View File
@@ -759,7 +759,7 @@ const ORCA_CLI_BROWSER_REFERENCE_MARKDOWN = [
"",
"Browser rules:",
"",
"- Actions (`click`, `fill`, `type`, `keypress`, `select`, `check`, `hover`, `scroll`, `goto`, `back`, `reload`, `mouse` and the rest) wait briefly for the page to settle and then report what they caused in `result.effects` (text mode prints it under the action): `url`/`title` when they changed, `newTabs` with their `browserPageId`, blocked or opened `popups`, `downloads` started, `consoleErrors`, a `passkeyRequest` still waiting, and `page`: `\"unchanged\"`, or the snapshot lines the action added or replaced, with their refs (`full: true` means the whole new page). `loading: true` means a navigation was still running: `wait --load load`, then `snapshot`. No `page` means the page could not be read: run `snapshot`. `settled: false` means the page was still changing when it was read. An action that opens a JavaScript dialog fails with `browser_dialog_opened` instead.",
"- Actions (`click`, `fill`, `type`, `keypress`, `select`, `check`, `hover`, `scroll`, `goto`, `back`, `reload`, `mouse` and the rest) wait briefly for the page to settle and then report what they caused in `result.effects` (text mode prints it under the action): `url`/`title` when they changed, `newTabs` with their `browserPageId`, blocked or opened `popups`, `downloads` started, `consoleErrors`, a `passkeyRequest` still waiting, and `page`: `\"unchanged\"`, or the snapshot lines the action added or replaced, with their refs (`full: true` means the whole new page). `loading: true` means a navigation was still running: `wait --load load`, then `snapshot`. No `page` means the page could not be read: run `snapshot`. `settled: false` means the page was still changing when it was read. An action that opens a JavaScript dialog fails with `browser_dialog_opened` instead. One that opens a file picker fails with `browser_file_chooser_opened`: Orca closes the picker so the system dialog never covers the user's screen; use `ORCA upload` on the file input or its button instead.",
"- Snapshot again only when `page.truncated` is true, after `wait`, after a tab switch, on a `browser_stale_ref`, or when a result has no `effects` (older Orca, or a server-hosted page).",
"- Pass `--expect-url <url>` with `--page` when acting on the wrong resource would matter, such as writing or sending in one conversation of a chat app. Use the exact absolute URL from `result.tab.url` in `ORCA tab current --json`, or `url` from a `snapshot --json` or `goto`. Path, query and fragment must all match, so a page that rewrites its URL while you work refuses until you read the URL again. If the page's URL differs, or a navigation to another URL is under way, the command fails with `browser_expected_url_mismatch`, and `error.data` holds `expectedUrl`, `actualUrl`, `browserPageId` and `mayHaveStarted`. `mayHaveStarted: false` means nothing ran; `true` means a step of a longer command (a typed run, a `select-all`) may have happened, so look at the page before retrying. Success adds `browserPageId` and `checkedUrl`, the URL last checked; the action itself may have navigated since, and `effects.url` says where. The check runs right before the action and again before each later step, but it is not a lock: a route change in the instant after a check can still receive the action. It compares the top-level page only, so it says nothing about an iframe or an app that shows different content at one URL; read the page for those. `exec` refuses the flag, because one raw batch can navigate and retarget. A runtime, client or desktop that cannot check the URL refuses with `incompatible_runtime` or `browser_expect_url_unsupported` rather than acting unguarded.",
"- Refs like `@e1` are assigned by `snapshot`, scoped to one tab, and invalidated by navigation or tab switch.",
@@ -0,0 +1,125 @@
import { EventEmitter } from 'node:events'
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { AgentBrowserBridge } from '../agent-browser-bridge'
import { runInRemoteCliBrowserScope } from '../../../runtime/desktop-browser-bridge/remote-cli-browser-scope'
import {
answerAgentBrowserCommands,
mockBrowserManager,
mockWebContents,
overrideBridgeWebContentsLookup,
resetAgentBrowserBridgeMocks
} from '../testing/agent-browser-bridge-test-harness'
const { execFileMock, webContentsFromIdMock, existsSyncMock, readFileSyncMock, stdinWrites } =
vi.hoisted(() => ({
execFileMock: vi.fn(),
webContentsFromIdMock: vi.fn(),
existsSyncMock: vi.fn(() => false),
readFileSyncMock: vi.fn(() => Buffer.from('')),
stdinWrites: new Array<string>()
}))
vi.mock('child_process', () => ({ execFile: execFileMock }))
vi.mock('fs', () => ({
existsSync: existsSyncMock,
readFileSync: readFileSyncMock,
accessSync: vi.fn(),
chmodSync: vi.fn(),
constants: { X_OK: 1 }
}))
vi.mock('os', () => ({ platform: () => 'darwin', arch: () => 'arm64' }))
vi.mock('electron', () => ({
app: { getPath: vi.fn(() => '/app'), getAppPath: vi.fn(() => '/project'), isPackaged: false },
webContents: { fromId: webContentsFromIdMock }
}))
const { CdpWsProxyMock } = vi.hoisted(() => {
const instances: unknown[] = []
const MockClass = vi.fn().mockImplementation(function (this: Record<string, unknown>) {
this.start = vi.fn(async () => 'ws://127.0.0.1:9222')
this.stop = vi.fn(async () => {})
this.getPort = vi.fn(() => 9222)
this.pointAtNextNode = vi.fn()
instances.push(this)
})
return { CdpWsProxyMock: Object.assign(MockClass, { instances }) }
})
vi.mock('../../cdp/transport/cdp-ws-proxy', () => ({ CdpWsProxy: CdpWsProxyMock }))
overrideBridgeWebContentsLookup(AgentBrowserBridge.prototype, webContentsFromIdMock)
describe('an agent click that opens a file picker', () => {
let debuggerEvents: EventEmitter
let intercepting: boolean
let bridge: AgentBrowserBridge
let onClick: () => void
beforeEach(() => {
resetAgentBrowserBridgeMocks({
webContentsFromIdMock,
existsSyncMock,
readFileSyncMock,
stdinWrites,
cdpWsProxyInstances: CdpWsProxyMock.instances
})
intercepting = false
onClick = () => {}
debuggerEvents = new EventEmitter()
const guest = mockWebContents(100)
guest.debugger = {
isAttached: vi.fn(() => true),
attach: vi.fn(),
detach: vi.fn(),
on: vi.fn((event, listener) => {
debuggerEvents.on(event, listener)
}),
removeListener: vi.fn((event, listener) => {
debuggerEvents.removeListener(event, listener)
}),
sendCommand: vi.fn(async (method: string, params?: unknown): Promise<unknown> => {
if (method === 'Page.setInterceptFileChooserDialog') {
intercepting =
typeof params === 'object' &&
params !== null &&
'enabled' in params &&
params.enabled === true
}
return {}
})
}
webContentsFromIdMock.mockReturnValue(guest)
answerAgentBrowserCommands(execFileMock, stdinWrites, (command, cb) => {
// Chromium shows the OS dialog unless interception is on when the page opens it.
if (command[0] === 'click' && intercepting) {
debuggerEvents.emit('message', {}, 'Page.fileChooserOpened', { backendNodeId: 7 })
}
if (command[0] === 'click') {
onClick()
}
cb(null, JSON.stringify({ success: true, data: {} }), '')
})
bridge = new AgentBrowserBridge(mockBrowserManager())
bridge.setActiveTab(100)
})
it('catches the picker instead of letting the OS dialog take over the screen', async () => {
await expect(bridge.click('@e3')).rejects.toMatchObject({
code: 'browser_file_chooser_opened'
})
expect(intercepting).toBe(false)
})
it("stops intercepting the desktop user's pickers when an SSH host loses the page mid-click", async () => {
const revoked = new AbortController()
onClick = () => revoked.abort()
const click = runInRemoteCliBrowserScope(
'workspace',
() => bridge.click('@e3'),
revoked.signal,
() => true
)
await expect(click).rejects.toBeDefined()
expect(intercepting).toBe(false)
})
})
@@ -9,6 +9,7 @@ import { runWithActionEffects } from '../observation/agent-browser-action-effect
import { getRequestedPageUrl, runExpectingPageUrl } from '../../cdp/input/browser-expected-page-url'
import { AGENT_BROWSER_CLEANUP_TIMEOUT_MS } from '../session-lifecycle/agent-browser-cleanup-options'
import { releaseHeldPageInput } from '../../cdp/input/guest-direct-key-input'
import { withFileChooserBlocked } from '../../cdp/input/guest-file-chooser'
import { onBrowserJavaScriptDialogOpened } from '../../guest-lifecycle/prompts/browser-javascript-dialogs'
import {
assertNoJavaScriptDialog,
@@ -75,6 +76,19 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeActionEf
})
}
/** Navigation, pointer moves, wheel and scrolling deliver no user activation, so they open no file picker to block. */
protected enqueueInertAction<T>(
worktreeId: string | undefined,
browserPageId: string | undefined,
execute: (sessionName: string, target: ResolvedBrowserCommandTarget) => Promise<T>,
options: EnqueueTargetedCommandOptions = {}
): Promise<T> {
return this.enqueuePageAction(worktreeId, browserPageId, execute, {
...options,
skipFileChooserBlock: true
})
}
protected async enqueueTargetedCommand<T>(
worktreeId: string | undefined,
browserPageId: string | undefined,
@@ -103,6 +117,8 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeActionEf
const inThisCall = captureActiveRpcCall()
// Decided at enqueue: a queued command runs under the caller of the command ahead of it.
const reportEffects = options.reportEffects === true && agentCommand.agent
// Page actions deliver agent or remote input; a desktop picker would activate Orca over the user's work.
const blockFileChooser = options.reportEffects === true && options.skipFileChooserBlock !== true
const expectedUrl = getRequestedPageUrl()
// A dialog blocks the page until it is answered. A command still waiting never runs; one
@@ -147,14 +163,22 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeActionEf
}
running = true
// The effects baseline can take seconds, so the check sits inside it, next to the action.
const wc = blockFileChooser ? this.getWebContents(target.webContentsId) : null
// Only the input itself, so the user's own pickers work during the effects wait.
const deliver = (): Promise<T> =>
wc
? withFileChooserBlocked(wc, () => execute(session, target), {
agent: agentCommand.agent
})
: execute(session, target)
const guarded = (): Promise<T> =>
expectedUrl === undefined
? execute(session, target)
? deliver()
: runExpectingPageUrl(
expectedUrl,
target.browserPageId,
() => this.livePageUrls(target),
() => execute(session, target)
deliver
)
const act = (): Promise<T> =>
reportEffects
@@ -8,6 +8,8 @@ export type EnqueueTargetedCommandOptions = {
ensureSession?: boolean
/** Page actions report what they caused when an agent runs them. */
reportEffects?: boolean
// Upload catches the picker itself; actions without user activation cannot open one.
skipFileChooserBlock?: boolean
// Text mutations must not fall back to a global tab from another worktree.
requireScopedTarget?: boolean
// Element arguments that may be snapshot refs.
@@ -88,7 +88,7 @@ export abstract class AgentBrowserBridgeCoreCommands extends AgentBrowserBridgeQ
browserPageId?: string,
options: { reportEffects?: boolean } = {}
): Promise<BrowserGotoResult> {
return this.enqueuePageAction(
return this.enqueueInertAction(
worktreeId,
browserPageId,
async (_sessionName, target) => {
@@ -42,7 +42,7 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge
worktreeId?: string,
browserPageId?: string
): Promise<BrowserScrollResult> {
return this.enqueuePageAction(worktreeId, browserPageId, async (sessionName) => {
return this.enqueueInertAction(worktreeId, browserPageId, async (sessionName) => {
const args = ['scroll', direction]
if (amount != null) {
args.push(String(amount))
@@ -56,7 +56,7 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge
worktreeId?: string,
browserPageId?: string
): Promise<unknown> {
return this.enqueuePageAction(
return this.enqueueInertAction(
worktreeId,
browserPageId,
async (sessionName) => {
@@ -35,7 +35,7 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
worktreeId?: string,
browserPageId?: string
): Promise<BrowserHoverResult> {
return this.enqueuePageAction(
return this.enqueueInertAction(
worktreeId,
browserPageId,
async (sessionName) => {
@@ -115,7 +115,7 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser
await chooser.release()
}
},
{ elements: [element] }
{ elements: [element], skipFileChooserBlock: true }
)
}
@@ -214,20 +214,20 @@ export abstract class AgentBrowserBridgeUtilityCommands extends AgentBrowserBrid
}
async back(worktreeId?: string, browserPageId?: string): Promise<BrowserBackResult> {
return this.enqueuePageAction(worktreeId, browserPageId, async (sessionName) => {
return this.enqueueInertAction(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['back'])) as BrowserBackResult
})
}
async forward(worktreeId?: string, browserPageId?: string): Promise<BrowserBackResult> {
return this.enqueuePageAction(worktreeId, browserPageId, async (sessionName) => {
return this.enqueueInertAction(worktreeId, browserPageId, async (sessionName) => {
return (await this.execAgentBrowser(sessionName, ['forward'])) as BrowserBackResult
})
}
async reload(worktreeId?: string, browserPageId?: string): Promise<BrowserReloadResult> {
// Reload directly because a process swap can destroy the helper session mid-command.
return this.enqueuePageAction(worktreeId, browserPageId, async (_sessionName, target) => {
return this.enqueueInertAction(worktreeId, browserPageId, async (_sessionName, target) => {
const wc = this.getWebContents(target.webContentsId)
if (!wc) {
throw new BrowserError('browser_no_tab', 'Tab is no longer available')
@@ -111,7 +111,7 @@ export abstract class AgentBrowserBridgePointerCommands extends AgentBrowserBrid
worktreeId?: string,
browserPageId?: string
): Promise<unknown> {
return this.enqueuePageAction(
return this.enqueueInertAction(
worktreeId,
browserPageId,
async (sessionName, target) =>
@@ -191,7 +191,7 @@ export abstract class AgentBrowserBridgePointerCommands extends AgentBrowserBrid
worktreeId?: string,
browserPageId?: string
): Promise<unknown> {
return this.enqueuePageAction(
return this.enqueueInertAction(
worktreeId,
browserPageId,
async (sessionName, target) =>
@@ -1,15 +1,23 @@
import type { WebContents } from 'electron'
import { withTimeout } from '../../../../shared/promise-timeout-fallback'
import { BrowserError } from '../../browser-error'
import { acquireElectronDebugger } from '../debugger/electron-debugger-lease'
import { sendGuestCdpCommand } from '../debugger/guest-cdp-command'
import { withTimeout as rejectAfterTimeout } from '../commands/cdp-screenshot'
import { runOutsideRemoteCliBrowserScope } from '../../../runtime/desktop-browser-bridge/remote-cli-browser-scope'
// A click's user activation lasts 5s in Chromium, so its picker cannot open any later.
const FILE_CHOOSER_OPEN_TIMEOUT_MS = 5_000
// A hung page must not wedge its command queue on setup or cleanup.
const PAGE_REPLY_TIMEOUT_MS = 2_000
export type OpenedFileChooser = { backendNodeId: number | null; sessionId: string | undefined }
export type FileChooserInterception = {
/** The next chooser the page opened, or null when none opened in time. */
next(): Promise<OpenedFileChooser | null>
/** Whether a chooser has opened since interception began. */
readonly opened: boolean
release(): Promise<void>
}
@@ -19,10 +27,14 @@ export type FileChooserInterception = {
*/
export async function interceptFileChooser(
wc: WebContents,
frameSessionId: string | undefined
frameSessionId: string | undefined,
// Cancel tells the page its picker closed; upload keeps it pending to set files.
{ cancel = false }: { cancel?: boolean } = {}
): Promise<FileChooserInterception> {
const sessionIds = [...new Set([undefined, frameSessionId])]
const { promise: opened, resolve: resolveOpened } = Promise.withResolvers<OpenedFileChooser>()
let openedAny = false
let abandoned = false
const onMessage = (
_event: unknown,
method: string,
@@ -31,44 +43,97 @@ export async function interceptFileChooser(
): void => {
const sessionId = eventSessionId || undefined
if (method === 'Page.fileChooserOpened' && sessionIds.includes(sessionId)) {
openedAny = true
resolveOpened({ backendNodeId: params?.backendNodeId ?? null, sessionId })
}
}
wc.debugger.on('message', onMessage)
// The getter throws once the guest is destroyed, e.g. after a click that closes its popup.
const dbg = wc.debugger
dbg.on('message', onMessage)
const release = async (): Promise<void> => {
wc.debugger.removeListener('message', onMessage)
await Promise.all(
sessionIds.map((sessionId) =>
sendGuestCdpCommand(
wc,
'Page.setInterceptFileChooserDialog',
{ enabled: false },
sessionId
).catch(() => {})
dbg.removeListener('message', onMessage)
if (wc.isDestroyed()) {
return
}
// Desktop-owned cleanup: a revoked host scope must not leave the user's pickers intercepted.
await runOutsideRemoteCliBrowserScope(() =>
Promise.all(
sessionIds.map((sessionId) =>
// Best-effort: a page that never answers keeps its queue moving.
withTimeout(
sendGuestCdpCommand(
wc,
'Page.setInterceptFileChooserDialog',
{ enabled: false },
sessionId
),
PAGE_REPLY_TIMEOUT_MS,
undefined
)
)
)
)
}
try {
await Promise.all(
sessionIds.map(async (sessionId) => {
// Cross-site frame sessions report choosers only with Page on; the page session has it.
if (sessionId !== undefined) {
// Both wait on the renderer. Release follows any sent enable in order; abandon stops later ones.
await rejectAfterTimeout(
Promise.all(
sessionIds.map(async (sessionId) => {
// Chromium reports choosers only with Page on; a page with no agent session may not have it.
await sendGuestCdpCommand(wc, 'Page.enable', {}, sessionId)
}
await sendGuestCdpCommand(
wc,
'Page.setInterceptFileChooserDialog',
{ enabled: true },
sessionId
)
})
if (abandoned) {
return
}
await sendGuestCdpCommand(
wc,
'Page.setInterceptFileChooserDialog',
cancel ? { enabled: true, cancel: true } : { enabled: true },
sessionId
)
})
),
PAGE_REPLY_TIMEOUT_MS,
'The page is not responding, so Orca did not send the input.'
)
} catch (error) {
abandoned = true
await release()
throw error
}
return {
next: () => withTimeout<OpenedFileChooser | null>(opened, FILE_CHOOSER_OPEN_TIMEOUT_MS, null),
get opened() {
return openedAny
},
release
}
}
/** Agent or remote input must not raise the OS file dialog, which activates Orca over the user's work. */
export async function withFileChooserBlocked<T>(
wc: WebContents,
run: () => Promise<T>,
{ agent }: { agent: boolean }
): Promise<T> {
const debuggerLease = acquireElectronDebugger(wc)
try {
const chooser = await interceptFileChooser(wc, undefined, { cancel: true })
let result: T
try {
result = await run()
} finally {
await chooser.release()
}
if (chooser.opened) {
throw new BrowserError(
'browser_file_chooser_opened',
agent
? 'The action opened a file picker, which Orca closed so the system dialog would not take over the screen. To choose files, run `upload --element <ref> --files <paths>` on the file input or the button that opens it.'
: "This page's file picker can't open from a remote view."
)
}
return result
} finally {
debuggerLease.release()
}
}