feat(chat): support non-image file attachments in the composer#4058
feat(chat): support non-image file attachments in the composer#4058andreidrSB wants to merge 7 commits into
Conversation
Additive union member: existing image rows/drafts decode unchanged. Limits shared with images (10 MB, 8 per message). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- inferFileExtension with a conservative SAFE_FILE_EXTENSIONS allowlist (.bin fallback) so resolve-by-id keeps working and risky extensions (.html/.svg-for-files) never land on the same-origin asset route - Normalizer accepts any MIME for type:"file" uploads; image gate and size limits unchanged - attachment GC keep-set includes file attachments (previously any non-image attachment would be pruned from disk) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
File attachments already live on the server's disk; append an '[Attached file: ...]' line to the turn input at the single sendTurn choke point and pass only image attachments to the provider. The agent reads the file with its own tools, so Claude/Codex/Grok need no adapter changes and remote environments work by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Content-Disposition: attachment for any attachment whose extension is not an image extension, so uploaded content is never rendered inline on the same-origin asset route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- drag/drop and paste accept non-image files (staged as name+size chips); images keep the existing preview tiles and expand dialog - timeline renders file attachments as download chips via the signed asset URL - drafts persist an optional attachment type (missing = image, so existing persisted drafts decode unchanged; no storage version bump) - limits and copy: 10 MB per attachment, 8 attachments per message Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ring Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and downloads Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0f54a4d. Configure here.
| export function cloneComposerImageForRetry( | ||
| image: ComposerImageAttachment, | ||
| ): ComposerImageAttachment { | ||
| export function cloneComposerImageForRetry(image: ComposerAttachment): ComposerAttachment { |
There was a problem hiding this comment.
File blob URLs never revoked
Medium Severity
blob: URLs are now created for all file attachments, but revokeUserMessagePreviewUrls and collectUserMessageBlobPreviewUrls only process images. This mismatch causes unrevoked blob: URLs for non-image attachments to accumulate, leading to memory leaks after messages are sent, cleaned up, or threads are switched.
Reviewed by Cursor Bugbot for commit 0f54a4d. Configure here.
| name: file.name || "image", | ||
| mimeType: file.type, | ||
| name: file.name || (isImage ? "image" : "file"), | ||
| mimeType: file.type || "application/octet-stream", |
There was a problem hiding this comment.
Typeless images staged as files
High Severity
The composer misclassifies actual image files as generic files when the browser provides an empty or generic file.type. This prevents them from rendering as images and being sent to vision models, instead only sending a disk path. Previously, such files were rejected.
Reviewed by Cursor Bugbot for commit 0f54a4d. Configure here.
| ? { | ||
| "Content-Disposition": `attachment; filename="${asset.path.replace(/^.*[/\\]/, "")}"`, | ||
| } | ||
| : {}), |
There was a problem hiding this comment.
Download uses storage filename
Low Severity
Non-image asset responses set Content-Disposition with the on-disk name (<attachmentId>.<ext>). Timeline chips set download to the original attachment.name, but same-origin Content-Disposition typically wins, so downloads save under the server id instead of the user-facing filename.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0f54a4d. Configure here.
ApprovabilityVerdict: Needs human review This PR introduces a significant new capability (non-image file attachments) spanning contracts, server orchestration, and client UI. Additionally, unresolved review comments identify potential bugs including image misclassification (high severity) and memory leaks from unrevoked blob URLs (medium severity). You can customize Macroscope's approvability policy. Learn more. |
1 participant
Implements #4057.
What
The composer currently rejects anything that isn't an image. This PR lets users drag/paste any file (PDF, logs, CSV, …) into the composer and have the agent read it — including on remote environments, where there was previously no way to hand the agent a file at all.
Images are untouched: same native image content blocks per provider, same preview tiles, same expand dialog. Limits unchanged (10 MB per attachment, 8 per message).
How
Non-image files reuse the existing upload path (base64 data URL → server writes to
<stateDir>/attachments/), and are delivered by reference at the singlesendTurnchoke point inProviderCommandReactor:is appended to the turn input, and the attachment list passed to the provider is filtered to images only. The agent reads the file with its own tools — zero changes to the Claude/Codex/Grok adapters, and remote environments work by construction (the file lands on the machine the agent runs on).
Details
ChatFileAttachmentcontract variant — additive union member; existing persisted image rows and localStorage drafts decode unchanged (persisted drafttypeis optional, missing ⇒ image, no storage-version bump).collectThreadAttachmentRelativePathsskipped non-image attachments while the pruning pass deletes anything not in the keep-set — without this, file attachments would be deleted from disk on the next projection pass.SAFE_FILE_EXTENSIONS,.binfallback) soresolveAttachmentPathById's fixed-extension scan keeps working and.html-class extensions never land on the same-origin asset route. Non-image asset requests getContent-Disposition: attachment(download, never inline render).<server-generated-id>.<allowlisted-ext>, so the original filename never touches a path.historyBootstrapsummarizes them for provider switches.Screenshots
Composer with a dropped CSV staged as a chip (images keep their tiles):
Sent message in the timeline with the file chip:
Verified
vp check0 errors (the new reactor test usesit.effect— no new manual-runner debt),vp run typecheckclean repo-wide, all package test suites green: server 1419, web 1313, contracts 183, mobile 505, relay 187.[Attached file: …]line with the resolved path.Known version-skew / follow-ups (called out per #4035 precedent)
type:"file"attachment (usual wire-evolution surface, handled by the version-mismatch banner).🤖 Generated with Claude Code
Note
Medium Risk
Touches attachment ingestion, same-origin asset serving, and the provider turn pipeline (security-sensitive), though mitigated by extension allowlists, download headers, and image-only provider payloads.
Overview
Adds
ChatFileAttachmentalongside images so the composer can stage PDFs, logs, CSVs, and other files—not only pictures—while keeping the same size/count limits.Server: uploads persist with allowlisted extensions (
inferFileExtension,.binfallback);ProviderCommandReactorappends[Attached file: …]paths to turn input and sends only images to providers; projection GC now keeps file attachments referenced in messages. Non-image assets getContent-Disposition: attachmenton/api/assetsto avoid inline rendering of risky types.Web: drag/paste accepts any file type; timeline shows file chips (name + size) vs image tiles; drafts persist
type: "file"with legacy image default whentypeis missing.Contracts: additive union on
ChatAttachment/ upload payloads; tests cover decode and limits.Reviewed by Cursor Bugbot for commit 0f54a4d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Support non-image file attachments in the chat composer and message timeline
ComposerFileAttachmentandComposerAttachmentunion types to the draft store so file and image attachments coexist in the same array, with backward-compatible hydration for legacy drafts.ChatFileAttachmentandUploadChatFileAttachmentcontract schemas, extending theChatAttachmentandUploadChatAttachmentunions.AssetAccessare marked withdownload: true, causing the HTTP asset route to set aContent-Disposition: attachmentheader.Macroscope summarized 0f54a4d.