feat(web): rename and delete files from the files view - #8162
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.
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.
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.
|
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.
One finding: the new permanent file-delete confirmation does not use the destructive confirm variant, so its confirm button renders with the default primary treatment instead of the destructive one every other permanent-delete prompt in the app uses. Details inline.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| yield* fileSystem |
There was a problem hiding this comment.
🔴 Critical workspace/WorkspaceFileSystem.ts:557
deleteEntry can remove a file outside the workspace root when another process swaps the checked parent directory for a symlink after directoryEscapesWorkspaceRoot returns. The subsequent path-based stat and remove are not protected by that check; renameEntry has the same link/remove race. Use directory-descriptor-relative, symlink-safe operations (or otherwise hold the parent validation and mutation atomically) so concurrent filesystem changes cannot bypass the root boundary.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/workspace/WorkspaceFileSystem.ts around line 557:
`deleteEntry` can remove a file outside the workspace root when another process swaps the checked parent directory for a symlink after `directoryEscapesWorkspaceRoot` returns. The subsequent path-based `stat` and `remove` are not protected by that check; `renameEntry` has the same `link`/`remove` race. Use directory-descriptor-relative, symlink-safe operations (or otherwise hold the parent validation and mutation atomically) so concurrent filesystem changes cannot bypass the root boundary.
Evidence trail:
Commit dc7004a6513329593871c73a7e9157722314cf97. Inspect `apps/server/src/workspace/WorkspaceFileSystem.ts:329-346` (canonical parent check), `:379-497` (rename validation and path-based link/remove), and `:500-560` (delete validation and path-based stat/remove). Inspect `apps/server/src/workspace/WorkspacePaths.ts:202-230` (lexical path resolution) and `apps/server/src/ws.ts:1946-1953` (RPC callers). Git command: `git show dc7004a6513329593871c73a7e9157722314cf97 -- apps/server/src/workspace/WorkspaceFileSystem.ts apps/server/src/workspace/WorkspacePaths.ts apps/server/src/ws.ts`.
There was a problem hiding this comment.
Not changing this. The endpoint requires operate scope, and operate scope already drives coding agents with full shell access in the same cwd, so anyone who can win this race can just run rm directly. The shipped writeFile path has the same check-then-act shape for the same reason. Hardening this call adds no capability boundary.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This XXL production feature adds signed uploads plus destructive rename/delete operations and changes shared save coordination across the web and server. Path-based containment and rollback race windows still plausibly permit outside-workspace effects or data loss, requiring human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
…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.
dc7004a to
f741fda
Compare
There was a problem hiding this comment.
Reviewed the new Effect service code (WorkspaceFileSystem.renameEntry/deleteEntry, WorkspaceUpload, the new contracts errors, and the RPC/web call sites) against the Effect service conventions. Service shape, namespace imports, Foo["Service"] typing, Effect.catchTags/structural catchIf usage, and layer wiring all look correct. Two convention issues on the error modelling side are noted inline.
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.
There was a problem hiding this comment.
UI consistency review of the new files-view upload/rename/delete surfaces. The extracted WorkspaceFileDropOverlay, the Button usages (icon-xs ghost header actions, icon-micro ghost-muted row actions), and RenameEntryDialog's Dialog composition all match existing repo patterns. Two smaller consistency points below.
Posted via Macroscope — UI Consistency
f741fda to
854003f
Compare
| cwd, | ||
| relativePath, | ||
| onPendingChange, | ||
| discardSavesRef, |
There was a problem hiding this comment.
🟠 High files/FilePreviewPanel.tsx:477
Deleting or renaming the active file can still let an in-flight persist complete against the old path, recreating the deleted or pre-rename file. FileSaveCoordinator.discard() clears the timer and revision but does not cancel or invalidate the write already started by persist, which can still invoke onConfirmed; make in-flight saves abortable or ignore their completion after discard.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/files/FilePreviewPanel.tsx around line 477:
Deleting or renaming the active file can still let an in-flight `persist` complete against the old path, recreating the deleted or pre-rename file. `FileSaveCoordinator.discard()` clears the timer and revision but does not cancel or invalidate the write already started by `persist`, which can still invoke `onConfirmed`; make in-flight saves abortable or ignore their completion after discard.
Evidence trail:
f741fda: apps/web/src/components/files/fileSaveCoordinator.ts:34-40,56-81; apps/web/src/components/files/FilePreviewPanel.tsx:419-429,1102-1115; apps/web/src/components/files/FileBrowserPanel.tsx:323-331; apps/web/src/components/files/RenameEntryDialog.tsx:80-100; packages/client-runtime/src/state/projectCommands.ts:45-46,95-126; packages/client-runtime/src/state/runtime.ts:188-204. Git command: git show f741fda -- apps/web/src/components/files/fileSaveCoordinator.ts apps/web/src/components/files/FilePreviewPanel.tsx packages/client-runtime/src/state/projectCommands.ts packages/client-runtime/src/state/runtime.ts
There was a problem hiding this comment.
Leaving this. The persist is already in flight server-side while the confirm is up and the client cannot cancel it. The window is sub-second against a human dialog, and deleting again recovers.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
| /** Drop unsaved edits without persisting; for files removed out from under the surface. */ | ||
| discard(): void { | ||
| this.disposed = true; | ||
| this.clearTimer(); |
There was a problem hiding this comment.
🟡 Medium files/fileSaveCoordinator.ts:37
discard() leaves the edited contents in optimisticFileAtom, so reopening a deleted or renamed path can display discarded data and a later edit can overwrite a recreated file with that stale content. Clear the optimistic query state when discarding, alongside resetting the coordinator revision.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/files/fileSaveCoordinator.ts around line 37:
`discard()` leaves the edited contents in `optimisticFileAtom`, so reopening a deleted or renamed path can display discarded data and a later edit can overwrite a recreated file with that stale content. Clear the optimistic query state when discarding, alongside resetting the coordinator revision.
Evidence trail:
f741fda apps/web/src/components/files/fileSaveCoordinator.ts:34-40
f741fda apps/web/src/components/files/projectFilesQueryState.ts:47-69, 100-115, 175-198
f741fda apps/web/src/components/files/FilePreviewPanel.tsx:485-486, 752-757, 1102-1115
f741fda packages/client-runtime/src/state/projectCommands.ts:47-50, 75-76
git show f741fda -- apps/web/src/components/files/fileSaveCoordinator.ts apps/web/src/components/files/projectFilesQueryState.ts apps/web/src/components/files/FilePreviewPanel.tsx
There was a problem hiding this comment.
Fixed in 2f882fd. Delete and rename now clear the file query cache, so reopening the path refetches instead of showing stale bytes.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
| onClose={() => setRenameTarget(null)} | ||
| onRenamed={(newRelativePath) => { | ||
| entriesQuery.refresh(); | ||
| onEntryRenamed?.(renameTarget, newRelativePath); |
There was a problem hiding this comment.
🟡 Medium files/FileBrowserPanel.tsx:643
Renaming a background file tab activates it, switching the preview away from the currently active file. onEntryRenamed is called for every successful rename, and the downstream openFile call always sets activeSurfaceId to the renamed path; preserve the active surface unless the renamed file was already active.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/files/FileBrowserPanel.tsx around line 643:
Renaming a background file tab activates it, switching the preview away from the currently active file. `onEntryRenamed` is called for every successful rename, and the downstream `openFile` call always sets `activeSurfaceId` to the renamed path; preserve the active surface unless the renamed file was already active.
Evidence trail:
Commit f741fda1: apps/web/src/components/files/FilePreviewPanel.tsx:1106-1115; apps/web/src/components/files/FileBrowserPanel.tsx:641-644; apps/web/src/rightPanelStore.ts:393-417 and 507-522.
There was a problem hiding this comment.
Fixed in 2f882fd. Renaming a background tab reopens the surface without stealing focus; the previously active surface is restored.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
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.
Three save-coordination gaps around overwrite uploads. The overwrite hold now starts when the conflict is discovered, before the confirm dialog opens, so a debounced save cannot land while the dialog is up. The coordinator counts overlapping holds instead of a boolean, so a rename finishing early cannot release an upload's hold. And resume compares against a persisted-revision watermark, so a snapshot that already saved is not written again over what the upload put on disk. Settle callbacks pair with the hold: only jobs that entered the overwrite phase fire onSettled.
04ab2f9 to
c0ff793
Compare
Creating the rival after deleting the claim lets ext4 recycle the claim inode and model a state that the real staged rename cannot produce.
Keep the original lstat cause inside Effect tryPromise’s tagged unknown error so ENOENT remains recoverable without an untagged error channel.
# Conflicts: # apps/web/src/lib/workspaceUploadQueue.ts
Renaming a symlink silently duplicated it on macOS: the source check used stat, so a symlink-to-file passed as a plain file, and the hard-link claim followed the symlink and linked its referent under the new name. The same-inode guard then saw different inodes and returned success with the symlink still on disk. The source check now uses lstat, matching deleteEntry, and a symlink source skips the hard-link claim for the O_EXCL claim plus rename path, which never dereferences the source. A test layer that resolves the source before linking reproduces the macOS link(2) semantics on Linux and pins the fix. Also disables the rename dialog's submit for . and .., which are path components the server rejects with a generic error, and notes in the files view doc that renaming and deleting are not available on mobile.
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 WorkspaceUploadBodyError. Everything else in the new Effect surface (the WorkspaceFileSystem service additions, WorkspaceUpload module, contracts errors and predicates, catchTags/structural catchIf usage, subpath namespace imports) matches 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.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 492b3d3. 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.
Two renames of the same file could run concurrently on the server, both hard-link their targets before either removed the source, and both report success, leaving the file under both names. Rename and delete now share one mutation lock, so the loser re-checks a source the winner already moved and fails. Deleting a file re-resolved its path after the canonical parent check, so a parent directory swapped for a symlink in that window redirected the removal outside the workspace. The delete now re-resolves the parent immediately before the removal and refuses when it no longer names the checked directory, shrinking the window to the gap before the unlink syscall itself. Renaming an open file left drafted review comments pointing at the old path, so a submitted draft referenced a file that no longer exists. File review comments now follow the rename.
# Conflicts: # apps/web/src/components/files/FileBrowserPanel.tsx # apps/web/src/lib/workspaceUploadQueue.ts
| // An overwrite upload replaced the bytes on disk; drop the | ||
| // stale optimistic overlay and pending edits so a later save | ||
| // cannot write the pre-upload snapshot over the upload. | ||
| clearProjectFileQueryData(environmentId, cwd, path); |
There was a problem hiding this comment.
🟡 Medium files/FilePreviewPanel.tsx:1140
Uploading over a background file leaves its cached readFile result unchanged, so reopening it within the 30-second stale window displays the pre-upload contents. onEntryUploaded only calls file.refresh() when editorShowsFile(path) is true; refresh the query for every uploaded path, not just the currently displayed file.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/files/FilePreviewPanel.tsx around line 1140:
Uploading over a background file leaves its cached `readFile` result unchanged, so reopening it within the 30-second stale window displays the pre-upload contents. `onEntryUploaded` only calls `file.refresh()` when `editorShowsFile(path)` is true; refresh the query for every uploaded path, not just the currently displayed file.
The FAT fallback rename may only replace its own exclusive claim, but a concurrent write to the same target could land between the claim and the rename and be lost. Writes now share the mutation lock with rename and delete, so a mutation never overlaps a write to the same workspace.
| Effect.catchTags({ | ||
| PlatformError: (cause) => | ||
| Effect.gen(function* () { | ||
| yield* fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore); |
There was a problem hiding this comment.
🟠 High workspace/WorkspaceFileSystem.ts:599
A failed source removal can delete another writer's replacement at target.absolutePath, causing data loss. The rollback unconditionally removes the target after the hard-link claim, without verifying that the target still refers to this rename's linked inode; check isSameFile before removing it.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/workspace/WorkspaceFileSystem.ts around line 599:
A failed source removal can delete another writer's replacement at `target.absolutePath`, causing data loss. The rollback unconditionally removes the target after the hard-link claim, without verifying that the target still refers to this rename's linked inode; check `isSameFile` before removing it.
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together. File rename and delete are destructive workspace mutations that need clear collision, trash, remote, and concurrent-refresh behavior. We have not selected that contract for the Files view. If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed. |
2 participants
What changed
Files in the files view can now be renamed and deleted from their context menu.
projects.renameEntry,projects.deleteEntry), gated by the same operate scope asprojects.writeFile.This applies to the web app and the desktop wrapper, over local, relay, and tunnel connections alike. The mobile files view is unchanged.
Note: this branch is stacked on #8151, so the diff includes those upload commits until it merges. The rename and delete work is the commits on top of that upload branch.
Why
The files tab can read files and, with #8151, put them there, but renaming or deleting one still means the shell or the composer. On a remote environment driven from app.t3.codes or the tunnel that detour is the whole task. If the files tab is already open, the context menu is where these belong.
UI changes
Video of the full flow (context menu, rename, conflict, delete): rename-delete-flow.mp4
Verification
vp test run apps/server/src/workspace/WorkspaceFileSystem.test.ts), 11 of them new for rename and delete, covering conflicts, races, symlink escapes, and the self-rename no-op.git diff --checkpass.Checklist
Made with Claude Fable 5 using the Claude Code harness.
Note
Medium Risk
Changes direct workspace filesystem mutations and a new signed upload HTTP path; mitigated by path canonicalization, size/token checks, and broad server and client tests, but bugs could still corrupt or expose files outside the project root.
Overview
Adds rename, delete, and signed HTTP upload for project workspace files, wired through new WebSocket RPCs (
projects.createUploadUrl,projects.renameEntry,projects.deleteEntry) with operate-scope auth and integration in the files browser.On the server,
WorkspaceFileSystemgains same-directory rename (no overwrite; hard-link claim with FAT/exFAT fallbacks) and file delete, serialized with writes via a mutation lock and canonical parent checks to block symlink escape.WorkspaceUploadmints expiring tokens, validatesContent-Length, streams bodies into.partstaging, then atomically commits (link or rename) with conflict draining and root checks;POST /api/workspace/upload/*exposes the upload endpoint.In the web app, the file tree context menu adds rename/delete (plus
RenameEntryDialog), drag/drop and picker uploads with a progress queue, and a sharedWorkspaceFileDropOverlay.FileSaveCoordinatornow supports suspend/resume/discard/reset so pending editor saves do not race rename, delete, or overwrite uploads;FilePreviewPanelcloses or retargets open tabs, clears stale cache, refreshes image assets, andrenameReviewCommentPathmoves file review comments on rename. Attachment uploads share a extracteduploadXhrhelper.Reviewed by Cursor Bugbot for commit 5556344. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add file upload, rename, and delete to the files view (up to 100 MiB)
FileBrowserPanel.renameEntryanddeleteEntrytoWorkspaceFileSystemunder a serialized mutation lock, plus a streaming upload endpoint validated by signed tokens.FileSaveCoordinatorgainssuspend,resume,discard, andresetto hold or drop unsaved edits around mutations;FilePreviewPanelwires these callbacks to prevent save races.writeFileconcurrency key in projectCommands.ts changed from per-path to per-project-root (environmentId+cwd) to serialize saves, renames, and deletes.Macroscope summarized 5556344.