Skip to content

Commit 74fa742

Browse files
committed
improvement(desktop): hold the read's own report across Stop, dismiss dev console notices, and keep failure diagnostics
1 parent 9677310 commit 74fa742

2 files changed

Lines changed: 90 additions & 27 deletions

File tree

‎apps/desktop/e2e/desktop-tools-live-sim.spec.ts‎

Lines changed: 84 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ test.describe('desktop tools against a live Sim', () => {
5353
let scratch: string
5454
/** Errors the current window reported, shown when a click is blocked. */
5555
let pageErrors: string[] = []
56+
let testStartedAt = 0
5657

5758
test.beforeAll(async () => {
5859
test.setTimeout(1_500_000)
@@ -70,16 +71,36 @@ test.describe('desktop tools against a live Sim', () => {
7071
})
7172

7273
test.beforeEach(() => {
74+
testStartedAt = Date.now()
7375
scratch = mkdtempSync(join(tmpdir(), 'sim-desktop-tools-live-'))
7476
})
7577

7678
test.afterEach(async () => {
7779
const testInfo = test.info()
78-
if (testInfo.status !== testInfo.expectedStatus)
79-
await app
80-
?.windows()[0]
81-
?.screenshot({ path: testInfo.outputPath('failure.png') })
82-
.catch(() => {})
80+
if (testInfo.status !== testInfo.expectedStatus) {
81+
const page = app?.windows()[0]
82+
await page?.screenshot({ path: testInfo.outputPath('failure.png') }).catch(() => {})
83+
writeFileSync(
84+
testInfo.outputPath('diagnostics.json'),
85+
JSON.stringify(
86+
{
87+
url: page?.url(),
88+
composer: await page
89+
?.getByRole('textbox')
90+
.last()
91+
.inputValue({ timeout: 2_000 })
92+
.catch(() => null),
93+
pageErrors,
94+
requests: proxy
95+
.seen(testStartedAt)
96+
.filter((entry) => !entry.path.startsWith('/_next/')),
97+
},
98+
null,
99+
2
100+
)
101+
)
102+
}
103+
proxy.clearHolds()
83104
await app?.close().catch(() => {})
84105
app = undefined
85106
proxy.rewriteChatBody(undefined)
@@ -178,23 +199,36 @@ test.describe('desktop tools against a live Sim', () => {
178199
const composer = (page: Page) => page.getByRole('textbox').last()
179200

180201
/** The dev app's error overlay, if it is up, with the errors the window reported. */
181-
async function devOverlayError(page: Page): Promise<string | undefined> {
202+
async function devOverlay(page: Page): Promise<{ text: string; consoleOnly: boolean } | null> {
182203
const overlay = page.locator('nextjs-portal [data-nextjs-dialog]')
183-
if ((await overlay.count()) === 0) return undefined
184-
return `Next.js error overlay: ${await overlay.first().innerText()}\n${pageErrors.join('\n')}`
204+
if ((await overlay.count()) === 0) return null
205+
const text = await overlay
206+
.first()
207+
.innerText()
208+
.catch(() => '')
209+
return {
210+
text: `${text}\n${pageErrors.join('\n')}`,
211+
consoleOnly: /^\s*Console Error/.test(text),
212+
}
185213
}
186214

187215
/**
188-
* Clicks `target`, failing at once with the dev app's error instead of waiting out a click the
189-
* Next.js error overlay intercepts.
216+
* Clicks `target`. When the dev app's error overlay intercepts the click, a console-error notice
217+
* (dev-only chrome over a logged error) is dismissed and the click retried; a runtime error fails
218+
* at once with its message instead of waiting out a blocked click.
190219
*/
191220
async function click(page: Page, target: Locator, timeout = 15_000): Promise<void> {
192-
const before = await devOverlayError(page)
193-
if (before) throw new Error(before)
194-
try {
195-
await target.click({ timeout })
196-
} catch (error) {
197-
throw new Error((await devOverlayError(page)) ?? String(error))
221+
for (let attempt = 0; ; attempt++) {
222+
try {
223+
await target.click({ timeout: attempt === 0 ? Math.min(timeout, 5_000) : timeout })
224+
return
225+
} catch (error) {
226+
const overlay = await devOverlay(page)
227+
if (overlay && !overlay.consoleOnly)
228+
throw new Error(`Next.js error overlay: ${overlay.text}`)
229+
if (overlay) await page.keyboard.press('Escape')
230+
else if (attempt > 0) throw error
231+
}
198232
}
199233
}
200234

@@ -211,15 +245,19 @@ test.describe('desktop tools against a live Sim', () => {
211245
.some((entry) => entry.method === 'POST' && entry.path === '/api/mothership/chat')
212246
const deadline = Date.now() + timeout
213247
while (!sent()) {
214-
const overlay = await devOverlayError(page)
215-
if (overlay) throw new Error(overlay)
216-
if (Date.now() > deadline) throw new Error(`The message was never sent: ${message}`)
248+
if (Date.now() > deadline)
249+
throw new Error(
250+
`The message was never sent: ${message} (composer: ${JSON.stringify(
251+
await composer(page).inputValue()
252+
)}; errors: ${pageErrors.join(' | ')})`
253+
)
217254
if ((await composer(page).inputValue()) !== message) await composer(page).fill(message)
218-
const clicked = await page
219-
.getByRole('button', { name: 'Send message' })
220-
.click({ timeout: 5_000 })
255+
const clicked = await click(page, page.getByRole('button', { name: 'Send message' }), 5_000)
221256
.then(() => true)
222-
.catch(() => false)
257+
.catch((error: unknown) => {
258+
if (String(error).includes('Next.js error overlay')) throw error
259+
return false
260+
})
223261
if (clicked)
224262
await expect
225263
.poll(sent, { timeout: 10_000 })
@@ -361,8 +399,12 @@ test.describe('desktop tools against a live Sim', () => {
361399
firstUpload.release()
362400
// The import outlived both view changes: its first file landed.
363401
await expect
364-
.poll(() => db.workspaceFileNames(user.workspaceId), { timeout: 30_000 })
365-
.toContain('a.txt')
402+
.poll(
403+
async () =>
404+
`${(await db.workspaceFileNames(user.workspaceId)).join(',')} | ${await callState(chatId)}`,
405+
{ timeout: 60_000 }
406+
)
407+
.toMatch(/^a\.txt /)
366408
await laterFolder.arrival(ARRIVAL_MS, 'The next folder')
367409

368410
// Hold the import's own report until Stop has settled the call, so the outcome is Stop's.
@@ -507,13 +549,28 @@ test.describe('desktop tools against a live Sim', () => {
507549
const page = await openApp(user, 'Stop chat')
508550
const chatId = user.chats['Stop chat']
509551
const lateClaim = proxy.hold(isDesktopClaim)
552+
// Should Electron give up on the held claim first, the read reports its own failure; hold that
553+
// too, so the call's outcome is Stop's.
554+
const report = proxy.hold(isToolReport)
510555
await send(page, '[stopped-claim] read my secret')
511556
const claim = await lateClaim.arrival(ARRIVAL_MS, 'The read’s claim')
512557
await click(page, page.getByRole('button', { name: 'Stop generation' }))
513558
// Stop settles the call nobody has claimed yet as never started.
514-
await expect.poll(async () => callState(chatId), { timeout: 15_000 }).toMatch(/^cancelled/)
559+
await expect.poll(() => callState(chatId), { timeout: 30_000 }).toMatch(/^cancelled/)
515560
lateClaim.release()
516-
await expect.poll(() => claim.status, { timeout: 15_000 }).toBe(410)
561+
report.release()
562+
// The claim held across Stop, and the same claim replayed afterwards, are both refused.
563+
const [call] = await db.toolCalls(chatId)
564+
const replay = await page.evaluate(
565+
(toolCallId) =>
566+
(globalThis as DesktopWindow).simDesktop.localFiles?.({ operation: 'read', toolCallId }),
567+
call.toolCallId
568+
)
569+
expect(replay?.ok).toBe(false)
570+
const answered = () =>
571+
proxy.seen(claim.at, '/api/desktop/tool/authorize').filter((entry) => entry.status)
572+
await expect.poll(() => answered().length, { timeout: 15_000 }).toBeGreaterThan(0)
573+
for (const entry of answered()) expect(entry.status).toBe(410)
517574
const [after] = await db.toolCalls(chatId)
518575
expect(after).toMatchObject({ status: 'cancelled', claimedBy: null })
519576
})

‎apps/desktop/e2e/fixtures/live-sim.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ export class SimProxy {
220220
return held
221221
}
222222

223+
/** Drops holds that never matched, so none outlives the test that set it. */
224+
clearHolds(): void {
225+
for (const held of this.holds) held.release()
226+
this.holds = []
227+
}
228+
223229
/** Rewrites the JSON body of chat turns the renderer sends, as a newer client would send it. */
224230
rewriteChatBody(rewrite: ((body: Record<string, unknown>) => void) | undefined): void {
225231
this.chatBodyRewrite = rewrite

0 commit comments

Comments
 (0)