feat(web): upload files into the project from the files view - #8151
feat(web): upload files into the project from the files view#8151msegec wants to merge 30 commits into
Conversation
Adds the createUploadUrl command atom and a client-side upload queue for workspace files: FIFO pump capped at 3 concurrent uploads per environment, XHR-based byte upload with progress, an overwrite confirm flow for ProjectUploadTargetExistsError, and retry/cancel/dismiss for failed rows.
Floor the workspace upload body limit at 1 byte so a 0-byte upload token can't disable NodeStream's max-body check for a chunked request with no Content-Length. Route the overwrite confirm dialog through readLocalApi() like every other caller instead of calling requestConfirmDialog directly. Extract the duplicated XHR upload helper (attachments, workspace) into apps/web/src/lib/uploadXhr.ts. Raise the workspace upload timeout to 10 minutes to match the 100 MiB max and the upload token TTL. Scope the files view upload docs to web and desktop.
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two Effect convention issues in apps/server/src/workspace/WorkspaceUpload.ts. Everything else (namespace imports for effect/* and local service modules, dependency acquisition via yield* Foo.Foo, Effect.catch over the fully handled channel, no runtime boundaries in service code) looks consistent with the conventions.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Reviewed the new files-view upload UI for consistency with the shared component system and existing upload surfaces. Four findings, all in apps/web/src/components/files/FileBrowserPanel.tsx: an unbounded upload list that can collapse the file tree, row action buttons that don't follow the established compact-row control contract, a divergent progress format, and a drop overlay duplicated verbatim from ChatView.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds a substantial authenticated workspace-write capability, including a new upload RPC, signed HTTP route, filesystem commit logic, and client-side queue/UI behavior. Its auth-directory changes and broad production blast radius require human review despite the extensive focused test coverage. You can add or adjust custom eligibility rules. Learn more. |
Store the non-overwrite upload with an atomic hard link so a concurrent upload gets a 409 instead of silently replacing the file, and ignore a second retry click while the retried job is already uploading. Share one drop-overlay component between the chat and files views, reuse the attachment progress formatter, cap the uploads strip height, size the row buttons to the compact-row contract, and name the mint target in the resolve error message.
github-actions
Bot
added
size:XXL
There was a problem hiding this comment.
One convention finding on the new ProjectCreateUploadUrlError declaration. The previously flagged Effect.catchTag("WorkspacePathOutsideRootError", ...) in apps/server/src/workspace/WorkspaceUpload.ts (line 181) is still present — Effect.catchTags({ ... }) is the convention even for a single tag.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
One finding on the new shared drop-overlay component; the four items from the previous run (upload strip max-height/scroll, micro row-action contract, shared progress formatter, extracted overlay) all look addressed.
Posted via Macroscope — UI Consistency
The lexical resolve cannot see symlinked directory components, so a signed claim for a path under an in-workspace symlink could write outside the project. Canonicalize the workspace root and the target directory before any bytes land and reject with 400, the same guard AssetAccess applies to signed reads.
…ages Check the deepest existing ancestor against canonical paths before recursive mkdir so a symlinked component cannot create directories outside the workspace, derive ProjectCreateUploadUrlError messages from a stage discriminator like the sibling file errors, and merge consumer classNames into the shared drop overlay instead of letting them replace the treatment.
The repo's Effect conventions check requires catchTags for statically known tagged failures even with a single tag.
…rupt cleanup The part file now uses a fixed-length UUID name beside the target, so a long target basename cannot exceed the 255-byte filename component limit. The canonical containment check now rejects only a real parent traversal, so in-root directories like '..config' upload fine. A part file left by fiber interruption is reclaimed with an ensuring finalizer, since Effect.catch does not run on interrupts.
…resh callback throws The success path cleared the job map and upload state before invoking onUploaded, so a throwing callback fell into the failure handler and recreated the entry as failed with no job left to retry. The callback now runs in its own guard and only logs.
There was a problem hiding this comment.
Reviewed the new Effect-side code (apps/server/src/workspace/WorkspaceUpload.ts, the HTTP route, ws handler, and contracts) against the service conventions. The upload module mirrors AttachmentUpload.ts (namespace subpath imports, dependencies acquired via yield* Foo.Foo, catchTags for known tags, catchIf only for a platform reason._tag), and ProjectCreateUploadUrlError now derives its message from a structural stage with a required cause. One error-modeling issue remains in the newly added rename/delete contracts.
Posted via Macroscope — Effect Service Conventions
The signed token base64url-encodes the workspace cwd, so a long but valid cwd could push the relative url past the 4096 bound and fail result encoding. 8192 clears a PATH_MAX cwd plus the longest relative path after encoding overhead.
d06e4a6 to
3ec000c
Compare
Directory targets are rejected at mint with a target-not-file stage and at store with a 409, so an overwrite can no longer end in a generic 500 while renaming the part file over a folder. The ancestor walk stops at the filesystem root. The replace confirm renders as destructive, the retry button uses the retry icon, and the target-exists check derives from the contracts schema. cause is optional on ProjectCreateUploadUrlError so validation stages construct without one.
Non-overwrite uploads landed via link plus rename, and link fails with EPERM on FAT and exFAT volumes, turning every such upload into a 500. Write the target directly with O_EXCL instead; the exclusive create is just as atomic against a concurrent file at the target and works on filesystems without hard links.
A failed fallback write removed the target before surfacing, which could delete a rival's file created after the exists check. The fallback now claims the name with an empty O_EXCL create and renames the part onto its own claim, so the failed create removes nothing and only a failed rename reclaims the name, by which point the name holds this upload's claim. The staged part is also reclaimed before the entries refresh, so the rebuilt index never lists a phantom part entry.
An interrupt between the fallback's claim and rename stranded a permanent empty file at the target, and a failed rename removed the target unconditionally, which in a double race could delete a rival's confirmed overwrite. The claim and rename now run uninterruptibly, and the reclaim only removes the target while it still holds the empty claim. Upload jobs now trim the file name at creation. The RPC schema trims the path on encode, so an untrimmed name stored the file under a different path than the one the progress row and overwrite dialog reported. Empty names fail up front.
A rival's confirmed overwrite can legitimately be zero bytes, so the failed-rename reclaim could not tell it from the empty claim and deleted it. The reclaim now also compares the inode captured at claim time and falls back to the size check only where the platform reports no inode. The rival simulation in the tests now replaces the inode like a real rename, and a zero-byte rival test pins the survival.
A stat failure between the wx claim and the inode capture surfaced the error while leaving the empty claim at the target, so every later upload to that path read the name as taken. The claim is now force-removed before the error surfaces.
Workspace uploads buffered the whole body in server memory, up to 100 MiB per request with no server-side concurrency cap, so parallel uploads could hold many full bodies at once. The body now streams straight into the .part staging file; the claimed size is enforced while bytes land, and oversized, truncated, and unreadable bodies still reject with 400. Cleanup on every exit is unchanged. The client also discarded the server's rejection detail and showed only a generic status line, and a file over the 100 MiB limit failed with an unhelpful mint error. Failed rows now show the server's plain-text reason when one is present, and oversized files fail immediately with the limit named.
There was a problem hiding this comment.
One finding on error-cause preservation for the new workspace upload body error. Everything else in the changed Effect code (namespace subpath imports, catchTags for known tags, structural catchIf on platform error reasons, environment-acquired WorkspacePaths/FileSystem/WorkspaceEntries dependencies, exported Schema.is predicate) follows the conventions.
Posted via Macroscope — Effect Service Conventions
Macroscope flagged that mapping the request stream error into a field-less WorkspaceUploadBodyError discards the underlying platform failure. The error now requires a cause, the wrap site threads the stream error through, and the 400 branch logs it the same way other degraded paths do.
|
Related: #8235 and #8092 add file attachments to the composer. This PR reuses the same signed-URL upload pattern but targets the project tree from the files view, so the two do not overlap in scope. Both #8092 and this branch touch |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high 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 20cb9c4. Configure here.
Four upload defects from PR review. A non-overwrite conflict answered 409 without consuming the request body, so clients mid-send saw a connection reset instead of the conflict message; the server now drains the body, bounded by the claimed size, before responding. A finished upload only refreshed the tree, leaving a replaced open preview showing stale bytes that a later save could write back; completion now reloads the open file when it was the target. Two queued uploads for the same target could run concurrently, prompting twice and racing the server rename; jobs sharing an environment, cwd, and path now run one at a time. A cancel that raced a completed request cleared the row while the server had committed the file; the entries refresh now still runs so the tree shows the file.
An upload completing between a selection commit and the passive effect flush read the previous selection and skipped reloading the replaced preview. The refs now update in a layout effect, which runs before the browser can deliver the completion event.
1 participant
What changed
The files view can now put files into the project, not just read them.
projects.createUploadUrl), then POSTs the raw bytes to/api/workspace/upload/<token>. The token carries the target path, size, and a 10 minute expiry, so file bytes never travel over the WebSocket..parttemp file and renames it into place, so a dropped connection never leaves a half-written file at the target path.This applies to the web app and the desktop wrapper, over local, relay, and tunnel connections alike. The mobile files view is unchanged.
Why
Getting a file into the workspace currently means the shell or composer attachments, and attachments only feed the agent's turn, they never land in the project. For anyone driving a remote environment from app.t3.codes or the tunnel there is no path at all. Reusing the signed URL pattern the asset routes already use keeps auth in one place and keeps the WebSocket free of file payloads.
UI changes
The panel gains one upload button beside refresh; everything else appears only while an upload is in flight.
Video of the full flow (drop, large upload with progress, overwrite confirm): upload-flow.mp4
Verification
vp test run apps/server/src/workspace/WorkspaceUpload.test.ts apps/server/src/server.test.ts apps/web/src/lib/workspaceUploadQueue.test.ts apps/web/src/lib/attachmentUploadQueue.test.ts).git diff --checkpass.Checklist
Made with Claude Fable 5 using the Claude Code harness.
Note
Medium Risk
New authenticated write path into project workspaces with signed tokens and filesystem edge cases; mitigated by path canonicalization, size limits, and broad server/integration tests.
Overview
Adds workspace file uploads from the Files panel (and reuses a shared drop overlay in chat): drag-and-drop or a picker, progress/cancel/retry rows, tree refresh, and preview reload when an open file is overwritten.
The flow is mint signed URL over WebSocket (
projects.createUploadUrl, operate scope) then POST bytes to/api/workspace/upload/<token>. Contracts define a 100 MiB cap and 10 minute token TTL. The server streams into a.partfile, enforces claimed size andContent-Lengthwhen present, validates tokens (including kind separation from attachment uploads), blocks paths outside the workspace (including post-mint symlink cases), handles overwrite/conflict with body draining for reliable HTTP errors, and commits with rename/link plus FAT/exFAT fallbacks.The web client adds
workspaceUploadQueue(per-environment concurrency, per-path serialization, overwrite confirm on mint failure or 409) and factorsuploadXhrout of attachment uploads for shared rejection handling. User docs cover the Files view upload behavior.Reviewed by Cursor Bugbot for commit 7d81edf. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add file uploads to workspace files panel
projects.createUploadUrlWebSocket RPC to mint signed upload URLs and a new HTTP POST route to stream request bodies directly to disk.projects.createUploadUrlRPC requiresAuthOrchestrationOperateScope; upload size is capped at 100 MiB viaPROJECT_UPLOAD_MAX_BYTES.Macroscope summarized 7d81edf.