feat(ssh): download folders from remote explorer#7793
Conversation
📝 WalkthroughWalkthroughAdds folder download support for remote SSH directories. The main process now handles 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/ipc/filesystem.test.ts (1)
724-858: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the overwrite-refusal and file-as-folder guards.
The PR emphasizes refusing to overwrite an existing destination and rejecting a file passed as a folder, but neither path is exercised. All four new cases feed
statMockan ENOENT (destination absent) and adirectoryremote stat. Consider adding cases where localstatresolves (destination exists → expect throw, nodownloadFile/rename) and whereprovider.statreturns a non-directory type (expect the "Cannot download a file as a folder" rejection).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b690f2b9-885f-4b1a-806a-fd0ed5751db0
📒 Files selected for processing (13)
src/main/ipc/filesystem.test.tssrc/main/ipc/filesystem.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/right-sidebar/FileExplorer.test.tsxsrc/renderer/src/components/right-sidebar/FileExplorerRow.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/web/web-preload-api.test.tssrc/renderer/src/web/web-preload-api.ts
d9e0436 to
c4a5234
Compare
|
Addressed CodeRabbit's test coverage note by adding guard tests for existing local destination folders and remote file-as-folder rejection. Re-ran focused filesystem tests: |
# Conflicts: # src/main/ipc/filesystem.ts
AmethystLiang
left a comment
AmethystLiang
left a comment
There was a problem hiding this comment.
Thanks for the contribution — reviewed and validated end to end.
- Remote folders download recursively with structure and hashes preserved.
- Hardened cancellation, SFTP lifecycle, safe no-clobber publication, filename/file-type handling, and paired-web/localization parity.
- Verified against a real Linux SSH/SFTP target, including successful download and existing-destination rejection.
Approving.
Restoring headless mobile tabs looks up the repo for the active worktree id; the mock lacked getRepo, so the test only passed incidentally. Add it explicitly and return undefined since wt-1 is a worktree id, not a registered repo.
- Folder downloads require SFTP, unavailable on system SSH (which offers only raw file operations). Add supportsFolderDownload flag to gate the feature in the UI layer. - Reject symlinks at directory-entry level, preventing tree escapes and eliminating unnecessary stat calls. - Check abort signal before opening dialog for better responsiveness when renderer closes. - Log cleanup errors without re-throwing to preserve underlying transfer failures.
Enforce fail-closed gating and add Windows path traversal validation to ensure downloads are only available when explicitly supported and safe.
Enforce fail-closed gating and add Windows path traversal validation to ensure downloads are only available when explicitly supported and safe.
Move FolderDownloadOptions next to the SFTP download implementation and narrow IFilesystemProvider.downloadFolder options to AbortSignal only so types.ts stays within the 300-line oxlint budget when merged with main.
3 participants
Summary
Adds SSH remote folder download support from the File Explorer context menu.
Fixes #7780.
Screenshots
Screen recording of downloading a folder over SSH:
Download.Folder.over.SSH.mp4
Testing
pnpm lint— fails on current base with unrelatedswitch-exhaustiveness-checkinsrc/renderer/src/components/right-sidebar/source-control-manual-review-url.ts:164:11(Cases not matched: null)pnpm typecheckpnpm testpnpm buildAdditional focused checks run:
pnpm vitest run --config config/vitest.config.ts src/main/ipc/filesystem.test.ts src/renderer/src/components/right-sidebar/FileExplorer.test.tsx src/renderer/src/web/web-preload-api.test.tspnpm exec oxlint src/main/ipc/filesystem.ts src/main/ipc/filesystem.test.ts src/preload/api-types.ts src/preload/index.ts src/renderer/src/web/web-preload-api.ts src/renderer/src/web/web-preload-api.test.ts src/renderer/src/components/right-sidebar/FileExplorerRow.tsx src/renderer/src/components/right-sidebar/FileExplorer.test.tsxpnpm run verify:localization-catalogpnpm run verify:localization-coverageAI Review Report
AI-assisted review checked:
path.join; no shortcut or shell behavior changes.stat/transfer work, avoiding click-to-dialog latency on remote connections.The review flagged the original pre-dialog SSH
statlatency; the implementation was updated so native folder selection happens first.Security Audit
Reviewed IPC and path-handling risks:
Notes
v24.18.0viafnm, matching the repo engine. Previous Node 26 local test failures were environment-related and do not reproduce on Node 24.