diff --git a/src/chrome/src/agent/agent.js b/src/chrome/src/agent/agent.js index 5ceefe2d2..e76d32572 100644 --- a/src/chrome/src/agent/agent.js +++ b/src/chrome/src/agent/agent.js @@ -16631,7 +16631,13 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d // small integer id (returned by download_files/list_downloads and // auto-pinned to the scratchpad) is easy to carry. Resolve it to the real // path here so the rest of the handler is unchanged. If both are present, - // downloadId wins: filePath may be stale or invented. + // prefer a valid downloadId because filePath may be stale or invented, but + // keep the supplied path as a fallback. Models occasionally add a guessed + // id to an otherwise valid absolute path; that extra bad id must not make + // the path-based upload fail. + const suppliedFilePath = typeof args.filePath === 'string' && args.filePath.trim() + ? args.filePath + : null; if (args.downloadId != null) { try { const items = await new Promise((resolve, reject) => { @@ -16641,12 +16647,17 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d }); }); const it = items && items[0]; - if (!it) return { success: false, error: `No download found for downloadId ${args.downloadId}. Use list_downloads to see valid ids.` }; - if (it.state !== 'complete') return { success: false, error: `Download ${args.downloadId} is "${it.state}", not complete — wait for it to finish (wait_for_stable) then retry.` }; - if (!it.filename) return { success: false, error: `Download ${args.downloadId} has no resolved local path yet. Retry shortly, or use list_downloads to find the path and pass filePath.` }; - args.filePath = it.filename; + if (!it) { + if (!suppliedFilePath) return { success: false, error: `No download found for downloadId ${args.downloadId}. Use list_downloads to see valid ids.` }; + } else if (it.state !== 'complete') { + if (!suppliedFilePath) return { success: false, error: `Download ${args.downloadId} is "${it.state}", not complete — wait for it to finish (wait_for_stable) then retry.` }; + } else if (!it.filename) { + if (!suppliedFilePath) return { success: false, error: `Download ${args.downloadId} has no resolved local path yet. Retry shortly, or use list_downloads to find the path and pass filePath.` }; + } else { + args.filePath = it.filename; + } } catch (e) { - return { success: false, error: `Could not resolve downloadId ${args.downloadId}: ${e.message}` }; + if (!suppliedFilePath) return { success: false, error: `Could not resolve downloadId ${args.downloadId}: ${e.message}` }; } } if (!args.filePath) { diff --git a/src/chrome/src/agent/tools.js b/src/chrome/src/agent/tools.js index 94fb26a5b..f5ccde587 100644 --- a/src/chrome/src/agent/tools.js +++ b/src/chrome/src/agent/tools.js @@ -923,7 +923,7 @@ export const AGENT_TOOLS = [ type: 'function', function: { name: 'upload_file', - description: 'Upload a file directly to an existing file input without opening the page or OS file-picker dialog. Do NOT click "Choose file", "Select a file", an upload drop zone, or the input first when the input already exists. Provide EITHER downloadId (preferred — the id from download_files/list_downloads; you do not need to recall the path) OR filePath (absolute local path). If no file input exists because the widget creates it lazily, one guarded click on its add-files control may initialize the widget; then retry upload_file with the exact selector returned or discovered. The file must exist on the local filesystem.', + description: 'Upload a file directly to an existing file input without opening the page or OS file-picker dialog. Do NOT click "Choose file", "Select a file", an upload drop zone, or the input first when the input already exists. Provide EITHER downloadId (preferred — the id from download_files/list_downloads; you do not need to recall the path) OR filePath (absolute local path). Never guess a downloadId. If both are accidentally provided, a valid downloadId is preferred; if that id cannot resolve, the supplied filePath is used as a fallback. If no file input exists because the widget creates it lazily, one guarded click on its add-files control may initialize the widget; then retry upload_file with the exact selector returned or discovered. The file must exist on the local filesystem.', parameters: { type: 'object', properties: { diff --git a/test/run.js b/test/run.js index 2df2aff08..fc08d710f 100644 --- a/test/run.js +++ b/test/run.js @@ -49900,7 +49900,7 @@ test('Chrome click paths suppress native file choosers and redirect to upload_fi } }); -test('upload_file prefers downloadId over a supplied stale filePath (chrome)', async () => { +test('upload_file prefers a valid downloadId and falls back to filePath for an invalid id (chrome)', async () => { const originalChrome = globalThis.chrome; const originalCdp = { attach: cdpClientCh.attach, @@ -49912,18 +49912,21 @@ test('upload_file prefers downloadId over a supplied stale filePath (chrome)', a }; const realPath = '/Users/x/Downloads/real.zip'; const stalePath = '/Users/Shared/made-up.zip'; + const exactPath = '/root/Downloads/github-avatar-carloslopez.jpg'; const uploaded = []; const releasedGroups = []; let queryCount = 0; let selectorMatches = ['input-501']; + let expectedPath = realPath; try { globalThis.chrome = { runtime: { lastError: null }, downloads: { search(query, cb) { - assert.deepEqual(query, { id: 9123 }); - cb([{ id: 9123, state: 'complete', filename: realPath }]); + if (query.id === 9123) cb([{ id: 9123, state: 'complete', filename: realPath }]); + else if (query.id === 0) cb([]); + else assert.fail(`unexpected download id ${query.id}`); }, }, }; @@ -49936,14 +49939,14 @@ test('upload_file prefers downloadId over a supplied stale filePath (chrome)', a releasedGroups.push(objectGroup); }; cdpClientCh.probeLocalFile = async (_tabId, filePath) => { - assert.equal(filePath, realPath, 'downloadId-resolved path should override stale filePath before probing'); + assert.equal(filePath, expectedPath); return { exists: true, readable: true, size: 123 }; }; cdpClientCh.setFileInputFiles = async (_tabId, objectId, files) => { assert.equal(objectId, 'input-501'); uploaded.push(files); }; - cdpClientCh.getFileInputFiles = async () => [{ name: 'real.zip', size: 123, readable: true }]; + cdpClientCh.getFileInputFiles = async () => [{ name: expectedPath.split('/').pop(), size: 123, readable: true }]; const agent = new AgentCh({}); const args = { selector: 'input[type=file]', downloadId: 9123, filePath: stalePath }; @@ -49955,6 +49958,15 @@ test('upload_file prefers downloadId over a supplied stale filePath (chrome)', a assert.deepEqual(uploaded, [[realPath]]); assert.deepEqual(releasedGroups, ['upload-query-1'], 'successful uploads must release selector handles'); + expectedPath = exactPath; + const fallbackArgs = { selector: 'input[type=file]', downloadId: 0, filePath: exactPath }; + const fallback = await agent.executeTool(42, 'upload_file', fallbackArgs); + assert.equal(fallback.success, true); + assert.equal(fallback.file, exactPath); + assert.equal(fallbackArgs.filePath, exactPath, 'an unresolved downloadId must not replace a supplied absolute path'); + assert.deepEqual(uploaded, [[realPath], [exactPath]]); + + expectedPath = realPath; selectorMatches = ['input-501', 'input-502']; const ambiguous = await agent.executeTool(42, 'upload_file', { selector: 'input[type=file]', @@ -49963,10 +49975,10 @@ test('upload_file prefers downloadId over a supplied stale filePath (chrome)', a assert.equal(ambiguous.success, false); assert.match(ambiguous.error, /matched 2 elements/); assert.match(ambiguous.error, /exact, unique selector/); - assert.deepEqual(uploaded, [[realPath]], 'ambiguous selectors must fail before attaching the file'); + assert.deepEqual(uploaded, [[realPath], [exactPath]], 'ambiguous selectors must fail before attaching the file'); assert.deepEqual( releasedGroups, - ['upload-query-1', 'upload-query-2'], + ['upload-query-1', 'upload-query-2', 'upload-query-3'], 'early upload failures must release selector handles', ); } finally {