From ada69cba8cf09dbc786f133cf6aa00fdd25734be Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Thu, 12 Nov 2026 00:18:39 +0000 Subject: [PATCH] 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 --- skill-guides/orca-cli/references/browser.md | 2 +- src/cli/bundled-skill-guides.ts | 2 +- .../agent-browser-bridge-file-chooser.test.ts | 125 ++++++++++++++++++ .../agent-browser-bridge-queue.ts | 28 +++- .../agent-browser-queued-command.ts | 2 + .../agent-browser-bridge-core-commands.ts | 2 +- .../agent-browser-bridge-input-commands.ts | 4 +- ...ent-browser-bridge-interaction-commands.ts | 4 +- .../agent-browser-bridge-page-commands.ts | 6 +- .../agent-browser-bridge-pointer-commands.ts | 4 +- .../browser/cdp/input/guest-file-chooser.ts | 111 ++++++++++++---- 11 files changed, 253 insertions(+), 37 deletions(-) create mode 100644 src/main/browser/agent-browser/command-queue/agent-browser-bridge-file-chooser.test.ts diff --git a/skill-guides/orca-cli/references/browser.md b/skill-guides/orca-cli/references/browser.md index 5dafc6b09fe..e62030329bb 100644 --- a/skill-guides/orca-cli/references/browser.md +++ b/skill-guides/orca-cli/references/browser.md @@ -64,7 +64,7 @@ ORCA exec --page --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 ` 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. diff --git a/src/cli/bundled-skill-guides.ts b/src/cli/bundled-skill-guides.ts index 83656f6f6eb..d8f59db7cdf 100644 --- a/src/cli/bundled-skill-guides.ts +++ b/src/cli/bundled-skill-guides.ts @@ -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 ` 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.", diff --git a/src/main/browser/agent-browser/command-queue/agent-browser-bridge-file-chooser.test.ts b/src/main/browser/agent-browser/command-queue/agent-browser-bridge-file-chooser.test.ts new file mode 100644 index 00000000000..2abf12b414c --- /dev/null +++ b/src/main/browser/agent-browser/command-queue/agent-browser-bridge-file-chooser.test.ts @@ -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() + })) + +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) { + 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 => { + 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) + }) +}) diff --git a/src/main/browser/agent-browser/command-queue/agent-browser-bridge-queue.ts b/src/main/browser/agent-browser/command-queue/agent-browser-bridge-queue.ts index 5b4969e02da..acd319bfb4a 100644 --- a/src/main/browser/agent-browser/command-queue/agent-browser-bridge-queue.ts +++ b/src/main/browser/agent-browser/command-queue/agent-browser-bridge-queue.ts @@ -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( + worktreeId: string | undefined, + browserPageId: string | undefined, + execute: (sessionName: string, target: ResolvedBrowserCommandTarget) => Promise, + options: EnqueueTargetedCommandOptions = {} + ): Promise { + return this.enqueuePageAction(worktreeId, browserPageId, execute, { + ...options, + skipFileChooserBlock: true + }) + } + protected async enqueueTargetedCommand( 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 => + wc + ? withFileChooserBlocked(wc, () => execute(session, target), { + agent: agentCommand.agent + }) + : execute(session, target) const guarded = (): Promise => expectedUrl === undefined - ? execute(session, target) + ? deliver() : runExpectingPageUrl( expectedUrl, target.browserPageId, () => this.livePageUrls(target), - () => execute(session, target) + deliver ) const act = (): Promise => reportEffects diff --git a/src/main/browser/agent-browser/command-queue/agent-browser-queued-command.ts b/src/main/browser/agent-browser/command-queue/agent-browser-queued-command.ts index 0a8ba7450f0..6d1ca18a4b7 100644 --- a/src/main/browser/agent-browser/command-queue/agent-browser-queued-command.ts +++ b/src/main/browser/agent-browser/command-queue/agent-browser-queued-command.ts @@ -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. diff --git a/src/main/browser/agent-browser/commands/agent-browser-bridge-core-commands.ts b/src/main/browser/agent-browser/commands/agent-browser-bridge-core-commands.ts index 20c148e2e32..8a7976f7b08 100644 --- a/src/main/browser/agent-browser/commands/agent-browser-bridge-core-commands.ts +++ b/src/main/browser/agent-browser/commands/agent-browser-bridge-core-commands.ts @@ -88,7 +88,7 @@ export abstract class AgentBrowserBridgeCoreCommands extends AgentBrowserBridgeQ browserPageId?: string, options: { reportEffects?: boolean } = {} ): Promise { - return this.enqueuePageAction( + return this.enqueueInertAction( worktreeId, browserPageId, async (_sessionName, target) => { diff --git a/src/main/browser/agent-browser/commands/agent-browser-bridge-input-commands.ts b/src/main/browser/agent-browser/commands/agent-browser-bridge-input-commands.ts index 6e48368c0f5..f275ccc1440 100644 --- a/src/main/browser/agent-browser/commands/agent-browser-bridge-input-commands.ts +++ b/src/main/browser/agent-browser/commands/agent-browser-bridge-input-commands.ts @@ -42,7 +42,7 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge worktreeId?: string, browserPageId?: string ): Promise { - 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 { - return this.enqueuePageAction( + return this.enqueueInertAction( worktreeId, browserPageId, async (sessionName) => { diff --git a/src/main/browser/agent-browser/commands/agent-browser-bridge-interaction-commands.ts b/src/main/browser/agent-browser/commands/agent-browser-bridge-interaction-commands.ts index c9314b50f63..6dd604ab103 100644 --- a/src/main/browser/agent-browser/commands/agent-browser-bridge-interaction-commands.ts +++ b/src/main/browser/agent-browser/commands/agent-browser-bridge-interaction-commands.ts @@ -35,7 +35,7 @@ export abstract class AgentBrowserBridgeInteractionCommands extends AgentBrowser worktreeId?: string, browserPageId?: string ): Promise { - 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 } ) } diff --git a/src/main/browser/agent-browser/commands/agent-browser-bridge-page-commands.ts b/src/main/browser/agent-browser/commands/agent-browser-bridge-page-commands.ts index 9b3cf635d60..788a5406d7f 100644 --- a/src/main/browser/agent-browser/commands/agent-browser-bridge-page-commands.ts +++ b/src/main/browser/agent-browser/commands/agent-browser-bridge-page-commands.ts @@ -214,20 +214,20 @@ export abstract class AgentBrowserBridgeUtilityCommands extends AgentBrowserBrid } async back(worktreeId?: string, browserPageId?: string): Promise { - 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 { - 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 { // 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') diff --git a/src/main/browser/agent-browser/commands/agent-browser-bridge-pointer-commands.ts b/src/main/browser/agent-browser/commands/agent-browser-bridge-pointer-commands.ts index 1650b6f8baa..eb5ec4e5d36 100644 --- a/src/main/browser/agent-browser/commands/agent-browser-bridge-pointer-commands.ts +++ b/src/main/browser/agent-browser/commands/agent-browser-bridge-pointer-commands.ts @@ -111,7 +111,7 @@ export abstract class AgentBrowserBridgePointerCommands extends AgentBrowserBrid worktreeId?: string, browserPageId?: string ): Promise { - 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 { - return this.enqueuePageAction( + return this.enqueueInertAction( worktreeId, browserPageId, async (sessionName, target) => diff --git a/src/main/browser/cdp/input/guest-file-chooser.ts b/src/main/browser/cdp/input/guest-file-chooser.ts index fbd17bbdbe2..106621de4bd 100644 --- a/src/main/browser/cdp/input/guest-file-chooser.ts +++ b/src/main/browser/cdp/input/guest-file-chooser.ts @@ -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 + /** Whether a chooser has opened since interception began. */ + readonly opened: boolean release(): Promise } @@ -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 { const sessionIds = [...new Set([undefined, frameSessionId])] const { promise: opened, resolve: resolveOpened } = Promise.withResolvers() + 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 => { - 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(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( + wc: WebContents, + run: () => Promise, + { agent }: { agent: boolean } +): Promise { + 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 --files ` 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() + } +}