Repository navigation
Conversation
- Add downloadFile() utility (packages/gui-plugins/src/file/download.ts) that handles text files via system save dialog and binary files via browser download (Blob URL or content URL) - Add DownloadContextMenu component to open-in-app.tsx with Download + Copy path menu items, rendered when the session is non-local (no desktop open-in-app available) - Add screen prop to OpenInAppContextMenuV2 so list and tree views can activate the download menu for remote server files - Wire up screen prop in list.tsx and pierre-tree.tsx
The blob was built from a double type assertion that discarded the byte array's real type. Copying into a fresh array keeps the blob's backing buffer an `ArrayBuffer` and removes the assertion.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
There was a problem hiding this comment.
The title and description describe a one-line Blob cast cleanup, but this branch adds the whole feature: a new download.ts helper and a Download / Copy path context menu in the session file list and Pierre tree when no local app can open files. Please update the description to explain the feature.
How I checked: I couldn't run this in the web app, so it isn't verified. tsc -b for packages/gui-plugins passes, oxlint reports nothing on the changed files, and bun test src/file passes (192 tests). There are no new tests. I followed the file-content path in packages/app/src/workspaces/files/artifact.ts by reading the code.
Public API: unchanged. Only an internal screen prop is added to the built-in OpenInAppContextMenuV2.
Concerns, in order (details inline):
- Downloads fail for many binary files. The app keeps no
bytesfor files it doesn't recognise as images, PDFs, documents and so on (zip, tar, executables), or for media over 256 MiB. For those files Download just shows "Couldn't load this file." These are the files remote users most need to save (#60180). The server already sends file reads as attachments (#62422), so downloading through the server would cover every file. It would also avoid holding the whole file in memory. - The streamed-URL path can leave the app. On the web with a server on another origin, it can navigate the tab away from the app instead of downloading.
- Errors aren't handled. A rejected
downloadFilegoes unhandled, so the user sees no error message. - Blob URL timing. The blob URL is revoked straight after
click(), which can cancel the download in some browsers. - Duplicated code.
DownloadContextMenucopies most ofOpenInAppContextMenuV2's trigger and hover logic.
Related: #60180
| link.href = url | ||
| link.download = name | ||
| link.click() | ||
| URL.revokeObjectURL(url) |
There was a problem hiding this comment.
Revoking the object URL straight after click() can cancel the download in some browsers, notably Safari and Firefox with large blobs. Revoke it after a delay, for example in a setTimeout.
This branch is hit for every binary the app doesn't recognise as an image, PDF, document and so on (zip, tar, executables): fileContentFromBytes keeps only { type: "binary", content: "", size } for them. It's also hit for media over MAX_MEDIA_BYTES. Download then shows "Couldn't load this file." for the files remote users most need to save. The server already sends file reads as attachments (#62422). Downloading through the server would cover every file, and it avoids pulling the whole file into the in-memory cache first (the forced sync above).
| } | ||
|
|
||
| // Binary: system.save takes a string, so stream or blob-download raw bytes instead. | ||
| if (content.url) { |
There was a problem hiding this comment.
content.url is the server's stream URL. When the server is on a different origin from the app (a common remote setup), browsers ignore the download attribute. The click then opens the image, audio or video in the current tab and navigates away from the app instead of saving it. Fetching the file into a blob, or using a URL the server marks as an attachment, avoids that.
| if (!resolved) return | ||
| const workspacePath = props.screen.file.resolve(resolved) | ||
|
|
||
| void downloadFile(ctx, props.screen, workspacePath).then((ok) => { |
There was a problem hiding this comment.
downloadFile can reject: file.sync, ctx.system.save or the blob creation can throw. Only a resolved false shows the toast here, so a rejection is unhandled and the user gets no feedback. Add a .catch that shows the same error toast.
| } | ||
|
|
||
| /** A context menu for files on a remote server, where no local app can open them. */ | ||
| function DownloadContextMenu( |
There was a problem hiding this comment.
This component repeats almost all of OpenInAppContextMenuV2's trigger, hover-highlight and onOpenChange logic. Only the menu items differ. Keep one context menu that switches its items (open-in-app vs. download). Two copies of the trigger and hover handling will drift apart.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Issue for this PR
Closes #63485
Type of change
What does this PR do?
When connected to a remote OpenCode server, the "Open in app" context menu items don't work because there's no local app to open remote files. This PR adds a Download option instead.
download.ts(new):downloadFile()loads file content through the session screen'sFilesAPI and saves it locally — text files viactx.system.save, binary files via a Blob download.open-in-app.tsx: NewDownloadContextMenucomponent shown when no local app can open files.OpenInAppContextMenuV2accepts an optionalscreenprop; when nostateis available andscreenis given, it renders the download menu.list.tsxandpierre-tree.tsx: PassscreentoOpenInAppContextMenuV2so the download menu is available in both file list views.How did you verify your code works?
bun turbo typecheck— 35/35 packages passbunx biome check— zero new warnings over dev baselineChecklist