Skip to content

Commit 4a7ede1

Browse files
committed
fix(desktop): report a click whose tab closed mid-release as unconfirmed
The tab being destroyed proves teardown, not that Chromium processed the release, so the rescued result no longer claims effectObserved and its note says the outcome could not be confirmed. Adds clickAt tests for each TabClosedOnReleaseError guard and a driver test for the rescued shape.
1 parent 23f493e commit 4a7ede1

4 files changed

Lines changed: 125 additions & 7 deletions

File tree

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
releaseFileInput,
2323
resolveFileInput,
2424
setFileInputFiles,
25+
TabClosedOnReleaseError,
2526
} from '@/main/browser-agent/cdp'
2627

2728
function createOopifFrameFixture() {
@@ -252,6 +253,78 @@ describe('browser-agent CDP instrumentation', () => {
252253
])
253254
})
254255

256+
describe('a tab that closes during a click', () => {
257+
/**
258+
* Rejects the mouse event matching `type` and `clickCount` with `failure`, first marking the
259+
* tab destroyed when `destroy` is `'at-rejection'`. `'before-release'` instead marks it
260+
* destroyed as the press resolves, so the release is sent to an already-dead tab.
261+
*/
262+
function failMouseEvent(
263+
contents: WebContents,
264+
fail: { type: string; clickCount: number },
265+
failure: Error,
266+
destroy: 'at-rejection' | 'before-release' | 'never'
267+
) {
268+
let destroyed = false
269+
vi.mocked(contents.isDestroyed).mockImplementation(() => destroyed)
270+
vi.mocked(contents.debugger.sendCommand).mockImplementation(async (_method, params) => {
271+
const { type, clickCount } = toRecord(params)
272+
if (destroy === 'before-release' && type === 'mousePressed') destroyed = true
273+
if (type === fail.type && clickCount === fail.clickCount) {
274+
if (destroy === 'at-rejection') destroyed = true
275+
throw failure
276+
}
277+
return {}
278+
})
279+
}
280+
281+
it('reports a final release rejected because the tab closed as TabClosedOnReleaseError', async () => {
282+
const contents = new WebContentsView().webContents
283+
failMouseEvent(
284+
contents,
285+
{ type: 'mouseReleased', clickCount: 1 },
286+
new Error('Target closed'),
287+
'at-rejection'
288+
)
289+
290+
await expect(clickAt(contents, 12, 24)).rejects.toBeInstanceOf(TabClosedOnReleaseError)
291+
})
292+
293+
it('keeps the original error when the release fails on a tab that is still alive', async () => {
294+
const contents = new WebContentsView().webContents
295+
const failure = new Error('release rejected')
296+
failMouseEvent(contents, { type: 'mouseReleased', clickCount: 1 }, failure, 'never')
297+
298+
await expect(clickAt(contents, 12, 24)).rejects.toBe(failure)
299+
})
300+
301+
it('keeps the original error when the tab was already gone before the release was sent', async () => {
302+
const contents = new WebContentsView().webContents
303+
const failure = new Error('Target closed')
304+
failMouseEvent(contents, { type: 'mouseReleased', clickCount: 1 }, failure, 'before-release')
305+
306+
await expect(clickAt(contents, 12, 24)).rejects.toBe(failure)
307+
})
308+
309+
it('keeps the original error when the press fails as the tab closes', async () => {
310+
const contents = new WebContentsView().webContents
311+
const failure = new Error('Target closed')
312+
failMouseEvent(contents, { type: 'mousePressed', clickCount: 1 }, failure, 'at-rejection')
313+
314+
await expect(clickAt(contents, 12, 24)).rejects.toBe(failure)
315+
})
316+
317+
it('keeps the original error when a non-final release of a multi-click fails as the tab closes', async () => {
318+
const contents = new WebContentsView().webContents
319+
const failure = new Error('Target closed')
320+
failMouseEvent(contents, { type: 'mouseReleased', clickCount: 1 }, failure, 'at-rejection')
321+
322+
await expect(
323+
clickAt(contents, 12, 24, true, { ...PRIMARY_CLICK, clickCount: 2 })
324+
).rejects.toBe(failure)
325+
})
326+
})
327+
255328
it('times out a hung press and sends cleanup before the tool watchdog can release', async () => {
256329
vi.useFakeTimers()
257330
try {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,11 +1095,12 @@ export function clearAgentContextMenu(contents: WebContents): void {
10951095

10961096
/**
10971097
* The final release of a click was sent to a live tab, and the tab was gone when that release
1098-
* was rejected. The press and release both reached the page; only the acknowledgement was lost.
1098+
* was rejected: the release was in flight when the tab closed. Whether it reached the page is
1099+
* unknown, so this proves neither delivery nor failure, only that the click must not be repeated.
10991100
*/
11001101
export class TabClosedOnReleaseError extends Error {
11011102
constructor() {
1102-
super('The tab closed as the click was released.')
1103+
super('The tab closed while the click release was in flight.')
11031104
this.name = 'TabClosedOnReleaseError'
11041105
}
11051106
}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2344,6 +2344,50 @@ describe('credential protection', () => {
23442344
})
23452345
})
23462346

2347+
it('reports a click whose tab closed during its release as unconfirmed, not failed', async () => {
2348+
const contents = await openPage()
2349+
let destroyed = false
2350+
vi.mocked(contents.isDestroyed).mockImplementation(() => destroyed)
2351+
vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => {
2352+
if (isPageCall(expression, 'describePointTarget')) {
2353+
return Promise.resolve({ found: true, element: 'Close', cursor: 'pointer' })
2354+
}
2355+
if (isPageCall(expression, 'readActiveElementState')) return Promise.resolve({})
2356+
if (isPageCall(expression, 'readPageActionState')) {
2357+
return Promise.resolve({
2358+
url: 'https://example.com/popup',
2359+
title: 'Popup',
2360+
focus: 'body',
2361+
mutationRevision: 0,
2362+
dialogs: [],
2363+
popups: [],
2364+
scroll: [0],
2365+
})
2366+
}
2367+
return Promise.resolve(undefined)
2368+
})
2369+
vi.mocked(contents.debugger.sendCommand).mockImplementation(async (method, params) => {
2370+
if (method === 'Input.dispatchMouseEvent' && toRecord(params).type === 'mouseReleased') {
2371+
destroyed = true
2372+
throw new Error('Target closed')
2373+
}
2374+
return {}
2375+
})
2376+
2377+
const result = await driver.executeTool('chat-test', 'browser_click_at', { x: 100, y: 200 })
2378+
2379+
expect(result).toMatchObject({
2380+
ok: true,
2381+
result: {
2382+
dispatched: true,
2383+
closedDuringDispatch: true,
2384+
doNotRetry: true,
2385+
possibleEffectObserved: true,
2386+
},
2387+
})
2388+
expect(toRecord(result.result)).not.toHaveProperty('effectObserved')
2389+
})
2390+
23472391
it('reports navigation obstructed by a dialog that opened with it', async () => {
23482392
const contents = await openPage()
23492393
let actionReads = 0

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,9 +1803,10 @@ function navigationRescue(
18031803
}
18041804

18051805
/**
1806-
* The click's final release was sent to a live tab that was destroyed before Chromium
1807-
* acknowledged it (cdp.clickAt raises TabClosedOnReleaseError only then), so the press and
1808-
* release reached the page. Any other dispatch failure keeps its own error.
1806+
* The tab closed while the click's final release was in flight (cdp.clickAt raises
1807+
* TabClosedOnReleaseError only then). Whether the release reached the page cannot be confirmed,
1808+
* so the result reports the tab change but not the click's effect, and forbids a repeat. Any other
1809+
* dispatch failure keeps its own error.
18091810
*/
18101811
function closedTabRescue(
18111812
error: unknown,
@@ -1817,7 +1818,6 @@ function closedTabRescue(
18171818
dispatched: true,
18181819
trusted: base.trusted,
18191820
activation: base.activation,
1820-
effectObserved: true,
18211821
possibleEffectObserved: true,
18221822
closedDuringDispatch: true,
18231823
doNotRetry: true,
@@ -1826,7 +1826,7 @@ function closedTabRescue(
18261826
...(activeTab
18271827
? { activeTab: { tabId: activeTab.id, url: activeTab.view.webContents.getURL() } }
18281828
: {}),
1829-
note: 'The tab closed as the click was released, so no postconditions could be read. Do not repeat the click. If this page was not expected to close itself, verify the outcome with browser_snapshot before relying on it.',
1829+
note: "The tab closed while the click's release was in flight, so the click's outcome could not be confirmed. Do not repeat the click; verify the outcome with browser_snapshot before relying on it.",
18301830
}
18311831
}
18321832

0 commit comments

Comments
 (0)