fix(browser): keep new markers out of find and time out a hung name claim

- browser_find collects its outline with markNew off, so a search never
  matches the `new` token and never consumes markers the next snapshot owes.
- An element counts as shown only once its line is emitted.
- The placeholder claim runs inside the allocation timeout, so a hung
  filesystem stops the download instead of leaving it paused with its name
  reserved; a late placeholder is removed when the claim settles.
- A successful move drops the placeholder pointer, and an already-aborted
  click sends no input at all, not even the pointer move.

Author: waleedlatif1

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

@@ -282,7 +282,7 @@ describe('browser-agent CDP instrumentation', () => {
     controller.abort()
 
     await expect(
-      clickAt(contents, 5, 6, false, PRIMARY_CLICK, controller.signal)
+      clickAt(contents, 5, 6, true, PRIMARY_CLICK, controller.signal)
     ).rejects.toMatchObject({ name: 'AbortError' })
     expect(contents.debugger.sendCommand).not.toHaveBeenCalled()
   })

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

@@ -984,7 +984,7 @@ export function clearAgentContextMenu(contents: WebContents): void {
 }
 
 /**
- * Clicks at viewport coordinates. An already-aborted `signal` rejects before anything is pressed.
+ * Clicks at viewport coordinates. An already-aborted `signal` rejects before any input is sent.
  * During a press-and-hold it ends the hold early: the click rejects with the abort reason and the
  * button is released at once, so a cancelled or timed-out click cannot stay held into the next
  * action. That release can still activate the control under the pointer.
@@ -997,6 +997,7 @@ export async function clickAt(
   click: PointerClick = PRIMARY_CLICK,
   signal?: AbortSignal
 ): Promise<void> {
+  signal?.throwIfAborted()
   if (moveBeforePress) await moveMouse(contents, x, y)
   signal?.throwIfAborted()
   const { button, clickCount, modifiers, holdMs } = click

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

@@ -4661,9 +4661,12 @@ describe('credential protection', () => {
     })
   })
 
-  it.each(['browser_snapshot', 'browser_find'] as const)(
-    'omits an absent scope from the serialized %s page call',
-    async (tool) => {
+  it.each([
+    ['browser_snapshot', 'true'],
+    ['browser_find', 'false'],
+  ] as const)(
+    'passes an absent scope as null to the serialized %s page call',
+    async (tool, markNew) => {
       const contents = await openPage()
       vi.mocked(contents.executeJavaScript).mockClear()
       await driver.executeTool('chat-test', tool, { query: 'Continue' })
@@ -4672,13 +4675,16 @@ describe('credential protection', () => {
         .mock.calls.map(([expression]) => expression)
         .filter((expression) => isPageCall(expression, 'collectSnapshot'))
       expect(expressions).toHaveLength(1)
-      expect(expressions[0]).toContain('.apply(null, [1])')
+      expect(expressions[0]).toContain(`.apply(null, [1,null,${markNew}])`)
     }
   )
 
-  it.each(['browser_snapshot', 'browser_find'] as const)(
+  it.each([
+    ['browser_snapshot', 'true'],
+    ['browser_find', 'false'],
+  ] as const)(
     'passes the current root ref to %s and invalidates previous refs',
-    async (tool) => {
+    async (tool, markNew) => {
       const contents = await openPage()
       respondWith(contents, {
         collectSnapshot: {
@@ -4700,7 +4706,7 @@ describe('credential protection', () => {
           .mock.calls.some(
             ([expression]) =>
               isPageCall(expression, 'collectSnapshot') &&
-              expression.includes('.apply(null, [1,0])')
+              expression.includes(`.apply(null, [1,0,${markNew}])`)
           )
       ).toBe(true)
       const stale = await driver.executeTool('chat-test', 'browser_click', { elementId: 0 })

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

@@ -2285,12 +2285,14 @@ function validateSnapshotRefs(
  * Captures the top page plus each cross-origin boundary frame in its own
  * isolated world. CDP is the privileged bridge the top document's same-origin
  * policy intentionally lacks; password redaction still runs inside every frame
- * before any result crosses back to the driver.
+ * before any result crosses back to the driver. `markNew` is false for reads the
+ * model never sees as an outline, so they neither carry nor consume `new` markers.
  */
 async function captureSnapshot(
   contents: WebContents,
   notAfter?: number,
-  elementId?: number
+  elementId?: number,
+  markNew = true
 ): Promise<unknown> {
   const state = driverScopeState()
   const tab = session.requireAutomationTab()
@@ -2326,7 +2328,7 @@ async function captureSnapshot(
     await execInPage(
       contents,
       collectSnapshot,
-      elementId === undefined ? [mainStartingElementId] : [mainStartingElementId, elementId],
+      [mainStartingElementId, elementId ?? null, markNew],
       false,
       notAfter
     )
@@ -2383,7 +2385,7 @@ async function captureSnapshot(
       const frameSnapshot = await execInPage(
         frame,
         collectSnapshot,
-        [frameStartingElementId],
+        [frameStartingElementId, null, markNew],
         false,
         notAfter
       )
@@ -2867,7 +2869,7 @@ async function executeToolInner(
       const maxResults = Math.min(50, Math.max(1, Math.floor(requestedMax ?? 20)))
       const contents = session.requireAutomationTab().view.webContents
       const snapshot = toRecord(
-        await captureSnapshot(contents, executionDeadline, num(params, 'elementId'))
+        await captureSnapshot(contents, executionDeadline, num(params, 'elementId'), false)
       )
       const outline = typeof snapshot.outline === 'string' ? snapshot.outline : ''
       const needle = query.toLowerCase()

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

@@ -768,6 +768,20 @@ describe('collectSnapshot', () => {
     expect(outlineOf(collectSnapshot())).not.toContain(' new')
   })
 
+  it('leaves new markers out of an unmarked read without consuming them', () => {
+    document.body.innerHTML = '<button>Compose</button>'
+    visible(document.querySelector('button') as HTMLButtonElement)
+    collectSnapshot()
+
+    const added = document.createElement('button')
+    added.textContent = 'Send'
+    document.body.append(visible(added))
+
+    expect(outlineOf(collectSnapshot(0, null, false))).not.toContain(' new')
+    const lines = outlineOf(collectSnapshot()).split('\n')
+    expect(lines.find((line) => line.includes('"Send"'))).toMatch(/ new$/)
+  })
+
   it('sanitizes a malicious role so it cannot forge a second snapshot line', () => {
     document.body.innerHTML = '<div tabindex="0" aria-label="Safe control"></div>'
     const control = visible(document.querySelector('div') as HTMLDivElement)

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

@@ -128,16 +128,22 @@ export function installPageHelpers(): void {
  * Builds the page snapshot: a structural outline (headings, landmarks) with
  * interactive elements carrying numeric ids, walking open shadow roots and
  * same-origin iframes. Rebuilds the element registry as a side effect.
+ * `markNew` false leaves the `new` markers out and records nothing as shown,
+ * for internal reads (such as a text search) whose outline the model never sees.
  */
-export function collectSnapshot(startingElementId = 0, elementId?: number): unknown {
+export function collectSnapshot(
+  startingElementId = 0,
+  elementId: number | null = null,
+  markNew = true
+): unknown {
   const resolver = window.__simAgentResolveElement
   const scopedRoot =
-    elementId === undefined
+    elementId === null
       ? undefined
       : resolver
         ? resolver(elementId, false)?.element
         : window.__simAgentElements?.[elementId]
-  if (elementId !== undefined) {
+  if (elementId !== null) {
     if (!scopedRoot?.isConnected) return { error: 'stale', reason: window.__simAgentStaleReason }
     if (scopedRoot.ownerDocument !== document) return { error: 'framed-snapshot' }
   }
@@ -238,9 +244,13 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn
   window.__simAgentElements = registry
   const previouslyShown = window.__simAgentShownElements
   const shown = previouslyShown ?? new WeakSet<Element>()
-  window.__simAgentShownElements = shown
+  if (markNew) window.__simAgentShownElements = shown
   /** Whether no earlier snapshot of this document listed the element; the first snapshot marks nothing. */
-  const isNew = (el: Element): boolean => previouslyShown !== undefined && !shown.has(el)
+  const isNew = (el: Element): boolean => markNew && previouslyShown !== undefined && !shown.has(el)
+  /** Records an element whose line made it into the outline, so the next snapshot knows it. */
+  const recordShown = (el: Element): void => {
+    if (markNew) shown.add(el)
+  }
   const lines: string[] = []
   let truncated = false
   let refCount = 0
@@ -585,11 +595,11 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn
       }
     }
     if (isNew(el)) parts.push('new')
-    shown.add(el)
     const suffix = parts.length > 0 ? ` ${parts.join(' ')}` : ''
     const lineIndex = lines.length
     if (push(`${indent}- ${role} ${quote(name)} [ref=${id}]${suffix}`)) {
       refLineIndexes[id] = lineIndex
+      recordShown(el)
     }
   }
 
@@ -608,9 +618,11 @@ export function collectSnapshot(startingElementId = 0, elementId?: number): unkn
     const id = registerElement(el, roleFor(el), text)
     textLineCount++
     const marker = isNew(el) ? ' new' : ''
-    shown.add(el)
     const lineIndex = lines.length
-    if (push(`${indent}- text ${quote(text)} [ref=${id}]${marker}`)) refLineIndexes[id] = lineIndex
+    if (push(`${indent}- text ${quote(text)} [ref=${id}]${marker}`)) {
+      refLineIndexes[id] = lineIndex
+      recordShown(el)
+    }
   }
 
   const headingLevel = (el: Element): number | null => {

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

@@ -4221,6 +4221,39 @@ describe('browser-agent session', () => {
     })
   })
 
+  it('stops a download whose placeholder claim hangs and removes the late placeholder', async () => {
+    vi.useFakeTimers()
+    try {
+      const directory = mkdtempSync(join(tmpdir(), 'sim-browser-downloads-'))
+      const gate = deferred<void>()
+      const claimFile = vi.fn(async (path: string) => {
+        await gate.promise
+        writeFileSync(path, '', { flag: 'wx' })
+      })
+      session = freshSession(win, {}, undefined, {
+        getDirectory: () => directory,
+        getFreeDiskBytes: () => Number.MAX_SAFE_INTEGER,
+        pathExists: () => false,
+        claimFile,
+      })
+      const contents = (session.ensureTab().view as unknown as MockView).webContents
+      const download = mockDownloadItem({ filename: 'hung-claim.bin', totalBytes: 100 })
+
+      startMockDownload(contents, download)
+      await vi.advanceTimersByTimeAsync(0)
+      expect(claimFile).toHaveBeenCalledOnce()
+      await vi.advanceTimersByTimeAsync(5_000)
+
+      expect(download.item.cancel).toHaveBeenCalledOnce()
+      expect(download.item.resume).not.toHaveBeenCalled()
+      download.emitDone('cancelled')
+      gate.resolve()
+      await vi.waitFor(() => expect(readdirSync(directory)).toEqual([]))
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('keeps a torn-down download name reserved until its pending claim settles', async () => {
     const directory = mkdtempSync(join(tmpdir(), 'sim-browser-downloads-'))
     const claims: Array<{ path: string; gate: ReturnType<typeof deferred<void>> }> = []

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

@@ -795,6 +795,8 @@ async function moveStagedBrowserDownload(active: ActiveBrowserDownload): Promise
   for (let attempt = 1; ; attempt++) {
     try {
       await moveFile(active.stagingPath, destination)
+      // The name now holds the finished file, which no cleanup may remove as a placeholder.
+      active.placeholderPath = undefined
       return destination
     } catch (error) {
       const code = (error as NodeJS.ErrnoException).code
@@ -1737,27 +1739,33 @@ function configureBrowserDownloads(ses: Session): void {
         })
     })
     let allocationExpired = false
+    const allocationLive = () =>
+      !allocationExpired &&
+      !active.terminal &&
+      !active.limitReason &&
+      activeBrowserDownloads.has(active)
     const allocation = uniqueDownloadPath(directory, filename, {
-      isActive: () =>
-        !allocationExpired &&
-        !active.terminal &&
-        !active.limitReason &&
-        activeBrowserDownloads.has(active),
+      isActive: allocationLive,
       pathExists: browserDownloadSettings?.pathExists,
       reservePath: (candidate) => {
-        if (
-          allocationExpired ||
-          active.terminal ||
-          active.limitReason ||
-          !activeBrowserDownloads.has(active) ||
-          activeDownloadPaths.has(candidate)
-        ) {
-          return false
-        }
+        if (!allocationLive() || activeDownloadPaths.has(candidate)) return false
         activeDownloadPaths.set(candidate, active)
         active.savePath = candidate
         return true
       },
+    }).then(async (savePath) => {
+      if (!savePath || !allocationLive()) return savePath
+      active.claimingDestination = true
+      try {
+        await claimBrowserDownloadDestination(active, savePath)
+      } finally {
+        active.claimingDestination = false
+        if (!allocationLive()) {
+          removeBrowserDownloadPlaceholder(active)
+          releaseActiveBrowserDownloadPath(active, savePath)
+        }
+      }
+      return savePath
     })
     active.destination = withBrowserDownloadTimeout(
       allocation,
@@ -1767,7 +1775,7 @@ function configureBrowserDownloads(ses: Session): void {
         allocationExpired = true
       }
     )
-      .then(async (savePath) => {
+      .then((savePath) => {
         if (active.terminal || !activeBrowserDownloads.has(active)) {
           releaseActiveBrowserDownloadPath(active, savePath ?? undefined)
           return null
@@ -1780,17 +1788,6 @@ function configureBrowserDownloads(ses: Session): void {
           publishActiveBrowserDownload(active)
           return null
         }
-        active.claimingDestination = true
-        try {
-          await claimBrowserDownloadDestination(active, savePath)
-        } finally {
-          active.claimingDestination = false
-          if (active.terminal || !activeBrowserDownloads.has(active)) {
-            removeBrowserDownloadPlaceholder(active)
-            releaseActiveBrowserDownloadPath(active, savePath)
-          }
-        }
-        if (active.terminal || !activeBrowserDownloads.has(active)) return null
         checkBrowserDownloadDiskSpace(active, 'admission')
         return savePath
       })