Skip to content

Commit 70882e6

Browse files
committed
fix(desktop-browser): give a wait's closing snapshot a full budget and confirm ref reuse names the same element
1 parent 005cb97 commit 70882e6

3 files changed

Lines changed: 49 additions & 13 deletions

File tree

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

Lines changed: 41 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,40 @@ 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+
vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => {
3104+
if (!isPageCall(expression, 'collectSnapshot')) return Promise.resolve(undefined)
3105+
return new Promise((resolve) =>
3106+
setTimeout(
3107+
() =>
3108+
resolve({
3109+
url: 'https://example.com/',
3110+
title: 'Example',
3111+
outline: '- heading "Order confirmed"',
3112+
truncated: false,
3113+
refIds: [],
3114+
refLineIndexes: {},
3115+
nextElementId: 1_000,
3116+
}),
3117+
8_000
3118+
)
3119+
)
3120+
})
3121+
vi.useFakeTimers()
3122+
try {
3123+
const waiting = driver.executeTool('chat-test', 'browser_wait_for', { timeoutMs: 1_000 })
3124+
await vi.advanceTimersByTimeAsync(30_000)
3125+
3126+
expect(await waiting).toMatchObject({
3127+
ok: true,
3128+
result: { observation: { ok: true, result: { outline: '- heading "Order confirmed"' } } },
3129+
})
3130+
} finally {
3131+
vi.useRealTimers()
3132+
}
3133+
})
3134+
31013135
it('accepts a later snapshot that keeps refs this scope issued for the same page', async () => {
31023136
const contents = await openPage()
31033137
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 &&

0 commit comments

Comments
 (0)