Repository navigation
Expose browser filesystem through MCP - #144
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…er-files # Conflicts: # README.md
Require non-empty session IDs and paths, document which actions use path, inject the Kernel client factory, and test the tool through a real MCP client and server.
dcruzeneil2
left a comment
There was a problem hiding this comment.
LGTM overall — thin, correctly-typed adapter over browsers.fs, consistent with the other manage_* tools, bugbot findings addressed properly, tests exercise the real MCP contract. no new privilege: exec_command --as_root and browser_repl already give full VM filesystem access, this just makes it structured. approving with one should-fix and some nits.
should-fix before merge
- bound
read/download/download_dir_zip.responseBuffer()buffers the whole file, base64s it, and returns it in one JSON-RPC message. nothing upstream caps this either — kernel/kernel'sReadFilebuffers the full body and the in-VM API streams whatever is on disk.download_dir_zipon/home/kernelis a one-liner for a model and will OOM or blow the response limit (the same budgetshell.tscapstimeout_secfor). suggest amax_bytesparam (default ~10–25MB); forread/downloadcallfileInfofirst and refuse over the cap with a hint to narrow vialist/get_info/exec_command; for the zip, stop at the cap and error.
nits
- paths are described as absolute but not enforced. only
upload/upload_zipcheckIsAbson the VM side;read/write/move/delete_*resolve relative paths against the API process cwd.encodedPath()already assumes absolute (prepends/), so a relativereadreturns a resource URI claiming a path that isn't where the file is. a.regex(/^\//)onpath/src_path/dest_pathmakes the schema match its own description. - SDK defaults (60s timeout,
maxRetries: 2) apply to every action. a largewrite/uploadthat times out is re-sent up to three times, and amovethat completed but lost its response gets retried and reports 404.maxRetries: 0for the mutating actions, orlongOperationOptionslikeshell.ts. session_idsays "Browser session ID." — SDK param isidOrName, every other session tool says "ID or name". also addmanage_browser_filesto the two lists of name-accepting tools (README ~L325 andbrowsers.tsnamedescription).readon binary returns mojibake silently; if the decoded text contains U+FFFD, append "appears binary; use download".- the
content-typefallback indownloadis dead — the VM API always returnsapplication/octet-stream— somime_typeis the only way to get a real type; say so in its description. - no tests for
upload_zipor themime_typeoverride.
Refuse read, download, and download_dir_zip results over max_bytes (default 10 MiB, maximum 25 MiB): read and download check file size first, and directory archives stop streaming at the cap. Require absolute paths, disable retries for mutating actions, accept session names, flag binary text reads, and document that mime_type is the only source of a download's type.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9093120. Configure here.
|
thanks @dcruzeneil2. everything from your review is addressed, mostly in 9093120, with a BugBot follow-up in 0e07de8. main is merged in at 4c06710, which also lowercases this tool's copy and adds its entry to the new annotations table so main's tests pass. should-fix
nits
CI and BugBot are green on 0e07de8. The PR body's test count is updated to 908. |

summary
manage_browser_filesMCP tool backed by the existing browser filesystem SDKread,download, anddownload_dir_zipatmax_bytes(default 10 MiB, maximum 25 MiB), refusing oversized files and archives instead of truncating themtesting
bun test(908 tests passed, after merging main)bunx tsc --noEmitbun run buildcompiled and typechecked successfully, then stopped during page-data collection becauseKERNEL_CLI_PROD_CLIENT_IDis not set in the local environmentNote
Medium Risk
Introduces destructive remote filesystem operations (write/delete/move/permissions) on browser VMs; mitigated by auth, size limits, and no automatic retries on mutations.
Overview
Adds a new
manage_browser_filesMCP tool (toolsetbrowser_files, aliasesbrowser_fs/manage_browser_files) so agents can work with files inside a live browser VM via the existingbrowsers.fsSDK.The tool is action-based (list, read, download, write, upload, zip upload/extract, directory zip download, mkdir, move, delete, permissions). Reads and downloads enforce
max_bytes(default 10 MiB, max 25 MiB) with pre-checks and streaming caps—oversized files/archives are rejected, not truncated. Binary payloads come back as embedded MCP resources; mutations usemaxRetries: 0. Paths must be absolute; content is utf8/base64 only (no host filesystem access).Wiring updates: register the toolset, include it in project-scoped tools and visibility tests, document it in README, and note that live session names work as
session_idfor this tool alongside other browser tools.Reviewed by Cursor Bugbot for commit 0e07de8. Bugbot is set up for automated code reviews on this repo. Configure here.