Skip to content

Commit 2cda264

Browse files
authored
fix(browser): harden desktop tool lifecycle (#7253)
* fix(browser): harden desktop tool lifecycle * fix(browser): preserve screenshot coordinate contract * fix(browser): close native execution races * fix(browser): reconcile native unload claims * fix(copilot): interrupt pending tool waits on stop
1 parent 87497b3 commit 2cda264

32 files changed

Lines changed: 2788 additions & 419 deletions

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

Lines changed: 105 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,8 @@ describe('browser-agent screenshot capture', () => {
548548
expect(shot).toEqual({
549549
dataUrl: `data:image/jpeg;base64,${Buffer.from('resized').toString('base64')}`,
550550
scale: 0.5,
551+
viewport: { width: 2048, height: 1024 },
552+
imageSize: { width: 1024, height: 512 },
551553
})
552554
})
553555

@@ -558,14 +560,115 @@ describe('browser-agent screenshot capture', () => {
558560

559561
const image = vi.mocked(nativeImage.createFromBuffer).mock.results[0].value
560562
expect(image.resize).not.toHaveBeenCalled()
561-
expect(shot).toEqual({ dataUrl: 'data:image/jpeg;base64,c2lt', scale: 0.5 })
563+
expect(shot).toEqual({
564+
dataUrl: 'data:image/jpeg;base64,c2lt',
565+
scale: 0.5,
566+
viewport: { width: 2048, height: 1024 },
567+
imageSize: { width: 1024, height: 512 },
568+
})
562569
})
563570

564571
it('returns the raw capture when the image cannot be decoded', async () => {
565572
const { contents } = captureFixture(null)
566573

567574
const shot = await captureScreenshot(contents)
568575

569-
expect(shot).toEqual({ dataUrl: 'data:image/jpeg;base64,c2lt', scale: 0.5 })
576+
expect(shot).toEqual({
577+
dataUrl: 'data:image/jpeg;base64,c2lt',
578+
scale: 0.5,
579+
viewport: { width: 2048, height: 1024 },
580+
imageSize: null,
581+
})
570582
})
583+
584+
it('does not expose deprecated device-pixel metrics as a CSS viewport', async () => {
585+
const { contents } = captureFixture({ width: 1024, height: 512 })
586+
vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
587+
if (method === 'Page.getLayoutMetrics') {
588+
return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } })
589+
}
590+
if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
591+
return Promise.resolve(undefined)
592+
})
593+
594+
const shot = await captureScreenshot(contents)
595+
596+
expect(shot.viewport).toBeNull()
597+
expect(shot.imageSize).toEqual({ width: 1024, height: 512 })
598+
})
599+
600+
it('accepts stable finite scroll offsets around the capture', async () => {
601+
const { contents } = captureFixture({ width: 1024, height: 512 })
602+
vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
603+
if (method === 'Page.getLayoutMetrics') {
604+
return Promise.resolve({
605+
cssLayoutViewport: {
606+
clientWidth: 2048,
607+
clientHeight: 1024,
608+
pageX: 12,
609+
pageY: 34,
610+
},
611+
})
612+
}
613+
if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
614+
return Promise.resolve(undefined)
615+
})
616+
617+
await expect(captureScreenshot(contents)).resolves.toMatchObject({
618+
viewport: { width: 2048, height: 1024 },
619+
imageSize: { width: 1024, height: 512 },
620+
})
621+
})
622+
623+
it.each([
624+
[
625+
'dimensions',
626+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 } },
627+
{ cssLayoutViewport: { clientWidth: 1024, clientHeight: 512 } },
628+
],
629+
[
630+
'metric units',
631+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 } },
632+
{ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } },
633+
],
634+
[
635+
'horizontal scroll offset',
636+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 0, pageY: 20 } },
637+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 10, pageY: 20 } },
638+
],
639+
[
640+
'vertical scroll offset',
641+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 10, pageY: 20 } },
642+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 10, pageY: 30 } },
643+
],
644+
[
645+
'offset validity',
646+
{ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 0, pageY: 0 } },
647+
{
648+
cssLayoutViewport: {
649+
clientWidth: 2048,
650+
clientHeight: 1024,
651+
pageX: 0,
652+
pageY: Number.NaN,
653+
},
654+
},
655+
],
656+
['availability', {}, {}],
657+
])(
658+
'rejects a capture when viewport %s change during CDP capture',
659+
async (_label, before, after) => {
660+
const { contents } = captureFixture({ width: 1024, height: 512 })
661+
let metricsRead = 0
662+
vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
663+
if (method === 'Page.getLayoutMetrics') {
664+
metricsRead++
665+
return Promise.resolve(metricsRead === 1 ? before : after)
666+
}
667+
if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
668+
return Promise.resolve(undefined)
669+
})
670+
671+
await expect(captureScreenshot(contents)).rejects.toThrow(/viewport changed/)
672+
}
673+
)
571674
})

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

Lines changed: 85 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,71 @@ const SCREENSHOT_CAPTURE_QUALITY = 90
382382
interface CdpViewport {
383383
clientWidth: number
384384
clientHeight: number
385+
pageX?: number
386+
pageY?: number
387+
}
388+
389+
interface ScreenshotViewportMetrics extends ScreenshotSize {
390+
pageX: number | null
391+
pageY: number | null
392+
unit: 'css' | 'device'
393+
}
394+
395+
interface ScreenshotSize {
396+
width: number
397+
height: number
398+
}
399+
400+
export interface ScreenshotCapture {
401+
dataUrl: string
402+
scale: number
403+
viewport: ScreenshotSize | null
404+
imageSize: ScreenshotSize | null
405+
}
406+
407+
function screenshotViewportMetrics(
408+
metrics: {
409+
cssLayoutViewport?: CdpViewport
410+
layoutViewport?: CdpViewport
411+
} | null
412+
): ScreenshotViewportMetrics | null {
413+
const viewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport
414+
const width = viewport?.clientWidth ?? 0
415+
const height = viewport?.clientHeight ?? 0
416+
if (width <= 0 || height <= 0) return null
417+
const pageX = viewport?.pageX
418+
const pageY = viewport?.pageY
419+
const hasPagePosition = pageX !== undefined || pageY !== undefined
420+
if (
421+
hasPagePosition &&
422+
(pageX === undefined ||
423+
pageY === undefined ||
424+
!Number.isFinite(pageX) ||
425+
!Number.isFinite(pageY))
426+
) {
427+
return null
428+
}
429+
return {
430+
width,
431+
height,
432+
pageX: pageX ?? null,
433+
pageY: pageY ?? null,
434+
unit: metrics?.cssLayoutViewport ? 'css' : 'device',
435+
}
436+
}
437+
438+
function sameScreenshotViewport(
439+
before: ScreenshotViewportMetrics | null,
440+
after: ScreenshotViewportMetrics | null
441+
): boolean {
442+
if (!before || !after) return false
443+
return (
444+
before.unit === after.unit &&
445+
before.width === after.width &&
446+
before.height === after.height &&
447+
before.pageX === after.pageX &&
448+
before.pageY === after.pageY
449+
)
385450
}
386451

387452
/**
@@ -401,44 +466,51 @@ interface CdpViewport {
401466
* (cssX = imageX / scale) — including on a 2x display, where an unclipped
402467
* capture arrives at device resolution and this is what brings it back down.
403468
*/
404-
export async function captureScreenshot(
405-
contents: WebContents
406-
): Promise<{ dataUrl: string; scale: number }> {
469+
export async function captureScreenshot(contents: WebContents): Promise<ScreenshotCapture> {
407470
const metrics = await send<{
408471
cssLayoutViewport?: CdpViewport
409472
layoutViewport?: CdpViewport
410473
}>(contents, 'Page.getLayoutMetrics').catch(() => null)
411474

412-
const viewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport
413-
const width = viewport?.clientWidth ?? 0
414-
const height = viewport?.clientHeight ?? 0
475+
const captureViewport = screenshotViewportMetrics(metrics)
476+
const width = captureViewport?.width ?? 0
477+
const height = captureViewport?.height ?? 0
478+
const cssWidth = metrics?.cssLayoutViewport?.clientWidth ?? 0
479+
const cssHeight = metrics?.cssLayoutViewport?.clientHeight ?? 0
480+
const cssViewport = cssWidth > 0 && cssHeight > 0 ? { width: cssWidth, height: cssHeight } : null
415481
const scale =
416482
width > 0 && height > 0 ? Math.min(1, MAX_SCREENSHOT_EDGE / Math.max(width, height)) : 1
417483

418484
const result = await send<{ data: string }>(contents, 'Page.captureScreenshot', {
419485
format: 'jpeg',
420486
quality: SCREENSHOT_CAPTURE_QUALITY,
421487
})
488+
const metricsAfterCapture = await send<{
489+
cssLayoutViewport?: CdpViewport
490+
layoutViewport?: CdpViewport
491+
}>(contents, 'Page.getLayoutMetrics').catch(() => null)
492+
if (!sameScreenshotViewport(captureViewport, screenshotViewportMetrics(metricsAfterCapture))) {
493+
throw new Error('The page viewport changed or could not be verified during screenshot capture')
494+
}
422495
const captured = `data:image/jpeg;base64,${result.data}`
423496

424497
const targetWidth = Math.round(width * scale)
425498
const targetHeight = Math.round(height * scale)
426-
// Without layout metrics there is no CSS frame of reference to resize
427-
// against, so the raw capture is the honest answer — the same fallback the
428-
// clipped path took.
429-
if (targetWidth <= 0 || targetHeight <= 0) return { dataUrl: captured, scale }
430-
431499
const image = nativeImage.createFromBuffer(Buffer.from(result.data, 'base64'))
432500
const size = image.isEmpty() ? { width: 0, height: 0 } : image.getSize()
433-
if (size.width === 0 || size.height === 0) return { dataUrl: captured, scale }
501+
if (size.width === 0 || size.height === 0) {
502+
return { dataUrl: captured, scale, viewport: cssViewport, imageSize: null }
503+
}
434504
if (size.width === targetWidth && size.height === targetHeight) {
435-
return { dataUrl: captured, scale }
505+
return { dataUrl: captured, scale, viewport: cssViewport, imageSize: size }
436506
}
437507

438508
const resized = image.resize({ width: targetWidth, height: targetHeight, quality: 'good' })
439509
return {
440510
dataUrl: `data:image/jpeg;base64,${resized.toJPEG(SCREENSHOT_QUALITY).toString('base64')}`,
441511
scale,
512+
viewport: cssViewport,
513+
imageSize: { width: targetWidth, height: targetHeight },
442514
}
443515
}
444516

0 commit comments

Comments
 (0)