Skip to content

Commit 140b92c

Browse files
authored
fix(desktop-browser): give a wait's closing snapshot a full budget and confirm ref reuse names the same element (#9966)
- browser_wait_for's native watchdog adds a standalone snapshot's 20s budget for the snapshot a wait now ends with; the renderer's wait grace widens to 30s to cover it - Ref reuse also requires the stored entry to point at the same element - Rename a local that shadowed elementTarget
1 parent 016755f commit 140b92c

6 files changed

Lines changed: 57 additions & 21 deletions

File tree

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

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,13 +1507,13 @@ describe('browserToolWatchdogMs', () => {
15071507
})
15081508

15091509
it.each([
1510-
['number', 30_000, 35_000],
1511-
['numeric string', '30000', 35_000],
1512-
['absent', undefined, 15_000],
1513-
['non-numeric', 'soon', 15_000],
1514-
['zero', 0, 15_000],
1515-
['negative', -5_000, 15_000],
1516-
['above the wait clamp', 500_000, 125_000],
1510+
['number', 30_000, 55_000],
1511+
['numeric string', '30000', 55_000],
1512+
['absent', undefined, 35_000],
1513+
['non-numeric', 'soon', 35_000],
1514+
['zero', 0, 35_000],
1515+
['negative', -5_000, 35_000],
1516+
['above the wait clamp', 500_000, 145_000],
15171517
])('normalizes browser_wait_for timeout (%s)', (_label, timeoutMs, expected) => {
15181518
const params = timeoutMs === undefined ? {} : { timeoutMs }
15191519

@@ -3098,6 +3098,39 @@ describe('credential protection', () => {
30983098
})
30993099
})
31003100

3101+
it('gives a wait that runs to its timeout a full snapshot budget for its closing observation', async () => {
3102+
const contents = await openPage()
3103+
let finishSnapshot: (value: unknown) => void = () => {}
3104+
vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => {
3105+
if (!isPageCall(expression, 'collectSnapshot')) return Promise.resolve(undefined)
3106+
return new Promise((resolve) => {
3107+
finishSnapshot = resolve
3108+
})
3109+
})
3110+
vi.useFakeTimers()
3111+
try {
3112+
const waiting = driver.executeTool('chat-test', 'browser_wait_for', { timeoutMs: 1_000 })
3113+
await vi.advanceTimersByTimeAsync(9_000)
3114+
finishSnapshot({
3115+
url: 'https://example.com/',
3116+
title: 'Example',
3117+
outline: '- heading "Order confirmed"',
3118+
truncated: false,
3119+
refIds: [],
3120+
refLineIndexes: {},
3121+
nextElementId: 1_000,
3122+
})
3123+
await vi.advanceTimersByTimeAsync(1_000)
3124+
3125+
expect(await waiting).toMatchObject({
3126+
ok: true,
3127+
result: { observation: { ok: true, result: { outline: '- heading "Order confirmed"' } } },
3128+
})
3129+
} finally {
3130+
vi.useRealTimers()
3131+
}
3132+
})
3133+
31013134
it('accepts a later snapshot that keeps refs this scope issued for the same page', async () => {
31023135
const contents = await openPage()
31033136
const snapshot = (refIds: number[], nextElementId: number) => ({

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,8 +1161,9 @@ export function browserToolWatchdogMs(
11611161
return BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS
11621162
}
11631163
if (tool === 'browser_wait_for') {
1164+
// A wait ends with a snapshot, which gets the same budget a standalone snapshot does.
11641165
const requested = normalizeBrowserWaitForTimeoutMs(params.timeoutMs)
1165-
return requested + WAIT_FOR_TOOL_WATCHDOG_GRACE_MS
1166+
return requested + WAIT_FOR_TOOL_WATCHDOG_GRACE_MS + DEFAULT_TOOL_WATCHDOG_MS
11661167
}
11671168
return DEFAULT_TOOL_WATCHDOG_MS
11681169
}
@@ -2971,8 +2972,8 @@ async function executeToolInner(
29712972
}
29722973
const waitedTab = session.requireAutomationTab()
29732974
const contents = waitedTab.view.webContents
2974-
const elementTarget = elementId === undefined ? undefined : pageTargetForElement(elementId)
2975-
if (elementTarget && elementTarget !== contents) {
2975+
const waitTarget = elementId === undefined ? undefined : pageTargetForElement(elementId)
2976+
if (waitTarget && waitTarget !== contents) {
29762977
throw new ToolError(
29772978
'Element-state waits are limited to the top page. Use a text or URL condition for framed content.'
29782979
)
@@ -2982,7 +2983,7 @@ async function executeToolInner(
29822983
const assertWaitTargetIsCurrent = (): void => {
29832984
assertCurrentExecution()
29842985
if (
2985-
elementTarget &&
2986+
waitTarget &&
29862987
(navigationEpoch(contents) !== waitedNavigationEpoch || contents.getURL() !== waitedUrl)
29872988
) {
29882989
throw new ToolError(
@@ -3025,12 +3026,12 @@ async function executeToolInner(
30253026
}
30263027
const urlMatched = !urlContains || contents.getURL().includes(urlContains)
30273028
let elementMatched = elementId === undefined
3028-
if (elementId !== undefined && elementState && elementTarget) {
3029+
if (elementId !== undefined && elementState && waitTarget) {
30293030
assertWaitTargetIsCurrent()
30303031
const state = toRecord(
30313032
unwrapPageResult(
30323033
await execInPage(
3033-
elementTarget,
3034+
waitTarget,
30343035
readPageActionState,
30353036
[false, elementId, 'registered'],
30363037
false,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,7 @@ export function collectSnapshot(
645645
const unchanged =
646646
prior !== undefined &&
647647
prior.namespace === refNamespace &&
648+
prior.element.deref() === el &&
648649
prior.locator.tag === locator.tag &&
649650
prior.locator.role === locator.role &&
650651
prior.locator.name === locator.name &&

‎apps/sim/lib/mothership/request/tools/executor.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,14 +266,14 @@ describe('pendingToolWaitBudgetMs', () => {
266266
})
267267

268268
it('matches the requested browser_wait_for renderer budget', () => {
269-
expect(pendingToolWaitBudgetMs({ name: 'browser_wait_for', status: 'executing' })).toBe(85_000)
269+
expect(pendingToolWaitBudgetMs({ name: 'browser_wait_for', status: 'executing' })).toBe(100_000)
270270
expect(
271271
pendingToolWaitBudgetMs({
272272
name: 'browser_wait_for',
273273
status: 'executing',
274274
params: { timeoutMs: 120_000 },
275275
})
276-
).toBe(195_000)
276+
).toBe(210_000)
277277
})
278278

279279
it('outlasts the wait window the desktop holds a terminal run for', () => {

‎apps/sim/lib/mothership/tools/client/browser-tool-execution.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,11 +1289,11 @@ describe('executeBrowserToolOnClient', () => {
12891289
* native queue while the desktop is still honoring that same wait.
12901290
*/
12911291
it.each([
1292-
['number', 30_000, 105_000],
1293-
['numeric string', '30000', 105_000],
1294-
['non-numeric', 'soon', 85_000],
1295-
['negative', -5_000, 85_000],
1296-
['above the desktop clamp', 500_000, 195_000],
1292+
['number', 30_000, 120_000],
1293+
['numeric string', '30000', 120_000],
1294+
['non-numeric', 'soon', 100_000],
1295+
['negative', -5_000, 100_000],
1296+
['above the desktop clamp', 500_000, 210_000],
12971297
])(
12981298
'budgets browser_wait_for above the desktop wait (%s)',
12991299
async (_label, timeoutMs, expected) => {

‎packages/browser-protocol/src/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ export type BrowserToolName = (typeof BROWSER_TOOL_NAMES)[number]
6565

6666
const BROWSER_WAIT_FOR_DEFAULT_TIMEOUT_MS = 10_000
6767
const BROWSER_WAIT_FOR_MAX_TIMEOUT_MS = 120_000
68-
const BROWSER_WAIT_FOR_RENDERER_GRACE_MS = 15_000
68+
/** Covers the desktop's wait grace and the snapshot a wait ends with (25s), plus transport. */
69+
const BROWSER_WAIT_FOR_RENDERER_GRACE_MS = 30_000
6970
export const BROWSER_TOOL_AUTHORIZATION_TIMEOUT_MS = 8_000
7071
export const BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS = 60_000
7172
export const BROWSER_TOOL_QUEUE_WAIT_TIMEOUT_MS = BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS

0 commit comments

Comments
 (0)