Skip to content

Commit 27d73bc

Browse files
committed
fix(browser): honor cancellation on pointer routes and keep agent tabs tracked across scopes
- Timed drags and hover routes stop as soon as their tool call is cancelled; a drag still releases the button through its existing cleanup. - A coordinate hover refuses durationMs without via points instead of silently moving straight to the target. - A scope migration carries the chat's agent tab registration with it, so the migrated chat's old agent tab is released when it switches tabs. - Overlay controls are found through shadow roots and with any tag casing.
1 parent f4b7dd4 commit 27d73bc

8 files changed

Lines changed: 149 additions & 9 deletions

File tree

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ import {
1515
captureScreenshot,
1616
clickAt,
1717
consumeAgentContextMenu,
18+
dragPointer,
1819
ensureInstrumented,
1920
evaluateInIsolatedFrame,
2021
insertText,
22+
movePointer,
2123
PRIMARY_CLICK,
2224
pointerPathSteps,
2325
releaseFileInput,
@@ -259,6 +261,67 @@ describe('browser-agent CDP instrumentation', () => {
259261
])
260262
})
261263

264+
it('stops a pointer route as soon as it is aborted', async () => {
265+
const contents = new WebContentsView().webContents
266+
const moves = () =>
267+
vi
268+
.mocked(contents.debugger.sendCommand)
269+
.mock.calls.filter(([method]) => String(method).startsWith('Input.dispatchMouseEvent'))
270+
const aborted = new AbortController()
271+
aborted.abort()
272+
await expect(
273+
movePointer(contents, { via: [], durationMs: null }, { x: 5, y: 5 }, aborted.signal)
274+
).rejects.toMatchObject({ name: 'AbortError' })
275+
expect(moves()).toHaveLength(0)
276+
277+
vi.useFakeTimers()
278+
try {
279+
const controller = new AbortController()
280+
const route = movePointer(
281+
contents,
282+
{ via: [{ x: 0, y: 0 }], durationMs: 5_000 },
283+
{ x: 500, y: 0 },
284+
controller.signal
285+
)
286+
const settled = expect(route).rejects.toMatchObject({ name: 'AbortError' })
287+
await vi.advanceTimersByTimeAsync(100)
288+
const sentBeforeAbort = moves().length
289+
controller.abort()
290+
await settled
291+
expect(moves()).toHaveLength(sentBeforeAbort)
292+
} finally {
293+
vi.useRealTimers()
294+
}
295+
})
296+
297+
it('releases the button when a timed drag is aborted mid-route', async () => {
298+
const contents = new WebContentsView().webContents
299+
const types = () =>
300+
vi
301+
.mocked(contents.debugger.sendCommand)
302+
.mock.calls.filter(([method]) => method === 'Input.dispatchMouseEvent')
303+
.map(([, params]) => toRecord(params).type)
304+
vi.useFakeTimers()
305+
try {
306+
const controller = new AbortController()
307+
const drag = dragPointer(
308+
contents,
309+
{ x: 0, y: 0 },
310+
{ x: 500, y: 0 },
311+
{ via: [], durationMs: 5_000 },
312+
controller.signal
313+
)
314+
const settled = expect(drag).rejects.toMatchObject({ name: 'AbortError' })
315+
await vi.advanceTimersByTimeAsync(100)
316+
controller.abort()
317+
await settled
318+
expect(types().at(-1)).toBe('mouseReleased')
319+
expect(types().filter((type) => type === 'mouseMoved').length).toBeLessThan(20)
320+
} finally {
321+
vi.useRealTimers()
322+
}
323+
})
324+
262325
it('keeps the default drag pace and lands exactly on every via point', () => {
263326
const direct = pointerPathSteps({ x: 0, y: 0 }, { via: [], durationMs: null }, { x: 120, y: 0 })
264327
expect(direct.stepDelayMs).toBe(20)

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,7 @@ export async function movePointer(
10181018
signal?: AbortSignal
10191019
): Promise<void> {
10201020
const [start, ...rest] = [...path.via, to]
1021+
signal?.throwIfAborted()
10211022
await moveMouse(contents, start.x, start.y)
10221023
if (rest.length === 0) return
10231024
const { points, stepDelayMs } = pointerPathSteps(
@@ -1026,8 +1027,8 @@ export async function movePointer(
10261027
to
10271028
)
10281029
for (const point of points) {
1029-
signal?.throwIfAborted()
10301030
await interruptibleSleep(stepDelayMs, signal)
1031+
signal?.throwIfAborted()
10311032
await moveMouse(contents, point.x, point.y)
10321033
}
10331034
}
@@ -1161,7 +1162,8 @@ export async function dragPointer(
11611162
contents: WebContents,
11621163
from: ViewportPoint,
11631164
to: ViewportPoint,
1164-
path: PointerPath = DIRECT_PATH
1165+
path: PointerPath = DIRECT_PATH,
1166+
signal?: AbortSignal
11651167
): Promise<{ nativeDragIntercepted: boolean }> {
11661168
const { points, stepDelayMs } = pointerPathSteps(from, path, to)
11671169
const interception: DragInterception = { intercepted: false, data: null }
@@ -1210,11 +1212,13 @@ export async function dragPointer(
12101212
// register the drag before the pointer sweeps across the page.
12111213
const heading = points[0] ?? to
12121214
await dragMove(from.x + Math.sign(heading.x - from.x || 1) * 4, from.y + 2)
1213-
await sleep(stepDelayMs)
1215+
await interruptibleSleep(stepDelayMs, signal)
12141216
for (const point of points) {
1217+
signal?.throwIfAborted()
12151218
await dragMove(point.x, point.y)
1216-
await sleep(stepDelayMs)
1219+
await interruptibleSleep(stepDelayMs, signal)
12171220
}
1221+
signal?.throwIfAborted()
12181222
// Hold over the target so drop zones running enter/over animations settle
12191223
// before the release lands.
12201224
await sleep(120)

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4684,6 +4684,11 @@ describe('credential protection', () => {
46844684
elementId: 0,
46854685
via: [{ x: 1, y: 2 }],
46864686
})
4687+
const unroutedPace = await driver.executeTool('chat-test', 'browser_hover', {
4688+
x: 10,
4689+
y: 10,
4690+
durationMs: 800,
4691+
})
46874692
const badPoint = await driver.executeTool('chat-test', 'browser_hover', {
46884693
x: 10,
46894694
y: 10,
@@ -4707,6 +4712,10 @@ describe('credential protection', () => {
47074712
ok: false,
47084713
error: expect.stringContaining('coordinate hover'),
47094714
})
4715+
expect(unroutedPace).toMatchObject({
4716+
ok: false,
4717+
error: expect.stringContaining('paces a hover route'),
4718+
})
47104719
expect(badPoint).toMatchObject({ ok: false, error: expect.stringContaining('via point') })
47114720
expect(tooLong).toMatchObject({ ok: false, error: expect.stringContaining('durationMs') })
47124721
expect(batched).toMatchObject({

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4512,6 +4512,11 @@ async function executeToolInner(
45124512
const x = requireNum(params, 'x')
45134513
const y = requireNum(params, 'y')
45144514
const path = pointerPath(params)
4515+
if (path.via.length === 0 && path.durationMs !== null) {
4516+
throw new ToolError(
4517+
'durationMs paces a hover route; pass via points for the pointer to travel through.'
4518+
)
4519+
}
45154520
assertCurrentExecution()
45164521
assertActiveContents(contents)
45174522
const pointTarget = unwrapPageResult(
@@ -5000,7 +5005,7 @@ async function executeToolInner(
50005005
assertActiveContents(contents, dragNavigationEpoch)
50015006
let interception: { nativeDragIntercepted: boolean }
50025007
try {
5003-
interception = await cdp.dragPointer(contents, from, to, path)
5008+
interception = await cdp.dragPointer(contents, from, to, path, signal)
50045009
} catch (error) {
50055010
throw new ToolError(
50065011
`Native drag dispatch failed (${getErrorMessage(error)}). The pointer may have been mid-drag; take a fresh snapshot to see the page's current state before retrying.`

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,30 @@ describe('collectSnapshot', () => {
667667
})
668668
})
669669

670+
it('names the controls of an overlay built from a web component', () => {
671+
document.body.innerHTML = `
672+
<button aria-label="Checkout">Checkout</button>
673+
<div id="banner" style="position: fixed"></div>`
674+
const host = document.getElementById('banner') as HTMLDivElement
675+
const shadow = host.attachShadow({ mode: 'open' })
676+
shadow.innerHTML = '<p>We use cookies</p><button aria-label="Close banner">×</button>'
677+
for (const element of [
678+
...Array.from(document.body.querySelectorAll('*')),
679+
...Array.from(shadow.querySelectorAll('*')),
680+
]) {
681+
visible(element)
682+
}
683+
const outline = outlineOf(collectSnapshot())
684+
const ref = refFor(outline, 'Checkout')
685+
const notice = shadow.querySelector('p') as HTMLParagraphElement
686+
Object.defineProperty(document, 'elementFromPoint', { configurable: true, value: () => notice })
687+
688+
expect(clickElement(ref, false)).toMatchObject({
689+
error: 'obstructed',
690+
blockerControls: [{ id: refFor(outline, 'Close banner'), name: 'Close banner' }],
691+
})
692+
})
693+
670694
it('refuses a coordinate click when an overlay owns every hit point', () => {
671695
document.body.innerHTML =
672696
'<button aria-label="Delete draft">Delete</button><div aria-label="Confirmation overlay"></div>'

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,22 @@ export function installPageHelpers(): void {
9898
* fixed or sticky layer — so an ordinary element in the way never lists unrelated page controls.
9999
*/
100100
window.__simAgentOverlayControls = (blocker: Element | null) => {
101+
/** The parent across shadow boundaries, so overlays built from web components qualify. */
102+
const composedParent = (element: Element): Element | null => {
103+
if (element.parentElement) return element.parentElement
104+
const root = element.getRootNode()
105+
return 'host' in root ? (root.host as Element) : null
106+
}
107+
const tagOf = (element: Element): string => String(element.tagName).toUpperCase()
101108
let overlay: Element | null = null
102-
for (let current = blocker; current && !overlay; current = current.parentElement) {
109+
for (let current = blocker; current && !overlay; current = composedParent(current)) {
103110
const role = current.getAttribute('role')
104111
const position = current.ownerDocument.defaultView?.getComputedStyle(current).position
105112
if (
106113
role === 'dialog' ||
107114
role === 'alertdialog' ||
108115
current.getAttribute('aria-modal') === 'true' ||
109-
current.tagName === 'DIALOG' ||
116+
tagOf(current) === 'DIALOG' ||
110117
position === 'fixed' ||
111118
position === 'sticky'
112119
) {
@@ -118,9 +125,17 @@ export function installPageHelpers(): void {
118125
const registry = window.__simAgentElements ?? []
119126
for (let id = 0; id < registry.length && controls.length < 4; id++) {
120127
const element = registry[id]
121-
if (!element || !overlay.contains(element)) continue
128+
let inOverlay = false
129+
for (
130+
let current: Element | null = element;
131+
current && !inOverlay;
132+
current = composedParent(current)
133+
) {
134+
inOverlay = current === overlay
135+
}
136+
if (!inOverlay) continue
122137
const role = element.getAttribute('role')
123-
const tag = element.tagName
138+
const tag = tagOf(element)
124139
if (
125140
tag !== 'BUTTON' &&
126141
tag !== 'A' &&

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,12 @@ export function activatePanelScope(scopeId: string | null): void {
155155

156156
/** Retags an active pending scope without tearing down the compositor. */
157157
export function migratePanelScope(fromScopeId: string, toScopeId: string): void {
158+
const agentView = agentViews.get(fromScopeId)
159+
if (agentView) {
160+
agentViews.delete(fromScopeId)
161+
if (!agentViews.has(toScopeId)) agentViews.set(toScopeId, agentView)
162+
else if (!isAgentView(agentView)) unparkView(agentView)
163+
}
158164
if (activePanelScopeId !== fromScopeId) return
159165
activePanelScopeId = toScopeId
160166
panelCaptureGeneration++

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,20 @@ describe('browser-agent session', () => {
522522
expect(session.migrateBrowserScope('chat-real', 'occupied')).toBe(false)
523523
})
524524

525+
it('carries the agent tab registration through a scope migration', () => {
526+
const first = session.withBrowserScope('pending:workspace', () => session.ensureTab())
527+
expect(session.migrateBrowserScope('pending:workspace', 'chat-real')).toBe(true)
528+
const removeChildView = (
529+
win as unknown as { contentView: { removeChildView: ReturnType<typeof vi.fn> } }
530+
).contentView.removeChildView
531+
removeChildView.mockClear()
532+
533+
// The migrated chat moves its agent to a new tab: the first stops being an agent tab.
534+
session.withBrowserScope('chat-real', () => session.addAutomationTab())
535+
536+
expect(removeChildView).toHaveBeenCalledWith(first.view)
537+
})
538+
525539
it('retains a migrated provisional alias until the durable scope is disposed', () => {
526540
const tab = session.withBrowserScope('pending:workspace', () => session.ensureTab())
527541
expect(session.migrateBrowserScope('pending:workspace', 'chat-real')).toBe(true)

0 commit comments

Comments
 (0)