From b67c5254f04489d11167efa0b9788be2b50bb3e1 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Fri, 30 Oct 2026 06:48:58 +0000 Subject: [PATCH] fix(browser): agent fill, select, upload and type change the page or fail with a reason (#36263) * fix(browser): fill, select, upload and type act on the agent's element or fail * refactor(browser): move agent key commands into their own bridge layer; fix test sender call --- skill-guides/orca-cli/references/browser.md | 4 +- src/cli/bundled-skill-guides.ts | 4 +- .../agent-browser-bridge-core-commands.ts | 18 +- .../browser/agent-browser-bridge-execution.ts | 91 +++- ...ent-browser-bridge-host-revocation.test.ts | 36 ++ .../agent-browser-bridge-input-commands.ts | 36 +- .../browser/agent-browser-bridge-input.ts | 67 --- ...ent-browser-bridge-interaction-commands.ts | 127 ++--- .../agent-browser-bridge-key-commands.ts | 138 ++++++ ...gent-browser-bridge-keypress-input.test.ts | 32 ++ .../agent-browser-bridge-text-input.test.ts | 436 ++++-------------- .../browser/browser-element-functions.test.ts | 155 +++++++ src/main/browser/browser-element-functions.ts | 81 ++++ src/main/browser/browser-upload-files.ts | 16 + src/main/browser/cdp-element-capture.ts | 29 ++ src/main/browser/cdp-keyboard-us-layout.ts | 41 ++ src/main/browser/cdp-ws-proxy.ts | 3 + .../browser/guest-direct-key-input.test.ts | 43 ++ src/main/browser/guest-direct-key-input.ts | 111 +++-- .../external-chromium-browser-process.ts | 5 + .../helpers/browser-agent-focus-scenarios.ts | 6 +- 21 files changed, 857 insertions(+), 622 deletions(-) delete mode 100644 src/main/browser/agent-browser-bridge-input.ts create mode 100644 src/main/browser/agent-browser-bridge-key-commands.ts create mode 100644 src/main/browser/browser-element-functions.test.ts create mode 100644 src/main/browser/browser-element-functions.ts create mode 100644 src/main/browser/browser-upload-files.ts create mode 100644 src/main/browser/cdp-element-capture.ts diff --git a/skill-guides/orca-cli/references/browser.md b/skill-guides/orca-cli/references/browser.md index a8435f8a8ce..cc84303d277 100644 --- a/skill-guides/orca-cli/references/browser.md +++ b/skill-guides/orca-cli/references/browser.md @@ -60,12 +60,14 @@ Browser rules: - Use `ORCA set device` or `ORCA viewport --mobile`, not `ORCA exec --command "set device ..."`, so the page also gets touch input and `pointer: coarse`. Each page load (reload, `goto`, link click) resets it, so run it again after navigating. - Prefer `wait --text`, `--url`, `--selector`, or `--load` after async page changes instead of bare timeouts. - Anything not listed above goes through `ORCA exec --page --command ""`. -- If `fill` or `type` fails on a custom input, try `ORCA focus --page --element @e1 --json` then `ORCA inserttext --page --text "text" --json`. +- `fill` sets a field's whole value in one step, at any size. `type` presses real keys into the focused element, so `minlength`/`maxlength`, key handlers, Enter and Tab act as they would for a person; `ORCA focus` the element first. Use `type` when the page reacts to keystrokes. +- On server-hosted pages `fill` and `type` still insert text the engine's own way. If either does nothing on a custom input there, try `ORCA focus --page --element @e1 --json` then `ORCA inserttext --page --text "text" --json`. - A client-hosted page renders in the paired desktop's browser engine, so every command against it needs that desktop online and returns `browser_host_unavailable` while it is closed, asleep, or disconnected. Server-hosted pages run with no desktop attached; prefer them for long or unattended automation. Common recoveries: - `browser_no_tab`: open a tab with `ORCA tab create --url --json`. - `browser_stale_ref`: run `ORCA snapshot --page --json` and retry on the same page with fresh refs. +- `browser_element_not_interactable`: the element can't take that command, and the message says why (not a text field, disabled, read-only, not a ``, not a file input). Re-snapshot and pick the right ref; for a custom dropdown, click it and then click the option.", "- `browser_tab_not_found`: run `ORCA tab list --json` before switching or closing.", "- `browser_host_unavailable`: the desktop hosting the page is offline. Bring it back, or recreate the page with server placement if the work must outlive the desktop session.", "", diff --git a/src/main/browser/agent-browser-bridge-core-commands.ts b/src/main/browser/agent-browser-bridge-core-commands.ts index 53f3185c1b4..a2a1ce407bb 100644 --- a/src/main/browser/agent-browser-bridge-core-commands.ts +++ b/src/main/browser/agent-browser-bridge-core-commands.ts @@ -8,7 +8,7 @@ import { assertClipboardTextWriteWithinLimitWithYield } from '../../shared/clipb import { normalizeBrowserNavigationUrl } from '../../shared/browser-url' import { BrowserError } from './cdp-bridge' import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' -import { focusedValueSetExpression } from './agent-browser-bridge-input' +import { FILL_ELEMENT_FUNCTION } from './browser-element-functions' import { EMBEDDED_NAVIGATION_TIMEOUT_MS } from './agent-browser-bridge-types' import { isAbortedNavigationError, @@ -159,22 +159,14 @@ export abstract class AgentBrowserBridgeCoreCommands extends AgentBrowserBridgeQ browserPageId?: string ): Promise { await assertClipboardTextWriteWithinLimitWithYield(value) - // agent-browser's CDP insertion loses focus in Electron guests; use the browser input pipeline. + // agent-browser's fill types into whatever holds focus and sends text per character; set the + // resolved element's value in one call instead. return this.enqueueTargetedCommand( worktreeId, browserPageId, async (sessionName) => { - if (!(await this.isExplicitContentEditableTarget(sessionName, element))) { - await this.execAgentBrowser(sessionName, ['focus', element]) - // One stdin edit avoids argv limits and repeated copying of the growing field value. - await this.execAgentBrowser(sessionName, ['eval', '--stdin'], { - stdinText: focusedValueSetExpression(JSON.stringify(value), { dispatchEvents: true }) - }) - return { filled: element } as BrowserFillResult - } - - await this.fillExplicitContentEditable(sessionName, element, value) - return { filled: element } as BrowserFillResult + await this.actOnElement(sessionName, element, 'fill', FILL_ELEMENT_FUNCTION, [value]) + return { filled: element } }, { requireScopedTarget: true, elements: [element] } ) diff --git a/src/main/browser/agent-browser-bridge-execution.ts b/src/main/browser/agent-browser-bridge-execution.ts index 7faee8ef652..c77d32425ed 100644 --- a/src/main/browser/agent-browser-bridge-execution.ts +++ b/src/main/browser/agent-browser-bridge-execution.ts @@ -1,10 +1,9 @@ import { execFile } from 'node:child_process' import type { WebContents } from 'electron' import { BrowserError } from './cdp-bridge' -import { - focusedRichTextEditExpression, - isExplicitContentEditableResult -} from './agent-browser-bridge-input' +import type { CapturedElement } from './cdp-element-capture' +import { sendGuestCdpCommand } from './guest-cdp-command' +import { readRecord, readString } from './guest-frame-input-sessions' import { isTabClosedTransportError, pageUnavailableMessageForSession @@ -170,29 +169,73 @@ export abstract class AgentBrowserBridgeExecution extends AgentBrowserBridgeTabs return this.runAgentBrowserRaw(sessionName, args, execOptions) } - protected async isExplicitContentEditableTarget( - sessionName: string, - element: string - ): Promise { - const result = await this.execAgentBrowser(sessionName, [ - 'get', - 'attr', - element, - 'contenteditable' - ]) - return isExplicitContentEditableResult(result) - } - - protected async fillExplicitContentEditable( + /** Runs a browser-element-functions action on the element the helper resolves, in its own frame. */ + protected async actOnElement( sessionName: string, element: string, - value: string - ): Promise { - await this.execAgentBrowser(sessionName, ['focus', element]) - // Send one stdin edit to avoid argv limits and focus changes between chunks. - await this.execAgentBrowser(sessionName, ['eval', '--stdin'], { - stdinText: focusedRichTextEditExpression(JSON.stringify(value), { selectAll: true }) + action: string, + functionDeclaration: string, + args: unknown[] = [] + ): Promise { + const session = this.sessions.get(sessionName) + if (!session) { + throw this.createPageUnavailableError(sessionName) + } + session.proxy.elementCapture.start() + session.proxy.pointAtNextNode(true) + let captured: CapturedElement | null + try { + // Any element read makes the helper resolve the ref or selector, in whichever frame owns it. + await this.execAgentBrowser(sessionName, ['get', 'attr', element, 'id']) + } finally { + session.proxy.pointAtNextNode(false) + captured = session.proxy.elementCapture.stop() + } + const wc = this.getWebContents(session.webContentsId) + if (!wc) { + throw this.createPageUnavailableError(sessionName) + } + if (!captured) { + throw new BrowserError('browser_error', `Could not resolve ${element} in the page`) + } + const response = await sendGuestCdpCommand( + wc, + 'Runtime.callFunctionOn', + { + objectId: captured.objectId, + functionDeclaration, + arguments: args.map((value) => ({ value })), + returnByValue: true + }, + captured.sessionId + ).catch((error: unknown) => { + if (error instanceof BrowserError) { + throw error + } + if (!this.getWebContents(session.webContentsId)) { + throw this.createPageUnavailableError(sessionName) + } + throw new BrowserError( + 'browser_cdp_error', + error instanceof Error ? error.message : String(error) + ) }) + const exception = readRecord(response, 'exceptionDetails') + if (exception) { + const description = readString(readRecord(exception, 'exception'), 'description') + throw new BrowserError( + 'browser_eval_error', + description ?? readString(exception, 'text') ?? 'Page function failed' + ) + } + const outcome = readRecord(response, 'result')?.value + if (typeof outcome === 'string') { + throw new BrowserError( + 'browser_element_not_interactable', + `Cannot ${action} ${element}: ${outcome}` + ) + } + return outcome } protected createPageUnavailableError(sessionName: string): BrowserError { diff --git a/src/main/browser/agent-browser-bridge-host-revocation.test.ts b/src/main/browser/agent-browser-bridge-host-revocation.test.ts index d889b1a8fe0..c96a976a8d0 100644 --- a/src/main/browser/agent-browser-bridge-host-revocation.test.ts +++ b/src/main/browser/agent-browser-bridge-host-revocation.test.ts @@ -85,6 +85,42 @@ describe('AgentBrowserBridge under a host revocation', () => { bridge.setActiveTab(100) }) + it('stops a long type at the next run once the host is revoked', async () => { + const wc = mockWebContents(100) + wc.getType.mockReturnValue('webview') + let resumeTyping = (): void => {} + const paused = new Promise((resolve) => { + resumeTyping = resolve + }) + let evaluations = 0 + wc.debugger.sendCommand.mockImplementation(async (method: string) => { + if (method === 'Runtime.evaluate') { + evaluations += 1 + // The first run's settle wait: hold it while the host is revoked. + if (evaluations === 2) { + await paused + } + return { result: { type: 'object', subtype: 'null' } } + } + return {} + }) + webContentsFromIdMock.mockReturnValue(wc) + const revoked = new AbortController() + + const typing = runInRemoteCliBrowserScope( + WORKSPACE, + () => bridge.type('x'.repeat(2000), undefined, 'tab-1'), + revoked.signal + ) + await vi.waitFor(() => expect(evaluations).toBe(2)) + const sentBeforeRevocation = wc.sendInputEvent.mock.calls.length + revoked.abort() + resumeTyping() + + await expect(typing).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' }) + expect(wc.sendInputEvent.mock.calls.length).toBe(sentBeforeRevocation) + }) + it('never presses the mouse once the host is revoked during a running click', async () => { const wc = mockWebContents(100) let answerHitTest = (): void => {} diff --git a/src/main/browser/agent-browser-bridge-input-commands.ts b/src/main/browser/agent-browser-bridge-input-commands.ts index 242fdbc7b99..c623655cd3a 100644 --- a/src/main/browser/agent-browser-bridge-input-commands.ts +++ b/src/main/browser/agent-browser-bridge-input-commands.ts @@ -1,36 +1,11 @@ -import type { - BrowserTypeResult, - BrowserSelectResult, - BrowserScrollResult -} from '../../shared/runtime-types' +import type { BrowserSelectResult, BrowserScrollResult } from '../../shared/runtime-types' import { assertClipboardTextWriteWithinLimitWithYield } from '../../shared/clipboard-text' import { iterateBrowserTextInsertionChunks } from '../../shared/browser-text-insertion-chunks' import { AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES } from './agent-browser-bridge-types' import { AgentBrowserBridgeCoreCommands } from './agent-browser-bridge-core-commands' +import { SELECT_OPTION_TARGET_FUNCTION } from './browser-element-functions' export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridgeCoreCommands { - async type( - input: string, - worktreeId?: string, - browserPageId?: string - ): Promise { - await assertClipboardTextWriteWithinLimitWithYield(input) - return this.enqueueTargetedCommand( - worktreeId, - browserPageId, - async (sessionName) => { - for (const chunk of iterateBrowserTextInsertionChunks( - input, - AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES - )) { - await this.execAgentBrowser(sessionName, ['keyboard', 'type', chunk]) - } - return { typed: true } as BrowserTypeResult - }, - { requireScopedTarget: true } - ) - } - async select( element: string, value: string, @@ -41,6 +16,13 @@ export abstract class AgentBrowserBridgeInputCommands extends AgentBrowserBridge worktreeId, browserPageId, async (sessionName) => { + // The helper reports success for any element; only a before the helper reports success', async () => { + succeedWith({ selected: ['Mango'] }) + wc.debugger.sendCommand.mockResolvedValue({ + result: { type: 'string', value: 'it is a combobox, not a ', 'input', 'it is disabled'], + ['', 'textarea', 'it is read-only'], + ['', 'input', 'it is not a text field or editable element'], + ['', 'select', 'it is a ').querySelector('input')! + const events: string[] = [] + input.addEventListener('input', () => events.push('input')) + + expect(callOn(input, FILL_ELEMENT_FUNCTION, 'abc')).toBe('the field rejected this value') + + expect(input.value).toBe('42') + expect(events).toEqual([]) + }) + + it('fills the input an aria-controlled spinbutton points at', () => { + const body = mount('
') + + callOn(body.querySelector('[role=spinbutton]')!, FILL_ELEMENT_FUNCTION, '200') + + expect(body.querySelector('#qty')!.value).toBe('200') + }) + + it('fills the text descendant of a spinbutton wrapper, skipping non-text inputs', () => { + const body = mount( + '
' + ) + + callOn(body.querySelector('[role=spinbutton]')!, FILL_ELEMENT_FUNCTION, '7') + + expect(body.querySelector('#n')!.value).toBe('7') + expect(body.querySelector('#h')!.value).toBe('') + }) + + it.each([ + ['replaces', 'new text', 'insertText'], + ['clears', '', 'delete'] + ])('%s contenteditable text through a browser edit command', (_name, value, command) => { + const editor = mount('
old
').querySelector('div')! + const execCommand = vi.fn(() => true) + Object.defineProperty(document, 'execCommand', { configurable: true, value: execCommand }) + + expect(callOn(editor, FILL_ELEMENT_FUNCTION, value)).toBeNull() + + expect(execCommand).toHaveBeenCalledWith(command, false, value) + expect(window.getSelection()?.toString()).toBe('old') + }) + + it('refuses a rich-text edit when a focus handler moves focus to another field', () => { + const body = mount('
old
') + const execCommand = vi.fn(() => true) + Object.defineProperty(document, 'execCommand', { configurable: true, value: execCommand }) + body.querySelector('div')!.addEventListener('focus', () => body.querySelector('input')!.focus()) + + expect(callOn(body.querySelector('div')!, FILL_ELEMENT_FUNCTION, 'new')).toBe( + 'it gave its focus to another element' + ) + expect(execCommand).not.toHaveBeenCalled() + }) + + it('refuses an input the page replaces when it takes focus', () => { + const input = mount('').querySelector('input')! + input.addEventListener('focus', () => input.replaceWith(document.createElement('input'))) + + expect(callOn(input, FILL_ELEMENT_FUNCTION, 'new')).toMatch(/replaced it/) + }) + + it('reports a rich-text edit the browser refused', () => { + const editor = mount('
old
').querySelector('div')! + Object.defineProperty(document, 'execCommand', { configurable: true, value: () => false }) + + expect(callOn(editor, FILL_ELEMENT_FUNCTION, 'new')).toBe('the browser editing command failed') + }) +}) + +describe('element target checks', () => { + it.each([ + ['
Pick
', 'div', 'it is a combobox, not a '], + ['', 'select', 'it is disabled'] + ])('refuses select on %s', (html, selector, reason) => { + const element = mount(html).querySelector(selector)! + + expect(callOn(element, SELECT_OPTION_TARGET_FUNCTION)).toMatch(reason) + }) + + it('accepts select on an enabled ') + + expect(callOn(body.querySelector('select')!, SELECT_OPTION_TARGET_FUNCTION)).toBeNull() + expect(callOn(body.querySelector('input')!, FILE_INPUT_TARGET_FUNCTION)).toBeNull() + }) + + it('refuses upload on an element that is not a file input', () => { + const button = mount('').querySelector('button')! + + expect(callOn(button, FILE_INPUT_TARGET_FUNCTION)).toBe('it is not a file input') + }) + + it('selects all of an input value and reports that it holds focus', () => { + const input = mount('').querySelector('input')! + input.setSelectionRange(3, 3) + + expect(callOn(input, SELECT_ALL_ELEMENT_FUNCTION)).toBe(true) + + expect([input.selectionStart, input.selectionEnd]).toEqual([0, 6]) + }) +}) diff --git a/src/main/browser/browser-element-functions.ts b/src/main/browser/browser-element-functions.ts new file mode 100644 index 00000000000..48efd8467b4 --- /dev/null +++ b/src/main/browser/browser-element-functions.ts @@ -0,0 +1,81 @@ +// Page functions for Runtime.callFunctionOn; `this` is the element the agent's ref resolved to. +// A returned string is the reason the element cannot take the action. + +export const FILL_ELEMENT_FUNCTION = `function (value) { + const isTextField = (node) => + node?.localName === 'textarea' || + (node?.localName === 'input' && !/^(hidden|button|checkbox|radio|file|image|reset|submit)$/i.test(node.type)); + const findTextField = (root) => Array.from(root?.querySelectorAll?.('input, textarea') ?? []).find(isTextField) ?? null; + let target = this; + // Spinbutton wrappers can take the ref while another input owns the value. + if (!isTextField(target) && target.getAttribute?.('role') === 'spinbutton') { + const root = target.getRootNode(); + for (const id of (target.getAttribute('aria-controls') ?? '').split(/\\s+/).filter(Boolean)) { + const controlled = root.getElementById?.(id) ?? null; + const field = isTextField(controlled) ? controlled : findTextField(controlled); + if (field) { target = field; break; } + } + if (target === this) target = findTextField(this) ?? this; + } + if (target.matches?.(':disabled')) return 'it is disabled'; + if (target.isContentEditable && !isTextField(target)) { + // Rich editors revert direct DOM writes; browser edit transactions keep their models in sync. + target.focus(); + // The edit lands wherever focus is, so a focus handler that moved it must stop the write. + const active = target.getRootNode().activeElement; + if (!active || !(active === target || active.contains(target) || target.contains(active))) { + return 'it gave its focus to another element'; + } + const selection = target.ownerDocument.defaultView?.getSelection?.(); + if (!selection) return 'its text selection is unavailable'; + selection.selectAllChildren(target); + let edited = false; + try { + edited = target.ownerDocument.execCommand(value.length === 0 ? 'delete' : 'insertText', false, value) === true; + } catch {} + return edited ? null : 'the browser editing command failed'; + } + if (target.localName === 'select') return 'it is a ; click it and choose the option instead'; + } + return this.matches(':disabled') ? 'it is disabled' : null; +}` + +export const FILE_INPUT_TARGET_FUNCTION = `function () { + if (this.localName !== 'input' || this.type !== 'file') return 'it is not a file input'; + return this.matches(':disabled') ? 'it is disabled' : null; +}` diff --git a/src/main/browser/browser-upload-files.ts b/src/main/browser/browser-upload-files.ts new file mode 100644 index 00000000000..d5e68250230 --- /dev/null +++ b/src/main/browser/browser-upload-files.ts @@ -0,0 +1,16 @@ +import { stat } from 'node:fs/promises' +import { BrowserError } from './browser-error' + +/** Chromium attaches a missing path as an empty file, so check the paths before handing them over. */ +export async function assertUploadFilesExist(filePaths: readonly string[]): Promise { + for (const filePath of filePaths) { + // Directories stay allowed: webkitdirectory inputs take them. + const exists = await stat(filePath).then( + () => true, + () => false + ) + if (!exists) { + throw new BrowserError('invalid_argument', `Cannot upload ${filePath}: it does not exist`) + } + } +} diff --git a/src/main/browser/cdp-element-capture.ts b/src/main/browser/cdp-element-capture.ts new file mode 100644 index 00000000000..760ea1eaf89 --- /dev/null +++ b/src/main/browser/cdp-element-capture.ts @@ -0,0 +1,29 @@ +// A helper-resolved remote object; its session is the debugger's, undefined for the page itself. +export type CapturedElement = { objectId: string; sessionId: string | undefined } + +/** Records the element the helper's calls run on, so Orca can act on that exact node. */ +export class CdpElementCapture { + // Undefined while not capturing. + private captured: CapturedElement | null | undefined + + start(): void { + this.captured = null + } + + stop(): CapturedElement | null { + const captured = this.captured ?? null + this.captured = undefined + return captured + } + + observe(method: string, params: Record | undefined, sessionId?: string): void { + const objectId = params?.objectId + if ( + this.captured !== undefined && + method === 'Runtime.callFunctionOn' && + typeof objectId === 'string' + ) { + this.captured = { objectId, sessionId } + } + } +} diff --git a/src/main/browser/cdp-keyboard-us-layout.ts b/src/main/browser/cdp-keyboard-us-layout.ts index 0bbab9946f7..5210c075525 100644 --- a/src/main/browser/cdp-keyboard-us-layout.ts +++ b/src/main/browser/cdp-keyboard-us-layout.ts @@ -237,3 +237,44 @@ export function parseCdpKeyEvent(raw: string): CdpKeyEvent | null { return { keyCode, key, code, modifiers, location, selfModifier, text } } + +/** The key a person presses to type one character, or null when no key types it. */ +function typedCharacterKeyEvent(char: string): CdpKeyEvent | null { + if (char === '\n') { + return parseCdpKeyEvent('Enter') + } + if (char === '\t') { + return parseCdpKeyEvent('Tab') + } + return parseCdpKeyEvent(char) ?? imeFallbackKeyEvent(char) +} + +const MAX_UNTYPABLE_RUN_LENGTH = 512 + +/** A key to press, or a run of text that no key types. */ +export type TypedInputStep = CdpKeyEvent | string + +/** The keys that type `input`, yielded lazily so long text is never held as events. */ +export function* typedInputSteps(input: string): Generator { + let untypable = '' + for (const char of input.replace(/\r\n?/g, '\n')) { + const key = typedCharacterKeyEvent(char) + if (!key) { + untypable += char + // Bounded like key runs, since a widget page receives this text one event per character. + if (untypable.length >= MAX_UNTYPABLE_RUN_LENGTH) { + yield untypable + untypable = '' + } + continue + } + if (untypable) { + yield untypable + untypable = '' + } + yield key + } + if (untypable) { + yield untypable + } +} diff --git a/src/main/browser/cdp-ws-proxy.ts b/src/main/browser/cdp-ws-proxy.ts index 0d3934d09e0..b5d84a5ee2c 100644 --- a/src/main/browser/cdp-ws-proxy.ts +++ b/src/main/browser/cdp-ws-proxy.ts @@ -7,6 +7,7 @@ import { CdpTargetDiscovery } from './cdp-target-discovery' import { CdpDebuggerChannel } from './cdp-debugger-channel' import { CdpPageNavigationCommands } from './cdp-page-navigation-commands' import { CdpDomFocusReplay } from './cdp-dom-focus-replay' +import { CdpElementCapture } from './cdp-element-capture' import { CdpPageCaptureCommands } from './cdp-page-capture-commands' import type { CapturePaintHold } from './cdp-screenshot' import type { AgentInputFocusGuard } from './agent-input-focus-guard' @@ -34,6 +35,7 @@ export class CdpWsProxy { private readonly pageCapture: CdpPageCaptureCommands private readonly input: CdpPageInputCommands private pointAtNextResolvedNode = false + readonly elementCapture = new CdpElementCapture() constructor( private readonly webContents: WebContents, @@ -248,6 +250,7 @@ export class CdpWsProxy { ) return } + this.elementCapture.observe(msg.method, msg.params, effectiveSessionId) if (msg.method === 'DOM.focus') { this.domFocusReplay.forwardDomFocus(client, clientId, msg.params ?? {}, effectiveSessionId) return diff --git a/src/main/browser/guest-direct-key-input.test.ts b/src/main/browser/guest-direct-key-input.test.ts index 8bf5c9f9228..0249a91439e 100644 --- a/src/main/browser/guest-direct-key-input.test.ts +++ b/src/main/browser/guest-direct-key-input.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import type { WebContents } from 'electron' import { dispatchPageKeyEvent, + dispatchPageKeyEvents, routePageTextInsertion, type GuestCdpSender } from './guest-direct-key-input' @@ -225,6 +226,23 @@ describe('dispatchPageKeyEvent', () => { expect(page.selectAll).toHaveBeenCalledTimes(1) }) + it('settles a run of widget keys with one focus probe and one page round trip', async () => { + const { page, webContents } = fakePage('webview') + const send = fakeSender() + const keys = ['a', 'b', 'c'].flatMap((key) => [ + { type: 'keyDown', key, text: key }, + { type: 'keyUp', key } + ]) + + await dispatchPageKeyEvents(webContents, keys, send) + + expect(page.sendInputEvent).toHaveBeenCalledTimes(9) + expect(send.mock.calls.map(([method]) => method)).toEqual([ + 'Runtime.evaluate', + 'Runtime.evaluate' + ]) + }) + it('types into a focused cross-site frame on its own session, without focusing the guest', async () => { const { page, webContents } = fakePage('webview') const send = crossSiteFrameSender({ attachable: true }) @@ -238,6 +256,31 @@ describe('dispatchPageKeyEvent', () => { expect(send).toHaveBeenLastCalledWith('Input.dispatchKeyEvent', params, 'S1') }) + it('follows focus that a frame key handler moves into the parent page mid-run', async () => { + const { page, webContents } = fakePage('webview') + const inFrame = crossSiteFrameSender({ attachable: true }) + let focusInFrame = true + const send = vi.fn(async (method, params, sessionId) => { + if (method === 'Input.dispatchKeyEvent' && params.type === 'keyUp') { + // The frame's handler sends focus to the parent after the first key. + focusInFrame = false + } + return focusInFrame || sessionId + ? inFrame(method, params, sessionId) + : fakeSender()(method, params, sessionId) + }) + const keys = ['a', 'b'].flatMap((key) => [ + { type: 'keyDown', key, text: key }, + { type: 'keyUp', key } + ]) + + await dispatchPageKeyEvents(webContents, keys, send) + + const frameKeys = send.mock.calls.filter(([method]) => method === 'Input.dispatchKeyEvent') + expect(frameKeys.map(([, params]) => params.key)).toEqual(['a', 'a']) + expect(page.sendInputEvent).toHaveBeenCalledWith(expect.objectContaining({ keyCode: 'b' })) + }) + it('borrows focus for CDP when the focused frame cannot be reached, then gives it back', async () => { const { page, webContents } = fakePage('webview') const send = crossSiteFrameSender({ attachable: false }) diff --git a/src/main/browser/guest-direct-key-input.ts b/src/main/browser/guest-direct-key-input.ts index d2f0b34896d..ff1a49aec5c 100644 --- a/src/main/browser/guest-direct-key-input.ts +++ b/src/main/browser/guest-direct-key-input.ts @@ -101,48 +101,88 @@ export async function dispatchPageKeyEvent( send: GuestCdpSender, guardFocus: AgentInputFocusGuard = NO_FOCUS_GUARD ): Promise { + return await dispatchPageKeyEvents(webContents, [params], send, guardFocus) +} + +const isTabDown = (params: Record): boolean => + params.key === 'Tab' && (params.type === 'keyDown' || params.type === 'rawKeyDown') + +/** Deliver a run of key events along one focus route; callers split runs where focus may move. */ +export async function dispatchPageKeyEvents( + webContents: WebContents, + events: readonly Record[], + send: GuestCdpSender, + guardFocus: AgentInputFocusGuard = NO_FOCUS_GUARD +): Promise { + let result: unknown = {} if (!isEmbeddedGuest(webContents)) { - return await send('Input.dispatchKeyEvent', params) + for (const params of events) { + result = await send('Input.dispatchKeyEvent', params) + } + return result } - const route = await resolveFocusedFrameRoute(webContents, send) - if (route.kind === 'session') { + let start = 0 + let route = await resolveFocusedFrameRoute(webContents, send) + // A frame's key handler can move focus to another frame, so each key finds its route again. + while (route.kind === 'session') { + const keyUp = events.findIndex((params, index) => index >= start && params.type === 'keyUp') + const end = keyUp === -1 ? events.length : keyUp + 1 + const key = events.slice(start, end) // Tab from the last frame field can escape into Orca's window. - const isTabDown = - params.key === 'Tab' && (params.type === 'keyDown' || params.type === 'rawKeyDown') - const release = isTabDown ? await guardFocus() : () => {} + const release = key.some(isTabDown) ? await guardFocus() : () => {} try { - return await sendToFrameSession( - webContents, - send, - route.sessionId, - 'Input.dispatchKeyEvent', - params - ) + for (const params of key) { + result = await sendToFrameSession( + webContents, + send, + route.sessionId, + 'Input.dispatchKeyEvent', + params + ) + } } finally { release() } + if (end >= events.length) { + return result + } + start = end + route = await resolveFocusedFrameRoute(webContents, send) } - const plan = planCdpKeyEvent(params) - if (plan && route.kind === 'widget') { + const remaining = events.slice(start) + const plans = remaining.map((params) => planCdpKeyEvent(params)) + if (route.kind === 'widget' && plans.every((plan) => plan !== null)) { // Tab from the last page field can escape into Orca's window. - const release = plan.events.some((event) => event.type === 'keyDown' && event.keyCode === 'Tab') - ? await guardFocus() - : () => {} + const release = remaining.some(isTabDown) ? await guardFocus() : () => {} try { - await withAgentInputInjection(webContents, () => - // Widget events bypass CDP send, so track held keys here. - trackRemoteCliInput(webContents.id, 'Input.dispatchKeyEvent', params, async () => { - for (const event of plan.events) { - webContents.sendInputEvent(event) - } - if (plan.commands.length > 0 || plan.events.length > 0) { - await waitForPageToHandleInput(send) - } - for (const command of plan.commands) { - runEditCommand(webContents, command) - } - }) - ) + await withAgentInputInjection(webContents, async () => { + // One page round trip settles the whole run; edit commands must see the keys before them. + let unsettled = false + for (const [index, plan] of plans.entries()) { + // Widget events bypass CDP send, so track held keys here. + await trackRemoteCliInput( + webContents.id, + 'Input.dispatchKeyEvent', + remaining[index], + async () => { + for (const event of plan.events) { + webContents.sendInputEvent(event) + } + unsettled ||= plan.events.length > 0 + if (plan.commands.length > 0) { + await waitForPageToHandleInput(send) + unsettled = false + for (const command of plan.commands) { + runEditCommand(webContents, command) + } + } + } + ) + } + if (unsettled) { + await waitForPageToHandleInput(send) + } + }) } finally { release() } @@ -150,7 +190,12 @@ export async function dispatchPageKeyEvent( } const release = await focusGuestForCdpInput(webContents, send, guardFocus) try { - return await withAgentInputInjection(webContents, () => send('Input.dispatchKeyEvent', params)) + for (const params of remaining) { + result = await withAgentInputInjection(webContents, () => + send('Input.dispatchKeyEvent', params) + ) + } + return result } finally { release() } diff --git a/src/main/orcad/external-chromium-browser-process.ts b/src/main/orcad/external-chromium-browser-process.ts index f5965afa60a..7b8d25a8b65 100644 --- a/src/main/orcad/external-chromium-browser-process.ts +++ b/src/main/orcad/external-chromium-browser-process.ts @@ -2,6 +2,7 @@ import { z } from 'zod' import type { RuntimeBrowserCommandHost } from '../runtime/browser/browser-command-contracts' import type { RuntimeBrowserCommands } from '../runtime/browser/browser-commands' import { BrowserError } from '../browser/browser-error' +import { assertUploadFilesExist } from '../browser/browser-upload-files' import { ExternalChromiumBrowserSession, type ExternalChromiumLaunch @@ -179,6 +180,10 @@ export class ExternalChromiumBrowserProcess { return { url: tab.url, title: tab.title } } + if (method === 'browserUpload') { + const files: unknown[] = Array.isArray(params.files) ? params.files : [] + await assertUploadFilesExist(files.map(String)) + } const command = externalChromiumCommandArguments(method, params) if (!command) { throw new BrowserError( diff --git a/tests/e2e/helpers/browser-agent-focus-scenarios.ts b/tests/e2e/helpers/browser-agent-focus-scenarios.ts index 037a888f91e..5e136767c1a 100644 --- a/tests/e2e/helpers/browser-agent-focus-scenarios.ts +++ b/tests/e2e/helpers/browser-agent-focus-scenarios.ts @@ -216,8 +216,8 @@ export async function runAgentKeysScenario( `JSON.stringify({ submits: window.__submits, keys: window.__keys })` ) ) - // Why: `type` inserts text without key events, so only the keypresses log keydowns. - .toBe(JSON.stringify({ submits: 1, keys: ['Z', 'Enter'] })) + // Why: `type` presses real keys, so its characters log keydowns like the keypresses. + .toBe(JSON.stringify({ submits: 1, keys: ['Z', 'q', 'r', 'Enter'] })) await afterAgentInput?.(pageId) } finally { await server.close() @@ -257,7 +257,7 @@ export async function runAgentClicksScenario( .toBe(JSON.stringify({ active: 'probe', focused: true, blurs: 0 })) expect((await readHost(page)).terminalFocused).toBe(true) - // Why: text with a newline cannot be typed as key events; CDP delivers it with focus borrowed. + // Why: fill writes multiline values straight into the field, never through the user's focus. await rpc(page, 'browser.fill', { page: pageId, element: '#notes', value: 'a\nb' }) await expect .poll(() => evalInPage(page, pageId, `document.getElementById('notes').value`))