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.

Author: waleedlatif1

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

@@ -4,7 +4,7 @@ import { describe, expect, it, vi } from 'vitest'
 vi.mock('electron', () => import('@/test/electron-mock'))
 
 import { Menu, WebContentsView } from 'electron'
-import { clickAt } from '@/main/browser-agent/cdp'
+import { clickAt, PRIMARY_CLICK } from '@/main/browser-agent/cdp'
 import {
   attachAgentContextMenu,
   BASE_ZOOM_FACTOR,
@@ -228,12 +228,7 @@ describe('attachAgentContextMenu', () => {
       ContextMenuListener,
     ][]
     const onContextMenu = listeners.find(([event]) => event === 'context-menu')![1]
-    await clickAt(contents, 10, 20, false, {
-      button: 'right',
-      clickCount: 1,
-      modifiers: 0,
-      holdMs: 0,
-    })
+    await clickAt(contents, 10, 20, false, { ...PRIMARY_CLICK, button: 'right' })
     vi.mocked(Menu.buildFromTemplate).mockClear()
 
     onContextMenu({}, params())
@@ -257,12 +252,7 @@ describe('attachAgentContextMenu', () => {
       ][]
       const onInput = listeners.find(([event]) => event === 'input-event')?.[1]
       const onContextMenu = listeners.find(([event]) => event === 'context-menu')![1]
-      await clickAt(contents, 10, 20, false, {
-        button: 'right',
-        clickCount: 1,
-        modifiers: 0,
-        holdMs: 0,
-      })
+      await clickAt(contents, 10, 20, false, { ...PRIMARY_CLICK, button: 'right' })
       vi.mocked(Menu.buildFromTemplate).mockClear()
 
       onInput?.({}, { type: inputEvent })

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

@@ -3624,6 +3624,17 @@ describe('credential protection', () => {
       error: expect.stringContaining('Batch action 0'),
     })
     expect(observed).toMatchObject({ ok: false, error: expect.stringContaining('cannot observe') })
+
+    const held = await driver.executeTool('chat-test', 'browser_batch', {
+      actions: [
+        { tool: 'browser_click', args: { elementId: 0, holdMs: 2000 } },
+        { tool: 'browser_click', args: { elementId: 0 } },
+      ],
+    })
+    expect(held).toMatchObject({
+      ok: false,
+      error: expect.stringContaining('cannot press and hold'),
+    })
   })
 
   it('keeps element ids valid when an observed action is refused before dispatch', async () => {

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

@@ -76,6 +76,7 @@ import {
   resolveFileInputTarget,
   scrollPage,
   selectOptionInElement,
+  serializePageCall,
   setFocusedInputValue,
   typeIntoElement,
 } from '@/main/browser-agent/page-functions'
@@ -187,6 +188,9 @@ function parseBatchActions(params: Record<string, unknown>): BatchAction[] {
     if ('observe' in action.args) {
       throw new ToolError(`Batch action ${index} cannot observe; pass observe on the batch itself.`)
     }
+    if (num(action.args, 'holdMs')) {
+      throw new ToolError(`Batch action ${index} cannot press and hold; run it as its own click.`)
+    }
     return { tool: action.tool, args: action.args }
   })
 }
@@ -1121,10 +1125,10 @@ const POINTER_BUTTONS: ReadonlySet<string> = new Set(['left', 'right', 'middle']
 /** Enough to walk a slider or list by keyboard in one call without flooding the page. */
 const MAX_KEY_REPEAT = 50
 
-/** The optional click gesture shared by `browser_click` and `browser_click_at`. */
 /** Longest press-and-hold a click may request; well inside the click tool's watchdog. */
 const MAX_POINTER_HOLD_MS = 10_000
 
+/** The optional click gesture shared by `browser_click` and `browser_click_at`. */
 function pointerClick(params: Record<string, unknown>): cdp.PointerClick {
   const button = str(params, 'button') ?? 'left'
   if (!POINTER_BUTTONS.has(button)) throw new ToolError('button must be left, right, or middle.')
@@ -1176,7 +1180,9 @@ function uploadPaths(params: Record<string, unknown>): string[] {
 }
 
 function isPrimaryClick(click: cdp.PointerClick): boolean {
-  return click.button === 'left' && click.clickCount === 1 && click.modifiers === 0
+  return (
+    click.button === 'left' && click.clickCount === 1 && click.modifiers === 0 && click.holdMs === 0
+  )
 }
 
 const DIALOG_ANSWERING_TOOLS: ReadonlySet<BrowserToolName> = new Set([
@@ -1287,7 +1293,7 @@ async function execInPage<Args extends unknown[], Result>(
       'The active tab is blank. Call browser_navigate before using page inspection or interaction tools.'
     )
   }
-  const invocation = `(${String(fn)}).apply(null, ${JSON.stringify(args)})`
+  const invocation = serializePageCall(fn as (...args: never[]) => unknown, args)
   const expression =
     typeof notAfter === 'number'
       ? `(Date.now() >= ${Math.floor(notAfter)} ? ({error: "expired"}) : ${invocation})`
@@ -3274,7 +3280,7 @@ async function executeToolInner(
         } else {
           if (!isPrimaryClick(click)) {
             throw new ToolError(
-              '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.'
+              '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.'
             )
           }
           const activationKey = prepared.activationKey

apps/desktop/src/main/browser-agent/keyboard.test.ts

@@ -208,10 +208,23 @@ describe('modifierKeyEvents', () => {
       code: 'ControlLeft',
       modifiers: 2,
     })
-    expect(up).toMatchObject({ type: 'keyUp', key: 'Control' })
+    expect(up).toMatchObject({ type: 'keyUp', key: 'Control', modifiers: 0 })
     expect(modifierKeyEvents(combo, 'linux')).toEqual({ downs: [], ups: [] })
   })
 
+  it('treats modifier aliases like their canonical key', () => {
+    for (const [alias, key, flag] of [
+      ['Ctrl', 'Control', 2],
+      ['Option', 'Alt', 1],
+      ['Cmd', 'Meta', 4],
+      ['Command', 'Meta', 4],
+    ] as const) {
+      const [down, up] = buildKeyDispatchPlan(parseKeyCombo(alias, 'linux'), 'linux')
+      expect(down).toMatchObject({ key, modifiers: flag })
+      expect(up).toMatchObject({ type: 'keyUp', key, modifiers: 0 })
+    }
+  })
+
   it('sends no extra events for a key without modifiers', () => {
     expect(modifierKeyEvents(parseKeyCombo('a', 'linux'), 'linux')).toEqual({ downs: [], ups: [] })
   })

apps/desktop/src/main/browser-agent/keyboard.ts

@@ -16,6 +16,11 @@ interface KeyDescriptor {
   keyCode: number
 }
 
+const CONTROL_KEY: KeyDescriptor = { key: 'Control', code: 'ControlLeft', keyCode: 17 }
+const SHIFT_KEY: KeyDescriptor = { key: 'Shift', code: 'ShiftLeft', keyCode: 16 }
+const ALT_KEY: KeyDescriptor = { key: 'Alt', code: 'AltLeft', keyCode: 18 }
+const META_KEY: KeyDescriptor = { key: 'Meta', code: 'MetaLeft', keyCode: 91 }
+
 const NAMED_KEYS: Record<string, KeyDescriptor> = {
   enter: { key: 'Enter', code: 'Enter', keyCode: 13 },
   escape: { key: 'Escape', code: 'Escape', keyCode: 27 },
@@ -51,14 +56,14 @@ const NAMED_KEYS: Record<string, KeyDescriptor> = {
   '`': { key: '`', code: 'Backquote', keyCode: 192 },
   plus: { key: '+', code: 'Equal', keyCode: 187 },
   insert: { key: 'Insert', code: 'Insert', keyCode: 45 },
-  control: { key: 'Control', code: 'ControlLeft', keyCode: 17 },
-  ctrl: { key: 'Control', code: 'ControlLeft', keyCode: 17 },
-  shift: { key: 'Shift', code: 'ShiftLeft', keyCode: 16 },
-  alt: { key: 'Alt', code: 'AltLeft', keyCode: 18 },
-  option: { key: 'Alt', code: 'AltLeft', keyCode: 18 },
-  meta: { key: 'Meta', code: 'MetaLeft', keyCode: 91 },
-  cmd: { key: 'Meta', code: 'MetaLeft', keyCode: 91 },
-  command: { key: 'Meta', code: 'MetaLeft', keyCode: 91 },
+  control: CONTROL_KEY,
+  ctrl: CONTROL_KEY,
+  shift: SHIFT_KEY,
+  alt: ALT_KEY,
+  option: ALT_KEY,
+  meta: META_KEY,
+  cmd: META_KEY,
+  command: META_KEY,
   ...Object.fromEntries(
     Array.from({ length: 12 }, (_, index) => [
       `f${index + 1}`,
@@ -108,12 +113,12 @@ const BASE_FOR_SHIFTED_CHARACTER: Record<string, string> = Object.fromEntries(
   Object.entries(SHIFTED_CHARACTERS).map(([base, shifted]) => [shifted, base])
 )
 
-/** Modifier keys in the order a chord presses them, each with the flag its key-down sets. */
+/** Modifier keys in the fixed order a chord presses them, each with the flag its key-down sets. */
 const MODIFIER_KEYS: readonly { flag: keyof KeyModifiers; descriptor: KeyDescriptor }[] = [
-  { flag: 'ctrl', descriptor: NAMED_KEYS.control },
-  { flag: 'alt', descriptor: NAMED_KEYS.alt },
-  { flag: 'shift', descriptor: NAMED_KEYS.shift },
-  { flag: 'meta', descriptor: NAMED_KEYS.meta },
+  { flag: 'ctrl', descriptor: CONTROL_KEY },
+  { flag: 'alt', descriptor: ALT_KEY },
+  { flag: 'shift', descriptor: SHIFT_KEY },
+  { flag: 'meta', descriptor: META_KEY },
 ]
 
 export interface KeyModifiers {
@@ -312,7 +317,11 @@ export function buildKeyDispatchPlan(
     ...(text !== undefined ? { text } : {}),
     ...(commands.length > 0 ? { commands } : {}),
   }
-  return [down, { ...base, type: 'keyUp' }]
+  const ownModifier = MODIFIER_KEYS.find(({ descriptor }) => descriptor.key === combo.key)
+  const upModifiers = ownModifier
+    ? cdpModifiers({ ...combo, [ownModifier.flag]: false })
+    : modifiers
+  return [down, { ...base, type: 'keyUp', modifiers: upModifiers }]
 }
 
 /**
@@ -326,25 +335,22 @@ export function modifierKeyEvents(
 ): { downs: cdp.CdpKeyEvent[]; ups: cdp.CdpKeyEvent[] } {
   const combo = normalizeComboForPlatform(rawCombo, platform)
   const held = { ctrl: false, meta: false, shift: false, alt: false }
-  const downs: cdp.CdpKeyEvent[] = []
-  const ups: cdp.CdpKeyEvent[] = []
-  for (const { flag, descriptor } of MODIFIER_KEYS) {
-    if (!combo[flag] || descriptor.key === combo.key) continue
+  const pressed = MODIFIER_KEYS.filter(
+    ({ flag, descriptor }) => combo[flag] && descriptor.key !== combo.key
+  )
+  const event = ({ key, code, keyCode }: KeyDescriptor) => ({
+    key,
+    code,
+    windowsVirtualKeyCode: keyCode,
+  })
+  const downs = pressed.map(({ flag, descriptor }) => {
     held[flag] = true
-    const event = {
-      key: descriptor.key,
-      code: descriptor.code,
-      windowsVirtualKeyCode: descriptor.keyCode,
-    }
-    downs.push({ ...event, type: 'rawKeyDown', modifiers: cdpModifiers(held) })
-    ups.unshift({ ...event, type: 'keyUp', modifiers: 0 })
-  }
-  let remaining = { ...held }
-  for (const up of ups) {
-    const flag = MODIFIER_KEYS.find((modifier) => modifier.descriptor.key === up.key)?.flag
-    if (flag) remaining = { ...remaining, [flag]: false }
-    up.modifiers = cdpModifiers(remaining)
-  }
+    return { ...event(descriptor), type: 'rawKeyDown' as const, modifiers: cdpModifiers(held) }
+  })
+  const ups = [...pressed].reverse().map(({ flag, descriptor }) => {
+    held[flag] = false
+    return { ...event(descriptor), type: 'keyUp' as const, modifiers: cdpModifiers(held) }
+  })
   return { downs, ups }
 }
 

apps/desktop/src/main/browser-agent/page-functions.test.ts

@@ -11,6 +11,7 @@ import {
   getElementScreenshotRect,
   getViewportInfo,
   hoverElement,
+  installPageHelpers,
   pageContainsText,
   pressKeyOnPage,
   readActiveElementState,
@@ -22,6 +23,7 @@ import {
   resolveFileInputTarget,
   scrollPage,
   selectOptionInElement,
+  serializePageCall,
   setFocusedInputValue,
   typeIntoElement,
 } from '@/main/browser-agent/page-functions'
@@ -60,8 +62,7 @@ function installDomShims(): void {
  * fails here exactly as it would in a real page.
  */
 function runSerialized(fn: (...args: never[]) => unknown, args: unknown[]): unknown {
-  const expression = `(${String(fn)}).apply(null, ${JSON.stringify(args)})`
-  return new Function(`return ${expression}`)()
+  return new Function(`return ${serializePageCall(fn, args)}`)()
 }
 
 function visible<T extends Element>(el: T): T {
@@ -116,6 +117,7 @@ beforeEach(() => {
   window.__simAgentMutationStates = undefined
   window.__simAgentNextElementId = 0
   window.__simAgentShownElements = undefined
+  installPageHelpers()
   window.__simAgentResolveElement = undefined
   installDomShims()
   Reflect.deleteProperty(document, 'activeElement')