fix(browser): preserve upload dispatch before acknowledgement

Author: icecrasher321

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

@@ -760,7 +760,7 @@ describe('browser-agent file input handles', () => {
       await expect(
         setFileInputFiles(contents, handle, ['/staged/a.pdf'], undefined, onDispatched)
       ).rejects.toThrow('disappeared')
-      expect(onDispatched).not.toHaveBeenCalled()
+      expect(onDispatched).toHaveBeenCalledTimes(1)
     } finally {
       await releaseFileInput(contents, handle)
     }
@@ -791,7 +791,7 @@ describe('browser-agent file input handles', () => {
     }
   })
 
-  it('reports dispatch only after acknowledgement and before a pending readback', async () => {
+  it('reports dispatch before acknowledgement so interruption cannot invite a retry', async () => {
     const { contents, frame, behavior, send } = await fileInputFixture()
     const handle = await resolveFileInput(contents, frame, 'captureUploadInput(4)')
     let acknowledge: () => void = () => {}
@@ -810,7 +810,7 @@ describe('browser-agent file input handles', () => {
       await vi.waitFor(() =>
         expect(send.mock.calls.some(([method]) => method === 'DOM.setFileInputFiles')).toBe(true)
       )
-      expect(onDispatched).not.toHaveBeenCalled()
+      expect(onDispatched).toHaveBeenCalledTimes(1)
       acknowledge()
       await vi.waitFor(() => expect(onDispatched).toHaveBeenCalledTimes(1))
       expect(send.mock.calls.some(([method]) => method === 'Runtime.releaseObject')).toBe(false)

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

@@ -867,7 +867,10 @@ export async function releaseFileInput(
   await releaseRemoteObject(contents, handle.objectId, handle.sessionId)
 }
 
-/** Sets files on the captured input in its original CDP session, then reads that exact input. */
+/**
+ * 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.
+ */
 export async function setFileInputFiles(
   contents: WebContents,
   handle: FileInputHandle,
@@ -880,13 +883,14 @@ export async function setFileInputFiles(
   if (!input.objectId) throw new Error('Chromium did not retain the upload input node')
   try {
     signal?.throwIfAborted()
-    await send(
+    const assignment = send(
       contents,
       'DOM.setFileInputFiles',
       { files, objectId: input.objectId },
       handle.sessionId
     )
     onDispatched?.()
+    await assignment
     try {
       const { value } = await callFileInput(contents, handle, 'files')
       if (!isRecordLike(value) || !Array.isArray(value.files)) {

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

@@ -2524,7 +2524,7 @@ describe('credential protection', () => {
             observation: {
               ok: false,
               doNotRetry: true,
-              note: expect.stringContaining('The action already ran'),
+              note: expect.stringContaining('The action was dispatched'),
             },
           },
         })
@@ -3892,6 +3892,69 @@ describe('credential protection', () => {
       expect(release).toHaveBeenCalledWith(contents, input)
     })
 
+    it.each(['cancelled', 'timed out'] as const)(
+      'does not retry a dispatched upload when acknowledgement is %s',
+      async (stop) => {
+        const contents = await openPage()
+        const input = { objectId: 'isolated-input', multiple: false }
+        vi.spyOn(cdp, 'resolveFileInput').mockResolvedValue(input)
+        stageUploadFiles.mockResolvedValue(['/staged/a.pdf'])
+        let acknowledge: () => void = () => {}
+        const acknowledgement = new Promise<void>((resolve) => {
+          acknowledge = resolve
+        })
+        const send = vi.mocked(contents.debugger.sendCommand)
+        send.mockImplementation(async (method, params) => {
+          if (method === 'Runtime.callFunctionOn') {
+            const mode = (params?.arguments as Array<{ value: string }>)[0].value
+            return mode === 'input'
+              ? { result: { objectId: 'original-input' } }
+              : { result: { value: { files: [{ name: 'a.pdf', size: 3 }] } } }
+          }
+          if (method === 'DOM.setFileInputFiles') await acknowledgement
+          return {}
+        })
+        vi.useFakeTimers()
+        try {
+          const pending = driver.executeTool(
+            'chat-test',
+            'browser_upload_file',
+            { elementId: 0, paths: ['files/a.pdf'] },
+            'unacknowledged-upload'
+          )
+          await vi.advanceTimersByTimeAsync(200)
+          expect(cdpCalls(contents, 'DOM.setFileInputFiles')).toHaveLength(1)
+          expect(cdpCalls(contents, 'Runtime.releaseObject')).toHaveLength(0)
+
+          if (stop === 'cancelled') driver.cancelTool('chat-test', 'unacknowledged-upload')
+          else
+            await vi.advanceTimersByTimeAsync(
+              driver.browserToolWatchdogMs('browser_upload_file', {})!
+            )
+
+          await expect(pending).resolves.toMatchObject({
+            ok: true,
+            result: {
+              dispatched: true,
+              observation: { ok: false, doNotRetry: true },
+            },
+          })
+          await expect(
+            driver.executeTool('chat-test', 'browser_list_tabs', {}, 'after-unacknowledged-upload')
+          ).resolves.toMatchObject({ ok: true })
+        } finally {
+          acknowledge()
+          await vi.advanceTimersByTimeAsync(200)
+          vi.useRealTimers()
+        }
+        expect(cdpCalls(contents, 'DOM.setFileInputFiles')).toHaveLength(1)
+        expect(cdpCalls(contents, 'Runtime.releaseObject').map(([, params]) => params)).toEqual([
+          { objectId: 'original-input' },
+          { objectId: 'isolated-input' },
+        ])
+      }
+    )
+
     it.each(['cancelled', 'timed out'] as const)(
       'retains an acknowledged upload when readback is %s and releases its handle when readback settles',
       async (stop) => {
@@ -3941,7 +4004,7 @@ describe('credential protection', () => {
               observation: {
                 ok: false,
                 doNotRetry: true,
-                note: expect.stringContaining('The action already ran'),
+                note: expect.stringContaining('The action was dispatched'),
               },
             },
           })

apps/desktop/src/main/browser-agent/post-action-observation.ts

@@ -12,13 +12,13 @@ const OBSERVABLE_ACTIONS: ReadonlySet<BrowserToolName> = new Set([
   'browser_hover',
 ])
 
-/** Keeps a completed action available when its optional observation fails or is interrupted. */
+/** Preserves a dispatched action when its acknowledgement or observation is interrupted. */
 export function withFailedPostActionObservation(result: unknown, error: unknown): unknown {
   const observation = {
     ok: false,
     error: getErrorMessage(error),
     doNotRetry: true,
-    note: 'The action already ran. Inspect its result; do not repeat it just because observation failed.',
+    note: 'The action was dispatched. Inspect its result; do not repeat it just because confirmation failed.',
   }
   return isRecordLike(result) ? { ...result, observation } : { result, observation }
 }