From 3d5a1978bc8e01a6858dc4843ff0218a8ee11fe5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 12 Nov 2026 19:53:29 -0800 Subject: [PATCH 1/4] improvement(desktop-browser): let a page finish rendering before observing an action --- .../src/main/browser-agent/driver.test.ts | 49 +++++++++++++++++++ apps/desktop/src/main/browser-agent/driver.ts | 34 +++++++++++-- .../src/main/browser-agent/page-functions.ts | 24 +++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index dceb49ecbd5..99824218871 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -3212,6 +3212,55 @@ describe('credential protection', () => { }) }) + it.each([ + ['still rendering', [0, 40, 90, 200], 4], + ['already quiet', [400], 1], + ['never settling', Array.from({ length: 200 }, () => 0), undefined], + ] as const)( + 'observes after an action only once a page %s has gone quiet, within the settle cap', + async (_label, quietReadings, expectedProbes) => { + const contents = await openPage() + const calls: string[] = [] + let probe = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'readDomQuiet')) { + calls.push('probe') + const quietMs = quietReadings[Math.min(probe++, quietReadings.length - 1)] + return Promise.resolve({ quietMs, loading: false }) + } + if (isPageCall(expression, 'collectSnapshot')) { + calls.push('snapshot') + return Promise.resolve({ + url: 'https://example.com/', + title: 'Example', + outline: '- heading "Results"', + truncated: false, + refIds: [], + refLineIndexes: {}, + nextElementId: 1_000, + }) + } + if (isPageCall(expression, 'clickElement')) return Promise.resolve(CLICK_TARGET) + return Promise.resolve(undefined) + }) + vi.useFakeTimers() + try { + const clicking = driver.executeTool('chat-test', 'browser_click', { + elementId: 0, + observe: {}, + }) + await vi.advanceTimersByTimeAsync(5_000) + + expect(await clicking).toMatchObject({ ok: true, result: { observation: { ok: true } } }) + const probesBeforeSnapshot = calls.indexOf('snapshot') + expect(probesBeforeSnapshot).toBeGreaterThan(0) + if (expectedProbes !== undefined) expect(probesBeforeSnapshot).toBe(expectedProbes) + } finally { + vi.useRealTimers() + } + } + ) + it('acts on a ref an earlier snapshot of the same page issued', 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 02ae25dc4b3..50c14a2ff07 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -70,6 +70,7 @@ import { readActiveElementState, readCheckableElementState, readChildFrameElementState, + readDomQuiet, readFormFieldState, readPageActionState, readPageText, @@ -2062,6 +2063,9 @@ async function prepareTypingSurface( const TRANSIENT_PROBE_ERRORS = new Set(['stale', 'not-visible', 'not-editable']) const SETTLE_GRACE_MS = 1_000 const SETTLE_PROBE_INTERVAL_MS = 250 +/** Structural quiet that counts as settled, and the most an observation waits for it. */ +const DOM_QUIET_MS = 150 +const DOM_SETTLE_MAX_MS = 2_000 /** * First-touch page probe with a short settle grace. A live view re-rendering @@ -2096,6 +2100,25 @@ async function execInPageWithSettleGrace( } } +/** + * Lets a page finish rendering what an action started (a route change, a fetched list) before it + * is observed, so the observation is not a half-rendered page. A page already quiet costs one probe. + */ +async function waitForDomQuiet(target: PageExecutionTarget, executionDeadline?: number) { + const until = Math.min( + Date.now() + DOM_SETTLE_MAX_MS, + executionDeadline ?? Number.POSITIVE_INFINITY + ) + for (;;) { + const state = toRecord( + await execInPage(target, readDomQuiet, [], false, executionDeadline).catch(() => null) + ) + const quietMs = typeof state.quietMs === 'number' ? state.quietMs : DOM_QUIET_MS + if ((quietMs >= DOM_QUIET_MS && state.loading !== true) || Date.now() + 25 >= until) return + await sleep(Math.min(100, Math.max(25, DOM_QUIET_MS - quietMs))) + } +} + function pointFromPrepared(prepared: Record): { x: number; y: number } { return { x: prepared.x as number, y: prepared.y as number } } @@ -5460,15 +5483,20 @@ export async function executeTool( } return result }, - (query) => - executeToolInner( + async (query) => { + await waitForDomQuiet( + session.requireAutomationTab().view.webContents, + executionDeadline + ) + return executeToolInner( query === undefined ? 'browser_snapshot' : 'browser_find', query === undefined ? {} : { query }, assertCurrentExecution, executionDeadline, invocationEpoch, executionController.signal - ), + ) + }, assertCurrentExecution ) const cancellableExecution = Promise.race([execution, cancellation]) diff --git a/apps/desktop/src/main/browser-agent/page-functions.ts b/apps/desktop/src/main/browser-agent/page-functions.ts index a938eba740d..a5ca0874f9c 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.ts @@ -56,6 +56,9 @@ declare global { observedRoots: WeakSet }> __simAgentNextElementId?: number + /** When the document last changed structurally (nodes or text), for settling before an observation. */ + __simAgentDomChangedAt?: number + __simAgentDomWatcher?: MutationObserver /** Every ref this document issued, resolvable while its element keeps that identity. */ __simAgentIssuedRefs?: Map< number, @@ -85,12 +88,33 @@ export function serializePageCall(fn: (...args: never[]) => unknown, args: unkno return `((${String(installPageHelpers)})(), (${String(fn)}).apply(null, ${JSON.stringify(args)}))` } +/** How long the document has gone without a structural change, and whether it is still loading. */ +export function readDomQuiet(): unknown { + return { + quietMs: performance.now() - (window.__simAgentDomChangedAt ?? 0), + loading: document.readyState !== 'complete', + } +} + /** * Installs the helpers page functions share. Page functions are serialized and run standalone, * so the driver runs this installer before each one instead of inlining shared code into every * function. */ export function installPageHelpers(): void { + // Structural changes only: attribute churn (animations, hover styles) would keep a page "busy". + if (!window.__simAgentDomWatcher && document.documentElement) { + window.__simAgentDomChangedAt = performance.now() + const watcher = new MutationObserver(() => { + window.__simAgentDomChangedAt = performance.now() + }) + watcher.observe(document.documentElement, { + subtree: true, + childList: true, + characterData: true, + }) + window.__simAgentDomWatcher = watcher + } /** * The open modal each document aria-hid together with everything else. MUI's ModalManager * aria-hides every child except the modal's mount node, and a `disablePortal` modal From 3c29351c1de312ab5bf662801b665a0b406ef923 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 12 Nov 2026 21:21:44 -0800 Subject: [PATCH 2/4] fix(desktop-browser): settle honors cancellation, watches shadow roots and frames, waits through mid-navigation probes --- .../src/main/browser-agent/driver.test.ts | 61 +++++++++++++++++++ apps/desktop/src/main/browser-agent/driver.ts | 29 ++++++--- .../main/browser-agent/page-functions.test.ts | 18 ++++++ .../src/main/browser-agent/page-functions.ts | 28 ++++++--- 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index 99824218871..5c7e7fcb559 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -1894,6 +1894,7 @@ describe('credential protection', () => { activeElementSecrecy: 'safe', readActiveElementState: {}, readPageActionState: {}, + readDomQuiet: { quietMs: 1_000, loading: false }, }) const pageCall = vi.mocked(contents.executeJavaScript).getMockImplementation() let releaseObservation: (value: unknown) => void = () => {} @@ -3261,6 +3262,66 @@ describe('credential protection', () => { } ) + it('keeps settling through failed quiet probes, and never observes once cancelled', async () => { + const contents = await openPage() + const calls: string[] = [] + let probes = 0 + let failing = 2 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'readDomQuiet')) { + calls.push('probe') + probes++ + if (failing > 0) { + failing-- + vi.mocked(contents.isLoading).mockReturnValue(true) + return Promise.reject(new Error('Execution context was destroyed')) + } + vi.mocked(contents.isLoading).mockReturnValue(false) + return Promise.resolve({ quietMs: failing === -1 ? 0 : 400, loading: false }) + } + if (isPageCall(expression, 'collectSnapshot')) { + calls.push('snapshot') + return Promise.resolve({ + url: 'https://example.com/', + title: 'Example', + outline: '- heading "Results"', + truncated: false, + refIds: [], + refLineIndexes: {}, + nextElementId: 1_000, + }) + } + if (isPageCall(expression, 'clickElement')) return Promise.resolve(CLICK_TARGET) + return Promise.resolve(undefined) + }) + vi.useFakeTimers() + try { + const settled = driver.executeTool('chat-test', 'browser_click', { + elementId: 0, + observe: {}, + }) + await vi.advanceTimersByTimeAsync(5_000) + expect(await settled).toMatchObject({ ok: true, result: { observation: { ok: true } } }) + expect(calls.indexOf('snapshot')).toBe(3) + + calls.length = 0 + failing = -1 + const cancelled = driver.executeTool( + 'chat-test', + 'browser_click', + { elementId: 0, observe: { query: 'Results' } }, + 'click-cancelled' + ) + await vi.advanceTimersByTimeAsync(300) + expect(driver.cancelTool('chat-test', 'click-cancelled')).toBe(true) + await vi.advanceTimersByTimeAsync(5_000) + await cancelled + expect(calls).not.toContain('snapshot') + } finally { + vi.useRealTimers() + } + }) + it('acts on a ref an earlier snapshot of the same page issued', 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 50c14a2ff07..5a35e1d789e 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -2104,18 +2104,26 @@ async function execInPageWithSettleGrace( * Lets a page finish rendering what an action started (a route change, a fetched list) before it * is observed, so the observation is not a half-rendered page. A page already quiet costs one probe. */ -async function waitForDomQuiet(target: PageExecutionTarget, executionDeadline?: number) { +async function waitForDomQuiet( + target: PageExecutionTarget, + signal: AbortSignal, + executionDeadline?: number +) { const until = Math.min( Date.now() + DOM_SETTLE_MAX_MS, executionDeadline ?? Number.POSITIVE_INFINITY ) - for (;;) { - const state = toRecord( - await execInPage(target, readDomQuiet, [], false, executionDeadline).catch(() => null) + while (!signal.aborted && Date.now() + 25 < until) { + const probe = await execInPage(target, readDomQuiet, [], false, executionDeadline).catch( + () => null ) - const quietMs = typeof state.quietMs === 'number' ? state.quietMs : DOM_QUIET_MS - if ((quietMs >= DOM_QUIET_MS && state.loading !== true) || Date.now() + 25 >= until) return - await sleep(Math.min(100, Math.max(25, DOM_QUIET_MS - quietMs))) + // A failed probe while the tab loads is a document swapping mid-navigation, so keep waiting; + // one that fails on a loaded tab (a page that cannot be scripted) has nothing to settle. + if (probe === null && !('isLoading' in target && target.isLoading())) return + const state = toRecord(probe) + const quietMs = typeof state.quietMs === 'number' ? state.quietMs : 0 + if (quietMs >= DOM_QUIET_MS && state.loading === false) return + await interruptibleSleep(Math.min(100, Math.max(25, DOM_QUIET_MS - quietMs)), signal) } } @@ -5486,8 +5494,15 @@ export async function executeTool( async (query) => { await waitForDomQuiet( session.requireAutomationTab().view.webContents, + executionController.signal, executionDeadline ) + assertCurrentExecution() + if (executionController.signal.aborted) { + throw new ToolError( + 'This browser action was cancelled before it could observe the page.' + ) + } return executeToolInner( query === undefined ? 'browser_snapshot' : 'browser_find', query === undefined ? {} : { query }, 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 2c394eba4b2..e527c3af909 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.test.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.test.ts @@ -17,6 +17,7 @@ import { readActiveElementState, readCheckableElementState, readChildFrameElementState, + readDomQuiet, readPageActionState, readPageText, readSelectElementState, @@ -526,6 +527,23 @@ describe('collectSnapshot', () => { expect(outline).not.toContain('button "x" [ref=999]') }) + it('counts changes inside a shadow root the snapshot walked as page activity', async () => { + document.body.innerHTML = '
' + const host = visible(document.querySelector('#host') as HTMLDivElement) + const shadow = host.attachShadow({ mode: 'open' }) + shadow.innerHTML = '' + visible(shadow.querySelector('button') as HTMLButtonElement) + collectSnapshot() + await new Promise((resolve) => setTimeout(resolve, 40)) + const before = (readDomQuiet() as { quietMs: number }).quietMs + + shadow.append(document.createElement('ul')) + await Promise.resolve() + + expect(before).toBeGreaterThanOrEqual(30) + expect((readDomQuiet() as { quietMs: number }).quietMs).toBeLessThan(before) + }) + it('marks elements that appeared since the previous snapshot as new', () => { document.body.innerHTML = '' visible(document.querySelector('button') as HTMLButtonElement) diff --git a/apps/desktop/src/main/browser-agent/page-functions.ts b/apps/desktop/src/main/browser-agent/page-functions.ts index a5ca0874f9c..4e726c82ee7 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.ts @@ -59,6 +59,8 @@ declare global { /** When the document last changed structurally (nodes or text), for settling before an observation. */ __simAgentDomChangedAt?: number __simAgentDomWatcher?: MutationObserver + /** Adds a document or open shadow root to the settle watcher, once. */ + __simAgentWatchDom: (root: Node) => void /** Every ref this document issued, resolvable while its element keeps that identity. */ __simAgentIssuedRefs?: Map< number, @@ -103,18 +105,22 @@ export function readDomQuiet(): unknown { */ export function installPageHelpers(): void { // Structural changes only: attribute churn (animations, hover styles) would keep a page "busy". - if (!window.__simAgentDomWatcher && document.documentElement) { + // Snapshots add the same-origin frames and open shadow roots they walk, which a document-level + // observer cannot see into. + if (!window.__simAgentDomWatcher) { window.__simAgentDomChangedAt = performance.now() - const watcher = new MutationObserver(() => { + window.__simAgentDomWatcher = new MutationObserver(() => { window.__simAgentDomChangedAt = performance.now() }) - watcher.observe(document.documentElement, { - subtree: true, - childList: true, - characterData: true, - }) - window.__simAgentDomWatcher = watcher } + const watcher = window.__simAgentDomWatcher + const watched = new WeakSet() + window.__simAgentWatchDom = (root: Node): void => { + if (watched.has(root)) return + watched.add(root) + watcher.observe(root, { subtree: true, childList: true, characterData: true }) + } + if (document.documentElement) window.__simAgentWatchDom(document.documentElement) /** * The open modal each document aria-hid together with everything else. MUI's ModalManager * aria-hides every child except the modal's mount node, and a `disablePortal` modal @@ -913,6 +919,7 @@ export function collectSnapshot( try { const innerDoc = (el as HTMLIFrameElement).contentDocument if (innerDoc?.body && isVisible(el)) { + window.__simAgentWatchDom(innerDoc.documentElement) if (!push(`${indent}- iframe:`)) return walk(innerDoc.body.childNodes, childDepth + 1, coveredText) } else if (scopedRoot && !innerDoc && visible) { @@ -925,7 +932,10 @@ export function collectSnapshot( } const shadow = (el as HTMLElement).shadowRoot - if (shadow) walk(shadow.childNodes, childDepth, coveredText) + if (shadow) { + window.__simAgentWatchDom(shadow) + walk(shadow.childNodes, childDepth, coveredText) + } walk(el.childNodes, childDepth, coveredText) } } From 54f1f07f6729b71cb67a26742b4e4b3a3b7d24b0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 12 Nov 2026 21:30:51 -0800 Subject: [PATCH 3/4] test(desktop-browser): drive the shadow-root settle test with a controlled clock --- .../main/browser-agent/page-functions.test.ts | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 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 e527c3af909..efc9711dfa8 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.test.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.test.ts @@ -528,20 +528,25 @@ describe('collectSnapshot', () => { }) it('counts changes inside a shadow root the snapshot walked as page activity', async () => { - document.body.innerHTML = '
' - const host = visible(document.querySelector('#host') as HTMLDivElement) - const shadow = host.attachShadow({ mode: 'open' }) - shadow.innerHTML = '' - visible(shadow.querySelector('button') as HTMLButtonElement) - collectSnapshot() - await new Promise((resolve) => setTimeout(resolve, 40)) - const before = (readDomQuiet() as { quietMs: number }).quietMs - - shadow.append(document.createElement('ul')) - await Promise.resolve() - - expect(before).toBeGreaterThanOrEqual(30) - expect((readDomQuiet() as { quietMs: number }).quietMs).toBeLessThan(before) + vi.useFakeTimers({ toFake: ['performance'] }) + try { + document.body.innerHTML = '
' + const host = visible(document.querySelector('#host') as HTMLDivElement) + const shadow = host.attachShadow({ mode: 'open' }) + shadow.innerHTML = '' + visible(shadow.querySelector('button') as HTMLButtonElement) + collectSnapshot() + await Promise.resolve() + vi.advanceTimersByTime(500) + expect((readDomQuiet() as { quietMs: number }).quietMs).toBeGreaterThanOrEqual(500) + + shadow.append(document.createElement('ul')) + await Promise.resolve() + + expect((readDomQuiet() as { quietMs: number }).quietMs).toBeLessThan(500) + } finally { + vi.useRealTimers() + } }) it('marks elements that appeared since the previous snapshot as new', () => { From d4d30de95a708765881a7a246b9dee037f9b1132 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 12 Nov 2026 21:37:44 -0800 Subject: [PATCH 4/4] test(desktop-browser): drop an unused counter --- apps/desktop/src/main/browser-agent/driver.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index 5c7e7fcb559..7c3ce842eaa 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -3265,12 +3265,10 @@ describe('credential protection', () => { it('keeps settling through failed quiet probes, and never observes once cancelled', async () => { const contents = await openPage() const calls: string[] = [] - let probes = 0 let failing = 2 vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { if (isPageCall(expression, 'readDomQuiet')) { calls.push('probe') - probes++ if (failing > 0) { failing-- vi.mocked(contents.isLoading).mockReturnValue(true)