fix(browser): distinguish unconfirmed upload outcomes

Author: waleedlatif1

apps/desktop/e2e/browser-tools.spec.ts

@@ -437,6 +437,102 @@ test.describe('browser tools', () => {
     })
   })
 
+  test('reports an unconfirmed upload when cancelled before Chromium acknowledgement arrives', async () => {
+    const { elementId, evaluate } = await openUploadTarget('root')
+    await app.evaluate(({ webContents }, site) => {
+      const contents = webContents
+        .getAllWebContents()
+        .find((contents) => contents.getURL() === `${site}/upload-target?shadow=0&steal=0`)
+      if (!contents) throw new Error('Missing upload acknowledgement fixture')
+      const original = contents.debugger.sendCommand
+      let release = () => {}
+      const acknowledgement = new Promise<void>((resolve) => {
+        release = resolve
+      })
+      const state = {
+        count: 0,
+        applied: false,
+        release,
+        restore: () => {
+          contents.debugger.sendCommand = original
+        },
+      }
+      const globals = globalThis as typeof globalThis & { heldUploadAcknowledgement?: typeof state }
+      globals.heldUploadAcknowledgement = state
+      contents.debugger.sendCommand = async (method, params, sessionId) => {
+        if (method !== 'DOM.setFileInputFiles') {
+          return original.call(contents.debugger, method, params, sessionId)
+        }
+        state.count++
+        const result = await original.call(contents.debugger, method, params, sessionId)
+        state.applied = true
+        await acknowledgement
+        return result
+      }
+    }, site)
+    const pending = execute('browser_upload_file', { elementId, paths: ['files/receipt.txt'] })
+    const toolCallId = `browser-fixture-${callCount}`
+    try {
+      await expect
+        .poll(() => evaluate('window.uploadTestState()'))
+        .toEqual({
+          original: [{ name: 'receipt.txt', text: 'receipt-bytes' }],
+          decoy: [],
+          currentCount: 1,
+        })
+      await expect
+        .poll(() =>
+          app.evaluate(
+            () =>
+              (
+                globalThis as typeof globalThis & {
+                  heldUploadAcknowledgement?: { applied: boolean }
+                }
+              ).heldUploadAcknowledgement?.applied
+          )
+        )
+        .toBe(true)
+      await window.evaluate(
+        async ({ toolCallId, scope }) => {
+          const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
+          if (!api.browserAgent.cancelTool) throw new Error('Browser cancellation is unavailable')
+          await api.browserAgent.cancelTool(toolCallId, scope)
+        },
+        { toolCallId, scope: SCOPE }
+      )
+
+      const upload = await pending
+      expect(upload.ok, JSON.stringify(upload)).toBe(true)
+      expect(upload.result).toMatchObject({
+        outcomeUnknown: true,
+        doNotRetry: true,
+        note: expect.stringContaining('Inspect the page'),
+      })
+      expect(upload.result).not.toHaveProperty('dispatched', true)
+      expect((await execute('browser_list_tabs', {})).ok).toBe(true)
+      expect(
+        await app.evaluate(
+          () =>
+            (
+              globalThis as typeof globalThis & {
+                heldUploadAcknowledgement?: { count: number }
+              }
+            ).heldUploadAcknowledgement?.count
+        )
+      ).toBe(1)
+    } finally {
+      await app.evaluate(() => {
+        const globals = globalThis as typeof globalThis & {
+          heldUploadAcknowledgement?: { release: () => void; restore: () => void }
+        }
+        globals.heldUploadAcknowledgement?.release()
+        globals.heldUploadAcknowledgement?.restore()
+        globals.heldUploadAcknowledgement = undefined
+      })
+      await pending
+    }
+  })
+
   for (const mutation of ['replace', 'disable', 'navigate-frame'] as const) {
     test(`refuses uploads when the target changes during staging: ${mutation}`, async () => {
       const { elementId, evaluate } = await openUploadTarget(

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

@@ -692,10 +692,18 @@ describe('browser-agent file input handles', () => {
     if (change === 'document-closed') document.defaultView = null
     if (change === 'type') input.type = 'text'
     if (change === 'multiple') input.multiple = false
+    const onDispatch = vi.fn()
     try {
       await expect(
-        setFileInputFiles(contents, handle, ['/staged/a.pdf', '/staged/b.pdf'])
+        setFileInputFiles(
+          contents,
+          handle,
+          ['/staged/a.pdf', '/staged/b.pdf'],
+          undefined,
+          onDispatch
+        )
       ).rejects.toThrow(/upload input|upload target/)
+      expect(onDispatch).not.toHaveBeenCalled()
       expect(send.mock.calls.some(([method]) => method === 'DOM.setFileInputFiles')).toBe(false)
     } finally {
       await releaseFileInput(contents, handle)
@@ -737,14 +745,14 @@ describe('browser-agent file input handles', () => {
     const { contents, frame, behavior, send } = await fileInputFixture()
     const handle = await resolveFileInput(contents, frame, 'captureUploadInput(4)')
     const controller = new AbortController()
-    const onDispatched = vi.fn()
+    const onDispatch = vi.fn()
     behavior.afterInputValidation = () => controller.abort()
     try {
       await expect(
-        setFileInputFiles(contents, handle, ['/staged/a.pdf'], controller.signal, onDispatched)
+        setFileInputFiles(contents, handle, ['/staged/a.pdf'], controller.signal, onDispatch)
       ).rejects.toThrow()
       expect(send.mock.calls.some(([method]) => method === 'DOM.setFileInputFiles')).toBe(false)
-      expect(onDispatched).not.toHaveBeenCalled()
+      expect(onDispatch).not.toHaveBeenCalled()
     } finally {
       await releaseFileInput(contents, handle)
     }
@@ -755,12 +763,12 @@ describe('browser-agent file input handles', () => {
     const { contents, frame, behavior, send } = await fileInputFixture(true)
     const handle = await resolveFileInput(contents, frame, 'captureUploadInput(4)')
     behavior.rejectSet = true
-    const onDispatched = vi.fn()
+    const onDispatch = vi.fn()
     try {
       await expect(
-        setFileInputFiles(contents, handle, ['/staged/a.pdf'], undefined, onDispatched)
+        setFileInputFiles(contents, handle, ['/staged/a.pdf'], undefined, onDispatch)
       ).rejects.toThrow('disappeared')
-      expect(onDispatched).toHaveBeenCalledTimes(1)
+      expect(onDispatch.mock.calls).toEqual([['pending']])
     } finally {
       await releaseFileInput(contents, handle)
     }
@@ -791,7 +799,7 @@ describe('browser-agent file input handles', () => {
     }
   })
 
-  it('reports dispatch before acknowledgement so interruption cannot invite a retry', async () => {
+  it('reports pending dispatch while acknowledgement is held, then acknowledges before readback', async () => {
     const { contents, frame, behavior, send } = await fileInputFixture()
     const handle = await resolveFileInput(contents, frame, 'captureUploadInput(4)')
     let acknowledge: () => void = () => {}
@@ -804,19 +812,19 @@ describe('browser-agent file input handles', () => {
     })
     behavior.beforeSet = () => acknowledgement
     behavior.beforeReadback = () => readback
-    const onDispatched = vi.fn()
-    const pending = setFileInputFiles(contents, handle, ['/staged/a.pdf'], undefined, onDispatched)
+    const onDispatch = vi.fn()
+    const pending = setFileInputFiles(contents, handle, ['/staged/a.pdf'], undefined, onDispatch)
     try {
       await vi.waitFor(() =>
         expect(send.mock.calls.some(([method]) => method === 'DOM.setFileInputFiles')).toBe(true)
       )
-      expect(onDispatched).toHaveBeenCalledTimes(1)
+      expect(onDispatch.mock.calls).toEqual([['pending']])
       acknowledge()
-      await vi.waitFor(() => expect(onDispatched).toHaveBeenCalledTimes(1))
+      await vi.waitFor(() => expect(onDispatch.mock.calls).toEqual([['pending'], ['acknowledged']]))
       expect(send.mock.calls.some(([method]) => method === 'Runtime.releaseObject')).toBe(false)
       releaseReadback()
       await expect(pending).resolves.toEqual({ files: [{ name: 'a.pdf', size: 12 }] })
-      expect(onDispatched).toHaveBeenCalledTimes(1)
+      expect(onDispatch.mock.calls).toEqual([['pending'], ['acknowledged']])
     } finally {
       acknowledge()
       releaseReadback()

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

@@ -868,29 +868,29 @@ export async function releaseFileInput(
 }
 
 /**
- * Sets files on the captured input in its original CDP session, then reads that exact input.
- * Marks dispatch before awaiting acknowledgement: cancellation cannot retract the command.
+ * Reports assignment dispatch and acknowledgement separately before reading the captured input.
+ * Stopping a wait cannot revoke a CDP command, so its outcome becomes uncertain before the send.
  */
 export async function setFileInputFiles(
   contents: WebContents,
   handle: FileInputHandle,
   files: readonly string[],
   signal?: AbortSignal,
-  onDispatched?: () => void
+  onDispatch?: (status: 'pending' | 'acknowledged') => void
 ): Promise<{ files: Array<{ name: string; size: number }> } | { readbackError: string }> {
   signal?.throwIfAborted()
   const input = await callFileInput(contents, handle, 'input', files.length)
   if (!input.objectId) throw new Error('Chromium did not retain the upload input node')
   try {
     signal?.throwIfAborted()
-    const assignment = send(
+    onDispatch?.('pending')
+    await send(
       contents,
       'DOM.setFileInputFiles',
       { files, objectId: input.objectId },
       handle.sessionId
     )
-    onDispatched?.()
-    await assignment
+    onDispatch?.('acknowledged')
     try {
       const { value } = await callFileInput(contents, handle, 'files')
       if (!isRecordLike(value) || !Array.isArray(value.files)) {

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

@@ -3835,7 +3835,7 @@ describe('credential protection', () => {
     })
 
     it.each(['staging', 'attachment'])(
-      'releases the pinned input after %s fails',
+      'releases the pinned input after %s fails before dispatch',
       async (failure) => {
         const contents = await openPage()
         const input = { objectId: 'isolated-input', multiple: false }
@@ -3855,7 +3855,7 @@ describe('credential protection', () => {
           'call-failed'
         )
 
-        expect(result).toMatchObject({
+        expect(result).toEqual({
           ok: false,
           error: expect.stringContaining(`${failure} failed`),
         })
@@ -3935,8 +3935,8 @@ describe('credential protection', () => {
           await expect(pending).resolves.toMatchObject({
             ok: true,
             result: {
-              dispatched: true,
-              observation: { ok: false, doNotRetry: true },
+              outcomeUnknown: true,
+              doNotRetry: true,
             },
           })
           await expect(
@@ -3971,8 +3971,9 @@ describe('credential protection', () => {
         )
         const setFiles = vi
           .spyOn(cdp, 'setFileInputFiles')
-          .mockImplementation(async (_contents, _handle, _files, _signal, onDispatched) => {
-            onDispatched?.()
+          .mockImplementation(async (_contents, _handle, _files, _signal, onDispatch) => {
+            onDispatch?.('pending')
+            onDispatch?.('acknowledged')
             return readback
           })
         const release = vi.spyOn(cdp, 'releaseFileInput').mockResolvedValue()
@@ -4021,6 +4022,98 @@ describe('credential protection', () => {
         expect(setFiles).toHaveBeenCalledTimes(1)
       }
     )
+
+    it.each(['cancelled', 'timed out'] as const)(
+      'reports an unconfirmed upload when its acknowledgment is %s without replaying it or affecting queued work',
+      async (stop) => {
+        const contents = await openPage()
+        const input = { objectId: 'isolated-input', multiple: false }
+        vi.spyOn(cdp, 'resolveFileInput').mockResolvedValue(input)
+        let acknowledgeUpload: () => void = () => {}
+        const acknowledgment = new Promise<void>((resolve) => {
+          acknowledgeUpload = resolve
+        })
+        let appliedUploads = 0
+        const setFiles = vi
+          .spyOn(cdp, 'setFileInputFiles')
+          .mockImplementation(async (_contents, _handle, _files, _signal, onDispatch) => {
+            onDispatch?.('pending')
+            appliedUploads++
+            await acknowledgment
+            onDispatch?.('acknowledged')
+            return { files: [{ name: 'a.pdf', size: 3 }] }
+          })
+        const release = vi.spyOn(cdp, 'releaseFileInput').mockResolvedValue()
+        stageUploadFiles.mockResolvedValue(['/staged/a.pdf'])
+        let releaseSnapshot: (value: unknown) => void = () => {}
+        const snapshot = new Promise<unknown>((resolve) => {
+          releaseSnapshot = resolve
+        })
+        let snapshotStarted = false
+        vi.mocked(contents.executeJavaScript).mockImplementation((expression) => {
+          if (isPageCall(expression, 'collectSnapshot')) {
+            snapshotStarted = true
+            return snapshot
+          }
+          return Promise.resolve({})
+        })
+
+        vi.useFakeTimers()
+        try {
+          const timersBefore = vi.getTimerCount()
+          const pending = driver.executeTool(
+            'chat-test',
+            'browser_upload_file',
+            { elementId: 0, paths: ['files/a.pdf'] },
+            'unconfirmed-upload'
+          )
+          await vi.advanceTimersByTimeAsync(200)
+          expect(appliedUploads).toBe(1)
+          const queued = driver.executeTool('chat-test', 'browser_snapshot', {}, 'next-snapshot')
+
+          if (stop === 'cancelled') driver.cancelTool('chat-test', 'unconfirmed-upload')
+          else
+            await vi.advanceTimersByTimeAsync(
+              driver.browserToolWatchdogMs('browser_upload_file', {})!
+            )
+
+          const result = await pending
+          expect(result).toMatchObject({
+            ok: true,
+            result: {
+              outcomeUnknown: true,
+              doNotRetry: true,
+              error: expect.any(String),
+              note: expect.stringContaining('The action may already have run'),
+            },
+          })
+          expect(result.result).not.toHaveProperty('dispatched')
+          await vi.advanceTimersByTimeAsync(0)
+          expect(snapshotStarted).toBe(true)
+          expect(release).not.toHaveBeenCalled()
+
+          acknowledgeUpload()
+          await vi.advanceTimersByTimeAsync(200)
+          expect(release).toHaveBeenCalledExactlyOnceWith(contents, input)
+          expect(appliedUploads).toBe(1)
+          expect(setFiles).toHaveBeenCalledTimes(1)
+          expect(result.result).toMatchObject({ outcomeUnknown: true, doNotRetry: true })
+          expect(result.result).not.toHaveProperty('dispatched')
+
+          driver.cancelTool('chat-test', 'next-snapshot')
+          await expect(queued).resolves.toEqual({
+            ok: false,
+            error: expect.stringContaining('cancelled'),
+          })
+          expect(vi.getTimerCount()).toBe(timersBefore)
+        } finally {
+          acknowledgeUpload()
+          releaseSnapshot({ outline: 'Late snapshot', refIds: [], nextElementId: 1 })
+          await vi.advanceTimersByTimeAsync(200)
+          vi.useRealTimers()
+        }
+      }
+    )
   })
 
   it('validates upload paths before touching the page', async () => {