Skip to content

Commit 40f4a8e

Browse files
authored
feat(browser): sharper computer use for the desktop browser agent (#8283)
* fix(browser): press chord modifiers as real keys A chord such as Control+Shift+Y was sent as one key-down with a modifier bitmask, so pages that track held keys never saw Control or Shift go down. Modifiers are now pressed in order before the main key and released in reverse, as a physical keyboard delivers them, and a bare modifier key can be pressed on its own. * feat(browser): let clicks press and hold Clicks released the button immediately, so press-and-hold controls could not be operated. A click now accepts holdMs (0 to 10000, single clicks only) and keeps the button down that long before release. * fix(browser): read disablePortal modals hidden with their app root MUI hides every <body> child except a modal's mount node; a disablePortal modal mounts inside the app root it just hid, so every node had an aria-hidden ancestor and the snapshot came back empty while the dialog was on screen. Visibility checks now skip aria-hidden only on ancestors above the single topmost modal hidden that way; portaled modals and other hidden regions are unchanged. * fix(browser): stage agent downloads synchronously so Electron never opens a Save dialog The save path was set only after an asynchronous non-conflicting-name lookup. When Electron's own path step won that race it opened a native Save dialog, so an agent download stalled forever at full size (and a user would see a surprise dialog). Downloads now write to a hidden staging file set during will-download and are renamed to the allocated name when they complete. * feat(browser): mark snapshot elements that appeared since the previous snapshot After an action the agent saw a fresh snapshot but no hint of what changed, so a popup, suggestion list, or validation message looked like any other line. The page now remembers which elements an earlier snapshot of the document listed, and later snapshots mark the rest new (the first snapshot marks nothing). * refactor(browser): tighten the computer-use desktop changes - Modifier aliases (Ctrl, Cmd, Command, Option) share one descriptor with their canonical key, so a bare alias sets its own flag; a bare modifier's key-up reports it released. - The framed-control fallback no longer treats a held click as a plain click, and a batch refuses press-and-hold so eight holds cannot outlast its watchdog. - installPageHelpers caches the modal lookup per invocation, and one serializePageCall builds the page expression for the driver and the tests. - Download completion drops a branch whose state was never published. * fix(browser): close the edge cases review found in the desktop agent changes - A held right-click renews its agent context-menu marker before release, so Windows' release-time menu stays suppressed after a hold over one second. - A failed chord releases only the keys whose press was attempted. - A disk probe that resolves after a download completed can no longer cancel it. - A disablePortal modal nested inside another open modal is picked as topmost. - holdMs is an integer bounded to 0-10000 in the tool contract. * fix(browser): harden holds, modal exemption, and download moves - An aborted press-and-hold rejects and releases the button at once instead of staying held into the next action. - The disablePortal exemption applies only when a <body> child hides the modal (MUI's mechanism), so a dialog the app hid itself stays hidden, and it is looked up per document so same-origin iframes get it too. - A completed download's move retries transient EBUSY/EPERM/EACCES errors, as Chromium's own final rename does, instead of discarding the file. - Keyboard docs describe the separate modifier presses; the partial-release test covers a failure at the first modifier. * fix(browser): claim agent download names on disk while bytes stage The allocated destination gets an empty placeholder created with O_EXCL, as Firefox does, so another program picking a name sees it taken and the final rename only ever replaces Sim's own placeholder. A name that something else grabbed first stops the download instead of being overwritten. Teardown and failures remove the placeholder once, so a late-settling download cannot delete a name a newer download has claimed. * fix(browser): close cancellation and teardown races in holds and downloads - An already-aborted click presses nothing, and a held click marks its outcome pending before dispatch, so cancelling mid-hold reports an unknown outcome with doNotRetry instead of an error that invites a retry. - The disablePortal exemption resets when a visibility walk crosses from an iframe into its host page, where the host's own aria-hidden applies. - Teardown keeps a download's name reserved until its placeholder claim settles, so a newer download cannot be handed a name the claim then takes. - Downloads are paused before the staging path is set, as on staging, so a pause failure leaves no staging file behind.
1 parent 0be1f38 commit 40f4a8e

13 files changed

Lines changed: 1290 additions & 138 deletions

File tree

‎apps/desktop/src/main/browser-agent/cdp.test.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { getErrorMessage } from '@sim/utils/errors'
2+
import { toRecord } from '@sim/utils/object'
23
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
34

45
vi.mock('electron', () => import('@/test/electron-mock'))
@@ -13,9 +14,11 @@ import {
1314
import {
1415
captureScreenshot,
1516
clickAt,
17+
consumeAgentContextMenu,
1618
ensureInstrumented,
1719
evaluateInIsolatedFrame,
1820
insertText,
21+
PRIMARY_CLICK,
1922
releaseFileInput,
2023
resolveFileInput,
2124
setColorScheme,
@@ -255,6 +258,77 @@ describe('browser-agent CDP instrumentation', () => {
255258
])
256259
})
257260

261+
it('holds the button down for holdMs before releasing it', async () => {
262+
const contents = new WebContentsView().webContents
263+
const types = () =>
264+
vi.mocked(contents.debugger.sendCommand).mock.calls.map(([, params]) => toRecord(params).type)
265+
vi.useFakeTimers()
266+
try {
267+
const click = clickAt(contents, 5, 6, false, { ...PRIMARY_CLICK, holdMs: 1500 })
268+
await vi.advanceTimersByTimeAsync(1000)
269+
expect(types()).toEqual(['mousePressed'])
270+
271+
await vi.advanceTimersByTimeAsync(500)
272+
await click
273+
expect(types()).toEqual(['mousePressed', 'mouseReleased'])
274+
} finally {
275+
vi.useRealTimers()
276+
}
277+
})
278+
279+
it('presses nothing when its click was aborted before dispatch', async () => {
280+
const contents = new WebContentsView().webContents
281+
const controller = new AbortController()
282+
controller.abort()
283+
284+
await expect(
285+
clickAt(contents, 5, 6, false, PRIMARY_CLICK, controller.signal)
286+
).rejects.toMatchObject({ name: 'AbortError' })
287+
expect(contents.debugger.sendCommand).not.toHaveBeenCalled()
288+
})
289+
290+
it('releases a held button as soon as its click is aborted', async () => {
291+
const contents = new WebContentsView().webContents
292+
const types = () =>
293+
vi.mocked(contents.debugger.sendCommand).mock.calls.map(([, params]) => toRecord(params).type)
294+
vi.useFakeTimers()
295+
try {
296+
const controller = new AbortController()
297+
const hold = { ...PRIMARY_CLICK, holdMs: 10_000 }
298+
const click = clickAt(contents, 5, 6, false, hold, controller.signal)
299+
await vi.advanceTimersByTimeAsync(100)
300+
expect(types()).toEqual(['mousePressed'])
301+
302+
controller.abort()
303+
await expect(click).rejects.toMatchObject({ name: 'AbortError' })
304+
expect(types()).toEqual(['mousePressed', 'mouseReleased'])
305+
} finally {
306+
vi.useRealTimers()
307+
}
308+
})
309+
310+
it('keeps a held right-click marked as the agent context menu until release', async () => {
311+
const contents = new WebContentsView().webContents
312+
vi.useFakeTimers()
313+
try {
314+
const rightHold = { ...PRIMARY_CLICK, button: 'right' as const, holdMs: 1500 }
315+
await Promise.all([
316+
clickAt(contents, 5, 6, false, rightHold),
317+
vi.advanceTimersByTimeAsync(1500),
318+
])
319+
expect(consumeAgentContextMenu(contents)).toBe(true)
320+
321+
const click = clickAt(contents, 5, 6, false, rightHold)
322+
await vi.advanceTimersByTimeAsync(0)
323+
expect(consumeAgentContextMenu(contents)).toBe(true)
324+
await vi.advanceTimersByTimeAsync(1500)
325+
await click
326+
expect(consumeAgentContextMenu(contents)).toBe(false)
327+
} finally {
328+
vi.useRealTimers()
329+
}
330+
})
331+
258332
it('releases the mouse after a partial click failure', async () => {
259333
const contents = new WebContentsView().webContents
260334
vi.mocked(contents.debugger.sendCommand)

‎apps/desktop/src/main/browser-agent/cdp.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import type { BrowserTheme } from '@sim/browser-protocol'
1212
import { createLogger } from '@sim/logger'
1313
import { getErrorMessage } from '@sim/utils/errors'
14-
import { sleep } from '@sim/utils/helpers'
14+
import { interruptibleSleep, sleep } from '@sim/utils/helpers'
1515
import { isRecordLike } from '@sim/utils/object'
1616
import type { NativeImage, WebContents, WebFrameMain } from 'electron'
1717

@@ -954,9 +954,16 @@ export interface PointerClick {
954954
clickCount: 1 | 2 | 3
955955
/** CDP modifier bitmask (Alt=1, Ctrl=2, Meta=4, Shift=8). */
956956
modifiers: number
957+
/** How long the button stays down before release; press-and-hold controls need it. */
958+
holdMs: number
957959
}
958960

959-
export const PRIMARY_CLICK: PointerClick = { button: 'left', clickCount: 1, modifiers: 0 }
961+
export const PRIMARY_CLICK: PointerClick = {
962+
button: 'left',
963+
clickCount: 1,
964+
modifiers: 0,
965+
holdMs: 0,
966+
}
960967

961968
const BUTTON_MASKS: Record<PointerClick['button'], number> = { left: 1, right: 2, middle: 4 }
962969
const agentContextClicks = new WeakMap<WebContents, number>()
@@ -976,15 +983,23 @@ export function clearAgentContextMenu(contents: WebContents): void {
976983
agentContextClicks.delete(contents)
977984
}
978985

986+
/**
987+
* Clicks at viewport coordinates. An already-aborted `signal` rejects before anything is pressed.
988+
* During a press-and-hold it ends the hold early: the click rejects with the abort reason and the
989+
* button is released at once, so a cancelled or timed-out click cannot stay held into the next
990+
* action. That release can still activate the control under the pointer.
991+
*/
979992
export async function clickAt(
980993
contents: WebContents,
981994
x: number,
982995
y: number,
983996
moveBeforePress = true,
984-
click: PointerClick = PRIMARY_CLICK
997+
click: PointerClick = PRIMARY_CLICK,
998+
signal?: AbortSignal
985999
): Promise<void> {
9861000
if (moveBeforePress) await moveMouse(contents, x, y)
987-
const { button, clickCount, modifiers } = click
1001+
signal?.throwIfAborted()
1002+
const { button, clickCount, modifiers, holdMs } = click
9881003
const buttons = BUTTON_MASKS[button]
9891004
let pressed = false
9901005
try {
@@ -1005,6 +1020,15 @@ export async function clickAt(
10051020
modifiers,
10061021
clickCount: count,
10071022
})
1023+
if (holdMs > 0) {
1024+
await interruptibleSleep(holdMs, signal)
1025+
// Windows opens the context menu on release, after the hold; renew a marker a
1026+
// press-time menu has not already consumed.
1027+
if (button === 'right' && agentContextClicks.has(contents)) {
1028+
agentContextClicks.set(contents, Date.now())
1029+
}
1030+
signal?.throwIfAborted()
1031+
}
10081032
await sendInput(contents, 'Input.dispatchMouseEvent', {
10091033
type: 'mouseReleased',
10101034
x,

‎apps/desktop/src/main/browser-agent/context-menu.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { describe, expect, it, vi } from 'vitest'
44
vi.mock('electron', () => import('@/test/electron-mock'))
55

66
import { Menu, WebContentsView } from 'electron'
7-
import { clickAt } from '@/main/browser-agent/cdp'
7+
import { clickAt, PRIMARY_CLICK } from '@/main/browser-agent/cdp'
88
import {
99
attachAgentContextMenu,
1010
BASE_ZOOM_FACTOR,
@@ -228,7 +228,7 @@ describe('attachAgentContextMenu', () => {
228228
ContextMenuListener,
229229
][]
230230
const onContextMenu = listeners.find(([event]) => event === 'context-menu')![1]
231-
await clickAt(contents, 10, 20, false, { button: 'right', clickCount: 1, modifiers: 0 })
231+
await clickAt(contents, 10, 20, false, { ...PRIMARY_CLICK, button: 'right' })
232232
vi.mocked(Menu.buildFromTemplate).mockClear()
233233

234234
onContextMenu({}, params())
@@ -252,7 +252,7 @@ describe('attachAgentContextMenu', () => {
252252
][]
253253
const onInput = listeners.find(([event]) => event === 'input-event')?.[1]
254254
const onContextMenu = listeners.find(([event]) => event === 'context-menu')![1]
255-
await clickAt(contents, 10, 20, false, { button: 'right', clickCount: 1, modifiers: 0 })
255+
await clickAt(contents, 10, 20, false, { ...PRIMARY_CLICK, button: 'right' })
256256
vi.mocked(Menu.buildFromTemplate).mockClear()
257257

258258
onInput?.({}, { type: inputEvent })

‎apps/desktop/src/main/browser-agent/driver.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3603,6 +3603,34 @@ describe('credential protection', () => {
36033603
})
36043604
})
36053605

3606+
it('reports a press-and-hold cancelled mid-hold as an unknown outcome', async () => {
3607+
const contents = await openPage()
3608+
respondWith(contents, {})
3609+
3610+
const pending = driver.executeTool(
3611+
'chat-test',
3612+
'browser_click',
3613+
{ elementId: 0, holdMs: 5_000 },
3614+
'hold-call'
3615+
)
3616+
await vi.waitFor(() =>
3617+
expect(
3618+
cdpCalls(contents, 'Input.dispatchMouseEvent').some(
3619+
([, params]) => toRecord(params).type === 'mousePressed'
3620+
)
3621+
).toBe(true)
3622+
)
3623+
driver.cancelTool('chat-test', 'hold-call')
3624+
3625+
await expect(pending).resolves.toMatchObject({
3626+
ok: true,
3627+
result: { outcomeUnknown: true, doNotRetry: true },
3628+
})
3629+
expect(
3630+
cdpCalls(contents, 'Input.dispatchMouseEvent').map(([, params]) => toRecord(params).type)
3631+
).toContain('mouseReleased')
3632+
})
3633+
36063634
it('rejects batches that name non-action tools or observe per action', async () => {
36073635
await openPage()
36083636

@@ -3624,6 +3652,17 @@ describe('credential protection', () => {
36243652
error: expect.stringContaining('Batch action 0'),
36253653
})
36263654
expect(observed).toMatchObject({ ok: false, error: expect.stringContaining('cannot observe') })
3655+
3656+
const held = await driver.executeTool('chat-test', 'browser_batch', {
3657+
actions: [
3658+
{ tool: 'browser_click', args: { elementId: 0, holdMs: 2000 } },
3659+
{ tool: 'browser_click', args: { elementId: 0 } },
3660+
],
3661+
})
3662+
expect(held).toMatchObject({
3663+
ok: false,
3664+
error: expect.stringContaining('cannot press and hold'),
3665+
})
36273666
})
36283667

36293668
it('keeps element ids valid when an observed action is refused before dispatch', async () => {

‎apps/desktop/src/main/browser-agent/driver.ts‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import {
7676
resolveFileInputTarget,
7777
scrollPage,
7878
selectOptionInElement,
79+
serializePageCall,
7980
setFocusedInputValue,
8081
typeIntoElement,
8182
} from '@/main/browser-agent/page-functions'
@@ -187,6 +188,9 @@ function parseBatchActions(params: Record<string, unknown>): BatchAction[] {
187188
if ('observe' in action.args) {
188189
throw new ToolError(`Batch action ${index} cannot observe; pass observe on the batch itself.`)
189190
}
191+
if (num(action.args, 'holdMs')) {
192+
throw new ToolError(`Batch action ${index} cannot press and hold; run it as its own click.`)
193+
}
190194
return { tool: action.tool, args: action.args }
191195
})
192196
}
@@ -1121,6 +1125,9 @@ const POINTER_BUTTONS: ReadonlySet<string> = new Set(['left', 'right', 'middle']
11211125
/** Enough to walk a slider or list by keyboard in one call without flooding the page. */
11221126
const MAX_KEY_REPEAT = 50
11231127

1128+
/** Longest press-and-hold a click may request; well inside the click tool's watchdog. */
1129+
const MAX_POINTER_HOLD_MS = 10_000
1130+
11241131
/** The optional click gesture shared by `browser_click` and `browser_click_at`. */
11251132
function pointerClick(params: Record<string, unknown>): cdp.PointerClick {
11261133
const button = str(params, 'button') ?? 'left'
@@ -1133,10 +1140,20 @@ function pointerClick(params: Record<string, unknown>): cdp.PointerClick {
11331140
if (!Array.isArray(names) || names.length > 4 || names.some((name) => typeof name !== 'string')) {
11341141
throw new ToolError('modifiers must be a list of modifier names such as ["Shift"] or ["Mod"].')
11351142
}
1143+
const holdMs = num(params, 'holdMs') ?? 0
1144+
if (!Number.isInteger(holdMs) || holdMs < 0 || holdMs > MAX_POINTER_HOLD_MS) {
1145+
throw new ToolError(
1146+
`holdMs must be a whole number of milliseconds from 0 to ${MAX_POINTER_HOLD_MS}.`
1147+
)
1148+
}
1149+
if (holdMs > 0 && clickCount !== 1) {
1150+
throw new ToolError('holdMs applies to a single press; use clickCount 1.')
1151+
}
11361152
return {
11371153
button: button as cdp.PointerClick['button'],
11381154
clickCount,
11391155
modifiers: cdpModifiers(parseModifiers(names)),
1156+
holdMs,
11401157
}
11411158
}
11421159

@@ -1163,7 +1180,9 @@ function uploadPaths(params: Record<string, unknown>): string[] {
11631180
}
11641181

11651182
function isPrimaryClick(click: cdp.PointerClick): boolean {
1166-
return click.button === 'left' && click.clickCount === 1 && click.modifiers === 0
1183+
return (
1184+
click.button === 'left' && click.clickCount === 1 && click.modifiers === 0 && click.holdMs === 0
1185+
)
11671186
}
11681187

11691188
const DIALOG_ANSWERING_TOOLS: ReadonlySet<BrowserToolName> = new Set([
@@ -1274,7 +1293,7 @@ async function execInPage<Args extends unknown[], Result>(
12741293
'The active tab is blank. Call browser_navigate before using page inspection or interaction tools.'
12751294
)
12761295
}
1277-
const invocation = `(${String(fn)}).apply(null, ${JSON.stringify(args)})`
1296+
const invocation = serializePageCall(fn as (...args: never[]) => unknown, args)
12781297
const expression =
12791298
typeof notAfter === 'number'
12801299
? `(Date.now() >= ${Math.floor(notAfter)} ? ({error: "expired"}) : ${invocation})`
@@ -3204,7 +3223,10 @@ async function executeToolInner(
32043223
try {
32053224
assertCurrentExecution()
32063225
assertElementActionCurrent(contents, elementId, target)
3207-
await cdp.clickAt(contents, x, y, false, click)
3226+
// A hold keeps the press in flight for seconds; cancelling it mid-gesture must read as
3227+
// an outcome that may have acted, never as a click that did not start.
3228+
if (click.holdMs > 0) onActionOutcome?.({ status: 'pending' })
3229+
await cdp.clickAt(contents, x, y, false, click, signal)
32083230
trusted = true
32093231
activation = 'native-pointer'
32103232
} catch (error) {
@@ -3244,7 +3266,8 @@ async function executeToolInner(
32443266
try {
32453267
assertCurrentExecution()
32463268
assertElementActionCurrent(contents, elementId, target)
3247-
await cdp.clickAt(contents, finalTopPoint.x, finalTopPoint.y, false, click)
3269+
if (click.holdMs > 0) onActionOutcome?.({ status: 'pending' })
3270+
await cdp.clickAt(contents, finalTopPoint.x, finalTopPoint.y, false, click, signal)
32483271
trusted = true
32493272
activation = 'native-pointer'
32503273
prepared = finalSurface
@@ -3261,7 +3284,7 @@ async function executeToolInner(
32613284
} else {
32623285
if (!isPrimaryClick(click)) {
32633286
throw new ToolError(
3264-
'This framed control has no reliable pointer position, so only a plain left click can activate it. Use browser_screenshot and browser_click_at for other buttons, click counts, or modifiers.'
3287+
'This framed control has no reliable pointer position, so only a plain left click can activate it. Use browser_screenshot and browser_click_at for other buttons, click counts, holds, or modifiers.'
32653288
)
32663289
}
32673290
const activationKey = prepared.activationKey
@@ -4622,8 +4645,9 @@ async function executeToolInner(
46224645
const beforeElement = await activeElementState(contents)
46234646
assertCurrentExecution()
46244647
assertActiveContents(contents, clickNavigationEpoch)
4648+
if (click.holdMs > 0) onActionOutcome?.({ status: 'pending' })
46254649
try {
4626-
await cdp.clickAt(contents, x, y, true, click)
4650+
await cdp.clickAt(contents, x, y, true, click, signal)
46274651
} catch (error) {
46284652
const rescued = navigationRescue(contents, clickNavigationEpoch, urlAtDispatch, {
46294653
trusted: true,

0 commit comments

Comments
 (0)