fix(browser): preserve observation identity and control semantics

Author: waleedlatif1

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

@@ -3363,29 +3363,49 @@ describe('credential protection', () => {
     expect(contents.executeJavaScript).not.toHaveBeenCalled()
   })
 
-  it('does not click a checkable control already in the requested state', async () => {
+  it.each([
+    { kind: 'input:checkbox', checked: true, disabled: false, readOnly: false },
+    { kind: 'input:radio', checked: false, disabled: false, readOnly: false },
+    { kind: 'role:radio', checked: false, disabled: false, readOnly: false },
+    { kind: 'role:menuitemradio', checked: false, disabled: false, readOnly: false },
+    { kind: 'input:checkbox', checked: true, disabled: true, readOnly: false },
+    { kind: 'input:checkbox', checked: true, disabled: false, readOnly: true },
+  ])('does not click a control already in the requested state: %j', async (before) => {
     const contents = await openPage()
     respondWith(contents, {
-      readCheckableElementState: {
-        checked: true,
-        disabled: false,
-        readOnly: false,
-        kind: 'input:checkbox',
-      },
+      readCheckableElementState: before,
     })
 
     const result = await driver.executeTool('chat-test', 'browser_set_checked', {
       elementId: 0,
-      checked: true,
+      checked: before.checked,
     })
 
     expect(result).toMatchObject({
       ok: true,
-      result: { checked: true, changed: false, dispatched: false },
+      result: { checked: before.checked, changed: false, dispatched: false },
     })
     expect(cdpCalls(contents, 'Input.dispatchMouseEvent')).toHaveLength(0)
   })
 
+  it.each([
+    { checked: true, kind: 'input:radio', error: 'cannot be unchecked' },
+    { checked: true, kind: 'role:radio', error: 'cannot be unchecked' },
+    { checked: true, kind: 'role:menuitemradio', error: 'cannot be unchecked' },
+    { checked: false, kind: 'input:checkbox', disabled: true, error: 'disabled' },
+    { checked: false, kind: 'input:checkbox', readOnly: true, error: 'read-only' },
+  ])('rejects a prohibited state change: %j', async (before) => {
+    const contents = await openPage()
+    respondWith(contents, { readCheckableElementState: before })
+    const result = await driver.executeTool('chat-test', 'browser_set_checked', {
+      elementId: 0,
+      checked: !before.checked,
+    })
+    expect(result.ok).toBe(false)
+    expect(result.error).toContain(before.error)
+    expect(cdpCalls(contents, 'Input.dispatchMouseEvent')).toHaveLength(0)
+  })
+
   it.each([false, 'mixed'])(
     'uses the trusted click path from %s and verifies a changed checkable control',
     async (initialState) => {
@@ -3527,6 +3547,8 @@ describe('credential protection', () => {
 
   it.each([
     ['detached', { present: true, rendered: false }],
+    ['collapsed', { present: true, rendered: true }],
+    ['expanded', { present: true, rendered: true }],
     ['unchecked', { present: true, rendered: true, checked: 'mixed' }],
     ['unchecked', { present: true, rendered: true }],
   ])('does not satisfy %s from an incompatible element state', async (state, targetState) => {
@@ -3546,6 +3568,55 @@ describe('credential protection', () => {
     }
   })
 
+  it.each([
+    ['expanded', { open: true }],
+    ['collapsed', { open: false }],
+    ['expanded', { ariaExpanded: 'true' }],
+    ['collapsed', { ariaExpanded: 'false' }],
+  ])('waits for %s on native and ARIA disclosure controls', async (state, semanticState) => {
+    const contents = await openPage()
+    respondWith(contents, {
+      readPageActionState: { targetState: { present: true, rendered: true, ...semanticState } },
+    })
+    await expect(
+      driver.executeTool('chat-test', 'browser_wait_for', {
+        elementId: 0,
+        state,
+        timeoutMs: 100,
+      })
+    ).resolves.toMatchObject({ ok: true, result: { found: true } })
+  })
+
+  it.each(['x', 'y', 'width', 'height', 'detached'])(
+    'rejects an element screenshot when %s changes during capture',
+    async (change) => {
+      const contents = await openPage()
+      let captured = false
+      vi.mocked(contents.executeJavaScript).mockImplementation(async (expression: string) => {
+        if (!isPageCall(expression, 'getElementScreenshotRect')) return undefined
+        if (captured && change === 'detached') return { error: 'stale' }
+        return { x: 20, y: 30, width: 200, height: 100, ...(captured ? { [change]: 50 } : {}) }
+      })
+      const capture = vi.spyOn(cdp, 'captureScreenshot').mockImplementation(async () => {
+        captured = true
+        return {
+          dataUrl: 'data:image/jpeg;base64,c2lt',
+          scale: 1,
+          viewport: { width: 800, height: 600 },
+          imageSize: { width: 200, height: 100 },
+        }
+      })
+      try {
+        const result = await driver.executeTool('chat-test', 'browser_screenshot', { elementId: 0 })
+        expect(result.ok).toBe(false)
+        expect(result.error).toMatch(change === 'detached' ? /stale/ : /element moved/)
+        expect(capture).toHaveBeenCalledTimes(1)
+      } finally {
+        capture.mockRestore()
+      }
+    }
+  )
+
   it('crops an element screenshot without changing the live viewport', async () => {
     const contents = await openPage()
     respondWith(contents, {

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

@@ -929,6 +929,14 @@ function browserElementStateMatches(
           : targetState.ariaChecked === 'false' || targetState.ariaPressed === 'false'
             ? false
             : undefined
+  const expanded =
+    typeof targetState.open === 'boolean'
+      ? targetState.open
+      : targetState.ariaExpanded === 'true'
+        ? true
+        : targetState.ariaExpanded === 'false'
+          ? false
+          : undefined
   const selected =
     typeof targetState.selected === 'boolean'
       ? targetState.selected
@@ -956,9 +964,9 @@ function browserElementStateMatches(
     case 'unchecked':
       return present && checked === false
     case 'expanded':
-      return present && targetState.ariaExpanded === 'true'
+      return present && expanded === true
     case 'collapsed':
-      return present && targetState.ariaExpanded === 'false'
+      return present && expanded === false
     case 'selected':
       return present && selected === true
     case 'unselected':
@@ -2560,6 +2568,25 @@ async function executeToolInner(
         )
       }
       assertCaptureIsCurrent()
+      if (elementId !== undefined && elementClip) {
+        const currentClip = toRecord(
+          unwrapPageResult(
+            await execInPage(
+              contents,
+              getElementScreenshotRect,
+              [elementId],
+              false,
+              executionDeadline
+            )
+          )
+        )
+        assertCaptureIsCurrent()
+        if (['x', 'y', 'width', 'height'].some((key) => currentClip[key] !== elementClip[key])) {
+          throw new ToolError(
+            'The element moved while its screenshot was being captured. Retry browser_screenshot before using image coordinates.'
+          )
+        }
+      }
       if (shot.dataUrl.length > 8_000_000) {
         throw new ToolError(
           'The screenshot result was too large to return safely. Use browser_snapshot or browser_read_text instead.'
@@ -3646,6 +3673,15 @@ async function executeToolInner(
           await execInPage(target, readCheckableElementState, [elementId], false, executionDeadline)
         )
       )
+      if (before.checked === checked) {
+        return {
+          checked,
+          changed: false,
+          dispatched: false,
+          element: before.kind,
+          refRecovered: before.refRecovered === true,
+        }
+      }
       if (before.disabled === true) throw new ToolError('That control is disabled.')
       if (before.readOnly === true) throw new ToolError('That control is read-only.')
       if (
@@ -3656,15 +3692,6 @@ async function executeToolInner(
       ) {
         throw new ToolError('Radio buttons cannot be unchecked directly. Select another option.')
       }
-      if (before.checked === checked) {
-        return {
-          checked,
-          changed: false,
-          dispatched: false,
-          element: before.kind,
-          refRecovered: before.refRecovered === true,
-        }
-      }
 
       const clickResult = toRecord(
         await executeToolInner(

apps/desktop/src/main/browser-agent/page-functions.test.ts

@@ -994,6 +994,29 @@ describe('semantic control state', () => {
     expect(collectSnapshot(10, 0)).toEqual({ error: 'framed-snapshot' })
   })
 
+  it.each(['detached', 'hidden', 'renamed'])(
+    'rejects a %s root before scoped capture without adopting a lookalike',
+    (change) => {
+      document.body.innerHTML =
+        '<div id="card" tabindex="0" aria-label="Selected card"><button>Save card</button></div>'
+      for (const element of document.querySelectorAll('*')) visible(element)
+      const root = document.querySelector('#card')!
+      const rootRef = refFor(outlineOf(collectSnapshot()), 'Selected card')
+      const replacement = root.cloneNode(true) as HTMLElement
+      for (const element of [replacement, ...replacement.querySelectorAll('*')]) visible(element)
+      if (change === 'detached') root.replaceWith(replacement)
+      else {
+        root.after(replacement)
+        if (change === 'hidden') root.setAttribute('hidden', '')
+        else root.setAttribute('aria-label', 'Different card')
+      }
+
+      expect(runSerialized(collectSnapshot, [100, rootRef])).toMatchObject({ error: 'stale' })
+      expect(window.__simAgentElements?.[rootRef]).toBe(root)
+      expect(runSerialized(collectSnapshot, [100, rootRef])).toMatchObject({ error: 'stale' })
+    }
+  )
+
   it('marks unreadable scoped frame content truncated', () => {
     const root = visible(document.createElement('div'))
     const frame = visible(document.createElement('iframe'))
@@ -1063,6 +1086,43 @@ describe('semantic control state', () => {
     expect(readPageActionState(false, 1, 'registered')).toEqual({ error: 'stale' })
   })
 
+  it.each([true, false])(
+    'reports native disclosure state as %s without inventing it on other elements',
+    (open) => {
+      document.body.innerHTML =
+        '<details><summary>Details</summary></details><dialog></dialog><button>Other</button>'
+      const details = document.querySelector('details')!
+      const dialog = document.querySelector('dialog')!
+      details.open = open
+      dialog.open = open
+      const elements = [
+        details,
+        document.querySelector('summary')!,
+        dialog,
+        document.querySelector('button')!,
+      ]
+      register(...elements.map(visible))
+      for (let id = 0; id < elements.length; id++) {
+        expect(runSerialized(readPageActionState, [false, id, 'registered'])).toMatchObject({
+          targetState: { open: id === 3 ? undefined : open },
+        })
+      }
+    }
+  )
+
+  it.each(['checkbox', 'radio'])('reads native %s state with XHTML tag casing', (type) => {
+    const input = visible(document.createElement('input'))
+    input.type = type
+    input.checked = true
+    Object.defineProperty(input, 'tagName', { value: 'input' })
+    document.body.append(input)
+    register(input)
+
+    expect(runSerialized(readPageActionState, [false, 0, 'registered'])).toMatchObject({
+      targetState: { checked: true },
+    })
+  })
+
   it('preserves native and ARIA mixed states instead of reporting unchecked', () => {
     document.body.innerHTML =
       '<input type="checkbox" /><div role="checkbox" aria-checked="mixed"></div>'
@@ -1086,6 +1146,26 @@ describe('semantic control state', () => {
     expect(readCheckableElementState(1)).toMatchObject({ disabled: true })
   })
 
+  it.each([true, false])(
+    'reads ARIA-disabled=%s across shadow boundaries for waits and checked state',
+    (disabled) => {
+      const host = visible(document.createElement('div'))
+      host.setAttribute('aria-disabled', String(disabled))
+      const nestedHost = visible(document.createElement('div'))
+      host.attachShadow({ mode: 'open' }).append(nestedHost)
+      const checkbox = visible(document.createElement('input'))
+      checkbox.type = 'checkbox'
+      nestedHost.attachShadow({ mode: 'open' }).append(checkbox)
+      document.body.append(host)
+      register(checkbox)
+
+      expect(runSerialized(readCheckableElementState, [0])).toMatchObject({ disabled })
+      expect(runSerialized(readPageActionState, [false, 0, 'registered'])).toMatchObject({
+        targetState: { disabled },
+      })
+    }
+  )
+
   it('reads native and ARIA checkable controls without mutating them', () => {
     document.body.innerHTML = `
       <input type="checkbox" checked />
@@ -1135,6 +1215,19 @@ describe('semantic control state', () => {
     expect(button.scrollIntoView).not.toHaveBeenCalled()
   })
 
+  it('rejects a replacement screenshot target even when its geometry matches', () => {
+    const button = visible(document.createElement('button'))
+    button.id = 'save'
+    button.textContent = 'Save'
+    document.body.append(button)
+    const ref = refFor(outlineOf(collectSnapshot()), 'Save')
+    expect(getElementScreenshotRect(ref)).toMatchObject({ width: 100, height: 20 })
+    button.replaceWith(visible(button.cloneNode(true) as HTMLElement))
+
+    expect(runSerialized(getElementScreenshotRect, [ref])).toMatchObject({ error: 'stale' })
+    expect(window.__simAgentElements?.[ref]).toBe(button)
+  })
+
   it('rejects same-origin frame crops rather than using frame-local coordinates', () => {
     const frame = document.createElement('iframe')
     document.body.append(frame)