Skip to content

Commit 4180246

Browse files
authored
improvement(desktop-browser): let a page finish rendering before observing an action (#9985)
* improvement(desktop-browser): let a page finish rendering before observing an action * fix(desktop-browser): settle honors cancellation, watches shadow roots and frames, waits through mid-navigation probes * test(desktop-browser): drive the shadow-root settle test with a controlled clock * test(desktop-browser): drop an unused counter
1 parent 187e196 commit 4180246

4 files changed

Lines changed: 212 additions & 4 deletions

File tree

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1894,6 +1894,7 @@ describe('credential protection', () => {
18941894
activeElementSecrecy: 'safe',
18951895
readActiveElementState: {},
18961896
readPageActionState: {},
1897+
readDomQuiet: { quietMs: 1_000, loading: false },
18971898
})
18981899
const pageCall = vi.mocked(contents.executeJavaScript).getMockImplementation()
18991900
let releaseObservation: (value: unknown) => void = () => {}
@@ -3212,6 +3213,113 @@ describe('credential protection', () => {
32123213
})
32133214
})
32143215

3216+
it.each([
3217+
['still rendering', [0, 40, 90, 200], 4],
3218+
['already quiet', [400], 1],
3219+
['never settling', Array.from({ length: 200 }, () => 0), undefined],
3220+
] as const)(
3221+
'observes after an action only once a page %s has gone quiet, within the settle cap',
3222+
async (_label, quietReadings, expectedProbes) => {
3223+
const contents = await openPage()
3224+
const calls: string[] = []
3225+
let probe = 0
3226+
vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => {
3227+
if (isPageCall(expression, 'readDomQuiet')) {
3228+
calls.push('probe')
3229+
const quietMs = quietReadings[Math.min(probe++, quietReadings.length - 1)]
3230+
return Promise.resolve({ quietMs, loading: false })
3231+
}
3232+
if (isPageCall(expression, 'collectSnapshot')) {
3233+
calls.push('snapshot')
3234+
return Promise.resolve({
3235+
url: 'https://example.com/',
3236+
title: 'Example',
3237+
outline: '- heading "Results"',
3238+
truncated: false,
3239+
refIds: [],
3240+
refLineIndexes: {},
3241+
nextElementId: 1_000,
3242+
})
3243+
}
3244+
if (isPageCall(expression, 'clickElement')) return Promise.resolve(CLICK_TARGET)
3245+
return Promise.resolve(undefined)
3246+
})
3247+
vi.useFakeTimers()
3248+
try {
3249+
const clicking = driver.executeTool('chat-test', 'browser_click', {
3250+
elementId: 0,
3251+
observe: {},
3252+
})
3253+
await vi.advanceTimersByTimeAsync(5_000)
3254+
3255+
expect(await clicking).toMatchObject({ ok: true, result: { observation: { ok: true } } })
3256+
const probesBeforeSnapshot = calls.indexOf('snapshot')
3257+
expect(probesBeforeSnapshot).toBeGreaterThan(0)
3258+
if (expectedProbes !== undefined) expect(probesBeforeSnapshot).toBe(expectedProbes)
3259+
} finally {
3260+
vi.useRealTimers()
3261+
}
3262+
}
3263+
)
3264+
3265+
it('keeps settling through failed quiet probes, and never observes once cancelled', async () => {
3266+
const contents = await openPage()
3267+
const calls: string[] = []
3268+
let failing = 2
3269+
vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => {
3270+
if (isPageCall(expression, 'readDomQuiet')) {
3271+
calls.push('probe')
3272+
if (failing > 0) {
3273+
failing--
3274+
vi.mocked(contents.isLoading).mockReturnValue(true)
3275+
return Promise.reject(new Error('Execution context was destroyed'))
3276+
}
3277+
vi.mocked(contents.isLoading).mockReturnValue(false)
3278+
return Promise.resolve({ quietMs: failing === -1 ? 0 : 400, loading: false })
3279+
}
3280+
if (isPageCall(expression, 'collectSnapshot')) {
3281+
calls.push('snapshot')
3282+
return Promise.resolve({
3283+
url: 'https://example.com/',
3284+
title: 'Example',
3285+
outline: '- heading "Results"',
3286+
truncated: false,
3287+
refIds: [],
3288+
refLineIndexes: {},
3289+
nextElementId: 1_000,
3290+
})
3291+
}
3292+
if (isPageCall(expression, 'clickElement')) return Promise.resolve(CLICK_TARGET)
3293+
return Promise.resolve(undefined)
3294+
})
3295+
vi.useFakeTimers()
3296+
try {
3297+
const settled = driver.executeTool('chat-test', 'browser_click', {
3298+
elementId: 0,
3299+
observe: {},
3300+
})
3301+
await vi.advanceTimersByTimeAsync(5_000)
3302+
expect(await settled).toMatchObject({ ok: true, result: { observation: { ok: true } } })
3303+
expect(calls.indexOf('snapshot')).toBe(3)
3304+
3305+
calls.length = 0
3306+
failing = -1
3307+
const cancelled = driver.executeTool(
3308+
'chat-test',
3309+
'browser_click',
3310+
{ elementId: 0, observe: { query: 'Results' } },
3311+
'click-cancelled'
3312+
)
3313+
await vi.advanceTimersByTimeAsync(300)
3314+
expect(driver.cancelTool('chat-test', 'click-cancelled')).toBe(true)
3315+
await vi.advanceTimersByTimeAsync(5_000)
3316+
await cancelled
3317+
expect(calls).not.toContain('snapshot')
3318+
} finally {
3319+
vi.useRealTimers()
3320+
}
3321+
})
3322+
32153323
it('acts on a ref an earlier snapshot of the same page issued', async () => {
32163324
const contents = await openPage()
32173325
respondWith(contents, {

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

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import {
7070
readActiveElementState,
7171
readCheckableElementState,
7272
readChildFrameElementState,
73+
readDomQuiet,
7374
readFormFieldState,
7475
readPageActionState,
7576
readPageText,
@@ -2062,6 +2063,9 @@ async function prepareTypingSurface(
20622063
const TRANSIENT_PROBE_ERRORS = new Set(['stale', 'not-visible', 'not-editable'])
20632064
const SETTLE_GRACE_MS = 1_000
20642065
const SETTLE_PROBE_INTERVAL_MS = 250
2066+
/** Structural quiet that counts as settled, and the most an observation waits for it. */
2067+
const DOM_QUIET_MS = 150
2068+
const DOM_SETTLE_MAX_MS = 2_000
20652069

20662070
/**
20672071
* First-touch page probe with a short settle grace. A live view re-rendering
@@ -2096,6 +2100,33 @@ async function execInPageWithSettleGrace<Args extends unknown[], Result>(
20962100
}
20972101
}
20982102

2103+
/**
2104+
* Lets a page finish rendering what an action started (a route change, a fetched list) before it
2105+
* is observed, so the observation is not a half-rendered page. A page already quiet costs one probe.
2106+
*/
2107+
async function waitForDomQuiet(
2108+
target: PageExecutionTarget,
2109+
signal: AbortSignal,
2110+
executionDeadline?: number
2111+
) {
2112+
const until = Math.min(
2113+
Date.now() + DOM_SETTLE_MAX_MS,
2114+
executionDeadline ?? Number.POSITIVE_INFINITY
2115+
)
2116+
while (!signal.aborted && Date.now() + 25 < until) {
2117+
const probe = await execInPage(target, readDomQuiet, [], false, executionDeadline).catch(
2118+
() => null
2119+
)
2120+
// A failed probe while the tab loads is a document swapping mid-navigation, so keep waiting;
2121+
// one that fails on a loaded tab (a page that cannot be scripted) has nothing to settle.
2122+
if (probe === null && !('isLoading' in target && target.isLoading())) return
2123+
const state = toRecord(probe)
2124+
const quietMs = typeof state.quietMs === 'number' ? state.quietMs : 0
2125+
if (quietMs >= DOM_QUIET_MS && state.loading === false) return
2126+
await interruptibleSleep(Math.min(100, Math.max(25, DOM_QUIET_MS - quietMs)), signal)
2127+
}
2128+
}
2129+
20992130
function pointFromPrepared(prepared: Record<string, unknown>): { x: number; y: number } {
21002131
return { x: prepared.x as number, y: prepared.y as number }
21012132
}
@@ -5460,15 +5491,27 @@ export async function executeTool(
54605491
}
54615492
return result
54625493
},
5463-
(query) =>
5464-
executeToolInner(
5494+
async (query) => {
5495+
await waitForDomQuiet(
5496+
session.requireAutomationTab().view.webContents,
5497+
executionController.signal,
5498+
executionDeadline
5499+
)
5500+
assertCurrentExecution()
5501+
if (executionController.signal.aborted) {
5502+
throw new ToolError(
5503+
'This browser action was cancelled before it could observe the page.'
5504+
)
5505+
}
5506+
return executeToolInner(
54655507
query === undefined ? 'browser_snapshot' : 'browser_find',
54665508
query === undefined ? {} : { query },
54675509
assertCurrentExecution,
54685510
executionDeadline,
54695511
invocationEpoch,
54705512
executionController.signal
5471-
),
5513+
)
5514+
},
54725515
assertCurrentExecution
54735516
)
54745517
const cancellableExecution = Promise.race([execution, cancellation])

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
readActiveElementState,
1818
readCheckableElementState,
1919
readChildFrameElementState,
20+
readDomQuiet,
2021
readPageActionState,
2122
readPageText,
2223
readSelectElementState,
@@ -526,6 +527,28 @@ describe('collectSnapshot', () => {
526527
expect(outline).not.toContain('button "x" [ref=999]')
527528
})
528529

530+
it('counts changes inside a shadow root the snapshot walked as page activity', async () => {
531+
vi.useFakeTimers({ toFake: ['performance'] })
532+
try {
533+
document.body.innerHTML = '<div id="host"></div>'
534+
const host = visible(document.querySelector('#host') as HTMLDivElement)
535+
const shadow = host.attachShadow({ mode: 'open' })
536+
shadow.innerHTML = '<button>Load</button>'
537+
visible(shadow.querySelector('button') as HTMLButtonElement)
538+
collectSnapshot()
539+
await Promise.resolve()
540+
vi.advanceTimersByTime(500)
541+
expect((readDomQuiet() as { quietMs: number }).quietMs).toBeGreaterThanOrEqual(500)
542+
543+
shadow.append(document.createElement('ul'))
544+
await Promise.resolve()
545+
546+
expect((readDomQuiet() as { quietMs: number }).quietMs).toBeLessThan(500)
547+
} finally {
548+
vi.useRealTimers()
549+
}
550+
})
551+
529552
it('marks elements that appeared since the previous snapshot as new', () => {
530553
document.body.innerHTML = '<button>Compose</button>'
531554
visible(document.querySelector('button') as HTMLButtonElement)

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ declare global {
5656
observedRoots: WeakSet<ParentNode>
5757
}>
5858
__simAgentNextElementId?: number
59+
/** When the document last changed structurally (nodes or text), for settling before an observation. */
60+
__simAgentDomChangedAt?: number
61+
__simAgentDomWatcher?: MutationObserver
62+
/** Adds a document or open shadow root to the settle watcher, once. */
63+
__simAgentWatchDom: (root: Node) => void
5964
/** Every ref this document issued, resolvable while its element keeps that identity. */
6065
__simAgentIssuedRefs?: Map<
6166
number,
@@ -85,12 +90,37 @@ export function serializePageCall(fn: (...args: never[]) => unknown, args: unkno
8590
return `((${String(installPageHelpers)})(), (${String(fn)}).apply(null, ${JSON.stringify(args)}))`
8691
}
8792

93+
/** How long the document has gone without a structural change, and whether it is still loading. */
94+
export function readDomQuiet(): unknown {
95+
return {
96+
quietMs: performance.now() - (window.__simAgentDomChangedAt ?? 0),
97+
loading: document.readyState !== 'complete',
98+
}
99+
}
100+
88101
/**
89102
* Installs the helpers page functions share. Page functions are serialized and run standalone,
90103
* so the driver runs this installer before each one instead of inlining shared code into every
91104
* function.
92105
*/
93106
export function installPageHelpers(): void {
107+
// Structural changes only: attribute churn (animations, hover styles) would keep a page "busy".
108+
// Snapshots add the same-origin frames and open shadow roots they walk, which a document-level
109+
// observer cannot see into.
110+
if (!window.__simAgentDomWatcher) {
111+
window.__simAgentDomChangedAt = performance.now()
112+
window.__simAgentDomWatcher = new MutationObserver(() => {
113+
window.__simAgentDomChangedAt = performance.now()
114+
})
115+
}
116+
const watcher = window.__simAgentDomWatcher
117+
const watched = new WeakSet<Node>()
118+
window.__simAgentWatchDom = (root: Node): void => {
119+
if (watched.has(root)) return
120+
watched.add(root)
121+
watcher.observe(root, { subtree: true, childList: true, characterData: true })
122+
}
123+
if (document.documentElement) window.__simAgentWatchDom(document.documentElement)
94124
/**
95125
* The open modal each document aria-hid together with everything else. MUI's ModalManager
96126
* aria-hides every <body> child except the modal's mount node, and a `disablePortal` modal
@@ -889,6 +919,7 @@ export function collectSnapshot(
889919
try {
890920
const innerDoc = (el as HTMLIFrameElement).contentDocument
891921
if (innerDoc?.body && isVisible(el)) {
922+
window.__simAgentWatchDom(innerDoc.documentElement)
892923
if (!push(`${indent}- iframe:`)) return
893924
walk(innerDoc.body.childNodes, childDepth + 1, coveredText)
894925
} else if (scopedRoot && !innerDoc && visible) {
@@ -901,7 +932,10 @@ export function collectSnapshot(
901932
}
902933

903934
const shadow = (el as HTMLElement).shadowRoot
904-
if (shadow) walk(shadow.childNodes, childDepth, coveredText)
935+
if (shadow) {
936+
window.__simAgentWatchDom(shadow)
937+
walk(shadow.childNodes, childDepth, coveredText)
938+
}
905939
walk(el.childNodes, childDepth, coveredText)
906940
}
907941
}

0 commit comments

Comments
 (0)