From f0fba3e58e2b6ead4980185245bc57a405b4bec5 Mon Sep 17 00:00:00 2001 From: Shoaib Ansari Date: Sat, 2 May 2026 12:57:17 +0530 Subject: [PATCH] fix: support both absolute and relative paths in browser_upload_file Previously the browser_upload_file tool had a schema/implementation mismatch: the schema claimed to accept absolute paths but the implementation always prefixed with uploadBasePath, causing absolute paths like /tmp/file.png to become invalid (./uploads//tmp/file.png). - Extract resolveUploadPath() helper that detects absolute paths (Unix / or Windows C:\) and uses them as-is - Update Zod schema description to document both absolute/relative path support - Remove misleading inline comment - Add unit tests for all path resolution cases Closes #45 --- src/__tests__/upload-file.test.ts | 35 +++++++++++++++++++++++++++++++ src/tools.ts | 16 +++++++++++--- 2 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/upload-file.test.ts diff --git a/src/__tests__/upload-file.test.ts b/src/__tests__/upload-file.test.ts new file mode 100644 index 0000000..9382bb5 --- /dev/null +++ b/src/__tests__/upload-file.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { resolveUploadPath } from "../tools"; + +describe("resolveUploadPath", () => { + it("returns absolute Unix paths as-is", () => { + expect(resolveUploadPath("/tmp/file.png", "./uploads")).toBe("/tmp/file.png"); + expect(resolveUploadPath("/var/log/test.pdf", "./uploads")).toBe("/var/log/test.pdf"); + }); + + it("returns Windows absolute paths as-is", () => { + expect(resolveUploadPath("C:\\Users\\test\\file.png", "./uploads")).toBe("C:\\Users\\test\\file.png"); + expect(resolveUploadPath("D:\\data\\document.pdf", "./uploads")).toBe("D:\\data\\document.pdf"); + }); + + it("prefixes relative paths with uploadBasePath", () => { + expect(resolveUploadPath("file.png", "./uploads")).toBe("./uploads/file.png"); + expect(resolveUploadPath("document.pdf", "./uploads")).toBe("./uploads/document.pdf"); + }); + + it("uses custom uploadBasePath for relative paths", () => { + expect(resolveUploadPath("file.png", "/custom/uploads")).toBe("/custom/uploads/file.png"); + expect(resolveUploadPath("test.txt", "uploads")).toBe("uploads/test.txt"); + }); + + it("handles mixed arrays correctly", () => { + const paths = ["/tmp/a.png", "b.pdf", "C:\\data\\c.jpg"]; + const resolved = paths.map((p) => resolveUploadPath(p, "./uploads")); + expect(resolved).toEqual(["/tmp/a.png", "./uploads/b.pdf", "C:\\data\\c.jpg"]); + }); + + it("handles relative paths with subdirectories", () => { + expect(resolveUploadPath("folder/file.png", "./uploads")).toBe("./uploads/folder/file.png"); + expect(resolveUploadPath("uploads/test/document.pdf", "./uploads")).toBe("./uploads/uploads/test/document.pdf"); + }); +}); diff --git a/src/tools.ts b/src/tools.ts index 19d0bc6..3dbcddf 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -16,6 +16,12 @@ import { } from "@playwright/test"; import type { TabManager } from "./utils/tab-manager"; +export function resolveUploadPath(filePath: string, uploadBasePath: string): string { + return filePath.startsWith("/") || filePath.match(/^[A-Za-z]:/) + ? filePath + : `${uploadBasePath}/${filePath}`; +} + type ToolSettings = { abortController?: AbortController; currentStep?: { description: string; data?: Record }; @@ -559,7 +565,11 @@ class PlaywrightTools { public uploadFileSchema = z.object({ ref: z.string().describe('The ref of the "button" that triggers a FileChooser to upload files'), elementDescription: z.string().describe("A description of the element, used for debugging"), - filePaths: z.array(z.string()).describe("Array of absolute file paths to upload"), + filePaths: z + .array(z.string()) + .describe( + "Array of file paths to upload. Can be absolute paths (e.g., '/tmp/file.png') or relative paths (e.g., 'document.pdf' which will be resolved against uploadBasePath)", + ), reasoning: z.string().describe("A quick one-line reasoning behind this action"), doesActionAdvanceUsTowardsGoal: z .boolean() @@ -570,13 +580,13 @@ class PlaywrightTools { public async uploadFile({ ref, elementDescription, - filePaths, // This is not a full path. It accepts a string filename which should be available in `uploads` directory + filePaths, }: z.infer) { const locator = this.page.locator(`aria-ref=${ref}`).describe(elementDescription); // We expect to find these files in the `./uploads` directory if no base path is configured const uploadBasePath = getConfig().uploadBasePath || "./uploads"; - const prefixedFilePaths = filePaths.map((filePath) => `${uploadBasePath}/${filePath}`); + const prefixedFilePaths = filePaths.map((filePath) => resolveUploadPath(filePath, uploadBasePath)); // File uploads are not cached for now as it needs a two step process // We can solve this later by introducing multi-action caching if needed