fix(browser): preserve screenshot coordinate contract

Author: waleedlatif1

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

@@ -549,6 +549,7 @@ describe('browser-agent screenshot capture', () => {
       dataUrl: `data:image/jpeg;base64,${Buffer.from('resized').toString('base64')}`,
       scale: 0.5,
       viewport: { width: 2048, height: 1024 },
+      imageSize: { width: 1024, height: 512 },
     })
   })
 
@@ -563,6 +564,7 @@ describe('browser-agent screenshot capture', () => {
       dataUrl: 'data:image/jpeg;base64,c2lt',
       scale: 0.5,
       viewport: { width: 2048, height: 1024 },
+      imageSize: { width: 1024, height: 512 },
     })
   })
 
@@ -575,6 +577,23 @@ describe('browser-agent screenshot capture', () => {
       dataUrl: 'data:image/jpeg;base64,c2lt',
       scale: 0.5,
       viewport: { width: 2048, height: 1024 },
+      imageSize: null,
     })
   })
+
+  it('does not expose deprecated device-pixel metrics as a CSS viewport', async () => {
+    const { contents } = captureFixture({ width: 1024, height: 512 })
+    vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
+      if (method === 'Page.getLayoutMetrics') {
+        return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } })
+      }
+      if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
+      return Promise.resolve(undefined)
+    })
+
+    const shot = await captureScreenshot(contents)
+
+    expect(shot.viewport).toBeNull()
+    expect(shot.imageSize).toEqual({ width: 1024, height: 512 })
+  })
 })

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

@@ -384,6 +384,18 @@ interface CdpViewport {
   clientHeight: number
 }
 
+interface ScreenshotSize {
+  width: number
+  height: number
+}
+
+export interface ScreenshotCapture {
+  dataUrl: string
+  scale: number
+  viewport: ScreenshotSize | null
+  imageSize: ScreenshotSize | null
+}
+
 /**
  * Screenshot via CDP (works while the view is hidden), bounded in resolution.
  *
@@ -401,18 +413,18 @@ interface CdpViewport {
  * (cssX = imageX / scale) — including on a 2x display, where an unclipped
  * capture arrives at device resolution and this is what brings it back down.
  */
-export async function captureScreenshot(
-  contents: WebContents
-): Promise<{ dataUrl: string; scale: number; viewport: { width: number; height: number } | null }> {
+export async function captureScreenshot(contents: WebContents): Promise<ScreenshotCapture> {
   const metrics = await send<{
     cssLayoutViewport?: CdpViewport
     layoutViewport?: CdpViewport
   }>(contents, 'Page.getLayoutMetrics').catch(() => null)
 
-  const viewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport
-  const width = viewport?.clientWidth ?? 0
-  const height = viewport?.clientHeight ?? 0
-  const cssViewport = width > 0 && height > 0 ? { width, height } : null
+  const captureViewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport
+  const width = captureViewport?.clientWidth ?? 0
+  const height = captureViewport?.clientHeight ?? 0
+  const cssWidth = metrics?.cssLayoutViewport?.clientWidth ?? 0
+  const cssHeight = metrics?.cssLayoutViewport?.clientHeight ?? 0
+  const cssViewport = cssWidth > 0 && cssHeight > 0 ? { width: cssWidth, height: cssHeight } : null
   const scale =
     width > 0 && height > 0 ? Math.min(1, MAX_SCREENSHOT_EDGE / Math.max(width, height)) : 1
 
@@ -424,27 +436,28 @@ export async function captureScreenshot(
 
   const targetWidth = Math.round(width * scale)
   const targetHeight = Math.round(height * scale)
-  // Without layout metrics there is no CSS frame of reference to resize
-  // against, so the raw capture is the honest answer — the same fallback the
-  // clipped path took.
-  if (targetWidth <= 0 || targetHeight <= 0) {
-    return { dataUrl: captured, scale, viewport: cssViewport }
-  }
-
   const image = nativeImage.createFromBuffer(Buffer.from(result.data, 'base64'))
   const size = image.isEmpty() ? { width: 0, height: 0 } : image.getSize()
   if (size.width === 0 || size.height === 0) {
-    return { dataUrl: captured, scale, viewport: cssViewport }
+    return { dataUrl: captured, scale, viewport: cssViewport, imageSize: null }
+  }
+  // Without layout metrics there is no CSS frame of reference to resize
+  // against, so the raw capture is the honest answer — the same fallback the
+  // clipped path took. Its decoded size still lets the driver establish the
+  // coordinate scale after obtaining the CSS viewport in-page.
+  if (targetWidth <= 0 || targetHeight <= 0) {
+    return { dataUrl: captured, scale, viewport: cssViewport, imageSize: size }
   }
   if (size.width === targetWidth && size.height === targetHeight) {
-    return { dataUrl: captured, scale, viewport: cssViewport }
+    return { dataUrl: captured, scale, viewport: cssViewport, imageSize: size }
   }
 
   const resized = image.resize({ width: targetWidth, height: targetHeight, quality: 'good' })
   return {
     dataUrl: `data:image/jpeg;base64,${resized.toJPEG(SCREENSHOT_QUALITY).toString('base64')}`,
     scale,
     viewport: cssViewport,
+    imageSize: { width: targetWidth, height: targetHeight },
   }
 }
 

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

@@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 vi.mock('electron', () => import('@/test/electron-mock'))
 
-import { BrowserWindow, Menu } from 'electron'
+import { BrowserWindow, Menu, nativeImage } from 'electron'
 import * as cdp from '@/main/browser-agent/cdp'
 import * as driverModule from '@/main/browser-agent/driver'
 import * as session from '@/main/browser-agent/session'
@@ -1175,6 +1175,15 @@ describe('credential protection', () => {
       .mock.calls.filter(([called]) => called === method)
   }
 
+  function mockScreenshotImage(size: { width: number; height: number } | null): void {
+    vi.mocked(nativeImage.createFromBuffer).mockReturnValueOnce({
+      isEmpty: vi.fn(() => size === null),
+      getSize: vi.fn(() => size ?? { width: 0, height: 0 }),
+      resize: vi.fn(() => ({ toJPEG: vi.fn(() => Buffer.from('resized')) })),
+      toJPEG: vi.fn(() => Buffer.alloc(0)),
+    } as unknown as ReturnType<typeof nativeImage.createFromBuffer>)
+  }
+
   it('refuses a keystroke while a password field holds focus', async () => {
     const contents = await openPage()
     respondWith(contents, { activeElementSecrecy: 'secret' })
@@ -2385,6 +2394,7 @@ describe('credential protection', () => {
 
   it('returns the screenshot scale for coordinate mapping', async () => {
     const contents = await openPage()
+    mockScreenshotImage({ width: 1024, height: 512 })
     vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
       if (method === 'Page.getLayoutMetrics') {
         return Promise.resolve({
@@ -2402,12 +2412,132 @@ describe('credential protection', () => {
 
     expect(result).toMatchObject({
       ok: true,
-      result: { scale: 0.5, viewport: { width: 2048, height: 1024 } },
+      result: {
+        scale: 0.5,
+        viewport: {
+          url: 'https://example.com/login',
+          title: 'Example',
+          width: 2048,
+          height: 1024,
+        },
+      },
     })
     expect(
       vi
         .mocked(contents.executeJavaScript)
         .mock.calls.some(([expression]) => isPageCall(String(expression), 'getViewportInfo'))
     ).toBe(false)
   })
+
+  it('uses the in-page CSS viewport when CDP exposes only deprecated device metrics', async () => {
+    const contents = await openPage()
+    mockScreenshotImage({ width: 1024, height: 512 })
+    vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
+      if (method === 'Page.getLayoutMetrics') {
+        return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } })
+      }
+      if (method === 'Page.captureScreenshot') {
+        return Promise.resolve({ data: 'c2lt' })
+      }
+      return Promise.resolve(undefined)
+    })
+    respondWith(contents, {
+      getViewportInfo: {
+        url: 'https://example.com/login',
+        title: 'Example',
+        width: 1024,
+        height: 512,
+      },
+    })
+
+    const result = await driver.executeTool('chat-test', 'browser_screenshot', {})
+
+    expect(result).toMatchObject({
+      ok: true,
+      result: {
+        scale: 1,
+        viewport: {
+          url: 'https://example.com/login',
+          title: 'Example',
+          width: 1024,
+          height: 512,
+        },
+      },
+    })
+    if (
+      !result.ok ||
+      typeof result.result !== 'object' ||
+      result.result === null ||
+      !('scale' in result.result) ||
+      typeof result.result.scale !== 'number'
+    ) {
+      throw new Error('browser_screenshot did not return a numeric coordinate scale')
+    }
+    expect(1024 / result.result.scale).toBe(1024)
+    expect(
+      vi
+        .mocked(contents.executeJavaScript)
+        .mock.calls.some(([expression]) => isPageCall(String(expression), 'getViewportInfo'))
+    ).toBe(true)
+  })
+
+  it('rejects an undecodable screenshot instead of returning an unverified scale', async () => {
+    const contents = await openPage()
+    mockScreenshotImage(null)
+    vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
+      if (method === 'Page.getLayoutMetrics') {
+        return Promise.resolve({
+          cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 },
+        })
+      }
+      if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
+      return Promise.resolve(undefined)
+    })
+
+    const result = await driver.executeTool('chat-test', 'browser_screenshot', {})
+
+    expect(result.ok).toBe(false)
+    expect(result.error).toMatch(/verify the screenshot dimensions/)
+  })
+
+  it('rejects a screenshot when no CSS viewport can be established', async () => {
+    const contents = await openPage()
+    mockScreenshotImage({ width: 1024, height: 512 })
+    vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
+      if (method === 'Page.getLayoutMetrics') {
+        return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } })
+      }
+      if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
+      return Promise.resolve(undefined)
+    })
+    respondWith(contents, { getViewportInfo: null })
+
+    const result = await driver.executeTool('chat-test', 'browser_screenshot', {})
+
+    expect(result.ok).toBe(false)
+    expect(result.error).toMatch(/verify the page viewport/)
+  })
+
+  it('rejects coordinate mapping when the viewport changes during capture', async () => {
+    const contents = await openPage()
+    mockScreenshotImage({ width: 1024, height: 256 })
+    vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => {
+      if (method === 'Page.getLayoutMetrics') return Promise.resolve({})
+      if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' })
+      return Promise.resolve(undefined)
+    })
+    respondWith(contents, {
+      getViewportInfo: {
+        url: 'https://example.com/login',
+        title: 'Example',
+        width: 1024,
+        height: 512,
+      },
+    })
+
+    const result = await driver.executeTool('chat-test', 'browser_screenshot', {})
+
+    expect(result.ok).toBe(false)
+    expect(result.error).toMatch(/viewport changed while the screenshot was captured/)
+  })
 })

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

@@ -2139,11 +2139,53 @@ async function executeToolInner(
           'The screenshot result was too large to return safely. Use browser_snapshot or browser_read_text instead.'
         )
       }
-      const viewport =
-        shot.viewport ?? (await execInPage(contents, getViewportInfo, []).catch(() => null))
+      if (!shot.imageSize) {
+        throw new ToolError(
+          'Could not verify the screenshot dimensions. Retry browser_screenshot or use browser_snapshot instead.'
+        )
+      }
+      const viewport = shot.viewport
+        ? {
+            url: contents.getURL().slice(0, 4096),
+            title: contents.getTitle().slice(0, 500),
+            ...shot.viewport,
+          }
+        : await execInPage(contents, getViewportInfo, []).catch(() => null)
+      let scale = shot.scale
+      const viewportWidth =
+        isRecordLike(viewport) && typeof viewport.width === 'number' ? viewport.width : 0
+      const viewportHeight =
+        isRecordLike(viewport) && typeof viewport.height === 'number' ? viewport.height : 0
+      if (
+        !Number.isFinite(viewportWidth) ||
+        !Number.isFinite(viewportHeight) ||
+        viewportWidth <= 0 ||
+        viewportHeight <= 0
+      ) {
+        throw new ToolError(
+          'Could not verify the page viewport for this screenshot. Retry browser_screenshot or use browser_snapshot instead.'
+        )
+      }
+      if (!shot.viewport) {
+        const widthScale = shot.imageSize.width / viewportWidth
+        const heightScale = shot.imageSize.height / viewportHeight
+        const scaleDelta = Math.abs(widthScale - heightScale)
+        if (
+          !Number.isFinite(widthScale) ||
+          !Number.isFinite(heightScale) ||
+          widthScale <= 0 ||
+          heightScale <= 0 ||
+          scaleDelta > Math.max(widthScale, heightScale) * 0.02
+        ) {
+          throw new ToolError(
+            'The page viewport changed while the screenshot was captured. Retry browser_screenshot before using image coordinates.'
+          )
+        }
+        scale = widthScale
+      }
       // scale maps image pixels back to CSS viewport pixels for the
       // coordinate tools: cssX = imageX / scale.
-      return { dataUrl: shot.dataUrl, viewport, scale: shot.scale }
+      return { dataUrl: shot.dataUrl, viewport, scale }
     }
 
     case 'browser_extract': {

apps/sim/app/api/copilot/confirm/route.ts

@@ -263,15 +263,15 @@ export const POST = withRouteHandler((req: NextRequest) => {
           )
         }
 
-        const isUnboundTerminalWorkflowOutcome =
+        const isErrorOrCancelledOutcome =
           status === ASYNC_TOOL_CONFIRMATION_STATUS.error ||
           status === ASYNC_TOOL_CONFIRMATION_STATUS.cancelled
         const isNativeClientTool =
           isBrowserToolName(existing.toolName) || isTerminalToolName(existing.toolName)
         const isPreclaimNativeTerminalOutcome =
           isNativeClientTool &&
           existing.status === ASYNC_TOOL_STATUS.pending &&
-          isUnboundTerminalWorkflowOutcome
+          isErrorOrCancelledOutcome
         const isMutableClientToolCall = isWorkflowTool
           ? isWorkflowToolExecutionClaimable(existing.status, existing.permissionDecision)
           : existing.status === ASYNC_TOOL_STATUS.running || isPreclaimNativeTerminalOutcome
@@ -315,7 +315,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
             if (status !== ASYNC_TOOL_CONFIRMATION_STATUS.background) {
               if (trustedExecution) {
                 effectiveStatus = getWorkflowToolConfirmationStatus(trustedExecution.status)
-              } else if (!isUnboundTerminalWorkflowOutcome) {
+              } else if (!isErrorOrCancelledOutcome) {
                 span.setAttribute(
                   TraceAttr.CopilotConfirmOutcome,
                   CopilotConfirmOutcome.ToolCallNotFound
@@ -328,7 +328,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
           } else if (trustedExecution) {
             executionId = trustedExecution.executionId
             effectiveStatus = getWorkflowToolConfirmationStatus(trustedExecution.status)
-          } else if (!isUnboundTerminalWorkflowOutcome) {
+          } else if (!isErrorOrCancelledOutcome) {
             effectiveStatus = ASYNC_TOOL_CONFIRMATION_STATUS.error
             executionId = undefined
           } else {