Repository navigation
feat(transfers): progress and cancel for SSH uploads and all remote downloads - #23849
Chihen-Tai wants to merge 27 commits into
Conversation
Runtime-environment imports read the entire file into memory and base64-encoded it before sending, so peak main-process memory was ~2.3x the file size. A 25 MB per-file / 100 MB per-drop ceiling existed only to keep that from exhausting the heap, and it was checked right after lstat — so dropping a 40 MB recording failed instantly with no bytes moved. The socket already lives in main, but the chunking loop lived in the renderer, so the whole base64 payload crossed IPC only to be sliced and passed back through main. Move the byte pump into main, next to the file handle and the socket: one open handle per file, 384 KiB slices (512 KiB on the wire, matching the previous chunk size), sent through the existing files.writeBase64Chunk method. Staging now records byteLength instead of file contents. The renderer keeps deconflict, commit, rollback and cleanup, and never sees file bytes. Peak memory per upload drops to one slice. The ceilings become user safety limits rather than memory guards, raised to 2 GB / 8 GB until the drop UI can show progress and cancel a running upload. No protocol change: files.writeBase64Chunk already existed and was already used by this path for payloads over 512 KB. Small files now use it with append: false, which is exactly what files.writeBase64 does internally, so mixed client/host versions are unaffected. Symlink rejection, O_NOFOLLOW, realpath containment and the stat identity check all move to upload time and now share one handle for the whole file, so every slice comes from the same inode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dropping a large file onto a remote terminal gave no feedback until the transfer finished or failed, so a slow upload looked like a hung one. That became easy to hit once streaming removed the 25 MB per-file import cap. The chunk loop now reports bytes acknowledged by the runtime, coalesced to at most one update per megabyte or 100ms so a multi-gigabyte file does not flood IPC. The renderer sums staged byte counts up front and tracks committed files plus the file in flight, so the drop toast shows one figure for the whole drop rather than per file. Client-local only: main to renderer, no RPC params or stream frames change, so there is no wire-compatibility surface. Cancellation is deliberately excluded; it needs an abort path threaded into the streamer plus temp-file cleanup, and belongs with UI that can trigger it. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the single-line progress toast with a panel: a header carrying the aggregate percent and a collapse toggle, then one row per dropped source showing name, transferred-of-total, a bar, percent, and a cancel control. Cancel is real rather than decorative. Each dropped source streams under one id that is both its progress key and its abort handle, so cancelling a row stops that source and leaves the rest of the drop running. The streamer checks between slices, which leaves a well-formed partial temp file for the existing cleanup to remove; nothing lands at the final path. A cancel that arrives between two files of a source is remembered so the next file aborts instead of the click doing nothing, and cancelled sources are excluded from the failure summary because a cancel is not a failure. Both figures in a row scale to the total's unit: formatting them independently rendered "900 / 32.5" when a file crossed a unit boundary. Per STYLEGUIDE, cancel is a back-out rather than a destructive action, so it stays a quiet ghost control with no colour, and the bar uses the existing Progress primitive's `primary` fill rather than a new hue. Splits the drop flow into terminal-runtime-drop-upload and terminal-drop-paste; the host file had grown past the max-lines limit. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cancelling every file removed the session immediately, so the panel vanished mid-gesture with its header still reading "Uploading 2 files" — the one moment the user most needs to see what happened. Finishing now settles the session instead of deleting it. The header switches to the outcome (cancelled, failed, uploaded X of N), the percent and collapse toggle drop away since nothing is moving, and the panel holds for 1.2s before fading out and dismissing itself. Reduced-motion skips the fade but keeps the hold, so the outcome is still readable. Dismissal moves from the drop flow to the panel, which owns the timing; only the error path still tears it down at once. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collapsing the file list left the header hanging where the expanded panel's top edge was, with dead space below it, instead of dropping to where a single-line toast sits. Sonner caches each toast's height and recomputes it only when the toast's element identity changes — `toast.jsx` is in that effect's dependency list. Our panel re-renders internally from its own store, so the cached height stayed at the expanded size and the shrunken content was top-aligned inside it. Collapse state moves into the session store and toggling re-issues toast.custom under the same id, which changes the element identity and makes sonner re-measure. Keeping the flag in the store is what makes the remount safe: local component state would reset on every toggle. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collapsing the file list added a second panel rendering the same session
instead of updating the first, so two identical cards ticked along together.
sonner's toast.custom mints an id, then spreads the caller's options over it:
const id = data?.id || toastsCounter++
this.create({ jsx: jsx(id), id, ...data })
return id
Passing `id: undefined` therefore reaches create() as an explicit undefined,
which mints a second id internally while custom() returns the first. The
stored id never matched the registered toast, so re-issuing under it found no
existing toast and added another.
The id key is now omitted entirely on the first publish and only supplied on
re-issue. A test pins that the first call carries no id key at all, since
passing undefined reads as harmless and is not.
Fixes stablyai#16077
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t jump An unstyled sonner toast is content-sized, so the expanded panel was as wide as its longest file name and the collapsed header shrank to fit "Uploading 2 files". Collapsing therefore moved the left edge as well as the bottom, which read as a jitter rather than a fold. The panel now takes the Toaster's own --width, so both states occupy the same box and only the height changes. File names are clipped inside that box instead of widening it. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collapsing swapped ChevronUp for ChevronDown, which replaces the DOM node, so there was nothing to tween and the arrow flipped in a single frame. One ChevronDown now rotates 180deg over 200ms. The rotation survives the toast re-issue because sonner keys the list item by toast id and only swaps its jsx, so React reconciles the same element rather than remounting it. motion-reduce drops the transition. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Uploading 2 files" reads as a local copy. This panel only ever appears for a drop onto a remote workspace, so it now says "to runtime", matching the wording the old single-line toast used. The cancelled and failed lines stay short: once nothing landed there, the destination is no longer the point. Fixes stablyai#16077 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-ups on the import ceilings. Staging and upload are separate IPC calls, so a source could be replaced or grown in between and the streamer would take the new size as authoritative — transferring a file the ceilings had already rejected, and invalidating the per-drop aggregate. The staged byteLength now rides the upload request and a mismatch is refused before any chunk is sent. formatByteCeiling rounded, so 2 GiB + 1 B rendered "2.0 GB" against a "2 GB" limit and the error read like a broken check. It now rounds up, so anything over the ceiling renders strictly greater than it. Symlink tests are skipped on Windows, where symlink() needs elevation or Developer Mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ty drop
Review follow-ups on the drop panel.
Electron does not order webContents.send against an invoke reply, so a final
progress event can arrive after completeFile and be counted a second time,
jumping the bar ahead. The tracker now only accepts progress inside an
explicit per-file window that completeFile closes.
onStart still fires when every source was skipped, so a drop that staged
nothing flashed an empty panel; it now returns before creating one.
releaseRuntimeUpload was a bare `void` on a promise, which surfaces as an
unhandled rejection if the IPC fails during cleanup — it now carries the same
`.catch(() => {})` the sibling cleanup calls use.
Comments trimmed to the one-line rule in AGENTS.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-progress Picks up the review follow-ups on the base branch: staged-size enforcement, the rounding fix for over-limit errors, and the Windows symlink test guards.
The ceiling lived only in staging, and the upload IPC takes the staged size as optional — so the limit held only for callers that had actually staged. The streamer now checks it itself, independent of whether a staged size was supplied. Both ceilings and their formatter move to runtime-import-limits so staging and the streamer share one definition rather than drifting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-progress Picks up the per-file ceiling check at the streaming boundary and the shared runtime-import-limits module.
…ings truncate() sets a logical length but does not guarantee a sparse file, so on NTFS the over-limit fixtures could allocate two to three gigabytes on Windows CI to prove a comparison. The ceiling tests now mock runtime-import-limits down to kilobytes and use small real files, and the formatter's one-byte-over case moves to a pure unit test on formatByteCeiling where no file is needed at all. Largest fixture in these suites drops from 3 GiB to 3 MiB. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…op-upload-progress
…rows Headings now pass count so i18next picks the plural form, with named placeholders. Drop-handler success mocks settle their row before finishing, and the runtime file client harness stubs cancel/release so progress-enabled imports resolve.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough
Pre-merge checks |
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 8309eb7b-1277-4fc2-9052-9643195067ec
📒 Files selected for processing (70)
src/main/ipc/filesystem-download-folder.tssrc/main/ipc/filesystem-download-progress.test.tssrc/main/ipc/filesystem-download-transfers.test.tssrc/main/ipc/filesystem-import-ssh-progress.test.tssrc/main/ipc/filesystem-import-ssh-progress.tssrc/main/ipc/filesystem-import-ssh.tssrc/main/ipc/filesystem-mutations.tssrc/main/ipc/filesystem/filesystem-download-handlers.tssrc/main/ipc/filesystem/remote-download-sessions.test.tssrc/main/ipc/filesystem/remote-download-sessions.tssrc/main/ipc/runtime-upload-cancellation.test.tssrc/main/ipc/runtime-upload-cancellation.tssrc/main/ipc/runtime-upload-file-stream.test.tssrc/main/ipc/runtime-upload-file-stream.tssrc/main/ipc/runtime-upload-progress.test.tssrc/main/ipc/runtime-upload-progress.tssrc/main/providers/filesystem-provider-contract.tssrc/main/providers/ssh-filesystem-download-progress.test.tssrc/main/providers/ssh-filesystem-download.tssrc/main/providers/ssh-filesystem-file-upload.tssrc/main/providers/ssh-filesystem-provider-download-folder.test.tssrc/main/providers/ssh-filesystem-provider-sftp.tssrc/main/providers/ssh-filesystem-provider.test.tssrc/main/providers/ssh-filesystem-provider.tssrc/main/ssh/sftp-upload.tssrc/main/ssh/ssh-connection.tssrc/main/ssh/ssh-relay-session.tssrc/main/ssh/ssh-system-fallback.test.tssrc/main/ssh/system-ssh-file-binary-transfer.tssrc/preload/api/filesystem-api.tssrc/preload/api/fs-bridge.tssrc/renderer/src/components/right-sidebar/FileExplorerRow.actions.test.tsxsrc/renderer/src/components/right-sidebar/file-explorer-row-file-transfer.tssrc/renderer/src/components/right-sidebar/useFileExplorerImport.tssrc/renderer/src/components/terminal-pane/terminal-drop-handler.test.tssrc/renderer/src/components/terminal-pane/terminal-drop-paste.tssrc/renderer/src/components/terminal-pane/terminal-native-file-drop.tssrc/renderer/src/components/terminal-pane/terminal-remote-file-download-open.tssrc/renderer/src/components/terminal-pane/terminal-runtime-drop-upload.tssrc/renderer/src/components/transfer-progress/TransferProgressPanel.tsxsrc/renderer/src/components/transfer-progress/download-progress-panel.test.tssrc/renderer/src/components/transfer-progress/download-progress-panel.tssrc/renderer/src/components/transfer-progress/open-transfer-progress-panel.tssrc/renderer/src/components/transfer-progress/transfer-progress-format.test.tssrc/renderer/src/components/transfer-progress/transfer-progress-format.tssrc/renderer/src/components/transfer-progress/transfer-progress-heading.test.tssrc/renderer/src/components/transfer-progress/transfer-progress-heading.tssrc/renderer/src/components/transfer-progress/transfer-session-state.test.tssrc/renderer/src/components/transfer-progress/transfer-session-state.tssrc/renderer/src/components/transfer-progress/upload-progress-panel.test.tssrc/renderer/src/components/transfer-progress/upload-progress-panel.tssrc/renderer/src/hooks/useGlobalFileDrop.tssrc/renderer/src/i18n/en-runtime-required.jsonsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/runtime/runtime-file-client-download-progress.test.tssrc/renderer/src/runtime/runtime-file-client-external-import.test.tssrc/renderer/src/runtime/runtime-file-client-test-harness.tssrc/renderer/src/runtime/runtime-file-client-types.tssrc/renderer/src/runtime/runtime-file-client.tssrc/renderer/src/runtime/runtime-file-import-client.tssrc/renderer/src/runtime/runtime-file-read-client.tssrc/renderer/src/runtime/runtime-file-upload-client.tssrc/renderer/src/runtime/runtime-upload-progress-tracker.test.tssrc/renderer/src/runtime/runtime-upload-progress-tracker.tssrc/renderer/src/runtime/ssh-upload-progress-client.test.tssrc/renderer/src/runtime/ssh-upload-progress-client.tssrc/renderer/src/web/preload-api/web-filesystem-api.tssrc/shared/remote-download-progress.test.tssrc/shared/remote-download-progress.tssrc/shared/runtime-upload-staging-contract.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Check cancellation after the final chunk is acknowledged. · runtime-upload-file-stream.ts:114-127
src/main/ipc/runtime-upload-file-stream.ts:114-127
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCheck cancellation after the final chunk is acknowledged.
cancelSignalis checked before each slice, but the finalsendChunkuses only the lifetime signal and is followed by no cancellation check. A user cancellation during that send can therefore let the upload resolve. The renderer then callsfiles.commitUploadand reports the file as imported.Suggested fix
if (afterReadStat.mtimeMs !== openedStat.mtimeMs || !isSameFile(afterReadStat, openedStat)) { throw new Error(`File changed during upload: '${displayPath}'`) } + throwIfCancelled(args.cancelSignal) return { byteLength: totalBytes }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: a26a9a1b-d1af-4283-aa68-02632077ee40
📒 Files selected for processing (15)
src/main/ipc/filesystem-download-progress.test.tssrc/main/ipc/filesystem-import-ssh-progress.test.tssrc/main/ipc/filesystem-import-ssh-progress.tssrc/main/ipc/filesystem-mutations.tssrc/main/ipc/filesystem/filesystem-download-handlers.tssrc/main/ipc/runtime-upload-cancel-ipc.test.tssrc/main/ipc/runtime-upload-cancel-ipc.tssrc/main/ipc/runtime-upload-cancellation.test.tssrc/main/ipc/runtime-upload-cancellation.tssrc/main/providers/filesystem-provider-contract.tssrc/main/ssh/sftp-upload.tssrc/renderer/src/i18n/en-runtime-required.jsonsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/runtime/runtime-file-client-download-progress.test.tssrc/renderer/src/runtime/runtime-file-read-client.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/i18n/locales/en.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes — the top commit 60803e5, which extends #16107's runtime upload panel to every remote transfer.
- SSH upload progress and cancel —
fs:importExternalPaths/fs:resolveDroppedPathsForAgenttake per-sourceuploadIds; main measures each source locally, counts bytes off the SFTP/system-SSH upload streams, reports onfs:uploadProgress, and a cancel deletes a cancelled exclusive partial remote file. - Remote download progress and cancel —
fs:downloadFile/fs:downloadFoldertake adownloadId; main tracks bytes fromfastGet, the system-SSHcatstream and a folder size pre-walk, reports onfs:downloadProgress, andfs:cancelDownloadaborts. - One shared panel — the upload panel, session store, heading and byte formatting move to
components/transfer-progress/and take a direction;openTransferProgressPanel,createUploadProgressPanelandrunRemoteDownloadWithProgresswrap each entry point.
ℹ️ Visual proof is still a TODO
The PR body's "Visual proof" section still reads "TODO: attach the upload, cancelled-upload, download and cancelled-download captures." The checklist requires before/after captures for UI changes; attaching them (or marking the row N/A) would let a reviewer see the cancel/outcome states without driving the dev app.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ Follow-up commits reviewed — no new issues found.
Reviewed changes — the two commits pushed since the initial review (60803e5), both closing cancel-edge gaps:
- Window-scoped upload ids —
scopeRuntimeUploadId(senderId, uploadId)keys cancellation, and a newruntime-upload-cancel-ipc.tsvalidates the request and scopes byevent.sender.id, so one window cannot cancel another window's upload. The remembered-cancel set is capped at 1024 with oldest-first eviction. - Only delete a partial this upload created —
FileUploadSession.uploadFilegainsonRemoteCreated, fired from ssh2WriteStream'sopenevent (verified the pinned v1.17 emitsopenon a successful exclusive open) and from aset -C; : > …marker command on the system-SSH path. Delete-on-cancel now requires it, so an EEXIST loser no longer removes a file another client just wrote. - Cancel after the last byte — an SSH import rolls back an
importedresult when the source was cancelled (covers empty folders and a late click);fs:downloadFileand the chunked runtime download both re-check the signal before promotion; the runtime download loop now throws on a zero-byte non-EOF chunk instead of spinning.
No new defects found in these changes. The earlier note about runtime-upload-progress-tracker.ts's cross-file progress guard is unchanged and still open.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the two commits pushed since the prior pullfrog review at 84dfdcd8e3 (eb51f43f91, 6bfce94e2b), which bound the identity reads a stalled relay can accumulate during an SSH import.
- Capped abandoned identity requests per connection —
readIdentitynow skips a new identity read once 64 requests on the same provider are still awaiting a reply, tracked through a module-levelWeakMapkeyed by the provider instance. The provider is cached perconnectionIdand rebuilt on reconnect, so the cap is shared by every import a connection runs and a fresh connection starts clean; a skipped read is unverifiable, so its entry is kept and reported rather than deleted. - Raised the rollback check timeout to 30 s —
ROLLBACK_LSTAT_TIMEOUT_MSgrew from 5 s to 30 s so a congested but healthy relay does not trip therelayStalledbreaker; the first check that times out still keeps every remaining entry without another relay request. - Added a regression test —
filesystem-import-ssh-rollback.test.tsrecords 64 stalled reads across two ledgers sharing one provider, asserts the relay saw exactly 64lstatcalls, and that a third ledger's identity resolvesnullwithout a 65th call. It fails if the limit is per-ledger rather than shared, or if the guard is removed.
Verified locally: pnpm test src/main/ipc/filesystem-import-ssh-rollback.test.ts passes (8/8). The changed files are filesystem-import-ssh-rollback.ts and its test only; requireSshFilesystemProvider confirms the provider instance is stable per connection, so the shared counter does span imports on one connection as intended. All prior pullfrog threads are resolved.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the four commits pushed since the prior pullfrog review at 6bfce94 (fe5852b55b, 05844df5ca, a90c8ae18f, 0932186331) plus the main merge resolution, all pre-merge cleanup.
- Handled a failed upload cancel request —
createUploadProgressPanelnow attaches.catch(() => {})to thefs:cancelRuntimeUploadcall, matching the download panel, so a failed cancel no longer surfaces as an unhandled rejection; the row stays "Cancelling…" for the existing 15 s unconfirmed fallback. - Fixed three inert tests — a new
beforeOpenhook on the fake remote lets the "lost the race" and FIFO cases cancel as the transport opens (they previously cancelled before the import started, so they never reached the race or the FIFO); a new case pins the pre-start cancel; and the zero-total progress case now uses a finished row so only the zero-total guard keeps the percentage null. - Made two test names match what they prove — the failed-cancel case now runs the rejection handler and asserts the row is left untouched, and the FIFO case is renamed to what it covers (rollback leaves the existing FIFO in place).
- Dropped review-iteration leftovers, no behavior change — the shared leftover helper is imported at its use site instead of re-exported from the rollback ledger, the ledger's doc comment moved back onto
SshImportCreatedLedger,REMOTE_TREE_MEASURE_BUDGET_MSis no longer exported, and the Windows note inssh-connectionis corrected.
The one behavior change follows the download panel's existing cancel pattern, and each changed test was verified to reach its target rather than pass vacuously. All prior pullfrog threads remain resolved.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
…gets The upload panel (and the SSH upload spinner) was an app-wide toast, so it followed the user into every workspace and stacked with uploads bound for other workspaces. It now shows only while the destination workspace is active, comes back with current progress on return, and a drop that finishes while the user is elsewhere ends without replaying its outcome. Failures stay visible everywhere and name the workspace when it is not the active one. The drop-handler tests share one SSH repo stub so the file stays under the max-lines limit after the main merge. Refs stablyai#24151
- A late progress event from one file no longer counts toward the next file of the same source: main echoes a per-file sequence and the tracker drops events that do not carry the current one. - The same path dropped twice gets two upload ids, so each copy has its own bar and cancel instead of the second overwriting the first. - Cancelling a source made only of directories stops it before the next directory is created and removes the root it already made. - A finished empty file or folder reads 100% instead of 0%. - A drop where nothing landed reads "Upload failed" unless every row was cancelled, so a cancel no longer hides a failure.
Cancelled sources were filtered out of the failure notice by path, so when the same path was dropped twice and one copy was cancelled, the other copy's failure was hidden too. That notice is the only signal once the panel is hidden in another workspace. The import now marks the cancelled source's own result, and the drop flow filters on that instead of the shared path.
|
Heads-up: #16107 has three new commits that this PR doesn't include yet (head is now
A trial merge of the new head into this branch conflicts in 11 files. Most come from your moving the panel into
Happy to help with the merge if that's useful. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the merge of #16107 plus the three follow-up commits pushed since the prior pullfrog review at 0932186 (3d812b9a4e, 9ff1f09eb2, ae043f637f, a9bec91d66), which bring workspace-scoped drop panels and report what a cancelled or failed drop left on the host.
- Merged #16107 into the branch — conflict resolutions keep this branch's window-scoped upload-cancel registry (
scopeRuntimeUploadId,registerRuntimeUploadCancelHandlers) and its extrakindprogress field; #16107's "cancelled only when every row was cancelled" heading rule andtoRowPercentmoved into the sharedtransfer-progress-heading/transfer-progress-formatmodules. The deadterminal-drop-upload-heading.ts,runtime-upload-session-stateandTerminalDropUploadToastare gone with no dangling references, and the main-retired tests stay deleted. - Workspace-scoped drop panels —
openTransferProgressPaneltakes an optional{ worktreeId }scope and drives the newopenWorktreeScopedToast(src/renderer/src/lib/worktree-scoped-toast.ts), which shows the panel only while that workspace is active, dismisses it on switch-away, and re-issues it under a fresh id on return. Terminal runtime and SSH drops now pass the destination workspace; downloads stay global as before. - Per-row cancels —
createUploadProgressPaneltracks cancels byuploadIdrather than source path and exposesisCancelled, so a path dropped twice can be cancelled once without hiding the other copy's failure. The runtime import re-checksisCancelledbefore each directory create and removes the no-clobber root it made, covering a folder-only source. - Leftover reporting for a closed panel —
failuresToReportkeeps a cancelled source only when its reason names files left on the host, and the drop failure toast names the destination workspace and the first leftover path (with(+N more)), the only notice once a scoped panel closes unseen.
Verified locally: the changed and new renderer plus main tests set pass (terminal-drop-handler, terminal-drop-upload-report, terminal-drop-workspace-label, worktree-scoped-toast, terminal-runtime-drop-upload, transfer-progress-format/heading, open/upload/download panel, runtime-file-client-external-import, runtime-upload-progress-tracker, filesystem-mutations-runtime-upload, runtime-upload-cancel-ipc, filesystem-import-ssh-progress, runtime-upload-progress).
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| if (signal.aborted) { | ||
| // Why: covers a cancel after the last byte and an empty folder, which raise nothing. | ||
| await ledger.rollback() | ||
| return { | ||
| sourcePath, | ||
| status: 'failed', | ||
| reason: describeCancelledImport(ledger.remaining), | ||
| cancelled: true | ||
| } |
There was a problem hiding this comment.
Cancel hides an upload failure
If an SSH upload fails—for example, because the remote disk is full—and the user clicks Cancel while the folder's failure cleanup is still running, this branch replaces the actual error with “Upload cancelled” and marks the result cancelled. The explorer then suppresses its failure toast, so the user does not learn why the upload failed.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the four commits pushed since the prior pullfrog review at a9bec91 (91e12f2e32, 1e50c47787, 03a44db0b7, e197ff1edf), the cancel fixes from review of the #16107 merge.
- Marked a runtime source cancelled only when the cancel stopped it —
wasStoppedByCancelnow also requires the error to read as the cancel reason, so a dropped connection or failed mutation that lands after a Cancel click is reported as a failure instead of being hidden ascancelled. - Checked a directory cancel before and after each runtime create — the shared
stopIfCancellednow runs on both sides offiles.createDirNoClobber, so a cancel before the first directory writes nothing, and one landing mid-request stops the source and runs the existing root cleanup. - Decided a cancel per result, not per dropped path —
ImportItemResultandResolveDroppedPathsResultcarry an optionalcancelled; SSH imports set it in main from the source's cancel handle,fs:resolveDroppedPathsForAgentpasses it through, and the explorer, editor and terminal drops read it. The upload panel drops itscancelledSourcePathsset, so cancelling one copy of a twice-dropped path no longer hides the other copy's failure.
Verified the surrounding code: the runtime cancel reason survives Electron's IPC wrapper (extractIpcErrorMessage), there are no dangling cancelledSourcePaths references, and each new test reaches its target (the wasStoppedByCancel reason check, the before/after directory checks, and the exact cancelled arrays in main and the dropped-paths handler). All prior pullfrog threads remain resolved.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the two commits pushed since the prior pullfrog review at 03a44db (20b936897c, 7debe2415a), which merge main's SSH file-transfer move and restore the rollback hook the move dropped.
- Merged
main's SSH file-transfer extraction —SshConnection.downloadFileandopenFileUploadSessionnow delegate intossh-connection-file-transfers.tsthroughfileTransferHost()and the work ledger; the branch's download progress/cancel observers and the upload cancel signal,onRemoteCreated/onBytesTransferred, and POSIXremoveCreatedEntryare carried across. The two SFTP observer listeners moved out ofsftp-upload.tsintosftp-upload-observers.ts, unchanged. - Restored
removeCreatedEntrythrough the tracked upload session —openTrackedSshUploadSessionnow forwards the optional method inside the session lifetime, so a cancelled SSH import's rollback uses the remote's guarded non-recursive removal instead of keeping folders and deleting files by path; a Windows host still omits the method, preserving its keep-folders fallback. New tests inssh-upload-session-lifetime.test.tspin both halves.
Verified locally: pnpm tc:node is clean, and ssh-upload-session-lifetime, ssh-connection-file-transfer, sftp-upload, filesystem-import-ssh-cancel-invariants, filesystem-import-ssh-progress, and ssh-connection-system-work-drain pass (58 tests). The relay-session upload path reaches this wrapper via ssh-relay-session.ts → rawTransfer.openFileUploadSession, so the restored method is on the live import path.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| invoke('fs:cancelRuntimeUpload', sender, { uploadId: 'b' }) | ||
| sender.emit('render-process-gone') | ||
| expect(isUploadCancelled(scopeRuntimeUploadId(3, 'b'))).toBe(false) |
There was a problem hiding this comment.
Closed-window coverage was removed
The rewritten test covers reloads and renderer crashes, but no longer checks that closing a window clears its remembered upload cancels. If that cleanup path regresses, this IPC suite could pass while stale cancels remain in main. Please retain a closed-window assertion.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the two commits since the prior pullfrog review at 7debe2415a: the merge of #16107's two newest commits (98d6c8efa6, bringing 363dea098b and de80e94010) and the doc commit 5d12792d62 (7 files, +125/-74).
- Adopted #16107's hardened upload-cancel IPC — the add/add conflict in
runtime-upload-cancel-ipc.tsresolves to #16107's version:abortWhenRendererGonewiring, an inlinereadUploadIdcheck replacing the branch's sharedparseTransferId, and no test-onlysender.onceguard. Behavior is equivalent — same validation, same per-sender scoping viascopeRuntimeUploadId(event.sender.id, uploadId)— and production always passes a WebContents.registerRuntimeUploadCancelHandlersstays imported (filesystem-mutations.ts:11) and called (:284). - Cancellation registry and tests aligned with #16107 — the registry, its IPC test and the cancellation test now match #16107 (adding the release-race case);
runtime-upload-progress.tskeeps the branch'skindfield, only a redundant comment dropped. - New cross-renderer cancel test —
filesystem-mutations-runtime-upload.test.tsdrives the realfs:uploadExternalFileToRuntimeand asserts a same-uploadIdcancel from another renderer does not abort the owner's upload while the owner's does; it holds only because registration and cancellation share the sender scope. - Doc-only —
transfer-id.tsscopesparseTransferIdto download ids; its sole caller isremote-download-sessions.ts.
The remaining conflicts resolved back to the branch's newer work: ssh-connection.ts keeps its sftp()/sftpUntracked work-ledger split, terminal-native-file-drop.ts keeps createUploadProgressPanel, terminal-drop-upload-report.ts keeps the leftover-note join, and en-runtime-required.json drops #16107's obsolete drop.handler.upload.heading keys (no remaining references) while keeping transferProgress.*.
Verified: pnpm test on the four touched suites (38 tests pass) and pnpm run check:code-quality:changed is clean. No open Pullfrog threads to retire.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| if (onTotalBytes) { | ||
| // Why: a timed-out walk left big trees, the ones that need a bar, with none; walking | ||
| // alongside the download costs no wait and supplies the total whenever it finishes. | ||
| void measureRemoteTree(sftp, sourcePath, options) |
There was a problem hiding this comment.
There was a problem hiding this comment.
ℹ️ No critical issues — one stale contract comment to update.
Reviewed changes — the single commit pushed since the prior pullfrog review at 5d12792d62 (60d329f9cd), which gives a folder download a moving bar.
- Folder size walk now runs alongside the download —
downloadFolderViaSftpstartsmeasureRemoteTreewithout awaiting it and reports the total when the walk finishes, dropping a total that arrives after the download settled; the 2 s budget and itsnullreturn are gone, and a failed walk or a throwing observer is swallowed so it never fails the download. - Indeterminate row bar while the total is unknown —
TransferProgressPanelrenders an animated,aria-valuenow-lessrole="progressbar"for an active row with no total, keeping the determinate<Progress>once the total is known and for ended rows. - Tests — the SFTP progress suite replaces the budget case with four cases (total lands mid-transfer, observer throws, slow walk does not block and its late total is dropped, walk failure still downloads), and a new
TransferProgressPanel.test.tsxcovers the animated/determinate/ended bar states.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the two commits pushed since the prior pullfrog review at 60d329f9cd (3689d8d21f, c20a5f85dc), which keep a failed SSH upload's real error and correct a stale contract comment.
- Kept a failed SSH upload's real error —
importOneSourceSshnow reports the failure toimportSshSourceWithProgressthrough anonFailurecallback that runs before any cleanup, and the cancel branch only relabels the result as cancelled when the failure did not precede the click; otherwise the real reason is returned (with any rollback leftovers appended). - Corrected the
onTotalBytescontract — the shared observer JSDoc now says a folder's total may arrive mid-transfer and is never reported after the transfer ends, matching the alongside-the-download walk.
The new tests are non-vacuous: the progress suite cancels from a beforeRemove hook so the click lands inside cleanup, and the rollback suite pins both directions (failure-then-cancel keeps the real reason; cancel-caused failure still reports as cancelled). The prior review's inline nit is addressed by c20a5f85dc, and its thread is resolved.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the single merge pushed since the prior pullfrog review at c20a5f85dc (2595868489), which merges main after #24559 reverted #24401.
- Restored the branch's transfers inline — main's revert deleted
ssh-connection-file-transfers.tsandssh-upload-session-lifetime.ts, and git auto-mergeddownloadFile/openFileUploadSessionto main's plain bodies, silently dropping this branch's download progress/cancel, upload cancel signal and callbacks, andremoveCreatedEntry. The merge restores both methods inssh-connection.tsto this branch's pre-#24401 form, keeping main's unrelated changes. - Verified faithful — the restored methods (
uploadDirectory,downloadFile,openFileUploadSession,writeFile,writeBuffer) match the previously approved03a44db0b7state;pnpm tc:nodeis clean, no references to the deleted modules or work ledger remain, and the SSH transfer suites pass (8 tests).
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the main merge 88f930fe74 (main at e2da3a1eb1), the only commit since the prior pullfrog review at 2595868489.
- Merged main's
LocalFileAccessfile-access feature — main's #24489 replacedfilesystem-authwithlocal-file-access-resolutionand addedaccess?: LocalFileAccesstofs:readFile/rename/stat/pathExists/importExternalPaths. All five conflicts (filesystem-mutations.tsargs,filesystem-api.ts/fs-bridge.tsfield + import,runtime-upload-file-stream.ts's removedauthorizeExternalPath,runtime-file-import-client.tsoptions) were resolved by keeping both sides. - Kept the branch's import path intact —
runtime-file-import-clientretains itsprogressoption andrunSshUploadWithProgress, adding main'saccessand...localAccess(...)to the local import call only, so remote destinations stay root-relative. - Extracted the runtime upload tracking —
trackRuntimeUploadForSendermoved from thefs:uploadExternalFileToRuntimehandler intoruntime-upload-cancel-ipc.ts, behavior-identical to the prior inline wiring (null when nouploadId, same throttle/emit, same release).
Verified: pnpm tc:node and pnpm tc:web are clean; filesystem-mutations-runtime-upload, runtime-upload-cancel-ipc, runtime-upload-cancellation, and runtime-file-client-external-import pass (34 tests); pnpm run check:code-quality:changed reports 0 findings. git show --remerge-diff 88f930fe74 confirms every conflict kept both sides and dropped no branch behavior.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
Thanks for this, @Chihen-Tai. Quick update: the legacy SSH relay has been removed (#27214, #27215), and SSH workspaces now run on Orca's managed server on the host, so their uploads and downloads go through the same remote-runtime transfer path as #16107 rather than the old desktop-side SFTP import. The idea here (one progress panel with cancel for remote transfers) still applies, but the SSH-specific half would need to be reworked onto that runtime path when this is rebased. |
Resolved against main's owner-routed runtime target (settings -> target), the shared streamExternalFileSlices streamer, and the terminal drop split. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d downloads SSH workspaces now run on the host's managed runtime, so their transfers use the same runtime path as remote Orca servers. This builds on stablyai#16107's runtime upload progress instead of the removed desktop-side SFTP import: - stablyai#16107's terminal-drop panel moves to components/transfer-progress and takes a direction; file explorer and editor drops use it too. - Runtime downloads (explorer Download, terminal "download & open") count the chunks the renderer already pulls, show the panel, and cancel between chunks through an AbortSignal; the temp file is discarded and nothing is committed. - SSH workspace files download through the host runtime instead of the SSH filesystem provider, which nothing registers anymore. Folder downloads keep their existing SSH-only path. - Runtime import cancel fixes: directories check the cancel before and after each create, and a source is marked cancelled only when the cancel is what stopped it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
88106e8 to
43c7743
Compare

ELI5
When you copy files to or from a remote workspace (an SSH host or a remote Orca server), Orca shows one progress panel: a row per item with bytes, a bar and a Cancel button. Before, only files dropped onto a remote terminal got a panel; explorer and editor drops and all downloads showed nothing until they finished.
What Changed
Before (main + #16107):
After:
Mechanism:
components/transfer-progress/and take a direction (upload/download).createUploadProgressPanelwraps feat(runtime): per-file progress and cancel for files dropped onto a remote workspace #16107'sRuntimeImportProgressHandlers;runRemoteDownloadWithProgresswraps a download. Rows end from the transfer's result, not the click: "Cancelling…" until the result arrives, "remote state unconfirmed" after 15 s.progresstoimportExternalPathsToRuntime, which already reports bytes over feat(runtime): per-file progress and cancel for files dropped onto a remote workspace #16107'sfs:uploadProgressand cancels throughfs:cancelRuntimeUpload. Local workspaces are untouched (their copy has no progress handlers).downloadRuntimeFilealready pulls the file chunk by chunk in the renderer (files.readChunk). It now takes an optional transfer: it reads the size withfiles.stat(failure only means "bytes, no percentage"), counts each appended chunk (throttled to one update per 100 ms), and checks anAbortSignalbefore every chunk and before the final commit. Older servers that answer in one preview reply still show the panel and can be cancelled before the save. No main-process or wire change.pickRemoteDownloadTarget); terminal "download & open" always callsdownloadRuntimeFile, which still falls back to the SSH connection when the context has no runtime owner. Folder downloads keep main's existing SSH-only path.cancelledonly when the cancel is what stopped it, so a failure after the click is still reported.Why
Linked Issue
Fixes #16077 (together with #16107)
Visual Proof
Captured from the dev app (this branch) against an Ubuntu 24.04 SSH host served by its managed runtime (orcad), driven through CDP. Crops show only the panel.
Testing
Automated (ran locally on this branch):
pnpm tc: pass.check:code-quality:changed, oxlint on changed files,check:max-lines-ratchet,check:dead-classes,check:owner-routing-ratchet,verify:localization-catalogs|extraction|coverage: pass.transfer-progress/,runtime/runtime-file-client-*,right-sidebar/FileExplorerRow.actions,terminal-pane/terminal-drop-*,terminal-runtime-drop-upload,terminal-pane-element-file-drop,main/ipc/runtime-upload-*,filesystem-mutations-runtime-upload): pass.src/main/ipc,src/renderer/src/{runtime,lib,components/terminal-pane,components/editor,...}) has 29 failures in files this PR does not touch; the same files fail on cleanmainon my machine (node:sqliteunavailable, aBrowserWindowmock, timing tests).Manual, macOS → Ubuntu 24.04 SSH host on the managed runtime (dev app, driven through CDP):
.orca-upload-*temp is removed on the host; the folder finishes and both files' sha256 match.orca/drops/path is pasted; sha256 matchesNot run by hand: editor drop and terminal "download & open" (same upload/download functions as above, covered by unit tests). The dev app needed
out/orcad-templatefrom a release build to connect at all; without it every host reports "isn't supported" (main's dev setup, unrelated to this PR).AI Disclosure
Implemented and tested with Claude Code (Claude Opus 5.5).
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)🤖 Generated with Claude Code