Skip to content

Commit da7decb

Browse files
committed
fix(desktop-browser): ask the user before a page's alert, confirm, or leave-site prompt is answered
1 parent 113a204 commit da7decb

8 files changed

Lines changed: 686 additions & 53 deletions

File tree

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
2+
import { createServer, type Server } from 'node:http'
3+
import { tmpdir } from 'node:os'
4+
import { dirname, join } from 'node:path'
5+
import { fileURLToPath } from 'node:url'
6+
import {
7+
type ElectronApplication,
8+
_electron as electron,
9+
expect,
10+
type Page,
11+
test,
12+
} from '@playwright/test'
13+
import type { BrowserPageDialog, BrowserToolName } from '@sim/browser-protocol'
14+
import type { SimDesktopApi } from '@sim/desktop-bridge'
15+
import { getErrorMessage } from '@sim/utils/errors'
16+
17+
const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url))
18+
const SCOPE = 'browser-page-dialogs-e2e'
19+
const SHELL_FIXTURE = '<!doctype html><title>Sim fixture</title><h1>Browser dialogs fixture</h1>'
20+
const FORM_FIXTURE = `<!doctype html><title>form</title>
21+
<input id="draft" aria-label="Draft">
22+
<button id="delete" onclick="document.title = 'confirm:' + confirm('Delete the report?')">Delete</button>
23+
<script>addEventListener('beforeunload', (event) => {
24+
if (document.getElementById('draft').value) { event.preventDefault(); event.returnValue = '' }
25+
})</script>`
26+
27+
type Bridge = typeof globalThis & {
28+
simDesktop: SimDesktopApi
29+
pageDialog?: BrowserPageDialog | null
30+
}
31+
32+
/**
33+
* A page's alert, confirm, and leave-site question belong to whoever is using
34+
* the page: the user on the tab they are looking at, the agent during its own
35+
* action. These checks drive the real shell and native tab views.
36+
*/
37+
test('page dialogs wait for the user on their page and stay automatic for the agent', async () => {
38+
const reportPath =
39+
process.env.DESKTOP_BROWSER_DIALOGS_REPORT_PATH ??
40+
test.info().outputPath('browser-page-dialogs.json')
41+
const checks: {
42+
name: string
43+
status: 'passed' | 'failed'
44+
durationMs: number
45+
error?: string
46+
}[] = []
47+
const check = async (name: string, run: () => Promise<void>) => {
48+
const started = Date.now()
49+
try {
50+
await test.step(name, run)
51+
checks.push({ name, status: 'passed', durationMs: Date.now() - started })
52+
} catch (error) {
53+
checks.push({
54+
name,
55+
status: 'failed',
56+
durationMs: Date.now() - started,
57+
error: getErrorMessage(error),
58+
})
59+
throw error
60+
}
61+
}
62+
const calls = new Map<string, { chatId: string; toolName: BrowserToolName; args: unknown }>()
63+
const server: Server = createServer(async (request, response) => {
64+
const path = new URL(request.url ?? '/', 'http://localhost').pathname
65+
if (path === '/api/desktop/tool/authorize') {
66+
let body = ''
67+
for await (const chunk of request) body += chunk.toString()
68+
const authorization = calls.get(JSON.parse(body).toolCallId)
69+
response.writeHead(authorization ? 200 : 403, { 'Content-Type': 'application/json' })
70+
response.end(JSON.stringify(authorization ?? {}))
71+
return
72+
}
73+
// The app origin serves the shell; the same server on localhost is the web.
74+
const isSite = request.headers.host?.startsWith('localhost') === true
75+
response.writeHead(200, {
76+
'Content-Type': 'text/html',
77+
...(isSite ? {} : { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; Path=/' }),
78+
})
79+
response.end(
80+
!isSite
81+
? SHELL_FIXTURE
82+
: path === '/form'
83+
? FORM_FIXTURE
84+
: '<!doctype html><title>next</title>'
85+
)
86+
})
87+
const userData = mkdtempSync(join(tmpdir(), 'sim-browser-dialogs-e2e-'))
88+
let app: ElectronApplication | undefined
89+
let passed = false
90+
try {
91+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
92+
const address = server.address()
93+
if (!address || typeof address === 'string') throw new Error('Missing fixture address')
94+
const origin = `http://127.0.0.1:${address.port}`
95+
const site = origin.replace('127.0.0.1', 'localhost')
96+
const shellApp = await electron.launch({
97+
args: [process.env.SIM_DESKTOP_E2E_MAIN ?? '.'],
98+
cwd: DESKTOP_DIR,
99+
env: { ...process.env, SIM_DESKTOP_ORIGIN: origin, SIM_DESKTOP_USER_DATA: userData },
100+
})
101+
app = shellApp
102+
// A dialog listener stops Playwright auto-dismissing page dialogs, so the desktop's own
103+
// handling decides their outcome exactly as it does in production.
104+
const leaveDialogsToDesktop = (page: Page) => page.on('dialog', () => {})
105+
shellApp.context().pages().forEach(leaveDialogsToDesktop)
106+
shellApp.context().on('page', leaveDialogsToDesktop)
107+
const shell = await shellApp.firstWindow()
108+
await expect(shell.getByRole('heading')).toHaveText('Browser dialogs fixture')
109+
await shell.evaluate(async (scope) => {
110+
const bridge = globalThis as Bridge
111+
const api = bridge.simDesktop.browserAgent
112+
await api.activateScope(scope)
113+
const updateBounds = () =>
114+
api.setPanelBounds(
115+
{ x: 0, y: 80, width: innerWidth, height: innerHeight - 80 },
116+
null,
117+
scope
118+
)
119+
updateBounds()
120+
window.setInterval(updateBounds, 200)
121+
bridge.pageDialog = null
122+
api.onPageState((state) => {
123+
bridge.pageDialog = state.dialog ?? null
124+
})
125+
}, SCOPE)
126+
127+
let callCount = 0
128+
const execute = async (tool: BrowserToolName, args: Record<string, unknown>) => {
129+
const callId = `browser-dialogs-${++callCount}`
130+
calls.set(callId, { chatId: SCOPE, toolName: tool, args })
131+
const result = await shell.evaluate(
132+
({ callId, tool, args, scope }) =>
133+
(globalThis as Bridge).simDesktop.browserAgent.executeTool(callId, tool, args, scope),
134+
{ callId, tool, args, scope: SCOPE }
135+
)
136+
expect(result.ok).toBe(true)
137+
return result.ok ? result.result : undefined
138+
}
139+
/** Browser-chrome actions are user gestures, so each follows a real click in Sim. */
140+
const panelAction = async (action: Record<string, unknown>) => {
141+
await shell.getByRole('heading').click()
142+
await shell.evaluate(
143+
({ action, scope }) =>
144+
(globalThis as Bridge).simDesktop.browserAgent.panelAction(
145+
action as unknown as Parameters<SimDesktopApi['browserAgent']['panelAction']>[0],
146+
scope
147+
),
148+
{ action, scope: SCOPE }
149+
)
150+
}
151+
const pageDialog = () => shell.evaluate(() => (globalThis as Bridge).pageDialog ?? null)
152+
const inPage = <T>(script: string) =>
153+
shellApp.evaluate(
154+
async ({ webContents }, { script, url }) =>
155+
(await webContents
156+
.getAllWebContents()
157+
.find((contents) => contents.getURL().startsWith(url))
158+
?.executeJavaScript(script)) as T,
159+
{ script, url: site }
160+
)
161+
/** Read from the shell: a script cannot run in a page while its dialog is open. */
162+
const pageTitle = () =>
163+
shellApp.evaluate(
164+
({ webContents }, url) =>
165+
webContents
166+
.getAllWebContents()
167+
.find((contents) => contents.getURL().startsWith(url))
168+
?.getTitle() ?? null,
169+
site
170+
)
171+
const pageUrl = () =>
172+
shellApp.evaluate(
173+
({ webContents }, url) =>
174+
webContents
175+
.getAllWebContents()
176+
.find((contents) => contents.getURL().startsWith(url))
177+
?.getURL() ?? null,
178+
site
179+
)
180+
/** Trusted input into the page, the way the user's own mouse and keys arrive. */
181+
const userInput = (selector: string, text = '') =>
182+
shellApp.evaluate(
183+
async ({ webContents }, { selector, text, url }) => {
184+
const contents = webContents
185+
.getAllWebContents()
186+
.find((candidate) => candidate.getURL().startsWith(url))
187+
if (!contents) throw new Error('No page')
188+
const rect = JSON.parse(
189+
await contents.executeJavaScript(
190+
`JSON.stringify(document.querySelector(${JSON.stringify(selector)}).getBoundingClientRect())`
191+
)
192+
)
193+
const point = { x: Math.round(rect.x + 5), y: Math.round(rect.y + 5) }
194+
contents.sendInputEvent({ type: 'mouseDown', ...point, button: 'left', clickCount: 1 })
195+
contents.sendInputEvent({ type: 'mouseUp', ...point, button: 'left', clickCount: 1 })
196+
for (const character of text)
197+
contents.sendInputEvent({ type: 'char', keyCode: character })
198+
},
199+
{ selector, text, url: site }
200+
)
201+
202+
await check('without a renderer that shows dialogs, the shell still answers them', async () => {
203+
await execute('browser_open_url', { url: `${site}/form` })
204+
await panelAction({ action: 'switch-tab', tabId: '1' })
205+
await userInput('#delete')
206+
await expect.poll(() => pageTitle()).toBe('confirm:false')
207+
expect(await pageDialog()).toBeNull()
208+
})
209+
210+
await panelAction({ action: 'enable-page-dialogs' })
211+
await inPage("document.title = 'form'")
212+
213+
await check("the user's confirm waits for their answer", async () => {
214+
await userInput('#delete')
215+
await expect
216+
.poll(pageDialog)
217+
.toMatchObject({ kind: 'confirm', message: 'Delete the report?' })
218+
expect(await pageTitle()).toBe('form')
219+
const dialog = await pageDialog()
220+
await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: true })
221+
await expect.poll(() => pageTitle()).toBe('confirm:true')
222+
await expect.poll(pageDialog).toBeNull()
223+
})
224+
225+
await check("the agent's dialogs never wait on the user", async () => {
226+
await inPage("document.title = 'form'")
227+
const snapshot = await execute('browser_snapshot', {})
228+
const ref = /button "Delete" \[ref=(\d+)\]/.exec(
229+
String((snapshot as { outline?: string }).outline)
230+
)?.[1]
231+
expect(ref, 'snapshot lists the Delete button').toBeTruthy()
232+
await execute('browser_click', { elementId: Number(ref) })
233+
await expect.poll(() => pageTitle()).toBe('confirm:false')
234+
expect(await pageDialog()).toBeNull()
235+
await inPage("document.title = 'form'")
236+
await execute('browser_click', { elementId: Number(ref), dialog: { accept: true } })
237+
await expect.poll(() => pageTitle()).toBe('confirm:true')
238+
expect(await pageDialog()).toBeNull()
239+
})
240+
241+
await check('leaving a draft from the URL bar asks, and Stay keeps it', async () => {
242+
await userInput('#draft', 'draft')
243+
await expect
244+
.poll(() => inPage<string>("document.getElementById('draft').value"))
245+
.toBe('draft')
246+
await panelAction({ action: 'navigate', url: `${site}/next` })
247+
await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' })
248+
const dialog = await pageDialog()
249+
await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: false })
250+
await expect.poll(pageDialog).toBeNull()
251+
expect(await pageUrl()).toBe(`${site}/form`)
252+
expect(await inPage<string>("document.getElementById('draft').value")).toBe('draft')
253+
})
254+
255+
await check('Leave lets the navigation through without asking again', async () => {
256+
await panelAction({ action: 'navigate', url: `${site}/next` })
257+
await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' })
258+
const dialog = await pageDialog()
259+
await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: true })
260+
await expect.poll(pageUrl).toBe(`${site}/next`)
261+
expect(await pageDialog()).toBeNull()
262+
})
263+
passed = true
264+
} finally {
265+
mkdirSync(dirname(reportPath), { recursive: true })
266+
writeFileSync(reportPath, JSON.stringify({ passed, checks }, null, 2))
267+
await app?.close()
268+
await new Promise<void>((resolve) => server.close(() => resolve()))
269+
rmSync(userData, { recursive: true, force: true })
270+
}
271+
})

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

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,22 @@ function createOopifFrameFixture() {
6161
}
6262
}
6363

64+
/** Callbacks for a page with no user to ask, so the shell answers every dialog. */
65+
const shellAnswersDialogs = {
66+
offerToUser: () => false,
67+
onDialogClosed: () => {},
68+
claimUserLeave: () => false,
69+
}
70+
6471
describe('browser-agent CDP instrumentation', () => {
6572
it('leaves file chooser dialogs native so users can upload files', async () => {
6673
const contents = new WebContentsView().webContents
6774

68-
await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null })
75+
await ensureInstrumented(contents, {
76+
onDialog: vi.fn(),
77+
dialogResponse: () => null,
78+
...shellAnswersDialogs,
79+
})
6980

7081
expect(contents.debugger.sendCommand).toHaveBeenCalledWith('Page.enable', undefined)
7182
expect(contents.debugger.sendCommand).not.toHaveBeenCalledWith(
@@ -86,10 +97,18 @@ describe('browser-agent CDP instrumentation', () => {
8697
})
8798

8899
await expect(
89-
ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null })
100+
ensureInstrumented(contents, {
101+
onDialog: vi.fn(),
102+
dialogResponse: () => null,
103+
...shellAnswersDialogs,
104+
})
90105
).rejects.toThrow('setup acknowledgement lost')
91106
await expect(
92-
ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null })
107+
ensureInstrumented(contents, {
108+
onDialog: vi.fn(),
109+
dialogResponse: () => null,
110+
...shellAnswersDialogs,
111+
})
93112
).resolves.toBeUndefined()
94113

95114
expect(autoAttachAttempts).toBe(2)
@@ -98,7 +117,11 @@ describe('browser-agent CDP instrumentation', () => {
98117
it('dismisses an OOPIF dialog on the flattened child session', async () => {
99118
const contents = new WebContentsView().webContents
100119
const onDialog = vi.fn()
101-
await ensureInstrumented(contents, { onDialog, dialogResponse: () => null })
120+
await ensureInstrumented(contents, {
121+
onDialog,
122+
dialogResponse: () => null,
123+
...shellAnswersDialogs,
124+
})
102125
const listener = vi
103126
.mocked(contents.debugger.on)
104127
.mock.calls.find(([event]) => event === 'message')?.[1] as
@@ -132,7 +155,7 @@ describe('browser-agent CDP instrumentation', () => {
132155
const contents = new WebContentsView().webContents
133156
const onDialog = vi.fn()
134157
const dialogResponse = vi.fn(() => ({ accept: true }))
135-
await ensureInstrumented(contents, { onDialog, dialogResponse })
158+
await ensureInstrumented(contents, { onDialog, dialogResponse, ...shellAnswersDialogs })
136159
const listener = vi
137160
.mocked(contents.debugger.on)
138161
.mock.calls.find(([event]) => event === 'message')?.[1] as
@@ -275,7 +298,11 @@ describe('browser-agent CDP instrumentation', () => {
275298
async (treeKind) => {
276299
const contents = new WebContentsView().webContents
277300
const { child, frameTree } = createOopifFrameFixture()
278-
await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null })
301+
await ensureInstrumented(contents, {
302+
onDialog: vi.fn(),
303+
dialogResponse: () => null,
304+
...shellAnswersDialogs,
305+
})
279306
const listener = vi
280307
.mocked(contents.debugger.on)
281308
.mock.calls.find(([event]) => event === 'message')?.[1] as
@@ -378,7 +405,11 @@ describe('browser-agent CDP instrumentation', () => {
378405
it('falls back to the root target when OOPIF isolated-world creation fails', async () => {
379406
const contents = new WebContentsView().webContents
380407
const { child, frameTree } = createOopifFrameFixture()
381-
await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null })
408+
await ensureInstrumented(contents, {
409+
onDialog: vi.fn(),
410+
dialogResponse: () => null,
411+
...shellAnswersDialogs,
412+
})
382413
const listener = vi
383414
.mocked(contents.debugger.on)
384415
.mock.calls.find(([event]) => event === 'message')?.[1] as
@@ -458,7 +489,11 @@ describe('browser-agent file input handles', () => {
458489
async function fileInputFixture(childSession = false) {
459490
const contents = new WebContentsView().webContents
460491
const { child, frameTree } = createOopifFrameFixture()
461-
await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null })
492+
await ensureInstrumented(contents, {
493+
onDialog: vi.fn(),
494+
dialogResponse: () => null,
495+
...shellAnswersDialogs,
496+
})
462497
if (childSession) {
463498
const onMessage = vi.mocked(contents.debugger.on).mock.calls[0]?.[1] as
464499
| ((event: unknown, method: string, params: unknown, sessionId?: string) => void)

0 commit comments

Comments
 (0)