From 61d11e035d95c5749cd311274814731c8ccbcbed Mon Sep 17 00:00:00 2001 From: Nikhil Sonti Date: Mon, 16 Mar 2026 18:15:59 -0700 Subject: [PATCH] fix: address review feedback and add upload_file tests - Only fall through to file chooser fallback when error is specifically about the node not being a file input; re-throw other errors (e.g. invalid file paths) immediately - Reject with clear error when fileChooserOpened event lacks backendNodeId instead of falling back to the wrong node - Add tests for direct file input upload and button-triggered file chooser Co-Authored-By: Claude Opus 4.6 (1M context) --- .../apps/server/src/browser/browser.ts | 15 ++- .../apps/server/tests/tools/input.test.ts | 97 +++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/packages/browseros-agent/apps/server/src/browser/browser.ts b/packages/browseros-agent/apps/server/src/browser/browser.ts index 686b9232..e3ed6b5f 100644 --- a/packages/browseros-agent/apps/server/src/browser/browser.ts +++ b/packages/browseros-agent/apps/server/src/browser/browser.ts @@ -1020,8 +1020,9 @@ export class Browser { try { await session.DOM.setFileInputFiles({ files, backendNodeId: element }) return - } catch { - // Element is not a file input — fall back to file chooser interception + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + if (!msg.includes('file input')) throw err } // Fallback: intercept the native file chooser dialog triggered by clicking the element @@ -1042,7 +1043,15 @@ export class Browser { (event) => { clearTimeout(timeout) unsubscribe() - resolve(event.backendNodeId ?? element) + if (event.backendNodeId == null) { + reject( + new Error( + 'fileChooserOpened event did not include a backendNodeId; cannot set files.', + ), + ) + return + } + resolve(event.backendNodeId) }, ) }) diff --git a/packages/browseros-agent/apps/server/tests/tools/input.test.ts b/packages/browseros-agent/apps/server/tests/tools/input.test.ts index 163fa59c..d8cb3540 100644 --- a/packages/browseros-agent/apps/server/tests/tools/input.test.ts +++ b/packages/browseros-agent/apps/server/tests/tools/input.test.ts @@ -9,6 +9,7 @@ import { scroll, select_option, uncheck, + upload_file, } from '../../src/tools/input' import { close_page, new_page } from '../../src/tools/navigation' import { evaluate_script, take_snapshot } from '../../src/tools/snapshot' @@ -371,3 +372,99 @@ describe('input tools', () => { }) }, 60_000) }) + +const UPLOAD_PAGE = `data:text/html,${encodeURIComponent(` + +

Upload Test

+ + + + +
+ +`)}` + +describe('upload_file tool', () => { + it('uploads a file via a direct ', async () => { + await withBrowser(async ({ execute }) => { + const newResult = await execute(new_page, { url: UPLOAD_PAGE }) + const pageId = pageIdOf(newResult) + + // Get the file input's backendNodeId from the snapshot + const snap = await execute(take_snapshot, { page: pageId }) + const snapText = textOf(snap) + const fileInputId = findElementId(snapText, 'Direct Upload') + + const tmpPath = '/tmp/browseros-test-upload.txt' + await Bun.write(tmpPath, 'test file content') + + const uploadResult = await execute(upload_file, { + page: pageId, + element: fileInputId, + files: [tmpPath], + }) + assert.ok(!uploadResult.isError, textOf(uploadResult)) + assert.ok(textOf(uploadResult).includes('file(s)')) + + const data = structuredOf<{ + action: string + fileCount: number + }>(uploadResult) + assert.strictEqual(data.action, 'upload_file') + assert.strictEqual(data.fileCount, 1) + + const result = await execute(evaluate_script, { + page: pageId, + expression: 'document.getElementById("result").textContent', + }) + assert.strictEqual( + textOf(result), + 'direct:browseros-test-upload.txt', + ) + + await execute(close_page, { page: pageId }) + }) + }, 60_000) + + it('uploads a file via a button that triggers a file chooser', async () => { + await withBrowser(async ({ execute }) => { + const newResult = await execute(new_page, { url: UPLOAD_PAGE }) + const pageId = pageIdOf(newResult) + + const snap = await execute(take_snapshot, { page: pageId }) + const snapText = textOf(snap) + const btnId = findElementId(snapText, 'Proxy Upload') + + const tmpPath = '/tmp/browseros-test-upload-btn.txt' + await Bun.write(tmpPath, 'button upload content') + + const uploadResult = await execute(upload_file, { + page: pageId, + element: btnId, + files: [tmpPath], + }) + assert.ok(!uploadResult.isError, textOf(uploadResult)) + + const result = await execute(evaluate_script, { + page: pageId, + expression: 'document.getElementById("result").textContent', + }) + assert.strictEqual( + textOf(result), + 'button:browseros-test-upload-btn.txt', + ) + + await execute(close_page, { page: pageId }) + }) + }, 60_000) +})