Skip to content

Commit 23f493e

Browse files
committed
fix(desktop-e2e): rescue only a click whose final release hit a tab that then closed
- cdp.clickAt raises TabClosedOnReleaseError only when the final mouseReleased was sent to a live tab and the tab is destroyed when it rejects; the driver rescues only that error (no string match, no pre-input/press/hold cases), with a note that does not claim the page closed itself and doNotRetry - background-executor A: assert each call is claimed within Sim's 15 s pickup window measured from issue, and always release the gated commands
1 parent ba9238d commit 23f493e

3 files changed

Lines changed: 71 additions & 47 deletions

File tree

‎apps/desktop/e2e/background-executor.spec.ts‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -122,32 +122,41 @@ test.describe('background executor', () => {
122122
)
123123
const readConsent = launched.app.waitForEvent('window')
124124
const localRead = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
125-
await answerShellDialog(await readConsent, 'Allow folder')
126-
127-
await window.goto(`${sim.origin}/workspace/ws-other/home`)
128-
await window.reload()
129-
await window.goto(`${sim.origin}/workspace/${WORKSPACE}/chat/${CHAT_C}`)
130-
131-
// Chat B's first command holds its queue until released, so a claim that waited for the
132-
// calls ahead of it to run could never happen in time.
133-
await check("A: every call is claimed while chat B's first command still runs", async () => {
134-
await expect
135-
.poll(
136-
() =>
137-
[...sim.calls.values()]
138-
.filter((call) => call.claimedAt === undefined)
139-
.map((call) => call.toolCallId),
140-
{ timeout: SIM_PICKUP_GRACE_MS }
141-
)
142-
.toEqual([])
143-
})
125+
try {
126+
await answerShellDialog(await readConsent, 'Allow folder')
127+
128+
await window.goto(`${sim.origin}/workspace/ws-other/home`)
129+
await window.reload()
130+
await window.goto(`${sim.origin}/workspace/${WORKSPACE}/chat/${CHAT_C}`)
131+
132+
// Chat B's first command holds its queue until released, so a claim that waited for the
133+
// calls ahead of it to run could never happen in time.
134+
await check("A: every call is claimed while chat B's first command still runs", async () => {
135+
await expect
136+
.poll(
137+
() =>
138+
[...sim.calls.values()]
139+
.filter((call) => call.claimedAt === undefined)
140+
.map((call) => call.toolCallId),
141+
{ timeout: SIM_PICKUP_GRACE_MS }
142+
)
143+
.toEqual([])
144+
for (const call of sim.calls.values()) {
145+
expect(
146+
(call.claimedAt ?? Number.POSITIVE_INFINITY) - call.issuedAt,
147+
call.toolCallId
148+
).toBeLessThan(SIM_PICKUP_GRACE_MS)
149+
}
150+
})
144151

145-
await check('A: the lease is renewed while calls wait and run', async () => {
146-
await expect
147-
.poll(() => terminalRuns.filter((id) => sim.requireCall(id).renewals === 0))
148-
.toEqual([])
149-
})
150-
writeFileSync(releaseCommands, '')
152+
await check('A: the lease is renewed while calls wait and run', async () => {
153+
await expect
154+
.poll(() => terminalRuns.filter((id) => sim.requireCall(id).renewals === 0))
155+
.toEqual([])
156+
})
157+
} finally {
158+
writeFileSync(releaseCommands, '')
159+
}
151160

152161
await check('A: every call completes exactly once with its own token', async () => {
153162
for (const id of [...clicks, ...terminalRuns, localRead]) {

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

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,17 @@ export function clearAgentContextMenu(contents: WebContents): void {
10931093
agentContextClicks.delete(contents)
10941094
}
10951095

1096+
/**
1097+
* 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.
1099+
*/
1100+
export class TabClosedOnReleaseError extends Error {
1101+
constructor() {
1102+
super('The tab closed as the click was released.')
1103+
this.name = 'TabClosedOnReleaseError'
1104+
}
1105+
}
1106+
10961107
/**
10971108
* Clicks at viewport coordinates. An already-aborted `signal` rejects before any input is sent.
10981109
* During a press-and-hold it ends the hold early: the click rejects with the abort reason and the
@@ -1140,15 +1151,21 @@ export async function clickAt(
11401151
}
11411152
signal?.throwIfAborted()
11421153
}
1143-
await sendInput(contents, 'Input.dispatchMouseEvent', {
1144-
type: 'mouseReleased',
1145-
x,
1146-
y,
1147-
button,
1148-
buttons: 0,
1149-
modifiers,
1150-
clickCount: count,
1151-
})
1154+
const finalReleaseToLiveTab = count === clickCount && !contents.isDestroyed()
1155+
try {
1156+
await sendInput(contents, 'Input.dispatchMouseEvent', {
1157+
type: 'mouseReleased',
1158+
x,
1159+
y,
1160+
button,
1161+
buttons: 0,
1162+
modifiers,
1163+
clickCount: count,
1164+
})
1165+
} catch (error) {
1166+
if (finalReleaseToLiveTab && contents.isDestroyed()) throw new TabClosedOnReleaseError()
1167+
throw error
1168+
}
11521169
}
11531170
pressed = false
11541171
} finally {

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,32 +1803,30 @@ function navigationRescue(
18031803
}
18041804

18051805
/**
1806-
* A page that closes itself from the click's own handler (a sign-in popup's Allow that calls
1807-
* window.close()) can destroy its tab before Chromium acknowledges the release, and Electron then
1808-
* rejects the pending input command with "target closed while handling command" although the press
1809-
* landed. The same click otherwise reads as a failed dispatch whenever the close wins that race.
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.
18101809
*/
18111810
function closedTabRescue(
1812-
contents: WebContents,
18131811
error: unknown,
18141812
base: { trusted: boolean; activation: string }
18151813
): Record<string, unknown> | null {
1816-
if (!contents.isDestroyed() && !getErrorMessage(error).includes('target closed')) return null
1814+
if (!(error instanceof cdp.TabClosedOnReleaseError)) return null
18171815
const activeTab = session.automationTab()
1818-
const nowActive = activeTab && activeTab.view.webContents !== contents ? activeTab : null
18191816
return {
18201817
dispatched: true,
18211818
trusted: base.trusted,
18221819
activation: base.activation,
18231820
effectObserved: true,
18241821
possibleEffectObserved: true,
18251822
closedDuringDispatch: true,
1823+
doNotRetry: true,
18261824
effect: { tabChanged: true },
18271825
dialogs: [],
1828-
...(nowActive
1829-
? { activeTab: { tabId: nowActive.id, url: nowActive.view.webContents.getURL() } }
1826+
...(activeTab
1827+
? { activeTab: { tabId: activeTab.id, url: activeTab.view.webContents.getURL() } }
18301828
: {}),
1831-
note: 'The tab closed while the click was being delivered, which is how a page that closes itself in response (a sign-in or permission popup) ends; that is why no postconditions are reported. Take a fresh browser_snapshot of the tab that is now active.',
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.',
18321830
}
18331831
}
18341832

@@ -3329,7 +3327,7 @@ async function executeToolInner(
33293327
const nativePointer = { trusted: true, activation: 'native-pointer' }
33303328
const rescued =
33313329
navigationRescue(contents, epochAtDispatch, urlAtDispatch, nativePointer) ??
3332-
closedTabRescue(contents, error, nativePointer)
3330+
closedTabRescue(error, nativePointer)
33333331
if (rescued) return rescued
33343332
throw new ToolError(
33353333
`Native click dispatch failed (${getErrorMessage(error)}). The action was not retried because a partial pointer press may already have reached the page. Take a fresh snapshot before continuing.`
@@ -3371,7 +3369,7 @@ async function executeToolInner(
33713369
const nativePointer = { trusted: true, activation: 'native-pointer' }
33723370
const rescued =
33733371
navigationRescue(contents, epochAtDispatch, urlAtDispatch, nativePointer) ??
3374-
closedTabRescue(contents, error, nativePointer)
3372+
closedTabRescue(error, nativePointer)
33753373
if (rescued) return rescued
33763374
throw new ToolError(
33773375
`Native framed click dispatch failed (${getErrorMessage(error)}). The action was not retried because a partial pointer press may already have reached the page. Take a fresh snapshot before continuing.`
@@ -4796,7 +4794,7 @@ async function executeToolInner(
47964794
const nativePointer = { trusted: true, activation: 'native-pointer' }
47974795
const rescued =
47984796
navigationRescue(contents, clickNavigationEpoch, urlAtDispatch, nativePointer) ??
4799-
closedTabRescue(contents, error, nativePointer)
4797+
closedTabRescue(error, nativePointer)
48004798
if (rescued) return rescued
48014799
throw new ToolError(
48024800
`Native click dispatch failed (${getErrorMessage(error)}). The action was not retried because a partial pointer press may already have reached the page. Take a fresh snapshot before continuing.`

0 commit comments

Comments
 (0)