From 13964967b58eaf612103a259b71714b5dfc4010d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 15 Sep 2026 17:10:34 -0700 Subject: [PATCH 1/3] fix(browser): preserve click targets and bound screenshot capture --- apps/desktop/e2e/browser-tools.spec.ts | 183 +++++++++++++++++- .../src/main/browser-agent/cdp.test.ts | 131 +++++++++---- apps/desktop/src/main/browser-agent/cdp.ts | 78 ++++++-- .../src/main/browser-agent/driver.test.ts | 72 +++---- apps/desktop/src/main/browser-agent/driver.ts | 29 ++- .../main/browser-agent/page-functions.test.ts | 66 +++++++ .../src/main/browser-agent/page-functions.ts | 20 +- .../client/browser-tool-execution.test.ts | 19 ++ .../tools/client/browser-tool-execution.ts | 5 +- 9 files changed, 484 insertions(+), 119 deletions(-) diff --git a/apps/desktop/e2e/browser-tools.spec.ts b/apps/desktop/e2e/browser-tools.spec.ts index d7edc1bf521..a6e3e67bc4a 100644 --- a/apps/desktop/e2e/browser-tools.spec.ts +++ b/apps/desktop/e2e/browser-tools.spec.ts @@ -27,6 +27,23 @@ const FORM = `Form fixture ` +const CLICK_FIXTURE = `Click fixture + + +` + test.describe('browser tools', () => { const calls = new Map< string, @@ -56,9 +73,11 @@ test.describe('browser tools', () => { } response.writeHead(200, { 'Content-Type': 'text/html' }) response.end( - path === '/form' - ? FORM - : 'Sim fixture

Browser tools fixture

' + path === '/click' + ? CLICK_FIXTURE + : path === '/form' + ? FORM + : 'Sim fixture

Browser tools fixture

' ) }) await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) @@ -78,15 +97,21 @@ test.describe('browser tools', () => { }, }) window = await app.firstWindow() + await app.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows()[0].webContents.setBackgroundThrottling(false) + ) await expect(window.getByRole('heading')).toHaveText('Browser tools fixture') await window.evaluate(async (scope) => { const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop await api.browserAgent.activateScope(scope) - api.browserAgent.setPanelBounds( - { x: 0, y: 80, width: innerWidth, height: innerHeight - 80 }, - null, - scope - ) + const updateBounds = () => + api.browserAgent.setPanelBounds( + { x: 0, y: 80, width: innerWidth, height: innerHeight - 80 }, + null, + scope + ) + updateBounds() + setInterval(updateBounds, 200) }, SCOPE) }) @@ -143,6 +168,148 @@ test.describe('browser tools', () => { }, origin) } + for (const mode of ['menu', 'sticky']) { + test(`clicks a ${mode} target without losing its identity`, async () => { + const opened = await execute('browser_open_url', { url: `${origin}/click?mode=${mode}` }) + expect(opened.ok, opened.error).toBe(true) + await app.evaluate( + async ({ webContents }, { origin, mode }) => { + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL().startsWith(`${origin}/click`)) + if (!contents) throw new Error('Missing click fixture') + await contents.executeJavaScript(` + history.scrollRestoration = 'manual'; + document.getElementById('target').style.top = ${mode === 'sticky' ? '1010' : 'innerHeight - 100'} + 'px'; + scrollTo(0, ${mode === 'sticky' ? '1000' : '0'}); + new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(() => { + document.body.dataset.scrolls = '0'; document.body.dataset.armed = 'true'; resolve(); + }))) + `) + }, + { origin, mode } + ) + const snapshot = await execute('browser_snapshot', {}) + expect(snapshot.ok, snapshot.error).toBe(true) + const outline = (snapshot.result as { outline: string }).outline + const line = outline.split('\n').find((line) => line.includes('"Choose option"')) + const match = line?.match(/\[ref=(\d+)\]/) + if (!match) throw new Error(`Missing target: ${outline}`) + const result = await execute('browser_click', { elementId: Number(match[1]) }) + expect(result.ok, result.error).toBe(true) + const state = await app.evaluate(async ({ webContents }, origin) => { + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL().startsWith(`${origin}/click`)) + if (!contents) throw new Error('Missing click fixture') + return contents.executeJavaScript( + '({clicks:document.body.dataset.clicks,scrolls:document.body.dataset.scrolls,scrollY})' + ) + }, origin) + expect(state.clicks).toBe('1') + if (mode === 'menu') expect(state).toMatchObject({ scrolls: '0', scrollY: 0 }) + else expect(state.scrollY).toBeLessThan(1000) + }) + } + + for (const mode of ['visible', 'hidden', 'minimized']) { + test(`captures a ${mode} window without changing its state`, async () => { + test.skip( + mode === 'minimized' && process.platform !== 'darwin', + 'Requires a window manager with minimize events' + ) + await openForm() + await app.evaluate(async ({ BrowserWindow }, mode) => { + const win = BrowserWindow.getAllWindows()[0] + win.blur() + if (mode === 'hidden') win.hide() + if (mode === 'minimized') { + const minimized = new Promise((resolve) => win.once('minimize', () => resolve())) + win.minimize() + await minimized + } + }, mode) + const state = () => + app.evaluate(async ({ BrowserWindow, webContents }, origin) => { + const win = BrowserWindow.getAllWindows()[0] + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL() === `${origin}/form`) + if (!contents) throw new Error('Missing screenshot fixture') + return { + visible: win.isVisible(), + minimized: win.isMinimized(), + bounds: win.getBounds(), + focused: BrowserWindow.getFocusedWindow()?.id ?? null, + page: await contents.executeJavaScript( + '({width:innerWidth,height:innerHeight,scrollX,scrollY,html:document.body.innerHTML,focus:document.activeElement?.id})' + ), + } + }, origin) + const before = await state() + for (let i = 0; i < 3; i++) { + const response = await execute('browser_screenshot', {}) + expect(response.ok, response.error).toBe(true) + const shot = response.result as { + dataUrl: string + scale: number + viewport: { width: number; height: number } + } + expect(shot.dataUrl.length).toBeGreaterThan(1000) + expect(shot.viewport.width).toBeGreaterThan(0) + expect(shot.viewport.height).toBeGreaterThan(0) + const image = await app.evaluate(({ nativeImage }, dataUrl) => { + const image = nativeImage.createFromDataURL(dataUrl) + return { empty: image.isEmpty(), ...image.getSize() } + }, shot.dataUrl) + expect(image).toEqual({ + empty: false, + width: Math.round(shot.viewport.width * shot.scale), + height: Math.round(shot.viewport.height * shot.scale), + }) + expect(await state()).toEqual(before) + } + }) + } + + test('captures fresh pixels after resizing and repainting the viewport', async () => { + await openForm() + const viewportWidth = () => + app.evaluate(async ({ webContents }, origin) => { + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL() === `${origin}/form`) + return contents?.executeJavaScript('innerWidth') + }, origin) + const beforeWidth = await viewportWidth() + await app.evaluate(({ BrowserWindow }) => BrowserWindow.getAllWindows()[0].setSize(1280, 900)) + await expect.poll(viewportWidth).not.toBe(beforeWidth) + for (const color of ['red', 'blue']) { + await app.evaluate( + async ({ webContents }, { origin, color }) => { + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL() === `${origin}/form`) + if (!contents) throw new Error('Missing screenshot fixture') + await contents.executeJavaScript( + `document.body.style.background = ${JSON.stringify(color)}; void 0` + ) + }, + { origin, color } + ) + const response = await execute('browser_screenshot', {}) + expect(response.ok, response.error).toBe(true) + const shot = response.result as { dataUrl: string } + const pixel = await app.evaluate(({ nativeImage }, dataUrl) => { + const image = nativeImage.createFromDataURL(dataUrl) + return Array.from(image.toBitmap().subarray(0, 4)) + }, shot.dataUrl) + const dominant = pixel[color === 'red' ? 2 : 0] + const other = pixel[color === 'red' ? 0 : 2] + expect(dominant - other, `${color}: ${pixel}`).toBeGreaterThan(150) + } + }) + test('opens with references, fills in order, and scrolls a horizontal pane', async () => { const ref = await openForm() const fill = await execute('browser_fill_form', { diff --git a/apps/desktop/src/main/browser-agent/cdp.test.ts b/apps/desktop/src/main/browser-agent/cdp.test.ts index c65c987f361..80ce5c2c48a 100644 --- a/apps/desktop/src/main/browser-agent/cdp.test.ts +++ b/apps/desktop/src/main/browser-agent/cdp.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -import { nativeImage, type WebContents, WebContentsView, type WebFrameMain } from 'electron' +import { type nativeImage, WebContentsView, type WebFrameMain } from 'electron' import { captureScreenshot, clickAt, @@ -497,7 +497,6 @@ describe('browser-agent screenshot capture', () => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) const resized = { @@ -509,25 +508,15 @@ describe('browser-agent screenshot capture', () => { resize: vi.fn(() => resized), toJPEG: vi.fn(() => Buffer.from('cropped')), } - // Shared module-level mock: without this, a later fixture reads the - // earlier test's decoded image. - vi.mocked(nativeImage.createFromBuffer).mockReset() - vi.mocked(nativeImage.createFromBuffer).mockReturnValue({ + const image = { isEmpty: vi.fn(() => imageSize === null), getSize: vi.fn(() => imageSize ?? { width: 0, height: 0 }), crop: vi.fn(() => cropped), resize: vi.fn(() => resized), - toJPEG: vi.fn(() => Buffer.alloc(0)), - } as unknown as ReturnType) - return { contents, resized, cropped } - } - - function screenshotParams(contents: WebContents): Record { - const call = vi - .mocked(contents.debugger.sendCommand) - .mock.calls.find(([method]) => method === 'Page.captureScreenshot') - if (!call) throw new Error('no capture was requested') - return call[1] as Record + toJPEG: vi.fn(() => Buffer.from('sim')), + } as unknown as ReturnType + vi.mocked(contents.capturePage).mockResolvedValue(image) + return { contents, resized, cropped, image } } it('never sends a clip, which would emulate the live page for the capture', async () => { @@ -535,16 +524,23 @@ describe('browser-agent screenshot capture', () => { await captureScreenshot(contents) - expect(screenshotParams(contents)).not.toHaveProperty('clip') + expect(contents.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: false }) + expect(contents.debugger.sendCommand).not.toHaveBeenCalledWith( + 'Page.captureScreenshot', + expect.anything() + ) }) it('crops the decoded image in memory without sending a CDP clip', async () => { - const { contents, cropped } = captureFixture({ width: 4096, height: 2048 }) + const { contents, cropped, image } = captureFixture({ width: 4096, height: 2048 }) const shot = await captureScreenshot(contents, { x: 100, y: 50, width: 200, height: 100 }) - const image = vi.mocked(nativeImage.createFromBuffer).mock.results[0].value - expect(screenshotParams(contents)).not.toHaveProperty('clip') + expect(contents.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: false }) + expect(contents.debugger.sendCommand).not.toHaveBeenCalledWith( + 'Page.captureScreenshot', + expect.anything() + ) expect(image.crop).toHaveBeenCalledWith({ x: 200, y: 100, width: 400, height: 200 }) expect(cropped.resize).not.toHaveBeenCalled() expect(shot).toEqual({ @@ -562,11 +558,10 @@ describe('browser-agent screenshot capture', () => { * (cssX = imageX / scale) assumes. */ it('downscales the returned image to the CSS-relative size', async () => { - const { contents, resized } = captureFixture({ width: 4096, height: 2048 }) + const { contents, resized, image } = captureFixture({ width: 4096, height: 2048 }) const shot = await captureScreenshot(contents) - const image = vi.mocked(nativeImage.createFromBuffer).mock.results[0].value expect(image.resize).toHaveBeenCalledWith({ width: 1024, height: 512, quality: 'good' }) expect(resized.toJPEG).toHaveBeenCalled() expect(shot).toEqual({ @@ -577,12 +572,11 @@ describe('browser-agent screenshot capture', () => { }) }) - it('skips the re-encode when the capture already matches the target size', async () => { - const { contents } = captureFixture({ width: 1024, height: 512 }) + it('skips resizing when the capture already matches the target size', async () => { + const { contents, image } = captureFixture({ width: 1024, height: 512 }) const shot = await captureScreenshot(contents) - const image = vi.mocked(nativeImage.createFromBuffer).mock.results[0].value expect(image.resize).not.toHaveBeenCalled() expect(shot).toEqual({ dataUrl: 'data:image/jpeg;base64,c2lt', @@ -592,16 +586,82 @@ describe('browser-agent screenshot capture', () => { }) }) - it('returns the raw capture when the image cannot be decoded', async () => { + it('rejects an empty native capture', async () => { const { contents } = captureFixture(null) + await expect(captureScreenshot(contents)).rejects.toThrow('empty image') + }) - const shot = await captureScreenshot(contents) + it('bounds a stalled capture and prevents overlapping native surface copies', async () => { + vi.useFakeTimers() + try { + const { contents, image } = captureFixture({ width: 1024, height: 512 }) + let release: (captured: typeof image) => void = () => {} + vi.mocked(contents.capturePage).mockImplementationOnce( + () => + new Promise((resolve) => { + release = resolve + }) + ) + const failed = expect(captureScreenshot(contents)).rejects.toThrow('pixel capture timed out') + await vi.advanceTimersByTimeAsync(5_000) + await failed + expect(vi.getTimerCount()).toBe(0) + await expect(captureScreenshot(contents)).rejects.toThrow( + 'previous screenshot capture is still pending' + ) + expect(contents.capturePage).toHaveBeenCalledOnce() + release(image) + await Promise.resolve() + await expect(captureScreenshot(contents)).resolves.toMatchObject({ + imageSize: { width: 1024, height: 512 }, + }) + expect(contents.capturePage).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) - expect(shot).toEqual({ - dataUrl: 'data:image/jpeg;base64,c2lt', - scale: 0.5, - viewport: { width: 2048, height: 1024 }, - imageSize: null, + it.each(['cancel', 'destroy'] as const)( + 'releases capture listeners and timer on %s', + async (reason) => { + vi.useFakeTimers() + try { + const { contents } = captureFixture({ width: 1024, height: 512 }) + vi.mocked(contents.capturePage).mockReturnValue(new Promise(() => {})) + const controller = new AbortController() + const failed = expect( + captureScreenshot(contents, undefined, controller.signal) + ).rejects.toThrow(reason === 'cancel' ? 'cancelled' : 'tab was closed') + await vi.advanceTimersByTimeAsync(0) + const destroyed = vi + .mocked(contents.once) + .mock.calls.find(([event]) => String(event) === 'destroyed')?.[1] as unknown as + | (() => void) + | undefined + expect(destroyed).toBeDefined() + if (reason === 'cancel') controller.abort() + else destroyed?.() + await failed + expect(contents.removeListener).toHaveBeenCalledWith('destroyed', destroyed) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + } + ) + + it('does not start capture after cancellation or keep a synchronous failure pending', async () => { + const { contents } = captureFixture({ width: 1024, height: 512 }) + const controller = new AbortController() + controller.abort() + await expect(captureScreenshot(contents, undefined, controller.signal)).rejects.toThrow() + expect(contents.capturePage).not.toHaveBeenCalled() + vi.mocked(contents.capturePage).mockImplementationOnce(() => { + throw new Error('native failure') + }) + await expect(captureScreenshot(contents)).rejects.toThrow('native failure') + await expect(captureScreenshot(contents)).resolves.toMatchObject({ + imageSize: { width: 1024, height: 512 }, }) }) @@ -611,7 +671,6 @@ describe('browser-agent screenshot capture', () => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) @@ -652,7 +711,6 @@ describe('browser-agent screenshot capture', () => { }, }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) @@ -697,7 +755,7 @@ describe('browser-agent screenshot capture', () => { ], ['availability', {}, {}], ])( - 'rejects a capture when viewport %s change during CDP capture', + 'rejects a capture when viewport %s change during native capture', async (_label, before, after) => { const { contents } = captureFixture({ width: 1024, height: 512 }) let metricsRead = 0 @@ -706,7 +764,6 @@ describe('browser-agent screenshot capture', () => { metricsRead++ return Promise.resolve(metricsRead === 1 ? before : after) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index e966850aaf2..bbe8921ebb6 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -11,7 +11,7 @@ import type { BrowserTheme } from '@sim/browser-protocol' import { createLogger } from '@sim/logger' import { sleep } from '@sim/utils/helpers' -import { nativeImage, type WebContents, type WebFrameMain } from 'electron' +import type { NativeImage, WebContents, WebFrameMain } from 'electron' const logger = createLogger('BrowserAgentCdp') @@ -370,14 +370,10 @@ export async function evaluateInIsolatedFrame( */ const MAX_SCREENSHOT_EDGE = 1024 const SCREENSHOT_QUALITY = 70 -/** - * Quality of the intermediate capture, before the in-process downscale - * re-encodes at {@link SCREENSHOT_QUALITY}. Higher than the final quality so - * the two lossy passes together land near where one pass did — the model reads - * text out of these frames, and compression artifacts on glyphs cost more than - * the transient bytes do. - */ -const SCREENSHOT_CAPTURE_QUALITY = 90 +const UNSCALED_SCREENSHOT_QUALITY = 90 +const SCREENSHOT_CAPTURE_TIMEOUT_MS = 5_000 +/** Native surface copies cannot be cancelled; never accumulate them on a stalled tab. */ +const pendingScreenshotCaptures = new WeakSet() interface CdpViewport { clientWidth: number @@ -401,7 +397,7 @@ export interface ScreenshotCapture { dataUrl: string scale: number viewport: ScreenshotSize | null - imageSize: ScreenshotSize | null + imageSize: ScreenshotSize } export interface ScreenshotClip { @@ -456,8 +452,48 @@ function sameScreenshotViewport( ) } +/** Captures pixels without changing viewport geometry or exposing a hidden window. */ +async function captureViewportImage( + contents: WebContents, + signal?: AbortSignal +): Promise { + signal?.throwIfAborted() + if (contents.isDestroyed()) throw new Error('The screenshot tab was closed') + if (pendingScreenshotCaptures.has(contents)) { + throw new Error('A previous screenshot capture is still pending on this tab') + } + pendingScreenshotCaptures.add(contents) + let timer: ReturnType | undefined + let onAbort = () => {} + let onDestroyed = () => {} + try { + const interrupted = new Promise((_resolve, reject) => { + onAbort = () => reject(new Error('Screenshot capture was cancelled')) + onDestroyed = () => reject(new Error('The screenshot tab was closed')) + signal?.addEventListener('abort', onAbort, { once: true }) + contents.once('destroyed', onDestroyed) + timer = setTimeout( + () => reject(new Error('Screenshot pixel capture timed out after 5 seconds')), + SCREENSHOT_CAPTURE_TIMEOUT_MS + ) + }) + const capture = (async () => { + try { + return await contents.capturePage(undefined, { stayHidden: false }) + } finally { + pendingScreenshotCaptures.delete(contents) + } + })() + return await Promise.race([capture, interrupted]) + } finally { + clearTimeout(timer) + signal?.removeEventListener('abort', onAbort) + contents.removeListener('destroyed', onDestroyed) + } +} + /** - * Screenshot via CDP (works while the view is hidden), bounded in resolution. + * Native viewport capture, bounded in time and resolution. * * The capture is deliberately UNCLIPPED. Chromium implements `clip` by applying * device-emulation parameters (viewport offset and scale) to the widget and @@ -473,7 +509,8 @@ function sameScreenshotViewport( */ export async function captureScreenshot( contents: WebContents, - clip?: ScreenshotClip + clip?: ScreenshotClip, + signal?: AbortSignal ): Promise { const metrics = await send<{ cssLayoutViewport?: CdpViewport @@ -492,10 +529,7 @@ export async function captureScreenshot( const scale = width > 0 && height > 0 ? Math.min(1, MAX_SCREENSHOT_EDGE / Math.max(width, height)) : 1 - const result = await send<{ data: string }>(contents, 'Page.captureScreenshot', { - format: 'jpeg', - quality: SCREENSHOT_CAPTURE_QUALITY, - }) + const image = await captureViewportImage(contents, signal) const metricsAfterCapture = await send<{ cssLayoutViewport?: CdpViewport layoutViewport?: CdpViewport @@ -503,15 +537,12 @@ export async function captureScreenshot( if (!sameScreenshotViewport(captureViewport, screenshotViewportMetrics(metricsAfterCapture))) { throw new Error('The page viewport changed or could not be verified during screenshot capture') } - const captured = `data:image/jpeg;base64,${result.data}` const targetWidth = Math.round(width * scale) const targetHeight = Math.round(height * scale) - const image = nativeImage.createFromBuffer(Buffer.from(result.data, 'base64')) const size = image.isEmpty() ? { width: 0, height: 0 } : image.getSize() if (size.width === 0 || size.height === 0) { - if (clip) throw new Error('The screenshot could not be decoded for element cropping') - return { dataUrl: captured, scale, viewport: cssViewport, imageSize: null } + throw new Error('Screenshot pixel capture returned an empty image') } if (clip && cssViewport) { const xScale = size.width / cssViewport.width @@ -554,7 +585,12 @@ export async function captureScreenshot( } } if (size.width === targetWidth && size.height === targetHeight) { - return { dataUrl: captured, scale, viewport: cssViewport, imageSize: size } + return { + dataUrl: `data:image/jpeg;base64,${image.toJPEG(UNSCALED_SCREENSHOT_QUALITY).toString('base64')}`, + scale, + viewport: cssViewport, + imageSize: size, + } } const resized = image.resize({ width: targetWidth, height: targetHeight, quality: 'good' }) diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index 5cb48a1cd3c..4a011ac895c 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -1,10 +1,10 @@ import { BROWSER_TOOL_QUEUE_WAIT_TIMEOUT_MS } from '@sim/browser-protocol' -import type { MenuItemConstructorOptions } from 'electron' +import type { MenuItemConstructorOptions, WebContents } from 'electron' import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -import { BrowserWindow, Menu, nativeImage } from 'electron' +import { BrowserWindow, Menu, type nativeImage } from 'electron' import * as cdp from '@/main/browser-agent/cdp' import * as driverModule from '@/main/browser-agent/driver' import * as session from '@/main/browser-agent/session' @@ -485,8 +485,11 @@ describe('executeTool', () => { ) await Promise.resolve() expect(captureScreenshot).toHaveBeenCalledOnce() + const signal = captureScreenshot.mock.calls[0][2] + expect(signal?.aborted).toBe(false) driver.disposeBrowserScope('chat-test') + expect(signal?.aborted).toBe(true) automationTab.mockClear() await expect(screenshot).resolves.toMatchObject({ ok: false, @@ -2282,8 +2285,11 @@ describe('credential protection', () => { expect(form.writes).toEqual([0]) }) - function mockScreenshotImage(size: { width: number; height: number } | null): void { - vi.mocked(nativeImage.createFromBuffer).mockReturnValueOnce({ + function mockScreenshotImage( + contents: WebContents, + size: { width: number; height: number } | null + ): void { + vi.mocked(contents.capturePage).mockResolvedValue({ isEmpty: vi.fn(() => size === null), getSize: vi.fn(() => size ?? { width: 0, height: 0 }), resize: vi.fn(() => ({ toJPEG: vi.fn(() => Buffer.from('resized')) })), @@ -3966,7 +3972,11 @@ describe('credential protection', () => { try { const result = await driver.executeTool('chat-test', 'browser_screenshot', { elementId: 0 }) - expect(capture).toHaveBeenCalledWith(contents, { x: 20, y: 30, width: 200, height: 100 }) + expect(capture).toHaveBeenCalledWith( + contents, + { x: 20, y: 30, width: 200, height: 100 }, + expect.any(AbortSignal) + ) expect(result).toMatchObject({ ok: true, result: { element: 'button', clip: { x: 20, y: 30, width: 200, height: 100 } }, @@ -4009,16 +4019,13 @@ describe('credential protection', () => { it('returns the screenshot scale for coordinate mapping', async () => { const contents = await openPage() - mockScreenshotImage({ width: 1024, height: 512 }) + mockScreenshotImage(contents, { width: 1024, height: 512 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, }) } - if (method === 'Page.captureScreenshot') { - return Promise.resolve({ data: 'c2lt' }) - } return Promise.resolve(undefined) }) respondWith(contents, { getViewportInfo: { width: 2048, height: 1024 } }) @@ -4046,14 +4053,11 @@ describe('credential protection', () => { it('uses the in-page CSS viewport when CDP exposes only deprecated device metrics', async () => { const contents = await openPage() - mockScreenshotImage({ width: 1024, height: 512 }) + mockScreenshotImage(contents, { width: 1024, height: 512 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) } - if (method === 'Page.captureScreenshot') { - return Promise.resolve({ data: 'c2lt' }) - } return Promise.resolve(undefined) }) respondWith(contents, { @@ -4102,12 +4106,11 @@ describe('credential protection', () => { const fullTitle = `Example ${'t'.repeat(600)}` vi.mocked(contents.getURL).mockReturnValue(fullUrl) vi.mocked(contents.getTitle).mockReturnValue(fullTitle) - mockScreenshotImage({ width: 1024, height: 512 }) + mockScreenshotImage(contents, { width: 1024, height: 512 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) respondWith(contents, { @@ -4135,33 +4138,31 @@ describe('credential protection', () => { }) }) - it('rejects an undecodable screenshot instead of returning an unverified scale', async () => { + it('rejects an empty screenshot instead of returning an unverified scale', async () => { const contents = await openPage() - mockScreenshotImage(null) + mockScreenshotImage(contents, null) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) expect(result.ok).toBe(false) - expect(result.error).toMatch(/verify the screenshot dimensions/) + expect(result.error).toMatch(/empty image/) }) it('rejects a screenshot when no CSS viewport can be established', async () => { const contents = await openPage() - mockScreenshotImage({ width: 1024, height: 512 }) + mockScreenshotImage(contents, { width: 1024, height: 512 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) respondWith(contents, { getViewportInfo: null }) @@ -4174,12 +4175,11 @@ describe('credential protection', () => { it('rejects coordinate mapping when the viewport changes during capture', async () => { const contents = await openPage() - mockScreenshotImage({ width: 1024, height: 256 }) + mockScreenshotImage(contents, { width: 1024, height: 256 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ layoutViewport: { clientWidth: 1024, clientHeight: 256 } }) } - if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) return Promise.resolve(undefined) }) respondWith(contents, { @@ -4199,20 +4199,22 @@ describe('credential protection', () => { it('rejects a screenshot when the document navigates during capture', async () => { const contents = await openPage() - mockScreenshotImage({ width: 1024, height: 512 }) + mockScreenshotImage(contents, { width: 1024, height: 512 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, }) } - if (method === 'Page.captureScreenshot') { - emitContentsEvent(contents, 'did-navigate') - return Promise.resolve({ data: 'c2lt' }) - } return Promise.resolve(undefined) }) + const image = await contents.capturePage() + vi.mocked(contents.capturePage).mockImplementation(async () => { + emitContentsEvent(contents, 'did-navigate') + return image + }) + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) expect(result.ok).toBe(false) @@ -4223,7 +4225,7 @@ describe('credential protection', () => { 'rejects a screenshot when the page %s changes during capture', async (identityField) => { const contents = await openPage() - mockScreenshotImage({ width: 1024, height: 512 }) + mockScreenshotImage(contents, { width: 1024, height: 512 }) const initialUrl = contents.getURL() const initialTitle = contents.getTitle() let currentUrl = initialUrl @@ -4236,14 +4238,16 @@ describe('credential protection', () => { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, }) } - if (method === 'Page.captureScreenshot') { - if (identityField === 'url') currentUrl = 'https://example.com/changed' - else currentTitle = 'Changed title' - return Promise.resolve({ data: 'c2lt' }) - } return Promise.resolve(undefined) }) + const image = await contents.capturePage() + vi.mocked(contents.capturePage).mockImplementation(async () => { + if (identityField === 'url') currentUrl = 'https://example.com/changed' + else currentTitle = 'Changed title' + return image + }) + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) expect(result.ok).toBe(false) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 807aaf81444..15af3363f71 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -2278,7 +2278,8 @@ async function executeToolInner( params: Record, assertCurrentExecution: () => void, executionDeadline: number | undefined, - invocationEpoch: number + invocationEpoch: number, + signal?: AbortSignal ): Promise { switch (tool) { case 'browser_navigate': { @@ -2630,15 +2631,11 @@ async function executeToolInner( } : undefined assertCaptureIsCurrent() - const shot = await cdp.captureScreenshot(contents, clip).catch((error) => { - logger.warn('Browser screenshot capture failed', { error: getErrorMessage(error) }) - return null - }) - if (!shot) { + const shot = await cdp.captureScreenshot(contents, clip, signal).catch((error) => { throw new ToolError( - 'Could not capture the page. Use browser_snapshot or browser_read_text instead.' + `Could not capture the page: ${getErrorMessage(error)}. Use browser_snapshot or browser_read_text instead.` ) - } + }) assertCaptureIsCurrent() if (elementId !== undefined && elementClip) { const currentClip = toRecord( @@ -2664,11 +2661,6 @@ async function executeToolInner( 'The screenshot result was too large to return safely. Use browser_snapshot or browser_read_text instead.' ) } - if (!shot.imageSize) { - throw new ToolError( - 'Could not verify the screenshot dimensions. Retry browser_screenshot or use browser_snapshot instead.' - ) - } const viewport = shot.viewport ? { url: capturedViewportUrl, @@ -4599,9 +4591,13 @@ export async function executeTool( throw new ToolError('This browser action was cancelled before it started.') } state.activeToolCallId = toolCallId ?? null + const executionController = new AbortController() let cancelActiveExecution: () => void = () => {} const cancellation = new Promise((_resolve, reject) => { - cancelActiveExecution = () => reject(new ToolError('This browser action was cancelled.')) + cancelActiveExecution = () => { + executionController.abort() + reject(new ToolError('This browser action was cancelled.')) + } }) state.activeToolCancel = cancelActiveExecution return await session.withBrowserScope(resolvedScopeId, async () => { @@ -4629,12 +4625,14 @@ export async function executeTool( params, assertCurrentExecution, executionDeadline, - invocationEpoch + invocationEpoch, + executionController.signal ) const guardedExecution = watchdogMs === null ? execution : raceAgainstWatchdog(execution, watchdogMs, () => { + executionController.abort() if (state.toolExecutionEpoch === executionEpoch) state.toolExecutionEpoch++ if ( tool === 'browser_snapshot' || @@ -4654,6 +4652,7 @@ export async function executeTool( }) return result } finally { + executionController.abort() if (keepHiddenPageActive && !state.disposed) { session.setAutomationActive(false) } diff --git a/apps/desktop/src/main/browser-agent/page-functions.test.ts b/apps/desktop/src/main/browser-agent/page-functions.test.ts index f5b3148ba3c..544438dc3c1 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.test.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.test.ts @@ -132,6 +132,72 @@ afterEach(() => { document.body.innerHTML = '' }) +describe('conditional click scrolling', () => { + it('leaves a reachable target in place', () => { + const target = visible(document.createElement('button')) + document.body.append(target) + register(target) + target.scrollIntoView = vi.fn() + expect(runSerialized(clickElement, [0, false])).toMatchObject({ x: 50, y: 10 }) + expect(target.scrollIntoView).not.toHaveBeenCalled() + }) + + it('rechecks the hit target after scrolling past a sticky obstruction', () => { + const target = visible(document.createElement('button')) + const obstruction = visible(document.createElement('div')) + document.body.append(target, obstruction) + register(target) + let scrolled = false + target.scrollIntoView = vi.fn(() => { + scrolled = true + }) + Object.defineProperty(document, 'elementFromPoint', { + configurable: true, + value: () => (scrolled ? target : obstruction), + }) + expect(runSerialized(clickElement, [0, false])).toMatchObject({ x: 50, y: 10 }) + expect(target.scrollIntoView).toHaveBeenCalledOnce() + }) + + it('reveals a parent control when only its nested button is initially reachable', () => { + const card = visible(document.createElement('div')) + card.setAttribute('role', 'button') + const nested = visible(document.createElement('button')) + card.append(nested) + document.body.append(card) + register(card) + let scrolled = false + card.scrollIntoView = vi.fn(() => { + scrolled = true + }) + const nestedClick = vi.fn() + nested.addEventListener('click', nestedClick) + Object.defineProperty(document, 'elementFromPoint', { + configurable: true, + value: () => (scrolled ? card : nested), + }) + expect(runSerialized(clickElement, [0, false])).toMatchObject({ x: 50, y: 10 }) + expect(card.scrollIntoView).toHaveBeenCalledOnce() + expect(nestedClick).not.toHaveBeenCalled() + }) + + it('rejects a target removed by scrolling without dispatching input', () => { + const target = visible(document.createElement('button')) + const obstruction = visible(document.createElement('div')) + document.body.append(target, obstruction) + register(target) + const click = vi.fn() + target.addEventListener('click', click) + target.scrollIntoView = () => target.remove() + Object.defineProperty(document, 'elementFromPoint', { + configurable: true, + value: () => obstruction, + }) + expect(runSerialized(clickElement, [0])).toMatchObject({ error: 'stale' }) + expect(click).not.toHaveBeenCalled() + }) +}) + describe('serialization contract', () => { // The driver ships each of these to the page as `String(fn)`, so a reference // to anything in module scope — a shared helper, an import, a constant — diff --git a/apps/desktop/src/main/browser-agent/page-functions.ts b/apps/desktop/src/main/browser-agent/page-functions.ts index 419ae827182..7d48440f182 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.ts @@ -929,7 +929,8 @@ export function clickElement( id: number, dispatchSynthetic = true, focusForKeyboard = false, - allowDisabled = false + allowDisabled = false, + scrollToTarget = false ): unknown { const isSecretField = (node: Element | null): boolean => { if (!node || String(node.tagName || '').toUpperCase() !== 'INPUT') return false @@ -974,7 +975,10 @@ export function clickElement( return { error: 'file-input' } } } - el.scrollIntoView({ block: 'center', inline: 'center', behavior: 'instant' }) + if (scrollToTarget) { + el.scrollIntoView({ block: 'center', inline: 'center', behavior: 'instant' }) + if (!el.isConnected) return { error: 'stale', reason: window.__simAgentStaleReason } + } const view = el.ownerDocument.defaultView if (!view) return { error: 'stale', reason: window.__simAgentStaleReason } @@ -1021,7 +1025,11 @@ export function clickElement( rect.right - rect.left > 1 && rect.bottom - rect.top > 1 ) - if (rects.length === 0) return { error: 'not-visible' } + if (rects.length === 0) { + return scrollToTarget + ? { error: 'not-visible' } + : clickElement(id, dispatchSynthetic, focusForKeyboard, allowDisabled, true) + } const composedParent = (node: Element): Element | null => { if (node.parentElement) return node.parentElement @@ -1202,6 +1210,9 @@ export function clickElement( if (suggestionsCoverFocusedEditable()) { return { error: 'suggestions-open', blocker: blockerLabel(blocker) } } + if (!scrollToTarget) { + return clickElement(id, dispatchSynthetic, focusForKeyboard, allowDisabled, true) + } // A hit INSIDE the requested element is not an overlay — it is the ref // wrapping its own control (a row containing a button, a card containing a // link). hitBelongsToTarget rejects both cases identically, so this was @@ -1247,6 +1258,9 @@ export function clickElement( if (parentElementAt) { const parentHit: Element | null = parentElementAt(pageX, pageY) if (parentHit !== frame) { + if (!scrollToTarget) { + return clickElement(id, dispatchSynthetic, focusForKeyboard, allowDisabled, true) + } return { error: 'obstructed', blocker: blockerLabel(parentHit) } } } diff --git a/apps/sim/lib/copilot/tools/client/browser-tool-execution.test.ts b/apps/sim/lib/copilot/tools/client/browser-tool-execution.test.ts index 26ac1d9200a..1fdb9165092 100644 --- a/apps/sim/lib/copilot/tools/client/browser-tool-execution.test.ts +++ b/apps/sim/lib/copilot/tools/client/browser-tool-execution.test.ts @@ -581,6 +581,25 @@ describe('executeBrowserToolOnClient', () => { } ) + it('reports an unconfirmed effect without retrying or marking completed input as failed', async () => { + const result = { dispatched: true, effectObserved: false, possibleEffectObserved: true } + mockExecuteBrowserTool.mockResolvedValue(result) + const toolCallId = nextToolCallId() + + executeBrowserToolOnClient(toolCallId, 'browser_click', { elementId: 1 }) + await flush() + executeBrowserToolOnClient(toolCallId, 'browser_click', { elementId: 1 }) + await flush() + + expect(mockExecuteBrowserTool).toHaveBeenCalledOnce() + expect(mockReportCompletion).toHaveBeenCalledWith( + toolCallId, + 'success', + 'Browser input completed; its effect is unconfirmed. Inspect the current state before retrying.', + result + ) + }) + it('uses unload-safe delivery when a stateful replay-guard rejection cannot be reported normally', async () => { const storageWrite = vi.spyOn(window.sessionStorage, 'setItem').mockImplementation(() => { throw new DOMException('Quota exceeded', 'QuotaExceededError') diff --git a/apps/sim/lib/copilot/tools/client/browser-tool-execution.ts b/apps/sim/lib/copilot/tools/client/browser-tool-execution.ts index c9b52d8b139..ced31164d4f 100644 --- a/apps/sim/lib/copilot/tools/client/browser-tool-execution.ts +++ b/apps/sim/lib/copilot/tools/client/browser-tool-execution.ts @@ -984,6 +984,7 @@ async function doExecuteBrowserTool( } nativeActionPending = false if (cancelled) return + const effectUnconfirmed = isRecordLike(result) && result.effectObserved === false const formStopped = toolName === 'browser_fill_form' && isRecordLike(result) && result.completed === false reportTerminalCompletion( @@ -993,7 +994,9 @@ async function doExecuteBrowserTool( : ASYNC_TOOL_CONFIRMATION_STATUS.success, message: formStopped ? 'Form filling stopped; inspect the partial result' - : 'Browser action completed', + : effectUnconfirmed + ? 'Browser input completed; its effect is unconfirmed. Inspect the current state before retrying.' + : 'Browser action completed', data: sanitizeResultForModel(toolName, result), }, 'Failed to report successful browser tool completion' From 03c971a7c30ea65651bf0649fa9abbac59dfc873 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 15 Sep 2026 18:00:36 -0700 Subject: [PATCH 2/3] fix(browser): preserve observations and support native form controls --- apps/desktop/e2e/browser-tools.spec.ts | 93 +++++++- .../src/main/browser-agent/cdp.test.ts | 4 +- apps/desktop/src/main/browser-agent/cdp.ts | 2 +- .../src/main/browser-agent/driver.test.ts | 135 ++++++++++++ apps/desktop/src/main/browser-agent/driver.ts | 70 +++++- .../main/browser-agent/page-functions.test.ts | 196 +++++++++++++++++ .../src/main/browser-agent/page-functions.ts | 200 +++++++++++++++--- .../lib/copilot/generated/tool-catalog-v1.ts | 45 +++- .../lib/copilot/generated/tool-schemas-v1.ts | 59 +++++- .../tools/server/generated-schema.test.ts | 24 +++ 10 files changed, 773 insertions(+), 55 deletions(-) diff --git a/apps/desktop/e2e/browser-tools.spec.ts b/apps/desktop/e2e/browser-tools.spec.ts index a6e3e67bc4a..fa12465e190 100644 --- a/apps/desktop/e2e/browser-tools.spec.ts +++ b/apps/desktop/e2e/browser-tools.spec.ts @@ -18,7 +18,15 @@ const SCOPE = 'browser-tools-e2e' const FORM = `Form fixture + + + + + + + + Other website @@ -144,7 +152,9 @@ test.describe('browser tools', () => { const result = response.result as { snapshot: { outline: string } } expect(result.snapshot.outline).toContain('Name') return (name: string) => { - const line = result.snapshot.outline.split('\n').find((line) => line.includes(`"${name}"`)) + const line = result.snapshot.outline + .split('\n') + .find((line) => line.includes(`"${name}"`) && /\[ref=\d+\]/.test(line)) const match = line?.match(/\[ref=(\d+)\]/) if (!match) throw new Error(`No reference for ${name}: ${result.snapshot.outline}`) return Number(match[1]) @@ -168,6 +178,87 @@ test.describe('browser tools', () => { }, origin) } + test('sets and clears multiple selections without partial writes for invalid options', async () => { + const ref = await openForm() + const selected = await execute('browser_select_option', { + elementId: ref('Regions'), + values: ['A', 'B'], + }) + expect(selected.ok, selected.error).toBe(true) + expect(selected.result).toMatchObject({ + values: ['a', 'b'], + effectObserved: true, + readback: { values: ['a', 'b'] }, + }) + const invalid = await execute('browser_select_option', { + elementId: ref('Regions'), + values: ['B', 'C'], + }) + expect(invalid.ok).toBe(false) + const values = await app.evaluate(async ({ webContents }, origin) => { + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL() === `${origin}/form`) + if (!contents) throw new Error('Missing form fixture') + return contents.executeJavaScript( + 'Array.from(document.getElementById("regions").selectedOptions, option => option.value)' + ) + }, origin) + expect(values).toEqual(['a', 'b']) + const cleared = await execute('browser_select_option', { + elementId: ref('Regions'), + values: [], + }) + expect(cleared.result).toMatchObject({ + values: [], + effectObserved: true, + readback: { values: [] }, + }) + }) + + test('fills structured native fields and leaves invalid dates unchanged', async () => { + const ref = await openForm() + for (const [name, text] of [ + ['Date', '2026-09-15'], + ['Time', '15:48'], + ['Appointment', '2026-09-15T15:48:00'], + ['Month', '2026-09'], + ['Week', '2026-W38'], + ['Color', '#AABBCC'], + ['Range', '75'], + ]) { + const response = await execute('browser_type', { elementId: ref(name), text }) + expect(response.ok, response.error).toBe(true) + expect(response.result).toMatchObject({ + trusted: false, + dispatched: true, + effectObserved: true, + }) + } + const invalid = await execute('browser_type', { elementId: ref('Date'), text: '2026-02-30' }) + expect(invalid.ok).toBe(false) + expect(invalid.error).toContain('Invalid value') + const state = await app.evaluate(async ({ webContents }, origin) => { + const contents = webContents + .getAllWebContents() + .find((wc) => wc.getURL() === `${origin}/form`) + if (!contents) throw new Error('Missing form fixture') + return contents.executeJavaScript( + '({date:document.getElementById("date").value,time:document.getElementById("time").value,appointment:document.getElementById("appointment").value,month:document.getElementById("month").value,week:document.getElementById("week").value,color:document.getElementById("color").value,range:document.getElementById("range").value,events:document.getElementById("date").dataset.events})' + ) + }, origin) + expect(state).toEqual({ + date: '2026-09-15', + time: '15:48', + appointment: '2026-09-15T15:48', + month: '2026-09', + week: '2026-W38', + color: '#aabbcc', + range: '75', + events: '1', + }) + }) + for (const mode of ['menu', 'sticky']) { test(`clicks a ${mode} target without losing its identity`, async () => { const opened = await execute('browser_open_url', { url: `${origin}/click?mode=${mode}` }) diff --git a/apps/desktop/src/main/browser-agent/cdp.test.ts b/apps/desktop/src/main/browser-agent/cdp.test.ts index 80ce5c2c48a..9f7b32b6071 100644 --- a/apps/desktop/src/main/browser-agent/cdp.test.ts +++ b/apps/desktop/src/main/browser-agent/cdp.test.ts @@ -524,7 +524,7 @@ describe('browser-agent screenshot capture', () => { await captureScreenshot(contents) - expect(contents.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: false }) + expect(contents.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: true }) expect(contents.debugger.sendCommand).not.toHaveBeenCalledWith( 'Page.captureScreenshot', expect.anything() @@ -536,7 +536,7 @@ describe('browser-agent screenshot capture', () => { const shot = await captureScreenshot(contents, { x: 100, y: 50, width: 200, height: 100 }) - expect(contents.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: false }) + expect(contents.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: true }) expect(contents.debugger.sendCommand).not.toHaveBeenCalledWith( 'Page.captureScreenshot', expect.anything() diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index bbe8921ebb6..7c30dc6bd07 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -479,7 +479,7 @@ async function captureViewportImage( }) const capture = (async () => { try { - return await contents.capturePage(undefined, { stayHidden: false }) + return await contents.capturePage(undefined, { stayHidden: true }) } finally { pendingScreenshotCaptures.delete(contents) } diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index 4a011ac895c..285112fb61c 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -2145,6 +2145,50 @@ describe('credential protection', () => { return { contents, values, writes, dialogs, selectionReads: () => selectionReads } } + it.each([ + { values: ['a', 'b'], labels: ['A', 'B'], expected: true }, + { values: ['a'], labels: ['A'], expected: false }, + { values: ['a', 'b'], labels: ['A', 'Other'], expected: false }, + ])( + 'verifies the entire multiple selection %j', + async ({ values: readbackValues, labels, expected }) => { + const contents = await openPage() + respondWith(contents, { + selectOptionInElement: { + selected: 'A', + value: 'a', + values: ['a', 'b'], + labels: ['A', 'B'], + }, + readSelectElementState: { selected: 'A', value: 'a', values: readbackValues, labels }, + }) + const result = await driver.executeTool('chat-test', 'browser_select_option', { + elementId: 0, + values: ['a', 'b'], + }) + expect(result, JSON.stringify(result)).toMatchObject({ + ok: true, + result: { effectObserved: expected, readback: { values: readbackValues } }, + }) + } + ) + + it.each([ + { value: 'a', values: ['b'] }, + { values: [1] }, + { values: Array.from({ length: 101 }, () => 'a') }, + {}, + ])('rejects invalid selection arguments before dispatch', async (params) => { + const contents = await openPage() + vi.mocked(contents.executeJavaScript).mockClear() + const result = await driver.executeTool('chat-test', 'browser_select_option', { + elementId: 0, + ...params, + }) + expect(result.ok).toBe(false) + expect(contents.executeJavaScript).not.toHaveBeenCalled() + }) + const formFields = [ { elementId: 1, kind: 'select', value: 'first' }, { elementId: 2, kind: 'select', value: 'second' }, @@ -2475,6 +2519,97 @@ describe('credential protection', () => { expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(1) }) + it('sets structured input values without dispatching text or select-all keystrokes', async () => { + const contents = await openPage() + respondWith(contents, { + focusElementForTyping: { focused: true, kind: 'input', valueInput: true, x: 24, y: 48 }, + setFocusedInputValue: { dispatched: true }, + readActiveElementState: { activeElement: 'input', valueLength: 10 }, + readPageActionState: {}, + }) + const result = await driver.executeTool('chat-test', 'browser_type', { + elementId: 0, + text: '2026-09-15', + }) + expect(result).toMatchObject({ ok: true, result: { dispatched: true, trusted: false } }) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(0) + expect(cdpCalls(contents, 'Input.dispatchKeyEvent')).toHaveLength(0) + }) + + it('does not retry a rejected structured value through synthetic typing', async () => { + const contents = await openPage() + respondWith(contents, { + focusElementForTyping: { focused: true, kind: 'input', valueInput: true, x: 24, y: 48 }, + setFocusedInputValue: { error: 'Invalid value; the field was not changed.' }, + readActiveElementState: {}, + readPageActionState: {}, + }) + const result = await driver.executeTool('chat-test', 'browser_type', { + elementId: 0, + text: 'invalid-date', + }) + expect(result).toMatchObject({ ok: false, error: expect.stringContaining('Invalid value') }) + expect( + vi + .mocked(contents.executeJavaScript) + .mock.calls.filter(([expression]) => isPageCall(String(expression), 'typeIntoElement')) + ).toHaveLength(0) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(0) + }) + + it('reports an interrupted structured write as uncertain without replaying it', async () => { + const contents = await openPage() + let writes = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'focusElementForTyping')) + return Promise.resolve({ focused: true, valueInput: true, x: 24, y: 48 }) + if (isPageCall(expression, 'setFocusedInputValue')) { + writes++ + return Promise.reject(new Error('Execution context was destroyed')) + } + return Promise.resolve({}) + }) + const result = await driver.executeTool('chat-test', 'browser_type', { + elementId: 0, + text: '2026-09-15', + }) + expect(result).toMatchObject({ + ok: false, + error: expect.stringContaining('may have reached the field and was not retried'), + }) + expect(writes).toBe(1) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(0) + expect( + vi + .mocked(contents.executeJavaScript) + .mock.calls.some(([expression]) => isPageCall(String(expression), 'typeIntoElement')) + ).toBe(false) + }) + + it('refuses a field whose input mode changes before dispatch', async () => { + const contents = await openPage() + let reads = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'focusElementForTyping')) + return Promise.resolve({ focused: true, valueInput: ++reads === 1, x: 24, y: 48 }) + return Promise.resolve({}) + }) + const result = await driver.executeTool('chat-test', 'browser_type', { + elementId: 0, + text: '2026-09-15', + }) + expect(result).toMatchObject({ + ok: false, + error: expect.stringContaining('field type changed'), + }) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(0) + expect( + vi + .mocked(contents.executeJavaScript) + .mock.calls.some(([expression]) => isPageCall(String(expression), 'setFocusedInputValue')) + ).toBe(false) + }) + it('accepts empty text and sends it through native insertion to clear a field', async () => { const contents = await openPage() respondWith(contents, { diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 15af3363f71..94b53b9db52 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -66,6 +66,7 @@ import { readSelectElementState, scrollPage, selectOptionInElement, + setFocusedInputValue, typeIntoElement, } from '@/main/browser-agent/page-functions' import * as session from '@/main/browser-agent/session' @@ -1290,6 +1291,7 @@ function unwrapPageResult(result: unknown): unknown { `No option matched that label or value. Available options: ${options.join(', ')}` ) } + throw new ToolError(String(code)) } return result } @@ -3336,16 +3338,19 @@ async function executeToolInner( assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) } - let trusted = true + const valueInput = initialSurface.valueInput === true + let trusted = !valueInput let nativeInserted = false let nativeInsertAttempted = false try { assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) - await dispatchKeyCombo( - contents, - parseKeyCombo(process.platform === 'darwin' ? 'Cmd+A' : 'Control+A') - ) + if (!valueInput) { + await dispatchKeyCombo( + contents, + parseKeyCombo(process.platform === 'darwin' ? 'Cmd+A' : 'Control+A') + ) + } // The guard above vetted the element we asked to focus, but the insert // below goes wherever focus actually is now, a round trip later. Login // forms that auto-advance from username to password move it in exactly @@ -3398,8 +3403,32 @@ async function executeToolInner( } assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) + if ((finalSurface.valueInput === true) !== valueInput) { + throw new ToolError('The field type changed before input. Take a fresh browser_snapshot.') + } nativeInsertAttempted = true - await cdp.insertText(contents, text) + if (valueInput) { + const written = unwrapPageResult( + await execInPage( + target, + setFocusedInputValue, + [elementId, text], + false, + executionDeadline + ).catch((error) => { + throw new ToolError( + `The structured field write did not acknowledge completion (${getErrorMessage(error)}). It may have reached the field and was not retried; inspect the page before continuing.` + ) + }) + ) + if (!isRecordLike(written) || written.dispatched !== true) { + throw new ToolError( + 'The field did not acknowledge the value write. Inspect it before retrying.' + ) + } + } else { + await cdp.insertText(contents, text) + } nativeInserted = true let submitted = false @@ -3837,6 +3866,19 @@ async function executeToolInner( } case 'browser_select_option': { + const values = params.values + if (values !== undefined && params.value !== undefined) { + throw new ToolError('Provide value or values, not both.') + } + if ( + values !== undefined && + (!Array.isArray(values) || + values.length > 100 || + values.some((value) => typeof value !== 'string')) + ) { + throw new ToolError('values must be an array of at most 100 strings.') + } + const selection = values === undefined ? requireStr(params, 'value') : (values as string[]) const contents = session.requireAutomationTab().view.webContents const elementId = requireNum(params, 'elementId') const target = pageTargetForElement(contents, elementId) @@ -3872,7 +3914,7 @@ async function executeToolInner( await execInPage( target, selectOptionInElement, - [elementId, requireStr(params, 'value')], + [elementId, selection], false, executionDeadline ) @@ -3886,10 +3928,22 @@ async function executeToolInner( } await sleep(50) const state = unwrapPageResult(await execInPage(target, readSelectElementState, [elementId])) + const selectedValues = selected.values + const readbackValues = isRecordLike(state) ? state.values : undefined + const selectedLabels = selected.labels + const readbackLabels = isRecordLike(state) ? state.labels : undefined const effectObserved = isRecordLike(state) && selected.selected === state.selected && - selected.value === state.value + selected.value === state.value && + (!Array.isArray(selectedValues) || + (Array.isArray(readbackValues) && + selectedValues.length === readbackValues.length && + selectedValues.every((value, index) => value === readbackValues[index]) && + Array.isArray(selectedLabels) && + Array.isArray(readbackLabels) && + selectedLabels.length === readbackLabels.length && + selectedLabels.every((label, index) => label === readbackLabels[index]))) return { ...selected, effectObserved, diff --git a/apps/desktop/src/main/browser-agent/page-functions.test.ts b/apps/desktop/src/main/browser-agent/page-functions.test.ts index 544438dc3c1..c47aa774ae5 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.test.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.test.ts @@ -21,6 +21,7 @@ import { readSelectElementState, scrollPage, selectOptionInElement, + setFocusedInputValue, typeIntoElement, } from '@/main/browser-agent/page-functions' @@ -799,6 +800,55 @@ describe('collectSnapshot', () => { expect(clickElement(ref)).toEqual({ error: 'file-input' }) }) + it('sets a complete multiple selection atomically and can clear it', () => { + document.body.innerHTML = + '' + const select = document.querySelector('select') as HTMLSelectElement + register(select) + const events = vi.fn() + select.addEventListener('change', events) + expect(selectOptionInElement(0, ['B', 'D'])).toEqual({ error: 'disabled' }) + expect(readSelectElementState(0)).toMatchObject({ values: ['a'] }) + expect(events).not.toHaveBeenCalled() + expect(selectOptionInElement(0, ['C', 'missing'])).toMatchObject({ error: 'no-option' }) + expect(readSelectElementState(0)).toMatchObject({ values: ['a'] }) + expect(selectOptionInElement(0, ['C', 'B'])).toMatchObject({ values: ['b', 'c'] }) + expect(readSelectElementState(0)).toMatchObject({ values: ['b', 'c'] }) + expect(events).toHaveBeenCalledOnce() + expect(selectOptionInElement(0, [])).toMatchObject({ selected: '', value: '', values: [] }) + expect(readSelectElementState(0)).toMatchObject({ values: [] }) + }) + + it('captures requested labels before event handlers replace a duplicate-value option', () => { + document.body.innerHTML = + '' + const select = document.querySelector('select') as HTMLSelectElement + register(select) + select.addEventListener('change', () => { + select.options[1].selected = false + select.options[2].selected = true + select.options[1].label = 'Rewritten' + }) + expect(selectOptionInElement(0, ['Fixed', 'Wanted'])).toMatchObject({ + values: ['fixed', 'shared'], + labels: ['Fixed', 'Wanted'], + }) + expect(readSelectElementState(0)).toMatchObject({ + values: ['fixed', 'shared'], + labels: ['Fixed', 'Other'], + }) + }) + + it('does not use multiple-selection arguments on a single-selection dropdown', () => { + document.body.innerHTML = + '' + const select = document.querySelector('select') as HTMLSelectElement + register(select) + expect(selectOptionInElement(0, ['B'])).toHaveProperty('error') + expect(select.value).toBe('a') + expect(selectOptionInElement(0, 'B')).toMatchObject({ value: 'b' }) + }) + it('keeps plain visible leaf text available as an actionable ref', () => { document.body.innerHTML = '
announce
' visible(document.querySelector('span') as HTMLSpanElement) @@ -806,6 +856,64 @@ describe('collectSnapshot', () => { expect(outlineOf(collectSnapshot())).toContain('text "announce" [ref=') }) + it('preserves mixed inline text in reading order without duplicating control labels', () => { + document.body.innerHTML = + '
Type "hello" in upper case.
' + for (const el of document.querySelectorAll('body, div, strong, button, span, b')) visible(el) + const outline = outlineOf(collectSnapshot()) + const labels = Array.from(outline.matchAll(/- text ("(?:[^"\\]|\\.)*")/g), (match) => + JSON.parse(match[1]) + ) + expect(labels).toEqual(['Type "', 'hello', '" in upper case.']) + expect(outline).toContain('button "Save draft"') + expect(outline).not.toContain('Hidden') + }) + + it('does not emit stale textarea defaults after the current value changes', () => { + document.body.innerHTML = '' + const input = visible(document.querySelector('textarea') as HTMLTextAreaElement) + input.value = 'Current draft' + expect(outlineOf(collectSnapshot())).not.toContain('Old draft') + input.value = '' + expect(outlineOf(collectSnapshot())).not.toContain('Old draft') + }) + + it('preserves direct text in open shadow roots and respects hidden hosts', () => { + document.body.innerHTML = '
' + const host = visible(document.querySelector('div') as HTMLDivElement) + const shadow = host.attachShadow({ mode: 'open' }) + shadow.innerHTML = 'Before middle after' + visible(shadow.querySelector('strong') as HTMLElement) + const outline = outlineOf(collectSnapshot()) + expect(outline.indexOf('text "Before"')).toBeLessThan(outline.indexOf('text "middle"')) + expect(outline.indexOf('text "middle"')).toBeLessThan(outline.indexOf('text "after"')) + host.hidden = true + expect(outlineOf(collectSnapshot())).not.toContain('Before') + }) + + it('gives interactive headings actionable refs while preserving static headings', () => { + document.body.innerHTML = + '

Overview

' + for (const el of document.querySelectorAll('h3, h2')) visible(el) + const clicked = vi.fn() + document.querySelector('h3')?.addEventListener('click', clicked) + const outline = outlineOf(collectSnapshot()) + expect(outline).toContain('tab "Details"') + expect(outline).toContain('aria-expanded=false') + expect(outline).toContain('heading "Overview" (h2)') + expect(clickElement(refFor(outline, 'Details'))).toMatchObject({ dispatched: true }) + expect(clicked).toHaveBeenCalledOnce() + }) + + it('exposes structured input types and multiple-selection controls', () => { + document.body.innerHTML = + '' + for (const el of document.querySelectorAll('input, select')) visible(el) + const outline = outlineOf(collectSnapshot()) + expect(outline).toContain('type="date"') + expect(outline).toMatch(/combobox "Countries" \[ref=\d+\] multiple/) + }) + it('retains sender and timestamp text omitted from a row accessibility label', () => { document.body.innerHTML = `
@@ -2108,3 +2216,91 @@ describe('describeFocusedEditable', () => { expect(describeFocusedEditable()).toEqual({ editable: true, kind: 'canvas' }) }) }) + +describe('setFocusedInputValue', () => { + for (const [type, value] of [ + ['date', '2026-09-15'], + ['time', '15:48'], + ['datetime-local', '2026-09-15T15:48'], + ['month', '2026-09'], + ['week', '2026-W38'], + ['color', '#aabbcc'], + ['range', '42'], + ]) { + it(`sets a validated ${type} value through the native setter`, () => { + document.body.innerHTML = `` + const input = visible(document.querySelector('input') as HTMLInputElement) + register(input) + input.focus() + const events: string[] = [] + input.addEventListener('input', () => events.push('input')) + input.addEventListener('change', () => events.push('change')) + expect(focusElementForTyping(0)).toMatchObject({ valueInput: true }) + expect(runSerialized(setFocusedInputValue, [0, value])).toEqual({ dispatched: true }) + expect(input.value).toBe(value) + expect(events).toEqual(['input', 'change']) + }) + } + + it('accepts native datetime normalization and bypasses an overridden value setter', () => { + document.body.innerHTML = '' + const input = document.querySelector('input') as HTMLInputElement + register(input) + input.focus() + const setter = vi.fn() + Object.defineProperty(input, 'value', { + configurable: true, + get() { + return Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.get?.call(this) + }, + set: setter, + }) + expect(setFocusedInputValue(0, '2026-09-15T15:48:00')).toEqual({ dispatched: true }) + expect(input.value).toBe('2026-09-15T15:48') + expect(setter).not.toHaveBeenCalled() + }) + + it('does not write to a newly focused input inside a registered container', () => { + document.body.innerHTML = '
' + const container = visible(document.querySelector('div') as HTMLDivElement) + visible(document.querySelector('input') as HTMLInputElement) + register(container) + expect(focusElementForTyping(0)).toMatchObject({ valueInput: true }) + const other = document.createElement('input') + other.type = 'date' + container.append(other) + other.focus() + expect(setFocusedInputValue(0, '2026-09-15')).toHaveProperty('error') + expect(other.value).toBe('') + }) + + it('rejects malformed values before changing the field or emitting events', () => { + document.body.innerHTML = '' + const input = document.querySelector('input') as HTMLInputElement + register(input) + input.focus() + const changed = vi.fn() + input.addEventListener('input', changed) + expect(setFocusedInputValue(0, '2026-02-30')).toMatchObject({ + error: expect.stringContaining('Invalid value'), + }) + expect(input.value).toBe('2026-01-01') + expect(changed).not.toHaveBeenCalled() + }) + + it('refuses changed focus, readonly fields, and credential hints', () => { + document.body.innerHTML = '' + const [input, other] = Array.from(document.querySelectorAll('input')) + register(input) + other.focus() + expect(setFocusedInputValue(0, '2026-09-15')).toEqual({ error: 'different' }) + input.focus() + input.readOnly = true + expect(setFocusedInputValue(0, '2026-09-15')).toEqual({ error: 'readonly' }) + input.readOnly = false + input.autocomplete = 'current-password' + expect(setFocusedInputValue(0, '2026-09-15')).toEqual({ error: 'password' }) + expect(input.value).toBe('') + expect(other.value).toBe('') + }) +}) diff --git a/apps/desktop/src/main/browser-agent/page-functions.ts b/apps/desktop/src/main/browser-agent/page-functions.ts index 7d48440f182..d59ea3a05d2 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.ts @@ -480,6 +480,9 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn if (el.getAttribute('aria-required') === 'true') parts.push('aria-required') if (tag === 'INPUT') { const input = el as HTMLInputElement + if (!['text', 'checkbox', 'radio', 'submit', 'button', 'reset'].includes(input.type)) { + parts.push(`type=${quote(input.type)}`) + } if (input.type === 'checkbox' || input.type === 'radio') { parts.push(input.indeterminate ? 'mixed' : input.checked ? 'checked' : 'unchecked') } @@ -489,8 +492,9 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn const textarea = el as HTMLTextAreaElement if (textarea.readOnly) parts.push('readonly') if (textarea.required) parts.push('required') - } else if (tag === 'SELECT' && (el as HTMLSelectElement).required) { - parts.push('required') + } else if (tag === 'SELECT') { + if ((el as HTMLSelectElement).required) parts.push('required') + if ((el as HTMLSelectElement).multiple) parts.push('multiple') } for (const attribute of ['aria-checked', 'aria-expanded', 'aria-pressed', 'aria-selected']) { const value = el.getAttribute(attribute) @@ -562,24 +566,42 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn ) } - const walk = (elements: Iterable, depth: number, suppressTextCoveredBy = ''): void => { + const walk = (nodes: Iterable, depth: number, suppressTextCoveredBy = ''): void => { if (refCount >= refCap || depth > depthCap) { truncated = true return } - for (const el of elements) { + for (const node of nodes) { visitedNodes++ if (refCount >= refCap || visitedNodes > nodeCap) { truncated = true return } + const indent = ' '.repeat(depth) + if (node.nodeType === Node.TEXT_NODE) { + const root = node.getRootNode() + const parent = node.parentElement ?? ('host' in root ? (root.host as Element) : null) + if (parent?.tagName.toUpperCase() === 'TEXTAREA') continue + const text = cut((node.textContent || '').replace(/\s+/g, ' ').trim(), 160) + if ( + text && + parent && + isVisible(parent) && + (!suppressTextCoveredBy || !suppressTextCoveredBy.includes(text)) + ) { + if (!push(`${indent}- text ${quote(text)}`)) return + } + continue + } + if (node.nodeType !== Node.ELEMENT_NODE) continue + const el = node as Element const tag = String(el.tagName || '').toUpperCase() if (tag === 'SCRIPT' || tag === 'STYLE' || tag === 'NOSCRIPT' || tag === 'TEMPLATE') continue - const indent = ' '.repeat(depth) let childDepth = depth let emittedInteractive = false let interactiveName = '' + let emittedText = '' const visible = isVisible(el) if (el.matches(landmarkSelector) && visible) { @@ -587,15 +609,15 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn childDepth = depth + 1 } else { const level = headingLevel(el) - if (level !== null && visible) { - const text = cut(((el as HTMLElement).innerText || '').replace(/\s+/g, ' ').trim(), 160) - if (text) push(`${indent}- heading ${quote(text)} (h${level})`) - } else if (visible && (el.matches(interactiveSelector) || pointerBoundary(el))) { + if (visible && (el.matches(interactiveSelector) || pointerBoundary(el))) { emitInteractive(el, indent) emittedInteractive = true interactiveName = nameFor(el) // Interactive containers rarely nest other interactives; still // recurse so e.g. a clickable card exposes its inner links. + } else if (level !== null && visible) { + emittedText = cut(((el as HTMLElement).innerText || '').replace(/\s+/g, ' ').trim(), 160) + if (emittedText) push(`${indent}- heading ${quote(emittedText)} (h${level})`) } else if (visible) { const visibleElementChild = Array.from(el.children).some(isVisible) const leafLabel = visibleElementChild @@ -609,18 +631,21 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn (!suppressTextCoveredBy || !suppressTextCoveredBy.includes(leafLabel)) ) { emitTextLeaf(el, indent, leafLabel) + emittedText = leafLabel } } } - const coveredText = emittedInteractive ? interactiveName : suppressTextCoveredBy + const coveredText = emittedInteractive + ? interactiveName + : emittedText || suppressTextCoveredBy if (tag === 'IFRAME' || tag === 'FRAME') { try { const innerDoc = (el as HTMLIFrameElement).contentDocument if (innerDoc?.body && isVisible(el)) { if (!push(`${indent}- iframe:`)) return - walk(innerDoc.body.children, childDepth + 1, coveredText) + walk(innerDoc.body.childNodes, childDepth + 1, coveredText) } else if (scopedRoot && !innerDoc && visible) { truncated = true } @@ -631,13 +656,13 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn } const shadow = (el as HTMLElement).shadowRoot - if (shadow) walk(shadow.children, childDepth, coveredText) - walk(el.children, childDepth, coveredText) + if (shadow) walk(shadow.childNodes, childDepth, coveredText) + walk(el.childNodes, childDepth, coveredText) } } if (scopedRoot) walk([scopedRoot], 0) - else if (document.body) walk(document.body.children, 0) + else if (document.body) walk(document.body.childNodes, 0) /** * React commonly replaces a control's DOM node while preserving its @@ -1354,6 +1379,7 @@ export function focusElementForTyping(id: number, moveFocus = true): unknown { .some((token) => token === 'current-password' || token === 'new-password') } + const valueInputTypes = ['date', 'time', 'datetime-local', 'month', 'week', 'color', 'range'] const resolver = window.__simAgentResolveElement const resolved = resolver?.(id) const el = resolver ? resolved?.element : (window.__simAgentElements || [])[id] @@ -1366,7 +1392,7 @@ export function focusElementForTyping(id: number, moveFocus = true): unknown { if (field.readOnly || field.getAttribute('aria-readonly') === 'true') return 'readonly' if (String(field.tagName || '').toUpperCase() === 'TEXTAREA') return 'writable' const type = String((field as HTMLInputElement).type || 'text').toLowerCase() - return ['text', 'search', 'email', 'url', 'tel', 'number'].includes(type) + return ['text', 'search', 'email', 'url', 'tel', 'number', ...valueInputTypes].includes(type) ? 'writable' : 'not-editable' } @@ -1380,7 +1406,16 @@ export function focusElementForTyping(id: number, moveFocus = true): unknown { if ( tag === 'TEXTAREA' || (tag === 'INPUT' && - ['text', 'search', 'email', 'url', 'tel', 'number', 'password'].includes(inputType)) || + [ + 'text', + 'search', + 'email', + 'url', + 'tel', + 'number', + 'password', + ...valueInputTypes, + ].includes(inputType)) || (node as HTMLElement).isContentEditable || // An ARIA-only textbox. The snapshot already advertises these as // `[textbox]` with a ref, and browser_insert_text accepts them, so @@ -1642,10 +1677,67 @@ export function focusElementForTyping(id: number, moveFocus = true): unknown { x: chosenPoint.x, y: chosenPoint.y, coveredByRelatedPopup, + valueInput: + editableTag === 'INPUT' && valueInputTypes.includes((editable as HTMLInputElement).type), refRecovered: resolved?.recovered === true, } } +/** Sets structured native inputs after the driver's ordinary typing actionability checks. */ +export function setFocusedInputValue(id: number, text: string): unknown { + const resolver = window.__simAgentResolveElement + const resolved = resolver?.(id) + const registered = resolver ? resolved?.element : (window.__simAgentElements || [])[id] + if (!registered?.isConnected) return { error: 'stale', reason: window.__simAgentStaleReason } + let active = registered.ownerDocument.activeElement + while (active?.shadowRoot?.activeElement) active = active.shadowRoot.activeElement + if (String(registered.tagName || '').toUpperCase() !== 'INPUT') { + return { + error: + 'Structured inputs require the field reference itself, not a container. Take a fresh browser_snapshot.', + } + } + if (active !== registered) return { error: 'different' } + const input = active as HTMLInputElement + const type = input.type.toLowerCase() + const hints = (input.getAttribute('autocomplete') || '').toLowerCase().split(/\s+/) + if ( + type === 'password' || + hints.some((hint) => hint === 'current-password' || hint === 'new-password') + ) { + return { error: 'password' } + } + if (!['date', 'time', 'datetime-local', 'month', 'week', 'color', 'range'].includes(type)) { + return { + error: + 'The focused field no longer accepts a structured input value. Take a fresh browser_snapshot.', + } + } + if (input.matches(':disabled') || input.getAttribute('aria-disabled') === 'true') + return { error: 'disabled' } + if (input.readOnly || input.getAttribute('aria-readonly') === 'true') return { error: 'readonly' } + const value = type === 'color' ? text.trim().toLowerCase() : text.trim() + const probe = input.cloneNode(false) as HTMLInputElement + probe.value = value + if ( + (value !== '' && probe.value === '') || + (['color', 'range'].includes(type) && probe.value !== value) + ) { + return { + error: `Invalid value for input[type=${type}]. Use the native format; the field was not changed.`, + } + } + const view = input.ownerDocument.defaultView + if (!view) return { error: 'stale' } + const setter = Object.getOwnPropertyDescriptor(view.HTMLInputElement.prototype, 'value')?.set + if (!setter) + return { error: 'The native input value setter is unavailable; the field was not changed.' } + setter.call(input, probe.value) + input.dispatchEvent(new view.Event('input', { bubbles: true, composed: true })) + input.dispatchEvent(new view.Event('change', { bubbles: true })) + return { dispatched: true } +} + /** * Reads back the focused element's state after a native key/type action so * the driver can report what actually happened instead of assuming success. @@ -2671,42 +2763,74 @@ export function scrollPage(direction: string, amount?: number, elementId?: numbe } } -export function selectOptionInElement(id: number, value: string): unknown { +export function selectOptionInElement(id: number, value: string | string[]): unknown { const resolver = window.__simAgentResolveElement const resolved = resolver?.(id) const el = resolver ? resolved?.element : (window.__simAgentElements || [])[id] if (!el || !el.isConnected) return { error: 'stale', reason: window.__simAgentStaleReason } if (String(el.tagName || '').toUpperCase() !== 'SELECT') return { error: 'not-select' } const select = el as HTMLSelectElement - if (select.disabled || select.getAttribute('aria-disabled') === 'true') { + if (select.matches(':disabled') || select.getAttribute('aria-disabled') === 'true') { return { error: 'disabled' } } - const wanted = value.trim().toLowerCase() - const option = Array.from(select.options).find( - (o) => o.value.trim().toLowerCase() === wanted || o.label.trim().toLowerCase() === wanted - ) - if (!option) { + if (Array.isArray(value) && !select.multiple) { return { - error: 'no-option', - options: Array.from(select.options) - .slice(0, 50) - .map((o) => - o.label + error: + 'Use value for a single-selection dropdown; values requires a multiple-selection control.', + } + } + const requested = Array.isArray(value) ? value : [value] + if (requested.length > 100 || requested.some((entry) => typeof entry !== 'string')) { + return { error: 'A selection requires at most 100 string values.' } + } + const options = Array.from(select.options) + const chosen = new Set() + for (const entry of requested) { + const wanted = entry.trim().toLowerCase() + const option = options.find( + (candidate) => + candidate.value.trim().toLowerCase() === wanted || + candidate.label.trim().toLowerCase() === wanted + ) + if (!option) { + return { + error: 'no-option', + options: options.slice(0, 50).map((candidate) => + candidate.label .trim() .slice(0, 200) .replace(/[\uD800-\uDBFF]$/, '') ), + } } + if ( + option.disabled || + (option.parentElement as HTMLOptGroupElement | null)?.disabled === true + ) { + return { error: 'disabled' } + } + chosen.add(option) } - if (option.disabled || (option.parentElement as HTMLOptGroupElement | null)?.disabled === true) { - return { error: 'disabled' } + const selected = options.filter((option) => chosen.has(option)) + const selection = { + selected: selected[0]?.label.trim() || '', + value: selected[0]?.value || '', + ...(select.multiple + ? { + values: selected.map((option) => option.value), + labels: selected.map((option) => option.label.trim()), + } + : {}), + } + if (select.multiple) { + for (const option of options) option.selected = chosen.has(option) + } else { + select.value = selected[0].value } - select.value = option.value select.dispatchEvent(new Event('input', { bubbles: true })) select.dispatchEvent(new Event('change', { bubbles: true })) return { - selected: option.label.trim(), - value: option.value, + ...selection, refRecovered: resolved?.recovered === true, } } @@ -2818,9 +2942,19 @@ export function readSelectElementState(id: number): unknown { if (!el || !el.isConnected) return { error: 'stale', reason: window.__simAgentStaleReason } if (String(el.tagName || '').toUpperCase() !== 'SELECT') return { error: 'not-select' } const select = el as HTMLSelectElement + const values: string[] = [] + const labels: string[] = [] + if (select.multiple) { + for (const option of select.selectedOptions) { + values.push(option.value) + labels.push(option.label.trim()) + if (values.length > 100) break + } + } return { selected: select.selectedOptions[0]?.label.trim() || '', value: select.value, + ...(select.multiple ? { values, labels } : {}), } } diff --git a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts index dbf390a7c80..d82e0066cb7 100644 --- a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts @@ -1626,16 +1626,27 @@ export const BrowserSelectOption: ToolCatalogEntry = { route: 'client', mode: 'async', parameters: { - type: 'object', + oneOf: [{ required: ['value'] }, { required: ['values'] }], properties: { elementId: { - type: 'number', description: "The element id to act on (from the current tab's most recent browser_snapshot). Treat refs as invalid across tab switches or later snapshots.", + type: 'number', + }, + value: { + description: "One option's visible label or value. Omit when supplying values.", + type: 'string', + }, + values: { + description: + 'The complete desired selection for a native multiple-selection control: at most 100 visible labels or values. Empty array clears the selection. Omit value when using this field.', + items: { type: 'string' }, + maxItems: 100, + type: 'array', }, - value: { type: 'string', description: "The option's visible label or its value." }, }, - required: ['elementId', 'value'], + required: ['elementId'], + type: 'object', }, resultSchema: { type: 'object', @@ -1644,6 +1655,12 @@ export const BrowserSelectOption: ToolCatalogEntry = { type: 'boolean', description: 'Whether the settled readback retained the requested selection.', }, + labels: { + type: 'array', + description: + 'Visible labels for the complete selected set in a multiple-selection control, in option order.', + items: { type: 'string' }, + }, note: { type: 'string', description: 'Guidance when the page reverted the selection.' }, notices: { type: 'array', @@ -1655,8 +1672,20 @@ export const BrowserSelectOption: ToolCatalogEntry = { type: 'object', description: 'Settled selected label and value.', properties: { + labels: { + type: 'array', + description: + 'Visible labels for the complete selected set in a multiple-selection control, in option order.', + items: { type: 'string' }, + }, selected: { type: 'string', description: 'Settled visible option label.' }, value: { type: 'string', description: 'Settled option value.' }, + values: { + type: 'array', + description: + 'Selected native option values in DOM order; included for multiple-selection controls.', + items: { type: 'string' }, + }, }, }, refRecovered: { @@ -1666,6 +1695,12 @@ export const BrowserSelectOption: ToolCatalogEntry = { }, selected: { type: 'string', description: 'Canonical visible label of the matched option.' }, value: { type: 'string', description: 'Canonical value of the matched option.' }, + values: { + type: 'array', + description: + 'Selected native option values in DOM order; included for multiple-selection controls.', + items: { type: 'string' }, + }, }, required: ['selected'], }, @@ -1818,7 +1853,7 @@ export const BrowserType: ToolCatalogEntry = { text: { type: 'string', description: - "The text to type. Replaces the element's current content. Must be non-empty — an empty string is rejected as a missing parameter; to clear a field, press Mod+A then Backspace with browser_press_key.", + 'The replacement value. Empty text clears an ordinary text field. For structured inputs use YYYY-MM-DD (date), HH:mm (time), YYYY-MM-DDTHH:mm (datetime-local), YYYY-MM (month), YYYY-Www (week), #rrggbb (color), or a numeric range value. Alternatively use Mod+A then Backspace to clear ordinary text with browser_press_key.', }, }, required: ['elementId', 'text'], diff --git a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts index cb977c36ed5..a1b5c10ecb9 100644 --- a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts @@ -1540,19 +1540,36 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, browser_select_option: { parameters: { - type: 'object', + oneOf: [ + { + required: ['value'], + }, + { + required: ['values'], + }, + ], properties: { elementId: { - type: 'number', description: "The element id to act on (from the current tab's most recent browser_snapshot). Treat refs as invalid across tab switches or later snapshots.", + type: 'number', }, value: { + description: "One option's visible label or value. Omit when supplying values.", type: 'string', - description: "The option's visible label or its value.", + }, + values: { + description: + 'The complete desired selection for a native multiple-selection control: at most 100 visible labels or values. Empty array clears the selection. Omit value when using this field.', + items: { + type: 'string', + }, + maxItems: 100, + type: 'array', }, }, - required: ['elementId', 'value'], + required: ['elementId'], + type: 'object', }, resultSchema: { type: 'object', @@ -1561,6 +1578,14 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { type: 'boolean', description: 'Whether the settled readback retained the requested selection.', }, + labels: { + type: 'array', + description: + 'Visible labels for the complete selected set in a multiple-selection control, in option order.', + items: { + type: 'string', + }, + }, note: { type: 'string', description: 'Guidance when the page reverted the selection.', @@ -1577,6 +1602,14 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { type: 'object', description: 'Settled selected label and value.', properties: { + labels: { + type: 'array', + description: + 'Visible labels for the complete selected set in a multiple-selection control, in option order.', + items: { + type: 'string', + }, + }, selected: { type: 'string', description: 'Settled visible option label.', @@ -1585,6 +1618,14 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { type: 'string', description: 'Settled option value.', }, + values: { + type: 'array', + description: + 'Selected native option values in DOM order; included for multiple-selection controls.', + items: { + type: 'string', + }, + }, }, }, refRecovered: { @@ -1600,6 +1641,14 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { type: 'string', description: 'Canonical value of the matched option.', }, + values: { + type: 'array', + description: + 'Selected native option values in DOM order; included for multiple-selection controls.', + items: { + type: 'string', + }, + }, }, required: ['selected'], }, @@ -1769,7 +1818,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { text: { type: 'string', description: - "The text to type. Replaces the element's current content. Must be non-empty — an empty string is rejected as a missing parameter; to clear a field, press Mod+A then Backspace with browser_press_key.", + 'The replacement value. Empty text clears an ordinary text field. For structured inputs use YYYY-MM-DD (date), HH:mm (time), YYYY-MM-DDTHH:mm (datetime-local), YYYY-MM (month), YYYY-Www (week), #rrggbb (color), or a numeric range value. Alternatively use Mod+A then Backspace to clear ordinary text with browser_press_key.', }, }, required: ['elementId', 'text'], diff --git a/apps/sim/lib/copilot/tools/server/generated-schema.test.ts b/apps/sim/lib/copilot/tools/server/generated-schema.test.ts index 1df1ba9e75a..3dc3af8aabb 100644 --- a/apps/sim/lib/copilot/tools/server/generated-schema.test.ts +++ b/apps/sim/lib/copilot/tools/server/generated-schema.test.ts @@ -5,6 +5,30 @@ import { describe, expect, it } from 'vitest' import { validateGeneratedToolPayload } from '@/lib/copilot/tools/server/generated-schema' import { OrchestrationError } from '@/lib/core/orchestration/types' +describe('validateGeneratedToolPayload browser_select_option parameters', () => { + it.each([ + { elementId: 0, value: 'a' }, + { elementId: 0, values: ['a', 'b'] }, + { elementId: 0, values: [] }, + ])('accepts a single selection mode %#', (payload) => { + expect(validateGeneratedToolPayload('browser_select_option', 'parameters', payload)).toBe( + payload + ) + }) + + it.each([ + { elementId: 0 }, + { elementId: 0, value: 'a', values: ['b'] }, + { elementId: 0, value: 'a', values: [] }, + { elementId: 0, values: [1] }, + { elementId: 0, values: Array.from({ length: 101 }, () => 'a') }, + ])('rejects missing, conflicting or malformed selection arguments %#', (payload) => { + expect(() => + validateGeneratedToolPayload('browser_select_option', 'parameters', payload) + ).toThrow(OrchestrationError) + }) +}) + describe('validateGeneratedToolPayload browser_fill_form parameters', () => { it('accepts mixed fields, including empty text and false checked state', () => { const payload = { From c227e6b9d7f41a017009cf1358d9bc0ef65fae08 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 15 Sep 2026 18:07:25 -0700 Subject: [PATCH 3/3] fix(browser): share snapshot text budget with inline fragments --- .../main/browser-agent/page-functions.test.ts | 17 +++++++++++++++++ .../src/main/browser-agent/page-functions.ts | 13 +++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/main/browser-agent/page-functions.test.ts b/apps/desktop/src/main/browser-agent/page-functions.test.ts index c47aa774ae5..d7226734454 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.test.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.test.ts @@ -759,6 +759,23 @@ describe('collectSnapshot', () => { expect(lines[0]).not.toContain('[ref=999]') }) + it('shares the text budget across inline fragments and leaves room for later controls', () => { + document.body.innerHTML = `${Array.from( + { length: 650 }, + (_, index) => `

Before ${index} inline ${index} after ${index}

` + ).join( + '' + )}${Array.from({ length: 100 }, (_, index) => ``).join('')}` + for (const element of document.querySelectorAll('*')) visible(element) + + const snapshot = collectSnapshot() as { outline: string; truncated: boolean } + expect(snapshot.truncated).toBe(true) + expect(snapshot.outline.match(/^- text /gm)).toHaveLength(120) + expect(snapshot.outline.match(/^- button /gm)).toHaveLength(100) + expect(snapshot.outline).toMatch(/button "Action 99" \[ref=\d+\]/) + expect(snapshot.outline).toMatch(/textbox "Final field" \[ref=\d+\]/) + }) + it('indexes only refs that were emitted before snapshot line truncation', () => { document.body.innerHTML = `${Array.from( { length: 599 }, diff --git a/apps/desktop/src/main/browser-agent/page-functions.ts b/apps/desktop/src/main/browser-agent/page-functions.ts index d59ea3a05d2..f2e4ff93a4e 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.ts @@ -163,8 +163,8 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn const lines: string[] = [] let truncated = false let refCount = 0 - let textRefCount = 0 - const textRefCap = 120 + let textLineCount = 0 + const textLineCap = 120 let visitedNodes = 0 const previousElementId = window.__simAgentNextElementId const safePreviousElementId = @@ -510,7 +510,7 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn } const emitTextLeaf = (el: Element, indent: string, renderedLabel?: string): void => { - if (refCount >= refCap || textRefCount >= textRefCap || lines.length >= lineCap) { + if (refCount >= refCap || textLineCount >= textLineCap || lines.length >= lineCap) { truncated = true return } @@ -522,7 +522,7 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn ) if (!text) return const id = registerElement(el, roleFor(el), text) - textRefCount++ + textLineCount++ const lineIndex = lines.length if (push(`${indent}- text ${quote(text)} [ref=${id}]`)) refLineIndexes[id] = lineIndex } @@ -589,7 +589,12 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn isVisible(parent) && (!suppressTextCoveredBy || !suppressTextCoveredBy.includes(text)) ) { + if (textLineCount >= textLineCap) { + truncated = true + continue + } if (!push(`${indent}- text ${quote(text)}`)) return + textLineCount++ } continue }